Re: [PATCH v4 0/3] Adjust the output of x-query-virtio-status

[Michael S. Tsirkin]

On Wed, Mar 13, 2024 at 09:20:08AM +0100, Markus Armbruster wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Wed, Feb 21, 2024 at 10:28:50PM +0800, Hyman Huang wrote:
> >> v4:
> >> - Rebase on master
> >> - Fix the syntax mistake within the commit message of [PATCH v3 1/3]
> >> - Adjust the linking file in hw/virtio/meson.build suggested by Markus
> >> 
> >> Please review,
> >> Yong
> >
> > I'm still not excited about this.
> > For one this will not scale when we add more than 64 feature bits.
> 
> x-query-virtio-status is meant to be a low effort, low level debugging
> aid.  Its feature set members correspond 1:1 to uint64_t members of the
> C struct, which I figure correspond 1:1 to 64-bit words in the binary
> virtio interface.
> If we run out of bits in the binary virtio interface, I guess we'd add
> another 64-bit word.  The C struct acquires another uint64_t member, and
> so does x-query-virtio-status.
> 
> What's wrong with that?

Nope, that last part about virtio binary interface is wrong. virtio does
not have a 64-bit word in it's ABI, it has an array of bits represented,
depending on a transport, as a dynamically sized array of 32-bit words
(PCI, MMIO) or a dynamically sized array of bytes (CCW).

We are beginning to get closer to filling up 64 bits for some devices
so I'm already looking at not baking 64 bit in new code.

> 
> > As long as we are changing this let's address this please.
> > I would also suggest just keeping the name in there, so
> > a decoded feature will be
> > [0, NAME]
> > and a non-decoded will be just
> > [23]
> >
> > will make for a smaller change.
> 
> I'm not sure I understand your suggestion.
> 
> [...]

For example, for the balloon device:

instead of e.g. 0x201 as this patch would do,
I propose [ [{0, "VIRTIO_BALLOON_F_MUST_TELL_HOST" }, {9, ""}] ].

-- 
MST