Re: [PATCH v4] virtio: add VIRTIO_F_NOTIFICATION_DATA feature support

[Eric Farman <farman@linux.ibm.com>]

On Wed, 2023-03-22 at 17:42 +0100, Cornelia Huck wrote:
> On Wed, Mar 22 2023, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Wed, Mar 22, 2023 at 05:10:31PM +0300, Viktor Prutyanov wrote:
> > > According to VirtIO spec v1.2, VIRTIO_F_NOTIFICATION_DATA feature
> > > indicates that the driver passes extra data along with the queue
> > > notifications.
> > > 
> > > In a split queue case, the extra data is 16-bit available index.
> > > In a
> > > packed queue case, the extra data is 1-bit wrap counter and 15-
> > > bit
> > > available index.
> > > 
> > > Add support for this feature for MMIO, channel I/O and modern PCI
> > > transports.
> > > 
> > > Signed-off-by: Viktor Prutyanov <viktor@daynix.com>
> > > ---
> > >  v4: remove VP_NOTIFY macro and legacy PCI support, add
> > >     virtio_ccw_kvm_notify_with_data to virtio_ccw
> > >  v3: support feature in virtio_ccw, remove VM_NOTIFY, use
> > > avail_idx_shadow,
> > >     remove byte swap, rename to vring_notification_data
> > >  v2: reject the feature in virtio_ccw, replace __le32 with u32
> > > 
> > >  Tested with disabled VIRTIO_F_NOTIFICATION_DATA on qemu-system-
> > > s390x
> > >  (virtio-blk-ccw), qemu-system-riscv64 (virtio-blk-device,
> > >  virtio-rng-device), qemu-system-x86_64 (virtio-blk-pci, virtio-
> > > net-pci)
> > >  to make sure nothing is broken.
> > >  Tested with enabled VIRTIO_F_NOTIFICATION_DATA on 64-bit RISC-V
> > > Linux
> > >  and my hardware implementation of virtio-rng.
> > 
> > what did you test? virtio pci? mmio? guessing not ccw...
> > 
> > Cornelia could you hack up something to quickly test ccw?
> 
> Hm, I'm not entirely sure how notification data is supposed to be
> used
> in real life -- Viktor, what is your virtio-rng implementation doing;
> can this be hacked into all transports?
> 
> (Also, if the other ccw folks have something handy, please speak up
> :)
> 

(Sorry for delay, caught the illness going through our house before I
had a chance to look at this.)

I applied v6 of this patch and hacked to QEMU to enable
VIRTIO_F_NOTIFICATION_DATA for virtio-blk, and have the -ccw code
ignore the additional data that then comes along the way. Not
surprisingly, things behave fine once I accommodate the new payload.

To Cornelia's point of how it should be used in real life... I didn't
go beyond using this info as a "debugging aid" here, based on time
constraints. But at least it looks reasonable.

Eric