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

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Jan 31, 2022 at 04:48:10PM +0100, Cornelia Huck wrote:
> On Mon, Jan 31 2022, "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.
> 
> Do you want to make that dependent on an extra feature bit, then? We
> certainly want to make that facility available if the admin vq is not
> negotiated but another future feature using misc is. (We probably don't
> want to bump the device version.)

I guess we could do that, yes. It's unfortunate that value of unused
registers is undefined for MMIO.

-- 
MST