* [PATCH] vhost: Don't set vring call if guest notifier is unused
@ 2025-03-26 8:25 oenhan
2025-04-01 10:49 ` Stefano Garzarella
2025-05-11 16:13 ` Michael S. Tsirkin
0 siblings, 2 replies; 5+ messages in thread
From: oenhan @ 2025-03-26 8:25 UTC (permalink / raw)
To: mst, sgarzare, marcel.apfelbaum, cohuck, pasic, farman,
borntraeger, leiyang
Cc: qemu-devel, qemu-stable, Huaitong Han, Zhiyuan Yuan, Jidong Xia
From: Huaitong Han <hanht2@chinatelecom.cn>
The vring call fd is set even when the guest does not use msix (e.g., in the
case of virtio pmd), leading to unnecessary CPU overhead for processing
interrupts. The previous patch optimized the condition where msix is enabled
but the queue vector is unset. However, there is a additional case where the
guest interrupt notifier is effectively unused and the vring call fd should
also be cleared: the guest does not use msix and the INTX_DISABLED bit in the PCI
config is set.
Fixes: 96a3d98d2c ("vhost: don't set vring call if no vector")
Test result:
https://raw.githubusercontent.com/oenhan/build_log/refs/heads/main/qemu/2503261015/build/meson-logs/testlog.txt
Reported-by: Zhiyuan Yuan <yuanzhiyuan@chinatelecom.cn>
Signed-off-by: Jidong Xia <xiajd@chinatelecom.cn>
Signed-off-by: Huaitong Han <hanht2@chinatelecom.cn>
---
hw/pci/pci.c | 2 +-
hw/s390x/virtio-ccw.c | 9 ++++++---
hw/virtio/vhost.c | 5 ++---
hw/virtio/virtio-pci.c | 15 ++++++++++-----
include/hw/pci/pci.h | 1 +
include/hw/virtio/virtio-bus.h | 2 +-
6 files changed, 21 insertions(+), 13 deletions(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 2844ec5556..503a897528 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1719,7 +1719,7 @@ static void pci_update_mappings(PCIDevice *d)
pci_update_vga(d);
}
-static inline int pci_irq_disabled(PCIDevice *d)
+int pci_irq_disabled(PCIDevice *d)
{
return pci_get_word(d->config + PCI_COMMAND) & PCI_COMMAND_INTX_DISABLE;
}
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 43f3b162c8..af482a22cd 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -936,11 +936,14 @@ static void virtio_ccw_vmstate_change(DeviceState *d, bool running)
}
}
-static bool virtio_ccw_query_guest_notifiers(DeviceState *d)
+static bool virtio_ccw_query_guest_notifiers_used(DeviceState *d, int n)
{
CcwDevice *dev = CCW_DEVICE(d);
+ VirtioCcwDevice *vdev = VIRTIO_CCW_DEVICE(d);
+ VirtIODevice *virtio_dev = virtio_bus_get_device(&vdev->bus);
- return !!(dev->sch->curr_status.pmcw.flags & PMCW_FLAGS_MASK_ENA);
+ return !!(dev->sch->curr_status.pmcw.flags & PMCW_FLAGS_MASK_ENA)
+ && virtio_queue_vector(virtio_dev, n) != VIRTIO_NO_VECTOR;
}
static int virtio_ccw_get_mappings(VirtioCcwDevice *dev)
@@ -1270,7 +1273,7 @@ static void virtio_ccw_bus_class_init(ObjectClass *klass, void *data)
bus_class->max_dev = 1;
k->notify = virtio_ccw_notify;
k->vmstate_change = virtio_ccw_vmstate_change;
- k->query_guest_notifiers = virtio_ccw_query_guest_notifiers;
+ k->query_guest_notifiers_used = virtio_ccw_query_guest_notifiers_used;
k->set_guest_notifiers = virtio_ccw_set_guest_notifiers;
k->save_queue = virtio_ccw_save_queue;
k->load_queue = virtio_ccw_load_queue;
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 6aa72fd434..32634da836 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1341,9 +1341,8 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
vhost_virtqueue_mask(dev, vdev, idx, false);
}
- if (k->query_guest_notifiers &&
- k->query_guest_notifiers(qbus->parent) &&
- virtio_queue_vector(vdev, idx) == VIRTIO_NO_VECTOR) {
+ if (k->query_guest_notifiers_used &&
+ !k->query_guest_notifiers_used(qbus->parent, idx)) {
file.fd = -1;
r = dev->vhost_ops->vhost_set_vring_call(dev, &file);
if (r) {
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 3ca3f849d3..25ff869618 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -30,6 +30,7 @@
#include "qemu/error-report.h"
#include "qemu/log.h"
#include "qemu/module.h"
+#include "hw/pci/pci.h"
#include "hw/pci/msi.h"
#include "hw/pci/msix.h"
#include "hw/loader.h"
@@ -1031,7 +1032,7 @@ static void virtio_pci_one_vector_mask(VirtIOPCIProxy *proxy,
/* If guest supports masking, keep irqfd but mask it.
* Otherwise, clean it up now.
- */
+ */
if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
k->guest_notifier_mask(vdev, queue_no, true);
} else {
@@ -1212,10 +1213,15 @@ static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign,
return 0;
}
-static bool virtio_pci_query_guest_notifiers(DeviceState *d)
+static bool virtio_pci_query_guest_notifiers_used(DeviceState *d, int n)
{
VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
- return msix_enabled(&proxy->pci_dev);
+ VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+
+ if (msix_enabled(&proxy->pci_dev))
+ return virtio_queue_vector(vdev, n) != VIRTIO_NO_VECTOR;
+ else
+ return !pci_irq_disabled(&proxy->pci_dev);
}
static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
@@ -2599,7 +2605,7 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
k->save_extra_state = virtio_pci_save_extra_state;
k->load_extra_state = virtio_pci_load_extra_state;
k->has_extra_state = virtio_pci_has_extra_state;
- k->query_guest_notifiers = virtio_pci_query_guest_notifiers;
+ k->query_guest_notifiers_used = virtio_pci_query_guest_notifiers_used;
k->set_guest_notifiers = virtio_pci_set_guest_notifiers;
k->set_host_notifier_mr = virtio_pci_set_host_notifier_mr;
k->vmstate_change = virtio_pci_vmstate_change;
@@ -2630,4 +2636,3 @@ static void virtio_pci_register_types(void)
}
type_init(virtio_pci_register_types)
-
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 822fbacdf0..de4ab28046 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -256,6 +256,7 @@ void pci_del_capability(PCIDevice *pci_dev, uint8_t cap_id, uint8_t cap_size);
uint8_t pci_find_capability(PCIDevice *pci_dev, uint8_t cap_id);
+int pci_irq_disabled(PCIDevice *d);
uint32_t pci_default_read_config(PCIDevice *d,
uint32_t address, int len);
diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index 7ab8c9dab0..de75a44895 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -48,7 +48,7 @@ struct VirtioBusClass {
int (*load_done)(DeviceState *d, QEMUFile *f);
int (*load_extra_state)(DeviceState *d, QEMUFile *f);
bool (*has_extra_state)(DeviceState *d);
- bool (*query_guest_notifiers)(DeviceState *d);
+ bool (*query_guest_notifiers_used)(DeviceState *d, int n);
int (*set_guest_notifiers)(DeviceState *d, int nvqs, bool assign);
int (*set_host_notifier_mr)(DeviceState *d, int n,
MemoryRegion *mr, bool assign);
--
2.43.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] vhost: Don't set vring call if guest notifier is unused
2025-03-26 8:25 [PATCH] vhost: Don't set vring call if guest notifier is unused oenhan
@ 2025-04-01 10:49 ` Stefano Garzarella
2025-04-07 7:53 ` Huaitong Han
2025-05-11 16:13 ` Michael S. Tsirkin
1 sibling, 1 reply; 5+ messages in thread
From: Stefano Garzarella @ 2025-04-01 10:49 UTC (permalink / raw)
To: oenhan
Cc: mst, marcel.apfelbaum, cohuck, pasic, farman, borntraeger,
leiyang, qemu-devel, qemu-stable, Huaitong Han, Zhiyuan Yuan,
Jidong Xia
On Wed, Mar 26, 2025 at 04:25:37PM +0800, oenhan@gmail.com wrote:
>From: Huaitong Han <hanht2@chinatelecom.cn>
>
>The vring call fd is set even when the guest does not use msix (e.g., in the
>case of virtio pmd), leading to unnecessary CPU overhead for processing
>interrupts. The previous patch optimized the condition where msix is
What would be the previous patch?
If you're referencing a patch already merged, please add them in the
same form you used in Fixes, for example:
Commit XXXXX ("vhost: ...") optimized the condition ...
>enabled
>but the queue vector is unset. However, there is a additional case where the
>guest interrupt notifier is effectively unused and the vring call fd should
>also be cleared: the guest does not use msix and the INTX_DISABLED bit in the PCI
>config is set.
I don't know this code very well, can you explain better what you are
changing with this patch and why change the name of
query_guest_notifiers?
>
>Fixes: 96a3d98d2c ("vhost: don't set vring call if no vector")
>
>Test result:
>https://raw.githubusercontent.com/oenhan/build_log/refs/heads/main/qemu/2503261015/build/meson-logs/testlog.txt
Does it make sense to have this link in the commit, will it always be
accessible?
>
>Reported-by: Zhiyuan Yuan <yuanzhiyuan@chinatelecom.cn>
>Signed-off-by: Jidong Xia <xiajd@chinatelecom.cn>
>Signed-off-by: Huaitong Han <hanht2@chinatelecom.cn>
>---
> hw/pci/pci.c | 2 +-
> hw/s390x/virtio-ccw.c | 9 ++++++---
> hw/virtio/vhost.c | 5 ++---
> hw/virtio/virtio-pci.c | 15 ++++++++++-----
> include/hw/pci/pci.h | 1 +
> include/hw/virtio/virtio-bus.h | 2 +-
> 6 files changed, 21 insertions(+), 13 deletions(-)
>
>diff --git a/hw/pci/pci.c b/hw/pci/pci.c
>index 2844ec5556..503a897528 100644
>--- a/hw/pci/pci.c
>+++ b/hw/pci/pci.c
>@@ -1719,7 +1719,7 @@ static void pci_update_mappings(PCIDevice *d)
> pci_update_vga(d);
> }
>
>-static inline int pci_irq_disabled(PCIDevice *d)
>+int pci_irq_disabled(PCIDevice *d)
> {
> return pci_get_word(d->config + PCI_COMMAND) & PCI_COMMAND_INTX_DISABLE;
> }
>diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
>index 43f3b162c8..af482a22cd 100644
>--- a/hw/s390x/virtio-ccw.c
>+++ b/hw/s390x/virtio-ccw.c
>@@ -936,11 +936,14 @@ static void virtio_ccw_vmstate_change(DeviceState *d, bool running)
> }
> }
>
>-static bool virtio_ccw_query_guest_notifiers(DeviceState *d)
>+static bool virtio_ccw_query_guest_notifiers_used(DeviceState *d, int n)
> {
> CcwDevice *dev = CCW_DEVICE(d);
>+ VirtioCcwDevice *vdev = VIRTIO_CCW_DEVICE(d);
>+ VirtIODevice *virtio_dev = virtio_bus_get_device(&vdev->bus);
>
>- return !!(dev->sch->curr_status.pmcw.flags & PMCW_FLAGS_MASK_ENA);
>+ return !!(dev->sch->curr_status.pmcw.flags & PMCW_FLAGS_MASK_ENA)
>+ && virtio_queue_vector(virtio_dev, n) != VIRTIO_NO_VECTOR;
> }
>
> static int virtio_ccw_get_mappings(VirtioCcwDevice *dev)
>@@ -1270,7 +1273,7 @@ static void virtio_ccw_bus_class_init(ObjectClass *klass, void *data)
> bus_class->max_dev = 1;
> k->notify = virtio_ccw_notify;
> k->vmstate_change = virtio_ccw_vmstate_change;
>- k->query_guest_notifiers = virtio_ccw_query_guest_notifiers;
>+ k->query_guest_notifiers_used = virtio_ccw_query_guest_notifiers_used;
> k->set_guest_notifiers = virtio_ccw_set_guest_notifiers;
> k->save_queue = virtio_ccw_save_queue;
> k->load_queue = virtio_ccw_load_queue;
>diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>index 6aa72fd434..32634da836 100644
>--- a/hw/virtio/vhost.c
>+++ b/hw/virtio/vhost.c
>@@ -1341,9 +1341,8 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
> vhost_virtqueue_mask(dev, vdev, idx, false);
> }
>
>- if (k->query_guest_notifiers &&
>- k->query_guest_notifiers(qbus->parent) &&
>- virtio_queue_vector(vdev, idx) == VIRTIO_NO_VECTOR) {
>+ if (k->query_guest_notifiers_used &&
>+ !k->query_guest_notifiers_used(qbus->parent, idx)) {
> file.fd = -1;
> r = dev->vhost_ops->vhost_set_vring_call(dev, &file);
> if (r) {
>diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
>index 3ca3f849d3..25ff869618 100644
>--- a/hw/virtio/virtio-pci.c
>+++ b/hw/virtio/virtio-pci.c
>@@ -30,6 +30,7 @@
> #include "qemu/error-report.h"
> #include "qemu/log.h"
> #include "qemu/module.h"
>+#include "hw/pci/pci.h"
> #include "hw/pci/msi.h"
> #include "hw/pci/msix.h"
> #include "hw/loader.h"
>@@ -1031,7 +1032,7 @@ static void virtio_pci_one_vector_mask(VirtIOPCIProxy *proxy,
>
> /* If guest supports masking, keep irqfd but mask it.
> * Otherwise, clean it up now.
>- */
>+ */
Unrelated change.
> if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
> k->guest_notifier_mask(vdev, queue_no, true);
> } else {
>@@ -1212,10 +1213,15 @@ static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign,
> return 0;
> }
>
>-static bool virtio_pci_query_guest_notifiers(DeviceState *d)
>+static bool virtio_pci_query_guest_notifiers_used(DeviceState *d, int n)
> {
> VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
>- return msix_enabled(&proxy->pci_dev);
>+ VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
>+
>+ if (msix_enabled(&proxy->pci_dev))
>+ return virtio_queue_vector(vdev, n) != VIRTIO_NO_VECTOR;
>+ else
>+ return !pci_irq_disabled(&proxy->pci_dev);
> }
>
> static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
>@@ -2599,7 +2605,7 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
> k->save_extra_state = virtio_pci_save_extra_state;
> k->load_extra_state = virtio_pci_load_extra_state;
> k->has_extra_state = virtio_pci_has_extra_state;
>- k->query_guest_notifiers = virtio_pci_query_guest_notifiers;
>+ k->query_guest_notifiers_used = virtio_pci_query_guest_notifiers_used;
> k->set_guest_notifiers = virtio_pci_set_guest_notifiers;
> k->set_host_notifier_mr = virtio_pci_set_host_notifier_mr;
> k->vmstate_change = virtio_pci_vmstate_change;
>@@ -2630,4 +2636,3 @@ static void virtio_pci_register_types(void)
> }
>
> type_init(virtio_pci_register_types)
>-
>diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
>index 822fbacdf0..de4ab28046 100644
>--- a/include/hw/pci/pci.h
>+++ b/include/hw/pci/pci.h
>@@ -256,6 +256,7 @@ void pci_del_capability(PCIDevice *pci_dev, uint8_t cap_id, uint8_t cap_size);
>
> uint8_t pci_find_capability(PCIDevice *pci_dev, uint8_t cap_id);
>
>+int pci_irq_disabled(PCIDevice *d);
>
> uint32_t pci_default_read_config(PCIDevice *d,
> uint32_t address, int len);
>diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
>index 7ab8c9dab0..de75a44895 100644
>--- a/include/hw/virtio/virtio-bus.h
>+++ b/include/hw/virtio/virtio-bus.h
>@@ -48,7 +48,7 @@ struct VirtioBusClass {
> int (*load_done)(DeviceState *d, QEMUFile *f);
> int (*load_extra_state)(DeviceState *d, QEMUFile *f);
> bool (*has_extra_state)(DeviceState *d);
>- bool (*query_guest_notifiers)(DeviceState *d);
>+ bool (*query_guest_notifiers_used)(DeviceState *d, int n);
> int (*set_guest_notifiers)(DeviceState *d, int nvqs, bool assign);
> int (*set_host_notifier_mr)(DeviceState *d, int n,
> MemoryRegion *mr, bool assign);
>--
>2.43.5
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] vhost: Don't set vring call if guest notifier is unused
2025-04-01 10:49 ` Stefano Garzarella
@ 2025-04-07 7:53 ` Huaitong Han
2025-04-16 9:38 ` Stefano Garzarella
0 siblings, 1 reply; 5+ messages in thread
From: Huaitong Han @ 2025-04-07 7:53 UTC (permalink / raw)
To: Stefano Garzarella
Cc: mst, marcel.apfelbaum, cohuck, pasic, farman, borntraeger,
leiyang, qemu-devel, qemu-stable, Huaitong Han, Zhiyuan Yuan,
Jidong Xia
Stefano Garzarella <sgarzare@redhat.com> 于2025年4月1日周二 18:49写道：
>
> On Wed, Mar 26, 2025 at 04:25:37PM +0800, oenhan@gmail.com wrote:
> >From: Huaitong Han <hanht2@chinatelecom.cn>
> >
> >The vring call fd is set even when the guest does not use msix (e.g., in the
> >case of virtio pmd), leading to unnecessary CPU overhead for processing
> >interrupts. The previous patch optimized the condition where msix is
>
> What would be the previous patch?
>
> If you're referencing a patch already merged, please add them in the
> same form you used in Fixes, for example:
> Commit XXXXX ("vhost: ...") optimized the condition ...
>
Accept
> >enabled
> >but the queue vector is unset. However, there is a additional case where the
> >guest interrupt notifier is effectively unused and the vring call fd should
> >also be cleared: the guest does not use msix and the INTX_DISABLED bit in the PCI
> >config is set.
>
> I don't know this code very well, can you explain better what you are
> changing with this patch and why change the name of
> query_guest_notifiers?
>
There's the case where the guest uses INTx, but the INTx_DISABLED bit
is set — meaning no notifier will actually be used either. This patch
generalizes the logic to cover these cases.
The current name might be misleading. It seems to imply the device has
notifiers enabled, but in fact, the check is meant to reflect whether
the guest is actively using any interrupt mechanism. I felt renaming
it could make the intent more clear and reduce confusion. Let me know
if you think it's better to keep the original name.
> >
> >Fixes: 96a3d98d2c ("vhost: don't set vring call if no vector")
> >
> >Test result:
> >https://raw.githubusercontent.com/oenhan/build_log/refs/heads/main/qemu/2503261015/build/meson-logs/testlog.txt
>
> Does it make sense to have this link in the commit, will it always be
> accessible?
MST initially asked me to make the test result public. I understand
that embedding a direct test log link into the commit log is not good,
I will remove the test result link from the commit log in the next
version, and instead include it in the email reply body.
>
> >
> >Reported-by: Zhiyuan Yuan <yuanzhiyuan@chinatelecom.cn>
> >Signed-off-by: Jidong Xia <xiajd@chinatelecom.cn>
> >Signed-off-by: Huaitong Han <hanht2@chinatelecom.cn>
> >---
> > hw/pci/pci.c | 2 +-
> > hw/s390x/virtio-ccw.c | 9 ++++++---
> > hw/virtio/vhost.c | 5 ++---
> > hw/virtio/virtio-pci.c | 15 ++++++++++-----
> > include/hw/pci/pci.h | 1 +
> > include/hw/virtio/virtio-bus.h | 2 +-
> > 6 files changed, 21 insertions(+), 13 deletions(-)
> >
> >diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> >index 2844ec5556..503a897528 100644
> >--- a/hw/pci/pci.c
> >+++ b/hw/pci/pci.c
> >@@ -1719,7 +1719,7 @@ static void pci_update_mappings(PCIDevice *d)
> > pci_update_vga(d);
> > }
> >
> >-static inline int pci_irq_disabled(PCIDevice *d)
> >+int pci_irq_disabled(PCIDevice *d)
> > {
> > return pci_get_word(d->config + PCI_COMMAND) & PCI_COMMAND_INTX_DISABLE;
> > }
> >diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> >index 43f3b162c8..af482a22cd 100644
> >--- a/hw/s390x/virtio-ccw.c
> >+++ b/hw/s390x/virtio-ccw.c
> >@@ -936,11 +936,14 @@ static void virtio_ccw_vmstate_change(DeviceState *d, bool running)
> > }
> > }
> >
> >-static bool virtio_ccw_query_guest_notifiers(DeviceState *d)
> >+static bool virtio_ccw_query_guest_notifiers_used(DeviceState *d, int n)
> > {
> > CcwDevice *dev = CCW_DEVICE(d);
> >+ VirtioCcwDevice *vdev = VIRTIO_CCW_DEVICE(d);
> >+ VirtIODevice *virtio_dev = virtio_bus_get_device(&vdev->bus);
> >
> >- return !!(dev->sch->curr_status.pmcw.flags & PMCW_FLAGS_MASK_ENA);
> >+ return !!(dev->sch->curr_status.pmcw.flags & PMCW_FLAGS_MASK_ENA)
> >+ && virtio_queue_vector(virtio_dev, n) != VIRTIO_NO_VECTOR;
> > }
> >
> > static int virtio_ccw_get_mappings(VirtioCcwDevice *dev)
> >@@ -1270,7 +1273,7 @@ static void virtio_ccw_bus_class_init(ObjectClass *klass, void *data)
> > bus_class->max_dev = 1;
> > k->notify = virtio_ccw_notify;
> > k->vmstate_change = virtio_ccw_vmstate_change;
> >- k->query_guest_notifiers = virtio_ccw_query_guest_notifiers;
> >+ k->query_guest_notifiers_used = virtio_ccw_query_guest_notifiers_used;
> > k->set_guest_notifiers = virtio_ccw_set_guest_notifiers;
> > k->save_queue = virtio_ccw_save_queue;
> > k->load_queue = virtio_ccw_load_queue;
> >diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> >index 6aa72fd434..32634da836 100644
> >--- a/hw/virtio/vhost.c
> >+++ b/hw/virtio/vhost.c
> >@@ -1341,9 +1341,8 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
> > vhost_virtqueue_mask(dev, vdev, idx, false);
> > }
> >
> >- if (k->query_guest_notifiers &&
> >- k->query_guest_notifiers(qbus->parent) &&
> >- virtio_queue_vector(vdev, idx) == VIRTIO_NO_VECTOR) {
> >+ if (k->query_guest_notifiers_used &&
> >+ !k->query_guest_notifiers_used(qbus->parent, idx)) {
> > file.fd = -1;
> > r = dev->vhost_ops->vhost_set_vring_call(dev, &file);
> > if (r) {
> >diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> >index 3ca3f849d3..25ff869618 100644
> >--- a/hw/virtio/virtio-pci.c
> >+++ b/hw/virtio/virtio-pci.c
> >@@ -30,6 +30,7 @@
> > #include "qemu/error-report.h"
> > #include "qemu/log.h"
> > #include "qemu/module.h"
> >+#include "hw/pci/pci.h"
> > #include "hw/pci/msi.h"
> > #include "hw/pci/msix.h"
> > #include "hw/loader.h"
> >@@ -1031,7 +1032,7 @@ static void virtio_pci_one_vector_mask(VirtIOPCIProxy *proxy,
> >
> > /* If guest supports masking, keep irqfd but mask it.
> > * Otherwise, clean it up now.
> >- */
> >+ */
>
> Unrelated change.
>
> > if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
> > k->guest_notifier_mask(vdev, queue_no, true);
> > } else {
> >@@ -1212,10 +1213,15 @@ static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign,
> > return 0;
> > }
> >
> >-static bool virtio_pci_query_guest_notifiers(DeviceState *d)
> >+static bool virtio_pci_query_guest_notifiers_used(DeviceState *d, int n)
> > {
> > VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
> >- return msix_enabled(&proxy->pci_dev);
> >+ VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> >+
> >+ if (msix_enabled(&proxy->pci_dev))
> >+ return virtio_queue_vector(vdev, n) != VIRTIO_NO_VECTOR;
> >+ else
> >+ return !pci_irq_disabled(&proxy->pci_dev);
> > }
> >
> > static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
> >@@ -2599,7 +2605,7 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
> > k->save_extra_state = virtio_pci_save_extra_state;
> > k->load_extra_state = virtio_pci_load_extra_state;
> > k->has_extra_state = virtio_pci_has_extra_state;
> >- k->query_guest_notifiers = virtio_pci_query_guest_notifiers;
> >+ k->query_guest_notifiers_used = virtio_pci_query_guest_notifiers_used;
> > k->set_guest_notifiers = virtio_pci_set_guest_notifiers;
> > k->set_host_notifier_mr = virtio_pci_set_host_notifier_mr;
> > k->vmstate_change = virtio_pci_vmstate_change;
> >@@ -2630,4 +2636,3 @@ static void virtio_pci_register_types(void)
> > }
> >
> > type_init(virtio_pci_register_types)
> >-
> >diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> >index 822fbacdf0..de4ab28046 100644
> >--- a/include/hw/pci/pci.h
> >+++ b/include/hw/pci/pci.h
> >@@ -256,6 +256,7 @@ void pci_del_capability(PCIDevice *pci_dev, uint8_t cap_id, uint8_t cap_size);
> >
> > uint8_t pci_find_capability(PCIDevice *pci_dev, uint8_t cap_id);
> >
> >+int pci_irq_disabled(PCIDevice *d);
> >
> > uint32_t pci_default_read_config(PCIDevice *d,
> > uint32_t address, int len);
> >diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
> >index 7ab8c9dab0..de75a44895 100644
> >--- a/include/hw/virtio/virtio-bus.h
> >+++ b/include/hw/virtio/virtio-bus.h
> >@@ -48,7 +48,7 @@ struct VirtioBusClass {
> > int (*load_done)(DeviceState *d, QEMUFile *f);
> > int (*load_extra_state)(DeviceState *d, QEMUFile *f);
> > bool (*has_extra_state)(DeviceState *d);
> >- bool (*query_guest_notifiers)(DeviceState *d);
> >+ bool (*query_guest_notifiers_used)(DeviceState *d, int n);
> > int (*set_guest_notifiers)(DeviceState *d, int nvqs, bool assign);
> > int (*set_host_notifier_mr)(DeviceState *d, int n,
> > MemoryRegion *mr, bool assign);
> >--
> >2.43.5
> >
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] vhost: Don't set vring call if guest notifier is unused
2025-04-07 7:53 ` Huaitong Han
@ 2025-04-16 9:38 ` Stefano Garzarella
0 siblings, 0 replies; 5+ messages in thread
From: Stefano Garzarella @ 2025-04-16 9:38 UTC (permalink / raw)
To: Huaitong Han
Cc: mst, marcel.apfelbaum, cohuck, pasic, farman, borntraeger,
leiyang, qemu-devel, qemu-stable, Huaitong Han, Zhiyuan Yuan,
Jidong Xia
On Mon, Apr 07, 2025 at 03:53:24PM +0800, Huaitong Han wrote:
>Stefano Garzarella <sgarzare@redhat.com> 于2025年4月1日周二 18:49写道：
>
>>
>> On Wed, Mar 26, 2025 at 04:25:37PM +0800, oenhan@gmail.com wrote:
>> >From: Huaitong Han <hanht2@chinatelecom.cn>
>> >
>> >The vring call fd is set even when the guest does not use msix (e.g., in the
>> >case of virtio pmd), leading to unnecessary CPU overhead for processing
>> >interrupts. The previous patch optimized the condition where msix is
>>
>> What would be the previous patch?
>>
>> If you're referencing a patch already merged, please add them in the
>> same form you used in Fixes, for example:
>> Commit XXXXX ("vhost: ...") optimized the condition ...
>>
>Accept
>
>> >enabled
>> >but the queue vector is unset. However, there is a additional case
>> >where the
>> >guest interrupt notifier is effectively unused and the vring call fd
>> >should
>> >also be cleared: the guest does not use msix and the INTX_DISABLED
>> >bit in the PCI
>> >config is set.
>>
>> I don't know this code very well, can you explain better what you are
>> changing with this patch and why change the name of
>> query_guest_notifiers?
>>
>There's the case where the guest uses INTx, but the INTx_DISABLED bit
>is set — meaning no notifier will actually be used either. This patch
>generalizes the logic to cover these cases.
>The current name might be misleading. It seems to imply the device has
>notifiers enabled, but in fact, the check is meant to reflect whether
>the guest is actively using any interrupt mechanism. I felt renaming
>it could make the intent more clear and reduce confusion. Let me know
>if you think it's better to keep the original name.
Okay, now it's a bit more clear, so I suggest to put these kind of
information in the commit description. It should always contains the
reason of changes.
In this case I don't know if it's better to split into 2 patches, one
where you fix the behavior and one where you update the name, but I
leave it to the maintainers to decide, even one would be fine if
explained well in the commit description.
>
>> >
>> >Fixes: 96a3d98d2c ("vhost: don't set vring call if no vector")
>> >
>> >Test result:
>> >https://raw.githubusercontent.com/oenhan/build_log/refs/heads/main/qemu/2503261015/build/meson-logs/testlog.txt
>>
>> Does it make sense to have this link in the commit, will it always be
>> accessible?
>MST initially asked me to make the test result public. I understand
>that embedding a direct test log link into the commit log is not good,
>I will remove the test result link from the commit log in the next
>version, and instead include it in the email reply body.
Yep, I think after --- should be fine.
Thanks,
Stefano
>
>>
>> >
>> >Reported-by: Zhiyuan Yuan <yuanzhiyuan@chinatelecom.cn>
>> >Signed-off-by: Jidong Xia <xiajd@chinatelecom.cn>
>> >Signed-off-by: Huaitong Han <hanht2@chinatelecom.cn>
>> >---
>> > hw/pci/pci.c | 2 +-
>> > hw/s390x/virtio-ccw.c | 9 ++++++---
>> > hw/virtio/vhost.c | 5 ++---
>> > hw/virtio/virtio-pci.c | 15 ++++++++++-----
>> > include/hw/pci/pci.h | 1 +
>> > include/hw/virtio/virtio-bus.h | 2 +-
>> > 6 files changed, 21 insertions(+), 13 deletions(-)
>> >
>> >diff --git a/hw/pci/pci.c b/hw/pci/pci.c
>> >index 2844ec5556..503a897528 100644
>> >--- a/hw/pci/pci.c
>> >+++ b/hw/pci/pci.c
>> >@@ -1719,7 +1719,7 @@ static void pci_update_mappings(PCIDevice *d)
>> > pci_update_vga(d);
>> > }
>> >
>> >-static inline int pci_irq_disabled(PCIDevice *d)
>> >+int pci_irq_disabled(PCIDevice *d)
>> > {
>> > return pci_get_word(d->config + PCI_COMMAND) & PCI_COMMAND_INTX_DISABLE;
>> > }
>> >diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
>> >index 43f3b162c8..af482a22cd 100644
>> >--- a/hw/s390x/virtio-ccw.c
>> >+++ b/hw/s390x/virtio-ccw.c
>> >@@ -936,11 +936,14 @@ static void virtio_ccw_vmstate_change(DeviceState *d, bool running)
>> > }
>> > }
>> >
>> >-static bool virtio_ccw_query_guest_notifiers(DeviceState *d)
>> >+static bool virtio_ccw_query_guest_notifiers_used(DeviceState *d, int n)
>> > {
>> > CcwDevice *dev = CCW_DEVICE(d);
>> >+ VirtioCcwDevice *vdev = VIRTIO_CCW_DEVICE(d);
>> >+ VirtIODevice *virtio_dev = virtio_bus_get_device(&vdev->bus);
>> >
>> >- return !!(dev->sch->curr_status.pmcw.flags & PMCW_FLAGS_MASK_ENA);
>> >+ return !!(dev->sch->curr_status.pmcw.flags & PMCW_FLAGS_MASK_ENA)
>> >+ && virtio_queue_vector(virtio_dev, n) != VIRTIO_NO_VECTOR;
>> > }
>> >
>> > static int virtio_ccw_get_mappings(VirtioCcwDevice *dev)
>> >@@ -1270,7 +1273,7 @@ static void virtio_ccw_bus_class_init(ObjectClass *klass, void *data)
>> > bus_class->max_dev = 1;
>> > k->notify = virtio_ccw_notify;
>> > k->vmstate_change = virtio_ccw_vmstate_change;
>> >- k->query_guest_notifiers = virtio_ccw_query_guest_notifiers;
>> >+ k->query_guest_notifiers_used = virtio_ccw_query_guest_notifiers_used;
>> > k->set_guest_notifiers = virtio_ccw_set_guest_notifiers;
>> > k->save_queue = virtio_ccw_save_queue;
>> > k->load_queue = virtio_ccw_load_queue;
>> >diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>> >index 6aa72fd434..32634da836 100644
>> >--- a/hw/virtio/vhost.c
>> >+++ b/hw/virtio/vhost.c
>> >@@ -1341,9 +1341,8 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
>> > vhost_virtqueue_mask(dev, vdev, idx, false);
>> > }
>> >
>> >- if (k->query_guest_notifiers &&
>> >- k->query_guest_notifiers(qbus->parent) &&
>> >- virtio_queue_vector(vdev, idx) == VIRTIO_NO_VECTOR) {
>> >+ if (k->query_guest_notifiers_used &&
>> >+ !k->query_guest_notifiers_used(qbus->parent, idx)) {
>> > file.fd = -1;
>> > r = dev->vhost_ops->vhost_set_vring_call(dev, &file);
>> > if (r) {
>> >diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
>> >index 3ca3f849d3..25ff869618 100644
>> >--- a/hw/virtio/virtio-pci.c
>> >+++ b/hw/virtio/virtio-pci.c
>> >@@ -30,6 +30,7 @@
>> > #include "qemu/error-report.h"
>> > #include "qemu/log.h"
>> > #include "qemu/module.h"
>> >+#include "hw/pci/pci.h"
>> > #include "hw/pci/msi.h"
>> > #include "hw/pci/msix.h"
>> > #include "hw/loader.h"
>> >@@ -1031,7 +1032,7 @@ static void virtio_pci_one_vector_mask(VirtIOPCIProxy *proxy,
>> >
>> > /* If guest supports masking, keep irqfd but mask it.
>> > * Otherwise, clean it up now.
>> >- */
>> >+ */
>>
>> Unrelated change.
>>
>> > if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
>> > k->guest_notifier_mask(vdev, queue_no, true);
>> > } else {
>> >@@ -1212,10 +1213,15 @@ static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign,
>> > return 0;
>> > }
>> >
>> >-static bool virtio_pci_query_guest_notifiers(DeviceState *d)
>> >+static bool virtio_pci_query_guest_notifiers_used(DeviceState *d, int n)
>> > {
>> > VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
>> >- return msix_enabled(&proxy->pci_dev);
>> >+ VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
>> >+
>> >+ if (msix_enabled(&proxy->pci_dev))
>> >+ return virtio_queue_vector(vdev, n) != VIRTIO_NO_VECTOR;
>> >+ else
>> >+ return !pci_irq_disabled(&proxy->pci_dev);
>> > }
>> >
>> > static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
>> >@@ -2599,7 +2605,7 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
>> > k->save_extra_state = virtio_pci_save_extra_state;
>> > k->load_extra_state = virtio_pci_load_extra_state;
>> > k->has_extra_state = virtio_pci_has_extra_state;
>> >- k->query_guest_notifiers = virtio_pci_query_guest_notifiers;
>> >+ k->query_guest_notifiers_used = virtio_pci_query_guest_notifiers_used;
>> > k->set_guest_notifiers = virtio_pci_set_guest_notifiers;
>> > k->set_host_notifier_mr = virtio_pci_set_host_notifier_mr;
>> > k->vmstate_change = virtio_pci_vmstate_change;
>> >@@ -2630,4 +2636,3 @@ static void virtio_pci_register_types(void)
>> > }
>> >
>> > type_init(virtio_pci_register_types)
>> >-
>> >diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
>> >index 822fbacdf0..de4ab28046 100644
>> >--- a/include/hw/pci/pci.h
>> >+++ b/include/hw/pci/pci.h
>> >@@ -256,6 +256,7 @@ void pci_del_capability(PCIDevice *pci_dev, uint8_t cap_id, uint8_t cap_size);
>> >
>> > uint8_t pci_find_capability(PCIDevice *pci_dev, uint8_t cap_id);
>> >
>> >+int pci_irq_disabled(PCIDevice *d);
>> >
>> > uint32_t pci_default_read_config(PCIDevice *d,
>> > uint32_t address, int len);
>> >diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
>> >index 7ab8c9dab0..de75a44895 100644
>> >--- a/include/hw/virtio/virtio-bus.h
>> >+++ b/include/hw/virtio/virtio-bus.h
>> >@@ -48,7 +48,7 @@ struct VirtioBusClass {
>> > int (*load_done)(DeviceState *d, QEMUFile *f);
>> > int (*load_extra_state)(DeviceState *d, QEMUFile *f);
>> > bool (*has_extra_state)(DeviceState *d);
>> >- bool (*query_guest_notifiers)(DeviceState *d);
>> >+ bool (*query_guest_notifiers_used)(DeviceState *d, int n);
>> > int (*set_guest_notifiers)(DeviceState *d, int nvqs, bool assign);
>> > int (*set_host_notifier_mr)(DeviceState *d, int n,
>> > MemoryRegion *mr, bool assign);
>> >--
>> >2.43.5
>> >
>>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] vhost: Don't set vring call if guest notifier is unused
2025-03-26 8:25 [PATCH] vhost: Don't set vring call if guest notifier is unused oenhan
2025-04-01 10:49 ` Stefano Garzarella
@ 2025-05-11 16:13 ` Michael S. Tsirkin
1 sibling, 0 replies; 5+ messages in thread
From: Michael S. Tsirkin @ 2025-05-11 16:13 UTC (permalink / raw)
To: oenhan
Cc: sgarzare, marcel.apfelbaum, cohuck, pasic, farman, borntraeger,
leiyang, qemu-devel, qemu-stable, Huaitong Han, Zhiyuan Yuan,
Jidong Xia
Thanks for the patch!
I an waiting for Stefan's comments to be addressed.
Additionally, something to improve:
On Wed, Mar 26, 2025 at 04:25:37PM +0800, oenhan@gmail.com wrote:
> From: Huaitong Han <hanht2@chinatelecom.cn>
>
> The vring call fd is set even when the guest does not use msix (e.g., in the
> case of virtio pmd), leading to unnecessary CPU overhead for processing
> interrupts. The previous patch optimized the condition where msix is enabled
> but the queue vector is unset. However, there is a additional case where the
> guest interrupt notifier is effectively unused and the vring call fd should
> also be cleared: the guest does not use msix and the INTX_DISABLED bit in the PCI
> config is set.
>
> Fixes: 96a3d98d2c ("vhost: don't set vring call if no vector")
this must be with no empty lines adjacent to the rest of trailers.
> Test result:
> https://raw.githubusercontent.com/oenhan/build_log/refs/heads/main/qemu/2503261015/build/meson-logs/testlog.txt
just include the summary here inline.
> Reported-by: Zhiyuan Yuan <yuanzhiyuan@chinatelecom.cn>
> Signed-off-by: Jidong Xia <xiajd@chinatelecom.cn>
> Signed-off-by: Huaitong Han <hanht2@chinatelecom.cn>
> ---
> hw/pci/pci.c | 2 +-
> hw/s390x/virtio-ccw.c | 9 ++++++---
> hw/virtio/vhost.c | 5 ++---
> hw/virtio/virtio-pci.c | 15 ++++++++++-----
> include/hw/pci/pci.h | 1 +
> include/hw/virtio/virtio-bus.h | 2 +-
> 6 files changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 2844ec5556..503a897528 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1719,7 +1719,7 @@ static void pci_update_mappings(PCIDevice *d)
> pci_update_vga(d);
> }
>
> -static inline int pci_irq_disabled(PCIDevice *d)
> +int pci_irq_disabled(PCIDevice *d)
> {
> return pci_get_word(d->config + PCI_COMMAND) & PCI_COMMAND_INTX_DISABLE;
> }
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index 43f3b162c8..af482a22cd 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -936,11 +936,14 @@ static void virtio_ccw_vmstate_change(DeviceState *d, bool running)
> }
> }
>
> -static bool virtio_ccw_query_guest_notifiers(DeviceState *d)
> +static bool virtio_ccw_query_guest_notifiers_used(DeviceState *d, int n)
> {
> CcwDevice *dev = CCW_DEVICE(d);
> + VirtioCcwDevice *vdev = VIRTIO_CCW_DEVICE(d);
> + VirtIODevice *virtio_dev = virtio_bus_get_device(&vdev->bus);
>
> - return !!(dev->sch->curr_status.pmcw.flags & PMCW_FLAGS_MASK_ENA);
> + return !!(dev->sch->curr_status.pmcw.flags & PMCW_FLAGS_MASK_ENA)
> + && virtio_queue_vector(virtio_dev, n) != VIRTIO_NO_VECTOR;
> }
>
> static int virtio_ccw_get_mappings(VirtioCcwDevice *dev)
> @@ -1270,7 +1273,7 @@ static void virtio_ccw_bus_class_init(ObjectClass *klass, void *data)
> bus_class->max_dev = 1;
> k->notify = virtio_ccw_notify;
> k->vmstate_change = virtio_ccw_vmstate_change;
> - k->query_guest_notifiers = virtio_ccw_query_guest_notifiers;
> + k->query_guest_notifiers_used = virtio_ccw_query_guest_notifiers_used;
> k->set_guest_notifiers = virtio_ccw_set_guest_notifiers;
> k->save_queue = virtio_ccw_save_queue;
> k->load_queue = virtio_ccw_load_queue;
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 6aa72fd434..32634da836 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1341,9 +1341,8 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
> vhost_virtqueue_mask(dev, vdev, idx, false);
> }
>
> - if (k->query_guest_notifiers &&
> - k->query_guest_notifiers(qbus->parent) &&
> - virtio_queue_vector(vdev, idx) == VIRTIO_NO_VECTOR) {
> + if (k->query_guest_notifiers_used &&
> + !k->query_guest_notifiers_used(qbus->parent, idx)) {
> file.fd = -1;
> r = dev->vhost_ops->vhost_set_vring_call(dev, &file);
> if (r) {
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 3ca3f849d3..25ff869618 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -30,6 +30,7 @@
> #include "qemu/error-report.h"
> #include "qemu/log.h"
> #include "qemu/module.h"
> +#include "hw/pci/pci.h"
> #include "hw/pci/msi.h"
> #include "hw/pci/msix.h"
> #include "hw/loader.h"
> @@ -1031,7 +1032,7 @@ static void virtio_pci_one_vector_mask(VirtIOPCIProxy *proxy,
>
> /* If guest supports masking, keep irqfd but mask it.
> * Otherwise, clean it up now.
> - */
> + */
> if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
> k->guest_notifier_mask(vdev, queue_no, true);
> } else {
> @@ -1212,10 +1213,15 @@ static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign,
> return 0;
> }
>
> -static bool virtio_pci_query_guest_notifiers(DeviceState *d)
> +static bool virtio_pci_query_guest_notifiers_used(DeviceState *d, int n)
> {
> VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
> - return msix_enabled(&proxy->pci_dev);
> + VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> +
> + if (msix_enabled(&proxy->pci_dev))
> + return virtio_queue_vector(vdev, n) != VIRTIO_NO_VECTOR;
> + else
> + return !pci_irq_disabled(&proxy->pci_dev);
> }
>
> static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
> @@ -2599,7 +2605,7 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
> k->save_extra_state = virtio_pci_save_extra_state;
> k->load_extra_state = virtio_pci_load_extra_state;
> k->has_extra_state = virtio_pci_has_extra_state;
> - k->query_guest_notifiers = virtio_pci_query_guest_notifiers;
> + k->query_guest_notifiers_used = virtio_pci_query_guest_notifiers_used;
> k->set_guest_notifiers = virtio_pci_set_guest_notifiers;
> k->set_host_notifier_mr = virtio_pci_set_host_notifier_mr;
> k->vmstate_change = virtio_pci_vmstate_change;
> @@ -2630,4 +2636,3 @@ static void virtio_pci_register_types(void)
> }
>
> type_init(virtio_pci_register_types)
> -
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 822fbacdf0..de4ab28046 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -256,6 +256,7 @@ void pci_del_capability(PCIDevice *pci_dev, uint8_t cap_id, uint8_t cap_size);
>
> uint8_t pci_find_capability(PCIDevice *pci_dev, uint8_t cap_id);
>
> +int pci_irq_disabled(PCIDevice *d);
>
> uint32_t pci_default_read_config(PCIDevice *d,
> uint32_t address, int len);
> diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
> index 7ab8c9dab0..de75a44895 100644
> --- a/include/hw/virtio/virtio-bus.h
> +++ b/include/hw/virtio/virtio-bus.h
> @@ -48,7 +48,7 @@ struct VirtioBusClass {
> int (*load_done)(DeviceState *d, QEMUFile *f);
> int (*load_extra_state)(DeviceState *d, QEMUFile *f);
> bool (*has_extra_state)(DeviceState *d);
> - bool (*query_guest_notifiers)(DeviceState *d);
> + bool (*query_guest_notifiers_used)(DeviceState *d, int n);
I don't see why we need to change the name, and you don't explain
in the commit log. The new name isn't really clear, either.
> int (*set_guest_notifiers)(DeviceState *d, int nvqs, bool assign);
> int (*set_host_notifier_mr)(DeviceState *d, int n,
> MemoryRegion *mr, bool assign);
> --
> 2.43.5
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-05-11 16:14 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-26 8:25 [PATCH] vhost: Don't set vring call if guest notifier is unused oenhan
2025-04-01 10:49 ` Stefano Garzarella
2025-04-07 7:53 ` Huaitong Han
2025-04-16 9:38 ` Stefano Garzarella
2025-05-11 16:13 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).