Re: [virtio-dev] Re: [PATCH v2 1/4] Add virtio Admin Virtqueue

[Cornelia Huck <cohuck@redhat.com>]

On Mon, Jan 31 2022, Halil Pasic <pasic@linux.ibm.com> wrote:

> On Mon, 31 Jan 2022 10:16:43 +0100
> Cornelia Huck <cohuck@redhat.com> wrote:
>> For ccw, I'd do something like
>> 
>> #define CCW_CMD_READ_MISC_CONF 0x82
>> 
>> struct virtio_misc_conf {
>>        be16 admin_queue_index;
>> };
>> 
>> bound to revision 3, which gets a payload data containing the length of
>> this structure (for future expansions).
>> 
>> Halil, do you think that would work?
>
>
> I think so. But I would like to review the actual proposal :)
>
> Some of the questions I have in mind are:
> * Are fields in this config protected with feature bits

I'd say they exist, as long as they are contained in the length provided
via the revision data. Whether the data in the field is valid depends on
feature bits.

> * Do we ever need to write this config? Adding a writable field to pci
>   is easier than to ccw (no need to invent a new ccw)

If we need that, we need to define a companion write command. If we
already think we might need to write some fields in the future, we could
introduce that command already right now.

> * Looks like we will have an addressability mismatch with
>   ccw compared to pci and mmio: ccw can only read a prefix of this new
>   config, while mmio and pci have access to the individual fields

Yeah, that's basically like the config space access, which we're also
modelling with reading up to the requested field. I'm not sure whether
there's a better way to model this.

> * Is this static stuff only? If not, do we need notifications for config
>   change like for the device specific config?

This needs more thought :) To the others reading this thread: do you
think "a mechanism for the device to make various extra configuration
information available to the driver" (i.e. static and read-only) is
enough, or do you think we need more? I think "initiate an action" like
the queue reset is not neccessarily served best by an interface like
this (I had thought about a multiplexing "initiate action" ccw for that
purpose.) The different transports likely have different preferences
here.