Re: [PATCH v6 18/28] KVM: arm64: Support SME priority registers

[Marc Zyngier <maz@kernel.org>]

On Wed, 25 Jun 2025 11:48:09 +0100,
Mark Brown <broonie@kernel.org> wrote:
> 
> SME has optional support for configuring the relative priorities of PEs
> in systems where they share a single SME hardware block, known as a
> SMCU. Currently we do not have any support for this in Linux and will
> also hide it from KVM guests, pending experience with practical
> implementations. The interface for configuring priority support is via
> two new system registers, these registers are always defined when SME is
> available.
> 
> The register SMPRI_EL1 allows control of SME execution priorities. Since
> we disable SME priority support for guests this register is RES0, define
> it as such and enable fine grained traps for SMPRI_EL1 to ensure that
> guests can't write to it even if the hardware supports priorites.  Since
> the register should be readable with fixed contents we only trap writes,
> not reads.
> 
> There is also an EL2 register SMPRIMAP_EL2 for virtualisation of
> priorities, this is RES0 when priority configuration is not supported
> but has no specific traps available.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  arch/arm64/include/asm/kvm_host.h     |  2 ++
>  arch/arm64/include/asm/vncr_mapping.h |  1 +
>  arch/arm64/kvm/sys_regs.c             | 23 ++++++++++++++++++++++-
>  3 files changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 29b8697c8144..5ce9e06324b5 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -495,6 +495,7 @@ enum vcpu_sysreg {
>  	SVCR,
>  	FPMR,
>  	SMIDR_EL1,	/* Streaming Mode Identification Register */
> +	SMPRI_EL1,	/* Streaming Mode Priority Register */
>

What is the point of making the sysreg file larger for the sole
purpose of returning a value that is firmly always 0? Can't that be
synthesised on the fly whenever needed?

>  	/* 32bit specific registers. */
>  	DACR32_EL2,	/* Domain Access Control Register */
> @@ -547,6 +548,7 @@ enum vcpu_sysreg {
>  	VNCR(CPACR_EL1),/* Coprocessor Access Control */
>  	VNCR(ZCR_EL1),	/* SVE Control */
>  	VNCR(SMCR_EL1),	/* SME Control */
> +	VNCR(SMPRIMAP_EL2),	/* Streaming Mode Priority Mapping Register */

This is slightly different, as there is no trap for this, and we rely
on sanitisation.

>  	VNCR(TTBR0_EL1),/* Translation Table Base Register 0 */
>  	VNCR(TTBR1_EL1),/* Translation Table Base Register 1 */
>  	VNCR(TCR_EL1),	/* Translation Control Register */
> diff --git a/arch/arm64/include/asm/vncr_mapping.h b/arch/arm64/include/asm/vncr_mapping.h
> index aede5d6efad3..454e076b77cb 100644
> --- a/arch/arm64/include/asm/vncr_mapping.h
> +++ b/arch/arm64/include/asm/vncr_mapping.h
> @@ -45,6 +45,7 @@
>  #define VNCR_ZCR_EL1            0x1E0
>  #define VNCR_HAFGRTR_EL2	0x1E8
>  #define VNCR_SMCR_EL1		0x1F0
> +#define VNCR_SMPRIMAP_EL2	0x1F0
>  #define VNCR_TTBR0_EL1          0x200
>  #define VNCR_TTBR1_EL1          0x210
>  #define VNCR_FAR_EL1            0x220
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index b11bb95e9e35..1fee8e534615 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1828,6 +1828,15 @@ static unsigned int fp8_visibility(const struct kvm_vcpu *vcpu,
>  	return REG_HIDDEN;
>  }
>  
> +static unsigned int sme_raz_visibility(const struct kvm_vcpu *vcpu,
> +				       const struct sys_reg_desc *rd)
> +{
> +	if (vcpu_has_sme(vcpu))
> +		return REG_RAZ;
> +
> +	return REG_HIDDEN;
> +}
> +
>  static u64 sanitise_id_aa64pfr0_el1(const struct kvm_vcpu *vcpu, u64 val)
>  {
>  	if (!vcpu_has_sve(vcpu))
> @@ -3030,7 +3039,14 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>  
>  	{ SYS_DESC(SYS_ZCR_EL1), NULL, reset_val, ZCR_EL1, 0, .visibility = sve_visibility },
>  	{ SYS_DESC(SYS_TRFCR_EL1), undef_access },
> -	{ SYS_DESC(SYS_SMPRI_EL1), undef_access },
> +
> +	/*
> +	 * SMPRI_EL1 is UNDEF when SME is disabled, the UNDEF is
> +	 * handled via FGU which is handled without consulting this
> +	 * table.
> +	 */
> +	{ SYS_DESC(SYS_SMPRI_EL1), trap_raz_wi, .visibility = sme_raz_visibility },
> +
>  	{ SYS_DESC(SYS_SMCR_EL1), NULL, reset_val, SMCR_EL1, 0, .visibility = sme_visibility },
>  	{ SYS_DESC(SYS_TTBR0_EL1), access_vm_reg, reset_unknown, TTBR0_EL1 },
>  	{ SYS_DESC(SYS_TTBR1_EL1), access_vm_reg, reset_unknown, TTBR1_EL1 },
> @@ -3387,6 +3403,8 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>  
>  	EL2_REG_VNCR(HCRX_EL2, reset_val, 0),
>  
> +	EL2_REG_FILTERED(SMPRIMAP_EL2, trap_raz_wi, reset_val, 0,
> +			 sme_el2_visibility),

Wut??? You clearly said it yourself: this register "has no specific
traps available". If you end-up here from a guest access, this is a
bug. So this "trap_raz_wi" makes no sense.

I also cannot see where this register is properly configured to be
fully RES0, as it should.

	M.

-- 
Without deviation from the norm, progress is not possible.