[PATCH] PMCR_EL0.N is RAZ/WI. At least a build failes in Ubuntu 22.04 LTS. Remove the set function.
Signed-off-by: Itaru Kitayama itaru.kitayama@fujitsu.com --- Seen a build failure with old Ubuntu 22.04 LTS, while the latest release has no build issue, a write to the bit fields is RAZ/WI, remove the function. --- tools/testing/selftests/kvm/arm64/vpmu_counter_access.c | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/tools/testing/selftests/kvm/arm64/vpmu_counter_access.c b/tools/testing/selftests/kvm/arm64/vpmu_counter_access.c index f16b3b27e32ed7ca57481f27d689d47783aa0345..56214a4430be90b3e1d840f2719b22dd44f0b49b 100644 --- a/tools/testing/selftests/kvm/arm64/vpmu_counter_access.c +++ b/tools/testing/selftests/kvm/arm64/vpmu_counter_access.c @@ -45,11 +45,6 @@ static uint64_t get_pmcr_n(uint64_t pmcr) return FIELD_GET(ARMV8_PMU_PMCR_N, pmcr); }
-static void set_pmcr_n(uint64_t *pmcr, uint64_t pmcr_n) -{ - u64p_replace_bits((__u64 *) pmcr, pmcr_n, ARMV8_PMU_PMCR_N); -} - static uint64_t get_counters_mask(uint64_t n) { uint64_t mask = BIT(ARMV8_PMU_CYCLE_IDX); @@ -490,7 +485,6 @@ static void test_create_vpmu_vm_with_pmcr_n(uint64_t pmcr_n, bool expect_fail) * Setting a larger value of PMCR.N should not modify the field, and * return a success. */ - set_pmcr_n(&pmcr, pmcr_n); vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_PMCR_EL0), pmcr); pmcr = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_PMCR_EL0));
--- base-commit: aae5a9834b388860844b294c70c8770dd26e528c change-id: 20250912-selftest-fix3-27f285e79d82
Best regards,
On Fri, 12 Sep 2025 09:27:40 +0100, Itaru Kitayama itaru.kitayama@linux.dev wrote:
Signed-off-by: Itaru Kitayama itaru.kitayama@fujitsu.com
This isn't an acceptable commit message.
Seen a build failure with old Ubuntu 22.04 LTS, while the latest release has no build issue, a write to the bit fields is RAZ/WI, remove the function.
tools/testing/selftests/kvm/arm64/vpmu_counter_access.c | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/tools/testing/selftests/kvm/arm64/vpmu_counter_access.c b/tools/testing/selftests/kvm/arm64/vpmu_counter_access.c index f16b3b27e32ed7ca57481f27d689d47783aa0345..56214a4430be90b3e1d840f2719b22dd44f0b49b 100644 --- a/tools/testing/selftests/kvm/arm64/vpmu_counter_access.c +++ b/tools/testing/selftests/kvm/arm64/vpmu_counter_access.c @@ -45,11 +45,6 @@ static uint64_t get_pmcr_n(uint64_t pmcr) return FIELD_GET(ARMV8_PMU_PMCR_N, pmcr); } -static void set_pmcr_n(uint64_t *pmcr, uint64_t pmcr_n) -{
- u64p_replace_bits((__u64 *) pmcr, pmcr_n, ARMV8_PMU_PMCR_N);
-}
static uint64_t get_counters_mask(uint64_t n) { uint64_t mask = BIT(ARMV8_PMU_CYCLE_IDX); @@ -490,7 +485,6 @@ static void test_create_vpmu_vm_with_pmcr_n(uint64_t pmcr_n, bool expect_fail) * Setting a larger value of PMCR.N should not modify the field, and * return a success. */
- set_pmcr_n(&pmcr, pmcr_n); vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_PMCR_EL0), pmcr); pmcr = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_PMCR_EL0));
So what are you fixing here? A build failure? A semantic defect? Something else? What makes this a valid change?
Frankly, I have no idea.
But KVM definitely allows PMCR_EL0.N to be written from userspace, and that's not going to change.
M.
On Sep 12, 2025, at 20:01, Marc Zyngier maz@kernel.org wrote:
On Fri, 12 Sep 2025 09:27:40 +0100, Itaru Kitayama itaru.kitayama@linux.dev wrote:
Signed-off-by: Itaru Kitayama itaru.kitayama@fujitsu.com
This isn't an acceptable commit message.
Seen a build failure with old Ubuntu 22.04 LTS, while the latest release has no build issue, a write to the bit fields is RAZ/WI, remove the function.
tools/testing/selftests/kvm/arm64/vpmu_counter_access.c | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/tools/testing/selftests/kvm/arm64/vpmu_counter_access.c b/tools/testing/selftests/kvm/arm64/vpmu_counter_access.c index f16b3b27e32ed7ca57481f27d689d47783aa0345..56214a4430be90b3e1d840f2719b22dd44f0b49b 100644 --- a/tools/testing/selftests/kvm/arm64/vpmu_counter_access.c +++ b/tools/testing/selftests/kvm/arm64/vpmu_counter_access.c @@ -45,11 +45,6 @@ static uint64_t get_pmcr_n(uint64_t pmcr) return FIELD_GET(ARMV8_PMU_PMCR_N, pmcr); }
-static void set_pmcr_n(uint64_t *pmcr, uint64_t pmcr_n) -{
- u64p_replace_bits((__u64 *) pmcr, pmcr_n, ARMV8_PMU_PMCR_N);
-}
static uint64_t get_counters_mask(uint64_t n) { uint64_t mask = BIT(ARMV8_PMU_CYCLE_IDX); @@ -490,7 +485,6 @@ static void test_create_vpmu_vm_with_pmcr_n(uint64_t pmcr_n, bool expect_fail) * Setting a larger value of PMCR.N should not modify the field, and * return a success. */
- set_pmcr_n(&pmcr, pmcr_n); vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_PMCR_EL0), pmcr); pmcr = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_PMCR_EL0));
So what are you fixing here? A build failure? A semantic defect? Something else? What makes this a valid change?
Frankly, I have no idea.
But KVM definitely allows PMCR_EL0.N to be written from userspace, and that's not going to change.
Then I’ll drop this patch.
Thanks, Itaru.
M.
-- Without deviation from the norm, progress is not possible.
On Fri, 12 Sep 2025 12:33:39 +0100, Itaru Kitayama itaru.kitayama@gmail.com wrote:
On Sep 12, 2025, at 20:01, Marc Zyngier maz@kernel.org wrote:
On Fri, 12 Sep 2025 09:27:40 +0100, Itaru Kitayama itaru.kitayama@linux.dev wrote:
Signed-off-by: Itaru Kitayama itaru.kitayama@fujitsu.com
This isn't an acceptable commit message.
Seen a build failure with old Ubuntu 22.04 LTS, while the latest release has no build issue, a write to the bit fields is RAZ/WI, remove the function.
tools/testing/selftests/kvm/arm64/vpmu_counter_access.c | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/tools/testing/selftests/kvm/arm64/vpmu_counter_access.c b/tools/testing/selftests/kvm/arm64/vpmu_counter_access.c index f16b3b27e32ed7ca57481f27d689d47783aa0345..56214a4430be90b3e1d840f2719b22dd44f0b49b 100644 --- a/tools/testing/selftests/kvm/arm64/vpmu_counter_access.c +++ b/tools/testing/selftests/kvm/arm64/vpmu_counter_access.c @@ -45,11 +45,6 @@ static uint64_t get_pmcr_n(uint64_t pmcr) return FIELD_GET(ARMV8_PMU_PMCR_N, pmcr); }
-static void set_pmcr_n(uint64_t *pmcr, uint64_t pmcr_n) -{
- u64p_replace_bits((__u64 *) pmcr, pmcr_n, ARMV8_PMU_PMCR_N);
-}
static uint64_t get_counters_mask(uint64_t n) { uint64_t mask = BIT(ARMV8_PMU_CYCLE_IDX); @@ -490,7 +485,6 @@ static void test_create_vpmu_vm_with_pmcr_n(uint64_t pmcr_n, bool expect_fail) * Setting a larger value of PMCR.N should not modify the field, and * return a success. */
- set_pmcr_n(&pmcr, pmcr_n); vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_PMCR_EL0), pmcr); pmcr = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_PMCR_EL0));
So what are you fixing here? A build failure? A semantic defect? Something else? What makes this a valid change?
Frankly, I have no idea.
But KVM definitely allows PMCR_EL0.N to be written from userspace, and that's not going to change.
Then I’ll drop this patch.
I'm not asking you to drop it, I'm asking you to explain. If you found a problem, let's discuss it and fix it. But as it stands, you're not giving me much to go on.
M.
linux-kselftest-mirror@lists.linaro.org
-
Itaru Kitayama -
Itaru Kitayama -
Marc Zyngier