| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CA+EHjTxaL7YwztugxyhS--948WUQ5BkME-P1cmrHvvJqEGEEBA@mail.gmail.com>
Date: Fri, 9 Jan 2026 18:01:00 +0000
From: Fuad Tabba <tabba@...gle.com>
To: Mark Brown <broonie@...nel.org>
Cc: Marc Zyngier <maz@...nel.org>, 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>, Oliver Upton <oupton@...nel.org>, Dave Martin <Dave.Martin@....com>,
Mark Rutland <mark.rutland@....com>, Ben Horgan <ben.horgan@....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, Peter Maydell <peter.maydell@...aro.org>,
Eric Auger <eric.auger@...hat.com>
Subject: Re: [PATCH v9 17/30] KVM: arm64: Support SME identification registers
for guests
.
On Tue, 23 Dec 2025 at 01:22, Mark Brown <broonie@...nel.org> wrote:
>
> The primary register for identifying SME is ID_AA64PFR1_EL1.SME. This
> is hidden from guests unless SME is enabled by the VMM.
> When it is visible it is writable and can be used to control the
> availability of SME2.
>
> There is also a new register ID_AA64SMFR0_EL1 which we make writable,
> forcing it to all bits 0 if SME is disabled. This includes the field
> SMEver giving the SME version, userspace is responsible for ensuring
> the value is consistent with ID_AA64PFR1_EL1.SME. It also includes
> FA64, a separately enableable extension which provides the full FPSIMD
> and SVE instruction set including FFR in streaming mode. Userspace can
> control the availability of FA64 by writing to this field. The other
> features enumerated there only add new instructions, there are no
> architectural controls for these.
>
> There is a further identification register SMIDR_EL1 which provides a
> basic description of the SME microarchitecture, in a manner similar to
> MIDR_EL1 for the PE. It also describes support for priority management
> and a basic affinity description for shared SME units, plus some RES0
> space. We do not support priority management for guests so this is
> hidden from guests, along with any new fields.
>
> As for MIDR_EL1 and REVIDR_EL1 we expose the implementer and revision
> information to guests with the raw value from the CPU we are running on,
> this may present issues for asymmetric systems or for migration as it
> does for the existing registers.
>
> Signed-off-by: Mark Brown <broonie@...nel.org>
> ---
> arch/arm64/include/asm/kvm_host.h | 3 +++
> arch/arm64/kvm/config.c | 8 +-----
> arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 11 ++++++++
> arch/arm64/kvm/hyp/nvhe/pkvm.c | 4 ++-
> arch/arm64/kvm/sys_regs.c | 40 +++++++++++++++++++++++++++---
> 5 files changed, 54 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 825b74f752d6..fead6988f47c 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -400,6 +400,7 @@ struct kvm_arch {
> u64 revidr_el1;
> u64 aidr_el1;
> u64 ctr_el0;
> + u64 smidr_el1;
>
> /* Masks for VNCR-backed and general EL2 sysregs */
> struct kvm_sysreg_masks *sysreg_masks;
> @@ -1543,6 +1544,8 @@ static inline u64 *__vm_id_reg(struct kvm_arch *ka, u32 reg)
> return &ka->revidr_el1;
> case SYS_AIDR_EL1:
> return &ka->aidr_el1;
> + case SYS_SMIDR_EL1:
> + return &ka->smidr_el1;
> default:
> WARN_ON_ONCE(1);
> return NULL;
> diff --git a/arch/arm64/kvm/config.c b/arch/arm64/kvm/config.c
> index 24bb3f36e9d5..7e26991b2df1 100644
> --- a/arch/arm64/kvm/config.c
> +++ b/arch/arm64/kvm/config.c
> @@ -274,14 +274,8 @@ static bool feat_anerr(struct kvm *kvm)
>
> static bool feat_sme_smps(struct kvm *kvm)
> {
> - /*
> - * Revists this if KVM ever supports SME -- this really should
> - * look at the guest's view of SMIDR_EL1. Funnily enough, this
> - * is not captured in the JSON file, but only as a note in the
> - * ARM ARM.
> - */
> return (kvm_has_feat(kvm, FEAT_SME) &&
> - (read_sysreg_s(SYS_SMIDR_EL1) & SMIDR_EL1_SMPS));
> + (kvm_read_vm_id_reg(kvm, SYS_SMIDR_EL1) & SMIDR_EL1_SMPS));
> }
>
> static bool feat_spe_fds(struct kvm *kvm)
> diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> index 8c3b3d6df99f..d921db152119 100644
> --- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> +++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> @@ -125,6 +125,17 @@ static inline u64 ctxt_midr_el1(struct kvm_cpu_context *ctxt)
> return kvm_read_vm_id_reg(kvm, SYS_MIDR_EL1);
> }
>
> +static inline u64 ctxt_smidr_el1(struct kvm_cpu_context *ctxt)
> +{
> + struct kvm *kvm = kern_hyp_va(ctxt_to_vcpu(ctxt)->kvm);
> +
> + if (!(ctxt_is_guest(ctxt) &&
> + test_bit(KVM_ARCH_FLAG_WRITABLE_IMP_ID_REGS, &kvm->arch.flags)))
> + return read_sysreg_s(SYS_SMIDR_EL1);
> +
> + return kvm_read_vm_id_reg(kvm, SYS_SMIDR_EL1);
> +}
AFAIKT, this isn't used anywhere in this series. Dead code from a
previous iteration?
> +
> static inline void __sysreg_save_common_state(struct kvm_cpu_context *ctxt)
> {
> *ctxt_mdscr_el1(ctxt) = read_sysreg(mdscr_el1);
> diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> index f4ec6695a6a5..b656449dff69 100644
> --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
> +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> @@ -351,8 +351,10 @@ static void pkvm_init_features_from_host(struct pkvm_hyp_vm *hyp_vm, const struc
> host_kvm->arch.vcpu_features,
> KVM_VCPU_MAX_FEATURES);
>
> - if (test_bit(KVM_ARCH_FLAG_WRITABLE_IMP_ID_REGS, &host_arch_flags))
> + if (test_bit(KVM_ARCH_FLAG_WRITABLE_IMP_ID_REGS, &host_arch_flags)) {
> hyp_vm->kvm.arch.midr_el1 = host_kvm->arch.midr_el1;
> + hyp_vm->kvm.arch.smidr_el1 = host_kvm->arch.smidr_el1;
> + }
>
> return;
> }
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 7e550f045f4d..a7ab02822023 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1893,6 +1893,10 @@ static unsigned int id_visibility(const struct kvm_vcpu *vcpu,
> if (!vcpu_has_sve(vcpu))
> return REG_RAZ;
> break;
> + case SYS_ID_AA64SMFR0_EL1:
> + if (!vcpu_has_sme(vcpu))
> + return REG_RAZ;
> + break;
> }
>
> return 0;
> @@ -1920,10 +1924,25 @@ static unsigned int raz_visibility(const struct kvm_vcpu *vcpu,
>
> /* cpufeature ID register access trap handlers */
>
> +static bool hidden_id_reg(struct kvm_vcpu *vcpu,
> + struct sys_reg_params *p,
> + const struct sys_reg_desc *r)
> +{
> + switch (reg_to_encoding(r)) {
> + case SYS_SMIDR_EL1:
> + return !vcpu_has_sme(vcpu);
> + default:
> + return false;
> + }
> +}
> +
> static bool access_id_reg(struct kvm_vcpu *vcpu,
> struct sys_reg_params *p,
> const struct sys_reg_desc *r)
> {
> + if (hidden_id_reg(vcpu, p, r))
> + return bad_trap(vcpu, p, r, "write to hidden ID register");
> +
The code in this file confuses me at times, that said, the
hidden_id_reg() check is added to access_id_reg(). However, SMIDR_EL1
is defined using IMPLEMENTATION_ID, which routes to
access_imp_id_reg(). This means the visibility check is bypassed for
SMIDR_EL1, and a guest without SME can still read the register.
> if (p->is_write)
> return write_to_read_only(vcpu, p, r);
>
> @@ -2012,7 +2031,9 @@ static u64 sanitise_id_aa64pfr1_el1(const struct kvm_vcpu *vcpu, u64 val)
> SYS_FIELD_GET(ID_AA64PFR0_EL1, RAS, pfr0) == ID_AA64PFR0_EL1_RAS_IMP))
> val &= ~ID_AA64PFR1_EL1_RAS_frac;
>
> - val &= ~ID_AA64PFR1_EL1_SME;
> + if (!kvm_has_sme(vcpu->kvm))
> + val &= ~ID_AA64PFR1_EL1_SME;
> +
> val &= ~ID_AA64PFR1_EL1_RNDR_trap;
> val &= ~ID_AA64PFR1_EL1_NMI;
> val &= ~ID_AA64PFR1_EL1_GCS;
> @@ -3038,6 +3059,9 @@ static bool access_imp_id_reg(struct kvm_vcpu *vcpu,
> case SYS_AIDR_EL1:
> p->regval = read_sysreg(aidr_el1);
> break;
> + case SYS_SMIDR_EL1:
> + p->regval = read_sysreg_s(SYS_SMIDR_EL1);
> + break;
In access_imp_id_reg(), we are returning the raw hardware value for
SYS_SMIDR_EL1. Since this register is not automatically sanitized by
the ID register infra, shouldn't we apply r->val here? Without this,
the bits we intended to hide leak to the guest.
I think this should apply to this function as a whole. For now, the
other two REVIDR_EL1 and AIDR_EL1 have a mask that covers the whole
register (so doing that preserves that behavior), but if we add more
registers similar to SMIDR_EL1, they also need to be sanitised.
> default:
> WARN_ON_ONCE(1);
> }
> @@ -3048,12 +3072,15 @@ static bool access_imp_id_reg(struct kvm_vcpu *vcpu,
> static u64 __ro_after_init boot_cpu_midr_val;
> static u64 __ro_after_init boot_cpu_revidr_val;
> static u64 __ro_after_init boot_cpu_aidr_val;
> +static u64 __ro_after_init boot_cpu_smidr_val;
>
> static void init_imp_id_regs(void)
> {
> boot_cpu_midr_val = read_sysreg(midr_el1);
> boot_cpu_revidr_val = read_sysreg(revidr_el1);
> boot_cpu_aidr_val = read_sysreg(aidr_el1);
> + if (system_supports_sme())
> + boot_cpu_smidr_val = read_sysreg_s(SYS_SMIDR_EL1);
> }
>
> static u64 reset_imp_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
> @@ -3065,6 +3092,8 @@ static u64 reset_imp_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
> return boot_cpu_revidr_val;
> case SYS_AIDR_EL1:
> return boot_cpu_aidr_val;
> + case SYS_SMIDR_EL1:
> + return boot_cpu_smidr_val;
Similarly, this should probably return boot_cpu_smidr_val & r->val.
Otherwise, the internal feat_sme_smps() check will report the feature
as present for the guest if the host supports it, creating an
inconsistency between KVM's internal view and the guest-visible ID
register.
Cheers,
/fuad
> default:
> KVM_BUG_ON(1, vcpu->kvm);
> return 0;
> @@ -3229,7 +3258,6 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> ID_AA64PFR1_EL1_MTE_frac |
> ID_AA64PFR1_EL1_NMI |
> ID_AA64PFR1_EL1_RNDR_trap |
> - ID_AA64PFR1_EL1_SME |
> ID_AA64PFR1_EL1_RES0 |
> ID_AA64PFR1_EL1_MPAM_frac |
> ID_AA64PFR1_EL1_MTE)),
> @@ -3239,7 +3267,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> ID_AA64PFR2_EL1_MTESTOREONLY),
> ID_UNALLOCATED(4,3),
> ID_WRITABLE(ID_AA64ZFR0_EL1, ~ID_AA64ZFR0_EL1_RES0),
> - ID_HIDDEN(ID_AA64SMFR0_EL1),
> + ID_WRITABLE(ID_AA64SMFR0_EL1, ~ID_AA64SMFR0_EL1_RES0),
> ID_UNALLOCATED(4,6),
> ID_WRITABLE(ID_AA64FPFR0_EL1, ~ID_AA64FPFR0_EL1_RES0),
>
> @@ -3446,7 +3474,11 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> { SYS_DESC(SYS_CLIDR_EL1), access_clidr, reset_clidr, CLIDR_EL1,
> .set_user = set_clidr, .val = ~CLIDR_EL1_RES0 },
> { SYS_DESC(SYS_CCSIDR2_EL1), undef_access },
> - { SYS_DESC(SYS_SMIDR_EL1), undef_access },
> + IMPLEMENTATION_ID(SMIDR_EL1, (SMIDR_EL1_NSMC | SMIDR_EL1_HIP |
> + SMIDR_EL1_AFFINITY2 |
> + SMIDR_EL1_IMPLEMENTER |
> + SMIDR_EL1_REVISION | SMIDR_EL1_SH |
> + SMIDR_EL1_AFFINITY)),
> IMPLEMENTATION_ID(AIDR_EL1, GENMASK_ULL(63, 0)),
> { SYS_DESC(SYS_CSSELR_EL1), access_csselr, reset_unknown, CSSELR_EL1 },
> ID_FILTERED(CTR_EL0, ctr_el0,
>
> --
> 2.47.3
>
Powered by blists - more mailing lists