[PATCH] powerpc/microwatt: Add mmu bits to device tree

[PATCH] powerpc/microwatt: Add mmu bits to device tree

[Joel Stanley]

In commit 5402e239d09f ("powerpc/64s: Get LPID bit width from device
tree") the kernel tried to determine the pid and lpid bits from the
device tree. If they are not found, there is a fallback, but Microwatt
wasn't covered as has the unusual configuration of being both !HV and bare
metal.

Set the values in the device tree to avoid having to add a special case.
The lpid value is the only one required, but add both for completeness.

Fixes: 5402e239d09f ("powerpc/64s: Get LPID bit width from device tree")
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 arch/powerpc/boot/dts/microwatt.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/boot/dts/microwatt.dts b/arch/powerpc/boot/dts/microwatt.dts
index 65b270a90f94..b69db1d275cd 100644
--- a/arch/powerpc/boot/dts/microwatt.dts
+++ b/arch/powerpc/boot/dts/microwatt.dts
@@ -90,6 +90,8 @@ PowerPC,Microwatt@0 {
 			64-bit;
 			d-cache-size = <0x1000>;
 			ibm,chip-id = <0>;
+			ibm,mmu-lpid-bits = <12>;
+			ibm,mmu-pid-bits = <20>;
 		};
 	};
 
-- 
2.35.1

Re: [PATCH] powerpc/microwatt: Add mmu bits to device tree

[Nicholas Piggin]

Excerpts from Joel Stanley's message of May 19, 2022 10:57 pm:
> In commit 5402e239d09f ("powerpc/64s: Get LPID bit width from device
> tree") the kernel tried to determine the pid and lpid bits from the
> device tree. If they are not found, there is a fallback, but Microwatt
> wasn't covered as has the unusual configuration of being both !HV and bare
> metal.
> 
> Set the values in the device tree to avoid having to add a special case.
> The lpid value is the only one required, but add both for completeness.
> 
> Fixes: 5402e239d09f ("powerpc/64s: Get LPID bit width from device tree")
> Signed-off-by: Joel Stanley <joel@jms.id.au>

LGTM... does Microwatt actually need 12 lpid bits, or does it just need 
the LPID 0 partition table entry? I wonder if you set it to 1. That's
a minor nit though.

Acked-by: Nicholas Piggin <npiggin@gmail.com>

> ---
>  arch/powerpc/boot/dts/microwatt.dts | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/powerpc/boot/dts/microwatt.dts b/arch/powerpc/boot/dts/microwatt.dts
> index 65b270a90f94..b69db1d275cd 100644
> --- a/arch/powerpc/boot/dts/microwatt.dts
> +++ b/arch/powerpc/boot/dts/microwatt.dts
> @@ -90,6 +90,8 @@ PowerPC,Microwatt@0 {
>  			64-bit;
>  			d-cache-size = <0x1000>;
>  			ibm,chip-id = <0>;
> +			ibm,mmu-lpid-bits = <12>;
> +			ibm,mmu-pid-bits = <20>;
>  		};
>  	};
>  
> -- 
> 2.35.1
> 
> 

Re: [PATCH] powerpc/microwatt: Add mmu bits to device tree

[Michael Neuling]

On Fri, 2022-05-20 at 10:06 +1000, Nicholas Piggin wrote:
> Excerpts from Joel Stanley's message of May 19, 2022 10:57 pm:
> > In commit 5402e239d09f ("powerpc/64s: Get LPID bit width from device
> > tree") the kernel tried to determine the pid and lpid bits from the
> > device tree. If they are not found, there is a fallback, but Microwatt
> > wasn't covered as has the unusual configuration of being both !HV and bare
> > metal.
> > 
> > Set the values in the device tree to avoid having to add a special case.
> > The lpid value is the only one required, but add both for completeness.
> > 
> > Fixes: 5402e239d09f ("powerpc/64s: Get LPID bit width from device tree")
> > Signed-off-by: Joel Stanley <joel@jms.id.au>
> 
> LGTM... does Microwatt actually need 12 lpid bits, or does it just need 
> the LPID 0 partition table entry? 

Microwatt just assumes LPID=0 as it doesn't actually have an LPID SPR. It just
uses the first entry in the partition table.

There are some details here:

   https://github.com/antonblanchard/microwatt/commit/18120f153d138f733fa7e8a89c3456bb93683f96

> I wonder if you set it to 1. That's a minor nit though.

.. or even set it to 0.

Mikey

Re: [PATCH] powerpc/microwatt: Add mmu bits to device tree

[Joel Stanley]

On Fri, 20 May 2022 at 00:06, Nicholas Piggin <npiggin@gmail.com> wrote:
>
> Excerpts from Joel Stanley's message of May 19, 2022 10:57 pm:
> > In commit 5402e239d09f ("powerpc/64s: Get LPID bit width from device
> > tree") the kernel tried to determine the pid and lpid bits from the
> > device tree. If they are not found, there is a fallback, but Microwatt
> > wasn't covered as has the unusual configuration of being both !HV and bare
> > metal.
> >
> > Set the values in the device tree to avoid having to add a special case.
> > The lpid value is the only one required, but add both for completeness.
> >
> > Fixes: 5402e239d09f ("powerpc/64s: Get LPID bit width from device tree")
> > Signed-off-by: Joel Stanley <joel@jms.id.au>
>
> LGTM... does Microwatt actually need 12 lpid bits, or does it just need
> the LPID 0 partition table entry? I wonder if you set it to 1. That's
> a minor nit though.

0, 1, 4, 5, 6, 7 fails. 8, 9, 10, 11, 12 work.

I'll let someone else explain why that is!

Oh, it's because we use it like this:

arch/powerpc/include/asm/book3s/64/mmu.h:#define PATB_SIZE_SHIFT
 (mmu_lpid_bits + 4)

and then

arch/powerpc/mm/book3s64/radix_pgtable.c:
set_ptcr_when_no_uv(__pa(partition_tb) |
arch/powerpc/mm/book3s64/radix_pgtable.c:
     (PATB_SIZE_SHIFT - 12));

 so anything less than 8 will result in a negative value there.

The ISA says (from Figure 22 in 3.1, or 21 in 3.0B):

 Partition Table Size=2**(12+PATS), PATS<= 24.

From this it's clear that 12 >= mmu_lpid_bits >= 24.

(except there's only 4 bits for PATS in the ISA, so really the maximum is 16?)

What's not clear to me is why we define PATB_SIZE_SHIFT as mmu_lpid_bits + 4.

Cheers,

Joel

>
> Acked-by: Nicholas Piggin <npiggin@gmail.com>
>
> > ---
> >  arch/powerpc/boot/dts/microwatt.dts | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/arch/powerpc/boot/dts/microwatt.dts b/arch/powerpc/boot/dts/microwatt.dts
> > index 65b270a90f94..b69db1d275cd 100644
> > --- a/arch/powerpc/boot/dts/microwatt.dts
> > +++ b/arch/powerpc/boot/dts/microwatt.dts
> > @@ -90,6 +90,8 @@ PowerPC,Microwatt@0 {
> >                       64-bit;
> >                       d-cache-size = <0x1000>;
> >                       ibm,chip-id = <0>;
> > +                     ibm,mmu-lpid-bits = <12>;
> > +                     ibm,mmu-pid-bits = <20>;
> >               };
> >       };
> >
> > --
> > 2.35.1
> >
> >

Re: [PATCH] powerpc/microwatt: Add mmu bits to device tree

[Michael Ellerman]

On Thu, 19 May 2022 22:27:06 +0930, Joel Stanley wrote:
> In commit 5402e239d09f ("powerpc/64s: Get LPID bit width from device
> tree") the kernel tried to determine the pid and lpid bits from the
> device tree. If they are not found, there is a fallback, but Microwatt
> wasn't covered as has the unusual configuration of being both !HV and bare
> metal.
> 
> Set the values in the device tree to avoid having to add a special case.
> The lpid value is the only one required, but add both for completeness.
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc/microwatt: Add mmu bits to device tree
      https://git.kernel.org/powerpc/c/0ef1ffc7189521e293b4c5532659385dfddf8939

cheers