Re: [PATCH v8 07/11] x86/hvm: Introduce viridian_save_vcpu_ctxt_one() func

[Alexandru Stefan ISAILA <aisaila@bitdefender.com>]

On Ma, 2018-06-26 at 11:07 +0000, Paul Durrant wrote:
> > 
> > -----Original Message-----
> > From: Alexandru Isaila [mailto:aisaila@bitdefender.com]
> > Sent: 25 June 2018 15:51
> > To: xen-devel@lists.xen.org
> > Cc: Ian Jackson <Ian.Jackson@citrix.com>; Wei Liu <wei.liu2@citrix.
> > com>;
> > jbeulich@suse.com; Andrew Cooper <Andrew.Cooper3@citrix.com>; Paul
> > Durrant <Paul.Durrant@citrix.com>; Alexandru Isaila
> > <aisaila@bitdefender.com>
> > Subject: [PATCH v8 07/11] x86/hvm: Introduce
> > viridian_save_vcpu_ctxt_one() func
> > 
> > This is used to save data from a single instance.
> > 
> > Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
> > ---
> >  xen/arch/x86/hvm/viridian.c | 26 ++++++++++++++++++--------
> >  1 file changed, 18 insertions(+), 8 deletions(-)
> > 
> > diff --git a/xen/arch/x86/hvm/viridian.c
> > b/xen/arch/x86/hvm/viridian.c
> > index 694eae6..ed164ab 100644
> > --- a/xen/arch/x86/hvm/viridian.c
> > +++ b/xen/arch/x86/hvm/viridian.c
> > @@ -1026,20 +1026,30 @@ static int viridian_load_domain_ctxt(struct
> > domain *d, hvm_domain_context_t *h)
> >  HVM_REGISTER_SAVE_RESTORE(VIRIDIAN_DOMAIN,
> > viridian_save_domain_ctxt,
> >                            viridian_load_domain_ctxt, 1,
> > HVMSR_PER_DOM);
> > 
> > -static int viridian_save_vcpu_ctxt(struct domain *d,
> > hvm_domain_context_t *h)
> > +static int viridian_save_vcpu_ctxt_one(struct vcpu *v,
> > hvm_domain_context_t *h)
> >  {
> > -    struct vcpu *v;
> > +    struct hvm_viridian_vcpu_context ctxt;
> > 
> > -    if ( !is_viridian_domain(d) )
> > +    if ( !is_viridian_domain(v->domain) )
> >          return 0;
> > 
> > +    memset(&ctxt, 0, sizeof(ctxt));
> > +    ctxt.vp_assist_msr = v-
> > >arch.hvm_vcpu.viridian.vp_assist.msr.raw;
> > +    ctxt.vp_assist_pending = v-
> > >arch.hvm_vcpu.viridian.vp_assist.pending;
> > +
> > +    if ( hvm_save_entry(VIRIDIAN_VCPU, v->vcpu_id, h, &ctxt) != 0
> > )
> > +        return 1;
> > +    return 0;
> Unlike the other patch I commented on, here you are actually
> processing the result of hvm_save_entry() by testing it for non-zero. 
> It all seems very arbitrary.
In patch 3 the check is done by the save function. On the next version
I will move all the checks for the return value to the save_one funcs
and add a comment to the handler definition for the return values
(0,1,2)

Alex
> 
> > 
> > +}
> > +
> > +static int viridian_save_vcpu_ctxt(struct domain *d,
> > hvm_domain_context_t *h)
> > +{
> > +    struct vcpu *v;
> > +
> >      for_each_vcpu( d, v ) {
> > -        struct hvm_viridian_vcpu_context ctxt = {
> > -            .vp_assist_msr = v-
> > >arch.hvm_vcpu.viridian.vp_assist.msr.raw,
> > -            .vp_assist_pending = v-
> > >arch.hvm_vcpu.viridian.vp_assist.pending,
> > -        };
> > +        int rc = viridian_save_vcpu_ctxt_one(v, h);
> > 
> > -        if ( hvm_save_entry(VIRIDIAN_VCPU, v->vcpu_id, h, &ctxt)
> > != 0 )
> > +        if ( rc != 0 )
> >              return 1;
> >      }
> > 
> > --
> > 2.7.4
> 
> ________________________
> This email was scanned by Bitdefender
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel