From: "Jitindar Singh, Suraj" <surajjs@amazon.com>
To: "jingzhangos@google.com" <jingzhangos@google.com>,
"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
"kvmarm@lists.linux.dev" <kvmarm@lists.linux.dev>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"maz@kernel.org" <maz@kernel.org>,
"oupton@google.com" <oupton@google.com>
Cc: "james.morse@arm.com" <james.morse@arm.com>,
"suzuki.poulose@arm.com" <suzuki.poulose@arm.com>,
"rananta@google.com" <rananta@google.com>,
"tabba@google.com" <tabba@google.com>,
"pbonzini@redhat.com" <pbonzini@redhat.com>,
"alexandru.elisei@arm.com" <alexandru.elisei@arm.com>,
"will@kernel.org" <will@kernel.org>,
"reijiw@google.com" <reijiw@google.com>
Subject: Re: [PATCH v11 5/5] KVM: arm64: Refactor writings for PMUVer/CSV2/CSV3
Date: Fri, 2 Jun 2023 22:27:57 +0000 [thread overview]
Message-ID: <c1b97c4c97d44435032137e639790c75face4076.camel@amazon.com> (raw)
In-Reply-To: <CAAdAUtiw91APuLbr=OWB0w0eD5n=_4_X=K3xtO4kXZZhOcdDQA@mail.gmail.com>
On Fri, 2023-06-02 at 10:15 -0700, Jing Zhang wrote:
> Hi Suraj,
>
> Let's continue the problem here you raised in the v9.
> For the SVE example, the problem would be gone by enabling the
> writable for SVE field? Since we are going to enable the writable for
> all ID regs one by one later.
> The double checking for CSV[2|3] is gone in this series.
>
> Thanks,
> Jing
Hi Jing,
Correct. We need to in theory set the writable bit for all the fields
we're manipulating in kvm_arm_read_id_reg(). I think I also ran into
issues with the GIC bits.
I guess for per vcpu values this raises the question of what it means
to set them at a VM level (although I doubt anyone is setting these
heterogeneously in practise).
Thanks
- Suraj
>
> On Thu, Jun 1, 2023 at 5:51 PM Jing Zhang <jingzhangos@google.com>
> wrote:
> >
> > Refactor writings for ID_AA64PFR0_EL1.[CSV2|CSV3],
> > ID_AA64DFR0_EL1.PMUVer and ID_DFR0_ELF.PerfMon based on utilities
> > specific to ID register.
> >
> > Signed-off-by: Jing Zhang <jingzhangos@google.com>
> > ---
> > arch/arm64/include/asm/cpufeature.h | 1 +
> > arch/arm64/kernel/cpufeature.c | 2 +-
> > arch/arm64/kvm/sys_regs.c | 291 +++++++++++++++++++-----
> > ----
> > 3 files changed, 203 insertions(+), 91 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/cpufeature.h
> > b/arch/arm64/include/asm/cpufeature.h
> > index 6bf013fb110d..dc769c2eb7a4 100644
> > --- a/arch/arm64/include/asm/cpufeature.h
> > +++ b/arch/arm64/include/asm/cpufeature.h
> > @@ -915,6 +915,7 @@ static inline unsigned int get_vmid_bits(u64
> > mmfr1)
> > return 8;
> > }
> >
> > +s64 arm64_ftr_safe_value(const struct arm64_ftr_bits *ftrp, s64
> > new, s64 cur);
> > struct arm64_ftr_reg *get_arm64_ftr_reg(u32 sys_id);
> >
> > extern struct arm64_ftr_override id_aa64mmfr1_override;
> > diff --git a/arch/arm64/kernel/cpufeature.c
> > b/arch/arm64/kernel/cpufeature.c
> > index 7d7128c65161..3317a7b6deac 100644
> > --- a/arch/arm64/kernel/cpufeature.c
> > +++ b/arch/arm64/kernel/cpufeature.c
> > @@ -798,7 +798,7 @@ static u64 arm64_ftr_set_value(const struct
> > arm64_ftr_bits *ftrp, s64 reg,
> > return reg;
> > }
> >
> > -static s64 arm64_ftr_safe_value(const struct arm64_ftr_bits *ftrp,
> > s64 new,
> > +s64 arm64_ftr_safe_value(const struct arm64_ftr_bits *ftrp, s64
> > new,
> > s64 cur)
> > {
> > s64 ret = 0;
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index 1a534e0fc4ca..50d4e25f42d3 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -41,6 +41,7 @@
> > * 64bit interface.
> > */
> >
> > +static int set_id_reg(struct kvm_vcpu *vcpu, const struct
> > sys_reg_desc *rd, u64 val);
> > static u64 kvm_arm_read_id_reg(const struct kvm_vcpu *vcpu, u32
> > encoding);
> > static u64 sys_reg_to_index(const struct sys_reg_desc *reg);
> >
> > @@ -1194,6 +1195,86 @@ static bool access_arch_timer(struct
> > kvm_vcpu *vcpu,
> > return true;
> > }
> >
> > +static s64 kvm_arm64_ftr_safe_value(u32 id, const struct
> > arm64_ftr_bits *ftrp,
> > + s64 new, s64 cur)
> > +{
> > + struct arm64_ftr_bits kvm_ftr = *ftrp;
> > +
> > + /* Some features have different safe value type in KVM than
> > host features */
> > + switch (id) {
> > + case SYS_ID_AA64DFR0_EL1:
> > + if (kvm_ftr.shift == ID_AA64DFR0_EL1_PMUVer_SHIFT)
> > + kvm_ftr.type = FTR_LOWER_SAFE;
> > + break;
> > + case SYS_ID_DFR0_EL1:
> > + if (kvm_ftr.shift == ID_DFR0_EL1_PerfMon_SHIFT)
> > + kvm_ftr.type = FTR_LOWER_SAFE;
> > + break;
> > + }
> > +
> > + return arm64_ftr_safe_value(&kvm_ftr, new, cur);
> > +}
> > +
> > +/**
> > + * arm64_check_features() - Check if a feature register value
> > constitutes
> > + * a subset of features indicated by the idreg's KVM sanitised
> > limit.
> > + *
> > + * This function will check if each feature field of @val is the
> > "safe" value
> > + * against idreg's KVM sanitised limit return from reset()
> > callback.
> > + * If a field value in @val is the same as the one in limit, it is
> > always
> > + * considered the safe value regardless For register fields that
> > are not in
> > + * writable, only the value in limit is considered the safe value.
> > + *
> > + * Return: 0 if all the fields are safe. Otherwise, return
> > negative errno.
> > + */
> > +static int arm64_check_features(struct kvm_vcpu *vcpu,
> > + const struct sys_reg_desc *rd,
> > + u64 val)
> > +{
> > + const struct arm64_ftr_reg *ftr_reg;
> > + const struct arm64_ftr_bits *ftrp = NULL;
> > + u32 id = reg_to_encoding(rd);
> > + u64 writable_mask = rd->val;
> > + u64 limit = 0;
> > + u64 mask = 0;
> > +
> > + /* For hidden and unallocated idregs without reset, only
> > val = 0 is allowed. */
> > + if (rd->reset) {
> > + limit = rd->reset(vcpu, rd);
> > + ftr_reg = get_arm64_ftr_reg(id);
> > + if (!ftr_reg)
> > + return -EINVAL;
> > + ftrp = ftr_reg->ftr_bits;
> > + }
> > +
> > + for (; ftrp && ftrp->width; ftrp++) {
> > + s64 f_val, f_lim, safe_val;
> > + u64 ftr_mask;
> > +
> > + ftr_mask = arm64_ftr_mask(ftrp);
> > + if ((ftr_mask & writable_mask) != ftr_mask)
> > + continue;
> > +
> > + f_val = arm64_ftr_value(ftrp, val);
> > + f_lim = arm64_ftr_value(ftrp, limit);
> > + mask |= ftr_mask;
> > +
> > + if (f_val == f_lim)
> > + safe_val = f_val;
> > + else
> > + safe_val = kvm_arm64_ftr_safe_value(id,
> > ftrp, f_val, f_lim);
> > +
> > + if (safe_val != f_val)
> > + return -E2BIG;
> > + }
> > +
> > + /* For fields that are not writable, values in limit are
> > the safe values. */
> > + if ((val & ~mask) != (limit & ~mask))
> > + return -E2BIG;
> > +
> > + return 0;
> > +}
> > +
> > static u8 vcpu_pmuver(const struct kvm_vcpu *vcpu)
> > {
> > if (kvm_vcpu_has_pmu(vcpu))
> > @@ -1231,9 +1312,17 @@ static u8 pmuver_to_perfmon(u8 pmuver)
> > }
> > }
> >
> > -static void pmuver_update(struct kvm_vcpu *vcpu, u8 pmuver, bool
> > valid_pmu)
> > +static int pmuver_update(struct kvm_vcpu *vcpu,
> > + const struct sys_reg_desc *rd,
> > + u64 val,
> > + u8 pmuver,
> > + bool valid_pmu)
> > {
> > - u64 val;
> > + int ret;
> > +
> > + ret = set_id_reg(vcpu, rd, val);
> > + if (ret)
> > + return ret;
> >
> > if (valid_pmu) {
> > val = IDREG(vcpu->kvm, SYS_ID_AA64DFR0_EL1);
> > @@ -1249,6 +1338,8 @@ static void pmuver_update(struct kvm_vcpu
> > *vcpu, u8 pmuver, bool valid_pmu)
> > assign_bit(KVM_ARCH_FLAG_VCPU_HAS_IMP_DEF_PMU,
> > &vcpu->kvm->arch.flags,
> > pmuver ==
> > ID_AA64DFR0_EL1_PMUVer_IMP_DEF);
> > }
> > +
> > + return 0;
> > }
> >
> > static u64 general_read_kvm_sanitised_reg(struct kvm_vcpu *vcpu,
> > const struct sys_reg_desc *rd)
> > @@ -1264,7 +1355,6 @@ static u64 kvm_arm_read_id_reg(const struct
> > kvm_vcpu *vcpu, u32 encoding)
> > case SYS_ID_AA64PFR0_EL1:
> > if (!vcpu_has_sve(vcpu))
> > val &=
> > ~ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_SVE);
> > - val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_AMU);
> > if (kvm_vgic_global_state.type == VGIC_V3) {
> > val &=
> > ~ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_GIC);
> > val |=
> > FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_GIC), 1);
> > @@ -1291,15 +1381,10 @@ static u64 kvm_arm_read_id_reg(const struct
> > kvm_vcpu *vcpu, u32 encoding)
> > val &=
> > ~ARM64_FEATURE_MASK(ID_AA64ISAR2_EL1_WFxT);
> > break;
> > case SYS_ID_AA64DFR0_EL1:
> > - /* Limit debug to ARMv8.0 */
> > - val &=
> > ~ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_DebugVer);
> > - val |=
> > FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_DebugVer), 6);
> > /* Set PMUver to the required version */
> > val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer);
> > val |=
> > FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer),
> > vcpu_pmuver(vcpu));
> > - /* Hide SPE from guests */
> > - val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMSVer);
> > break;
> > case SYS_ID_DFR0_EL1:
> > val &= ~ARM64_FEATURE_MASK(ID_DFR0_EL1_PerfMon);
> > @@ -1398,38 +1483,56 @@ static unsigned int sve_visibility(const
> > struct kvm_vcpu *vcpu,
> > return REG_HIDDEN;
> > }
> >
> > -static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
> > - const struct sys_reg_desc *rd,
> > - u64 val)
> > +static u64 read_sanitised_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
> > + const struct sys_reg_desc
> > *rd)
> > {
> > - u64 new_val = val;
> > - u8 csv2, csv3;
> > + u64 val;
> > + u32 id = reg_to_encoding(rd);
> >
> > + val = read_sanitised_ftr_reg(id);
> > /*
> > - * Allow AA64PFR0_EL1.CSV2 to be set from userspace as long
> > as
> > - * it doesn't promise more than what is actually provided
> > (the
> > - * guest could otherwise be covered in ectoplasmic
> > residue).
> > + * The default is to expose CSV2 == 1 if the HW isn't
> > affected.
> > + * Although this is a per-CPU feature, we make it global
> > because
> > + * asymmetric systems are just a nuisance.
> > + *
> > + * Userspace can override this as long as it doesn't
> > promise
> > + * the impossible.
> > */
> > - csv2 = cpuid_feature_extract_unsigned_field(val,
> > ID_AA64PFR0_EL1_CSV2_SHIFT);
> > - if (csv2 > 1 ||
> > - (csv2 && arm64_get_spectre_v2_state() !=
> > SPECTRE_UNAFFECTED))
> > - return -EINVAL;
> > + if (arm64_get_spectre_v2_state() == SPECTRE_UNAFFECTED) {
> > + val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV2);
> > + val |=
> > FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV2), 1);
> > + }
> > + if (arm64_get_meltdown_state() == SPECTRE_UNAFFECTED) {
> > + val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV3);
> > + val |=
> > FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV3), 1);
> > + }
> >
> > - /* Same thing for CSV3 */
> > - csv3 = cpuid_feature_extract_unsigned_field(val,
> > ID_AA64PFR0_EL1_CSV3_SHIFT);
> > - if (csv3 > 1 ||
> > - (csv3 && arm64_get_meltdown_state() !=
> > SPECTRE_UNAFFECTED))
> > - return -EINVAL;
> > + val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_AMU);
> >
> > - /* We can only differ with CSV[23], and anything else is an
> > error */
> > - val ^= read_id_reg(vcpu, rd);
> > - val &= ~(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV2) |
> > - ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV3));
> > - if (val)
> > - return -EINVAL;
> > + return val;
> > +}
> >
> > - IDREG(vcpu->kvm, reg_to_encoding(rd)) = new_val;
> > - return 0;
> > +static u64 read_sanitised_id_aa64dfr0_el1(struct kvm_vcpu *vcpu,
> > + const struct sys_reg_desc
> > *rd)
> > +{
> > + u64 val;
> > + u32 id = reg_to_encoding(rd);
> > +
> > + val = read_sanitised_ftr_reg(id);
> > + /* Limit debug to ARMv8.0 */
> > + val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_DebugVer);
> > + val |=
> > FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_DebugVer), 6);
> > + /*
> > + * Initialise the default PMUver before there is a chance
> > to
> > + * create an actual PMU.
> > + */
> > + val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer);
> > + val |=
> > FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer),
> > + kvm_arm_pmu_get_pmuver_limit());
> > + /* Hide SPE from guests */
> > + val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMSVer);
> > +
> > + return val;
> > }
> >
> > static int set_id_aa64dfr0_el1(struct kvm_vcpu *vcpu,
> > @@ -1457,14 +1560,35 @@ static int set_id_aa64dfr0_el1(struct
> > kvm_vcpu *vcpu,
> > if (kvm_vcpu_has_pmu(vcpu) != valid_pmu)
> > return -EINVAL;
> >
> > - /* We can only differ with PMUver, and anything else is an
> > error */
> > - val ^= read_id_reg(vcpu, rd);
> > - val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer);
> > - if (val)
> > - return -EINVAL;
> > + if (!valid_pmu) {
> > + /*
> > + * Ignore the PMUVer field in @val. The PMUVer
> > would be determined
> > + * by arch flags bit
> > KVM_ARCH_FLAG_VCPU_HAS_IMP_DEF_PMU,
> > + */
> > + pmuver = FIELD_GET(ID_AA64DFR0_EL1_PMUVer_MASK,
> > + IDREG(vcpu->kvm,
> > SYS_ID_AA64DFR0_EL1));
> > + val &= ~ID_AA64DFR0_EL1_PMUVer_MASK;
> > + val |= FIELD_PREP(ID_AA64DFR0_EL1_PMUVer_MASK,
> > pmuver);
> > + }
> >
> > - pmuver_update(vcpu, pmuver, valid_pmu);
> > - return 0;
> > + return pmuver_update(vcpu, rd, val, pmuver, valid_pmu);
> > +}
> > +
> > +static u64 read_sanitised_id_dfr0_el1(struct kvm_vcpu *vcpu,
> > + const struct sys_reg_desc
> > *rd)
> > +{
> > + u64 val;
> > + u32 id = reg_to_encoding(rd);
> > +
> > + val = read_sanitised_ftr_reg(id);
> > + /*
> > + * Initialise the default PMUver before there is a chance
> > to
> > + * create an actual PMU.
> > + */
> > + val &= ~ARM64_FEATURE_MASK(ID_DFR0_EL1_PerfMon);
> > + val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_DFR0_EL1_PerfMon),
> > kvm_arm_pmu_get_pmuver_limit());
> > +
> > + return val;
> > }
> >
> > static int set_id_dfr0_el1(struct kvm_vcpu *vcpu,
> > @@ -1493,14 +1617,18 @@ static int set_id_dfr0_el1(struct kvm_vcpu
> > *vcpu,
> > if (kvm_vcpu_has_pmu(vcpu) != valid_pmu)
> > return -EINVAL;
> >
> > - /* We can only differ with PerfMon, and anything else is an
> > error */
> > - val ^= read_id_reg(vcpu, rd);
> > - val &= ~ARM64_FEATURE_MASK(ID_DFR0_EL1_PerfMon);
> > - if (val)
> > - return -EINVAL;
> > + if (!valid_pmu) {
> > + /*
> > + * Ignore the PerfMon field in @val. The PerfMon
> > would be determined
> > + * by arch flags bit
> > KVM_ARCH_FLAG_VCPU_HAS_IMP_DEF_PMU,
> > + */
> > + perfmon = FIELD_GET(ID_DFR0_EL1_PerfMon_MASK,
> > + IDREG(vcpu->kvm,
> > SYS_ID_DFR0_EL1));
> > + val &= ~ID_DFR0_EL1_PerfMon_MASK;
> > + val |= FIELD_PREP(ID_DFR0_EL1_PerfMon_MASK,
> > perfmon);
> > + }
> >
> > - pmuver_update(vcpu, perfmon_to_pmuver(perfmon), valid_pmu);
> > - return 0;
> > + return pmuver_update(vcpu, rd, val,
> > perfmon_to_pmuver(perfmon), valid_pmu);
> > }
> >
> > /*
> > @@ -1520,11 +1648,14 @@ static int get_id_reg(struct kvm_vcpu
> > *vcpu, const struct sys_reg_desc *rd,
> > static int set_id_reg(struct kvm_vcpu *vcpu, const struct
> > sys_reg_desc *rd,
> > u64 val)
> > {
> > - /* This is what we mean by invariant: you can't change it.
> > */
> > - if (val != read_id_reg(vcpu, rd))
> > - return -EINVAL;
> > + u32 id = reg_to_encoding(rd);
> > + int ret = 0;
> >
> > - return 0;
> > + ret = arm64_check_features(vcpu, rd, val);
> > + if (!ret)
> > + IDREG(vcpu->kvm, id) = val;
> > +
> > + return ret;
> > }
> >
> > static int get_raz_reg(struct kvm_vcpu *vcpu, const struct
> > sys_reg_desc *rd,
> > @@ -1875,9 +2006,13 @@ static const struct sys_reg_desc
> > sys_reg_descs[] = {
> > /* CRm=1 */
> > AA32_ID_SANITISED(ID_PFR0_EL1),
> > AA32_ID_SANITISED(ID_PFR1_EL1),
> > - { SYS_DESC(SYS_ID_DFR0_EL1), .access = access_id_reg,
> > - .get_user = get_id_reg, .set_user = set_id_dfr0_el1,
> > - .visibility = aa32_id_visibility, },
> > + { SYS_DESC(SYS_ID_DFR0_EL1),
> > + .access = access_id_reg,
> > + .get_user = get_id_reg,
> > + .set_user = set_id_dfr0_el1,
> > + .visibility = aa32_id_visibility,
> > + .reset = read_sanitised_id_dfr0_el1,
> > + .val = ID_DFR0_EL1_PerfMon_MASK, },
> > ID_HIDDEN(ID_AFR0_EL1),
> > AA32_ID_SANITISED(ID_MMFR0_EL1),
> > AA32_ID_SANITISED(ID_MMFR1_EL1),
> > @@ -1906,8 +2041,12 @@ static const struct sys_reg_desc
> > sys_reg_descs[] = {
> >
> > /* AArch64 ID registers */
> > /* CRm=4 */
> > - { SYS_DESC(SYS_ID_AA64PFR0_EL1), .access = access_id_reg,
> > - .get_user = get_id_reg, .set_user = set_id_aa64pfr0_el1,
> > },
> > + { SYS_DESC(SYS_ID_AA64PFR0_EL1),
> > + .access = access_id_reg,
> > + .get_user = get_id_reg,
> > + .set_user = set_id_reg,
> > + .reset = read_sanitised_id_aa64pfr0_el1,
> > + .val = ID_AA64PFR0_EL1_CSV2_MASK |
> > ID_AA64PFR0_EL1_CSV3_MASK, },
> > ID_SANITISED(ID_AA64PFR1_EL1),
> > ID_UNALLOCATED(4,2),
> > ID_UNALLOCATED(4,3),
> > @@ -1917,8 +2056,12 @@ static const struct sys_reg_desc
> > sys_reg_descs[] = {
> > ID_UNALLOCATED(4,7),
> >
> > /* CRm=5 */
> > - { SYS_DESC(SYS_ID_AA64DFR0_EL1), .access = access_id_reg,
> > - .get_user = get_id_reg, .set_user = set_id_aa64dfr0_el1,
> > },
> > + { SYS_DESC(SYS_ID_AA64DFR0_EL1),
> > + .access = access_id_reg,
> > + .get_user = get_id_reg,
> > + .set_user = set_id_aa64dfr0_el1,
> > + .reset = read_sanitised_id_aa64dfr0_el1,
> > + .val = ID_AA64DFR0_EL1_PMUVer_MASK, },
> > ID_SANITISED(ID_AA64DFR1_EL1),
> > ID_UNALLOCATED(5,2),
> > ID_UNALLOCATED(5,3),
> > @@ -3454,38 +3597,6 @@ void kvm_arm_init_id_regs(struct kvm *kvm)
> > idreg++;
> > id = reg_to_encoding(idreg);
> > }
> > -
> > - /*
> > - * The default is to expose CSV2 == 1 if the HW isn't
> > affected.
> > - * Although this is a per-CPU feature, we make it global
> > because
> > - * asymmetric systems are just a nuisance.
> > - *
> > - * Userspace can override this as long as it doesn't
> > promise
> > - * the impossible.
> > - */
> > - val = IDREG(kvm, SYS_ID_AA64PFR0_EL1);
> > -
> > - if (arm64_get_spectre_v2_state() == SPECTRE_UNAFFECTED) {
> > - val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV2);
> > - val |=
> > FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV2), 1);
> > - }
> > - if (arm64_get_meltdown_state() == SPECTRE_UNAFFECTED) {
> > - val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV3);
> > - val |=
> > FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV3), 1);
> > - }
> > -
> > - IDREG(kvm, SYS_ID_AA64PFR0_EL1) = val;
> > - /*
> > - * Initialise the default PMUver before there is a chance
> > to
> > - * create an actual PMU.
> > - */
> > - val = IDREG(kvm, SYS_ID_AA64DFR0_EL1);
> > -
> > - val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer);
> > - val |=
> > FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer),
> > - kvm_arm_pmu_get_pmuver_limit());
> > -
> > - IDREG(kvm, SYS_ID_AA64DFR0_EL1) = val;
> > }
> >
> > int __init kvm_sys_reg_table_init(void)
> > --
> > 2.41.0.rc0.172.g3f132b7071-goog
> >
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: "Jitindar Singh, Suraj" <surajjs@amazon.com>
To: "jingzhangos@google.com" <jingzhangos@google.com>,
"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
"kvmarm@lists.linux.dev" <kvmarm@lists.linux.dev>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"maz@kernel.org" <maz@kernel.org>,
"oupton@google.com" <oupton@google.com>
Cc: "james.morse@arm.com" <james.morse@arm.com>,
"suzuki.poulose@arm.com" <suzuki.poulose@arm.com>,
"rananta@google.com" <rananta@google.com>,
"tabba@google.com" <tabba@google.com>,
"pbonzini@redhat.com" <pbonzini@redhat.com>,
"alexandru.elisei@arm.com" <alexandru.elisei@arm.com>,
"will@kernel.org" <will@kernel.org>,
"reijiw@google.com" <reijiw@google.com>
Subject: Re: [PATCH v11 5/5] KVM: arm64: Refactor writings for PMUVer/CSV2/CSV3
Date: Fri, 2 Jun 2023 22:27:57 +0000 [thread overview]
Message-ID: <c1b97c4c97d44435032137e639790c75face4076.camel@amazon.com> (raw)
In-Reply-To: <CAAdAUtiw91APuLbr=OWB0w0eD5n=_4_X=K3xtO4kXZZhOcdDQA@mail.gmail.com>
On Fri, 2023-06-02 at 10:15 -0700, Jing Zhang wrote:
> Hi Suraj,
>
> Let's continue the problem here you raised in the v9.
> For the SVE example, the problem would be gone by enabling the
> writable for SVE field? Since we are going to enable the writable for
> all ID regs one by one later.
> The double checking for CSV[2|3] is gone in this series.
>
> Thanks,
> Jing
Hi Jing,
Correct. We need to in theory set the writable bit for all the fields
we're manipulating in kvm_arm_read_id_reg(). I think I also ran into
issues with the GIC bits.
I guess for per vcpu values this raises the question of what it means
to set them at a VM level (although I doubt anyone is setting these
heterogeneously in practise).
Thanks
- Suraj
>
> On Thu, Jun 1, 2023 at 5:51 PM Jing Zhang <jingzhangos@google.com>
> wrote:
> >
> > Refactor writings for ID_AA64PFR0_EL1.[CSV2|CSV3],
> > ID_AA64DFR0_EL1.PMUVer and ID_DFR0_ELF.PerfMon based on utilities
> > specific to ID register.
> >
> > Signed-off-by: Jing Zhang <jingzhangos@google.com>
> > ---
> > arch/arm64/include/asm/cpufeature.h | 1 +
> > arch/arm64/kernel/cpufeature.c | 2 +-
> > arch/arm64/kvm/sys_regs.c | 291 +++++++++++++++++++-----
> > ----
> > 3 files changed, 203 insertions(+), 91 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/cpufeature.h
> > b/arch/arm64/include/asm/cpufeature.h
> > index 6bf013fb110d..dc769c2eb7a4 100644
> > --- a/arch/arm64/include/asm/cpufeature.h
> > +++ b/arch/arm64/include/asm/cpufeature.h
> > @@ -915,6 +915,7 @@ static inline unsigned int get_vmid_bits(u64
> > mmfr1)
> > return 8;
> > }
> >
> > +s64 arm64_ftr_safe_value(const struct arm64_ftr_bits *ftrp, s64
> > new, s64 cur);
> > struct arm64_ftr_reg *get_arm64_ftr_reg(u32 sys_id);
> >
> > extern struct arm64_ftr_override id_aa64mmfr1_override;
> > diff --git a/arch/arm64/kernel/cpufeature.c
> > b/arch/arm64/kernel/cpufeature.c
> > index 7d7128c65161..3317a7b6deac 100644
> > --- a/arch/arm64/kernel/cpufeature.c
> > +++ b/arch/arm64/kernel/cpufeature.c
> > @@ -798,7 +798,7 @@ static u64 arm64_ftr_set_value(const struct
> > arm64_ftr_bits *ftrp, s64 reg,
> > return reg;
> > }
> >
> > -static s64 arm64_ftr_safe_value(const struct arm64_ftr_bits *ftrp,
> > s64 new,
> > +s64 arm64_ftr_safe_value(const struct arm64_ftr_bits *ftrp, s64
> > new,
> > s64 cur)
> > {
> > s64 ret = 0;
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index 1a534e0fc4ca..50d4e25f42d3 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -41,6 +41,7 @@
> > * 64bit interface.
> > */
> >
> > +static int set_id_reg(struct kvm_vcpu *vcpu, const struct
> > sys_reg_desc *rd, u64 val);
> > static u64 kvm_arm_read_id_reg(const struct kvm_vcpu *vcpu, u32
> > encoding);
> > static u64 sys_reg_to_index(const struct sys_reg_desc *reg);
> >
> > @@ -1194,6 +1195,86 @@ static bool access_arch_timer(struct
> > kvm_vcpu *vcpu,
> > return true;
> > }
> >
> > +static s64 kvm_arm64_ftr_safe_value(u32 id, const struct
> > arm64_ftr_bits *ftrp,
> > + s64 new, s64 cur)
> > +{
> > + struct arm64_ftr_bits kvm_ftr = *ftrp;
> > +
> > + /* Some features have different safe value type in KVM than
> > host features */
> > + switch (id) {
> > + case SYS_ID_AA64DFR0_EL1:
> > + if (kvm_ftr.shift == ID_AA64DFR0_EL1_PMUVer_SHIFT)
> > + kvm_ftr.type = FTR_LOWER_SAFE;
> > + break;
> > + case SYS_ID_DFR0_EL1:
> > + if (kvm_ftr.shift == ID_DFR0_EL1_PerfMon_SHIFT)
> > + kvm_ftr.type = FTR_LOWER_SAFE;
> > + break;
> > + }
> > +
> > + return arm64_ftr_safe_value(&kvm_ftr, new, cur);
> > +}
> > +
> > +/**
> > + * arm64_check_features() - Check if a feature register value
> > constitutes
> > + * a subset of features indicated by the idreg's KVM sanitised
> > limit.
> > + *
> > + * This function will check if each feature field of @val is the
> > "safe" value
> > + * against idreg's KVM sanitised limit return from reset()
> > callback.
> > + * If a field value in @val is the same as the one in limit, it is
> > always
> > + * considered the safe value regardless For register fields that
> > are not in
> > + * writable, only the value in limit is considered the safe value.
> > + *
> > + * Return: 0 if all the fields are safe. Otherwise, return
> > negative errno.
> > + */
> > +static int arm64_check_features(struct kvm_vcpu *vcpu,
> > + const struct sys_reg_desc *rd,
> > + u64 val)
> > +{
> > + const struct arm64_ftr_reg *ftr_reg;
> > + const struct arm64_ftr_bits *ftrp = NULL;
> > + u32 id = reg_to_encoding(rd);
> > + u64 writable_mask = rd->val;
> > + u64 limit = 0;
> > + u64 mask = 0;
> > +
> > + /* For hidden and unallocated idregs without reset, only
> > val = 0 is allowed. */
> > + if (rd->reset) {
> > + limit = rd->reset(vcpu, rd);
> > + ftr_reg = get_arm64_ftr_reg(id);
> > + if (!ftr_reg)
> > + return -EINVAL;
> > + ftrp = ftr_reg->ftr_bits;
> > + }
> > +
> > + for (; ftrp && ftrp->width; ftrp++) {
> > + s64 f_val, f_lim, safe_val;
> > + u64 ftr_mask;
> > +
> > + ftr_mask = arm64_ftr_mask(ftrp);
> > + if ((ftr_mask & writable_mask) != ftr_mask)
> > + continue;
> > +
> > + f_val = arm64_ftr_value(ftrp, val);
> > + f_lim = arm64_ftr_value(ftrp, limit);
> > + mask |= ftr_mask;
> > +
> > + if (f_val == f_lim)
> > + safe_val = f_val;
> > + else
> > + safe_val = kvm_arm64_ftr_safe_value(id,
> > ftrp, f_val, f_lim);
> > +
> > + if (safe_val != f_val)
> > + return -E2BIG;
> > + }
> > +
> > + /* For fields that are not writable, values in limit are
> > the safe values. */
> > + if ((val & ~mask) != (limit & ~mask))
> > + return -E2BIG;
> > +
> > + return 0;
> > +}
> > +
> > static u8 vcpu_pmuver(const struct kvm_vcpu *vcpu)
> > {
> > if (kvm_vcpu_has_pmu(vcpu))
> > @@ -1231,9 +1312,17 @@ static u8 pmuver_to_perfmon(u8 pmuver)
> > }
> > }
> >
> > -static void pmuver_update(struct kvm_vcpu *vcpu, u8 pmuver, bool
> > valid_pmu)
> > +static int pmuver_update(struct kvm_vcpu *vcpu,
> > + const struct sys_reg_desc *rd,
> > + u64 val,
> > + u8 pmuver,
> > + bool valid_pmu)
> > {
> > - u64 val;
> > + int ret;
> > +
> > + ret = set_id_reg(vcpu, rd, val);
> > + if (ret)
> > + return ret;
> >
> > if (valid_pmu) {
> > val = IDREG(vcpu->kvm, SYS_ID_AA64DFR0_EL1);
> > @@ -1249,6 +1338,8 @@ static void pmuver_update(struct kvm_vcpu
> > *vcpu, u8 pmuver, bool valid_pmu)
> > assign_bit(KVM_ARCH_FLAG_VCPU_HAS_IMP_DEF_PMU,
> > &vcpu->kvm->arch.flags,
> > pmuver ==
> > ID_AA64DFR0_EL1_PMUVer_IMP_DEF);
> > }
> > +
> > + return 0;
> > }
> >
> > static u64 general_read_kvm_sanitised_reg(struct kvm_vcpu *vcpu,
> > const struct sys_reg_desc *rd)
> > @@ -1264,7 +1355,6 @@ static u64 kvm_arm_read_id_reg(const struct
> > kvm_vcpu *vcpu, u32 encoding)
> > case SYS_ID_AA64PFR0_EL1:
> > if (!vcpu_has_sve(vcpu))
> > val &=
> > ~ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_SVE);
> > - val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_AMU);
> > if (kvm_vgic_global_state.type == VGIC_V3) {
> > val &=
> > ~ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_GIC);
> > val |=
> > FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_GIC), 1);
> > @@ -1291,15 +1381,10 @@ static u64 kvm_arm_read_id_reg(const struct
> > kvm_vcpu *vcpu, u32 encoding)
> > val &=
> > ~ARM64_FEATURE_MASK(ID_AA64ISAR2_EL1_WFxT);
> > break;
> > case SYS_ID_AA64DFR0_EL1:
> > - /* Limit debug to ARMv8.0 */
> > - val &=
> > ~ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_DebugVer);
> > - val |=
> > FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_DebugVer), 6);
> > /* Set PMUver to the required version */
> > val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer);
> > val |=
> > FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer),
> > vcpu_pmuver(vcpu));
> > - /* Hide SPE from guests */
> > - val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMSVer);
> > break;
> > case SYS_ID_DFR0_EL1:
> > val &= ~ARM64_FEATURE_MASK(ID_DFR0_EL1_PerfMon);
> > @@ -1398,38 +1483,56 @@ static unsigned int sve_visibility(const
> > struct kvm_vcpu *vcpu,
> > return REG_HIDDEN;
> > }
> >
> > -static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
> > - const struct sys_reg_desc *rd,
> > - u64 val)
> > +static u64 read_sanitised_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
> > + const struct sys_reg_desc
> > *rd)
> > {
> > - u64 new_val = val;
> > - u8 csv2, csv3;
> > + u64 val;
> > + u32 id = reg_to_encoding(rd);
> >
> > + val = read_sanitised_ftr_reg(id);
> > /*
> > - * Allow AA64PFR0_EL1.CSV2 to be set from userspace as long
> > as
> > - * it doesn't promise more than what is actually provided
> > (the
> > - * guest could otherwise be covered in ectoplasmic
> > residue).
> > + * The default is to expose CSV2 == 1 if the HW isn't
> > affected.
> > + * Although this is a per-CPU feature, we make it global
> > because
> > + * asymmetric systems are just a nuisance.
> > + *
> > + * Userspace can override this as long as it doesn't
> > promise
> > + * the impossible.
> > */
> > - csv2 = cpuid_feature_extract_unsigned_field(val,
> > ID_AA64PFR0_EL1_CSV2_SHIFT);
> > - if (csv2 > 1 ||
> > - (csv2 && arm64_get_spectre_v2_state() !=
> > SPECTRE_UNAFFECTED))
> > - return -EINVAL;
> > + if (arm64_get_spectre_v2_state() == SPECTRE_UNAFFECTED) {
> > + val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV2);
> > + val |=
> > FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV2), 1);
> > + }
> > + if (arm64_get_meltdown_state() == SPECTRE_UNAFFECTED) {
> > + val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV3);
> > + val |=
> > FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV3), 1);
> > + }
> >
> > - /* Same thing for CSV3 */
> > - csv3 = cpuid_feature_extract_unsigned_field(val,
> > ID_AA64PFR0_EL1_CSV3_SHIFT);
> > - if (csv3 > 1 ||
> > - (csv3 && arm64_get_meltdown_state() !=
> > SPECTRE_UNAFFECTED))
> > - return -EINVAL;
> > + val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_AMU);
> >
> > - /* We can only differ with CSV[23], and anything else is an
> > error */
> > - val ^= read_id_reg(vcpu, rd);
> > - val &= ~(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV2) |
> > - ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV3));
> > - if (val)
> > - return -EINVAL;
> > + return val;
> > +}
> >
> > - IDREG(vcpu->kvm, reg_to_encoding(rd)) = new_val;
> > - return 0;
> > +static u64 read_sanitised_id_aa64dfr0_el1(struct kvm_vcpu *vcpu,
> > + const struct sys_reg_desc
> > *rd)
> > +{
> > + u64 val;
> > + u32 id = reg_to_encoding(rd);
> > +
> > + val = read_sanitised_ftr_reg(id);
> > + /* Limit debug to ARMv8.0 */
> > + val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_DebugVer);
> > + val |=
> > FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_DebugVer), 6);
> > + /*
> > + * Initialise the default PMUver before there is a chance
> > to
> > + * create an actual PMU.
> > + */
> > + val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer);
> > + val |=
> > FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer),
> > + kvm_arm_pmu_get_pmuver_limit());
> > + /* Hide SPE from guests */
> > + val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMSVer);
> > +
> > + return val;
> > }
> >
> > static int set_id_aa64dfr0_el1(struct kvm_vcpu *vcpu,
> > @@ -1457,14 +1560,35 @@ static int set_id_aa64dfr0_el1(struct
> > kvm_vcpu *vcpu,
> > if (kvm_vcpu_has_pmu(vcpu) != valid_pmu)
> > return -EINVAL;
> >
> > - /* We can only differ with PMUver, and anything else is an
> > error */
> > - val ^= read_id_reg(vcpu, rd);
> > - val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer);
> > - if (val)
> > - return -EINVAL;
> > + if (!valid_pmu) {
> > + /*
> > + * Ignore the PMUVer field in @val. The PMUVer
> > would be determined
> > + * by arch flags bit
> > KVM_ARCH_FLAG_VCPU_HAS_IMP_DEF_PMU,
> > + */
> > + pmuver = FIELD_GET(ID_AA64DFR0_EL1_PMUVer_MASK,
> > + IDREG(vcpu->kvm,
> > SYS_ID_AA64DFR0_EL1));
> > + val &= ~ID_AA64DFR0_EL1_PMUVer_MASK;
> > + val |= FIELD_PREP(ID_AA64DFR0_EL1_PMUVer_MASK,
> > pmuver);
> > + }
> >
> > - pmuver_update(vcpu, pmuver, valid_pmu);
> > - return 0;
> > + return pmuver_update(vcpu, rd, val, pmuver, valid_pmu);
> > +}
> > +
> > +static u64 read_sanitised_id_dfr0_el1(struct kvm_vcpu *vcpu,
> > + const struct sys_reg_desc
> > *rd)
> > +{
> > + u64 val;
> > + u32 id = reg_to_encoding(rd);
> > +
> > + val = read_sanitised_ftr_reg(id);
> > + /*
> > + * Initialise the default PMUver before there is a chance
> > to
> > + * create an actual PMU.
> > + */
> > + val &= ~ARM64_FEATURE_MASK(ID_DFR0_EL1_PerfMon);
> > + val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_DFR0_EL1_PerfMon),
> > kvm_arm_pmu_get_pmuver_limit());
> > +
> > + return val;
> > }
> >
> > static int set_id_dfr0_el1(struct kvm_vcpu *vcpu,
> > @@ -1493,14 +1617,18 @@ static int set_id_dfr0_el1(struct kvm_vcpu
> > *vcpu,
> > if (kvm_vcpu_has_pmu(vcpu) != valid_pmu)
> > return -EINVAL;
> >
> > - /* We can only differ with PerfMon, and anything else is an
> > error */
> > - val ^= read_id_reg(vcpu, rd);
> > - val &= ~ARM64_FEATURE_MASK(ID_DFR0_EL1_PerfMon);
> > - if (val)
> > - return -EINVAL;
> > + if (!valid_pmu) {
> > + /*
> > + * Ignore the PerfMon field in @val. The PerfMon
> > would be determined
> > + * by arch flags bit
> > KVM_ARCH_FLAG_VCPU_HAS_IMP_DEF_PMU,
> > + */
> > + perfmon = FIELD_GET(ID_DFR0_EL1_PerfMon_MASK,
> > + IDREG(vcpu->kvm,
> > SYS_ID_DFR0_EL1));
> > + val &= ~ID_DFR0_EL1_PerfMon_MASK;
> > + val |= FIELD_PREP(ID_DFR0_EL1_PerfMon_MASK,
> > perfmon);
> > + }
> >
> > - pmuver_update(vcpu, perfmon_to_pmuver(perfmon), valid_pmu);
> > - return 0;
> > + return pmuver_update(vcpu, rd, val,
> > perfmon_to_pmuver(perfmon), valid_pmu);
> > }
> >
> > /*
> > @@ -1520,11 +1648,14 @@ static int get_id_reg(struct kvm_vcpu
> > *vcpu, const struct sys_reg_desc *rd,
> > static int set_id_reg(struct kvm_vcpu *vcpu, const struct
> > sys_reg_desc *rd,
> > u64 val)
> > {
> > - /* This is what we mean by invariant: you can't change it.
> > */
> > - if (val != read_id_reg(vcpu, rd))
> > - return -EINVAL;
> > + u32 id = reg_to_encoding(rd);
> > + int ret = 0;
> >
> > - return 0;
> > + ret = arm64_check_features(vcpu, rd, val);
> > + if (!ret)
> > + IDREG(vcpu->kvm, id) = val;
> > +
> > + return ret;
> > }
> >
> > static int get_raz_reg(struct kvm_vcpu *vcpu, const struct
> > sys_reg_desc *rd,
> > @@ -1875,9 +2006,13 @@ static const struct sys_reg_desc
> > sys_reg_descs[] = {
> > /* CRm=1 */
> > AA32_ID_SANITISED(ID_PFR0_EL1),
> > AA32_ID_SANITISED(ID_PFR1_EL1),
> > - { SYS_DESC(SYS_ID_DFR0_EL1), .access = access_id_reg,
> > - .get_user = get_id_reg, .set_user = set_id_dfr0_el1,
> > - .visibility = aa32_id_visibility, },
> > + { SYS_DESC(SYS_ID_DFR0_EL1),
> > + .access = access_id_reg,
> > + .get_user = get_id_reg,
> > + .set_user = set_id_dfr0_el1,
> > + .visibility = aa32_id_visibility,
> > + .reset = read_sanitised_id_dfr0_el1,
> > + .val = ID_DFR0_EL1_PerfMon_MASK, },
> > ID_HIDDEN(ID_AFR0_EL1),
> > AA32_ID_SANITISED(ID_MMFR0_EL1),
> > AA32_ID_SANITISED(ID_MMFR1_EL1),
> > @@ -1906,8 +2041,12 @@ static const struct sys_reg_desc
> > sys_reg_descs[] = {
> >
> > /* AArch64 ID registers */
> > /* CRm=4 */
> > - { SYS_DESC(SYS_ID_AA64PFR0_EL1), .access = access_id_reg,
> > - .get_user = get_id_reg, .set_user = set_id_aa64pfr0_el1,
> > },
> > + { SYS_DESC(SYS_ID_AA64PFR0_EL1),
> > + .access = access_id_reg,
> > + .get_user = get_id_reg,
> > + .set_user = set_id_reg,
> > + .reset = read_sanitised_id_aa64pfr0_el1,
> > + .val = ID_AA64PFR0_EL1_CSV2_MASK |
> > ID_AA64PFR0_EL1_CSV3_MASK, },
> > ID_SANITISED(ID_AA64PFR1_EL1),
> > ID_UNALLOCATED(4,2),
> > ID_UNALLOCATED(4,3),
> > @@ -1917,8 +2056,12 @@ static const struct sys_reg_desc
> > sys_reg_descs[] = {
> > ID_UNALLOCATED(4,7),
> >
> > /* CRm=5 */
> > - { SYS_DESC(SYS_ID_AA64DFR0_EL1), .access = access_id_reg,
> > - .get_user = get_id_reg, .set_user = set_id_aa64dfr0_el1,
> > },
> > + { SYS_DESC(SYS_ID_AA64DFR0_EL1),
> > + .access = access_id_reg,
> > + .get_user = get_id_reg,
> > + .set_user = set_id_aa64dfr0_el1,
> > + .reset = read_sanitised_id_aa64dfr0_el1,
> > + .val = ID_AA64DFR0_EL1_PMUVer_MASK, },
> > ID_SANITISED(ID_AA64DFR1_EL1),
> > ID_UNALLOCATED(5,2),
> > ID_UNALLOCATED(5,3),
> > @@ -3454,38 +3597,6 @@ void kvm_arm_init_id_regs(struct kvm *kvm)
> > idreg++;
> > id = reg_to_encoding(idreg);
> > }
> > -
> > - /*
> > - * The default is to expose CSV2 == 1 if the HW isn't
> > affected.
> > - * Although this is a per-CPU feature, we make it global
> > because
> > - * asymmetric systems are just a nuisance.
> > - *
> > - * Userspace can override this as long as it doesn't
> > promise
> > - * the impossible.
> > - */
> > - val = IDREG(kvm, SYS_ID_AA64PFR0_EL1);
> > -
> > - if (arm64_get_spectre_v2_state() == SPECTRE_UNAFFECTED) {
> > - val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV2);
> > - val |=
> > FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV2), 1);
> > - }
> > - if (arm64_get_meltdown_state() == SPECTRE_UNAFFECTED) {
> > - val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV3);
> > - val |=
> > FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV3), 1);
> > - }
> > -
> > - IDREG(kvm, SYS_ID_AA64PFR0_EL1) = val;
> > - /*
> > - * Initialise the default PMUver before there is a chance
> > to
> > - * create an actual PMU.
> > - */
> > - val = IDREG(kvm, SYS_ID_AA64DFR0_EL1);
> > -
> > - val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer);
> > - val |=
> > FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer),
> > - kvm_arm_pmu_get_pmuver_limit());
> > -
> > - IDREG(kvm, SYS_ID_AA64DFR0_EL1) = val;
> > }
> >
> > int __init kvm_sys_reg_table_init(void)
> > --
> > 2.41.0.rc0.172.g3f132b7071-goog
> >
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2023-06-02 22:28 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-02 0:51 [PATCH v11 0/5] Support writable CPU ID registers from userspace Jing Zhang
2023-06-02 0:51 ` Jing Zhang
2023-06-02 0:51 ` [PATCH v11 1/5] KVM: arm64: Save ID registers' sanitized value per guest Jing Zhang
2023-06-02 0:51 ` Jing Zhang
2023-06-02 0:51 ` [PATCH v11 2/5] KVM: arm64: Use per guest ID register for ID_AA64PFR0_EL1.[CSV2|CSV3] Jing Zhang
2023-06-02 0:51 ` Jing Zhang
2023-06-02 0:51 ` [PATCH v11 3/5] KVM: arm64: Use per guest ID register for ID_AA64DFR0_EL1.PMUVer Jing Zhang
2023-06-02 0:51 ` Jing Zhang
2023-06-02 0:51 ` [PATCH v11 4/5] KVM: arm64: Reuse fields of sys_reg_desc for idreg Jing Zhang
2023-06-02 0:51 ` Jing Zhang
2023-06-02 0:51 ` [PATCH v11 5/5] KVM: arm64: Refactor writings for PMUVer/CSV2/CSV3 Jing Zhang
2023-06-02 0:51 ` Jing Zhang
2023-06-02 17:15 ` Jing Zhang
2023-06-02 17:15 ` Jing Zhang
2023-06-02 22:27 ` Jitindar Singh, Suraj [this message]
2023-06-02 22:27 ` Jitindar Singh, Suraj
2023-06-03 0:08 ` Jing Zhang
2023-06-03 0:08 ` Jing Zhang
2023-06-02 19:21 ` Jitindar Singh, Suraj
2023-06-02 19:21 ` Jitindar Singh, Suraj
2023-06-03 0:03 ` Jing Zhang
2023-06-03 0:03 ` Jing Zhang
2023-06-02 22:14 ` [PATCH 0/3] RE: Support writable CPU ID registers from userspace [v11] Suraj Jitindar Singh
2023-06-02 22:14 ` Suraj Jitindar Singh
2023-06-02 22:14 ` [PATCH 1/3] KVM: arm64: Update id_reg limit value based on per vcpu flags Suraj Jitindar Singh
2023-06-02 22:14 ` Suraj Jitindar Singh
2023-06-02 22:14 ` [PATCH 2/3] KVM: arm64: Move non per vcpu flag checks out of kvm_arm_update_id_reg() Suraj Jitindar Singh
2023-06-02 22:14 ` Suraj Jitindar Singh
2023-06-02 22:14 ` [PATCH 3/3] KVM: arm64: Use per guest ID register for ID_AA64PFR1_EL1.MTE Suraj Jitindar Singh
2023-06-02 22:14 ` Suraj Jitindar Singh
2023-06-03 8:28 ` Marc Zyngier
2023-06-03 8:28 ` Marc Zyngier
2023-06-05 16:39 ` Cornelia Huck
2023-06-05 16:39 ` Cornelia Huck
2023-06-06 16:42 ` Marc Zyngier
2023-06-06 16:42 ` Marc Zyngier
2023-06-07 10:09 ` Cornelia Huck
2023-06-07 10:09 ` Cornelia Huck
2023-06-08 17:57 ` Catalin Marinas
2023-06-08 17:57 ` Catalin Marinas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c1b97c4c97d44435032137e639790c75face4076.camel@amazon.com \
--to=surajjs@amazon.com \
--cc=alexandru.elisei@arm.com \
--cc=james.morse@arm.com \
--cc=jingzhangos@google.com \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.linux.dev \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=maz@kernel.org \
--cc=oupton@google.com \
--cc=pbonzini@redhat.com \
--cc=rananta@google.com \
--cc=reijiw@google.com \
--cc=suzuki.poulose@arm.com \
--cc=tabba@google.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.