linux-kernel - Re: [PATCH v4] virtio: add VIRTIO_F_NOTIFICATION

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 30 Mar 2023 13:48:04 -0400
From:   Eric Farman <farman@...ux.ibm.com>
To:     Cornelia Huck <cohuck@...hat.com>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        Viktor Prutyanov <viktor@...nix.com>
Cc:     jasowang@...hat.com, pasic@...ux.ibm.com,
        linux-s390@...r.kernel.org, kvm@...r.kernel.org,
        virtualization@...ts.linux-foundation.org,
        linux-kernel@...r.kernel.org, yan@...nix.com
Subject: Re: [PATCH v4] virtio: add VIRTIO_F_NOTIFICATION_DATA feature
 support

On Wed, 2023-03-22 at 17:42 +0100, Cornelia Huck wrote:
> On Wed, Mar 22 2023, "Michael S. Tsirkin" <mst@...hat.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@...nix.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