[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <86o6u6c2qg.wl-maz@kernel.org>
Date: Sun, 29 Jun 2025 10:32:23 +0100
From: Marc Zyngier <maz@...nel.org>
To: Mark Brown <broonie@...nel.org>
Cc: Oliver Upton <oliver.upton@...ux.dev>,
Joey Gouly <joey.gouly@....com>,
Catalin Marinas <catalin.marinas@....com>,
Suzuki K Poulose <suzuki.poulose@....com>,
Will Deacon <will@...nel.org>,
Paolo Bonzini <pbonzini@...hat.com>,
Jonathan Corbet <corbet@....net>,
Shuah Khan <shuah@...nel.org>,
Dave Martin <Dave.Martin@....com>,
Fuad Tabba <tabba@...gle.com>,
Mark Rutland <mark.rutland@....com>,
linux-arm-kernel@...ts.infradead.org,
kvmarm@...ts.linux.dev,
linux-kernel@...r.kernel.org,
kvm@...r.kernel.org,
linux-doc@...r.kernel.org,
linux-kselftest@...r.kernel.org
Subject: Re: [PATCH v6 18/28] KVM: arm64: Support SME priority registers
On Wed, 25 Jun 2025 11:48:09 +0100,
Mark Brown <broonie@...nel.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@...nel.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.
Powered by blists - more mailing lists