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

["Murilo Opsfelder Araújo" <mopsfelder@gmail.com>]

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