Re: [Xen-devel] [PATCH v4 6/8] x86: switch xen guest implementation to use hypervisor framework

[Wei Liu <wl@xen.org>]

On Fri, Nov 29, 2019 at 02:50:18PM +0100, Jan Beulich wrote:
> On 21.11.2019 19:50, Wei Liu wrote:
> > +void __init hypervisor_setup(void)
> > +{
> > +    if ( hops && hops->setup )
> > +        hops->setup();
> > +}
> > +
> > +void hypervisor_ap_setup(void)
> > +{
> > +    if ( hops && hops->ap_setup )
> > +        hops->ap_setup();
> > +}
> > +
> > +void hypervisor_resume(void)
> > +{
> > +    if ( hops && hops->resume )
> > +        hops->resume();
> > +}
> 
> I assume these don't get executed frequently enough for
> alternatives indirect call patching to matter? Down the road, if
> any more frequently executed hooks should appear, we will want
> to switch to that model though. This has the added benefit of
> there then being only one runtime instance of struct
> hypervisor_ops; all per-hypervisor ones would become
> __initconstrel.
> 

All the routines listed here are run very infrequently.

I agree that when more frequently executed hooks appear we will want to
look into alternatives.

> > @@ -326,6 +310,31 @@ void hypervisor_resume(void)
> >          pv_console_init();
> >  }
> >  
> > +static const struct hypervisor_ops xg_ops = {
> 
> With this new xg_ prefix, ...
> 
> > +    .name = "Xen",
> > +    .setup = xen_setup,
> > +    .ap_setup = xen_ap_setup,
> > +    .resume = xen_resume,
> 
> ... I'd like to suggest to use it for these ones too (if they
> need to have a prefix in the first place, which I doubt, but
> which I know others view differently).
> 

Since they are only visible to this file I think not having a prefix
should be fine. 

> > +const struct hypervisor_ops * __init xen_probe(void)
> 
> For this one the call is a little more difficult, but for
> consistency I think xg_ would be slightly better here as well.

No problem. Changed it to xg_probe.

> 
> Nit: Strictly speaking there's a stray blank after *. We
> normally have ones only ahead of *, but not after.

Fixed.

Wei.

> 
> Jan

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