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

[Halil Pasic <pasic@linux.ibm.com>]

On Mon, 31 Jan 2022 09:52:54 -0500
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Jan 31, 2022 at 03:26:36PM +0100, Cornelia Huck wrote:
> > On Mon, Jan 31 2022, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >   
> > > On Mon, Jan 31, 2022 at 10:16:43AM +0100, Cornelia Huck wrote:  
> > >> On Sun, Jan 30 2022, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > >>   
> > >> > On Sun, Jan 30, 2022 at 05:12:46PM +0200, Max Gurtovoy wrote:  
> > >> >> #define VIRTIO_PCI_CAP_MISC_CFG 10
> > >> >> 
> > >> >> and
> > >> >> 
> > >> >> struct virtio_pci_misc_cfg {
> > >> >>     le16 admin_queue_index; /* read-only for driver */
> > >> >> };
> > >> >> 
> > >> >> Is agreed by all for V3 ? instead of the net and blk AQ index definitions.  
> > >> >
> > >> > We need to add it to MMIO and CCW I guess too.  
> > >> 
> > >> That seems ok for pci.
> > >> 
> > >> 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?
> > >> 
> > >> For mmio, I'd need to think a bit more. Any mmio experts around?  
> > >
> > > Not an expert but I think we can rely on a feature
> > > bit to be acked since admin vq is only needed
> > > after feature negotiation is complete.  
> > 
> > You mean a register that is valid conditionally? I don't see an easy way
> > to add some kind of "misc" interface for mmio, unlike for the other
> > transports.
> > 
> > So something like:
> > 
> > AdminQueueIndex/0x0c4/R
> > If VIRTIO_F_ADMIN_VQ has been negotiated, reading from this register
> > returns the queue index of the administration virtqueue.  
> 
> No, I mean a register that switches 100+ between device specific
> and misc space.
> 

Maybe adding a register that tells us where does the "misc config
start" is another option. I don't think we need an open ended
device-config in practice. I have no idea if there are any upper limits
on MMIO address space though. If we are constrained there, the switching
is certainly more efficient. Otherwise, I think having the misc config
somewhere after device specific config is simpler.

BTW I don't really like this "misc" as the name. FWIW it is less
miscellaneous that the device specific config, since it is common
for all devices. I don't have a good name for it, but I would prefer
"common" over "misc"

Regards,
Halil