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

On Thu, 30 Jul 2026 at 06:44, Akihiko Odaki
<[email protected]> wrote:
>
> 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.

So Sashiko was right :) Fixing this would be good, thanks for pointing it out.

Cheers,
/fuad

>
> 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
> >>
>