linux-kernel - Re: [PATCH v6] 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <AM0PR04MB4723A8D079076FA56AB45845D48C9@AM0PR04MB4723.eurprd04.prod.outlook.com>
Date:   Sun, 2 Apr 2023 08:17:49 +0000
From:   Alvaro Karsz <alvaro.karsz@...id-run.com>
To:     Viktor Prutyanov <viktor@...nix.com>,
        "mst@...hat.com" <mst@...hat.com>,
        "jasowang@...hat.com" <jasowang@...hat.com>
CC:     "cohuck@...hat.com" <cohuck@...hat.com>,
        "pasic@...ux.ibm.com" <pasic@...ux.ibm.com>,
        "farman@...ux.ibm.com" <farman@...ux.ibm.com>,
        "linux-s390@...r.kernel.org" <linux-s390@...r.kernel.org>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "virtualization@...ts.linux-foundation.org" 
        <virtualization@...ts.linux-foundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "yan@...nix.com" <yan@...nix.com>
Subject: Re: [PATCH v6] virtio: add VIRTIO_F_NOTIFICATION_DATA feature support

Hi Viktor,

> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 4c3bb0ddeb9b..f9c6604352b4 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -2752,6 +2752,23 @@ void vring_del_virtqueue(struct virtqueue *_vq)
>  }
>  EXPORT_SYMBOL_GPL(vring_del_virtqueue);
> 
> +u32 vring_notification_data(struct virtqueue *_vq)
> +{
> +       struct vring_virtqueue *vq = to_vvq(_vq);
> +       u16 next;
> +
> +       if (vq->packed_ring)
> +               next = (vq->packed.next_avail_idx &
> +                               ~(-(1 << VRING_PACKED_EVENT_F_WRAP_CTR))) |
> +                       vq->packed.avail_wrap_counter <<
> +                               VRING_PACKED_EVENT_F_WRAP_CTR;
> +       else
> +               next = vq->split.avail_idx_shadow;
> +
> +       return next << 16 | _vq->index;
> +}
> +EXPORT_SYMBOL_GPL(vring_notification_data);
> +
>  /* Manipulates transport-specific feature bits. */
>  void vring_transport_features(struct virtio_device *vdev)
>  {
> @@ -2771,6 +2788,8 @@ void vring_transport_features(struct virtio_device *vdev)
>                         break;
>                 case VIRTIO_F_ORDER_PLATFORM:
>                         break;
> +               case VIRTIO_F_NOTIFICATION_DATA:
> +                       break;

This function is used by virtio_vdpa as well (drivers/virtio/virtio_vdpa.c:virtio_vdpa_finalize_features).
A vDPA device can offer this feature and it will be accepted, even though VIRTIO_F_NOTIFICATION_DATA is not a thing for the vDPA transport at the moment.

I don't know if this is bad, since offering VIRTIO_F_NOTIFICATION_DATA is meaningless for a vDPA device at the moment.

I submitted a patch adding support for vDPA transport.
https://lore.kernel.org/virtualization/20230402081034.1021886-1-alvaro.karsz@solid-run.com/T/#u

>                 default:
>                         /* We don't understand this bit. */
>                         __virtio_clear_bit(vdev, i);