> -----Original Message-----
> From: Wei Liu [mailto:[email protected]]
> Sent: 02 January 2019 15:55
> To: Paul Durrant <[email protected]>
> Cc: [email protected]; Jan Beulich <[email protected]>;
> Andrew Cooper <[email protected]>; Wei Liu <[email protected]>;
> Roger Pau Monne <[email protected]>
> 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
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel