| Message ID | 20200129202034.15052-3-liuwe@microsoft.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | More Hyper-V infrastructures | expand |
On Wed, Jan 29, 2020 at 08:20:24PM +0000, Wei Liu wrote: > We want to be able to handle AP setup error in the upper layer. Thanks, some comments below. > > Signed-off-by: Wei Liu <liuwe@microsoft.com> > --- > xen/arch/x86/guest/hypervisor.c | 6 ++++-- > xen/arch/x86/guest/xen/xen.c | 11 +++++++++-- > xen/include/asm-x86/guest/hypervisor.h | 6 +++--- > 3 files changed, 16 insertions(+), 7 deletions(-) > > diff --git a/xen/arch/x86/guest/hypervisor.c b/xen/arch/x86/guest/hypervisor.c > index 4f27b98740..e72c92ffdf 100644 > --- a/xen/arch/x86/guest/hypervisor.c > +++ b/xen/arch/x86/guest/hypervisor.c > @@ -52,10 +52,12 @@ void __init hypervisor_setup(void) > ops->setup(); > } > > -void hypervisor_ap_setup(void) > +int hypervisor_ap_setup(void) > { > if ( ops && ops->ap_setup ) > - ops->ap_setup(); > + return ops->ap_setup(); > + > + return 0; > } > > void hypervisor_resume(void) > diff --git a/xen/arch/x86/guest/xen/xen.c b/xen/arch/x86/guest/xen/xen.c > index 6dbc5f953f..eed8a6edae 100644 > --- a/xen/arch/x86/guest/xen/xen.c > +++ b/xen/arch/x86/guest/xen/xen.c > @@ -257,11 +257,18 @@ static void __init setup(void) > init_evtchn(); > } > > -static void ap_setup(void) > +static int ap_setup(void) > { > + int rc; > + > set_vcpu_id(); > - map_vcpuinfo(); > + rc = map_vcpuinfo(); map_vcpuinfo should be changed so that the BUG_ON is removed, and an error is only returned if VCPUOP_register_vcpu_info fails and vcpu >= XEN_LEGACY_MAX_VCPUS, else no error should be returned. > + if ( rc ) > + return rc; > + > init_evtchn(); > + > + return 0; In order to keep this shorter, you could do: if ( !rc ) init_evtchn(); return rc; Thanks, Roger.
On Thu, Jan 30, 2020 at 11:01:29AM +0100, Roger Pau Monné wrote: > On Wed, Jan 29, 2020 at 08:20:24PM +0000, Wei Liu wrote: > > We want to be able to handle AP setup error in the upper layer. > > Thanks, some comments below. > > > > > Signed-off-by: Wei Liu <liuwe@microsoft.com> > > --- > > xen/arch/x86/guest/hypervisor.c | 6 ++++-- > > xen/arch/x86/guest/xen/xen.c | 11 +++++++++-- > > xen/include/asm-x86/guest/hypervisor.h | 6 +++--- > > 3 files changed, 16 insertions(+), 7 deletions(-) > > > > diff --git a/xen/arch/x86/guest/hypervisor.c b/xen/arch/x86/guest/hypervisor.c > > index 4f27b98740..e72c92ffdf 100644 > > --- a/xen/arch/x86/guest/hypervisor.c > > +++ b/xen/arch/x86/guest/hypervisor.c > > @@ -52,10 +52,12 @@ void __init hypervisor_setup(void) > > ops->setup(); > > } > > > > -void hypervisor_ap_setup(void) > > +int hypervisor_ap_setup(void) > > { > > if ( ops && ops->ap_setup ) > > - ops->ap_setup(); > > + return ops->ap_setup(); > > + > > + return 0; > > } > > > > void hypervisor_resume(void) > > diff --git a/xen/arch/x86/guest/xen/xen.c b/xen/arch/x86/guest/xen/xen.c > > index 6dbc5f953f..eed8a6edae 100644 > > --- a/xen/arch/x86/guest/xen/xen.c > > +++ b/xen/arch/x86/guest/xen/xen.c > > @@ -257,11 +257,18 @@ static void __init setup(void) > > init_evtchn(); > > } > > > > -static void ap_setup(void) > > +static int ap_setup(void) > > { > > + int rc; > > + > > set_vcpu_id(); > > - map_vcpuinfo(); > > + rc = map_vcpuinfo(); > > map_vcpuinfo should be changed so that the BUG_ON is removed, and an > error is only returned if VCPUOP_register_vcpu_info fails and vcpu >= > XEN_LEGACY_MAX_VCPUS, else no error should be returned. Done. > > > + if ( rc ) > > + return rc; > > + > > init_evtchn(); > > + > > + return 0; > > In order to keep this shorter, you could do: > > if ( !rc ) > init_evtchn(); > > return rc; Done. Wei. > > Thanks, Roger.
diff --git a/xen/arch/x86/guest/hypervisor.c b/xen/arch/x86/guest/hypervisor.c index 4f27b98740..e72c92ffdf 100644 --- a/xen/arch/x86/guest/hypervisor.c +++ b/xen/arch/x86/guest/hypervisor.c @@ -52,10 +52,12 @@ void __init hypervisor_setup(void) ops->setup(); } -void hypervisor_ap_setup(void) +int hypervisor_ap_setup(void) { if ( ops && ops->ap_setup ) - ops->ap_setup(); + return ops->ap_setup(); + + return 0; } void hypervisor_resume(void) diff --git a/xen/arch/x86/guest/xen/xen.c b/xen/arch/x86/guest/xen/xen.c index 6dbc5f953f..eed8a6edae 100644 --- a/xen/arch/x86/guest/xen/xen.c +++ b/xen/arch/x86/guest/xen/xen.c @@ -257,11 +257,18 @@ static void __init setup(void) init_evtchn(); } -static void ap_setup(void) +static int ap_setup(void) { + int rc; + set_vcpu_id(); - map_vcpuinfo(); + rc = map_vcpuinfo(); + if ( rc ) + return rc; + init_evtchn(); + + return 0; } int xg_alloc_unused_page(mfn_t *mfn) diff --git a/xen/include/asm-x86/guest/hypervisor.h b/xen/include/asm-x86/guest/hypervisor.h index 392f4b90ae..b503854c5b 100644 --- a/xen/include/asm-x86/guest/hypervisor.h +++ b/xen/include/asm-x86/guest/hypervisor.h @@ -25,7 +25,7 @@ struct hypervisor_ops { /* Main setup routine */ void (*setup)(void); /* AP setup */ - void (*ap_setup)(void); + int (*ap_setup)(void); /* Resume from suspension */ void (*resume)(void); }; @@ -34,7 +34,7 @@ struct hypervisor_ops { const char *hypervisor_probe(void); void hypervisor_setup(void); -void hypervisor_ap_setup(void); +int hypervisor_ap_setup(void); void hypervisor_resume(void); #else @@ -44,7 +44,7 @@ void hypervisor_resume(void); static inline const char *hypervisor_probe(void) { return NULL; } static inline void hypervisor_setup(void) { ASSERT_UNREACHABLE(); } -static inline void hypervisor_ap_setup(void) { ASSERT_UNREACHABLE(); } +static inline int hypervisor_ap_setup(void) { ASSERT_UNREACHABLE(); return 0; } static inline void hypervisor_resume(void) { ASSERT_UNREACHABLE(); } #endif /* CONFIG_GUEST */
We want to be able to handle AP setup error in the upper layer. Signed-off-by: Wei Liu <liuwe@microsoft.com> --- xen/arch/x86/guest/hypervisor.c | 6 ++++-- xen/arch/x86/guest/xen/xen.c | 11 +++++++++-- xen/include/asm-x86/guest/hypervisor.h | 6 +++--- 3 files changed, 16 insertions(+), 7 deletions(-)