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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 15 Dec 2022, Jan Beulich wrote:
> 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?

Actually the tests are not arm64 specific. Michal, I think you should
remove the comment above from the commit message. (What/if/when gets
enabled as a test in gitlab-ci is a different matter.)


> > --- /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
>