> On Sep 12, 2025, at 21:11, Marc Zyngier <[email protected]> wrote:
> 
> On Fri, 12 Sep 2025 12:33:39 +0100,
> Itaru Kitayama <[email protected]> wrote:
>> 
>> 
>> 
>>> On Sep 12, 2025, at 20:01, Marc Zyngier <[email protected]> wrote:
>>> 
>>> On Fri, 12 Sep 2025 09:27:40 +0100,
>>> Itaru Kitayama <[email protected]> wrote:
>>>> 
>>>> Signed-off-by: Itaru Kitayama <[email protected]>
>>> 
>>> 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 build error I’ve seen with Ubuntu 22.04 LTS is below: 

gcc -D_GNU_SOURCE=  
-I/home/itaru/projects/linux/tools/testing/selftests/cgroup/lib/include -DDEBUG 
-Wall -Wstrict-prototypes -Wuninitialized -O0 -g -std=gnu99 
-Wno-gnu-variable-sized-type-not-at-end -MD -MP -DCONFIG_64BIT 
-fno-builtin-memcmp -fno-builtin-memcpy -fno-builtin-memset 
-fno-builtin-strnlen -fno-stack-protector -fno-PIE -fno-strict-aliasing 
-I/home/itaru/projects/linux/tools/testing/selftests/../../../tools/include 
-I/home/itaru/projects/linux/tools/testing/selftests/../../../tools/arch/arm64/include
 -I/home/itaru/projects/linux/tools/testing/selftests/../../../usr/include/ 
-Iinclude -Iarm64 -Iinclude/arm64 -I ../rseq -I..  -isystem 
/home/itaru/projects/linux/tools/testing/selftests/../../../usr/include 
-I/home/itaru/projects/linux/tools/testing/selftests/../../../tools/arch/arm64/include/generated/
   -c arm64/vpmu_counter_access.c -o 
/home/itaru/projects/linux/tools/testing/selftests/kvm/arm64/vpmu_counter_access.o
In file included from 
/home/itaru/projects/linux/tools/testing/selftests/../../../tools/arch/arm64/include/asm/sysreg.h:1098,
                 from 
/home/itaru/projects/linux/tools/testing/selftests/../../../tools/arch/arm64/include/asm/esr.h:10,
                 from include/arm64/processor.h:16,
                 from arm64/vpmu_counter_access.c:16:
In function ‘field_multiplier’,
    inlined from ‘field_mask’ at 
/home/itaru/projects/linux/tools/testing/selftests/../../../tools/include/linux/bitfield.h:141:17,
    inlined from ‘u64_encode_bits’ at 
/home/itaru/projects/linux/tools/testing/selftests/../../../tools/include/linux/bitfield.h:172:1,
    inlined from ‘u64p_replace_bits’ at 
/home/itaru/projects/linux/tools/testing/selftests/../../../tools/include/linux/bitfield.h:172:1,
    inlined from ‘set_pmcr_n’ at arm64/vpmu_counter_access.c:50:2:
/home/itaru/projects/linux/tools/testing/selftests/../../../tools/include/linux/bitfield.h:136:17:
 error: call to ‘__bad_mask’ declared with attribute error: bad bitfield mask
  136 |                 __bad_mask();
      |                 ^~~~~~~~~~~~
In function ‘field_multiplier’,
    inlined from ‘u64_encode_bits’ at 
/home/itaru/projects/linux/tools/testing/selftests/../../../tools/include/linux/bitfield.h:172:1,
    inlined from ‘u64p_replace_bits’ at 
/home/itaru/projects/linux/tools/testing/selftests/../../../tools/include/linux/bitfield.h:172:1,
    inlined from ‘set_pmcr_n’ at arm64/vpmu_counter_access.c:50:2:
/home/itaru/projects/linux/tools/testing/selftests/../../../tools/include/linux/bitfield.h:136:17:
 error: call to ‘__bad_mask’ declared with attribute error: bad bitfield mask
  136 |                 __bad_mask();
      |                 ^~~~~~~~~~~~
arm64/vpmu_counter_access.c: At top level:
cc1: note: unrecognized command-line option 
‘-Wno-gnu-variable-sized-type-not-at-end’ may have been intended to silence 
earlier diagnostics
make: *** [Makefile.kvm:303: 
/home/itaru/projects/linux/tools/testing/selftests/kvm/arm64/vpmu_counter_access.o]
 Error 1

Thanks,
Itaru.

> M.
> 
> -- 
> Without deviation from the norm, progress is not possible.



Reply via email to