* [PATCH 0/3] KVM: arm64: Another set of CSV2-related fixes
@ 2020-11-03 17:14 Marc Zyngier
2020-11-03 17:14 ` [PATCH 1/3] KVM: arm64: Allow setting of ID_AA64PFR0_EL1.CSV2 from userspace Marc Zyngier
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Marc Zyngier @ 2020-11-03 17:14 UTC (permalink / raw)
To: kvm, kvmarm, linux-arm-kernel
Cc: Peng Liang, Will Deacon, James Morse, Julien Thierry,
Suzuki K Poulose, kernel-team
This small series addresses a couple of Spectre-v2 related issues:
- Fix a live migration regression introduced with the setting of CSV2
on systems that are not affected by Spectre-v2, but that don't
directly expose it in ID_AA64PFR0_EL1
- Inject an UNDEF exception if the guest tries to access any of
SCXTNUM_ELx, as we don't advertise it to guests.
Patches on top of 5.10-rc2.
Marc Zyngier (3):
KVM: arm64: Allow setting of ID_AA64PFR0_EL1.CSV2 from userspace
KVM: arm64: Rename access_amu() to undef_access()
KVM: arm64: Handle SCXTNUM_ELx traps
arch/arm64/include/asm/kvm_host.h | 2 +
arch/arm64/include/asm/sysreg.h | 4 ++
arch/arm64/kvm/arm.c | 16 +++++++
arch/arm64/kvm/sys_regs.c | 70 +++++++++++++++++++++++--------
4 files changed, 74 insertions(+), 18 deletions(-)
--
2.28.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/3] KVM: arm64: Allow setting of ID_AA64PFR0_EL1.CSV2 from userspace
2020-11-03 17:14 [PATCH 0/3] KVM: arm64: Another set of CSV2-related fixes Marc Zyngier
@ 2020-11-03 17:14 ` Marc Zyngier
2020-11-05 22:44 ` Will Deacon
2020-11-03 17:14 ` [PATCH 2/3] KVM: arm64: Rename access_amu() to undef_access() Marc Zyngier
2020-11-03 17:14 ` [PATCH 3/3] KVM: arm64: Handle SCXTNUM_ELx traps Marc Zyngier
2 siblings, 1 reply; 10+ messages in thread
From: Marc Zyngier @ 2020-11-03 17:14 UTC (permalink / raw)
To: kvm, kvmarm, linux-arm-kernel
Cc: Peng Liang, Will Deacon, James Morse, Julien Thierry,
Suzuki K Poulose, kernel-team
We now expose ID_AA64PFR0_EL1.CSV2=1 to guests running on hosts
that are immune to Spectre-v2, but that don't have this field set,
most likely because they predate the specification.
However, this prevents the migration of guests that have started on
a host the doesn't fake this CSV2 setting to one that does, as KVM
rejects the write to ID_AA64PFR0_EL2 on the grounds that it isn't
what is already there.
In order to fix this, allow userspace to set this field as long as
this doesn't result in a promising more than what is already there
(setting CSV2 to 0 is acceptable, but setting it to 1 when it is
already set to 0 isn't).
Fixes: e1026237f9067 ("KVM: arm64: Set CSV2 for guests on hardware unaffected by Spectre-v2")
Reported-by: Peng Liang <liangpeng10@huawei.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
arch/arm64/include/asm/kvm_host.h | 2 ++
arch/arm64/kvm/arm.c | 16 +++++++++++++
arch/arm64/kvm/sys_regs.c | 38 +++++++++++++++++++++++++++----
3 files changed, 52 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 781d029b8aa8..0cd9f0f75c13 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -118,6 +118,8 @@ struct kvm_arch {
*/
unsigned long *pmu_filter;
unsigned int pmuver;
+
+ u8 pfr0_csv2;
};
struct kvm_vcpu_fault_info {
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 5750ec34960e..c0ffb019ca8b 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -102,6 +102,20 @@ static int kvm_arm_default_max_vcpus(void)
return vgic_present ? kvm_vgic_get_max_vcpus() : KVM_MAX_VCPUS;
}
+static void set_default_csv2(struct kvm *kvm)
+{
+ /*
+ * 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.
+ */
+ if (arm64_get_spectre_v2_state() == SPECTRE_UNAFFECTED)
+ kvm->arch.pfr0_csv2 = 1;
+}
+
/**
* kvm_arch_init_vm - initializes a VM data structure
* @kvm: pointer to the KVM struct
@@ -127,6 +141,8 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
/* The maximum number of VCPUs is limited by the host's GIC model */
kvm->arch.max_vcpus = kvm_arm_default_max_vcpus();
+ set_default_csv2(kvm);
+
return ret;
out_free_stage2_pgd:
kvm_free_stage2_pgd(&kvm->arch.mmu);
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index fb12d3ef423a..61789027b92b 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1128,9 +1128,8 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu,
if (!vcpu_has_sve(vcpu))
val &= ~(0xfUL << ID_AA64PFR0_SVE_SHIFT);
val &= ~(0xfUL << ID_AA64PFR0_AMU_SHIFT);
- if (!(val & (0xfUL << ID_AA64PFR0_CSV2_SHIFT)) &&
- arm64_get_spectre_v2_state() == SPECTRE_UNAFFECTED)
- val |= (1UL << ID_AA64PFR0_CSV2_SHIFT);
+ val &= ~(0xfUL << ID_AA64PFR0_CSV2_SHIFT);
+ val |= ((u64)vcpu->kvm->arch.pfr0_csv2 << ID_AA64PFR0_CSV2_SHIFT);
} else if (id == SYS_ID_AA64PFR1_EL1) {
val &= ~(0xfUL << ID_AA64PFR1_MTE_SHIFT);
} else if (id == SYS_ID_AA64ISAR1_EL1 && !vcpu_has_ptrauth(vcpu)) {
@@ -1260,6 +1259,36 @@ static int set_id_aa64zfr0_el1(struct kvm_vcpu *vcpu,
return 0;
}
+static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
+ const struct sys_reg_desc *rd,
+ const struct kvm_one_reg *reg, void __user *uaddr)
+{
+ const u64 id = sys_reg_to_index(rd);
+ int err;
+ u64 val;
+ u8 csv2;
+
+ err = reg_from_user(&val, uaddr, id);
+ if (err)
+ return err;
+
+ /*
+ * 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).
+ */
+ csv2 = FIELD_GET(0xfUL << ID_AA64PFR0_CSV2_SHIFT, val);
+ if (csv2 > vcpu->kvm->arch.pfr0_csv2)
+ return -EINVAL;
+ vcpu->kvm->arch.pfr0_csv2 = csv2;
+
+ /* This is what we mean by invariant: you can't change it. */
+ if (val != read_id_reg(vcpu, rd, false))
+ return -EINVAL;
+
+ return 0;
+}
+
/*
* cpufeature ID register user accessors
*
@@ -1514,7 +1543,8 @@ static const struct sys_reg_desc sys_reg_descs[] = {
/* AArch64 ID registers */
/* CRm=4 */
- ID_SANITISED(ID_AA64PFR0_EL1),
+ { SYS_DESC(SYS_ID_AA64PFR0_EL1), .access = access_id_reg,
+ .get_user = get_id_reg, .set_user = set_id_aa64pfr0_el1, },
ID_SANITISED(ID_AA64PFR1_EL1),
ID_UNALLOCATED(4,2),
ID_UNALLOCATED(4,3),
--
2.28.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] KVM: arm64: Rename access_amu() to undef_access()
2020-11-03 17:14 [PATCH 0/3] KVM: arm64: Another set of CSV2-related fixes Marc Zyngier
2020-11-03 17:14 ` [PATCH 1/3] KVM: arm64: Allow setting of ID_AA64PFR0_EL1.CSV2 from userspace Marc Zyngier
@ 2020-11-03 17:14 ` Marc Zyngier
2020-11-05 22:50 ` Will Deacon
2020-11-03 17:14 ` [PATCH 3/3] KVM: arm64: Handle SCXTNUM_ELx traps Marc Zyngier
2 siblings, 1 reply; 10+ messages in thread
From: Marc Zyngier @ 2020-11-03 17:14 UTC (permalink / raw)
To: kvm, kvmarm, linux-arm-kernel
Cc: Peng Liang, Will Deacon, James Morse, Julien Thierry,
Suzuki K Poulose, kernel-team
The only thing that access_amu() does is to inject an UNDEF exception,
so let's rename it to the clearer undef_access(). We'll reuse that
for some other system registers.
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
arch/arm64/kvm/sys_regs.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 61789027b92b..fafaa81bf1f6 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1038,8 +1038,8 @@ static bool access_pmuserenr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
{ SYS_DESC(SYS_PMEVTYPERn_EL0(n)), \
access_pmu_evtyper, reset_unknown, (PMEVTYPER0_EL0 + n), }
-static bool access_amu(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
- const struct sys_reg_desc *r)
+static bool undef_access(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
+ const struct sys_reg_desc *r)
{
kvm_inject_undefined(vcpu);
@@ -1047,10 +1047,10 @@ static bool access_amu(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
}
/* Macro to expand the AMU counter and type registers*/
-#define AMU_AMEVCNTR0_EL0(n) { SYS_DESC(SYS_AMEVCNTR0_EL0(n)), access_amu }
-#define AMU_AMEVTYPER0_EL0(n) { SYS_DESC(SYS_AMEVTYPER0_EL0(n)), access_amu }
-#define AMU_AMEVCNTR1_EL0(n) { SYS_DESC(SYS_AMEVCNTR1_EL0(n)), access_amu }
-#define AMU_AMEVTYPER1_EL0(n) { SYS_DESC(SYS_AMEVTYPER1_EL0(n)), access_amu }
+#define AMU_AMEVCNTR0_EL0(n) { SYS_DESC(SYS_AMEVCNTR0_EL0(n)), undef_access }
+#define AMU_AMEVTYPER0_EL0(n) { SYS_DESC(SYS_AMEVTYPER0_EL0(n)), undef_access }
+#define AMU_AMEVCNTR1_EL0(n) { SYS_DESC(SYS_AMEVCNTR1_EL0(n)), undef_access }
+#define AMU_AMEVTYPER1_EL0(n) { SYS_DESC(SYS_AMEVTYPER1_EL0(n)), undef_access }
static bool trap_ptrauth(struct kvm_vcpu *vcpu,
struct sys_reg_params *p,
@@ -1679,14 +1679,14 @@ static const struct sys_reg_desc sys_reg_descs[] = {
{ SYS_DESC(SYS_TPIDR_EL0), NULL, reset_unknown, TPIDR_EL0 },
{ SYS_DESC(SYS_TPIDRRO_EL0), NULL, reset_unknown, TPIDRRO_EL0 },
- { SYS_DESC(SYS_AMCR_EL0), access_amu },
- { SYS_DESC(SYS_AMCFGR_EL0), access_amu },
- { SYS_DESC(SYS_AMCGCR_EL0), access_amu },
- { SYS_DESC(SYS_AMUSERENR_EL0), access_amu },
- { SYS_DESC(SYS_AMCNTENCLR0_EL0), access_amu },
- { SYS_DESC(SYS_AMCNTENSET0_EL0), access_amu },
- { SYS_DESC(SYS_AMCNTENCLR1_EL0), access_amu },
- { SYS_DESC(SYS_AMCNTENSET1_EL0), access_amu },
+ { SYS_DESC(SYS_AMCR_EL0), undef_access },
+ { SYS_DESC(SYS_AMCFGR_EL0), undef_access },
+ { SYS_DESC(SYS_AMCGCR_EL0), undef_access },
+ { SYS_DESC(SYS_AMUSERENR_EL0), undef_access },
+ { SYS_DESC(SYS_AMCNTENCLR0_EL0), undef_access },
+ { SYS_DESC(SYS_AMCNTENSET0_EL0), undef_access },
+ { SYS_DESC(SYS_AMCNTENCLR1_EL0), undef_access },
+ { SYS_DESC(SYS_AMCNTENSET1_EL0), undef_access },
AMU_AMEVCNTR0_EL0(0),
AMU_AMEVCNTR0_EL0(1),
AMU_AMEVCNTR0_EL0(2),
--
2.28.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] KVM: arm64: Handle SCXTNUM_ELx traps
2020-11-03 17:14 [PATCH 0/3] KVM: arm64: Another set of CSV2-related fixes Marc Zyngier
2020-11-03 17:14 ` [PATCH 1/3] KVM: arm64: Allow setting of ID_AA64PFR0_EL1.CSV2 from userspace Marc Zyngier
2020-11-03 17:14 ` [PATCH 2/3] KVM: arm64: Rename access_amu() to undef_access() Marc Zyngier
@ 2020-11-03 17:14 ` Marc Zyngier
2020-11-05 22:50 ` Will Deacon
2 siblings, 1 reply; 10+ messages in thread
From: Marc Zyngier @ 2020-11-03 17:14 UTC (permalink / raw)
To: kvm, kvmarm, linux-arm-kernel
Cc: Peng Liang, Will Deacon, James Morse, Julien Thierry,
Suzuki K Poulose, kernel-team
As the kernel never sets HCR_EL2.EnSCXT, accesses to SCXTNUM_ELx
will trap to EL2. Let's handle that as gracefully as possible
by injecting an UNDEF exception into the guest. This is consistent
with the guest's view of ID_AA64PFR0_EL1.CSV2 being at most 1.
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
arch/arm64/include/asm/sysreg.h | 4 ++++
arch/arm64/kvm/sys_regs.c | 4 ++++
2 files changed, 8 insertions(+)
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 174817ba119c..e2ef4c2edf06 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -372,6 +372,8 @@
#define SYS_CONTEXTIDR_EL1 sys_reg(3, 0, 13, 0, 1)
#define SYS_TPIDR_EL1 sys_reg(3, 0, 13, 0, 4)
+#define SYS_SCXTNUM_EL1 sys_reg(3, 0, 13, 0, 7)
+
#define SYS_CNTKCTL_EL1 sys_reg(3, 0, 14, 1, 0)
#define SYS_CCSIDR_EL1 sys_reg(3, 1, 0, 0, 0)
@@ -404,6 +406,8 @@
#define SYS_TPIDR_EL0 sys_reg(3, 3, 13, 0, 2)
#define SYS_TPIDRRO_EL0 sys_reg(3, 3, 13, 0, 3)
+#define SYS_SCXTNUM_EL0 sys_reg(3, 3, 13, 0, 7)
+
/* Definitions for system register interface to AMU for ARMv8.4 onwards */
#define SYS_AM_EL0(crm, op2) sys_reg(3, 3, 13, (crm), (op2))
#define SYS_AMCR_EL0 SYS_AM_EL0(2, 0)
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index fafaa81bf1f6..d66930edb60c 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1651,6 +1651,8 @@ static const struct sys_reg_desc sys_reg_descs[] = {
{ SYS_DESC(SYS_CONTEXTIDR_EL1), access_vm_reg, reset_val, CONTEXTIDR_EL1, 0 },
{ SYS_DESC(SYS_TPIDR_EL1), NULL, reset_unknown, TPIDR_EL1 },
+ { SYS_DESC(SYS_SCXTNUM_EL1), undef_access },
+
{ SYS_DESC(SYS_CNTKCTL_EL1), NULL, reset_val, CNTKCTL_EL1, 0},
{ SYS_DESC(SYS_CCSIDR_EL1), access_ccsidr },
@@ -1679,6 +1681,8 @@ static const struct sys_reg_desc sys_reg_descs[] = {
{ SYS_DESC(SYS_TPIDR_EL0), NULL, reset_unknown, TPIDR_EL0 },
{ SYS_DESC(SYS_TPIDRRO_EL0), NULL, reset_unknown, TPIDRRO_EL0 },
+ { SYS_DESC(SYS_SCXTNUM_EL0), undef_access },
+
{ SYS_DESC(SYS_AMCR_EL0), undef_access },
{ SYS_DESC(SYS_AMCFGR_EL0), undef_access },
{ SYS_DESC(SYS_AMCGCR_EL0), undef_access },
--
2.28.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] KVM: arm64: Allow setting of ID_AA64PFR0_EL1.CSV2 from userspace
2020-11-03 17:14 ` [PATCH 1/3] KVM: arm64: Allow setting of ID_AA64PFR0_EL1.CSV2 from userspace Marc Zyngier
@ 2020-11-05 22:44 ` Will Deacon
2020-11-06 9:57 ` Marc Zyngier
0 siblings, 1 reply; 10+ messages in thread
From: Will Deacon @ 2020-11-05 22:44 UTC (permalink / raw)
To: Marc Zyngier
Cc: kvm, kvmarm, linux-arm-kernel, Peng Liang, James Morse,
Julien Thierry, Suzuki K Poulose, kernel-team
On Tue, Nov 03, 2020 at 05:14:43PM +0000, Marc Zyngier wrote:
> We now expose ID_AA64PFR0_EL1.CSV2=1 to guests running on hosts
> that are immune to Spectre-v2, but that don't have this field set,
> most likely because they predate the specification.
>
> However, this prevents the migration of guests that have started on
> a host the doesn't fake this CSV2 setting to one that does, as KVM
> rejects the write to ID_AA64PFR0_EL2 on the grounds that it isn't
> what is already there.
>
> In order to fix this, allow userspace to set this field as long as
> this doesn't result in a promising more than what is already there
> (setting CSV2 to 0 is acceptable, but setting it to 1 when it is
> already set to 0 isn't).
>
> Fixes: e1026237f9067 ("KVM: arm64: Set CSV2 for guests on hardware unaffected by Spectre-v2")
> Reported-by: Peng Liang <liangpeng10@huawei.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
> arch/arm64/include/asm/kvm_host.h | 2 ++
> arch/arm64/kvm/arm.c | 16 +++++++++++++
> arch/arm64/kvm/sys_regs.c | 38 +++++++++++++++++++++++++++----
> 3 files changed, 52 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 781d029b8aa8..0cd9f0f75c13 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -118,6 +118,8 @@ struct kvm_arch {
> */
> unsigned long *pmu_filter;
> unsigned int pmuver;
> +
> + u8 pfr0_csv2;
> };
>
> struct kvm_vcpu_fault_info {
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 5750ec34960e..c0ffb019ca8b 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -102,6 +102,20 @@ static int kvm_arm_default_max_vcpus(void)
> return vgic_present ? kvm_vgic_get_max_vcpus() : KVM_MAX_VCPUS;
> }
>
> +static void set_default_csv2(struct kvm *kvm)
> +{
> + /*
> + * 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.
> + */
> + if (arm64_get_spectre_v2_state() == SPECTRE_UNAFFECTED)
> + kvm->arch.pfr0_csv2 = 1;
> +}
> +
> /**
> * kvm_arch_init_vm - initializes a VM data structure
> * @kvm: pointer to the KVM struct
> @@ -127,6 +141,8 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
> /* The maximum number of VCPUs is limited by the host's GIC model */
> kvm->arch.max_vcpus = kvm_arm_default_max_vcpus();
>
> + set_default_csv2(kvm);
> +
> return ret;
> out_free_stage2_pgd:
> kvm_free_stage2_pgd(&kvm->arch.mmu);
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index fb12d3ef423a..61789027b92b 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1128,9 +1128,8 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu,
> if (!vcpu_has_sve(vcpu))
> val &= ~(0xfUL << ID_AA64PFR0_SVE_SHIFT);
> val &= ~(0xfUL << ID_AA64PFR0_AMU_SHIFT);
> - if (!(val & (0xfUL << ID_AA64PFR0_CSV2_SHIFT)) &&
> - arm64_get_spectre_v2_state() == SPECTRE_UNAFFECTED)
> - val |= (1UL << ID_AA64PFR0_CSV2_SHIFT);
> + val &= ~(0xfUL << ID_AA64PFR0_CSV2_SHIFT);
> + val |= ((u64)vcpu->kvm->arch.pfr0_csv2 << ID_AA64PFR0_CSV2_SHIFT);
> } else if (id == SYS_ID_AA64PFR1_EL1) {
> val &= ~(0xfUL << ID_AA64PFR1_MTE_SHIFT);
> } else if (id == SYS_ID_AA64ISAR1_EL1 && !vcpu_has_ptrauth(vcpu)) {
> @@ -1260,6 +1259,36 @@ static int set_id_aa64zfr0_el1(struct kvm_vcpu *vcpu,
> return 0;
> }
>
> +static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
> + const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> + const u64 id = sys_reg_to_index(rd);
> + int err;
> + u64 val;
> + u8 csv2;
> +
> + err = reg_from_user(&val, uaddr, id);
> + if (err)
> + return err;
> +
> + /*
> + * 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).
> + */
> + csv2 = FIELD_GET(0xfUL << ID_AA64PFR0_CSV2_SHIFT, val);
cpuid_feature_extract_unsigned_field() instead?
> + if (csv2 > vcpu->kvm->arch.pfr0_csv2)
> + return -EINVAL;
> + vcpu->kvm->arch.pfr0_csv2 = csv2;
> +
> + /* This is what we mean by invariant: you can't change it. */
> + if (val != read_id_reg(vcpu, rd, false))
> + return -EINVAL;
I think it's quite confusing to return -EINVAL in the case that we have
actually updated arch.pfr0_csv2, as it's indistinguishable from the case
when csv2 was invalid and the field wasn't updated.
Will
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] KVM: arm64: Rename access_amu() to undef_access()
2020-11-03 17:14 ` [PATCH 2/3] KVM: arm64: Rename access_amu() to undef_access() Marc Zyngier
@ 2020-11-05 22:50 ` Will Deacon
2020-11-06 10:54 ` Marc Zyngier
0 siblings, 1 reply; 10+ messages in thread
From: Will Deacon @ 2020-11-05 22:50 UTC (permalink / raw)
To: Marc Zyngier
Cc: kvm, kvmarm, linux-arm-kernel, Peng Liang, James Morse,
Julien Thierry, Suzuki K Poulose, kernel-team
On Tue, Nov 03, 2020 at 05:14:44PM +0000, Marc Zyngier wrote:
> The only thing that access_amu() does is to inject an UNDEF exception,
> so let's rename it to the clearer undef_access(). We'll reuse that
> for some other system registers.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
> arch/arm64/kvm/sys_regs.c | 28 ++++++++++++++--------------
> 1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 61789027b92b..fafaa81bf1f6 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1038,8 +1038,8 @@ static bool access_pmuserenr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> { SYS_DESC(SYS_PMEVTYPERn_EL0(n)), \
> access_pmu_evtyper, reset_unknown, (PMEVTYPER0_EL0 + n), }
>
> -static bool access_amu(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> - const struct sys_reg_desc *r)
> +static bool undef_access(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> + const struct sys_reg_desc *r)
> {
> kvm_inject_undefined(vcpu);
This seems to be identical to trap_ptrauth(). Shall we give it the same
treatment?
Will
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] KVM: arm64: Handle SCXTNUM_ELx traps
2020-11-03 17:14 ` [PATCH 3/3] KVM: arm64: Handle SCXTNUM_ELx traps Marc Zyngier
@ 2020-11-05 22:50 ` Will Deacon
0 siblings, 0 replies; 10+ messages in thread
From: Will Deacon @ 2020-11-05 22:50 UTC (permalink / raw)
To: Marc Zyngier
Cc: kvm, kvmarm, linux-arm-kernel, Peng Liang, James Morse,
Julien Thierry, Suzuki K Poulose, kernel-team
On Tue, Nov 03, 2020 at 05:14:45PM +0000, Marc Zyngier wrote:
> As the kernel never sets HCR_EL2.EnSCXT, accesses to SCXTNUM_ELx
> will trap to EL2. Let's handle that as gracefully as possible
> by injecting an UNDEF exception into the guest. This is consistent
> with the guest's view of ID_AA64PFR0_EL1.CSV2 being at most 1.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
> arch/arm64/include/asm/sysreg.h | 4 ++++
> arch/arm64/kvm/sys_regs.c | 4 ++++
> 2 files changed, 8 insertions(+)
Acked-by: Will Deacon <will@kernel.org>
Will
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] KVM: arm64: Allow setting of ID_AA64PFR0_EL1.CSV2 from userspace
2020-11-05 22:44 ` Will Deacon
@ 2020-11-06 9:57 ` Marc Zyngier
0 siblings, 0 replies; 10+ messages in thread
From: Marc Zyngier @ 2020-11-06 9:57 UTC (permalink / raw)
To: Will Deacon
Cc: kvm, kvmarm, linux-arm-kernel, Peng Liang, James Morse,
Julien Thierry, Suzuki K Poulose, kernel-team
On 2020-11-05 22:44, Will Deacon wrote:
>> + if (csv2 > vcpu->kvm->arch.pfr0_csv2)
>> + return -EINVAL;
>> + vcpu->kvm->arch.pfr0_csv2 = csv2;
>> +
>> + /* This is what we mean by invariant: you can't change it. */
>> + if (val != read_id_reg(vcpu, rd, false))
>> + return -EINVAL;
>
> I think it's quite confusing to return -EINVAL in the case that we have
> actually updated arch.pfr0_csv2, as it's indistinguishable from the
> case
> when csv2 was invalid and the field wasn't updated.
-EINVAL is the right error code here (you're setting an invalid value
for
the whole register). The bug is that we have now changed CSV2 for
everyone.
I'll have a look at fixing this, though it might involve some locking.
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] KVM: arm64: Rename access_amu() to undef_access()
2020-11-05 22:50 ` Will Deacon
@ 2020-11-06 10:54 ` Marc Zyngier
0 siblings, 0 replies; 10+ messages in thread
From: Marc Zyngier @ 2020-11-06 10:54 UTC (permalink / raw)
To: Will Deacon
Cc: kvm, kvmarm, linux-arm-kernel, Peng Liang, James Morse,
Julien Thierry, Suzuki K Poulose, kernel-team
On 2020-11-05 22:50, Will Deacon wrote:
> On Tue, Nov 03, 2020 at 05:14:44PM +0000, Marc Zyngier wrote:
>> The only thing that access_amu() does is to inject an UNDEF exception,
>> so let's rename it to the clearer undef_access(). We'll reuse that
>> for some other system registers.
>>
>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>> ---
>> arch/arm64/kvm/sys_regs.c | 28 ++++++++++++++--------------
>> 1 file changed, 14 insertions(+), 14 deletions(-)
>>
>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
>> index 61789027b92b..fafaa81bf1f6 100644
>> --- a/arch/arm64/kvm/sys_regs.c
>> +++ b/arch/arm64/kvm/sys_regs.c
>> @@ -1038,8 +1038,8 @@ static bool access_pmuserenr(struct kvm_vcpu
>> *vcpu, struct sys_reg_params *p,
>> { SYS_DESC(SYS_PMEVTYPERn_EL0(n)), \
>> access_pmu_evtyper, reset_unknown, (PMEVTYPER0_EL0 + n), }
>>
>> -static bool access_amu(struct kvm_vcpu *vcpu, struct sys_reg_params
>> *p,
>> - const struct sys_reg_desc *r)
>> +static bool undef_access(struct kvm_vcpu *vcpu, struct sys_reg_params
>> *p,
>> + const struct sys_reg_desc *r)
>> {
>> kvm_inject_undefined(vcpu);
>
> This seems to be identical to trap_ptrauth(). Shall we give it the same
> treatment?
Yes. MTE is also doing the same thing, so let's grind them all.
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] KVM: arm64: Handle SCXTNUM_ELx traps
2020-11-12 22:21 [PATCH 0/3] KVM/arm64 fixes for 5.10, take #3 Marc Zyngier
@ 2020-11-12 22:21 ` Marc Zyngier
0 siblings, 0 replies; 10+ messages in thread
From: Marc Zyngier @ 2020-11-12 22:21 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Peng Liang, Will Deacon, James Morse, Julien Thierry,
Suzuki K Poulose, kernel-team, kvmarm, kvm, linux-arm-kernel
As the kernel never sets HCR_EL2.EnSCXT, accesses to SCXTNUM_ELx
will trap to EL2. Let's handle that as gracefully as possible
by injecting an UNDEF exception into the guest. This is consistent
with the guest's view of ID_AA64PFR0_EL1.CSV2 being at most 1.
Signed-off-by: Marc Zyngier <maz@kernel.org>
Acked-by: Will Deacon <will@kernel.org>
Link: https://lore.kernel.org/r/20201110141308.451654-4-maz@kernel.org
---
arch/arm64/include/asm/sysreg.h | 4 ++++
arch/arm64/kvm/sys_regs.c | 4 ++++
2 files changed, 8 insertions(+)
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index d52c1b3ce589..a427a5653369 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -372,6 +372,8 @@
#define SYS_CONTEXTIDR_EL1 sys_reg(3, 0, 13, 0, 1)
#define SYS_TPIDR_EL1 sys_reg(3, 0, 13, 0, 4)
+#define SYS_SCXTNUM_EL1 sys_reg(3, 0, 13, 0, 7)
+
#define SYS_CNTKCTL_EL1 sys_reg(3, 0, 14, 1, 0)
#define SYS_CCSIDR_EL1 sys_reg(3, 1, 0, 0, 0)
@@ -404,6 +406,8 @@
#define SYS_TPIDR_EL0 sys_reg(3, 3, 13, 0, 2)
#define SYS_TPIDRRO_EL0 sys_reg(3, 3, 13, 0, 3)
+#define SYS_SCXTNUM_EL0 sys_reg(3, 3, 13, 0, 7)
+
/* Definitions for system register interface to AMU for ARMv8.4 onwards */
#define SYS_AM_EL0(crm, op2) sys_reg(3, 3, 13, (crm), (op2))
#define SYS_AMCR_EL0 SYS_AM_EL0(2, 0)
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index b0022f37c8f1..c1726fb7f3d9 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1598,6 +1598,8 @@ static const struct sys_reg_desc sys_reg_descs[] = {
{ SYS_DESC(SYS_CONTEXTIDR_EL1), access_vm_reg, reset_val, CONTEXTIDR_EL1, 0 },
{ SYS_DESC(SYS_TPIDR_EL1), NULL, reset_unknown, TPIDR_EL1 },
+ { SYS_DESC(SYS_SCXTNUM_EL1), undef_access },
+
{ SYS_DESC(SYS_CNTKCTL_EL1), NULL, reset_val, CNTKCTL_EL1, 0},
{ SYS_DESC(SYS_CCSIDR_EL1), access_ccsidr },
@@ -1626,6 +1628,8 @@ static const struct sys_reg_desc sys_reg_descs[] = {
{ SYS_DESC(SYS_TPIDR_EL0), NULL, reset_unknown, TPIDR_EL0 },
{ SYS_DESC(SYS_TPIDRRO_EL0), NULL, reset_unknown, TPIDRRO_EL0 },
+ { SYS_DESC(SYS_SCXTNUM_EL0), undef_access },
+
{ SYS_DESC(SYS_AMCR_EL0), undef_access },
{ SYS_DESC(SYS_AMCFGR_EL0), undef_access },
{ SYS_DESC(SYS_AMCGCR_EL0), undef_access },
--
2.28.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-11-12 22:22 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-03 17:14 [PATCH 0/3] KVM: arm64: Another set of CSV2-related fixes Marc Zyngier
2020-11-03 17:14 ` [PATCH 1/3] KVM: arm64: Allow setting of ID_AA64PFR0_EL1.CSV2 from userspace Marc Zyngier
2020-11-05 22:44 ` Will Deacon
2020-11-06 9:57 ` Marc Zyngier
2020-11-03 17:14 ` [PATCH 2/3] KVM: arm64: Rename access_amu() to undef_access() Marc Zyngier
2020-11-05 22:50 ` Will Deacon
2020-11-06 10:54 ` Marc Zyngier
2020-11-03 17:14 ` [PATCH 3/3] KVM: arm64: Handle SCXTNUM_ELx traps Marc Zyngier
2020-11-05 22:50 ` Will Deacon
2020-11-12 22:21 [PATCH 0/3] KVM/arm64 fixes for 5.10, take #3 Marc Zyngier
2020-11-12 22:21 ` [PATCH 3/3] KVM: arm64: Handle SCXTNUM_ELx traps Marc Zyngier
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).