Hi Amit, Thanks for looking into this patch. My responses to your review comments inline below:
Amit Machhiwal <[email protected]> writes: > 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()) { kvmhv_is_nestedv2() depends on a static-key which is only set in kvmhv_nested_init(). This kunit testcase however can be run before kvmhv_nested_init() is called thereby rendering this check ineffective. > + 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? Fair point. However the GSB related code that these test-cases are exercising will never be executed in a non nested-papr APIv2 environment. To properly validate the GSB management it should be exercised with a hypervisor having support for nested-papr APIv2. Hence it makes sense to only run these test cases with the relevant support is 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"); As mentioned previously test for 'kvmhv_is_nestedv2()' may not be correct when this kunit test is being executed. Also I have proposed a minor change to kunit at [1] to address possibility of being able to skip a kunit-suite in its entirety. Will rework this patch if the proposed kunit changes are accepted. [1] https://lore.kernel.org/all/[email protected]/ <snip> -- Cheers ~ Vaibhav
