Re: [Xen-devel] [PATCH for-next v2 7/9] x86: switch xen implementation to use hypervisor framework

[Wei Liu <wl@xen.org>]

On Mon, Oct 21, 2019 at 11:56:36AM +0200, Roger Pau Monné wrote:
[...]
> >  static struct hypervisor_ops *hops __read_mostly;
> >  
> > @@ -31,7 +31,34 @@ bool hypervisor_probe(void)
> >      if ( hops )
> >          return true;
> >  
> > -    return false;
> > +    /* Too early to use cpu_has_hypervisor */
> > +    if ( !(cpuid_ecx(1) & cpufeat_mask(X86_FEATURE_HYPERVISOR)) )
> > +        return false;
> > +
> > +#ifdef CONFIG_XEN_GUEST
> > +    if ( xen_probe() )
> > +        hops = &xen_hypervisor_ops;
> > +#endif
> 
> I think you likely want something like:
> 
>     if ( xen_probe() )
>     {
>         hops = &xen_hypervisor_ops;
> 	return true;
>     }
>     if ( hyperv_probe() )
>     {
>         ....
> 	return true;
>     }
> 
>     return false;
> 
> In order to return after a successful probe, or else you lose cycles
> probing for hypervisors when a match has been found, and also in the
> Xen case you risk detecting the HyperV support in Xen and thus using
> that instead of the Xen one.
> 

Good point.

> Long term if we gain more guests support I would likely want to see
> hypervisor_ops turning into an array and gaining a probe function so
> that this can be done in a loop instead of having this function.
> 

That was my plan from the get-go but Xen lacked appropriate
infrastructure, hence I resorted to something akin to HVM hooks.

[...]
> > -void __init hypervisor_setup(void)
> > +void __init xen_setup(void)
> >  {
> >      init_memmap();
> >  
> > @@ -277,7 +272,7 @@ void __init hypervisor_setup(void)
> >      init_evtchn();
> >  }
> >  
> > -void hypervisor_ap_setup(void)
> > +void xen_ap_setup(void)
> >  {
> >      set_vcpu_id();
> >      map_vcpuinfo();
> > @@ -307,7 +302,7 @@ static void ap_resume(void *unused)
> >      init_evtchn();
> >  }
> >  
> > -void hypervisor_resume(void)
> > +void xen_resume(void)
> 
> I think xen_{setup/ap_setup/resume} can be made static now?

Indeed. I will fix this.

Wei.

> 
> Thanks, Roger.

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