Re: [Xen-devel] [PATCH for-next RFC 5/8] x86: factor out hypervisor agnostic code

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Sep 23, 2019 at 11:09:28AM +0100, Wei Liu wrote:
> The only implementation there is Xen.
> 
> No functional change.
> 
> Signed-off-by: Wei Liu <liuwe@xxxxxxxxxxxxx>
> ---
>  xen/arch/x86/guest/Makefile            |   2 +
>  xen/arch/x86/guest/hypervisor.c        | 112 +++++++++++++++++++++++++
>  xen/arch/x86/guest/xen/xen.c           |  81 +-----------------
>  xen/include/asm-x86/guest.h            |   1 +
>  xen/include/asm-x86/guest/hypervisor.h |  58 +++++++++++++
>  xen/include/asm-x86/guest/xen.h        |  21 ++---
>  6 files changed, 182 insertions(+), 93 deletions(-)
>  create mode 100644 xen/arch/x86/guest/hypervisor.c
>  create mode 100644 xen/include/asm-x86/guest/hypervisor.h
> 
> diff --git a/xen/arch/x86/guest/Makefile b/xen/arch/x86/guest/Makefile
> index 6806f04947..f63d64bbee 100644
> --- a/xen/arch/x86/guest/Makefile
> +++ b/xen/arch/x86/guest/Makefile
> @@ -1 +1,3 @@
> +obj-y += hypervisor.o
> +
>  subdir-$(CONFIG_XEN_GUEST) += xen
> diff --git a/xen/arch/x86/guest/hypervisor.c b/xen/arch/x86/guest/hypervisor.c
> new file mode 100644
> index 0000000000..b0a724bf13
> --- /dev/null
> +++ b/xen/arch/x86/guest/hypervisor.c
> @@ -0,0 +1,112 @@
> +/******************************************************************************
> + * arch/x86/guest/hypervisor.c
> + *
> + * Support for detecting and running under a hypervisor.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; If not, see <http://www.gnu.org/licenses/>.
> + *
> + * Copyright (c) 2017 Citrix Systems Ltd.
> + */
> +
> +#include <xen/init.h>
> +#include <xen/mm.h>
> +#include <xen/rangeset.h>
> +
> +#include <asm/guest.h>
> +#include <asm/processor.h>
> +
> +static struct rangeset *mem;
> +
> +void __init probe_hypervisor(void)

IMO would be nice to take the opportunity to rename this to
hypervisor_probe, to match with the rest of the hypervisor_ functions.

> diff --git a/xen/include/asm-x86/guest/xen.h b/xen/include/asm-x86/guest/xen.h
> index b015ed1883..d031f1f70d 100644
> --- a/xen/include/asm-x86/guest/xen.h
> +++ b/xen/include/asm-x86/guest/xen.h
> @@ -32,12 +32,10 @@ extern bool xen_guest;
>  extern bool pv_console;
>  extern uint32_t xen_cpuid_base;
>  
> -void probe_hypervisor(void);
> -void hypervisor_setup(void);
> -void hypervisor_ap_setup(void);
> -int hypervisor_alloc_unused_page(mfn_t *mfn);
> -int hypervisor_free_unused_page(mfn_t mfn);
> -void hypervisor_resume(void);
> +void probe_xen(void);
> +void xen_setup(void);
> +void xen_ap_setup(void);
> +void xen_resume(void);
>  
>  DECLARE_PER_CPU(unsigned int, vcpu_id);
>  DECLARE_PER_CPU(struct vcpu_info *, vcpu_info);
> @@ -47,16 +45,7 @@ DECLARE_PER_CPU(struct vcpu_info *, vcpu_info);
>  #define xen_guest 0
>  #define pv_console 0
>  
> -static inline void probe_hypervisor(void) {}
> -
> -static inline void hypervisor_setup(void)
> -{
> -    ASSERT_UNREACHABLE();
> -}
> -static inline void hypervisor_ap_setup(void)
> -{
> -    ASSERT_UNREACHABLE();
> -}
> +static inline void probe_xen(void) {}

Why do you need this?

AFAICT probe_xen is used in the same way as the rest of the xen_*
functions, and hence I wonder why you need a stub for it?

I guess this is a forward-looking change for when probe_xen will be
called unconditionally to check for Xen support?

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel