| Message ID | 20190923100931.29670-7-liuwe@microsoft.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Port Xen to Hyper-V | expand |
On Mon, Sep 23, 2019 at 11:09:29AM +0100, Wei Liu wrote: > We need indication whether it has succeeded or not. > > Signed-off-by: Wei Liu <liuwe@microsoft.com> The code LGTM, I have just a suggestion on the approach. > --- > xen/arch/x86/guest/hypervisor.c | 5 ++++- > xen/arch/x86/guest/xen/xen.c | 7 ++++--- > xen/include/asm-x86/guest/xen.h | 4 ++-- > 3 files changed, 10 insertions(+), 6 deletions(-) > > diff --git a/xen/arch/x86/guest/hypervisor.c b/xen/arch/x86/guest/hypervisor.c > index b0a724bf13..fb572b0402 100644 > --- a/xen/arch/x86/guest/hypervisor.c > +++ b/xen/arch/x86/guest/hypervisor.c > @@ -34,7 +34,10 @@ void __init probe_hypervisor(void) > if ( !(cpuid_ecx(1) & cpufeat_mask(X86_FEATURE_HYPERVISOR)) ) > return; > > - probe_xen(); > + if ( probe_xen() ) > + return; > + > + /* Hyper-V probing to follow. */ Instead of having to append a new probe_foo here every time support for running on a new hypervisor is added, you could do something similar to what's done in REGISTER_VPCI_INIT, where each hypervisor would register it's own set of helpers and probe function in a specific section, and then you would just iterate over all the guest support that's compiled in Xen. That would also prevent you from having to export a dummy probe_xen helper if CONFIG_XEN_GUEST is not defined. Anyway, maybe that's overkill... Thanks, Roger.
On Wed, Sep 25, 2019 at 12:44:27PM +0200, Roger Pau Monné wrote: > On Mon, Sep 23, 2019 at 11:09:29AM +0100, Wei Liu wrote: > > We need indication whether it has succeeded or not. > > > > Signed-off-by: Wei Liu <liuwe@microsoft.com> > > The code LGTM, I have just a suggestion on the approach. > > > --- > > xen/arch/x86/guest/hypervisor.c | 5 ++++- > > xen/arch/x86/guest/xen/xen.c | 7 ++++--- > > xen/include/asm-x86/guest/xen.h | 4 ++-- > > 3 files changed, 10 insertions(+), 6 deletions(-) > > > > diff --git a/xen/arch/x86/guest/hypervisor.c b/xen/arch/x86/guest/hypervisor.c > > index b0a724bf13..fb572b0402 100644 > > --- a/xen/arch/x86/guest/hypervisor.c > > +++ b/xen/arch/x86/guest/hypervisor.c > > @@ -34,7 +34,10 @@ void __init probe_hypervisor(void) > > if ( !(cpuid_ecx(1) & cpufeat_mask(X86_FEATURE_HYPERVISOR)) ) > > return; > > > > - probe_xen(); > > + if ( probe_xen() ) > > + return; > > + > > + /* Hyper-V probing to follow. */ > > Instead of having to append a new probe_foo here every time support for > running on a new hypervisor is added, you could do something similar > to what's done in REGISTER_VPCI_INIT, where each hypervisor would > register it's own set of helpers and probe function in a specific > section, and then you would just iterate over all the guest support > that's compiled in Xen. > > That would also prevent you from having to export a dummy probe_xen > helper if CONFIG_XEN_GUEST is not defined. > > Anyway, maybe that's overkill... Let's see what Andrew and Jan think. Either works for me -- it is just a matter of writing code, but obviously the less work the better. :-) Wei. > > Thanks, Roger.
diff --git a/xen/arch/x86/guest/hypervisor.c b/xen/arch/x86/guest/hypervisor.c index b0a724bf13..fb572b0402 100644 --- a/xen/arch/x86/guest/hypervisor.c +++ b/xen/arch/x86/guest/hypervisor.c @@ -34,7 +34,10 @@ void __init probe_hypervisor(void) if ( !(cpuid_ecx(1) & cpufeat_mask(X86_FEATURE_HYPERVISOR)) ) return; - probe_xen(); + if ( probe_xen() ) + return; + + /* Hyper-V probing to follow. */ } static void __init init_memmap(void) diff --git a/xen/arch/x86/guest/xen/xen.c b/xen/arch/x86/guest/xen/xen.c index a730e6ad1b..8390b045f0 100644 --- a/xen/arch/x86/guest/xen/xen.c +++ b/xen/arch/x86/guest/xen/xen.c @@ -66,20 +66,21 @@ static void __init find_xen_leaves(void) } } -void __init probe_xen(void) +bool __init probe_xen(void) { if ( xen_guest ) - return; + return true; find_xen_leaves(); if ( !xen_cpuid_base ) - return; + return false; /* Fill the hypercall page. */ wrmsrl(cpuid_ebx(xen_cpuid_base + 2), __pa(hypercall_page)); xen_guest = true; + return true; } static void map_shared_info(void) diff --git a/xen/include/asm-x86/guest/xen.h b/xen/include/asm-x86/guest/xen.h index d031f1f70d..7eda3d4956 100644 --- a/xen/include/asm-x86/guest/xen.h +++ b/xen/include/asm-x86/guest/xen.h @@ -32,7 +32,7 @@ extern bool xen_guest; extern bool pv_console; extern uint32_t xen_cpuid_base; -void probe_xen(void); +bool probe_xen(void); void xen_setup(void); void xen_ap_setup(void); void xen_resume(void); @@ -45,7 +45,7 @@ DECLARE_PER_CPU(struct vcpu_info *, vcpu_info); #define xen_guest 0 #define pv_console 0 -static inline void probe_xen(void) {} +static inline bool probe_xen(void) { return false; } #endif /* CONFIG_XEN_GUEST */ #endif /* __X86_GUEST_XEN_H__ */
We need indication whether it has succeeded or not. Signed-off-by: Wei Liu <liuwe@microsoft.com> --- xen/arch/x86/guest/hypervisor.c | 5 ++++- xen/arch/x86/guest/xen/xen.c | 7 ++++--- xen/include/asm-x86/guest/xen.h | 4 ++-- 3 files changed, 10 insertions(+), 6 deletions(-)