[PATCH] riscv: Start of DRAM should at least be aligned on PMD size for the direct mapping

[PATCH] riscv: Start of DRAM should at least be aligned on PMD size for the direct mapping

[Alexandre Ghiti]

So that we do not end up mapping the whole linear mapping using 4K
pages, which is slow at boot time, and also very likely at runtime.

So make sure we align the start of DRAM on a PMD boundary.

Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
---
 arch/riscv/mm/init.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 4fa420faa780..4a43ec275c6d 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -214,8 +214,13 @@ static void __init setup_bootmem(void)
 	memblock_reserve(vmlinux_start, vmlinux_end - vmlinux_start);
 
 	phys_ram_end = memblock_end_of_DRAM();
+
+	/*
+	 * Make sure we align the start of the memory on a PMD boundary so that
+	 * at worst, we map the linear mapping with PMD mappings.
+	 */
 	if (!IS_ENABLED(CONFIG_XIP_KERNEL))
-		phys_ram_base = memblock_start_of_DRAM();
+		phys_ram_base = memblock_start_of_DRAM() & PMD_MASK;
 
 	/*
 	 * In 64-bit, any use of __va/__pa before this point is wrong as we
-- 
2.39.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] riscv: Start of DRAM should at least be aligned on PMD size for the direct mapping

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 1404 bytes --]

Hey Alex,

On Tue, Jul 04, 2023 at 02:18:37PM +0200, Alexandre Ghiti wrote:
> So that we do not end up mapping the whole linear mapping using 4K
> pages, which is slow at boot time, and also very likely at runtime.
> 
> So make sure we align the start of DRAM on a PMD boundary.
> 
> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Obviously correct me if I am wrong here, but was this not reported by
Song Shuai as a regression?
Accordingly, should this not have Reported-by, Closes/Link & Fixes tags?

Cheers,
Conor.

> ---
>  arch/riscv/mm/init.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 4fa420faa780..4a43ec275c6d 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -214,8 +214,13 @@ static void __init setup_bootmem(void)
>  	memblock_reserve(vmlinux_start, vmlinux_end - vmlinux_start);
>  
>  	phys_ram_end = memblock_end_of_DRAM();
> +
> +	/*
> +	 * Make sure we align the start of the memory on a PMD boundary so that
> +	 * at worst, we map the linear mapping with PMD mappings.
> +	 */
>  	if (!IS_ENABLED(CONFIG_XIP_KERNEL))
> -		phys_ram_base = memblock_start_of_DRAM();
> +		phys_ram_base = memblock_start_of_DRAM() & PMD_MASK;
>  
>  	/*
>  	 * In 64-bit, any use of __va/__pa before this point is wrong as we
> -- 
> 2.39.2
> 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] riscv: Start of DRAM should at least be aligned on PMD size for the direct mapping

[Alexandre Ghiti]

On Tue, Jul 4, 2023 at 2:26 PM Conor Dooley <conor.dooley@microchip.com> wrote:
>
> Hey Alex,
>
> On Tue, Jul 04, 2023 at 02:18:37PM +0200, Alexandre Ghiti wrote:
> > So that we do not end up mapping the whole linear mapping using 4K
> > pages, which is slow at boot time, and also very likely at runtime.
> >
> > So make sure we align the start of DRAM on a PMD boundary.
> >
> > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
>
> Obviously correct me if I am wrong here, but was this not reported by
> Song Shuai as a regression?
> Accordingly, should this not have Reported-by, Closes/Link & Fixes tags?

Sure we should add the reported by from Song as he did the proper report :)

Reported-by: Song Shuai <suagrfillet@gmail.com>
Closes: https://lore.kernel.org/linux-riscv/20230625140931.1266216-1-songshuaishuai@tinylab.org/

And yes sorry, I thought it was there before, but it was actually when
I retrieved the first 2MB that the problem appeared, so:

Fixes: 3335068f8721 ("riscv: Use PUD/P4D/PGD pages for the linear mapping")

Thanks!

>
> Cheers,
> Conor.
>
> > ---
> >  arch/riscv/mm/init.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > index 4fa420faa780..4a43ec275c6d 100644
> > --- a/arch/riscv/mm/init.c
> > +++ b/arch/riscv/mm/init.c
> > @@ -214,8 +214,13 @@ static void __init setup_bootmem(void)
> >       memblock_reserve(vmlinux_start, vmlinux_end - vmlinux_start);
> >
> >       phys_ram_end = memblock_end_of_DRAM();
> > +
> > +     /*
> > +      * Make sure we align the start of the memory on a PMD boundary so that
> > +      * at worst, we map the linear mapping with PMD mappings.
> > +      */
> >       if (!IS_ENABLED(CONFIG_XIP_KERNEL))
> > -             phys_ram_base = memblock_start_of_DRAM();
> > +             phys_ram_base = memblock_start_of_DRAM() & PMD_MASK;
> >
> >       /*
> >        * In 64-bit, any use of __va/__pa before this point is wrong as we
> > --
> > 2.39.2
> >

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] riscv: Start of DRAM should at least be aligned on PMD size for the direct mapping

[Song Shuai]

在 2023/7/4 21:16, Alexandre Ghiti 写道:
> On Tue, Jul 4, 2023 at 2:26 PM Conor Dooley <conor.dooley@microchip.com> wrote:
>>
>> Hey Alex,
>>
>> On Tue, Jul 04, 2023 at 02:18:37PM +0200, Alexandre Ghiti wrote:
>>> So that we do not end up mapping the whole linear mapping using 4K
>>> pages, which is slow at boot time, and also very likely at runtime.
>>>
>>> So make sure we align the start of DRAM on a PMD boundary.
>>>
>>> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
>>
>> Obviously correct me if I am wrong here, but was this not reported by
>> Song Shuai as a regression?
>> Accordingly, should this not have Reported-by, Closes/Link & Fixes tags?
> 
> Sure we should add the reported by from Song as he did the proper report :)
> 
> Reported-by: Song Shuai <suagrfillet@gmail.com>
> Closes: https://lore.kernel.org/linux-riscv/20230625140931.1266216-1-songshuaishuai@tinylab.org/
> 
> And yes sorry, I thought it was there before, but it was actually when
> I retrieved the first 2MB that the problem appeared, so:
> 
> Fixes: 3335068f8721 ("riscv: Use PUD/P4D/PGD pages for the linear mapping")
> 
> Thanks!
And you can add my tested-by:

Tested-by: Song Shuai <suagrfillet@gmail.com>
> 
>>
>> Cheers,
>> Conor.
>>
>>> ---
>>>   arch/riscv/mm/init.c | 7 ++++++-
>>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
>>> index 4fa420faa780..4a43ec275c6d 100644
>>> --- a/arch/riscv/mm/init.c
>>> +++ b/arch/riscv/mm/init.c
>>> @@ -214,8 +214,13 @@ static void __init setup_bootmem(void)
>>>        memblock_reserve(vmlinux_start, vmlinux_end - vmlinux_start);
>>>
>>>        phys_ram_end = memblock_end_of_DRAM();
>>> +
>>> +     /*
>>> +      * Make sure we align the start of the memory on a PMD boundary so that
>>> +      * at worst, we map the linear mapping with PMD mappings.
>>> +      */
>>>        if (!IS_ENABLED(CONFIG_XIP_KERNEL))
>>> -             phys_ram_base = memblock_start_of_DRAM();
>>> +             phys_ram_base = memblock_start_of_DRAM() & PMD_MASK;
>>>
>>>        /*
>>>         * In 64-bit, any use of __va/__pa before this point is wrong as we
>>> --
>>> 2.39.2
>>>

-- 
Thanks
Song Shuai

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] riscv: Start of DRAM should at least be aligned on PMD size for the direct mapping

[Palmer Dabbelt]

On Tue, 04 Jul 2023 05:18:37 PDT (-0700), alexghiti@rivosinc.com wrote:
> So that we do not end up mapping the whole linear mapping using 4K
> pages, which is slow at boot time, and also very likely at runtime.
>
> So make sure we align the start of DRAM on a PMD boundary.
>
> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> ---
>  arch/riscv/mm/init.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 4fa420faa780..4a43ec275c6d 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -214,8 +214,13 @@ static void __init setup_bootmem(void)
>  	memblock_reserve(vmlinux_start, vmlinux_end - vmlinux_start);
>
>  	phys_ram_end = memblock_end_of_DRAM();
> +
> +	/*
> +	 * Make sure we align the start of the memory on a PMD boundary so that
> +	 * at worst, we map the linear mapping with PMD mappings.
> +	 */
>  	if (!IS_ENABLED(CONFIG_XIP_KERNEL))
> -		phys_ram_base = memblock_start_of_DRAM();
> +		phys_ram_base = memblock_start_of_DRAM() & PMD_MASK;

This rounds down, which IIUC will result in mappings outside what 
memblock detected as the start af DRAM.  I'd expect that to cause bad 
behavior somewhere.

Shouldn't we be rounding up?

>
>  	/*
>  	 * In 64-bit, any use of __va/__pa before this point is wrong as we

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] riscv: Start of DRAM should at least be aligned on PMD size for the direct mapping

[Alexandre Ghiti]

(sorry for the delay!)

On Thu, Jul 6, 2023 at 7:05 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Tue, 04 Jul 2023 05:18:37 PDT (-0700), alexghiti@rivosinc.com wrote:
> > So that we do not end up mapping the whole linear mapping using 4K
> > pages, which is slow at boot time, and also very likely at runtime.
> >
> > So make sure we align the start of DRAM on a PMD boundary.
> >
> > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> > ---
> >  arch/riscv/mm/init.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > index 4fa420faa780..4a43ec275c6d 100644
> > --- a/arch/riscv/mm/init.c
> > +++ b/arch/riscv/mm/init.c
> > @@ -214,8 +214,13 @@ static void __init setup_bootmem(void)
> >       memblock_reserve(vmlinux_start, vmlinux_end - vmlinux_start);
> >
> >       phys_ram_end = memblock_end_of_DRAM();
> > +
> > +     /*
> > +      * Make sure we align the start of the memory on a PMD boundary so that
> > +      * at worst, we map the linear mapping with PMD mappings.
> > +      */
> >       if (!IS_ENABLED(CONFIG_XIP_KERNEL))
> > -             phys_ram_base = memblock_start_of_DRAM();
> > +             phys_ram_base = memblock_start_of_DRAM() & PMD_MASK;
>
> This rounds down, which IIUC will result in mappings outside what
> memblock detected as the start af DRAM.  I'd expect that to cause bad
> behavior somewhere.

Actually we are not mapping this new region as it is not present in
the memblock regions, we are just re-aligning the virtual and physical
address: phys_ram_base is only used for the virtual to physical
translations.

>
> Shouldn't we be rounding up?

Doing so would remove memory from the memory map, but I'm not sure
this is correct, we could remove memory that contains "something" that
needs to be accessed using the linear mapping (ACPI tables? DT?).

More testing is welcome as I can be wrong of course.

Thanks,

Alex


>
> >
> >       /*
> >        * In 64-bit, any use of __va/__pa before this point is wrong as we

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] riscv: Start of DRAM should at least be aligned on PMD size for the direct mapping

[Palmer Dabbelt]

On Tue, 04 Jul 2023 14:18:37 +0200, Alexandre Ghiti wrote:
> So that we do not end up mapping the whole linear mapping using 4K
> pages, which is slow at boot time, and also very likely at runtime.
> 
> So make sure we align the start of DRAM on a PMD boundary.
> 
> 

Applied, thanks!

[1/1] riscv: Start of DRAM should at least be aligned on PMD size for the direct mapping
      https://git.kernel.org/palmer/c/9d3e8e1ff0d8

Best regards,
-- 
Palmer Dabbelt <palmer@rivosinc.com>


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] riscv: Start of DRAM should at least be aligned on PMD size for the direct mapping

[patchwork-bot+linux-riscv]

Hello:

This patch was applied to riscv/linux.git (fixes)
by Palmer Dabbelt <palmer@rivosinc.com>:

On Tue,  4 Jul 2023 14:18:37 +0200 you wrote:
> So that we do not end up mapping the whole linear mapping using 4K
> pages, which is slow at boot time, and also very likely at runtime.
> 
> So make sure we align the start of DRAM on a PMD boundary.
> 
> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> 
> [...]

Here is the summary with links:
  - riscv: Start of DRAM should at least be aligned on PMD size for the direct mapping
    https://git.kernel.org/riscv/c/9d3e8e1ff0d8

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv