On Wed, 25 Jun 2025 11:48:08 +0100,
Mark Brown <broonie@xxxxxxxxxx> 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 and affinity is not
> meaningful for guests so we mask out everything except for the
> microarchitecture description.
Both are extremely useful and meaningful to guests. You just have made
the choice not to expose this.
>
> 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.
MIDR/REVIDR are writable from userspace in order to alleviate this
problem. So should be SMIDR.
>
> Signed-off-by: Mark Brown <broonie@xxxxxxxxxx>
> ---
> arch/arm64/include/asm/kvm_host.h | 1 +
> arch/arm64/kvm/sys_regs.c | 46 +++++++++++++++++++++++++++++++++++----
> 2 files changed, 43 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index c26099f74648..29b8697c8144 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -494,6 +494,7 @@ enum vcpu_sysreg {
> /* FP/SIMD/SVE */
> SVCR,
> FPMR,
> + SMIDR_EL1, /* Streaming Mode Identification Register */
No. We have long made ID registers to be per-VM, not per-vcpu.
SMIDR_EL1 must have the same behaviour.
>
> /* 32bit specific registers. */
> DACR32_EL2, /* Domain Access Control Register */
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index caa90dae8184..b11bb95e9e35 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -774,6 +774,38 @@ static u64 reset_mpidr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
> return mpidr;
> }
>
> +static u64 reset_smidr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
> +{
> + u64 smidr = 0;
> +
> + if (!system_supports_sme())
> + return smidr;
> +
> + smidr = read_sysreg_s(SYS_SMIDR_EL1);
> +
> + /*
> + * Mask out everything except for the implementer and revison,
> + * in particular priority management is not implemented.
> + */
> + smidr &= SMIDR_EL1_IMPLEMENTER_MASK | SMIDR_EL1_REVISION_MASK;
> +
> + vcpu_write_sys_reg(vcpu, smidr, SMIDR_EL1);
> +
> + return smidr;
> +}
> +
> +static bool access_smidr(struct kvm_vcpu *vcpu,
> + struct sys_reg_params *p,
> + const struct sys_reg_desc *r)
> +{
> + if (p->is_write)
> + return write_to_read_only(vcpu, p, r);
> +
> + p->regval = vcpu_read_sys_reg(vcpu, r->reg);
> +
> + return true;
> +}
We already have 2 similar copies of this function. We're not adding a
third one.
> +
> static unsigned int pmu_visibility(const struct kvm_vcpu *vcpu,
> const struct sys_reg_desc *r)
> {
> @@ -1607,7 +1639,9 @@ static u64 __kvm_read_sanitised_id_reg(const struct kvm_vcpu *vcpu,
> val &= ~ARM64_FEATURE_MASK(ID_AA64PFR1_EL1_MTE_frac);
> }
>
> - val &= ~ARM64_FEATURE_MASK(ID_AA64PFR1_EL1_SME);
> + if (!vcpu_has_sme(vcpu))
> + val &= ~ARM64_FEATURE_MASK(ID_AA64PFR1_EL1_SME);
> +
> val &= ~ARM64_FEATURE_MASK(ID_AA64PFR1_EL1_RNDR_trap);
> val &= ~ARM64_FEATURE_MASK(ID_AA64PFR1_EL1_NMI);
> val &= ~ARM64_FEATURE_MASK(ID_AA64PFR1_EL1_GCS);
> @@ -1723,6 +1757,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;
> @@ -2905,7 +2943,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_RAS_frac |
> @@ -2913,7 +2950,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> ID_WRITABLE(ID_AA64PFR2_EL1, ID_AA64PFR2_EL1_FPMR),
> 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),
>
> @@ -3112,7 +3149,8 @@ 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 },
> + { SYS_DESC(SYS_SMIDR_EL1), .access = access_smidr, .reset = reset_smidr,
> + .reg = SMIDR_EL1, .visibility = sme_visibility },
> 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,
>
Please also handle the comment about FEAT_SME_SMPS in config.c.
M.
--
Without deviation from the norm, progress is not possible.
