[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <5C133488-95DD-4B1E-98C9-A9B13B6AF8F2@linux.dev>
Date: Thu, 18 Sep 2025 13:59:36 +0900
From: Itaru Kitayama <itaru.kitayama@...ux.dev>
To: Marc Zyngier <maz@...nel.org>
Cc: 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 Sep 18, 2025, at 3:44, Marc Zyngier <maz@...nel.org> wrote:
>
> On Mon, 15 Sep 2025 22:31:31 +0100,
> Itaru Kitayama <itaru.kitayama@...ux.dev> wrote:
>>
>>
>>
>>> On Sep 12, 2025, at 21:11, Marc Zyngier <maz@...nel.org> wrote:
>>>
>>> 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.
>>>
>>
>> You are right, while the bit fields are write ignored, to be
>> consistent with the handling of other bit fields of the register,
>> I’m fully convinced that checking the write operation in the
>> vpmu_counter_access.c file should be kept.
>
> The bit field is *not* ignored when written from userspace. That's how
> we configure the PMU if the guest runs at EL1.
>
>> The build error I’ve seen with Ubuntu 22.04 LTS is below:
>
> [snip]
>
> Can you please detail what compiler version this is? I'm unlikely to
> install an ancient version of Ubuntu, but I can pick the corresponding
> compiler version.
>
Sure, here it is:
[itaru@vm4 ~]$ apt-cache show gcc
Package: gcc
Architecture: arm64
Version: 4:11.2.0-1ubuntu1
Priority: optional
Section: devel
Source: gcc-defaults (1.193ubuntu1)
Origin: Ubuntu
Maintainer: Ubuntu Developers <ubuntu-devel-discuss@...ts.ubuntu.com>
Original-Maintainer: Debian GCC Maintainers <debian-gcc@...ts.debian.org>
Bugs: https://bugs.launchpad.net/ubuntu/+filebug
Installed-Size: 50
Provides: c-compiler, gcc-aarch64-linux-gnu (= 4:11.2.0-1ubuntu1)
Depends: cpp (= 4:11.2.0-1ubuntu1), gcc-11 (>= 11.2.0-1~)
Recommends: libc6-dev | libc-dev
Suggests: gcc-multilib, make, manpages-dev, autoconf, automake, libtool, flex, bison, gdb, gcc-doc
Conflicts: gcc-doc (<< 1:2.95.3)
Filename: pool/main/g/gcc-defaults/gcc_11.2.0-1ubuntu1_arm64.deb
Size: 5128
MD5sum: 2ec2c9fcec3deb45052f307f85f37f7f
SHA1: 19491fe4fb89a6cbd389f573bf3abf46e841f3d6
SHA256: 426deed543cc32f388ad8336354e312fb76a47450194d2d775d855ad5878f82e
SHA512: c7e16c35037b0987458bf51a341045cdc1ac8da5bada4eb1c43922a83c5e18203beca8df9937b95de6c4185605f6e9054c92858d2842de0f6859cd21c0b33f01
Description-en: GNU C compiler
This is the GNU C compiler, a fairly portable optimizing compiler for C.
.
This is a dependency package providing the default GNU C compiler.
Description-md5: c7efd71c7c651a9ac8b2adf36b137790
Task: ubuntustudio-video, ubuntustudio-publishing, ubuntu-mate-core, ubuntu-mate-desktop
Build-Essential: yes
Thanks,
Itaru.
> M.
>
> --
> Without deviation from the norm, progress is not possible.
Powered by blists - more mailing lists