* [Qemu-devel] [PATCH v2 1/3] hw/vfio/platform: irqfd setup sequence update
2015-09-24 20:08 [Qemu-devel] [PATCH v2 0/3] hw/vfio/platform: irqfd setup changes Eric Auger
@ 2015-09-24 20:08 ` Eric Auger
2015-09-24 20:08 ` [Qemu-devel] [PATCH v2 2/3] hw/vfio/platform: change interrupt/unmask fields into pointer Eric Auger
2015-09-24 20:08 ` [Qemu-devel] [PATCH v2 3/3] hw/vfio/platform: do not set resamplefd for edge-sensitive IRQS Eric Auger
2 siblings, 0 replies; 4+ messages in thread
From: Eric Auger @ 2015-09-24 20:08 UTC (permalink / raw)
To: eric.auger, eric.auger, qemu-devel, alex.williamson
Cc: christoffer.dall, patches
With current implementation, eventfd VFIO signaling is first set up and
then irqfd is setup, if supported and allowed.
This start sequence causes several issues with IRQ forwarding setup
which, if supported, is transparently attempted on irqfd setup:
IRQ forwarding setup is likely to fail if the IRQ is detected as under
injection into the guest (active at irqchip level or VFIO masked).
This currently always happens because the current sequence explicitly
VFIO-masks the IRQ before setting irqfd.
Even if that masking were removed, we couldn't prevent the case where
the IRQ is under injection into the guest.
So the simpler solution is to remove this 2-step startup and directly
attempt irqfd setup. This is what this patch does.
Also in case the eventfd setup fails, there is no reason to go farther:
let's abort.
Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
hw/vfio/platform.c | 51 +++++++++++++++++++++++++++++----------------------
1 file changed, 29 insertions(+), 22 deletions(-)
diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index a6726cd..d864342 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -310,18 +310,29 @@ static void vfio_platform_eoi(VFIODevice *vbasedev)
/**
* vfio_start_eventfd_injection - starts the virtual IRQ injection using
* user-side handled eventfds
- * @intp: the IRQ struct pointer
+ * @sbdev: the sysbus device handle
+ * @irq: the qemu irq handle
*/
-static int vfio_start_eventfd_injection(VFIOINTp *intp)
+static void vfio_start_eventfd_injection(SysBusDevice *sbdev, qemu_irq irq)
{
int ret;
+ VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev);
+ VFIOINTp *intp;
+
+ QLIST_FOREACH(intp, &vdev->intp_list, next) {
+ if (intp->qemuirq == irq) {
+ break;
+ }
+ }
+ assert(intp);
ret = vfio_set_trigger_eventfd(intp, vfio_intp_interrupt);
if (ret) {
- error_report("vfio: Error: Failed to pass IRQ fd to the driver: %m");
+ error_report("vfio: failed to start eventfd signaling for IRQ %d: %m",
+ intp->pin);
+ abort();
}
- return ret;
}
/*
@@ -359,6 +370,15 @@ static int vfio_set_resample_eventfd(VFIOINTp *intp)
return ret;
}
+/**
+ * vfio_start_irqfd_injection - starts the virtual IRQ injection using
+ * irqfd
+ *
+ * @sbdev: the sysbus device handle
+ * @irq: the qemu irq handle
+ *
+ * In case the irqfd setup fails, we fallback to userspace handled eventfd
+ */
static void vfio_start_irqfd_injection(SysBusDevice *sbdev, qemu_irq irq)
{
VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev);
@@ -366,7 +386,7 @@ static void vfio_start_irqfd_injection(SysBusDevice *sbdev, qemu_irq irq)
if (!kvm_irqfds_enabled() || !kvm_resamplefds_enabled() ||
!vdev->irqfd_allowed) {
- return;
+ goto fail_irqfd;
}
QLIST_FOREACH(intp, &vdev->intp_list, next) {
@@ -376,13 +396,6 @@ static void vfio_start_irqfd_injection(SysBusDevice *sbdev, qemu_irq irq)
}
assert(intp);
- /* Get to a known interrupt state */
- qemu_set_fd_handler(event_notifier_get_fd(&intp->interrupt),
- NULL, NULL, vdev);
-
- vfio_mask_single_irqindex(&vdev->vbasedev, intp->pin);
- qemu_set_irq(intp->qemuirq, 0);
-
if (kvm_irqchip_add_irqfd_notifier(kvm_state, &intp->interrupt,
&intp->unmask, irq) < 0) {
goto fail_irqfd;
@@ -395,9 +408,6 @@ static void vfio_start_irqfd_injection(SysBusDevice *sbdev, qemu_irq irq)
goto fail_vfio;
}
- /* Let's resume injection with irqfd setup */
- vfio_unmask_single_irqindex(&vdev->vbasedev, intp->pin);
-
intp->kvm_accel = true;
trace_vfio_platform_start_irqfd_injection(intp->pin,
@@ -406,9 +416,11 @@ static void vfio_start_irqfd_injection(SysBusDevice *sbdev, qemu_irq irq)
return;
fail_vfio:
kvm_irqchip_remove_irqfd_notifier(kvm_state, &intp->interrupt, irq);
+ error_report("vfio: failed to start eventfd signaling for IRQ %d: %m",
+ intp->pin);
+ abort();
fail_irqfd:
- vfio_start_eventfd_injection(intp);
- vfio_unmask_single_irqindex(&vdev->vbasedev, intp->pin);
+ vfio_start_eventfd_injection(sbdev, irq);
return;
}
@@ -646,7 +658,6 @@ static void vfio_platform_realize(DeviceState *dev, Error **errp)
VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(dev);
SysBusDevice *sbdev = SYS_BUS_DEVICE(dev);
VFIODevice *vbasedev = &vdev->vbasedev;
- VFIOINTp *intp;
int i, ret;
vbasedev->type = VFIO_DEVICE_TYPE_PLATFORM;
@@ -665,10 +676,6 @@ static void vfio_platform_realize(DeviceState *dev, Error **errp)
vfio_map_region(vdev, i);
sysbus_init_mmio(sbdev, &vdev->regions[i]->mem);
}
-
- QLIST_FOREACH(intp, &vdev->intp_list, next) {
- vfio_start_eventfd_injection(intp);
- }
}
static const VMStateDescription vfio_platform_vmstate = {
--
1.8.3.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* [Qemu-devel] [PATCH v2 2/3] hw/vfio/platform: change interrupt/unmask fields into pointer
2015-09-24 20:08 [Qemu-devel] [PATCH v2 0/3] hw/vfio/platform: irqfd setup changes Eric Auger
2015-09-24 20:08 ` [Qemu-devel] [PATCH v2 1/3] hw/vfio/platform: irqfd setup sequence update Eric Auger
@ 2015-09-24 20:08 ` Eric Auger
2015-09-24 20:08 ` [Qemu-devel] [PATCH v2 3/3] hw/vfio/platform: do not set resamplefd for edge-sensitive IRQS Eric Auger
2 siblings, 0 replies; 4+ messages in thread
From: Eric Auger @ 2015-09-24 20:08 UTC (permalink / raw)
To: eric.auger, eric.auger, qemu-devel, alex.williamson
Cc: christoffer.dall, patches
unmask EventNotifier might not be initialized in case of edge
sensitive irq. Using EventNotifier pointers make life simpler to
handle the edge-sensitive irqfd setup.
Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
hw/vfio/platform.c | 35 ++++++++++++++++++++---------------
include/hw/vfio/vfio-platform.h | 4 ++--
2 files changed, 22 insertions(+), 17 deletions(-)
diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index d864342..cab1517 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -57,15 +57,20 @@ static VFIOINTp *vfio_init_intp(VFIODevice *vbasedev,
sysbus_init_irq(sbdev, &intp->qemuirq);
/* Get an eventfd for trigger */
- ret = event_notifier_init(&intp->interrupt, 0);
+ intp->interrupt = g_malloc0(sizeof(EventNotifier));
+ ret = event_notifier_init(intp->interrupt, 0);
if (ret) {
+ g_free(intp->interrupt);
g_free(intp);
error_report("vfio: Error: trigger event_notifier_init failed ");
return NULL;
}
/* Get an eventfd for resample/unmask */
- ret = event_notifier_init(&intp->unmask, 0);
+ intp->unmask = g_malloc0(sizeof(EventNotifier));
+ ret = event_notifier_init(intp->unmask, 0);
if (ret) {
+ g_free(intp->interrupt);
+ g_free(intp->unmask);
g_free(intp);
error_report("vfio: Error: resamplefd event_notifier_init failed");
return NULL;
@@ -100,7 +105,7 @@ static int vfio_set_trigger_eventfd(VFIOINTp *intp,
irq_set->start = 0;
irq_set->count = 1;
pfd = (int32_t *)&irq_set->data;
- *pfd = event_notifier_get_fd(&intp->interrupt);
+ *pfd = event_notifier_get_fd(intp->interrupt);
qemu_set_fd_handler(*pfd, (IOHandler *)handler, NULL, intp);
ret = ioctl(vbasedev->fd, VFIO_DEVICE_SET_IRQS, irq_set);
g_free(irq_set);
@@ -182,7 +187,7 @@ static void vfio_intp_mmap_enable(void *opaque)
static void vfio_intp_inject_pending_lockheld(VFIOINTp *intp)
{
trace_vfio_platform_intp_inject_pending_lockheld(intp->pin,
- event_notifier_get_fd(&intp->interrupt));
+ event_notifier_get_fd(intp->interrupt));
intp->state = VFIO_IRQ_ACTIVE;
@@ -224,18 +229,18 @@ static void vfio_intp_interrupt(VFIOINTp *intp)
trace_vfio_intp_interrupt_set_pending(intp->pin);
QSIMPLEQ_INSERT_TAIL(&vdev->pending_intp_queue,
intp, pqnext);
- ret = event_notifier_test_and_clear(&intp->interrupt);
+ ret = event_notifier_test_and_clear(intp->interrupt);
qemu_mutex_unlock(&vdev->intp_mutex);
return;
}
trace_vfio_platform_intp_interrupt(intp->pin,
- event_notifier_get_fd(&intp->interrupt));
+ event_notifier_get_fd(intp->interrupt));
- ret = event_notifier_test_and_clear(&intp->interrupt);
+ ret = event_notifier_test_and_clear(intp->interrupt);
if (!ret) {
error_report("Error when clearing fd=%d (ret = %d)",
- event_notifier_get_fd(&intp->interrupt), ret);
+ event_notifier_get_fd(intp->interrupt), ret);
}
intp->state = VFIO_IRQ_ACTIVE;
@@ -283,7 +288,7 @@ static void vfio_platform_eoi(VFIODevice *vbasedev)
QLIST_FOREACH(intp, &vdev->intp_list, next) {
if (intp->state == VFIO_IRQ_ACTIVE) {
trace_vfio_platform_eoi(intp->pin,
- event_notifier_get_fd(&intp->interrupt));
+ event_notifier_get_fd(intp->interrupt));
intp->state = VFIO_IRQ_INACTIVE;
/* deassert the virtual IRQ */
@@ -360,7 +365,7 @@ static int vfio_set_resample_eventfd(VFIOINTp *intp)
irq_set->start = 0;
irq_set->count = 1;
pfd = (int32_t *)&irq_set->data;
- *pfd = event_notifier_get_fd(&intp->unmask);
+ *pfd = event_notifier_get_fd(intp->unmask);
qemu_set_fd_handler(*pfd, NULL, NULL, NULL);
ret = ioctl(vbasedev->fd, VFIO_DEVICE_SET_IRQS, irq_set);
g_free(irq_set);
@@ -396,8 +401,8 @@ static void vfio_start_irqfd_injection(SysBusDevice *sbdev, qemu_irq irq)
}
assert(intp);
- if (kvm_irqchip_add_irqfd_notifier(kvm_state, &intp->interrupt,
- &intp->unmask, irq) < 0) {
+ if (kvm_irqchip_add_irqfd_notifier(kvm_state, intp->interrupt,
+ intp->unmask, irq) < 0) {
goto fail_irqfd;
}
@@ -411,11 +416,11 @@ static void vfio_start_irqfd_injection(SysBusDevice *sbdev, qemu_irq irq)
intp->kvm_accel = true;
trace_vfio_platform_start_irqfd_injection(intp->pin,
- event_notifier_get_fd(&intp->interrupt),
- event_notifier_get_fd(&intp->unmask));
+ event_notifier_get_fd(intp->interrupt),
+ event_notifier_get_fd(intp->unmask));
return;
fail_vfio:
- kvm_irqchip_remove_irqfd_notifier(kvm_state, &intp->interrupt, irq);
+ kvm_irqchip_remove_irqfd_notifier(kvm_state, intp->interrupt, irq);
error_report("vfio: failed to start eventfd signaling for IRQ %d: %m",
intp->pin);
abort();
diff --git a/include/hw/vfio/vfio-platform.h b/include/hw/vfio/vfio-platform.h
index c5cf1d7..b468f80 100644
--- a/include/hw/vfio/vfio-platform.h
+++ b/include/hw/vfio/vfio-platform.h
@@ -34,8 +34,8 @@ enum {
typedef struct VFIOINTp {
QLIST_ENTRY(VFIOINTp) next; /* entry for IRQ list */
QSIMPLEQ_ENTRY(VFIOINTp) pqnext; /* entry for pending IRQ queue */
- EventNotifier interrupt; /* eventfd triggered on interrupt */
- EventNotifier unmask; /* eventfd for unmask on QEMU bypass */
+ EventNotifier *interrupt; /* eventfd triggered on interrupt */
+ EventNotifier *unmask; /* eventfd for unmask on QEMU bypass */
qemu_irq qemuirq;
struct VFIOPlatformDevice *vdev; /* back pointer to device */
int state; /* inactive, pending, active */
--
1.8.3.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* [Qemu-devel] [PATCH v2 3/3] hw/vfio/platform: do not set resamplefd for edge-sensitive IRQS
2015-09-24 20:08 [Qemu-devel] [PATCH v2 0/3] hw/vfio/platform: irqfd setup changes Eric Auger
2015-09-24 20:08 ` [Qemu-devel] [PATCH v2 1/3] hw/vfio/platform: irqfd setup sequence update Eric Auger
2015-09-24 20:08 ` [Qemu-devel] [PATCH v2 2/3] hw/vfio/platform: change interrupt/unmask fields into pointer Eric Auger
@ 2015-09-24 20:08 ` Eric Auger
2 siblings, 0 replies; 4+ messages in thread
From: Eric Auger @ 2015-09-24 20:08 UTC (permalink / raw)
To: eric.auger, eric.auger, qemu-devel, alex.williamson
Cc: christoffer.dall, patches
In irqfd mode, current code attempts to set a resamplefd whatever
the type of the IRQ. For an edge-sensitive IRQ this attempt fails
and as a consequence, the whole irqfd setup fails and we fall back
to the slow mode. This patch bypasses the resamplefd setting for
non level-sentive IRQs.
Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
v1 -> v2:
- introduce vfio_irq_is_automasked helper function. in case of
edge-sensitive IRQ, do not allocate/initialize unmask EventNotifier
nor call vfio_set_resample_eventfd
---
hw/vfio/platform.c | 42 +++++++++++++++++++++++++++---------------
trace-events | 4 +++-
2 files changed, 30 insertions(+), 16 deletions(-)
diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index cab1517..5c1156c 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -32,6 +32,11 @@
* Functions used whatever the injection method
*/
+static inline bool vfio_irq_is_automasked(VFIOINTp *intp)
+{
+ return intp->flags & VFIO_IRQ_INFO_AUTOMASKED;
+}
+
/**
* vfio_init_intp - allocate, initialize the IRQ struct pointer
* and add it into the list of IRQs
@@ -65,15 +70,17 @@ static VFIOINTp *vfio_init_intp(VFIODevice *vbasedev,
error_report("vfio: Error: trigger event_notifier_init failed ");
return NULL;
}
- /* Get an eventfd for resample/unmask */
- intp->unmask = g_malloc0(sizeof(EventNotifier));
- ret = event_notifier_init(intp->unmask, 0);
- if (ret) {
- g_free(intp->interrupt);
- g_free(intp->unmask);
- g_free(intp);
- error_report("vfio: Error: resamplefd event_notifier_init failed");
- return NULL;
+ if (vfio_irq_is_automasked(intp)) {
+ /* Get an eventfd for resample/unmask */
+ intp->unmask = g_malloc0(sizeof(EventNotifier));
+ ret = event_notifier_init(intp->unmask, 0);
+ if (ret) {
+ g_free(intp->interrupt);
+ g_free(intp->unmask);
+ g_free(intp);
+ error_report("vfio: Error: resamplefd event_notifier_init failed");
+ return NULL;
+ }
}
QLIST_INSERT_HEAD(&vdev->intp_list, intp, next);
@@ -294,7 +301,7 @@ static void vfio_platform_eoi(VFIODevice *vbasedev)
/* deassert the virtual IRQ */
qemu_set_irq(intp->qemuirq, 0);
- if (intp->flags & VFIO_IRQ_INFO_AUTOMASKED) {
+ if (vfio_irq_is_automasked(intp)) {
/* unmasks the physical level-sensitive IRQ */
vfio_unmask_single_irqindex(vbasedev, intp->pin);
}
@@ -409,15 +416,20 @@ static void vfio_start_irqfd_injection(SysBusDevice *sbdev, qemu_irq irq)
if (vfio_set_trigger_eventfd(intp, NULL) < 0) {
goto fail_vfio;
}
- if (vfio_set_resample_eventfd(intp) < 0) {
- goto fail_vfio;
+ if (vfio_irq_is_automasked(intp)) {
+ if (vfio_set_resample_eventfd(intp) < 0) {
+ goto fail_vfio;
+ }
+ trace_vfio_platform_start_level_irqfd_injection(intp->pin,
+ event_notifier_get_fd(intp->interrupt),
+ event_notifier_get_fd(intp->unmask));
+ } else {
+ trace_vfio_platform_start_edge_irqfd_injection(intp->pin,
+ event_notifier_get_fd(intp->interrupt));
}
intp->kvm_accel = true;
- trace_vfio_platform_start_irqfd_injection(intp->pin,
- event_notifier_get_fd(intp->interrupt),
- event_notifier_get_fd(intp->unmask));
return;
fail_vfio:
kvm_irqchip_remove_irqfd_notifier(kvm_state, intp->interrupt, irq);
diff --git a/trace-events b/trace-events
index 25c53e0..8f7829e 100644
--- a/trace-events
+++ b/trace-events
@@ -1621,7 +1621,9 @@ vfio_platform_intp_interrupt(int pin, int fd) "Inject IRQ #%d (fd = %d)"
vfio_platform_intp_inject_pending_lockheld(int pin, int fd) "Inject pending IRQ #%d (fd = %d)"
vfio_platform_populate_interrupts(int pin, int count, int flags) "- IRQ index %d: count %d, flags=0x%x"
vfio_intp_interrupt_set_pending(int index) "irq %d is set PENDING"
-vfio_platform_start_irqfd_injection(int index, int fd, int resamplefd) "IRQ index=%d, fd = %d, resamplefd = %d"
+vfio_platform_start_level_irqfd_injection(int index, int fd, int resamplefd) "IRQ index=%d, fd = %d, resamplefd = %d"
+vfio_platform_start_edge_irqfd_injection(int index, int fd) "IRQ index=%d, fd = %d"
+
#hw/acpi/memory_hotplug.c
mhp_acpi_invalid_slot_selected(uint32_t slot) "0x%"PRIx32
--
1.8.3.2
^ permalink raw reply related [flat|nested] 4+ messages in thread