Re: [Xen-devel] [PATCH v19 12/13] x86/hvm: Remove redundant save functions

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.09.18 at 15:54, <aisaila@xxxxxxxxxxxxxxx> wrote:
> On Mon, 2018-09-10 at 07:42 -0600, Jan Beulich wrote:
>> > > > On 10.09.18 at 15:33, <aisaila@xxxxxxxxxxxxxxx> wrote:
>> > 
>> > On Mon, 2018-09-10 at 15:36 +0300, Alexandru Isaila wrote:
>> > > This patch removes the redundant save functions and renames the
>> > > save_one* to save. It then changes the domain param to vcpu in
>> > > the
>> > > save funcs and adapts print messages in order to match the format
>> > > of
>> > > the
>> > > other save related messages.
>> > > 
>> > > Signed-off-by: Alexandru Isaila <aisaila@xxxxxxxxxxxxxxx>
>> > > 
>> > > ---
>> > > Changes since V18:
>> > >  - Add const struct domain to rtc_save and hpet_save
>> > >  - Latched the vCPU into a local variable in hvm_save_one()
>> > >  - Add HVMSR_PER_VCPU kind check to the bounds if.
>> > > ---
>> > >  xen/arch/x86/cpu/mcheck/vmce.c | 18 +-------
>> > >  xen/arch/x86/emul-i8254.c      |  5 ++-
>> > >  xen/arch/x86/hvm/hpet.c        |  7 ++--
>> > >  xen/arch/x86/hvm/hvm.c         | 75 +++-------------------------
>> > > ----
>> > > --
>> > >  xen/arch/x86/hvm/irq.c         | 15 ++++---
>> > >  xen/arch/x86/hvm/mtrr.c        | 22 ++--------
>> > >  xen/arch/x86/hvm/pmtimer.c     |  5 ++-
>> > >  xen/arch/x86/hvm/rtc.c         |  5 ++-
>> > >  xen/arch/x86/hvm/save.c        | 28 +++++++------
>> > >  xen/arch/x86/hvm/vioapic.c     |  5 ++-
>> > >  xen/arch/x86/hvm/viridian.c    | 23 ++---------
>> > >  xen/arch/x86/hvm/vlapic.c      | 38 ++---------------
>> > >  xen/arch/x86/hvm/vpic.c        |  5 ++-
>> > >  xen/include/asm-x86/hvm/save.h |  8 +---
>> > >  14 files changed, 63 insertions(+), 196 deletions(-)
>> > > 
>> > > @@ -141,6 +138,8 @@ int hvm_save_one(struct domain *d, unsigned
>> > > int
>> > > typecode, unsigned int instance,
>> > >      int rv;
>> > >      hvm_domain_context_t ctxt = { };
>> > >      const struct hvm_save_descriptor *desc;
>> > > +    struct vcpu *v = (hvm_sr_handlers[typecode].kind ==
>> > > HVMSR_PER_VCPU) ?
>> > > +                     d->vcpu[instance] : d->vcpu[0];
>> > >  
>> > 
>> > Sorry for the inconvenience but I've just realized that this has to
>> > be
>> > initialize after the bounds check. I will have this in mine
>> 
>> Also to eliminate redundancy I'd prefer if you moved the conditional
>> expression inside the square brackets.
>> 
> Are these changes worth waiting 24h?

That's up to you in this case, I'd say.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel