Re: [PATCH 2/4] xen/version: Drop compat/kernel.c

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.01.2023 21:09, Andrew Cooper wrote:
> kernel.c is mostly in an #ifndef COMPAT guard, because compat/kernel.c
> reincludes kernel.c to recompile xen_version() in a compat form.
> 
> However, the xen_version hypercall is almost guest-ABI-agnostic; only
> XENVER_platform_parameters has a compat split.  Handle this locally, and do
> away with the reinclude entirely.

And we henceforth mean to not introduce any interface structures here
which would require translation (or we're willing to accept the clutter
of handling those "locally" as well). Fine with me, just wanting to
mention it.

> --- a/xen/common/compat/kernel.c
> +++ /dev/null
> @@ -1,53 +0,0 @@
> -/******************************************************************************
> - * kernel.c
> - */
> -
> -EMIT_FILE;
> -
> -#include <xen/init.h>
> -#include <xen/lib.h>
> -#include <xen/errno.h>
> -#include <xen/version.h>
> -#include <xen/sched.h>
> -#include <xen/guest_access.h>
> -#include <asm/current.h>
> -#include <compat/xen.h>
> -#include <compat/version.h>
> -
> -extern xen_commandline_t saved_cmdline;
> -
> -#define xen_extraversion compat_extraversion
> -#define xen_extraversion_t compat_extraversion_t
> -
> -#define xen_compile_info compat_compile_info
> -#define xen_compile_info_t compat_compile_info_t
> -
> -CHECK_TYPE(capabilities_info);

This and ...

> -#define xen_platform_parameters compat_platform_parameters
> -#define xen_platform_parameters_t compat_platform_parameters_t
> -#undef HYPERVISOR_VIRT_START
> -#define HYPERVISOR_VIRT_START HYPERVISOR_COMPAT_VIRT_START(current->domain)
> -
> -#define xen_changeset_info compat_changeset_info
> -#define xen_changeset_info_t compat_changeset_info_t
> -
> -#define xen_feature_info compat_feature_info
> -#define xen_feature_info_t compat_feature_info_t
> -
> -CHECK_TYPE(domain_handle);

... this go away without any replacement. Considering they're both
char[], that's probably fine, but could do with mentioning in the
description.

> @@ -520,12 +518,27 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) 
> arg)
>      
>      case XENVER_platform_parameters:
>      {
> -        xen_platform_parameters_t params = {
> -            .virt_start = HYPERVISOR_VIRT_START
> -        };

With this gone the oddly (but intentionally) placed braces can then
also go away.

Preferably with these minor adjustments
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan