On Wed, 25 Jun 2025 11:48:09 +0100,
Mark Brown <broo...@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 <broo...@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.

Reply via email to