Re: [PATCH] powerpc/64s: Fix NULL AT_BASE_PLATFORM when using DT CPU features

Re: [PATCH] powerpc/64s: Fix NULL AT_BASE_PLATFORM when using DT CPU features

[Li Yi (Adam)]

> Date: Wed, 14 Mar 2018 10:14:11 +1100
> From: Michael Ellerman <mpe@ellerman.id.au>
> To: linuxppc-dev@ozlabs.org
> Cc: sukadev@linux.vnet.ibm.com, npiggin@gmail.com
> Subject: [PATCH] powerpc/64s: Fix NULL AT_BASE_PLATFORM when using DT
>         CPU features
> Message-ID: <20180313231411.9537-1-mpe@ellerman.id.au>
>
> When running virtualised the powerpc kernel is able to run the system
> in "compat mode" - which means the kernel and hardware are pretending
> to userspace that the CPU is an older version than it actually is.
>
> AT_BASE_PLATFORM is an AUXV entry that we export to userspace for use
> when we're running in that mode, which tells userspace the "platform"
> string for the real CPU version, as opposed to the faked version.
>
> Although we don't support compat mode when using DT CPU features, and
> arguably don't need to set AT_BASE_PLATFORM, the existing cputable
> based code always sets it even when we're running bare metal. That
> means the lack of AT_BASE_PLATFORM is a user-visible artifact of the
> fact that the kernel is using DT CPU features, which we don't want.
>
> So set it in the DT CPU features code also.
>
> This results in eg:
>   $ LD_SHOW_AUXV=1 /bin/true | grep "AT_.*PLATFORM"
>   AT_PLATFORM:     power9
>   AT_BASE_PLATFORM:power9
>

Is this issue related with DD2.2 CPU?
I tested on a Boston system with DD2.1 CPU, with kernel 4.14.23,
AT_BASE_PLATFORM value is correct:

[root@boston-sh-04 ~]# LD_SHOW_AUXV=1 /bin/true | grep "AT_.*PLATFORM"
AT_PLATFORM:     power9
AT_BASE_PLATFORM:power9
[root@boston-sh-04 ~]# uname -a
Linux boston-sh-04 4.14.23 #1 SMP Sat Mar 3 12:14:56 CST 2018 ppc64le
ppc64le ppc64le GNU/Linux

Thanks,
-Yi

> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
>  arch/powerpc/kernel/dt_cpu_ftrs.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
> index 945e2c29ad2d..0bcfb0f256e1 100644
> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
> @@ -720,6 +720,9 @@ static void __init cpufeatures_setup_finished(void)
>                 cur_cpu_spec->cpu_features |= CPU_FTR_HVMODE;
>         }
>
> +       /* Make sure powerpc_base_platform is non-NULL */
> +       powerpc_base_platform = cur_cpu_spec->platform;
> +
>         system_registers.lpcr = mfspr(SPRN_LPCR);
>         system_registers.hfscr = mfspr(SPRN_HFSCR);
>         system_registers.fscr = mfspr(SPRN_FSCR);
> --
> 2.14.1

Re: [PATCH] powerpc/64s: Fix NULL AT_BASE_PLATFORM when using DT CPU features

[Gustavo Romero]

Hi Yi,

On 03/14/2018 05:34 AM, Li Yi (Adam) wrote:
>> So set it in the DT CPU features code also.
>>
>> This results in eg:
>>    $ LD_SHOW_AUXV=1 /bin/true | grep "AT_.*PLATFORM"
>>    AT_PLATFORM:     power9
>>    AT_BASE_PLATFORM:power9
>>
> 
> Is this issue related with DD2.2 CPU?
> I tested on a Boston system with DD2.1 CPU, with kernel 4.14.23,
> AT_BASE_PLATFORM value is correct:
> 
> [root@boston-sh-04 ~]# LD_SHOW_AUXV=1 /bin/true | grep "AT_.*PLATFORM"
> AT_PLATFORM:     power9
> AT_BASE_PLATFORM:power9
> [root@boston-sh-04 ~]# uname -a
> Linux boston-sh-04 4.14.23 #1 SMP Sat Mar 3 12:14:56 CST 2018 ppc64le
> ppc64le ppc64le GNU/Linux

It's not related to the processor version, rather it's related to if you
are under a VM or running on baremetal. The issue happens on baremetal
when you are using DT to get the CPU features. So I guess the test you
performed above is on a VM? On DD2.1 baremetal AT_BASE_PLATFORM is
missing as on a DD2.2 baremetal (if mpe's patch is not applied):

root@r222l:~# LD_SHOW_AUXV=1 /bin/true | grep "AT_.*PLATFORM"
AT_PLATFORM:     power9
root@r222l:~# lscpu | fgrep Model
Model:               2.1 (pvr 004e 1201)
Model name:          POWER9, altivec supported
root@r222l:~# uname -a
Linux r222l 4.13.0-32-generic #35-Ubuntu SMP Thu Jan 25 09:05:20 UTC 2018 ppc64le ppc64le ppc64le GNU/Linux
   

Regards,
Gustavo

> 
> Thanks,
> -Yi
> 
>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>> ---
>>   arch/powerpc/kernel/dt_cpu_ftrs.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
>> index 945e2c29ad2d..0bcfb0f256e1 100644
>> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
>> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
>> @@ -720,6 +720,9 @@ static void __init cpufeatures_setup_finished(void)
>>                  cur_cpu_spec->cpu_features |= CPU_FTR_HVMODE;
>>          }
>>
>> +       /* Make sure powerpc_base_platform is non-NULL */
>> +       powerpc_base_platform = cur_cpu_spec->platform;
>> +
>>          system_registers.lpcr = mfspr(SPRN_LPCR);
>>          system_registers.hfscr = mfspr(SPRN_HFSCR);
>>          system_registers.fscr = mfspr(SPRN_FSCR);
>> --
>> 2.14.1
> 

Re: [PATCH] powerpc/64s: Fix NULL AT_BASE_PLATFORM when using DT CPU features

[Nicholas Piggin]

On Wed, 14 Mar 2018 10:14:11 +1100
Michael Ellerman <mpe@ellerman.id.au> wrote:

> When running virtualised the powerpc kernel is able to run the system
> in "compat mode" - which means the kernel and hardware are pretending
> to userspace that the CPU is an older version than it actually is.
> 
> AT_BASE_PLATFORM is an AUXV entry that we export to userspace for use
> when we're running in that mode, which tells userspace the "platform"
> string for the real CPU version, as opposed to the faked version.
> 
> Although we don't support compat mode when using DT CPU features, and
> arguably don't need to set AT_BASE_PLATFORM, the existing cputable
> based code always sets it even when we're running bare metal. That
> means the lack of AT_BASE_PLATFORM is a user-visible artifact of the
> fact that the kernel is using DT CPU features, which we don't want.
> 
> So set it in the DT CPU features code also.
> 
> This results in eg:
>   $ LD_SHOW_AUXV=1 /bin/true | grep "AT_.*PLATFORM"
>   AT_PLATFORM:     power9
>   AT_BASE_PLATFORM:power9
> 
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

Thanks, I missed this one. Seems fine to me.

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

> ---
>  arch/powerpc/kernel/dt_cpu_ftrs.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
> index 945e2c29ad2d..0bcfb0f256e1 100644
> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
> @@ -720,6 +720,9 @@ static void __init cpufeatures_setup_finished(void)
>  		cur_cpu_spec->cpu_features |= CPU_FTR_HVMODE;
>  	}
>  
> +	/* Make sure powerpc_base_platform is non-NULL */
> +	powerpc_base_platform = cur_cpu_spec->platform;
> +
>  	system_registers.lpcr = mfspr(SPRN_LPCR);
>  	system_registers.hfscr = mfspr(SPRN_HFSCR);
>  	system_registers.fscr = mfspr(SPRN_FSCR);

[PATCH] powerpc/64s: Fix NULL AT_BASE_PLATFORM when using DT CPU features

[Michael Ellerman]

When running virtualised the powerpc kernel is able to run the system
in "compat mode" - which means the kernel and hardware are pretending
to userspace that the CPU is an older version than it actually is.

AT_BASE_PLATFORM is an AUXV entry that we export to userspace for use
when we're running in that mode, which tells userspace the "platform"
string for the real CPU version, as opposed to the faked version.

Although we don't support compat mode when using DT CPU features, and
arguably don't need to set AT_BASE_PLATFORM, the existing cputable
based code always sets it even when we're running bare metal. That
means the lack of AT_BASE_PLATFORM is a user-visible artifact of the
fact that the kernel is using DT CPU features, which we don't want.

So set it in the DT CPU features code also.

This results in eg:
  $ LD_SHOW_AUXV=1 /bin/true | grep "AT_.*PLATFORM"
  AT_PLATFORM:     power9
  AT_BASE_PLATFORM:power9

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/kernel/dt_cpu_ftrs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
index 945e2c29ad2d..0bcfb0f256e1 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -720,6 +720,9 @@ static void __init cpufeatures_setup_finished(void)
 		cur_cpu_spec->cpu_features |= CPU_FTR_HVMODE;
 	}
 
+	/* Make sure powerpc_base_platform is non-NULL */
+	powerpc_base_platform = cur_cpu_spec->platform;
+
 	system_registers.lpcr = mfspr(SPRN_LPCR);
 	system_registers.hfscr = mfspr(SPRN_HFSCR);
 	system_registers.fscr = mfspr(SPRN_FSCR);
-- 
2.14.1