Re: [PATCH v9 04/16] KVM: arm64: selftests: Test SET_NR_COUNTERS after first run

On 2026/07/30 0:23, Fuad Tabba wrote:
> On Mon, 20 Jul 2026 at 07:09, <[email protected]> wrote:
>>
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>> - [Medium] The test ignores the guest's exit state and `ucall` status after executing `vcpu_run()`.
> 
> vcpu_run() does the check.

vcpu_run() itself doesn't. run_vcpu() calls vcpu_run() and then performs 
the check (their names are so confusing here), but the new test bypasses 
run_vcpu() and therefore does not perform the check. I'm planning to 
rework the test to use run_vcpu() with the next version.

Regards,
Akihiko Odaki

> 
> /fuad
> 
>> --
>>
>> --- Patch [4]: [PATCH v9 04/16] KVM: arm64: selftests: Test SET_NR_COUNTERS after first run ---
>> commit 0382d3a32df9349f988ca69c7b11c92e61c3d384
>> Author: Akihiko Odaki <[email protected]>
>>
>> KVM: arm64: selftests: Test SET_NR_COUNTERS after first run
>>
>> This commit adds a regression test that runs one vCPU and then tries to change
>> the VM-scoped PMU counter count through an idle sibling to verify that
>> SET_NR_COUNTERS fails with EBUSY once any vCPU has run.
>>
>>> diff --git a/tools/testing/selftests/kvm/arm64/vpmu_counter_access.c b/tools/testing/selftests/kvm/arm64/vpmu_counter_access.c
>>> index 22223395969e0..3b5bb421f401b 100644
>>> --- a/tools/testing/selftests/kvm/arm64/vpmu_counter_access.c
>>> +++ b/tools/testing/selftests/kvm/arm64/vpmu_counter_access.c
>>
>> [ ... ]
>>
>>> @@ -622,6 +627,37 @@ static bool kvm_supports_nr_counters_attr(void)
>>>        return supported;
>>>   }
>>>
>>> +static void test_set_nr_counters_after_vcpu_run(void)
>>> +{
>>> +     struct kvm_vcpu *running_vcpu, *stopped_vcpu;
>>> +     unsigned int nr_counters = 0;
>>> +     struct kvm_vcpu_init init;
>>> +     struct kvm_vm *vm;
>>> +     int ret;
>>> +     u64 irq = 23;
>>> +
>>> +     vm = vm_create(2);
>>> +     vm_ioctl(vm, KVM_ARM_PREFERRED_TARGET, &init);
>>> +     init.features[0] |= BIT(KVM_ARM_VCPU_PMU_V3);
>>> +     running_vcpu = aarch64_vcpu_add(vm, 0, &init, guest_code_done);
>>> +     stopped_vcpu = aarch64_vcpu_add(vm, 1, &init, guest_code_done);
>>> +     kvm_arch_vm_finalize_vcpus(vm);
>>> +
>>> +     vcpu_device_attr_set(running_vcpu, KVM_ARM_VCPU_PMU_V3_CTRL,
>>> +                          KVM_ARM_VCPU_PMU_V3_IRQ, &irq);
>>> +     vcpu_device_attr_set(running_vcpu, KVM_ARM_VCPU_PMU_V3_CTRL,
>>> +                          KVM_ARM_VCPU_PMU_V3_INIT, NULL);
>>> +     vcpu_run(running_vcpu);
>>
>> [Severity: Medium]
>> Does this code need to check the guest's exit state and ucall payload?
>>
>> If the guest payload crashes or hits an unexpected exception, this will silently
>> ignore the failure and proceed. Since the test only checks for an EBUSY error on
>> the subsequent ioctl (which will still be returned because the vCPU has been
>> run), the test will pass despite the guest crash, potentially masking
>> virtualization bugs.
>>
>>> +
>>> +     ret = __vcpu_device_attr_set(stopped_vcpu, KVM_ARM_VCPU_PMU_V3_CTRL,
>>> +                                  KVM_ARM_VCPU_PMU_V3_SET_NR_COUNTERS,
>>> +                                  &nr_counters);
>>> +     TEST_ASSERT(ret == -1 && errno == EBUSY,
>>> +                 KVM_IOCTL_ERROR(KVM_SET_DEVICE_ATTR, ret));
>>> +
>>> +     kvm_vm_free(vm);
>>> +}
>>> +
>>
>> --
>> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
>>