Re: [Xen-devel] [PATCH v5 02/12] x86/hypervisor: make hypervisor_ap_setup return an error code

2020-01-30 Thread Wei Liu
On Thu, Jan 30, 2020 at 11:01:29AM +0100, Roger Pau Monné wrote:
> On Wed, Jan 29, 2020 at 08:20:24PM +, 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 
> > ---
> >  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.

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

Re: [Xen-devel] [PATCH v5 02/12] x86/hypervisor: make hypervisor_ap_setup return an error code

2020-01-30 Thread Roger Pau Monné
On Wed, Jan 29, 2020 at 08:20:24PM +, 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 
> ---
>  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.

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