Re: [PATCH] kvm powerpc/book3s-apiv2: Add suite initialization to skip GSB tests without APIv2 support

[Vaibhav Jain <vaibhav@linux.ibm.com>]

Hi Amit,

Thanks for looking into this patch. My responses to your review comments
inline below:

Amit Machhiwal <amachhiw@linux.ibm.com> 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 <ebiggers@kernel.org>
>> Closes: https://lore.kernel.org/all/20260603064225.GC18149@sol
>> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
>> 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/20260604162805.556135-1-vaibhav@linux.ibm.com/

<snip>

-- 
Cheers
~ Vaibhav