Re: [Xen-devel] [PATCH v4 6/8] x86: switch xen guest implementation to use hypervisor framework
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
Re: [Xen-devel] [PATCH v4 6/8] x86: switch xen guest implementation to use hypervisor framework
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.
> @@ -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).
> +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.
Nit: Strictly speaking there's a stray blank after *. We
normally have ones only ahead of *, but not after.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4 6/8] x86: switch xen guest implementation to use hypervisor framework
On Fri, Nov 22, 2019 at 11:02:30AM +, Durrant, Paul wrote: > > -Original Message- > > From: Xen-devel On Behalf Of Wei > > Liu > > Sent: 21 November 2019 19:51 > > To: Xen Development List > > Cc: Wei Liu ; Wei Liu ; Andrew Cooper > > ; Michael Kelley ; Jan > > Beulich ; Roger Pau Monné > > Subject: [Xen-devel] [PATCH v4 6/8] x86: switch xen guest implementation > > to use hypervisor framework > > > > Signed-off-by: Wei Liu > [snip] > > diff --git a/xen/include/asm-x86/guest/xen.h b/xen/include/asm- > > x86/guest/xen.h > > index 01dc3ee6f6..db90b550a7 100644 > > --- a/xen/include/asm-x86/guest/xen.h > > +++ b/xen/include/asm-x86/guest/xen.h > > @@ -23,6 +23,7 @@ > > > > #include > > #include > > +#include > > > > #define XEN_shared_info ((struct shared_info > > *)fix_to_virt(FIX_XEN_SHARED_INFO)) > > > > @@ -32,7 +33,7 @@ extern bool xen_guest; > > extern bool pv_console; > > extern uint32_t xen_cpuid_base; > > > > -void probe_hypervisor(void); > > +const struct hypervisor_ops *xen_probe(void); > > int xg_alloc_unused_page(mfn_t *mfn); > > int xg_free_unused_page(mfn_t mfn); > > > > @@ -44,7 +45,7 @@ DECLARE_PER_CPU(struct vcpu_info *, vcpu_info); > > #define xen_guest 0 > > #define pv_console 0 > > Nit: These should be #defined to false rather than 0. The rest LGTM so with > those fixed, They were part of existing code so I didn't touch them. Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4 6/8] x86: switch xen guest implementation to use hypervisor framework
> -Original Message-
> From: Xen-devel On Behalf Of Wei
> Liu
> Sent: 21 November 2019 19:51
> To: Xen Development List
> Cc: Wei Liu ; Wei Liu ; Andrew Cooper
> ; Michael Kelley ; Jan
> Beulich ; Roger Pau Monné
> Subject: [Xen-devel] [PATCH v4 6/8] x86: switch xen guest implementation
> to use hypervisor framework
>
> Signed-off-by: Wei Liu
[snip]
> diff --git a/xen/include/asm-x86/guest/xen.h b/xen/include/asm-
> x86/guest/xen.h
> index 01dc3ee6f6..db90b550a7 100644
> --- a/xen/include/asm-x86/guest/xen.h
> +++ b/xen/include/asm-x86/guest/xen.h
> @@ -23,6 +23,7 @@
>
> #include
> #include
> +#include
>
> #define XEN_shared_info ((struct shared_info
> *)fix_to_virt(FIX_XEN_SHARED_INFO))
>
> @@ -32,7 +33,7 @@ extern bool xen_guest;
> extern bool pv_console;
> extern uint32_t xen_cpuid_base;
>
> -void probe_hypervisor(void);
> +const struct hypervisor_ops *xen_probe(void);
> int xg_alloc_unused_page(mfn_t *mfn);
> int xg_free_unused_page(mfn_t mfn);
>
> @@ -44,7 +45,7 @@ DECLARE_PER_CPU(struct vcpu_info *, vcpu_info);
> #define xen_guest 0
> #define pv_console 0
Nit: These should be #defined to false rather than 0. The rest LGTM so with
those fixed,
Reviewed-by: Paul Durrant
>
> -static inline void probe_hypervisor(void) {}
> +static inline const struct hypervisor_ops *xen_probe(void) { return NULL;
> }
>
> #endif /* CONFIG_XEN_GUEST */
> #endif /* __X86_GUEST_XEN_H__ */
> --
> 2.20.1
>
>
> ___
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
