Re: [Xen-devel] [PATCH 2/5] x86/vioapic: allow the vIO APIC to have a variable number of pins

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 03.03.17 at 16:30, <roger.pau@xxxxxxxxxx> wrote:
> On Fri, Mar 03, 2017 at 06:02:58AM -0700, Jan Beulich wrote:
>> >>> On 03.03.17 at 13:53, <roger.pau@xxxxxxxxxx> wrote:
>> > On Fri, Mar 03, 2017 at 04:56:20AM -0700, Jan Beulich wrote:
>> >> >>> On 23.02.17 at 12:52, <roger.pau@xxxxxxxxxx> wrote:
>> >> > @@ -424,40 +424,141 @@ void vioapic_update_EOI(struct domain *d, u8 
>> >> > vector)
>> >> >      spin_unlock(&d->arch.hvm_domain.irq_lock);
>> >> >  }
>> >> >  
>> >> > -static int ioapic_save(struct domain *d, hvm_domain_context_t *h)
>> >> > +#define VIOAPIC_SAVE_CONST offsetof(struct hvm_hw_vioapic, redirtbl)
>> >> 
>> >> What is this redirtbl field, which oddly enough is a pointer (not allowed
>> >> in the public interface) good for? I can't seem to find any use of it
>> >> other than as marker (for use with offsetof()). With all the
>> >> complications here and below plus the fixed number of entries
>> >> remaining to be fixed for actual migration purposes I wonder if you
>> >> wouldn't be better off by keeping the structure mostly as is, simply
>> >> converting the redirtbl[] field to a union (guarded by a __XEN__
>> >> conditional) containing both a fixed size array and a variable size
>> >> one (or to be precise, a zero size one, as a variable size one is not
>> >> allowed there).
> 
> In that same file hvm_msr struct is using a variable size array defined as
> follows:
> 
> #if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
>     } msr[];
> #elif defined(__GNUC__)
>     } msr[0];
> #else
>     } msr[1 /* variable size */];
> #endif
> 
> I guess using the same should be fine.

Well, the structures here are for migration, and you don't wan to
change the data that gets migrated, so preferably you'd leave the
structures here alone, also ...

> Note that I'm also adding a new field to the struct here (nr_pins) so this is

... in this regard, not the least to ...

> going to look a little bit weird IMHO:

... avoid this weirdness.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel