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

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

On Vi, 2018-06-08 at 08:42 -0600, Jan Beulich wrote:
> >
> > >
> > > >
> > > > On 08.06.18 at 14:46, <Paul.Durrant@citrix.com> wrote:
> > > From: Alexandru Stefan ISAILA [mailto:aisaila@bitdefender.com]
> > > Sent: 08 June 2018 09:51
> > > On Vi, 2018-06-08 at 08:33 +0000, Paul Durrant wrote:
> > > >
> > > > 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)
> I think either there should be a 1st patch introducing the new field
> and macro
> arg, or patches 1...7 remain the way they are and patch 8 introduces
> and
> uses that field without otherwise touching the handlers. In any event
> all later
> patches then shift down by one in numbering; apart from the numbering
> I
> mostly agree with ...
>
> >
> > - 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.
> ... all of this. However, I have to admit I'm not certain yet whether
> the
> extra argument can indeed go away again in the end: There are save
> records which aren't per-vCPU, and I'm not convinced we want to alter
> their handling.
>
So the final plan for the series is like this:
- Base everything on Roger's series
- Keep patches 1-7
- Have patch 8 add an extra arg to HVM_REGISTER_SAVE_RESTORE
and hvm_sr_handlers
- Have patch 9 like the patch 15 form now and have it call the
save_one handlers
- Have the next patches remove the redundant save handlers and
rename the save one
- The final patch should remove the extra arg. This one can be
kept or not.

Is this how I should go? Any thoughts are appreciated.

Thanks,
Alex


________________________
This email was scanned by Bitdefender
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel