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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 03.08.18 at 15:53, <aisaila@xxxxxxxxxxxxxxx> wrote:
> @@ -155,7 +152,7 @@ int hvm_save_one(struct domain *d, unsigned int typecode, 
> unsigned int instance,
>      if ( !ctxt.data )
>          return -ENOMEM;
>  
> -    if ( (rv = hvm_sr_handlers[typecode].save(d, &ctxt)) != 0 )
> +    if ( (rv = hvm_sr_handlers[typecode].save(d->vcpu[instance], &ctxt)) != 
> 0 )

There is no bounds check whatsoever before this use of instance as an
array index. You want to check against vCPU count for HVMSR_PER_VCPU
records, and pass vCPU0 for HVMSR_PER_DOM ones. I'm relatively sure
I've said so already on an earlier iteration of the series.

> @@ -107,7 +105,6 @@ typedef int (*hvm_load_handler) (struct domain *d,
>  void hvm_register_savevm(uint16_t typecode,
>                           const char *name, 
>                           hvm_save_handler save_state,
> -                         hvm_save_vcpu_handler save_one,
>                           hvm_load_handler load_state,
>                           size_t size, int kind);
>  
> @@ -117,13 +114,12 @@ 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, _save_one, _load, _num, _k)  \
> +#define HVM_REGISTER_SAVE_RESTORE(_x, _save, _load, _num, _k)  \
>  static int __init __hvm_register_##_x##_save_and_restore(void)            \
>  {                                                                         \
>      hvm_register_savevm(HVM_SAVE_CODE(_x),                                \
>                          #_x,                                              \
>                          &_save,                                           \
> -                        _save_one,                                        \
>                          &_load,                                           \
>                          (_num) * (HVM_SAVE_LENGTH(_x)                     \
>                                    + sizeof (struct hvm_save_descriptor)), \

As to patch splitting: One option looks to be to fold 12 and 13, but
that would make for a pretty big patch doing at least two things at
the same time. Another option could be to simply store NULL here
into the now unused field in order to have what is now patch 12 in
the _next_ step remove that field (and its use in hvm_save()) and
do the renaming (i.e. the dropping of _one).

Jan



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