* [PATCH v3 1/5] virtio: add support for configure interrupt
2021-01-26 7:42 [PATCH v3 0/5] vhost-vdpa: add support for configure interrupt Cindy Lu
@ 2021-01-26 7:42 ` Cindy Lu
2021-01-27 5:35 ` Jason Wang
2021-01-26 7:42 ` [PATCH v3 2/5] vhost_net: enable configure interrupt when vhost_net start Cindy Lu
` (3 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Cindy Lu @ 2021-01-26 7:42 UTC (permalink / raw)
To: mst, jasowang, qemu-devel; +Cc: lulu
Add configure notifier and virtio_set_config_notifier_fd_handler
in virtio
Signed-off-by: Cindy Lu <lulu@redhat.com>
---
hw/virtio/virtio.c | 26 ++++++++++++++++++++++++++
include/hw/virtio/virtio-bus.h | 2 ++
include/hw/virtio/virtio.h | 6 ++++++
3 files changed, 34 insertions(+)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index ceb58fda6c..25e164cf8a 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3502,6 +3502,16 @@ 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 (!vdev->use_config_notifier) {
+ return;
+ }
+ if (event_notifier_test_and_clear(n)) {
+ virtio_notify_config(vdev);
+ }
+}
void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
bool with_irqfd)
{
@@ -3518,6 +3528,17 @@ void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
}
}
+void virtio_set_config_notifier_fd_handler(VirtIODevice *vdev, bool assign,
+ bool with_irqfd)
+{
+ if (assign && !with_irqfd) {
+ event_notifier_set_handler(&vdev->config_notifier,
+ virtio_config_read);
+ } else {
+ event_notifier_set_handler(&vdev->config_notifier, NULL);
+ }
+}
+
EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq)
{
return &vq->guest_notifier;
@@ -3591,6 +3612,11 @@ 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/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index ef8abe49c5..97e13ec484 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -93,6 +93,8 @@ struct VirtioBusClass {
*/
bool has_variable_vring_alignment;
AddressSpace *(*get_dma_as)(DeviceState *d);
+ int (*set_config_notifiers)(DeviceState *d, bool assign);
+
};
struct VirtioBusState {
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index b7ece7a6a8..79f2f78625 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -108,6 +108,8 @@ struct VirtIODevice
bool use_guest_notifier_mask;
AddressSpace *dma_as;
QLIST_HEAD(, VirtQueue) *vector_queues;
+ EventNotifier config_notifier;
+ bool use_config_notifier;
};
struct VirtioDeviceClass {
@@ -310,11 +312,15 @@ 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,
bool with_irqfd);
+void virtio_set_config_notifier_fd_handler(VirtIODevice *vdev, bool assign,
+ bool with_irqfd);
+
int virtio_device_start_ioeventfd(VirtIODevice *vdev);
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,
--
2.21.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v3 1/5] virtio: add support for configure interrupt
2021-01-26 7:42 ` [PATCH v3 1/5] virtio: " Cindy Lu
@ 2021-01-27 5:35 ` Jason Wang
0 siblings, 0 replies; 16+ messages in thread
From: Jason Wang @ 2021-01-27 5:35 UTC (permalink / raw)
To: Cindy Lu, mst, qemu-devel
On 2021/1/26 下午3:42, Cindy Lu wrote:
> Add configure notifier and virtio_set_config_notifier_fd_handler
> in virtio
The title and commit log is kind of misleading.
I think we should use "virtio: add guest notifier for config interrupt"
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
> hw/virtio/virtio.c | 26 ++++++++++++++++++++++++++
> include/hw/virtio/virtio-bus.h | 2 ++
> include/hw/virtio/virtio.h | 6 ++++++
> 3 files changed, 34 insertions(+)
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index ceb58fda6c..25e164cf8a 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -3502,6 +3502,16 @@ 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 (!vdev->use_config_notifier) {
> + return;
> + }
> + if (event_notifier_test_and_clear(n)) {
> + virtio_notify_config(vdev);
> + }
> +}
> void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
> bool with_irqfd)
> {
> @@ -3518,6 +3528,17 @@ void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
> }
> }
>
> +void virtio_set_config_notifier_fd_handler(VirtIODevice *vdev, bool assign,
> + bool with_irqfd)
> +{
> + if (assign && !with_irqfd) {
> + event_notifier_set_handler(&vdev->config_notifier,
> + virtio_config_read);
> + } else {
> + event_notifier_set_handler(&vdev->config_notifier, NULL);
> + }
> +}
> +
> EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq)
> {
> return &vq->guest_notifier;
> @@ -3591,6 +3612,11 @@ 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/virtio-bus.h b/include/hw/virtio/virtio-bus.h
> index ef8abe49c5..97e13ec484 100644
> --- a/include/hw/virtio/virtio-bus.h
> +++ b/include/hw/virtio/virtio-bus.h
> @@ -93,6 +93,8 @@ struct VirtioBusClass {
> */
> bool has_variable_vring_alignment;
> AddressSpace *(*get_dma_as)(DeviceState *d);
> + int (*set_config_notifiers)(DeviceState *d, bool assign);
> +
> };
>
> struct VirtioBusState {
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index b7ece7a6a8..79f2f78625 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -108,6 +108,8 @@ struct VirtIODevice
> bool use_guest_notifier_mask;
> AddressSpace *dma_as;
> QLIST_HEAD(, VirtQueue) *vector_queues;
> + EventNotifier config_notifier;
> + bool use_config_notifier;
Let's document those two fields.
Thanks
> };
>
> struct VirtioDeviceClass {
> @@ -310,11 +312,15 @@ 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,
> bool with_irqfd);
> +void virtio_set_config_notifier_fd_handler(VirtIODevice *vdev, bool assign,
> + bool with_irqfd);
> +
> int virtio_device_start_ioeventfd(VirtIODevice *vdev);
> 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,
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 2/5] vhost_net: enable configure interrupt when vhost_net start
2021-01-26 7:42 [PATCH v3 0/5] vhost-vdpa: add support for configure interrupt Cindy Lu
2021-01-26 7:42 ` [PATCH v3 1/5] virtio: " Cindy Lu
@ 2021-01-26 7:42 ` Cindy Lu
2021-01-27 5:38 ` Jason Wang
2021-01-26 7:42 ` [PATCH v3 3/5] virtio-pci: add support for configure interrupt Cindy Lu
` (2 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Cindy Lu @ 2021-01-26 7:42 UTC (permalink / raw)
To: mst, jasowang, qemu-devel; +Cc: lulu
While peer is vhost vdpa, setup the configure interrupt function
vhost_net_start and release the resource when vhost_net_stop
Signed-off-by: Cindy Lu <lulu@redhat.com>
---
hw/net/vhost_net.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 24d555e764..0660da474a 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -345,6 +345,15 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
error_report("Error binding guest notifier: %d", -r);
goto err;
}
+ if (ncs->peer && ncs->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
+ if (k->set_config_notifiers) {
+ r = k->set_config_notifiers(qbus->parent, true);
+ if (r < 0) {
+ error_report("Error binding config notifier: %d", -r);
+ goto err;
+ }
+ }
+ }
for (i = 0; i < total_queues; i++) {
peer = qemu_get_peer(ncs, i);
@@ -391,7 +400,15 @@ void vhost_net_stop(VirtIODevice *dev, NetClientState *ncs,
for (i = 0; i < total_queues; i++) {
vhost_net_stop_one(get_vhost_net(ncs[i].peer), dev);
}
-
+ if (ncs->peer && ncs->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
+ if (k->set_config_notifiers) {
+ r = k->set_config_notifiers(qbus->parent, false);
+ if (r < 0) {
+ error_report("Error unbinding config notifier: %d", -r);
+ }
+ assert(r >= 0);
+ }
+ }
r = k->set_guest_notifiers(qbus->parent, total_queues * 2, false);
if (r < 0) {
fprintf(stderr, "vhost guest notifier cleanup failed: %d\n", r);
--
2.21.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v3 2/5] vhost_net: enable configure interrupt when vhost_net start
2021-01-26 7:42 ` [PATCH v3 2/5] vhost_net: enable configure interrupt when vhost_net start Cindy Lu
@ 2021-01-27 5:38 ` Jason Wang
2021-01-27 7:25 ` Cindy Lu
0 siblings, 1 reply; 16+ messages in thread
From: Jason Wang @ 2021-01-27 5:38 UTC (permalink / raw)
To: Cindy Lu, mst, qemu-devel
On 2021/1/26 下午3:42, Cindy Lu wrote:
> While peer is vhost vdpa, setup the configure interrupt function
> vhost_net_start and release the resource when vhost_net_stop
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
> hw/net/vhost_net.c | 19 ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index 24d555e764..0660da474a 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -345,6 +345,15 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
> error_report("Error binding guest notifier: %d", -r);
> goto err;
> }
> + if (ncs->peer && ncs->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> + if (k->set_config_notifiers) {
> + r = k->set_config_notifiers(qbus->parent, true);
> + if (r < 0) {
> + error_report("Error binding config notifier: %d", -r);
> + goto err;
> + }
> + }
> + }
>
> for (i = 0; i < total_queues; i++) {
> peer = qemu_get_peer(ncs, i);
> @@ -391,7 +400,15 @@ void vhost_net_stop(VirtIODevice *dev, NetClientState *ncs,
> for (i = 0; i < total_queues; i++) {
> vhost_net_stop_one(get_vhost_net(ncs[i].peer), dev);
> }
> -
> + if (ncs->peer && ncs->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> + if (k->set_config_notifiers) {
It looks to me that checking k->set_config_notifier is sufficient here.
Thanks
> + r = k->set_config_notifiers(qbus->parent, false);
> + if (r < 0) {
> + error_report("Error unbinding config notifier: %d", -r);
> + }
> + assert(r >= 0);
> + }
> + }
> r = k->set_guest_notifiers(qbus->parent, total_queues * 2, false);
> if (r < 0) {
> fprintf(stderr, "vhost guest notifier cleanup failed: %d\n", r);
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3 2/5] vhost_net: enable configure interrupt when vhost_net start
2021-01-27 5:38 ` Jason Wang
@ 2021-01-27 7:25 ` Cindy Lu
0 siblings, 0 replies; 16+ messages in thread
From: Cindy Lu @ 2021-01-27 7:25 UTC (permalink / raw)
To: Jason Wang; +Cc: QEMU Developers, Michael Tsirkin
On Wed, Jan 27, 2021 at 1:38 PM Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 2021/1/26 下午3:42, Cindy Lu wrote:
> > While peer is vhost vdpa, setup the configure interrupt function
> > vhost_net_start and release the resource when vhost_net_stop
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > ---
> > hw/net/vhost_net.c | 19 ++++++++++++++++++-
> > 1 file changed, 18 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > index 24d555e764..0660da474a 100644
> > --- a/hw/net/vhost_net.c
> > +++ b/hw/net/vhost_net.c
> > @@ -345,6 +345,15 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
> > error_report("Error binding guest notifier: %d", -r);
> > goto err;
> > }
> > + if (ncs->peer && ncs->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> > + if (k->set_config_notifiers) {
> > + r = k->set_config_notifiers(qbus->parent, true);
> > + if (r < 0) {
> > + error_report("Error binding config notifier: %d", -r);
> > + goto err;
> > + }
> > + }
> > + }
> >
> > for (i = 0; i < total_queues; i++) {
> > peer = qemu_get_peer(ncs, i);
> > @@ -391,7 +400,15 @@ void vhost_net_stop(VirtIODevice *dev, NetClientState *ncs,
> > for (i = 0; i < total_queues; i++) {
> > vhost_net_stop_one(get_vhost_net(ncs[i].peer), dev);
> > }
> > -
> > + if (ncs->peer && ncs->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> > + if (k->set_config_notifiers) {
>
>
> It looks to me that checking k->set_config_notifier is sufficient here.
>
> Thanks
>
>
sure will fix this
> > + r = k->set_config_notifiers(qbus->parent, false);
> > + if (r < 0) {
> > + error_report("Error unbinding config notifier: %d", -r);
> > + }
> > + assert(r >= 0);
> > + }
> > + }
> > r = k->set_guest_notifiers(qbus->parent, total_queues * 2, false);
> > if (r < 0) {
> > fprintf(stderr, "vhost guest notifier cleanup failed: %d\n", r);
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 3/5] virtio-pci: add support for configure interrupt
2021-01-26 7:42 [PATCH v3 0/5] vhost-vdpa: add support for configure interrupt Cindy Lu
2021-01-26 7:42 ` [PATCH v3 1/5] virtio: " Cindy Lu
2021-01-26 7:42 ` [PATCH v3 2/5] vhost_net: enable configure interrupt when vhost_net start Cindy Lu
@ 2021-01-26 7:42 ` Cindy Lu
2021-01-27 5:44 ` Jason Wang
2021-01-26 7:42 ` [PATCH v3 4/5] virtio-mmio: " Cindy Lu
2021-01-26 7:42 ` [PATCH v3 5/5] vhost-vdpa: add callback function " Cindy Lu
4 siblings, 1 reply; 16+ messages in thread
From: Cindy Lu @ 2021-01-26 7:42 UTC (permalink / raw)
To: mst, jasowang, qemu-devel; +Cc: lulu
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 | 92 ++++++++++++++++++++++++++++++++++--------
1 file changed, 75 insertions(+), 17 deletions(-)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 36524a5728..8e192600b8 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -664,7 +664,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];
@@ -691,23 +690,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;
@@ -722,7 +715,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;
@@ -731,7 +725,7 @@ static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
if (vector >= msix_nr_vectors_allocated(dev)) {
continue;
}
- ret = kvm_virtio_pci_vq_vector_use(proxy, queue_no, vector);
+ ret = kvm_virtio_pci_vq_vector_use(proxy, vector);
if (ret < 0) {
goto undo;
}
@@ -739,7 +733,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;
@@ -755,13 +751,69 @@ 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);
}
return ret;
}
+static int kvm_virtio_pci_vector_config_use(VirtIOPCIProxy *proxy)
+{
+
+ VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+ unsigned int vector;
+ int ret;
+ EventNotifier *n = virtio_get_config_notifier(vdev);
+
+ vector = vdev->config_vector ;
+ 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:
+ kvm_virtio_pci_irqfd_release(proxy, n, vector);
+ return ret;
+}
+static void kvm_virtio_pci_vector_config_release(VirtIOPCIProxy *proxy)
+{
+ PCIDevice *dev = &proxy->pci_dev;
+ VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+ unsigned int vector;
+ EventNotifier *n = virtio_get_config_notifier(vdev);
+ vector = vdev->config_vector ;
+ if (vector >= msix_nr_vectors_allocated(dev)) {
+ return;
+ }
+ kvm_virtio_pci_irqfd_release(proxy, n, vector);
+ kvm_virtio_pci_vq_vector_release(proxy, vector);
+}
+
+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_config_notifier_fd_handler(vdev, true, true);
+ kvm_virtio_pci_vector_config_use(proxy);
+ } else {
+ virtio_set_config_notifier_fd_handler(vdev, false, true);
+ kvm_virtio_pci_vector_config_release(proxy);
+ event_notifier_cleanup(notifier);
+ }
+ return r;
+}
+
static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
{
PCIDevice *dev = &proxy->pci_dev;
@@ -769,7 +821,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;
@@ -782,7 +835,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);
}
@@ -823,7 +878,7 @@ 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;
}
@@ -835,13 +890,15 @@ static void virtio_pci_vq_vector_mask(VirtIOPCIProxy *proxy,
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);
/* If guest supports masking, keep irqfd but mask it.
* Otherwise, clean it up now.
*/
if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
k->guest_notifier_mask(vdev, queue_no, true);
} else {
- kvm_virtio_pci_irqfd_release(proxy, queue_no, vector);
+ kvm_virtio_pci_irqfd_release(proxy, n, vector);
}
}
@@ -2137,6 +2194,7 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
k->ioeventfd_assign = virtio_pci_ioeventfd_assign;
k->get_dma_as = virtio_pci_get_dma_as;
k->queue_enabled = virtio_pci_queue_enabled;
+ k->set_config_notifiers = virtio_pci_set_config_notifier;
}
static const TypeInfo virtio_pci_bus_info = {
--
2.21.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v3 3/5] virtio-pci: add support for configure interrupt
2021-01-26 7:42 ` [PATCH v3 3/5] virtio-pci: add support for configure interrupt Cindy Lu
@ 2021-01-27 5:44 ` Jason Wang
2021-01-27 7:39 ` Cindy Lu
0 siblings, 1 reply; 16+ messages in thread
From: Jason Wang @ 2021-01-27 5:44 UTC (permalink / raw)
To: Cindy Lu, mst, qemu-devel
On 2021/1/26 下午3:42, Cindy Lu wrote:
> 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 | 92 ++++++++++++++++++++++++++++++++++--------
> 1 file changed, 75 insertions(+), 17 deletions(-)
>
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 36524a5728..8e192600b8 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -664,7 +664,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];
> @@ -691,23 +690,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;
>
> @@ -722,7 +715,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;
new line is needed.
> for (queue_no = 0; queue_no < nvqs; queue_no++) {
> if (!virtio_queue_get_num(vdev, queue_no)) {
> break;
> @@ -731,7 +725,7 @@ static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
> if (vector >= msix_nr_vectors_allocated(dev)) {
> continue;
> }
> - ret = kvm_virtio_pci_vq_vector_use(proxy, queue_no, vector);
> + ret = kvm_virtio_pci_vq_vector_use(proxy, vector);
> if (ret < 0) {
> goto undo;
> }
> @@ -739,7 +733,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;
> @@ -755,13 +751,69 @@ 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);
> }
> return ret;
> }
>
> +static int kvm_virtio_pci_vector_config_use(VirtIOPCIProxy *proxy)
> +{
> +
> + VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> + unsigned int vector;
> + int ret;
> + EventNotifier *n = virtio_get_config_notifier(vdev);
> +
> + vector = vdev->config_vector ;
> + 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:
> + kvm_virtio_pci_irqfd_release(proxy, n, vector);
> + return ret;
> +}
newline is needed.
> +static void kvm_virtio_pci_vector_config_release(VirtIOPCIProxy *proxy)
> +{
> + PCIDevice *dev = &proxy->pci_dev;
> + VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> + unsigned int vector;
> + EventNotifier *n = virtio_get_config_notifier(vdev);
> + vector = vdev->config_vector ;
> + if (vector >= msix_nr_vectors_allocated(dev)) {
> + return;
> + }
> + kvm_virtio_pci_irqfd_release(proxy, n, vector);
> + kvm_virtio_pci_vq_vector_release(proxy, vector);
> +}
> +
> +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_config_notifier_fd_handler(vdev, true, true);
> + kvm_virtio_pci_vector_config_use(proxy);
> + } else {
> + virtio_set_config_notifier_fd_handler(vdev, false, true);
> + kvm_virtio_pci_vector_config_release(proxy);
> + event_notifier_cleanup(notifier);
> + }
> + return r;
> +}
Two questions, don't we need to check whether or not MSIX is enalbed in
this case? Instead of introducing new helpers that is easy to be buggy I
still prefer to re-use virtio_pci_set_guest_notifier:
We can switch to use accepts EventNotifier instead of virtqueue index.
And we need also convert virtqueue_set_guest_notifier_fd_handler() to
accept EventNotifier.
> +
> static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
> {
> PCIDevice *dev = &proxy->pci_dev;
> @@ -769,7 +821,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;
newline is needed.
Thanks
> for (queue_no = 0; queue_no < nvqs; queue_no++) {
> if (!virtio_queue_get_num(vdev, queue_no)) {
> break;
> @@ -782,7 +835,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);
> }
> @@ -823,7 +878,7 @@ 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;
> }
> @@ -835,13 +890,15 @@ static void virtio_pci_vq_vector_mask(VirtIOPCIProxy *proxy,
> 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);
> /* If guest supports masking, keep irqfd but mask it.
> * Otherwise, clean it up now.
> */
> if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
> k->guest_notifier_mask(vdev, queue_no, true);
> } else {
> - kvm_virtio_pci_irqfd_release(proxy, queue_no, vector);
> + kvm_virtio_pci_irqfd_release(proxy, n, vector);
> }
> }
>
> @@ -2137,6 +2194,7 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
> k->ioeventfd_assign = virtio_pci_ioeventfd_assign;
> k->get_dma_as = virtio_pci_get_dma_as;
> k->queue_enabled = virtio_pci_queue_enabled;
> + k->set_config_notifiers = virtio_pci_set_config_notifier;
> }
>
> static const TypeInfo virtio_pci_bus_info = {
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3 3/5] virtio-pci: add support for configure interrupt
2021-01-27 5:44 ` Jason Wang
@ 2021-01-27 7:39 ` Cindy Lu
0 siblings, 0 replies; 16+ messages in thread
From: Cindy Lu @ 2021-01-27 7:39 UTC (permalink / raw)
To: Jason Wang; +Cc: QEMU Developers, Michael Tsirkin
On Wed, Jan 27, 2021 at 1:44 PM Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 2021/1/26 下午3:42, Cindy Lu wrote:
> > 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 | 92 ++++++++++++++++++++++++++++++++++--------
> > 1 file changed, 75 insertions(+), 17 deletions(-)
> >
> > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > index 36524a5728..8e192600b8 100644
> > --- a/hw/virtio/virtio-pci.c
> > +++ b/hw/virtio/virtio-pci.c
> > @@ -664,7 +664,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];
> > @@ -691,23 +690,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;
> >
> > @@ -722,7 +715,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;
>
>
> new line is needed.
>
will fix this
>
> > for (queue_no = 0; queue_no < nvqs; queue_no++) {
> > if (!virtio_queue_get_num(vdev, queue_no)) {
> > break;
> > @@ -731,7 +725,7 @@ static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
> > if (vector >= msix_nr_vectors_allocated(dev)) {
> > continue;
> > }
> > - ret = kvm_virtio_pci_vq_vector_use(proxy, queue_no, vector);
> > + ret = kvm_virtio_pci_vq_vector_use(proxy, vector);
> > if (ret < 0) {
> > goto undo;
> > }
> > @@ -739,7 +733,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;
> > @@ -755,13 +751,69 @@ 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);
> > }
> > return ret;
> > }
> >
> > +static int kvm_virtio_pci_vector_config_use(VirtIOPCIProxy *proxy)
> > +{
> > +
> > + VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> > + unsigned int vector;
> > + int ret;
> > + EventNotifier *n = virtio_get_config_notifier(vdev);
> > +
> > + vector = vdev->config_vector ;
> > + 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:
> > + kvm_virtio_pci_irqfd_release(proxy, n, vector);
> > + return ret;
> > +}
>
>
> newline is needed.
>
will fix this
>
> > +static void kvm_virtio_pci_vector_config_release(VirtIOPCIProxy *proxy)
> > +{
> > + PCIDevice *dev = &proxy->pci_dev;
> > + VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> > + unsigned int vector;
> > + EventNotifier *n = virtio_get_config_notifier(vdev);
> > + vector = vdev->config_vector ;
> > + if (vector >= msix_nr_vectors_allocated(dev)) {
> > + return;
> > + }
> > + kvm_virtio_pci_irqfd_release(proxy, n, vector);
> > + kvm_virtio_pci_vq_vector_release(proxy, vector);
> > +}
> > +
> > +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_config_notifier_fd_handler(vdev, true, true);
> > + kvm_virtio_pci_vector_config_use(proxy);
> > + } else {
> > + virtio_set_config_notifier_fd_handler(vdev, false, true);
> > + kvm_virtio_pci_vector_config_release(proxy);
> > + event_notifier_cleanup(notifier);
> > + }
> > + return r;
> > +}
>
>
> Two questions, don't we need to check whether or not MSIX is enalbed in
> this case? Instead of introducing new helpers that is easy to be buggy I
> still prefer to re-use virtio_pci_set_guest_notifier:
>
> We can switch to use accepts EventNotifier instead of virtqueue index.
> And we need also convert virtqueue_set_guest_notifier_fd_handler() to
> accept EventNotifier.
>
sure, I will fix this
>
> > +
> > static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
> > {
> > PCIDevice *dev = &proxy->pci_dev;
> > @@ -769,7 +821,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;
>
>
> newline is needed.
>
> Thanks
>
>
> > for (queue_no = 0; queue_no < nvqs; queue_no++) {
> > if (!virtio_queue_get_num(vdev, queue_no)) {
> > break;
> > @@ -782,7 +835,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);
> > }
> > @@ -823,7 +878,7 @@ 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;
> > }
> > @@ -835,13 +890,15 @@ static void virtio_pci_vq_vector_mask(VirtIOPCIProxy *proxy,
> > 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);
> > /* If guest supports masking, keep irqfd but mask it.
> > * Otherwise, clean it up now.
> > */
> > if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
> > k->guest_notifier_mask(vdev, queue_no, true);
> > } else {
> > - kvm_virtio_pci_irqfd_release(proxy, queue_no, vector);
> > + kvm_virtio_pci_irqfd_release(proxy, n, vector);
> > }
> > }
> >
> > @@ -2137,6 +2194,7 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
> > k->ioeventfd_assign = virtio_pci_ioeventfd_assign;
> > k->get_dma_as = virtio_pci_get_dma_as;
> > k->queue_enabled = virtio_pci_queue_enabled;
> > + k->set_config_notifiers = virtio_pci_set_config_notifier;
> > }
> >
> > static const TypeInfo virtio_pci_bus_info = {
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 4/5] virtio-mmio: add support for configure interrupt
2021-01-26 7:42 [PATCH v3 0/5] vhost-vdpa: add support for configure interrupt Cindy Lu
` (2 preceding siblings ...)
2021-01-26 7:42 ` [PATCH v3 3/5] virtio-pci: add support for configure interrupt Cindy Lu
@ 2021-01-26 7:42 ` Cindy Lu
2021-01-26 7:42 ` [PATCH v3 5/5] vhost-vdpa: add callback function " Cindy Lu
4 siblings, 0 replies; 16+ messages in thread
From: Cindy Lu @ 2021-01-26 7:42 UTC (permalink / raw)
To: mst, jasowang, qemu-devel; +Cc: lulu
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 | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index e1b5c3b81e..d580f1ea9a 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -666,6 +666,21 @@ assign_error:
return r;
}
+static int virtio_mmio_set_config_notifier(DeviceState *d, bool assign)
+{
+ VirtIOMMIOProxy *proxy = VIRTIO_MMIO(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_config_notifier_fd_handler(vdev, true, false);
+ } else {
+ virtio_set_config_notifier_fd_handler(vdev, false, false);
+ event_notifier_cleanup(notifier);
+ }
+ return r;
+}
static void virtio_mmio_pre_plugged(DeviceState *d, Error **errp)
{
VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
@@ -780,6 +795,7 @@ static void virtio_mmio_bus_class_init(ObjectClass *klass, void *data)
k->ioeventfd_assign = virtio_mmio_ioeventfd_assign;
k->pre_plugged = virtio_mmio_pre_plugged;
k->has_variable_vring_alignment = true;
+ k->set_config_notifiers = virtio_mmio_set_config_notifier;
bus_class->max_dev = 1;
bus_class->get_dev_path = virtio_mmio_bus_get_dev_path;
}
--
2.21.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 5/5] vhost-vdpa: add callback function for configure interrupt
2021-01-26 7:42 [PATCH v3 0/5] vhost-vdpa: add support for configure interrupt Cindy Lu
` (3 preceding siblings ...)
2021-01-26 7:42 ` [PATCH v3 4/5] virtio-mmio: " Cindy Lu
@ 2021-01-26 7:42 ` Cindy Lu
2021-01-26 14:53 ` Stefano Garzarella
2021-01-27 5:47 ` Jason Wang
4 siblings, 2 replies; 16+ messages in thread
From: Cindy Lu @ 2021-01-26 7:42 UTC (permalink / raw)
To: mst, jasowang, qemu-devel; +Cc: lulu
Add call back function for configure interrupt.
Set the notifier's fd to the kernel driver when vdpa start.
also set -1 while vdpa stop. then the kernel will release
the related resource
Signed-off-by: Cindy Lu <lulu@redhat.com>
---
hw/virtio/trace-events | 2 ++
hw/virtio/vhost-vdpa.c | 37 ++++++++++++++++++++++++++++++-
include/hw/virtio/vhost-backend.h | 4 ++++
3 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 2060a144a2..6710835b46 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..cc1d39d663 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -467,20 +467,47 @@ static int vhost_vdpa_get_config(struct vhost_dev *dev, uint8_t *config,
}
return ret;
}
+static void vhost_vdpa_config_notify_start(struct vhost_dev *dev,
+ struct VirtIODevice *vdev, bool start)
+{
+ int fd, r;
+ if (start) {
+ fd = event_notifier_get_fd(&vdev->config_notifier);
+ vdev->use_config_notifier = true;
+ } else {
+ fd = -1;
+ vdev->use_config_notifier = false;
+ }
+ /*set the fd call back to vdpa driver*/
+ r = dev->vhost_ops->vhost_set_config_call(dev, &fd);
+ if (r) {
+ vdev->use_config_notifier = false;
+ info_report("vhost_vdpa_config_notify not started!");
+ }
+ /*active the config_notifier when vdev->use_config_notifier is true*/
+ if ((vdev->use_config_notifier) && (start)) {
+ event_notifier_set(&vdev->config_notifier);
+ }
+ return;
+}
static int vhost_vdpa_dev_start(struct vhost_dev *dev, bool started)
{
struct vhost_vdpa *v = dev->opaque;
trace_vhost_vdpa_dev_start(dev, started);
+ VirtIODevice *vdev = dev->vdev;
+
if (started) {
uint8_t status = 0;
memory_listener_register(&v->listener, &address_space_memory);
vhost_vdpa_set_vring_ready(dev);
vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
vhost_vdpa_call(dev, VHOST_VDPA_GET_STATUS, &status);
-
+ /*set the configure interrupt call back*/
+ vhost_vdpa_config_notify_start(dev, vdev, true);
return !(status & VIRTIO_CONFIG_S_DRIVER_OK);
} else {
+ vhost_vdpa_config_notify_start(dev, vdev, false);
vhost_vdpa_reset_device(dev);
vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE |
VIRTIO_CONFIG_S_DRIVER);
@@ -546,6 +573,13 @@ static int vhost_vdpa_set_vring_call(struct vhost_dev *dev,
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 +645,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,
};
diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
index 8a6f8e2a7a..1a2fee8994 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -125,6 +125,9 @@ 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 +173,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] 16+ messages in thread* Re: [PATCH v3 5/5] vhost-vdpa: add callback function for configure interrupt
2021-01-26 7:42 ` [PATCH v3 5/5] vhost-vdpa: add callback function " Cindy Lu
@ 2021-01-26 14:53 ` Stefano Garzarella
2021-01-27 2:51 ` Cindy Lu
2021-01-27 5:47 ` Jason Wang
1 sibling, 1 reply; 16+ messages in thread
From: Stefano Garzarella @ 2021-01-26 14:53 UTC (permalink / raw)
To: Cindy Lu; +Cc: jasowang, qemu-devel, mst
On Tue, Jan 26, 2021 at 03:42:54PM +0800, Cindy Lu wrote:
>Add call back function for configure interrupt.
>Set the notifier's fd to the kernel driver when vdpa start.
>also set -1 while vdpa stop. then the kernel will release
>the related resource
>
>Signed-off-by: Cindy Lu <lulu@redhat.com>
>---
> hw/virtio/trace-events | 2 ++
> hw/virtio/vhost-vdpa.c | 37 ++++++++++++++++++++++++++++++-
> include/hw/virtio/vhost-backend.h | 4 ++++
> 3 files changed, 42 insertions(+), 1 deletion(-)
>
>diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
>index 2060a144a2..6710835b46 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..cc1d39d663 100644
>--- a/hw/virtio/vhost-vdpa.c
>+++ b/hw/virtio/vhost-vdpa.c
>@@ -467,20 +467,47 @@ static int vhost_vdpa_get_config(struct vhost_dev *dev, uint8_t *config,
> }
> return ret;
> }
>+static void vhost_vdpa_config_notify_start(struct vhost_dev *dev,
>+ struct VirtIODevice *vdev, bool start)
>+{
>+ int fd, r;
>+ if (start) {
>+ fd = event_notifier_get_fd(&vdev->config_notifier);
>+ vdev->use_config_notifier = true;
>+ } else {
^
>+ fd = -1;
>+ vdev->use_config_notifier = false;
>+ }
^
>+ /*set the fd call back to vdpa driver*/
^
It seems to me that there is an extra space in these places.
>+ r = dev->vhost_ops->vhost_set_config_call(dev, &fd);
>+ if (r) {
>+ vdev->use_config_notifier = false;
>+ info_report("vhost_vdpa_config_notify not started!");
>+ }
>+ /*active the config_notifier when vdev->use_config_notifier is true*/
>+ if ((vdev->use_config_notifier) && (start)) {
>+ event_notifier_set(&vdev->config_notifier);
>+ }
>+ return;
>
>+}
Here and also in other patches I would leave a new line between two
functions.
> static int vhost_vdpa_dev_start(struct vhost_dev *dev, bool started)
> {
> struct vhost_vdpa *v = dev->opaque;
> trace_vhost_vdpa_dev_start(dev, started);
>+ VirtIODevice *vdev = dev->vdev;
>+
> if (started) {
> uint8_t status = 0;
> memory_listener_register(&v->listener, &address_space_memory);
> vhost_vdpa_set_vring_ready(dev);
> vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
> vhost_vdpa_call(dev, VHOST_VDPA_GET_STATUS, &status);
>-
>+ /*set the configure interrupt call back*/
>+ vhost_vdpa_config_notify_start(dev, vdev, true);
> return !(status & VIRTIO_CONFIG_S_DRIVER_OK);
> } else {
>+ vhost_vdpa_config_notify_start(dev, vdev, false);
> vhost_vdpa_reset_device(dev);
> vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE |
> VIRTIO_CONFIG_S_DRIVER);
>@@ -546,6 +573,13 @@ static int vhost_vdpa_set_vring_call(struct vhost_dev *dev,
> 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 +645,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,
> };
>diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
>index 8a6f8e2a7a..1a2fee8994 100644
>--- a/include/hw/virtio/vhost-backend.h
>+++ b/include/hw/virtio/vhost-backend.h
>@@ -125,6 +125,9 @@ 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);
^
One more space :-)
Thanks,
Stefano
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3 5/5] vhost-vdpa: add callback function for configure interrupt
2021-01-26 14:53 ` Stefano Garzarella
@ 2021-01-27 2:51 ` Cindy Lu
0 siblings, 0 replies; 16+ messages in thread
From: Cindy Lu @ 2021-01-27 2:51 UTC (permalink / raw)
To: Stefano Garzarella; +Cc: Jason Wang, QEMU Developers, Michael Tsirkin
On Tue, Jan 26, 2021 at 10:53 PM Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> On Tue, Jan 26, 2021 at 03:42:54PM +0800, Cindy Lu wrote:
> >Add call back function for configure interrupt.
> >Set the notifier's fd to the kernel driver when vdpa start.
> >also set -1 while vdpa stop. then the kernel will release
> >the related resource
> >
> >Signed-off-by: Cindy Lu <lulu@redhat.com>
> >---
> > hw/virtio/trace-events | 2 ++
> > hw/virtio/vhost-vdpa.c | 37 ++++++++++++++++++++++++++++++-
> > include/hw/virtio/vhost-backend.h | 4 ++++
> > 3 files changed, 42 insertions(+), 1 deletion(-)
> >
> >diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> >index 2060a144a2..6710835b46 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..cc1d39d663 100644
> >--- a/hw/virtio/vhost-vdpa.c
> >+++ b/hw/virtio/vhost-vdpa.c
> >@@ -467,20 +467,47 @@ static int vhost_vdpa_get_config(struct vhost_dev *dev, uint8_t *config,
> > }
> > return ret;
> > }
> >+static void vhost_vdpa_config_notify_start(struct vhost_dev *dev,
> >+ struct VirtIODevice *vdev, bool start)
> >+{
> >+ int fd, r;
> >+ if (start) {
> >+ fd = event_notifier_get_fd(&vdev->config_notifier);
> >+ vdev->use_config_notifier = true;
> >+ } else {
> ^
> >+ fd = -1;
> >+ vdev->use_config_notifier = false;
> >+ }
> ^
> >+ /*set the fd call back to vdpa driver*/
> ^
> It seems to me that there is an extra space in these places.
>
sure, I will fix this
> >+ r = dev->vhost_ops->vhost_set_config_call(dev, &fd);
> >+ if (r) {
> >+ vdev->use_config_notifier = false;
> >+ info_report("vhost_vdpa_config_notify not started!");
> >+ }
> >+ /*active the config_notifier when vdev->use_config_notifier is true*/
> >+ if ((vdev->use_config_notifier) && (start)) {
> >+ event_notifier_set(&vdev->config_notifier);
> >+ }
> >+ return;
> >
> >+}
>
> Here and also in other patches I would leave a new line between two
> functions.
>
sure. will fix this
> > static int vhost_vdpa_dev_start(struct vhost_dev *dev, bool started)
> > {
> > struct vhost_vdpa *v = dev->opaque;
> > trace_vhost_vdpa_dev_start(dev, started);
> >+ VirtIODevice *vdev = dev->vdev;
> >+
> > if (started) {
> > uint8_t status = 0;
> > memory_listener_register(&v->listener, &address_space_memory);
> > vhost_vdpa_set_vring_ready(dev);
> > vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
> > vhost_vdpa_call(dev, VHOST_VDPA_GET_STATUS, &status);
> >-
> >+ /*set the configure interrupt call back*/
> >+ vhost_vdpa_config_notify_start(dev, vdev, true);
> > return !(status & VIRTIO_CONFIG_S_DRIVER_OK);
> > } else {
> >+ vhost_vdpa_config_notify_start(dev, vdev, false);
> > vhost_vdpa_reset_device(dev);
> > vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE |
> > VIRTIO_CONFIG_S_DRIVER);
> >@@ -546,6 +573,13 @@ static int vhost_vdpa_set_vring_call(struct vhost_dev *dev,
> > 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 +645,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,
> > };
> >diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
> >index 8a6f8e2a7a..1a2fee8994 100644
> >--- a/include/hw/virtio/vhost-backend.h
> >+++ b/include/hw/virtio/vhost-backend.h
> >@@ -125,6 +125,9 @@ 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);
> ^
> One more space :-)
>
Thanks Stefano , will fix this
> Thanks,
> Stefano
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 5/5] vhost-vdpa: add callback function for configure interrupt
2021-01-26 7:42 ` [PATCH v3 5/5] vhost-vdpa: add callback function " Cindy Lu
2021-01-26 14:53 ` Stefano Garzarella
@ 2021-01-27 5:47 ` Jason Wang
2021-01-27 7:51 ` Cindy Lu
1 sibling, 1 reply; 16+ messages in thread
From: Jason Wang @ 2021-01-27 5:47 UTC (permalink / raw)
To: Cindy Lu, mst, qemu-devel
On 2021/1/26 下午3:42, Cindy Lu wrote:
> Add call back function for configure interrupt.
> Set the notifier's fd to the kernel driver when vdpa start.
> also set -1 while vdpa stop. then the kernel will release
> the related resource
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
> hw/virtio/trace-events | 2 ++
> hw/virtio/vhost-vdpa.c | 37 ++++++++++++++++++++++++++++++-
> include/hw/virtio/vhost-backend.h | 4 ++++
> 3 files changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index 2060a144a2..6710835b46 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..cc1d39d663 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -467,20 +467,47 @@ static int vhost_vdpa_get_config(struct vhost_dev *dev, uint8_t *config,
> }
> return ret;
> }
> +static void vhost_vdpa_config_notify_start(struct vhost_dev *dev,
> + struct VirtIODevice *vdev, bool start)
> +{
> + int fd, r;
> + if (start) {
> + fd = event_notifier_get_fd(&vdev->config_notifier);
> + vdev->use_config_notifier = true;
> + } else {
> + fd = -1;
> + vdev->use_config_notifier = false;
> + }
> + /*set the fd call back to vdpa driver*/
I guess checkpatch.pl might warn here. Please try to silent checkpath.pl
before submitting patches.
> + r = dev->vhost_ops->vhost_set_config_call(dev, &fd);
> + if (r) {
> + vdev->use_config_notifier = false;
> + info_report("vhost_vdpa_config_notify not started!");
This looks kind of fragile. Do we need some workaround here like:
1) filter out the features that depends on config interrupt
or
2) A timer to watch the change of config space
Thanks
> + }
> + /*active the config_notifier when vdev->use_config_notifier is true*/
> + if ((vdev->use_config_notifier) && (start)) {
> + event_notifier_set(&vdev->config_notifier);
> + }
> + return;
>
> +}
> static int vhost_vdpa_dev_start(struct vhost_dev *dev, bool started)
> {
> struct vhost_vdpa *v = dev->opaque;
> trace_vhost_vdpa_dev_start(dev, started);
> + VirtIODevice *vdev = dev->vdev;
> +
> if (started) {
> uint8_t status = 0;
> memory_listener_register(&v->listener, &address_space_memory);
> vhost_vdpa_set_vring_ready(dev);
> vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
> vhost_vdpa_call(dev, VHOST_VDPA_GET_STATUS, &status);
> -
> + /*set the configure interrupt call back*/
> + vhost_vdpa_config_notify_start(dev, vdev, true);
> return !(status & VIRTIO_CONFIG_S_DRIVER_OK);
> } else {
> + vhost_vdpa_config_notify_start(dev, vdev, false);
> vhost_vdpa_reset_device(dev);
> vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE |
> VIRTIO_CONFIG_S_DRIVER);
> @@ -546,6 +573,13 @@ static int vhost_vdpa_set_vring_call(struct vhost_dev *dev,
> 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 +645,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,
> };
> diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
> index 8a6f8e2a7a..1a2fee8994 100644
> --- a/include/hw/virtio/vhost-backend.h
> +++ b/include/hw/virtio/vhost-backend.h
> @@ -125,6 +125,9 @@ 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 +173,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] 16+ messages in thread* Re: [PATCH v3 5/5] vhost-vdpa: add callback function for configure interrupt
2021-01-27 5:47 ` Jason Wang
@ 2021-01-27 7:51 ` Cindy Lu
2021-01-27 8:16 ` Jason Wang
0 siblings, 1 reply; 16+ messages in thread
From: Cindy Lu @ 2021-01-27 7:51 UTC (permalink / raw)
To: Jason Wang; +Cc: QEMU Developers, Michael Tsirkin
On Wed, Jan 27, 2021 at 1:47 PM Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 2021/1/26 下午3:42, Cindy Lu wrote:
> > Add call back function for configure interrupt.
> > Set the notifier's fd to the kernel driver when vdpa start.
> > also set -1 while vdpa stop. then the kernel will release
> > the related resource
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > ---
> > hw/virtio/trace-events | 2 ++
> > hw/virtio/vhost-vdpa.c | 37 ++++++++++++++++++++++++++++++-
> > include/hw/virtio/vhost-backend.h | 4 ++++
> > 3 files changed, 42 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> > index 2060a144a2..6710835b46 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..cc1d39d663 100644
> > --- a/hw/virtio/vhost-vdpa.c
> > +++ b/hw/virtio/vhost-vdpa.c
> > @@ -467,20 +467,47 @@ static int vhost_vdpa_get_config(struct vhost_dev *dev, uint8_t *config,
> > }
> > return ret;
> > }
> > +static void vhost_vdpa_config_notify_start(struct vhost_dev *dev,
> > + struct VirtIODevice *vdev, bool start)
> > +{
> > + int fd, r;
> > + if (start) {
> > + fd = event_notifier_get_fd(&vdev->config_notifier);
> > + vdev->use_config_notifier = true;
> > + } else {
> > + fd = -1;
> > + vdev->use_config_notifier = false;
> > + }
> > + /*set the fd call back to vdpa driver*/
>
>
> I guess checkpatch.pl might warn here. Please try to silent checkpath.pl
> before submitting patches.
>
Actually I do have run this script, but seems not warned here. I will
pay attention next time
>
> > + r = dev->vhost_ops->vhost_set_config_call(dev, &fd);
> > + if (r) {
> > + vdev->use_config_notifier = false;
> > + info_report("vhost_vdpa_config_notify not started!");
>
>
> This looks kind of fragile. Do we need some workaround here like:
>
> 1) filter out the features that depends on config interrupt
>
> or
>
> 2) A timer to watch the change of config space
>
> Thanks
>
sure I will add this check
>
> > + }
> > + /*active the config_notifier when vdev->use_config_notifier is true*/
> > + if ((vdev->use_config_notifier) && (start)) {
> > + event_notifier_set(&vdev->config_notifier);
> > + }
> > + return;
> >
> > +}
> > static int vhost_vdpa_dev_start(struct vhost_dev *dev, bool started)
> > {
> > struct vhost_vdpa *v = dev->opaque;
> > trace_vhost_vdpa_dev_start(dev, started);
> > + VirtIODevice *vdev = dev->vdev;
> > +
> > if (started) {
> > uint8_t status = 0;
> > memory_listener_register(&v->listener, &address_space_memory);
> > vhost_vdpa_set_vring_ready(dev);
> > vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
> > vhost_vdpa_call(dev, VHOST_VDPA_GET_STATUS, &status);
> > -
> > + /*set the configure interrupt call back*/
> > + vhost_vdpa_config_notify_start(dev, vdev, true);
> > return !(status & VIRTIO_CONFIG_S_DRIVER_OK);
> > } else {
> > + vhost_vdpa_config_notify_start(dev, vdev, false);
> > vhost_vdpa_reset_device(dev);
> > vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE |
> > VIRTIO_CONFIG_S_DRIVER);
> > @@ -546,6 +573,13 @@ static int vhost_vdpa_set_vring_call(struct vhost_dev *dev,
> > 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 +645,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,
> > };
> > diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
> > index 8a6f8e2a7a..1a2fee8994 100644
> > --- a/include/hw/virtio/vhost-backend.h
> > +++ b/include/hw/virtio/vhost-backend.h
> > @@ -125,6 +125,9 @@ 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 +173,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] 16+ messages in thread* Re: [PATCH v3 5/5] vhost-vdpa: add callback function for configure interrupt
2021-01-27 7:51 ` Cindy Lu
@ 2021-01-27 8:16 ` Jason Wang
0 siblings, 0 replies; 16+ messages in thread
From: Jason Wang @ 2021-01-27 8:16 UTC (permalink / raw)
To: Cindy Lu; +Cc: QEMU Developers, Michael Tsirkin
On 2021/1/27 下午3:51, Cindy Lu wrote:
>>> + /*set the fd call back to vdpa driver*/
>> I guess checkpatch.pl might warn here. Please try to silent checkpath.pl
>> before submitting patches.
>>
> Actually I do have run this script, but seems not warned here. I will
> pay attention next time
Oh right, I remember kernel checkpath did the check but I don't know if
the qemu one check the comment sytle.
Thanks
^ permalink raw reply [flat|nested] 16+ messages in thread