Re: [PATCH RFC v2 06/13] virtio-pci: implement support for extended features

[Paolo Abeni <pabeni@redhat.com>]

On 7/15/25 6:21 PM, Paolo Abeni wrote:
> On 7/15/25 9:42 AM, Akihiko Odaki wrote:
>> On 2025/07/11 22:02, Paolo Abeni wrote:
>>> @@ -158,7 +159,10 @@ struct VirtIOPCIProxy {
>>>       uint32_t nvectors;
>>>       uint32_t dfselect;
>>>       uint32_t gfselect;
>>> -    uint32_t guest_features[2];
>>> +    union {
>>> +        uint32_t guest_features[2];
>>> +        uint32_t guest_features128[4];
>>> +    };
>>
>> I don't see anything preventing you from directly extending guest_features.
> 
> Uhmm... I have a vague memory of some interim revisions doing that and
> failing miserably (but I have no log of the actual details). I'll try to
> have another shot at it.

The VMSTATE_ARRAY() macro has explicit checks on the specified array
matching exactly the specified array size. Using a single:

	uint32_t guest_features[4];

variable, this statement

	 VMSTATE_UINT32_ARRAY(guest_features, VirtIOPCIProxy, 2),

causes the following build error:

--
include/migration/vmstate.h:259:48: error: invalid operands to binary -
(have ‘uint32_t (*)[2]’ {aka ‘unsigned int (*)[2]’} and ‘uint32_t
(*)[4]’ {aka ‘unsigned int (*)[4]’})
  259 | #define type_check_array(t1,t2,n) ((t1(*)[n])0 - (t2*)0)
      |                                                ^
include/migration/vmstate.h:282:6: note: in expansion of macro
‘type_check_array’
  282 |      type_check_array(_type, typeof_field(_state, _field), _num))
      |      ^~~~~~~~~~~~~~~~
include/migration/vmstate.h:373:19: note: in expansion of macro
‘vmstate_offset_array’
  373 |     .offset     = vmstate_offset_array(_state, _field, _type,
_num), \
      |                   ^~~~~~~~~~~~~~~~~~~~
include/migration/vmstate.h:1090:5: note: in expansion of macro
‘VMSTATE_ARRAY’
 1090 |     VMSTATE_ARRAY(_f, _s, _n, _v, vmstate_info_uint32, uint32_t)
      |     ^~~~~~~~~~~~~
include/migration/vmstate.h:1096:5: note: in expansion of macro
‘VMSTATE_UINT32_ARRAY_V’
 1096 |     VMSTATE_UINT32_ARRAY_V(_f, _s, _n, 0)
      |     ^~~~~~~~~~~~~~~~~~~~~~
../hw/virtio/virtio-pci.c:168:9: note: in expansion of macro
‘VMSTATE_UINT32_ARRAY’
  168 |         VMSTATE_UINT32_ARRAY(guest_features, VirtIOPCIProxy, 2),
      |         ^~~~~~~~~~~~~~~~~~~~
--

I'll keep the union here.

Thanks,

Paolo