From: Reiji Watanabe <[email protected]>
KVM does not yet support userspace modifying PMCR_EL0.N (With
the previous patch, KVM ignores what is written by userspace).
Add support userspace limiting PMCR_EL0.N.
Disallow userspace to set PMCR_EL0.N to a value that is greater
than the host value as KVM doesn't support more event counters
than what the host HW implements. Also, make this register
immutable after the VM has started running. To maintain the
existing expectations, instead of returning an error, KVM
returns a success for these two cases.
Finally, ignore writes to read-only bits that are cleared on
vCPU reset, and RES{0,1} bits (including writable bits that
KVM doesn't support yet), as those bits shouldn't be modified
(at least with the current KVM).
Signed-off-by: Reiji Watanabe <[email protected]>
Signed-off-by: Raghavendra Rao Ananta <[email protected]>
---
arch/arm64/kvm/sys_regs.c | 58 +++++++++++++++++++++++++++++++++++++--
1 file changed, 56 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index d1db1f292645e..a0efc71dbee1e 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -16,6 +16,7 @@
#include <linux/mm.h>
#include <linux/printk.h>
#include <linux/uaccess.h>
+#include <linux/perf/arm_pmu.h>
#include <asm/cacheflush.h>
#include <asm/cputype.h>
@@ -1087,6 +1088,59 @@ static int get_pmcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r,
return 0;
}
+static int set_pmcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r,
+ u64 val)
+{
+ struct kvm *kvm = vcpu->kvm;
+ u64 new_n, mutable_mask;
+
+ mutex_lock(&kvm->arch.config_lock);
+
+ /*
+ * Make PMCR immutable once the VM has started running, but do
+ * not return an error (-EBUSY) to meet the existing expectations.
+ */
+ if (kvm_vm_has_ran_once(vcpu->kvm)) {
+ mutex_unlock(&kvm->arch.config_lock);
+ return 0;
+ }
+
+ new_n = (val >> ARMV8_PMU_PMCR_N_SHIFT) & ARMV8_PMU_PMCR_N_MASK;
+ if (new_n != kvm->arch.pmcr_n) {
+ u8 pmcr_n_limit = kvm->arch.arm_pmu->num_events - 1;
+
+ /*
+ * The vCPU can't have more counters than the PMU hardware
+ * implements. Ignore this error to maintain compatibility
+ * with the existing KVM behavior.
+ */
+ if (new_n <= pmcr_n_limit)
+ kvm->arch.pmcr_n = new_n;
+ }
+ mutex_unlock(&kvm->arch.config_lock);
+
+ /*
+ * Ignore writes to RES0 bits, read only bits that are cleared on
+ * vCPU reset, and writable bits that KVM doesn't support yet.
+ * (i.e. only PMCR.N and bits [7:0] are mutable from userspace)
+ * The LP bit is RES0 when FEAT_PMUv3p5 is not supported on the vCPU.
+ * But, we leave the bit as it is here, as the vCPU's PMUver might
+ * be changed later (NOTE: the bit will be cleared on first vCPU run
+ * if necessary).
+ */
+ mutable_mask = (ARMV8_PMU_PMCR_MASK |
+ (ARMV8_PMU_PMCR_N_MASK << ARMV8_PMU_PMCR_N_SHIFT));
+ val &= mutable_mask;
+ val |= (__vcpu_sys_reg(vcpu, r->reg) & ~mutable_mask);
+
+ /* The LC bit is RES1 when AArch32 is not supported */
+ if (!kvm_supports_32bit_el0())
+ val |= ARMV8_PMU_PMCR_LC;
+
+ __vcpu_sys_reg(vcpu, r->reg) = val;
+ return 0;
+}
+
/* Silly macro to expand the DBG{BCR,BVR,WVR,WCR}n_EL1 registers in one go */
#define DBG_BCR_BVR_WCR_WVR_EL1(n) \
{ SYS_DESC(SYS_DBGBVRn_EL1(n)), \
@@ -2150,8 +2204,8 @@ static const struct sys_reg_desc sys_reg_descs[] = {
{ SYS_DESC(SYS_CTR_EL0), access_ctr },
{ SYS_DESC(SYS_SVCR), undef_access },
- { PMU_SYS_REG(PMCR_EL0), .access = access_pmcr,
- .reset = reset_pmcr, .reg = PMCR_EL0, .get_user = get_pmcr },
+ { PMU_SYS_REG(PMCR_EL0), .access = access_pmcr, .reset = reset_pmcr,
+ .reg = PMCR_EL0, .get_user = get_pmcr, .set_user = set_pmcr },
{ PMU_SYS_REG(PMCNTENSET_EL0),
.access = access_pmcnten, .reg = PMCNTENSET_EL0 },
{ PMU_SYS_REG(PMCNTENCLR_EL0),
--
2.42.0.582.g8ccd20d70d-goog
Hi Raghavendra,
kernel test robot noticed the following build errors:
[auto build test ERROR on 6465e260f48790807eef06b583b38ca9789b6072]
url: https://github.com/intel-lab-lkp/linux/commits/Raghavendra-Rao-Ananta/KVM-arm64-PMU-Introduce-helpers-to-set-the-guest-s-PMU/20230927-095821
base: 6465e260f48790807eef06b583b38ca9789b6072
patch link: https://lore.kernel.org/r/20230926234008.2348607-8-rananta%40google.com
patch subject: [PATCH v6 07/11] KVM: arm64: PMU: Allow userspace to limit PMCR_EL0.N for the guest
config: arm64-randconfig-003-20230928 (https://download.01.org/0day-ci/archive/20230929/[email protected]/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230929/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
arch/arm64/kvm/sys_regs.c: In function 'set_pmcr':
>> arch/arm64/kvm/sys_regs.c:1110:52: error: invalid use of undefined type 'struct arm_pmu'
1110 | u8 pmcr_n_limit = kvm->arch.arm_pmu->num_events - 1;
| ^~
arch/arm64/kvm/sys_regs.c: At top level:
arch/arm64/kvm/sys_regs.c:2207:66: warning: initialized field overwritten [-Woverride-init]
2207 | { PMU_SYS_REG(PMCR_EL0), .access = access_pmcr, .reset = reset_pmcr,
| ^~~~~~~~~~
arch/arm64/kvm/sys_regs.c:2207:66: note: (near initialization for 'sys_reg_descs[233].reset')
In file included from include/uapi/linux/posix_types.h:5,
from include/uapi/linux/types.h:14,
from include/linux/types.h:6,
from include/linux/kasan-checks.h:5,
from include/asm-generic/rwonce.h:26,
from arch/arm64/include/asm/rwonce.h:71,
from include/linux/compiler.h:246,
from include/linux/build_bug.h:5,
from include/linux/bitfield.h:10,
from arch/arm64/kvm/sys_regs.c:12:
include/linux/stddef.h:8:14: warning: initialized field overwritten [-Woverride-init]
8 | #define NULL ((void *)0)
| ^
arch/arm64/kvm/sys_regs.c:2221:46: note: in expansion of macro 'NULL'
2221 | .access = access_pmswinc, .reset = NULL },
| ^~~~
include/linux/stddef.h:8:14: note: (near initialization for 'sys_reg_descs[237].reset')
8 | #define NULL ((void *)0)
| ^
arch/arm64/kvm/sys_regs.c:2221:46: note: in expansion of macro 'NULL'
2221 | .access = access_pmswinc, .reset = NULL },
| ^~~~
arch/arm64/kvm/sys_regs.c:2223:45: warning: initialized field overwritten [-Woverride-init]
2223 | .access = access_pmselr, .reset = reset_pmselr, .reg = PMSELR_EL0 },
| ^~~~~~~~~~~~
arch/arm64/kvm/sys_regs.c:2223:45: note: (near initialization for 'sys_reg_descs[238].reset')
include/linux/stddef.h:8:14: warning: initialized field overwritten [-Woverride-init]
8 | #define NULL ((void *)0)
| ^
arch/arm64/kvm/sys_regs.c:2225:45: note: in expansion of macro 'NULL'
2225 | .access = access_pmceid, .reset = NULL },
| ^~~~
include/linux/stddef.h:8:14: note: (near initialization for 'sys_reg_descs[239].reset')
8 | #define NULL ((void *)0)
| ^
arch/arm64/kvm/sys_regs.c:2225:45: note: in expansion of macro 'NULL'
2225 | .access = access_pmceid, .reset = NULL },
| ^~~~
include/linux/stddef.h:8:14: warning: initialized field overwritten [-Woverride-init]
8 | #define NULL ((void *)0)
| ^
arch/arm64/kvm/sys_regs.c:2227:45: note: in expansion of macro 'NULL'
2227 | .access = access_pmceid, .reset = NULL },
| ^~~~
include/linux/stddef.h:8:14: note: (near initialization for 'sys_reg_descs[240].reset')
8 | #define NULL ((void *)0)
| ^
arch/arm64/kvm/sys_regs.c:2227:45: note: in expansion of macro 'NULL'
2227 | .access = access_pmceid, .reset = NULL },
| ^~~~
arch/arm64/kvm/sys_regs.c:2229:49: warning: initialized field overwritten [-Woverride-init]
2229 | .access = access_pmu_evcntr, .reset = reset_unknown,
| ^~~~~~~~~~~~~
arch/arm64/kvm/sys_regs.c:2229:49: note: (near initialization for 'sys_reg_descs[241].reset')
include/linux/stddef.h:8:14: warning: initialized field overwritten [-Woverride-init]
8 | #define NULL ((void *)0)
| ^
arch/arm64/kvm/sys_regs.c:2232:50: note: in expansion of macro 'NULL'
2232 | .access = access_pmu_evtyper, .reset = NULL },
| ^~~~
include/linux/stddef.h:8:14: note: (near initialization for 'sys_reg_descs[242].reset')
8 | #define NULL ((void *)0)
| ^
arch/arm64/kvm/sys_regs.c:2232:50: note: in expansion of macro 'NULL'
2232 | .access = access_pmu_evtyper, .reset = NULL },
| ^~~~
include/linux/stddef.h:8:14: warning: initialized field overwritten [-Woverride-init]
8 | #define NULL ((void *)0)
| ^
arch/arm64/kvm/sys_regs.c:2234:49: note: in expansion of macro 'NULL'
2234 | .access = access_pmu_evcntr, .reset = NULL },
| ^~~~
include/linux/stddef.h:8:14: note: (near initialization for 'sys_reg_descs[243].reset')
8 | #define NULL ((void *)0)
| ^
arch/arm64/kvm/sys_regs.c:2234:49: note: in expansion of macro 'NULL'
2234 | .access = access_pmu_evcntr, .reset = NULL },
| ^~~~
arch/arm64/kvm/sys_regs.c:1162:20: warning: initialized field overwritten [-Woverride-init]
1162 | .reset = reset_pmevcntr, .get_user = get_pmu_evcntr, \
| ^~~~~~~~~~~~~~
arch/arm64/kvm/sys_regs.c:2330:9: note: in expansion of macro 'PMU_PMEVCNTR_EL0'
2330 | PMU_PMEVCNTR_EL0(0),
| ^~~~~~~~~~~~~~~~
arch/arm64/kvm/sys_regs.c:1162:20: note: (near initialization for 'sys_reg_descs[327].reset')
1162 | .reset = reset_pmevcntr, .get_user = get_pmu_evcntr, \
| ^~~~~~~~~~~~~~
arch/arm64/kvm/sys_regs.c:2330:9: note: in expansion of macro 'PMU_PMEVCNTR_EL0'
2330 | PMU_PMEVCNTR_EL0(0),
| ^~~~~~~~~~~~~~~~
arch/arm64/kvm/sys_regs.c:1162:20: warning: initialized field overwritten [-Woverride-init]
1162 | .reset = reset_pmevcntr, .get_user = get_pmu_evcntr, \
| ^~~~~~~~~~~~~~
vim +1110 arch/arm64/kvm/sys_regs.c
1090
1091 static int set_pmcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r,
1092 u64 val)
1093 {
1094 struct kvm *kvm = vcpu->kvm;
1095 u64 new_n, mutable_mask;
1096
1097 mutex_lock(&kvm->arch.config_lock);
1098
1099 /*
1100 * Make PMCR immutable once the VM has started running, but do
1101 * not return an error (-EBUSY) to meet the existing expectations.
1102 */
1103 if (kvm_vm_has_ran_once(vcpu->kvm)) {
1104 mutex_unlock(&kvm->arch.config_lock);
1105 return 0;
1106 }
1107
1108 new_n = (val >> ARMV8_PMU_PMCR_N_SHIFT) & ARMV8_PMU_PMCR_N_MASK;
1109 if (new_n != kvm->arch.pmcr_n) {
> 1110 u8 pmcr_n_limit = kvm->arch.arm_pmu->num_events - 1;
1111
1112 /*
1113 * The vCPU can't have more counters than the PMU hardware
1114 * implements. Ignore this error to maintain compatibility
1115 * with the existing KVM behavior.
1116 */
1117 if (new_n <= pmcr_n_limit)
1118 kvm->arch.pmcr_n = new_n;
1119 }
1120 mutex_unlock(&kvm->arch.config_lock);
1121
1122 /*
1123 * Ignore writes to RES0 bits, read only bits that are cleared on
1124 * vCPU reset, and writable bits that KVM doesn't support yet.
1125 * (i.e. only PMCR.N and bits [7:0] are mutable from userspace)
1126 * The LP bit is RES0 when FEAT_PMUv3p5 is not supported on the vCPU.
1127 * But, we leave the bit as it is here, as the vCPU's PMUver might
1128 * be changed later (NOTE: the bit will be cleared on first vCPU run
1129 * if necessary).
1130 */
1131 mutable_mask = (ARMV8_PMU_PMCR_MASK |
1132 (ARMV8_PMU_PMCR_N_MASK << ARMV8_PMU_PMCR_N_SHIFT));
1133 val &= mutable_mask;
1134 val |= (__vcpu_sys_reg(vcpu, r->reg) & ~mutable_mask);
1135
1136 /* The LC bit is RES1 when AArch32 is not supported */
1137 if (!kvm_supports_32bit_el0())
1138 val |= ARMV8_PMU_PMCR_LC;
1139
1140 __vcpu_sys_reg(vcpu, r->reg) = val;
1141 return 0;
1142 }
1143
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki