* [Qemu-devel] [PATCH uq/master 1/9] event_notifier: add event_notifier_set
2012-07-05 15:16 [Qemu-devel] [PATCH uq/master 0/9] remove event_notifier_get_fd from non-KVM code Paolo Bonzini
@ 2012-07-05 15:16 ` Paolo Bonzini
2012-07-05 15:16 ` [Qemu-devel] [PATCH uq/master 2/9] event_notifier: remove event_notifier_test Paolo Bonzini
` (8 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2012-07-05 15:16 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, mst, jan.kiszka, mtosatti, avi, anthony.perard,
stefano.stabellini
EventNotifier right now cannot be used as an inter-thread communication
primitive. It only works if something else (the kernel) sets the eventfd.
Add a primitive to signal an EventNotifier that another thread is waiting
on.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
event_notifier.c | 7 +++++++
event_notifier.h | 1 +
2 files changed, 8 insertions(+)
diff --git a/event_notifier.c b/event_notifier.c
index 0b82981..2b210f4 100644
--- a/event_notifier.c
+++ b/event_notifier.c
@@ -38,6 +38,13 @@ int event_notifier_get_fd(EventNotifier *e)
return e->fd;
}
+int event_notifier_set(EventNotifier *e)
+{
+ uint64_t value = 1;
+ int r = write(e->fd, &value, sizeof(value));
+ return r == sizeof(value);
+}
+
int event_notifier_test_and_clear(EventNotifier *e)
{
uint64_t value;
diff --git a/event_notifier.h b/event_notifier.h
index 886222c..efca852 100644
--- a/event_notifier.h
+++ b/event_notifier.h
@@ -22,6 +22,7 @@ struct EventNotifier {
int event_notifier_init(EventNotifier *, int active);
void event_notifier_cleanup(EventNotifier *);
int event_notifier_get_fd(EventNotifier *);
+int event_notifier_set(EventNotifier *);
int event_notifier_test_and_clear(EventNotifier *);
int event_notifier_test(EventNotifier *);
--
1.7.10.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH uq/master 2/9] event_notifier: remove event_notifier_test
2012-07-05 15:16 [Qemu-devel] [PATCH uq/master 0/9] remove event_notifier_get_fd from non-KVM code Paolo Bonzini
2012-07-05 15:16 ` [Qemu-devel] [PATCH uq/master 1/9] event_notifier: add event_notifier_set Paolo Bonzini
@ 2012-07-05 15:16 ` Paolo Bonzini
2012-07-12 9:10 ` Avi Kivity
2012-07-05 15:16 ` [Qemu-devel] [PATCH uq/master 3/9] event_notifier: add event_notifier_init_fd Paolo Bonzini
` (7 subsequent siblings)
9 siblings, 1 reply; 16+ messages in thread
From: Paolo Bonzini @ 2012-07-05 15:16 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, mst, jan.kiszka, mtosatti, avi, anthony.perard,
stefano.stabellini
This is broken; since the eventfd is used in nonblocking mode there
is a race between reading and writing.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
event_notifier.c | 15 ---------------
event_notifier.h | 1 -
2 files changed, 16 deletions(-)
diff --git a/event_notifier.c b/event_notifier.c
index 2b210f4..c339bfe 100644
--- a/event_notifier.c
+++ b/event_notifier.c
@@ -51,18 +51,3 @@ int event_notifier_test_and_clear(EventNotifier *e)
int r = read(e->fd, &value, sizeof(value));
return r == sizeof(value);
}
-
-int event_notifier_test(EventNotifier *e)
-{
- uint64_t value;
- int r = read(e->fd, &value, sizeof(value));
- if (r == sizeof(value)) {
- /* restore previous value. */
- int s = write(e->fd, &value, sizeof(value));
- /* never blocks because we use EFD_SEMAPHORE.
- * If we didn't we'd get EAGAIN on overflow
- * and we'd have to write code to ignore it. */
- assert(s == sizeof(value));
- }
- return r == sizeof(value);
-}
diff --git a/event_notifier.h b/event_notifier.h
index efca852..9b2edf4 100644
--- a/event_notifier.h
+++ b/event_notifier.h
@@ -24,6 +24,5 @@ void event_notifier_cleanup(EventNotifier *);
int event_notifier_get_fd(EventNotifier *);
int event_notifier_set(EventNotifier *);
int event_notifier_test_and_clear(EventNotifier *);
-int event_notifier_test(EventNotifier *);
#endif
--
1.7.10.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH uq/master 2/9] event_notifier: remove event_notifier_test
2012-07-05 15:16 ` [Qemu-devel] [PATCH uq/master 2/9] event_notifier: remove event_notifier_test Paolo Bonzini
@ 2012-07-12 9:10 ` Avi Kivity
2012-07-12 10:30 ` Paolo Bonzini
0 siblings, 1 reply; 16+ messages in thread
From: Avi Kivity @ 2012-07-12 9:10 UTC (permalink / raw)
To: Paolo Bonzini
Cc: kvm, mst, jan.kiszka, mtosatti, qemu-devel, anthony.perard,
stefano.stabellini
On 07/05/2012 06:16 PM, Paolo Bonzini wrote:
> This is broken; since the eventfd is used in nonblocking mode there
> is a race between reading and writing.
>
> diff --git a/event_notifier.c b/event_notifier.c
> index 2b210f4..c339bfe 100644
> --- a/event_notifier.c
> +++ b/event_notifier.c
> @@ -51,18 +51,3 @@ int event_notifier_test_and_clear(EventNotifier *e)
> int r = read(e->fd, &value, sizeof(value));
> return r == sizeof(value);
> }
> -
> -int event_notifier_test(EventNotifier *e)
> -{
> - uint64_t value;
> - int r = read(e->fd, &value, sizeof(value));
> - if (r == sizeof(value)) {
> - /* restore previous value. */
> - int s = write(e->fd, &value, sizeof(value));
> - /* never blocks because we use EFD_SEMAPHORE.
> - * If we didn't we'd get EAGAIN on overflow
> - * and we'd have to write code to ignore it. */
> - assert(s == sizeof(value));
> - }
> - return r == sizeof(value);
> -}
I don't see the race. Mind explaining?
It does however require than a poller be extra careful when reading; and
the function is silly anyway.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH uq/master 2/9] event_notifier: remove event_notifier_test
2012-07-12 9:10 ` Avi Kivity
@ 2012-07-12 10:30 ` Paolo Bonzini
2012-07-12 11:04 ` Avi Kivity
0 siblings, 1 reply; 16+ messages in thread
From: Paolo Bonzini @ 2012-07-12 10:30 UTC (permalink / raw)
To: Avi Kivity
Cc: kvm, mst, jan.kiszka, mtosatti, qemu-devel, anthony.perard,
stefano.stabellini
Il 12/07/2012 11:10, Avi Kivity ha scritto:
> On 07/05/2012 06:16 PM, Paolo Bonzini wrote:
>> This is broken; since the eventfd is used in nonblocking mode there
>> is a race between reading and writing.
>>
>
>> diff --git a/event_notifier.c b/event_notifier.c
>> index 2b210f4..c339bfe 100644
>> --- a/event_notifier.c
>> +++ b/event_notifier.c
>> @@ -51,18 +51,3 @@ int event_notifier_test_and_clear(EventNotifier *e)
>> int r = read(e->fd, &value, sizeof(value));
>> return r == sizeof(value);
>> }
>> -
>> -int event_notifier_test(EventNotifier *e)
>> -{
>> - uint64_t value;
>> - int r = read(e->fd, &value, sizeof(value));
>> - if (r == sizeof(value)) {
>> - /* restore previous value. */
>> - int s = write(e->fd, &value, sizeof(value));
>> - /* never blocks because we use EFD_SEMAPHORE.
>> - * If we didn't we'd get EAGAIN on overflow
>> - * and we'd have to write code to ignore it. */
>> - assert(s == sizeof(value));
>> - }
>> - return r == sizeof(value);
>> -}
>
> I don't see the race. Mind explaining?
The assertion can actually fire, there's nothing that prevents this from
happening:
event_notifier_test()
read(fd, &value, 8)
write(fd, <large value>, 8)
write(fd, &value, 8)
event_notifier_set will always write a 1 and it will take a large amount
of writes to reach overflow :) but that may not be true of other writers
using the same file descriptor.
Then, the comment is wrong in two ways. First, we do not use
EFD_SEMAPHORE (though even if we did the only difference is that value
will be always one). Second, we cannot write code to ignore EAGAIN,
because then we've lost the value.
With blocking I/O things would not be much better, because then
event_notifier_test() might block on the write. That would be quite
surprising.
If we cared, we could implement the function more easily and corectly
with poll(), checking for POLLIN in the revents. But I don't see a
sensible use case for it anyway.
Paolo
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH uq/master 2/9] event_notifier: remove event_notifier_test
2012-07-12 10:30 ` Paolo Bonzini
@ 2012-07-12 11:04 ` Avi Kivity
2012-07-12 11:16 ` Paolo Bonzini
0 siblings, 1 reply; 16+ messages in thread
From: Avi Kivity @ 2012-07-12 11:04 UTC (permalink / raw)
To: Paolo Bonzini
Cc: kvm, mst, jan.kiszka, mtosatti, qemu-devel, anthony.perard,
stefano.stabellini
On 07/12/2012 01:30 PM, Paolo Bonzini wrote:
> Il 12/07/2012 11:10, Avi Kivity ha scritto:
>> On 07/05/2012 06:16 PM, Paolo Bonzini wrote:
>>> This is broken; since the eventfd is used in nonblocking mode there
>>> is a race between reading and writing.
>>>
>>
>>> diff --git a/event_notifier.c b/event_notifier.c
>>> index 2b210f4..c339bfe 100644
>>> --- a/event_notifier.c
>>> +++ b/event_notifier.c
>>> @@ -51,18 +51,3 @@ int event_notifier_test_and_clear(EventNotifier *e)
>>> int r = read(e->fd, &value, sizeof(value));
>>> return r == sizeof(value);
>>> }
>>> -
>>> -int event_notifier_test(EventNotifier *e)
>>> -{
>>> - uint64_t value;
>>> - int r = read(e->fd, &value, sizeof(value));
>>> - if (r == sizeof(value)) {
>>> - /* restore previous value. */
>>> - int s = write(e->fd, &value, sizeof(value));
>>> - /* never blocks because we use EFD_SEMAPHORE.
>>> - * If we didn't we'd get EAGAIN on overflow
>>> - * and we'd have to write code to ignore it. */
>>> - assert(s == sizeof(value));
>>> - }
>>> - return r == sizeof(value);
>>> -}
>>
>> I don't see the race. Mind explaining?
>
> The assertion can actually fire, there's nothing that prevents this from
> happening:
>
> event_notifier_test()
> read(fd, &value, 8)
> write(fd, <large value>, 8)
> write(fd, &value, 8)
>
> event_notifier_set will always write a 1 and it will take a large amount
> of writes to reach overflow :) but that may not be true of other writers
> using the same file descriptor.
The first write would have overflowed without event_notifier_test(), and
there's no reasonable way to deal with it; nor is there any reason to,
since the limit is so large.
> Then, the comment is wrong in two ways. First, we do not use
> EFD_SEMAPHORE (though even if we did the only difference is that value
> will be always one). Second, we cannot write code to ignore EAGAIN,
> because then we've lost the value.
>
> With blocking I/O things would not be much better, because then
> event_notifier_test() might block on the write. That would be quite
> surprising.
>
> If we cared, we could implement the function more easily and corectly
> with poll(), checking for POLLIN in the revents. But I don't see a
> sensible use case for it anyway.
Right, it's useless. I'll adjust the comment (and the whitespace fix)
and apply.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH uq/master 2/9] event_notifier: remove event_notifier_test
2012-07-12 11:04 ` Avi Kivity
@ 2012-07-12 11:16 ` Paolo Bonzini
0 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2012-07-12 11:16 UTC (permalink / raw)
To: Avi Kivity
Cc: kvm, mst, jan.kiszka, mtosatti, qemu-devel, anthony.perard,
stefano.stabellini
Il 12/07/2012 13:04, Avi Kivity ha scritto:
> Right, it's useless. I'll adjust the comment (and the whitespace fix)
> and apply.
Ok, thanks very much!
Paolo
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH uq/master 3/9] event_notifier: add event_notifier_init_fd
2012-07-05 15:16 [Qemu-devel] [PATCH uq/master 0/9] remove event_notifier_get_fd from non-KVM code Paolo Bonzini
2012-07-05 15:16 ` [Qemu-devel] [PATCH uq/master 1/9] event_notifier: add event_notifier_set Paolo Bonzini
2012-07-05 15:16 ` [Qemu-devel] [PATCH uq/master 2/9] event_notifier: remove event_notifier_test Paolo Bonzini
@ 2012-07-05 15:16 ` Paolo Bonzini
2012-07-12 9:11 ` Avi Kivity
2012-07-05 15:16 ` [Qemu-devel] [PATCH uq/master 4/9] ivshmem: use EventNotifier and memory API Paolo Bonzini
` (6 subsequent siblings)
9 siblings, 1 reply; 16+ messages in thread
From: Paolo Bonzini @ 2012-07-05 15:16 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, mst, jan.kiszka, mtosatti, avi, anthony.perard,
stefano.stabellini
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
event_notifier.c | 7 +++++++
event_notifier.h | 3 ++-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/event_notifier.c b/event_notifier.c
index c339bfe..99c376c 100644
--- a/event_notifier.c
+++ b/event_notifier.c
@@ -10,11 +10,18 @@
* See the COPYING file in the top-level directory.
*/
+#include "qemu-common.h"
#include "event_notifier.h"
+
#ifdef CONFIG_EVENTFD
#include <sys/eventfd.h>
#endif
+void event_notifier_init_fd(EventNotifier *e, int fd)
+{
+ e->fd = fd;
+}
+
int event_notifier_init(EventNotifier *e, int active)
{
#ifdef CONFIG_EVENTFD
diff --git a/event_notifier.h b/event_notifier.h
index 9b2edf4..30c12dd 100644
--- a/event_notifier.h
+++ b/event_notifier.h
@@ -16,9 +16,10 @@
#include "qemu-common.h"
struct EventNotifier {
- int fd;
+ int fd;
};
+void event_notifier_init_fd(EventNotifier *, int fd);
int event_notifier_init(EventNotifier *, int active);
void event_notifier_cleanup(EventNotifier *);
int event_notifier_get_fd(EventNotifier *);
--
1.7.10.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH uq/master 4/9] ivshmem: use EventNotifier and memory API
2012-07-05 15:16 [Qemu-devel] [PATCH uq/master 0/9] remove event_notifier_get_fd from non-KVM code Paolo Bonzini
` (2 preceding siblings ...)
2012-07-05 15:16 ` [Qemu-devel] [PATCH uq/master 3/9] event_notifier: add event_notifier_init_fd Paolo Bonzini
@ 2012-07-05 15:16 ` Paolo Bonzini
2012-07-05 15:16 ` [Qemu-devel] [PATCH uq/master 5/9] ivshmem: wrap ivshmem_del_eventfd loops with transaction Paolo Bonzini
` (5 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2012-07-05 15:16 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, mst, jan.kiszka, mtosatti, avi, anthony.perard,
stefano.stabellini
All of ivshmem's usage of eventfd now has a corresponding API in
EventNotifier. Simplify the code by using it, and also use the
memory API consistently to set up and tear down the ioeventfds.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/ivshmem.c | 63 ++++++++++++++++++++++++++++++++--------------------------
1 file changed, 35 insertions(+), 28 deletions(-)
diff --git a/hw/ivshmem.c b/hw/ivshmem.c
index 05559b6..3cdbea2 100644
--- a/hw/ivshmem.c
+++ b/hw/ivshmem.c
@@ -23,6 +23,7 @@
#include "kvm.h"
#include "migration.h"
#include "qerror.h"
+#include "event_notifier.h"
#include <sys/mman.h>
#include <sys/types.h>
@@ -45,7 +46,7 @@
typedef struct Peer {
int nb_eventfds;
- int *eventfds;
+ EventNotifier *eventfds;
} Peer;
typedef struct EventfdEntry {
@@ -63,7 +64,6 @@ typedef struct IVShmemState {
CharDriverState *server_chr;
MemoryRegion ivshmem_mmio;
- pcibus_t mmio_addr;
/* We might need to register the BAR before we actually have the memory.
* So prepare a container MemoryRegion for the BAR immediately and
* add a subregion when we have the memory.
@@ -168,7 +168,6 @@ static void ivshmem_io_write(void *opaque, target_phys_addr_t addr,
{
IVShmemState *s = opaque;
- uint64_t write_one = 1;
uint16_t dest = val >> 16;
uint16_t vector = val & 0xff;
@@ -194,12 +193,8 @@ static void ivshmem_io_write(void *opaque, target_phys_addr_t addr,
/* check doorbell range */
if (vector < s->peers[dest].nb_eventfds) {
- IVSHMEM_DPRINTF("Writing %" PRId64 " to VM %d on vector %d\n",
- write_one, dest, vector);
- if (write(s->peers[dest].eventfds[vector],
- &(write_one), 8) != 8) {
- IVSHMEM_DPRINTF("error writing to eventfd\n");
- }
+ IVSHMEM_DPRINTF("Notifying VM %d on vector %d\n", dest, vector);
+ event_notifier_set(&s->peers[dest].eventfds[vector]);
}
break;
default:
@@ -279,12 +274,13 @@ static void fake_irqfd(void *opaque, const uint8_t *buf, int size) {
msix_notify(pdev, entry->vector);
}
-static CharDriverState* create_eventfd_chr_device(void * opaque, int eventfd,
- int vector)
+static CharDriverState* create_eventfd_chr_device(void * opaque, EventNotifier *n,
+ int vector)
{
/* create a event character device based on the passed eventfd */
IVShmemState *s = opaque;
CharDriverState * chr;
+ int eventfd = event_notifier_get_fd(n);
chr = qemu_chr_open_eventfd(eventfd);
@@ -347,6 +343,26 @@ static void create_shared_memory_BAR(IVShmemState *s, int fd) {
pci_register_bar(&s->dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->bar);
}
+static void ivshmem_add_eventfd(IVShmemState *s, int posn, int i)
+{
+ memory_region_add_eventfd(&s->ivshmem_mmio,
+ DOORBELL,
+ 4,
+ true,
+ (posn << 16) | i,
+ event_notifier_get_fd(&s->peers[posn].eventfds[i]));
+}
+
+static void ivshmem_del_eventfd(IVShmemState *s, int posn, int i)
+{
+ memory_region_del_eventfd(&s->ivshmem_mmio,
+ DOORBELL,
+ 4,
+ true,
+ (posn << 16) | i,
+ event_notifier_get_fd(&s->peers[posn].eventfds[i]));
+}
+
static void close_guest_eventfds(IVShmemState *s, int posn)
{
int i, guest_curr_max;
@@ -354,9 +370,8 @@ static void close_guest_eventfds(IVShmemState *s, int posn)
guest_curr_max = s->peers[posn].nb_eventfds;
for (i = 0; i < guest_curr_max; i++) {
- kvm_set_ioeventfd_mmio(s->peers[posn].eventfds[i],
- s->mmio_addr + DOORBELL, (posn << 16) | i, 0, 4);
- close(s->peers[posn].eventfds[i]);
+ ivshmem_del_eventfd(s, posn, i);
+ event_notifier_cleanup(&s->peers[posn].eventfds[i]);
}
g_free(s->peers[posn].eventfds);
@@ -369,12 +384,7 @@ static void setup_ioeventfds(IVShmemState *s) {
for (i = 0; i <= s->max_peer; i++) {
for (j = 0; j < s->peers[i].nb_eventfds; j++) {
- memory_region_add_eventfd(&s->ivshmem_mmio,
- DOORBELL,
- 4,
- true,
- (i << 16) | j,
- s->peers[i].eventfds[j]);
+ ivshmem_add_eventfd(s, i, j);
}
}
}
@@ -476,14 +486,14 @@ static void ivshmem_read(void *opaque, const uint8_t * buf, int flags)
if (guest_max_eventfd == 0) {
/* one eventfd per MSI vector */
- s->peers[incoming_posn].eventfds = (int *) g_malloc(s->vectors *
- sizeof(int));
+ s->peers[incoming_posn].eventfds = g_new(EventNotifier, s->vectors);
}
/* this is an eventfd for a particular guest VM */
IVSHMEM_DPRINTF("eventfds[%ld][%d] = %d\n", incoming_posn,
guest_max_eventfd, incoming_fd);
- s->peers[incoming_posn].eventfds[guest_max_eventfd] = incoming_fd;
+ event_notifier_init_fd(&s->peers[incoming_posn].eventfds[guest_max_eventfd],
+ incoming_fd);
/* increment count for particular guest */
s->peers[incoming_posn].nb_eventfds++;
@@ -495,15 +505,12 @@ static void ivshmem_read(void *opaque, const uint8_t * buf, int flags)
if (incoming_posn == s->vm_id) {
s->eventfd_chr[guest_max_eventfd] = create_eventfd_chr_device(s,
- s->peers[s->vm_id].eventfds[guest_max_eventfd],
+ &s->peers[s->vm_id].eventfds[guest_max_eventfd],
guest_max_eventfd);
}
if (ivshmem_has_feature(s, IVSHMEM_IOEVENTFD)) {
- if (kvm_set_ioeventfd_mmio(incoming_fd, s->mmio_addr + DOORBELL,
- (incoming_posn << 16) | guest_max_eventfd, 1, 4) < 0) {
- fprintf(stderr, "ivshmem: ioeventfd not available\n");
- }
+ ivshmem_add_eventfd(s, incoming_posn, guest_max_eventfd);
}
return;
--
1.7.10.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH uq/master 5/9] ivshmem: wrap ivshmem_del_eventfd loops with transaction
2012-07-05 15:16 [Qemu-devel] [PATCH uq/master 0/9] remove event_notifier_get_fd from non-KVM code Paolo Bonzini
` (3 preceding siblings ...)
2012-07-05 15:16 ` [Qemu-devel] [PATCH uq/master 4/9] ivshmem: use EventNotifier and memory API Paolo Bonzini
@ 2012-07-05 15:16 ` Paolo Bonzini
2012-07-05 15:16 ` [Qemu-devel] [PATCH uq/master 6/9] memory: pass EventNotifier, not eventfd Paolo Bonzini
` (4 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2012-07-05 15:16 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, mst, jan.kiszka, mtosatti, avi, anthony.perard,
stefano.stabellini
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/ivshmem.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/hw/ivshmem.c b/hw/ivshmem.c
index 3cdbea2..19e164a 100644
--- a/hw/ivshmem.c
+++ b/hw/ivshmem.c
@@ -369,8 +369,12 @@ static void close_guest_eventfds(IVShmemState *s, int posn)
guest_curr_max = s->peers[posn].nb_eventfds;
+ memory_region_transaction_begin();
for (i = 0; i < guest_curr_max; i++) {
ivshmem_del_eventfd(s, posn, i);
+ }
+ memory_region_transaction_commit();
+ for (i = 0; i < guest_curr_max; i++) {
event_notifier_cleanup(&s->peers[posn].eventfds[i]);
}
--
1.7.10.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH uq/master 6/9] memory: pass EventNotifier, not eventfd
2012-07-05 15:16 [Qemu-devel] [PATCH uq/master 0/9] remove event_notifier_get_fd from non-KVM code Paolo Bonzini
` (4 preceding siblings ...)
2012-07-05 15:16 ` [Qemu-devel] [PATCH uq/master 5/9] ivshmem: wrap ivshmem_del_eventfd loops with transaction Paolo Bonzini
@ 2012-07-05 15:16 ` Paolo Bonzini
2012-07-05 15:16 ` [Qemu-devel] [PATCH uq/master 7/9] event_notifier: add event_notifier_set_handler Paolo Bonzini
` (3 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2012-07-05 15:16 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, mst, jan.kiszka, mtosatti, avi, anthony.perard,
stefano.stabellini
Under Win32, EventNotifiers will not have event_notifier_get_fd, so we
cannot call it in common code such as hw/virtio-pci.c. Pass a pointer to
the notifier, and only retrieve the file descriptor in kvm-specific code.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
exec.c | 8 ++++----
hw/ivshmem.c | 4 ++--
hw/vhost.c | 4 ++--
hw/virtio-pci.c | 4 ++--
hw/xen_pt.c | 2 +-
kvm-all.c | 19 +++++++++++++------
memory.c | 18 +++++++++---------
memory.h | 9 +++++----
xen-all.c | 6 ++++--
9 files changed, 42 insertions(+), 32 deletions(-)
diff --git a/exec.c b/exec.c
index 8244d54..29b5078 100644
--- a/exec.c
+++ b/exec.c
@@ -3212,13 +3212,13 @@ static void core_log_global_stop(MemoryListener *listener)
static void core_eventfd_add(MemoryListener *listener,
MemoryRegionSection *section,
- bool match_data, uint64_t data, int fd)
+ bool match_data, uint64_t data, EventNotifier *e)
{
}
static void core_eventfd_del(MemoryListener *listener,
MemoryRegionSection *section,
- bool match_data, uint64_t data, int fd)
+ bool match_data, uint64_t data, EventNotifier *e)
{
}
@@ -3278,13 +3278,13 @@ static void io_log_global_stop(MemoryListener *listener)
static void io_eventfd_add(MemoryListener *listener,
MemoryRegionSection *section,
- bool match_data, uint64_t data, int fd)
+ bool match_data, uint64_t data, EventNotifier *e)
{
}
static void io_eventfd_del(MemoryListener *listener,
MemoryRegionSection *section,
- bool match_data, uint64_t data, int fd)
+ bool match_data, uint64_t data, EventNotifier *e)
{
}
diff --git a/hw/ivshmem.c b/hw/ivshmem.c
index 19e164a..bba21c5 100644
--- a/hw/ivshmem.c
+++ b/hw/ivshmem.c
@@ -350,7 +350,7 @@ static void ivshmem_add_eventfd(IVShmemState *s, int posn, int i)
4,
true,
(posn << 16) | i,
- event_notifier_get_fd(&s->peers[posn].eventfds[i]));
+ &s->peers[posn].eventfds[i]);
}
static void ivshmem_del_eventfd(IVShmemState *s, int posn, int i)
@@ -360,7 +360,7 @@ static void ivshmem_del_eventfd(IVShmemState *s, int posn, int i)
4,
true,
(posn << 16) | i,
- event_notifier_get_fd(&s->peers[posn].eventfds[i]));
+ &s->peers[posn].eventfds[i]);
}
static void close_guest_eventfds(IVShmemState *s, int posn)
diff --git a/hw/vhost.c b/hw/vhost.c
index 43664e7..0fd8da8 100644
--- a/hw/vhost.c
+++ b/hw/vhost.c
@@ -737,13 +737,13 @@ static void vhost_virtqueue_cleanup(struct vhost_dev *dev,
static void vhost_eventfd_add(MemoryListener *listener,
MemoryRegionSection *section,
- bool match_data, uint64_t data, int fd)
+ bool match_data, uint64_t data, EventNotifier *e)
{
}
static void vhost_eventfd_del(MemoryListener *listener,
MemoryRegionSection *section,
- bool match_data, uint64_t data, int fd)
+ bool match_data, uint64_t data, EventNotifier *e)
{
}
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 9342eed..a555728 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -174,10 +174,10 @@ static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
return r;
}
memory_region_add_eventfd(&proxy->bar, VIRTIO_PCI_QUEUE_NOTIFY, 2,
- true, n, event_notifier_get_fd(notifier));
+ true, n, notifier);
} else {
memory_region_del_eventfd(&proxy->bar, VIRTIO_PCI_QUEUE_NOTIFY, 2,
- true, n, event_notifier_get_fd(notifier));
+ true, n, notifier);
/* Handle the race condition where the guest kicked and we deassigned
* before we got around to handling the kick.
*/
diff --git a/hw/xen_pt.c b/hw/xen_pt.c
index 3b6d186..fdf68aa 100644
--- a/hw/xen_pt.c
+++ b/hw/xen_pt.c
@@ -634,7 +634,7 @@ static void xen_pt_log_global_fns(MemoryListener *l)
}
static void xen_pt_eventfd_fns(MemoryListener *l, MemoryRegionSection *s,
- bool match_data, uint64_t data, int fd)
+ bool match_data, uint64_t data, EventNotifier *n)
{
}
diff --git a/kvm-all.c b/kvm-all.c
index f8e4328..56f723e 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -32,6 +32,7 @@
#include "bswap.h"
#include "memory.h"
#include "exec-memory.h"
+#include "event_notifier.h"
/* This check must be after config-host.h is included */
#ifdef CONFIG_EVENTFD
@@ -800,23 +801,29 @@ static void kvm_io_ioeventfd_del(MemoryRegionSection *section,
static void kvm_eventfd_add(MemoryListener *listener,
MemoryRegionSection *section,
- bool match_data, uint64_t data, int fd)
+ bool match_data, uint64_t data,
+ EventNotifier *e)
{
if (section->address_space == get_system_memory()) {
- kvm_mem_ioeventfd_add(section, match_data, data, fd);
+ kvm_mem_ioeventfd_add(section, match_data, data,
+ event_notifier_get_fd(e));
} else {
- kvm_io_ioeventfd_add(section, match_data, data, fd);
+ kvm_io_ioeventfd_add(section, match_data, data,
+ event_notifier_get_fd(e));
}
}
static void kvm_eventfd_del(MemoryListener *listener,
MemoryRegionSection *section,
- bool match_data, uint64_t data, int fd)
+ bool match_data, uint64_t data,
+ EventNotifier *e)
{
if (section->address_space == get_system_memory()) {
- kvm_mem_ioeventfd_del(section, match_data, data, fd);
+ kvm_mem_ioeventfd_del(section, match_data, data,
+ event_notifier_get_fd(e));
} else {
- kvm_io_ioeventfd_del(section, match_data, data, fd);
+ kvm_io_ioeventfd_del(section, match_data, data,
+ event_notifier_get_fd(e));
}
}
diff --git a/memory.c b/memory.c
index aab4a31..643871b 100644
--- a/memory.c
+++ b/memory.c
@@ -156,7 +156,7 @@ struct MemoryRegionIoeventfd {
AddrRange addr;
bool match_data;
uint64_t data;
- int fd;
+ EventNotifier *e;
};
static bool memory_region_ioeventfd_before(MemoryRegionIoeventfd a,
@@ -181,9 +181,9 @@ static bool memory_region_ioeventfd_before(MemoryRegionIoeventfd a,
return false;
}
}
- if (a.fd < b.fd) {
+ if (a.e < b.e) {
return true;
- } else if (a.fd > b.fd) {
+ } else if (a.e > b.e) {
return false;
}
return false;
@@ -597,7 +597,7 @@ static void address_space_add_del_ioeventfds(AddressSpace *as,
.size = int128_get64(fd->addr.size),
};
MEMORY_LISTENER_CALL(eventfd_del, Forward, §ion,
- fd->match_data, fd->data, fd->fd);
+ fd->match_data, fd->data, fd->e);
++iold;
} else if (inew < fds_new_nb
&& (iold == fds_old_nb
@@ -610,7 +610,7 @@ static void address_space_add_del_ioeventfds(AddressSpace *as,
.size = int128_get64(fd->addr.size),
};
MEMORY_LISTENER_CALL(eventfd_add, Reverse, §ion,
- fd->match_data, fd->data, fd->fd);
+ fd->match_data, fd->data, fd->e);
++inew;
} else {
++iold;
@@ -1195,14 +1195,14 @@ void memory_region_add_eventfd(MemoryRegion *mr,
unsigned size,
bool match_data,
uint64_t data,
- int fd)
+ EventNotifier *e)
{
MemoryRegionIoeventfd mrfd = {
.addr.start = int128_make64(addr),
.addr.size = int128_make64(size),
.match_data = match_data,
.data = data,
- .fd = fd,
+ .e = e,
};
unsigned i;
@@ -1225,14 +1225,14 @@ void memory_region_del_eventfd(MemoryRegion *mr,
unsigned size,
bool match_data,
uint64_t data,
- int fd)
+ EventNotifier *e)
{
MemoryRegionIoeventfd mrfd = {
.addr.start = int128_make64(addr),
.addr.size = int128_make64(size),
.match_data = match_data,
.data = data,
- .fd = fd,
+ .e = e,
};
unsigned i;
diff --git a/memory.h b/memory.h
index 740c48e..bd1bbae 100644
--- a/memory.h
+++ b/memory.h
@@ -198,9 +198,9 @@ struct MemoryListener {
void (*log_global_start)(MemoryListener *listener);
void (*log_global_stop)(MemoryListener *listener);
void (*eventfd_add)(MemoryListener *listener, MemoryRegionSection *section,
- bool match_data, uint64_t data, int fd);
+ bool match_data, uint64_t data, EventNotifier *e);
void (*eventfd_del)(MemoryListener *listener, MemoryRegionSection *section,
- bool match_data, uint64_t data, int fd);
+ bool match_data, uint64_t data, EventNotifier *e);
/* Lower = earlier (during add), later (during del) */
unsigned priority;
MemoryRegion *address_space_filter;
@@ -541,7 +541,7 @@ void memory_region_add_eventfd(MemoryRegion *mr,
unsigned size,
bool match_data,
uint64_t data,
- int fd);
+ EventNotifier *e);
/**
* memory_region_del_eventfd: Cancel an eventfd.
@@ -561,7 +561,8 @@ void memory_region_del_eventfd(MemoryRegion *mr,
unsigned size,
bool match_data,
uint64_t data,
- int fd);
+ EventNotifier *e);
+
/**
* memory_region_add_subregion: Add a subregion to a container.
*
diff --git a/xen-all.c b/xen-all.c
index 59f2323..61def2e 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -560,13 +560,15 @@ static void xen_log_global_stop(MemoryListener *listener)
static void xen_eventfd_add(MemoryListener *listener,
MemoryRegionSection *section,
- bool match_data, uint64_t data, int fd)
+ bool match_data, uint64_t data,
+ EventNotifier *e)
{
}
static void xen_eventfd_del(MemoryListener *listener,
MemoryRegionSection *section,
- bool match_data, uint64_t data, int fd)
+ bool match_data, uint64_t data,
+ EventNotifier *e)
{
}
--
1.7.10.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH uq/master 7/9] event_notifier: add event_notifier_set_handler
2012-07-05 15:16 [Qemu-devel] [PATCH uq/master 0/9] remove event_notifier_get_fd from non-KVM code Paolo Bonzini
` (5 preceding siblings ...)
2012-07-05 15:16 ` [Qemu-devel] [PATCH uq/master 6/9] memory: pass EventNotifier, not eventfd Paolo Bonzini
@ 2012-07-05 15:16 ` Paolo Bonzini
2012-07-05 15:16 ` [Qemu-devel] [PATCH uq/master 8/9] virtio: move common ioeventfd handling out of virtio-pci Paolo Bonzini
` (2 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2012-07-05 15:16 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, mst, jan.kiszka, mtosatti, avi, anthony.perard,
stefano.stabellini
Win32 event notifiers are not file descriptors, so they will not be able
to use qemu_set_fd_handler. But even if for now we only have a POSIX
version of EventNotifier, we can add a specific function that wraps
the call.
The wrapper passes the EventNotifier as the opaque value so that it will
be used with container_of.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
event_notifier.c | 7 +++++++
event_notifier.h | 3 +++
2 files changed, 10 insertions(+)
diff --git a/event_notifier.c b/event_notifier.c
index 99c376c..2c207e1 100644
--- a/event_notifier.c
+++ b/event_notifier.c
@@ -12,6 +12,7 @@
#include "qemu-common.h"
#include "event_notifier.h"
+#include "qemu-char.h"
#ifdef CONFIG_EVENTFD
#include <sys/eventfd.h>
@@ -45,6 +46,12 @@ int event_notifier_get_fd(EventNotifier *e)
return e->fd;
}
+int event_notifier_set_handler(EventNotifier *e,
+ EventNotifierHandler *handler)
+{
+ return qemu_set_fd_handler(e->fd, (IOHandler *)handler, NULL, e);
+}
+
int event_notifier_set(EventNotifier *e)
{
uint64_t value = 1;
diff --git a/event_notifier.h b/event_notifier.h
index 30c12dd..e5888ed 100644
--- a/event_notifier.h
+++ b/event_notifier.h
@@ -19,11 +19,14 @@ struct EventNotifier {
int fd;
};
+typedef void EventNotifierHandler(EventNotifier *);
+
void event_notifier_init_fd(EventNotifier *, int fd);
int event_notifier_init(EventNotifier *, int active);
void event_notifier_cleanup(EventNotifier *);
int event_notifier_get_fd(EventNotifier *);
int event_notifier_set(EventNotifier *);
int event_notifier_test_and_clear(EventNotifier *);
+int event_notifier_set_handler(EventNotifier *, EventNotifierHandler *);
#endif
--
1.7.10.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH uq/master 8/9] virtio: move common ioeventfd handling out of virtio-pci
2012-07-05 15:16 [Qemu-devel] [PATCH uq/master 0/9] remove event_notifier_get_fd from non-KVM code Paolo Bonzini
` (6 preceding siblings ...)
2012-07-05 15:16 ` [Qemu-devel] [PATCH uq/master 7/9] event_notifier: add event_notifier_set_handler Paolo Bonzini
@ 2012-07-05 15:16 ` Paolo Bonzini
2012-07-05 15:16 ` [Qemu-devel] [PATCH uq/master 9/9] virtio: move common irqfd " Paolo Bonzini
2012-07-12 9:30 ` [Qemu-devel] [PATCH uq/master 0/9] remove event_notifier_get_fd from non-KVM code Avi Kivity
9 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2012-07-05 15:16 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, mst, jan.kiszka, mtosatti, avi, anthony.perard,
stefano.stabellini
All transports can use the same event handler for the ioeventfd, though
the exact setup (address/memory region) will be specific.
This lets virtio use event_notifier_set_handler.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/virtio-pci.c | 36 ++----------------------------------
hw/virtio.c | 22 ++++++++++++++++++++++
hw/virtio.h | 1 +
3 files changed, 25 insertions(+), 34 deletions(-)
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index a555728..36770fd 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -173,46 +173,18 @@ static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
__func__, r);
return r;
}
+ virtio_queue_set_host_notifier_fd_handler(vq, true);
memory_region_add_eventfd(&proxy->bar, VIRTIO_PCI_QUEUE_NOTIFY, 2,
true, n, notifier);
} else {
memory_region_del_eventfd(&proxy->bar, VIRTIO_PCI_QUEUE_NOTIFY, 2,
true, n, notifier);
- /* Handle the race condition where the guest kicked and we deassigned
- * before we got around to handling the kick.
- */
- if (event_notifier_test_and_clear(notifier)) {
- virtio_queue_notify_vq(vq);
- }
-
+ virtio_queue_set_host_notifier_fd_handler(vq, false);
event_notifier_cleanup(notifier);
}
return r;
}
-static void virtio_pci_host_notifier_read(void *opaque)
-{
- VirtQueue *vq = opaque;
- EventNotifier *n = virtio_queue_get_host_notifier(vq);
- if (event_notifier_test_and_clear(n)) {
- virtio_queue_notify_vq(vq);
- }
-}
-
-static void virtio_pci_set_host_notifier_fd_handler(VirtIOPCIProxy *proxy,
- int n, bool assign)
-{
- VirtQueue *vq = virtio_get_queue(proxy->vdev, n);
- EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
- if (assign) {
- qemu_set_fd_handler(event_notifier_get_fd(notifier),
- virtio_pci_host_notifier_read, NULL, vq);
- } else {
- qemu_set_fd_handler(event_notifier_get_fd(notifier),
- NULL, NULL, NULL);
- }
-}
-
static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
{
int n, r;
@@ -232,8 +204,6 @@ static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
if (r < 0) {
goto assign_error;
}
-
- virtio_pci_set_host_notifier_fd_handler(proxy, n, true);
}
proxy->ioeventfd_started = true;
return;
@@ -244,7 +214,6 @@ assign_error:
continue;
}
- virtio_pci_set_host_notifier_fd_handler(proxy, n, false);
r = virtio_pci_set_host_notifier_internal(proxy, n, false);
assert(r >= 0);
}
@@ -266,7 +235,6 @@ static void virtio_pci_stop_ioeventfd(VirtIOPCIProxy *proxy)
continue;
}
- virtio_pci_set_host_notifier_fd_handler(proxy, n, false);
r = virtio_pci_set_host_notifier_internal(proxy, n, false);
assert(r >= 0);
}
diff --git a/hw/virtio.c b/hw/virtio.c
index 168abe4..197edf0 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -988,6 +988,28 @@ EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq)
{
return &vq->guest_notifier;
}
+
+static void virtio_queue_host_notifier_read(EventNotifier *n)
+{
+ VirtQueue *vq = container_of(n, VirtQueue, host_notifier);
+ if (event_notifier_test_and_clear(n)) {
+ virtio_queue_notify_vq(vq);
+ }
+}
+
+void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign)
+{
+ if (assign) {
+ event_notifier_set_handler(&vq->host_notifier,
+ virtio_queue_host_notifier_read);
+ } else {
+ event_notifier_set_handler(&vq->host_notifier, NULL);
+ /* Test and clear notifier before after disabling event,
+ * in case poll callback didn't have time to run. */
+ virtio_queue_host_notifier_read(&vq->host_notifier);
+ }
+}
+
EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq)
{
return &vq->host_notifier;
diff --git a/hw/virtio.h b/hw/virtio.h
index 85aabe5..2949485 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -232,6 +232,7 @@ VirtQueue *virtio_get_queue(VirtIODevice *vdev, int n);
int virtio_queue_get_id(VirtQueue *vq);
EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq);
EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq);
+void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign);
void virtio_queue_notify_vq(VirtQueue *vq);
void virtio_irq(VirtQueue *vq);
#endif
--
1.7.10.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH uq/master 9/9] virtio: move common irqfd handling out of virtio-pci
2012-07-05 15:16 [Qemu-devel] [PATCH uq/master 0/9] remove event_notifier_get_fd from non-KVM code Paolo Bonzini
` (7 preceding siblings ...)
2012-07-05 15:16 ` [Qemu-devel] [PATCH uq/master 8/9] virtio: move common ioeventfd handling out of virtio-pci Paolo Bonzini
@ 2012-07-05 15:16 ` Paolo Bonzini
2012-07-12 9:30 ` [Qemu-devel] [PATCH uq/master 0/9] remove event_notifier_get_fd from non-KVM code Avi Kivity
9 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2012-07-05 15:16 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, mst, jan.kiszka, mtosatti, avi, anthony.perard,
stefano.stabellini
All transports can use the same event handler for the irqfd, though the
exact mechanics of the assignment will be specific. Note that there
are three states: handled by the kernel, handled in userspace, disabled.
This also lets virtio use event_notifier_set_handler.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/virtio-pci.c | 37 ++++++++++---------------------------
hw/virtio.c | 24 ++++++++++++++++++++++++
hw/virtio.h | 2 ++
kvm-all.c | 10 ++++++++++
kvm-stub.c | 10 ++++++++++
kvm.h | 2 ++
6 files changed, 58 insertions(+), 27 deletions(-)
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 36770fd..a66c946 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -496,25 +496,15 @@ static unsigned virtio_pci_get_features(void *opaque)
return proxy->host_features;
}
-static void virtio_pci_guest_notifier_read(void *opaque)
-{
- VirtQueue *vq = opaque;
- EventNotifier *n = virtio_queue_get_guest_notifier(vq);
- if (event_notifier_test_and_clear(n)) {
- virtio_irq(vq);
- }
-}
-
static int kvm_virtio_pci_vq_vector_use(VirtIOPCIProxy *proxy,
unsigned int queue_no,
unsigned int vector,
MSIMessage msg)
{
VirtQueue *vq = virtio_get_queue(proxy->vdev, queue_no);
+ EventNotifier *n = virtio_queue_get_guest_notifier(vq);
VirtIOIRQFD *irqfd = &proxy->vector_irqfd[vector];
- int fd, ret;
-
- fd = event_notifier_get_fd(virtio_queue_get_guest_notifier(vq));
+ int ret;
if (irqfd->users == 0) {
ret = kvm_irqchip_add_msi_route(kvm_state, msg);
@@ -525,7 +515,7 @@ static int kvm_virtio_pci_vq_vector_use(VirtIOPCIProxy *proxy,
}
irqfd->users++;
- ret = kvm_irqchip_add_irqfd(kvm_state, fd, irqfd->virq);
+ ret = kvm_irqchip_add_irq_notifier(kvm_state, n, irqfd->virq);
if (ret < 0) {
if (--irqfd->users == 0) {
kvm_irqchip_release_virq(kvm_state, irqfd->virq);
@@ -533,8 +523,7 @@ static int kvm_virtio_pci_vq_vector_use(VirtIOPCIProxy *proxy,
return ret;
}
- qemu_set_fd_handler(fd, NULL, NULL, NULL);
-
+ virtio_queue_set_guest_notifier_fd_handler(vq, true, true);
return 0;
}
@@ -543,19 +532,18 @@ static void kvm_virtio_pci_vq_vector_release(VirtIOPCIProxy *proxy,
unsigned int vector)
{
VirtQueue *vq = virtio_get_queue(proxy->vdev, queue_no);
+ EventNotifier *n = virtio_queue_get_guest_notifier(vq);
VirtIOIRQFD *irqfd = &proxy->vector_irqfd[vector];
- int fd, ret;
-
- fd = event_notifier_get_fd(virtio_queue_get_guest_notifier(vq));
+ int ret;
- ret = kvm_irqchip_remove_irqfd(kvm_state, fd, irqfd->virq);
+ ret = kvm_irqchip_remove_irq_notifier(kvm_state, n, irqfd->virq);
assert(ret == 0);
if (--irqfd->users == 0) {
kvm_irqchip_release_virq(kvm_state, irqfd->virq);
}
- qemu_set_fd_handler(fd, virtio_pci_guest_notifier_read, NULL, vq);
+ virtio_queue_set_guest_notifier_fd_handler(vq, true, false);
}
static int kvm_virtio_pci_vector_use(PCIDevice *dev, unsigned vector,
@@ -617,14 +605,9 @@ static int virtio_pci_set_guest_notifier(void *opaque, int n, bool assign)
if (r < 0) {
return r;
}
- qemu_set_fd_handler(event_notifier_get_fd(notifier),
- virtio_pci_guest_notifier_read, NULL, vq);
+ virtio_queue_set_guest_notifier_fd_handler(vq, true, false);
} else {
- qemu_set_fd_handler(event_notifier_get_fd(notifier),
- NULL, NULL, NULL);
- /* Test and clear notifier before closing it,
- * in case poll callback didn't have time to run. */
- virtio_pci_guest_notifier_read(vq);
+ virtio_queue_set_guest_notifier_fd_handler(vq, false, false);
event_notifier_cleanup(notifier);
}
diff --git a/hw/virtio.c b/hw/virtio.c
index 197edf0..d146f86 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -984,6 +984,30 @@ VirtQueue *virtio_get_queue(VirtIODevice *vdev, int n)
return vdev->vq + n;
}
+static void virtio_queue_guest_notifier_read(EventNotifier *n)
+{
+ VirtQueue *vq = container_of(n, VirtQueue, guest_notifier);
+ if (event_notifier_test_and_clear(n)) {
+ virtio_irq(vq);
+ }
+}
+
+void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
+ bool with_irqfd)
+{
+ if (assign && !with_irqfd) {
+ event_notifier_set_handler(&vq->guest_notifier,
+ virtio_queue_guest_notifier_read);
+ } else {
+ event_notifier_set_handler(&vq->guest_notifier, 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);
+ }
+}
+
EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq)
{
return &vq->guest_notifier;
diff --git a/hw/virtio.h b/hw/virtio.h
index 2949485..96f4dbb 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -231,6 +231,8 @@ void virtio_queue_set_last_avail_idx(VirtIODevice *vdev, int n, uint16_t idx);
VirtQueue *virtio_get_queue(VirtIODevice *vdev, int n);
int virtio_queue_get_id(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);
EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq);
void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign);
void virtio_queue_notify_vq(VirtQueue *vq);
diff --git a/kvm-all.c b/kvm-all.c
index 56f723e..ca0ce35 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1163,11 +1163,21 @@ int kvm_irqchip_add_irqfd(KVMState *s, int fd, int virq)
return kvm_irqchip_assign_irqfd(s, fd, virq, true);
}
+int kvm_irqchip_add_irq_notifier(KVMState *s, EventNotifier *n, int virq)
+{
+ return kvm_irqchip_add_irqfd(s, event_notifier_get_fd(n), virq);
+}
+
int kvm_irqchip_remove_irqfd(KVMState *s, int fd, int virq)
{
return kvm_irqchip_assign_irqfd(s, fd, virq, false);
}
+int kvm_irqchip_remove_irq_notifier(KVMState *s, EventNotifier *n, int virq)
+{
+ return kvm_irqchip_remove_irqfd(s, event_notifier_get_fd(n), virq);
+}
+
static int kvm_irqchip_create(KVMState *s)
{
QemuOptsList *list = qemu_find_opts("machine");
diff --git a/kvm-stub.c b/kvm-stub.c
index ec9a364..d23b11c 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -147,7 +147,17 @@ int kvm_irqchip_add_irqfd(KVMState *s, int fd, int virq)
return -ENOSYS;
}
+int kvm_irqchip_add_irq_notifier(KVMState *s, EventNotifier *n, int virq)
+{
+ return -ENOSYS;
+}
+
int kvm_irqchip_remove_irqfd(KVMState *s, int fd, int virq)
{
return -ENOSYS;
}
+
+int kvm_irqchip_remove_irq_notifier(KVMState *s, EventNotifier *n, int virq)
+{
+ return -ENOSYS;
+}
diff --git a/kvm.h b/kvm.h
index 9c7b0ea..99003f4 100644
--- a/kvm.h
+++ b/kvm.h
@@ -218,4 +218,6 @@ void kvm_irqchip_release_virq(KVMState *s, int virq);
int kvm_irqchip_add_irqfd(KVMState *s, int fd, int virq);
int kvm_irqchip_remove_irqfd(KVMState *s, int fd, int virq);
+int kvm_irqchip_add_irq_notifier(KVMState *s, EventNotifier *n, int virq);
+int kvm_irqchip_remove_irq_notifier(KVMState *s, EventNotifier *n, int virq);
#endif
--
1.7.10.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH uq/master 0/9] remove event_notifier_get_fd from non-KVM code
2012-07-05 15:16 [Qemu-devel] [PATCH uq/master 0/9] remove event_notifier_get_fd from non-KVM code Paolo Bonzini
` (8 preceding siblings ...)
2012-07-05 15:16 ` [Qemu-devel] [PATCH uq/master 9/9] virtio: move common irqfd " Paolo Bonzini
@ 2012-07-12 9:30 ` Avi Kivity
9 siblings, 0 replies; 16+ messages in thread
From: Avi Kivity @ 2012-07-12 9:30 UTC (permalink / raw)
To: Paolo Bonzini
Cc: kvm, mst, jan.kiszka, mtosatti, qemu-devel, anthony.perard,
stefano.stabellini
On 07/05/2012 06:16 PM, Paolo Bonzini wrote:
> This is part 1 of a three-part series that expands usage of EventNotifier
> in QEMU (including AIO and the main loop). I started working on this when
> playing with the threaded block layer; the part of that work that I hope
> will be in 1.2 is generalizing posix-aio-compat.c to be a generic portable
> thread pool + porting AIO to Win32 (part 2). On top of this, discard
> can be easily made asynchronous (part 3), which is a prerequisite for
> enabling it.
>
> This first part does the necessary changes for porting EventNotifier
> to Win32. The Win32 version will not have event_notifier_get_fd,
> and thus I want to remove all calls in portable code. Instead, all
> functions used in portable code after this series take an EventNotifier;
> KVM-specific implementations retrieve the file descriptor internally
> (these calls are in hw/ivshmem.c, hw/vhost.c, kvm-all.c).
>
> Patches 1 to 6 cover ivshmem and the memory API, first adding the
> required EventNotifier APIs and then using them. Patches 7 to 9 do the
> same with KVM ioeventfd and irqfd, refactoring transport-independent
> code in the process from virtio-pci to virtio (the two steps are a bit
> hard to separate).
Looks good, all that is needed is an explanation for patch 2.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 16+ messages in thread