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

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

On Lu, 2018-07-02 at 09:38 +0000, Paul Durrant wrote:
> >
> > -----Original Message-----
> > From: Alexandru Isaila [mailto:aisaila@bitdefender.com]
> > Sent: 28 June 2018 10:26
> > 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 v9 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;
> Jan also queried the return values from the _save_one functions. I
> assume you need to make your 'continue' value reasonably global such
> that it can still be used when you move the iteration up in the outer
> save function. He suggested passing through the return of
> hvm_save_entry() but we also need to be sure it won't clash will the
> 'continue' value in future so it may now be necessary to actually
> define what the possible return values are.
>
It returns 0 for ok and -1 for error so I think the return check could
be removed.

But whit this we will introduce a potential fail point if someone
changes the return of hvm_save_entry(). At this moment the patch works
both ways. Its up to Jan to have the last word here if he is sure that
I should just return hvm_save_entry().

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