| Message ID | 20170708025030.15845-3-rth@twiddle.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 8 July 2017 at 03:50, Richard Henderson <rth@twiddle.net> wrote: > We had a check using TARGET_VIRT_ADDR_SPACE_BITS to make sure > that the allocation coming in from the command-line option was > not too large, but that didn't include target-specific knowledge > about other restrictions on user-space. > > Remove several target-specific hacks in linux-user/main.c. > > For MIPS and Nios, we can replace them with proper adjustments > to the respective target's TARGET_VIRT_ADDR_SPACE_BITS definition. > > For ARM, we had no existing ifdef but I suspect that the current > default value of 0xf7000000 was chosen with this in mind. Define > a workable value in linux-user/arm/, and also document why the > special case is required. > > Signed-off-by: Richard Henderson <rth@twiddle.net> > --- > linux-user/arm/target_cpu.h | 4 ++++ > target/mips/mips-defs.h | 6 +++++- > target/nios2/cpu.h | 6 +++++- > linux-user/main.c | 38 +++++++++++++++++++++++++------------- > 4 files changed, 39 insertions(+), 15 deletions(-) > > diff --git a/linux-user/arm/target_cpu.h b/linux-user/arm/target_cpu.h > index d888219..c4f79eb 100644 > --- a/linux-user/arm/target_cpu.h > +++ b/linux-user/arm/target_cpu.h > @@ -19,6 +19,10 @@ > #ifndef ARM_TARGET_CPU_H > #define ARM_TARGET_CPU_H > > +/* We need to be able to map the commpage. > + See validate_guest_space in linux-user/elfload.c. */ > +#define MAX_RESERVED_VA 0xfff00000ul > + This should be 0xffff0000, but you'll need the bugfix patch I just sent out first. (Why "UL" ? That's usually a wrong choice compared to either U or ULL.) Otherwise Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
On 10/03/2017 12:24 PM, Peter Maydell wrote: > On 8 July 2017 at 03:50, Richard Henderson <rth@twiddle.net> wrote: >> We had a check using TARGET_VIRT_ADDR_SPACE_BITS to make sure >> that the allocation coming in from the command-line option was >> not too large, but that didn't include target-specific knowledge >> about other restrictions on user-space. >> >> Remove several target-specific hacks in linux-user/main.c. >> >> For MIPS and Nios, we can replace them with proper adjustments >> to the respective target's TARGET_VIRT_ADDR_SPACE_BITS definition. >> >> For ARM, we had no existing ifdef but I suspect that the current >> default value of 0xf7000000 was chosen with this in mind. Define >> a workable value in linux-user/arm/, and also document why the >> special case is required. >> >> Signed-off-by: Richard Henderson <rth@twiddle.net> >> --- >> linux-user/arm/target_cpu.h | 4 ++++ >> target/mips/mips-defs.h | 6 +++++- >> target/nios2/cpu.h | 6 +++++- >> linux-user/main.c | 38 +++++++++++++++++++++++++------------- >> 4 files changed, 39 insertions(+), 15 deletions(-) >> >> diff --git a/linux-user/arm/target_cpu.h b/linux-user/arm/target_cpu.h >> index d888219..c4f79eb 100644 >> --- a/linux-user/arm/target_cpu.h >> +++ b/linux-user/arm/target_cpu.h >> @@ -19,6 +19,10 @@ >> #ifndef ARM_TARGET_CPU_H >> #define ARM_TARGET_CPU_H >> >> +/* We need to be able to map the commpage. >> + See validate_guest_space in linux-user/elfload.c. */ >> +#define MAX_RESERVED_VA 0xfff00000ul >> + > > This should be 0xffff0000, but you'll need the bugfix patch I just sent > out first. > > (Why "UL" ? That's usually a wrong choice compared to either U or ULL.) Because that matches the type of +unsigned long reserved_va = MAX_RESERVED_VA; Which, arguably, should be uintptr_t or size_t something instead, but that would certainly be for a different patch. If you prefer, since this is a 32-bit value, I could trim the define to U and still be correct. r~
diff --git a/linux-user/arm/target_cpu.h b/linux-user/arm/target_cpu.h index d888219..c4f79eb 100644 --- a/linux-user/arm/target_cpu.h +++ b/linux-user/arm/target_cpu.h @@ -19,6 +19,10 @@ #ifndef ARM_TARGET_CPU_H #define ARM_TARGET_CPU_H +/* We need to be able to map the commpage. + See validate_guest_space in linux-user/elfload.c. */ +#define MAX_RESERVED_VA 0xfff00000ul + static inline void cpu_clone_regs(CPUARMState *env, target_ulong newsp) { if (newsp) { diff --git a/target/mips/mips-defs.h b/target/mips/mips-defs.h index 047554e..d239069 100644 --- a/target/mips/mips-defs.h +++ b/target/mips/mips-defs.h @@ -15,7 +15,11 @@ #else #define TARGET_LONG_BITS 32 #define TARGET_PHYS_ADDR_SPACE_BITS 40 -#define TARGET_VIRT_ADDR_SPACE_BITS 32 +# ifdef CONFIG_USER_ONLY +# define TARGET_VIRT_ADDR_SPACE_BITS 31 +# else +# define TARGET_VIRT_ADDR_SPACE_BITS 32 +#endif #endif /* Masks used to mark instructions to indicate which ISA level they diff --git a/target/nios2/cpu.h b/target/nios2/cpu.h index 13931f3..da3f637 100644 --- a/target/nios2/cpu.h +++ b/target/nios2/cpu.h @@ -227,7 +227,11 @@ qemu_irq *nios2_cpu_pic_init(Nios2CPU *cpu); void nios2_check_interrupts(CPUNios2State *env); #define TARGET_PHYS_ADDR_SPACE_BITS 32 -#define TARGET_VIRT_ADDR_SPACE_BITS 32 +#ifdef CONFIG_USER_ONLY +# define TARGET_VIRT_ADDR_SPACE_BITS 31 +#else +# define TARGET_VIRT_ADDR_SPACE_BITS 32 +#endif #define cpu_init(cpu_model) CPU(cpu_nios2_init(cpu_model)) diff --git a/linux-user/main.c b/linux-user/main.c index ad03c9e..e000533 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -60,23 +60,38 @@ do { \ } \ } while (0) -#if (TARGET_LONG_BITS == 32) && (HOST_LONG_BITS == 64) /* * When running 32-on-64 we should make sure we can fit all of the possible * guest address space into a contiguous chunk of virtual host memory. * * This way we will never overlap with our own libraries or binaries or stack * or anything else that QEMU maps. + * + * Many cpus reserve the high bit (or more than one for some 64-bit cpus) + * of the address for the kernel. Some cpus rely on this and user space + * uses the high bit(s) for pointer tagging and the like. For them, we + * must preserve the expected address space. */ -# if defined(TARGET_MIPS) || defined(TARGET_NIOS2) -/* - * MIPS only supports 31 bits of virtual address space for user space. - * Nios2 also only supports 31 bits. - */ -unsigned long reserved_va = 0x77000000; +#ifndef MAX_RESERVED_VA +# if HOST_LONG_BITS > TARGET_VIRT_ADDR_SPACE_BITS +# if TARGET_VIRT_ADDR_SPACE_BITS == 32 && \ + (TARGET_LONG_BITS == 32 || defined(TARGET_ABI32)) +/* There are a number of places where we assign reserved_va to a variable + of type abi_ulong and expect it to fit. Avoid the last page. */ +# define MAX_RESERVED_VA (0xfffffffful & TARGET_PAGE_MASK) +# else +# define MAX_RESERVED_VA (1ul << TARGET_VIRT_ADDR_SPACE_BITS) +# endif # else -unsigned long reserved_va = 0xf7000000; +# define MAX_RESERVED_VA 0 # endif +#endif + +/* That said, reserving *too* much vm space via mmap can run into problems + with rlimits, oom due to page table creation, etc. We will still try it, + if directed by the command-line option, but not by default. */ +#if HOST_LONG_BITS == 64 && TARGET_VIRT_ADDR_SPACE_BITS <= 32 +unsigned long reserved_va = MAX_RESERVED_VA; #else unsigned long reserved_va; #endif @@ -3975,11 +3990,8 @@ static void handle_arg_reserved_va(const char *arg) unsigned long unshifted = reserved_va; p++; reserved_va <<= shift; - if (((reserved_va >> shift) != unshifted) -#if HOST_LONG_BITS > TARGET_VIRT_ADDR_SPACE_BITS - || (reserved_va > (1ul << TARGET_VIRT_ADDR_SPACE_BITS)) -#endif - ) { + if (reserved_va >> shift != unshifted + || (MAX_RESERVED_VA && reserved_va > MAX_RESERVED_VA)) { fprintf(stderr, "Reserved virtual address too big\n"); exit(EXIT_FAILURE); }
We had a check using TARGET_VIRT_ADDR_SPACE_BITS to make sure that the allocation coming in from the command-line option was not too large, but that didn't include target-specific knowledge about other restrictions on user-space. Remove several target-specific hacks in linux-user/main.c. For MIPS and Nios, we can replace them with proper adjustments to the respective target's TARGET_VIRT_ADDR_SPACE_BITS definition. For ARM, we had no existing ifdef but I suspect that the current default value of 0xf7000000 was chosen with this in mind. Define a workable value in linux-user/arm/, and also document why the special case is required. Signed-off-by: Richard Henderson <rth@twiddle.net> --- linux-user/arm/target_cpu.h | 4 ++++ target/mips/mips-defs.h | 6 +++++- target/nios2/cpu.h | 6 +++++- linux-user/main.c | 38 +++++++++++++++++++++++++------------- 4 files changed, 39 insertions(+), 15 deletions(-)