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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The test suite is not actually skipped because `init_gs_test_suite` unconditionally returns 0.
- [Low] Calling `kunit_mark_skipped()` on a `kunit_suite` pointer causes a compilation error.
--

commit f9ed140af60f93f156fdd320b1d30456dcf507f4
Author: Vaibhav Jain <vaibhav@linux.ibm.com>

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

This commit adds a suite_init callback to check for APIv2 nested
virtualization capabilities before running the guest state buffer tests.
If the host lacks support, it attempts to skip the entire kunit_suite to
prevent test failures on incapable systems.

Closes: https://lore.kernel.org/all/20260603064225.GC18149@sol

> diff --git a/arch/powerpc/kvm/test-guest-state-buffer.c b/arch/powerpc/kvm/test-guest-state-buffer.c
> index 5ccca306997a8..232c00493a25d 100644
> --- a/arch/powerpc/kvm/test-guest-state-buffer.c
> +++ b/arch/powerpc/kvm/test-guest-state-buffer.c

[ ... ]

> @@ -521,6 +521,21 @@ 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 = H_NOT_AVAILABLE;
> +	unsigned long host_capabilities = 0;
> +
> +	/* Enable test suite only if APIv2 is supported */
> +	if (kvmhv_on_pseries())
> +		rc = plpar_guest_get_capabilities(0, &host_capabilities);
> +
> +	if (rc)
> +		kunit_mark_skipped(suite, "Unsupported hypervisor");

[Severity: Low]
Does this regression cause a compilation error?

Looking at include/kunit/test.h, the kunit_mark_skipped() macro expects a
struct kunit pointer as its first argument, but here we are passing a struct
kunit_suite pointer.

Because the macro expands to WRITE_ONCE((test)->status, KUNIT_SKIPPED) and
struct kunit_suite lacks a status field, won't this break the build? I
notice the commit message mentions this relies on a proposed patch, but
could it be adapted to work with the existing API?

> +
> +	return 0;

[Severity: Medium]
Is this a regression where the test suite won't actually be skipped because
the function unconditionally returns 0?

In the KUnit framework, a suite_init callback must return a non-zero error
code to abort initialization and prevent the test cases from executing.

Because init_gs_test_suite() unconditionally returns 0, won't KUnit
consider the initialization successful and proceed to run all test cases
regardless of the APIv2 check?

> +}
> +
>  static struct kunit_case guest_state_buffer_testcases[] = {
>  	KUNIT_CASE(test_creating_buffer),
>  	KUNIT_CASE(test_adding_element),

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260617123820.467601-1-vaibhav@linux.ibm.com?part=1