* [PULL v2 01/91] vhost: dirty log should be per backend type
2024-07-02 20:13 [PULL v2 00/91] virtio: features,fixes Michael S. Tsirkin
@ 2024-07-02 20:13 ` Michael S. Tsirkin
2024-07-02 20:13 ` [PULL v2 02/91] vhost: Perform memory section dirty scans once per iteration Michael S. Tsirkin
` (17 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2024-07-02 20:13 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Si-Wei Liu
From: Si-Wei Liu <si-wei.liu@oracle.com>
There could be a mix of both vhost-user and vhost-kernel clients
in the same QEMU process, where separate vhost loggers for the
specific vhost type have to be used. Make the vhost logger per
backend type, and have them properly reference counted.
Suggested-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
Message-Id: <1710448055-11709-1-git-send-email-si-wei.liu@oracle.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/virtio/vhost.c | 45 +++++++++++++++++++++++++++++++++------------
1 file changed, 33 insertions(+), 12 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 4acd77e890..a1e8b79e1a 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -43,8 +43,8 @@
do { } while (0)
#endif
-static struct vhost_log *vhost_log;
-static struct vhost_log *vhost_log_shm;
+static struct vhost_log *vhost_log[VHOST_BACKEND_TYPE_MAX];
+static struct vhost_log *vhost_log_shm[VHOST_BACKEND_TYPE_MAX];
/* Memslots used by backends that support private memslots (without an fd). */
static unsigned int used_memslots;
@@ -287,6 +287,10 @@ static int vhost_set_backend_type(struct vhost_dev *dev,
r = -1;
}
+ if (r == 0) {
+ assert(dev->vhost_ops->backend_type == backend_type);
+ }
+
return r;
}
@@ -319,16 +323,22 @@ static struct vhost_log *vhost_log_alloc(uint64_t size, bool share)
return log;
}
-static struct vhost_log *vhost_log_get(uint64_t size, bool share)
+static struct vhost_log *vhost_log_get(VhostBackendType backend_type,
+ uint64_t size, bool share)
{
- struct vhost_log *log = share ? vhost_log_shm : vhost_log;
+ struct vhost_log *log;
+
+ assert(backend_type > VHOST_BACKEND_TYPE_NONE);
+ assert(backend_type < VHOST_BACKEND_TYPE_MAX);
+
+ log = share ? vhost_log_shm[backend_type] : vhost_log[backend_type];
if (!log || log->size != size) {
log = vhost_log_alloc(size, share);
if (share) {
- vhost_log_shm = log;
+ vhost_log_shm[backend_type] = log;
} else {
- vhost_log = log;
+ vhost_log[backend_type] = log;
}
} else {
++log->refcnt;
@@ -340,11 +350,20 @@ static struct vhost_log *vhost_log_get(uint64_t size, bool share)
static void vhost_log_put(struct vhost_dev *dev, bool sync)
{
struct vhost_log *log = dev->log;
+ VhostBackendType backend_type;
if (!log) {
return;
}
+ assert(dev->vhost_ops);
+ backend_type = dev->vhost_ops->backend_type;
+
+ if (backend_type == VHOST_BACKEND_TYPE_NONE ||
+ backend_type >= VHOST_BACKEND_TYPE_MAX) {
+ return;
+ }
+
--log->refcnt;
if (log->refcnt == 0) {
/* Sync only the range covered by the old log */
@@ -352,13 +371,13 @@ static void vhost_log_put(struct vhost_dev *dev, bool sync)
vhost_log_sync_range(dev, 0, dev->log_size * VHOST_LOG_CHUNK - 1);
}
- if (vhost_log == log) {
+ if (vhost_log[backend_type] == log) {
g_free(log->log);
- vhost_log = NULL;
- } else if (vhost_log_shm == log) {
+ vhost_log[backend_type] = NULL;
+ } else if (vhost_log_shm[backend_type] == log) {
qemu_memfd_free(log->log, log->size * sizeof(*(log->log)),
log->fd);
- vhost_log_shm = NULL;
+ vhost_log_shm[backend_type] = NULL;
}
g_free(log);
@@ -376,7 +395,8 @@ static bool vhost_dev_log_is_shared(struct vhost_dev *dev)
static inline void vhost_dev_log_resize(struct vhost_dev *dev, uint64_t size)
{
- struct vhost_log *log = vhost_log_get(size, vhost_dev_log_is_shared(dev));
+ struct vhost_log *log = vhost_log_get(dev->vhost_ops->backend_type,
+ size, vhost_dev_log_is_shared(dev));
uint64_t log_base = (uintptr_t)log->log;
int r;
@@ -2044,7 +2064,8 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev, bool vrings)
uint64_t log_base;
hdev->log_size = vhost_get_log_size(hdev);
- hdev->log = vhost_log_get(hdev->log_size,
+ hdev->log = vhost_log_get(hdev->vhost_ops->backend_type,
+ hdev->log_size,
vhost_dev_log_is_shared(hdev));
log_base = (uintptr_t)hdev->log->log;
r = hdev->vhost_ops->vhost_set_log_base(hdev,
--
MST
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL v2 02/91] vhost: Perform memory section dirty scans once per iteration
2024-07-02 20:13 [PULL v2 00/91] virtio: features,fixes Michael S. Tsirkin
2024-07-02 20:13 ` [PULL v2 01/91] vhost: dirty log should be per backend type Michael S. Tsirkin
@ 2024-07-02 20:13 ` Michael S. Tsirkin
2024-07-02 20:13 ` [PULL v2 03/91] vhost-vdpa: check vhost_vdpa_set_vring_ready() return value Michael S. Tsirkin
` (16 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2024-07-02 20:13 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Si-Wei Liu, Joao Martins, Jason Wang
From: Si-Wei Liu <si-wei.liu@oracle.com>
On setups with one or more virtio-net devices with vhost on,
dirty tracking iteration increases cost the bigger the number
amount of queues are set up e.g. on idle guests migration the
following is observed with virtio-net with vhost=on:
48 queues -> 78.11% [.] vhost_dev_sync_region.isra.13
8 queues -> 40.50% [.] vhost_dev_sync_region.isra.13
1 queue -> 6.89% [.] vhost_dev_sync_region.isra.13
2 devices, 1 queue -> 18.60% [.] vhost_dev_sync_region.isra.14
With high memory rates the symptom is lack of convergence as soon
as it has a vhost device with a sufficiently high number of queues,
the sufficient number of vhost devices.
On every migration iteration (every 100msecs) it will redundantly
query the *shared log* the number of queues configured with vhost
that exist in the guest. For the virtqueue data, this is necessary,
but not for the memory sections which are the same. So essentially
we end up scanning the dirty log too often.
To fix that, select a vhost device responsible for scanning the
log with regards to memory sections dirty tracking. It is selected
when we enable the logger (during migration) and cleared when we
disable the logger. If the vhost logger device goes away for some
reason, the logger will be re-selected from the rest of vhost
devices.
After making mem-section logger a singleton instance, constant cost
of 7%-9% (like the 1 queue report) will be seen, no matter how many
queues or how many vhost devices are configured:
48 queues -> 8.71% [.] vhost_dev_sync_region.isra.13
2 devices, 8 queues -> 7.97% [.] vhost_dev_sync_region.isra.14
Co-developed-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
Message-Id: <1710448055-11709-2-git-send-email-si-wei.liu@oracle.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
---
include/hw/virtio/vhost.h | 1 +
hw/virtio/vhost.c | 67 +++++++++++++++++++++++++++++++++++----
2 files changed, 62 insertions(+), 6 deletions(-)
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 02477788df..d75faf46e9 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -129,6 +129,7 @@ struct vhost_dev {
void *opaque;
struct vhost_log *log;
QLIST_ENTRY(vhost_dev) entry;
+ QLIST_ENTRY(vhost_dev) logdev_entry;
QLIST_HEAD(, vhost_iommu) iommu_list;
IOMMUNotifier n;
const VhostDevConfigOps *config_ops;
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index a1e8b79e1a..06fc71746e 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -45,6 +45,7 @@
static struct vhost_log *vhost_log[VHOST_BACKEND_TYPE_MAX];
static struct vhost_log *vhost_log_shm[VHOST_BACKEND_TYPE_MAX];
+static QLIST_HEAD(, vhost_dev) vhost_log_devs[VHOST_BACKEND_TYPE_MAX];
/* Memslots used by backends that support private memslots (without an fd). */
static unsigned int used_memslots;
@@ -149,6 +150,47 @@ bool vhost_dev_has_iommu(struct vhost_dev *dev)
}
}
+static inline bool vhost_dev_should_log(struct vhost_dev *dev)
+{
+ assert(dev->vhost_ops);
+ assert(dev->vhost_ops->backend_type > VHOST_BACKEND_TYPE_NONE);
+ assert(dev->vhost_ops->backend_type < VHOST_BACKEND_TYPE_MAX);
+
+ return dev == QLIST_FIRST(&vhost_log_devs[dev->vhost_ops->backend_type]);
+}
+
+static inline void vhost_dev_elect_mem_logger(struct vhost_dev *hdev, bool add)
+{
+ VhostBackendType backend_type;
+
+ assert(hdev->vhost_ops);
+
+ backend_type = hdev->vhost_ops->backend_type;
+ assert(backend_type > VHOST_BACKEND_TYPE_NONE);
+ assert(backend_type < VHOST_BACKEND_TYPE_MAX);
+
+ if (add && !QLIST_IS_INSERTED(hdev, logdev_entry)) {
+ if (QLIST_EMPTY(&vhost_log_devs[backend_type])) {
+ QLIST_INSERT_HEAD(&vhost_log_devs[backend_type],
+ hdev, logdev_entry);
+ } else {
+ /*
+ * The first vhost_device in the list is selected as the shared
+ * logger to scan memory sections. Put new entry next to the head
+ * to avoid inadvertent change to the underlying logger device.
+ * This is done in order to get better cache locality and to avoid
+ * performance churn on the hot path for log scanning. Even when
+ * new devices come and go quickly, it wouldn't end up changing
+ * the active leading logger device at all.
+ */
+ QLIST_INSERT_AFTER(QLIST_FIRST(&vhost_log_devs[backend_type]),
+ hdev, logdev_entry);
+ }
+ } else if (!add && QLIST_IS_INSERTED(hdev, logdev_entry)) {
+ QLIST_REMOVE(hdev, logdev_entry);
+ }
+}
+
static int vhost_sync_dirty_bitmap(struct vhost_dev *dev,
MemoryRegionSection *section,
hwaddr first,
@@ -166,12 +208,14 @@ static int vhost_sync_dirty_bitmap(struct vhost_dev *dev,
start_addr = MAX(first, start_addr);
end_addr = MIN(last, end_addr);
- for (i = 0; i < dev->mem->nregions; ++i) {
- struct vhost_memory_region *reg = dev->mem->regions + i;
- vhost_dev_sync_region(dev, section, start_addr, end_addr,
- reg->guest_phys_addr,
- range_get_last(reg->guest_phys_addr,
- reg->memory_size));
+ if (vhost_dev_should_log(dev)) {
+ for (i = 0; i < dev->mem->nregions; ++i) {
+ struct vhost_memory_region *reg = dev->mem->regions + i;
+ vhost_dev_sync_region(dev, section, start_addr, end_addr,
+ reg->guest_phys_addr,
+ range_get_last(reg->guest_phys_addr,
+ reg->memory_size));
+ }
}
for (i = 0; i < dev->nvqs; ++i) {
struct vhost_virtqueue *vq = dev->vqs + i;
@@ -383,6 +427,7 @@ static void vhost_log_put(struct vhost_dev *dev, bool sync)
g_free(log);
}
+ vhost_dev_elect_mem_logger(dev, false);
dev->log = NULL;
dev->log_size = 0;
}
@@ -998,6 +1043,15 @@ static int vhost_dev_set_log(struct vhost_dev *dev, bool enable_log)
goto err_vq;
}
}
+
+ /*
+ * At log start we select our vhost_device logger that will scan the
+ * memory sections and skip for the others. This is possible because
+ * the log is shared amongst all vhost devices for a given type of
+ * backend.
+ */
+ vhost_dev_elect_mem_logger(dev, enable_log);
+
return 0;
err_vq:
for (; i >= 0; --i) {
@@ -2075,6 +2129,7 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev, bool vrings)
VHOST_OPS_DEBUG(r, "vhost_set_log_base failed");
goto fail_log;
}
+ vhost_dev_elect_mem_logger(hdev, true);
}
if (vrings) {
r = vhost_dev_set_vring_enable(hdev, true);
--
MST
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL v2 03/91] vhost-vdpa: check vhost_vdpa_set_vring_ready() return value
2024-07-02 20:13 [PULL v2 00/91] virtio: features,fixes Michael S. Tsirkin
2024-07-02 20:13 ` [PULL v2 01/91] vhost: dirty log should be per backend type Michael S. Tsirkin
2024-07-02 20:13 ` [PULL v2 02/91] vhost: Perform memory section dirty scans once per iteration Michael S. Tsirkin
@ 2024-07-02 20:13 ` Michael S. Tsirkin
2024-07-02 20:13 ` [PULL v2 04/91] virtio/virtio-pci: Handle extra notification data Michael S. Tsirkin
` (15 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2024-07-02 20:13 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Stefano Garzarella, Eugenio Pérez, Jason Wang,
Philippe Mathieu-Daudé
From: Stefano Garzarella <sgarzare@redhat.com>
vhost_vdpa_set_vring_ready() could already fail, but if Linux's
patch [1] will be merged, it may fail with more chance if
userspace does not activate virtqueues before DRIVER_OK when
VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK is not negotiated.
So better check its return value anyway.
[1] https://lore.kernel.org/virtualization/20240206145154.118044-1-sgarzare@redhat.com/T/#u
Acked-by: Eugenio Pérez <eperezma@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Message-Id: <20240322092315.31885-1-sgarzare@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
net/vhost-vdpa.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 85e73dd6a7..eda714d1a4 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -399,7 +399,10 @@ static int vhost_vdpa_net_data_load(NetClientState *nc)
}
for (int i = 0; i < v->dev->nvqs; ++i) {
- vhost_vdpa_set_vring_ready(v, i + v->dev->vq_index);
+ int ret = vhost_vdpa_set_vring_ready(v, i + v->dev->vq_index);
+ if (ret < 0) {
+ return ret;
+ }
}
return 0;
}
@@ -1238,7 +1241,10 @@ static int vhost_vdpa_net_cvq_load(NetClientState *nc)
assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA);
- vhost_vdpa_set_vring_ready(v, v->dev->vq_index);
+ r = vhost_vdpa_set_vring_ready(v, v->dev->vq_index);
+ if (unlikely(r < 0)) {
+ return r;
+ }
if (v->shadow_vqs_enabled) {
n = VIRTIO_NET(v->dev->vdev);
@@ -1277,7 +1283,10 @@ static int vhost_vdpa_net_cvq_load(NetClientState *nc)
}
for (int i = 0; i < v->dev->vq_index; ++i) {
- vhost_vdpa_set_vring_ready(v, i);
+ r = vhost_vdpa_set_vring_ready(v, i);
+ if (unlikely(r < 0)) {
+ return r;
+ }
}
return 0;
--
MST
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL v2 04/91] virtio/virtio-pci: Handle extra notification data
2024-07-02 20:13 [PULL v2 00/91] virtio: features,fixes Michael S. Tsirkin
` (2 preceding siblings ...)
2024-07-02 20:13 ` [PULL v2 03/91] vhost-vdpa: check vhost_vdpa_set_vring_ready() return value Michael S. Tsirkin
@ 2024-07-02 20:13 ` Michael S. Tsirkin
2024-07-02 20:13 ` [PULL v2 05/91] virtio: Prevent creation of device using notification-data with ioeventfd Michael S. Tsirkin
` (14 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2024-07-02 20:13 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Jonah Palmer, Eugenio Pérez
From: Jonah Palmer <jonah.palmer@oracle.com>
Add support to virtio-pci devices for handling the extra data sent
from the driver to the device when the VIRTIO_F_NOTIFICATION_DATA
transport feature has been negotiated.
The extra data that's passed to the virtio-pci device when this
feature is enabled varies depending on the device's virtqueue
layout.
In a split virtqueue layout, this data includes:
- upper 16 bits: shadow_avail_idx
- lower 16 bits: virtqueue index
In a packed virtqueue layout, this data includes:
- upper 16 bits: 1-bit wrap counter & 15-bit shadow_avail_idx
- lower 16 bits: virtqueue index
Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
Message-Id: <20240315165557.26942-2-jonah.palmer@oracle.com>
Reviewed-by: Eugenio Pérez <eperezma@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/hw/virtio/virtio.h | 2 ++
hw/virtio/virtio-pci.c | 12 +++++++++---
hw/virtio/virtio.c | 18 ++++++++++++++++++
3 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 7d5ffdc145..1451926a13 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -307,6 +307,8 @@ int virtio_queue_ready(VirtQueue *vq);
int virtio_queue_empty(VirtQueue *vq);
+void virtio_queue_set_shadow_avail_idx(VirtQueue *vq, uint16_t idx);
+
/* Host binding interface. */
uint32_t virtio_config_readb(VirtIODevice *vdev, uint32_t addr);
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index b1d02f4b3d..cffc7efcae 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -384,7 +384,7 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
{
VirtIOPCIProxy *proxy = opaque;
VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
- uint16_t vector;
+ uint16_t vector, vq_idx;
hwaddr pa;
switch (addr) {
@@ -408,8 +408,14 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
vdev->queue_sel = val;
break;
case VIRTIO_PCI_QUEUE_NOTIFY:
- if (val < VIRTIO_QUEUE_MAX) {
- virtio_queue_notify(vdev, val);
+ vq_idx = val;
+ if (vq_idx < VIRTIO_QUEUE_MAX && virtio_queue_get_num(vdev, vq_idx)) {
+ if (virtio_vdev_has_feature(vdev, VIRTIO_F_NOTIFICATION_DATA)) {
+ VirtQueue *vq = virtio_get_queue(vdev, vq_idx);
+
+ virtio_queue_set_shadow_avail_idx(vq, val >> 16);
+ }
+ virtio_queue_notify(vdev, vq_idx);
}
break;
case VIRTIO_PCI_STATUS:
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 893a072c9d..f7c99e3a96 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2264,6 +2264,24 @@ void virtio_queue_set_align(VirtIODevice *vdev, int n, int align)
}
}
+void virtio_queue_set_shadow_avail_idx(VirtQueue *vq, uint16_t shadow_avail_idx)
+{
+ if (!vq->vring.desc) {
+ return;
+ }
+
+ /*
+ * 16-bit data for packed VQs include 1-bit wrap counter and
+ * 15-bit shadow_avail_idx.
+ */
+ if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
+ vq->shadow_avail_wrap_counter = (shadow_avail_idx >> 15) & 0x1;
+ vq->shadow_avail_idx = shadow_avail_idx & 0x7FFF;
+ } else {
+ vq->shadow_avail_idx = shadow_avail_idx;
+ }
+}
+
static void virtio_queue_notify_vq(VirtQueue *vq)
{
if (vq->vring.desc && vq->handle_output) {
--
MST
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL v2 05/91] virtio: Prevent creation of device using notification-data with ioeventfd
2024-07-02 20:13 [PULL v2 00/91] virtio: features,fixes Michael S. Tsirkin
` (3 preceding siblings ...)
2024-07-02 20:13 ` [PULL v2 04/91] virtio/virtio-pci: Handle extra notification data Michael S. Tsirkin
@ 2024-07-02 20:13 ` Michael S. Tsirkin
2024-07-02 20:13 ` [PULL v2 06/91] virtio-mmio: Handle extra notification data Michael S. Tsirkin
` (13 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2024-07-02 20:13 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Jonah Palmer
From: Jonah Palmer <jonah.palmer@oracle.com>
Prevent the realization of a virtio device that attempts to use the
VIRTIO_F_NOTIFICATION_DATA transport feature without disabling
ioeventfd.
Due to ioeventfd not being able to carry the extra data associated with
this feature, having both enabled is a functional mismatch and therefore
Qemu should not continue the device's realization process.
Although the device does not yet know if the feature will be
successfully negotiated, many devices using this feature wont actually
work without this extra data and would fail FEATURES_OK anyway.
If ioeventfd is able to work with the extra notification data in the
future, this compatibility check can be removed.
Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
Message-Id: <20240315165557.26942-3-jonah.palmer@oracle.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/virtio/virtio.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index f7c99e3a96..28cd406e16 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2980,6 +2980,20 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val)
return ret;
}
+static void virtio_device_check_notification_compatibility(VirtIODevice *vdev,
+ Error **errp)
+{
+ VirtioBusState *bus = VIRTIO_BUS(qdev_get_parent_bus(DEVICE(vdev)));
+ VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
+ DeviceState *proxy = DEVICE(BUS(bus)->parent);
+
+ if (virtio_host_has_feature(vdev, VIRTIO_F_NOTIFICATION_DATA) &&
+ k->ioeventfd_enabled(proxy)) {
+ error_setg(errp,
+ "notification_data=on without ioeventfd=off is not supported");
+ }
+}
+
size_t virtio_get_config_size(const VirtIOConfigSizeParams *params,
uint64_t host_features)
{
@@ -3740,6 +3754,14 @@ static void virtio_device_realize(DeviceState *dev, Error **errp)
}
}
+ /* Devices should not use both ioeventfd and notification data feature */
+ virtio_device_check_notification_compatibility(vdev, &err);
+ if (err != NULL) {
+ error_propagate(errp, err);
+ vdc->unrealize(dev);
+ return;
+ }
+
virtio_bus_device_plugged(vdev, &err);
if (err != NULL) {
error_propagate(errp, err);
--
MST
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL v2 06/91] virtio-mmio: Handle extra notification data
2024-07-02 20:13 [PULL v2 00/91] virtio: features,fixes Michael S. Tsirkin
` (4 preceding siblings ...)
2024-07-02 20:13 ` [PULL v2 05/91] virtio: Prevent creation of device using notification-data with ioeventfd Michael S. Tsirkin
@ 2024-07-02 20:13 ` Michael S. Tsirkin
2024-07-02 20:14 ` [PULL v2 07/91] virtio-ccw: " Michael S. Tsirkin
` (12 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2024-07-02 20:13 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Jonah Palmer
From: Jonah Palmer <jonah.palmer@oracle.com>
Add support to virtio-mmio devices for handling the extra data sent from
the driver to the device when the VIRTIO_F_NOTIFICATION_DATA transport
feature has been negotiated.
The extra data that's passed to the virtio-mmio device when this feature
is enabled varies depending on the device's virtqueue layout.
The data passed to the virtio-mmio device is in the same format as the
data passed to virtio-pci devices.
Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
Message-Id: <20240315165557.26942-4-jonah.palmer@oracle.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/virtio/virtio-mmio.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index 22f9fbcf5a..320428ac0d 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -248,6 +248,7 @@ static void virtio_mmio_write(void *opaque, hwaddr offset, uint64_t value,
{
VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque;
VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+ uint16_t vq_idx;
trace_virtio_mmio_write_offset(offset, value);
@@ -407,8 +408,14 @@ static void virtio_mmio_write(void *opaque, hwaddr offset, uint64_t value,
}
break;
case VIRTIO_MMIO_QUEUE_NOTIFY:
- if (value < VIRTIO_QUEUE_MAX) {
- virtio_queue_notify(vdev, value);
+ vq_idx = value;
+ if (vq_idx < VIRTIO_QUEUE_MAX && virtio_queue_get_num(vdev, vq_idx)) {
+ if (virtio_vdev_has_feature(vdev, VIRTIO_F_NOTIFICATION_DATA)) {
+ VirtQueue *vq = virtio_get_queue(vdev, vq_idx);
+
+ virtio_queue_set_shadow_avail_idx(vq, (value >> 16) & 0xFFFF);
+ }
+ virtio_queue_notify(vdev, vq_idx);
}
break;
case VIRTIO_MMIO_INTERRUPT_ACK:
--
MST
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL v2 07/91] virtio-ccw: Handle extra notification data
2024-07-02 20:13 [PULL v2 00/91] virtio: features,fixes Michael S. Tsirkin
` (5 preceding siblings ...)
2024-07-02 20:13 ` [PULL v2 06/91] virtio-mmio: Handle extra notification data Michael S. Tsirkin
@ 2024-07-02 20:14 ` Michael S. Tsirkin
2024-07-02 20:14 ` [PULL v2 08/91] vhost/vhost-user: Add VIRTIO_F_NOTIFICATION_DATA to vhost feature bits Michael S. Tsirkin
` (11 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2024-07-02 20:14 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Jonah Palmer, Halil Pasic, Christian Borntraeger,
Eric Farman, Richard Henderson, David Hildenbrand,
Ilya Leoshkevich, Thomas Huth, qemu-s390x
From: Jonah Palmer <jonah.palmer@oracle.com>
Add support to virtio-ccw devices for handling the extra data sent from
the driver to the device when the VIRTIO_F_NOTIFICATION_DATA transport
feature has been negotiated.
The extra data that's passed to the virtio-ccw device when this feature
is enabled varies depending on the device's virtqueue layout.
That data passed to the virtio-ccw device is in the same format as the
data passed to virtio-pci devices.
Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
Message-Id: <20240315165557.26942-5-jonah.palmer@oracle.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/s390x/s390-virtio-ccw.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index cd063f8b64..8cd912f20e 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -126,9 +126,11 @@ static void subsystem_reset(void)
static int virtio_ccw_hcall_notify(const uint64_t *args)
{
uint64_t subch_id = args[0];
- uint64_t queue = args[1];
+ uint64_t data = args[1];
SubchDev *sch;
+ VirtIODevice *vdev;
int cssid, ssid, schid, m;
+ uint16_t vq_idx = data;
if (ioinst_disassemble_sch_ident(subch_id, &m, &cssid, &ssid, &schid)) {
return -EINVAL;
@@ -137,12 +139,19 @@ static int virtio_ccw_hcall_notify(const uint64_t *args)
if (!sch || !css_subch_visible(sch)) {
return -EINVAL;
}
- if (queue >= VIRTIO_QUEUE_MAX) {
+
+ vdev = virtio_ccw_get_vdev(sch);
+ if (vq_idx >= VIRTIO_QUEUE_MAX || !virtio_queue_get_num(vdev, vq_idx)) {
return -EINVAL;
}
- virtio_queue_notify(virtio_ccw_get_vdev(sch), queue);
- return 0;
+ if (virtio_vdev_has_feature(vdev, VIRTIO_F_NOTIFICATION_DATA)) {
+ virtio_queue_set_shadow_avail_idx(virtio_get_queue(vdev, vq_idx),
+ (data >> 16) & 0xFFFF);
+ }
+
+ virtio_queue_notify(vdev, vq_idx);
+ return 0;
}
static int virtio_ccw_hcall_early_printk(const uint64_t *args)
--
MST
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL v2 08/91] vhost/vhost-user: Add VIRTIO_F_NOTIFICATION_DATA to vhost feature bits
2024-07-02 20:13 [PULL v2 00/91] virtio: features,fixes Michael S. Tsirkin
` (6 preceding siblings ...)
2024-07-02 20:14 ` [PULL v2 07/91] virtio-ccw: " Michael S. Tsirkin
@ 2024-07-02 20:14 ` Michael S. Tsirkin
2024-07-02 20:14 ` [PULL v2 09/91] Fix vhost user assertion when sending more than one fd Michael S. Tsirkin
` (10 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2024-07-02 20:14 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Jonah Palmer, Lei Yang, Eugenio Pérez,
Srujana Challa, Raphael Norwitz, Kevin Wolf, Hanna Reitz,
Jason Wang, Paolo Bonzini, Fam Zheng, Stefan Hajnoczi, qemu-block,
virtio-fs
From: Jonah Palmer <jonah.palmer@oracle.com>
Add support for the VIRTIO_F_NOTIFICATION_DATA feature across a variety
of vhost devices.
The inclusion of VIRTIO_F_NOTIFICATION_DATA in the feature bits arrays
for these devices ensures that the backend is capable of offering and
providing support for this feature, and that it can be disabled if the
backend does not support it.
Tested-by: Lei Yang <leiyang@redhat.com>
Reviewed-by: Eugenio Pérez <eperezma@redhat.com>
Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
Message-Id: <20240315165557.26942-6-jonah.palmer@oracle.com>
Acked-by: Srujana Challa <schalla@marvell.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/block/vhost-user-blk.c | 1 +
hw/net/vhost_net.c | 2 ++
hw/scsi/vhost-scsi.c | 1 +
hw/scsi/vhost-user-scsi.c | 1 +
hw/virtio/vhost-user-fs.c | 2 +-
hw/virtio/vhost-user-vsock.c | 1 +
net/vhost-vdpa.c | 1 +
7 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 9e6bbc6950..bc2677dbef 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -51,6 +51,7 @@ static const int user_feature_bits[] = {
VIRTIO_F_RING_PACKED,
VIRTIO_F_IOMMU_PLATFORM,
VIRTIO_F_RING_RESET,
+ VIRTIO_F_NOTIFICATION_DATA,
VHOST_INVALID_FEATURE_BIT
};
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index fd1a93701a..18898afe81 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -48,6 +48,7 @@ static const int kernel_feature_bits[] = {
VIRTIO_F_IOMMU_PLATFORM,
VIRTIO_F_RING_PACKED,
VIRTIO_F_RING_RESET,
+ VIRTIO_F_NOTIFICATION_DATA,
VIRTIO_NET_F_HASH_REPORT,
VHOST_INVALID_FEATURE_BIT
};
@@ -55,6 +56,7 @@ static const int kernel_feature_bits[] = {
/* Features supported by others. */
static const int user_feature_bits[] = {
VIRTIO_F_NOTIFY_ON_EMPTY,
+ VIRTIO_F_NOTIFICATION_DATA,
VIRTIO_RING_F_INDIRECT_DESC,
VIRTIO_RING_F_EVENT_IDX,
diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index ae26bc19a4..3d5fe0994d 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -38,6 +38,7 @@ static const int kernel_feature_bits[] = {
VIRTIO_RING_F_EVENT_IDX,
VIRTIO_SCSI_F_HOTPLUG,
VIRTIO_F_RING_RESET,
+ VIRTIO_F_NOTIFICATION_DATA,
VHOST_INVALID_FEATURE_BIT
};
diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index a63b1f4948..0b050805a8 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -36,6 +36,7 @@ static const int user_feature_bits[] = {
VIRTIO_RING_F_EVENT_IDX,
VIRTIO_SCSI_F_HOTPLUG,
VIRTIO_F_RING_RESET,
+ VIRTIO_F_NOTIFICATION_DATA,
VHOST_INVALID_FEATURE_BIT
};
diff --git a/hw/virtio/vhost-user-fs.c b/hw/virtio/vhost-user-fs.c
index cca2cd41be..ae48cc1c96 100644
--- a/hw/virtio/vhost-user-fs.c
+++ b/hw/virtio/vhost-user-fs.c
@@ -33,7 +33,7 @@ static const int user_feature_bits[] = {
VIRTIO_F_RING_PACKED,
VIRTIO_F_IOMMU_PLATFORM,
VIRTIO_F_RING_RESET,
-
+ VIRTIO_F_NOTIFICATION_DATA,
VHOST_INVALID_FEATURE_BIT
};
diff --git a/hw/virtio/vhost-user-vsock.c b/hw/virtio/vhost-user-vsock.c
index 9431b9792c..802b44a07d 100644
--- a/hw/virtio/vhost-user-vsock.c
+++ b/hw/virtio/vhost-user-vsock.c
@@ -21,6 +21,7 @@ static const int user_feature_bits[] = {
VIRTIO_RING_F_INDIRECT_DESC,
VIRTIO_RING_F_EVENT_IDX,
VIRTIO_F_NOTIFY_ON_EMPTY,
+ VIRTIO_F_NOTIFICATION_DATA,
VHOST_INVALID_FEATURE_BIT
};
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index eda714d1a4..daa38428c5 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -62,6 +62,7 @@ const int vdpa_feature_bits[] = {
VIRTIO_F_RING_PACKED,
VIRTIO_F_RING_RESET,
VIRTIO_F_VERSION_1,
+ VIRTIO_F_NOTIFICATION_DATA,
VIRTIO_NET_F_CSUM,
VIRTIO_NET_F_CTRL_GUEST_OFFLOADS,
VIRTIO_NET_F_CTRL_MAC_ADDR,
--
MST
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL v2 09/91] Fix vhost user assertion when sending more than one fd
2024-07-02 20:13 [PULL v2 00/91] virtio: features,fixes Michael S. Tsirkin
` (7 preceding siblings ...)
2024-07-02 20:14 ` [PULL v2 08/91] vhost/vhost-user: Add VIRTIO_F_NOTIFICATION_DATA to vhost feature bits Michael S. Tsirkin
@ 2024-07-02 20:14 ` Michael S. Tsirkin
2024-07-02 20:14 ` [PULL v2 10/91] vhost-vsock: add VIRTIO_F_RING_PACKED to feature_bits Michael S. Tsirkin
` (9 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2024-07-02 20:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Christian Pötzsch
From: Christian Pötzsch <christian.poetzsch@kernkonzept.com>
If the client sends more than one region this assert triggers. The
reason is that two fd's are 8 bytes and VHOST_MEMORY_BASELINE_NREGIONS
is exactly 8.
The assert is wrong because it should not test for the size of the fd
array, but for the numbers of regions.
Signed-off-by: Christian Pötzsch <christian.poetzsch@kernkonzept.com>
Message-Id: <20240426083313.3081272-1-christian.poetzsch@kernkonzept.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
subprojects/libvhost-user/libvhost-user.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
index a879149fef..8adb277d54 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -568,7 +568,7 @@ vu_message_read_default(VuDev *dev, int conn_fd, VhostUserMsg *vmsg)
if (cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SCM_RIGHTS) {
fd_size = cmsg->cmsg_len - CMSG_LEN(0);
vmsg->fd_num = fd_size / sizeof(int);
- assert(fd_size < VHOST_MEMORY_BASELINE_NREGIONS);
+ assert(vmsg->fd_num <= VHOST_MEMORY_BASELINE_NREGIONS);
memcpy(vmsg->fds, CMSG_DATA(cmsg), fd_size);
break;
}
--
MST
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL v2 10/91] vhost-vsock: add VIRTIO_F_RING_PACKED to feature_bits
2024-07-02 20:13 [PULL v2 00/91] virtio: features,fixes Michael S. Tsirkin
` (8 preceding siblings ...)
2024-07-02 20:14 ` [PULL v2 09/91] Fix vhost user assertion when sending more than one fd Michael S. Tsirkin
@ 2024-07-02 20:14 ` Michael S. Tsirkin
2024-07-02 20:14 ` [PULL v2 11/91] hw/virtio: Fix obtain the buffer id from the last descriptor Michael S. Tsirkin
` (8 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2024-07-02 20:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Halil Pasic, Marc Hartmayer
From: Halil Pasic <pasic@linux.ibm.com>
Not having VIRTIO_F_RING_PACKED in feature_bits[] is a problem when the
vhost-vsock device does not offer the feature bit VIRTIO_F_RING_PACKED
but the in QEMU device is configured to try to use the packed layout
(the virtio property "packed" is on).
As of today, the Linux kernel vhost-vsock device does not support the
packed queue layout (as vhost does not support packed), and does not
offer VIRTIO_F_RING_PACKED. Thus when for example a vhost-vsock-ccw is
used with packed=on, VIRTIO_F_RING_PACKED ends up being negotiated,
despite the fact that the device does not actually support it, and
one gets to keep the pieces.
Fixes: 74b3e46630 ("virtio: add property to enable packed virtqueue")
Reported-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
Message-Id: <20240429113334.2454197-1-pasic@linux.ibm.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/virtio/vhost-vsock-common.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/virtio/vhost-vsock-common.c b/hw/virtio/vhost-vsock-common.c
index 12ea87d7a7..fd88df2560 100644
--- a/hw/virtio/vhost-vsock-common.c
+++ b/hw/virtio/vhost-vsock-common.c
@@ -22,6 +22,7 @@
const int feature_bits[] = {
VIRTIO_VSOCK_F_SEQPACKET,
VIRTIO_F_RING_RESET,
+ VIRTIO_F_RING_PACKED,
VHOST_INVALID_FEATURE_BIT
};
--
MST
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL v2 11/91] hw/virtio: Fix obtain the buffer id from the last descriptor
2024-07-02 20:13 [PULL v2 00/91] virtio: features,fixes Michael S. Tsirkin
` (9 preceding siblings ...)
2024-07-02 20:14 ` [PULL v2 10/91] vhost-vsock: add VIRTIO_F_RING_PACKED to feature_bits Michael S. Tsirkin
@ 2024-07-02 20:14 ` Michael S. Tsirkin
2024-07-02 20:14 ` [PULL v2 12/91] virtio-pci: only reset pm state during resetting Michael S. Tsirkin
` (7 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2024-07-02 20:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Wafer, Jason Wang, Eugenio Pérez
From: Wafer <wafer@jaguarmicro.com>
The virtio-1.3 specification
<https://docs.oasis-open.org/virtio/virtio/v1.3/virtio-v1.3.html> writes:
2.8.6 Next Flag: Descriptor Chaining
Buffer ID is included in the last descriptor in the list.
If the feature (_F_INDIRECT_DESC) has been negotiated, install only
one descriptor in the virtqueue.
Therefor the buffer id should be obtained from the first descriptor.
In descriptor chaining scenarios, the buffer id should be obtained
from the last descriptor.
Fixes: 86044b24e8 ("virtio: basic packed virtqueue support")
Signed-off-by: Wafer <wafer@jaguarmicro.com>
Reviewed-by: Jason Wang <jasowang@redhat.com>
Reviewed-by: Eugenio Pérez <eperezma@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Message-Id: <20240510072753.26158-2-wafer@jaguarmicro.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/virtio/virtio.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 28cd406e16..3678ec2f88 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1745,6 +1745,11 @@ static void *virtqueue_packed_pop(VirtQueue *vq, size_t sz)
&indirect_desc_cache);
} while (rc == VIRTQUEUE_READ_DESC_MORE);
+ if (desc_cache != &indirect_desc_cache) {
+ /* Buffer ID is included in the last descriptor in the list. */
+ id = desc.id;
+ }
+
/* Now copy what we have collected and mapped */
elem = virtqueue_alloc_element(sz, out_num, in_num);
for (i = 0; i < out_num; i++) {
--
MST
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL v2 12/91] virtio-pci: only reset pm state during resetting
2024-07-02 20:13 [PULL v2 00/91] virtio: features,fixes Michael S. Tsirkin
` (10 preceding siblings ...)
2024-07-02 20:14 ` [PULL v2 11/91] hw/virtio: Fix obtain the buffer id from the last descriptor Michael S. Tsirkin
@ 2024-07-02 20:14 ` Michael S. Tsirkin
2024-07-02 20:14 ` [PULL v2 13/91] vhost-user-gpu: fix import of DMABUF Michael S. Tsirkin
` (6 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2024-07-02 20:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Jiqian Chen
From: Jiqian Chen <Jiqian.Chen@amd.com>
Fix bug imported by 27ce0f3afc9dd ("fix Power Management Control Register for PCI Express virtio devices"
After this change, observe that QEMU may erroneously clear the power status of the device,
or may erroneously clear non writable registers, such as NO_SOFT_RESET, etc.
Only state of PM_CTRL is writable.
Only when flag VIRTIO_PCI_FLAG_INIT_PM is set, need to reset state.
Fixes: 27ce0f3afc9dd ("fix Power Management Control Register for PCI Express virtio devices"
Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
Message-Id: <20240515073526.17297-2-Jiqian.Chen@amd.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/virtio/virtio-pci.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index cffc7efcae..7d62e92365 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -2306,10 +2306,16 @@ static void virtio_pci_bus_reset_hold(Object *obj, ResetType type)
virtio_pci_reset(qdev);
if (pci_is_express(dev)) {
+ VirtIOPCIProxy *proxy = VIRTIO_PCI(dev);
+
pcie_cap_deverr_reset(dev);
pcie_cap_lnkctl_reset(dev);
- pci_set_word(dev->config + dev->exp.pm_cap + PCI_PM_CTRL, 0);
+ if (proxy->flags & VIRTIO_PCI_FLAG_INIT_PM) {
+ pci_word_test_and_clear_mask(
+ dev->config + dev->exp.pm_cap + PCI_PM_CTRL,
+ PCI_PM_CTRL_STATE_MASK);
+ }
}
}
--
MST
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL v2 13/91] vhost-user-gpu: fix import of DMABUF
2024-07-02 20:13 [PULL v2 00/91] virtio: features,fixes Michael S. Tsirkin
` (11 preceding siblings ...)
2024-07-02 20:14 ` [PULL v2 12/91] virtio-pci: only reset pm state during resetting Michael S. Tsirkin
@ 2024-07-02 20:14 ` Michael S. Tsirkin
2024-07-02 20:14 ` [PULL v2 14/91] Revert "vhost-user: fix lost reconnect" Michael S. Tsirkin
` (5 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2024-07-02 20:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
When using vhost-user-gpu with GL, qemu -display gtk doesn't show output
and prints: qemu: eglCreateImageKHR failed
Since commit 9ac06df8b ("virtio-gpu-udmabuf: correct naming of
QemuDmaBuf size properties"), egl_dmabuf_import_texture() uses
backing_{width,height} for the texture dimension.
Fixes: 9ac06df8b ("virtio-gpu-udmabuf: correct naming of QemuDmaBuf size properties")
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20240515105237.1074116-1-marcandre.lureau@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/display/vhost-user-gpu.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c
index e4b398d26c..63c64ddde6 100644
--- a/hw/display/vhost-user-gpu.c
+++ b/hw/display/vhost-user-gpu.c
@@ -281,8 +281,9 @@ vhost_user_gpu_handle_display(VhostUserGPU *g, VhostUserGpuMsg *msg)
modifier = m2->modifier;
}
- dmabuf = qemu_dmabuf_new(m->fd_width, m->fd_height,
- m->fd_stride, 0, 0, 0, 0,
+ dmabuf = qemu_dmabuf_new(m->width, m->height,
+ m->fd_stride, 0, 0,
+ m->fd_width, m->fd_height,
m->fd_drm_fourcc, modifier,
fd, false, m->fd_flags &
VIRTIO_GPU_RESOURCE_FLAG_Y_0_TOP);
--
MST
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL v2 14/91] Revert "vhost-user: fix lost reconnect"
2024-07-02 20:13 [PULL v2 00/91] virtio: features,fixes Michael S. Tsirkin
` (12 preceding siblings ...)
2024-07-02 20:14 ` [PULL v2 13/91] vhost-user-gpu: fix import of DMABUF Michael S. Tsirkin
@ 2024-07-02 20:14 ` Michael S. Tsirkin
2024-07-02 20:14 ` [PULL v2 15/91] vhost-user: fix lost reconnect again Michael S. Tsirkin
` (4 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2024-07-02 20:14 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Li Feng, Raphael Norwitz, Kevin Wolf, Hanna Reitz,
Paolo Bonzini, Fam Zheng, Alex Bennée, qemu-block
From: Li Feng <fengli@smartx.com>
This reverts commit f02a4b8e6431598612466f76aac64ab492849abf.
Since the current patch cannot completely fix the lost reconnect
problem, there is a scenario that is not considered:
- When the virtio-blk driver is removed from the guest os,
s->connected has no chance to be set to false, resulting in
subsequent reconnection not being executed.
The next patch will completely fix this issue with a better approach.
Signed-off-by: Li Feng <fengli@smartx.com>
Message-Id: <20240516025753.130171-2-fengli@smartx.com>
Reviewed-by: Raphael Norwitz <raphael@enfabrica.net>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/hw/virtio/vhost-user.h | 3 +--
hw/block/vhost-user-blk.c | 2 +-
hw/scsi/vhost-user-scsi.c | 3 +--
hw/virtio/vhost-user-base.c | 2 +-
hw/virtio/vhost-user.c | 10 ++--------
5 files changed, 6 insertions(+), 14 deletions(-)
diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
index d7c09ffd34..324cd8663a 100644
--- a/include/hw/virtio/vhost-user.h
+++ b/include/hw/virtio/vhost-user.h
@@ -108,7 +108,6 @@ typedef void (*vu_async_close_fn)(DeviceState *cb);
void vhost_user_async_close(DeviceState *d,
CharBackend *chardev, struct vhost_dev *vhost,
- vu_async_close_fn cb,
- IOEventHandler *event_cb);
+ vu_async_close_fn cb);
#endif
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index bc2677dbef..15cc24d017 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -385,7 +385,7 @@ static void vhost_user_blk_event(void *opaque, QEMUChrEvent event)
case CHR_EVENT_CLOSED:
/* defer close until later to avoid circular close */
vhost_user_async_close(dev, &s->chardev, &s->dev,
- vhost_user_blk_disconnect, vhost_user_blk_event);
+ vhost_user_blk_disconnect);
break;
case CHR_EVENT_BREAK:
case CHR_EVENT_MUX_IN:
diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index 0b050805a8..421cd654f8 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -215,8 +215,7 @@ static void vhost_user_scsi_event(void *opaque, QEMUChrEvent event)
case CHR_EVENT_CLOSED:
/* defer close until later to avoid circular close */
vhost_user_async_close(dev, &vs->conf.chardev, &vsc->dev,
- vhost_user_scsi_disconnect,
- vhost_user_scsi_event);
+ vhost_user_scsi_disconnect);
break;
case CHR_EVENT_BREAK:
case CHR_EVENT_MUX_IN:
diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c
index a83167191e..4b54255682 100644
--- a/hw/virtio/vhost-user-base.c
+++ b/hw/virtio/vhost-user-base.c
@@ -254,7 +254,7 @@ static void vub_event(void *opaque, QEMUChrEvent event)
case CHR_EVENT_CLOSED:
/* defer close until later to avoid circular close */
vhost_user_async_close(dev, &vub->chardev, &vub->vhost_dev,
- vub_disconnect, vub_event);
+ vub_disconnect);
break;
case CHR_EVENT_BREAK:
case CHR_EVENT_MUX_IN:
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index cdf9af4a4b..c929097e87 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -2776,7 +2776,6 @@ typedef struct {
DeviceState *dev;
CharBackend *cd;
struct vhost_dev *vhost;
- IOEventHandler *event_cb;
} VhostAsyncCallback;
static void vhost_user_async_close_bh(void *opaque)
@@ -2791,10 +2790,7 @@ static void vhost_user_async_close_bh(void *opaque)
*/
if (vhost->vdev) {
data->cb(data->dev);
- } else if (data->event_cb) {
- qemu_chr_fe_set_handlers(data->cd, NULL, NULL, data->event_cb,
- NULL, data->dev, NULL, true);
- }
+ }
g_free(data);
}
@@ -2806,8 +2802,7 @@ static void vhost_user_async_close_bh(void *opaque)
*/
void vhost_user_async_close(DeviceState *d,
CharBackend *chardev, struct vhost_dev *vhost,
- vu_async_close_fn cb,
- IOEventHandler *event_cb)
+ vu_async_close_fn cb)
{
if (!runstate_check(RUN_STATE_SHUTDOWN)) {
/*
@@ -2823,7 +2818,6 @@ void vhost_user_async_close(DeviceState *d,
data->dev = d;
data->cd = chardev;
data->vhost = vhost;
- data->event_cb = event_cb;
/* Disable any further notifications on the chardev */
qemu_chr_fe_set_handlers(chardev,
--
MST
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL v2 15/91] vhost-user: fix lost reconnect again
2024-07-02 20:13 [PULL v2 00/91] virtio: features,fixes Michael S. Tsirkin
` (13 preceding siblings ...)
2024-07-02 20:14 ` [PULL v2 14/91] Revert "vhost-user: fix lost reconnect" Michael S. Tsirkin
@ 2024-07-02 20:14 ` Michael S. Tsirkin
2024-07-02 20:14 ` [PULL v2 16/91] hw/cxl/mailbox: change CCI cmd set structure to be a member, not a reference Michael S. Tsirkin
` (3 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2024-07-02 20:14 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Li Feng, Raphael Norwitz, Kevin Wolf, Hanna Reitz,
Paolo Bonzini, Fam Zheng, Alex Bennée, qemu-block
From: Li Feng <fengli@smartx.com>
When the vhost-user is reconnecting to the backend, and if the vhost-user fails
at the get_features in vhost_dev_init(), then the reconnect will fail
and it will not be retriggered forever.
The reason is:
When the vhost-user fail at get_features, the vhost_dev_cleanup will be called
immediately.
vhost_dev_cleanup calls 'memset(hdev, 0, sizeof(struct vhost_dev))'.
The reconnect path is:
vhost_user_blk_event
vhost_user_async_close(.. vhost_user_blk_disconnect ..)
qemu_chr_fe_set_handlers <----- clear the notifier callback
schedule vhost_user_async_close_bh
The vhost->vdev is null, so the vhost_user_blk_disconnect will not be
called, then the event fd callback will not be reinstalled.
We need to ensure that even if vhost_dev_init initialization fails, the event
handler still needs to be reinstalled when s->connected is false.
All vhost-user devices have this issue, including vhost-user-blk/scsi.
Fixes: 71e076a07d ("hw/virtio: generalise CHR_EVENT_CLOSED handling")
Signed-off-by: Li Feng <fengli@smartx.com>
Message-Id: <20240516025753.130171-3-fengli@smartx.com>
Reviewed-by: Raphael Norwitz <raphael@enfabrica.net>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/block/vhost-user-blk.c | 3 ++-
hw/scsi/vhost-user-scsi.c | 3 ++-
hw/virtio/vhost-user-base.c | 3 ++-
hw/virtio/vhost-user.c | 10 +---------
4 files changed, 7 insertions(+), 12 deletions(-)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 15cc24d017..fdbc30b9ce 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -354,7 +354,7 @@ static void vhost_user_blk_disconnect(DeviceState *dev)
VHostUserBlk *s = VHOST_USER_BLK(vdev);
if (!s->connected) {
- return;
+ goto done;
}
s->connected = false;
@@ -362,6 +362,7 @@ static void vhost_user_blk_disconnect(DeviceState *dev)
vhost_dev_cleanup(&s->dev);
+done:
/* Re-instate the event handler for new connections */
qemu_chr_fe_set_handlers(&s->chardev, NULL, NULL, vhost_user_blk_event,
NULL, dev, NULL, true);
diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index 421cd654f8..cc91ade525 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -182,7 +182,7 @@ static void vhost_user_scsi_disconnect(DeviceState *dev)
VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
if (!s->connected) {
- return;
+ goto done;
}
s->connected = false;
@@ -190,6 +190,7 @@ static void vhost_user_scsi_disconnect(DeviceState *dev)
vhost_dev_cleanup(&vsc->dev);
+done:
/* Re-instate the event handler for new connections */
qemu_chr_fe_set_handlers(&vs->conf.chardev, NULL, NULL,
vhost_user_scsi_event, NULL, dev, NULL, true);
diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c
index 4b54255682..11e72b1e3b 100644
--- a/hw/virtio/vhost-user-base.c
+++ b/hw/virtio/vhost-user-base.c
@@ -225,13 +225,14 @@ static void vub_disconnect(DeviceState *dev)
VHostUserBase *vub = VHOST_USER_BASE(vdev);
if (!vub->connected) {
- return;
+ goto done;
}
vub->connected = false;
vub_stop(vdev);
vhost_dev_cleanup(&vub->vhost_dev);
+done:
/* Re-instate the event handler for new connections */
qemu_chr_fe_set_handlers(&vub->chardev,
NULL, NULL, vub_event,
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index c929097e87..c407ea8939 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -2781,16 +2781,8 @@ typedef struct {
static void vhost_user_async_close_bh(void *opaque)
{
VhostAsyncCallback *data = opaque;
- struct vhost_dev *vhost = data->vhost;
- /*
- * If the vhost_dev has been cleared in the meantime there is
- * nothing left to do as some other path has completed the
- * cleanup.
- */
- if (vhost->vdev) {
- data->cb(data->dev);
- }
+ data->cb(data->dev);
g_free(data);
}
--
MST
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL v2 16/91] hw/cxl/mailbox: change CCI cmd set structure to be a member, not a reference
2024-07-02 20:13 [PULL v2 00/91] virtio: features,fixes Michael S. Tsirkin
` (14 preceding siblings ...)
2024-07-02 20:14 ` [PULL v2 15/91] vhost-user: fix lost reconnect again Michael S. Tsirkin
@ 2024-07-02 20:14 ` Michael S. Tsirkin
2024-07-02 20:14 ` [PULL v2 17/91] hw/cxl/mailbox: interface to add CCI commands to an existing CCI Michael S. Tsirkin
` (2 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2024-07-02 20:14 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Gregory Price, Gregory Price, Jonathan Cameron,
Fan Ni
From: Gregory Price <gourry.memverge@gmail.com>
This allows devices to have fully customized CCIs, along with complex
devices where wrapper devices can override or add additional CCI
commands without having to replicate full command structures or
pollute a base device with every command that might ever be used.
Signed-off-by: Gregory Price <gregory.price@memverge.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Fan Ni <fan.ni@samsung.com>
Message-Id: <20240523174651.1089554-2-nifan.cxl@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/hw/cxl/cxl_device.h | 2 +-
hw/cxl/cxl-mailbox-utils.c | 19 +++++++++++++++----
2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
index 279b276bda..ccc4611875 100644
--- a/include/hw/cxl/cxl_device.h
+++ b/include/hw/cxl/cxl_device.h
@@ -164,7 +164,7 @@ typedef struct CXLEventLog {
} CXLEventLog;
typedef struct CXLCCI {
- const struct cxl_cmd (*cxl_cmd_set)[256];
+ struct cxl_cmd cxl_cmd_set[256][256];
struct cel_log {
uint16_t opcode;
uint16_t effect;
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index e5eb97cb91..2c9f50f0f9 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -1447,10 +1447,21 @@ void cxl_init_cci(CXLCCI *cci, size_t payload_max)
bg_timercb, cci);
}
+static void cxl_copy_cci_commands(CXLCCI *cci, const struct cxl_cmd (*cxl_cmds)[256])
+{
+ for (int set = 0; set < 256; set++) {
+ for (int cmd = 0; cmd < 256; cmd++) {
+ if (cxl_cmds[set][cmd].handler) {
+ cci->cxl_cmd_set[set][cmd] = cxl_cmds[set][cmd];
+ }
+ }
+ }
+}
+
void cxl_initialize_mailbox_swcci(CXLCCI *cci, DeviceState *intf,
DeviceState *d, size_t payload_max)
{
- cci->cxl_cmd_set = cxl_cmd_set_sw;
+ cxl_copy_cci_commands(cci, cxl_cmd_set_sw);
cci->d = d;
cci->intf = intf;
cxl_init_cci(cci, payload_max);
@@ -1458,7 +1469,7 @@ void cxl_initialize_mailbox_swcci(CXLCCI *cci, DeviceState *intf,
void cxl_initialize_mailbox_t3(CXLCCI *cci, DeviceState *d, size_t payload_max)
{
- cci->cxl_cmd_set = cxl_cmd_set;
+ cxl_copy_cci_commands(cci, cxl_cmd_set);
cci->d = d;
/* No separation for PCI MB as protocol handled in PCI device */
@@ -1476,7 +1487,7 @@ static const struct cxl_cmd cxl_cmd_set_t3_ld[256][256] = {
void cxl_initialize_t3_ld_cci(CXLCCI *cci, DeviceState *d, DeviceState *intf,
size_t payload_max)
{
- cci->cxl_cmd_set = cxl_cmd_set_t3_ld;
+ cxl_copy_cci_commands(cci, cxl_cmd_set_t3_ld);
cci->d = d;
cci->intf = intf;
cxl_init_cci(cci, payload_max);
@@ -1496,7 +1507,7 @@ void cxl_initialize_t3_fm_owned_ld_mctpcci(CXLCCI *cci, DeviceState *d,
DeviceState *intf,
size_t payload_max)
{
- cci->cxl_cmd_set = cxl_cmd_set_t3_fm_owned_ld_mctp;
+ cxl_copy_cci_commands(cci, cxl_cmd_set_t3_fm_owned_ld_mctp);
cci->d = d;
cci->intf = intf;
cxl_init_cci(cci, payload_max);
--
MST
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL v2 17/91] hw/cxl/mailbox: interface to add CCI commands to an existing CCI
2024-07-02 20:13 [PULL v2 00/91] virtio: features,fixes Michael S. Tsirkin
` (15 preceding siblings ...)
2024-07-02 20:14 ` [PULL v2 16/91] hw/cxl/mailbox: change CCI cmd set structure to be a member, not a reference Michael S. Tsirkin
@ 2024-07-02 20:14 ` Michael S. Tsirkin
2024-07-02 20:14 ` [PULL v2 18/91] hw/cxl/cxl-mailbox-utils: Add dc_event_log_size field to output payload of identify memory device command Michael S. Tsirkin
2024-07-02 20:14 ` [PULL v2 19/91] hw/cxl/cxl-mailbox-utils: Add dynamic capacity region representative and mailbox command support Michael S. Tsirkin
18 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2024-07-02 20:14 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Gregory Price, Gregory Price, Jonathan Cameron,
Fan Ni
From: Gregory Price <gourry.memverge@gmail.com>
This enables wrapper devices to customize the base device's CCI
(for example, with custom commands outside the specification)
without the need to change the base device.
The also enabled the base device to dispatch those commands without
requiring additional driver support.
Heavily edited by Jonathan Cameron to increase code reuse
Signed-off-by: Gregory Price <gregory.price@memverge.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Fan Ni <fan.ni@samsung.com>
Message-Id: <20240523174651.1089554-3-nifan.cxl@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/hw/cxl/cxl_device.h | 2 ++
hw/cxl/cxl-mailbox-utils.c | 19 +++++++++++++++++--
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
index ccc4611875..a5f8e25020 100644
--- a/include/hw/cxl/cxl_device.h
+++ b/include/hw/cxl/cxl_device.h
@@ -301,6 +301,8 @@ void cxl_initialize_mailbox_t3(CXLCCI *cci, DeviceState *d, size_t payload_max);
void cxl_initialize_mailbox_swcci(CXLCCI *cci, DeviceState *intf,
DeviceState *d, size_t payload_max);
void cxl_init_cci(CXLCCI *cci, size_t payload_max);
+void cxl_add_cci_commands(CXLCCI *cci, const struct cxl_cmd (*cxl_cmd_set)[256],
+ size_t payload_max);
int cxl_process_cci_message(CXLCCI *cci, uint8_t set, uint8_t cmd,
size_t len_in, uint8_t *pl_in,
size_t *len_out, uint8_t *pl_out,
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index 2c9f50f0f9..2a64c58e2f 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -1424,9 +1424,9 @@ static void bg_timercb(void *opaque)
}
}
-void cxl_init_cci(CXLCCI *cci, size_t payload_max)
+static void cxl_rebuild_cel(CXLCCI *cci)
{
- cci->payload_max = payload_max;
+ cci->cel_size = 0; /* Reset for a fresh build */
for (int set = 0; set < 256; set++) {
for (int cmd = 0; cmd < 256; cmd++) {
if (cci->cxl_cmd_set[set][cmd].handler) {
@@ -1440,6 +1440,13 @@ void cxl_init_cci(CXLCCI *cci, size_t payload_max)
}
}
}
+}
+
+void cxl_init_cci(CXLCCI *cci, size_t payload_max)
+{
+ cci->payload_max = payload_max;
+ cxl_rebuild_cel(cci);
+
cci->bg.complete_pct = 0;
cci->bg.starttime = 0;
cci->bg.runtime = 0;
@@ -1458,6 +1465,14 @@ static void cxl_copy_cci_commands(CXLCCI *cci, const struct cxl_cmd (*cxl_cmds)[
}
}
+void cxl_add_cci_commands(CXLCCI *cci, const struct cxl_cmd (*cxl_cmd_set)[256],
+ size_t payload_max)
+{
+ cci->payload_max = MAX(payload_max, cci->payload_max);
+ cxl_copy_cci_commands(cci, cxl_cmd_set);
+ cxl_rebuild_cel(cci);
+}
+
void cxl_initialize_mailbox_swcci(CXLCCI *cci, DeviceState *intf,
DeviceState *d, size_t payload_max)
{
--
MST
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL v2 18/91] hw/cxl/cxl-mailbox-utils: Add dc_event_log_size field to output payload of identify memory device command
2024-07-02 20:13 [PULL v2 00/91] virtio: features,fixes Michael S. Tsirkin
` (16 preceding siblings ...)
2024-07-02 20:14 ` [PULL v2 17/91] hw/cxl/mailbox: interface to add CCI commands to an existing CCI Michael S. Tsirkin
@ 2024-07-02 20:14 ` Michael S. Tsirkin
2024-07-02 20:14 ` [PULL v2 19/91] hw/cxl/cxl-mailbox-utils: Add dynamic capacity region representative and mailbox command support Michael S. Tsirkin
18 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2024-07-02 20:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Fan Ni, Gregory Price, Jonathan Cameron
From: Fan Ni <fan.ni@samsung.com>
Based on CXL spec r3.1 Table 8-127 (Identify Memory Device Output
Payload), dynamic capacity event log size should be part of
output of the Identify command.
Add dc_event_log_size to the output payload for the host to get the info.
Reviewed-by: Gregory Price <gregory.price@memverge.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Fan Ni <fan.ni@samsung.com>
Message-Id: <20240523174651.1089554-4-nifan.cxl@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/cxl/cxl-mailbox-utils.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index 2a64c58e2f..626acc1d0d 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -21,6 +21,7 @@
#include "sysemu/hostmem.h"
#define CXL_CAPACITY_MULTIPLIER (256 * MiB)
+#define CXL_DC_EVENT_LOG_SIZE 8
/*
* How to add a new command, example. The command set FOO, with cmd BAR.
@@ -780,8 +781,9 @@ static CXLRetCode cmd_identify_memory_device(const struct cxl_cmd *cmd,
uint16_t inject_poison_limit;
uint8_t poison_caps;
uint8_t qos_telemetry_caps;
+ uint16_t dc_event_log_size;
} QEMU_PACKED *id;
- QEMU_BUILD_BUG_ON(sizeof(*id) != 0x43);
+ QEMU_BUILD_BUG_ON(sizeof(*id) != 0x45);
CXLType3Dev *ct3d = CXL_TYPE3(cci->d);
CXLType3Class *cvc = CXL_TYPE3_GET_CLASS(ct3d);
CXLDeviceState *cxl_dstate = &ct3d->cxl_dstate;
@@ -807,6 +809,7 @@ static CXLRetCode cmd_identify_memory_device(const struct cxl_cmd *cmd,
st24_le_p(id->poison_list_max_mer, 256);
/* No limit - so limited by main poison record limit */
stw_le_p(&id->inject_poison_limit, 0);
+ stw_le_p(&id->dc_event_log_size, CXL_DC_EVENT_LOG_SIZE);
*len_out = sizeof(*id);
return CXL_MBOX_SUCCESS;
--
MST
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL v2 19/91] hw/cxl/cxl-mailbox-utils: Add dynamic capacity region representative and mailbox command support
2024-07-02 20:13 [PULL v2 00/91] virtio: features,fixes Michael S. Tsirkin
` (17 preceding siblings ...)
2024-07-02 20:14 ` [PULL v2 18/91] hw/cxl/cxl-mailbox-utils: Add dc_event_log_size field to output payload of identify memory device command Michael S. Tsirkin
@ 2024-07-02 20:14 ` Michael S. Tsirkin
18 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2024-07-02 20:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Fan Ni, Gregory Price, Jonathan Cameron
From: Fan Ni <fan.ni@samsung.com>
Per cxl spec r3.1, add dynamic capacity (DC) region representative based on
Table 8-165 and extend the cxl type3 device definition to include DC region
information. Also, based on info in 8.2.9.9.9.1, add 'Get Dynamic Capacity
Configuration' mailbox support.
Note: we store region decode length as byte-wise length on the device, which
should be divided by 256 * MiB before being returned to the host
for "Get Dynamic Capacity Configuration" mailbox command per
specification.
Reviewed-by: Gregory Price <gregory.price@memverge.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Fan Ni <fan.ni@samsung.com>
Message-Id: <20240523174651.1089554-5-nifan.cxl@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/hw/cxl/cxl_device.h | 16 +++++++
hw/cxl/cxl-mailbox-utils.c | 96 +++++++++++++++++++++++++++++++++++++
2 files changed, 112 insertions(+)
diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
index a5f8e25020..e839370266 100644
--- a/include/hw/cxl/cxl_device.h
+++ b/include/hw/cxl/cxl_device.h
@@ -422,6 +422,17 @@ typedef struct CXLPoison {
typedef QLIST_HEAD(, CXLPoison) CXLPoisonList;
#define CXL_POISON_LIST_LIMIT 256
+#define DCD_MAX_NUM_REGION 8
+
+typedef struct CXLDCRegion {
+ uint64_t base; /* aligned to 256*MiB */
+ uint64_t decode_len; /* aligned to 256*MiB */
+ uint64_t len;
+ uint64_t block_size;
+ uint32_t dsmadhandle;
+ uint8_t flags;
+} CXLDCRegion;
+
struct CXLType3Dev {
/* Private */
PCIDevice parent_obj;
@@ -454,6 +465,11 @@ struct CXLType3Dev {
unsigned int poison_list_cnt;
bool poison_list_overflowed;
uint64_t poison_list_overflow_ts;
+
+ struct dynamic_capacity {
+ uint8_t num_regions; /* 0-8 regions */
+ CXLDCRegion regions[DCD_MAX_NUM_REGION];
+ } dc;
};
#define TYPE_CXL_TYPE3 "cxl-type3"
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index 626acc1d0d..bede28e3c8 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -22,6 +22,8 @@
#define CXL_CAPACITY_MULTIPLIER (256 * MiB)
#define CXL_DC_EVENT_LOG_SIZE 8
+#define CXL_NUM_EXTENTS_SUPPORTED 512
+#define CXL_NUM_TAGS_SUPPORTED 0
/*
* How to add a new command, example. The command set FOO, with cmd BAR.
@@ -80,6 +82,8 @@ enum {
#define GET_POISON_LIST 0x0
#define INJECT_POISON 0x1
#define CLEAR_POISON 0x2
+ DCD_CONFIG = 0x48,
+ #define GET_DC_CONFIG 0x0
PHYSICAL_SWITCH = 0x51,
#define IDENTIFY_SWITCH_DEVICE 0x0
#define GET_PHYSICAL_PORT_STATE 0x1
@@ -1238,6 +1242,88 @@ static CXLRetCode cmd_media_clear_poison(const struct cxl_cmd *cmd,
return CXL_MBOX_SUCCESS;
}
+/*
+ * CXL r3.1 section 8.2.9.9.9.1: Get Dynamic Capacity Configuration
+ * (Opcode: 4800h)
+ */
+static CXLRetCode cmd_dcd_get_dyn_cap_config(const struct cxl_cmd *cmd,
+ uint8_t *payload_in,
+ size_t len_in,
+ uint8_t *payload_out,
+ size_t *len_out,
+ CXLCCI *cci)
+{
+ CXLType3Dev *ct3d = CXL_TYPE3(cci->d);
+ struct {
+ uint8_t region_cnt;
+ uint8_t start_rid;
+ } QEMU_PACKED *in = (void *)payload_in;
+ struct {
+ uint8_t num_regions;
+ uint8_t regions_returned;
+ uint8_t rsvd1[6];
+ struct {
+ uint64_t base;
+ uint64_t decode_len;
+ uint64_t region_len;
+ uint64_t block_size;
+ uint32_t dsmadhandle;
+ uint8_t flags;
+ uint8_t rsvd2[3];
+ } QEMU_PACKED records[];
+ } QEMU_PACKED *out = (void *)payload_out;
+ struct {
+ uint32_t num_extents_supported;
+ uint32_t num_extents_available;
+ uint32_t num_tags_supported;
+ uint32_t num_tags_available;
+ } QEMU_PACKED *extra_out;
+ uint16_t record_count;
+ uint16_t i;
+ uint16_t out_pl_len;
+ uint8_t start_rid;
+
+ start_rid = in->start_rid;
+ if (start_rid >= ct3d->dc.num_regions) {
+ return CXL_MBOX_INVALID_INPUT;
+ }
+
+ record_count = MIN(ct3d->dc.num_regions - in->start_rid, in->region_cnt);
+
+ out_pl_len = sizeof(*out) + record_count * sizeof(out->records[0]);
+ extra_out = (void *)(payload_out + out_pl_len);
+ out_pl_len += sizeof(*extra_out);
+ assert(out_pl_len <= CXL_MAILBOX_MAX_PAYLOAD_SIZE);
+
+ out->num_regions = ct3d->dc.num_regions;
+ out->regions_returned = record_count;
+ for (i = 0; i < record_count; i++) {
+ stq_le_p(&out->records[i].base,
+ ct3d->dc.regions[start_rid + i].base);
+ stq_le_p(&out->records[i].decode_len,
+ ct3d->dc.regions[start_rid + i].decode_len /
+ CXL_CAPACITY_MULTIPLIER);
+ stq_le_p(&out->records[i].region_len,
+ ct3d->dc.regions[start_rid + i].len);
+ stq_le_p(&out->records[i].block_size,
+ ct3d->dc.regions[start_rid + i].block_size);
+ stl_le_p(&out->records[i].dsmadhandle,
+ ct3d->dc.regions[start_rid + i].dsmadhandle);
+ out->records[i].flags = ct3d->dc.regions[start_rid + i].flags;
+ }
+ /*
+ * TODO: Assign values once extents and tags are introduced
+ * to use.
+ */
+ stl_le_p(&extra_out->num_extents_supported, CXL_NUM_EXTENTS_SUPPORTED);
+ stl_le_p(&extra_out->num_extents_available, CXL_NUM_EXTENTS_SUPPORTED);
+ stl_le_p(&extra_out->num_tags_supported, CXL_NUM_TAGS_SUPPORTED);
+ stl_le_p(&extra_out->num_tags_available, CXL_NUM_TAGS_SUPPORTED);
+
+ *len_out = out_pl_len;
+ return CXL_MBOX_SUCCESS;
+}
+
#define IMMEDIATE_CONFIG_CHANGE (1 << 1)
#define IMMEDIATE_DATA_CHANGE (1 << 2)
#define IMMEDIATE_POLICY_CHANGE (1 << 3)
@@ -1282,6 +1368,11 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = {
cmd_media_clear_poison, 72, 0 },
};
+static const struct cxl_cmd cxl_cmd_set_dcd[256][256] = {
+ [DCD_CONFIG][GET_DC_CONFIG] = { "DCD_GET_DC_CONFIG",
+ cmd_dcd_get_dyn_cap_config, 2, 0 },
+};
+
static const struct cxl_cmd cxl_cmd_set_sw[256][256] = {
[INFOSTAT][IS_IDENTIFY] = { "IDENTIFY", cmd_infostat_identify, 0, 0 },
[INFOSTAT][BACKGROUND_OPERATION_STATUS] = { "BACKGROUND_OPERATION_STATUS",
@@ -1487,7 +1578,12 @@ void cxl_initialize_mailbox_swcci(CXLCCI *cci, DeviceState *intf,
void cxl_initialize_mailbox_t3(CXLCCI *cci, DeviceState *d, size_t payload_max)
{
+ CXLType3Dev *ct3d = CXL_TYPE3(d);
+
cxl_copy_cci_commands(cci, cxl_cmd_set);
+ if (ct3d->dc.num_regions) {
+ cxl_copy_cci_commands(cci, cxl_cmd_set_dcd);
+ }
cci->d = d;
/* No separation for PCI MB as protocol handled in PCI device */
--
MST
^ permalink raw reply related [flat|nested] 20+ messages in thread