Re: [Xen-devel] [PATCH v7 08/15] x86/cpu: Remove loop form vmce_save_vcpu_ctxt() func

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Alexandru Stefan ISAILA [mailto:aisaila@xxxxxxxxxxxxxxx]
> Sent: 08 June 2018 09:51
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>; xen-devel@xxxxxxxxxxxxx
> Cc: Ian Jackson <Ian.Jackson@xxxxxxxxxx>; jbeulich@xxxxxxxx; Andrew
> Cooper <Andrew.Cooper3@xxxxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>
> Subject: Re: [PATCH v7 08/15] x86/cpu: Remove loop form
> vmce_save_vcpu_ctxt() func
> 
> On Vi, 2018-06-08 at 08:33 +0000, Paul Durrant wrote:
> > >
> > > -----Original Message-----
> > > From: Alexandru Isaila [mailto:aisaila@xxxxxxxxxxxxxxx]
> > > Sent: 07 June 2018 15:59
> > > To: xen-devel@xxxxxxxxxxxxx
> > > Cc: Ian Jackson <Ian.Jackson@xxxxxxxxxx>; Wei Liu <wei.liu2@citrix.
> > > com>;
> > > jbeulich@xxxxxxxx; Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>;
> Paul
> > > Durrant <Paul.Durrant@xxxxxxxxxx>; Alexandru Isaila
> > > <aisaila@xxxxxxxxxxxxxxx>
> > > Subject: [PATCH v7 08/15] x86/cpu: Remove loop form
> > > vmce_save_vcpu_ctxt() func
> > >
> > > Signed-off-by: Alexandru Isaila <aisaila@xxxxxxxxxxxxxxx>
> > > ---
> > >  xen/arch/x86/cpu/mcheck/vmce.c | 27 +++++++--------------------
> > >  1 file changed, 7 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/xen/arch/x86/cpu/mcheck/vmce.c
> > > b/xen/arch/x86/cpu/mcheck/vmce.c
> > > index 404f27e..ead1f73 100644
> > > --- a/xen/arch/x86/cpu/mcheck/vmce.c
> > > +++ b/xen/arch/x86/cpu/mcheck/vmce.c
> > > @@ -349,30 +349,17 @@ int vmce_wrmsr(uint32_t msr, uint64_t val)
> > >      return ret;
> > >  }
> > >
> > > -static void vmce_save_vcpu_ctxt_one(struct vcpu *v, struct
> > > hvm_vmce_vcpu *ctxt)
> > > -{
> > > -    ctxt->caps = v->arch.vmce.mcg_cap;
> > > -    ctxt->mci_ctl2_bank0 = v->arch.vmce.bank[0].mci_ctl2;
> > > -    ctxt->mci_ctl2_bank1 = v->arch.vmce.bank[1].mci_ctl2;
> > > -    ctxt->mcg_ext_ctl = v->arch.vmce.mcg_ext_ctl;
> > > -}
> > > -
> > >  static int vmce_save_vcpu_ctxt(struct domain *d,
> > > hvm_domain_context_t
> > > *h)
> > >  {
> > > -    struct vcpu *v;
> > > -    int err = 0;
> > > -
> > > -    for_each_vcpu ( d, v )
> > > -    {
> > > -        struct hvm_vmce_vcpu ctxt;
> > > +    struct hvm_vmce_vcpu ctxt;
> > > +    struct vcpu *v = NULL;
> > >
> > > -        vmce_save_vcpu_ctxt_one(v, &ctxt);
> > > -        err = hvm_save_entry(VMCE_VCPU, v->vcpu_id, h, &ctxt);
> > > -        if ( err )
> > > -            break;
> > > -    }
> > > +    ctxt.caps = v->arch.vmce.mcg_cap;
> > There's a typo in the commit title (s/form/from), but I don't
> > understand what you're doing here. You set v to NULL above and
> > dereference it below. AFAICT, until patch #15 is applied context
> > saving will be completely broken.
> Yes, this is true, but it could't find a better way to split the last
> patch further.

Can't you do it (something like) this way?

- Each of patches #1 - #7 register their save_one handler via an extra arg to 
HVM_REGISTER_SAVE_RESTORE (and hence extra field in hvm_sr_handlers)
- Move (current) patch #15 to patch #8 but have it call the save_one handlers
- Then have 7 patches that remove the now redundant save handlers, renaming 
XXX_save_one to XXX_save and passing NULL as the now useless argument to 
HVM_REGISTER_SAVE_RESTORE
- Then have a final patch deleting the useless arg from 
HVM_REGISTER_SAVE_RESTORE, cleaning up the callers and also renaming the field 
in hvm_sr_handlers from save_one to save.

That should keep the series bisectable AFAICT.

  Paul

> 
> Alex
> > >
> > > +    ctxt.mci_ctl2_bank0 = v->arch.vmce.bank[0].mci_ctl2;
> > > +    ctxt.mci_ctl2_bank1 = v->arch.vmce.bank[1].mci_ctl2;
> > > +    ctxt.mcg_ext_ctl = v->arch.vmce.mcg_ext_ctl;
> > >
> > > -    return err;
> > > +    return hvm_save_entry(VMCE_VCPU, v->vcpu_id, h, &ctxt);
> > >  }
> > >
> > >  static int vmce_load_vcpu_ctxt(struct domain *d,
> > > hvm_domain_context_t
> > > *h)
> > > --
> > > 2.7.4
> >
> > ________________________
> > This email was scanned by Bitdefender
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel