kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 00/25] KVM: arm64: Make CPU ID registers writable by userspace
@ 2021-10-12  4:35 Reiji Watanabe
  2021-10-12  4:35 ` [RFC PATCH 01/25] KVM: arm64: Add has_reset_once flag for vcpu Reiji Watanabe
                   ` (24 more replies)
  0 siblings, 25 replies; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-12  4:35 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Will Deacon, Andrew Jones,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

In KVM/arm64, values of ID registers for a guest are mostly same as
its host's values except for bits for feature that KVM doesn't support
and for opt-in features that userspace didn't configure.  Userspace
can use KVM_SET_ONE_REG to a set ID register value, but it fails
if userspace attempts to modify the register value.

This patch series adds support to allow userspace to modify a value of
ID registers (as long as KVM can support features that are indicated
in the registers) so userspace can have more control of configuring
and unconfiguring features for guests.
The patch series affects both VHE or non-VHE including protected VMs
for now but should be changed not to affect for protected VMs, which
will have a different way of configuring ID registers [1] based on
its different requirements.
There was a patch series that tried to achieve the same thing [2].
A few snippets of codes in this series were inspired by or came from [2].

Since an initial value of ID registers will be the host value with bits
cleared for unsupported features and for opt-in features that were not
configured, the initial value userspace can see (via KVM_GET_ONE_REG) is
the upper limit that can be set for the register.  Any requests to change
the value that conflicts with opt-in features' configuration will fail.

When a guest tries to use a CPU feature that is not exposed to the guest,
trapping it (to emulate a real CPU's behavior) would generally be a
desirable behavior (when it's possible with no or little side effects).
The later patches in the series add codes for this.  Only features that
can be trapped independently will be trapped by this series though.

The series is based on 5.15-rc5 with the patch series [3] applied.

Patch 01 introduces 'has_reset_once' flag for a vCPU to indicate if the
vCPU reset has been done once.  This is used to initialize ID registers
only at the first vCPU reset.

Patch 02 extends sys_regs[] of kvm_cpu_context to save values of ID
registers for the vCPU. For now, the sanitized host's values are saved
in the array at the first vCPU reset.

Patch 03 introduces arm64_check_features(), which will do a common
validation checking for ID registers.

Patch 04 introduces structure id_reg_info to manage the ID register
specific control of the register value for the guest.

Patch 05 introduces a function to keep consistency of ID register values
between vCPUs at the first KVM_RUN. Also, the patch adds code to prevent
userspace from changing ID register value after the first KVM_RUN.

Patches 06-12 add id_reg_info for ID registers to make them
writable with some specific handling for the registers.

Patch 13 changes KVM_SET_ONE_REG behavior for ID registers to
allow userspace to change ID registers that don't have id_reg_info.

Patch 14 introduces validity checking of feature fractional
fields of ID registers at the first KVM_RUN.

Patch 15 introduces a new capability KVM_CAP_ARM_ID_REG_WRITABLE
to identify that ID registers are writable by userspace.

Patches 16-17 changes the way of using vcpu->arch.cptr_el2/mdcr_el2 to
track certain bits of cptr_el2/mdcr_el2 in the vcpu->arch fields and use
them when setting them for the guest.  The following patches will update
the vcpu->arch fields based on available features for the guest.

Patch 18 introduces struct feature_config_ctrl and some utility
functions to enable trapping of features that are disabled for a guest.

Patches 19-23 add feature_config_ctrl for CPU features, which are
used to program configuration registers to trap each feature.

Patch 24 enables trapping CPU features that are disabled for the
guest based on feature_config_ctrl that were added by patch 18-23.

Patch 25 adds a selftest to validate reading/writing ID registers.

[1] https://lore.kernel.org/kvmarm/20211010145636.1950948-1-tabba@google.com/
[2] https://lore.kernel.org/kvm/20201102033422.657391-1-liangpeng10@huawei.com/
[3] https://lore.kernel.org/kvmarm/20211007233439.1826892-1-rananta@google.com/

Reiji Watanabe (25):
  KVM: arm64: Add has_reset_once flag for vcpu
  KVM: arm64: Save ID registers' sanitized value per vCPU
  KVM: arm64: Introduce a validation function for an ID register
  KVM: arm64: Introduce struct id_reg_info
  KVM: arm64: Keep consistency of ID registers between vCPUs
  KVM: arm64: Make ID_AA64PFR0_EL1 writable
  KVM: arm64: Make ID_AA64PFR1_EL1 writable
  KVM: arm64: Make ID_AA64ISAR0_EL1 writable
  KVM: arm64: Make ID_AA64ISAR1_EL1 writable
  KVM: arm64: Make ID_AA64DFR0_EL1 writable
  KVM: arm64: Make ID_DFR0_EL1 writable
  KVM: arm64: Make MVFR1_EL1 writable
  KVM: arm64: Make ID registers without id_reg_info writable
  KVM: arm64: Add consistency checking for frac fields of ID registers
  KVM: arm64: Introduce KVM_CAP_ARM_ID_REG_WRITABLE capability
  KVM: arm64: Use vcpu->arch cptr_el2 to track value of cptr_el2 for VHE
  KVM: arm64: Use vcpu->arch.mdcr_el2 to track value of mdcr_el2
  KVM: arm64: Introduce framework to trap disabled features
  KVM: arm64: Trap disabled features of ID_AA64PFR0_EL1
  KVM: arm64: Trap disabled features of ID_AA64PFR1_EL1
  KVM: arm64: Trap disabled features of ID_AA64DFR0_EL1
  KVM: arm64: Trap disabled features of ID_AA64MMFR1_EL1
  KVM: arm64: Trap disabled features of ID_AA64ISAR1_EL1
  KVM: arm64: Activate trapping of disabled CPU features for the guest
  KVM: arm64: selftests: Introduce id_reg_test

 Documentation/virt/kvm/api.rst                |    8 +
 arch/arm64/include/asm/cpufeature.h           |    1 +
 arch/arm64/include/asm/kvm_arm.h              |   32 +
 arch/arm64/include/asm/kvm_host.h             |   18 +-
 arch/arm64/include/asm/sysreg.h               |    1 +
 arch/arm64/kernel/cpufeature.c                |   26 +
 arch/arm64/kvm/arm.c                          |   30 +-
 arch/arm64/kvm/debug.c                        |   13 +-
 arch/arm64/kvm/hyp/vhe/switch.c               |   14 +-
 arch/arm64/kvm/reset.c                        |    4 +
 arch/arm64/kvm/sys_regs.c                     | 1010 +++++++++++--
 include/uapi/linux/kvm.h                      |    1 +
 tools/arch/arm64/include/asm/sysreg.h         |    1 +
 tools/testing/selftests/kvm/.gitignore        |    1 +
 tools/testing/selftests/kvm/Makefile          |    1 +
 .../selftests/kvm/aarch64/id_reg_test.c       | 1296 +++++++++++++++++
 16 files changed, 2306 insertions(+), 151 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/aarch64/id_reg_test.c

-- 
2.33.0.882.g93a45727a2-goog


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

* [RFC PATCH 01/25] KVM: arm64: Add has_reset_once flag for vcpu
  2021-10-12  4:35 [RFC PATCH 00/25] KVM: arm64: Make CPU ID registers writable by userspace Reiji Watanabe
@ 2021-10-12  4:35 ` Reiji Watanabe
  2021-10-15 10:12   ` Andrew Jones
  2021-10-12  4:35 ` [RFC PATCH 02/25] KVM: arm64: Save ID registers' sanitized value per vCPU Reiji Watanabe
                   ` (23 subsequent siblings)
  24 siblings, 1 reply; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-12  4:35 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Will Deacon, Andrew Jones,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

Introduce 'has_reset_once' flag in kvm_vcpu_arch, which indicates
if the vCPU reset has been done once, for later use.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/include/asm/kvm_host.h | 2 ++
 arch/arm64/kvm/reset.c            | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index f8be56d5342b..9b5e7a3b6011 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -384,6 +384,7 @@ struct kvm_vcpu_arch {
 		u64 last_steal;
 		gpa_t base;
 	} steal;
+	bool has_reset_once;
 };
 
 /* Pointer to the vcpu's SVE FFR for sve_{save,load}_state() */
@@ -449,6 +450,7 @@ struct kvm_vcpu_arch {
 
 #define vcpu_has_sve(vcpu) (system_supports_sve() &&			\
 			    ((vcpu)->arch.flags & KVM_ARM64_GUEST_HAS_SVE))
+#define	vcpu_has_reset_once(vcpu) ((vcpu)->arch.has_reset_once)
 
 #ifdef CONFIG_ARM64_PTR_AUTH
 #define vcpu_has_ptrauth(vcpu)						\
diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index 5ce36b0a3343..4d34e5c1586c 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -305,6 +305,10 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
 	if (loaded)
 		kvm_arch_vcpu_load(vcpu, smp_processor_id());
 	preempt_enable();
+
+	if (!ret && !vcpu->arch.has_reset_once)
+		vcpu->arch.has_reset_once = true;
+
 	return ret;
 }
 
-- 
2.33.0.882.g93a45727a2-goog


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

* [RFC PATCH 02/25] KVM: arm64: Save ID registers' sanitized value per vCPU
  2021-10-12  4:35 [RFC PATCH 00/25] KVM: arm64: Make CPU ID registers writable by userspace Reiji Watanabe
  2021-10-12  4:35 ` [RFC PATCH 01/25] KVM: arm64: Add has_reset_once flag for vcpu Reiji Watanabe
@ 2021-10-12  4:35 ` Reiji Watanabe
  2021-10-15 13:09   ` Andrew Jones
  2021-10-12  4:35 ` [RFC PATCH 03/25] KVM: arm64: Introduce a validation function for an ID register Reiji Watanabe
                   ` (22 subsequent siblings)
  24 siblings, 1 reply; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-12  4:35 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Will Deacon, Andrew Jones,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

Extend sys_regs[] of kvm_cpu_context for ID registers and save ID
registers' sanitized value in the array for the vCPU at the first
vCPU reset. Use the saved ones when ID registers are read by
userspace (via KVM_GET_ONE_REG) or the guest.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/include/asm/kvm_host.h | 10 ++++++++++
 arch/arm64/kvm/sys_regs.c         | 26 ++++++++++++++++++--------
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 9b5e7a3b6011..0cd351099adf 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -145,6 +145,14 @@ struct kvm_vcpu_fault_info {
 	u64 disr_el1;		/* Deferred [SError] Status Register */
 };
 
+/*
+ * (Op0, Op1, CRn, CRm, Op2) of ID registers is (3, 0, 0, crm, op2),
+ * where 0<=crm<8, 0<=op2<8.
+ */
+#define KVM_ARM_ID_REG_MAX_NUM 64
+#define IDREG_IDX(id)		((sys_reg_CRm(id) << 3) | sys_reg_Op2(id))
+#define IDREG_SYS_IDX(id)	(ID_REG_BASE + IDREG_IDX(id))
+
 enum vcpu_sysreg {
 	__INVALID_SYSREG__,   /* 0 is reserved as an invalid value */
 	MPIDR_EL1,	/* MultiProcessor Affinity Register */
@@ -209,6 +217,8 @@ enum vcpu_sysreg {
 	CNTP_CVAL_EL0,
 	CNTP_CTL_EL0,
 
+	ID_REG_BASE,
+	ID_REG_END = ID_REG_BASE + KVM_ARM_ID_REG_MAX_NUM - 1,
 	/* Memory Tagging Extension registers */
 	RGSR_EL1,	/* Random Allocation Tag Seed Register */
 	GCR_EL1,	/* Tag Control Register */
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 1d46e185f31e..72ca518e7944 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -273,7 +273,7 @@ static bool trap_loregion(struct kvm_vcpu *vcpu,
 			  struct sys_reg_params *p,
 			  const struct sys_reg_desc *r)
 {
-	u64 val = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1);
+	u64 val = __vcpu_sys_reg(vcpu, IDREG_SYS_IDX(SYS_ID_AA64MMFR1_EL1));
 	u32 sr = reg_to_encoding(r);
 
 	if (!(val & (0xfUL << ID_AA64MMFR1_LOR_SHIFT))) {
@@ -1059,12 +1059,11 @@ static bool access_arch_timer(struct kvm_vcpu *vcpu,
 	return true;
 }
 
-/* Read a sanitised cpufeature ID register by sys_reg_desc */
 static u64 read_id_reg(const struct kvm_vcpu *vcpu,
 		struct sys_reg_desc const *r, bool raz)
 {
 	u32 id = reg_to_encoding(r);
-	u64 val = raz ? 0 : read_sanitised_ftr_reg(id);
+	u64 val = raz ? 0 : __vcpu_sys_reg(vcpu, IDREG_SYS_IDX(id));
 
 	switch (id) {
 	case SYS_ID_AA64PFR0_EL1:
@@ -1174,6 +1173,16 @@ static unsigned int sve_visibility(const struct kvm_vcpu *vcpu,
 	return REG_HIDDEN;
 }
 
+static void reset_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd)
+{
+	u32 id = reg_to_encoding(rd);
+
+	if (vcpu_has_reset_once(vcpu))
+		return;
+
+	__vcpu_sys_reg(vcpu, IDREG_SYS_IDX(id)) = read_sanitised_ftr_reg(id);
+}
+
 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)
@@ -1219,9 +1228,7 @@ static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
 /*
  * cpufeature ID register user accessors
  *
- * For now, these registers are immutable for userspace, so no values
- * are stored, and for set_id_reg() we don't allow the effective value
- * to be changed.
+ * We don't allow the effective value to be changed.
  */
 static int __get_id_reg(const struct kvm_vcpu *vcpu,
 			const struct sys_reg_desc *rd, void __user *uaddr,
@@ -1375,6 +1382,7 @@ static unsigned int mte_visibility(const struct kvm_vcpu *vcpu,
 #define ID_SANITISED(name) {			\
 	SYS_DESC(SYS_##name),			\
 	.access	= access_id_reg,		\
+	.reset	= reset_id_reg,			\
 	.get_user = get_id_reg,			\
 	.set_user = set_id_reg,			\
 	.visibility = id_visibility,		\
@@ -1830,8 +1838,10 @@ static bool trap_dbgdidr(struct kvm_vcpu *vcpu,
 	if (p->is_write) {
 		return ignore_write(vcpu, p);
 	} else {
-		u64 dfr = read_sanitised_ftr_reg(SYS_ID_AA64DFR0_EL1);
-		u64 pfr = read_sanitised_ftr_reg(SYS_ID_AA64PFR0_EL1);
+		u64 dfr = __vcpu_sys_reg(vcpu,
+					 IDREG_SYS_IDX(SYS_ID_AA64DFR0_EL1));
+		u64 pfr = __vcpu_sys_reg(vcpu,
+					 IDREG_SYS_IDX(SYS_ID_AA64PFR0_EL1));
 		u32 el3 = !!cpuid_feature_extract_unsigned_field(pfr, ID_AA64PFR0_EL3_SHIFT);
 
 		p->regval = ((((dfr >> ID_AA64DFR0_WRPS_SHIFT) & 0xf) << 28) |
-- 
2.33.0.882.g93a45727a2-goog


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

* [RFC PATCH 03/25] KVM: arm64: Introduce a validation function for an ID register
  2021-10-12  4:35 [RFC PATCH 00/25] KVM: arm64: Make CPU ID registers writable by userspace Reiji Watanabe
  2021-10-12  4:35 ` [RFC PATCH 01/25] KVM: arm64: Add has_reset_once flag for vcpu Reiji Watanabe
  2021-10-12  4:35 ` [RFC PATCH 02/25] KVM: arm64: Save ID registers' sanitized value per vCPU Reiji Watanabe
@ 2021-10-12  4:35 ` Reiji Watanabe
  2021-10-15 13:30   ` Andrew Jones
  2021-10-12  4:35 ` [RFC PATCH 04/25] KVM: arm64: Introduce struct id_reg_info Reiji Watanabe
                   ` (21 subsequent siblings)
  24 siblings, 1 reply; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-12  4:35 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Will Deacon, Andrew Jones,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

Introduce arm64_check_features(), which does a basic validity checking
of an ID register value against the register's limit value that KVM
can support.
This function will be used by the following patches to check if an ID
register value that userspace tries to set can be supported by KVM on
the host.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/include/asm/cpufeature.h |  1 +
 arch/arm64/kernel/cpufeature.c      | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index ef6be92b1921..eda7ddbed8cf 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -631,6 +631,7 @@ void check_local_cpu_capabilities(void);
 
 u64 read_sanitised_ftr_reg(u32 id);
 u64 __read_sysreg_by_encoding(u32 sys_id);
+int arm64_check_features(u32 sys_reg, u64 val, u64 limit);
 
 static inline bool cpu_supports_mixed_endian_el0(void)
 {
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 6ec7036ef7e1..d146ef759435 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -3114,3 +3114,29 @@ ssize_t cpu_show_meltdown(struct device *dev, struct device_attribute *attr,
 		return sprintf(buf, "Vulnerable\n");
 	}
 }
+
+/*
+ * Check if all features that are indicated in the given ID register value
+ * ('val') are also indicated in the 'limit'.
+ */
+int arm64_check_features(u32 sys_reg, u64 val, u64 limit)
+{
+	struct arm64_ftr_reg *reg = get_arm64_ftr_reg(sys_reg);
+	const struct arm64_ftr_bits *ftrp;
+	u64 exposed_mask = 0;
+
+	if (!reg)
+		return -ENOENT;
+
+	for (ftrp = reg->ftr_bits; ftrp->width; ftrp++) {
+		if (arm64_ftr_value(ftrp, val) > arm64_ftr_value(ftrp, limit))
+			return -E2BIG;
+
+		exposed_mask |= arm64_ftr_mask(ftrp);
+	}
+
+	if (val & ~exposed_mask)
+		return -E2BIG;
+
+	return 0;
+}
-- 
2.33.0.882.g93a45727a2-goog


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

* [RFC PATCH 04/25] KVM: arm64: Introduce struct id_reg_info
  2021-10-12  4:35 [RFC PATCH 00/25] KVM: arm64: Make CPU ID registers writable by userspace Reiji Watanabe
                   ` (2 preceding siblings ...)
  2021-10-12  4:35 ` [RFC PATCH 03/25] KVM: arm64: Introduce a validation function for an ID register Reiji Watanabe
@ 2021-10-12  4:35 ` Reiji Watanabe
  2021-10-15 13:47   ` Andrew Jones
  2021-10-12  4:35 ` [RFC PATCH 05/25] KVM: arm64: Keep consistency of ID registers between vCPUs Reiji Watanabe
                   ` (20 subsequent siblings)
  24 siblings, 1 reply; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-12  4:35 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Will Deacon, Andrew Jones,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

This patch lays the groundwork to make ID registers writable.

Introduce struct id_reg_info for an ID register to manage the
register specific control of its value for the guest.
It is used to do register specific initialization, validation
of the ID register and etc.  Not all ID registers must have
the id_reg_info. ID registers that don't have the id_reg_info
are handled in a common way that is applied to all other
ID registers.

At present, changing an ID register from userspace is allowed only
if the ID register has the id_reg_info, but that will be changed
by the following patches.

No ID register has the structure yet and the following patches
will add the id_reg_info for some ID registers.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/kvm/sys_regs.c | 120 +++++++++++++++++++++++++++++++++++---
 1 file changed, 111 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 72ca518e7944..8a0b88f9a975 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -263,6 +263,76 @@ static bool trap_raz_wi(struct kvm_vcpu *vcpu,
 		return read_zero(vcpu, p);
 }
 
+struct id_reg_info {
+	u32	sys_reg;	/* Register ID */
+	u64	sys_val;	/* Sanitized system value */
+
+	/*
+	 * Limit value of the register for a vcpu. The value is sys_val
+	 * with bits cleared for unsupported features for the guest.
+	 */
+	u64	vcpu_limit_val;
+
+	/* Bits that are not validated when userspace sets the register. */
+	u64	ignore_mask;
+
+	/* Initialization function of the id_reg_info */
+	void (*init)(struct id_reg_info *id_reg);
+
+	/* Register specific validation function */
+	int (*validate)(struct kvm_vcpu *vcpu, u64 val);
+
+	/* Return the reset value of the register for the vCPU */
+	u64 (*get_reset_val)(struct kvm_vcpu *vcpu, struct id_reg_info *id_reg);
+};
+
+static void id_reg_info_init(struct id_reg_info *id_reg)
+{
+	id_reg->sys_val = read_sanitised_ftr_reg(id_reg->sys_reg);
+	id_reg->vcpu_limit_val = id_reg->sys_val;
+}
+
+/*
+ * An ID register that needs special handling to control the value for the
+ * guest must have its own id_reg_info in id_reg_info_table.
+ * (i.e. the reset value is different from the host's sanitized value,
+ * the value is affected by opt-in features, some fields needs specific
+ * validation, etc.)
+ */
+#define	GET_ID_REG_INFO(id)	(id_reg_info_table[IDREG_IDX(id)])
+static struct id_reg_info *id_reg_info_table[KVM_ARM_ID_REG_MAX_NUM] = {};
+
+static int validate_id_reg(struct kvm_vcpu *vcpu,
+			   const struct sys_reg_desc *rd, u64 val)
+{
+	u32 id = reg_to_encoding(rd);
+	const struct id_reg_info *id_reg = GET_ID_REG_INFO(id);
+	u64 limit;
+	u64 check_val = val;
+	int err;
+
+	if (id_reg) {
+		/*
+		 * Clear bits with ignore_mask, which we don't want
+		 * arm64_check_features to check.
+		 */
+		check_val &= ~id_reg->ignore_mask;
+		limit = id_reg->vcpu_limit_val;
+	} else
+		limit = read_sanitised_ftr_reg(id);
+
+	/* Check if the value indicates any feature that is not in the limit. */
+	err = arm64_check_features(id, check_val, limit);
+	if (err)
+		return err;
+
+	if (id_reg && id_reg->validate)
+		/* Run the ID register specific validity check. */
+		err = id_reg->validate(vcpu, val);
+
+	return err;
+}
+
 /*
  * ARMv8.1 mandates at least a trivial LORegion implementation, where all the
  * RW registers are RES0 (which we can implement as RAZ/WI). On an ARMv8.0
@@ -1176,11 +1246,19 @@ static unsigned int sve_visibility(const struct kvm_vcpu *vcpu,
 static void reset_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd)
 {
 	u32 id = reg_to_encoding(rd);
+	struct id_reg_info *id_reg;
+	u64 val;
 
 	if (vcpu_has_reset_once(vcpu))
 		return;
 
-	__vcpu_sys_reg(vcpu, IDREG_SYS_IDX(id)) = read_sanitised_ftr_reg(id);
+	id_reg = GET_ID_REG_INFO(id);
+	if (id_reg && id_reg->get_reset_val)
+		val = id_reg->get_reset_val(vcpu, id_reg);
+	else
+		val = read_sanitised_ftr_reg(id);
+
+	__vcpu_sys_reg(vcpu, IDREG_SYS_IDX(id)) = val;
 }
 
 static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
@@ -1225,11 +1303,7 @@ static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
 	return 0;
 }
 
-/*
- * cpufeature ID register user accessors
- *
- * We don't allow the effective value to be changed.
- */
+/* cpufeature ID register user accessors */
 static int __get_id_reg(const struct kvm_vcpu *vcpu,
 			const struct sys_reg_desc *rd, void __user *uaddr,
 			bool raz)
@@ -1240,11 +1314,12 @@ static int __get_id_reg(const struct kvm_vcpu *vcpu,
 	return reg_to_user(uaddr, &val, id);
 }
 
-static int __set_id_reg(const struct kvm_vcpu *vcpu,
+static int __set_id_reg(struct kvm_vcpu *vcpu,
 			const struct sys_reg_desc *rd, void __user *uaddr,
 			bool raz)
 {
 	const u64 id = sys_reg_to_index(rd);
+	u32 encoding = reg_to_encoding(rd);
 	int err;
 	u64 val;
 
@@ -1252,10 +1327,18 @@ static int __set_id_reg(const struct kvm_vcpu *vcpu,
 	if (err)
 		return err;
 
-	/* This is what we mean by invariant: you can't change it. */
-	if (val != read_id_reg(vcpu, rd, raz))
+	/* Don't allow to change the reg unless the reg has id_reg_info */
+	if (val != read_id_reg(vcpu, rd, raz) && !GET_ID_REG_INFO(encoding))
 		return -EINVAL;
 
+	if (raz)
+		return (val == 0) ? 0 : -EINVAL;
+
+	err = validate_id_reg(vcpu, rd, val);
+	if (err)
+		return err;
+
+	__vcpu_sys_reg(vcpu, IDREG_SYS_IDX(encoding)) = val;
 	return 0;
 }
 
@@ -2818,6 +2901,23 @@ int kvm_arm_copy_sys_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices)
 	return write_demux_regids(uindices);
 }
 
+static void id_reg_info_init_all(void)
+{
+	int i;
+	struct id_reg_info *id_reg;
+
+	for (i = 0; i < ARRAY_SIZE(id_reg_info_table); i++) {
+		id_reg = (struct id_reg_info *)id_reg_info_table[i];
+		if (!id_reg)
+			continue;
+
+		if (id_reg->init)
+			id_reg->init(id_reg);
+		else
+			id_reg_info_init(id_reg);
+	}
+}
+
 void kvm_sys_reg_table_init(void)
 {
 	unsigned int i;
@@ -2852,4 +2952,6 @@ void kvm_sys_reg_table_init(void)
 			break;
 	/* Clear all higher bits. */
 	cache_levels &= (1 << (i*3))-1;
+
+	id_reg_info_init_all();
 }
-- 
2.33.0.882.g93a45727a2-goog


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

* [RFC PATCH 05/25] KVM: arm64: Keep consistency of ID registers between vCPUs
  2021-10-12  4:35 [RFC PATCH 00/25] KVM: arm64: Make CPU ID registers writable by userspace Reiji Watanabe
                   ` (3 preceding siblings ...)
  2021-10-12  4:35 ` [RFC PATCH 04/25] KVM: arm64: Introduce struct id_reg_info Reiji Watanabe
@ 2021-10-12  4:35 ` Reiji Watanabe
  2021-10-12  4:35 ` [RFC PATCH 06/25] KVM: arm64: Make ID_AA64PFR0_EL1 writable Reiji Watanabe
                   ` (19 subsequent siblings)
  24 siblings, 0 replies; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-12  4:35 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Will Deacon, Andrew Jones,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

All vCPUs that are owned by a VM must have the same values of ID
registers.

Return an error at the very first KVM_RUN for a vCPU if the vCPU has
different values in any ID registers from any other vCPUs that have
already started KVM_RUN once.  Also, return an error if userspace
tries to change a value of ID register for a vCPU that already
started KVM_RUN once.

Changing ID register is still not allowed at present though.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/include/asm/kvm_host.h |  2 ++
 arch/arm64/kvm/arm.c              |  3 +++
 arch/arm64/kvm/sys_regs.c         | 31 +++++++++++++++++++++++++++++++
 3 files changed, 36 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 0cd351099adf..69af669308b0 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -745,6 +745,8 @@ int kvm_arm_vcpu_arch_has_attr(struct kvm_vcpu *vcpu,
 long kvm_vm_ioctl_mte_copy_tags(struct kvm *kvm,
 				struct kvm_arm_copy_mte_tags *copy_tags);
 
+int kvm_id_regs_consistency_check(const struct kvm_vcpu *vcpu);
+
 /* Guest/host FPSIMD coordination helpers */
 int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu);
 void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu);
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index fe102cd2e518..45ca72a37872 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -620,6 +620,9 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu)
 
 	ret = kvm_arm_pmu_v3_enable(vcpu);
 
+	if (kvm_id_regs_consistency_check(vcpu))
+		return -EPERM;
+
 	return ret;
 }
 
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 8a0b88f9a975..2fe3121d50ca 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1331,6 +1331,10 @@ static int __set_id_reg(struct kvm_vcpu *vcpu,
 	if (val != read_id_reg(vcpu, rd, raz) && !GET_ID_REG_INFO(encoding))
 		return -EINVAL;
 
+	/* Don't allow to change the reg after the first KVM_RUN. */
+	if (vcpu->arch.has_run_once)
+		return -EINVAL;
+
 	if (raz)
 		return (val == 0) ? 0 : -EINVAL;
 
@@ -2901,6 +2905,33 @@ int kvm_arm_copy_sys_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices)
 	return write_demux_regids(uindices);
 }
 
+int kvm_id_regs_consistency_check(const struct kvm_vcpu *vcpu)
+{
+	int i;
+	const struct kvm_vcpu *t_vcpu;
+
+	/*
+	 * Make sure vcpu->arch.has_run_once is visible for others so that
+	 * ID regs' consistency between two vCPUs is checked by either one
+	 * at least.
+	 */
+	smp_mb();
+	WARN_ON(!vcpu->arch.has_run_once);
+
+	kvm_for_each_vcpu(i, t_vcpu, vcpu->kvm) {
+		if (!t_vcpu->arch.has_run_once)
+			/* ID regs still could be updated. */
+			continue;
+
+		if (memcmp(&__vcpu_sys_reg(vcpu, ID_REG_BASE),
+			   &__vcpu_sys_reg(t_vcpu, ID_REG_BASE),
+			   sizeof(__vcpu_sys_reg(vcpu, ID_REG_BASE)) *
+					KVM_ARM_ID_REG_MAX_NUM))
+			return -EINVAL;
+	}
+	return 0;
+}
+
 static void id_reg_info_init_all(void)
 {
 	int i;
-- 
2.33.0.882.g93a45727a2-goog


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

* [RFC PATCH 06/25] KVM: arm64: Make ID_AA64PFR0_EL1 writable
  2021-10-12  4:35 [RFC PATCH 00/25] KVM: arm64: Make CPU ID registers writable by userspace Reiji Watanabe
                   ` (4 preceding siblings ...)
  2021-10-12  4:35 ` [RFC PATCH 05/25] KVM: arm64: Keep consistency of ID registers between vCPUs Reiji Watanabe
@ 2021-10-12  4:35 ` Reiji Watanabe
  2021-10-12  4:35 ` [RFC PATCH 07/25] KVM: arm64: Make ID_AA64PFR1_EL1 writable Reiji Watanabe
                   ` (18 subsequent siblings)
  24 siblings, 0 replies; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-12  4:35 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Will Deacon, Andrew Jones,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

This patch adds id_reg_info for ID_AA64PFR0_EL1 to make it writable by
userspace.

The CSV2/CSV3 fields of the register were already writable and values
that were written for them affected all vCPUs before. Now they only
affect the vCPU.

Return an error if userspace tries to set SVE field of the register
to a value that conflicts with SVE configuration for the guest (via
KVM_ARM_VCPU_INIT).  SIMD/FP/SVE fields of the requested value are
validated according to Arm ARM.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/include/asm/kvm_host.h |   3 -
 arch/arm64/kvm/arm.c              |  18 -----
 arch/arm64/kvm/sys_regs.c         | 122 +++++++++++++++++-------------
 3 files changed, 68 insertions(+), 75 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 69af669308b0..691cb6ee0f5c 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -131,9 +131,6 @@ struct kvm_arch {
 	unsigned long *pmu_filter;
 	unsigned int pmuver;
 
-	u8 pfr0_csv2;
-	u8 pfr0_csv3;
-
 	/* Memory Tagging Extension enabled for the guest */
 	bool mte_enabled;
 };
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 45ca72a37872..820f35051d77 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -114,22 +114,6 @@ static int kvm_arm_default_max_vcpus(void)
 	return vgic_present ? kvm_vgic_get_max_vcpus() : KVM_MAX_VCPUS;
 }
 
-static void set_default_spectre(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;
-	if (arm64_get_meltdown_state() == SPECTRE_UNAFFECTED)
-		kvm->arch.pfr0_csv3 = 1;
-}
-
 /**
  * kvm_arch_init_vm - initializes a VM data structure
  * @kvm:	pointer to the KVM struct
@@ -155,8 +139,6 @@ 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_spectre(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 2fe3121d50ca..63eb207b387f 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -292,6 +292,70 @@ static void id_reg_info_init(struct id_reg_info *id_reg)
 	id_reg->vcpu_limit_val = id_reg->sys_val;
 }
 
+static int validate_id_aa64pfr0_el1(struct kvm_vcpu *vcpu, u64 val)
+{
+	int fp, simd;
+	bool vcpu_has_sve = vcpu_has_sve(vcpu);
+	bool pfr0_has_sve = id_aa64pfr0_sve(val);
+
+	simd = cpuid_feature_extract_signed_field(val, ID_AA64PFR0_ASIMD_SHIFT);
+	fp = cpuid_feature_extract_signed_field(val, ID_AA64PFR0_FP_SHIFT);
+	if (simd != fp)
+		return -EINVAL;
+
+	/* fp must be supported when sve is supported */
+	if (pfr0_has_sve && (fp < 0))
+		return -EINVAL;
+
+	/* Check if there is a conflict with a request via KVM_ARM_VCPU_INIT */
+	if (vcpu_has_sve ^ pfr0_has_sve)
+		return -EPERM;
+
+	return 0;
+}
+
+static void init_id_aa64pfr0_el1_info(struct id_reg_info *id_reg)
+{
+	u64 limit;
+
+	id_reg->sys_val = read_sanitised_ftr_reg(id_reg->sys_reg);
+	limit = id_reg->sys_val;
+
+	limit &= ~(ARM64_FEATURE_MASK(ID_AA64PFR0_AMU));
+	if (!system_supports_sve())
+		limit &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_SVE);
+
+	/*
+	 * The default is to expose CSV2 == 1 and CSV3 == 1 if the HW
+	 * isn't affected.  Userspace can override this as long as it
+	 * doesn't promise the impossible.
+	 */
+	limit &= ~(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV2) |
+		   ARM64_FEATURE_MASK(ID_AA64PFR0_CSV3));
+
+	if (arm64_get_spectre_v2_state() == SPECTRE_UNAFFECTED)
+		limit |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV2), 1);
+	if (arm64_get_meltdown_state() == SPECTRE_UNAFFECTED)
+		limit |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV3), 1);
+
+	id_reg->vcpu_limit_val = limit;
+}
+
+static u64 get_reset_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
+				     struct id_reg_info *idr)
+{
+	return vcpu_has_sve(vcpu) ?
+	       idr->vcpu_limit_val :
+	       (idr->vcpu_limit_val & ~(ARM64_FEATURE_MASK(ID_AA64PFR0_SVE)));
+}
+
+static struct id_reg_info id_aa64pfr0_el1_info = {
+	.sys_reg = SYS_ID_AA64PFR0_EL1,
+	.init = init_id_aa64pfr0_el1_info,
+	.validate = validate_id_aa64pfr0_el1,
+	.get_reset_val = get_reset_id_aa64pfr0_el1,
+};
+
 /*
  * An ID register that needs special handling to control the value for the
  * guest must have its own id_reg_info in id_reg_info_table.
@@ -300,7 +364,9 @@ static void id_reg_info_init(struct id_reg_info *id_reg)
  * validation, etc.)
  */
 #define	GET_ID_REG_INFO(id)	(id_reg_info_table[IDREG_IDX(id)])
-static struct id_reg_info *id_reg_info_table[KVM_ARM_ID_REG_MAX_NUM] = {};
+static struct id_reg_info *id_reg_info_table[KVM_ARM_ID_REG_MAX_NUM] = {
+	[IDREG_IDX(SYS_ID_AA64PFR0_EL1)] = &id_aa64pfr0_el1_info,
+};
 
 static int validate_id_reg(struct kvm_vcpu *vcpu,
 			   const struct sys_reg_desc *rd, u64 val)
@@ -1136,15 +1202,6 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu,
 	u64 val = raz ? 0 : __vcpu_sys_reg(vcpu, IDREG_SYS_IDX(id));
 
 	switch (id) {
-	case SYS_ID_AA64PFR0_EL1:
-		if (!vcpu_has_sve(vcpu))
-			val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_SVE);
-		val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_AMU);
-		val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_CSV2);
-		val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV2), (u64)vcpu->kvm->arch.pfr0_csv2);
-		val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_CSV3);
-		val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV3), (u64)vcpu->kvm->arch.pfr0_csv3);
-		break;
 	case SYS_ID_AA64PFR1_EL1:
 		val &= ~ARM64_FEATURE_MASK(ID_AA64PFR1_MTE);
 		if (kvm_has_mte(vcpu->kvm)) {
@@ -1261,48 +1318,6 @@ static void reset_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd)
 	__vcpu_sys_reg(vcpu, IDREG_SYS_IDX(id)) = val;
 }
 
-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);
-	u8 csv2, csv3;
-	int err;
-	u64 val;
-
-	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 = cpuid_feature_extract_unsigned_field(val, ID_AA64PFR0_CSV2_SHIFT);
-	if (csv2 > 1 ||
-	    (csv2 && arm64_get_spectre_v2_state() != SPECTRE_UNAFFECTED))
-		return -EINVAL;
-
-	/* Same thing for CSV3 */
-	csv3 = cpuid_feature_extract_unsigned_field(val, ID_AA64PFR0_CSV3_SHIFT);
-	if (csv3 > 1 ||
-	    (csv3 && arm64_get_meltdown_state() != SPECTRE_UNAFFECTED))
-		return -EINVAL;
-
-	/* We can only differ with CSV[23], and anything else is an error */
-	val ^= read_id_reg(vcpu, rd, false);
-	val &= ~((0xFUL << ID_AA64PFR0_CSV2_SHIFT) |
-		 (0xFUL << ID_AA64PFR0_CSV3_SHIFT));
-	if (val)
-		return -EINVAL;
-
-	vcpu->kvm->arch.pfr0_csv2 = csv2;
-	vcpu->kvm->arch.pfr0_csv3 = csv3 ;
-
-	return 0;
-}
-
 /* cpufeature ID register user accessors */
 static int __get_id_reg(const struct kvm_vcpu *vcpu,
 			const struct sys_reg_desc *rd, void __user *uaddr,
@@ -1590,8 +1605,7 @@ 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, },
+	ID_SANITISED(ID_AA64PFR0_EL1),
 	ID_SANITISED(ID_AA64PFR1_EL1),
 	ID_UNALLOCATED(4,2),
 	ID_UNALLOCATED(4,3),
-- 
2.33.0.882.g93a45727a2-goog


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

* [RFC PATCH 07/25] KVM: arm64: Make ID_AA64PFR1_EL1 writable
  2021-10-12  4:35 [RFC PATCH 00/25] KVM: arm64: Make CPU ID registers writable by userspace Reiji Watanabe
                   ` (5 preceding siblings ...)
  2021-10-12  4:35 ` [RFC PATCH 06/25] KVM: arm64: Make ID_AA64PFR0_EL1 writable Reiji Watanabe
@ 2021-10-12  4:35 ` Reiji Watanabe
  2021-10-12  4:35 ` [RFC PATCH 08/25] KVM: arm64: Make ID_AA64ISAR0_EL1 writable Reiji Watanabe
                   ` (17 subsequent siblings)
  24 siblings, 0 replies; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-12  4:35 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Will Deacon, Andrew Jones,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

This patch adds id_reg_info for ID_AA64PFR1_EL1 to make it writable
by userspace.

Return an error if userspace tries to set MTE field of the register
to a value that conflicts with KVM_CAP_ARM_MTE configuration for
the guest.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/include/asm/sysreg.h |  1 +
 arch/arm64/kvm/sys_regs.c       | 51 ++++++++++++++++++++++++++-------
 2 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index b268082d67ed..aa0692595122 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -793,6 +793,7 @@
 #define ID_AA64PFR0_ELx_32BIT_64BIT	0x2
 
 /* id_aa64pfr1 */
+#define ID_AA64PFR1_CSV2FRAC_SHIFT	32
 #define ID_AA64PFR1_MPAMFRAC_SHIFT	16
 #define ID_AA64PFR1_RASFRAC_SHIFT	12
 #define ID_AA64PFR1_MTE_SHIFT		8
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 63eb207b387f..0e4423a81cb9 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -314,6 +314,20 @@ static int validate_id_aa64pfr0_el1(struct kvm_vcpu *vcpu, u64 val)
 	return 0;
 }
 
+static int validate_id_aa64pfr1_el1(struct kvm_vcpu *vcpu, u64 val)
+{
+	bool kvm_mte = kvm_has_mte(vcpu->kvm);
+	unsigned int mte;
+
+	mte = cpuid_feature_extract_unsigned_field(val, ID_AA64PFR1_MTE_SHIFT);
+
+	/* Check if there is a conflict with a request via KVM_ARM_VCPU_INIT. */
+	if (kvm_mte ^ (mte > 0))
+		return -EPERM;
+
+	return 0;
+}
+
 static void init_id_aa64pfr0_el1_info(struct id_reg_info *id_reg)
 {
 	u64 limit;
@@ -341,6 +355,14 @@ static void init_id_aa64pfr0_el1_info(struct id_reg_info *id_reg)
 	id_reg->vcpu_limit_val = limit;
 }
 
+static void init_id_aa64pfr1_el1_info(struct id_reg_info *id_reg)
+{
+	id_reg->sys_val = read_sanitised_ftr_reg(id_reg->sys_reg);
+	id_reg->vcpu_limit_val = (system_supports_mte() ?
+		id_reg->sys_val :
+		(id_reg->sys_val & ~ARM64_FEATURE_MASK(ID_AA64PFR1_MTE)));
+}
+
 static u64 get_reset_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
 				     struct id_reg_info *idr)
 {
@@ -349,6 +371,14 @@ static u64 get_reset_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
 	       (idr->vcpu_limit_val & ~(ARM64_FEATURE_MASK(ID_AA64PFR0_SVE)));
 }
 
+static u64 get_reset_id_aa64pfr1_el1(struct kvm_vcpu *vcpu,
+				     struct id_reg_info *idr)
+{
+	return kvm_has_mte(vcpu->kvm) ?
+	       idr->vcpu_limit_val :
+	       (idr->vcpu_limit_val & ~(ARM64_FEATURE_MASK(ID_AA64PFR1_MTE)));
+}
+
 static struct id_reg_info id_aa64pfr0_el1_info = {
 	.sys_reg = SYS_ID_AA64PFR0_EL1,
 	.init = init_id_aa64pfr0_el1_info,
@@ -356,6 +386,16 @@ static struct id_reg_info id_aa64pfr0_el1_info = {
 	.get_reset_val = get_reset_id_aa64pfr0_el1,
 };
 
+static struct id_reg_info id_aa64pfr1_el1_info = {
+	.sys_reg = SYS_ID_AA64PFR1_EL1,
+	.ignore_mask = ARM64_FEATURE_MASK(ID_AA64PFR1_RASFRAC) |
+		       ARM64_FEATURE_MASK(ID_AA64PFR1_MPAMFRAC) |
+		       ARM64_FEATURE_MASK(ID_AA64PFR1_CSV2FRAC),
+	.init = init_id_aa64pfr1_el1_info,
+	.validate = validate_id_aa64pfr1_el1,
+	.get_reset_val = get_reset_id_aa64pfr1_el1,
+};
+
 /*
  * An ID register that needs special handling to control the value for the
  * guest must have its own id_reg_info in id_reg_info_table.
@@ -366,6 +406,7 @@ static struct id_reg_info id_aa64pfr0_el1_info = {
 #define	GET_ID_REG_INFO(id)	(id_reg_info_table[IDREG_IDX(id)])
 static struct id_reg_info *id_reg_info_table[KVM_ARM_ID_REG_MAX_NUM] = {
 	[IDREG_IDX(SYS_ID_AA64PFR0_EL1)] = &id_aa64pfr0_el1_info,
+	[IDREG_IDX(SYS_ID_AA64PFR1_EL1)] = &id_aa64pfr1_el1_info,
 };
 
 static int validate_id_reg(struct kvm_vcpu *vcpu,
@@ -1202,16 +1243,6 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu,
 	u64 val = raz ? 0 : __vcpu_sys_reg(vcpu, IDREG_SYS_IDX(id));
 
 	switch (id) {
-	case SYS_ID_AA64PFR1_EL1:
-		val &= ~ARM64_FEATURE_MASK(ID_AA64PFR1_MTE);
-		if (kvm_has_mte(vcpu->kvm)) {
-			u64 pfr, mte;
-
-			pfr = read_sanitised_ftr_reg(SYS_ID_AA64PFR1_EL1);
-			mte = cpuid_feature_extract_unsigned_field(pfr, ID_AA64PFR1_MTE_SHIFT);
-			val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR1_MTE), mte);
-		}
-		break;
 	case SYS_ID_AA64ISAR1_EL1:
 		if (!vcpu_has_ptrauth(vcpu))
 			val &= ~(ARM64_FEATURE_MASK(ID_AA64ISAR1_APA) |
-- 
2.33.0.882.g93a45727a2-goog


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

* [RFC PATCH 08/25] KVM: arm64: Make ID_AA64ISAR0_EL1 writable
  2021-10-12  4:35 [RFC PATCH 00/25] KVM: arm64: Make CPU ID registers writable by userspace Reiji Watanabe
                   ` (6 preceding siblings ...)
  2021-10-12  4:35 ` [RFC PATCH 07/25] KVM: arm64: Make ID_AA64PFR1_EL1 writable Reiji Watanabe
@ 2021-10-12  4:35 ` Reiji Watanabe
  2021-10-12  4:35 ` [RFC PATCH 09/25] KVM: arm64: Make ID_AA64ISAR1_EL1 writable Reiji Watanabe
                   ` (16 subsequent siblings)
  24 siblings, 0 replies; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-12  4:35 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Will Deacon, Andrew Jones,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

This patch adds id_reg_info for ID_AA64ISAR0_EL1 to make it writable
by userspace.

Updating sm3, sm4, sha1, sha2 and sha3 fields are allowed only
if values of those fields follow Arm ARM.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/kvm/sys_regs.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 0e4423a81cb9..73b5dad0b2ee 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -328,6 +328,31 @@ static int validate_id_aa64pfr1_el1(struct kvm_vcpu *vcpu, u64 val)
 	return 0;
 }
 
+static int validate_id_aa64isar0_el1(struct kvm_vcpu *vcpu, u64 val)
+{
+	unsigned int sm3, sm4, sha1, sha2, sha3;
+
+	/* Run consistency checkings according to Arm ARM */
+	sm3 = cpuid_feature_extract_unsigned_field(val, ID_AA64ISAR0_SM3_SHIFT);
+	sm4 = cpuid_feature_extract_unsigned_field(val, ID_AA64ISAR0_SM4_SHIFT);
+	if (sm3 != sm4)
+		return -EINVAL;
+
+	sha1 = cpuid_feature_extract_unsigned_field(val,
+						    ID_AA64ISAR0_SHA1_SHIFT);
+	sha2 = cpuid_feature_extract_unsigned_field(val,
+						    ID_AA64ISAR0_SHA2_SHIFT);
+	if ((sha1 == 0) ^ (sha2 == 0))
+		return -EINVAL;
+
+	sha3 = cpuid_feature_extract_unsigned_field(val,
+						    ID_AA64ISAR0_SHA3_SHIFT);
+	if (((sha2 == 2) ^ (sha3 == 1)) || (!sha1 && sha3))
+		return -EINVAL;
+
+	return 0;
+}
+
 static void init_id_aa64pfr0_el1_info(struct id_reg_info *id_reg)
 {
 	u64 limit;
@@ -396,6 +421,11 @@ static struct id_reg_info id_aa64pfr1_el1_info = {
 	.get_reset_val = get_reset_id_aa64pfr1_el1,
 };
 
+static struct id_reg_info id_aa64isar0_el1_info = {
+	.sys_reg = SYS_ID_AA64ISAR0_EL1,
+	.validate = validate_id_aa64isar0_el1,
+};
+
 /*
  * An ID register that needs special handling to control the value for the
  * guest must have its own id_reg_info in id_reg_info_table.
@@ -407,6 +437,7 @@ static struct id_reg_info id_aa64pfr1_el1_info = {
 static struct id_reg_info *id_reg_info_table[KVM_ARM_ID_REG_MAX_NUM] = {
 	[IDREG_IDX(SYS_ID_AA64PFR0_EL1)] = &id_aa64pfr0_el1_info,
 	[IDREG_IDX(SYS_ID_AA64PFR1_EL1)] = &id_aa64pfr1_el1_info,
+	[IDREG_IDX(SYS_ID_AA64ISAR0_EL1)] = &id_aa64isar0_el1_info,
 };
 
 static int validate_id_reg(struct kvm_vcpu *vcpu,
-- 
2.33.0.882.g93a45727a2-goog


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

* [RFC PATCH 09/25] KVM: arm64: Make ID_AA64ISAR1_EL1 writable
  2021-10-12  4:35 [RFC PATCH 00/25] KVM: arm64: Make CPU ID registers writable by userspace Reiji Watanabe
                   ` (7 preceding siblings ...)
  2021-10-12  4:35 ` [RFC PATCH 08/25] KVM: arm64: Make ID_AA64ISAR0_EL1 writable Reiji Watanabe
@ 2021-10-12  4:35 ` Reiji Watanabe
  2021-10-12  4:35 ` [RFC PATCH 10/25] KVM: arm64: Make ID_AA64DFR0_EL1 writable Reiji Watanabe
                   ` (15 subsequent siblings)
  24 siblings, 0 replies; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-12  4:35 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Will Deacon, Andrew Jones,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

This patch adds id_reg_info for ID_AA64ISAR1_EL1 to make it
writable by userspace.

Return an error if userspace tries to set PTRAUTH related fields
of the register to values that conflict with PTRAUTH configuration
for the guest (via KVM_ARM_VCPU_INIT).

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/kvm/sys_regs.c | 79 +++++++++++++++++++++++++++++++++++----
 1 file changed, 72 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 73b5dad0b2ee..7819a07ee248 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -263,6 +263,24 @@ static bool trap_raz_wi(struct kvm_vcpu *vcpu,
 		return read_zero(vcpu, p);
 }
 
+#define PTRAUTH_MASK	(ARM64_FEATURE_MASK(ID_AA64ISAR1_APA) |	\
+			 ARM64_FEATURE_MASK(ID_AA64ISAR1_API) | \
+			 ARM64_FEATURE_MASK(ID_AA64ISAR1_GPA) |	\
+			 ARM64_FEATURE_MASK(ID_AA64ISAR1_GPI))
+
+#define aa64isar1_has_apa(val)	\
+	(cpuid_feature_extract_unsigned_field(val, ID_AA64ISAR1_APA_SHIFT) >= \
+	 ID_AA64ISAR1_APA_ARCHITECTED)
+#define aa64isar1_has_api(val)	\
+	(cpuid_feature_extract_unsigned_field(val, ID_AA64ISAR1_API_SHIFT) >= \
+	 ID_AA64ISAR1_API_IMP_DEF)
+#define aa64isar1_has_gpa(val)	\
+	(cpuid_feature_extract_unsigned_field(val, ID_AA64ISAR1_GPA_SHIFT) >= \
+	 ID_AA64ISAR1_GPA_ARCHITECTED)
+#define aa64isar1_has_gpi(val)	\
+	(cpuid_feature_extract_unsigned_field(val, ID_AA64ISAR1_GPI_SHIFT) >= \
+	 ID_AA64ISAR1_GPI_IMP_DEF)
+
 struct id_reg_info {
 	u32	sys_reg;	/* Register ID */
 	u64	sys_val;	/* Sanitized system value */
@@ -353,6 +371,37 @@ static int validate_id_aa64isar0_el1(struct kvm_vcpu *vcpu, u64 val)
 	return 0;
 }
 
+static int validate_id_aa64isar1_el1(struct kvm_vcpu *vcpu, u64 val)
+{
+	unsigned int gpi, gpa, api, apa;
+	bool generic, address;
+	bool vcpu_ptrauth = vcpu_has_ptrauth(vcpu);
+
+	gpi = aa64isar1_has_gpi(val);
+	gpa = aa64isar1_has_gpa(val);
+	api = aa64isar1_has_api(val);
+	apa = aa64isar1_has_apa(val);
+	if ((gpi && gpa) || (api && apa))
+		return -EINVAL;
+
+	generic = gpi || gpa;
+	address = api || apa;
+
+	/*
+	 * Since the current KVM guest implementation works by enabling
+	 * both address/generic pointer authentication features,
+	 * return an error if they conflict.
+	 */
+	if (generic ^ address)
+		return -EPERM;
+
+	/* Check if there is a conflict with a request via KVM_ARM_VCPU_INIT */
+	if (vcpu_ptrauth ^ (generic && address))
+		return -EPERM;
+
+	return 0;
+}
+
 static void init_id_aa64pfr0_el1_info(struct id_reg_info *id_reg)
 {
 	u64 limit;
@@ -388,6 +437,14 @@ static void init_id_aa64pfr1_el1_info(struct id_reg_info *id_reg)
 		(id_reg->sys_val & ~ARM64_FEATURE_MASK(ID_AA64PFR1_MTE)));
 }
 
+static void init_id_aa64isar1_el1_info(struct id_reg_info *id_reg)
+{
+	id_reg->sys_val = read_sanitised_ftr_reg(id_reg->sys_reg);
+	id_reg->vcpu_limit_val = system_has_full_ptr_auth() ?
+				 id_reg->sys_val :
+				 (id_reg->sys_val & ~PTRAUTH_MASK);
+}
+
 static u64 get_reset_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
 				     struct id_reg_info *idr)
 {
@@ -404,6 +461,13 @@ static u64 get_reset_id_aa64pfr1_el1(struct kvm_vcpu *vcpu,
 	       (idr->vcpu_limit_val & ~(ARM64_FEATURE_MASK(ID_AA64PFR1_MTE)));
 }
 
+static u64 get_reset_id_aa64isar1_el1(struct kvm_vcpu *vcpu,
+				      struct id_reg_info *idr)
+{
+	return vcpu_has_ptrauth(vcpu) ?
+	       idr->vcpu_limit_val : (idr->vcpu_limit_val & ~PTRAUTH_MASK);
+}
+
 static struct id_reg_info id_aa64pfr0_el1_info = {
 	.sys_reg = SYS_ID_AA64PFR0_EL1,
 	.init = init_id_aa64pfr0_el1_info,
@@ -426,6 +490,13 @@ static struct id_reg_info id_aa64isar0_el1_info = {
 	.validate = validate_id_aa64isar0_el1,
 };
 
+static struct id_reg_info id_aa64isar1_el1_info = {
+	.sys_reg = SYS_ID_AA64ISAR1_EL1,
+	.init = init_id_aa64isar1_el1_info,
+	.validate = validate_id_aa64isar1_el1,
+	.get_reset_val = get_reset_id_aa64isar1_el1,
+};
+
 /*
  * An ID register that needs special handling to control the value for the
  * guest must have its own id_reg_info in id_reg_info_table.
@@ -438,6 +509,7 @@ static struct id_reg_info *id_reg_info_table[KVM_ARM_ID_REG_MAX_NUM] = {
 	[IDREG_IDX(SYS_ID_AA64PFR0_EL1)] = &id_aa64pfr0_el1_info,
 	[IDREG_IDX(SYS_ID_AA64PFR1_EL1)] = &id_aa64pfr1_el1_info,
 	[IDREG_IDX(SYS_ID_AA64ISAR0_EL1)] = &id_aa64isar0_el1_info,
+	[IDREG_IDX(SYS_ID_AA64ISAR1_EL1)] = &id_aa64isar1_el1_info,
 };
 
 static int validate_id_reg(struct kvm_vcpu *vcpu,
@@ -1274,13 +1346,6 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu,
 	u64 val = raz ? 0 : __vcpu_sys_reg(vcpu, IDREG_SYS_IDX(id));
 
 	switch (id) {
-	case SYS_ID_AA64ISAR1_EL1:
-		if (!vcpu_has_ptrauth(vcpu))
-			val &= ~(ARM64_FEATURE_MASK(ID_AA64ISAR1_APA) |
-				 ARM64_FEATURE_MASK(ID_AA64ISAR1_API) |
-				 ARM64_FEATURE_MASK(ID_AA64ISAR1_GPA) |
-				 ARM64_FEATURE_MASK(ID_AA64ISAR1_GPI));
-		break;
 	case SYS_ID_AA64DFR0_EL1:
 		/* Limit debug to ARMv8.0 */
 		val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_DEBUGVER);
-- 
2.33.0.882.g93a45727a2-goog


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

* [RFC PATCH 10/25] KVM: arm64: Make ID_AA64DFR0_EL1 writable
  2021-10-12  4:35 [RFC PATCH 00/25] KVM: arm64: Make CPU ID registers writable by userspace Reiji Watanabe
                   ` (8 preceding siblings ...)
  2021-10-12  4:35 ` [RFC PATCH 09/25] KVM: arm64: Make ID_AA64ISAR1_EL1 writable Reiji Watanabe
@ 2021-10-12  4:35 ` Reiji Watanabe
  2021-10-12  4:35 ` [RFC PATCH 11/25] KVM: arm64: Make ID_DFR0_EL1 writable Reiji Watanabe
                   ` (14 subsequent siblings)
  24 siblings, 0 replies; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-12  4:35 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Will Deacon, Andrew Jones,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

This patch adds id_reg_info for ID_AA64DFR0_EL1 to make it writable
by userspace.

Return an error if userspace tries to set PMUVER field of the
register to a value that conflicts with the PMU configuration.

Since number of context-aware breakpoints must be no more than number
of supported breakpoints according to Arm ARM, return an error
if userspace tries to set CTX_CMPS field to such value.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/kvm/sys_regs.c | 108 ++++++++++++++++++++++++++++++++++----
 1 file changed, 97 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 7819a07ee248..61e61f4bb81c 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -402,6 +402,46 @@ static int validate_id_aa64isar1_el1(struct kvm_vcpu *vcpu, u64 val)
 	return 0;
 }
 
+static bool id_reg_has_pmu(u64 val, u64 shift, u64 min)
+{
+	u64 pmu = ((val >> shift) & 0xf);
+
+	/*
+	 * Treat IMPLEMENTATION DEFINED functionality as unimplemented for
+	 * ID_AA64DFR0_EL1.PMUVer/ID_DFR0_EL1.PerfMon.
+	 */
+	if (pmu == 0xf)
+		pmu = 0;
+
+	return (pmu >= min);
+}
+
+static int validate_id_aa64dfr0_el1(struct kvm_vcpu *vcpu, u64 val)
+{
+	unsigned int brps, ctx_cmps;
+	bool vcpu_pmu = kvm_vcpu_has_pmu(vcpu);
+	bool dfr0_pmu = id_reg_has_pmu(val, ID_AA64DFR0_PMUVER_SHIFT,
+				       ID_AA64DFR0_PMUVER_8_0);
+
+	brps = cpuid_feature_extract_unsigned_field(val,
+						ID_AA64DFR0_BRPS_SHIFT);
+	ctx_cmps = cpuid_feature_extract_unsigned_field(val,
+						ID_AA64DFR0_CTX_CMPS_SHIFT);
+
+	/*
+	 * Number of context-aware breakpoints can be no more than number of
+	 * supported breakpoints.
+	 */
+	if (ctx_cmps > brps)
+		return -EINVAL;
+
+	/* Check if there is a conflict with a request via KVM_ARM_VCPU_INIT */
+	if (vcpu_pmu ^ dfr0_pmu)
+		return -EPERM;
+
+	return 0;
+}
+
 static void init_id_aa64pfr0_el1_info(struct id_reg_info *id_reg)
 {
 	u64 limit;
@@ -445,6 +485,47 @@ static void init_id_aa64isar1_el1_info(struct id_reg_info *id_reg)
 				 (id_reg->sys_val & ~PTRAUTH_MASK);
 }
 
+/*
+ * ID_AA64DFR0_EL1.PMUVer/ID_DFR0_EL1.PerfMon == 0xf indicates
+ * IMPLEMENTATION DEFINED form of performance monitors supported,
+ * PMUv3 not supported (NOTE: 0x0 indicates PMU is not supported).
+ * This function is to cap a value of those two fields with the
+ * given 'cap_val' treating 0xf in the fields as 0.
+ */
+static u64 id_reg_cap_pmu(u64 val, u64 shift, u64 cap_val)
+{
+	u64 mask = 0xf;
+	u64 pmu;
+
+	pmu = (val >> shift) & mask;
+	pmu = (pmu == 0xf) ? 0 : pmu;
+	pmu = min(pmu, cap_val);
+
+	val &= ~(0xfULL << shift);
+	val |= (pmu & 0xf) << shift;
+	return val;
+}
+
+static void init_id_aa64dfr0_el1_info(struct id_reg_info *id_reg)
+{
+	u64 limit;
+
+	id_reg->sys_val = read_sanitised_ftr_reg(id_reg->sys_reg);
+	limit = id_reg->sys_val;
+
+	/* Limit guests to PMUv3 for ARMv8.4 */
+	limit = id_reg_cap_pmu(limit, ID_AA64DFR0_PMUVER_SHIFT,
+			       ID_AA64DFR0_PMUVER_8_4);
+	/* Limit debug to ARMv8.0 */
+	limit &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_DEBUGVER);
+	limit |= (FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_DEBUGVER), 6));
+
+	/* Hide SPE from guests */
+	limit &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_PMSVER);
+
+	id_reg->vcpu_limit_val = limit;
+}
+
 static u64 get_reset_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
 				     struct id_reg_info *idr)
 {
@@ -468,6 +549,14 @@ static u64 get_reset_id_aa64isar1_el1(struct kvm_vcpu *vcpu,
 	       idr->vcpu_limit_val : (idr->vcpu_limit_val & ~PTRAUTH_MASK);
 }
 
+static u64 get_reset_id_aa64dfr0_el1(struct kvm_vcpu *vcpu,
+				     struct id_reg_info *idr)
+{
+	return kvm_vcpu_has_pmu(vcpu) ?
+	       idr->vcpu_limit_val :
+	       (idr->vcpu_limit_val & ~(ARM64_FEATURE_MASK(ID_AA64DFR0_PMUVER)));
+}
+
 static struct id_reg_info id_aa64pfr0_el1_info = {
 	.sys_reg = SYS_ID_AA64PFR0_EL1,
 	.init = init_id_aa64pfr0_el1_info,
@@ -497,6 +586,13 @@ static struct id_reg_info id_aa64isar1_el1_info = {
 	.get_reset_val = get_reset_id_aa64isar1_el1,
 };
 
+static struct id_reg_info id_aa64dfr0_el1_info = {
+	.sys_reg = SYS_ID_AA64DFR0_EL1,
+	.init = init_id_aa64dfr0_el1_info,
+	.validate = validate_id_aa64dfr0_el1,
+	.get_reset_val = get_reset_id_aa64dfr0_el1,
+};
+
 /*
  * An ID register that needs special handling to control the value for the
  * guest must have its own id_reg_info in id_reg_info_table.
@@ -508,6 +604,7 @@ static struct id_reg_info id_aa64isar1_el1_info = {
 static struct id_reg_info *id_reg_info_table[KVM_ARM_ID_REG_MAX_NUM] = {
 	[IDREG_IDX(SYS_ID_AA64PFR0_EL1)] = &id_aa64pfr0_el1_info,
 	[IDREG_IDX(SYS_ID_AA64PFR1_EL1)] = &id_aa64pfr1_el1_info,
+	[IDREG_IDX(SYS_ID_AA64DFR0_EL1)] = &id_aa64dfr0_el1_info,
 	[IDREG_IDX(SYS_ID_AA64ISAR0_EL1)] = &id_aa64isar0_el1_info,
 	[IDREG_IDX(SYS_ID_AA64ISAR1_EL1)] = &id_aa64isar1_el1_info,
 };
@@ -1346,17 +1443,6 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu,
 	u64 val = raz ? 0 : __vcpu_sys_reg(vcpu, IDREG_SYS_IDX(id));
 
 	switch (id) {
-	case SYS_ID_AA64DFR0_EL1:
-		/* Limit debug to ARMv8.0 */
-		val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_DEBUGVER);
-		val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_DEBUGVER), 6);
-		/* Limit guests to PMUv3 for ARMv8.4 */
-		val = cpuid_feature_cap_perfmon_field(val,
-						      ID_AA64DFR0_PMUVER_SHIFT,
-						      kvm_vcpu_has_pmu(vcpu) ? ID_AA64DFR0_PMUVER_8_4 : 0);
-		/* Hide SPE from guests */
-		val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_PMSVER);
-		break;
 	case SYS_ID_DFR0_EL1:
 		/* Limit guests to PMUv3 for ARMv8.4 */
 		val = cpuid_feature_cap_perfmon_field(val,
-- 
2.33.0.882.g93a45727a2-goog


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

* [RFC PATCH 11/25] KVM: arm64: Make ID_DFR0_EL1 writable
  2021-10-12  4:35 [RFC PATCH 00/25] KVM: arm64: Make CPU ID registers writable by userspace Reiji Watanabe
                   ` (9 preceding siblings ...)
  2021-10-12  4:35 ` [RFC PATCH 10/25] KVM: arm64: Make ID_AA64DFR0_EL1 writable Reiji Watanabe
@ 2021-10-12  4:35 ` Reiji Watanabe
  2021-10-12  4:35 ` [RFC PATCH 12/25] KVM: arm64: Make MVFR1_EL1 writable Reiji Watanabe
                   ` (13 subsequent siblings)
  24 siblings, 0 replies; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-12  4:35 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Will Deacon, Andrew Jones,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

This patch adds id_reg_info for ID_DFR0_EL1 to make it writable
by userspace.

Return an error if userspace tries to set PerfMon field of the
register to a value that conflicts with the PMU configuration.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/kvm/sys_regs.c | 57 +++++++++++++++++++++++++++++++--------
 1 file changed, 46 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 61e61f4bb81c..84c064dfc63a 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -442,6 +442,25 @@ static int validate_id_aa64dfr0_el1(struct kvm_vcpu *vcpu, u64 val)
 	return 0;
 }
 
+static int validate_id_dfr0_el1(struct kvm_vcpu *vcpu, u64 val)
+{
+	bool vcpu_pmu = kvm_vcpu_has_pmu(vcpu);
+	unsigned int perfmon = cpuid_feature_extract_unsigned_field(val,
+						ID_DFR0_PERFMON_SHIFT);
+	bool dfr0_pmu = id_reg_has_pmu(val, ID_DFR0_PERFMON_SHIFT,
+						ID_DFR0_PERFMON_8_0);
+
+	if (perfmon == 1 || perfmon == 2)
+		/* PMUv1 or PMUv2 is not allowed on ARMv8. */
+		return -EINVAL;
+
+	/* Check if there is a conflict with a request via KVM_ARM_VCPU_INIT */
+	if (vcpu_pmu ^ dfr0_pmu)
+		return -EPERM;
+
+	return 0;
+}
+
 static void init_id_aa64pfr0_el1_info(struct id_reg_info *id_reg)
 {
 	u64 limit;
@@ -526,6 +545,16 @@ static void init_id_aa64dfr0_el1_info(struct id_reg_info *id_reg)
 	id_reg->vcpu_limit_val = limit;
 }
 
+static void init_id_dfr0_el1_info(struct id_reg_info *id_reg)
+{
+	id_reg->sys_val = read_sanitised_ftr_reg(id_reg->sys_reg);
+
+	/* Limit guests to PMUv3 for ARMv8.4 */
+	id_reg->vcpu_limit_val = id_reg_cap_pmu(id_reg->sys_val,
+						ID_DFR0_PERFMON_SHIFT,
+						ID_DFR0_PERFMON_8_4);
+}
+
 static u64 get_reset_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
 				     struct id_reg_info *idr)
 {
@@ -557,6 +586,14 @@ static u64 get_reset_id_aa64dfr0_el1(struct kvm_vcpu *vcpu,
 	       (idr->vcpu_limit_val & ~(ARM64_FEATURE_MASK(ID_AA64DFR0_PMUVER)));
 }
 
+static u64 get_reset_id_dfr0_el1(struct kvm_vcpu *vcpu,
+				 struct id_reg_info *idr)
+{
+	return kvm_vcpu_has_pmu(vcpu) ?
+	       idr->vcpu_limit_val :
+	       (idr->vcpu_limit_val & ~(ARM64_FEATURE_MASK(ID_DFR0_PERFMON)));
+}
+
 static struct id_reg_info id_aa64pfr0_el1_info = {
 	.sys_reg = SYS_ID_AA64PFR0_EL1,
 	.init = init_id_aa64pfr0_el1_info,
@@ -593,6 +630,13 @@ static struct id_reg_info id_aa64dfr0_el1_info = {
 	.get_reset_val = get_reset_id_aa64dfr0_el1,
 };
 
+static struct id_reg_info id_dfr0_el1_info = {
+	.sys_reg = SYS_ID_DFR0_EL1,
+	.init = init_id_dfr0_el1_info,
+	.validate = validate_id_dfr0_el1,
+	.get_reset_val = get_reset_id_dfr0_el1,
+};
+
 /*
  * An ID register that needs special handling to control the value for the
  * guest must have its own id_reg_info in id_reg_info_table.
@@ -602,6 +646,7 @@ static struct id_reg_info id_aa64dfr0_el1_info = {
  */
 #define	GET_ID_REG_INFO(id)	(id_reg_info_table[IDREG_IDX(id)])
 static struct id_reg_info *id_reg_info_table[KVM_ARM_ID_REG_MAX_NUM] = {
+	[IDREG_IDX(SYS_ID_DFR0_EL1)] = &id_dfr0_el1_info,
 	[IDREG_IDX(SYS_ID_AA64PFR0_EL1)] = &id_aa64pfr0_el1_info,
 	[IDREG_IDX(SYS_ID_AA64PFR1_EL1)] = &id_aa64pfr1_el1_info,
 	[IDREG_IDX(SYS_ID_AA64DFR0_EL1)] = &id_aa64dfr0_el1_info,
@@ -1440,18 +1485,8 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu,
 		struct sys_reg_desc const *r, bool raz)
 {
 	u32 id = reg_to_encoding(r);
-	u64 val = raz ? 0 : __vcpu_sys_reg(vcpu, IDREG_SYS_IDX(id));
 
-	switch (id) {
-	case SYS_ID_DFR0_EL1:
-		/* Limit guests to PMUv3 for ARMv8.4 */
-		val = cpuid_feature_cap_perfmon_field(val,
-						      ID_DFR0_PERFMON_SHIFT,
-						      kvm_vcpu_has_pmu(vcpu) ? ID_DFR0_PERFMON_8_4 : 0);
-		break;
-	}
-
-	return val;
+	return raz ? 0 : __vcpu_sys_reg(vcpu, IDREG_SYS_IDX(id));
 }
 
 static unsigned int id_visibility(const struct kvm_vcpu *vcpu,
-- 
2.33.0.882.g93a45727a2-goog


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

* [RFC PATCH 12/25] KVM: arm64: Make MVFR1_EL1 writable
  2021-10-12  4:35 [RFC PATCH 00/25] KVM: arm64: Make CPU ID registers writable by userspace Reiji Watanabe
                   ` (10 preceding siblings ...)
  2021-10-12  4:35 ` [RFC PATCH 11/25] KVM: arm64: Make ID_DFR0_EL1 writable Reiji Watanabe
@ 2021-10-12  4:35 ` Reiji Watanabe
  2021-10-12  4:35 ` [RFC PATCH 13/25] KVM: arm64: Make ID registers without id_reg_info writable Reiji Watanabe
                   ` (12 subsequent siblings)
  24 siblings, 0 replies; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-12  4:35 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Will Deacon, Andrew Jones,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

This patch adds id_reg_info for MVFR1_EL1 to make it writable
by userspace.

There are only a few valid combinations of values that can be set
for FPHP and SIMDHP fields according to Arm ARM.  Return an error
when userspace tries to set those fields to values that don't match
any of the valid combinations.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/kvm/sys_regs.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 84c064dfc63a..71cfd62f9c85 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -461,6 +461,35 @@ static int validate_id_dfr0_el1(struct kvm_vcpu *vcpu, u64 val)
 	return 0;
 }
 
+static int validate_mvfr1_el1(struct kvm_vcpu *vcpu, u64 val)
+{
+	unsigned int fphp, simdhp;
+	struct fphp_simdhp {
+		unsigned int fphp;
+		unsigned int simdhp;
+	};
+	/* Permitted fphp/simdhp value combinations according to Arm ARM */
+	struct fphp_simdhp valid_fphp_simdhp[3] = {{0, 0}, {2, 1}, {3, 2}};
+	int i;
+	bool is_valid_fphp_simdhp = false;
+
+	fphp = cpuid_feature_extract_unsigned_field(val, MVFR1_FPHP_SHIFT);
+	simdhp = cpuid_feature_extract_unsigned_field(val, MVFR1_SIMDHP_SHIFT);
+
+	for (i = 0; i < ARRAY_SIZE(valid_fphp_simdhp); i++) {
+		if (valid_fphp_simdhp[i].fphp == fphp &&
+		    valid_fphp_simdhp[i].simdhp == simdhp) {
+			is_valid_fphp_simdhp = true;
+			break;
+		}
+	}
+
+	if (!is_valid_fphp_simdhp)
+		return -EINVAL;
+
+	return 0;
+}
+
 static void init_id_aa64pfr0_el1_info(struct id_reg_info *id_reg)
 {
 	u64 limit;
@@ -637,6 +666,11 @@ static struct id_reg_info id_dfr0_el1_info = {
 	.get_reset_val = get_reset_id_dfr0_el1,
 };
 
+static struct id_reg_info mvfr1_el1_info = {
+	.sys_reg = SYS_MVFR1_EL1,
+	.validate = validate_mvfr1_el1,
+};
+
 /*
  * An ID register that needs special handling to control the value for the
  * guest must have its own id_reg_info in id_reg_info_table.
@@ -647,6 +681,7 @@ static struct id_reg_info id_dfr0_el1_info = {
 #define	GET_ID_REG_INFO(id)	(id_reg_info_table[IDREG_IDX(id)])
 static struct id_reg_info *id_reg_info_table[KVM_ARM_ID_REG_MAX_NUM] = {
 	[IDREG_IDX(SYS_ID_DFR0_EL1)] = &id_dfr0_el1_info,
+	[IDREG_IDX(SYS_MVFR1_EL1)] = &mvfr1_el1_info,
 	[IDREG_IDX(SYS_ID_AA64PFR0_EL1)] = &id_aa64pfr0_el1_info,
 	[IDREG_IDX(SYS_ID_AA64PFR1_EL1)] = &id_aa64pfr1_el1_info,
 	[IDREG_IDX(SYS_ID_AA64DFR0_EL1)] = &id_aa64dfr0_el1_info,
-- 
2.33.0.882.g93a45727a2-goog


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

* [RFC PATCH 13/25] KVM: arm64: Make ID registers without id_reg_info writable
  2021-10-12  4:35 [RFC PATCH 00/25] KVM: arm64: Make CPU ID registers writable by userspace Reiji Watanabe
                   ` (11 preceding siblings ...)
  2021-10-12  4:35 ` [RFC PATCH 12/25] KVM: arm64: Make MVFR1_EL1 writable Reiji Watanabe
@ 2021-10-12  4:35 ` Reiji Watanabe
  2021-10-12  4:35 ` [RFC PATCH 14/25] KVM: arm64: Add consistency checking for frac fields of ID registers Reiji Watanabe
                   ` (11 subsequent siblings)
  24 siblings, 0 replies; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-12  4:35 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Will Deacon, Andrew Jones,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

Make ID registers that don't have id_reg_info writable.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/kvm/sys_regs.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 71cfd62f9c85..2c092136cdff 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1625,12 +1625,8 @@ static int __set_id_reg(struct kvm_vcpu *vcpu,
 	if (err)
 		return err;
 
-	/* Don't allow to change the reg unless the reg has id_reg_info */
-	if (val != read_id_reg(vcpu, rd, raz) && !GET_ID_REG_INFO(encoding))
-		return -EINVAL;
-
 	/* Don't allow to change the reg after the first KVM_RUN. */
-	if (vcpu->arch.has_run_once)
+	if ((val != read_id_reg(vcpu, rd, raz)) && vcpu->arch.has_run_once)
 		return -EINVAL;
 
 	if (raz)
-- 
2.33.0.882.g93a45727a2-goog


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

* [RFC PATCH 14/25] KVM: arm64: Add consistency checking for frac fields of ID registers
  2021-10-12  4:35 [RFC PATCH 00/25] KVM: arm64: Make CPU ID registers writable by userspace Reiji Watanabe
                   ` (12 preceding siblings ...)
  2021-10-12  4:35 ` [RFC PATCH 13/25] KVM: arm64: Make ID registers without id_reg_info writable Reiji Watanabe
@ 2021-10-12  4:35 ` Reiji Watanabe
  2021-10-12  4:35 ` [RFC PATCH 15/25] KVM: arm64: Introduce KVM_CAP_ARM_ID_REG_WRITABLE capability Reiji Watanabe
                   ` (10 subsequent siblings)
  24 siblings, 0 replies; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-12  4:35 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Will Deacon, Andrew Jones,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

Feature fractional field of an ID register cannot be simply validated
at KVM_SET_ONE_REG because its validity depends on its (main) feature
field value, which could be in a different ID register (and might be
set later).
Validate fractional fields at the first KVM_RUN instead.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/kvm/sys_regs.c | 98 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 94 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 2c092136cdff..536e313992d4 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -632,9 +632,6 @@ static struct id_reg_info id_aa64pfr0_el1_info = {
 
 static struct id_reg_info id_aa64pfr1_el1_info = {
 	.sys_reg = SYS_ID_AA64PFR1_EL1,
-	.ignore_mask = ARM64_FEATURE_MASK(ID_AA64PFR1_RASFRAC) |
-		       ARM64_FEATURE_MASK(ID_AA64PFR1_MPAMFRAC) |
-		       ARM64_FEATURE_MASK(ID_AA64PFR1_CSV2FRAC),
 	.init = init_id_aa64pfr1_el1_info,
 	.validate = validate_id_aa64pfr1_el1,
 	.get_reset_val = get_reset_id_aa64pfr1_el1,
@@ -3198,10 +3195,79 @@ int kvm_arm_copy_sys_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices)
 	return write_demux_regids(uindices);
 }
 
+/* ID register's fractional field information with its feature field. */
+struct feature_frac {
+	u32	frac_id;
+	u32	id;
+	u64	frac_mask;
+	u64	mask;
+};
+
+static const struct feature_frac feature_frac_table[] = {
+	{
+		.frac_id = SYS_ID_AA64PFR1_EL1,
+		.frac_mask = ARM64_FEATURE_MASK(ID_AA64PFR1_RASFRAC),
+		.id = SYS_ID_AA64PFR0_EL1,
+		.mask = ARM64_FEATURE_MASK(ID_AA64PFR0_RAS),
+	},
+	{
+		.frac_id = SYS_ID_AA64PFR1_EL1,
+		.frac_mask = ARM64_FEATURE_MASK(ID_AA64PFR1_MPAMFRAC),
+		.id = SYS_ID_AA64PFR0_EL1,
+		.mask = ARM64_FEATURE_MASK(ID_AA64PFR0_MPAM),
+	},
+	{
+		.frac_id = SYS_ID_AA64PFR1_EL1,
+		.frac_mask = ARM64_FEATURE_MASK(ID_AA64PFR1_CSV2FRAC),
+		.id = SYS_ID_AA64PFR0_EL1,
+		.mask = ARM64_FEATURE_MASK(ID_AA64PFR0_CSV2),
+	},
+};
+
+/*
+ * Return non-zero if the feature/fractional fields pair are not
+ * supported. Return zero otherwise.
+ */
+static int vcpu_id_reg_feature_frac_check(const struct kvm_vcpu *vcpu,
+					  const struct feature_frac *ftr_frac)
+{
+	const struct id_reg_info *id_reg;
+	u32 id;
+	u64 val, lim;
+	int err;
+
+	/* Check the feature field */
+	id = ftr_frac->id;
+	val = __vcpu_sys_reg(vcpu, IDREG_SYS_IDX(id)) & ftr_frac->mask;
+	id_reg = GET_ID_REG_INFO(id);
+	lim = id_reg ? id_reg->vcpu_limit_val : read_sanitised_ftr_reg(id);
+	lim &= ftr_frac->mask;
+	err = arm64_check_features(id, val, lim);
+	if (err)
+		/* The feature version is larger than the limit. */
+		return err;
+
+	if (val != lim)
+		/*
+		 * The feature version is smaller than the limit.
+		 * Any fractional version should be fine.
+		 */
+		return 0;
+
+	/* Check the fractional field */
+	id = ftr_frac->frac_id;
+	val = __vcpu_sys_reg(vcpu, IDREG_SYS_IDX(id)) & ftr_frac->frac_mask;
+	id_reg = GET_ID_REG_INFO(id);
+	lim = id_reg ? id_reg->vcpu_limit_val : read_sanitised_ftr_reg(id);
+	lim &= ftr_frac->frac_mask;
+	return arm64_check_features(id, val, lim);
+}
+
 int kvm_id_regs_consistency_check(const struct kvm_vcpu *vcpu)
 {
-	int i;
+	int i, err;
 	const struct kvm_vcpu *t_vcpu;
+	const struct feature_frac *frac;
 
 	/*
 	 * Make sure vcpu->arch.has_run_once is visible for others so that
@@ -3222,6 +3288,17 @@ int kvm_id_regs_consistency_check(const struct kvm_vcpu *vcpu)
 					KVM_ARM_ID_REG_MAX_NUM))
 			return -EINVAL;
 	}
+
+	/*
+	 * Check ID registers' fractional fields, which aren't checked
+	 * when userspace sets.
+	 */
+	for (i = 0; i < ARRAY_SIZE(feature_frac_table); i++) {
+		frac = &feature_frac_table[i];
+		err = vcpu_id_reg_feature_frac_check(vcpu, frac);
+		if (err)
+			return err;
+	}
 	return 0;
 }
 
@@ -3240,6 +3317,19 @@ static void id_reg_info_init_all(void)
 		else
 			id_reg_info_init(id_reg);
 	}
+
+	/*
+	 * Update ignore_mask of ID registers based on fractional fields
+	 * information.  Any ID register that have fractional fields
+	 * is expected to have its own id_reg_info.
+	 */
+	for (i = 0; i < ARRAY_SIZE(feature_frac_table); i++) {
+		id_reg = GET_ID_REG_INFO(feature_frac_table[i].frac_id);
+		if (WARN_ON_ONCE(!id_reg))
+			continue;
+
+		id_reg->ignore_mask |= feature_frac_table[i].frac_mask;
+	}
 }
 
 void kvm_sys_reg_table_init(void)
-- 
2.33.0.882.g93a45727a2-goog


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

* [RFC PATCH 15/25] KVM: arm64: Introduce KVM_CAP_ARM_ID_REG_WRITABLE capability
  2021-10-12  4:35 [RFC PATCH 00/25] KVM: arm64: Make CPU ID registers writable by userspace Reiji Watanabe
                   ` (13 preceding siblings ...)
  2021-10-12  4:35 ` [RFC PATCH 14/25] KVM: arm64: Add consistency checking for frac fields of ID registers Reiji Watanabe
@ 2021-10-12  4:35 ` Reiji Watanabe
  2021-10-12  4:35 ` [RFC PATCH 16/25] KVM: arm64: Use vcpu->arch cptr_el2 to track value of cptr_el2 for VHE Reiji Watanabe
                   ` (9 subsequent siblings)
  24 siblings, 0 replies; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-12  4:35 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Will Deacon, Andrew Jones,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

Introduce a new capability KVM_CAP_ARM_ID_REG_WRITABLE to indicate
that ID registers are writable by userspace.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 Documentation/virt/kvm/api.rst | 8 ++++++++
 arch/arm64/kvm/arm.c           | 1 +
 include/uapi/linux/kvm.h       | 1 +
 3 files changed, 10 insertions(+)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index a6729c8cf063..f7dfb5127310 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -7265,3 +7265,11 @@ The argument to KVM_ENABLE_CAP is also a bitmask, and must be a subset
 of the result of KVM_CHECK_EXTENSION.  KVM will forward to userspace
 the hypercalls whose corresponding bit is in the argument, and return
 ENOSYS for the others.
+
+8.35 KVM_CAP_ARM_ID_REG_WRITABLE
+--------------------------------
+
+:Architectures: arm64
+
+This capability indicates that userspace can modify the ID registers
+via KVM_SET_ONE_REG ioctl.
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 820f35051d77..29c13a32dd21 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -197,6 +197,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	case KVM_CAP_SET_GUEST_DEBUG:
 	case KVM_CAP_VCPU_ATTRIBUTES:
 	case KVM_CAP_PTP_KVM:
+	case KVM_CAP_ARM_ID_REG_WRITABLE:
 		r = 1;
 		break;
 	case KVM_CAP_SET_GUEST_DEBUG2:
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index a067410ebea5..3345a57f05a6 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1112,6 +1112,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_BINARY_STATS_FD 203
 #define KVM_CAP_EXIT_ON_EMULATION_FAILURE 204
 #define KVM_CAP_ARM_MTE 205
+#define KVM_CAP_ARM_ID_REG_WRITABLE 206
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
2.33.0.882.g93a45727a2-goog


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

* [RFC PATCH 16/25] KVM: arm64: Use vcpu->arch cptr_el2 to track value of cptr_el2 for VHE
  2021-10-12  4:35 [RFC PATCH 00/25] KVM: arm64: Make CPU ID registers writable by userspace Reiji Watanabe
                   ` (14 preceding siblings ...)
  2021-10-12  4:35 ` [RFC PATCH 15/25] KVM: arm64: Introduce KVM_CAP_ARM_ID_REG_WRITABLE capability Reiji Watanabe
@ 2021-10-12  4:35 ` Reiji Watanabe
  2021-10-12  4:35 ` [RFC PATCH 17/25] KVM: arm64: Use vcpu->arch.mdcr_el2 to track value of mdcr_el2 Reiji Watanabe
                   ` (8 subsequent siblings)
  24 siblings, 0 replies; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-12  4:35 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Will Deacon, Andrew Jones,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

Track the baseline guest value for cptr_el2 in struct kvm_vcpu_arch
for VHE.  Use this value when setting cptr_el2 for the guest.

Currently this value is unchanged, but the following patches will set
trapping bits based on features supported for the guest.

No functional change intended.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/include/asm/kvm_arm.h | 16 ++++++++++++++++
 arch/arm64/kvm/arm.c             |  5 ++++-
 arch/arm64/kvm/hyp/vhe/switch.c  | 14 ++------------
 3 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index 327120c0089f..f11ba1b6699d 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -288,6 +288,22 @@
 				 GENMASK(19, 14) |	\
 				 BIT(11))
 
+/*
+ * With VHE (HCR.E2H == 1), accesses to CPACR_EL1 are routed to
+ * CPTR_EL2. In general, CPACR_EL1 has the same layout as CPTR_EL2,
+ * except for some missing controls, such as TAM.
+ * In this case, CPTR_EL2.TAM has the same position with or without
+ * VHE (HCR.E2H == 1) which allows us to use here the CPTR_EL2.TAM
+ * shift value for trapping the AMU accesses.
+ */
+#define CPTR_EL2_VHE_GUEST_DEFAULT	(CPACR_EL1_TTA | CPTR_EL2_TAM)
+
+/*
+ * Bits that are copied from vcpu->arch.cptr_el2 to set cptr_el2 for
+ * guest with VHE.
+ */
+#define CPTR_EL2_VHE_GUEST_TRACKED_MASK	(CPACR_EL1_TTA | CPTR_EL2_TAM)
+
 /* Hyp Debug Configuration Register bits */
 #define MDCR_EL2_E2TB_MASK	(UL(0x3))
 #define MDCR_EL2_E2TB_SHIFT	(UL(24))
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 29c13a32dd21..32fd6edbd9e1 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1108,7 +1108,10 @@ static int kvm_arch_vcpu_ioctl_vcpu_init(struct kvm_vcpu *vcpu,
 	}
 
 	vcpu_reset_hcr(vcpu);
-	vcpu->arch.cptr_el2 = CPTR_EL2_DEFAULT;
+	if (has_vhe())
+		vcpu->arch.cptr_el2 = CPTR_EL2_VHE_GUEST_DEFAULT;
+	else
+		vcpu->arch.cptr_el2 = CPTR_EL2_DEFAULT;
 
 	/*
 	 * Handle the "start in power-off" case.
diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
index ded2c66675f0..b924e9d5e6fa 100644
--- a/arch/arm64/kvm/hyp/vhe/switch.c
+++ b/arch/arm64/kvm/hyp/vhe/switch.c
@@ -38,20 +38,10 @@ static void __activate_traps(struct kvm_vcpu *vcpu)
 	___activate_traps(vcpu);
 
 	val = read_sysreg(cpacr_el1);
-	val |= CPACR_EL1_TTA;
+	val &= ~CPTR_EL2_VHE_GUEST_TRACKED_MASK;
+	val |= (vcpu->arch.cptr_el2 & CPTR_EL2_VHE_GUEST_TRACKED_MASK);
 	val &= ~CPACR_EL1_ZEN;
 
-	/*
-	 * With VHE (HCR.E2H == 1), accesses to CPACR_EL1 are routed to
-	 * CPTR_EL2. In general, CPACR_EL1 has the same layout as CPTR_EL2,
-	 * except for some missing controls, such as TAM.
-	 * In this case, CPTR_EL2.TAM has the same position with or without
-	 * VHE (HCR.E2H == 1) which allows us to use here the CPTR_EL2.TAM
-	 * shift value for trapping the AMU accesses.
-	 */
-
-	val |= CPTR_EL2_TAM;
-
 	if (update_fp_enabled(vcpu)) {
 		if (vcpu_has_sve(vcpu))
 			val |= CPACR_EL1_ZEN;
-- 
2.33.0.882.g93a45727a2-goog


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

* [RFC PATCH 17/25] KVM: arm64: Use vcpu->arch.mdcr_el2 to track value of mdcr_el2
  2021-10-12  4:35 [RFC PATCH 00/25] KVM: arm64: Make CPU ID registers writable by userspace Reiji Watanabe
                   ` (15 preceding siblings ...)
  2021-10-12  4:35 ` [RFC PATCH 16/25] KVM: arm64: Use vcpu->arch cptr_el2 to track value of cptr_el2 for VHE Reiji Watanabe
@ 2021-10-12  4:35 ` Reiji Watanabe
  2021-10-12  4:35 ` [RFC PATCH 18/25] KVM: arm64: Introduce framework to trap disabled features Reiji Watanabe
                   ` (7 subsequent siblings)
  24 siblings, 0 replies; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-12  4:35 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Will Deacon, Andrew Jones,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

Track the baseline guest value for mdcr_el2 in struct kvm_vcpu_arch.
Use this value when setting mdcr_el2 for the guest.

Currently this value is unchanged, but the following patches will set
trapping bits based on features supported for the guest.

No functional change intended.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/include/asm/kvm_arm.h | 16 ++++++++++++++++
 arch/arm64/kvm/arm.c             |  1 +
 arch/arm64/kvm/debug.c           | 13 ++++---------
 3 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index f11ba1b6699d..e560e5549472 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -332,6 +332,22 @@
 				 BIT(18) |		\
 				 GENMASK(16, 15))
 
+/*
+ * The default value for the guest below also clears MDCR_EL2_E2PB_MASK
+ * and MDCR_EL2_E2TB_MASK to disable guest access to the profiling and
+ * trace buffers.
+ */
+#define MDCR_GUEST_FLAGS_DEFAULT				\
+	(MDCR_EL2_TPM  | MDCR_EL2_TPMS | MDCR_EL2_TTRF |	\
+	 MDCR_EL2_TPMCR | MDCR_EL2_TDRA | MDCR_EL2_TDOSA)
+
+/* Bits that are copied from vcpu->arch.mdcr_el2 to set mdcr_el2 for guest. */
+#define MDCR_GUEST_FLAGS_TRACKED_MASK				\
+	(MDCR_EL2_TPM  | MDCR_EL2_TPMS | MDCR_EL2_TTRF |	\
+	 MDCR_EL2_TPMCR | MDCR_EL2_TDRA | MDCR_EL2_TDOSA |	\
+	 (MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT))
+
+
 /* For compatibility with fault code shared with 32-bit */
 #define FSC_FAULT	ESR_ELx_FSC_FAULT
 #define FSC_ACCESS	ESR_ELx_FSC_ACCESS
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 32fd6edbd9e1..38301ddf8372 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1108,6 +1108,7 @@ static int kvm_arch_vcpu_ioctl_vcpu_init(struct kvm_vcpu *vcpu,
 	}
 
 	vcpu_reset_hcr(vcpu);
+	vcpu->arch.mdcr_el2 = MDCR_GUEST_FLAGS_DEFAULT;
 	if (has_vhe())
 		vcpu->arch.cptr_el2 = CPTR_EL2_VHE_GUEST_DEFAULT;
 	else
diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
index db9361338b2a..83330968a411 100644
--- a/arch/arm64/kvm/debug.c
+++ b/arch/arm64/kvm/debug.c
@@ -84,16 +84,11 @@ void kvm_arm_init_debug(void)
 static void kvm_arm_setup_mdcr_el2(struct kvm_vcpu *vcpu)
 {
 	/*
-	 * This also clears MDCR_EL2_E2PB_MASK and MDCR_EL2_E2TB_MASK
-	 * to disable guest access to the profiling and trace buffers
+	 * Keep the vcpu->arch.mdcr_el2 bits that are specified by
+	 * MDCR_GUEST_FLAGS_TRACKED_MASK.
 	 */
-	vcpu->arch.mdcr_el2 = __this_cpu_read(mdcr_el2) & MDCR_EL2_HPMN_MASK;
-	vcpu->arch.mdcr_el2 |= (MDCR_EL2_TPM |
-				MDCR_EL2_TPMS |
-				MDCR_EL2_TTRF |
-				MDCR_EL2_TPMCR |
-				MDCR_EL2_TDRA |
-				MDCR_EL2_TDOSA);
+	vcpu->arch.mdcr_el2 &= MDCR_GUEST_FLAGS_TRACKED_MASK;
+	vcpu->arch.mdcr_el2 |= __this_cpu_read(mdcr_el2) & MDCR_EL2_HPMN_MASK;
 
 	/* Is the VM being debugged by userspace? */
 	if (vcpu->guest_debug)
-- 
2.33.0.882.g93a45727a2-goog


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

* [RFC PATCH 18/25] KVM: arm64: Introduce framework to trap disabled features
  2021-10-12  4:35 [RFC PATCH 00/25] KVM: arm64: Make CPU ID registers writable by userspace Reiji Watanabe
                   ` (16 preceding siblings ...)
  2021-10-12  4:35 ` [RFC PATCH 17/25] KVM: arm64: Use vcpu->arch.mdcr_el2 to track value of mdcr_el2 Reiji Watanabe
@ 2021-10-12  4:35 ` Reiji Watanabe
  2021-10-12  4:35 ` [RFC PATCH 19/25] KVM: arm64: Trap disabled features of ID_AA64PFR0_EL1 Reiji Watanabe
                   ` (6 subsequent siblings)
  24 siblings, 0 replies; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-12  4:35 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Will Deacon, Andrew Jones,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

When a CPU feature that is supported on the host is not exposed to
its guest, emulating a real CPU's behavior (by trapping or disabling
guest's using the feature) is generally a desirable behavior (when
it's possible without any or little side effect).

Introduce feature_config_ctrl structure, which manages feature
information to program configuration register to trap or disable
the feature when the feature is not exposed to the guest, and
functions that uses the structure to activate trapping the feature.

At present, no feature has feature_config_ctrl yet and the following
patches will add the feature_config_ctrl for several features.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/kvm/sys_regs.c | 117 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 117 insertions(+)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 536e313992d4..55c514e21214 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -281,6 +281,35 @@ static bool trap_raz_wi(struct kvm_vcpu *vcpu,
 	(cpuid_feature_extract_unsigned_field(val, ID_AA64ISAR1_GPI_SHIFT) >= \
 	 ID_AA64ISAR1_GPI_IMP_DEF)
 
+enum vcpu_config_reg {
+	VCPU_HCR_EL2 = 1,
+	VCPU_MDCR_EL2,
+	VCPU_CPTR_EL2,
+};
+
+/*
+ * Feature information to program configuration register to trap or disable
+ * guest's using a feature when the feature is not exposed to the guest.
+ */
+struct feature_config_ctrl {
+	/* ID register/field for the feature */
+	u32	ftr_reg;	/* ID register */
+	bool	ftr_signed;	/* Is the feature field signed ? */
+	u8	ftr_shift;	/* Field of ID register for the feature */
+	s8	ftr_min;	/* Min value that indicate the feature */
+
+	/*
+	 * Function to check trapping is needed. This is used when the above
+	 * fields are not enough to determine if trapping is needed.
+	 */
+	bool	(*ftr_need_trap)(struct kvm_vcpu *vcpu);
+
+	/* Configuration register information to trap the feature. */
+	enum vcpu_config_reg cfg_reg;	/* Configuration register */
+	u64	cfg_mask;	/* Field of the configuration register */
+	u64	cfg_val;	/* Value that are set for the field */
+};
+
 struct id_reg_info {
 	u32	sys_reg;	/* Register ID */
 	u64	sys_val;	/* Sanitized system value */
@@ -302,6 +331,9 @@ struct id_reg_info {
 
 	/* Return the reset value of the register for the vCPU */
 	u64 (*get_reset_val)(struct kvm_vcpu *vcpu, struct id_reg_info *id_reg);
+
+	/* Information to trap features that are disabled for the guest */
+	const struct feature_config_ctrl *(*trap_features)[];
 };
 
 static void id_reg_info_init(struct id_reg_info *id_reg)
@@ -717,6 +749,47 @@ static int validate_id_reg(struct kvm_vcpu *vcpu,
 	return err;
 }
 
+static void feature_trap_activate(struct kvm_vcpu *vcpu,
+				  const struct feature_config_ctrl *config)
+{
+	u64 *reg_ptr, reg_val;
+
+	switch (config->cfg_reg) {
+	case VCPU_HCR_EL2:
+		reg_ptr = &vcpu->arch.hcr_el2;
+		break;
+	case VCPU_MDCR_EL2:
+		reg_ptr = &vcpu->arch.mdcr_el2;
+		break;
+	case VCPU_CPTR_EL2:
+		reg_ptr = &vcpu->arch.cptr_el2;
+		break;
+	}
+
+	/* Update cfg_mask fields with cfg_val */
+	reg_val = (*reg_ptr & ~config->cfg_mask);
+	reg_val |= config->cfg_val;
+	*reg_ptr = reg_val;
+}
+
+static inline bool feature_avail(const struct feature_config_ctrl *ctrl,
+				 u64 id_val)
+{
+	int field_val = cpuid_feature_extract_field(id_val,
+				ctrl->ftr_shift, ctrl->ftr_signed);
+
+	return (field_val >= ctrl->ftr_min);
+}
+
+static inline bool vcpu_feature_is_available(struct kvm_vcpu *vcpu,
+					const struct feature_config_ctrl *ctrl)
+{
+	u64 val;
+
+	val = __vcpu_sys_reg(vcpu, IDREG_SYS_IDX(ctrl->ftr_reg));
+	return feature_avail(ctrl, val);
+}
+
 /*
  * ARMv8.1 mandates at least a trivial LORegion implementation, where all the
  * RW registers are RES0 (which we can implement as RAZ/WI). On an ARMv8.0
@@ -1570,6 +1643,42 @@ static int reg_from_user(u64 *val, const void __user *uaddr, u64 id);
 static int reg_to_user(void __user *uaddr, const u64 *val, u64 id);
 static u64 sys_reg_to_index(const struct sys_reg_desc *reg);
 
+static void id_reg_features_trap_activate(struct kvm_vcpu *vcpu,
+					  const struct id_reg_info *id_reg)
+{
+	u64 val;
+	int i = 0;
+	const struct feature_config_ctrl **ctrlp_array, *ctrl;
+
+	if (!id_reg || !id_reg->trap_features)
+		/* No information to trap a feature */
+		return;
+
+	val = __vcpu_sys_reg(vcpu, IDREG_SYS_IDX(id_reg->sys_reg));
+	if (val == id_reg->sys_val)
+		/* No feature needs to be trapped (no feature is disabled). */
+		return;
+
+	ctrlp_array = *id_reg->trap_features;
+	while ((ctrl = ctrlp_array[i++]) != NULL) {
+		if (ctrl->ftr_need_trap && ctrl->ftr_need_trap(vcpu)) {
+			feature_trap_activate(vcpu, ctrl);
+			continue;
+		}
+
+		if (!feature_avail(ctrl, id_reg->sys_val))
+			/* The feature is not supported on the host. */
+			continue;
+
+		if (feature_avail(ctrl, val))
+			/* The feature is enabled for the guest. */
+			continue;
+
+		/* The feature is supported but disabled. */
+		feature_trap_activate(vcpu, ctrl);
+	}
+}
+
 /* Visibility overrides for SVE-specific control registers */
 static unsigned int sve_visibility(const struct kvm_vcpu *vcpu,
 				   const struct sys_reg_desc *rd)
@@ -3195,6 +3304,14 @@ int kvm_arm_copy_sys_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices)
 	return write_demux_regids(uindices);
 }
 
+void kvm_vcpu_id_regs_trap_activate(struct kvm_vcpu *vcpu)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(id_reg_info_table); i++)
+		id_reg_features_trap_activate(vcpu, id_reg_info_table[i]);
+}
+
 /* ID register's fractional field information with its feature field. */
 struct feature_frac {
 	u32	frac_id;
-- 
2.33.0.882.g93a45727a2-goog


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

* [RFC PATCH 19/25] KVM: arm64: Trap disabled features of ID_AA64PFR0_EL1
  2021-10-12  4:35 [RFC PATCH 00/25] KVM: arm64: Make CPU ID registers writable by userspace Reiji Watanabe
                   ` (17 preceding siblings ...)
  2021-10-12  4:35 ` [RFC PATCH 18/25] KVM: arm64: Introduce framework to trap disabled features Reiji Watanabe
@ 2021-10-12  4:35 ` Reiji Watanabe
  2021-10-12  4:35 ` [RFC PATCH 20/25] KVM: arm64: Trap disabled features of ID_AA64PFR1_EL1 Reiji Watanabe
                   ` (5 subsequent siblings)
  24 siblings, 0 replies; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-12  4:35 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Will Deacon, Andrew Jones,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

Add feature_config_ctrl for RAS and AMU, which are indicated in
ID_AA64PFR0_EL1, to program configuration registers to trap
guest's using those features when they are not exposed to the guest.

Introduce trap_ras_regs() to change a behavior of guest's access to
the registers, which is currently raz/wi, depending on the feature's
availability for the guest (and inject undefined instruction
exception when guest's RAS register access are trapped and RAS is
not exposed to the guest).  In order to keep the current visibility
of the RAS registers from userspace (always visible), a visibility
function for RAS registers is not added.

No code is added for AMU's access/visibility handler because the
current code already injects the exception for Guest's AMU register
access unconditionally because AMU is never exposed to the guest.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/kvm/sys_regs.c | 54 +++++++++++++++++++++++++++++++++------
 1 file changed, 46 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 55c514e21214..2b45db310151 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -310,6 +310,27 @@ struct feature_config_ctrl {
 	u64	cfg_val;	/* Value that are set for the field */
 };
 
+/* For ID_AA64PFR0_EL1 */
+static struct feature_config_ctrl ftr_ctrl_ras = {
+	.ftr_reg = SYS_ID_AA64PFR0_EL1,
+	.ftr_shift = ID_AA64PFR0_RAS_SHIFT,
+	.ftr_min = ID_AA64PFR0_RAS_V1,
+	.ftr_signed = FTR_UNSIGNED,
+	.cfg_reg = VCPU_HCR_EL2,
+	.cfg_mask = (HCR_TERR | HCR_TEA | HCR_FIEN),
+	.cfg_val = (HCR_TERR | HCR_TEA),
+};
+
+static struct feature_config_ctrl ftr_ctrl_amu = {
+	.ftr_reg = SYS_ID_AA64PFR0_EL1,
+	.ftr_shift = ID_AA64PFR0_AMU_SHIFT,
+	.ftr_min = ID_AA64PFR0_AMU,
+	.ftr_signed = FTR_UNSIGNED,
+	.cfg_reg = VCPU_CPTR_EL2,
+	.cfg_mask = CPTR_EL2_TAM,
+	.cfg_val = CPTR_EL2_TAM,
+};
+
 struct id_reg_info {
 	u32	sys_reg;	/* Register ID */
 	u64	sys_val;	/* Sanitized system value */
@@ -660,6 +681,11 @@ static struct id_reg_info id_aa64pfr0_el1_info = {
 	.init = init_id_aa64pfr0_el1_info,
 	.validate = validate_id_aa64pfr0_el1,
 	.get_reset_val = get_reset_id_aa64pfr0_el1,
+	.trap_features = &(const struct feature_config_ctrl *[]) {
+		&ftr_ctrl_ras,
+		&ftr_ctrl_amu,
+		NULL,
+	},
 };
 
 static struct id_reg_info id_aa64pfr1_el1_info = {
@@ -790,6 +816,18 @@ static inline bool vcpu_feature_is_available(struct kvm_vcpu *vcpu,
 	return feature_avail(ctrl, val);
 }
 
+static bool trap_ras_regs(struct kvm_vcpu *vcpu,
+			  struct sys_reg_params *p,
+			  const struct sys_reg_desc *r)
+{
+	if (!vcpu_feature_is_available(vcpu, &ftr_ctrl_ras)) {
+		kvm_inject_undefined(vcpu);
+		return false;
+	}
+
+	return trap_raz_wi(vcpu, p, r);
+}
+
 /*
  * ARMv8.1 mandates at least a trivial LORegion implementation, where all the
  * RW registers are RES0 (which we can implement as RAZ/WI). On an ARMv8.0
@@ -2052,14 +2090,14 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	{ SYS_DESC(SYS_AFSR1_EL1), access_vm_reg, reset_unknown, AFSR1_EL1 },
 	{ SYS_DESC(SYS_ESR_EL1), access_vm_reg, reset_unknown, ESR_EL1 },
 
-	{ SYS_DESC(SYS_ERRIDR_EL1), trap_raz_wi },
-	{ SYS_DESC(SYS_ERRSELR_EL1), trap_raz_wi },
-	{ SYS_DESC(SYS_ERXFR_EL1), trap_raz_wi },
-	{ SYS_DESC(SYS_ERXCTLR_EL1), trap_raz_wi },
-	{ SYS_DESC(SYS_ERXSTATUS_EL1), trap_raz_wi },
-	{ SYS_DESC(SYS_ERXADDR_EL1), trap_raz_wi },
-	{ SYS_DESC(SYS_ERXMISC0_EL1), trap_raz_wi },
-	{ SYS_DESC(SYS_ERXMISC1_EL1), trap_raz_wi },
+	{ SYS_DESC(SYS_ERRIDR_EL1), trap_ras_regs },
+	{ SYS_DESC(SYS_ERRSELR_EL1), trap_ras_regs },
+	{ SYS_DESC(SYS_ERXFR_EL1), trap_ras_regs },
+	{ SYS_DESC(SYS_ERXCTLR_EL1), trap_ras_regs },
+	{ SYS_DESC(SYS_ERXSTATUS_EL1), trap_ras_regs },
+	{ SYS_DESC(SYS_ERXADDR_EL1), trap_ras_regs },
+	{ SYS_DESC(SYS_ERXMISC0_EL1), trap_ras_regs },
+	{ SYS_DESC(SYS_ERXMISC1_EL1), trap_ras_regs },
 
 	MTE_REG(TFSR_EL1),
 	MTE_REG(TFSRE0_EL1),
-- 
2.33.0.882.g93a45727a2-goog


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

* [RFC PATCH 20/25] KVM: arm64: Trap disabled features of ID_AA64PFR1_EL1
  2021-10-12  4:35 [RFC PATCH 00/25] KVM: arm64: Make CPU ID registers writable by userspace Reiji Watanabe
                   ` (18 preceding siblings ...)
  2021-10-12  4:35 ` [RFC PATCH 19/25] KVM: arm64: Trap disabled features of ID_AA64PFR0_EL1 Reiji Watanabe
@ 2021-10-12  4:35 ` Reiji Watanabe
  2021-10-12  4:35 ` [RFC PATCH 21/25] KVM: arm64: Trap disabled features of ID_AA64DFR0_EL1 Reiji Watanabe
                   ` (4 subsequent siblings)
  24 siblings, 0 replies; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-12  4:35 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Will Deacon, Andrew Jones,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

Add feature_config_ctrl for MTE, which is indicated in
ID_AA64PFR1_EL1, to program configuration register to trap the
guest's using the feature when it is not exposed to the guest.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/kvm/sys_regs.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 2b45db310151..595ab1c79bae 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -331,6 +331,17 @@ static struct feature_config_ctrl ftr_ctrl_amu = {
 	.cfg_val = CPTR_EL2_TAM,
 };
 
+/* For ID_AA64PFR1_EL1 */
+static struct feature_config_ctrl ftr_ctrl_mte = {
+	.ftr_reg = SYS_ID_AA64PFR1_EL1,
+	.ftr_shift = ID_AA64PFR1_MTE_SHIFT,
+	.ftr_min = ID_AA64PFR1_MTE_EL0,
+	.ftr_signed = FTR_UNSIGNED,
+	.cfg_reg = VCPU_HCR_EL2,
+	.cfg_mask = (HCR_TID5 | HCR_DCT | HCR_ATA),
+	.cfg_val = HCR_TID5,
+};
+
 struct id_reg_info {
 	u32	sys_reg;	/* Register ID */
 	u64	sys_val;	/* Sanitized system value */
@@ -693,6 +704,10 @@ static struct id_reg_info id_aa64pfr1_el1_info = {
 	.init = init_id_aa64pfr1_el1_info,
 	.validate = validate_id_aa64pfr1_el1,
 	.get_reset_val = get_reset_id_aa64pfr1_el1,
+	.trap_features = &(const struct feature_config_ctrl *[]) {
+		&ftr_ctrl_mte,
+		NULL,
+	},
 };
 
 static struct id_reg_info id_aa64isar0_el1_info = {
-- 
2.33.0.882.g93a45727a2-goog


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

* [RFC PATCH 21/25] KVM: arm64: Trap disabled features of ID_AA64DFR0_EL1
  2021-10-12  4:35 [RFC PATCH 00/25] KVM: arm64: Make CPU ID registers writable by userspace Reiji Watanabe
                   ` (19 preceding siblings ...)
  2021-10-12  4:35 ` [RFC PATCH 20/25] KVM: arm64: Trap disabled features of ID_AA64PFR1_EL1 Reiji Watanabe
@ 2021-10-12  4:35 ` Reiji Watanabe
  2021-10-12  4:35 ` [RFC PATCH 22/25] KVM: arm64: Trap disabled features of ID_AA64MMFR1_EL1 Reiji Watanabe
                   ` (3 subsequent siblings)
  24 siblings, 0 replies; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-12  4:35 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Will Deacon, Andrew Jones,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

Add feature_config_ctrl for PMUv3, PMS and TraceFilt, which are
indicated in ID_AA64DFR0_EL1, to program configuration registers
to trap guest's using those features when they are not exposed to
the guest.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/kvm/sys_regs.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 595ab1c79bae..23a3bcac4986 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -342,6 +342,38 @@ static struct feature_config_ctrl ftr_ctrl_mte = {
 	.cfg_val = HCR_TID5,
 };
 
+/* For ID_AA64DFR0_EL1 */
+static struct feature_config_ctrl ftr_ctrl_pmuv3 = {
+	.ftr_reg = SYS_ID_AA64DFR0_EL1,
+	.ftr_shift = ID_AA64DFR0_PMUVER_SHIFT,
+	.ftr_min = ID_AA64DFR0_PMUVER_8_0,
+	.ftr_signed = FTR_UNSIGNED,
+	.cfg_reg = VCPU_MDCR_EL2,
+	.cfg_mask = MDCR_EL2_TPM,
+	.cfg_val = MDCR_EL2_TPM,
+};
+
+static struct feature_config_ctrl ftr_ctrl_pms = {
+	.ftr_reg = SYS_ID_AA64DFR0_EL1,
+	.ftr_shift = ID_AA64DFR0_PMSVER_SHIFT,
+	.ftr_min = ID_AA64DFR0_PMSVER_8_2,
+	.ftr_signed = FTR_UNSIGNED,
+	.cfg_reg = VCPU_MDCR_EL2,
+	.cfg_mask = (MDCR_EL2_TPMS |
+			(MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT)),
+	.cfg_val = MDCR_EL2_TPMS,
+};
+
+static struct feature_config_ctrl ftr_ctrl_tracefilt = {
+	.ftr_reg = SYS_ID_AA64DFR0_EL1,
+	.ftr_shift = ID_AA64DFR0_TRACE_FILT_SHIFT,
+	.ftr_min = 1,
+	.ftr_signed = FTR_UNSIGNED,
+	.cfg_reg = VCPU_MDCR_EL2,
+	.cfg_mask = MDCR_EL2_TTRF,
+	.cfg_val = MDCR_EL2_TTRF,
+};
+
 struct id_reg_info {
 	u32	sys_reg;	/* Register ID */
 	u64	sys_val;	/* Sanitized system value */
@@ -727,6 +759,12 @@ static struct id_reg_info id_aa64dfr0_el1_info = {
 	.init = init_id_aa64dfr0_el1_info,
 	.validate = validate_id_aa64dfr0_el1,
 	.get_reset_val = get_reset_id_aa64dfr0_el1,
+	.trap_features = &(const struct feature_config_ctrl *[]) {
+		&ftr_ctrl_pmuv3,
+		&ftr_ctrl_pms,
+		&ftr_ctrl_tracefilt,
+		NULL,
+	},
 };
 
 static struct id_reg_info id_dfr0_el1_info = {
-- 
2.33.0.882.g93a45727a2-goog


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

* [RFC PATCH 22/25] KVM: arm64: Trap disabled features of ID_AA64MMFR1_EL1
  2021-10-12  4:35 [RFC PATCH 00/25] KVM: arm64: Make CPU ID registers writable by userspace Reiji Watanabe
                   ` (20 preceding siblings ...)
  2021-10-12  4:35 ` [RFC PATCH 21/25] KVM: arm64: Trap disabled features of ID_AA64DFR0_EL1 Reiji Watanabe
@ 2021-10-12  4:35 ` Reiji Watanabe
  2021-10-12  4:35 ` [RFC PATCH 23/25] KVM: arm64: Trap disabled features of ID_AA64ISAR1_EL1 Reiji Watanabe
                   ` (2 subsequent siblings)
  24 siblings, 0 replies; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-12  4:35 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Will Deacon, Andrew Jones,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

Add feature_config_ctrl for LORegions, which is indicated in
ID_AA64MMFR1_EL1, to program configuration register to trap
guest's using the feature when it is not exposed to the guest.

Change trap_loregion() to use vcpu_feature_is_available()
to simplify checking of the feature's availability.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/kvm/sys_regs.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 23a3bcac4986..68df4e4eba05 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -374,6 +374,17 @@ static struct feature_config_ctrl ftr_ctrl_tracefilt = {
 	.cfg_val = MDCR_EL2_TTRF,
 };
 
+/* For ID_AA64MMFR1_EL1 */
+static struct feature_config_ctrl ftr_ctrl_lor = {
+	.ftr_reg = SYS_ID_AA64MMFR1_EL1,
+	.ftr_shift = ID_AA64MMFR1_LOR_SHIFT,
+	.ftr_min = 1,
+	.ftr_signed = FTR_UNSIGNED,
+	.cfg_reg = VCPU_HCR_EL2,
+	.cfg_mask = HCR_TLOR,
+	.cfg_val = HCR_TLOR,
+};
+
 struct id_reg_info {
 	u32	sys_reg;	/* Register ID */
 	u64	sys_val;	/* Sanitized system value */
@@ -767,6 +778,14 @@ static struct id_reg_info id_aa64dfr0_el1_info = {
 	},
 };
 
+static struct id_reg_info id_aa64mmfr1_el1_info = {
+	.sys_reg = SYS_ID_AA64MMFR1_EL1,
+	.trap_features = &(const struct feature_config_ctrl *[]) {
+		&ftr_ctrl_lor,
+		NULL,
+	},
+};
+
 static struct id_reg_info id_dfr0_el1_info = {
 	.sys_reg = SYS_ID_DFR0_EL1,
 	.init = init_id_dfr0_el1_info,
@@ -795,6 +814,7 @@ static struct id_reg_info *id_reg_info_table[KVM_ARM_ID_REG_MAX_NUM] = {
 	[IDREG_IDX(SYS_ID_AA64DFR0_EL1)] = &id_aa64dfr0_el1_info,
 	[IDREG_IDX(SYS_ID_AA64ISAR0_EL1)] = &id_aa64isar0_el1_info,
 	[IDREG_IDX(SYS_ID_AA64ISAR1_EL1)] = &id_aa64isar1_el1_info,
+	[IDREG_IDX(SYS_ID_AA64MMFR1_EL1)] = &id_aa64mmfr1_el1_info,
 };
 
 static int validate_id_reg(struct kvm_vcpu *vcpu,
@@ -891,10 +911,9 @@ static bool trap_loregion(struct kvm_vcpu *vcpu,
 			  struct sys_reg_params *p,
 			  const struct sys_reg_desc *r)
 {
-	u64 val = __vcpu_sys_reg(vcpu, IDREG_SYS_IDX(SYS_ID_AA64MMFR1_EL1));
 	u32 sr = reg_to_encoding(r);
 
-	if (!(val & (0xfUL << ID_AA64MMFR1_LOR_SHIFT))) {
+	if (!vcpu_feature_is_available(vcpu, &ftr_ctrl_lor)) {
 		kvm_inject_undefined(vcpu);
 		return false;
 	}
-- 
2.33.0.882.g93a45727a2-goog


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

* [RFC PATCH 23/25] KVM: arm64: Trap disabled features of ID_AA64ISAR1_EL1
  2021-10-12  4:35 [RFC PATCH 00/25] KVM: arm64: Make CPU ID registers writable by userspace Reiji Watanabe
                   ` (21 preceding siblings ...)
  2021-10-12  4:35 ` [RFC PATCH 22/25] KVM: arm64: Trap disabled features of ID_AA64MMFR1_EL1 Reiji Watanabe
@ 2021-10-12  4:35 ` Reiji Watanabe
  2021-10-12  4:35 ` [RFC PATCH 24/25] KVM: arm64: Activate trapping of disabled CPU features for the guest Reiji Watanabe
  2021-10-12  4:35 ` [RFC PATCH 25/25] KVM: arm64: selftests: Introduce id_reg_test Reiji Watanabe
  24 siblings, 0 replies; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-12  4:35 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Will Deacon, Andrew Jones,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

Add feature_config_ctrl for PTRAUTH, which is indicated in
ID_AA64ISAR1_EL1, to program configuration register to trap
guest's using the feature when it is not exposed to the guest.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/kvm/sys_regs.c | 41 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 68df4e4eba05..e74b33741dc4 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -281,6 +281,35 @@ static bool trap_raz_wi(struct kvm_vcpu *vcpu,
 	(cpuid_feature_extract_unsigned_field(val, ID_AA64ISAR1_GPI_SHIFT) >= \
 	 ID_AA64ISAR1_GPI_IMP_DEF)
 
+/*
+ * Return true if ptrauth needs to be trapped.
+ * (i.e. if ptrauth is supported on the host but not exposed to the guest)
+ */
+static bool vcpu_need_trap_ptrauth(struct kvm_vcpu *vcpu)
+{
+	u64 val;
+	unsigned int gpi, gpa, api, apa;
+	unsigned int generic, address;
+
+	if (!system_has_full_ptr_auth())
+		/* The feature is not supported. */
+		return false;
+
+	val = __vcpu_sys_reg(vcpu, IDREG_SYS_IDX(SYS_ID_AA64ISAR1_EL1));
+	gpi = aa64isar1_has_gpi(val);
+	gpa = aa64isar1_has_gpa(val);
+	api = aa64isar1_has_api(val);
+	apa = aa64isar1_has_apa(val);
+	generic = gpi || gpa;
+	address = api || apa;
+	if (generic && address)
+		/* The feature is available. */
+		return false;
+
+	/* The feature is supported but hidden. */
+	return true;
+}
+
 enum vcpu_config_reg {
 	VCPU_HCR_EL2 = 1,
 	VCPU_MDCR_EL2,
@@ -385,6 +414,14 @@ static struct feature_config_ctrl ftr_ctrl_lor = {
 	.cfg_val = HCR_TLOR,
 };
 
+/* For SYS_ID_AA64ISAR1_EL1 */
+static struct feature_config_ctrl ftr_ctrl_ptrauth = {
+	.ftr_need_trap = vcpu_need_trap_ptrauth,
+	.cfg_reg = VCPU_HCR_EL2,
+	.cfg_mask = (HCR_API | HCR_APK),
+	.cfg_val = 0,
+};
+
 struct id_reg_info {
 	u32	sys_reg;	/* Register ID */
 	u64	sys_val;	/* Sanitized system value */
@@ -763,6 +800,10 @@ static struct id_reg_info id_aa64isar1_el1_info = {
 	.init = init_id_aa64isar1_el1_info,
 	.validate = validate_id_aa64isar1_el1,
 	.get_reset_val = get_reset_id_aa64isar1_el1,
+	.trap_features = &(const struct feature_config_ctrl *[]) {
+		&ftr_ctrl_ptrauth,
+		NULL,
+	},
 };
 
 static struct id_reg_info id_aa64dfr0_el1_info = {
-- 
2.33.0.882.g93a45727a2-goog


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

* [RFC PATCH 24/25] KVM: arm64: Activate trapping of disabled CPU features for the guest
  2021-10-12  4:35 [RFC PATCH 00/25] KVM: arm64: Make CPU ID registers writable by userspace Reiji Watanabe
                   ` (22 preceding siblings ...)
  2021-10-12  4:35 ` [RFC PATCH 23/25] KVM: arm64: Trap disabled features of ID_AA64ISAR1_EL1 Reiji Watanabe
@ 2021-10-12  4:35 ` Reiji Watanabe
  2021-10-12  4:35 ` [RFC PATCH 25/25] KVM: arm64: selftests: Introduce id_reg_test Reiji Watanabe
  24 siblings, 0 replies; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-12  4:35 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Will Deacon, Andrew Jones,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

Call kvm_vcpu_id_regs_trap_activate() at the first KVM_RUN
to activate trapping of disabled CPU features.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/include/asm/kvm_host.h | 1 +
 arch/arm64/kvm/arm.c              | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 691cb6ee0f5c..41f785fdc2e6 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -743,6 +743,7 @@ long kvm_vm_ioctl_mte_copy_tags(struct kvm *kvm,
 				struct kvm_arm_copy_mte_tags *copy_tags);
 
 int kvm_id_regs_consistency_check(const struct kvm_vcpu *vcpu);
+void kvm_vcpu_id_regs_trap_activate(struct kvm_vcpu *vcpu);
 
 /* Guest/host FPSIMD coordination helpers */
 int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu);
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 38301ddf8372..6b392d7feab7 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -606,6 +606,8 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu)
 	if (kvm_id_regs_consistency_check(vcpu))
 		return -EPERM;
 
+	kvm_vcpu_id_regs_trap_activate(vcpu);
+
 	return ret;
 }
 
-- 
2.33.0.882.g93a45727a2-goog


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

* [RFC PATCH 25/25] KVM: arm64: selftests: Introduce id_reg_test
  2021-10-12  4:35 [RFC PATCH 00/25] KVM: arm64: Make CPU ID registers writable by userspace Reiji Watanabe
                   ` (23 preceding siblings ...)
  2021-10-12  4:35 ` [RFC PATCH 24/25] KVM: arm64: Activate trapping of disabled CPU features for the guest Reiji Watanabe
@ 2021-10-12  4:35 ` Reiji Watanabe
  24 siblings, 0 replies; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-12  4:35 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm
  Cc: kvm, linux-arm-kernel, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Paolo Bonzini, Will Deacon, Andrew Jones,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata, Reiji Watanabe

Introduce a test for aarch64 to validate basic behavior of
KVM_GET_ONE_REG and KVM_SET_ONE_REG for ID registers.

This test runs only when KVM_CAP_ARM_ID_REG_WRITABLE is supported.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 tools/arch/arm64/include/asm/sysreg.h         |    1 +
 tools/testing/selftests/kvm/.gitignore        |    1 +
 tools/testing/selftests/kvm/Makefile          |    1 +
 .../selftests/kvm/aarch64/id_reg_test.c       | 1296 +++++++++++++++++
 4 files changed, 1299 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/aarch64/id_reg_test.c

diff --git a/tools/arch/arm64/include/asm/sysreg.h b/tools/arch/arm64/include/asm/sysreg.h
index 7640fa27be94..be3947c125f1 100644
--- a/tools/arch/arm64/include/asm/sysreg.h
+++ b/tools/arch/arm64/include/asm/sysreg.h
@@ -793,6 +793,7 @@
 #define ID_AA64PFR0_ELx_32BIT_64BIT	0x2
 
 /* id_aa64pfr1 */
+#define ID_AA64PFR1_CSV2FRAC_SHIFT	32
 #define ID_AA64PFR1_MPAMFRAC_SHIFT	16
 #define ID_AA64PFR1_RASFRAC_SHIFT	12
 #define ID_AA64PFR1_MTE_SHIFT		8
diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
index 02444fc69bae..82f37dd7faa2 100644
--- a/tools/testing/selftests/kvm/.gitignore
+++ b/tools/testing/selftests/kvm/.gitignore
@@ -2,6 +2,7 @@
 /aarch64/arch_timer
 /aarch64/debug-exceptions
 /aarch64/get-reg-list
+/aarch64/id_reg_test
 /aarch64/psci_cpu_on_test
 /aarch64/vgic_init
 /s390x/memop
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 79947dde0b66..65e4bb1ebfcb 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -89,6 +89,7 @@ TEST_GEN_PROGS_x86_64 += kvm_binary_stats_test
 TEST_GEN_PROGS_aarch64 += aarch64/arch_timer
 TEST_GEN_PROGS_aarch64 += aarch64/debug-exceptions
 TEST_GEN_PROGS_aarch64 += aarch64/get-reg-list
+TEST_GEN_PROGS_aarch64 += aarch64/id_reg_test
 TEST_GEN_PROGS_aarch64 += aarch64/psci_cpu_on_test
 TEST_GEN_PROGS_aarch64 += aarch64/vgic_init
 TEST_GEN_PROGS_aarch64 += demand_paging_test
diff --git a/tools/testing/selftests/kvm/aarch64/id_reg_test.c b/tools/testing/selftests/kvm/aarch64/id_reg_test.c
new file mode 100644
index 000000000000..c3ac955e6b2d
--- /dev/null
+++ b/tools/testing/selftests/kvm/aarch64/id_reg_test.c
@@ -0,0 +1,1296 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#define _GNU_SOURCE
+
+#include <stdlib.h>
+#include <time.h>
+#include <pthread.h>
+#include <linux/kvm.h>
+#include <linux/sizes.h>
+
+#include "kvm_util.h"
+#include "processor.h"
+
+/*
+ * id_reg_test.c - Tests reading/writing the aarch64's ID registers
+ *
+ * The test validates KVM_SET_ONE_REG/KVM_GET_ONE_REG ioctl for ID
+ * registers as well as reading ID register from the guest works fine.
+ */
+
+/* Reserved ID registers */
+#define	SYS_ID_REG_3_3_EL1		sys_reg(3, 0, 0, 3, 3)
+#define	SYS_ID_REG_3_7_EL1		sys_reg(3, 0, 0, 3, 7)
+
+#define	SYS_ID_REG_4_2_EL1		sys_reg(3, 0, 0, 4, 2)
+#define	SYS_ID_REG_4_3_EL1		sys_reg(3, 0, 0, 4, 3)
+#define	SYS_ID_REG_4_5_EL1		sys_reg(3, 0, 0, 4, 5)
+#define	SYS_ID_REG_4_6_EL1		sys_reg(3, 0, 0, 4, 6)
+#define	SYS_ID_REG_4_7_EL1		sys_reg(3, 0, 0, 4, 7)
+
+#define	SYS_ID_REG_5_2_EL1		sys_reg(3, 0, 0, 5, 2)
+#define	SYS_ID_REG_5_3_EL1		sys_reg(3, 0, 0, 5, 3)
+#define	SYS_ID_REG_5_6_EL1		sys_reg(3, 0, 0, 5, 6)
+#define	SYS_ID_REG_5_7_EL1		sys_reg(3, 0, 0, 5, 7)
+
+#define	SYS_ID_REG_6_2_EL1		sys_reg(3, 0, 0, 6, 2)
+#define	SYS_ID_REG_6_3_EL1		sys_reg(3, 0, 0, 6, 3)
+#define	SYS_ID_REG_6_4_EL1		sys_reg(3, 0, 0, 6, 4)
+#define	SYS_ID_REG_6_5_EL1		sys_reg(3, 0, 0, 6, 5)
+#define	SYS_ID_REG_6_6_EL1		sys_reg(3, 0, 0, 6, 6)
+#define	SYS_ID_REG_6_7_EL1		sys_reg(3, 0, 0, 6, 7)
+
+#define	SYS_ID_REG_7_3_EL1		sys_reg(3, 0, 0, 7, 3)
+#define	SYS_ID_REG_7_4_EL1		sys_reg(3, 0, 0, 7, 4)
+#define	SYS_ID_REG_7_5_EL1		sys_reg(3, 0, 0, 7, 5)
+#define	SYS_ID_REG_7_6_EL1		sys_reg(3, 0, 0, 7, 6)
+#define	SYS_ID_REG_7_7_EL1		sys_reg(3, 0, 0, 7, 7)
+
+#define	READ_ID_REG_FN(name)	read_## name ## _EL1
+
+#define	DEFINE_READ_SYS_REG(reg_name)			\
+uint64_t read_##reg_name(void)				\
+{							\
+	return read_sysreg_s(SYS_##reg_name);		\
+}
+
+#define DEFINE_READ_ID_REG(name)	\
+	DEFINE_READ_SYS_REG(name ## _EL1)
+
+#define	__ID_REG(reg_name)		\
+	.name = #reg_name,		\
+	.id = SYS_## reg_name ##_EL1,	\
+	.read_reg = READ_ID_REG_FN(reg_name),
+
+#define	ID_REG_ENT(reg_name)	\
+	[ID_IDX(reg_name)] = { __ID_REG(reg_name) }
+
+/* Functions to read each ID register */
+/* CRm=1 */
+DEFINE_READ_ID_REG(ID_PFR0)
+DEFINE_READ_ID_REG(ID_PFR1)
+DEFINE_READ_ID_REG(ID_DFR0)
+DEFINE_READ_ID_REG(ID_AFR0)
+DEFINE_READ_ID_REG(ID_MMFR0)
+DEFINE_READ_ID_REG(ID_MMFR1)
+DEFINE_READ_ID_REG(ID_MMFR2)
+DEFINE_READ_ID_REG(ID_MMFR3)
+
+/* CRm=2 */
+DEFINE_READ_ID_REG(ID_ISAR0)
+DEFINE_READ_ID_REG(ID_ISAR1)
+DEFINE_READ_ID_REG(ID_ISAR2)
+DEFINE_READ_ID_REG(ID_ISAR3)
+DEFINE_READ_ID_REG(ID_ISAR4)
+DEFINE_READ_ID_REG(ID_ISAR5)
+DEFINE_READ_ID_REG(ID_MMFR4)
+DEFINE_READ_ID_REG(ID_ISAR6)
+
+/* CRm=3 */
+DEFINE_READ_ID_REG(MVFR0)
+DEFINE_READ_ID_REG(MVFR1)
+DEFINE_READ_ID_REG(MVFR2)
+DEFINE_READ_ID_REG(ID_REG_3_3)
+DEFINE_READ_ID_REG(ID_PFR2)
+DEFINE_READ_ID_REG(ID_DFR1)
+DEFINE_READ_ID_REG(ID_MMFR5)
+DEFINE_READ_ID_REG(ID_REG_3_7)
+
+/* CRm=4 */
+DEFINE_READ_ID_REG(ID_AA64PFR0)
+DEFINE_READ_ID_REG(ID_AA64PFR1)
+DEFINE_READ_ID_REG(ID_REG_4_2)
+DEFINE_READ_ID_REG(ID_REG_4_3)
+DEFINE_READ_ID_REG(ID_AA64ZFR0)
+DEFINE_READ_ID_REG(ID_REG_4_5)
+DEFINE_READ_ID_REG(ID_REG_4_6)
+DEFINE_READ_ID_REG(ID_REG_4_7)
+
+/* CRm=5 */
+DEFINE_READ_ID_REG(ID_AA64DFR0)
+DEFINE_READ_ID_REG(ID_AA64DFR1)
+DEFINE_READ_ID_REG(ID_REG_5_2)
+DEFINE_READ_ID_REG(ID_REG_5_3)
+DEFINE_READ_ID_REG(ID_AA64AFR0)
+DEFINE_READ_ID_REG(ID_AA64AFR1)
+DEFINE_READ_ID_REG(ID_REG_5_6)
+DEFINE_READ_ID_REG(ID_REG_5_7)
+
+/* CRm=6 */
+DEFINE_READ_ID_REG(ID_AA64ISAR0)
+DEFINE_READ_ID_REG(ID_AA64ISAR1)
+DEFINE_READ_ID_REG(ID_REG_6_2)
+DEFINE_READ_ID_REG(ID_REG_6_3)
+DEFINE_READ_ID_REG(ID_REG_6_4)
+DEFINE_READ_ID_REG(ID_REG_6_5)
+DEFINE_READ_ID_REG(ID_REG_6_6)
+DEFINE_READ_ID_REG(ID_REG_6_7)
+
+/* CRm=7 */
+DEFINE_READ_ID_REG(ID_AA64MMFR0)
+DEFINE_READ_ID_REG(ID_AA64MMFR1)
+DEFINE_READ_ID_REG(ID_AA64MMFR2)
+DEFINE_READ_ID_REG(ID_REG_7_3)
+DEFINE_READ_ID_REG(ID_REG_7_4)
+DEFINE_READ_ID_REG(ID_REG_7_5)
+DEFINE_READ_ID_REG(ID_REG_7_6)
+DEFINE_READ_ID_REG(ID_REG_7_7)
+
+#define	ID_IDX(name)	REG_IDX_## name
+
+enum id_reg_idx {
+	/* CRm=1 */
+	ID_IDX(ID_PFR0) = 0,
+	ID_IDX(ID_PFR1),
+	ID_IDX(ID_DFR0),
+	ID_IDX(ID_AFR0),
+	ID_IDX(ID_MMFR0),
+	ID_IDX(ID_MMFR1),
+	ID_IDX(ID_MMFR2),
+	ID_IDX(ID_MMFR3),
+
+	/* CRm=2 */
+	ID_IDX(ID_ISAR0),
+	ID_IDX(ID_ISAR1),
+	ID_IDX(ID_ISAR2),
+	ID_IDX(ID_ISAR3),
+	ID_IDX(ID_ISAR4),
+	ID_IDX(ID_ISAR5),
+	ID_IDX(ID_MMFR4),
+	ID_IDX(ID_ISAR6),
+
+	/* CRm=3 */
+	ID_IDX(MVFR0),
+	ID_IDX(MVFR1),
+	ID_IDX(MVFR2),
+	ID_IDX(ID_REG_3_3),
+	ID_IDX(ID_PFR2),
+	ID_IDX(ID_DFR1),
+	ID_IDX(ID_MMFR5),
+	ID_IDX(ID_REG_3_7),
+
+	/* CRm=4 */
+	ID_IDX(ID_AA64PFR0),
+	ID_IDX(ID_AA64PFR1),
+	ID_IDX(ID_REG_4_2),
+	ID_IDX(ID_REG_4_3),
+	ID_IDX(ID_AA64ZFR0),
+	ID_IDX(ID_REG_4_5),
+	ID_IDX(ID_REG_4_6),
+	ID_IDX(ID_REG_4_7),
+
+	/* CRm=5 */
+	ID_IDX(ID_AA64DFR0),
+	ID_IDX(ID_AA64DFR1),
+	ID_IDX(ID_REG_5_2),
+	ID_IDX(ID_REG_5_3),
+	ID_IDX(ID_AA64AFR0),
+	ID_IDX(ID_AA64AFR1),
+	ID_IDX(ID_REG_5_6),
+	ID_IDX(ID_REG_5_7),
+
+	/* CRm=6 */
+	ID_IDX(ID_AA64ISAR0),
+	ID_IDX(ID_AA64ISAR1),
+	ID_IDX(ID_REG_6_2),
+	ID_IDX(ID_REG_6_3),
+	ID_IDX(ID_REG_6_4),
+	ID_IDX(ID_REG_6_5),
+	ID_IDX(ID_REG_6_6),
+	ID_IDX(ID_REG_6_7),
+
+	/* CRm=7 */
+	ID_IDX(ID_AA64MMFR0),
+	ID_IDX(ID_AA64MMFR1),
+	ID_IDX(ID_AA64MMFR2),
+	ID_IDX(ID_REG_7_3),
+	ID_IDX(ID_REG_7_4),
+	ID_IDX(ID_REG_7_5),
+	ID_IDX(ID_REG_7_6),
+	ID_IDX(ID_REG_7_7),
+};
+
+struct id_reg_test_info {
+	char		*name;
+	uint32_t	id;
+	bool		can_clear;
+	uint64_t	fixed_mask;
+	uint64_t	org_val;
+	uint64_t	user_val;
+	uint64_t	(*read_reg)(void);
+};
+
+#define	ID_REG_INFO(name)	(&id_reg_list[ID_IDX(name)])
+static struct id_reg_test_info id_reg_list[] = {
+	/* CRm=1 */
+	ID_REG_ENT(ID_PFR0),
+	ID_REG_ENT(ID_PFR1),
+	ID_REG_ENT(ID_DFR0),
+	ID_REG_ENT(ID_AFR0),
+	ID_REG_ENT(ID_MMFR0),
+	ID_REG_ENT(ID_MMFR1),
+	ID_REG_ENT(ID_MMFR2),
+	ID_REG_ENT(ID_MMFR3),
+
+	/* CRm=2 */
+	ID_REG_ENT(ID_ISAR0),
+	ID_REG_ENT(ID_ISAR1),
+	ID_REG_ENT(ID_ISAR2),
+	ID_REG_ENT(ID_ISAR3),
+	ID_REG_ENT(ID_ISAR4),
+	ID_REG_ENT(ID_ISAR5),
+	ID_REG_ENT(ID_MMFR4),
+	ID_REG_ENT(ID_ISAR6),
+
+	/* CRm=3 */
+	ID_REG_ENT(MVFR0),
+	ID_REG_ENT(MVFR1),
+	ID_REG_ENT(MVFR2),
+	ID_REG_ENT(ID_REG_3_3),
+	ID_REG_ENT(ID_PFR2),
+	ID_REG_ENT(ID_DFR1),
+	ID_REG_ENT(ID_MMFR5),
+	ID_REG_ENT(ID_REG_3_7),
+
+	/* CRm=4 */
+	ID_REG_ENT(ID_AA64PFR0),
+	ID_REG_ENT(ID_AA64PFR1),
+	ID_REG_ENT(ID_REG_4_2),
+	ID_REG_ENT(ID_REG_4_3),
+	ID_REG_ENT(ID_AA64ZFR0),
+	ID_REG_ENT(ID_REG_4_5),
+	ID_REG_ENT(ID_REG_4_6),
+	ID_REG_ENT(ID_REG_4_7),
+
+	/* CRm=5 */
+	ID_REG_ENT(ID_AA64DFR0),
+	ID_REG_ENT(ID_AA64DFR1),
+	ID_REG_ENT(ID_REG_5_2),
+	ID_REG_ENT(ID_REG_5_3),
+	ID_REG_ENT(ID_AA64AFR0),
+	ID_REG_ENT(ID_AA64AFR1),
+	ID_REG_ENT(ID_REG_5_6),
+	ID_REG_ENT(ID_REG_5_7),
+
+	/* CRm=6 */
+	ID_REG_ENT(ID_AA64ISAR0),
+	ID_REG_ENT(ID_AA64ISAR1),
+	ID_REG_ENT(ID_REG_6_2),
+	ID_REG_ENT(ID_REG_6_3),
+	ID_REG_ENT(ID_REG_6_4),
+	ID_REG_ENT(ID_REG_6_5),
+	ID_REG_ENT(ID_REG_6_6),
+	ID_REG_ENT(ID_REG_6_7),
+
+	/* CRm=7 */
+	ID_REG_ENT(ID_AA64MMFR0),
+	ID_REG_ENT(ID_AA64MMFR1),
+	ID_REG_ENT(ID_AA64MMFR2),
+	ID_REG_ENT(ID_REG_7_3),
+	ID_REG_ENT(ID_REG_7_4),
+	ID_REG_ENT(ID_REG_7_5),
+	ID_REG_ENT(ID_REG_7_6),
+	ID_REG_ENT(ID_REG_7_7),
+};
+
+/* Utilities to get a feature field from ID register value */
+static inline int
+cpuid_signed_field_width(uint64_t id_val, int field, int width)
+{
+	return (s64)(id_val << (64 - width - field)) >> (64 - width);
+}
+
+static unsigned int
+cpuid_unsigned_field_width(uint64_t id_val, int field, int width)
+{
+	return (uint64_t)(id_val << (64 - width - field)) >> (64 - width);
+}
+
+static inline int __attribute_const__
+cpuid_extract_field_width(uint64_t id_val, int field, int width, bool sign)
+{
+	return (sign) ? cpuid_signed_field_width(id_val, field, width) :
+			cpuid_unsigned_field_width(id_val, field, width);
+}
+
+#define	GET_ID_FIELD(regval, shift, is_signed)	\
+	cpuid_extract_field_width(regval, shift, 4, is_signed)
+
+#define	GET_ID_UFIELD(regval, shift)	\
+	cpuid_unsigned_field_width(regval, shift, 4)
+
+#define	UPDATE_ID_UFIELD(regval, shift, fval)	\
+	(((regval) & ~(0xfULL << (shift))) |	\
+	 (((uint64_t)((fval) & 0xf)) << (shift)))
+
+void test_pmu_init(struct kvm_vm *vm, uint32_t vcpu)
+{
+	struct kvm_device_attr attr = {
+		.group = KVM_ARM_VCPU_PMU_V3_CTRL,
+		.attr = KVM_ARM_VCPU_PMU_V3_INIT,
+	};
+	vcpu_ioctl(vm, vcpu, KVM_SET_DEVICE_ATTR, &attr);
+}
+
+void test_sve_init(struct kvm_vm *vm, uint32_t vcpu)
+{
+	int feature = KVM_ARM_VCPU_SVE;
+
+	vcpu_ioctl(vm, vcpu, KVM_ARM_VCPU_FINALIZE, &feature);
+}
+
+#define	MAX_CAPS	2
+struct feature_test_info {
+	char	*name;	/* Feature Name (Debug information) */
+	struct id_reg_test_info	*sreg;	/* ID register for the feature */
+	int	shift;	/* Field of the ID register for the feature */
+	int	min;	/* Min value to indicate the feature */
+	bool	is_sign;	/* Is the field signed or unsigned ? */
+	int	ncaps;		/* Number of valid Capabilities in caps[] */
+	long	caps[MAX_CAPS];	/* Capabilities to indicate this feature */
+	/* struct kvm_enable_cap to use the capability if needed */
+	struct kvm_enable_cap	*opt_in_cap;
+	bool	run_test;	/* Does guest run test for this feature ? */
+	/* Initialization function for the feature as needed */
+	void	(*init_feature)(struct kvm_vm *vm, uint32_t vcpuid);
+	/* struct kvm_vcpu_init to opt-in the feature if needed */
+	struct kvm_vcpu_init	*vcpu_init;
+};
+
+/* Test for optin CPU features */
+static struct feature_test_info feature_test_info_table[] = {
+	{
+		.name = "SVE",
+		.sreg = ID_REG_INFO(ID_AA64PFR0),
+		.shift = ID_AA64PFR0_SVE_SHIFT,
+		.min = 1,
+		.caps = {KVM_CAP_ARM_SVE},
+		.ncaps = 1,
+		.init_feature = test_sve_init,
+		.vcpu_init = &(struct kvm_vcpu_init) {
+			.features = {1ULL << KVM_ARM_VCPU_SVE},
+		},
+	},
+	{
+		.name = "MTE",
+		.sreg = ID_REG_INFO(ID_AA64PFR1),
+		.shift = ID_AA64PFR1_MTE_SHIFT,
+		.min = 2,
+		.caps = {KVM_CAP_ARM_MTE},
+		.ncaps = 1,
+		.opt_in_cap = &(struct kvm_enable_cap) {
+				.cap = KVM_CAP_ARM_MTE,
+		},
+	},
+	{
+		.name = "PMUV3",
+		.sreg = ID_REG_INFO(ID_AA64DFR0),
+		.shift = ID_AA64DFR0_PMUVER_SHIFT,
+		.min = 1,
+		.init_feature = test_pmu_init,
+		.caps = {KVM_CAP_ARM_PMU_V3},
+		.ncaps = 1,
+		.vcpu_init = &(struct kvm_vcpu_init) {
+			.features = {1ULL << KVM_ARM_VCPU_PMU_V3},
+		},
+	},
+	{
+		.name = "PERFMON",
+		.sreg = ID_REG_INFO(ID_DFR0),
+		.shift = ID_DFR0_PERFMON_SHIFT,
+		.min = 3,
+		.init_feature = test_pmu_init,
+		.caps = {KVM_CAP_ARM_PMU_V3},
+		.ncaps = 1,
+		.vcpu_init = &(struct kvm_vcpu_init) {
+			.features = {1ULL << KVM_ARM_VCPU_PMU_V3},
+		},
+	},
+};
+
+static int walk_id_reg_list(int (*fn)(struct id_reg_test_info *sreg, void *arg),
+			    void *arg)
+{
+	int ret = 0, i;
+
+	for (i = 0; i < ARRAY_SIZE(id_reg_list); i++) {
+		ret = fn(&id_reg_list[i], arg);
+		if (ret)
+			break;
+	}
+	return ret;
+}
+
+static int guest_code_id_reg_check_one(struct id_reg_test_info *sreg, void *arg)
+{
+	uint64_t val = sreg->read_reg();
+
+	GUEST_ASSERT_2(val == sreg->user_val, sreg->name, sreg->user_val);
+	return 0;
+}
+
+static void guest_code_id_reg_check_all(uint32_t cpu)
+{
+	walk_id_reg_list(guest_code_id_reg_check_one, NULL);
+	GUEST_DONE();
+}
+
+static void guest_code_do_nothing(uint32_t cpu)
+{
+	GUEST_DONE();
+}
+
+static void guest_code_feature_check(uint32_t cpu)
+{
+	int i;
+	struct feature_test_info *finfo;
+
+	for (i = 0; i < ARRAY_SIZE(feature_test_info_table); i++) {
+		finfo = &feature_test_info_table[i];
+		if (finfo->run_test)
+			guest_code_id_reg_check_one(finfo->sreg, NULL);
+	}
+
+	GUEST_DONE();
+}
+
+static void guest_code_ptrauth_check(uint32_t cpuid)
+{
+	struct id_reg_test_info *sreg = ID_REG_INFO(ID_AA64ISAR1);
+	uint64_t val = sreg->read_reg();
+
+	GUEST_ASSERT_2(val == sreg->user_val, "PTRAUTH", val);
+	GUEST_DONE();
+}
+
+static int reset_id_reg_info_one(struct id_reg_test_info *sreg, void *arg)
+{
+	sreg->user_val = sreg->org_val;
+	return 0;
+}
+
+static void reset_id_reg_info(void)
+{
+	walk_id_reg_list(reset_id_reg_info_one, NULL);
+}
+
+static struct kvm_vm *test_vm_create_cap(uint32_t nvcpus,
+		void (*guest_code)(uint32_t), struct kvm_vcpu_init *init,
+		struct kvm_enable_cap *cap)
+{
+	struct kvm_vm *vm;
+	uint32_t cpuid;
+	uint64_t mem_pages;
+
+	mem_pages = DEFAULT_GUEST_PHY_PAGES + DEFAULT_STACK_PGS * nvcpus;
+	mem_pages += mem_pages / (PTES_PER_MIN_PAGE * 2);
+	mem_pages = vm_adjust_num_guest_pages(VM_MODE_DEFAULT, mem_pages);
+
+	vm = vm_create(VM_MODE_DEFAULT,
+		DEFAULT_GUEST_PHY_PAGES + (DEFAULT_STACK_PGS * nvcpus),
+		O_RDWR);
+	if (cap)
+		vm_enable_cap(vm, cap);
+
+	kvm_vm_elf_load(vm, program_invocation_name);
+
+	if (init && init->target == -1) {
+		struct kvm_vcpu_init preferred;
+
+		vm_ioctl(vm, KVM_ARM_PREFERRED_TARGET, &preferred);
+		init->target = preferred.target;
+	}
+
+	vm_init_descriptor_tables(vm);
+	for (cpuid = 0; cpuid < nvcpus; cpuid++) {
+		if (init)
+			aarch64_vcpu_add_default(vm, cpuid, init, guest_code);
+		else
+			vm_vcpu_add_default(vm, cpuid, guest_code);
+
+		vcpu_init_descriptor_tables(vm, cpuid);
+	}
+
+	ucall_init(vm, NULL);
+	return vm;
+}
+
+static struct kvm_vm *test_vm_create(uint32_t nvcpus,
+				     void (*guest_code)(uint32_t),
+				     struct kvm_vcpu_init *init)
+{
+	return test_vm_create_cap(nvcpus, guest_code, init, NULL);
+}
+
+static void test_vm_free(struct kvm_vm *vm)
+{
+	ucall_uninit(vm);
+	kvm_vm_free(vm);
+}
+
+#define	TEST_RUN(vm, cpu)	\
+	(test_vcpu_run(__func__, __LINE__, vm, cpu, true))
+
+#define	TEST_RUN_NO_SYNC_DATA(vm, cpu)	\
+	(test_vcpu_run(__func__, __LINE__, vm, cpu, false))
+
+static int test_vcpu_run(const char *test_name, int line,
+			 struct kvm_vm *vm, uint32_t vcpuid, bool sync_data)
+{
+	struct ucall uc;
+	int ret;
+
+	if (sync_data) {
+		sync_global_to_guest(vm, id_reg_list);
+		sync_global_to_guest(vm, feature_test_info_table);
+	}
+
+	vcpu_args_set(vm, vcpuid, 1, vcpuid);
+
+	ret = _vcpu_run(vm, vcpuid);
+	if (ret) {
+		ret = errno;
+		goto sync_exit;
+	}
+
+	switch (get_ucall(vm, vcpuid, &uc)) {
+	case UCALL_SYNC:
+	case UCALL_DONE:
+		ret = 0;
+		break;
+	case UCALL_ABORT:
+		TEST_FAIL(
+		    "%s (%s) at line %d (user %s at line %d), args[3]=0x%lx",
+		    (char *)uc.args[0], (char *)uc.args[2], (int)uc.args[1],
+		    test_name, line, uc.args[3]);
+		break;
+	default:
+		TEST_FAIL("Unexpected guest exit\n");
+	}
+
+sync_exit:
+	if (sync_data) {
+		sync_global_from_guest(vm, id_reg_list);
+		sync_global_from_guest(vm, feature_test_info_table);
+	}
+	return ret;
+}
+
+static int set_id_regs_after_run_test_one(struct id_reg_test_info *sreg,
+					  void *arg)
+{
+	struct kvm_vm *vm = arg;
+	struct kvm_one_reg one_reg;
+	uint32_t vcpuid = 0;
+	uint64_t reg_val;
+	int ret;
+
+	one_reg.addr = (uint64_t)&reg_val;
+	one_reg.id = KVM_ARM64_SYS_REG(sreg->id);
+
+	vcpu_ioctl(vm, vcpuid, KVM_GET_ONE_REG, &one_reg);
+	if ((reg_val != 0) && (sreg->can_clear)) {
+		reg_val = 0;
+		ret = _vcpu_ioctl(vm, vcpuid, KVM_SET_ONE_REG, &one_reg);
+		TEST_ASSERT(ret && errno == EINVAL,
+			    "Changing ID reg value should fail\n");
+	}
+
+	vcpu_ioctl(vm, vcpuid, KVM_GET_ONE_REG, &one_reg);
+	ret = _vcpu_ioctl(vm, vcpuid, KVM_SET_ONE_REG, &one_reg);
+	TEST_ASSERT(ret == 0, "Setting the same ID reg value should work\n");
+
+	return 0;
+}
+
+static int set_id_regs_test_one(struct id_reg_test_info *sreg, void *arg)
+{
+	struct kvm_vm *vm = arg;
+	struct kvm_one_reg one_reg;
+	uint32_t vcpuid = 0;
+	uint64_t reg_val;
+
+	one_reg.addr = (uint64_t)&reg_val;
+	reset_id_reg_info();
+
+	one_reg.id = KVM_ARM64_SYS_REG(sreg->id);
+	if (sreg->can_clear) {
+		/* Change the register to 0 when possible */
+		reg_val = 0;
+		vcpu_ioctl(vm, vcpuid, KVM_SET_ONE_REG, &one_reg);
+		vcpu_ioctl(vm, vcpuid, KVM_GET_ONE_REG, &one_reg);
+		TEST_ASSERT(reg_val == 0,
+		    "GET(%s) didn't return 0 but 0x%lx", sreg->name, reg_val);
+	}
+
+	/* Check if we can restore the initial value */
+	reg_val = sreg->org_val;
+	vcpu_ioctl(vm, vcpuid, KVM_SET_ONE_REG, &one_reg);
+	vcpu_ioctl(vm, vcpuid, KVM_GET_ONE_REG, &one_reg);
+	TEST_ASSERT(reg_val == sreg->org_val,
+		    "GET(%s) didn't return 0x%lx but 0x%lx",
+		    sreg->name, sreg->org_val, reg_val);
+	sreg->user_val = sreg->org_val;
+	return 0;
+}
+
+static void set_id_regs_test(void)
+{
+	struct kvm_vm *vm;
+	int ret;
+
+	reset_id_reg_info();
+	vm = test_vm_create(1, guest_code_id_reg_check_all, NULL);
+
+	ret = walk_id_reg_list(set_id_regs_test_one, vm);
+	assert(!ret);
+
+	ret = TEST_RUN(vm, 0);
+	TEST_ASSERT(!ret, "%s TEST_RUN failed, ret=0x%x", __func__, ret);
+
+	ret = walk_id_reg_list(set_id_regs_after_run_test_one, vm);
+	assert(!ret);
+}
+
+static int clear_id_reg_when_possible(struct id_reg_test_info *sreg, void *arg)
+{
+	uint64_t reg_val = 0;
+	struct kvm_one_reg one_reg;
+	struct kvm_vm *vm = (struct kvm_vm *)((uint64_t *)arg)[0];
+	uint32_t vcpu = ((uint64_t *)arg)[1];
+
+	if (sreg->can_clear) {
+		assert(sreg->org_val);
+		one_reg.addr = (uint64_t)&reg_val;
+		one_reg.id = KVM_ARM64_SYS_REG(sreg->id);
+		vcpu_ioctl(vm, vcpu, KVM_SET_ONE_REG, &one_reg);
+		sreg->user_val = 0;
+	}
+	return sreg->can_clear;
+}
+
+static void clear_any_id_reg(struct kvm_vm *vm, uint32_t vcpuid)
+{
+	int ret;
+	uint64_t args[2] = {(uint64_t)vm, vcpuid};
+
+	ret = walk_id_reg_list(clear_id_reg_when_possible, args);
+
+	/* Return non-zero means one of non-zero registers was cleared */
+	assert(ret);
+	sync_global_to_guest(vm, id_reg_list);
+}
+
+static bool caps_are_supported(long *caps, int ncaps)
+{
+	int i;
+
+	for (i = 0; i < ncaps; i++) {
+		if (kvm_check_cap(caps[i]) <= 0)
+			return false;
+	}
+	return true;
+}
+
+static void test_feature_ptrauth(void)
+{
+	struct kvm_one_reg one_reg;
+	struct kvm_vcpu_init init;
+	struct kvm_vm *vm = NULL;
+	struct id_reg_test_info *sreg = ID_REG_INFO(ID_AA64ISAR1);
+	uint32_t vcpu = 0;
+	int64_t rval;
+	int ret;
+	int apa, api, gpa, gpi;
+	char *name = "PTRAUTH";
+	long caps[2] = {KVM_CAP_ARM_PTRAUTH_ADDRESS,
+			KVM_CAP_ARM_PTRAUTH_GENERIC};
+
+	reset_id_reg_info();
+	one_reg.addr = (uint64_t)&rval;
+	one_reg.id = KVM_ARM64_SYS_REG(sreg->id);
+
+	if (caps_are_supported(caps, 2)) {
+
+		/* Test with feature enabled */
+		memset(&init, 0, sizeof(init));
+		init.target = -1;
+		init.features[0] = (1ULL << KVM_ARM_VCPU_PTRAUTH_ADDRESS |
+				    1ULL << KVM_ARM_VCPU_PTRAUTH_GENERIC);
+		vm = test_vm_create_cap(1, guest_code_ptrauth_check, &init,
+					NULL);
+		vcpu_ioctl(vm, vcpu, KVM_GET_ONE_REG, &one_reg);
+		apa = GET_ID_UFIELD(rval, ID_AA64ISAR1_APA_SHIFT);
+		api = GET_ID_UFIELD(rval, ID_AA64ISAR1_API_SHIFT);
+		gpa = GET_ID_UFIELD(rval, ID_AA64ISAR1_GPA_SHIFT);
+		gpi = GET_ID_UFIELD(rval, ID_AA64ISAR1_GPI_SHIFT);
+
+		TEST_ASSERT((apa > 0) || (api > 0),
+			    "Either apa(0x%x) or api(0x%x) must be available",
+			    apa, gpa);
+		TEST_ASSERT((gpa > 0) || (gpi > 0),
+			    "Either gpa(0x%x) or gpi(0x%x) must be available",
+			    gpa, gpi);
+
+		TEST_ASSERT((apa > 0) ^ (api > 0),
+			    "Both apa(0x%x) and api(0x%x) must not be available",
+			    apa, api);
+		TEST_ASSERT((gpa > 0) ^ (gpi > 0),
+			    "Both gpa(0x%x) and gpi(0x%x) must not be available",
+			    gpa, gpi);
+
+		sreg->user_val = rval;
+
+		pr_debug("%s: Test with %s enabled (%s: 0x%lx)\n",
+			 __func__, name, sreg->name, sreg->user_val);
+		ret = TEST_RUN(vm, vcpu);
+		TEST_ASSERT(!ret, "%s:KVM_RUN failed with %s enabled",
+			    __func__, name);
+		test_vm_free(vm);
+	}
+
+	/* Test with feature disabled */
+	reset_id_reg_info();
+
+	vm = test_vm_create(1, guest_code_feature_check, NULL);
+	vcpu_ioctl(vm, vcpu, KVM_GET_ONE_REG, &one_reg);
+
+	apa = GET_ID_UFIELD(rval, ID_AA64ISAR1_APA_SHIFT);
+	api = GET_ID_UFIELD(rval, ID_AA64ISAR1_API_SHIFT);
+	gpa = GET_ID_UFIELD(rval, ID_AA64ISAR1_GPA_SHIFT);
+	gpi = GET_ID_UFIELD(rval, ID_AA64ISAR1_GPI_SHIFT);
+	TEST_ASSERT(!apa && !api && !gpa && !gpi,
+	    "apa(0x%x), api(0x%x), gpa(0x%x), gpi(0x%x) must be zero",
+	    apa, api, gpa, gpi);
+
+	pr_debug("%s: Test with %s disabled (%s: 0x%lx)\n",
+		 __func__, name, sreg->name, sreg->user_val);
+
+	ret = TEST_RUN(vm, vcpu);
+	TEST_ASSERT(!ret, "%s TEST_RUN failed with %s enabled, ret=0x%x",
+		    __func__, name, ret);
+
+	test_vm_free(vm);
+}
+
+static void test_feature(struct feature_test_info *finfo)
+{
+	struct id_reg_test_info *sreg = finfo->sreg;
+	struct kvm_one_reg one_reg;
+	struct kvm_vcpu_init init, *initp = NULL;
+	struct kvm_vm *vm = NULL;
+	int64_t fval, reg_val;
+	uint32_t vcpu = 0;
+	bool is_sign = finfo->is_sign;
+	int min = finfo->min;
+	int shift = finfo->shift;
+	int ret;
+
+	pr_debug("%s: %s (reg %s)\n", __func__, finfo->name, sreg->name);
+
+	reset_id_reg_info();
+	finfo->run_test = 1;	/* Indicate that guest runs the test on it */
+	one_reg.addr = (uint64_t)&reg_val;
+	one_reg.id = KVM_ARM64_SYS_REG(sreg->id);
+
+	/* Test with feature enabled */
+
+	/* Need KVM_ARM_VCPU_INIT or opt-in capability ? */
+	if (finfo->vcpu_init || finfo->opt_in_cap) {
+		if ((finfo->ncaps == 0) ||
+		    (caps_are_supported(finfo->caps, finfo->ncaps))) {
+			if (finfo->vcpu_init) {
+				/*
+				 * Need to enable the feature via
+				 * KVM_ARM_VCPU_INIT.
+				 */
+				memset(&init, 0, sizeof(init));
+				init = *finfo->vcpu_init;
+				init.target = -1;
+				initp = &init;
+			}
+
+			vm = test_vm_create_cap(1, guest_code_feature_check,
+						initp, finfo->opt_in_cap);
+			vcpu_ioctl(vm, vcpu, KVM_GET_ONE_REG, &one_reg);
+			fval = GET_ID_FIELD(reg_val, shift, is_sign);
+			TEST_ASSERT(fval >= min,
+				    "%s field of %s is too small (%ld)",
+				    finfo->name, sreg->name, fval);
+			sreg->user_val = reg_val;
+		}
+	} else {
+		/* Check if the feature is available */
+		if (GET_ID_FIELD(sreg->org_val, shift, is_sign) >= min)
+			vm = test_vm_create(1, guest_code_feature_check, NULL);
+	}
+
+	if (vm) {
+		if (finfo->init_feature)
+			/* Run any required extra process to use the feature */
+			finfo->init_feature(vm, vcpu);
+
+		pr_debug("%s: Test with %s enabled (%s: 0x%lx)\n",
+			 __func__, finfo->name, sreg->name, sreg->user_val);
+
+		ret = TEST_RUN(vm, vcpu);
+		TEST_ASSERT(!ret, "%s:TEST_RUN failed with %s enabled",
+			    __func__, finfo->name);
+		test_vm_free(vm);
+	}
+
+	/* Test with feature disabled */
+	reset_id_reg_info();
+
+	vm = test_vm_create(1, guest_code_feature_check, NULL);
+	vcpu_ioctl(vm, vcpu, KVM_GET_ONE_REG, &one_reg);
+	fval = GET_ID_FIELD(reg_val, shift, is_sign);
+	if (finfo->vcpu_init || finfo->opt_in_cap) {
+		/*
+		 * If the feature needs to be enabled with KVM_ARM_VCPU_INIT
+		 * or opt-in capabilities, the default value of the ID register
+		 * shouldn't indicate the feature.
+		 */
+		TEST_ASSERT(fval < min, "%s field of %s is too big (%ld)",
+		    finfo->name, sreg->name, fval);
+	} else {
+		/* Update the relevant field to hide the feature. */
+		fval = is_sign ? 0xf : 0x0;
+		reg_val = UPDATE_ID_UFIELD(reg_val, shift, fval);
+		ret = _vcpu_ioctl(vm, vcpu, KVM_SET_ONE_REG, &one_reg);
+		TEST_ASSERT(ret == 0, "Disabling %s failed %d\n",
+			    finfo->name, ret);
+		sreg->user_val = reg_val;
+	}
+
+	pr_debug("%s: Test with %s disabled (%s: 0x%lx)\n",
+		 __func__, finfo->name, sreg->name, sreg->user_val);
+
+	ret = TEST_RUN(vm, vcpu);
+	finfo->run_test = 0;
+	test_vm_free(vm);
+}
+
+static void test_feature_all(void)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(feature_test_info_table); i++)
+		test_feature(&feature_test_info_table[i]);
+}
+
+int test_set_reg(struct id_reg_test_info *sreg, uint64_t new_val,
+		 bool guest_run)
+{
+	struct kvm_vm *vm;
+	int ret;
+	uint32_t vcpu = 0;
+	uint64_t reg_val;
+	struct kvm_one_reg one_reg;
+
+	reset_id_reg_info();
+
+	vm = test_vm_create(1, guest_code_id_reg_check_all, NULL);
+	one_reg.id = KVM_ARM64_SYS_REG(sreg->id);
+	one_reg.addr = (uint64_t)&reg_val;
+
+	reg_val = new_val;
+	ret = _vcpu_ioctl(vm, vcpu, KVM_SET_ONE_REG, &one_reg);
+	if (!guest_run)
+		return ret;
+
+	TEST_ASSERT(!ret, "SET_REG(%s=0x%lx) failed, ret=0x%x",
+		    sreg->name, new_val, ret);
+	sreg->user_val = new_val;
+	ret = TEST_RUN(vm, vcpu);
+	test_vm_free(vm);
+	return ret;
+}
+
+int test_feature_frac_vm(struct id_reg_test_info *sreg, uint64_t new_val,
+		      struct id_reg_test_info *frac_sreg, uint64_t frac_new_val)
+{
+	struct kvm_vm *vm;
+	int ret;
+	uint32_t vcpu = 0;
+	uint64_t reg_val;
+	struct kvm_one_reg one_reg;
+
+	reset_id_reg_info();
+
+	vm = test_vm_create(1, guest_code_id_reg_check_all, NULL);
+
+	/* Set feature reg field */
+	one_reg.id = KVM_ARM64_SYS_REG(sreg->id);
+	one_reg.addr = (uint64_t)&reg_val;
+	reg_val = new_val;
+	ret = _vcpu_ioctl(vm, vcpu, KVM_SET_ONE_REG, &one_reg);
+	TEST_ASSERT(!ret, "SET_REG(%s=0x%lx) failed, ret=0x%x",
+		    sreg->name, new_val, ret);
+	sreg->user_val = new_val;
+
+	/* Set fractional reg field */
+	one_reg.id = KVM_ARM64_SYS_REG(frac_sreg->id);
+	one_reg.addr = (uint64_t)&reg_val;
+	reg_val = frac_new_val;
+	vcpu_ioctl(vm, vcpu, KVM_SET_ONE_REG, &one_reg);
+	TEST_ASSERT(!ret, "SET_REG(%s=0x%lx) failed, ret=0x%x",
+		    frac_sreg->name, frac_new_val, ret);
+
+	frac_sreg->user_val = frac_new_val;
+	ret = TEST_RUN(vm, vcpu);
+	test_vm_free(vm);
+	return ret;
+}
+
+struct frac_info {
+	char	*name;
+	struct id_reg_test_info *sreg;
+	struct id_reg_test_info *frac_sreg;
+	int	shift;
+	int	frac_shift;
+};
+
+struct frac_info frac_info_table[] = {
+	{
+		.name = "RAS",
+		.sreg = ID_REG_INFO(ID_AA64PFR0),
+		.shift = ID_AA64PFR0_RAS_SHIFT,
+		.frac_sreg = ID_REG_INFO(ID_AA64PFR1),
+		.frac_shift = ID_AA64PFR1_RASFRAC_SHIFT,
+	},
+	{
+		.name = "MPAM",
+		.sreg = ID_REG_INFO(ID_AA64PFR0),
+		.shift = ID_AA64PFR0_MPAM_SHIFT,
+		.frac_sreg = ID_REG_INFO(ID_AA64PFR1),
+		.frac_shift = ID_AA64PFR1_MPAMFRAC_SHIFT,
+	},
+	{
+		.name = "CSV2",
+		.sreg = ID_REG_INFO(ID_AA64PFR0),
+		.shift = ID_AA64PFR0_CSV2_SHIFT,
+		.frac_sreg = ID_REG_INFO(ID_AA64PFR1),
+		.frac_shift = ID_AA64PFR1_CSV2FRAC_SHIFT,
+	},
+};
+
+void test_feature_frac_one(struct frac_info *frac)
+{
+	uint64_t reg_val, org_fval, frac_reg_val, frac_org_fval;
+	int ret, shift, frac_shift;
+	struct id_reg_test_info *sreg, *frac_sreg;
+
+	reset_id_reg_info();
+
+	sreg = frac->sreg;
+	shift = frac->shift;
+	frac_sreg = frac->frac_sreg;
+	frac_shift = frac->frac_shift;
+
+	pr_debug("%s(%s Frac) reg:%s(shift:%d) frac reg:%s(shift:%d)\n",
+		__func__, frac->name, sreg->name, shift,
+		frac_sreg->name, frac_shift);
+
+	frac_org_fval = GET_ID_UFIELD(frac_sreg->org_val, frac_shift);
+	if (frac_org_fval > 0) {
+		/* Test with smaller frac value */
+		frac_reg_val = UPDATE_ID_UFIELD(frac_sreg->org_val,
+						frac_shift, frac_org_fval - 1);
+		ret = test_set_reg(frac_sreg, frac_reg_val, false);
+		TEST_ASSERT(!ret, "SET smaller %s frac (val:%lx) failed(%d)",
+			    frac->name, frac_reg_val, ret);
+
+		ret = test_feature_frac_vm(sreg, sreg->org_val,
+					   frac_sreg, frac_reg_val);
+		TEST_ASSERT(!ret, "Test smaller %s frac (val:%lx) failed(%d)",
+			    frac->name, frac_reg_val, ret);
+	}
+
+	reset_id_reg_info();
+
+	if (frac_org_fval != 0xf) {
+		/* Test with larger frac value */
+		frac_reg_val = UPDATE_ID_UFIELD(frac_sreg->org_val, frac_shift,
+						frac_org_fval + 1);
+
+		/* Setting larger frac shouldn't fail (at ioctl) */
+		ret = test_set_reg(frac_sreg, frac_reg_val, false);
+		TEST_ASSERT(!ret,
+			"SET larger %s frac (%s org:%lx, val:%lx) failed(%d)",
+			frac->name, frac_sreg->name, frac_sreg->org_val,
+			frac_reg_val, ret);
+
+		/* KVM_RUN with larger frac should fail */
+		ret = test_feature_frac_vm(sreg, sreg->org_val,
+					   frac_sreg, frac_reg_val);
+		TEST_ASSERT(ret,
+			"Test with larger %s frac (%s org:%lx, val:%lx) worked",
+			frac->name, frac_sreg->name, frac_sreg->org_val,
+			frac_reg_val);
+	}
+
+	reset_id_reg_info();
+
+	org_fval = GET_ID_UFIELD(sreg->org_val, shift);
+	if (org_fval == 0) {
+		/* Setting larger val for the feature should fail */
+		reg_val = UPDATE_ID_UFIELD(sreg->org_val, shift, org_fval + 1);
+		ret = test_set_reg(sreg, reg_val, false);
+		TEST_ASSERT(ret, "SET larger %s (val:%lx) worked",
+			    frac->name, reg_val);
+		return;
+	}
+
+	/* Test with smaller feature value */
+	reg_val = UPDATE_ID_UFIELD(sreg->org_val, shift, org_fval - 1);
+	ret = test_set_reg(sreg, reg_val, false);
+	TEST_ASSERT(!ret, "SET smaller %s (val:%lx) failed(%d)",
+		    frac->name, reg_val, ret);
+
+	ret = test_feature_frac_vm(sreg, reg_val, frac_sreg, frac_sreg->org_val);
+	TEST_ASSERT(!ret, "Test with smaller %s (val:%lx) failed(%d)",
+		    frac->name, reg_val, ret);
+
+	if (frac_org_fval > 0) {
+		/* Test with smaller feature and frac value */
+		frac_reg_val = UPDATE_ID_UFIELD(frac_sreg->org_val,
+						frac_shift, frac_org_fval - 1);
+		ret = test_feature_frac_vm(sreg, reg_val, frac_sreg,
+					   frac_reg_val);
+		TEST_ASSERT(!ret,
+		    "Test with smaller %s and frac (val:%lx) failed(%d)",
+		    frac->name, reg_val, ret);
+	}
+
+	if (frac_org_fval != 0xf) {
+		/* Test with smaller feature and larger frac value */
+		frac_reg_val = UPDATE_ID_UFIELD(frac_sreg->org_val,
+						frac_shift, frac_org_fval + 1);
+		ret = test_feature_frac_vm(sreg, reg_val, frac_sreg,
+					   frac_reg_val);
+		TEST_ASSERT(!ret,
+		    "Test with smaller %s and larger frac (val:%lx) failed(%d)",
+		    frac->name, reg_val, ret);
+	}
+}
+
+void test_feature_frac_all(void)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(frac_info_table); i++)
+		test_feature_frac_one(&frac_info_table[i]);
+}
+
+/* Structure for test_consistent_vcpus/test_inconsistent_vcpus */
+struct thread_args {
+	pthread_t	thread;
+	struct kvm_vm	*vm;
+	uint32_t	vcpuid;
+	void		*data;
+};
+
+/* Structures for test_inconsistent_vcpus */
+struct run_inconsistent_test {
+	bool		modify_id_reg;
+	volatile bool	complete;
+};
+
+struct inconsistent_test_info {
+	uint32_t nvcpus;
+	struct run_inconsistent_test *vcpu_run;
+};
+
+static void *inconsistent_test_thread(void *arg)
+{
+	struct thread_args *targs = arg;
+	struct kvm_vm *vm = targs->vm;
+	uint32_t vcpuid = targs->vcpuid;
+	struct inconsistent_test_info *info = targs->data;
+	int64_t ret;
+	int i;
+	struct timespec ts = {
+		.tv_sec = 0,
+		.tv_nsec = 1000000,
+	};
+
+	if (info->vcpu_run[vcpuid].modify_id_reg) {
+		/* Make inconsistency in ID regs between vCPUs */
+		clear_any_id_reg(vm, vcpuid);
+
+		/* Wait for all other vCPUs to exit from KVM_RUN */
+		for (i = 0; i < info->nvcpus; i++) {
+			if (i == vcpuid)
+				continue;
+
+			while (!info->vcpu_run[i].complete)
+				nanosleep(&ts, NULL);
+		}
+	}
+
+	ret = TEST_RUN_NO_SYNC_DATA(vm, vcpuid);
+	if (info->vcpu_run[vcpuid].modify_id_reg) {
+		TEST_ASSERT(ret == EPERM,
+			    "%s: KVM_RUN retturned unexpected result (ret=%ld)",
+			    __func__, ret);
+	} else {
+		TEST_ASSERT(ret == 0, "%s: KVM_RUN failed (ret=%ld)",
+			    __func__, ret);
+		info->vcpu_run[vcpuid].complete = true;
+	}
+
+	return (void *)ret;
+}
+
+static int start_test_thread(struct kvm_vm *vm, uint32_t nvcpus,
+			     void *(*test_func)(void *arg), void *data)
+{
+	int i, ret, test_ret;
+	uint64_t thread_ret;
+	pthread_t *threads;
+	struct thread_args *args;
+
+	threads = calloc(nvcpus, sizeof(pthread_t));
+	TEST_ASSERT(threads, "Failed to allocate threads.");
+
+	args = calloc(nvcpus, sizeof(struct thread_args));
+	TEST_ASSERT(args, "Failed to allocate args.");
+
+	for (i = 0; i < nvcpus; i++) {
+		args[i].vm = vm;
+		args[i].vcpuid = i;
+		args[i].data = data;
+		ret = pthread_create(&threads[i], NULL, test_func, &args[i]);
+		TEST_ASSERT(!ret, "pthread_create failed: %d\n", ret);
+	}
+
+	test_ret = 0;
+	for (i = 0; i < nvcpus; i++) {
+		thread_ret = 0;
+		ret = pthread_join(threads[i], (void **)&thread_ret);
+		TEST_ASSERT(!ret, "pthread_join failed: %d\n", ret);
+		if (thread_ret != 0)
+			test_ret = (int32_t)thread_ret;
+	}
+	free(args);
+	free(threads);
+	return test_ret;
+}
+
+/*
+ * Create multiple vCPUs and we will set ID registers to different values
+ * from others to make sure that KVM_RUN will fail when it detects inconsistent
+ * ID registers across vCPUs.
+ */
+void test_inconsistent_vcpus(uint32_t nvcpus)
+{
+	struct kvm_vm *vm;
+	int ret;
+	struct inconsistent_test_info info;
+
+	assert(nvcpus > 1);
+
+	reset_id_reg_info();
+	info.nvcpus = nvcpus;
+	info.vcpu_run = calloc(nvcpus, sizeof(struct run_inconsistent_test));
+	assert(info.vcpu_run);
+
+	vm = test_vm_create(nvcpus, guest_code_do_nothing, NULL);
+
+	sync_global_to_guest(vm, id_reg_list);
+
+	/* Let vCPU0 modify its ID register */
+	info.vcpu_run[0].modify_id_reg = 1;
+	ret = start_test_thread(vm, nvcpus, inconsistent_test_thread, &info);
+	TEST_ASSERT(ret == EPERM, "inconsistent_test_thread failed\n");
+	test_vm_free(vm);
+	free(info.vcpu_run);
+}
+
+static void *test_vcpu_thread(void *arg)
+{
+	struct thread_args *targs = arg;
+	int64_t ret;
+
+	ret = TEST_RUN_NO_SYNC_DATA(targs->vm, targs->vcpuid);
+	return (void *)ret;
+}
+
+void test_consistent_vcpus(uint32_t nvcpus)
+{
+	struct kvm_vm *vm;
+	int ret;
+
+	assert(nvcpus > 1);
+	reset_id_reg_info();
+
+	vm = test_vm_create(nvcpus, guest_code_do_nothing, NULL);
+	sync_global_to_guest(vm, id_reg_list);
+
+	ret = start_test_thread(vm, nvcpus, test_vcpu_thread, NULL);
+	TEST_ASSERT(!ret, "test_vcpu_thread failed\n");
+	test_vm_free(vm);
+}
+
+void run_test(void)
+{
+	uint32_t nvcpus = 3;
+
+	set_id_regs_test();
+	test_feature_all();
+	test_feature_ptrauth();
+	test_feature_frac_all();
+	test_consistent_vcpus(nvcpus);
+	test_inconsistent_vcpus(nvcpus);
+}
+
+static int init_id_reg_info_one(struct id_reg_test_info *sreg, void *arg)
+{
+	uint64_t reg_val;
+	uint32_t vcpuid = 0;
+	int ret;
+	struct kvm_one_reg one_reg;
+	struct kvm_vm *vm = arg;
+
+	one_reg.addr = (uint64_t)&reg_val;
+	one_reg.id = KVM_ARM64_SYS_REG(sreg->id);
+	vcpu_ioctl(vm, vcpuid, KVM_GET_ONE_REG, &one_reg);
+	sreg->org_val = reg_val;
+	sreg->user_val = reg_val;
+	if (sreg->org_val) {
+		reg_val = 0;
+		ret = _vcpu_ioctl(vm, vcpuid, KVM_SET_ONE_REG, &one_reg);
+		if (!ret)
+			sreg->can_clear = true;
+	}
+
+	pr_debug("%s (0x%x): 0x%lx, %s\n", sreg->name, sreg->id,
+		 sreg->org_val, sreg->can_clear ? "can clear" : "");
+
+	return 0;
+}
+
+static void init_id_reg_info(void)
+{
+	struct kvm_vm *vm;
+
+	vm = test_vm_create(1, guest_code_do_nothing, NULL);
+	walk_id_reg_list(init_id_reg_info_one, vm);
+	test_vm_free(vm);
+}
+
+int main(void)
+{
+
+	setbuf(stdout, NULL);
+
+	if (kvm_check_cap(KVM_CAP_ARM_ID_REG_WRITABLE) <= 0) {
+		print_skip("KVM_CAP_ARM_ID_REG_WRITABLE is not supported\n");
+		exit(KSFT_SKIP);
+	}
+
+	init_id_reg_info();
+	run_test();
+	return 0;
+}
-- 
2.33.0.882.g93a45727a2-goog


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

* Re: [RFC PATCH 01/25] KVM: arm64: Add has_reset_once flag for vcpu
  2021-10-12  4:35 ` [RFC PATCH 01/25] KVM: arm64: Add has_reset_once flag for vcpu Reiji Watanabe
@ 2021-10-15 10:12   ` Andrew Jones
  2021-10-16 19:54     ` Reiji Watanabe
  0 siblings, 1 reply; 36+ messages in thread
From: Andrew Jones @ 2021-10-15 10:12 UTC (permalink / raw)
  To: Reiji Watanabe
  Cc: Marc Zyngier, kvmarm, kvm, linux-arm-kernel, James Morse,
	Alexandru Elisei, Suzuki K Poulose, Paolo Bonzini, Will Deacon,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata

On Mon, Oct 11, 2021 at 09:35:11PM -0700, Reiji Watanabe wrote:
> Introduce 'has_reset_once' flag in kvm_vcpu_arch, which indicates
> if the vCPU reset has been done once, for later use.
> 
> Signed-off-by: Reiji Watanabe <reijiw@google.com>
> ---
>  arch/arm64/include/asm/kvm_host.h | 2 ++
>  arch/arm64/kvm/reset.c            | 4 ++++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index f8be56d5342b..9b5e7a3b6011 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -384,6 +384,7 @@ struct kvm_vcpu_arch {
>  		u64 last_steal;
>  		gpa_t base;
>  	} steal;
> +	bool has_reset_once;
>  };
>  
>  /* Pointer to the vcpu's SVE FFR for sve_{save,load}_state() */
> @@ -449,6 +450,7 @@ struct kvm_vcpu_arch {
>  
>  #define vcpu_has_sve(vcpu) (system_supports_sve() &&			\
>  			    ((vcpu)->arch.flags & KVM_ARM64_GUEST_HAS_SVE))
> +#define	vcpu_has_reset_once(vcpu) ((vcpu)->arch.has_reset_once)
>  
>  #ifdef CONFIG_ARM64_PTR_AUTH
>  #define vcpu_has_ptrauth(vcpu)						\
> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> index 5ce36b0a3343..4d34e5c1586c 100644
> --- a/arch/arm64/kvm/reset.c
> +++ b/arch/arm64/kvm/reset.c
> @@ -305,6 +305,10 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
>  	if (loaded)
>  		kvm_arch_vcpu_load(vcpu, smp_processor_id());
>  	preempt_enable();
> +
> +	if (!ret && !vcpu->arch.has_reset_once)
> +		vcpu->arch.has_reset_once = true;
> +
>  	return ret;
>  }
>  
> -- 
> 2.33.0.882.g93a45727a2-goog
>

Hi Reiji,

Can't we use kvm_vcpu_initialized(vcpu)? vcpu->arch.target should
only be >= when we've successfully reset the vcpu at least once.

Thanks,
drew


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

* Re: [RFC PATCH 02/25] KVM: arm64: Save ID registers' sanitized value per vCPU
  2021-10-12  4:35 ` [RFC PATCH 02/25] KVM: arm64: Save ID registers' sanitized value per vCPU Reiji Watanabe
@ 2021-10-15 13:09   ` Andrew Jones
  2021-10-17  0:42     ` Reiji Watanabe
  0 siblings, 1 reply; 36+ messages in thread
From: Andrew Jones @ 2021-10-15 13:09 UTC (permalink / raw)
  To: Reiji Watanabe
  Cc: Marc Zyngier, kvmarm, kvm, linux-arm-kernel, James Morse,
	Alexandru Elisei, Suzuki K Poulose, Paolo Bonzini, Will Deacon,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata

On Mon, Oct 11, 2021 at 09:35:12PM -0700, Reiji Watanabe wrote:
> Extend sys_regs[] of kvm_cpu_context for ID registers and save ID
> registers' sanitized value in the array for the vCPU at the first
> vCPU reset. Use the saved ones when ID registers are read by
> userspace (via KVM_GET_ONE_REG) or the guest.
> 
> Signed-off-by: Reiji Watanabe <reijiw@google.com>
> ---
>  arch/arm64/include/asm/kvm_host.h | 10 ++++++++++
>  arch/arm64/kvm/sys_regs.c         | 26 ++++++++++++++++++--------
>  2 files changed, 28 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 9b5e7a3b6011..0cd351099adf 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -145,6 +145,14 @@ struct kvm_vcpu_fault_info {
>  	u64 disr_el1;		/* Deferred [SError] Status Register */
>  };
>  
> +/*
> + * (Op0, Op1, CRn, CRm, Op2) of ID registers is (3, 0, 0, crm, op2),
> + * where 0<=crm<8, 0<=op2<8.

crm is 4 bits, so this should be 0 <= crm < 16 and...

> + */
> +#define KVM_ARM_ID_REG_MAX_NUM 64

...this should be 128. Or am I missing something?


> +#define IDREG_IDX(id)		((sys_reg_CRm(id) << 3) | sys_reg_Op2(id))
> +#define IDREG_SYS_IDX(id)	(ID_REG_BASE + IDREG_IDX(id))
> +
>  enum vcpu_sysreg {
>  	__INVALID_SYSREG__,   /* 0 is reserved as an invalid value */
>  	MPIDR_EL1,	/* MultiProcessor Affinity Register */
> @@ -209,6 +217,8 @@ enum vcpu_sysreg {
>  	CNTP_CVAL_EL0,
>  	CNTP_CTL_EL0,
>  
> +	ID_REG_BASE,
> +	ID_REG_END = ID_REG_BASE + KVM_ARM_ID_REG_MAX_NUM - 1,
>  	/* Memory Tagging Extension registers */
>  	RGSR_EL1,	/* Random Allocation Tag Seed Register */
>  	GCR_EL1,	/* Tag Control Register */
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 1d46e185f31e..72ca518e7944 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -273,7 +273,7 @@ static bool trap_loregion(struct kvm_vcpu *vcpu,
>  			  struct sys_reg_params *p,
>  			  const struct sys_reg_desc *r)
>  {
> -	u64 val = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1);
> +	u64 val = __vcpu_sys_reg(vcpu, IDREG_SYS_IDX(SYS_ID_AA64MMFR1_EL1));
>  	u32 sr = reg_to_encoding(r);
>  
>  	if (!(val & (0xfUL << ID_AA64MMFR1_LOR_SHIFT))) {
> @@ -1059,12 +1059,11 @@ static bool access_arch_timer(struct kvm_vcpu *vcpu,
>  	return true;
>  }
>  
> -/* Read a sanitised cpufeature ID register by sys_reg_desc */
>  static u64 read_id_reg(const struct kvm_vcpu *vcpu,
>  		struct sys_reg_desc const *r, bool raz)
>  {
>  	u32 id = reg_to_encoding(r);
> -	u64 val = raz ? 0 : read_sanitised_ftr_reg(id);
> +	u64 val = raz ? 0 : __vcpu_sys_reg(vcpu, IDREG_SYS_IDX(id));
>  
>  	switch (id) {
>  	case SYS_ID_AA64PFR0_EL1:
> @@ -1174,6 +1173,16 @@ static unsigned int sve_visibility(const struct kvm_vcpu *vcpu,
>  	return REG_HIDDEN;
>  }
>  
> +static void reset_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd)

Since not all ID registers will use this, then maybe name it
reset_sanitised_id_reg?

> +{
> +	u32 id = reg_to_encoding(rd);
> +
> +	if (vcpu_has_reset_once(vcpu))
> +		return;

Ah, I see my kvm_vcpu_initialized() won't work since vcpu->arch.target is
set before the first reset. While vcpu->arch.target is only being used
like a "is_initialized" boolean at this time, I guess we better keep it
in case we ever want to implement CPU models (which this series gets us a
step closer to). 

> +
> +	__vcpu_sys_reg(vcpu, IDREG_SYS_IDX(id)) = read_sanitised_ftr_reg(id);
> +}
> +
>  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)
> @@ -1219,9 +1228,7 @@ static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
>  /*
>   * cpufeature ID register user accessors
>   *
> - * For now, these registers are immutable for userspace, so no values
> - * are stored, and for set_id_reg() we don't allow the effective value
> - * to be changed.
> + * We don't allow the effective value to be changed.
>   */
>  static int __get_id_reg(const struct kvm_vcpu *vcpu,
>  			const struct sys_reg_desc *rd, void __user *uaddr,
> @@ -1375,6 +1382,7 @@ static unsigned int mte_visibility(const struct kvm_vcpu *vcpu,
>  #define ID_SANITISED(name) {			\
>  	SYS_DESC(SYS_##name),			\
>  	.access	= access_id_reg,		\
> +	.reset	= reset_id_reg,			\
>  	.get_user = get_id_reg,			\
>  	.set_user = set_id_reg,			\
>  	.visibility = id_visibility,		\
> @@ -1830,8 +1838,10 @@ static bool trap_dbgdidr(struct kvm_vcpu *vcpu,
>  	if (p->is_write) {
>  		return ignore_write(vcpu, p);
>  	} else {
> -		u64 dfr = read_sanitised_ftr_reg(SYS_ID_AA64DFR0_EL1);
> -		u64 pfr = read_sanitised_ftr_reg(SYS_ID_AA64PFR0_EL1);
> +		u64 dfr = __vcpu_sys_reg(vcpu,
> +					 IDREG_SYS_IDX(SYS_ID_AA64DFR0_EL1));
> +		u64 pfr = __vcpu_sys_reg(vcpu,
> +					 IDREG_SYS_IDX(SYS_ID_AA64PFR0_EL1));

Please avoid these ugly line breaks when we're well under Linux's max
length, which is 100.

>  		u32 el3 = !!cpuid_feature_extract_unsigned_field(pfr, ID_AA64PFR0_EL3_SHIFT);
>  
>  		p->regval = ((((dfr >> ID_AA64DFR0_WRPS_SHIFT) & 0xf) << 28) |
> -- 
> 2.33.0.882.g93a45727a2-goog
>

Thanks,
drew


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

* Re: [RFC PATCH 03/25] KVM: arm64: Introduce a validation function for an ID register
  2021-10-12  4:35 ` [RFC PATCH 03/25] KVM: arm64: Introduce a validation function for an ID register Reiji Watanabe
@ 2021-10-15 13:30   ` Andrew Jones
       [not found]     ` <CAAeT=Fy-enk=X_PaRSDEKQ01yQzdyU=bcpq8cuCZhtpzC=JvnQ@mail.gmail.com>
  0 siblings, 1 reply; 36+ messages in thread
From: Andrew Jones @ 2021-10-15 13:30 UTC (permalink / raw)
  To: Reiji Watanabe
  Cc: Marc Zyngier, kvmarm, kvm, linux-arm-kernel, James Morse,
	Alexandru Elisei, Suzuki K Poulose, Paolo Bonzini, Will Deacon,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata

On Mon, Oct 11, 2021 at 09:35:13PM -0700, Reiji Watanabe wrote:
> Introduce arm64_check_features(), which does a basic validity checking
> of an ID register value against the register's limit value that KVM
> can support.
> This function will be used by the following patches to check if an ID
> register value that userspace tries to set can be supported by KVM on
> the host.
> 
> Signed-off-by: Reiji Watanabe <reijiw@google.com>
> ---
>  arch/arm64/include/asm/cpufeature.h |  1 +
>  arch/arm64/kernel/cpufeature.c      | 26 ++++++++++++++++++++++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index ef6be92b1921..eda7ddbed8cf 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -631,6 +631,7 @@ void check_local_cpu_capabilities(void);
>  
>  u64 read_sanitised_ftr_reg(u32 id);
>  u64 __read_sysreg_by_encoding(u32 sys_id);
> +int arm64_check_features(u32 sys_reg, u64 val, u64 limit);
>  
>  static inline bool cpu_supports_mixed_endian_el0(void)
>  {
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 6ec7036ef7e1..d146ef759435 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -3114,3 +3114,29 @@ ssize_t cpu_show_meltdown(struct device *dev, struct device_attribute *attr,
>  		return sprintf(buf, "Vulnerable\n");
>  	}
>  }
> +
> +/*
> + * Check if all features that are indicated in the given ID register value
> + * ('val') are also indicated in the 'limit'.

Maybe use @<param> syntax to reference the parameters, even though this
file doesn't seem to adopt that anywhere else...

> + */
> +int arm64_check_features(u32 sys_reg, u64 val, u64 limit)
> +{
> +	struct arm64_ftr_reg *reg = get_arm64_ftr_reg(sys_reg);
> +	const struct arm64_ftr_bits *ftrp;
> +	u64 exposed_mask = 0;
> +
> +	if (!reg)
> +		return -ENOENT;
> +
> +	for (ftrp = reg->ftr_bits; ftrp->width; ftrp++) {
> +		if (arm64_ftr_value(ftrp, val) > arm64_ftr_value(ftrp, limit))

Hmm. Are we sure that '>' is the correct operator for all comparisons? It
seems like we need a arm64_ftr_compare() function that takes
arm64_ftr_bits.type and .sign into account.

> +			return -E2BIG;
> +
> +		exposed_mask |= arm64_ftr_mask(ftrp);
> +	}
> +
> +	if (val & ~exposed_mask)
> +		return -E2BIG;

I'm not sure we want this. I think it implies that any RAO bits need to be
cleared before calling this function, which could be inconvenient.

Thanks,
drew

> +
> +	return 0;
> +}
> -- 
> 2.33.0.882.g93a45727a2-goog
> 


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

* Re: [RFC PATCH 04/25] KVM: arm64: Introduce struct id_reg_info
  2021-10-12  4:35 ` [RFC PATCH 04/25] KVM: arm64: Introduce struct id_reg_info Reiji Watanabe
@ 2021-10-15 13:47   ` Andrew Jones
  2021-10-17  4:43     ` Reiji Watanabe
  0 siblings, 1 reply; 36+ messages in thread
From: Andrew Jones @ 2021-10-15 13:47 UTC (permalink / raw)
  To: Reiji Watanabe
  Cc: Marc Zyngier, kvmarm, kvm, linux-arm-kernel, James Morse,
	Alexandru Elisei, Suzuki K Poulose, Paolo Bonzini, Will Deacon,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata

On Mon, Oct 11, 2021 at 09:35:14PM -0700, Reiji Watanabe wrote:
> This patch lays the groundwork to make ID registers writable.
> 
> Introduce struct id_reg_info for an ID register to manage the
> register specific control of its value for the guest.
> It is used to do register specific initialization, validation
> of the ID register and etc.  Not all ID registers must have
> the id_reg_info. ID registers that don't have the id_reg_info
> are handled in a common way that is applied to all other
> ID registers.
> 
> At present, changing an ID register from userspace is allowed only
> if the ID register has the id_reg_info, but that will be changed
> by the following patches.
> 
> No ID register has the structure yet and the following patches
> will add the id_reg_info for some ID registers.
> 
> Signed-off-by: Reiji Watanabe <reijiw@google.com>
> ---
>  arch/arm64/kvm/sys_regs.c | 120 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 111 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 72ca518e7944..8a0b88f9a975 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -263,6 +263,76 @@ static bool trap_raz_wi(struct kvm_vcpu *vcpu,
>  		return read_zero(vcpu, p);
>  }
>  
> +struct id_reg_info {
> +	u32	sys_reg;	/* Register ID */
> +	u64	sys_val;	/* Sanitized system value */
> +
> +	/*
> +	 * Limit value of the register for a vcpu. The value is sys_val
> +	 * with bits cleared for unsupported features for the guest.
> +	 */
> +	u64	vcpu_limit_val;

Maybe I'll see a need for both later, but at the moment I'd think we only
need sys_val with the bits cleared for disabled features.

> +
> +	/* Bits that are not validated when userspace sets the register. */
> +	u64	ignore_mask;
> +
> +	/* Initialization function of the id_reg_info */
> +	void (*init)(struct id_reg_info *id_reg);
> +
> +	/* Register specific validation function */
> +	int (*validate)(struct kvm_vcpu *vcpu, u64 val);
> +
> +	/* Return the reset value of the register for the vCPU */
> +	u64 (*get_reset_val)(struct kvm_vcpu *vcpu, struct id_reg_info *id_reg);
> +};
> +
> +static void id_reg_info_init(struct id_reg_info *id_reg)
> +{
> +	id_reg->sys_val = read_sanitised_ftr_reg(id_reg->sys_reg);
> +	id_reg->vcpu_limit_val = id_reg->sys_val;
> +}
> +
> +/*
> + * An ID register that needs special handling to control the value for the
> + * guest must have its own id_reg_info in id_reg_info_table.
> + * (i.e. the reset value is different from the host's sanitized value,
> + * the value is affected by opt-in features, some fields needs specific
> + * validation, etc.)
> + */
> +#define	GET_ID_REG_INFO(id)	(id_reg_info_table[IDREG_IDX(id)])
> +static struct id_reg_info *id_reg_info_table[KVM_ARM_ID_REG_MAX_NUM] = {};
> +
> +static int validate_id_reg(struct kvm_vcpu *vcpu,
> +			   const struct sys_reg_desc *rd, u64 val)
> +{
> +	u32 id = reg_to_encoding(rd);
> +	const struct id_reg_info *id_reg = GET_ID_REG_INFO(id);
> +	u64 limit;
> +	u64 check_val = val;
> +	int err;
> +
> +	if (id_reg) {
> +		/*
> +		 * Clear bits with ignore_mask, which we don't want
> +		 * arm64_check_features to check.
> +		 */
> +		check_val &= ~id_reg->ignore_mask;
> +		limit = id_reg->vcpu_limit_val;
> +	} else
> +		limit = read_sanitised_ftr_reg(id);
> +
> +	/* Check if the value indicates any feature that is not in the limit. */
> +	err = arm64_check_features(id, check_val, limit);
> +	if (err)
> +		return err;
> +
> +	if (id_reg && id_reg->validate)
> +		/* Run the ID register specific validity check. */
> +		err = id_reg->validate(vcpu, val);
> +
> +	return err;
> +}
> +
>  /*
>   * ARMv8.1 mandates at least a trivial LORegion implementation, where all the
>   * RW registers are RES0 (which we can implement as RAZ/WI). On an ARMv8.0
> @@ -1176,11 +1246,19 @@ static unsigned int sve_visibility(const struct kvm_vcpu *vcpu,
>  static void reset_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd)
>  {
>  	u32 id = reg_to_encoding(rd);
> +	struct id_reg_info *id_reg;
> +	u64 val;
>  
>  	if (vcpu_has_reset_once(vcpu))
>  		return;
>  
> -	__vcpu_sys_reg(vcpu, IDREG_SYS_IDX(id)) = read_sanitised_ftr_reg(id);
> +	id_reg = GET_ID_REG_INFO(id);
> +	if (id_reg && id_reg->get_reset_val)
> +		val = id_reg->get_reset_val(vcpu, id_reg);
> +	else
> +		val = read_sanitised_ftr_reg(id);
> +
> +	__vcpu_sys_reg(vcpu, IDREG_SYS_IDX(id)) = val;
>  }
>  
>  static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
> @@ -1225,11 +1303,7 @@ static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
>  	return 0;
>  }
>  
> -/*
> - * cpufeature ID register user accessors
> - *
> - * We don't allow the effective value to be changed.
> - */
> +/* cpufeature ID register user accessors */
>  static int __get_id_reg(const struct kvm_vcpu *vcpu,
>  			const struct sys_reg_desc *rd, void __user *uaddr,
>  			bool raz)
> @@ -1240,11 +1314,12 @@ static int __get_id_reg(const struct kvm_vcpu *vcpu,
>  	return reg_to_user(uaddr, &val, id);
>  }
>  
> -static int __set_id_reg(const struct kvm_vcpu *vcpu,
> +static int __set_id_reg(struct kvm_vcpu *vcpu,
>  			const struct sys_reg_desc *rd, void __user *uaddr,
>  			bool raz)
>  {
>  	const u64 id = sys_reg_to_index(rd);
> +	u32 encoding = reg_to_encoding(rd);
>  	int err;
>  	u64 val;
>  
> @@ -1252,10 +1327,18 @@ static int __set_id_reg(const struct kvm_vcpu *vcpu,
>  	if (err)
>  		return err;
>  
> -	/* This is what we mean by invariant: you can't change it. */
> -	if (val != read_id_reg(vcpu, rd, raz))
> +	/* Don't allow to change the reg unless the reg has id_reg_info */
> +	if (val != read_id_reg(vcpu, rd, raz) && !GET_ID_REG_INFO(encoding))
>  		return -EINVAL;
>  
> +	if (raz)
> +		return (val == 0) ? 0 : -EINVAL;

This is already covered by the val != read_id_reg(vcpu, rd, raz) check.

> +
> +	err = validate_id_reg(vcpu, rd, val);
> +	if (err)
> +		return err;
> +
> +	__vcpu_sys_reg(vcpu, IDREG_SYS_IDX(encoding)) = val;
>  	return 0;
>  }
>  
> @@ -2818,6 +2901,23 @@ int kvm_arm_copy_sys_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices)
>  	return write_demux_regids(uindices);
>  }
>  
> +static void id_reg_info_init_all(void)
> +{
> +	int i;
> +	struct id_reg_info *id_reg;
> +
> +	for (i = 0; i < ARRAY_SIZE(id_reg_info_table); i++) {
> +		id_reg = (struct id_reg_info *)id_reg_info_table[i];
> +		if (!id_reg)
> +			continue;
> +
> +		if (id_reg->init)
> +			id_reg->init(id_reg);
> +		else
> +			id_reg_info_init(id_reg);

Maybe call id_reg->init(id_reg) from within id_reg_info_init() in case we
wanted to apply some common id register initialization at some point?

> +	}
> +}
> +
>  void kvm_sys_reg_table_init(void)
>  {
>  	unsigned int i;
> @@ -2852,4 +2952,6 @@ void kvm_sys_reg_table_init(void)
>  			break;
>  	/* Clear all higher bits. */
>  	cache_levels &= (1 << (i*3))-1;
> +
> +	id_reg_info_init_all();
>  }
> -- 
> 2.33.0.882.g93a45727a2-goog
>

Thanks,
drew 


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

* Re: [RFC PATCH 01/25] KVM: arm64: Add has_reset_once flag for vcpu
  2021-10-15 10:12   ` Andrew Jones
@ 2021-10-16 19:54     ` Reiji Watanabe
  0 siblings, 0 replies; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-16 19:54 UTC (permalink / raw)
  To: Andrew Jones
  Cc: Marc Zyngier, kvmarm, kvm, linux-arm-kernel, James Morse,
	Alexandru Elisei, Suzuki K Poulose, Paolo Bonzini, Will Deacon,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata

Hi Andrew,

On Fri, Oct 15, 2021 at 3:13 AM Andrew Jones <drjones@redhat.com> wrote:
>
> On Mon, Oct 11, 2021 at 09:35:11PM -0700, Reiji Watanabe wrote:
> > Introduce 'has_reset_once' flag in kvm_vcpu_arch, which indicates
> > if the vCPU reset has been done once, for later use.
> >
> > Signed-off-by: Reiji Watanabe <reijiw@google.com>
> > ---
> >  arch/arm64/include/asm/kvm_host.h | 2 ++
> >  arch/arm64/kvm/reset.c            | 4 ++++
> >  2 files changed, 6 insertions(+)
> >
> > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > index f8be56d5342b..9b5e7a3b6011 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -384,6 +384,7 @@ struct kvm_vcpu_arch {
> >               u64 last_steal;
> >               gpa_t base;
> >       } steal;
> > +     bool has_reset_once;
> >  };
> >
> >  /* Pointer to the vcpu's SVE FFR for sve_{save,load}_state() */
> > @@ -449,6 +450,7 @@ struct kvm_vcpu_arch {
> >
> >  #define vcpu_has_sve(vcpu) (system_supports_sve() &&                 \
> >                           ((vcpu)->arch.flags & KVM_ARM64_GUEST_HAS_SVE))
> > +#define      vcpu_has_reset_once(vcpu) ((vcpu)->arch.has_reset_once)
> >
> >  #ifdef CONFIG_ARM64_PTR_AUTH
> >  #define vcpu_has_ptrauth(vcpu)                                               \
> > diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> > index 5ce36b0a3343..4d34e5c1586c 100644
> > --- a/arch/arm64/kvm/reset.c
> > +++ b/arch/arm64/kvm/reset.c
> > @@ -305,6 +305,10 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
> >       if (loaded)
> >               kvm_arch_vcpu_load(vcpu, smp_processor_id());
> >       preempt_enable();
> > +
> > +     if (!ret && !vcpu->arch.has_reset_once)
> > +             vcpu->arch.has_reset_once = true;
> > +
> >       return ret;
> >  }
> >
> > --
> > 2.33.0.882.g93a45727a2-goog
> >
>
> Hi Reiji,
>
> Can't we use kvm_vcpu_initialized(vcpu)? vcpu->arch.target should
> only be >= when we've successfully reset the vcpu at least once.

Thank you for reviewing the patch (and other patches as well) !

As you already noticed, we can't simply use kvm_vcpu_initialized()
because vcpu->arch.target is currently set earlier than the first
vcpu reset.

Thanks,
Reiji

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

* Re: [RFC PATCH 02/25] KVM: arm64: Save ID registers' sanitized value per vCPU
  2021-10-15 13:09   ` Andrew Jones
@ 2021-10-17  0:42     ` Reiji Watanabe
  2021-10-18 14:30       ` Andrew Jones
  0 siblings, 1 reply; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-17  0:42 UTC (permalink / raw)
  To: Andrew Jones
  Cc: Marc Zyngier, kvmarm, kvm, linux-arm-kernel, James Morse,
	Alexandru Elisei, Suzuki K Poulose, Paolo Bonzini, Will Deacon,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata

On Fri, Oct 15, 2021 at 6:09 AM Andrew Jones <drjones@redhat.com> wrote:
>
> On Mon, Oct 11, 2021 at 09:35:12PM -0700, Reiji Watanabe wrote:
> > Extend sys_regs[] of kvm_cpu_context for ID registers and save ID
> > registers' sanitized value in the array for the vCPU at the first
> > vCPU reset. Use the saved ones when ID registers are read by
> > userspace (via KVM_GET_ONE_REG) or the guest.
> >
> > Signed-off-by: Reiji Watanabe <reijiw@google.com>
> > ---
> >  arch/arm64/include/asm/kvm_host.h | 10 ++++++++++
> >  arch/arm64/kvm/sys_regs.c         | 26 ++++++++++++++++++--------
> >  2 files changed, 28 insertions(+), 8 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > index 9b5e7a3b6011..0cd351099adf 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -145,6 +145,14 @@ struct kvm_vcpu_fault_info {
> >       u64 disr_el1;           /* Deferred [SError] Status Register */
> >  };
> >
> > +/*
> > + * (Op0, Op1, CRn, CRm, Op2) of ID registers is (3, 0, 0, crm, op2),
> > + * where 0<=crm<8, 0<=op2<8.
>
> crm is 4 bits, so this should be 0 <= crm < 16 and...
>
> > + */
> > +#define KVM_ARM_ID_REG_MAX_NUM 64
>
> ...this should be 128. Or am I missing something?

Registers with (3, 0, 0, 0<=crm<8, op2) are defined/allocated including
reserved (RAZ) ones (please see Table D12-2 in ARM DDI 0487G.b),
and the code supports those only for now.

I understand that registers with crm >= 8 could be defined in the future
(I'm not so sure if they will be really ID registers though),
but then we can include them later as needed.

> > +#define IDREG_IDX(id)                ((sys_reg_CRm(id) << 3) | sys_reg_Op2(id))
> > +#define IDREG_SYS_IDX(id)    (ID_REG_BASE + IDREG_IDX(id))
> > +
> >  enum vcpu_sysreg {
> >       __INVALID_SYSREG__,   /* 0 is reserved as an invalid value */
> >       MPIDR_EL1,      /* MultiProcessor Affinity Register */
> > @@ -209,6 +217,8 @@ enum vcpu_sysreg {
> >       CNTP_CVAL_EL0,
> >       CNTP_CTL_EL0,
> >
> > +     ID_REG_BASE,
> > +     ID_REG_END = ID_REG_BASE + KVM_ARM_ID_REG_MAX_NUM - 1,
> >       /* Memory Tagging Extension registers */
> >       RGSR_EL1,       /* Random Allocation Tag Seed Register */
> >       GCR_EL1,        /* Tag Control Register */
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index 1d46e185f31e..72ca518e7944 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -273,7 +273,7 @@ static bool trap_loregion(struct kvm_vcpu *vcpu,
> >                         struct sys_reg_params *p,
> >                         const struct sys_reg_desc *r)
> >  {
> > -     u64 val = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1);
> > +     u64 val = __vcpu_sys_reg(vcpu, IDREG_SYS_IDX(SYS_ID_AA64MMFR1_EL1));
> >       u32 sr = reg_to_encoding(r);
> >
> >       if (!(val & (0xfUL << ID_AA64MMFR1_LOR_SHIFT))) {
> > @@ -1059,12 +1059,11 @@ static bool access_arch_timer(struct kvm_vcpu *vcpu,
> >       return true;
> >  }
> >
> > -/* Read a sanitised cpufeature ID register by sys_reg_desc */
> >  static u64 read_id_reg(const struct kvm_vcpu *vcpu,
> >               struct sys_reg_desc const *r, bool raz)
> >  {
> >       u32 id = reg_to_encoding(r);
> > -     u64 val = raz ? 0 : read_sanitised_ftr_reg(id);
> > +     u64 val = raz ? 0 : __vcpu_sys_reg(vcpu, IDREG_SYS_IDX(id));
> >
> >       switch (id) {
> >       case SYS_ID_AA64PFR0_EL1:
> > @@ -1174,6 +1173,16 @@ static unsigned int sve_visibility(const struct kvm_vcpu *vcpu,
> >       return REG_HIDDEN;
> >  }
> >
> > +static void reset_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd)
>
> Since not all ID registers will use this, then maybe name it
> reset_sanitised_id_reg?

Thank you for the suggestion.

I named it 'reset_id_reg' according to the naming conventions of
set_id_reg, get_id_reg, and access_id_reg which are used for the same
set of ID registers (ID_SANITISED ones) as reset_id_reg.
I would think it's better to use consistent names for all of them.
So, I am a bit reluctant to change only the name of reset_id_reg.

What do you think about the names of those other three functions ?


> > +{
> > +     u32 id = reg_to_encoding(rd);
> > +
> > +     if (vcpu_has_reset_once(vcpu))
> > +             return;
>
> Ah, I see my kvm_vcpu_initialized() won't work since vcpu->arch.target is
> set before the first reset. While vcpu->arch.target is only being used
> like a "is_initialized" boolean at this time, I guess we better keep it
> in case we ever want to implement CPU models (which this series gets us a
> step closer to).

Thank you for sharing your thoughts and I agree with you.


> > +
> > +     __vcpu_sys_reg(vcpu, IDREG_SYS_IDX(id)) = read_sanitised_ftr_reg(id);
> > +}
> > +
> >  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)
> > @@ -1219,9 +1228,7 @@ static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
> >  /*
> >   * cpufeature ID register user accessors
> >   *
> > - * For now, these registers are immutable for userspace, so no values
> > - * are stored, and for set_id_reg() we don't allow the effective value
> > - * to be changed.
> > + * We don't allow the effective value to be changed.
> >   */
> >  static int __get_id_reg(const struct kvm_vcpu *vcpu,
> >                       const struct sys_reg_desc *rd, void __user *uaddr,
> > @@ -1375,6 +1382,7 @@ static unsigned int mte_visibility(const struct kvm_vcpu *vcpu,
> >  #define ID_SANITISED(name) {                 \
> >       SYS_DESC(SYS_##name),                   \
> >       .access = access_id_reg,                \
> > +     .reset  = reset_id_reg,                 \
> >       .get_user = get_id_reg,                 \
> >       .set_user = set_id_reg,                 \
> >       .visibility = id_visibility,            \
> > @@ -1830,8 +1838,10 @@ static bool trap_dbgdidr(struct kvm_vcpu *vcpu,
> >       if (p->is_write) {
> >               return ignore_write(vcpu, p);
> >       } else {
> > -             u64 dfr = read_sanitised_ftr_reg(SYS_ID_AA64DFR0_EL1);
> > -             u64 pfr = read_sanitised_ftr_reg(SYS_ID_AA64PFR0_EL1);
> > +             u64 dfr = __vcpu_sys_reg(vcpu,
> > +                                      IDREG_SYS_IDX(SYS_ID_AA64DFR0_EL1));
> > +             u64 pfr = __vcpu_sys_reg(vcpu,
> > +                                      IDREG_SYS_IDX(SYS_ID_AA64PFR0_EL1));
>
> Please avoid these ugly line breaks when we're well under Linux's max
> length, which is 100.

Yes, I will fix them (as well as all other similar line breaks
for other patches in my series).

Thanks,
Reiji

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

* Re: [RFC PATCH 04/25] KVM: arm64: Introduce struct id_reg_info
  2021-10-15 13:47   ` Andrew Jones
@ 2021-10-17  4:43     ` Reiji Watanabe
  0 siblings, 0 replies; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-17  4:43 UTC (permalink / raw)
  To: Andrew Jones
  Cc: Marc Zyngier, kvmarm, kvm, linux-arm-kernel, James Morse,
	Alexandru Elisei, Suzuki K Poulose, Paolo Bonzini, Will Deacon,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata

> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -263,6 +263,76 @@ static bool trap_raz_wi(struct kvm_vcpu *vcpu,
> >               return read_zero(vcpu, p);
> >  }
> >
> > +struct id_reg_info {
> > +     u32     sys_reg;        /* Register ID */
> > +     u64     sys_val;        /* Sanitized system value */
> > +
> > +     /*
> > +      * Limit value of the register for a vcpu. The value is sys_val
> > +      * with bits cleared for unsupported features for the guest.
> > +      */
> > +     u64     vcpu_limit_val;
>
> Maybe I'll see a need for both later, but at the moment I'd think we only
> need sys_val with the bits cleared for disabled features.

Uh, yes, sys_val is used in patch-15 and I should have introduced
the field in the patch.  I will fix it in v2.


> > -static int __set_id_reg(const struct kvm_vcpu *vcpu,
> > +static int __set_id_reg(struct kvm_vcpu *vcpu,
> >                       const struct sys_reg_desc *rd, void __user *uaddr,
> >                       bool raz)
> >  {
> >       const u64 id = sys_reg_to_index(rd);
> > +     u32 encoding = reg_to_encoding(rd);
> >       int err;
> >       u64 val;
> >
> > @@ -1252,10 +1327,18 @@ static int __set_id_reg(const struct kvm_vcpu *vcpu,
> >       if (err)
> >               return err;
> >
> > -     /* This is what we mean by invariant: you can't change it. */
> > -     if (val != read_id_reg(vcpu, rd, raz))
> > +     /* Don't allow to change the reg unless the reg has id_reg_info */
> > +     if (val != read_id_reg(vcpu, rd, raz) && !GET_ID_REG_INFO(encoding))
> >               return -EINVAL;
> >
> > +     if (raz)
> > +             return (val == 0) ? 0 : -EINVAL;
>
> This is already covered by the val != read_id_reg(vcpu, rd, raz) check.

Yes, it can simply return 0 for raz case in this patch.
I will fix this in v2.


> > +     err = validate_id_reg(vcpu, rd, val);
> > +     if (err)
> > +             return err;
> > +
> > +     __vcpu_sys_reg(vcpu, IDREG_SYS_IDX(encoding)) = val;
> >       return 0;
> >  }
> >
> > @@ -2818,6 +2901,23 @@ int kvm_arm_copy_sys_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices)
> >       return write_demux_regids(uindices);
> >  }
> >
> > +static void id_reg_info_init_all(void)
> > +{
> > +     int i;
> > +     struct id_reg_info *id_reg;
> > +
> > +     for (i = 0; i < ARRAY_SIZE(id_reg_info_table); i++) {
> > +             id_reg = (struct id_reg_info *)id_reg_info_table[i];
> > +             if (!id_reg)
> > +                     continue;
> > +
> > +             if (id_reg->init)
> > +                     id_reg->init(id_reg);
> > +             else
> > +                     id_reg_info_init(id_reg);
>
> Maybe call id_reg->init(id_reg) from within id_reg_info_init() in case we
> wanted to apply some common id register initialization at some point?

Thank you for the nice suggestion.
That sounds like a better idea. I'll look into fixing it in v2.

Thanks,
Reiji

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

* Re: [RFC PATCH 02/25] KVM: arm64: Save ID registers' sanitized value per vCPU
  2021-10-17  0:42     ` Reiji Watanabe
@ 2021-10-18 14:30       ` Andrew Jones
  2021-10-18 23:54         ` Reiji Watanabe
  0 siblings, 1 reply; 36+ messages in thread
From: Andrew Jones @ 2021-10-18 14:30 UTC (permalink / raw)
  To: Reiji Watanabe
  Cc: Marc Zyngier, kvmarm, kvm, linux-arm-kernel, James Morse,
	Alexandru Elisei, Suzuki K Poulose, Paolo Bonzini, Will Deacon,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata

On Sat, Oct 16, 2021 at 05:42:59PM -0700, Reiji Watanabe wrote:
> On Fri, Oct 15, 2021 at 6:09 AM Andrew Jones <drjones@redhat.com> wrote:
> >
> > On Mon, Oct 11, 2021 at 09:35:12PM -0700, Reiji Watanabe wrote:
> > > Extend sys_regs[] of kvm_cpu_context for ID registers and save ID
> > > registers' sanitized value in the array for the vCPU at the first
> > > vCPU reset. Use the saved ones when ID registers are read by
> > > userspace (via KVM_GET_ONE_REG) or the guest.
> > >
> > > Signed-off-by: Reiji Watanabe <reijiw@google.com>
> > > ---
> > >  arch/arm64/include/asm/kvm_host.h | 10 ++++++++++
> > >  arch/arm64/kvm/sys_regs.c         | 26 ++++++++++++++++++--------
> > >  2 files changed, 28 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > > index 9b5e7a3b6011..0cd351099adf 100644
> > > --- a/arch/arm64/include/asm/kvm_host.h
> > > +++ b/arch/arm64/include/asm/kvm_host.h
> > > @@ -145,6 +145,14 @@ struct kvm_vcpu_fault_info {
> > >       u64 disr_el1;           /* Deferred [SError] Status Register */
> > >  };
> > >
> > > +/*
> > > + * (Op0, Op1, CRn, CRm, Op2) of ID registers is (3, 0, 0, crm, op2),
> > > + * where 0<=crm<8, 0<=op2<8.
> >
> > crm is 4 bits, so this should be 0 <= crm < 16 and...
> >
> > > + */
> > > +#define KVM_ARM_ID_REG_MAX_NUM 64
> >
> > ...this should be 128. Or am I missing something?
> 
> Registers with (3, 0, 0, 0<=crm<8, op2) are defined/allocated including
> reserved (RAZ) ones (please see Table D12-2 in ARM DDI 0487G.b),
> and the code supports those only for now.
> 
> I understand that registers with crm >= 8 could be defined in the future
> (I'm not so sure if they will be really ID registers though),
> but then we can include them later as needed.

Oh, I see. Thanks. Looking at the table I see CRm=0,op2={1,2,3,4,7} are
also missing, but it certainly doesn't matter that we allocate a few
unused entries, especially since we also allocate entries for all the
RAZ ones.

> 
> > > +#define IDREG_IDX(id)                ((sys_reg_CRm(id) << 3) | sys_reg_Op2(id))
> > > +#define IDREG_SYS_IDX(id)    (ID_REG_BASE + IDREG_IDX(id))
> > > +
> > >  enum vcpu_sysreg {
> > >       __INVALID_SYSREG__,   /* 0 is reserved as an invalid value */
> > >       MPIDR_EL1,      /* MultiProcessor Affinity Register */
> > > @@ -209,6 +217,8 @@ enum vcpu_sysreg {
> > >       CNTP_CVAL_EL0,
> > >       CNTP_CTL_EL0,
> > >
> > > +     ID_REG_BASE,
> > > +     ID_REG_END = ID_REG_BASE + KVM_ARM_ID_REG_MAX_NUM - 1,
> > >       /* Memory Tagging Extension registers */
> > >       RGSR_EL1,       /* Random Allocation Tag Seed Register */
> > >       GCR_EL1,        /* Tag Control Register */
> > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > > index 1d46e185f31e..72ca518e7944 100644
> > > --- a/arch/arm64/kvm/sys_regs.c
> > > +++ b/arch/arm64/kvm/sys_regs.c
> > > @@ -273,7 +273,7 @@ static bool trap_loregion(struct kvm_vcpu *vcpu,
> > >                         struct sys_reg_params *p,
> > >                         const struct sys_reg_desc *r)
> > >  {
> > > -     u64 val = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1);
> > > +     u64 val = __vcpu_sys_reg(vcpu, IDREG_SYS_IDX(SYS_ID_AA64MMFR1_EL1));
> > >       u32 sr = reg_to_encoding(r);
> > >
> > >       if (!(val & (0xfUL << ID_AA64MMFR1_LOR_SHIFT))) {
> > > @@ -1059,12 +1059,11 @@ static bool access_arch_timer(struct kvm_vcpu *vcpu,
> > >       return true;
> > >  }
> > >
> > > -/* Read a sanitised cpufeature ID register by sys_reg_desc */
> > >  static u64 read_id_reg(const struct kvm_vcpu *vcpu,
> > >               struct sys_reg_desc const *r, bool raz)
> > >  {
> > >       u32 id = reg_to_encoding(r);
> > > -     u64 val = raz ? 0 : read_sanitised_ftr_reg(id);
> > > +     u64 val = raz ? 0 : __vcpu_sys_reg(vcpu, IDREG_SYS_IDX(id));
> > >
> > >       switch (id) {
> > >       case SYS_ID_AA64PFR0_EL1:
> > > @@ -1174,6 +1173,16 @@ static unsigned int sve_visibility(const struct kvm_vcpu *vcpu,
> > >       return REG_HIDDEN;
> > >  }
> > >
> > > +static void reset_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd)
> >
> > Since not all ID registers will use this, then maybe name it
> > reset_sanitised_id_reg?
> 
> Thank you for the suggestion.
> 
> I named it 'reset_id_reg' according to the naming conventions of
> set_id_reg, get_id_reg, and access_id_reg which are used for the same
> set of ID registers (ID_SANITISED ones) as reset_id_reg.
> I would think it's better to use consistent names for all of them.
> So, I am a bit reluctant to change only the name of reset_id_reg.
> 
> What do you think about the names of those other three functions ?

I think I like the shorter names, so please disregard my suggestion.

Thanks,
drew


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

* Re: [RFC PATCH 02/25] KVM: arm64: Save ID registers' sanitized value per vCPU
  2021-10-18 14:30       ` Andrew Jones
@ 2021-10-18 23:54         ` Reiji Watanabe
  0 siblings, 0 replies; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-18 23:54 UTC (permalink / raw)
  To: Andrew Jones
  Cc: Marc Zyngier, kvmarm, kvm, linux-arm-kernel, James Morse,
	Alexandru Elisei, Suzuki K Poulose, Paolo Bonzini, Will Deacon,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata

> > > > +static void reset_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd)
> > >
> > > Since not all ID registers will use this, then maybe name it
> > > reset_sanitised_id_reg?
> >
> > Thank you for the suggestion.
> >
> > I named it 'reset_id_reg' according to the naming conventions of
> > set_id_reg, get_id_reg, and access_id_reg which are used for the same
> > set of ID registers (ID_SANITISED ones) as reset_id_reg.
> > I would think it's better to use consistent names for all of them.
> > So, I am a bit reluctant to change only the name of reset_id_reg.
> >
> > What do you think about the names of those other three functions ?
>
> I think I like the shorter names, so please disregard my suggestion.

Thank you for the confirmation. I will keep those names as they are.

Regards,
Reiji

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

* Re: [RFC PATCH 03/25] KVM: arm64: Introduce a validation function for an ID register
       [not found]           ` <20211019062516.smjbbil5ugbipwno@gator.home>
@ 2021-10-19  7:26             ` Reiji Watanabe
  0 siblings, 0 replies; 36+ messages in thread
From: Reiji Watanabe @ 2021-10-19  7:26 UTC (permalink / raw)
  To: Andrew Jones
  Cc: Marc Zyngier, kvmarm, kvm, linux-arm-kernel, James Morse,
	Alexandru Elisei, Suzuki K Poulose, Paolo Bonzini, Will Deacon,
	Peng Liang, Peter Shier, Ricardo Koller, Oliver Upton,
	Jing Zhang, Raghavendra Rao Anata

I've just noticed that I mistakenly dropped all the CC-list
when I replied to Andrew's first email.
So, I've added all of them back.  I'm sorry for the mistake.

The following email still includes all the discussion with Andrew
except for the first comment about using @<param> for the source
code comments of arm64_check_features(), which I will fix in v2.

On Mon, Oct 18, 2021 at 11:25 PM Andrew Jones <drjones@redhat.com> wrote:
>
> On Mon, Oct 18, 2021 at 03:59:35PM -0700, Reiji Watanabe wrote:
> > > > > > +int arm64_check_features(u32 sys_reg, u64 val, u64 limit)
> > > > > > +{
> > > > > > +     struct arm64_ftr_reg *reg = get_arm64_ftr_reg(sys_reg);
> > > > > > +     const struct arm64_ftr_bits *ftrp;
> > > > > > +     u64 exposed_mask = 0;
> > > > > > +
> > > > > > +     if (!reg)
> > > > > > +             return -ENOENT;
> > > > > > +
> > > > > > +     for (ftrp = reg->ftr_bits; ftrp->width; ftrp++) {
> > > > > > +             if (arm64_ftr_value(ftrp, val) > arm64_ftr_value(ftrp, limit))
> > > > >
> > > > > Hmm. Are we sure that '>' is the correct operator for all comparisons? It
> > > > > seems like we need a arm64_ftr_compare() function that takes
> > > > > arm64_ftr_bits.type and .sign into account.
> > > >
> > > > Thank you for bringing up this point.
> > > >
> > > > I agree that '>' is not always the appropriate operator (mostly
> > > > inappropriate) for cases where .type is not FTR_LOWER_SAFE.
> > > > (The code takes .sign into account though)
> > > >
> > > > Looking at each field with .type != FTR_LOWER_SAFE, I don't think
> > > > we can simply determine how to check the field based on .type
> > > > because the appropriate way highly depends on the field.
> > > >
> > > >  - For ID_AA64DFR0_EL1.PMUVER/DEBUGVER (FTR_EXACT),
> > > >    we can simply use '>'.
> > > >  - For ID_AA64ISAR1_EL1.APA/API (FTR_EXACT),
> > > >    it looks like those fields must match between val and limit or
> > > >    the ones in val can be zero.
> > > >  - For ID_AA64MMFR0.TGRAN4_2/TGRAN64_2/TGRAN16_2 (FTR_EXACT),
> > > >    when the field indicates 0x2 (4KB/64KB/16KB granule support at
> > > >    stage 2) in the limit, 0x1 (4KB/64KB/16KB granule not supported
> > > >    at stage 2) in val can be allowed, but 0x0 (identified in the
> > > >    ID_AA64MMFR0_EL1.TGRAN4/TGRAN64/TGRAN16) cannot simply be allowed.
> > > >  - For ID_AA64MMFR1_EL1.SPECSEI (FTR_HIGHER_SAFE),
> > > >    it looks like '<' is a safer option (I'm not so sure how it is
> > > >    actually used though).
> > > >
> > > > Currently, I am looking at including those fields in ignore_mask of
> > > > their id_reg_info (for the function ignore the fields) and updating
> > > > its validate function for them (except for ID_AA64DFR0_EL1.PMUVER
> > > > and DEBUGVER, which we can use the function for).
> > >
> > > Yes, in the least I think we need to document the limitations of this
> > > arm64_check_features() function in a comment. I still wonder if we
> > > shouldn't implement a arm64_ftr_compare() which makes appropriate
> > > comparisons, even if it has to key off the register fields to determine
> > > what that comparison should be. Either that, or maybe we shouldn't try
> > > to introduce a common arm64_check_features() at all and we should just
> > > handle each register needed by KVM in its specific way.
> >
> > I understand your point and I completely agree with your comment above.
> > Yes, the function's comparison should be done based on arm64_ftr_bits
> > as long as it is in the file using arm64_ftr_bits.
> > I will fix this in v2.
> >
> >
> > > > > > +                     return -E2BIG;
> > > > > > +
> > > > > > +             exposed_mask |= arm64_ftr_mask(ftrp);
> > > > > > +     }
> > > > > > +
> > > > > > +     if (val & ~exposed_mask)
> > > > > > +             return -E2BIG;
> > > > >
> > > > > I'm not sure we want this. I think it implies that any RAO bits need to be
> > > > > cleared before calling this function, which could be inconvenient.
> > > >
> > > > # Does 'RA0' mean RAZ (read as zero) ?
> > >
> > > RAO (O, not 0) means read-as-one.
> >
> > Uh, that was O, I see...
> > That's why I wasn't able to find the word in ARM Glossary.  Thank you !
> >
> >
> > > > The code is used to check if any feature fields that the host
> > > > kernel doesn't recognize (arm64_ftr_bits is not defined for
> > > > the field) yet are not exposed to the guest.  So, removing the
> > > > checking would be inconvenient for the use case (in the current
> > > > version of the series at least).
> > > >
> > > > On the other hand, I agree it might be inconvenient for some
> > > > potential use cases.  One of the solutions that I could think of
> > > > now would be making the checking conditional by adding another
> > > > parameter (so its caller can choose the behavior).
> > > > I will be happy to make the change when we need it.
> > >
> > > I agree with wanting to avoid bits that shouldn't be set from being set,
> > > but the read-as-one bits will be set, but they won't show up in ftr_bits.
> >
> > I would guess you meant RES1 rather than RAO (or not ?).
> > (I would think RAO is more like for a specific CPU hardware rather
> >  than for CPU architecture specification)
> >
> > For every single register in cpufeature.c that has arm64_ftr_bits,
> > I checked 'Field descriptions' from the page below.
> >   https://developer.arm.com/documentation/ddi0595/2021-03/AArch64-Registers
> >
> > I found one register that has a RES1 field, which is bit 31 of CTR_EL0.
> > (I didn't find any field that is defined as RAO though)
> > The RES1 field is actually defined in ftr_ctr as follows.
> > -------------------------------------------------------------------
> > <arch/arm64/kernel/cpufeature.c>
> >
> > <...>
> > static const struct arm64_ftr_bits ftr_ctr[] = {
> >         ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_EXACT, 31, 1, 1), /* RES1 */
> >         ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE,
> > CTR_DIC_SHIFT, 1, 1),
> >         ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE,
> > CTR_IDC_SHIFT, 1, 1),
> > <...>
> > -------------------------------------------------------------------
> >
> > It looks like RES1 field is okay.
> > Or do you have any specific registers/fields that you are
> > concerned about ?
>
> You're right, I meant the RES1 bits that we set ourselves, not bits HW
> presents as ones. And, as you've confirmed RES1 is covered by
> arm64_ftr_bits, then I apologize for the noise.

No, it wasn't noise.  It was good for me to confirm that.
Thank you so much for the comment.

Regards,
Reiji

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

end of thread, other threads:[~2021-10-19  7:26 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-12  4:35 [RFC PATCH 00/25] KVM: arm64: Make CPU ID registers writable by userspace Reiji Watanabe
2021-10-12  4:35 ` [RFC PATCH 01/25] KVM: arm64: Add has_reset_once flag for vcpu Reiji Watanabe
2021-10-15 10:12   ` Andrew Jones
2021-10-16 19:54     ` Reiji Watanabe
2021-10-12  4:35 ` [RFC PATCH 02/25] KVM: arm64: Save ID registers' sanitized value per vCPU Reiji Watanabe
2021-10-15 13:09   ` Andrew Jones
2021-10-17  0:42     ` Reiji Watanabe
2021-10-18 14:30       ` Andrew Jones
2021-10-18 23:54         ` Reiji Watanabe
2021-10-12  4:35 ` [RFC PATCH 03/25] KVM: arm64: Introduce a validation function for an ID register Reiji Watanabe
2021-10-15 13:30   ` Andrew Jones
     [not found]     ` <CAAeT=Fy-enk=X_PaRSDEKQ01yQzdyU=bcpq8cuCZhtpzC=JvnQ@mail.gmail.com>
     [not found]       ` <20211018144215.fvz7lrqiqlwhadms@gator.home>
     [not found]         ` <CAAeT=FyvRg7cD9-N81BM4gz0FaZHcaoWWQptniB5zDKdL=OkXg@mail.gmail.com>
     [not found]           ` <20211019062516.smjbbil5ugbipwno@gator.home>
2021-10-19  7:26             ` Reiji Watanabe
2021-10-12  4:35 ` [RFC PATCH 04/25] KVM: arm64: Introduce struct id_reg_info Reiji Watanabe
2021-10-15 13:47   ` Andrew Jones
2021-10-17  4:43     ` Reiji Watanabe
2021-10-12  4:35 ` [RFC PATCH 05/25] KVM: arm64: Keep consistency of ID registers between vCPUs Reiji Watanabe
2021-10-12  4:35 ` [RFC PATCH 06/25] KVM: arm64: Make ID_AA64PFR0_EL1 writable Reiji Watanabe
2021-10-12  4:35 ` [RFC PATCH 07/25] KVM: arm64: Make ID_AA64PFR1_EL1 writable Reiji Watanabe
2021-10-12  4:35 ` [RFC PATCH 08/25] KVM: arm64: Make ID_AA64ISAR0_EL1 writable Reiji Watanabe
2021-10-12  4:35 ` [RFC PATCH 09/25] KVM: arm64: Make ID_AA64ISAR1_EL1 writable Reiji Watanabe
2021-10-12  4:35 ` [RFC PATCH 10/25] KVM: arm64: Make ID_AA64DFR0_EL1 writable Reiji Watanabe
2021-10-12  4:35 ` [RFC PATCH 11/25] KVM: arm64: Make ID_DFR0_EL1 writable Reiji Watanabe
2021-10-12  4:35 ` [RFC PATCH 12/25] KVM: arm64: Make MVFR1_EL1 writable Reiji Watanabe
2021-10-12  4:35 ` [RFC PATCH 13/25] KVM: arm64: Make ID registers without id_reg_info writable Reiji Watanabe
2021-10-12  4:35 ` [RFC PATCH 14/25] KVM: arm64: Add consistency checking for frac fields of ID registers Reiji Watanabe
2021-10-12  4:35 ` [RFC PATCH 15/25] KVM: arm64: Introduce KVM_CAP_ARM_ID_REG_WRITABLE capability Reiji Watanabe
2021-10-12  4:35 ` [RFC PATCH 16/25] KVM: arm64: Use vcpu->arch cptr_el2 to track value of cptr_el2 for VHE Reiji Watanabe
2021-10-12  4:35 ` [RFC PATCH 17/25] KVM: arm64: Use vcpu->arch.mdcr_el2 to track value of mdcr_el2 Reiji Watanabe
2021-10-12  4:35 ` [RFC PATCH 18/25] KVM: arm64: Introduce framework to trap disabled features Reiji Watanabe
2021-10-12  4:35 ` [RFC PATCH 19/25] KVM: arm64: Trap disabled features of ID_AA64PFR0_EL1 Reiji Watanabe
2021-10-12  4:35 ` [RFC PATCH 20/25] KVM: arm64: Trap disabled features of ID_AA64PFR1_EL1 Reiji Watanabe
2021-10-12  4:35 ` [RFC PATCH 21/25] KVM: arm64: Trap disabled features of ID_AA64DFR0_EL1 Reiji Watanabe
2021-10-12  4:35 ` [RFC PATCH 22/25] KVM: arm64: Trap disabled features of ID_AA64MMFR1_EL1 Reiji Watanabe
2021-10-12  4:35 ` [RFC PATCH 23/25] KVM: arm64: Trap disabled features of ID_AA64ISAR1_EL1 Reiji Watanabe
2021-10-12  4:35 ` [RFC PATCH 24/25] KVM: arm64: Activate trapping of disabled CPU features for the guest Reiji Watanabe
2021-10-12  4:35 ` [RFC PATCH 25/25] KVM: arm64: selftests: Introduce id_reg_test Reiji Watanabe

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).