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

[Wei Liu <wl@xxxxxxx>]

On Wed, Sep 25, 2019 at 12:23:54PM +0200, Roger Pau Monné wrote:
> > -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).

Sure. I can do that. I always thought that strange too.

> 
> >  {
> >      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?
> 

It's done in a later patch. I believe you've already seen it.

> Also it might be nice to introduce something like:
> 
> enum guest_type {
>     XEN_GUEST,
> } guest_type;
> 

This works for me.

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

I debated this. These changes require more code churn with no apparent
benefit, but if people agree this is better I can make those changes.

> > +{
> > +    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?
> 

Yes. There are a lot of comparable concepts in Hyper-V. For example,
there is a page called VP assist page which is more or less the same as
Xen's vcpuinfo. Its format, content and interfaces may be different, but
conceptually it is the same as vcpuinfo.

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

Xen itself still needs to manage the address space, no?

I agree the rangeset code in xen.c isn't pretty, but it does the job and
is not too intrusive.

Wei.

> Thanks, Roger.

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