| Message ID | 20220125155542.3753-1-jszhang@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | riscv: mm: remove the BUG_ON check of mapping the last 4K bytes of memory | expand |
On 1/25/22 16:55, Jisheng Zhang wrote: > remove the BUG_ON check of mapping the last 4K bytes of the addressable > memory since "this is true for every kernel actually" as pointed out > by Alexandre. > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > Reviewed-by: Alexandre Ghiti <alex@ghiti.fr> > --- > arch/riscv/mm/init.c | 8 -------- > 1 file changed, 8 deletions(-) > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c > index cf4d018b7d66..8347d0fda8cd 100644 > --- a/arch/riscv/mm/init.c > +++ b/arch/riscv/mm/init.c > @@ -811,14 +811,6 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa) > BUG_ON((PAGE_OFFSET % PGDIR_SIZE) != 0); > BUG_ON((kernel_map.phys_addr % PMD_SIZE) != 0); > > -#ifdef CONFIG_64BIT > - /* > - * The last 4K bytes of the addressable memory can not be mapped because > - * of IS_ERR_VALUE macro. > - */ > - BUG_ON((kernel_map.virt_addr + kernel_map.size) > ADDRESS_SPACE_END - SZ_4K); > -#endif This BUG_ON seems pretty legit to me: I re-read the exchanges we had, and I see that I didn't notice that in your v2, you actually removed the BUG_ON. So that's my bad, what I meant in the first place was that the BUG_ON is true for 32-bit and 64-bit kernels actually. Sorry my RB was not right on this one :( Alex > - > pt_ops_set_early(); > > /* Setup early PGD for fixmap */
On Tue, 25 Jan 2022 08:10:41 PST (-0800), alex@ghiti.fr wrote: > > On 1/25/22 16:55, Jisheng Zhang wrote: >> remove the BUG_ON check of mapping the last 4K bytes of the addressable >> memory since "this is true for every kernel actually" as pointed out >> by Alexandre. >> >> Signed-off-by: Jisheng Zhang <jszhang@kernel.org> >> Reviewed-by: Alexandre Ghiti <alex@ghiti.fr> >> --- >> arch/riscv/mm/init.c | 8 -------- >> 1 file changed, 8 deletions(-) >> >> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c >> index cf4d018b7d66..8347d0fda8cd 100644 >> --- a/arch/riscv/mm/init.c >> +++ b/arch/riscv/mm/init.c >> @@ -811,14 +811,6 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa) >> BUG_ON((PAGE_OFFSET % PGDIR_SIZE) != 0); >> BUG_ON((kernel_map.phys_addr % PMD_SIZE) != 0); >> >> -#ifdef CONFIG_64BIT >> - /* >> - * The last 4K bytes of the addressable memory can not be mapped because >> - * of IS_ERR_VALUE macro. >> - */ >> - BUG_ON((kernel_map.virt_addr + kernel_map.size) > ADDRESS_SPACE_END - SZ_4K); >> -#endif > > > This BUG_ON seems pretty legit to me: I re-read the exchanges we had, > and I see that I didn't notice that in your v2, you actually removed the > BUG_ON. So that's my bad, what I meant in the first place was that the > BUG_ON is true for 32-bit and 64-bit kernels actually. There's actually an ifndef 64BIT above that sort of handles this case (though I didn't check to see if we're getting the limits correct, so it may not work properly). That's shrinking the memory, rather than just firing a BUG, and it's not really any more code so we should go that way for both. I could see leaving a BUG in there, maybe just explicitly using IS_ERR_VALUE as that's really what we're checking for (though if that's not 4K a bunch of stuff will break, so maybe it just doesn't matter). > Sorry my RB was not right on this one :( > > Alex > > >> - >> pt_ops_set_early(); >> >> /* Setup early PGD for fixmap */
Hi Palmer, On 2/15/22 00:41, Palmer Dabbelt wrote: > On Tue, 25 Jan 2022 08:10:41 PST (-0800), alex@ghiti.fr wrote: >> >> On 1/25/22 16:55, Jisheng Zhang wrote: >>> remove the BUG_ON check of mapping the last 4K bytes of the addressable >>> memory since "this is true for every kernel actually" as pointed out >>> by Alexandre. >>> >>> Signed-off-by: Jisheng Zhang <jszhang@kernel.org> >>> Reviewed-by: Alexandre Ghiti <alex@ghiti.fr> >>> --- >>> arch/riscv/mm/init.c | 8 -------- >>> 1 file changed, 8 deletions(-) >>> >>> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c >>> index cf4d018b7d66..8347d0fda8cd 100644 >>> --- a/arch/riscv/mm/init.c >>> +++ b/arch/riscv/mm/init.c >>> @@ -811,14 +811,6 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa) >>> BUG_ON((PAGE_OFFSET % PGDIR_SIZE) != 0); >>> BUG_ON((kernel_map.phys_addr % PMD_SIZE) != 0); >>> >>> -#ifdef CONFIG_64BIT >>> - /* >>> - * The last 4K bytes of the addressable memory can not be >>> mapped because >>> - * of IS_ERR_VALUE macro. >>> - */ >>> - BUG_ON((kernel_map.virt_addr + kernel_map.size) > >>> ADDRESS_SPACE_END - SZ_4K); >>> -#endif >> >> >> This BUG_ON seems pretty legit to me: I re-read the exchanges we had, >> and I see that I didn't notice that in your v2, you actually removed the >> BUG_ON. So that's my bad, what I meant in the first place was that the >> BUG_ON is true for 32-bit and 64-bit kernels actually. > > There's actually an ifndef 64BIT above that sort of handles this case > (though I didn't check to see if we're getting the limits correct, so > it may not work properly). That's shrinking the memory, rather than > just firing a BUG, and it's not really any more code so we should go > that way for both. I could see leaving a BUG in there, maybe just > explicitly using IS_ERR_VALUE as that's really what we're checking for > (though if that's not 4K a bunch of stuff will break, so maybe it just > doesn't matter). I think you're talking about: memory_limit = KERN_VIRT_SIZE - (IS_ENABLED(CONFIG_64BIT) ? SZ_4G : 0); If yes, that's shrinking the available physical memory we can map in the linear mapping, whereas the BUG_ON here is triggered if the kernel mapping is very (very) large and overlaps the last *4K* of the address space, so I still think the BUG_ON is legit although very unlikely. However, I'm wondering if the shrinking still holds with the recent move of KASAN, I may be back with a fix for that. Alex > >> Sorry my RB was not right on this one :( >> >> Alex >> >> >>> - >>> pt_ops_set_early(); >>> >>> /* Setup early PGD for fixmap */
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c index cf4d018b7d66..8347d0fda8cd 100644 --- a/arch/riscv/mm/init.c +++ b/arch/riscv/mm/init.c @@ -811,14 +811,6 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa) BUG_ON((PAGE_OFFSET % PGDIR_SIZE) != 0); BUG_ON((kernel_map.phys_addr % PMD_SIZE) != 0); -#ifdef CONFIG_64BIT - /* - * The last 4K bytes of the addressable memory can not be mapped because - * of IS_ERR_VALUE macro. - */ - BUG_ON((kernel_map.virt_addr + kernel_map.size) > ADDRESS_SPACE_END - SZ_4K); -#endif - pt_ops_set_early(); /* Setup early PGD for fixmap */