All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/4] KVM: arm64: Fix get-reg-list regression
@ 2020-11-05  9:10 Andrew Jones
  2020-11-05  9:10   ` Andrew Jones
                   ` (5 more replies)
  0 siblings, 6 replies; 8+ messages in thread
From: Andrew Jones @ 2020-11-05  9:10 UTC (permalink / raw)
  To: kvmarm; +Cc: maz, xu910121, Dave.Martin

张东旭 <xu910121@sina.com> reported a regression seen with CentOS
when migrating from an old kernel to a new one. The problem was
that QEMU rejected the migration since KVM_GET_REG_LIST reported
a register was missing on the destination. Extra registers are OK
on the destination, but not missing ones. The regression reproduces
with upstream kernels when migrating from a 4.15 or later kernel,
up to one with commit 73433762fcae ("KVM: arm64/sve: System register
context switch and access support"), to a kernel that includes that
commit, e.g. the latest mainline (5.10-rc2).

The first patch of this series is the fix. The next two patches,
which don't have any intended functional changes, allow ID_SANITISED
to be used for registers that flip between exposing features and
being RAZ, which allows some code to be removed.

v3:
 - Improve commit messages [Dave]
 - Add new patch to consolidate REG_HIDDEN* flags [Dave]

v2:
 - CC stable [Marc]
 - Only one RAZ flag is enough [Marc]
 - Move id_visibility() up by read_id_reg() since they'll likely
   be maintained together [drew]


Andrew Jones (4):
  KVM: arm64: Don't hide ID registers from userspace
  KVM: arm64: Consolidate REG_HIDDEN_GUEST/USER
  KVM: arm64: Check RAZ visibility in ID register accessors
  KVM: arm64: Remove AA64ZFR0_EL1 accessors

 arch/arm64/kvm/sys_regs.c | 108 ++++++++++++--------------------------
 arch/arm64/kvm/sys_regs.h |  16 +++---
 2 files changed, 41 insertions(+), 83 deletions(-)

-- 
2.26.2

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 1/4] KVM: arm64: Don't hide ID registers from userspace
  2020-11-05  9:10 [PATCH v3 0/4] KVM: arm64: Fix get-reg-list regression Andrew Jones
@ 2020-11-05  9:10   ` Andrew Jones
  2020-11-05  9:10 ` [PATCH v3 2/4] KVM: arm64: Consolidate REG_HIDDEN_GUEST/USER Andrew Jones
                     ` (4 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Andrew Jones @ 2020-11-05  9:10 UTC (permalink / raw)
  To: kvmarm; +Cc: maz, xu910121, Dave.Martin, stable

ID registers are RAZ until they've been allocated a purpose, but
that doesn't mean they should be removed from the KVM_GET_REG_LIST
list. So far we only have one register, SYS_ID_AA64ZFR0_EL1, that
is hidden from userspace when its function, SVE, is not present.

Expose SYS_ID_AA64ZFR0_EL1 to userspace as RAZ when SVE is not
implemented. Removing the userspace visibility checks is enough
to reexpose it, as it will already return zero to userspace when
SVE is not present. The register already behaves as RAZ for the
guest when SVE is not present.

Fixes: 73433762fcae ("KVM: arm64/sve: System register context switch and access support")
Cc: stable@vger.kernel.org#v5.2+
Reported-by: 张东旭 <xu910121@sina.com>
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 arch/arm64/kvm/sys_regs.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index fb12d3ef423a..6ff0c15531ca 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1195,16 +1195,6 @@ static unsigned int sve_visibility(const struct kvm_vcpu *vcpu,
 	return REG_HIDDEN_USER | REG_HIDDEN_GUEST;
 }
 
-/* Visibility overrides for SVE-specific ID registers */
-static unsigned int sve_id_visibility(const struct kvm_vcpu *vcpu,
-				      const struct sys_reg_desc *rd)
-{
-	if (vcpu_has_sve(vcpu))
-		return 0;
-
-	return REG_HIDDEN_USER;
-}
-
 /* Generate the emulated ID_AA64ZFR0_EL1 value exposed to the guest */
 static u64 guest_id_aa64zfr0_el1(const struct kvm_vcpu *vcpu)
 {
@@ -1231,9 +1221,6 @@ static int get_id_aa64zfr0_el1(struct kvm_vcpu *vcpu,
 {
 	u64 val;
 
-	if (WARN_ON(!vcpu_has_sve(vcpu)))
-		return -ENOENT;
-
 	val = guest_id_aa64zfr0_el1(vcpu);
 	return reg_to_user(uaddr, &val, reg->id);
 }
@@ -1246,9 +1233,6 @@ static int set_id_aa64zfr0_el1(struct kvm_vcpu *vcpu,
 	int err;
 	u64 val;
 
-	if (WARN_ON(!vcpu_has_sve(vcpu)))
-		return -ENOENT;
-
 	err = reg_from_user(&val, uaddr, id);
 	if (err)
 		return err;
@@ -1518,7 +1502,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	ID_SANITISED(ID_AA64PFR1_EL1),
 	ID_UNALLOCATED(4,2),
 	ID_UNALLOCATED(4,3),
-	{ SYS_DESC(SYS_ID_AA64ZFR0_EL1), access_id_aa64zfr0_el1, .get_user = get_id_aa64zfr0_el1, .set_user = set_id_aa64zfr0_el1, .visibility = sve_id_visibility },
+	{ SYS_DESC(SYS_ID_AA64ZFR0_EL1), access_id_aa64zfr0_el1, .get_user = get_id_aa64zfr0_el1, .set_user = set_id_aa64zfr0_el1, },
 	ID_UNALLOCATED(4,5),
 	ID_UNALLOCATED(4,6),
 	ID_UNALLOCATED(4,7),
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 1/4] KVM: arm64: Don't hide ID registers from userspace
@ 2020-11-05  9:10   ` Andrew Jones
  0 siblings, 0 replies; 8+ messages in thread
From: Andrew Jones @ 2020-11-05  9:10 UTC (permalink / raw)
  To: kvmarm; +Cc: maz, xu910121, Dave.Martin, stable

ID registers are RAZ until they've been allocated a purpose, but
that doesn't mean they should be removed from the KVM_GET_REG_LIST
list. So far we only have one register, SYS_ID_AA64ZFR0_EL1, that
is hidden from userspace when its function, SVE, is not present.

Expose SYS_ID_AA64ZFR0_EL1 to userspace as RAZ when SVE is not
implemented. Removing the userspace visibility checks is enough
to reexpose it, as it will already return zero to userspace when
SVE is not present. The register already behaves as RAZ for the
guest when SVE is not present.

Fixes: 73433762fcae ("KVM: arm64/sve: System register context switch and access support")
Cc: stable@vger.kernel.org#v5.2+
Reported-by: 张东旭 <xu910121@sina.com>
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 arch/arm64/kvm/sys_regs.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index fb12d3ef423a..6ff0c15531ca 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1195,16 +1195,6 @@ static unsigned int sve_visibility(const struct kvm_vcpu *vcpu,
 	return REG_HIDDEN_USER | REG_HIDDEN_GUEST;
 }
 
-/* Visibility overrides for SVE-specific ID registers */
-static unsigned int sve_id_visibility(const struct kvm_vcpu *vcpu,
-				      const struct sys_reg_desc *rd)
-{
-	if (vcpu_has_sve(vcpu))
-		return 0;
-
-	return REG_HIDDEN_USER;
-}
-
 /* Generate the emulated ID_AA64ZFR0_EL1 value exposed to the guest */
 static u64 guest_id_aa64zfr0_el1(const struct kvm_vcpu *vcpu)
 {
@@ -1231,9 +1221,6 @@ static int get_id_aa64zfr0_el1(struct kvm_vcpu *vcpu,
 {
 	u64 val;
 
-	if (WARN_ON(!vcpu_has_sve(vcpu)))
-		return -ENOENT;
-
 	val = guest_id_aa64zfr0_el1(vcpu);
 	return reg_to_user(uaddr, &val, reg->id);
 }
@@ -1246,9 +1233,6 @@ static int set_id_aa64zfr0_el1(struct kvm_vcpu *vcpu,
 	int err;
 	u64 val;
 
-	if (WARN_ON(!vcpu_has_sve(vcpu)))
-		return -ENOENT;
-
 	err = reg_from_user(&val, uaddr, id);
 	if (err)
 		return err;
@@ -1518,7 +1502,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	ID_SANITISED(ID_AA64PFR1_EL1),
 	ID_UNALLOCATED(4,2),
 	ID_UNALLOCATED(4,3),
-	{ SYS_DESC(SYS_ID_AA64ZFR0_EL1), access_id_aa64zfr0_el1, .get_user = get_id_aa64zfr0_el1, .set_user = set_id_aa64zfr0_el1, .visibility = sve_id_visibility },
+	{ SYS_DESC(SYS_ID_AA64ZFR0_EL1), access_id_aa64zfr0_el1, .get_user = get_id_aa64zfr0_el1, .set_user = set_id_aa64zfr0_el1, },
 	ID_UNALLOCATED(4,5),
 	ID_UNALLOCATED(4,6),
 	ID_UNALLOCATED(4,7),
-- 
2.26.2

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 2/4] KVM: arm64: Consolidate REG_HIDDEN_GUEST/USER
  2020-11-05  9:10 [PATCH v3 0/4] KVM: arm64: Fix get-reg-list regression Andrew Jones
  2020-11-05  9:10   ` Andrew Jones
@ 2020-11-05  9:10 ` Andrew Jones
  2020-11-05  9:10 ` [PATCH v3 3/4] KVM: arm64: Check RAZ visibility in ID register accessors Andrew Jones
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Andrew Jones @ 2020-11-05  9:10 UTC (permalink / raw)
  To: kvmarm; +Cc: maz, xu910121, Dave.Martin

REG_HIDDEN_GUEST and REG_HIDDEN_USER are always used together.
Consolidate them into a single REG_HIDDEN flag. We can always
add another flag later if some register needs to expose itself
differently to the guest than it does to userspace.

No functional change intended.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 arch/arm64/kvm/sys_regs.c | 12 ++++++------
 arch/arm64/kvm/sys_regs.h | 18 ++++--------------
 2 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 6ff0c15531ca..1b227f4eb707 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1069,7 +1069,7 @@ static bool trap_ptrauth(struct kvm_vcpu *vcpu,
 static unsigned int ptrauth_visibility(const struct kvm_vcpu *vcpu,
 			const struct sys_reg_desc *rd)
 {
-	return vcpu_has_ptrauth(vcpu) ? 0 : REG_HIDDEN_USER | REG_HIDDEN_GUEST;
+	return vcpu_has_ptrauth(vcpu) ? 0 : REG_HIDDEN;
 }
 
 #define __PTRAUTH_KEY(k)						\
@@ -1192,7 +1192,7 @@ static unsigned int sve_visibility(const struct kvm_vcpu *vcpu,
 	if (vcpu_has_sve(vcpu))
 		return 0;
 
-	return REG_HIDDEN_USER | REG_HIDDEN_GUEST;
+	return REG_HIDDEN;
 }
 
 /* Generate the emulated ID_AA64ZFR0_EL1 value exposed to the guest */
@@ -2169,7 +2169,7 @@ static void perform_access(struct kvm_vcpu *vcpu,
 	trace_kvm_sys_access(*vcpu_pc(vcpu), params, r);
 
 	/* Check for regs disabled by runtime config */
-	if (sysreg_hidden_from_guest(vcpu, r)) {
+	if (sysreg_hidden(vcpu, r)) {
 		kvm_inject_undefined(vcpu);
 		return;
 	}
@@ -2668,7 +2668,7 @@ int kvm_arm_sys_reg_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg
 		return get_invariant_sys_reg(reg->id, uaddr);
 
 	/* Check for regs disabled by runtime config */
-	if (sysreg_hidden_from_user(vcpu, r))
+	if (sysreg_hidden(vcpu, r))
 		return -ENOENT;
 
 	if (r->get_user)
@@ -2693,7 +2693,7 @@ int kvm_arm_sys_reg_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg
 		return set_invariant_sys_reg(reg->id, uaddr);
 
 	/* Check for regs disabled by runtime config */
-	if (sysreg_hidden_from_user(vcpu, r))
+	if (sysreg_hidden(vcpu, r))
 		return -ENOENT;
 
 	if (r->set_user)
@@ -2764,7 +2764,7 @@ static int walk_one_sys_reg(const struct kvm_vcpu *vcpu,
 	if (!(rd->reg || rd->get_user))
 		return 0;
 
-	if (sysreg_hidden_from_user(vcpu, rd))
+	if (sysreg_hidden(vcpu, rd))
 		return 0;
 
 	if (!copy_reg_to_user(rd, uind))
diff --git a/arch/arm64/kvm/sys_regs.h b/arch/arm64/kvm/sys_regs.h
index 5a6fc30f5989..2641b2ee6a91 100644
--- a/arch/arm64/kvm/sys_regs.h
+++ b/arch/arm64/kvm/sys_regs.h
@@ -59,8 +59,7 @@ struct sys_reg_desc {
 				   const struct sys_reg_desc *rd);
 };
 
-#define REG_HIDDEN_USER		(1 << 0) /* hidden from userspace ioctls */
-#define REG_HIDDEN_GUEST	(1 << 1) /* hidden from guest */
+#define REG_HIDDEN		(1 << 0) /* hidden from userspace and guest */
 
 static __printf(2, 3)
 inline void print_sys_reg_msg(const struct sys_reg_params *p,
@@ -111,22 +110,13 @@ static inline void reset_val(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r
 	__vcpu_sys_reg(vcpu, r->reg) = r->val;
 }
 
-static inline bool sysreg_hidden_from_guest(const struct kvm_vcpu *vcpu,
-					    const struct sys_reg_desc *r)
-{
-	if (likely(!r->visibility))
-		return false;
-
-	return r->visibility(vcpu, r) & REG_HIDDEN_GUEST;
-}
-
-static inline bool sysreg_hidden_from_user(const struct kvm_vcpu *vcpu,
-					   const struct sys_reg_desc *r)
+static inline bool sysreg_hidden(const struct kvm_vcpu *vcpu,
+				 const struct sys_reg_desc *r)
 {
 	if (likely(!r->visibility))
 		return false;
 
-	return r->visibility(vcpu, r) & REG_HIDDEN_USER;
+	return r->visibility(vcpu, r) & REG_HIDDEN;
 }
 
 static inline int cmp_sys_reg(const struct sys_reg_desc *i1,
-- 
2.26.2

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 3/4] KVM: arm64: Check RAZ visibility in ID register accessors
  2020-11-05  9:10 [PATCH v3 0/4] KVM: arm64: Fix get-reg-list regression Andrew Jones
  2020-11-05  9:10   ` Andrew Jones
  2020-11-05  9:10 ` [PATCH v3 2/4] KVM: arm64: Consolidate REG_HIDDEN_GUEST/USER Andrew Jones
@ 2020-11-05  9:10 ` Andrew Jones
  2020-11-05  9:10 ` [PATCH v3 4/4] KVM: arm64: Remove AA64ZFR0_EL1 accessors Andrew Jones
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Andrew Jones @ 2020-11-05  9:10 UTC (permalink / raw)
  To: kvmarm; +Cc: maz, xu910121, Dave.Martin

The instruction encodings of ID registers are preallocated. Until an
encoding is assigned a purpose the register is RAZ. KVM's general ID
register accessor functions already support both paths, RAZ or not.
If for each ID register we can determine if it's RAZ or not, then all
ID registers can build on the general functions. The register visibility
function allows us to check whether a register should be completely
hidden or not, extending it to also report when the register should
be RAZ or not allows us to use it for ID registers as well.

Check for RAZ visibility in the ID register accessor functions,
allowing the RAZ case to be handled in a generic way for all system
registers.

The new REG_RAZ flag will be used in a later patch. This patch has
no intended functional change.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 arch/arm64/kvm/sys_regs.c | 19 ++++++++++++++++---
 arch/arm64/kvm/sys_regs.h | 10 ++++++++++
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 1b227f4eb707..47893b7e982f 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1153,6 +1153,12 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu,
 	return val;
 }
 
+static unsigned int id_visibility(const struct kvm_vcpu *vcpu,
+				  const struct sys_reg_desc *r)
+{
+	return 0;
+}
+
 /* cpufeature ID register access trap handlers */
 
 static bool __access_id_reg(struct kvm_vcpu *vcpu,
@@ -1171,7 +1177,9 @@ static bool access_id_reg(struct kvm_vcpu *vcpu,
 			  struct sys_reg_params *p,
 			  const struct sys_reg_desc *r)
 {
-	return __access_id_reg(vcpu, p, r, false);
+	bool raz = sysreg_visible_as_raz(vcpu, r);
+
+	return __access_id_reg(vcpu, p, r, raz);
 }
 
 static bool access_raz_id_reg(struct kvm_vcpu *vcpu,
@@ -1283,13 +1291,17 @@ static int __set_id_reg(const struct kvm_vcpu *vcpu,
 static int get_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
 		      const struct kvm_one_reg *reg, void __user *uaddr)
 {
-	return __get_id_reg(vcpu, rd, uaddr, false);
+	bool raz = sysreg_visible_as_raz(vcpu, rd);
+
+	return __get_id_reg(vcpu, rd, uaddr, raz);
 }
 
 static int set_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
 		      const struct kvm_one_reg *reg, void __user *uaddr)
 {
-	return __set_id_reg(vcpu, rd, uaddr, false);
+	bool raz = sysreg_visible_as_raz(vcpu, rd);
+
+	return __set_id_reg(vcpu, rd, uaddr, raz);
 }
 
 static int get_raz_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
@@ -1381,6 +1393,7 @@ static bool access_mte_regs(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
 	.access	= access_id_reg,		\
 	.get_user = get_id_reg,			\
 	.set_user = set_id_reg,			\
+	.visibility = id_visibility,		\
 }
 
 /*
diff --git a/arch/arm64/kvm/sys_regs.h b/arch/arm64/kvm/sys_regs.h
index 2641b2ee6a91..0f95964339b1 100644
--- a/arch/arm64/kvm/sys_regs.h
+++ b/arch/arm64/kvm/sys_regs.h
@@ -60,6 +60,7 @@ struct sys_reg_desc {
 };
 
 #define REG_HIDDEN		(1 << 0) /* hidden from userspace and guest */
+#define REG_RAZ			(1 << 1) /* RAZ from userspace and guest */
 
 static __printf(2, 3)
 inline void print_sys_reg_msg(const struct sys_reg_params *p,
@@ -119,6 +120,15 @@ static inline bool sysreg_hidden(const struct kvm_vcpu *vcpu,
 	return r->visibility(vcpu, r) & REG_HIDDEN;
 }
 
+static inline bool sysreg_visible_as_raz(const struct kvm_vcpu *vcpu,
+					 const struct sys_reg_desc *r)
+{
+	if (likely(!r->visibility))
+		return false;
+
+	return r->visibility(vcpu, r) & REG_RAZ;
+}
+
 static inline int cmp_sys_reg(const struct sys_reg_desc *i1,
 			      const struct sys_reg_desc *i2)
 {
-- 
2.26.2

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 4/4] KVM: arm64: Remove AA64ZFR0_EL1 accessors
  2020-11-05  9:10 [PATCH v3 0/4] KVM: arm64: Fix get-reg-list regression Andrew Jones
                   ` (2 preceding siblings ...)
  2020-11-05  9:10 ` [PATCH v3 3/4] KVM: arm64: Check RAZ visibility in ID register accessors Andrew Jones
@ 2020-11-05  9:10 ` Andrew Jones
  2020-11-06 16:35 ` [PATCH v3 0/4] KVM: arm64: Fix get-reg-list regression Marc Zyngier
  2020-11-10 11:13 ` Dave Martin
  5 siblings, 0 replies; 8+ messages in thread
From: Andrew Jones @ 2020-11-05  9:10 UTC (permalink / raw)
  To: kvmarm; +Cc: maz, xu910121, Dave.Martin

The AA64ZFR0_EL1 accessors are just the general accessors with
its visibility function open-coded. It also skips the if-else
chain in read_id_reg, but there's no reason not to go there.
Indeed consolidating ID register accessors and removing lines
of code make it worthwhile.

Remove the AA64ZFR0_EL1 accessors, replacing them with the
general accessors for sanitized ID registers.

No functional change intended.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 arch/arm64/kvm/sys_regs.c | 61 +++++++--------------------------------
 1 file changed, 11 insertions(+), 50 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 47893b7e982f..d0868d0e8ff4 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1156,6 +1156,16 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu,
 static unsigned int id_visibility(const struct kvm_vcpu *vcpu,
 				  const struct sys_reg_desc *r)
 {
+	u32 id = sys_reg((u32)r->Op0, (u32)r->Op1,
+			 (u32)r->CRn, (u32)r->CRm, (u32)r->Op2);
+
+	switch (id) {
+	case SYS_ID_AA64ZFR0_EL1:
+		if (!vcpu_has_sve(vcpu))
+			return REG_RAZ;
+		break;
+	}
+
 	return 0;
 }
 
@@ -1203,55 +1213,6 @@ static unsigned int sve_visibility(const struct kvm_vcpu *vcpu,
 	return REG_HIDDEN;
 }
 
-/* Generate the emulated ID_AA64ZFR0_EL1 value exposed to the guest */
-static u64 guest_id_aa64zfr0_el1(const struct kvm_vcpu *vcpu)
-{
-	if (!vcpu_has_sve(vcpu))
-		return 0;
-
-	return read_sanitised_ftr_reg(SYS_ID_AA64ZFR0_EL1);
-}
-
-static bool access_id_aa64zfr0_el1(struct kvm_vcpu *vcpu,
-				   struct sys_reg_params *p,
-				   const struct sys_reg_desc *rd)
-{
-	if (p->is_write)
-		return write_to_read_only(vcpu, p, rd);
-
-	p->regval = guest_id_aa64zfr0_el1(vcpu);
-	return true;
-}
-
-static int get_id_aa64zfr0_el1(struct kvm_vcpu *vcpu,
-		const struct sys_reg_desc *rd,
-		const struct kvm_one_reg *reg, void __user *uaddr)
-{
-	u64 val;
-
-	val = guest_id_aa64zfr0_el1(vcpu);
-	return reg_to_user(uaddr, &val, reg->id);
-}
-
-static int set_id_aa64zfr0_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;
-
-	err = reg_from_user(&val, uaddr, id);
-	if (err)
-		return err;
-
-	/* This is what we mean by invariant: you can't change it. */
-	if (val != guest_id_aa64zfr0_el1(vcpu))
-		return -EINVAL;
-
-	return 0;
-}
-
 /*
  * cpufeature ID register user accessors
  *
@@ -1515,7 +1476,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	ID_SANITISED(ID_AA64PFR1_EL1),
 	ID_UNALLOCATED(4,2),
 	ID_UNALLOCATED(4,3),
-	{ SYS_DESC(SYS_ID_AA64ZFR0_EL1), access_id_aa64zfr0_el1, .get_user = get_id_aa64zfr0_el1, .set_user = set_id_aa64zfr0_el1, },
+	ID_SANITISED(ID_AA64ZFR0_EL1),
 	ID_UNALLOCATED(4,5),
 	ID_UNALLOCATED(4,6),
 	ID_UNALLOCATED(4,7),
-- 
2.26.2

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 0/4] KVM: arm64: Fix get-reg-list regression
  2020-11-05  9:10 [PATCH v3 0/4] KVM: arm64: Fix get-reg-list regression Andrew Jones
                   ` (3 preceding siblings ...)
  2020-11-05  9:10 ` [PATCH v3 4/4] KVM: arm64: Remove AA64ZFR0_EL1 accessors Andrew Jones
@ 2020-11-06 16:35 ` Marc Zyngier
  2020-11-10 11:13 ` Dave Martin
  5 siblings, 0 replies; 8+ messages in thread
From: Marc Zyngier @ 2020-11-06 16:35 UTC (permalink / raw)
  To: Andrew Jones, kvmarm; +Cc: xu910121, Dave.Martin

On Thu, 5 Nov 2020 10:10:18 +0100, Andrew Jones wrote:
> 张东旭 <xu910121@sina.com> reported a regression seen with CentOS
> when migrating from an old kernel to a new one. The problem was
> that QEMU rejected the migration since KVM_GET_REG_LIST reported
> a register was missing on the destination. Extra registers are OK
> on the destination, but not missing ones. The regression reproduces
> with upstream kernels when migrating from a 4.15 or later kernel,
> up to one with commit 73433762fcae ("KVM: arm64/sve: System register
> context switch and access support"), to a kernel that includes that
> commit, e.g. the latest mainline (5.10-rc2).
> 
> [...]

Applied to next, thanks!

[1/4] KVM: arm64: Don't hide ID registers from userspace
      commit: f81cb2c3ad41ac6d8cb2650e3d72d5f67db1aa28
[2/4] KVM: arm64: Consolidate REG_HIDDEN_GUEST/USER
      commit: 01fe5ace92ddb8732e3331355e7ba9cb6f2ef787
[3/4] KVM: arm64: Check RAZ visibility in ID register accessors
      commit: 912dee572691ffb2b387dd8b4f183d549a6b24d1
[4/4] KVM: arm64: Remove AA64ZFR0_EL1 accessors
      commit: c512298eed0360923d0cbc4a1f30bc0509af0d50

Cheers,

	M.
-- 
Without deviation from the norm, progress is not possible.


_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 0/4] KVM: arm64: Fix get-reg-list regression
  2020-11-05  9:10 [PATCH v3 0/4] KVM: arm64: Fix get-reg-list regression Andrew Jones
                   ` (4 preceding siblings ...)
  2020-11-06 16:35 ` [PATCH v3 0/4] KVM: arm64: Fix get-reg-list regression Marc Zyngier
@ 2020-11-10 11:13 ` Dave Martin
  5 siblings, 0 replies; 8+ messages in thread
From: Dave Martin @ 2020-11-10 11:13 UTC (permalink / raw)
  To: Andrew Jones; +Cc: maz, xu910121, kvmarm

On Thu, Nov 05, 2020 at 10:10:18AM +0100, Andrew Jones wrote:
> 张东旭 <xu910121@sina.com> reported a regression seen with CentOS
> when migrating from an old kernel to a new one. The problem was
> that QEMU rejected the migration since KVM_GET_REG_LIST reported
> a register was missing on the destination. Extra registers are OK
> on the destination, but not missing ones. The regression reproduces
> with upstream kernels when migrating from a 4.15 or later kernel,
> up to one with commit 73433762fcae ("KVM: arm64/sve: System register
> context switch and access support"), to a kernel that includes that
> commit, e.g. the latest mainline (5.10-rc2).
> 
> The first patch of this series is the fix. The next two patches,
> which don't have any intended functional changes, allow ID_SANITISED
> to be used for registers that flip between exposing features and
> being RAZ, which allows some code to be removed.
> 
> v3:
>  - Improve commit messages [Dave]
>  - Add new patch to consolidate REG_HIDDEN* flags [Dave]
> 
> v2:
>  - CC stable [Marc]
>  - Only one RAZ flag is enough [Marc]
>  - Move id_visibility() up by read_id_reg() since they'll likely
>    be maintained together [drew]
> 
> 
> Andrew Jones (4):
>   KVM: arm64: Don't hide ID registers from userspace
>   KVM: arm64: Consolidate REG_HIDDEN_GUEST/USER
>   KVM: arm64: Check RAZ visibility in ID register accessors
>   KVM: arm64: Remove AA64ZFR0_EL1 accessors
> 
>  arch/arm64/kvm/sys_regs.c | 108 ++++++++++++--------------------------
>  arch/arm64/kvm/sys_regs.h |  16 +++---
>  2 files changed, 41 insertions(+), 83 deletions(-)

Thanks for the updates.

Looks like I missed the opportunity to review this, but just for the
record (even if it doesn't appear in the tree):

Reviewed-by: Dave Martin <Dave.Martin@arm.com>

Cheers
---Dave
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-11-10 11:13 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-05  9:10 [PATCH v3 0/4] KVM: arm64: Fix get-reg-list regression Andrew Jones
2020-11-05  9:10 ` [PATCH v3 1/4] KVM: arm64: Don't hide ID registers from userspace Andrew Jones
2020-11-05  9:10   ` Andrew Jones
2020-11-05  9:10 ` [PATCH v3 2/4] KVM: arm64: Consolidate REG_HIDDEN_GUEST/USER Andrew Jones
2020-11-05  9:10 ` [PATCH v3 3/4] KVM: arm64: Check RAZ visibility in ID register accessors Andrew Jones
2020-11-05  9:10 ` [PATCH v3 4/4] KVM: arm64: Remove AA64ZFR0_EL1 accessors Andrew Jones
2020-11-06 16:35 ` [PATCH v3 0/4] KVM: arm64: Fix get-reg-list regression Marc Zyngier
2020-11-10 11:13 ` Dave Martin

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.