Re: [Xen-devel] [PATCH v14 08/11] x86/hvm: Add handler for save_one funcs

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 31.07.18 at 14:55, <aisaila@xxxxxxxxxxxxxxx> wrote:
> On Ma, 2018-07-31 at 06:34 -0600, Jan Beulich wrote:
>> > > > On 25.07.18 at 14:14, <aisaila@xxxxxxxxxxxxxxx> wrote:
>> > --- a/xen/arch/x86/hvm/vlapic.c
>> > +++ b/xen/arch/x86/hvm/vlapic.c
>> > @@ -1576,9 +1576,9 @@ static int lapic_load_regs(struct domain *d,
>> > hvm_domain_context_t *h)
>> >      return 0;
>> >  }
>> >  
>> > -HVM_REGISTER_SAVE_RESTORE(LAPIC, lapic_save_hidden,
>> > lapic_load_hidden,
>> > +HVM_REGISTER_SAVE_RESTORE(LAPIC, lapic_save_hidden, NULL,
>> > lapic_load_hidden,
>> >                            1, HVMSR_PER_VCPU);
>> > -HVM_REGISTER_SAVE_RESTORE(LAPIC_REGS, lapic_save_regs,
>> > lapic_load_regs,
>> > +HVM_REGISTER_SAVE_RESTORE(LAPIC_REGS, lapic_save_regs, NULL,
>> > lapic_load_regs,
>> These are per-vCPU as well - why do they get NULL inserted here,
>> rather than there being another (two) prereq patch(es)?
> 
> Both LAPIC save functions have for for (vcpu) so the look like a
> save_one function already, no need to do anything here.

Quite the opposite - presence of a loop over all vCPU-s clearly
says they're not save-one functions. Otherwise you wouldn't
have found the need to touch the functions the way you do in
patch 10.

>> > @@ -114,12 +117,13 @@ void hvm_register_savevm(uint16_t typecode,
>> >  
>> >  /* Syntactic sugar around that function: specify the max number of
>> >   * saves, and this calculates the size of buffer needed */
>> > -#define HVM_REGISTER_SAVE_RESTORE(_x, _save, _load, _num,
>> > _k)             \
>> > +#define HVM_REGISTER_SAVE_RESTORE(_x, _save, _save_one, _load,
>> > _num, _k)  \
>> >  static int __init
>> > __hvm_register_##_x##_save_and_restore(void)            \
>> >  {                                                                 
>> >         \
>> >      hvm_register_savevm(HVM_SAVE_CODE(_x),                        
>> >         \
>> >                          #_x,                                      
>> >         \
>> >                          &_save,                                   
>> >         \
>> > +                        _save_one,                                
>> >         \
>> While I generally appreciate the omission of the &, I'd
>> prefer if you added it for consistency with the neighboring
>> lines.
> 
> This was done so we can add NULL in the places that do not have
> save_one functions.

??? (I cannot connect your response to my remark.)

Jan



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