Re: [Xen-devel] [PATCH for-next RFC 4/8] x86: factor out xen variants for hypervisor setup code

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

On Mon, Sep 23, 2019 at 11:09:27AM +0100, Wei Liu wrote:
> We will add Hyper-V specific implementations in the future.
> 
> No functional change.
> 
> Signed-off-by: Wei Liu <liuwe@xxxxxxxxxxxxx>
> ---
>  xen/arch/x86/guest/xen/xen.c | 32 ++++++++++++++++++++++++++------
>  1 file changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/x86/guest/xen/xen.c b/xen/arch/x86/guest/xen/xen.c
> index 78fc603996..f93c8fbd1c 100644
> --- a/xen/arch/x86/guest/xen/xen.c
> +++ b/xen/arch/x86/guest/xen/xen.c
> @@ -67,7 +67,7 @@ static void __init find_xen_leaves(void)
>      }
>  }
>  
> -void __init probe_hypervisor(void)
> +static void __init probe_xen(void)

While here I would rename to xen_probe, to match with the other
functions (ie: xen_setup below for example).

>  {
>      if ( xen_guest )
>          return;
> @@ -87,6 +87,11 @@ void __init probe_hypervisor(void)
>      xen_guest = true;
>  }
>  
> +void __init probe_hypervisor(void)

Shouldn't this live in a separate file, like guest/guest.c or some
such?

Also it might be nice to introduce something like:

enum guest_type {
    XEN_GUEST,
} guest_type;

So that you can add a switch here, even if the only case is Xen ATM. I
guess this will be done in a later patch, or introduce an
hypervisor_ops struct that contain pointers to functions to allow for
different implementations.

> +{
> +    probe_xen();
> +}
> +
>  static void map_shared_info(void)
>  {
>      mfn_t mfn;
> @@ -249,10 +254,8 @@ static void init_evtchn(void)
>      }
>  }
>  
> -void __init hypervisor_setup(void)
> +static void __init xen_setup(void)
>  {
> -    init_memmap();
> -
>      map_shared_info();
>  
>      set_vcpu_id();
> @@ -277,13 +280,25 @@ void __init hypervisor_setup(void)
>      init_evtchn();
>  }
>  
> -void hypervisor_ap_setup(void)
> +void __init hypervisor_setup(void)
> +{
> +    init_memmap();

I wonder, do you also require to map hypervisor data into the guest
physmap when running on HyperV?

Is there a way when running on HyperV to request unused physical
address space ranges? What Xen currently does in init_memmap is quite
crappy.

Thanks, Roger.

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