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

Mark Brown posted 28 patches 7 months, 2 weeks ago
There is a newer version of this series
[PATCH v6 18/28] KVM: arm64: Support SME priority registers
Posted by Mark Brown 7 months, 2 weeks ago
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 */
 
 	/* 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 */
 	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),
 	EL2_REG_FILTERED(SMCR_EL2, access_smcr_el2, reset_val, 0,
 			 sme_el2_visibility),
 
@@ -5306,6 +5324,9 @@ void kvm_calculate_traps(struct kvm_vcpu *vcpu)
 	compute_fgu(kvm, HFGITR2_GROUP);
 	compute_fgu(kvm, HDFGRTR2_GROUP);
 
+	if (kvm_has_feat(kvm, ID_AA64PFR1_EL1, SME, IMP))
+		kvm->arch.fgt[HFGWTR_GROUP] |= HFGWTR_EL2_nSMPRI_EL1_MASK;
+
 	set_bit(KVM_ARCH_FLAG_FGU_INITIALIZED, &kvm->arch.flags);
 out:
 	mutex_unlock(&kvm->arch.config_lock);

-- 
2.39.5
Re: [PATCH v6 18/28] KVM: arm64: Support SME priority registers
Posted by Marc Zyngier 7 months, 1 week ago
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.
Re: [PATCH v6 18/28] KVM: arm64: Support SME priority registers
Posted by Mark Brown 7 months, 1 week ago
On Sun, Jun 29, 2025 at 10:32:23AM +0100, Marc Zyngier wrote:
> Mark Brown <broonie@kernel.org> wrote:

> > +++ 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?

This was patterned of what you'd done with SVCR, I'd formed the
impression that the idea was that for registers that really exist like
this one it was more robust and less code to set them up in the sysreg
file and have everything look standard than do custom handling.  That
case was a bit different as the arch FP code needs a variable to point
at, I'll remove this and synthesise instead.

> > +	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 see, so the callback should be NULL?  Access to the register will get
trapped as part of the general trapping of EL2 access by a NV hypervisor
so it wasn't clear to me that that we shouldn't have the handling.

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

Me either now that you point it out, thanks.