Re: [Qemu-devel] [PATCH 11/23] memory: add ioeventfd support
On 07/25/2011 10:08 PM, Anthony Liguori wrote:
+static void as_memory_ioeventfd_add(AddressSpace *as,
MemoryRegionIoeventfd *fd)
+{
+int r;
+
+if (!fd-match_data || fd-addr.size != 4) {
+abort();
+}
+
+r = kvm_set_ioeventfd_mmio_long(fd-fd, fd-addr.start,
fd-data, true);
+if (r 0) {
+abort();
+}
asserts would be friendlier.
I thought asserts were disabled by default. But I see it isn't so; will
update.
I really dislike baking ioeventfd into this API. There is only one
user of ioeventfd in the tree.
Two: virtio-pci and ivshmem.
I worry that by having things like ioeventfd the API, we're making it
too difficult to side-step the API which prevents future optimizations.
I'd prefer virtio-pci to have ugliness in it where it circumvented the
layering vs. having such a device specific thing in generic code.
It's impossible (or at least, impossible without further information
from the API) to do this and retain correctness. Currently virtio-pci
is broken wrt bridges and overlapping BARs; it's probably also broken on
targets that bridge the I/O address space to MMIO.
With the memory API, this is fixed in a natural way by making the I/O
address space a subregion of the bridge which does the conversion; the
code will automatically add the needed offset and use MMIO ioeventfd
instead of portio.
On a more general note, I don't want this to be a lean and mean API that
throws any complexity to the users; instead I want to make writing
devices as simple as possible and own all the smarts.
(an example - undecoded address bits can be specified to the API which
then takes care of replication or shifting).
--
error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH 11/23] memory: add ioeventfd support
As with the rest of the memory API, the caller associates an eventfd
with an address, and the memory API takes care of registering or
unregistering when the address is made visible or invisible to the
guest.
Signed-off-by: Avi Kivity a...@redhat.com
---
memory.c | 230 ++
memory.h | 20 ++
2 files changed, 250 insertions(+), 0 deletions(-)
diff --git a/memory.c b/memory.c
index e4446a0..a5cde0c 100644
--- a/memory.c
+++ b/memory.c
@@ -15,6 +15,7 @@
#include exec-memory.h
#include ioport.h
#include bitops.h
+#include kvm.h
#include assert.h
typedef struct AddrRange AddrRange;
@@ -64,6 +65,50 @@ struct CoalescedMemoryRange {
QTAILQ_ENTRY(CoalescedMemoryRange) link;
};
+struct MemoryRegionIoeventfd {
+AddrRange addr;
+bool match_data;
+uint64_t data;
+int fd;
+};
+
+static bool memory_region_ioeventfd_before(MemoryRegionIoeventfd a,
+ MemoryRegionIoeventfd b)
+{
+if (a.addr.start b.addr.start) {
+return true;
+} else if (a.addr.start b.addr.start) {
+return false;
+} else if (a.addr.size b.addr.size) {
+return true;
+} else if (a.addr.size b.addr.size) {
+return false;
+} else if (a.match_data b.match_data) {
+return true;
+} else if (a.match_data b.match_data) {
+return false;
+} else if (a.match_data) {
+if (a.data b.data) {
+return true;
+} else if (a.data b.data) {
+return false;
+}
+}
+if (a.fd b.fd) {
+return true;
+} else if (a.fd b.fd) {
+return false;
+}
+return false;
+}
+
+static bool memory_region_ioeventfd_equal(MemoryRegionIoeventfd a,
+ MemoryRegionIoeventfd b)
+{
+return !memory_region_ioeventfd_before(a, b)
+ !memory_region_ioeventfd_before(b, a);
+}
+
typedef struct FlatRange FlatRange;
typedef struct FlatView FlatView;
@@ -92,6 +137,8 @@ struct AddressSpace {
const AddressSpaceOps *ops;
MemoryRegion *root;
FlatView current_map;
+int ioeventfd_nb;
+MemoryRegionIoeventfd *ioeventfds;
};
struct AddressSpaceOps {
@@ -99,6 +146,8 @@ struct AddressSpaceOps {
void (*range_del)(AddressSpace *as, FlatRange *fr);
void (*log_start)(AddressSpace *as, FlatRange *fr);
void (*log_stop)(AddressSpace *as, FlatRange *fr);
+void (*ioeventfd_add)(AddressSpace *as, MemoryRegionIoeventfd *fd);
+void (*ioeventfd_del)(AddressSpace *as, MemoryRegionIoeventfd *fd);
};
#define FOR_EACH_FLAT_RANGE(var, view) \
@@ -201,11 +250,37 @@ static void as_memory_log_stop(AddressSpace *as,
FlatRange *fr)
cpu_physical_log_stop(fr-addr.start, fr-addr.size);
}
+static void as_memory_ioeventfd_add(AddressSpace *as, MemoryRegionIoeventfd
*fd)
+{
+int r;
+
+if (!fd-match_data || fd-addr.size != 4) {
+abort();
+}
+
+r = kvm_set_ioeventfd_mmio_long(fd-fd, fd-addr.start, fd-data, true);
+if (r 0) {
+abort();
+}
+}
+
+static void as_memory_ioeventfd_del(AddressSpace *as, MemoryRegionIoeventfd
*fd)
+{
+int r;
+
+r = kvm_set_ioeventfd_mmio_long(fd-fd, fd-addr.start, fd-data, false);
+if (r 0) {
+abort();
+}
+}
+
static const AddressSpaceOps address_space_ops_memory = {
.range_add = as_memory_range_add,
.range_del = as_memory_range_del,
.log_start = as_memory_log_start,
.log_stop = as_memory_log_stop,
+.ioeventfd_add = as_memory_ioeventfd_add,
+.ioeventfd_del = as_memory_ioeventfd_del,
};
static AddressSpace address_space_memory = {
@@ -281,9 +356,35 @@ static void as_io_range_del(AddressSpace *as, FlatRange
*fr)
isa_unassign_ioport(fr-addr.start, fr-addr.size);
}
+static void as_io_ioeventfd_add(AddressSpace *as, MemoryRegionIoeventfd *fd)
+{
+int r;
+
+if (!fd-match_data || fd-addr.size != 2) {
+abort();
+}
+
+r = kvm_set_ioeventfd_pio_word(fd-fd, fd-addr.start, fd-data, true);
+if (r 0) {
+abort();
+}
+}
+
+static void as_io_ioeventfd_del(AddressSpace *as, MemoryRegionIoeventfd *fd)
+{
+int r;
+
+r = kvm_set_ioeventfd_pio_word(fd-fd, fd-addr.start, fd-data, false);
+if (r 0) {
+abort();
+}
+}
+
static const AddressSpaceOps address_space_ops_io = {
.range_add = as_io_range_add,
.range_del = as_io_range_del,
+.ioeventfd_add = as_io_ioeventfd_add,
+.ioeventfd_del = as_io_ioeventfd_del,
};
static AddressSpace address_space_io = {
@@ -382,6 +483,69 @@ static FlatView generate_memory_topology(MemoryRegion *mr)
return view;
}
+static void address_space_add_del_ioeventfds(AddressSpace *as,
+ MemoryRegionIoeventfd *fds_new,
+ unsigned fds_new_nb,
+ MemoryRegionIoeventfd
Re: [Qemu-devel] [PATCH 11/23] memory: add ioeventfd support
On Mon, 25 Jul 2011, Avi Kivity wrote:
As with the rest of the memory API, the caller associates an eventfd
with an address, and the memory API takes care of registering or
unregistering when the address is made visible or invisible to the
guest.
Signed-off-by: Avi Kivity a...@redhat.com
[..snip..]
+static bool memory_region_ioeventfd_before(MemoryRegionIoeventfd a,
+ MemoryRegionIoeventfd b)
+{
+if (a.addr.start b.addr.start) {
+return true;
+} else if (a.addr.start b.addr.start) {
+return false;
+} else if (a.addr.size b.addr.size) {
+return true;
+} else if (a.addr.size b.addr.size) {
+return false;
+} else if (a.match_data b.match_data) {
+return true;
+} else if (a.match_data b.match_data) {
+return false;
+} else if (a.match_data) {
+if (a.data b.data) {
+return true;
+} else if (a.data b.data) {
+return false;
+}
+}
+if (a.fd b.fd) {
+return true;
+} else if (a.fd b.fd) {
+return false;
+}
Braces..
+return false;
+}
+
[..snip..]
--
mailto:av1...@comtv.ru
Re: [Qemu-devel] [PATCH 11/23] memory: add ioeventfd support
On 07/25/2011 06:16 PM, malc wrote:
On Mon, 25 Jul 2011, Avi Kivity wrote:
As with the rest of the memory API, the caller associates an eventfd
with an address, and the memory API takes care of registering or
unregistering when the address is made visible or invisible to the
guest.
Signed-off-by: Avi Kivitya...@redhat.com
[..snip..]
+static bool memory_region_ioeventfd_before(MemoryRegionIoeventfd a,
+ MemoryRegionIoeventfd b)
+{
+if (a.addr.start b.addr.start) {
+return true;
+} else if (a.addr.start b.addr.start) {
+return false;
+} else if (a.addr.size b.addr.size) {
+return true;
+} else if (a.addr.size b.addr.size) {
+return false;
+} else if (a.match_data b.match_data) {
+return true;
+} else if (a.match_data b.match_data) {
+return false;
+} else if (a.match_data) {
+if (a.data b.data) {
+return true;
+} else if (a.data b.data) {
+return false;
+}
+}
+if (a.fd b.fd) {
+return true;
+} else if (a.fd b.fd) {
+return false;
+}
Braces..
Where? add? remove?
--
error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 11/23] memory: add ioeventfd support
On Mon, 25 Jul 2011, Avi Kivity wrote:
On 07/25/2011 06:16 PM, malc wrote:
On Mon, 25 Jul 2011, Avi Kivity wrote:
As with the rest of the memory API, the caller associates an eventfd
with an address, and the memory API takes care of registering or
unregistering when the address is made visible or invisible to the
guest.
Signed-off-by: Avi Kivitya...@redhat.com
[..snip..]
+static bool memory_region_ioeventfd_before(MemoryRegionIoeventfd a,
+ MemoryRegionIoeventfd b)
+{
+if (a.addr.start b.addr.start) {
+return true;
+} else if (a.addr.start b.addr.start) {
+return false;
+} else if (a.addr.size b.addr.size) {
+return true;
+} else if (a.addr.size b.addr.size) {
+return false;
+} else if (a.match_data b.match_data) {
+return true;
+} else if (a.match_data b.match_data) {
+return false;
+} else if (a.match_data) {
+if (a.data b.data) {
+return true;
+} else if (a.data b.data) {
+return false;
+}
+}
+if (a.fd b.fd) {
+return true;
+} else {
if (a.fd b.fd) {
+return false;
+}
}
Braces..
Where? add? remove?
You authored the coding_style document btw.
--
mailto:av1...@comtv.ru
Re: [Qemu-devel] [PATCH 11/23] memory: add ioeventfd support
On 07/25/2011 06:22 PM, malc wrote:
+}
+if (a.fd b.fd) {
+return true;
+} else {
if (a.fd b.fd) {
+return false;
+}
}
CODING_STYLE permits braceless else if.
Braces..
Where? add? remove?
You authored the coding_style document btw.
It's CODING_STYLE.
--
error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 11/23] memory: add ioeventfd support
On Mon, 25 Jul 2011, Avi Kivity wrote:
On 07/25/2011 06:22 PM, malc wrote:
+}
+if (a.fd b.fd) {
+return true;
+} else {
if (a.fd b.fd) {
+return false;
+}
}
CODING_STYLE permits braceless else if.
The example uses it, the text however suggests it's illegal.
[..snip..]
--
mailto:av1...@comtv.ru
Re: [Qemu-devel] [PATCH 11/23] memory: add ioeventfd support
On 07/25/2011 06:38 PM, malc wrote:
On Mon, 25 Jul 2011, Avi Kivity wrote:
On 07/25/2011 06:22 PM, malc wrote:
+}
+if (a.fdb.fd) {
+return true;
+} else {
if (a.fdb.fd) {
+return false;
+}
}
CODING_STYLE permits braceless else if.
The example uses it, the text however suggests it's illegal.
I'll post a patch. It's perfectly reasonable IMO, and there ~1800
instances in the tree.
--
error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 11/23] memory: add ioeventfd support
On 07/25/2011 09:02 AM, Avi Kivity wrote:
As with the rest of the memory API, the caller associates an eventfd
with an address, and the memory API takes care of registering or
unregistering when the address is made visible or invisible to the
guest.
Signed-off-by: Avi Kivitya...@redhat.com
---
memory.c | 230 ++
memory.h | 20 ++
2 files changed, 250 insertions(+), 0 deletions(-)
diff --git a/memory.c b/memory.c
index e4446a0..a5cde0c 100644
--- a/memory.c
+++ b/memory.c
@@ -15,6 +15,7 @@
#include exec-memory.h
#include ioport.h
#include bitops.h
+#include kvm.h
#includeassert.h
typedef struct AddrRange AddrRange;
@@ -64,6 +65,50 @@ struct CoalescedMemoryRange {
QTAILQ_ENTRY(CoalescedMemoryRange) link;
};
+struct MemoryRegionIoeventfd {
+AddrRange addr;
+bool match_data;
+uint64_t data;
+int fd;
+};
+
+static bool memory_region_ioeventfd_before(MemoryRegionIoeventfd a,
+ MemoryRegionIoeventfd b)
+{
+if (a.addr.start b.addr.start) {
+return true;
+} else if (a.addr.start b.addr.start) {
+return false;
+} else if (a.addr.size b.addr.size) {
+return true;
+} else if (a.addr.size b.addr.size) {
+return false;
+} else if (a.match_data b.match_data) {
+return true;
+} else if (a.match_data b.match_data) {
+return false;
+} else if (a.match_data) {
+if (a.data b.data) {
+return true;
+} else if (a.data b.data) {
+return false;
+}
+}
+if (a.fd b.fd) {
+return true;
+} else if (a.fd b.fd) {
+return false;
+}
+return false;
+}
+
+static bool memory_region_ioeventfd_equal(MemoryRegionIoeventfd a,
+ MemoryRegionIoeventfd b)
+{
+return !memory_region_ioeventfd_before(a, b)
+ !memory_region_ioeventfd_before(b, a);
+}
+
typedef struct FlatRange FlatRange;
typedef struct FlatView FlatView;
@@ -92,6 +137,8 @@ struct AddressSpace {
const AddressSpaceOps *ops;
MemoryRegion *root;
FlatView current_map;
+int ioeventfd_nb;
+MemoryRegionIoeventfd *ioeventfds;
};
struct AddressSpaceOps {
@@ -99,6 +146,8 @@ struct AddressSpaceOps {
void (*range_del)(AddressSpace *as, FlatRange *fr);
void (*log_start)(AddressSpace *as, FlatRange *fr);
void (*log_stop)(AddressSpace *as, FlatRange *fr);
+void (*ioeventfd_add)(AddressSpace *as, MemoryRegionIoeventfd *fd);
+void (*ioeventfd_del)(AddressSpace *as, MemoryRegionIoeventfd *fd);
};
#define FOR_EACH_FLAT_RANGE(var, view) \
@@ -201,11 +250,37 @@ static void as_memory_log_stop(AddressSpace *as,
FlatRange *fr)
cpu_physical_log_stop(fr-addr.start, fr-addr.size);
}
+static void as_memory_ioeventfd_add(AddressSpace *as, MemoryRegionIoeventfd
*fd)
+{
+int r;
+
+if (!fd-match_data || fd-addr.size != 4) {
+abort();
+}
+
+r = kvm_set_ioeventfd_mmio_long(fd-fd, fd-addr.start, fd-data, true);
+if (r 0) {
+abort();
+}
asserts would be friendlier.
I really dislike baking ioeventfd into this API. There is only one user
of ioeventfd in the tree.
I worry that by having things like ioeventfd the API, we're making it
too difficult to side-step the API which prevents future optimizations.
I'd prefer virtio-pci to have ugliness in it where it circumvented the
layering vs. having such a device specific thing in generic code.
Regards,
Anthony Liguori
+}
+
+static void as_memory_ioeventfd_del(AddressSpace *as, MemoryRegionIoeventfd
*fd)
+{
+int r;
+
+r = kvm_set_ioeventfd_mmio_long(fd-fd, fd-addr.start, fd-data, false);
+if (r 0) {
+abort();
+}
+}
+
static const AddressSpaceOps address_space_ops_memory = {
.range_add = as_memory_range_add,
.range_del = as_memory_range_del,
.log_start = as_memory_log_start,
.log_stop = as_memory_log_stop,
+.ioeventfd_add = as_memory_ioeventfd_add,
+.ioeventfd_del = as_memory_ioeventfd_del,
};
static AddressSpace address_space_memory = {
@@ -281,9 +356,35 @@ static void as_io_range_del(AddressSpace *as, FlatRange
*fr)
isa_unassign_ioport(fr-addr.start, fr-addr.size);
}
+static void as_io_ioeventfd_add(AddressSpace *as, MemoryRegionIoeventfd *fd)
+{
+int r;
+
+if (!fd-match_data || fd-addr.size != 2) {
+abort();
+}
+
+r = kvm_set_ioeventfd_pio_word(fd-fd, fd-addr.start, fd-data, true);
+if (r 0) {
+abort();
+}
+}
+
+static void as_io_ioeventfd_del(AddressSpace *as, MemoryRegionIoeventfd *fd)
+{
+int r;
+
+r = kvm_set_ioeventfd_pio_word(fd-fd, fd-addr.start, fd-data, false);
+if (r 0) {
+abort();
+}
+}
+
static const AddressSpaceOps address_space_ops_io = {
.range_add = as_io_range_add,
.range_del = as_io_range_del,
+
