| Message ID | 20181114133920.7134-3-steve.capper@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | 52-bit userspace VAs | expand |
Hi Steve, On Wed, Nov 14, 2018 at 01:39:17PM +0000, Steve Capper wrote: > diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h > index 3e2091708b8e..da41a2655b69 100644 > --- a/arch/arm64/include/asm/processor.h > +++ b/arch/arm64/include/asm/processor.h > @@ -25,6 +25,9 @@ > #define USER_DS (TASK_SIZE_64 - 1) > > #ifndef __ASSEMBLY__ > + > +#define DEFAULT_MAP_WINDOW_64 (UL(1) << VA_BITS) > + > #ifdef __KERNEL__ That's a strange place to place DEFAULT_MAP_WINDOW_64. Did you have any #include dependency issues? If yes, we could look at cleaning them up, maybe moving these definitions into a separate file. (also, if you do a clean-up I don't think we need __KERNEL__ anymore) > > #include <linux/build_bug.h> > @@ -51,13 +54,16 @@ > TASK_SIZE_32 : TASK_SIZE_64) > #define TASK_SIZE_OF(tsk) (test_tsk_thread_flag(tsk, TIF_32BIT) ? \ > TASK_SIZE_32 : TASK_SIZE_64) > +#define DEFAULT_MAP_WINDOW (test_thread_flag(TIF_32BIT) ? \ > + TASK_SIZE_32 : DEFAULT_MAP_WINDOW_64) > #else > #define TASK_SIZE TASK_SIZE_64 > +#define DEFAULT_MAP_WINDOW DEFAULT_MAP_WINDOW_64 > #endif /* CONFIG_COMPAT */ > > -#define TASK_UNMAPPED_BASE (PAGE_ALIGN(TASK_SIZE / 4)) > +#define TASK_UNMAPPED_BASE (PAGE_ALIGN(DEFAULT_MAP_WINDOW / 4)) > +#define STACK_TOP_MAX DEFAULT_MAP_WINDOW_64 > > -#define STACK_TOP_MAX TASK_SIZE_64 > #ifdef CONFIG_COMPAT > #define AARCH32_VECTORS_BASE 0xffff0000 > #define STACK_TOP (test_thread_flag(TIF_32BIT) ? \ > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c > index 9d9582cac6c4..e5a1dc0beef9 100644 > --- a/arch/arm64/mm/init.c > +++ b/arch/arm64/mm/init.c > @@ -609,7 +609,7 @@ void __init mem_init(void) > * detected at build time already. > */ > #ifdef CONFIG_COMPAT > - BUILD_BUG_ON(TASK_SIZE_32 > TASK_SIZE_64); > + BUILD_BUG_ON(TASK_SIZE_32 > DEFAULT_MAP_WINDOW_64); > #endif Since you are at this, can you please remove the useless white space (I guess it was there before when we had more BUILD_BUG_ONs). > diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c > index 30ac0c975f8a..d1ec7136e3e1 100644 > --- a/drivers/firmware/efi/libstub/arm-stub.c > +++ b/drivers/firmware/efi/libstub/arm-stub.c > @@ -33,7 +33,7 @@ > #define EFI_RT_VIRTUAL_SIZE SZ_512M > > #ifdef CONFIG_ARM64 > -# define EFI_RT_VIRTUAL_LIMIT TASK_SIZE_64 > +# define EFI_RT_VIRTUAL_LIMIT DEFAULT_MAP_WINDOW_64 > #else > # define EFI_RT_VIRTUAL_LIMIT TASK_SIZE > #endif Just curious, would anything happen if we leave this to TASK_SIZE_64?
On Tue, 27 Nov 2018 at 18:09, Catalin Marinas <catalin.marinas@arm.com> wrote: > > Hi Steve, > > On Wed, Nov 14, 2018 at 01:39:17PM +0000, Steve Capper wrote: > > diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h > > index 3e2091708b8e..da41a2655b69 100644 > > --- a/arch/arm64/include/asm/processor.h > > +++ b/arch/arm64/include/asm/processor.h > > @@ -25,6 +25,9 @@ > > #define USER_DS (TASK_SIZE_64 - 1) > > > > #ifndef __ASSEMBLY__ > > + > > +#define DEFAULT_MAP_WINDOW_64 (UL(1) << VA_BITS) > > + > > #ifdef __KERNEL__ > > That's a strange place to place DEFAULT_MAP_WINDOW_64. Did you have any > #include dependency issues? If yes, we could look at cleaning them up, > maybe moving these definitions into a separate file. > > (also, if you do a clean-up I don't think we need __KERNEL__ anymore) > > > > > #include <linux/build_bug.h> > > @@ -51,13 +54,16 @@ > > TASK_SIZE_32 : TASK_SIZE_64) > > #define TASK_SIZE_OF(tsk) (test_tsk_thread_flag(tsk, TIF_32BIT) ? \ > > TASK_SIZE_32 : TASK_SIZE_64) > > +#define DEFAULT_MAP_WINDOW (test_thread_flag(TIF_32BIT) ? \ > > + TASK_SIZE_32 : DEFAULT_MAP_WINDOW_64) > > #else > > #define TASK_SIZE TASK_SIZE_64 > > +#define DEFAULT_MAP_WINDOW DEFAULT_MAP_WINDOW_64 > > #endif /* CONFIG_COMPAT */ > > > > -#define TASK_UNMAPPED_BASE (PAGE_ALIGN(TASK_SIZE / 4)) > > +#define TASK_UNMAPPED_BASE (PAGE_ALIGN(DEFAULT_MAP_WINDOW / 4)) > > +#define STACK_TOP_MAX DEFAULT_MAP_WINDOW_64 > > > > -#define STACK_TOP_MAX TASK_SIZE_64 > > #ifdef CONFIG_COMPAT > > #define AARCH32_VECTORS_BASE 0xffff0000 > > #define STACK_TOP (test_thread_flag(TIF_32BIT) ? \ > > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c > > index 9d9582cac6c4..e5a1dc0beef9 100644 > > --- a/arch/arm64/mm/init.c > > +++ b/arch/arm64/mm/init.c > > @@ -609,7 +609,7 @@ void __init mem_init(void) > > * detected at build time already. > > */ > > #ifdef CONFIG_COMPAT > > - BUILD_BUG_ON(TASK_SIZE_32 > TASK_SIZE_64); > > + BUILD_BUG_ON(TASK_SIZE_32 > DEFAULT_MAP_WINDOW_64); > > #endif > > Since you are at this, can you please remove the useless white space (I > guess it was there before when we had more BUILD_BUG_ONs). > > > diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c > > index 30ac0c975f8a..d1ec7136e3e1 100644 > > --- a/drivers/firmware/efi/libstub/arm-stub.c > > +++ b/drivers/firmware/efi/libstub/arm-stub.c > > @@ -33,7 +33,7 @@ > > #define EFI_RT_VIRTUAL_SIZE SZ_512M > > > > #ifdef CONFIG_ARM64 > > -# define EFI_RT_VIRTUAL_LIMIT TASK_SIZE_64 > > +# define EFI_RT_VIRTUAL_LIMIT DEFAULT_MAP_WINDOW_64 > > #else > > # define EFI_RT_VIRTUAL_LIMIT TASK_SIZE > > #endif > > Just curious, would anything happen if we leave this to TASK_SIZE_64? > Not really. The kernel virtual mapping of the EFI runtime services regions are randomized based on the this value, so they may end up way up in memory, but EFI doesn't really care about that.
On Tue, Nov 27, 2018 at 05:09:32PM +0000, Catalin Marinas wrote: > Hi Steve, Hi Catalin, > > On Wed, Nov 14, 2018 at 01:39:17PM +0000, Steve Capper wrote: > > diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h > > index 3e2091708b8e..da41a2655b69 100644 > > --- a/arch/arm64/include/asm/processor.h > > +++ b/arch/arm64/include/asm/processor.h > > @@ -25,6 +25,9 @@ > > #define USER_DS (TASK_SIZE_64 - 1) > > > > #ifndef __ASSEMBLY__ > > + > > +#define DEFAULT_MAP_WINDOW_64 (UL(1) << VA_BITS) > > + > > #ifdef __KERNEL__ > > That's a strange place to place DEFAULT_MAP_WINDOW_64. Did you have any > #include dependency issues? If yes, we could look at cleaning them up, > maybe moving these definitions into a separate file. > > (also, if you do a clean-up I don't think we need __KERNEL__ anymore) > Okay, I will investigate cleaning this up. > > > > #include <linux/build_bug.h> > > @@ -51,13 +54,16 @@ > > TASK_SIZE_32 : TASK_SIZE_64) > > #define TASK_SIZE_OF(tsk) (test_tsk_thread_flag(tsk, TIF_32BIT) ? \ > > TASK_SIZE_32 : TASK_SIZE_64) > > +#define DEFAULT_MAP_WINDOW (test_thread_flag(TIF_32BIT) ? \ > > + TASK_SIZE_32 : DEFAULT_MAP_WINDOW_64) > > #else > > #define TASK_SIZE TASK_SIZE_64 > > +#define DEFAULT_MAP_WINDOW DEFAULT_MAP_WINDOW_64 > > #endif /* CONFIG_COMPAT */ > > > > -#define TASK_UNMAPPED_BASE (PAGE_ALIGN(TASK_SIZE / 4)) > > +#define TASK_UNMAPPED_BASE (PAGE_ALIGN(DEFAULT_MAP_WINDOW / 4)) > > +#define STACK_TOP_MAX DEFAULT_MAP_WINDOW_64 > > > > -#define STACK_TOP_MAX TASK_SIZE_64 > > #ifdef CONFIG_COMPAT > > #define AARCH32_VECTORS_BASE 0xffff0000 > > #define STACK_TOP (test_thread_flag(TIF_32BIT) ? \ > > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c > > index 9d9582cac6c4..e5a1dc0beef9 100644 > > --- a/arch/arm64/mm/init.c > > +++ b/arch/arm64/mm/init.c > > @@ -609,7 +609,7 @@ void __init mem_init(void) > > * detected at build time already. > > */ > > #ifdef CONFIG_COMPAT > > - BUILD_BUG_ON(TASK_SIZE_32 > TASK_SIZE_64); > > + BUILD_BUG_ON(TASK_SIZE_32 > DEFAULT_MAP_WINDOW_64); > > #endif > > Since you are at this, can you please remove the useless white space (I > guess it was there before when we had more BUILD_BUG_ONs). > Sure thing. > > diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c > > index 30ac0c975f8a..d1ec7136e3e1 100644 > > --- a/drivers/firmware/efi/libstub/arm-stub.c > > +++ b/drivers/firmware/efi/libstub/arm-stub.c > > @@ -33,7 +33,7 @@ > > #define EFI_RT_VIRTUAL_SIZE SZ_512M > > > > #ifdef CONFIG_ARM64 > > -# define EFI_RT_VIRTUAL_LIMIT TASK_SIZE_64 > > +# define EFI_RT_VIRTUAL_LIMIT DEFAULT_MAP_WINDOW_64 > > #else > > # define EFI_RT_VIRTUAL_LIMIT TASK_SIZE > > #endif > > Just curious, would anything happen if we leave this to TASK_SIZE_64? > Then it doesn't compile :-). TASK_SIZE_64 is a variable that is outside the EFI stub's knowledge (and indeed is initialised after the stub has already executed). Cheers,
diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h index 433b9554c6a1..bc9bd9e77d9d 100644 --- a/arch/arm64/include/asm/elf.h +++ b/arch/arm64/include/asm/elf.h @@ -117,7 +117,7 @@ * 64-bit, this is above 4GB to leave the entire 32-bit address * space open for things that want to use the area for 32-bit pointers. */ -#define ELF_ET_DYN_BASE (2 * TASK_SIZE_64 / 3) +#define ELF_ET_DYN_BASE (2 * DEFAULT_MAP_WINDOW_64 / 3) #ifndef __ASSEMBLY__ diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h index 3e2091708b8e..da41a2655b69 100644 --- a/arch/arm64/include/asm/processor.h +++ b/arch/arm64/include/asm/processor.h @@ -25,6 +25,9 @@ #define USER_DS (TASK_SIZE_64 - 1) #ifndef __ASSEMBLY__ + +#define DEFAULT_MAP_WINDOW_64 (UL(1) << VA_BITS) + #ifdef __KERNEL__ #include <linux/build_bug.h> @@ -51,13 +54,16 @@ TASK_SIZE_32 : TASK_SIZE_64) #define TASK_SIZE_OF(tsk) (test_tsk_thread_flag(tsk, TIF_32BIT) ? \ TASK_SIZE_32 : TASK_SIZE_64) +#define DEFAULT_MAP_WINDOW (test_thread_flag(TIF_32BIT) ? \ + TASK_SIZE_32 : DEFAULT_MAP_WINDOW_64) #else #define TASK_SIZE TASK_SIZE_64 +#define DEFAULT_MAP_WINDOW DEFAULT_MAP_WINDOW_64 #endif /* CONFIG_COMPAT */ -#define TASK_UNMAPPED_BASE (PAGE_ALIGN(TASK_SIZE / 4)) +#define TASK_UNMAPPED_BASE (PAGE_ALIGN(DEFAULT_MAP_WINDOW / 4)) +#define STACK_TOP_MAX DEFAULT_MAP_WINDOW_64 -#define STACK_TOP_MAX TASK_SIZE_64 #ifdef CONFIG_COMPAT #define AARCH32_VECTORS_BASE 0xffff0000 #define STACK_TOP (test_thread_flag(TIF_32BIT) ? \ diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c index 9d9582cac6c4..e5a1dc0beef9 100644 --- a/arch/arm64/mm/init.c +++ b/arch/arm64/mm/init.c @@ -609,7 +609,7 @@ void __init mem_init(void) * detected at build time already. */ #ifdef CONFIG_COMPAT - BUILD_BUG_ON(TASK_SIZE_32 > TASK_SIZE_64); + BUILD_BUG_ON(TASK_SIZE_32 > DEFAULT_MAP_WINDOW_64); #endif #ifdef CONFIG_SPARSEMEM_VMEMMAP diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c index 922cfb813109..952cec5b611a 100644 --- a/drivers/firmware/efi/arm-runtime.c +++ b/drivers/firmware/efi/arm-runtime.c @@ -38,7 +38,7 @@ static struct ptdump_info efi_ptdump_info = { .mm = &efi_mm, .markers = (struct addr_marker[]){ { 0, "UEFI runtime start" }, - { TASK_SIZE_64, "UEFI runtime end" } + { DEFAULT_MAP_WINDOW_64, "UEFI runtime end" } }, .base_addr = 0, }; diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c index 30ac0c975f8a..d1ec7136e3e1 100644 --- a/drivers/firmware/efi/libstub/arm-stub.c +++ b/drivers/firmware/efi/libstub/arm-stub.c @@ -33,7 +33,7 @@ #define EFI_RT_VIRTUAL_SIZE SZ_512M #ifdef CONFIG_ARM64 -# define EFI_RT_VIRTUAL_LIMIT TASK_SIZE_64 +# define EFI_RT_VIRTUAL_LIMIT DEFAULT_MAP_WINDOW_64 #else # define EFI_RT_VIRTUAL_LIMIT TASK_SIZE #endif
We wish to introduce a 52-bit virtual address space for userspace but maintain compatibility with software that assumes the maximum VA space size is 48 bit. In order to achieve this, on 52-bit VA systems, we make mmap behave as if it were running on a 48-bit VA system (unless userspace explicitly requests a VA where addr[51:48] != 0). On a system running a 52-bit userspace we need TASK_SIZE to represent the 52-bit limit as it is used in various places to distinguish between kernelspace and userspace addresses. Thus we need a new limit for mmap, stack, ELF loader and EFI (which uses TTBR0) to represent the non-extended VA space. This patch introduces DEFAULT_MAP_WINDOW and DEFAULT_MAP_WINDOW_64 and switches the appropriate logic to use that instead of TASK_SIZE. Signed-off-by: Steve Capper <steve.capper@arm.com> --- Changed in V3: corrections to allow COMPAT 32-bit EL0 mode to work --- arch/arm64/include/asm/elf.h | 2 +- arch/arm64/include/asm/processor.h | 10 ++++++++-- arch/arm64/mm/init.c | 2 +- drivers/firmware/efi/arm-runtime.c | 2 +- drivers/firmware/efi/libstub/arm-stub.c | 2 +- 5 files changed, 12 insertions(+), 6 deletions(-)