|
From: | Jonah Palmer |
Subject: | Re: [PATCH v1 2/8] virtio-pci: Lock ioeventfd state with VIRTIO_F_NOTIFICATION_DATA |
Date: | Mon, 11 Mar 2024 10:53:25 -0400 |
User-agent: | Mozilla Thunderbird |
On 3/8/24 2:19 PM, Michael S. Tsirkin wrote:
On Fri, Mar 08, 2024 at 12:45:13PM -0500, Jonah Palmer wrote:On 3/8/24 12:36 PM, Eugenio Perez Martin wrote:On Fri, Mar 8, 2024 at 6: 01 PM Michael S. Tsirkin <mst@ redhat. com> wrote: > > On Mon, Mar 04, 2024 at 02: 46: 06PM -0500, Jonah Palmer wrote: > > Prevent ioeventfd from being enabled/disabled when a virtio-pci > > device ZjQcmQRYFpfptBannerStart This Message Is From an External Sender This message came from outside your organization. Report Suspicious <https://us-phishalarm-ewt.proofpoint.com/EWT/v1/ACWV5N9M2RV99hQ!Op20OCZE8kFi__wOXJ_Z0URZ2e_9fdaYz2tejZvKqiDgOm6ijq_imUptzxsrej_4riwCrBGeKmQ9VKXqnbV1ujbfiOV5-E2e1s3pKqpqUL-gRIuMQLDLygRD1hoX3Q$> ZjQcmQRYFpfptBannerEnd On Fri, Mar 8, 2024 at 6:01 PM Michael S. Tsirkin <mst@redhat.com> wrote:On Mon, Mar 04, 2024 at 02:46:06PM -0500, Jonah Palmer wrote:Prevent ioeventfd from being enabled/disabled when a virtio-pci device has negotiated the VIRTIO_F_NOTIFICATION_DATA transport feature. Due to ioeventfd not being able to carry the extra data associated with this feature, the ioeventfd should be left in a disabled state for emulated virtio-pci devices using this feature. Reviewed-by: Eugenio Pérez <eperezma@redhat.com> Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>I thought hard about this. I propose that for now, instead of disabling ioevetfd silently we error out unless user disabled it for us. WDYT?Yes, error is a better plan than silently disabling it. In the (unlikely?) case we are able to make notification data work with eventfd in the future, it makes the change more evident.Will do in v2. I assume we'll also make this the case for virtio-mmio and virtio-ccw?Guess so. Pls note freeze is imminent.
Got it. Also, would you mind elaborating a bit more on "error out"? E.g. do we want to prevent the Qemu from starting at all if a device is attempting to use both VIRTIO_F_NOTIFICATION_DATA and ioeventfd? Or do you mean something like still keep ioeventfd disabled but also log an error message unless it was explicitly disabled by the user?
--- hw/virtio/virtio-pci.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index d12edc567f..287b8f7720 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -417,13 +417,15 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val) } break; case VIRTIO_PCI_STATUS: - if (!(val & VIRTIO_CONFIG_S_DRIVER_OK)) { + if (!(val & VIRTIO_CONFIG_S_DRIVER_OK) && + !virtio_vdev_has_feature(vdev, VIRTIO_F_NOTIFICATION_DATA)) { virtio_pci_stop_ioeventfd(proxy); } virtio_set_status(vdev, val & 0xFF); - if (val & VIRTIO_CONFIG_S_DRIVER_OK) { + if ((val & VIRTIO_CONFIG_S_DRIVER_OK) && + !virtio_vdev_has_feature(vdev, VIRTIO_F_NOTIFICATION_DATA)) { virtio_pci_start_ioeventfd(proxy); } -- 2.39.3
[Prev in Thread] | Current Thread | [Next in Thread] |