[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <86348rdg5o.wl-maz@kernel.org>
Date: Fri, 12 Sep 2025 13:11:31 +0100
From: Marc Zyngier <maz@...nel.org>
To: Itaru Kitayama <itaru.kitayama@...il.com>
Cc: Itaru Kitayama <itaru.kitayama@...ux.dev>,
Oliver Upton <oliver.upton@...ux.dev>,
Joey Gouly <joey.gouly@....com>,
K Poulose Suzuki <suzuki.poulose@....com>,
Zenghui Yu <yuzenghui@...wei.com>,
Paolo Bonzini <pbonzini@...hat.com>,
Shuah Khan <shuah@...nel.org>,
linux-arm-kernel@...ts.infradead.org,
kvmarm@...ts.linux.dev,
kvm@...r.kernel.org,
linux-kselftest@...r.kernel.org,
linux-kernel@...r.kernel.org,
Itaru Kitayama <itaru.kitayama@...itsu.com>
Subject: Re: [PATCH] PMCR_EL0.N is RAZ/WI. At least a build failes in Ubuntu 22.04 LTS. Remove the set function.
On Fri, 12 Sep 2025 12:33:39 +0100,
Itaru Kitayama <itaru.kitayama@...il.com> wrote:
>
>
>
> > On Sep 12, 2025, at 20:01, Marc Zyngier <maz@...nel.org> wrote:
> >
> > On Fri, 12 Sep 2025 09:27:40 +0100,
> > Itaru Kitayama <itaru.kitayama@...ux.dev> wrote:
> >>
> >> Signed-off-by: Itaru Kitayama <itaru.kitayama@...itsu.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.
--
Without deviation from the norm, progress is not possible.
Powered by blists - more mailing lists