Re: [XTF-ARM] tests: Hypercall xen_version testing

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.12.2022 16:25, Michal Orzel wrote:
> Add a new test hyp-xen-version to perform functional testing of
> xen_version hypercall. Check the following commands (more can be added
> later on):
>  - XENVER_version,
>  - XENVER_extraversion,
>  - XENVER_compile_info,
>  - XENVER_changeset
>  - XENVER_get_features,
>  - passing invalid command.
> 
> For now, enable this test only for arm64.

What's wrong with exposing this uniformly?

> --- /dev/null
> +++ b/tests/hyp-xen-version/main.c
> @@ -0,0 +1,105 @@
> +/**
> + * @file tests/hyp-xen-version/main.c
> + * @ref test-hyp-xen-version
> + *
> + * @page test-hyp-xen-version Hypercall xen_version
> + *
> + * Functional testing of xen_version hypercall.
> + *
> + * @see tests/hyp-xen-version/main.c
> + */
> +#include <xtf.h>
> +
> +const char test_title[] = "Hypercall xen_version testing";
> +
> +#define INVALID_CMD -1
> +
> +void test_main(void)
> +{
> +    int ret;
> +
> +    printk("Checking XENVER_version:\n");
> +    {
> +        /*
> +        * Version is returned directly in format: ((major << 16) | minor),
> +        * so no need to check the return value for an error.
> +        */
> +        ret = hypercall_xen_version(XENVER_version, NULL);
> +        printk(" version: %u.%u\n", ret >> 16, ret & 0xFFFF);
> +    }
> +
> +    printk("Checking XENVER_extraversion:\n");
> +    {
> +        xen_extraversion_t xen_ev;
> +        memset(&xen_ev, 0, sizeof(xen_ev));
> +
> +        ret = hypercall_xen_version(XENVER_extraversion, xen_ev);
> +        if ( ret < 0 )
> +            return xtf_error("Error %d\n", ret);

This, ...

> +        printk(" extraversion: %s\n", xen_ev);
> +    }
> +
> +    printk("Checking XENVER_compile_info:\n");
> +    {
> +        xen_compile_info_t xen_ci;
> +        memset(&xen_ci, 0, sizeof(xen_ci));
> +
> +        ret = hypercall_xen_version(XENVER_compile_info, &xen_ci);
> +        if ( ret < 0 )
> +            return xtf_error("Error %d\n", ret);

... this, and ...

> +        printk(" compiler:       %s\n", xen_ci.compiler);
> +        printk(" compile_by:     %s\n", xen_ci.compile_by);
> +        printk(" compile_domain: %s\n", xen_ci.compile_domain);
> +        printk(" compile_date:   %s\n", xen_ci.compile_date);
> +    }
> +
> +    printk("Checking XENVER_changeset:\n");
> +    {
> +        xen_changeset_info_t xen_cs;
> +        memset(&xen_cs, 0, sizeof(xen_cs));
> +
> +        ret = hypercall_xen_version(XENVER_changeset, &xen_cs);
> +        if ( ret < 0 )
> +            return xtf_error("Error %d\n", ret);

... this can fail because of XSM denying access. (Others can of course
also fail for this reason, but here possible failure is kind of
"intended" - see the dummy xsm_xen_version() handling.) Therefore I
would like to suggest that you also special case getting back -EPERM,
resulting in e.g. just a warning instead of an error.

Jan