[PATCH] powerpc/powernv: Get more flushing requirements from device-tree

[PATCH] powerpc/powernv: Get more flushing requirements from device-tree

[Russell Currey]

The device-tree properties no-need-l1d-flush-msr-pr-1-to-0,
no-need-l1d-flush-kernel-on-user-access and
no-need-store-drain-on-priv-state-switch are the equivalents of
H_CPU_BEHAV_NO_L1D_FLUSH_ENTRY, H_CPU_BEHAV_NO_L1D_FLUSH_UACCESS
and H_CPU_BEHAV_NO_STF_BARRIER from the H_GET_CPU_CHARACTERISTICS
hcall on pseries, respectively.

Since commit 84ed26fd00c5 ("powerpc/security: Add a security feature for
STF barrier") powernv systems with this device-tree property have been
enabling the STF barrier when they have no need for it.  This patch
fixes this by clearing the STF barrier feature on those systems.

In commit d02fa40d759f ("powerpc/powernv: Remove POWER9 PVR version
check for entry and uaccess flushes") the condition for disabling the
L1D flush on kernel entry and user access was changed from any non-P9
CPU to only checking P7 and P8.  Without the appropriate device-tree
checks for newer processors on powernv, these flushes are unnecessarily
enabled on those systems.  This patch fixes that too.

Reported-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Russell Currey <ruscur@russell.cc>
---
 arch/powerpc/platforms/powernv/setup.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
index 105d889abd51..824c3ad7a0fa 100644
--- a/arch/powerpc/platforms/powernv/setup.c
+++ b/arch/powerpc/platforms/powernv/setup.c
@@ -96,6 +96,15 @@ static void __init init_fw_feat_flags(struct device_node *np)
 
 	if (fw_feature_is("disabled", "needs-spec-barrier-for-bound-checks", np))
 		security_ftr_clear(SEC_FTR_BNDS_CHK_SPEC_BAR);
+
+	if (fw_feature_is("enabled", "no-need-l1d-flush-msr-pr-1-to-0", np))
+		security_ftr_clear(SEC_FTR_L1D_FLUSH_ENTRY);
+
+	if (fw_feature_is("enabled", "no-need-l1d-flush-kernel-on-user-access", np))
+		security_ftr_clear(SEC_FTR_L1D_FLUSH_UACCESS);
+
+	if (fw_feature_is("enabled", "no-need-store-drain-on-priv-state-switch", np))
+		security_ftr_clear(SEC_FTR_STF_BARRIER);
 }
 
 static void __init pnv_setup_security_mitigations(void)
-- 
2.35.1

Re: [PATCH] powerpc/powernv: Get more flushing requirements from device-tree

[Murilo Opsfelder Araújo]

Hi, Russell.

I think this patch could have been split in half with their corresponding Fixes: tag.

This may sound nitpicking but doing this would certainly help distros doing their backports.

More comments below.

On 3/22/22 04:47, Russell Currey wrote:
> The device-tree properties no-need-l1d-flush-msr-pr-1-to-0,
> no-need-l1d-flush-kernel-on-user-access and
> no-need-store-drain-on-priv-state-switch are the equivalents of
> H_CPU_BEHAV_NO_L1D_FLUSH_ENTRY, H_CPU_BEHAV_NO_L1D_FLUSH_UACCESS
> and H_CPU_BEHAV_NO_STF_BARRIER from the H_GET_CPU_CHARACTERISTICS
> hcall on pseries, respectively.
> 
> Since commit 84ed26fd00c5 ("powerpc/security: Add a security feature for
> STF barrier") powernv systems with this device-tree property have been
> enabling the STF barrier when they have no need for it.  This patch
> fixes this by clearing the STF barrier feature on those systems.
> 
> In commit d02fa40d759f ("powerpc/powernv: Remove POWER9 PVR version
> check for entry and uaccess flushes") the condition for disabling the
> L1D flush on kernel entry and user access was changed from any non-P9
> CPU to only checking P7 and P8.  Without the appropriate device-tree
> checks for newer processors on powernv, these flushes are unnecessarily
> enabled on those systems.  This patch fixes that too.
> 
> Reported-by: Joel Stanley <joel@jms.id.au>
> Signed-off-by: Russell Currey <ruscur@russell.cc>
> ---
>   arch/powerpc/platforms/powernv/setup.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
> index 105d889abd51..824c3ad7a0fa 100644
> --- a/arch/powerpc/platforms/powernv/setup.c
> +++ b/arch/powerpc/platforms/powernv/setup.c
> @@ -96,6 +96,15 @@ static void __init init_fw_feat_flags(struct device_node *np)
>   
>   	if (fw_feature_is("disabled", "needs-spec-barrier-for-bound-checks", np))
>   		security_ftr_clear(SEC_FTR_BNDS_CHK_SPEC_BAR);
> +
> +	if (fw_feature_is("enabled", "no-need-l1d-flush-msr-pr-1-to-0", np))
> +		security_ftr_clear(SEC_FTR_L1D_FLUSH_ENTRY);
> +
> +	if (fw_feature_is("enabled", "no-need-l1d-flush-kernel-on-user-access", np))
> +		security_ftr_clear(SEC_FTR_L1D_FLUSH_UACCESS);
> +

This first diff in one patch with:

Fixes: d02fa40d759f (powerpc/powernv: Remove POWER9 PVR version check for entry and uaccess flushes)

> +	if (fw_feature_is("enabled", "no-need-store-drain-on-priv-state-switch", np))
> +		security_ftr_clear(SEC_FTR_STF_BARRIER);

And this second diff in another one with:

Fixes: 84ed26fd00c5 (powerpc/security: Add a security feature for STF barrier)

And commit messages could be updated for both commits accordingly.

>   }
>   
>   static void __init pnv_setup_security_mitigations(void)

Cheers!

-- 
Murilo

Re: [PATCH] powerpc/powernv: Get more flushing requirements from device-tree

[Russell Currey]

On Wed, 2022-03-23 at 16:26 -0300, Murilo Opsfelder Araújo wrote:
> Hi, Russell.
> 
> I think this patch could have been split in half with their
> corresponding Fixes: tag.
> 
> This may sound nitpicking but doing this would certainly help distros
> doing their backports.

Hi Murilo,

I didn't use the Fixes: tag originally since as far as I'm aware this
issue doesn't impact any systems "out in the wild" - so I didn't think
there would be interest in any backports.  I should have split and
tagged the commits anyway though, in case others wanted to make that
decision.

Will resend.