Re: [Xen-devel] [PATCH 3/8] viridian: extend init/deinit hooks into synic and time modules

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@xxxxxxxxxx]
> Sent: 02 January 2019 15:55
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Jan Beulich <jbeulich@xxxxxxxx>;
> Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>;
> Roger Pau Monne <roger.pau@xxxxxxxxxx>
> Subject: Re: [PATCH 3/8] viridian: extend init/deinit hooks into synic and
> time modules
> 
> On Thu, Dec 20, 2018 at 04:33:40PM +0000, Paul Durrant wrote:
> > diff --git a/xen/arch/x86/hvm/viridian/viridian.c
> b/xen/arch/x86/hvm/viridian/viridian.c
> > index e200e2ed1d..4c0f04df8c 100644
> > --- a/xen/arch/x86/hvm/viridian/viridian.c
> > +++ b/xen/arch/x86/hvm/viridian/viridian.c
> > @@ -424,6 +424,9 @@ int viridian_vcpu_init(struct vcpu *v)
> >      if ( !v->arch.hvm.viridian )
> >          return -ENOMEM;
> >
> > +    viridian_synic_vcpu_init(v);
> > +    viridian_time_vcpu_init(v);
> > +
> 
> Should these functions' return values be checked? They may fail judging
> from the fact that they aren't void functions.

Yes, indeed they should.

> 
> >      return 0;
> >  }
> >
> > @@ -434,6 +437,9 @@ int viridian_domain_init(struct domain *d)
> >      if ( !d->arch.hvm.viridian )
> >          return -ENOMEM;
> >
> > +    viridian_synic_domain_init(d);
> > +    viridian_time_domain_init(d);
> > +
> >      return 0;
> >  }
> >
> > @@ -443,7 +449,10 @@ void viridian_vcpu_deinit(struct vcpu *v)
> >          return;
> >
> >      if ( is_viridian_vcpu(v) )
> > -        viridian_synic_wrmsr(v, HV_X64_MSR_VP_ASSIST_PAGE, 0);
> 
> Where is this now? It's not in the synic deinit function as far as I can
> tell. Oh, it is used to unmap the vp_assist page so the call to
> viridian_synic_wrmsr is superseded by an unmap call directly. It should
> be fine here.

Yes, that's right.

  Paul

> 
> Wei.
> 
> > +    {
> > +        viridian_time_vcpu_deinit(v);
> > +        viridian_synic_vcpu_deinit(v);
> > +    }
> >
> >      xfree(v->arch.hvm.viridian);
> >      v->arch.hvm.viridian = NULL;
> > @@ -459,6 +468,9 @@ void viridian_domain_deinit(struct domain *d)
> >      if ( !d->arch.hvm.viridian )
> >          return;
> >
> > +    viridian_time_domain_deinit(d);
> > +    viridian_synic_domain_deinit(d);
> > +
> >      xfree(d->arch.hvm.viridian);
> >      d->arch.hvm.viridian = NULL;
> >  }
> > --
> > 2.20.1.2.gb21ebb671
> >

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