Re: [PATCH 4/4] powerpc/powernv: Remove POWER9 PVR version check for entry and uaccess flushes

2021-05-08 Thread Nicholas Piggin
Excerpts from Joel Stanley's message of May 5, 2021 11:43 am:
> On Tue, 4 May 2021 at 09:16, Nicholas Piggin  wrote:
>>
>> Excerpts from Joel Stanley's message of May 4, 2021 10:51 am:
>> > On Mon, 3 May 2021 at 13:04, Nicholas Piggin  wrote:
>> >>
>> >> These aren't necessarily POWER9 only, and it's not to say some new
>> >> vulnerability may not get discovered on other processors for which
>> >> we would like the flexibility of having the workaround enabled by
>> >> firmware.
>> >>
>> >> Remove the restriction that they only apply to POWER9.
>> >
>> > I was wondering how these worked which led me to reviewing your patch.
>> > From what I could see, these are enabled by default (SEC_FTR_DEFAULT
>> > in arch/powerpc/include/asm/security_features.h), so unless all
>> > non-POWER9 machines have set the "please don't" bit in their firmware
>> > this patch will enable the feature for those machines. Is that what
>> > you wanted?
>>
>> Yes. POWER7/8 should be affected (it's similar mechanism that requires
>> the meltdown RFI flush, which those processors need).
>>
>> POWER10 we haven't released a bare metal firmware with the right bits
>> yet. Not urgent at the moment but wouldn't hurt to specify them and
>> add the Linux code for them.
> 
> Thanks for the explanation. This could go in the commit message if you 
> re-spin.
> 
> Reviewed-by: Joel Stanley 
> 

I was talking about the same thing with Michael and he dug up an old
email chain that proves me wrong. P7/8 are actually slightly different.
I'm not sure what I can explain of it in public unfortunately.

How about this?

---

These aren't necessarily POWER9 only, and it's not to say some new
vulnerability may not get discovered on other processors for which
we would like the flexibility of having the workaround enabled by
firmware.

Remove the restriction that the workarounds only apply to POWER9.

However POWER7 and POWER8 are not affected, and they may not have
older firmware that does not advertise this, so clear these workarounds
manually.

Signed-off-by: Nicholas Piggin 
---
 arch/powerpc/platforms/powernv/setup.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/setup.c 
b/arch/powerpc/platforms/powernv/setup.c
index a8db3f153063..874fb016384a 100644
--- a/arch/powerpc/platforms/powernv/setup.c
+++ b/arch/powerpc/platforms/powernv/setup.c
@@ -123,10 +123,14 @@ static void pnv_setup_security_mitigations(void)
}
 
/*
-* If we are non-Power9 bare metal, we don't need to flush on kernel
-* entry or after user access: they fix a P9 specific vulnerability.
+* The issues addressed by the entry and uaccess flush don't affect P7
+* or P8, so on bare metal disable them explicitly in case firmware
+* does not include these bits. POWER9 and newer processors should
+* have the right firmware bits.
 */
-   if (!pvr_version_is(PVR_POWER9)) {
+   if (pvr_version_is(PVR_POWER7) || pvr_version_is(PVR_POWER7p) ||
+   pvr_version_is(PVR_POWER8E) || pvr_version_is(PVR_POWER8NVL) ||
+   pvr_version_is(PVR_POWER8)) {
security_ftr_clear(SEC_FTR_L1D_FLUSH_ENTRY);
security_ftr_clear(SEC_FTR_L1D_FLUSH_UACCESS);
}
-- 
2.23.0



Re: [PATCH 4/4] powerpc/powernv: Remove POWER9 PVR version check for entry and uaccess flushes

2021-05-04 Thread Joel Stanley
On Tue, 4 May 2021 at 09:16, Nicholas Piggin  wrote:
>
> Excerpts from Joel Stanley's message of May 4, 2021 10:51 am:
> > On Mon, 3 May 2021 at 13:04, Nicholas Piggin  wrote:
> >>
> >> These aren't necessarily POWER9 only, and it's not to say some new
> >> vulnerability may not get discovered on other processors for which
> >> we would like the flexibility of having the workaround enabled by
> >> firmware.
> >>
> >> Remove the restriction that they only apply to POWER9.
> >
> > I was wondering how these worked which led me to reviewing your patch.
> > From what I could see, these are enabled by default (SEC_FTR_DEFAULT
> > in arch/powerpc/include/asm/security_features.h), so unless all
> > non-POWER9 machines have set the "please don't" bit in their firmware
> > this patch will enable the feature for those machines. Is that what
> > you wanted?
>
> Yes. POWER7/8 should be affected (it's similar mechanism that requires
> the meltdown RFI flush, which those processors need).
>
> POWER10 we haven't released a bare metal firmware with the right bits
> yet. Not urgent at the moment but wouldn't hurt to specify them and
> add the Linux code for them.

Thanks for the explanation. This could go in the commit message if you re-spin.

Reviewed-by: Joel Stanley 


Re: [PATCH 4/4] powerpc/powernv: Remove POWER9 PVR version check for entry and uaccess flushes

2021-05-04 Thread Nicholas Piggin
Excerpts from Joel Stanley's message of May 4, 2021 10:51 am:
> On Mon, 3 May 2021 at 13:04, Nicholas Piggin  wrote:
>>
>> These aren't necessarily POWER9 only, and it's not to say some new
>> vulnerability may not get discovered on other processors for which
>> we would like the flexibility of having the workaround enabled by
>> firmware.
>>
>> Remove the restriction that they only apply to POWER9.
> 
> I was wondering how these worked which led me to reviewing your patch.
> From what I could see, these are enabled by default (SEC_FTR_DEFAULT
> in arch/powerpc/include/asm/security_features.h), so unless all
> non-POWER9 machines have set the "please don't" bit in their firmware
> this patch will enable the feature for those machines. Is that what
> you wanted?

Yes. POWER7/8 should be affected (it's similar mechanism that requires
the meltdown RFI flush, which those processors need).

POWER10 we haven't released a bare metal firmware with the right bits
yet. Not urgent at the moment but wouldn't hurt to specify them and
add the Linux code for them.

Thanks,
Nick

> 
>>
>> Signed-off-by: Nicholas Piggin 
>> ---
>>  arch/powerpc/platforms/powernv/setup.c | 9 -
>>  1 file changed, 9 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/powernv/setup.c 
>> b/arch/powerpc/platforms/powernv/setup.c
>> index a8db3f153063..6ec67223f8c7 100644
>> --- a/arch/powerpc/platforms/powernv/setup.c
>> +++ b/arch/powerpc/platforms/powernv/setup.c
>> @@ -122,15 +122,6 @@ static void pnv_setup_security_mitigations(void)
>> type = L1D_FLUSH_ORI;
>> }
>>
>> -   /*
>> -* If we are non-Power9 bare metal, we don't need to flush on kernel
>> -* entry or after user access: they fix a P9 specific vulnerability.
>> -*/
>> -   if (!pvr_version_is(PVR_POWER9)) {
>> -   security_ftr_clear(SEC_FTR_L1D_FLUSH_ENTRY);
>> -   security_ftr_clear(SEC_FTR_L1D_FLUSH_UACCESS);
>> -   }
>> -
>> enable = security_ftr_enabled(SEC_FTR_FAVOUR_SECURITY) && \
>>  (security_ftr_enabled(SEC_FTR_L1D_FLUSH_PR)   || \
>>   security_ftr_enabled(SEC_FTR_L1D_FLUSH_HV));
>> --
>> 2.23.0
>>
> 


Re: [PATCH 4/4] powerpc/powernv: Remove POWER9 PVR version check for entry and uaccess flushes

2021-05-03 Thread Joel Stanley
On Mon, 3 May 2021 at 13:04, Nicholas Piggin  wrote:
>
> These aren't necessarily POWER9 only, and it's not to say some new
> vulnerability may not get discovered on other processors for which
> we would like the flexibility of having the workaround enabled by
> firmware.
>
> Remove the restriction that they only apply to POWER9.

I was wondering how these worked which led me to reviewing your patch.
>From what I could see, these are enabled by default (SEC_FTR_DEFAULT
in arch/powerpc/include/asm/security_features.h), so unless all
non-POWER9 machines have set the "please don't" bit in their firmware
this patch will enable the feature for those machines. Is that what
you wanted?

>
> Signed-off-by: Nicholas Piggin 
> ---
>  arch/powerpc/platforms/powernv/setup.c | 9 -
>  1 file changed, 9 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/setup.c 
> b/arch/powerpc/platforms/powernv/setup.c
> index a8db3f153063..6ec67223f8c7 100644
> --- a/arch/powerpc/platforms/powernv/setup.c
> +++ b/arch/powerpc/platforms/powernv/setup.c
> @@ -122,15 +122,6 @@ static void pnv_setup_security_mitigations(void)
> type = L1D_FLUSH_ORI;
> }
>
> -   /*
> -* If we are non-Power9 bare metal, we don't need to flush on kernel
> -* entry or after user access: they fix a P9 specific vulnerability.
> -*/
> -   if (!pvr_version_is(PVR_POWER9)) {
> -   security_ftr_clear(SEC_FTR_L1D_FLUSH_ENTRY);
> -   security_ftr_clear(SEC_FTR_L1D_FLUSH_UACCESS);
> -   }
> -
> enable = security_ftr_enabled(SEC_FTR_FAVOUR_SECURITY) && \
>  (security_ftr_enabled(SEC_FTR_L1D_FLUSH_PR)   || \
>   security_ftr_enabled(SEC_FTR_L1D_FLUSH_HV));
> --
> 2.23.0
>


[PATCH 4/4] powerpc/powernv: Remove POWER9 PVR version check for entry and uaccess flushes

2021-05-03 Thread Nicholas Piggin
These aren't necessarily POWER9 only, and it's not to say some new
vulnerability may not get discovered on other processors for which
we would like the flexibility of having the workaround enabled by
firmware.

Remove the restriction that they only apply to POWER9.

Signed-off-by: Nicholas Piggin 
---
 arch/powerpc/platforms/powernv/setup.c | 9 -
 1 file changed, 9 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/setup.c 
b/arch/powerpc/platforms/powernv/setup.c
index a8db3f153063..6ec67223f8c7 100644
--- a/arch/powerpc/platforms/powernv/setup.c
+++ b/arch/powerpc/platforms/powernv/setup.c
@@ -122,15 +122,6 @@ static void pnv_setup_security_mitigations(void)
type = L1D_FLUSH_ORI;
}
 
-   /*
-* If we are non-Power9 bare metal, we don't need to flush on kernel
-* entry or after user access: they fix a P9 specific vulnerability.
-*/
-   if (!pvr_version_is(PVR_POWER9)) {
-   security_ftr_clear(SEC_FTR_L1D_FLUSH_ENTRY);
-   security_ftr_clear(SEC_FTR_L1D_FLUSH_UACCESS);
-   }
-
enable = security_ftr_enabled(SEC_FTR_FAVOUR_SECURITY) && \
 (security_ftr_enabled(SEC_FTR_L1D_FLUSH_PR)   || \
  security_ftr_enabled(SEC_FTR_L1D_FLUSH_HV));
-- 
2.23.0