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

Fuad Tabba <[email protected]> Wed, 29 Jul 2026 16:23:13 +0100
Newsgroups dev.linux.lists.sashiko-reviews,dev.linux.lists.kvmarm,org.kernel.vger.kvm
Message-ID <CA+EHjTxsm6LorPDR9irVHL0gtmsUG6zUKQLBA2=8Ruy+V4UmBw@mail.gmail.com>
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.

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