* [PATCH v7 01/10] virtio: introduce macro IRTIO_CONFIG_IRQ_IDX 聽
2021-06-02 3:47 [PATCH v7 00/10] vhost-vdpa: add support for configure interrupt Cindy Lu
@ 2021-06-02 3:47 ` Cindy Lu
2021-06-02 3:47 ` [PATCH v7 02/10] virtio-pci:decouple virtqueue from interrupt setting process Cindy Lu
` (8 subsequent siblings)
9 siblings, 0 replies; 26+ messages in thread
From: Cindy Lu @ 2021-06-02 3:47 UTC (permalink / raw)
To: lulu, mst, jasowang, qemu-devel
In order to support configure interrupt for vhost-vdpa
introduce VIRTIO_CONFIG_IRQ_IDX -1as a queue index,
then we can reuse the function guest_notifier_mask and
guest_notifier_pending.
we add the check of queue index, if the driver
not support configure interrupt, the function will just return
Signed-off-by: Cindy Lu <lulu@redhat.com>
---
hw/display/vhost-user-gpu.c | 6 ++++++
hw/net/virtio-net.c | 10 +++++++---
hw/virtio/vhost-user-fs.c | 9 +++++++--
hw/virtio/vhost-vsock-common.c | 6 ++++++
hw/virtio/virtio-crypto.c | 6 ++++++
include/hw/virtio/virtio.h | 2 ++
6 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c
index 6cdaa1c73b..c232d55986 100644
--- a/hw/display/vhost-user-gpu.c
+++ b/hw/display/vhost-user-gpu.c
@@ -483,6 +483,9 @@ vhost_user_gpu_guest_notifier_pending(VirtIODevice *vdev, int idx)
{
VhostUserGPU *g = VHOST_USER_GPU(vdev);
+ if (idx == VIRTIO_CONFIG_IRQ_IDX) {
+ return false;
+ }
return vhost_virtqueue_pending(&g->vhost->dev, idx);
}
@@ -491,6 +494,9 @@ vhost_user_gpu_guest_notifier_mask(VirtIODevice *vdev, int idx, bool mask)
{
VhostUserGPU *g = VHOST_USER_GPU(vdev);
+ if (idx == VIRTIO_CONFIG_IRQ_IDX) {
+ return;
+ }
vhost_virtqueue_mask(&g->vhost->dev, vdev, idx, mask);
}
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 66b9ff4511..f50235b5d6 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3052,7 +3052,10 @@ static bool virtio_net_guest_notifier_pending(VirtIODevice *vdev, int idx)
VirtIONet *n = VIRTIO_NET(vdev);
NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(idx));
assert(n->vhost_started);
- return vhost_net_virtqueue_pending(get_vhost_net(nc->peer), idx);
+ if (idx != VIRTIO_CONFIG_IRQ_IDX) {
+ return vhost_net_virtqueue_pending(get_vhost_net(nc->peer), idx);
+ }
+ return false;
}
static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx,
@@ -3061,8 +3064,9 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx,
VirtIONet *n = VIRTIO_NET(vdev);
NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(idx));
assert(n->vhost_started);
- vhost_net_virtqueue_mask(get_vhost_net(nc->peer),
- vdev, idx, mask);
+ if (idx != VIRTIO_CONFIG_IRQ_IDX) {
+ vhost_net_virtqueue_mask(get_vhost_net(nc->peer), vdev, idx, mask);
+ }
}
static void virtio_net_set_config_size(VirtIONet *n, uint64_t host_features)
diff --git a/hw/virtio/vhost-user-fs.c b/hw/virtio/vhost-user-fs.c
index 6f7f91533d..7e8ca25f71 100644
--- a/hw/virtio/vhost-user-fs.c
+++ b/hw/virtio/vhost-user-fs.c
@@ -156,11 +156,13 @@ static void vuf_handle_output(VirtIODevice *vdev, VirtQueue *vq)
*/
}
-static void vuf_guest_notifier_mask(VirtIODevice *vdev, int idx,
- bool mask)
+static void vuf_guest_notifier_mask(VirtIODevice *vdev, int idx, bool mask)
{
VHostUserFS *fs = VHOST_USER_FS(vdev);
+ if (idx == VIRTIO_CONFIG_IRQ_IDX) {
+ return;
+ }
vhost_virtqueue_mask(&fs->vhost_dev, vdev, idx, mask);
}
@@ -168,6 +170,9 @@ static bool vuf_guest_notifier_pending(VirtIODevice *vdev, int idx)
{
VHostUserFS *fs = VHOST_USER_FS(vdev);
+ if (idx == VIRTIO_CONFIG_IRQ_IDX) {
+ return false;
+ }
return vhost_virtqueue_pending(&fs->vhost_dev, idx);
}
diff --git a/hw/virtio/vhost-vsock-common.c b/hw/virtio/vhost-vsock-common.c
index 4ad6e234ad..2112b44802 100644
--- a/hw/virtio/vhost-vsock-common.c
+++ b/hw/virtio/vhost-vsock-common.c
@@ -101,6 +101,9 @@ static void vhost_vsock_common_guest_notifier_mask(VirtIODevice *vdev, int idx,
{
VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(vdev);
+ if (idx == VIRTIO_CONFIG_IRQ_IDX) {
+ return;
+ }
vhost_virtqueue_mask(&vvc->vhost_dev, vdev, idx, mask);
}
@@ -109,6 +112,9 @@ static bool vhost_vsock_common_guest_notifier_pending(VirtIODevice *vdev,
{
VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(vdev);
+ if (idx == VIRTIO_CONFIG_IRQ_IDX) {
+ return false;
+ }
return vhost_virtqueue_pending(&vvc->vhost_dev, idx);
}
diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
index 54f9bbb789..1d5192f8b4 100644
--- a/hw/virtio/virtio-crypto.c
+++ b/hw/virtio/virtio-crypto.c
@@ -948,6 +948,9 @@ static void virtio_crypto_guest_notifier_mask(VirtIODevice *vdev, int idx,
assert(vcrypto->vhost_started);
+ if (idx == VIRTIO_CONFIG_IRQ_IDX) {
+ return;
+ }
cryptodev_vhost_virtqueue_mask(vdev, queue, idx, mask);
}
@@ -958,6 +961,9 @@ static bool virtio_crypto_guest_notifier_pending(VirtIODevice *vdev, int idx)
assert(vcrypto->vhost_started);
+ if (idx == VIRTIO_CONFIG_IRQ_IDX) {
+ return false;
+ }
return cryptodev_vhost_virtqueue_pending(vdev, queue, idx);
}
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index b7ece7a6a8..63cb9455ed 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -67,6 +67,8 @@ typedef struct VirtQueueElement
#define VIRTIO_NO_VECTOR 0xffff
+#define VIRTIO_CONFIG_IRQ_IDX -1
+
#define TYPE_VIRTIO_DEVICE "virtio-device"
OBJECT_DECLARE_TYPE(VirtIODevice, VirtioDeviceClass, VIRTIO_DEVICE)
--
2.21.3
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v7 02/10] virtio-pci:decouple virtqueue from interrupt setting process
2021-06-02 3:47 [PATCH v7 00/10] vhost-vdpa: add support for configure interrupt Cindy Lu
2021-06-02 3:47 ` [PATCH v7 01/10] virtio: introduce macro IRTIO_CONFIG_IRQ_IDX 聽 Cindy Lu
@ 2021-06-02 3:47 ` Cindy Lu
2021-06-03 5:51 ` Jason Wang
2021-06-02 3:47 ` [PATCH v7 03/10] virtio: decouple virtqueue from set notifier fd handler Cindy Lu
` (7 subsequent siblings)
9 siblings, 1 reply; 26+ messages in thread
From: Cindy Lu @ 2021-06-02 3:47 UTC (permalink / raw)
To: lulu, mst, jasowang, qemu-devel
Decouple virtqueue from interrupt setting process to support config interrupt
Now the code for interrupt/vector are coupling
with the vq number, this patch will decouple the vritqueue
numbers from these functions.
Signed-off-by: Cindy Lu <lulu@redhat.com>
---
hw/virtio/virtio-pci.c | 51 ++++++++++++++++++++++++------------------
1 file changed, 29 insertions(+), 22 deletions(-)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index b321604d9b..c5c080ec94 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -693,23 +693,17 @@ static void kvm_virtio_pci_vq_vector_release(VirtIOPCIProxy *proxy,
}
static int kvm_virtio_pci_irqfd_use(VirtIOPCIProxy *proxy,
- unsigned int queue_no,
+ EventNotifier *n,
unsigned int vector)
{
VirtIOIRQFD *irqfd = &proxy->vector_irqfd[vector];
- VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
- VirtQueue *vq = virtio_get_queue(vdev, queue_no);
- EventNotifier *n = virtio_queue_get_guest_notifier(vq);
return kvm_irqchip_add_irqfd_notifier_gsi(kvm_state, n, NULL, irqfd->virq);
}
static void kvm_virtio_pci_irqfd_release(VirtIOPCIProxy *proxy,
- unsigned int queue_no,
+ EventNotifier *n ,
unsigned int vector)
{
- VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
- VirtQueue *vq = virtio_get_queue(vdev, queue_no);
- EventNotifier *n = virtio_queue_get_guest_notifier(vq);
VirtIOIRQFD *irqfd = &proxy->vector_irqfd[vector];
int ret;
@@ -724,7 +718,8 @@ static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
unsigned int vector;
int ret, queue_no;
-
+ VirtQueue *vq;
+ EventNotifier *n;
for (queue_no = 0; queue_no < nvqs; queue_no++) {
if (!virtio_queue_get_num(vdev, queue_no)) {
break;
@@ -741,7 +736,9 @@ static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
* Otherwise, delay until unmasked in the frontend.
*/
if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
- ret = kvm_virtio_pci_irqfd_use(proxy, queue_no, vector);
+ vq = virtio_get_queue(vdev, queue_no);
+ n = virtio_queue_get_guest_notifier(vq);
+ ret = kvm_virtio_pci_irqfd_use(proxy, n, vector);
if (ret < 0) {
kvm_virtio_pci_vq_vector_release(proxy, vector);
goto undo;
@@ -757,7 +754,9 @@ undo:
continue;
}
if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
- kvm_virtio_pci_irqfd_release(proxy, queue_no, vector);
+ vq = virtio_get_queue(vdev, queue_no);
+ n = virtio_queue_get_guest_notifier(vq);
+ kvm_virtio_pci_irqfd_release(proxy, n, vector);
}
kvm_virtio_pci_vq_vector_release(proxy, vector);
}
@@ -771,7 +770,8 @@ static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
unsigned int vector;
int queue_no;
VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
-
+ VirtQueue *vq;
+ EventNotifier *n;
for (queue_no = 0; queue_no < nvqs; queue_no++) {
if (!virtio_queue_get_num(vdev, queue_no)) {
break;
@@ -784,7 +784,9 @@ static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
* Otherwise, it was cleaned when masked in the frontend.
*/
if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
- kvm_virtio_pci_irqfd_release(proxy, queue_no, vector);
+ vq = virtio_get_queue(vdev, queue_no);
+ n = virtio_queue_get_guest_notifier(vq);
+ kvm_virtio_pci_irqfd_release(proxy, n, vector);
}
kvm_virtio_pci_vq_vector_release(proxy, vector);
}
@@ -793,12 +795,11 @@ static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
static int virtio_pci_vq_vector_unmask(VirtIOPCIProxy *proxy,
unsigned int queue_no,
unsigned int vector,
- MSIMessage msg)
+ MSIMessage msg,
+ EventNotifier *n)
{
VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
- VirtQueue *vq = virtio_get_queue(vdev, queue_no);
- EventNotifier *n = virtio_queue_get_guest_notifier(vq);
VirtIOIRQFD *irqfd;
int ret = 0;
@@ -825,14 +826,15 @@ static int virtio_pci_vq_vector_unmask(VirtIOPCIProxy *proxy,
event_notifier_set(n);
}
} else {
- ret = kvm_virtio_pci_irqfd_use(proxy, queue_no, vector);
+ ret = kvm_virtio_pci_irqfd_use(proxy, n, vector);
}
return ret;
}
static void virtio_pci_vq_vector_mask(VirtIOPCIProxy *proxy,
unsigned int queue_no,
- unsigned int vector)
+ unsigned int vector,
+ EventNotifier *n)
{
VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
@@ -843,7 +845,7 @@ static void virtio_pci_vq_vector_mask(VirtIOPCIProxy *proxy,
if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
k->guest_notifier_mask(vdev, queue_no, true);
} else {
- kvm_virtio_pci_irqfd_release(proxy, queue_no, vector);
+ kvm_virtio_pci_irqfd_release(proxy, n, vector);
}
}
@@ -853,6 +855,7 @@ static int virtio_pci_vector_unmask(PCIDevice *dev, unsigned vector,
VirtIOPCIProxy *proxy = container_of(dev, VirtIOPCIProxy, pci_dev);
VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
VirtQueue *vq = virtio_vector_first_queue(vdev, vector);
+ EventNotifier *n;
int ret, index, unmasked = 0;
while (vq) {
@@ -861,7 +864,8 @@ static int virtio_pci_vector_unmask(PCIDevice *dev, unsigned vector,
break;
}
if (index < proxy->nvqs_with_notifiers) {
- ret = virtio_pci_vq_vector_unmask(proxy, index, vector, msg);
+ n = virtio_queue_get_guest_notifier(vq);
+ ret = virtio_pci_vq_vector_unmask(proxy, index, vector, msg, n);
if (ret < 0) {
goto undo;
}
@@ -877,7 +881,8 @@ undo:
while (vq && unmasked >= 0) {
index = virtio_get_queue_index(vq);
if (index < proxy->nvqs_with_notifiers) {
- virtio_pci_vq_vector_mask(proxy, index, vector);
+ n = virtio_queue_get_guest_notifier(vq);
+ virtio_pci_vq_vector_mask(proxy, index, vector, n);
--unmasked;
}
vq = virtio_vector_next_queue(vq);
@@ -890,15 +895,17 @@ static void virtio_pci_vector_mask(PCIDevice *dev, unsigned vector)
VirtIOPCIProxy *proxy = container_of(dev, VirtIOPCIProxy, pci_dev);
VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
VirtQueue *vq = virtio_vector_first_queue(vdev, vector);
+ EventNotifier *n;
int index;
while (vq) {
index = virtio_get_queue_index(vq);
+ n = virtio_queue_get_guest_notifier(vq);
if (!virtio_queue_get_num(vdev, index)) {
break;
}
if (index < proxy->nvqs_with_notifiers) {
- virtio_pci_vq_vector_mask(proxy, index, vector);
+ virtio_pci_vq_vector_mask(proxy, index, vector, n);
}
vq = virtio_vector_next_queue(vq);
}
--
2.21.3
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v7 02/10] virtio-pci:decouple virtqueue from interrupt setting process
2021-06-02 3:47 ` [PATCH v7 02/10] virtio-pci:decouple virtqueue from interrupt setting process Cindy Lu
@ 2021-06-03 5:51 ` Jason Wang
0 siblings, 0 replies; 26+ messages in thread
From: Jason Wang @ 2021-06-03 5:51 UTC (permalink / raw)
To: Cindy Lu, mst, qemu-devel
在 2021/6/2 上午11:47, Cindy Lu 写道:
> Decouple virtqueue from interrupt setting process to support config interrupt
> Now the code for interrupt/vector are coupling
> with the vq number, this patch will decouple the vritqueue
> numbers from these functions.
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
> hw/virtio/virtio-pci.c | 51 ++++++++++++++++++++++++------------------
> 1 file changed, 29 insertions(+), 22 deletions(-)
>
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index b321604d9b..c5c080ec94 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -693,23 +693,17 @@ static void kvm_virtio_pci_vq_vector_release(VirtIOPCIProxy *proxy,
> }
>
> static int kvm_virtio_pci_irqfd_use(VirtIOPCIProxy *proxy,
> - unsigned int queue_no,
> + EventNotifier *n,
> unsigned int vector)
> {
> VirtIOIRQFD *irqfd = &proxy->vector_irqfd[vector];
> - VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> - VirtQueue *vq = virtio_get_queue(vdev, queue_no);
> - EventNotifier *n = virtio_queue_get_guest_notifier(vq);
> return kvm_irqchip_add_irqfd_notifier_gsi(kvm_state, n, NULL, irqfd->virq);
> }
>
> static void kvm_virtio_pci_irqfd_release(VirtIOPCIProxy *proxy,
> - unsigned int queue_no,
> + EventNotifier *n ,
> unsigned int vector)
> {
> - VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> - VirtQueue *vq = virtio_get_queue(vdev, queue_no);
> - EventNotifier *n = virtio_queue_get_guest_notifier(vq);
> VirtIOIRQFD *irqfd = &proxy->vector_irqfd[vector];
> int ret;
>
> @@ -724,7 +718,8 @@ static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
> VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
> unsigned int vector;
> int ret, queue_no;
> -
> + VirtQueue *vq;
> + EventNotifier *n;
Let's leave a newline here.
> for (queue_no = 0; queue_no < nvqs; queue_no++) {
> if (!virtio_queue_get_num(vdev, queue_no)) {
> break;
> @@ -741,7 +736,9 @@ static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
> * Otherwise, delay until unmasked in the frontend.
> */
> if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
> - ret = kvm_virtio_pci_irqfd_use(proxy, queue_no, vector);
> + vq = virtio_get_queue(vdev, queue_no);
> + n = virtio_queue_get_guest_notifier(vq);
> + ret = kvm_virtio_pci_irqfd_use(proxy, n, vector);
> if (ret < 0) {
> kvm_virtio_pci_vq_vector_release(proxy, vector);
> goto undo;
> @@ -757,7 +754,9 @@ undo:
> continue;
> }
> if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
> - kvm_virtio_pci_irqfd_release(proxy, queue_no, vector);
> + vq = virtio_get_queue(vdev, queue_no);
> + n = virtio_queue_get_guest_notifier(vq);
> + kvm_virtio_pci_irqfd_release(proxy, n, vector);
> }
> kvm_virtio_pci_vq_vector_release(proxy, vector);
> }
> @@ -771,7 +770,8 @@ static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
> unsigned int vector;
> int queue_no;
> VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
> -
> + VirtQueue *vq;
> + EventNotifier *n;
Similar here.
> for (queue_no = 0; queue_no < nvqs; queue_no++) {
> if (!virtio_queue_get_num(vdev, queue_no)) {
> break;
> @@ -784,7 +784,9 @@ static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
> * Otherwise, it was cleaned when masked in the frontend.
> */
> if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
> - kvm_virtio_pci_irqfd_release(proxy, queue_no, vector);
> + vq = virtio_get_queue(vdev, queue_no);
> + n = virtio_queue_get_guest_notifier(vq);
> + kvm_virtio_pci_irqfd_release(proxy, n, vector);
> }
> kvm_virtio_pci_vq_vector_release(proxy, vector);
> }
> @@ -793,12 +795,11 @@ static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
> static int virtio_pci_vq_vector_unmask(VirtIOPCIProxy *proxy,
> unsigned int queue_no,
> unsigned int vector,
> - MSIMessage msg)
> + MSIMessage msg,
> + EventNotifier *n)
> {
A question: if this function needs to be used by configure interrupt, it
needs rename. Otherwise we don't need to bother since it only deal with
vq vector.
> VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
> - VirtQueue *vq = virtio_get_queue(vdev, queue_no);
> - EventNotifier *n = virtio_queue_get_guest_notifier(vq);
> VirtIOIRQFD *irqfd;
> int ret = 0;
>
> @@ -825,14 +826,15 @@ static int virtio_pci_vq_vector_unmask(VirtIOPCIProxy *proxy,
> event_notifier_set(n);
> }
> } else {
> - ret = kvm_virtio_pci_irqfd_use(proxy, queue_no, vector);
> + ret = kvm_virtio_pci_irqfd_use(proxy, n, vector);
> }
> return ret;
> }
>
> static void virtio_pci_vq_vector_mask(VirtIOPCIProxy *proxy,
> unsigned int queue_no,
> - unsigned int vector)
> + unsigned int vector,
> + EventNotifier *n)
> {
> VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
> @@ -843,7 +845,7 @@ static void virtio_pci_vq_vector_mask(VirtIOPCIProxy *proxy,
> if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
> k->guest_notifier_mask(vdev, queue_no, true);
> } else {
> - kvm_virtio_pci_irqfd_release(proxy, queue_no, vector);
> + kvm_virtio_pci_irqfd_release(proxy, n, vector);
> }
> }
>
> @@ -853,6 +855,7 @@ static int virtio_pci_vector_unmask(PCIDevice *dev, unsigned vector,
> VirtIOPCIProxy *proxy = container_of(dev, VirtIOPCIProxy, pci_dev);
> VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> VirtQueue *vq = virtio_vector_first_queue(vdev, vector);
> + EventNotifier *n;
> int ret, index, unmasked = 0;
>
> while (vq) {
> @@ -861,7 +864,8 @@ static int virtio_pci_vector_unmask(PCIDevice *dev, unsigned vector,
> break;
> }
> if (index < proxy->nvqs_with_notifiers) {
> - ret = virtio_pci_vq_vector_unmask(proxy, index, vector, msg);
> + n = virtio_queue_get_guest_notifier(vq);
> + ret = virtio_pci_vq_vector_unmask(proxy, index, vector, msg, n);
> if (ret < 0) {
> goto undo;
> }
> @@ -877,7 +881,8 @@ undo:
> while (vq && unmasked >= 0) {
> index = virtio_get_queue_index(vq);
> if (index < proxy->nvqs_with_notifiers) {
> - virtio_pci_vq_vector_mask(proxy, index, vector);
> + n = virtio_queue_get_guest_notifier(vq);
> + virtio_pci_vq_vector_mask(proxy, index, vector, n);
> --unmasked;
> }
> vq = virtio_vector_next_queue(vq);
> @@ -890,15 +895,17 @@ static void virtio_pci_vector_mask(PCIDevice *dev, unsigned vector)
> VirtIOPCIProxy *proxy = container_of(dev, VirtIOPCIProxy, pci_dev);
> VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> VirtQueue *vq = virtio_vector_first_queue(vdev, vector);
> + EventNotifier *n;
> int index;
>
> while (vq) {
> index = virtio_get_queue_index(vq);
> + n = virtio_queue_get_guest_notifier(vq);
Indentation looks wrong.
Thanks
> if (!virtio_queue_get_num(vdev, index)) {
> break;
> }
> if (index < proxy->nvqs_with_notifiers) {
> - virtio_pci_vq_vector_mask(proxy, index, vector);
> + virtio_pci_vq_vector_mask(proxy, index, vector, n);
> }
> vq = virtio_vector_next_queue(vq);
> }
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v7 03/10] virtio: decouple virtqueue from set notifier fd handler
2021-06-02 3:47 [PATCH v7 00/10] vhost-vdpa: add support for configure interrupt Cindy Lu
2021-06-02 3:47 ` [PATCH v7 01/10] virtio: introduce macro IRTIO_CONFIG_IRQ_IDX 聽 Cindy Lu
2021-06-02 3:47 ` [PATCH v7 02/10] virtio-pci:decouple virtqueue from interrupt setting process Cindy Lu
@ 2021-06-02 3:47 ` Cindy Lu
2021-06-03 6:01 ` Jason Wang
2021-06-02 3:47 ` [PATCH v7 04/10] vhost: add new call back function for config interrupt Cindy Lu
` (6 subsequent siblings)
9 siblings, 1 reply; 26+ messages in thread
From: Cindy Lu @ 2021-06-02 3:47 UTC (permalink / raw)
To: lulu, mst, jasowang, qemu-devel
To support config interrupt.we need to decouple virtqueue number
from virtio_queue_set_guest_notifier_fd_handler,
Signed-off-by: Cindy Lu <lulu@redhat.com>
---
hw/s390x/virtio-ccw.c | 6 +++---
hw/virtio/virtio-mmio.c | 4 ++--
hw/virtio/virtio-pci.c | 4 ++--
hw/virtio/virtio.c | 17 +++++++++++------
include/hw/virtio/virtio.h | 2 +-
5 files changed, 19 insertions(+), 14 deletions(-)
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 8195f3546e..58bb5232fd 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -1028,11 +1028,11 @@ static int virtio_ccw_set_guest_notifier(VirtioCcwDevice *dev, int n,
if (r < 0) {
return r;
}
- virtio_queue_set_guest_notifier_fd_handler(vq, true, with_irqfd);
+ virtio_set_notifier_fd_handler(vdev, n, true, with_irqfd);
if (with_irqfd) {
r = virtio_ccw_add_irqfd(dev, n);
if (r) {
- virtio_queue_set_guest_notifier_fd_handler(vq, false,
+ virtio_set_notifier_fd_handler(vdev, n, false,
with_irqfd);
return r;
}
@@ -1056,7 +1056,7 @@ static int virtio_ccw_set_guest_notifier(VirtioCcwDevice *dev, int n,
if (with_irqfd) {
virtio_ccw_remove_irqfd(dev, n);
}
- virtio_queue_set_guest_notifier_fd_handler(vq, false, with_irqfd);
+ virtio_set_notifier_fd_handler(vdev, n, false, with_irqfd);
event_notifier_cleanup(notifier);
}
return 0;
diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index 342c918ea7..13772d52bb 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -658,9 +658,9 @@ static int virtio_mmio_set_guest_notifier(DeviceState *d, int n, bool assign,
if (r < 0) {
return r;
}
- virtio_queue_set_guest_notifier_fd_handler(vq, true, with_irqfd);
+ virtio_set_notifier_fd_handler(vdev, n, true, with_irqfd);
} else {
- virtio_queue_set_guest_notifier_fd_handler(vq, false, with_irqfd);
+ virtio_set_notifier_fd_handler(vdev, n, false, with_irqfd);
event_notifier_cleanup(notifier);
}
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index c5c080ec94..6a4ef413a4 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -958,9 +958,9 @@ static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign,
if (r < 0) {
return r;
}
- virtio_queue_set_guest_notifier_fd_handler(vq, true, with_irqfd);
+ virtio_set_notifier_fd_handler(vdev, n, true, with_irqfd);
} else {
- virtio_queue_set_guest_notifier_fd_handler(vq, false, with_irqfd);
+ virtio_set_notifier_fd_handler(vdev, n, false, with_irqfd);
event_notifier_cleanup(notifier);
}
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 07f4e60b30..c5d786bb5e 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3505,19 +3505,24 @@ static void virtio_queue_guest_notifier_read(EventNotifier *n)
}
}
-void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
- bool with_irqfd)
+
+void virtio_set_notifier_fd_handler(VirtIODevice *vdev, int queue_no,
+ bool assign, bool with_irqfd)
{
+ EventNotifier *e ;
+ EventNotifierHandler *handler;
+ VirtQueue *vq = virtio_get_queue(vdev, queue_no);
+ e = &vq->guest_notifier;
+ handler = virtio_queue_guest_notifier_read;
if (assign && !with_irqfd) {
- event_notifier_set_handler(&vq->guest_notifier,
- virtio_queue_guest_notifier_read);
+ event_notifier_set_handler(e, handler);
} else {
- event_notifier_set_handler(&vq->guest_notifier, NULL);
+ event_notifier_set_handler(e, NULL);
}
if (!assign) {
/* Test and clear notifier before closing it,
* in case poll callback didn't have time to run. */
- virtio_queue_guest_notifier_read(&vq->guest_notifier);
+ handler(e);
}
}
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 63cb9455ed..447899dea5 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -310,7 +310,7 @@ void virtio_queue_update_used_idx(VirtIODevice *vdev, int n);
VirtQueue *virtio_get_queue(VirtIODevice *vdev, int n);
uint16_t virtio_get_queue_index(VirtQueue *vq);
EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq);
-void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
+void virtio_set_notifier_fd_handler(VirtIODevice *vdev, int n, bool assign,
bool with_irqfd);
int virtio_device_start_ioeventfd(VirtIODevice *vdev);
int virtio_device_grab_ioeventfd(VirtIODevice *vdev);
--
2.21.3
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v7 03/10] virtio: decouple virtqueue from set notifier fd handler
2021-06-02 3:47 ` [PATCH v7 03/10] virtio: decouple virtqueue from set notifier fd handler Cindy Lu
@ 2021-06-03 6:01 ` Jason Wang
0 siblings, 0 replies; 26+ messages in thread
From: Jason Wang @ 2021-06-03 6:01 UTC (permalink / raw)
To: Cindy Lu, mst, qemu-devel
在 2021/6/2 上午11:47, Cindy Lu 写道:
> To support config interrupt.we need to decouple virtqueue number
s/we/ We/
> from virtio_queue_set_guest_notifier_fd_handler,
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
> hw/s390x/virtio-ccw.c | 6 +++---
> hw/virtio/virtio-mmio.c | 4 ++--
> hw/virtio/virtio-pci.c | 4 ++--
> hw/virtio/virtio.c | 17 +++++++++++------
> include/hw/virtio/virtio.h | 2 +-
> 5 files changed, 19 insertions(+), 14 deletions(-)
>
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index 8195f3546e..58bb5232fd 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -1028,11 +1028,11 @@ static int virtio_ccw_set_guest_notifier(VirtioCcwDevice *dev, int n,
> if (r < 0) {
> return r;
> }
> - virtio_queue_set_guest_notifier_fd_handler(vq, true, with_irqfd);
> + virtio_set_notifier_fd_handler(vdev, n, true, with_irqfd);
> if (with_irqfd) {
> r = virtio_ccw_add_irqfd(dev, n);
> if (r) {
> - virtio_queue_set_guest_notifier_fd_handler(vq, false,
> + virtio_set_notifier_fd_handler(vdev, n, false,
> with_irqfd);
> return r;
> }
> @@ -1056,7 +1056,7 @@ static int virtio_ccw_set_guest_notifier(VirtioCcwDevice *dev, int n,
> if (with_irqfd) {
> virtio_ccw_remove_irqfd(dev, n);
> }
> - virtio_queue_set_guest_notifier_fd_handler(vq, false, with_irqfd);
> + virtio_set_notifier_fd_handler(vdev, n, false, with_irqfd);
> event_notifier_cleanup(notifier);
> }
> return 0;
> diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
> index 342c918ea7..13772d52bb 100644
> --- a/hw/virtio/virtio-mmio.c
> +++ b/hw/virtio/virtio-mmio.c
> @@ -658,9 +658,9 @@ static int virtio_mmio_set_guest_notifier(DeviceState *d, int n, bool assign,
> if (r < 0) {
> return r;
> }
> - virtio_queue_set_guest_notifier_fd_handler(vq, true, with_irqfd);
> + virtio_set_notifier_fd_handler(vdev, n, true, with_irqfd);
> } else {
> - virtio_queue_set_guest_notifier_fd_handler(vq, false, with_irqfd);
> + virtio_set_notifier_fd_handler(vdev, n, false, with_irqfd);
> event_notifier_cleanup(notifier);
> }
>
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index c5c080ec94..6a4ef413a4 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -958,9 +958,9 @@ static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign,
> if (r < 0) {
> return r;
> }
> - virtio_queue_set_guest_notifier_fd_handler(vq, true, with_irqfd);
> + virtio_set_notifier_fd_handler(vdev, n, true, with_irqfd);
> } else {
> - virtio_queue_set_guest_notifier_fd_handler(vq, false, with_irqfd);
> + virtio_set_notifier_fd_handler(vdev, n, false, with_irqfd);
> event_notifier_cleanup(notifier);
> }
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 07f4e60b30..c5d786bb5e 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -3505,19 +3505,24 @@ static void virtio_queue_guest_notifier_read(EventNotifier *n)
> }
> }
>
> -void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
> - bool with_irqfd)
> +
> +void virtio_set_notifier_fd_handler(VirtIODevice *vdev, int queue_no,
> + bool assign, bool with_irqfd)
Let's use "virtio_set_guest_notifier_fd_handler()".
> {
> + EventNotifier *e ;
> + EventNotifierHandler *handler;
> + VirtQueue *vq = virtio_get_queue(vdev, queue_no);
> + e = &vq->guest_notifier;
So you still depends on the vq implicitly.
How about keep virtio_queue_set_guest_notifier_fd_handler() and factor
its core logic to
virtio_set_guest_notifier_fd_handler() by passing the EventNotifier to
this new helper?
> + handler = virtio_queue_guest_notifier_read;
> if (assign && !with_irqfd) {
> - event_notifier_set_handler(&vq->guest_notifier,
> - virtio_queue_guest_notifier_read);
> + event_notifier_set_handler(e, handler);
> } else {
> - event_notifier_set_handler(&vq->guest_notifier, NULL);
> + event_notifier_set_handler(e, NULL);
> }
> if (!assign) {
> /* Test and clear notifier before closing it,
> * in case poll callback didn't have time to run. */
> - virtio_queue_guest_notifier_read(&vq->guest_notifier);
> + handler(e);
Any reason for this change?
Thanks
> }
> }
>
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 63cb9455ed..447899dea5 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -310,7 +310,7 @@ void virtio_queue_update_used_idx(VirtIODevice *vdev, int n);
> VirtQueue *virtio_get_queue(VirtIODevice *vdev, int n);
> uint16_t virtio_get_queue_index(VirtQueue *vq);
> EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq);
> -void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
> +void virtio_set_notifier_fd_handler(VirtIODevice *vdev, int n, bool assign,
> bool with_irqfd);
> int virtio_device_start_ioeventfd(VirtIODevice *vdev);
> int virtio_device_grab_ioeventfd(VirtIODevice *vdev);
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v7 04/10] vhost: add new call back function for config interrupt
2021-06-02 3:47 [PATCH v7 00/10] vhost-vdpa: add support for configure interrupt Cindy Lu
` (2 preceding siblings ...)
2021-06-02 3:47 ` [PATCH v7 03/10] virtio: decouple virtqueue from set notifier fd handler Cindy Lu
@ 2021-06-02 3:47 ` Cindy Lu
2021-06-03 6:04 ` Jason Wang
2021-06-02 3:47 ` [PATCH v7 05/10] vhost-vdpa: add support for config interrupt call back Cindy Lu
` (5 subsequent siblings)
9 siblings, 1 reply; 26+ messages in thread
From: Cindy Lu @ 2021-06-02 3:47 UTC (permalink / raw)
To: lulu, mst, jasowang, qemu-devel
To support configure interrupt, we need to
add a new call back function for config interrupt.
Signed-off-by: Cindy Lu <lulu@redhat.com>
---
include/hw/virtio/vhost-backend.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
index 8a6f8e2a7a..adaf6982d2 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -125,6 +125,8 @@ typedef int (*vhost_get_device_id_op)(struct vhost_dev *dev, uint32_t *dev_id);
typedef bool (*vhost_force_iommu_op)(struct vhost_dev *dev);
+typedef int (*vhost_set_config_call_op)(struct vhost_dev *dev,
+ int *fd);
typedef struct VhostOps {
VhostBackendType backend_type;
vhost_backend_init vhost_backend_init;
@@ -170,6 +172,7 @@ typedef struct VhostOps {
vhost_vq_get_addr_op vhost_vq_get_addr;
vhost_get_device_id_op vhost_get_device_id;
vhost_force_iommu_op vhost_force_iommu;
+ vhost_set_config_call_op vhost_set_config_call;
} VhostOps;
extern const VhostOps user_ops;
--
2.21.3
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v7 04/10] vhost: add new call back function for config interrupt
2021-06-02 3:47 ` [PATCH v7 04/10] vhost: add new call back function for config interrupt Cindy Lu
@ 2021-06-03 6:04 ` Jason Wang
0 siblings, 0 replies; 26+ messages in thread
From: Jason Wang @ 2021-06-03 6:04 UTC (permalink / raw)
To: Cindy Lu, mst, qemu-devel
在 2021/6/2 上午11:47, Cindy Lu 写道:
> To support configure interrupt, we need to
> add a new call back function for config interrupt.
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
> ---
> include/hw/virtio/vhost-backend.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
> index 8a6f8e2a7a..adaf6982d2 100644
> --- a/include/hw/virtio/vhost-backend.h
> +++ b/include/hw/virtio/vhost-backend.h
> @@ -125,6 +125,8 @@ typedef int (*vhost_get_device_id_op)(struct vhost_dev *dev, uint32_t *dev_id);
>
> typedef bool (*vhost_force_iommu_op)(struct vhost_dev *dev);
>
> +typedef int (*vhost_set_config_call_op)(struct vhost_dev *dev,
> + int *fd);
> typedef struct VhostOps {
> VhostBackendType backend_type;
> vhost_backend_init vhost_backend_init;
> @@ -170,6 +172,7 @@ typedef struct VhostOps {
> vhost_vq_get_addr_op vhost_vq_get_addr;
> vhost_get_device_id_op vhost_get_device_id;
> vhost_force_iommu_op vhost_force_iommu;
> + vhost_set_config_call_op vhost_set_config_call;
> } VhostOps;
>
> extern const VhostOps user_ops;
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v7 05/10] vhost-vdpa: add support for config interrupt call back
2021-06-02 3:47 [PATCH v7 00/10] vhost-vdpa: add support for configure interrupt Cindy Lu
` (3 preceding siblings ...)
2021-06-02 3:47 ` [PATCH v7 04/10] vhost: add new call back function for config interrupt Cindy Lu
@ 2021-06-02 3:47 ` Cindy Lu
2021-06-03 6:06 ` Jason Wang
2021-06-02 3:47 ` [PATCH v7 06/10] vhost:add support for configure interrupt Cindy Lu
` (4 subsequent siblings)
9 siblings, 1 reply; 26+ messages in thread
From: Cindy Lu @ 2021-06-02 3:47 UTC (permalink / raw)
To: lulu, mst, jasowang, qemu-devel
Add new call back function in vhost-vdpa, this call back function only
supported in vhost-vdpa backend
Signed-off-by: Cindy Lu <lulu@redhat.com>
---
hw/virtio/trace-events | 2 ++
hw/virtio/vhost-vdpa.c | 7 +++++++
2 files changed, 9 insertions(+)
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index c62727f879..60a15f0186 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -52,6 +52,8 @@ vhost_vdpa_set_vring_call(void *dev, unsigned int index, int fd) "dev: %p index:
vhost_vdpa_get_features(void *dev, uint64_t features) "dev: %p features: 0x%"PRIx64
vhost_vdpa_set_owner(void *dev) "dev: %p"
vhost_vdpa_vq_get_addr(void *dev, void *vq, uint64_t desc_user_addr, uint64_t avail_user_addr, uint64_t used_user_addr) "dev: %p vq: %p desc_user_addr: 0x%"PRIx64" avail_user_addr: 0x%"PRIx64" used_user_addr: 0x%"PRIx64
+vhost_vdpa_set_config_call(void *dev, int *fd)"dev: %p fd: %p"
+
# virtio.c
virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned out_num) "elem %p size %zd in_num %u out_num %u"
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 01d2101d09..9ba2a2bed4 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -545,6 +545,12 @@ static int vhost_vdpa_set_vring_call(struct vhost_dev *dev,
trace_vhost_vdpa_set_vring_call(dev, file->index, file->fd);
return vhost_vdpa_call(dev, VHOST_SET_VRING_CALL, file);
}
+static int vhost_vdpa_set_config_call(struct vhost_dev *dev,
+ int *fd)
+{
+ trace_vhost_vdpa_set_config_call(dev, fd);
+ return vhost_vdpa_call(dev, VHOST_VDPA_SET_CONFIG_CALL, fd);
+}
static int vhost_vdpa_get_features(struct vhost_dev *dev,
uint64_t *features)
@@ -611,4 +617,5 @@ const VhostOps vdpa_ops = {
.vhost_get_device_id = vhost_vdpa_get_device_id,
.vhost_vq_get_addr = vhost_vdpa_vq_get_addr,
.vhost_force_iommu = vhost_vdpa_force_iommu,
+ .vhost_set_config_call = vhost_vdpa_set_config_call,
};
--
2.21.3
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v7 05/10] vhost-vdpa: add support for config interrupt call back
2021-06-02 3:47 ` [PATCH v7 05/10] vhost-vdpa: add support for config interrupt call back Cindy Lu
@ 2021-06-03 6:06 ` Jason Wang
2021-06-07 6:34 ` Cindy Lu
0 siblings, 1 reply; 26+ messages in thread
From: Jason Wang @ 2021-06-03 6:06 UTC (permalink / raw)
To: Cindy Lu, mst, qemu-devel
在 2021/6/2 上午11:47, Cindy Lu 写道:
> Add new call back function in vhost-vdpa, this call back function only
> supported in vhost-vdpa backend
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
> hw/virtio/trace-events | 2 ++
> hw/virtio/vhost-vdpa.c | 7 +++++++
> 2 files changed, 9 insertions(+)
>
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index c62727f879..60a15f0186 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -52,6 +52,8 @@ vhost_vdpa_set_vring_call(void *dev, unsigned int index, int fd) "dev: %p index:
> vhost_vdpa_get_features(void *dev, uint64_t features) "dev: %p features: 0x%"PRIx64
> vhost_vdpa_set_owner(void *dev) "dev: %p"
> vhost_vdpa_vq_get_addr(void *dev, void *vq, uint64_t desc_user_addr, uint64_t avail_user_addr, uint64_t used_user_addr) "dev: %p vq: %p desc_user_addr: 0x%"PRIx64" avail_user_addr: 0x%"PRIx64" used_user_addr: 0x%"PRIx64
> +vhost_vdpa_set_config_call(void *dev, int *fd)"dev: %p fd: %p"
This tracing information is sub-optimal, I think we should show the fd
instead of its address here.
Other looks good.
Thanks
> +
>
> # virtio.c
> virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned out_num) "elem %p size %zd in_num %u out_num %u"
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index 01d2101d09..9ba2a2bed4 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -545,6 +545,12 @@ static int vhost_vdpa_set_vring_call(struct vhost_dev *dev,
> trace_vhost_vdpa_set_vring_call(dev, file->index, file->fd);
> return vhost_vdpa_call(dev, VHOST_SET_VRING_CALL, file);
> }
> +static int vhost_vdpa_set_config_call(struct vhost_dev *dev,
> + int *fd)
> +{
> + trace_vhost_vdpa_set_config_call(dev, fd);
> + return vhost_vdpa_call(dev, VHOST_VDPA_SET_CONFIG_CALL, fd);
> +}
>
> static int vhost_vdpa_get_features(struct vhost_dev *dev,
> uint64_t *features)
> @@ -611,4 +617,5 @@ const VhostOps vdpa_ops = {
> .vhost_get_device_id = vhost_vdpa_get_device_id,
> .vhost_vq_get_addr = vhost_vdpa_vq_get_addr,
> .vhost_force_iommu = vhost_vdpa_force_iommu,
> + .vhost_set_config_call = vhost_vdpa_set_config_call,
> };
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v7 05/10] vhost-vdpa: add support for config interrupt call back
2021-06-03 6:06 ` Jason Wang
@ 2021-06-07 6:34 ` Cindy Lu
0 siblings, 0 replies; 26+ messages in thread
From: Cindy Lu @ 2021-06-07 6:34 UTC (permalink / raw)
To: Jason Wang; +Cc: QEMU Developers, Michael Tsirkin
On Thu, Jun 3, 2021 at 2:06 PM Jason Wang <jasowang@redhat.com> wrote:
>
>
> 在 2021/6/2 上午11:47, Cindy Lu 写道:
> > Add new call back function in vhost-vdpa, this call back function only
> > supported in vhost-vdpa backend
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > ---
> > hw/virtio/trace-events | 2 ++
> > hw/virtio/vhost-vdpa.c | 7 +++++++
> > 2 files changed, 9 insertions(+)
> >
> > diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> > index c62727f879..60a15f0186 100644
> > --- a/hw/virtio/trace-events
> > +++ b/hw/virtio/trace-events
> > @@ -52,6 +52,8 @@ vhost_vdpa_set_vring_call(void *dev, unsigned int index, int fd) "dev: %p index:
> > vhost_vdpa_get_features(void *dev, uint64_t features) "dev: %p features: 0x%"PRIx64
> > vhost_vdpa_set_owner(void *dev) "dev: %p"
> > vhost_vdpa_vq_get_addr(void *dev, void *vq, uint64_t desc_user_addr, uint64_t avail_user_addr, uint64_t used_user_addr) "dev: %p vq: %p desc_user_addr: 0x%"PRIx64" avail_user_addr: 0x%"PRIx64" used_user_addr: 0x%"PRIx64
> > +vhost_vdpa_set_config_call(void *dev, int *fd)"dev: %p fd: %p"
>
>
> This tracing information is sub-optimal, I think we should show the fd
> instead of its address here.
>
> Other looks good.
>
> Thanks
>
>
sure, I will fix this
> > +
> >
> > # virtio.c
> > virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned out_num) "elem %p size %zd in_num %u out_num %u"
> > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> > index 01d2101d09..9ba2a2bed4 100644
> > --- a/hw/virtio/vhost-vdpa.c
> > +++ b/hw/virtio/vhost-vdpa.c
> > @@ -545,6 +545,12 @@ static int vhost_vdpa_set_vring_call(struct vhost_dev *dev,
> > trace_vhost_vdpa_set_vring_call(dev, file->index, file->fd);
> > return vhost_vdpa_call(dev, VHOST_SET_VRING_CALL, file);
> > }
> > +static int vhost_vdpa_set_config_call(struct vhost_dev *dev,
> > + int *fd)
> > +{
> > + trace_vhost_vdpa_set_config_call(dev, fd);
> > + return vhost_vdpa_call(dev, VHOST_VDPA_SET_CONFIG_CALL, fd);
> > +}
> >
> > static int vhost_vdpa_get_features(struct vhost_dev *dev,
> > uint64_t *features)
> > @@ -611,4 +617,5 @@ const VhostOps vdpa_ops = {
> > .vhost_get_device_id = vhost_vdpa_get_device_id,
> > .vhost_vq_get_addr = vhost_vdpa_vq_get_addr,
> > .vhost_force_iommu = vhost_vdpa_force_iommu,
> > + .vhost_set_config_call = vhost_vdpa_set_config_call,
> > };
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v7 06/10] vhost:add support for configure interrupt
2021-06-02 3:47 [PATCH v7 00/10] vhost-vdpa: add support for configure interrupt Cindy Lu
` (4 preceding siblings ...)
2021-06-02 3:47 ` [PATCH v7 05/10] vhost-vdpa: add support for config interrupt call back Cindy Lu
@ 2021-06-02 3:47 ` Cindy Lu
2021-06-03 6:28 ` Jason Wang
2021-06-02 3:47 ` [PATCH v7 07/10] virtio-mmio: add " Cindy Lu
` (3 subsequent siblings)
9 siblings, 1 reply; 26+ messages in thread
From: Cindy Lu @ 2021-06-02 3:47 UTC (permalink / raw)
To: lulu, mst, jasowang, qemu-devel
Add configure notifier support in vhost and virtio driver
When backend support VIRTIO_NET_F_STATUS,setup the configure
interrupt function in vhost_dev_start and release the related
resource when vhost_dev_stop
Signed-off-by: Cindy Lu <lulu@redhat.com>
---
hw/net/vhost_net.c | 9 +++++
hw/net/virtio-net.c | 6 ++++
hw/virtio/vhost.c | 68 ++++++++++++++++++++++++++++++++++++--
hw/virtio/virtio.c | 23 +++++++++++--
include/hw/virtio/vhost.h | 2 ++
include/hw/virtio/virtio.h | 3 ++
include/net/vhost_net.h | 3 ++
7 files changed, 109 insertions(+), 5 deletions(-)
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 24d555e764..5d0c35f18d 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -426,6 +426,15 @@ void vhost_net_virtqueue_mask(VHostNetState *net, VirtIODevice *dev,
vhost_virtqueue_mask(&net->dev, dev, idx, mask);
}
+bool vhost_net_config_pending(VHostNetState *net, int idx)
+{
+ return vhost_config_pending(&net->dev, idx);
+}
+void vhost_net_config_mask(VHostNetState *net, VirtIODevice *dev,
+ bool mask)
+{
+ vhost_config_mask(&net->dev, dev, mask);
+}
VHostNetState *get_vhost_net(NetClientState *nc)
{
VHostNetState *vhost_net = 0;
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index f50235b5d6..02033be748 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3055,6 +3055,9 @@ static bool virtio_net_guest_notifier_pending(VirtIODevice *vdev, int idx)
if (idx != VIRTIO_CONFIG_IRQ_IDX) {
return vhost_net_virtqueue_pending(get_vhost_net(nc->peer), idx);
}
+ if (idx == VIRTIO_CONFIG_IRQ_IDX) {
+ return vhost_net_config_pending(get_vhost_net(nc->peer), idx);
+ }
return false;
}
@@ -3067,6 +3070,9 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx,
if (idx != VIRTIO_CONFIG_IRQ_IDX) {
vhost_net_virtqueue_mask(get_vhost_net(nc->peer), vdev, idx, mask);
}
+ if (idx == VIRTIO_CONFIG_IRQ_IDX) {
+ vhost_net_config_mask(get_vhost_net(nc->peer), vdev, mask);
+ }
}
static void virtio_net_set_config_size(VirtIONet *n, uint64_t host_features)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index e2163a0d63..3b05f09d98 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -21,6 +21,7 @@
#include "qemu/error-report.h"
#include "qemu/memfd.h"
#include "standard-headers/linux/vhost_types.h"
+#include "standard-headers/linux/virtio_net.h"
#include "exec/address-spaces.h"
#include "hw/virtio/virtio-bus.h"
#include "hw/virtio/virtio-access.h"
@@ -1505,6 +1506,16 @@ bool vhost_virtqueue_pending(struct vhost_dev *hdev, int n)
return event_notifier_test_and_clear(&vq->masked_notifier);
}
+bool vhost_config_pending(struct vhost_dev *hdev, int n)
+{
+ assert(hdev->vhost_ops);
+ VirtIODevice *vdev = hdev->vdev;
+ if ((hdev->started == false) ||
+ (hdev->vhost_ops->vhost_set_config_call == NULL)) {
+ return false;
+ }
+ return event_notifier_test_and_clear(&vdev->masked_config_notifier);
+}
/* Mask/unmask events from this vq. */
void vhost_virtqueue_mask(struct vhost_dev *hdev, VirtIODevice *vdev, int n,
bool mask)
@@ -1529,6 +1540,30 @@ void vhost_virtqueue_mask(struct vhost_dev *hdev, VirtIODevice *vdev, int n,
VHOST_OPS_DEBUG("vhost_set_vring_call failed");
}
}
+void vhost_config_mask(struct vhost_dev *hdev, VirtIODevice *vdev,
+ bool mask)
+{
+ int fd;
+ int r;
+ EventNotifier *masked_config_notifier = &vdev->masked_config_notifier;
+ EventNotifier *config_notifier = &vdev->config_notifier;
+ assert(hdev->vhost_ops);
+
+ if ((hdev->started == false) ||
+ (hdev->vhost_ops->vhost_set_config_call == NULL)) {
+ return ;
+ }
+ if (mask) {
+ assert(vdev->use_guest_notifier_mask);
+ fd = event_notifier_get_fd(masked_config_notifier);
+ } else {
+ fd = event_notifier_get_fd(config_notifier);
+ }
+ r = hdev->vhost_ops->vhost_set_config_call(hdev, &fd);
+ if (r < 0) {
+ error_report("vhost_set_config_call failed");
+ }
+}
uint64_t vhost_get_features(struct vhost_dev *hdev, const int *feature_bits,
uint64_t features)
@@ -1708,6 +1743,7 @@ int vhost_dev_get_inflight(struct vhost_dev *dev, uint16_t queue_size,
int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
{
int i, r;
+ int fd = 0;
/* should only be called after backend is connected */
assert(hdev->vhost_ops);
@@ -1739,7 +1775,14 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
goto fail_vq;
}
}
-
+ r = event_notifier_init(&vdev->masked_config_notifier, 0);
+ if (r < 0) {
+ return r;
+ }
+ event_notifier_test_and_clear(&vdev->masked_config_notifier);
+ if (!vdev->use_guest_notifier_mask) {
+ vhost_config_mask(hdev, vdev, true);
+ }
if (hdev->log_enabled) {
uint64_t log_base;
@@ -1773,6 +1816,19 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
vhost_device_iotlb_miss(hdev, vq->used_phys, true);
}
}
+ if (!(hdev->features & (0x1ULL << VIRTIO_NET_F_STATUS))) {
+ return 0;
+ }
+ if (hdev->vhost_ops->vhost_set_config_call) {
+ fd = event_notifier_get_fd(&vdev->config_notifier);
+ r = hdev->vhost_ops->vhost_set_config_call(hdev, &fd);
+ if (!r) {
+ event_notifier_set(&vdev->config_notifier);
+ }
+ if (r) {
+ goto fail_log;
+ }
+ }
return 0;
fail_log:
vhost_log_put(hdev, false);
@@ -1795,10 +1851,18 @@ fail_features:
void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev)
{
int i;
+ int fd;
/* should only be called after backend is connected */
assert(hdev->vhost_ops);
-
+ event_notifier_test_and_clear(&vdev->masked_config_notifier);
+ event_notifier_test_and_clear(&vdev->config_notifier);
+ if ((hdev->features & (0x1ULL << VIRTIO_NET_F_STATUS))) {
+ if (hdev->vhost_ops->vhost_set_config_call) {
+ fd = -1;
+ hdev->vhost_ops->vhost_set_config_call(hdev, &fd);
+ }
+ }
if (hdev->vhost_ops->vhost_dev_start) {
hdev->vhost_ops->vhost_dev_start(hdev, false);
}
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index c5d786bb5e..09ed3f67d9 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3506,14 +3506,27 @@ static void virtio_queue_guest_notifier_read(EventNotifier *n)
}
+static void virtio_config_read(EventNotifier *n)
+{
+ VirtIODevice *vdev = container_of(n, VirtIODevice, config_notifier);
+
+ if (event_notifier_test_and_clear(n)) {
+ virtio_notify_config(vdev);
+ }
+}
void virtio_set_notifier_fd_handler(VirtIODevice *vdev, int queue_no,
bool assign, bool with_irqfd)
{
EventNotifier *e ;
EventNotifierHandler *handler;
- VirtQueue *vq = virtio_get_queue(vdev, queue_no);
- e = &vq->guest_notifier;
- handler = virtio_queue_guest_notifier_read;
+ if (queue_no != VIRTIO_CONFIG_IRQ_IDX) {
+ VirtQueue *vq = virtio_get_queue(vdev, queue_no);
+ e = &vq->guest_notifier;
+ handler = virtio_queue_guest_notifier_read;
+ } else {
+ e = &vdev->config_notifier;
+ handler = virtio_config_read;
+ }
if (assign && !with_irqfd) {
event_notifier_set_handler(e, handler);
} else {
@@ -3599,6 +3612,10 @@ EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq)
return &vq->host_notifier;
}
+EventNotifier *virtio_get_config_notifier(VirtIODevice *vdev)
+{
+ return &vdev->config_notifier;
+}
void virtio_queue_set_host_notifier_enabled(VirtQueue *vq, bool enabled)
{
vq->host_notifier_enabled = enabled;
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 4a8bc75415..b8814ece32 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -108,6 +108,8 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev);
void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev);
int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev);
void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev);
+bool vhost_config_pending(struct vhost_dev *hdev, int n);
+void vhost_config_mask(struct vhost_dev *hdev, VirtIODevice *vdev, bool mask);
/* Test and clear masked event pending status.
* Should be called after unmask to avoid losing events.
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 447899dea5..5856517d43 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -110,6 +110,8 @@ struct VirtIODevice
bool use_guest_notifier_mask;
AddressSpace *dma_as;
QLIST_HEAD(, VirtQueue) *vector_queues;
+ EventNotifier config_notifier;
+ EventNotifier masked_config_notifier;
};
struct VirtioDeviceClass {
@@ -317,6 +319,7 @@ int virtio_device_grab_ioeventfd(VirtIODevice *vdev);
void virtio_device_release_ioeventfd(VirtIODevice *vdev);
bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev);
EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq);
+EventNotifier *virtio_get_config_notifier(VirtIODevice *vdev);
void virtio_queue_set_host_notifier_enabled(VirtQueue *vq, bool enabled);
void virtio_queue_host_notifier_read(EventNotifier *n);
void virtio_queue_aio_set_host_notifier_handler(VirtQueue *vq, AioContext *ctx,
diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
index 172b0051d8..0d38c97c94 100644
--- a/include/net/vhost_net.h
+++ b/include/net/vhost_net.h
@@ -36,6 +36,9 @@ int vhost_net_set_config(struct vhost_net *net, const uint8_t *data,
bool vhost_net_virtqueue_pending(VHostNetState *net, int n);
void vhost_net_virtqueue_mask(VHostNetState *net, VirtIODevice *dev,
int idx, bool mask);
+bool vhost_net_config_pending(VHostNetState *net, int n);
+void vhost_net_config_mask(VHostNetState *net, VirtIODevice *dev,
+ bool mask);
int vhost_net_notify_migration_done(VHostNetState *net, char* mac_addr);
VHostNetState *get_vhost_net(NetClientState *nc);
--
2.21.3
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v7 06/10] vhost:add support for configure interrupt
2021-06-02 3:47 ` [PATCH v7 06/10] vhost:add support for configure interrupt Cindy Lu
@ 2021-06-03 6:28 ` Jason Wang
2021-06-08 3:20 ` Cindy Lu
0 siblings, 1 reply; 26+ messages in thread
From: Jason Wang @ 2021-06-03 6:28 UTC (permalink / raw)
To: Cindy Lu, mst, qemu-devel
在 2021/6/2 上午11:47, Cindy Lu 写道:
> Add configure notifier support in vhost and virtio driver
> When backend support VIRTIO_NET_F_STATUS,
So config interrupt is the basic facility of the virtio device. We need
to make the code not specific to this feature.
But we can leave the specific device (like virtio-net, or vhost-net) to
decide whether or not it can be used.
For net, it is also used by guest announcement.
> setup the configure
> interrupt function in vhost_dev_start and release the related
> resource when vhost_dev_stop
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
> hw/net/vhost_net.c | 9 +++++
> hw/net/virtio-net.c | 6 ++++
> hw/virtio/vhost.c | 68 ++++++++++++++++++++++++++++++++++++--
> hw/virtio/virtio.c | 23 +++++++++++--
> include/hw/virtio/vhost.h | 2 ++
> include/hw/virtio/virtio.h | 3 ++
> include/net/vhost_net.h | 3 ++
> 7 files changed, 109 insertions(+), 5 deletions(-)
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index 24d555e764..5d0c35f18d 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -426,6 +426,15 @@ void vhost_net_virtqueue_mask(VHostNetState *net, VirtIODevice *dev,
> vhost_virtqueue_mask(&net->dev, dev, idx, mask);
> }
>
> +bool vhost_net_config_pending(VHostNetState *net, int idx)
> +{
> + return vhost_config_pending(&net->dev, idx);
> +}
> +void vhost_net_config_mask(VHostNetState *net, VirtIODevice *dev,
> + bool mask)
> +{
> + vhost_config_mask(&net->dev, dev, mask);
> +}
> VHostNetState *get_vhost_net(NetClientState *nc)
> {
> VHostNetState *vhost_net = 0;
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index f50235b5d6..02033be748 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -3055,6 +3055,9 @@ static bool virtio_net_guest_notifier_pending(VirtIODevice *vdev, int idx)
> if (idx != VIRTIO_CONFIG_IRQ_IDX) {
> return vhost_net_virtqueue_pending(get_vhost_net(nc->peer), idx);
> }
> + if (idx == VIRTIO_CONFIG_IRQ_IDX) {
> + return vhost_net_config_pending(get_vhost_net(nc->peer), idx);
> + }
> return false;
> }
>
> @@ -3067,6 +3070,9 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx,
> if (idx != VIRTIO_CONFIG_IRQ_IDX) {
> vhost_net_virtqueue_mask(get_vhost_net(nc->peer), vdev, idx, mask);
> }
> + if (idx == VIRTIO_CONFIG_IRQ_IDX) {
> + vhost_net_config_mask(get_vhost_net(nc->peer), vdev, mask);
> + }
> }
>
> static void virtio_net_set_config_size(VirtIONet *n, uint64_t host_features)
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index e2163a0d63..3b05f09d98 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -21,6 +21,7 @@
> #include "qemu/error-report.h"
> #include "qemu/memfd.h"
> #include "standard-headers/linux/vhost_types.h"
> +#include "standard-headers/linux/virtio_net.h"
Inclusion of device specific header in the general vhost code looks like
a layer violation.
> #include "exec/address-spaces.h"
> #include "hw/virtio/virtio-bus.h"
> #include "hw/virtio/virtio-access.h"
> @@ -1505,6 +1506,16 @@ bool vhost_virtqueue_pending(struct vhost_dev *hdev, int n)
> return event_notifier_test_and_clear(&vq->masked_notifier);
> }
>
> +bool vhost_config_pending(struct vhost_dev *hdev, int n)
> +{
> + assert(hdev->vhost_ops);
I think we can remove this.
> + VirtIODevice *vdev = hdev->vdev;
> + if ((hdev->started == false) ||
We don't check this in vhost_virtqueue_pending(), any reason to do this?
> + (hdev->vhost_ops->vhost_set_config_call == NULL)) {
I think we need first check what happens if we try to down the link for
vhost-vdpa network backend in the monitor. (Or should we block that from
the monitor?)
Then we can decided whether or not we need this.
> + return false;
> + }
> + return event_notifier_test_and_clear(&vdev->masked_config_notifier);
> +}
> /* Mask/unmask events from this vq. */
> void vhost_virtqueue_mask(struct vhost_dev *hdev, VirtIODevice *vdev, int n,
> bool mask)
> @@ -1529,6 +1540,30 @@ void vhost_virtqueue_mask(struct vhost_dev *hdev, VirtIODevice *vdev, int n,
> VHOST_OPS_DEBUG("vhost_set_vring_call failed");
> }
> }
> +void vhost_config_mask(struct vhost_dev *hdev, VirtIODevice *vdev,
> + bool mask)
> +{
> + int fd;
> + int r;
> + EventNotifier *masked_config_notifier = &vdev->masked_config_notifier;
> + EventNotifier *config_notifier = &vdev->config_notifier;
> + assert(hdev->vhost_ops);
> +
> + if ((hdev->started == false) ||
> + (hdev->vhost_ops->vhost_set_config_call == NULL)) {
> + return ;
> + }
> + if (mask) {
> + assert(vdev->use_guest_notifier_mask);
> + fd = event_notifier_get_fd(masked_config_notifier);
> + } else {
> + fd = event_notifier_get_fd(config_notifier);
> + }
> + r = hdev->vhost_ops->vhost_set_config_call(hdev, &fd);
> + if (r < 0) {
> + error_report("vhost_set_config_call failed");
VHOST_OPS_DEBUG() please.
> + }
> +}
>
> uint64_t vhost_get_features(struct vhost_dev *hdev, const int *feature_bits,
> uint64_t features)
> @@ -1708,6 +1743,7 @@ int vhost_dev_get_inflight(struct vhost_dev *dev, uint16_t queue_size,
> int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
> {
> int i, r;
> + int fd = 0;
>
> /* should only be called after backend is connected */
> assert(hdev->vhost_ops);
> @@ -1739,7 +1775,14 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
> goto fail_vq;
> }
> }
> -
> + r = event_notifier_init(&vdev->masked_config_notifier, 0);
> + if (r < 0) {
> + return r;
> + }
> + event_notifier_test_and_clear(&vdev->masked_config_notifier);
> + if (!vdev->use_guest_notifier_mask) {
> + vhost_config_mask(hdev, vdev, true);
> + }
Let's leave a new line for separating the logical blocks.
> if (hdev->log_enabled) {
> uint64_t log_base;
>
> @@ -1773,6 +1816,19 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
> vhost_device_iotlb_miss(hdev, vq->used_phys, true);
> }
> }
> + if (!(hdev->features & (0x1ULL << VIRTIO_NET_F_STATUS))) {
> + return 0;
> + }
Let's move this to vhost-net instead of doing it in the general vhost code.
Or we can even remove this, otherwise it will introduce some burden if
we need to implement other features that depends on the config interrupt
for vhost-net/vDPA.
> + if (hdev->vhost_ops->vhost_set_config_call) {
> + fd = event_notifier_get_fd(&vdev->config_notifier);
> + r = hdev->vhost_ops->vhost_set_config_call(hdev, &fd);
> + if (!r) {
> + event_notifier_set(&vdev->config_notifier);
Any reason you need to trigger the config interrupt here?
> + }
> + if (r) {
> + goto fail_log;
> + }
> + }
> return 0;
> fail_log:
> vhost_log_put(hdev, false);
> @@ -1795,10 +1851,18 @@ fail_features:
> void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev)
> {
> int i;
> + int fd;
>
> /* should only be called after backend is connected */
> assert(hdev->vhost_ops);
> -
> + event_notifier_test_and_clear(&vdev->masked_config_notifier);
> + event_notifier_test_and_clear(&vdev->config_notifier);
If we do this in the start, there's no need for such cleanup here.
> + if ((hdev->features & (0x1ULL << VIRTIO_NET_F_STATUS))) {
Let's move this to vhost-net or I'd rather remove this.
Thanks
> + if (hdev->vhost_ops->vhost_set_config_call) {
> + fd = -1;
> + hdev->vhost_ops->vhost_set_config_call(hdev, &fd);
> + }
> + }
> if (hdev->vhost_ops->vhost_dev_start) {
> hdev->vhost_ops->vhost_dev_start(hdev, false);
> }
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index c5d786bb5e..09ed3f67d9 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -3506,14 +3506,27 @@ static void virtio_queue_guest_notifier_read(EventNotifier *n)
> }
>
>
> +static void virtio_config_read(EventNotifier *n)
> +{
> + VirtIODevice *vdev = container_of(n, VirtIODevice, config_notifier);
> +
> + if (event_notifier_test_and_clear(n)) {
> + virtio_notify_config(vdev);
> + }
> +}
> void virtio_set_notifier_fd_handler(VirtIODevice *vdev, int queue_no,
> bool assign, bool with_irqfd)
> {
> EventNotifier *e ;
> EventNotifierHandler *handler;
> - VirtQueue *vq = virtio_get_queue(vdev, queue_no);
> - e = &vq->guest_notifier;
> - handler = virtio_queue_guest_notifier_read;
> + if (queue_no != VIRTIO_CONFIG_IRQ_IDX) {
> + VirtQueue *vq = virtio_get_queue(vdev, queue_no);
> + e = &vq->guest_notifier;
> + handler = virtio_queue_guest_notifier_read;
> + } else {
> + e = &vdev->config_notifier;
> + handler = virtio_config_read;
> + }
> if (assign && !with_irqfd) {
> event_notifier_set_handler(e, handler);
> } else {
> @@ -3599,6 +3612,10 @@ EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq)
> return &vq->host_notifier;
> }
>
> +EventNotifier *virtio_get_config_notifier(VirtIODevice *vdev)
> +{
> + return &vdev->config_notifier;
> +}
> void virtio_queue_set_host_notifier_enabled(VirtQueue *vq, bool enabled)
> {
> vq->host_notifier_enabled = enabled;
> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> index 4a8bc75415..b8814ece32 100644
> --- a/include/hw/virtio/vhost.h
> +++ b/include/hw/virtio/vhost.h
> @@ -108,6 +108,8 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev);
> void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev);
> int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev);
> void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev);
> +bool vhost_config_pending(struct vhost_dev *hdev, int n);
> +void vhost_config_mask(struct vhost_dev *hdev, VirtIODevice *vdev, bool mask);
>
> /* Test and clear masked event pending status.
> * Should be called after unmask to avoid losing events.
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 447899dea5..5856517d43 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -110,6 +110,8 @@ struct VirtIODevice
> bool use_guest_notifier_mask;
> AddressSpace *dma_as;
> QLIST_HEAD(, VirtQueue) *vector_queues;
> + EventNotifier config_notifier;
> + EventNotifier masked_config_notifier;
> };
>
> struct VirtioDeviceClass {
> @@ -317,6 +319,7 @@ int virtio_device_grab_ioeventfd(VirtIODevice *vdev);
> void virtio_device_release_ioeventfd(VirtIODevice *vdev);
> bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev);
> EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq);
> +EventNotifier *virtio_get_config_notifier(VirtIODevice *vdev);
> void virtio_queue_set_host_notifier_enabled(VirtQueue *vq, bool enabled);
> void virtio_queue_host_notifier_read(EventNotifier *n);
> void virtio_queue_aio_set_host_notifier_handler(VirtQueue *vq, AioContext *ctx,
> diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
> index 172b0051d8..0d38c97c94 100644
> --- a/include/net/vhost_net.h
> +++ b/include/net/vhost_net.h
> @@ -36,6 +36,9 @@ int vhost_net_set_config(struct vhost_net *net, const uint8_t *data,
> bool vhost_net_virtqueue_pending(VHostNetState *net, int n);
> void vhost_net_virtqueue_mask(VHostNetState *net, VirtIODevice *dev,
> int idx, bool mask);
> +bool vhost_net_config_pending(VHostNetState *net, int n);
> +void vhost_net_config_mask(VHostNetState *net, VirtIODevice *dev,
> + bool mask);
> int vhost_net_notify_migration_done(VHostNetState *net, char* mac_addr);
> VHostNetState *get_vhost_net(NetClientState *nc);
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v7 06/10] vhost:add support for configure interrupt
2021-06-03 6:28 ` Jason Wang
@ 2021-06-08 3:20 ` Cindy Lu
0 siblings, 0 replies; 26+ messages in thread
From: Cindy Lu @ 2021-06-08 3:20 UTC (permalink / raw)
To: Jason Wang; +Cc: QEMU Developers, Michael Tsirkin
On Thu, Jun 3, 2021 at 2:28 PM Jason Wang <jasowang@redhat.com> wrote:
>
>
> 在 2021/6/2 上午11:47, Cindy Lu 写道:
> > Add configure notifier support in vhost and virtio driver
> > When backend support VIRTIO_NET_F_STATUS,
>
>
> So config interrupt is the basic facility of the virtio device. We need
> to make the code not specific to this feature.
>
> But we can leave the specific device (like virtio-net, or vhost-net) to
> decide whether or not it can be used.
>
> For net, it is also used by guest announcement.
>
sure, I will rewrite this part
>
> > setup the configure
> > interrupt function in vhost_dev_start and release the related
> > resource when vhost_dev_stop
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > ---
> > hw/net/vhost_net.c | 9 +++++
> > hw/net/virtio-net.c | 6 ++++
> > hw/virtio/vhost.c | 68 ++++++++++++++++++++++++++++++++++++--
> > hw/virtio/virtio.c | 23 +++++++++++--
> > include/hw/virtio/vhost.h | 2 ++
> > include/hw/virtio/virtio.h | 3 ++
> > include/net/vhost_net.h | 3 ++
> > 7 files changed, 109 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > index 24d555e764..5d0c35f18d 100644
> > --- a/hw/net/vhost_net.c
> > +++ b/hw/net/vhost_net.c
> > @@ -426,6 +426,15 @@ void vhost_net_virtqueue_mask(VHostNetState *net, VirtIODevice *dev,
> > vhost_virtqueue_mask(&net->dev, dev, idx, mask);
> > }
> >
> > +bool vhost_net_config_pending(VHostNetState *net, int idx)
> > +{
> > + return vhost_config_pending(&net->dev, idx);
> > +}
> > +void vhost_net_config_mask(VHostNetState *net, VirtIODevice *dev,
> > + bool mask)
> > +{
> > + vhost_config_mask(&net->dev, dev, mask);
> > +}
> > VHostNetState *get_vhost_net(NetClientState *nc)
> > {
> > VHostNetState *vhost_net = 0;
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index f50235b5d6..02033be748 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -3055,6 +3055,9 @@ static bool virtio_net_guest_notifier_pending(VirtIODevice *vdev, int idx)
> > if (idx != VIRTIO_CONFIG_IRQ_IDX) {
> > return vhost_net_virtqueue_pending(get_vhost_net(nc->peer), idx);
> > }
> > + if (idx == VIRTIO_CONFIG_IRQ_IDX) {
> > + return vhost_net_config_pending(get_vhost_net(nc->peer), idx);
> > + }
> > return false;
> > }
> >
> > @@ -3067,6 +3070,9 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx,
> > if (idx != VIRTIO_CONFIG_IRQ_IDX) {
> > vhost_net_virtqueue_mask(get_vhost_net(nc->peer), vdev, idx, mask);
> > }
> > + if (idx == VIRTIO_CONFIG_IRQ_IDX) {
> > + vhost_net_config_mask(get_vhost_net(nc->peer), vdev, mask);
> > + }
> > }
> >
> > static void virtio_net_set_config_size(VirtIONet *n, uint64_t host_features)
> > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> > index e2163a0d63..3b05f09d98 100644
> > --- a/hw/virtio/vhost.c
> > +++ b/hw/virtio/vhost.c
> > @@ -21,6 +21,7 @@
> > #include "qemu/error-report.h"
> > #include "qemu/memfd.h"
> > #include "standard-headers/linux/vhost_types.h"
> > +#include "standard-headers/linux/virtio_net.h"
>
>
> Inclusion of device specific header in the general vhost code looks like
> a layer violation.
>
>
> > #include "exec/address-spaces.h"
> > #include "hw/virtio/virtio-bus.h"
> > #include "hw/virtio/virtio-access.h"
> > @@ -1505,6 +1506,16 @@ bool vhost_virtqueue_pending(struct vhost_dev *hdev, int n)
> > return event_notifier_test_and_clear(&vq->masked_notifier);
> > }
> >
> > +bool vhost_config_pending(struct vhost_dev *hdev, int n)
> > +{
> > + assert(hdev->vhost_ops);
>
>
> I think we can remove this.
>
>
> > + VirtIODevice *vdev = hdev->vdev;
> > + if ((hdev->started == false) ||
>
>
> We don't check this in vhost_virtqueue_pending(), any reason to do this?
>
>
> > + (hdev->vhost_ops->vhost_set_config_call == NULL)) {
>
>
> I think we need first check what happens if we try to down the link for
> vhost-vdpa network backend in the monitor. (Or should we block that from
> the monitor?)
>
> Then we can decided whether or not we need this.
>
>
I will remove this part
> > + return false;
> > + }
> > + return event_notifier_test_and_clear(&vdev->masked_config_notifier);
> > +}
> > /* Mask/unmask events from this vq. */
> > void vhost_virtqueue_mask(struct vhost_dev *hdev, VirtIODevice *vdev, int n,
> > bool mask)
> > @@ -1529,6 +1540,30 @@ void vhost_virtqueue_mask(struct vhost_dev *hdev, VirtIODevice *vdev, int n,
> > VHOST_OPS_DEBUG("vhost_set_vring_call failed");
> > }
> > }
> > +void vhost_config_mask(struct vhost_dev *hdev, VirtIODevice *vdev,
> > + bool mask)
> > +{
> > + int fd;
> > + int r;
> > + EventNotifier *masked_config_notifier = &vdev->masked_config_notifier;
> > + EventNotifier *config_notifier = &vdev->config_notifier;
> > + assert(hdev->vhost_ops);
> > +
> > + if ((hdev->started == false) ||
> > + (hdev->vhost_ops->vhost_set_config_call == NULL)) {
> > + return ;
> > + }
> > + if (mask) {
> > + assert(vdev->use_guest_notifier_mask);
> > + fd = event_notifier_get_fd(masked_config_notifier);
> > + } else {
> > + fd = event_notifier_get_fd(config_notifier);
> > + }
> > + r = hdev->vhost_ops->vhost_set_config_call(hdev, &fd);
> > + if (r < 0) {
> > + error_report("vhost_set_config_call failed");
>
>
> VHOST_OPS_DEBUG() please.
>
will fix this
>
> > + }
> > +}
> >
> > uint64_t vhost_get_features(struct vhost_dev *hdev, const int *feature_bits,
> > uint64_t features)
> > @@ -1708,6 +1743,7 @@ int vhost_dev_get_inflight(struct vhost_dev *dev, uint16_t queue_size,
> > int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
> > {
> > int i, r;
> > + int fd = 0;
> >
> > /* should only be called after backend is connected */
> > assert(hdev->vhost_ops);
> > @@ -1739,7 +1775,14 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
> > goto fail_vq;
> > }
> > }
> > -
> > + r = event_notifier_init(&vdev->masked_config_notifier, 0);
> > + if (r < 0) {
> > + return r;
> > + }
> > + event_notifier_test_and_clear(&vdev->masked_config_notifier);
> > + if (!vdev->use_guest_notifier_mask) {
> > + vhost_config_mask(hdev, vdev, true);
> > + }
>
>
> Let's leave a new line for separating the logical blocks.
>
>
will fix this
> > if (hdev->log_enabled) {
> > uint64_t log_base;
> >
> > @@ -1773,6 +1816,19 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
> > vhost_device_iotlb_miss(hdev, vq->used_phys, true);
> > }
> > }
> > + if (!(hdev->features & (0x1ULL << VIRTIO_NET_F_STATUS))) {
> > + return 0;
> > + }
>
>
> Let's move this to vhost-net instead of doing it in the general vhost code.
>
> Or we can even remove this, otherwise it will introduce some burden if
> we need to implement other features that depends on the config interrupt
> for vhost-net/vDPA.
>
>
will fix this
> > + if (hdev->vhost_ops->vhost_set_config_call) {
> > + fd = event_notifier_get_fd(&vdev->config_notifier);
> > + r = hdev->vhost_ops->vhost_set_config_call(hdev, &fd);
> > + if (!r) {
> > + event_notifier_set(&vdev->config_notifier);
>
>
> Any reason you need to trigger the config interrupt here?
>
>
> > + }
> > + if (r) {
> > + goto fail_log;
> > + }
> > + }
> > return 0;
> > fail_log:
> > vhost_log_put(hdev, false);
> > @@ -1795,10 +1851,18 @@ fail_features:
> > void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev)
> > {
> > int i;
> > + int fd;
> >
> > /* should only be called after backend is connected */
> > assert(hdev->vhost_ops);
> > -
> > + event_notifier_test_and_clear(&vdev->masked_config_notifier);
> > + event_notifier_test_and_clear(&vdev->config_notifier);
>
>
> If we do this in the start, there's no need for such cleanup here.
>
>
> > + if ((hdev->features & (0x1ULL << VIRTIO_NET_F_STATUS))) {
>
>
> Let's move this to vhost-net or I'd rather remove this.
>
> Thanks
>
>
will fix this
> > + if (hdev->vhost_ops->vhost_set_config_call) {
> > + fd = -1;
> > + hdev->vhost_ops->vhost_set_config_call(hdev, &fd);
> > + }
> > + }
> > if (hdev->vhost_ops->vhost_dev_start) {
> > hdev->vhost_ops->vhost_dev_start(hdev, false);
> > }
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > index c5d786bb5e..09ed3f67d9 100644
> > --- a/hw/virtio/virtio.c
> > +++ b/hw/virtio/virtio.c
> > @@ -3506,14 +3506,27 @@ static void virtio_queue_guest_notifier_read(EventNotifier *n)
> > }
> >
> >
> > +static void virtio_config_read(EventNotifier *n)
> > +{
> > + VirtIODevice *vdev = container_of(n, VirtIODevice, config_notifier);
> > +
> > + if (event_notifier_test_and_clear(n)) {
> > + virtio_notify_config(vdev);
> > + }
> > +}
> > void virtio_set_notifier_fd_handler(VirtIODevice *vdev, int queue_no,
> > bool assign, bool with_irqfd)
> > {
> > EventNotifier *e ;
> > EventNotifierHandler *handler;
> > - VirtQueue *vq = virtio_get_queue(vdev, queue_no);
> > - e = &vq->guest_notifier;
> > - handler = virtio_queue_guest_notifier_read;
> > + if (queue_no != VIRTIO_CONFIG_IRQ_IDX) {
> > + VirtQueue *vq = virtio_get_queue(vdev, queue_no);
> > + e = &vq->guest_notifier;
> > + handler = virtio_queue_guest_notifier_read;
> > + } else {
> > + e = &vdev->config_notifier;
> > + handler = virtio_config_read;
> > + }
> > if (assign && !with_irqfd) {
> > event_notifier_set_handler(e, handler);
> > } else {
> > @@ -3599,6 +3612,10 @@ EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq)
> > return &vq->host_notifier;
> > }
> >
> > +EventNotifier *virtio_get_config_notifier(VirtIODevice *vdev)
> > +{
> > + return &vdev->config_notifier;
> > +}
> > void virtio_queue_set_host_notifier_enabled(VirtQueue *vq, bool enabled)
> > {
> > vq->host_notifier_enabled = enabled;
> > diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> > index 4a8bc75415..b8814ece32 100644
> > --- a/include/hw/virtio/vhost.h
> > +++ b/include/hw/virtio/vhost.h
> > @@ -108,6 +108,8 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev);
> > void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev);
> > int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev);
> > void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev);
> > +bool vhost_config_pending(struct vhost_dev *hdev, int n);
> > +void vhost_config_mask(struct vhost_dev *hdev, VirtIODevice *vdev, bool mask);
> >
> > /* Test and clear masked event pending status.
> > * Should be called after unmask to avoid losing events.
> > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> > index 447899dea5..5856517d43 100644
> > --- a/include/hw/virtio/virtio.h
> > +++ b/include/hw/virtio/virtio.h
> > @@ -110,6 +110,8 @@ struct VirtIODevice
> > bool use_guest_notifier_mask;
> > AddressSpace *dma_as;
> > QLIST_HEAD(, VirtQueue) *vector_queues;
> > + EventNotifier config_notifier;
> > + EventNotifier masked_config_notifier;
> > };
> >
> > struct VirtioDeviceClass {
> > @@ -317,6 +319,7 @@ int virtio_device_grab_ioeventfd(VirtIODevice *vdev);
> > void virtio_device_release_ioeventfd(VirtIODevice *vdev);
> > bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev);
> > EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq);
> > +EventNotifier *virtio_get_config_notifier(VirtIODevice *vdev);
> > void virtio_queue_set_host_notifier_enabled(VirtQueue *vq, bool enabled);
> > void virtio_queue_host_notifier_read(EventNotifier *n);
> > void virtio_queue_aio_set_host_notifier_handler(VirtQueue *vq, AioContext *ctx,
> > diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
> > index 172b0051d8..0d38c97c94 100644
> > --- a/include/net/vhost_net.h
> > +++ b/include/net/vhost_net.h
> > @@ -36,6 +36,9 @@ int vhost_net_set_config(struct vhost_net *net, const uint8_t *data,
> > bool vhost_net_virtqueue_pending(VHostNetState *net, int n);
> > void vhost_net_virtqueue_mask(VHostNetState *net, VirtIODevice *dev,
> > int idx, bool mask);
> > +bool vhost_net_config_pending(VHostNetState *net, int n);
> > +void vhost_net_config_mask(VHostNetState *net, VirtIODevice *dev,
> > + bool mask);
> > int vhost_net_notify_migration_done(VHostNetState *net, char* mac_addr);
> > VHostNetState *get_vhost_net(NetClientState *nc);
> >
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v7 07/10] virtio-mmio: add support for configure interrupt
2021-06-02 3:47 [PATCH v7 00/10] vhost-vdpa: add support for configure interrupt Cindy Lu
` (5 preceding siblings ...)
2021-06-02 3:47 ` [PATCH v7 06/10] vhost:add support for configure interrupt Cindy Lu
@ 2021-06-02 3:47 ` Cindy Lu
2021-06-03 6:35 ` Jason Wang
2021-06-02 3:47 ` [PATCH v7 08/10] virtio-pci: decouple virtqueue from kvm_virtio_pci_vector_use Cindy Lu
` (2 subsequent siblings)
9 siblings, 1 reply; 26+ messages in thread
From: Cindy Lu @ 2021-06-02 3:47 UTC (permalink / raw)
To: lulu, mst, jasowang, qemu-devel
Add configure interrupt support for virtio-mmio bus. This
interrupt will working while backend is vhost-vdpa
Signed-off-by: Cindy Lu <lulu@redhat.com>
---
hw/virtio/virtio-mmio.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index 13772d52bb..423267d51c 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -670,7 +670,26 @@ static int virtio_mmio_set_guest_notifier(DeviceState *d, int n, bool assign,
return 0;
}
+static int virtio_mmio_set_config_notifier(DeviceState *d, bool assign)
+{
+ VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
+ VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+ VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
+ EventNotifier *notifier = virtio_get_config_notifier(vdev);
+ int r = 0;
+ if (assign) {
+ r = event_notifier_init(notifier, 0);
+ virtio_set_notifier_fd_handler(vdev, -1, true, false);
+ } else {
+ virtio_set_notifier_fd_handler(vdev, -1, false, false);
+ event_notifier_cleanup(notifier);
+ }
+ if (vdc->guest_notifier_mask && vdev->use_guest_notifier_mask) {
+ vdc->guest_notifier_mask(vdev, -1, !assign);
+ }
+ return r;
+}
static int virtio_mmio_set_guest_notifiers(DeviceState *d, int nvqs,
bool assign)
{
@@ -692,8 +711,15 @@ static int virtio_mmio_set_guest_notifiers(DeviceState *d, int nvqs,
goto assign_error;
}
}
+ r = virtio_mmio_set_config_notifier(d, assign);
+ if (r < 0) {
+ goto config_assign_error;
+ }
return 0;
+config_assign_error:
+ assert(assign);
+ r = virtio_mmio_set_config_notifier(d, false);
assign_error:
/* We get here on assignment failure. Recover by undoing for VQs 0 .. n. */
--
2.21.3
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v7 07/10] virtio-mmio: add support for configure interrupt
2021-06-02 3:47 ` [PATCH v7 07/10] virtio-mmio: add " Cindy Lu
@ 2021-06-03 6:35 ` Jason Wang
2021-06-07 6:35 ` Cindy Lu
0 siblings, 1 reply; 26+ messages in thread
From: Jason Wang @ 2021-06-03 6:35 UTC (permalink / raw)
To: Cindy Lu, mst, qemu-devel
在 2021/6/2 上午11:47, Cindy Lu 写道:
> Add configure interrupt support for virtio-mmio bus. This
> interrupt will working while backend is vhost-vdpa
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
> hw/virtio/virtio-mmio.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
> index 13772d52bb..423267d51c 100644
> --- a/hw/virtio/virtio-mmio.c
> +++ b/hw/virtio/virtio-mmio.c
> @@ -670,7 +670,26 @@ static int virtio_mmio_set_guest_notifier(DeviceState *d, int n, bool assign,
>
> return 0;
> }
> +static int virtio_mmio_set_config_notifier(DeviceState *d, bool assign)
> +{
> + VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
> + VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> + VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
>
> + EventNotifier *notifier = virtio_get_config_notifier(vdev);
> + int r = 0;
> + if (assign) {
> + r = event_notifier_init(notifier, 0);
The return value is ignored.
> + virtio_set_notifier_fd_handler(vdev, -1, true, false);
You'd better use the macro you introduced in patch 1 here?
> + } else {
> + virtio_set_notifier_fd_handler(vdev, -1, false, false);
> + event_notifier_cleanup(notifier);
> + }
> + if (vdc->guest_notifier_mask && vdev->use_guest_notifier_mask) {
> + vdc->guest_notifier_mask(vdev, -1, !assign);
> + }
> + return r;
> +}
> static int virtio_mmio_set_guest_notifiers(DeviceState *d, int nvqs,
> bool assign)
> {
> @@ -692,8 +711,15 @@ static int virtio_mmio_set_guest_notifiers(DeviceState *d, int nvqs,
> goto assign_error;
> }
> }
> + r = virtio_mmio_set_config_notifier(d, assign);
> + if (r < 0) {
> + goto config_assign_error;
> + }
>
> return 0;
> +config_assign_error:
> + assert(assign);
> + r = virtio_mmio_set_config_notifier(d, false);
This looks wired. We only have a single configure interrupt, so assign
fails should mean unassigned?
Thanks
>
> assign_error:
> /* We get here on assignment failure. Recover by undoing for VQs 0 .. n. */
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v7 07/10] virtio-mmio: add support for configure interrupt
2021-06-03 6:35 ` Jason Wang
@ 2021-06-07 6:35 ` Cindy Lu
0 siblings, 0 replies; 26+ messages in thread
From: Cindy Lu @ 2021-06-07 6:35 UTC (permalink / raw)
To: Jason Wang; +Cc: QEMU Developers, Michael Tsirkin
On Thu, Jun 3, 2021 at 2:36 PM Jason Wang <jasowang@redhat.com> wrote:
>
>
> 在 2021/6/2 上午11:47, Cindy Lu 写道:
> > Add configure interrupt support for virtio-mmio bus. This
> > interrupt will working while backend is vhost-vdpa
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > ---
> > hw/virtio/virtio-mmio.c | 26 ++++++++++++++++++++++++++
> > 1 file changed, 26 insertions(+)
> >
> > diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
> > index 13772d52bb..423267d51c 100644
> > --- a/hw/virtio/virtio-mmio.c
> > +++ b/hw/virtio/virtio-mmio.c
> > @@ -670,7 +670,26 @@ static int virtio_mmio_set_guest_notifier(DeviceState *d, int n, bool assign,
> >
> > return 0;
> > }
> > +static int virtio_mmio_set_config_notifier(DeviceState *d, bool assign)
> > +{
> > + VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
> > + VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> > + VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
> >
> > + EventNotifier *notifier = virtio_get_config_notifier(vdev);
> > + int r = 0;
> > + if (assign) {
> > + r = event_notifier_init(notifier, 0);
>
>
> The return value is ignored.
>
will add the check here
>
> > + virtio_set_notifier_fd_handler(vdev, -1, true, false);
>
>
> You'd better use the macro you introduced in patch 1 here?
>
sorry, Seems I missed this, I will correct this
>
> > + } else {
> > + virtio_set_notifier_fd_handler(vdev, -1, false, false);
> > + event_notifier_cleanup(notifier);
> > + }
> > + if (vdc->guest_notifier_mask && vdev->use_guest_notifier_mask) {
> > + vdc->guest_notifier_mask(vdev, -1, !assign);
> > + }
> > + return r;
> > +}
> > static int virtio_mmio_set_guest_notifiers(DeviceState *d, int nvqs,
> > bool assign)
> > {
> > @@ -692,8 +711,15 @@ static int virtio_mmio_set_guest_notifiers(DeviceState *d, int nvqs,
> > goto assign_error;
> > }
> > }
> > + r = virtio_mmio_set_config_notifier(d, assign);
> > + if (r < 0) {
> > + goto config_assign_error;
> > + }
> >
> > return 0;
> > +config_assign_error:
> > + assert(assign);
> > + r = virtio_mmio_set_config_notifier(d, false);
>
>
> This looks wired. We only have a single configure interrupt, so assign
> fails should mean unassigned?
>
> Thanks
>
sure Will correct this
> >
> > assign_error:
> > /* We get here on assignment failure. Recover by undoing for VQs 0 .. n. */
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v7 08/10] virtio-pci: decouple virtqueue from kvm_virtio_pci_vector_use
2021-06-02 3:47 [PATCH v7 00/10] vhost-vdpa: add support for configure interrupt Cindy Lu
` (6 preceding siblings ...)
2021-06-02 3:47 ` [PATCH v7 07/10] virtio-mmio: add " Cindy Lu
@ 2021-06-02 3:47 ` Cindy Lu
2021-06-03 6:39 ` Jason Wang
2021-06-02 3:47 ` [PATCH v7 09/10] virtio-pci: add support for configure interrupt Cindy Lu
2021-06-02 3:47 ` [PATCH v7 10/10] virtio-net: add peer_deleted check in virtio_net_handle_rx Cindy Lu
9 siblings, 1 reply; 26+ messages in thread
From: Cindy Lu @ 2021-06-02 3:47 UTC (permalink / raw)
To: lulu, mst, jasowang, qemu-devel
inorder to support configure interrupt, we need to decouple
virtqueue from vector use and vector release function
this patch introduce vector_release_one and vector_use_one
to support one vector.
Signed-off-by: Cindy Lu <lulu@redhat.com>
---
hw/virtio/virtio-pci.c | 122 ++++++++++++++++++++---------------------
1 file changed, 61 insertions(+), 61 deletions(-)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 6a4ef413a4..f863c89de6 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -666,7 +666,6 @@ static uint32_t virtio_read_config(PCIDevice *pci_dev,
}
static int kvm_virtio_pci_vq_vector_use(VirtIOPCIProxy *proxy,
- unsigned int queue_no,
unsigned int vector)
{
VirtIOIRQFD *irqfd = &proxy->vector_irqfd[vector];
@@ -710,85 +709,86 @@ static void kvm_virtio_pci_irqfd_release(VirtIOPCIProxy *proxy,
ret = kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state, n, irqfd->virq);
assert(ret == 0);
}
-
-static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
+static int virtio_pci_get_notifier(VirtIOPCIProxy *proxy, int queue_no,
+ EventNotifier **n, unsigned int *vector)
{
PCIDevice *dev = &proxy->pci_dev;
VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
- VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
- unsigned int vector;
- int ret, queue_no;
VirtQueue *vq;
- EventNotifier *n;
- for (queue_no = 0; queue_no < nvqs; queue_no++) {
+
+ if (queue_no == VIRTIO_CONFIG_IRQ_IDX) {
+ return -1;
+ } else {
if (!virtio_queue_get_num(vdev, queue_no)) {
- break;
- }
- vector = virtio_queue_vector(vdev, queue_no);
- if (vector >= msix_nr_vectors_allocated(dev)) {
- continue;
- }
- ret = kvm_virtio_pci_vq_vector_use(proxy, queue_no, vector);
- if (ret < 0) {
- goto undo;
- }
- /* If guest supports masking, set up irqfd now.
- * Otherwise, delay until unmasked in the frontend.
- */
- if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
- vq = virtio_get_queue(vdev, queue_no);
- n = virtio_queue_get_guest_notifier(vq);
- ret = kvm_virtio_pci_irqfd_use(proxy, n, vector);
- if (ret < 0) {
- kvm_virtio_pci_vq_vector_release(proxy, vector);
- goto undo;
- }
+ return -1;
}
+ *vector = virtio_queue_vector(vdev, queue_no);
+ vq = virtio_get_queue(vdev, queue_no);
+ *n = virtio_queue_get_guest_notifier(vq);
+ }
+ if (*vector >= msix_nr_vectors_allocated(dev)) {
+ return -1;
}
return 0;
+}
+static int kvm_virtio_pci_vector_use_one(VirtIOPCIProxy *proxy, int queue_no)
+{
+ unsigned int vector;
+ int ret;
+ EventNotifier *n;
+ ret = virtio_pci_get_notifier(proxy, queue_no, &n, &vector);
+ if (ret < 0) {
+ return ret;
+ }
+ ret = kvm_virtio_pci_vq_vector_use(proxy, vector);
+ if (ret < 0) {
+ goto undo;
+ }
+ ret = kvm_virtio_pci_irqfd_use(proxy, n, vector);
+ if (ret < 0) {
+ goto undo;
+ }
+ return 0;
undo:
- while (--queue_no >= 0) {
- vector = virtio_queue_vector(vdev, queue_no);
- if (vector >= msix_nr_vectors_allocated(dev)) {
- continue;
- }
- if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
- vq = virtio_get_queue(vdev, queue_no);
- n = virtio_queue_get_guest_notifier(vq);
- kvm_virtio_pci_irqfd_release(proxy, n, vector);
- }
- kvm_virtio_pci_vq_vector_release(proxy, vector);
+ kvm_virtio_pci_irqfd_release(proxy, n, vector);
+ return ret;
+}
+static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
+{
+ int queue_no;
+ int ret = 0;
+ for (queue_no = 0; queue_no < nvqs; queue_no++) {
+ ret = kvm_virtio_pci_vector_use_one(proxy, queue_no);
}
return ret;
}
-static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
+
+static void kvm_virtio_pci_vector_release_one(VirtIOPCIProxy *proxy,
+ int queue_no)
{
- PCIDevice *dev = &proxy->pci_dev;
VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
unsigned int vector;
- int queue_no;
- VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
- VirtQueue *vq;
EventNotifier *n;
+ int ret;
+ VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+ ret = virtio_pci_get_notifier(proxy, queue_no, &n, &vector);
+ if (ret < 0) {
+ return;
+ }
+
+ if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
+ kvm_virtio_pci_irqfd_release(proxy, n, vector);
+ }
+ kvm_virtio_pci_vq_vector_release(proxy, vector);
+}
+static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
+{
+ int queue_no;
+
for (queue_no = 0; queue_no < nvqs; queue_no++) {
- if (!virtio_queue_get_num(vdev, queue_no)) {
- break;
- }
- vector = virtio_queue_vector(vdev, queue_no);
- if (vector >= msix_nr_vectors_allocated(dev)) {
- continue;
- }
- /* If guest supports masking, clean up irqfd now.
- * Otherwise, it was cleaned when masked in the frontend.
- */
- if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
- vq = virtio_get_queue(vdev, queue_no);
- n = virtio_queue_get_guest_notifier(vq);
- kvm_virtio_pci_irqfd_release(proxy, n, vector);
- }
- kvm_virtio_pci_vq_vector_release(proxy, vector);
+ kvm_virtio_pci_vector_release_one(proxy, queue_no);
}
}
--
2.21.3
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v7 08/10] virtio-pci: decouple virtqueue from kvm_virtio_pci_vector_use
2021-06-02 3:47 ` [PATCH v7 08/10] virtio-pci: decouple virtqueue from kvm_virtio_pci_vector_use Cindy Lu
@ 2021-06-03 6:39 ` Jason Wang
2021-06-07 6:36 ` Cindy Lu
0 siblings, 1 reply; 26+ messages in thread
From: Jason Wang @ 2021-06-03 6:39 UTC (permalink / raw)
To: Cindy Lu, mst, qemu-devel
在 2021/6/2 上午11:47, Cindy Lu 写道:
> inorder
s/inorder/In order/
> to support configure interrupt, we need to decouple
> virtqueue from vector use and vector release function
> this patch introduce vector_release_one and vector_use_one
> to support one vector.
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
I think we need to reorder the patches to let such decoupling comes
first in this series.
> ---
> hw/virtio/virtio-pci.c | 122 ++++++++++++++++++++---------------------
> 1 file changed, 61 insertions(+), 61 deletions(-)
>
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 6a4ef413a4..f863c89de6 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -666,7 +666,6 @@ static uint32_t virtio_read_config(PCIDevice *pci_dev,
> }
>
> static int kvm_virtio_pci_vq_vector_use(VirtIOPCIProxy *proxy,
> - unsigned int queue_no,
> unsigned int vector)
> {
> VirtIOIRQFD *irqfd = &proxy->vector_irqfd[vector];
> @@ -710,85 +709,86 @@ static void kvm_virtio_pci_irqfd_release(VirtIOPCIProxy *proxy,
> ret = kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state, n, irqfd->virq);
> assert(ret == 0);
> }
> -
> -static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
> +static int virtio_pci_get_notifier(VirtIOPCIProxy *proxy, int queue_no,
> + EventNotifier **n, unsigned int *vector)
> {
> PCIDevice *dev = &proxy->pci_dev;
> VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> - VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
> - unsigned int vector;
> - int ret, queue_no;
> VirtQueue *vq;
> - EventNotifier *n;
> - for (queue_no = 0; queue_no < nvqs; queue_no++) {
> +
> + if (queue_no == VIRTIO_CONFIG_IRQ_IDX) {
> + return -1;
> + } else {
> if (!virtio_queue_get_num(vdev, queue_no)) {
> - break;
> - }
> - vector = virtio_queue_vector(vdev, queue_no);
> - if (vector >= msix_nr_vectors_allocated(dev)) {
> - continue;
> - }
> - ret = kvm_virtio_pci_vq_vector_use(proxy, queue_no, vector);
> - if (ret < 0) {
> - goto undo;
> - }
> - /* If guest supports masking, set up irqfd now.
> - * Otherwise, delay until unmasked in the frontend.
> - */
> - if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
> - vq = virtio_get_queue(vdev, queue_no);
> - n = virtio_queue_get_guest_notifier(vq);
> - ret = kvm_virtio_pci_irqfd_use(proxy, n, vector);
> - if (ret < 0) {
> - kvm_virtio_pci_vq_vector_release(proxy, vector);
> - goto undo;
> - }
> + return -1;
> }
> + *vector = virtio_queue_vector(vdev, queue_no);
> + vq = virtio_get_queue(vdev, queue_no);
> + *n = virtio_queue_get_guest_notifier(vq);
> + }
> + if (*vector >= msix_nr_vectors_allocated(dev)) {
> + return -1;
> }
> return 0;
> +}
>
> +static int kvm_virtio_pci_vector_use_one(VirtIOPCIProxy *proxy, int queue_no)
> +{
To ease the reviewer, let's separate this patch into two.
1) factoring out the core logic
2) decouple the vq
Thanks
> + unsigned int vector;
> + int ret;
> + EventNotifier *n;
> + ret = virtio_pci_get_notifier(proxy, queue_no, &n, &vector);
> + if (ret < 0) {
> + return ret;
> + }
> + ret = kvm_virtio_pci_vq_vector_use(proxy, vector);
> + if (ret < 0) {
> + goto undo;
> + }
> + ret = kvm_virtio_pci_irqfd_use(proxy, n, vector);
> + if (ret < 0) {
> + goto undo;
> + }
> + return 0;
> undo:
> - while (--queue_no >= 0) {
> - vector = virtio_queue_vector(vdev, queue_no);
> - if (vector >= msix_nr_vectors_allocated(dev)) {
> - continue;
> - }
> - if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
> - vq = virtio_get_queue(vdev, queue_no);
> - n = virtio_queue_get_guest_notifier(vq);
> - kvm_virtio_pci_irqfd_release(proxy, n, vector);
> - }
> - kvm_virtio_pci_vq_vector_release(proxy, vector);
> + kvm_virtio_pci_irqfd_release(proxy, n, vector);
> + return ret;
> +}
> +static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
> +{
> + int queue_no;
> + int ret = 0;
> + for (queue_no = 0; queue_no < nvqs; queue_no++) {
> + ret = kvm_virtio_pci_vector_use_one(proxy, queue_no);
> }
> return ret;
> }
>
> -static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
> +
> +static void kvm_virtio_pci_vector_release_one(VirtIOPCIProxy *proxy,
> + int queue_no)
> {
> - PCIDevice *dev = &proxy->pci_dev;
> VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> unsigned int vector;
> - int queue_no;
> - VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
> - VirtQueue *vq;
> EventNotifier *n;
> + int ret;
> + VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
> + ret = virtio_pci_get_notifier(proxy, queue_no, &n, &vector);
> + if (ret < 0) {
> + return;
> + }
> +
> + if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
> + kvm_virtio_pci_irqfd_release(proxy, n, vector);
> + }
> + kvm_virtio_pci_vq_vector_release(proxy, vector);
> +}
> +static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
> +{
> + int queue_no;
> +
> for (queue_no = 0; queue_no < nvqs; queue_no++) {
> - if (!virtio_queue_get_num(vdev, queue_no)) {
> - break;
> - }
> - vector = virtio_queue_vector(vdev, queue_no);
> - if (vector >= msix_nr_vectors_allocated(dev)) {
> - continue;
> - }
> - /* If guest supports masking, clean up irqfd now.
> - * Otherwise, it was cleaned when masked in the frontend.
> - */
> - if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
> - vq = virtio_get_queue(vdev, queue_no);
> - n = virtio_queue_get_guest_notifier(vq);
> - kvm_virtio_pci_irqfd_release(proxy, n, vector);
> - }
> - kvm_virtio_pci_vq_vector_release(proxy, vector);
> + kvm_virtio_pci_vector_release_one(proxy, queue_no);
> }
> }
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v7 08/10] virtio-pci: decouple virtqueue from kvm_virtio_pci_vector_use
2021-06-03 6:39 ` Jason Wang
@ 2021-06-07 6:36 ` Cindy Lu
0 siblings, 0 replies; 26+ messages in thread
From: Cindy Lu @ 2021-06-07 6:36 UTC (permalink / raw)
To: Jason Wang; +Cc: QEMU Developers, Michael Tsirkin
On Thu, Jun 3, 2021 at 2:39 PM Jason Wang <jasowang@redhat.com> wrote:
>
>
> 在 2021/6/2 上午11:47, Cindy Lu 写道:
> > inorder
>
>
> s/inorder/In order/
>
>
> > to support configure interrupt, we need to decouple
> > virtqueue from vector use and vector release function
> > this patch introduce vector_release_one and vector_use_one
> > to support one vector.
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
>
>
> I think we need to reorder the patches to let such decoupling comes
> first in this series.
>
>
> > ---
> > hw/virtio/virtio-pci.c | 122 ++++++++++++++++++++---------------------
> > 1 file changed, 61 insertions(+), 61 deletions(-)
> >
> > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > index 6a4ef413a4..f863c89de6 100644
> > --- a/hw/virtio/virtio-pci.c
> > +++ b/hw/virtio/virtio-pci.c
> > @@ -666,7 +666,6 @@ static uint32_t virtio_read_config(PCIDevice *pci_dev,
> > }
> >
> > static int kvm_virtio_pci_vq_vector_use(VirtIOPCIProxy *proxy,
> > - unsigned int queue_no,
> > unsigned int vector)
> > {
> > VirtIOIRQFD *irqfd = &proxy->vector_irqfd[vector];
> > @@ -710,85 +709,86 @@ static void kvm_virtio_pci_irqfd_release(VirtIOPCIProxy *proxy,
> > ret = kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state, n, irqfd->virq);
> > assert(ret == 0);
> > }
> > -
> > -static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
> > +static int virtio_pci_get_notifier(VirtIOPCIProxy *proxy, int queue_no,
> > + EventNotifier **n, unsigned int *vector)
> > {
> > PCIDevice *dev = &proxy->pci_dev;
> > VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> > - VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
> > - unsigned int vector;
> > - int ret, queue_no;
> > VirtQueue *vq;
> > - EventNotifier *n;
> > - for (queue_no = 0; queue_no < nvqs; queue_no++) {
> > +
> > + if (queue_no == VIRTIO_CONFIG_IRQ_IDX) {
> > + return -1;
> > + } else {
> > if (!virtio_queue_get_num(vdev, queue_no)) {
> > - break;
> > - }
> > - vector = virtio_queue_vector(vdev, queue_no);
> > - if (vector >= msix_nr_vectors_allocated(dev)) {
> > - continue;
> > - }
> > - ret = kvm_virtio_pci_vq_vector_use(proxy, queue_no, vector);
> > - if (ret < 0) {
> > - goto undo;
> > - }
> > - /* If guest supports masking, set up irqfd now.
> > - * Otherwise, delay until unmasked in the frontend.
> > - */
> > - if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
> > - vq = virtio_get_queue(vdev, queue_no);
> > - n = virtio_queue_get_guest_notifier(vq);
> > - ret = kvm_virtio_pci_irqfd_use(proxy, n, vector);
> > - if (ret < 0) {
> > - kvm_virtio_pci_vq_vector_release(proxy, vector);
> > - goto undo;
> > - }
> > + return -1;
> > }
> > + *vector = virtio_queue_vector(vdev, queue_no);
> > + vq = virtio_get_queue(vdev, queue_no);
> > + *n = virtio_queue_get_guest_notifier(vq);
> > + }
> > + if (*vector >= msix_nr_vectors_allocated(dev)) {
> > + return -1;
> > }
> > return 0;
> > +}
> >
> > +static int kvm_virtio_pci_vector_use_one(VirtIOPCIProxy *proxy, int queue_no)
> > +{
>
>
> To ease the reviewer, let's separate this patch into two.
>
> 1) factoring out the core logic
> 2) decouple the vq
>
> Thanks
>
Thanks Jason, I will change this part
>
> > + unsigned int vector;
> > + int ret;
> > + EventNotifier *n;
> > + ret = virtio_pci_get_notifier(proxy, queue_no, &n, &vector);
> > + if (ret < 0) {
> > + return ret;
> > + }
> > + ret = kvm_virtio_pci_vq_vector_use(proxy, vector);
> > + if (ret < 0) {
> > + goto undo;
> > + }
> > + ret = kvm_virtio_pci_irqfd_use(proxy, n, vector);
> > + if (ret < 0) {
> > + goto undo;
> > + }
> > + return 0;
> > undo:
> > - while (--queue_no >= 0) {
> > - vector = virtio_queue_vector(vdev, queue_no);
> > - if (vector >= msix_nr_vectors_allocated(dev)) {
> > - continue;
> > - }
> > - if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
> > - vq = virtio_get_queue(vdev, queue_no);
> > - n = virtio_queue_get_guest_notifier(vq);
> > - kvm_virtio_pci_irqfd_release(proxy, n, vector);
> > - }
> > - kvm_virtio_pci_vq_vector_release(proxy, vector);
> > + kvm_virtio_pci_irqfd_release(proxy, n, vector);
> > + return ret;
> > +}
> > +static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
> > +{
> > + int queue_no;
> > + int ret = 0;
> > + for (queue_no = 0; queue_no < nvqs; queue_no++) {
> > + ret = kvm_virtio_pci_vector_use_one(proxy, queue_no);
> > }
> > return ret;
> > }
> >
> > -static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
> > +
> > +static void kvm_virtio_pci_vector_release_one(VirtIOPCIProxy *proxy,
> > + int queue_no)
> > {
> > - PCIDevice *dev = &proxy->pci_dev;
> > VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> > unsigned int vector;
> > - int queue_no;
> > - VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
> > - VirtQueue *vq;
> > EventNotifier *n;
> > + int ret;
> > + VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
> > + ret = virtio_pci_get_notifier(proxy, queue_no, &n, &vector);
> > + if (ret < 0) {
> > + return;
> > + }
> > +
> > + if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
> > + kvm_virtio_pci_irqfd_release(proxy, n, vector);
> > + }
> > + kvm_virtio_pci_vq_vector_release(proxy, vector);
> > +}
> > +static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
> > +{
> > + int queue_no;
> > +
> > for (queue_no = 0; queue_no < nvqs; queue_no++) {
> > - if (!virtio_queue_get_num(vdev, queue_no)) {
> > - break;
> > - }
> > - vector = virtio_queue_vector(vdev, queue_no);
> > - if (vector >= msix_nr_vectors_allocated(dev)) {
> > - continue;
> > - }
> > - /* If guest supports masking, clean up irqfd now.
> > - * Otherwise, it was cleaned when masked in the frontend.
> > - */
> > - if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
> > - vq = virtio_get_queue(vdev, queue_no);
> > - n = virtio_queue_get_guest_notifier(vq);
> > - kvm_virtio_pci_irqfd_release(proxy, n, vector);
> > - }
> > - kvm_virtio_pci_vq_vector_release(proxy, vector);
> > + kvm_virtio_pci_vector_release_one(proxy, queue_no);
> > }
> > }
> >
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v7 09/10] virtio-pci: add support for configure interrupt
2021-06-02 3:47 [PATCH v7 00/10] vhost-vdpa: add support for configure interrupt Cindy Lu
` (7 preceding siblings ...)
2021-06-02 3:47 ` [PATCH v7 08/10] virtio-pci: decouple virtqueue from kvm_virtio_pci_vector_use Cindy Lu
@ 2021-06-02 3:47 ` Cindy Lu
2021-06-03 6:45 ` Jason Wang
2021-06-02 3:47 ` [PATCH v7 10/10] virtio-net: add peer_deleted check in virtio_net_handle_rx Cindy Lu
9 siblings, 1 reply; 26+ messages in thread
From: Cindy Lu @ 2021-06-02 3:47 UTC (permalink / raw)
To: lulu, mst, jasowang, qemu-devel
Add support for configure interrupt, use kvm_irqfd_assign and set the
gsi to kernel. When the configure notifier was eventfd_signal by host
kernel, this will finally inject an msix interrupt to guest
Signed-off-by: Cindy Lu <lulu@redhat.com>
---
hw/virtio/virtio-pci.c | 63 ++++++++++++++++++++++++++++++++++++++++--
1 file changed, 60 insertions(+), 3 deletions(-)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index f863c89de6..1e03f11a85 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -717,7 +717,8 @@ static int virtio_pci_get_notifier(VirtIOPCIProxy *proxy, int queue_no,
VirtQueue *vq;
if (queue_no == VIRTIO_CONFIG_IRQ_IDX) {
- return -1;
+ *n = virtio_get_config_notifier(vdev);
+ *vector = vdev->config_vector;
} else {
if (!virtio_queue_get_num(vdev, queue_no)) {
return -1;
@@ -764,6 +765,10 @@ static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
return ret;
}
+static int kvm_virtio_pci_vector_config_use(VirtIOPCIProxy *proxy)
+{
+ return kvm_virtio_pci_vector_use_one(proxy, VIRTIO_CONFIG_IRQ_IDX);
+}
static void kvm_virtio_pci_vector_release_one(VirtIOPCIProxy *proxy,
int queue_no)
@@ -792,6 +797,28 @@ static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
}
}
+static void kvm_virtio_pci_vector_config_release(VirtIOPCIProxy *proxy)
+{
+ kvm_virtio_pci_vector_release_one(proxy, VIRTIO_CONFIG_IRQ_IDX);
+}
+static int virtio_pci_set_config_notifier(DeviceState *d, bool assign)
+{
+ VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
+ VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+ EventNotifier *notifier = virtio_get_config_notifier(vdev);
+ int r = 0;
+ if (assign) {
+ r = event_notifier_init(notifier, 0);
+ virtio_set_notifier_fd_handler(vdev, VIRTIO_CONFIG_IRQ_IDX, true, true);
+ kvm_virtio_pci_vector_config_use(proxy);
+ } else {
+ virtio_set_notifier_fd_handler(vdev, VIRTIO_CONFIG_IRQ_IDX,
+ false, true);
+ kvm_virtio_pci_vector_config_release(proxy);
+ event_notifier_cleanup(notifier);
+ }
+ return r;
+}
static int virtio_pci_vq_vector_unmask(VirtIOPCIProxy *proxy,
unsigned int queue_no,
unsigned int vector,
@@ -873,9 +900,17 @@ static int virtio_pci_vector_unmask(PCIDevice *dev, unsigned vector,
}
vq = virtio_vector_next_queue(vq);
}
-
+ n = virtio_get_config_notifier(vdev);
+ ret = virtio_pci_vq_vector_unmask(proxy, VIRTIO_CONFIG_IRQ_IDX,
+ vector, msg, n);
+ if (ret < 0) {
+ goto config_undo;
+ }
return 0;
+config_undo:
+ n = virtio_get_config_notifier(vdev);
+ virtio_pci_vq_vector_mask(proxy, VIRTIO_CONFIG_IRQ_IDX, vector, n);
undo:
vq = virtio_vector_first_queue(vdev, vector);
while (vq && unmasked >= 0) {
@@ -909,6 +944,8 @@ static void virtio_pci_vector_mask(PCIDevice *dev, unsigned vector)
}
vq = virtio_vector_next_queue(vq);
}
+ n = virtio_get_config_notifier(vdev);
+ virtio_pci_vq_vector_mask(proxy, VIRTIO_CONFIG_IRQ_IDX, vector, n);
}
static void virtio_pci_vector_poll(PCIDevice *dev,
@@ -942,6 +979,20 @@ static void virtio_pci_vector_poll(PCIDevice *dev,
msix_set_pending(dev, vector);
}
}
+ /*check for config interrupt*/
+ vector = vdev->config_vector;
+ notifier = virtio_get_config_notifier(vdev);
+ if (vector < vector_start || vector >= vector_end ||
+ !msix_is_masked(dev, vector)) {
+ return;
+ }
+ if (k->guest_notifier_pending) {
+ if (k->guest_notifier_pending(vdev, VIRTIO_CONFIG_IRQ_IDX)) {
+ msix_set_pending(dev, vector);
+ }
+ } else if (event_notifier_test_and_clear(notifier)) {
+ msix_set_pending(dev, vector);
+ }
}
static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign,
@@ -1002,6 +1053,7 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
msix_unset_vector_notifiers(&proxy->pci_dev);
if (proxy->vector_irqfd) {
kvm_virtio_pci_vector_release(proxy, nvqs);
+ kvm_virtio_pci_vector_config_release(proxy);
g_free(proxy->vector_irqfd);
proxy->vector_irqfd = NULL;
}
@@ -1029,6 +1081,10 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
goto assign_error;
}
}
+ r = virtio_pci_set_config_notifier(d, assign);
+ if (r < 0) {
+ goto config_error;
+ }
r = msix_set_vector_notifiers(&proxy->pci_dev,
virtio_pci_vector_unmask,
virtio_pci_vector_mask,
@@ -1045,7 +1101,8 @@ notifiers_error:
assert(assign);
kvm_virtio_pci_vector_release(proxy, nvqs);
}
-
+config_error:
+ kvm_virtio_pci_vector_config_release(proxy);
assign_error:
/* We get here on assignment failure. Recover by undoing for VQs 0 .. n. */
assert(assign);
--
2.21.3
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v7 09/10] virtio-pci: add support for configure interrupt
2021-06-02 3:47 ` [PATCH v7 09/10] virtio-pci: add support for configure interrupt Cindy Lu
@ 2021-06-03 6:45 ` Jason Wang
2021-06-07 6:44 ` Cindy Lu
0 siblings, 1 reply; 26+ messages in thread
From: Jason Wang @ 2021-06-03 6:45 UTC (permalink / raw)
To: Cindy Lu, mst, qemu-devel
在 2021/6/2 上午11:47, Cindy Lu 写道:
> Add support for configure interrupt, use kvm_irqfd_assign and set the
> gsi to kernel. When the configure notifier was eventfd_signal by host
> kernel, this will finally inject an msix interrupt to guest
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
> hw/virtio/virtio-pci.c | 63 ++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 60 insertions(+), 3 deletions(-)
>
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index f863c89de6..1e03f11a85 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -717,7 +717,8 @@ static int virtio_pci_get_notifier(VirtIOPCIProxy *proxy, int queue_no,
> VirtQueue *vq;
>
> if (queue_no == VIRTIO_CONFIG_IRQ_IDX) {
> - return -1;
> + *n = virtio_get_config_notifier(vdev);
> + *vector = vdev->config_vector;
> } else {
> if (!virtio_queue_get_num(vdev, queue_no)) {
> return -1;
> @@ -764,6 +765,10 @@ static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
> return ret;
> }
>
> +static int kvm_virtio_pci_vector_config_use(VirtIOPCIProxy *proxy)
> +{
> + return kvm_virtio_pci_vector_use_one(proxy, VIRTIO_CONFIG_IRQ_IDX);
> +}
>
> static void kvm_virtio_pci_vector_release_one(VirtIOPCIProxy *proxy,
> int queue_no)
> @@ -792,6 +797,28 @@ static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
> }
> }
>
> +static void kvm_virtio_pci_vector_config_release(VirtIOPCIProxy *proxy)
> +{
> + kvm_virtio_pci_vector_release_one(proxy, VIRTIO_CONFIG_IRQ_IDX);
> +}
Newline please, can this survive from checkpatch.pl?
> +static int virtio_pci_set_config_notifier(DeviceState *d, bool assign)
> +{
> + VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
> + VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> + EventNotifier *notifier = virtio_get_config_notifier(vdev);
> + int r = 0;
> + if (assign) {
> + r = event_notifier_init(notifier, 0);
> + virtio_set_notifier_fd_handler(vdev, VIRTIO_CONFIG_IRQ_IDX, true, true);
> + kvm_virtio_pci_vector_config_use(proxy);
> + } else {
> + virtio_set_notifier_fd_handler(vdev, VIRTIO_CONFIG_IRQ_IDX,
> + false, true);
> + kvm_virtio_pci_vector_config_release(proxy);
> + event_notifier_cleanup(notifier);
> + }
> + return r;
> +}
> static int virtio_pci_vq_vector_unmask(VirtIOPCIProxy *proxy,
> unsigned int queue_no,
> unsigned int vector,
> @@ -873,9 +900,17 @@ static int virtio_pci_vector_unmask(PCIDevice *dev, unsigned vector,
> }
> vq = virtio_vector_next_queue(vq);
> }
> -
> + n = virtio_get_config_notifier(vdev);
> + ret = virtio_pci_vq_vector_unmask(proxy, VIRTIO_CONFIG_IRQ_IDX,
> + vector, msg, n);
> + if (ret < 0) {
> + goto config_undo;
> + }
I'd do this before the loop, but need to validate whether the vector is
the one that is used by the config interrupt.
> return 0;
>
> +config_undo:
> + n = virtio_get_config_notifier(vdev);
> + virtio_pci_vq_vector_mask(proxy, VIRTIO_CONFIG_IRQ_IDX, vector, n);
So unmask fail means it is still masked, we don't need to mask it again.
> undo:
> vq = virtio_vector_first_queue(vdev, vector);
> while (vq && unmasked >= 0) {
> @@ -909,6 +944,8 @@ static void virtio_pci_vector_mask(PCIDevice *dev, unsigned vector)
> }
> vq = virtio_vector_next_queue(vq);
> }
> + n = virtio_get_config_notifier(vdev);
> + virtio_pci_vq_vector_mask(proxy, VIRTIO_CONFIG_IRQ_IDX, vector, n);
> }
>
> static void virtio_pci_vector_poll(PCIDevice *dev,
> @@ -942,6 +979,20 @@ static void virtio_pci_vector_poll(PCIDevice *dev,
> msix_set_pending(dev, vector);
> }
> }
> + /*check for config interrupt*/
> + vector = vdev->config_vector;
> + notifier = virtio_get_config_notifier(vdev);
> + if (vector < vector_start || vector >= vector_end ||
> + !msix_is_masked(dev, vector)) {
> + return;
> + }
> + if (k->guest_notifier_pending) {
> + if (k->guest_notifier_pending(vdev, VIRTIO_CONFIG_IRQ_IDX)) {
> + msix_set_pending(dev, vector);
> + }
> + } else if (event_notifier_test_and_clear(notifier)) {
> + msix_set_pending(dev, vector);
> + }
Let's consider to unify the codes with vq vector here.
> }
>
> static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign,
> @@ -1002,6 +1053,7 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
> msix_unset_vector_notifiers(&proxy->pci_dev);
> if (proxy->vector_irqfd) {
> kvm_virtio_pci_vector_release(proxy, nvqs);
> + kvm_virtio_pci_vector_config_release(proxy);
> g_free(proxy->vector_irqfd);
> proxy->vector_irqfd = NULL;
> }
> @@ -1029,6 +1081,10 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
> goto assign_error;
> }
> }
> + r = virtio_pci_set_config_notifier(d, assign);
> + if (r < 0) {
> + goto config_error;
> + }
> r = msix_set_vector_notifiers(&proxy->pci_dev,
> virtio_pci_vector_unmask,
> virtio_pci_vector_mask,
> @@ -1045,7 +1101,8 @@ notifiers_error:
> assert(assign);
> kvm_virtio_pci_vector_release(proxy, nvqs);
> }
> -
Newline should be kept here.
Thanks
> +config_error:
> + kvm_virtio_pci_vector_config_release(proxy);
> assign_error:
> /* We get here on assignment failure. Recover by undoing for VQs 0 .. n. */
> assert(assign);
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v7 09/10] virtio-pci: add support for configure interrupt
2021-06-03 6:45 ` Jason Wang
@ 2021-06-07 6:44 ` Cindy Lu
0 siblings, 0 replies; 26+ messages in thread
From: Cindy Lu @ 2021-06-07 6:44 UTC (permalink / raw)
To: Jason Wang; +Cc: QEMU Developers, Michael Tsirkin
On Thu, Jun 3, 2021 at 2:45 PM Jason Wang <jasowang@redhat.com> wrote:
>
>
> 在 2021/6/2 上午11:47, Cindy Lu 写道:
> > Add support for configure interrupt, use kvm_irqfd_assign and set the
> > gsi to kernel. When the configure notifier was eventfd_signal by host
> > kernel, this will finally inject an msix interrupt to guest
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > ---
> > hw/virtio/virtio-pci.c | 63 ++++++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 60 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > index f863c89de6..1e03f11a85 100644
> > --- a/hw/virtio/virtio-pci.c
> > +++ b/hw/virtio/virtio-pci.c
> > @@ -717,7 +717,8 @@ static int virtio_pci_get_notifier(VirtIOPCIProxy *proxy, int queue_no,
> > VirtQueue *vq;
> >
> > if (queue_no == VIRTIO_CONFIG_IRQ_IDX) {
> > - return -1;
> > + *n = virtio_get_config_notifier(vdev);
> > + *vector = vdev->config_vector;
> > } else {
> > if (!virtio_queue_get_num(vdev, queue_no)) {
> > return -1;
> > @@ -764,6 +765,10 @@ static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
> > return ret;
> > }
> >
> > +static int kvm_virtio_pci_vector_config_use(VirtIOPCIProxy *proxy)
> > +{
> > + return kvm_virtio_pci_vector_use_one(proxy, VIRTIO_CONFIG_IRQ_IDX);
> > +}
> >
> > static void kvm_virtio_pci_vector_release_one(VirtIOPCIProxy *proxy,
> > int queue_no)
> > @@ -792,6 +797,28 @@ static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
> > }
> > }
> >
> > +static void kvm_virtio_pci_vector_config_release(VirtIOPCIProxy *proxy)
> > +{
> > + kvm_virtio_pci_vector_release_one(proxy, VIRTIO_CONFIG_IRQ_IDX);
> > +}
>
>
> Newline please, can this survive from checkpatch.pl?
>
sure, will fix this , I have tried checkpatch.pl?, but seems it not
working correctly in my environment
>
> > +static int virtio_pci_set_config_notifier(DeviceState *d, bool assign)
> > +{
> > + VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
> > + VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> > + EventNotifier *notifier = virtio_get_config_notifier(vdev);
> > + int r = 0;
> > + if (assign) {
> > + r = event_notifier_init(notifier, 0);
> > + virtio_set_notifier_fd_handler(vdev, VIRTIO_CONFIG_IRQ_IDX, true, true);
> > + kvm_virtio_pci_vector_config_use(proxy);
> > + } else {
> > + virtio_set_notifier_fd_handler(vdev, VIRTIO_CONFIG_IRQ_IDX,
> > + false, true);
> > + kvm_virtio_pci_vector_config_release(proxy);
> > + event_notifier_cleanup(notifier);
> > + }
> > + return r;
> > +}
> > static int virtio_pci_vq_vector_unmask(VirtIOPCIProxy *proxy,
> > unsigned int queue_no,
> > unsigned int vector,
> > @@ -873,9 +900,17 @@ static int virtio_pci_vector_unmask(PCIDevice *dev, unsigned vector,
> > }
> > vq = virtio_vector_next_queue(vq);
> > }
> > -
> > + n = virtio_get_config_notifier(vdev);
> > + ret = virtio_pci_vq_vector_unmask(proxy, VIRTIO_CONFIG_IRQ_IDX,
> > + vector, msg, n);
> > + if (ret < 0) {
> > + goto config_undo;
> > + }
>
>
> I'd do this before the loop, but need to validate whether the vector is
> the one that is used by the config interrupt.
>
sure ,I will move this section
>
> > return 0;
> >
> > +config_undo:
> > + n = virtio_get_config_notifier(vdev);
> > + virtio_pci_vq_vector_mask(proxy, VIRTIO_CONFIG_IRQ_IDX, vector, n);
>
>
> So unmask fail means it is still masked, we don't need to mask it again.
>
>
> > undo:
> > vq = virtio_vector_first_queue(vdev, vector);
> > while (vq && unmasked >= 0) {
> > @@ -909,6 +944,8 @@ static void virtio_pci_vector_mask(PCIDevice *dev, unsigned vector)
> > }
> > vq = virtio_vector_next_queue(vq);
> > }
> > + n = virtio_get_config_notifier(vdev);
> > + virtio_pci_vq_vector_mask(proxy, VIRTIO_CONFIG_IRQ_IDX, vector, n);
> > }
> >
> > static void virtio_pci_vector_poll(PCIDevice *dev,
> > @@ -942,6 +979,20 @@ static void virtio_pci_vector_poll(PCIDevice *dev,
> > msix_set_pending(dev, vector);
> > }
> > }
> > + /*check for config interrupt*/
> > + vector = vdev->config_vector;
> > + notifier = virtio_get_config_notifier(vdev);
> > + if (vector < vector_start || vector >= vector_end ||
> > + !msix_is_masked(dev, vector)) {
> > + return;
> > + }
> > + if (k->guest_notifier_pending) {
> > + if (k->guest_notifier_pending(vdev, VIRTIO_CONFIG_IRQ_IDX)) {
> > + msix_set_pending(dev, vector);
> > + }
> > + } else if (event_notifier_test_and_clear(notifier)) {
> > + msix_set_pending(dev, vector);
> > + }
>
>
> Let's consider to unify the codes with vq vector here.
>
>
Will do this
> > }
> >
> > static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign,
> > @@ -1002,6 +1053,7 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
> > msix_unset_vector_notifiers(&proxy->pci_dev);
> > if (proxy->vector_irqfd) {
> > kvm_virtio_pci_vector_release(proxy, nvqs);
> > + kvm_virtio_pci_vector_config_release(proxy);
> > g_free(proxy->vector_irqfd);
> > proxy->vector_irqfd = NULL;
> > }
> > @@ -1029,6 +1081,10 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
> > goto assign_error;
> > }
> > }
> > + r = virtio_pci_set_config_notifier(d, assign);
> > + if (r < 0) {
> > + goto config_error;
> > + }
> > r = msix_set_vector_notifiers(&proxy->pci_dev,
> > virtio_pci_vector_unmask,
> > virtio_pci_vector_mask,
> > @@ -1045,7 +1101,8 @@ notifiers_error:
> > assert(assign);
> > kvm_virtio_pci_vector_release(proxy, nvqs);
> > }
> > -
>
>
> Newline should be kept here.
>
> Thanks
>
>
will correct this
> > +config_error:
> > + kvm_virtio_pci_vector_config_release(proxy);
> > assign_error:
> > /* We get here on assignment failure. Recover by undoing for VQs 0 .. n. */
> > assert(assign);
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v7 10/10] virtio-net: add peer_deleted check in virtio_net_handle_rx
2021-06-02 3:47 [PATCH v7 00/10] vhost-vdpa: add support for configure interrupt Cindy Lu
` (8 preceding siblings ...)
2021-06-02 3:47 ` [PATCH v7 09/10] virtio-pci: add support for configure interrupt Cindy Lu
@ 2021-06-02 3:47 ` Cindy Lu
2021-06-03 6:58 ` Jason Wang
9 siblings, 1 reply; 26+ messages in thread
From: Cindy Lu @ 2021-06-02 3:47 UTC (permalink / raw)
To: lulu, mst, jasowang, qemu-devel
During the test, We found this function will continue running
while the peer is deleted, this will cause the crash. so add
check for this. this only exist in machines type microvm
reproduce step :
load the VM with
qemu-system-x86_64 -M microvm
...
-netdev tap,id=tap0,vhost=on,script=no,downscript=no \
-device virtio-net-device,netdev=tap0 \
..
enter the VM's console
shutdown the VM
(gdb) bt
0 0x000055555595b926 in qemu_net_queue_flush (queue=0x0) at ../net/queue.c:275
1 0x0000555555a046ea in qemu_flush_or_purge_queued_packets (nc=0x555556ccb920, purge=false)
at ../net/net.c:624
2 0x0000555555a04736 in qemu_flush_queued_packets (nc=0x555556ccb920) at ../net/net.c:637
3 0x0000555555ccc01a in virtio_net_handle_rx (vdev=0x555557360ed0, vq=0x7ffff40d6010)
at ../hw/net/virtio-net.c:1401
4 0x0000555555ce907a in virtio_queue_notify_vq (vq=0x7ffff40d6010) at ../hw/virtio/virtio.c:2346
5 0x0000555555cec07c in virtio_queue_host_notifier_read (n=0x7ffff40d608c)
at ../hw/virtio/virtio.c:3606
6 0x00005555560376ac in aio_dispatch_handler (ctx=0x555556a857e0, node=0x555556f013d0)
at ../util/aio-posix.c:329
7 0x00005555560377a4 in aio_dispatch_ready_handlers (ctx=0x555556a857e0,
ready_list=0x7fffffffdfe0) at ../util/aio-posix.c:359
8 0x0000555556038209 in aio_poll (ctx=0x555556a857e0, blocking=false) at ../util/aio-posix.c:662
9 0x0000555555e51c6f in monitor_cleanup () at ../monitor/monitor.c:637
10 0x0000555555d2d626 in qemu_cleanup () at ../softmmu/runstate.c:821
11 0x000055555585b19b in main (argc=21, argv=0x7fffffffe1c8, envp=0x7fffffffe278)
Signed-off-by: Cindy Lu <lulu@redhat.com>
---
hw/net/virtio-net.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 02033be748..927a808654 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1397,7 +1397,9 @@ static void virtio_net_handle_rx(VirtIODevice *vdev, VirtQueue *vq)
{
VirtIONet *n = VIRTIO_NET(vdev);
int queue_index = vq2q(virtio_get_queue_index(vq));
-
+ if (n->nic->peer_deleted) {
+ return;
+ }
qemu_flush_queued_packets(qemu_get_subqueue(n->nic, queue_index));
}
--
2.21.3
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v7 10/10] virtio-net: add peer_deleted check in virtio_net_handle_rx
2021-06-02 3:47 ` [PATCH v7 10/10] virtio-net: add peer_deleted check in virtio_net_handle_rx Cindy Lu
@ 2021-06-03 6:58 ` Jason Wang
2021-06-07 6:20 ` Cindy Lu
0 siblings, 1 reply; 26+ messages in thread
From: Jason Wang @ 2021-06-03 6:58 UTC (permalink / raw)
To: Cindy Lu, mst, qemu-devel
在 2021/6/2 上午11:47, Cindy Lu 写道:
> During the test, We found this function will continue running
> while the peer is deleted, this will cause the crash. so add
> check for this. this only exist in machines type microvm
Any idea why it only happens on microvm?
>
> reproduce step :
> load the VM with
> qemu-system-x86_64 -M microvm
> ...
> -netdev tap,id=tap0,vhost=on,script=no,downscript=no \
> -device virtio-net-device,netdev=tap0 \
> ..
> enter the VM's console
> shutdown the VM
> (gdb) bt
>
> 0 0x000055555595b926 in qemu_net_queue_flush (queue=0x0) at ../net/queue.c:275
So which piece of code trigger this? When the nc has a NIC peer we don't
free it until the NIC is freed.
> 1 0x0000555555a046ea in qemu_flush_or_purge_queued_packets (nc=0x555556ccb920, purge=false)
> at ../net/net.c:624
> 2 0x0000555555a04736 in qemu_flush_queued_packets (nc=0x555556ccb920) at ../net/net.c:637
> 3 0x0000555555ccc01a in virtio_net_handle_rx (vdev=0x555557360ed0, vq=0x7ffff40d6010)
> at ../hw/net/virtio-net.c:1401
> 4 0x0000555555ce907a in virtio_queue_notify_vq (vq=0x7ffff40d6010) at ../hw/virtio/virtio.c:2346
> 5 0x0000555555cec07c in virtio_queue_host_notifier_read (n=0x7ffff40d608c)
> at ../hw/virtio/virtio.c:3606
> 6 0x00005555560376ac in aio_dispatch_handler (ctx=0x555556a857e0, node=0x555556f013d0)
> at ../util/aio-posix.c:329
> 7 0x00005555560377a4 in aio_dispatch_ready_handlers (ctx=0x555556a857e0,
> ready_list=0x7fffffffdfe0) at ../util/aio-posix.c:359
> 8 0x0000555556038209 in aio_poll (ctx=0x555556a857e0, blocking=false) at ../util/aio-posix.c:662
> 9 0x0000555555e51c6f in monitor_cleanup () at ../monitor/monitor.c:637
> 10 0x0000555555d2d626 in qemu_cleanup () at ../softmmu/runstate.c:821
> 11 0x000055555585b19b in main (argc=21, argv=0x7fffffffe1c8, envp=0x7fffffffe278)
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
> hw/net/virtio-net.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 02033be748..927a808654 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -1397,7 +1397,9 @@ static void virtio_net_handle_rx(VirtIODevice *vdev, VirtQueue *vq)
> {
> VirtIONet *n = VIRTIO_NET(vdev);
> int queue_index = vq2q(virtio_get_queue_index(vq));
> -
> + if (n->nic->peer_deleted) {
> + return;
This needs to be fixed in the network core instead of virtio-net.
Thanks
> + }
> qemu_flush_queued_packets(qemu_get_subqueue(n->nic, queue_index));
> }
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v7 10/10] virtio-net: add peer_deleted check in virtio_net_handle_rx
2021-06-03 6:58 ` Jason Wang
@ 2021-06-07 6:20 ` Cindy Lu
0 siblings, 0 replies; 26+ messages in thread
From: Cindy Lu @ 2021-06-07 6:20 UTC (permalink / raw)
To: Jason Wang; +Cc: QEMU Developers, Michael Tsirkin
[-- Attachment #1: Type: text/plain, Size: 2928 bytes --]
On Thu, Jun 3, 2021 at 2:58 PM Jason Wang <jasowang@redhat.com> wrote:
>
> 在 2021/6/2 上午11:47, Cindy Lu 写道:
> > During the test, We found this function will continue running
> > while the peer is deleted, this will cause the crash. so add
> > check for this. this only exist in machines type microvm
>
>
> Any idea why it only happens on microvm?
>
>
> >
> > reproduce step :
> > load the VM with
> > qemu-system-x86_64 -M microvm
> > ...
> > -netdev tap,id=tap0,vhost=on,script=no,downscript=no \
> > -device virtio-net-device,netdev=tap0 \
> > ..
> > enter the VM's console
> > shutdown the VM
> > (gdb) bt
> >
> > 0 0x000055555595b926 in qemu_net_queue_flush (queue=0x0) at
> ../net/queue.c:275
>
>
> So which piece of code trigger this? When the nc has a NIC peer we don't
> free it until the NIC is freed.
>
>
> > 1 0x0000555555a046ea in qemu_flush_or_purge_queued_packets
> (nc=0x555556ccb920, purge=false)
> > at ../net/net.c:624
> > 2 0x0000555555a04736 in qemu_flush_queued_packets (nc=0x555556ccb920)
> at ../net/net.c:637
> > 3 0x0000555555ccc01a in virtio_net_handle_rx (vdev=0x555557360ed0,
> vq=0x7ffff40d6010)
> > at ../hw/net/virtio-net.c:1401
> > 4 0x0000555555ce907a in virtio_queue_notify_vq (vq=0x7ffff40d6010) at
> ../hw/virtio/virtio.c:2346
> > 5 0x0000555555cec07c in virtio_queue_host_notifier_read
> (n=0x7ffff40d608c)
> > at ../hw/virtio/virtio.c:3606
> > 6 0x00005555560376ac in aio_dispatch_handler (ctx=0x555556a857e0,
> node=0x555556f013d0)
> > at ../util/aio-posix.c:329
> > 7 0x00005555560377a4 in aio_dispatch_ready_handlers (ctx=0x555556a857e0,
> > ready_list=0x7fffffffdfe0) at ../util/aio-posix.c:359
> > 8 0x0000555556038209 in aio_poll (ctx=0x555556a857e0, blocking=false)
> at ../util/aio-posix.c:662
> > 9 0x0000555555e51c6f in monitor_cleanup () at ../monitor/monitor.c:637
> > 10 0x0000555555d2d626 in qemu_cleanup () at ../softmmu/runstate.c:821
> > 11 0x000055555585b19b in main (argc=21, argv=0x7fffffffe1c8,
> envp=0x7fffffffe278)
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > ---
> > hw/net/virtio-net.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index 02033be748..927a808654 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -1397,7 +1397,9 @@ static void virtio_net_handle_rx(VirtIODevice
> *vdev, VirtQueue *vq)
> > {
> > VirtIONet *n = VIRTIO_NET(vdev);
> > int queue_index = vq2q(virtio_get_queue_index(vq));
> > -
> > + if (n->nic->peer_deleted) {
> > + return;
>
>
> This needs to be fixed in the network core instead of virtio-net.
>
> Thanks
>
>
> sure I will fix this problem
> > + }
> > qemu_flush_queued_packets(qemu_get_subqueue(n->nic, queue_index));
> > }
> >
>
>
[-- Attachment #2: Type: text/html, Size: 3806 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread