Hi Vaibhav, Thanks for the patch. Please find my comments inline.
On 2026/06/04 02:59 PM, Vaibhav Jain wrote: > The guest state buffer (GSB) test suite currently fails on systems that > do not support the PAPR APIv2 nested virtualization. This happens because > the tests attempt to use APIv2-specific functionality without first > checking if the host supports it. This was recently reported [1] when > test-guest-state-buffer kunit tests were being run on Qemu without enabling > Qemu capability 'cap-nested-papr' which enabled APIv2 nested virtualization > for PPC64 Pseries Qemu machine. > > Add a suite_init callback that checks for APIv2 support by calling > plpar_guest_get_capabilities(). If the host does not support APIv2 > (indicated by H_SUCCESS not being returned), mark all test cases in the > suite as KUNIT_SKIPPED. This prevents test failures on systems without > APIv2 support while still allowing the tests to run on capable systems. > > [1] https://lore.kernel.org/all/20260603064225.GC18149@sol/ > > Reported-by: Eric Biggers <[email protected]> > Closes: https://lore.kernel.org/all/20260603064225.GC18149@sol > Signed-off-by: Vaibhav Jain <[email protected]> > Assisted-by: Bob:Claude-3.7-Sonnet Bob-Shell > --- > arch/powerpc/kvm/test-guest-state-buffer.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/arch/powerpc/kvm/test-guest-state-buffer.c > b/arch/powerpc/kvm/test-guest-state-buffer.c > index 5ccca306997a..f84b40fa55db 100644 > --- a/arch/powerpc/kvm/test-guest-state-buffer.c > +++ b/arch/powerpc/kvm/test-guest-state-buffer.c > @@ -521,6 +521,24 @@ static void test_gs_hostwide_counters(struct kunit *test) > kvmppc_gsb_free(gsb); > } > > +static int init_gs_test_suite(struct kunit_suite *suite) > +{ > + long rc; > + unsigned long host_capabilities; > + struct kunit_case *test_case; > + > + /* Enable test suite only if APIv2 is supported */ > + rc = plpar_guest_get_capabilities(0, &host_capabilities); I believe we don't really need an hcall overhead to check the availability of APIv2. We could simply check: diff --git a/arch/powerpc/kvm/test-guest-state-buffer.c b/arch/powerpc/kvm/test-guest-state-buffer.c index 5ccca306997a..a263e7f31e15 100644 --- a/arch/powerpc/kvm/test-guest-state-buffer.c +++ b/arch/powerpc/kvm/test-guest-state-buffer.c @@ -521,6 +521,18 @@ static void test_gs_hostwide_counters(struct kunit *test) kvmppc_gsb_free(gsb); } +static int init_gs_test_suite(struct kunit_suite *suite) +{ + struct kunit_case *test_case; + + if (!kvmhv_is_nestedv2()) { + kunit_suite_for_each_test_case(suite, test_case) + WRITE_ONCE(test_case->status, KUNIT_SKIPPED); + } + + return 0; +} + Also, I understand that these tests exercise gsb related tests specific to APIv2 but I see that only 'test_gs_hostwide_counters' relies on an APIv2 specific 'H_GUEST_GET_STATE' hcall but rest of the tests just operate on in-memory gsb. So, do we really want to skip all the tests when APIv2 is not available? If not, we could simply skip this one test as: diff --git a/arch/powerpc/kvm/test-guest-state-buffer.c b/arch/powerpc/kvm/test-guest-state-buffer.c index 5ccca306997a..89999b80fdfc 100644 --- a/arch/powerpc/kvm/test-guest-state-buffer.c +++ b/arch/powerpc/kvm/test-guest-state-buffer.c @@ -462,7 +462,10 @@ static void test_gs_hostwide_counters(struct kunit *test) int rc; if (!kvmhv_on_pseries()) - kunit_skip(test, "This test need a kmv-hv guest"); + kunit_skip(test, "This test need a kvm-hv guest"); + + if (!kvmhv_is_nestedv2()) + kunit_skip(test, "This test needs an spapr nested APIv2 support"); gsm = kvmppc_gsm_new(&gs_msg_test_hostwide_ops, &test_data, GSM_SEND, GFP_KERNEL); Please let me know your views. Thanks, Amit > + > + if (rc != H_SUCCESS) { > + /* Skip all testcases if no APIv2 support */ > + kunit_suite_for_each_test_case(suite, test_case) > + WRITE_ONCE(test_case->status, KUNIT_SKIPPED); > + } > + > + return 0; > +} > + > static struct kunit_case guest_state_buffer_testcases[] = { > KUNIT_CASE(test_creating_buffer), > KUNIT_CASE(test_adding_element), > @@ -535,6 +553,7 @@ static struct kunit_case guest_state_buffer_testcases[] = > { > static struct kunit_suite guest_state_buffer_test_suite = { > .name = "guest_state_buffer_test", > .test_cases = guest_state_buffer_testcases, > + .suite_init = init_gs_test_suite, > }; > > kunit_test_suites(&guest_state_buffer_test_suite); > -- > 2.54.0 >
