[PATCH] RISC-V: Fix Maximum Physical Memory 2GiB option for 64bit systems

[PATCH] RISC-V: Fix Maximum Physical Memory 2GiB option for 64bit systems

[Anup Patel]

The Maximum Physical Memory 2GiB option for 64bit systems is currently
broken because kernel hangs at boot-time when this option is enabled
and the underlying system has more than 2GiB memory.

This issue can be easily reproduced on SiFive Unleashed board where
we have 8GiB of memory.

This patch fixes above issue by reserving unusable memory region in
setup_bootmem().

Signed-off-by: Anup Patel <anup.patel@wdc.com>
---
 arch/riscv/mm/init.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 5fd8c922e1c2..6b063f20a9d0 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -121,6 +121,14 @@ void __init setup_bootmem(void)
 			 */
 			memblock_reserve(reg->base, vmlinux_end - reg->base);
 			mem_size = min(reg->size, (phys_addr_t)-PAGE_OFFSET);
+
+			/*
+			 * Reserve from the end of usable area to the end of
+			 * region
+			 */
+			if ((reg->base + mem_size) < end)
+				memblock_reserve(reg->base + mem_size,
+						 end - reg->base - mem_size);
 		}
 	}
 	BUG_ON(mem_size == 0);
-- 
2.17.1

Re: [PATCH] RISC-V: Fix Maximum Physical Memory 2GiB option for 64bit systems

[Mike Rapoport]

On Tue, Apr 02, 2019 at 06:02:38AM +0000, Anup Patel wrote:
> The Maximum Physical Memory 2GiB option for 64bit systems is currently
> broken because kernel hangs at boot-time when this option is enabled
> and the underlying system has more than 2GiB memory.
> 
> This issue can be easily reproduced on SiFive Unleashed board where
> we have 8GiB of memory.
> 
> This patch fixes above issue by reserving unusable memory region in
> setup_bootmem().
> 
> Signed-off-by: Anup Patel <anup.patel@wdc.com>
> ---
>  arch/riscv/mm/init.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 5fd8c922e1c2..6b063f20a9d0 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -121,6 +121,14 @@ void __init setup_bootmem(void)
>  			 */
>  			memblock_reserve(reg->base, vmlinux_end - reg->base);
>  			mem_size = min(reg->size, (phys_addr_t)-PAGE_OFFSET);
> +
> +			/*
> +			 * Reserve from the end of usable area to the end of
> +			 * region
> +			 */
> +			if ((reg->base + mem_size) < end)
> +				memblock_reserve(reg->base + mem_size,
> +						 end - reg->base - mem_size);

The memory above MAXPHYSMEM should not be reserved. It should be either
removed from memblock with memblock_remove or not added at the first place.

Frankly, I fail to understand the logic behind setting PAGE_OFFSET to
MAXPHYSMEM and then using PAGE_OFFSET as the limit for accessible physical
memory. Still, as it is there, you can set MAX_MEMBLOCK_ADDR=PAGE_OFFSET in
arch/riscv/include/page.h and then early_init_dt_add_memory_arch() will
simply ignore the memory above PAGE_OFFSET.

More sustainable fix for the long term, IMHO, would be to break
PAGE_OFFSET and MAXPHYSMEM interdependency.


>  		}
>  	}
>  	BUG_ON(mem_size == 0);
> -- 
> 2.17.1
> 

-- 
Sincerely yours,
Mike.

Re: [PATCH] RISC-V: Fix Maximum Physical Memory 2GiB option for 64bit systems

[Anup Patel]

On Tue, Apr 2, 2019 at 2:05 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
>
> On Tue, Apr 02, 2019 at 06:02:38AM +0000, Anup Patel wrote:
> > The Maximum Physical Memory 2GiB option for 64bit systems is currently
> > broken because kernel hangs at boot-time when this option is enabled
> > and the underlying system has more than 2GiB memory.
> >
> > This issue can be easily reproduced on SiFive Unleashed board where
> > we have 8GiB of memory.
> >
> > This patch fixes above issue by reserving unusable memory region in
> > setup_bootmem().
> >
> > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> > ---
> >  arch/riscv/mm/init.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > index 5fd8c922e1c2..6b063f20a9d0 100644
> > --- a/arch/riscv/mm/init.c
> > +++ b/arch/riscv/mm/init.c
> > @@ -121,6 +121,14 @@ void __init setup_bootmem(void)
> >                        */
> >                       memblock_reserve(reg->base, vmlinux_end - reg->base);
> >                       mem_size = min(reg->size, (phys_addr_t)-PAGE_OFFSET);
> > +
> > +                     /*
> > +                      * Reserve from the end of usable area to the end of
> > +                      * region
> > +                      */
> > +                     if ((reg->base + mem_size) < end)
> > +                             memblock_reserve(reg->base + mem_size,
> > +                                              end - reg->base - mem_size);
>
> The memory above MAXPHYSMEM should not be reserved. It should be either
> removed from memblock with memblock_remove or not added at the first place.

Sure, I will try memblock_remove() instead of memblock_reserve().

>
> Frankly, I fail to understand the logic behind setting PAGE_OFFSET to
> MAXPHYSMEM and then using PAGE_OFFSET as the limit for accessible physical
> memory. Still, as it is there, you can set MAX_MEMBLOCK_ADDR=PAGE_OFFSET in
> arch/riscv/include/page.h and then early_init_dt_add_memory_arch() will
> simply ignore the memory above PAGE_OFFSET.
>
> More sustainable fix for the long term, IMHO, would be to break
> PAGE_OFFSET and MAXPHYSMEM interdependency.

I agree with you. There is too much dependency on PAGE_OFFSET value.

Another thing that I had attempted in my setup_vm() patches, is to reduce
initial page tables memory consumption by making page table array size
independent of -PAGE_OFFSET.

Regards,
Anup

Re: [PATCH] RISC-V: Fix Maximum Physical Memory 2GiB option for 64bit systems

[Anup Patel]

On Tue, Apr 2, 2019 at 2:05 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
>
> On Tue, Apr 02, 2019 at 06:02:38AM +0000, Anup Patel wrote:
> > The Maximum Physical Memory 2GiB option for 64bit systems is currently
> > broken because kernel hangs at boot-time when this option is enabled
> > and the underlying system has more than 2GiB memory.
> >
> > This issue can be easily reproduced on SiFive Unleashed board where
> > we have 8GiB of memory.
> >
> > This patch fixes above issue by reserving unusable memory region in
> > setup_bootmem().
> >
> > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> > ---
> >  arch/riscv/mm/init.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > index 5fd8c922e1c2..6b063f20a9d0 100644
> > --- a/arch/riscv/mm/init.c
> > +++ b/arch/riscv/mm/init.c
> > @@ -121,6 +121,14 @@ void __init setup_bootmem(void)
> >                        */
> >                       memblock_reserve(reg->base, vmlinux_end - reg->base);
> >                       mem_size = min(reg->size, (phys_addr_t)-PAGE_OFFSET);
> > +
> > +                     /*
> > +                      * Reserve from the end of usable area to the end of
> > +                      * region
> > +                      */
> > +                     if ((reg->base + mem_size) < end)
> > +                             memblock_reserve(reg->base + mem_size,
> > +                                              end - reg->base - mem_size);
>
> The memory above MAXPHYSMEM should not be reserved. It should be either
> removed from memblock with memblock_remove or not added at the first place.
>
> Frankly, I fail to understand the logic behind setting PAGE_OFFSET to
> MAXPHYSMEM and then using PAGE_OFFSET as the limit for accessible physical
> memory. Still, as it is there, you can set MAX_MEMBLOCK_ADDR=PAGE_OFFSET in
> arch/riscv/include/page.h and then early_init_dt_add_memory_arch() will
> simply ignore the memory above PAGE_OFFSET.

Little explanation about current code ...

The current code, assumes PAGE_OFFSET to have value such that
upper-bits are 1s lower bits are 0s.
For example,
0xc0000000 (32bit),
0xffffffff80000000 (64bit), and
0xffffffe000000000 (64bit)

For above PAGE_OFFSET values, -PAGE_OFFSET is size of virtual
address space and maximum supported physical memory is also
-PAGE_OFFSET hence MAXPHYMEM is tied with -PAGE_OFFSET.

If we try to force some arbitrary PAGE_OFFSET then things break.

Regards,
Anup

Re: [PATCH] RISC-V: Fix Maximum Physical Memory 2GiB option for 64bit systems

[Mike Rapoport]

On Tue, Apr 02, 2019 at 03:00:02PM +0530, Anup Patel wrote:
> On Tue, Apr 2, 2019 at 2:05 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
> >
> > On Tue, Apr 02, 2019 at 06:02:38AM +0000, Anup Patel wrote:
> > > The Maximum Physical Memory 2GiB option for 64bit systems is currently
> > > broken because kernel hangs at boot-time when this option is enabled
> > > and the underlying system has more than 2GiB memory.
> > >
> > > This issue can be easily reproduced on SiFive Unleashed board where
> > > we have 8GiB of memory.
> > >
> > > This patch fixes above issue by reserving unusable memory region in
> > > setup_bootmem().
> > >
> > > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> > > ---
> > >  arch/riscv/mm/init.c | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > >
> > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > > index 5fd8c922e1c2..6b063f20a9d0 100644
> > > --- a/arch/riscv/mm/init.c
> > > +++ b/arch/riscv/mm/init.c
> > > @@ -121,6 +121,14 @@ void __init setup_bootmem(void)
> > >                        */
> > >                       memblock_reserve(reg->base, vmlinux_end - reg->base);
> > >                       mem_size = min(reg->size, (phys_addr_t)-PAGE_OFFSET);
> > > +
> > > +                     /*
> > > +                      * Reserve from the end of usable area to the end of
> > > +                      * region
> > > +                      */
> > > +                     if ((reg->base + mem_size) < end)
> > > +                             memblock_reserve(reg->base + mem_size,
> > > +                                              end - reg->base - mem_size);
> >
> > The memory above MAXPHYSMEM should not be reserved. It should be either
> > removed from memblock with memblock_remove or not added at the first place.
> >
> > Frankly, I fail to understand the logic behind setting PAGE_OFFSET to
> > MAXPHYSMEM and then using PAGE_OFFSET as the limit for accessible physical
> > memory. Still, as it is there, you can set MAX_MEMBLOCK_ADDR=PAGE_OFFSET in
> > arch/riscv/include/page.h and then early_init_dt_add_memory_arch() will
> > simply ignore the memory above PAGE_OFFSET.
> 
> Little explanation about current code ...
> 
> The current code, assumes PAGE_OFFSET to have value such that
> upper-bits are 1s lower bits are 0s.
> For example,
> 0xc0000000 (32bit),
> 0xffffffff80000000 (64bit), and
> 0xffffffe000000000 (64bit)
> 
> For above PAGE_OFFSET values, -PAGE_OFFSET is size of virtual
> address space and maximum supported physical memory is also
> -PAGE_OFFSET hence MAXPHYMEM is tied with -PAGE_OFFSET.

Right, but I what I don't understand is *why* PAGE_OFFSET is used as the
maximal supported physical address.
 
> If we try to force some arbitrary PAGE_OFFSET then things break.

Apparently they will :)
 
> Regards,
> Anup
> 

-- 
Sincerely yours,
Mike.