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

[Amit Machhiwal <amachhiw@linux.ibm.com>]

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()) {
+               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
>