| Message ID | f813bb55a79a8a917aaf913003548c20b2c43964.1364979441.git.mst@redhat.com |
|---|---|
| State | New |
| Headers | show |
Il 03/04/2013 10:59, Michael S. Tsirkin ha scritto: > This device is used for kvm unit tests, > currently it supports testing performance of ioeventfd. > Using updated kvm unittest, here's an example output: > mmio-no-eventfd:pci-mem 8796 > mmio-wildcard-eventfd:pci-mem 3609 > mmio-datamatch-eventfd:pci-mem 3685 > portio-no-eventfd:pci-io 5287 > portio-wildcard-eventfd:pci-io 1762 > portio-datamatch-eventfd:pci-io 1777 There is too much magic in this device, that is shared between the testcase and the device. I think this device should be structured differently. For example, you could have a single EventNotifier and 3 BARs: 1) BAR0/BAR1 are as in your device. All they do is count writes and report them into a register in BAR2. 2) BAR2 has a control register for BAR0 and one for BAR1, that enables/disables ioeventfd (bit 0 = enable/disable, bit 1 = wildcard/datamatch, bits 2-3 = log2(width)). It also has a zero-on-read counter that is incremented for each write to BAR0 or BAR1, and clears the EventNotifier. Paolo > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > hw/i386/Makefile.objs | 2 +- > hw/pci-testdev.c | 306 ++++++++++++++++++++++++++++++++++++++++++++++++++ > hw/pci/pci.h | 1 + > 3 files changed, 308 insertions(+), 1 deletion(-) > create mode 100644 hw/pci-testdev.c > > diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs > index a78c0b2..7497e7a 100644 > --- a/hw/i386/Makefile.objs > +++ b/hw/i386/Makefile.objs > @@ -11,7 +11,7 @@ obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o > obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o xen_pt_msi.o > obj-y += kvm/ > obj-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o > -obj-y += pc-testdev.o > +obj-y += pc-testdev.o pci-testdev.o > > obj-y := $(addprefix ../,$(obj-y)) > > diff --git a/hw/pci-testdev.c b/hw/pci-testdev.c > new file mode 100644 > index 0000000..9486624 > --- /dev/null > +++ b/hw/pci-testdev.c > @@ -0,0 +1,306 @@ > +#include "hw/hw.h" > +#include "hw/pci/pci.h" > +#include "qemu/event_notifier.h" > +#include "qemu/osdep.h" > + > +typedef struct PCITestDevHdr { > + uint8_t test; > + uint8_t width; > + uint8_t pad0[2]; > + uint32_t offset; > + uint8_t data; > + uint8_t pad1[3]; > + uint32_t count; > + uint8_t name[]; > +} PCITestDevHdr; > + > +typedef struct IOTest { > + MemoryRegion *mr; > + EventNotifier notifier; > + bool hasnotifier; > + unsigned size; > + bool match_data; > + PCITestDevHdr *hdr; > + unsigned bufsize; > +} IOTest; > + > +#define IOTEST_DATAMATCH 0xFA > +#define IOTEST_NOMATCH 0xCE > + > +#define IOTEST_IOSIZE 128 > +#define IOTEST_MEMSIZE 2048 > + > +static const char *iotest_test[] = { > + "no-eventfd", > + "wildcard-eventfd", > + "datamatch-eventfd" > +}; > + > +static const char *iotest_type[] = { > + "mmio", > + "portio" > +}; > + > +#define IOTEST_TEST(i) (iotest_test[((i) % ARRAY_SIZE(iotest_test))]) > +#define IOTEST_TYPE(i) (iotest_type[((i) / ARRAY_SIZE(iotest_test))]) > +#define IOTEST_MAX_TEST (ARRAY_SIZE(iotest_test)) > +#define IOTEST_MAX_TYPE (ARRAY_SIZE(iotest_type)) > +#define IOTEST_MAX (IOTEST_MAX_TEST * IOTEST_MAX_TYPE) > + > +enum { > + IOTEST_ACCESS_NAME, > + IOTEST_ACCESS_DATA, > + IOTEST_ACCESS_MAX, > +}; > + > +#define IOTEST_ACCESS_TYPE uint8_t > +#define IOTEST_ACCESS_WIDTH (sizeof(uint8_t)) > + > +typedef struct PCITestDevState { > + PCIDevice dev; > + MemoryRegion mmio; > + MemoryRegion portio; > + IOTest *tests; > + int current; > +} PCITestDevState; > + > +#define IOTEST_IS_MEM(i) (strcmp(IOTEST_TYPE(i), "portio")) > +#define IOTEST_REGION(d, i) (IOTEST_IS_MEM(i) ? &(d)->mmio : &(d)->portio) > +#define IOTEST_SIZE(i) (IOTEST_IS_MEM(i) ? IOTEST_MEMSIZE : IOTEST_IOSIZE) > +#define IOTEST_PCI_BAR(i) (IOTEST_IS_MEM(i) ? PCI_BASE_ADDRESS_SPACE_MEMORY : \ > + PCI_BASE_ADDRESS_SPACE_IO) > + > +static int pci_testdev_start(IOTest *test) > +{ > + test->hdr->count = 0; > + if (!test->hasnotifier) { > + return 0; > + } > + event_notifier_test_and_clear(&test->notifier); > + memory_region_add_eventfd(test->mr, > + le32_to_cpu(test->hdr->offset), > + test->size, > + test->match_data, > + test->hdr->data, > + &test->notifier); > + return 0; > +} > + > +static void pci_testdev_stop(IOTest *test) > +{ > + if (!test->hasnotifier) { > + return; > + } > + memory_region_del_eventfd(test->mr, > + le32_to_cpu(test->hdr->offset), > + test->size, > + test->match_data, > + test->hdr->data, > + &test->notifier); > +} > + > +static void > +pci_testdev_reset(PCITestDevState *d) > +{ > + if (d->current == -1) { > + return; > + } > + pci_testdev_stop(&d->tests[d->current]); > + d->current = -1; > +} > + > +static void pci_testdev_inc(IOTest *test, unsigned inc) > +{ > + uint32_t c = le32_to_cpu(test->hdr->count); > + test->hdr->count = cpu_to_le32(c + inc); > +} > + > +static void > +pci_testdev_write(void *opaque, hwaddr addr, uint64_t val, > + unsigned size, int type) > +{ > + PCITestDevState *d = opaque; > + IOTest *test; > + int t, r; > + > + if (addr == offsetof(PCITestDevHdr, test)) { > + pci_testdev_reset(d); > + if (val >= IOTEST_MAX_TEST) { > + return; > + } > + t = type * IOTEST_MAX_TEST + val; > + r = pci_testdev_start(&d->tests[t]); > + if (r < 0) { > + return; > + } > + d->current = t; > + return; > + } > + if (d->current < 0) { > + return; > + } > + test = &d->tests[d->current]; > + if (addr != le32_to_cpu(test->hdr->offset)) { > + return; > + } > + if (test->match_data && test->size != size) { > + return; > + } > + if (test->match_data && val != test->hdr->data) { > + return; > + } > + pci_testdev_inc(test, 1); > +} > + > +static uint64_t > +pci_testdev_read(void *opaque, hwaddr addr, unsigned size) > +{ > + PCITestDevState *d = opaque; > + const char *buf; > + IOTest *test; > + if (d->current < 0) { > + return 0; > + } > + test = &d->tests[d->current]; > + buf = (const char *)test->hdr; > + if (addr + size >= test->bufsize) { > + return 0; > + } > + if (test->hasnotifier) { > + event_notifier_test_and_clear(&test->notifier); > + } > + return buf[addr]; > +} > + > +static void > +pci_testdev_mmio_write(void *opaque, hwaddr addr, uint64_t val, > + unsigned size) > +{ > + pci_testdev_write(opaque, addr, val, size, 0); > +} > + > +static void > +pci_testdev_pio_write(void *opaque, hwaddr addr, uint64_t val, > + unsigned size) > +{ > + pci_testdev_write(opaque, addr, val, size, 1); > +} > + > +static const MemoryRegionOps pci_testdev_mmio_ops = { > + .read = pci_testdev_read, > + .write = pci_testdev_mmio_write, > + .endianness = DEVICE_LITTLE_ENDIAN, > + .impl = { > + .min_access_size = 1, > + .max_access_size = 1, > + }, > +}; > + > +static const MemoryRegionOps pci_testdev_pio_ops = { > + .read = pci_testdev_read, > + .write = pci_testdev_pio_write, > + .endianness = DEVICE_LITTLE_ENDIAN, > + .impl = { > + .min_access_size = 1, > + .max_access_size = 1, > + }, > +}; > + > +static int pci_testdev_init(PCIDevice *pci_dev) > +{ > + PCITestDevState *d = DO_UPCAST(PCITestDevState, dev, pci_dev); > + uint8_t *pci_conf; > + char *name; > + int r, i; > + > + pci_conf = d->dev.config; > + > + pci_conf[PCI_INTERRUPT_PIN] = 0; /* no interrupt pin */ > + > + memory_region_init_io(&d->mmio, &pci_testdev_mmio_ops, d, > + "pci-testdev-mmio", IOTEST_MEMSIZE * 2); > + memory_region_init_io(&d->portio, &pci_testdev_pio_ops, d, > + "pci-testdev-portio", IOTEST_IOSIZE * 2); > + pci_register_bar(&d->dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &d->mmio); > + pci_register_bar(&d->dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &d->portio); > + > + d->current = -1; > + d->tests = g_malloc0(IOTEST_MAX * sizeof *d->tests); > + for (i = 0; i < IOTEST_MAX; ++i) { > + IOTest *test = &d->tests[i]; > + name = g_strdup_printf("%s-%s", IOTEST_TYPE(i), IOTEST_TEST(i)); > + test->bufsize = sizeof(PCITestDevHdr) + strlen(name) + 1; > + test->hdr = g_malloc0(test->bufsize); > + memcpy(test->hdr->name, name, strlen(name) + 1); > + g_free(name); > + test->hdr->offset = cpu_to_le32(IOTEST_SIZE(i) + i * IOTEST_ACCESS_WIDTH); > + test->size = IOTEST_ACCESS_WIDTH; > + test->match_data = strcmp(IOTEST_TEST(i), "wildcard-eventfd"); > + test->hdr->test = i; > + test->hdr->data = test->match_data ? IOTEST_DATAMATCH : IOTEST_NOMATCH; > + test->hdr->width = IOTEST_ACCESS_WIDTH; > + test->mr = IOTEST_REGION(d, i); > + if (!strcmp(IOTEST_TEST(i), "no-eventfd")) { > + test->hasnotifier = false; > + continue; > + } > + r = event_notifier_init(&test->notifier, 0); > + assert(r >= 0); > + test->hasnotifier = true; > + } > + > + return 0; > +} > + > +static void > +pci_testdev_uninit(PCIDevice *dev) > +{ > + PCITestDevState *d = DO_UPCAST(PCITestDevState, dev, dev); > + int i; > + > + pci_testdev_reset(d); > + for (i = 0; i < IOTEST_MAX; ++i) { > + if (d->tests[i].hasnotifier) { > + event_notifier_cleanup(&d->tests[i].notifier); > + } > + g_free(d->tests[i].hdr); > + } > + g_free(d->tests); > + memory_region_destroy(&d->mmio); > + memory_region_destroy(&d->portio); > +} > + > +static void qdev_pci_testdev_reset(DeviceState *dev) > +{ > + PCITestDevState *d = DO_UPCAST(PCITestDevState, dev.qdev, dev); > + pci_testdev_reset(d); > +} > + > +static void pci_testdev_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > + > + k->init = pci_testdev_init; > + k->exit = pci_testdev_uninit; > + k->vendor_id = PCI_VENDOR_ID_REDHAT; > + k->device_id = PCI_DEVICE_ID_REDHAT_TEST; > + k->revision = 0x00; > + k->class_id = PCI_CLASS_OTHERS; > + dc->desc = "PCI Test Device"; > + dc->reset = qdev_pci_testdev_reset; > +} > + > +static const TypeInfo pci_testdev_info = { > + .name = "pci-testdev", > + .parent = TYPE_PCI_DEVICE, > + .instance_size = sizeof(PCITestDevState), > + .class_init = pci_testdev_class_init, > +}; > + > +static void pci_testdev_register_types(void) > +{ > + type_register_static(&pci_testdev_info); > +} > + > +type_init(pci_testdev_register_types) > diff --git a/hw/pci/pci.h b/hw/pci/pci.h > index 774369c..d81198c 100644 > --- a/hw/pci/pci.h > +++ b/hw/pci/pci.h > @@ -84,6 +84,7 @@ > #define PCI_DEVICE_ID_REDHAT_SERIAL 0x0002 > #define PCI_DEVICE_ID_REDHAT_SERIAL2 0x0003 > #define PCI_DEVICE_ID_REDHAT_SERIAL4 0x0004 > +#define PCI_DEVICE_ID_REDHAT_TEST 0x0005 > #define PCI_DEVICE_ID_REDHAT_QXL 0x0100 > > #define FMT_PCIBUS PRIx64 >
On Wed, Apr 03, 2013 at 11:28:55AM +0200, Paolo Bonzini wrote: > Il 03/04/2013 10:59, Michael S. Tsirkin ha scritto: > > This device is used for kvm unit tests, > > currently it supports testing performance of ioeventfd. > > Using updated kvm unittest, here's an example output: > > mmio-no-eventfd:pci-mem 8796 > > mmio-wildcard-eventfd:pci-mem 3609 > > mmio-datamatch-eventfd:pci-mem 3685 > > portio-no-eventfd:pci-io 5287 > > portio-wildcard-eventfd:pci-io 1762 > > portio-datamatch-eventfd:pci-io 1777 > > There is too much magic in this device, that is shared between the > testcase and the device. Paolo, it looks like there's some misunderstanding. There's no magic. Each BAR has the structure specified in both test and qemu: struct pci_test_dev_hdr { uint8_t test; -- test number uint8_t width; - how much to write uint8_t pad0[2]; uint32_t offset; - where to write uint32_t data; - what to write uint32_t count; - incremented on write uint8_t name[]; -- test name for debugging }; What you propose below has a bit less features if you add these you will get pretty much same thing. > I think this device should be structured > differently. For example, you could have a single EventNotifier and 3 BARs: > > 1) BAR0/BAR1 are as in your device. All they do is count writes and > report them into a register in BAR2. > > 2) BAR2 has a control register for BAR0 and one for BAR1, that > enables/disables ioeventfd (bit 0 = enable/disable, bit 1 = > wildcard/datamatch, bits 2-3 = log2(width)). It also has a zero-on-read > counter that is incremented for each write to BAR0 or BAR1, and clears > the EventNotifier. > > Paolo This is pretty close to what I have, only I put control at the beginning of each BAR, and I support arbitrary length writes guest side, and I allow testing both datamatch and non data match. I also do not want guest to control things like ioeventfd assignment. These are host side things and while you could argue security does not matter for a test device, I think it will set a bad example. For this reason, this is structured like virtio: host tells guest what to write and where. So any new benchmark can be added pretty much on qemu side without changing test. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > --- > > hw/i386/Makefile.objs | 2 +- > > hw/pci-testdev.c | 306 ++++++++++++++++++++++++++++++++++++++++++++++++++ > > hw/pci/pci.h | 1 + > > 3 files changed, 308 insertions(+), 1 deletion(-) > > create mode 100644 hw/pci-testdev.c > > > > diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs > > index a78c0b2..7497e7a 100644 > > --- a/hw/i386/Makefile.objs > > +++ b/hw/i386/Makefile.objs > > @@ -11,7 +11,7 @@ obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o > > obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o xen_pt_msi.o > > obj-y += kvm/ > > obj-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o > > -obj-y += pc-testdev.o > > +obj-y += pc-testdev.o pci-testdev.o > > > > obj-y := $(addprefix ../,$(obj-y)) > > > > diff --git a/hw/pci-testdev.c b/hw/pci-testdev.c > > new file mode 100644 > > index 0000000..9486624 > > --- /dev/null > > +++ b/hw/pci-testdev.c > > @@ -0,0 +1,306 @@ > > +#include "hw/hw.h" > > +#include "hw/pci/pci.h" > > +#include "qemu/event_notifier.h" > > +#include "qemu/osdep.h" > > + > > +typedef struct PCITestDevHdr { > > + uint8_t test; > > + uint8_t width; > > + uint8_t pad0[2]; > > + uint32_t offset; > > + uint8_t data; > > + uint8_t pad1[3]; > > + uint32_t count; > > + uint8_t name[]; > > +} PCITestDevHdr; > > + > > +typedef struct IOTest { > > + MemoryRegion *mr; > > + EventNotifier notifier; > > + bool hasnotifier; > > + unsigned size; > > + bool match_data; > > + PCITestDevHdr *hdr; > > + unsigned bufsize; > > +} IOTest; > > + > > +#define IOTEST_DATAMATCH 0xFA > > +#define IOTEST_NOMATCH 0xCE > > + > > +#define IOTEST_IOSIZE 128 > > +#define IOTEST_MEMSIZE 2048 > > + > > +static const char *iotest_test[] = { > > + "no-eventfd", > > + "wildcard-eventfd", > > + "datamatch-eventfd" > > +}; > > + > > +static const char *iotest_type[] = { > > + "mmio", > > + "portio" > > +}; > > + > > +#define IOTEST_TEST(i) (iotest_test[((i) % ARRAY_SIZE(iotest_test))]) > > +#define IOTEST_TYPE(i) (iotest_type[((i) / ARRAY_SIZE(iotest_test))]) > > +#define IOTEST_MAX_TEST (ARRAY_SIZE(iotest_test)) > > +#define IOTEST_MAX_TYPE (ARRAY_SIZE(iotest_type)) > > +#define IOTEST_MAX (IOTEST_MAX_TEST * IOTEST_MAX_TYPE) > > + > > +enum { > > + IOTEST_ACCESS_NAME, > > + IOTEST_ACCESS_DATA, > > + IOTEST_ACCESS_MAX, > > +}; > > + > > +#define IOTEST_ACCESS_TYPE uint8_t > > +#define IOTEST_ACCESS_WIDTH (sizeof(uint8_t)) > > + > > +typedef struct PCITestDevState { > > + PCIDevice dev; > > + MemoryRegion mmio; > > + MemoryRegion portio; > > + IOTest *tests; > > + int current; > > +} PCITestDevState; > > + > > +#define IOTEST_IS_MEM(i) (strcmp(IOTEST_TYPE(i), "portio")) > > +#define IOTEST_REGION(d, i) (IOTEST_IS_MEM(i) ? &(d)->mmio : &(d)->portio) > > +#define IOTEST_SIZE(i) (IOTEST_IS_MEM(i) ? IOTEST_MEMSIZE : IOTEST_IOSIZE) > > +#define IOTEST_PCI_BAR(i) (IOTEST_IS_MEM(i) ? PCI_BASE_ADDRESS_SPACE_MEMORY : \ > > + PCI_BASE_ADDRESS_SPACE_IO) > > + > > +static int pci_testdev_start(IOTest *test) > > +{ > > + test->hdr->count = 0; > > + if (!test->hasnotifier) { > > + return 0; > > + } > > + event_notifier_test_and_clear(&test->notifier); > > + memory_region_add_eventfd(test->mr, > > + le32_to_cpu(test->hdr->offset), > > + test->size, > > + test->match_data, > > + test->hdr->data, > > + &test->notifier); > > + return 0; > > +} > > + > > +static void pci_testdev_stop(IOTest *test) > > +{ > > + if (!test->hasnotifier) { > > + return; > > + } > > + memory_region_del_eventfd(test->mr, > > + le32_to_cpu(test->hdr->offset), > > + test->size, > > + test->match_data, > > + test->hdr->data, > > + &test->notifier); > > +} > > + > > +static void > > +pci_testdev_reset(PCITestDevState *d) > > +{ > > + if (d->current == -1) { > > + return; > > + } > > + pci_testdev_stop(&d->tests[d->current]); > > + d->current = -1; > > +} > > + > > +static void pci_testdev_inc(IOTest *test, unsigned inc) > > +{ > > + uint32_t c = le32_to_cpu(test->hdr->count); > > + test->hdr->count = cpu_to_le32(c + inc); > > +} > > + > > +static void > > +pci_testdev_write(void *opaque, hwaddr addr, uint64_t val, > > + unsigned size, int type) > > +{ > > + PCITestDevState *d = opaque; > > + IOTest *test; > > + int t, r; > > + > > + if (addr == offsetof(PCITestDevHdr, test)) { > > + pci_testdev_reset(d); > > + if (val >= IOTEST_MAX_TEST) { > > + return; > > + } > > + t = type * IOTEST_MAX_TEST + val; > > + r = pci_testdev_start(&d->tests[t]); > > + if (r < 0) { > > + return; > > + } > > + d->current = t; > > + return; > > + } > > + if (d->current < 0) { > > + return; > > + } > > + test = &d->tests[d->current]; > > + if (addr != le32_to_cpu(test->hdr->offset)) { > > + return; > > + } > > + if (test->match_data && test->size != size) { > > + return; > > + } > > + if (test->match_data && val != test->hdr->data) { > > + return; > > + } > > + pci_testdev_inc(test, 1); > > +} > > + > > +static uint64_t > > +pci_testdev_read(void *opaque, hwaddr addr, unsigned size) > > +{ > > + PCITestDevState *d = opaque; > > + const char *buf; > > + IOTest *test; > > + if (d->current < 0) { > > + return 0; > > + } > > + test = &d->tests[d->current]; > > + buf = (const char *)test->hdr; > > + if (addr + size >= test->bufsize) { > > + return 0; > > + } > > + if (test->hasnotifier) { > > + event_notifier_test_and_clear(&test->notifier); > > + } > > + return buf[addr]; > > +} > > + > > +static void > > +pci_testdev_mmio_write(void *opaque, hwaddr addr, uint64_t val, > > + unsigned size) > > +{ > > + pci_testdev_write(opaque, addr, val, size, 0); > > +} > > + > > +static void > > +pci_testdev_pio_write(void *opaque, hwaddr addr, uint64_t val, > > + unsigned size) > > +{ > > + pci_testdev_write(opaque, addr, val, size, 1); > > +} > > + > > +static const MemoryRegionOps pci_testdev_mmio_ops = { > > + .read = pci_testdev_read, > > + .write = pci_testdev_mmio_write, > > + .endianness = DEVICE_LITTLE_ENDIAN, > > + .impl = { > > + .min_access_size = 1, > > + .max_access_size = 1, > > + }, > > +}; > > + > > +static const MemoryRegionOps pci_testdev_pio_ops = { > > + .read = pci_testdev_read, > > + .write = pci_testdev_pio_write, > > + .endianness = DEVICE_LITTLE_ENDIAN, > > + .impl = { > > + .min_access_size = 1, > > + .max_access_size = 1, > > + }, > > +}; > > + > > +static int pci_testdev_init(PCIDevice *pci_dev) > > +{ > > + PCITestDevState *d = DO_UPCAST(PCITestDevState, dev, pci_dev); > > + uint8_t *pci_conf; > > + char *name; > > + int r, i; > > + > > + pci_conf = d->dev.config; > > + > > + pci_conf[PCI_INTERRUPT_PIN] = 0; /* no interrupt pin */ > > + > > + memory_region_init_io(&d->mmio, &pci_testdev_mmio_ops, d, > > + "pci-testdev-mmio", IOTEST_MEMSIZE * 2); > > + memory_region_init_io(&d->portio, &pci_testdev_pio_ops, d, > > + "pci-testdev-portio", IOTEST_IOSIZE * 2); > > + pci_register_bar(&d->dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &d->mmio); > > + pci_register_bar(&d->dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &d->portio); > > + > > + d->current = -1; > > + d->tests = g_malloc0(IOTEST_MAX * sizeof *d->tests); > > + for (i = 0; i < IOTEST_MAX; ++i) { > > + IOTest *test = &d->tests[i]; > > + name = g_strdup_printf("%s-%s", IOTEST_TYPE(i), IOTEST_TEST(i)); > > + test->bufsize = sizeof(PCITestDevHdr) + strlen(name) + 1; > > + test->hdr = g_malloc0(test->bufsize); > > + memcpy(test->hdr->name, name, strlen(name) + 1); > > + g_free(name); > > + test->hdr->offset = cpu_to_le32(IOTEST_SIZE(i) + i * IOTEST_ACCESS_WIDTH); > > + test->size = IOTEST_ACCESS_WIDTH; > > + test->match_data = strcmp(IOTEST_TEST(i), "wildcard-eventfd"); > > + test->hdr->test = i; > > + test->hdr->data = test->match_data ? IOTEST_DATAMATCH : IOTEST_NOMATCH; > > + test->hdr->width = IOTEST_ACCESS_WIDTH; > > + test->mr = IOTEST_REGION(d, i); > > + if (!strcmp(IOTEST_TEST(i), "no-eventfd")) { > > + test->hasnotifier = false; > > + continue; > > + } > > + r = event_notifier_init(&test->notifier, 0); > > + assert(r >= 0); > > + test->hasnotifier = true; > > + } > > + > > + return 0; > > +} > > + > > +static void > > +pci_testdev_uninit(PCIDevice *dev) > > +{ > > + PCITestDevState *d = DO_UPCAST(PCITestDevState, dev, dev); > > + int i; > > + > > + pci_testdev_reset(d); > > + for (i = 0; i < IOTEST_MAX; ++i) { > > + if (d->tests[i].hasnotifier) { > > + event_notifier_cleanup(&d->tests[i].notifier); > > + } > > + g_free(d->tests[i].hdr); > > + } > > + g_free(d->tests); > > + memory_region_destroy(&d->mmio); > > + memory_region_destroy(&d->portio); > > +} > > + > > +static void qdev_pci_testdev_reset(DeviceState *dev) > > +{ > > + PCITestDevState *d = DO_UPCAST(PCITestDevState, dev.qdev, dev); > > + pci_testdev_reset(d); > > +} > > + > > +static void pci_testdev_class_init(ObjectClass *klass, void *data) > > +{ > > + DeviceClass *dc = DEVICE_CLASS(klass); > > + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > > + > > + k->init = pci_testdev_init; > > + k->exit = pci_testdev_uninit; > > + k->vendor_id = PCI_VENDOR_ID_REDHAT; > > + k->device_id = PCI_DEVICE_ID_REDHAT_TEST; > > + k->revision = 0x00; > > + k->class_id = PCI_CLASS_OTHERS; > > + dc->desc = "PCI Test Device"; > > + dc->reset = qdev_pci_testdev_reset; > > +} > > + > > +static const TypeInfo pci_testdev_info = { > > + .name = "pci-testdev", > > + .parent = TYPE_PCI_DEVICE, > > + .instance_size = sizeof(PCITestDevState), > > + .class_init = pci_testdev_class_init, > > +}; > > + > > +static void pci_testdev_register_types(void) > > +{ > > + type_register_static(&pci_testdev_info); > > +} > > + > > +type_init(pci_testdev_register_types) > > diff --git a/hw/pci/pci.h b/hw/pci/pci.h > > index 774369c..d81198c 100644 > > --- a/hw/pci/pci.h > > +++ b/hw/pci/pci.h > > @@ -84,6 +84,7 @@ > > #define PCI_DEVICE_ID_REDHAT_SERIAL 0x0002 > > #define PCI_DEVICE_ID_REDHAT_SERIAL2 0x0003 > > #define PCI_DEVICE_ID_REDHAT_SERIAL4 0x0004 > > +#define PCI_DEVICE_ID_REDHAT_TEST 0x0005 > > #define PCI_DEVICE_ID_REDHAT_QXL 0x0100 > > > > #define FMT_PCIBUS PRIx64 > >
Il 03/04/2013 11:45, Michael S. Tsirkin ha scritto: > On Wed, Apr 03, 2013 at 11:28:55AM +0200, Paolo Bonzini wrote: >> Il 03/04/2013 10:59, Michael S. Tsirkin ha scritto: >>> This device is used for kvm unit tests, >>> currently it supports testing performance of ioeventfd. >>> Using updated kvm unittest, here's an example output: >>> mmio-no-eventfd:pci-mem 8796 >>> mmio-wildcard-eventfd:pci-mem 3609 >>> mmio-datamatch-eventfd:pci-mem 3685 >>> portio-no-eventfd:pci-io 5287 >>> portio-wildcard-eventfd:pci-io 1762 >>> portio-datamatch-eventfd:pci-io 1777 >> >> There is too much magic in this device, that is shared between the >> testcase and the device. > > Paolo, it looks like there's some misunderstanding. > There's no magic. Each BAR has the structure specified > in both test and qemu: > > struct pci_test_dev_hdr { > uint8_t test; -- test number > uint8_t width; - how much to write > uint8_t pad0[2]; > uint32_t offset; - where to write > uint32_t data; - what to write > uint32_t count; - incremented on write > uint8_t name[]; -- test name for debugging > }; > > What you propose below has a bit less features > if you add these you will get pretty much same thing. I think it has the same features, except the guest is in charge of enabling/disabling ioeventfd. >> I think this device should be structured >> differently. For example, you could have a single EventNotifier and 3 BARs: >> >> 1) BAR0/BAR1 are as in your device. All they do is count writes and >> report them into a register in BAR2. >> >> 2) BAR2 has a control register for BAR0 and one for BAR1, that >> enables/disables ioeventfd (bit 0 = enable/disable, bit 1 = >> wildcard/datamatch, bits 2-3 = log2(width)). It also has a zero-on-read >> counter that is incremented for each write to BAR0 or BAR1, and clears >> the EventNotifier. > > This is pretty close to what I have, only I put control > at the beginning of each BAR, and I support arbitrary > length writes guest side, and I allow testing both > datamatch and non data match. Yes, all of this is possible with my design too, the differences are: * "what to write" is a fixed value (could be 0, or all-ones, whatever). * the offset is a fixed value too (could be 0, or could be written in other bits of the control register) * no test names for debugging, because the guest picks the tests For example: mmio-no-eventfd:pci-mem 0000 to BAR2's first control register mmio-wildcard-eventfd:pci-mem 1001 to BAR2's first control register mmio-datamatch-eventfd:pci-mem 1011 to BAR2's first control register portio-no-eventfd:pci-io 0000 to BAR2's second register portio-wildcard-eventfd:pci-io 1001 to BAR2's second register portio-datamatch-eventfd:pci-io 1011 to BAR2's second register > I also do not want guest to control things like ioeventfd > assignment. These are host side things and while you could > argue security does not matter for a test device, > I think it will set a bad example. Test devices exist for the purpose of doing things that you won't do in normal devices. :) > So any new benchmark can be added pretty much on qemu > side without changing test. You could say the same with the control registers (new benchmarks can be added on the test side without changing QEMU, for example benchmarks using different write sizes. (BTW, and unrelated to this, please use default-configs/ and hw/Makefile.objs instead of hw/i386/Makefile.objs to enable the device). Paolo > > >>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> >>> --- >>> hw/i386/Makefile.objs | 2 +- >>> hw/pci-testdev.c | 306 ++++++++++++++++++++++++++++++++++++++++++++++++++ >>> hw/pci/pci.h | 1 + >>> 3 files changed, 308 insertions(+), 1 deletion(-) >>> create mode 100644 hw/pci-testdev.c >>> >>> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs >>> index a78c0b2..7497e7a 100644 >>> --- a/hw/i386/Makefile.objs >>> +++ b/hw/i386/Makefile.objs >>> @@ -11,7 +11,7 @@ obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o >>> obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o xen_pt_msi.o >>> obj-y += kvm/ >>> obj-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o >>> -obj-y += pc-testdev.o >>> +obj-y += pc-testdev.o pci-testdev.o >>> >>> obj-y := $(addprefix ../,$(obj-y)) >>> >>> diff --git a/hw/pci-testdev.c b/hw/pci-testdev.c >>> new file mode 100644 >>> index 0000000..9486624 >>> --- /dev/null >>> +++ b/hw/pci-testdev.c >>> @@ -0,0 +1,306 @@ >>> +#include "hw/hw.h" >>> +#include "hw/pci/pci.h" >>> +#include "qemu/event_notifier.h" >>> +#include "qemu/osdep.h" >>> + >>> +typedef struct PCITestDevHdr { >>> + uint8_t test; >>> + uint8_t width; >>> + uint8_t pad0[2]; >>> + uint32_t offset; >>> + uint8_t data; >>> + uint8_t pad1[3]; >>> + uint32_t count; >>> + uint8_t name[]; >>> +} PCITestDevHdr; >>> + >>> +typedef struct IOTest { >>> + MemoryRegion *mr; >>> + EventNotifier notifier; >>> + bool hasnotifier; >>> + unsigned size; >>> + bool match_data; >>> + PCITestDevHdr *hdr; >>> + unsigned bufsize; >>> +} IOTest; >>> + >>> +#define IOTEST_DATAMATCH 0xFA >>> +#define IOTEST_NOMATCH 0xCE >>> + >>> +#define IOTEST_IOSIZE 128 >>> +#define IOTEST_MEMSIZE 2048 >>> + >>> +static const char *iotest_test[] = { >>> + "no-eventfd", >>> + "wildcard-eventfd", >>> + "datamatch-eventfd" >>> +}; >>> + >>> +static const char *iotest_type[] = { >>> + "mmio", >>> + "portio" >>> +}; >>> + >>> +#define IOTEST_TEST(i) (iotest_test[((i) % ARRAY_SIZE(iotest_test))]) >>> +#define IOTEST_TYPE(i) (iotest_type[((i) / ARRAY_SIZE(iotest_test))]) >>> +#define IOTEST_MAX_TEST (ARRAY_SIZE(iotest_test)) >>> +#define IOTEST_MAX_TYPE (ARRAY_SIZE(iotest_type)) >>> +#define IOTEST_MAX (IOTEST_MAX_TEST * IOTEST_MAX_TYPE) >>> + >>> +enum { >>> + IOTEST_ACCESS_NAME, >>> + IOTEST_ACCESS_DATA, >>> + IOTEST_ACCESS_MAX, >>> +}; >>> + >>> +#define IOTEST_ACCESS_TYPE uint8_t >>> +#define IOTEST_ACCESS_WIDTH (sizeof(uint8_t)) >>> + >>> +typedef struct PCITestDevState { >>> + PCIDevice dev; >>> + MemoryRegion mmio; >>> + MemoryRegion portio; >>> + IOTest *tests; >>> + int current; >>> +} PCITestDevState; >>> + >>> +#define IOTEST_IS_MEM(i) (strcmp(IOTEST_TYPE(i), "portio")) >>> +#define IOTEST_REGION(d, i) (IOTEST_IS_MEM(i) ? &(d)->mmio : &(d)->portio) >>> +#define IOTEST_SIZE(i) (IOTEST_IS_MEM(i) ? IOTEST_MEMSIZE : IOTEST_IOSIZE) >>> +#define IOTEST_PCI_BAR(i) (IOTEST_IS_MEM(i) ? PCI_BASE_ADDRESS_SPACE_MEMORY : \ >>> + PCI_BASE_ADDRESS_SPACE_IO) >>> + >>> +static int pci_testdev_start(IOTest *test) >>> +{ >>> + test->hdr->count = 0; >>> + if (!test->hasnotifier) { >>> + return 0; >>> + } >>> + event_notifier_test_and_clear(&test->notifier); >>> + memory_region_add_eventfd(test->mr, >>> + le32_to_cpu(test->hdr->offset), >>> + test->size, >>> + test->match_data, >>> + test->hdr->data, >>> + &test->notifier); >>> + return 0; >>> +} >>> + >>> +static void pci_testdev_stop(IOTest *test) >>> +{ >>> + if (!test->hasnotifier) { >>> + return; >>> + } >>> + memory_region_del_eventfd(test->mr, >>> + le32_to_cpu(test->hdr->offset), >>> + test->size, >>> + test->match_data, >>> + test->hdr->data, >>> + &test->notifier); >>> +} >>> + >>> +static void >>> +pci_testdev_reset(PCITestDevState *d) >>> +{ >>> + if (d->current == -1) { >>> + return; >>> + } >>> + pci_testdev_stop(&d->tests[d->current]); >>> + d->current = -1; >>> +} >>> + >>> +static void pci_testdev_inc(IOTest *test, unsigned inc) >>> +{ >>> + uint32_t c = le32_to_cpu(test->hdr->count); >>> + test->hdr->count = cpu_to_le32(c + inc); >>> +} >>> + >>> +static void >>> +pci_testdev_write(void *opaque, hwaddr addr, uint64_t val, >>> + unsigned size, int type) >>> +{ >>> + PCITestDevState *d = opaque; >>> + IOTest *test; >>> + int t, r; >>> + >>> + if (addr == offsetof(PCITestDevHdr, test)) { >>> + pci_testdev_reset(d); >>> + if (val >= IOTEST_MAX_TEST) { >>> + return; >>> + } >>> + t = type * IOTEST_MAX_TEST + val; >>> + r = pci_testdev_start(&d->tests[t]); >>> + if (r < 0) { >>> + return; >>> + } >>> + d->current = t; >>> + return; >>> + } >>> + if (d->current < 0) { >>> + return; >>> + } >>> + test = &d->tests[d->current]; >>> + if (addr != le32_to_cpu(test->hdr->offset)) { >>> + return; >>> + } >>> + if (test->match_data && test->size != size) { >>> + return; >>> + } >>> + if (test->match_data && val != test->hdr->data) { >>> + return; >>> + } >>> + pci_testdev_inc(test, 1); >>> +} >>> + >>> +static uint64_t >>> +pci_testdev_read(void *opaque, hwaddr addr, unsigned size) >>> +{ >>> + PCITestDevState *d = opaque; >>> + const char *buf; >>> + IOTest *test; >>> + if (d->current < 0) { >>> + return 0; >>> + } >>> + test = &d->tests[d->current]; >>> + buf = (const char *)test->hdr; >>> + if (addr + size >= test->bufsize) { >>> + return 0; >>> + } >>> + if (test->hasnotifier) { >>> + event_notifier_test_and_clear(&test->notifier); >>> + } >>> + return buf[addr]; >>> +} >>> + >>> +static void >>> +pci_testdev_mmio_write(void *opaque, hwaddr addr, uint64_t val, >>> + unsigned size) >>> +{ >>> + pci_testdev_write(opaque, addr, val, size, 0); >>> +} >>> + >>> +static void >>> +pci_testdev_pio_write(void *opaque, hwaddr addr, uint64_t val, >>> + unsigned size) >>> +{ >>> + pci_testdev_write(opaque, addr, val, size, 1); >>> +} >>> + >>> +static const MemoryRegionOps pci_testdev_mmio_ops = { >>> + .read = pci_testdev_read, >>> + .write = pci_testdev_mmio_write, >>> + .endianness = DEVICE_LITTLE_ENDIAN, >>> + .impl = { >>> + .min_access_size = 1, >>> + .max_access_size = 1, >>> + }, >>> +}; >>> + >>> +static const MemoryRegionOps pci_testdev_pio_ops = { >>> + .read = pci_testdev_read, >>> + .write = pci_testdev_pio_write, >>> + .endianness = DEVICE_LITTLE_ENDIAN, >>> + .impl = { >>> + .min_access_size = 1, >>> + .max_access_size = 1, >>> + }, >>> +}; >>> + >>> +static int pci_testdev_init(PCIDevice *pci_dev) >>> +{ >>> + PCITestDevState *d = DO_UPCAST(PCITestDevState, dev, pci_dev); >>> + uint8_t *pci_conf; >>> + char *name; >>> + int r, i; >>> + >>> + pci_conf = d->dev.config; >>> + >>> + pci_conf[PCI_INTERRUPT_PIN] = 0; /* no interrupt pin */ >>> + >>> + memory_region_init_io(&d->mmio, &pci_testdev_mmio_ops, d, >>> + "pci-testdev-mmio", IOTEST_MEMSIZE * 2); >>> + memory_region_init_io(&d->portio, &pci_testdev_pio_ops, d, >>> + "pci-testdev-portio", IOTEST_IOSIZE * 2); >>> + pci_register_bar(&d->dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &d->mmio); >>> + pci_register_bar(&d->dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &d->portio); >>> + >>> + d->current = -1; >>> + d->tests = g_malloc0(IOTEST_MAX * sizeof *d->tests); >>> + for (i = 0; i < IOTEST_MAX; ++i) { >>> + IOTest *test = &d->tests[i]; >>> + name = g_strdup_printf("%s-%s", IOTEST_TYPE(i), IOTEST_TEST(i)); >>> + test->bufsize = sizeof(PCITestDevHdr) + strlen(name) + 1; >>> + test->hdr = g_malloc0(test->bufsize); >>> + memcpy(test->hdr->name, name, strlen(name) + 1); >>> + g_free(name); >>> + test->hdr->offset = cpu_to_le32(IOTEST_SIZE(i) + i * IOTEST_ACCESS_WIDTH); >>> + test->size = IOTEST_ACCESS_WIDTH; >>> + test->match_data = strcmp(IOTEST_TEST(i), "wildcard-eventfd"); >>> + test->hdr->test = i; >>> + test->hdr->data = test->match_data ? IOTEST_DATAMATCH : IOTEST_NOMATCH; >>> + test->hdr->width = IOTEST_ACCESS_WIDTH; >>> + test->mr = IOTEST_REGION(d, i); >>> + if (!strcmp(IOTEST_TEST(i), "no-eventfd")) { >>> + test->hasnotifier = false; >>> + continue; >>> + } >>> + r = event_notifier_init(&test->notifier, 0); >>> + assert(r >= 0); >>> + test->hasnotifier = true; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +static void >>> +pci_testdev_uninit(PCIDevice *dev) >>> +{ >>> + PCITestDevState *d = DO_UPCAST(PCITestDevState, dev, dev); >>> + int i; >>> + >>> + pci_testdev_reset(d); >>> + for (i = 0; i < IOTEST_MAX; ++i) { >>> + if (d->tests[i].hasnotifier) { >>> + event_notifier_cleanup(&d->tests[i].notifier); >>> + } >>> + g_free(d->tests[i].hdr); >>> + } >>> + g_free(d->tests); >>> + memory_region_destroy(&d->mmio); >>> + memory_region_destroy(&d->portio); >>> +} >>> + >>> +static void qdev_pci_testdev_reset(DeviceState *dev) >>> +{ >>> + PCITestDevState *d = DO_UPCAST(PCITestDevState, dev.qdev, dev); >>> + pci_testdev_reset(d); >>> +} >>> + >>> +static void pci_testdev_class_init(ObjectClass *klass, void *data) >>> +{ >>> + DeviceClass *dc = DEVICE_CLASS(klass); >>> + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); >>> + >>> + k->init = pci_testdev_init; >>> + k->exit = pci_testdev_uninit; >>> + k->vendor_id = PCI_VENDOR_ID_REDHAT; >>> + k->device_id = PCI_DEVICE_ID_REDHAT_TEST; >>> + k->revision = 0x00; >>> + k->class_id = PCI_CLASS_OTHERS; >>> + dc->desc = "PCI Test Device"; >>> + dc->reset = qdev_pci_testdev_reset; >>> +} >>> + >>> +static const TypeInfo pci_testdev_info = { >>> + .name = "pci-testdev", >>> + .parent = TYPE_PCI_DEVICE, >>> + .instance_size = sizeof(PCITestDevState), >>> + .class_init = pci_testdev_class_init, >>> +}; >>> + >>> +static void pci_testdev_register_types(void) >>> +{ >>> + type_register_static(&pci_testdev_info); >>> +} >>> + >>> +type_init(pci_testdev_register_types) >>> diff --git a/hw/pci/pci.h b/hw/pci/pci.h >>> index 774369c..d81198c 100644 >>> --- a/hw/pci/pci.h >>> +++ b/hw/pci/pci.h >>> @@ -84,6 +84,7 @@ >>> #define PCI_DEVICE_ID_REDHAT_SERIAL 0x0002 >>> #define PCI_DEVICE_ID_REDHAT_SERIAL2 0x0003 >>> #define PCI_DEVICE_ID_REDHAT_SERIAL4 0x0004 >>> +#define PCI_DEVICE_ID_REDHAT_TEST 0x0005 >>> #define PCI_DEVICE_ID_REDHAT_QXL 0x0100 >>> >>> #define FMT_PCIBUS PRIx64 >>>
On Wed, Apr 03, 2013 at 11:53:59AM +0200, Paolo Bonzini wrote: > Il 03/04/2013 11:45, Michael S. Tsirkin ha scritto: > > On Wed, Apr 03, 2013 at 11:28:55AM +0200, Paolo Bonzini wrote: > >> Il 03/04/2013 10:59, Michael S. Tsirkin ha scritto: > >>> This device is used for kvm unit tests, > >>> currently it supports testing performance of ioeventfd. > >>> Using updated kvm unittest, here's an example output: > >>> mmio-no-eventfd:pci-mem 8796 > >>> mmio-wildcard-eventfd:pci-mem 3609 > >>> mmio-datamatch-eventfd:pci-mem 3685 > >>> portio-no-eventfd:pci-io 5287 > >>> portio-wildcard-eventfd:pci-io 1762 > >>> portio-datamatch-eventfd:pci-io 1777 > >> > >> There is too much magic in this device, that is shared between the > >> testcase and the device. > > > > Paolo, it looks like there's some misunderstanding. > > There's no magic. Each BAR has the structure specified > > in both test and qemu: > > > > struct pci_test_dev_hdr { > > uint8_t test; -- test number > > uint8_t width; - how much to write > > uint8_t pad0[2]; > > uint32_t offset; - where to write > > uint32_t data; - what to write > > uint32_t count; - incremented on write > > uint8_t name[]; -- test name for debugging > > }; > > > > What you propose below has a bit less features > > if you add these you will get pretty much same thing. > > I think it has the same features, except the guest is in charge of > enabling/disabling ioeventfd. > > >> I think this device should be structured > >> differently. For example, you could have a single EventNotifier and 3 BARs: > >> > >> 1) BAR0/BAR1 are as in your device. All they do is count writes and > >> report them into a register in BAR2. > >> > >> 2) BAR2 has a control register for BAR0 and one for BAR1, that > >> enables/disables ioeventfd (bit 0 = enable/disable, bit 1 = > >> wildcard/datamatch, bits 2-3 = log2(width)). It also has a zero-on-read > >> counter that is incremented for each write to BAR0 or BAR1, and clears > >> the EventNotifier. > > > > This is pretty close to what I have, only I put control > > at the beginning of each BAR, and I support arbitrary > > length writes guest side, and I allow testing both > > datamatch and non data match. > > Yes, all of this is possible with my design too, the differences are: > * "what to write" is a fixed value (could be 0, or all-ones, whatever). > * the offset is a fixed value too (could be 0, or could be written in > other bits of the control register) > * no test names for debugging, because the guest picks the tests Heh but host and guest must agree on how to trigger each test. This means each time I add a test I need to update guest and host side. > For example: > > mmio-no-eventfd:pci-mem 0000 to BAR2's first control register > mmio-wildcard-eventfd:pci-mem 1001 to BAR2's first control register > mmio-datamatch-eventfd:pci-mem 1011 to BAR2's first control register > portio-no-eventfd:pci-io 0000 to BAR2's second register > portio-wildcard-eventfd:pci-io 1001 to BAR2's second register > portio-datamatch-eventfd:pci-io 1011 to BAR2's second register Exactly. I prefer a flexible structure so I can change things host side. For example register multiple datamatches with same address and compare speed of access on first and last one. Similarly for address. But there's no less magic because you share constants. > > I also do not want guest to control things like ioeventfd > > assignment. These are host side things and while you could > > argue security does not matter for a test device, > > I think it will set a bad example. > > Test devices exist for the purpose of doing things that you won't do in > normal devices. :) You might not care about setting bad examples for ISA but I care about setting bad examples for PCI. In particular this one actually is a completely normal PCI device. And what it does it very much like what we are discussing for virtio pci new layout. > > So any new benchmark can be added pretty much on qemu > > side without changing test. > > You could say the same with the control registers (new benchmarks can be > added on the test side without changing QEMU, for example benchmarks > using different write sizes. And then we'll get back to a layout kind of like what we have here, except guest side. Again, I have in mind several more tests so this is structured in a way that will make adding them easier. > (BTW, and unrelated to this, please use default-configs/ and > hw/Makefile.objs instead of hw/i386/Makefile.objs to enable the device). > > Paolo Okay but why does pc-test device sit in hw/i386/Makefile.objs ? > > > > > >>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > >>> --- > >>> hw/i386/Makefile.objs | 2 +- > >>> hw/pci-testdev.c | 306 ++++++++++++++++++++++++++++++++++++++++++++++++++ > >>> hw/pci/pci.h | 1 + > >>> 3 files changed, 308 insertions(+), 1 deletion(-) > >>> create mode 100644 hw/pci-testdev.c > >>> > >>> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs > >>> index a78c0b2..7497e7a 100644 > >>> --- a/hw/i386/Makefile.objs > >>> +++ b/hw/i386/Makefile.objs > >>> @@ -11,7 +11,7 @@ obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o > >>> obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o xen_pt_msi.o > >>> obj-y += kvm/ > >>> obj-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o > >>> -obj-y += pc-testdev.o > >>> +obj-y += pc-testdev.o pci-testdev.o > >>> > >>> obj-y := $(addprefix ../,$(obj-y)) > >>> > >>> diff --git a/hw/pci-testdev.c b/hw/pci-testdev.c > >>> new file mode 100644 > >>> index 0000000..9486624 > >>> --- /dev/null > >>> +++ b/hw/pci-testdev.c > >>> @@ -0,0 +1,306 @@ > >>> +#include "hw/hw.h" > >>> +#include "hw/pci/pci.h" > >>> +#include "qemu/event_notifier.h" > >>> +#include "qemu/osdep.h" > >>> + > >>> +typedef struct PCITestDevHdr { > >>> + uint8_t test; > >>> + uint8_t width; > >>> + uint8_t pad0[2]; > >>> + uint32_t offset; > >>> + uint8_t data; > >>> + uint8_t pad1[3]; > >>> + uint32_t count; > >>> + uint8_t name[]; > >>> +} PCITestDevHdr; > >>> + > >>> +typedef struct IOTest { > >>> + MemoryRegion *mr; > >>> + EventNotifier notifier; > >>> + bool hasnotifier; > >>> + unsigned size; > >>> + bool match_data; > >>> + PCITestDevHdr *hdr; > >>> + unsigned bufsize; > >>> +} IOTest; > >>> + > >>> +#define IOTEST_DATAMATCH 0xFA > >>> +#define IOTEST_NOMATCH 0xCE > >>> + > >>> +#define IOTEST_IOSIZE 128 > >>> +#define IOTEST_MEMSIZE 2048 > >>> + > >>> +static const char *iotest_test[] = { > >>> + "no-eventfd", > >>> + "wildcard-eventfd", > >>> + "datamatch-eventfd" > >>> +}; > >>> + > >>> +static const char *iotest_type[] = { > >>> + "mmio", > >>> + "portio" > >>> +}; > >>> + > >>> +#define IOTEST_TEST(i) (iotest_test[((i) % ARRAY_SIZE(iotest_test))]) > >>> +#define IOTEST_TYPE(i) (iotest_type[((i) / ARRAY_SIZE(iotest_test))]) > >>> +#define IOTEST_MAX_TEST (ARRAY_SIZE(iotest_test)) > >>> +#define IOTEST_MAX_TYPE (ARRAY_SIZE(iotest_type)) > >>> +#define IOTEST_MAX (IOTEST_MAX_TEST * IOTEST_MAX_TYPE) > >>> + > >>> +enum { > >>> + IOTEST_ACCESS_NAME, > >>> + IOTEST_ACCESS_DATA, > >>> + IOTEST_ACCESS_MAX, > >>> +}; > >>> + > >>> +#define IOTEST_ACCESS_TYPE uint8_t > >>> +#define IOTEST_ACCESS_WIDTH (sizeof(uint8_t)) > >>> + > >>> +typedef struct PCITestDevState { > >>> + PCIDevice dev; > >>> + MemoryRegion mmio; > >>> + MemoryRegion portio; > >>> + IOTest *tests; > >>> + int current; > >>> +} PCITestDevState; > >>> + > >>> +#define IOTEST_IS_MEM(i) (strcmp(IOTEST_TYPE(i), "portio")) > >>> +#define IOTEST_REGION(d, i) (IOTEST_IS_MEM(i) ? &(d)->mmio : &(d)->portio) > >>> +#define IOTEST_SIZE(i) (IOTEST_IS_MEM(i) ? IOTEST_MEMSIZE : IOTEST_IOSIZE) > >>> +#define IOTEST_PCI_BAR(i) (IOTEST_IS_MEM(i) ? PCI_BASE_ADDRESS_SPACE_MEMORY : \ > >>> + PCI_BASE_ADDRESS_SPACE_IO) > >>> + > >>> +static int pci_testdev_start(IOTest *test) > >>> +{ > >>> + test->hdr->count = 0; > >>> + if (!test->hasnotifier) { > >>> + return 0; > >>> + } > >>> + event_notifier_test_and_clear(&test->notifier); > >>> + memory_region_add_eventfd(test->mr, > >>> + le32_to_cpu(test->hdr->offset), > >>> + test->size, > >>> + test->match_data, > >>> + test->hdr->data, > >>> + &test->notifier); > >>> + return 0; > >>> +} > >>> + > >>> +static void pci_testdev_stop(IOTest *test) > >>> +{ > >>> + if (!test->hasnotifier) { > >>> + return; > >>> + } > >>> + memory_region_del_eventfd(test->mr, > >>> + le32_to_cpu(test->hdr->offset), > >>> + test->size, > >>> + test->match_data, > >>> + test->hdr->data, > >>> + &test->notifier); > >>> +} > >>> + > >>> +static void > >>> +pci_testdev_reset(PCITestDevState *d) > >>> +{ > >>> + if (d->current == -1) { > >>> + return; > >>> + } > >>> + pci_testdev_stop(&d->tests[d->current]); > >>> + d->current = -1; > >>> +} > >>> + > >>> +static void pci_testdev_inc(IOTest *test, unsigned inc) > >>> +{ > >>> + uint32_t c = le32_to_cpu(test->hdr->count); > >>> + test->hdr->count = cpu_to_le32(c + inc); > >>> +} > >>> + > >>> +static void > >>> +pci_testdev_write(void *opaque, hwaddr addr, uint64_t val, > >>> + unsigned size, int type) > >>> +{ > >>> + PCITestDevState *d = opaque; > >>> + IOTest *test; > >>> + int t, r; > >>> + > >>> + if (addr == offsetof(PCITestDevHdr, test)) { > >>> + pci_testdev_reset(d); > >>> + if (val >= IOTEST_MAX_TEST) { > >>> + return; > >>> + } > >>> + t = type * IOTEST_MAX_TEST + val; > >>> + r = pci_testdev_start(&d->tests[t]); > >>> + if (r < 0) { > >>> + return; > >>> + } > >>> + d->current = t; > >>> + return; > >>> + } > >>> + if (d->current < 0) { > >>> + return; > >>> + } > >>> + test = &d->tests[d->current]; > >>> + if (addr != le32_to_cpu(test->hdr->offset)) { > >>> + return; > >>> + } > >>> + if (test->match_data && test->size != size) { > >>> + return; > >>> + } > >>> + if (test->match_data && val != test->hdr->data) { > >>> + return; > >>> + } > >>> + pci_testdev_inc(test, 1); > >>> +} > >>> + > >>> +static uint64_t > >>> +pci_testdev_read(void *opaque, hwaddr addr, unsigned size) > >>> +{ > >>> + PCITestDevState *d = opaque; > >>> + const char *buf; > >>> + IOTest *test; > >>> + if (d->current < 0) { > >>> + return 0; > >>> + } > >>> + test = &d->tests[d->current]; > >>> + buf = (const char *)test->hdr; > >>> + if (addr + size >= test->bufsize) { > >>> + return 0; > >>> + } > >>> + if (test->hasnotifier) { > >>> + event_notifier_test_and_clear(&test->notifier); > >>> + } > >>> + return buf[addr]; > >>> +} > >>> + > >>> +static void > >>> +pci_testdev_mmio_write(void *opaque, hwaddr addr, uint64_t val, > >>> + unsigned size) > >>> +{ > >>> + pci_testdev_write(opaque, addr, val, size, 0); > >>> +} > >>> + > >>> +static void > >>> +pci_testdev_pio_write(void *opaque, hwaddr addr, uint64_t val, > >>> + unsigned size) > >>> +{ > >>> + pci_testdev_write(opaque, addr, val, size, 1); > >>> +} > >>> + > >>> +static const MemoryRegionOps pci_testdev_mmio_ops = { > >>> + .read = pci_testdev_read, > >>> + .write = pci_testdev_mmio_write, > >>> + .endianness = DEVICE_LITTLE_ENDIAN, > >>> + .impl = { > >>> + .min_access_size = 1, > >>> + .max_access_size = 1, > >>> + }, > >>> +}; > >>> + > >>> +static const MemoryRegionOps pci_testdev_pio_ops = { > >>> + .read = pci_testdev_read, > >>> + .write = pci_testdev_pio_write, > >>> + .endianness = DEVICE_LITTLE_ENDIAN, > >>> + .impl = { > >>> + .min_access_size = 1, > >>> + .max_access_size = 1, > >>> + }, > >>> +}; > >>> + > >>> +static int pci_testdev_init(PCIDevice *pci_dev) > >>> +{ > >>> + PCITestDevState *d = DO_UPCAST(PCITestDevState, dev, pci_dev); > >>> + uint8_t *pci_conf; > >>> + char *name; > >>> + int r, i; > >>> + > >>> + pci_conf = d->dev.config; > >>> + > >>> + pci_conf[PCI_INTERRUPT_PIN] = 0; /* no interrupt pin */ > >>> + > >>> + memory_region_init_io(&d->mmio, &pci_testdev_mmio_ops, d, > >>> + "pci-testdev-mmio", IOTEST_MEMSIZE * 2); > >>> + memory_region_init_io(&d->portio, &pci_testdev_pio_ops, d, > >>> + "pci-testdev-portio", IOTEST_IOSIZE * 2); > >>> + pci_register_bar(&d->dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &d->mmio); > >>> + pci_register_bar(&d->dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &d->portio); > >>> + > >>> + d->current = -1; > >>> + d->tests = g_malloc0(IOTEST_MAX * sizeof *d->tests); > >>> + for (i = 0; i < IOTEST_MAX; ++i) { > >>> + IOTest *test = &d->tests[i]; > >>> + name = g_strdup_printf("%s-%s", IOTEST_TYPE(i), IOTEST_TEST(i)); > >>> + test->bufsize = sizeof(PCITestDevHdr) + strlen(name) + 1; > >>> + test->hdr = g_malloc0(test->bufsize); > >>> + memcpy(test->hdr->name, name, strlen(name) + 1); > >>> + g_free(name); > >>> + test->hdr->offset = cpu_to_le32(IOTEST_SIZE(i) + i * IOTEST_ACCESS_WIDTH); > >>> + test->size = IOTEST_ACCESS_WIDTH; > >>> + test->match_data = strcmp(IOTEST_TEST(i), "wildcard-eventfd"); > >>> + test->hdr->test = i; > >>> + test->hdr->data = test->match_data ? IOTEST_DATAMATCH : IOTEST_NOMATCH; > >>> + test->hdr->width = IOTEST_ACCESS_WIDTH; > >>> + test->mr = IOTEST_REGION(d, i); > >>> + if (!strcmp(IOTEST_TEST(i), "no-eventfd")) { > >>> + test->hasnotifier = false; > >>> + continue; > >>> + } > >>> + r = event_notifier_init(&test->notifier, 0); > >>> + assert(r >= 0); > >>> + test->hasnotifier = true; > >>> + } > >>> + > >>> + return 0; > >>> +} > >>> + > >>> +static void > >>> +pci_testdev_uninit(PCIDevice *dev) > >>> +{ > >>> + PCITestDevState *d = DO_UPCAST(PCITestDevState, dev, dev); > >>> + int i; > >>> + > >>> + pci_testdev_reset(d); > >>> + for (i = 0; i < IOTEST_MAX; ++i) { > >>> + if (d->tests[i].hasnotifier) { > >>> + event_notifier_cleanup(&d->tests[i].notifier); > >>> + } > >>> + g_free(d->tests[i].hdr); > >>> + } > >>> + g_free(d->tests); > >>> + memory_region_destroy(&d->mmio); > >>> + memory_region_destroy(&d->portio); > >>> +} > >>> + > >>> +static void qdev_pci_testdev_reset(DeviceState *dev) > >>> +{ > >>> + PCITestDevState *d = DO_UPCAST(PCITestDevState, dev.qdev, dev); > >>> + pci_testdev_reset(d); > >>> +} > >>> + > >>> +static void pci_testdev_class_init(ObjectClass *klass, void *data) > >>> +{ > >>> + DeviceClass *dc = DEVICE_CLASS(klass); > >>> + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > >>> + > >>> + k->init = pci_testdev_init; > >>> + k->exit = pci_testdev_uninit; > >>> + k->vendor_id = PCI_VENDOR_ID_REDHAT; > >>> + k->device_id = PCI_DEVICE_ID_REDHAT_TEST; > >>> + k->revision = 0x00; > >>> + k->class_id = PCI_CLASS_OTHERS; > >>> + dc->desc = "PCI Test Device"; > >>> + dc->reset = qdev_pci_testdev_reset; > >>> +} > >>> + > >>> +static const TypeInfo pci_testdev_info = { > >>> + .name = "pci-testdev", > >>> + .parent = TYPE_PCI_DEVICE, > >>> + .instance_size = sizeof(PCITestDevState), > >>> + .class_init = pci_testdev_class_init, > >>> +}; > >>> + > >>> +static void pci_testdev_register_types(void) > >>> +{ > >>> + type_register_static(&pci_testdev_info); > >>> +} > >>> + > >>> +type_init(pci_testdev_register_types) > >>> diff --git a/hw/pci/pci.h b/hw/pci/pci.h > >>> index 774369c..d81198c 100644 > >>> --- a/hw/pci/pci.h > >>> +++ b/hw/pci/pci.h > >>> @@ -84,6 +84,7 @@ > >>> #define PCI_DEVICE_ID_REDHAT_SERIAL 0x0002 > >>> #define PCI_DEVICE_ID_REDHAT_SERIAL2 0x0003 > >>> #define PCI_DEVICE_ID_REDHAT_SERIAL4 0x0004 > >>> +#define PCI_DEVICE_ID_REDHAT_TEST 0x0005 > >>> #define PCI_DEVICE_ID_REDHAT_QXL 0x0100 > >>> > >>> #define FMT_PCIBUS PRIx64 > >>>
> Exactly. I prefer a flexible structure so I can change > things host side. For example register multiple > datamatches with same address and compare speed > of access on first and last one. > Similarly for address. Ok, this makes sense. >> (BTW, and unrelated to this, please use default-configs/ and >> hw/Makefile.objs instead of hw/i386/Makefile.objs to enable the device). > > Okay but why does pc-test device sit in hw/i386/Makefile.objs ? Because someone wasn't looking. :) The hw/ reorganization patches I've posted fix that. Paolo >>> >>> >>>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> >>>>> --- >>>>> hw/i386/Makefile.objs | 2 +- >>>>> hw/pci-testdev.c | 306 ++++++++++++++++++++++++++++++++++++++++++++++++++ >>>>> hw/pci/pci.h | 1 + >>>>> 3 files changed, 308 insertions(+), 1 deletion(-) >>>>> create mode 100644 hw/pci-testdev.c >>>>> >>>>> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs >>>>> index a78c0b2..7497e7a 100644 >>>>> --- a/hw/i386/Makefile.objs >>>>> +++ b/hw/i386/Makefile.objs >>>>> @@ -11,7 +11,7 @@ obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o >>>>> obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o xen_pt_msi.o >>>>> obj-y += kvm/ >>>>> obj-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o >>>>> -obj-y += pc-testdev.o >>>>> +obj-y += pc-testdev.o pci-testdev.o >>>>> >>>>> obj-y := $(addprefix ../,$(obj-y)) >>>>> >>>>> diff --git a/hw/pci-testdev.c b/hw/pci-testdev.c >>>>> new file mode 100644 >>>>> index 0000000..9486624 >>>>> --- /dev/null >>>>> +++ b/hw/pci-testdev.c >>>>> @@ -0,0 +1,306 @@ >>>>> +#include "hw/hw.h" >>>>> +#include "hw/pci/pci.h" >>>>> +#include "qemu/event_notifier.h" >>>>> +#include "qemu/osdep.h" >>>>> + >>>>> +typedef struct PCITestDevHdr { >>>>> + uint8_t test; >>>>> + uint8_t width; >>>>> + uint8_t pad0[2]; >>>>> + uint32_t offset; >>>>> + uint8_t data; >>>>> + uint8_t pad1[3]; >>>>> + uint32_t count; >>>>> + uint8_t name[]; >>>>> +} PCITestDevHdr; >>>>> + >>>>> +typedef struct IOTest { >>>>> + MemoryRegion *mr; >>>>> + EventNotifier notifier; >>>>> + bool hasnotifier; >>>>> + unsigned size; >>>>> + bool match_data; >>>>> + PCITestDevHdr *hdr; >>>>> + unsigned bufsize; >>>>> +} IOTest; >>>>> + >>>>> +#define IOTEST_DATAMATCH 0xFA >>>>> +#define IOTEST_NOMATCH 0xCE >>>>> + >>>>> +#define IOTEST_IOSIZE 128 >>>>> +#define IOTEST_MEMSIZE 2048 >>>>> + >>>>> +static const char *iotest_test[] = { >>>>> + "no-eventfd", >>>>> + "wildcard-eventfd", >>>>> + "datamatch-eventfd" >>>>> +}; >>>>> + >>>>> +static const char *iotest_type[] = { >>>>> + "mmio", >>>>> + "portio" >>>>> +}; >>>>> + >>>>> +#define IOTEST_TEST(i) (iotest_test[((i) % ARRAY_SIZE(iotest_test))]) >>>>> +#define IOTEST_TYPE(i) (iotest_type[((i) / ARRAY_SIZE(iotest_test))]) >>>>> +#define IOTEST_MAX_TEST (ARRAY_SIZE(iotest_test)) >>>>> +#define IOTEST_MAX_TYPE (ARRAY_SIZE(iotest_type)) >>>>> +#define IOTEST_MAX (IOTEST_MAX_TEST * IOTEST_MAX_TYPE) >>>>> + >>>>> +enum { >>>>> + IOTEST_ACCESS_NAME, >>>>> + IOTEST_ACCESS_DATA, >>>>> + IOTEST_ACCESS_MAX, >>>>> +}; >>>>> + >>>>> +#define IOTEST_ACCESS_TYPE uint8_t >>>>> +#define IOTEST_ACCESS_WIDTH (sizeof(uint8_t)) >>>>> + >>>>> +typedef struct PCITestDevState { >>>>> + PCIDevice dev; >>>>> + MemoryRegion mmio; >>>>> + MemoryRegion portio; >>>>> + IOTest *tests; >>>>> + int current; >>>>> +} PCITestDevState; >>>>> + >>>>> +#define IOTEST_IS_MEM(i) (strcmp(IOTEST_TYPE(i), "portio")) >>>>> +#define IOTEST_REGION(d, i) (IOTEST_IS_MEM(i) ? &(d)->mmio : &(d)->portio) >>>>> +#define IOTEST_SIZE(i) (IOTEST_IS_MEM(i) ? IOTEST_MEMSIZE : IOTEST_IOSIZE) >>>>> +#define IOTEST_PCI_BAR(i) (IOTEST_IS_MEM(i) ? PCI_BASE_ADDRESS_SPACE_MEMORY : \ >>>>> + PCI_BASE_ADDRESS_SPACE_IO) >>>>> + >>>>> +static int pci_testdev_start(IOTest *test) >>>>> +{ >>>>> + test->hdr->count = 0; >>>>> + if (!test->hasnotifier) { >>>>> + return 0; >>>>> + } >>>>> + event_notifier_test_and_clear(&test->notifier); >>>>> + memory_region_add_eventfd(test->mr, >>>>> + le32_to_cpu(test->hdr->offset), >>>>> + test->size, >>>>> + test->match_data, >>>>> + test->hdr->data, >>>>> + &test->notifier); >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static void pci_testdev_stop(IOTest *test) >>>>> +{ >>>>> + if (!test->hasnotifier) { >>>>> + return; >>>>> + } >>>>> + memory_region_del_eventfd(test->mr, >>>>> + le32_to_cpu(test->hdr->offset), >>>>> + test->size, >>>>> + test->match_data, >>>>> + test->hdr->data, >>>>> + &test->notifier); >>>>> +} >>>>> + >>>>> +static void >>>>> +pci_testdev_reset(PCITestDevState *d) >>>>> +{ >>>>> + if (d->current == -1) { >>>>> + return; >>>>> + } >>>>> + pci_testdev_stop(&d->tests[d->current]); >>>>> + d->current = -1; >>>>> +} >>>>> + >>>>> +static void pci_testdev_inc(IOTest *test, unsigned inc) >>>>> +{ >>>>> + uint32_t c = le32_to_cpu(test->hdr->count); >>>>> + test->hdr->count = cpu_to_le32(c + inc); >>>>> +} >>>>> + >>>>> +static void >>>>> +pci_testdev_write(void *opaque, hwaddr addr, uint64_t val, >>>>> + unsigned size, int type) >>>>> +{ >>>>> + PCITestDevState *d = opaque; >>>>> + IOTest *test; >>>>> + int t, r; >>>>> + >>>>> + if (addr == offsetof(PCITestDevHdr, test)) { >>>>> + pci_testdev_reset(d); >>>>> + if (val >= IOTEST_MAX_TEST) { >>>>> + return; >>>>> + } >>>>> + t = type * IOTEST_MAX_TEST + val; >>>>> + r = pci_testdev_start(&d->tests[t]); >>>>> + if (r < 0) { >>>>> + return; >>>>> + } >>>>> + d->current = t; >>>>> + return; >>>>> + } >>>>> + if (d->current < 0) { >>>>> + return; >>>>> + } >>>>> + test = &d->tests[d->current]; >>>>> + if (addr != le32_to_cpu(test->hdr->offset)) { >>>>> + return; >>>>> + } >>>>> + if (test->match_data && test->size != size) { >>>>> + return; >>>>> + } >>>>> + if (test->match_data && val != test->hdr->data) { >>>>> + return; >>>>> + } >>>>> + pci_testdev_inc(test, 1); >>>>> +} >>>>> + >>>>> +static uint64_t >>>>> +pci_testdev_read(void *opaque, hwaddr addr, unsigned size) >>>>> +{ >>>>> + PCITestDevState *d = opaque; >>>>> + const char *buf; >>>>> + IOTest *test; >>>>> + if (d->current < 0) { >>>>> + return 0; >>>>> + } >>>>> + test = &d->tests[d->current]; >>>>> + buf = (const char *)test->hdr; >>>>> + if (addr + size >= test->bufsize) { >>>>> + return 0; >>>>> + } >>>>> + if (test->hasnotifier) { >>>>> + event_notifier_test_and_clear(&test->notifier); >>>>> + } >>>>> + return buf[addr]; >>>>> +} >>>>> + >>>>> +static void >>>>> +pci_testdev_mmio_write(void *opaque, hwaddr addr, uint64_t val, >>>>> + unsigned size) >>>>> +{ >>>>> + pci_testdev_write(opaque, addr, val, size, 0); >>>>> +} >>>>> + >>>>> +static void >>>>> +pci_testdev_pio_write(void *opaque, hwaddr addr, uint64_t val, >>>>> + unsigned size) >>>>> +{ >>>>> + pci_testdev_write(opaque, addr, val, size, 1); >>>>> +} >>>>> + >>>>> +static const MemoryRegionOps pci_testdev_mmio_ops = { >>>>> + .read = pci_testdev_read, >>>>> + .write = pci_testdev_mmio_write, >>>>> + .endianness = DEVICE_LITTLE_ENDIAN, >>>>> + .impl = { >>>>> + .min_access_size = 1, >>>>> + .max_access_size = 1, >>>>> + }, >>>>> +}; >>>>> + >>>>> +static const MemoryRegionOps pci_testdev_pio_ops = { >>>>> + .read = pci_testdev_read, >>>>> + .write = pci_testdev_pio_write, >>>>> + .endianness = DEVICE_LITTLE_ENDIAN, >>>>> + .impl = { >>>>> + .min_access_size = 1, >>>>> + .max_access_size = 1, >>>>> + }, >>>>> +}; >>>>> + >>>>> +static int pci_testdev_init(PCIDevice *pci_dev) >>>>> +{ >>>>> + PCITestDevState *d = DO_UPCAST(PCITestDevState, dev, pci_dev); >>>>> + uint8_t *pci_conf; >>>>> + char *name; >>>>> + int r, i; >>>>> + >>>>> + pci_conf = d->dev.config; >>>>> + >>>>> + pci_conf[PCI_INTERRUPT_PIN] = 0; /* no interrupt pin */ >>>>> + >>>>> + memory_region_init_io(&d->mmio, &pci_testdev_mmio_ops, d, >>>>> + "pci-testdev-mmio", IOTEST_MEMSIZE * 2); >>>>> + memory_region_init_io(&d->portio, &pci_testdev_pio_ops, d, >>>>> + "pci-testdev-portio", IOTEST_IOSIZE * 2); >>>>> + pci_register_bar(&d->dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &d->mmio); >>>>> + pci_register_bar(&d->dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &d->portio); >>>>> + >>>>> + d->current = -1; >>>>> + d->tests = g_malloc0(IOTEST_MAX * sizeof *d->tests); >>>>> + for (i = 0; i < IOTEST_MAX; ++i) { >>>>> + IOTest *test = &d->tests[i]; >>>>> + name = g_strdup_printf("%s-%s", IOTEST_TYPE(i), IOTEST_TEST(i)); >>>>> + test->bufsize = sizeof(PCITestDevHdr) + strlen(name) + 1; >>>>> + test->hdr = g_malloc0(test->bufsize); >>>>> + memcpy(test->hdr->name, name, strlen(name) + 1); >>>>> + g_free(name); >>>>> + test->hdr->offset = cpu_to_le32(IOTEST_SIZE(i) + i * IOTEST_ACCESS_WIDTH); >>>>> + test->size = IOTEST_ACCESS_WIDTH; >>>>> + test->match_data = strcmp(IOTEST_TEST(i), "wildcard-eventfd"); >>>>> + test->hdr->test = i; >>>>> + test->hdr->data = test->match_data ? IOTEST_DATAMATCH : IOTEST_NOMATCH; >>>>> + test->hdr->width = IOTEST_ACCESS_WIDTH; >>>>> + test->mr = IOTEST_REGION(d, i); >>>>> + if (!strcmp(IOTEST_TEST(i), "no-eventfd")) { >>>>> + test->hasnotifier = false; >>>>> + continue; >>>>> + } >>>>> + r = event_notifier_init(&test->notifier, 0); >>>>> + assert(r >= 0); >>>>> + test->hasnotifier = true; >>>>> + } >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static void >>>>> +pci_testdev_uninit(PCIDevice *dev) >>>>> +{ >>>>> + PCITestDevState *d = DO_UPCAST(PCITestDevState, dev, dev); >>>>> + int i; >>>>> + >>>>> + pci_testdev_reset(d); >>>>> + for (i = 0; i < IOTEST_MAX; ++i) { >>>>> + if (d->tests[i].hasnotifier) { >>>>> + event_notifier_cleanup(&d->tests[i].notifier); >>>>> + } >>>>> + g_free(d->tests[i].hdr); >>>>> + } >>>>> + g_free(d->tests); >>>>> + memory_region_destroy(&d->mmio); >>>>> + memory_region_destroy(&d->portio); >>>>> +} >>>>> + >>>>> +static void qdev_pci_testdev_reset(DeviceState *dev) >>>>> +{ >>>>> + PCITestDevState *d = DO_UPCAST(PCITestDevState, dev.qdev, dev); >>>>> + pci_testdev_reset(d); >>>>> +} >>>>> + >>>>> +static void pci_testdev_class_init(ObjectClass *klass, void *data) >>>>> +{ >>>>> + DeviceClass *dc = DEVICE_CLASS(klass); >>>>> + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); >>>>> + >>>>> + k->init = pci_testdev_init; >>>>> + k->exit = pci_testdev_uninit; >>>>> + k->vendor_id = PCI_VENDOR_ID_REDHAT; >>>>> + k->device_id = PCI_DEVICE_ID_REDHAT_TEST; >>>>> + k->revision = 0x00; >>>>> + k->class_id = PCI_CLASS_OTHERS; >>>>> + dc->desc = "PCI Test Device"; >>>>> + dc->reset = qdev_pci_testdev_reset; >>>>> +} >>>>> + >>>>> +static const TypeInfo pci_testdev_info = { >>>>> + .name = "pci-testdev", >>>>> + .parent = TYPE_PCI_DEVICE, >>>>> + .instance_size = sizeof(PCITestDevState), >>>>> + .class_init = pci_testdev_class_init, >>>>> +}; >>>>> + >>>>> +static void pci_testdev_register_types(void) >>>>> +{ >>>>> + type_register_static(&pci_testdev_info); >>>>> +} >>>>> + >>>>> +type_init(pci_testdev_register_types) >>>>> diff --git a/hw/pci/pci.h b/hw/pci/pci.h >>>>> index 774369c..d81198c 100644 >>>>> --- a/hw/pci/pci.h >>>>> +++ b/hw/pci/pci.h >>>>> @@ -84,6 +84,7 @@ >>>>> #define PCI_DEVICE_ID_REDHAT_SERIAL 0x0002 >>>>> #define PCI_DEVICE_ID_REDHAT_SERIAL2 0x0003 >>>>> #define PCI_DEVICE_ID_REDHAT_SERIAL4 0x0004 >>>>> +#define PCI_DEVICE_ID_REDHAT_TEST 0x0005 >>>>> #define PCI_DEVICE_ID_REDHAT_QXL 0x0100 >>>>> >>>>> #define FMT_PCIBUS PRIx64 >>>>>
On Wed, Apr 03, 2013 at 12:25:02PM +0200, Paolo Bonzini wrote: > > > Exactly. I prefer a flexible structure so I can change > > things host side. For example register multiple > > datamatches with same address and compare speed > > of access on first and last one. > > Similarly for address. > > Ok, this makes sense. > > >> (BTW, and unrelated to this, please use default-configs/ and > >> hw/Makefile.objs instead of hw/i386/Makefile.objs to enable the device). > > > > Okay but why does pc-test device sit in hw/i386/Makefile.objs ? > > Because someone wasn't looking. :) The hw/ reorganization patches I've > posted fix that. > > Paolo Okay. Still not sure how to merge this, if it goes in through my tree and that's the only comment, I'll just fix it silently ... > >>> > >>> > >>>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > >>>>> --- > >>>>> hw/i386/Makefile.objs | 2 +- > >>>>> hw/pci-testdev.c | 306 ++++++++++++++++++++++++++++++++++++++++++++++++++ > >>>>> hw/pci/pci.h | 1 + > >>>>> 3 files changed, 308 insertions(+), 1 deletion(-) > >>>>> create mode 100644 hw/pci-testdev.c > >>>>> > >>>>> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs > >>>>> index a78c0b2..7497e7a 100644 > >>>>> --- a/hw/i386/Makefile.objs > >>>>> +++ b/hw/i386/Makefile.objs > >>>>> @@ -11,7 +11,7 @@ obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o > >>>>> obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o xen_pt_msi.o > >>>>> obj-y += kvm/ > >>>>> obj-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o > >>>>> -obj-y += pc-testdev.o > >>>>> +obj-y += pc-testdev.o pci-testdev.o > >>>>> > >>>>> obj-y := $(addprefix ../,$(obj-y)) > >>>>> > >>>>> diff --git a/hw/pci-testdev.c b/hw/pci-testdev.c > >>>>> new file mode 100644 > >>>>> index 0000000..9486624 > >>>>> --- /dev/null > >>>>> +++ b/hw/pci-testdev.c > >>>>> @@ -0,0 +1,306 @@ > >>>>> +#include "hw/hw.h" > >>>>> +#include "hw/pci/pci.h" > >>>>> +#include "qemu/event_notifier.h" > >>>>> +#include "qemu/osdep.h" > >>>>> + > >>>>> +typedef struct PCITestDevHdr { > >>>>> + uint8_t test; > >>>>> + uint8_t width; > >>>>> + uint8_t pad0[2]; > >>>>> + uint32_t offset; > >>>>> + uint8_t data; > >>>>> + uint8_t pad1[3]; > >>>>> + uint32_t count; > >>>>> + uint8_t name[]; > >>>>> +} PCITestDevHdr; > >>>>> + > >>>>> +typedef struct IOTest { > >>>>> + MemoryRegion *mr; > >>>>> + EventNotifier notifier; > >>>>> + bool hasnotifier; > >>>>> + unsigned size; > >>>>> + bool match_data; > >>>>> + PCITestDevHdr *hdr; > >>>>> + unsigned bufsize; > >>>>> +} IOTest; > >>>>> + > >>>>> +#define IOTEST_DATAMATCH 0xFA > >>>>> +#define IOTEST_NOMATCH 0xCE > >>>>> + > >>>>> +#define IOTEST_IOSIZE 128 > >>>>> +#define IOTEST_MEMSIZE 2048 > >>>>> + > >>>>> +static const char *iotest_test[] = { > >>>>> + "no-eventfd", > >>>>> + "wildcard-eventfd", > >>>>> + "datamatch-eventfd" > >>>>> +}; > >>>>> + > >>>>> +static const char *iotest_type[] = { > >>>>> + "mmio", > >>>>> + "portio" > >>>>> +}; > >>>>> + > >>>>> +#define IOTEST_TEST(i) (iotest_test[((i) % ARRAY_SIZE(iotest_test))]) > >>>>> +#define IOTEST_TYPE(i) (iotest_type[((i) / ARRAY_SIZE(iotest_test))]) > >>>>> +#define IOTEST_MAX_TEST (ARRAY_SIZE(iotest_test)) > >>>>> +#define IOTEST_MAX_TYPE (ARRAY_SIZE(iotest_type)) > >>>>> +#define IOTEST_MAX (IOTEST_MAX_TEST * IOTEST_MAX_TYPE) > >>>>> + > >>>>> +enum { > >>>>> + IOTEST_ACCESS_NAME, > >>>>> + IOTEST_ACCESS_DATA, > >>>>> + IOTEST_ACCESS_MAX, > >>>>> +}; > >>>>> + > >>>>> +#define IOTEST_ACCESS_TYPE uint8_t > >>>>> +#define IOTEST_ACCESS_WIDTH (sizeof(uint8_t)) > >>>>> + > >>>>> +typedef struct PCITestDevState { > >>>>> + PCIDevice dev; > >>>>> + MemoryRegion mmio; > >>>>> + MemoryRegion portio; > >>>>> + IOTest *tests; > >>>>> + int current; > >>>>> +} PCITestDevState; > >>>>> + > >>>>> +#define IOTEST_IS_MEM(i) (strcmp(IOTEST_TYPE(i), "portio")) > >>>>> +#define IOTEST_REGION(d, i) (IOTEST_IS_MEM(i) ? &(d)->mmio : &(d)->portio) > >>>>> +#define IOTEST_SIZE(i) (IOTEST_IS_MEM(i) ? IOTEST_MEMSIZE : IOTEST_IOSIZE) > >>>>> +#define IOTEST_PCI_BAR(i) (IOTEST_IS_MEM(i) ? PCI_BASE_ADDRESS_SPACE_MEMORY : \ > >>>>> + PCI_BASE_ADDRESS_SPACE_IO) > >>>>> + > >>>>> +static int pci_testdev_start(IOTest *test) > >>>>> +{ > >>>>> + test->hdr->count = 0; > >>>>> + if (!test->hasnotifier) { > >>>>> + return 0; > >>>>> + } > >>>>> + event_notifier_test_and_clear(&test->notifier); > >>>>> + memory_region_add_eventfd(test->mr, > >>>>> + le32_to_cpu(test->hdr->offset), > >>>>> + test->size, > >>>>> + test->match_data, > >>>>> + test->hdr->data, > >>>>> + &test->notifier); > >>>>> + return 0; > >>>>> +} > >>>>> + > >>>>> +static void pci_testdev_stop(IOTest *test) > >>>>> +{ > >>>>> + if (!test->hasnotifier) { > >>>>> + return; > >>>>> + } > >>>>> + memory_region_del_eventfd(test->mr, > >>>>> + le32_to_cpu(test->hdr->offset), > >>>>> + test->size, > >>>>> + test->match_data, > >>>>> + test->hdr->data, > >>>>> + &test->notifier); > >>>>> +} > >>>>> + > >>>>> +static void > >>>>> +pci_testdev_reset(PCITestDevState *d) > >>>>> +{ > >>>>> + if (d->current == -1) { > >>>>> + return; > >>>>> + } > >>>>> + pci_testdev_stop(&d->tests[d->current]); > >>>>> + d->current = -1; > >>>>> +} > >>>>> + > >>>>> +static void pci_testdev_inc(IOTest *test, unsigned inc) > >>>>> +{ > >>>>> + uint32_t c = le32_to_cpu(test->hdr->count); > >>>>> + test->hdr->count = cpu_to_le32(c + inc); > >>>>> +} > >>>>> + > >>>>> +static void > >>>>> +pci_testdev_write(void *opaque, hwaddr addr, uint64_t val, > >>>>> + unsigned size, int type) > >>>>> +{ > >>>>> + PCITestDevState *d = opaque; > >>>>> + IOTest *test; > >>>>> + int t, r; > >>>>> + > >>>>> + if (addr == offsetof(PCITestDevHdr, test)) { > >>>>> + pci_testdev_reset(d); > >>>>> + if (val >= IOTEST_MAX_TEST) { > >>>>> + return; > >>>>> + } > >>>>> + t = type * IOTEST_MAX_TEST + val; > >>>>> + r = pci_testdev_start(&d->tests[t]); > >>>>> + if (r < 0) { > >>>>> + return; > >>>>> + } > >>>>> + d->current = t; > >>>>> + return; > >>>>> + } > >>>>> + if (d->current < 0) { > >>>>> + return; > >>>>> + } > >>>>> + test = &d->tests[d->current]; > >>>>> + if (addr != le32_to_cpu(test->hdr->offset)) { > >>>>> + return; > >>>>> + } > >>>>> + if (test->match_data && test->size != size) { > >>>>> + return; > >>>>> + } > >>>>> + if (test->match_data && val != test->hdr->data) { > >>>>> + return; > >>>>> + } > >>>>> + pci_testdev_inc(test, 1); > >>>>> +} > >>>>> + > >>>>> +static uint64_t > >>>>> +pci_testdev_read(void *opaque, hwaddr addr, unsigned size) > >>>>> +{ > >>>>> + PCITestDevState *d = opaque; > >>>>> + const char *buf; > >>>>> + IOTest *test; > >>>>> + if (d->current < 0) { > >>>>> + return 0; > >>>>> + } > >>>>> + test = &d->tests[d->current]; > >>>>> + buf = (const char *)test->hdr; > >>>>> + if (addr + size >= test->bufsize) { > >>>>> + return 0; > >>>>> + } > >>>>> + if (test->hasnotifier) { > >>>>> + event_notifier_test_and_clear(&test->notifier); > >>>>> + } > >>>>> + return buf[addr]; > >>>>> +} > >>>>> + > >>>>> +static void > >>>>> +pci_testdev_mmio_write(void *opaque, hwaddr addr, uint64_t val, > >>>>> + unsigned size) > >>>>> +{ > >>>>> + pci_testdev_write(opaque, addr, val, size, 0); > >>>>> +} > >>>>> + > >>>>> +static void > >>>>> +pci_testdev_pio_write(void *opaque, hwaddr addr, uint64_t val, > >>>>> + unsigned size) > >>>>> +{ > >>>>> + pci_testdev_write(opaque, addr, val, size, 1); > >>>>> +} > >>>>> + > >>>>> +static const MemoryRegionOps pci_testdev_mmio_ops = { > >>>>> + .read = pci_testdev_read, > >>>>> + .write = pci_testdev_mmio_write, > >>>>> + .endianness = DEVICE_LITTLE_ENDIAN, > >>>>> + .impl = { > >>>>> + .min_access_size = 1, > >>>>> + .max_access_size = 1, > >>>>> + }, > >>>>> +}; > >>>>> + > >>>>> +static const MemoryRegionOps pci_testdev_pio_ops = { > >>>>> + .read = pci_testdev_read, > >>>>> + .write = pci_testdev_pio_write, > >>>>> + .endianness = DEVICE_LITTLE_ENDIAN, > >>>>> + .impl = { > >>>>> + .min_access_size = 1, > >>>>> + .max_access_size = 1, > >>>>> + }, > >>>>> +}; > >>>>> + > >>>>> +static int pci_testdev_init(PCIDevice *pci_dev) > >>>>> +{ > >>>>> + PCITestDevState *d = DO_UPCAST(PCITestDevState, dev, pci_dev); > >>>>> + uint8_t *pci_conf; > >>>>> + char *name; > >>>>> + int r, i; > >>>>> + > >>>>> + pci_conf = d->dev.config; > >>>>> + > >>>>> + pci_conf[PCI_INTERRUPT_PIN] = 0; /* no interrupt pin */ > >>>>> + > >>>>> + memory_region_init_io(&d->mmio, &pci_testdev_mmio_ops, d, > >>>>> + "pci-testdev-mmio", IOTEST_MEMSIZE * 2); > >>>>> + memory_region_init_io(&d->portio, &pci_testdev_pio_ops, d, > >>>>> + "pci-testdev-portio", IOTEST_IOSIZE * 2); > >>>>> + pci_register_bar(&d->dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &d->mmio); > >>>>> + pci_register_bar(&d->dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &d->portio); > >>>>> + > >>>>> + d->current = -1; > >>>>> + d->tests = g_malloc0(IOTEST_MAX * sizeof *d->tests); > >>>>> + for (i = 0; i < IOTEST_MAX; ++i) { > >>>>> + IOTest *test = &d->tests[i]; > >>>>> + name = g_strdup_printf("%s-%s", IOTEST_TYPE(i), IOTEST_TEST(i)); > >>>>> + test->bufsize = sizeof(PCITestDevHdr) + strlen(name) + 1; > >>>>> + test->hdr = g_malloc0(test->bufsize); > >>>>> + memcpy(test->hdr->name, name, strlen(name) + 1); > >>>>> + g_free(name); > >>>>> + test->hdr->offset = cpu_to_le32(IOTEST_SIZE(i) + i * IOTEST_ACCESS_WIDTH); > >>>>> + test->size = IOTEST_ACCESS_WIDTH; > >>>>> + test->match_data = strcmp(IOTEST_TEST(i), "wildcard-eventfd"); > >>>>> + test->hdr->test = i; > >>>>> + test->hdr->data = test->match_data ? IOTEST_DATAMATCH : IOTEST_NOMATCH; > >>>>> + test->hdr->width = IOTEST_ACCESS_WIDTH; > >>>>> + test->mr = IOTEST_REGION(d, i); > >>>>> + if (!strcmp(IOTEST_TEST(i), "no-eventfd")) { > >>>>> + test->hasnotifier = false; > >>>>> + continue; > >>>>> + } > >>>>> + r = event_notifier_init(&test->notifier, 0); > >>>>> + assert(r >= 0); > >>>>> + test->hasnotifier = true; > >>>>> + } > >>>>> + > >>>>> + return 0; > >>>>> +} > >>>>> + > >>>>> +static void > >>>>> +pci_testdev_uninit(PCIDevice *dev) > >>>>> +{ > >>>>> + PCITestDevState *d = DO_UPCAST(PCITestDevState, dev, dev); > >>>>> + int i; > >>>>> + > >>>>> + pci_testdev_reset(d); > >>>>> + for (i = 0; i < IOTEST_MAX; ++i) { > >>>>> + if (d->tests[i].hasnotifier) { > >>>>> + event_notifier_cleanup(&d->tests[i].notifier); > >>>>> + } > >>>>> + g_free(d->tests[i].hdr); > >>>>> + } > >>>>> + g_free(d->tests); > >>>>> + memory_region_destroy(&d->mmio); > >>>>> + memory_region_destroy(&d->portio); > >>>>> +} > >>>>> + > >>>>> +static void qdev_pci_testdev_reset(DeviceState *dev) > >>>>> +{ > >>>>> + PCITestDevState *d = DO_UPCAST(PCITestDevState, dev.qdev, dev); > >>>>> + pci_testdev_reset(d); > >>>>> +} > >>>>> + > >>>>> +static void pci_testdev_class_init(ObjectClass *klass, void *data) > >>>>> +{ > >>>>> + DeviceClass *dc = DEVICE_CLASS(klass); > >>>>> + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > >>>>> + > >>>>> + k->init = pci_testdev_init; > >>>>> + k->exit = pci_testdev_uninit; > >>>>> + k->vendor_id = PCI_VENDOR_ID_REDHAT; > >>>>> + k->device_id = PCI_DEVICE_ID_REDHAT_TEST; > >>>>> + k->revision = 0x00; > >>>>> + k->class_id = PCI_CLASS_OTHERS; > >>>>> + dc->desc = "PCI Test Device"; > >>>>> + dc->reset = qdev_pci_testdev_reset; > >>>>> +} > >>>>> + > >>>>> +static const TypeInfo pci_testdev_info = { > >>>>> + .name = "pci-testdev", > >>>>> + .parent = TYPE_PCI_DEVICE, > >>>>> + .instance_size = sizeof(PCITestDevState), > >>>>> + .class_init = pci_testdev_class_init, > >>>>> +}; > >>>>> + > >>>>> +static void pci_testdev_register_types(void) > >>>>> +{ > >>>>> + type_register_static(&pci_testdev_info); > >>>>> +} > >>>>> + > >>>>> +type_init(pci_testdev_register_types) > >>>>> diff --git a/hw/pci/pci.h b/hw/pci/pci.h > >>>>> index 774369c..d81198c 100644 > >>>>> --- a/hw/pci/pci.h > >>>>> +++ b/hw/pci/pci.h > >>>>> @@ -84,6 +84,7 @@ > >>>>> #define PCI_DEVICE_ID_REDHAT_SERIAL 0x0002 > >>>>> #define PCI_DEVICE_ID_REDHAT_SERIAL2 0x0003 > >>>>> #define PCI_DEVICE_ID_REDHAT_SERIAL4 0x0004 > >>>>> +#define PCI_DEVICE_ID_REDHAT_TEST 0x0005 > >>>>> #define PCI_DEVICE_ID_REDHAT_QXL 0x0100 > >>>>> > >>>>> #define FMT_PCIBUS PRIx64 > >>>>>
Il 03/04/2013 12:33, Michael S. Tsirkin ha scritto: > > Because someone wasn't looking. :) The hw/ reorganization patches I've > > posted fix that. > > Still not sure how to merge this, if it goes in through my tree > and that's the only comment, I'll just fix it silently ... Indeed. In fact, considering my hw/ reorganization patches will be committed soon, please put it in hw/pci (matching hw/isa/pc-testdev.c). Another comment is that you should add a description in docs/specs. Paolo
On Wed, Apr 03, 2013 at 12:34:24PM +0200, Paolo Bonzini wrote: > Il 03/04/2013 12:33, Michael S. Tsirkin ha scritto: > > > Because someone wasn't looking. :) The hw/ reorganization patches I've > > > posted fix that. > > > > Still not sure how to merge this, if it goes in through my tree > > and that's the only comment, I'll just fix it silently ... > > Indeed. In fact, considering my hw/ reorganization patches will be > committed soon, please put it in hw/pci (matching hw/isa/pc-testdev.c). Paolo, hw/pci is pci core, I haven't looked at your reorg patches, but please do not move devices there. Sorting devices by connection is also wrong I think, by function would be better. > Another comment is that you should add a description in docs/specs. > > Paolo pc-test has none ... I'll see what I can do.
Il 03/04/2013 12:38, Michael S. Tsirkin ha scritto: > On Wed, Apr 03, 2013 at 12:34:24PM +0200, Paolo Bonzini wrote: >> Il 03/04/2013 12:33, Michael S. Tsirkin ha scritto: >>>> Because someone wasn't looking. :) The hw/ reorganization patches I've >>>> posted fix that. >>> >>> Still not sure how to merge this, if it goes in through my tree >>> and that's the only comment, I'll just fix it silently ... >> >> Indeed. In fact, considering my hw/ reorganization patches will be >> committed soon, please put it in hw/pci (matching hw/isa/pc-testdev.c). > > Paolo, hw/pci is pci core, I haven't looked at your reorg patches, > but please do not move devices there. > Sorting devices by connection is also wrong I think, by function > would be better. Indeed that's how most devices are sorted. For example, PCI host devices/bridges/etc. are in hw/pci (together with the PCI core, making hw/pci basically all that goes in through your tree), ISA host devices are in hw/isa, etc. However, there are a few exceptions. You cannot really sort out 600 files without exceptions. All USB devices are in hw/usb, and the existing test devices (debugexit, testdev) are in hw/isa. There is only one exception you should care about, namely that VFIO and ivshmem are also in hw/pci. Here is the list of files in hw/pci: host-apb.c host-bonito.c host-dec.c host-dec.h host-grackle.c host-gt64xxx.c host-piix.c host-ppc4xx.c host-ppce500.c host-prep.c host-q35.c host-sh.c host-spapr.c host-uninorth.c host-versatile.c i82801b11.c ioh3420.c ioh3420.h ivshmem.c msi.c msi.h msix.c msix.h pam.c pci-hotplug.c pci-stub.c pci.c pci.h pci_bridge.c pci_bridge.h pci_bridge_dev.c pci_bus.h pci_host.c pci_host.h pci_ids.h pci_regs.h pcie.c pcie.h pcie_aer.c pcie_aer.h pcie_host.c pcie_host.h pcie_port.c pcie_port.h pcie_regs.h shpc.c shpc.h slotid_cap.c slotid_cap.h vfio.c xio3130_downstream.c xio3130_downstream.h xio3130_upstream.c xio3130_upstream.h Paolo > >> Another comment is that you should add a description in docs/specs. >> >> Paolo > > pc-test has none ... I'll see what I can do. >
On Wed, Apr 03, 2013 at 01:48:41PM +0200, Paolo Bonzini wrote: > Il 03/04/2013 12:38, Michael S. Tsirkin ha scritto: > > On Wed, Apr 03, 2013 at 12:34:24PM +0200, Paolo Bonzini wrote: > >> Il 03/04/2013 12:33, Michael S. Tsirkin ha scritto: > >>>> Because someone wasn't looking. :) The hw/ reorganization patches I've > >>>> posted fix that. > >>> > >>> Still not sure how to merge this, if it goes in through my tree > >>> and that's the only comment, I'll just fix it silently ... > >> > >> Indeed. In fact, considering my hw/ reorganization patches will be > >> committed soon, please put it in hw/pci (matching hw/isa/pc-testdev.c). > > > > Paolo, hw/pci is pci core, I haven't looked at your reorg patches, > > but please do not move devices there. > > Sorting devices by connection is also wrong I think, by function > > would be better. > > Indeed that's how most devices are sorted. For example, PCI host > devices/bridges/etc. are in hw/pci (together with the PCI core, making > hw/pci basically all that goes in through your tree), Well host bridges often do lots of things besides pci on the same chip. > ISA host devices > are in hw/isa, etc. what do you mean "host ISA device"? > However, there are a few exceptions. You cannot really sort out 600 > files without exceptions. All USB devices are in hw/usb, and the > existing test devices (debugexit, testdev) are in hw/isa. > > There is only one exception you should care about, namely that VFIO and > ivshmem are also in hw/pci. This makes no sense really. Pls add hw/misc or just keep misc stuff in hw/ We don't need to have everything in subdirectories. > Here is the list of files in hw/pci: > > host-apb.c > host-bonito.c > host-dec.c > host-dec.h > host-grackle.c > host-gt64xxx.c > host-piix.c > host-ppc4xx.c > host-ppce500.c > host-prep.c > host-q35.c > host-sh.c > host-spapr.c > host-uninorth.c > host-versatile.c > i82801b11.c > ioh3420.c > ioh3420.h > ivshmem.c > msi.c > msi.h > msix.c > msix.h > pam.c > pci-hotplug.c > pci-stub.c > pci.c > pci.h > pci_bridge.c > pci_bridge.h > pci_bridge_dev.c > pci_bus.h > pci_host.c > pci_host.h > pci_ids.h > pci_regs.h > pcie.c > pcie.h > pcie_aer.c > pcie_aer.h > pcie_host.c > pcie_host.h > pcie_port.c > pcie_port.h > pcie_regs.h > shpc.c > shpc.h > slotid_cap.c > slotid_cap.h > vfio.c > xio3130_downstream.c > xio3130_downstream.h > xio3130_upstream.c > xio3130_upstream.h > > Paolo This messes up things. pci core is separate from devices using it, and it's important to me. Really just add hw/bridge/ and put all kind of bridge devices there. > > > >> Another comment is that you should add a description in docs/specs. > >> > >> Paolo > > > > pc-test has none ... I'll see what I can do. > >
Il 03/04/2013 14:00, Michael S. Tsirkin ha scritto: > On Wed, Apr 03, 2013 at 01:48:41PM +0200, Paolo Bonzini wrote: >> Il 03/04/2013 12:38, Michael S. Tsirkin ha scritto: >>> On Wed, Apr 03, 2013 at 12:34:24PM +0200, Paolo Bonzini wrote: >>>> Il 03/04/2013 12:33, Michael S. Tsirkin ha scritto: >>>>>> Because someone wasn't looking. :) The hw/ reorganization patches I've >>>>>> posted fix that. >>>>> >>>>> Still not sure how to merge this, if it goes in through my tree >>>>> and that's the only comment, I'll just fix it silently ... >>>> >>>> Indeed. In fact, considering my hw/ reorganization patches will be >>>> committed soon, please put it in hw/pci (matching hw/isa/pc-testdev.c). >>> >>> Paolo, hw/pci is pci core, I haven't looked at your reorg patches, >>> but please do not move devices there. >>> Sorting devices by connection is also wrong I think, by function >>> would be better. >> >> Indeed that's how most devices are sorted. For example, PCI host >> devices/bridges/etc. are in hw/pci (together with the PCI core, making >> hw/pci basically all that goes in through your tree), > > Well host bridges often do lots of things besides pci on the same > chip. Still the PCIness is quite important, seeing that almost all such devices have a name that looks like *_pci.c. >> ISA host devices >> are in hw/isa, etc. > > what do you mean "host ISA device"? PCI-ISA bridges and SuperIO chips: piix4.c, vt82c686.c, lpc_ich9.c, i82378.c. PIIX3 would also be there, it needs to be split out of piix_pci.c. >> However, there are a few exceptions. You cannot really sort out 600 >> files without exceptions. All USB devices are in hw/usb, and the >> existing test devices (debugexit, testdev) are in hw/isa. >> >> There is only one exception you should care about, namely that VFIO and >> ivshmem are also in hw/pci. > > This makes no sense really. > Pls add hw/misc or just keep misc stuff in hw/ hw/misc exists already, I'll move those two there. >> Here is the list of files in hw/pci: >> >> host-apb.c >> host-bonito.c >> host-dec.c >> host-dec.h >> host-grackle.c >> host-gt64xxx.c >> host-piix.c >> host-ppc4xx.c >> host-ppce500.c >> host-prep.c >> host-q35.c >> host-sh.c >> host-spapr.c >> host-uninorth.c >> host-versatile.c >> i82801b11.c >> ioh3420.c >> ioh3420.h >> ivshmem.c >> msi.c >> msi.h >> msix.c >> msix.h >> pam.c >> pci-hotplug.c >> pci-stub.c >> pci.c >> pci.h >> pci_bridge.c >> pci_bridge.h >> pci_bridge_dev.c >> pci_bus.h >> pci_host.c >> pci_host.h >> pci_ids.h >> pci_regs.h >> pcie.c >> pcie.h >> pcie_aer.c >> pcie_aer.h >> pcie_host.c >> pcie_host.h >> pcie_port.c >> pcie_port.h >> pcie_regs.h >> shpc.c >> shpc.h >> slotid_cap.c >> slotid_cap.h >> vfio.c >> xio3130_downstream.c >> xio3130_downstream.h >> xio3130_upstream.c >> xio3130_upstream.h >> >> Paolo > > This messes up things. pci core is separate from devices using it, > and it's important to me. Really just add hw/bridge/ and put all > kind of bridge devices there. Ok, I'll add hw/pci/bridge, and remove the "host-" prefix for host PCI devices. Paolo
On Wed, Apr 03, 2013 at 02:05:32PM +0200, Paolo Bonzini wrote: > Il 03/04/2013 14:00, Michael S. Tsirkin ha scritto: > > On Wed, Apr 03, 2013 at 01:48:41PM +0200, Paolo Bonzini wrote: > >> Il 03/04/2013 12:38, Michael S. Tsirkin ha scritto: > >>> On Wed, Apr 03, 2013 at 12:34:24PM +0200, Paolo Bonzini wrote: > >>>> Il 03/04/2013 12:33, Michael S. Tsirkin ha scritto: > >>>>>> Because someone wasn't looking. :) The hw/ reorganization patches I've > >>>>>> posted fix that. > >>>>> > >>>>> Still not sure how to merge this, if it goes in through my tree > >>>>> and that's the only comment, I'll just fix it silently ... > >>>> > >>>> Indeed. In fact, considering my hw/ reorganization patches will be > >>>> committed soon, please put it in hw/pci (matching hw/isa/pc-testdev.c). > >>> > >>> Paolo, hw/pci is pci core, I haven't looked at your reorg patches, > >>> but please do not move devices there. > >>> Sorting devices by connection is also wrong I think, by function > >>> would be better. > >> > >> Indeed that's how most devices are sorted. For example, PCI host > >> devices/bridges/etc. are in hw/pci (together with the PCI core, making > >> hw/pci basically all that goes in through your tree), > > > > Well host bridges often do lots of things besides pci on the same > > chip. > > Still the PCIness is quite important, seeing that almost all such > devices have a name that looks like *_pci.c. Some of them do. For some of them name is not a very successful name. > >> ISA host devices > >> are in hw/isa, etc. > > > > what do you mean "host ISA device"? > > PCI-ISA bridges and SuperIO chips: piix4.c, vt82c686.c, lpc_ich9.c, > i82378.c. PIIX3 would also be there, it needs to be split out of > piix_pci.c. > > >> However, there are a few exceptions. You cannot really sort out 600 > >> files without exceptions. All USB devices are in hw/usb, and the > >> existing test devices (debugexit, testdev) are in hw/isa. > >> > >> There is only one exception you should care about, namely that VFIO and > >> ivshmem are also in hw/pci. > > > > This makes no sense really. > > Pls add hw/misc or just keep misc stuff in hw/ > > hw/misc exists already, I'll move those two there. > > >> Here is the list of files in hw/pci: > >> > >> host-apb.c > >> host-bonito.c > >> host-dec.c > >> host-dec.h > >> host-grackle.c > >> host-gt64xxx.c > >> host-piix.c > >> host-ppc4xx.c > >> host-ppce500.c > >> host-prep.c > >> host-q35.c > >> host-sh.c > >> host-spapr.c > >> host-uninorth.c > >> host-versatile.c > >> i82801b11.c > >> ioh3420.c > >> ioh3420.h > >> ivshmem.c > >> msi.c > >> msi.h > >> msix.c > >> msix.h > >> pam.c > >> pci-hotplug.c > >> pci-stub.c > >> pci.c > >> pci.h > >> pci_bridge.c > >> pci_bridge.h > >> pci_bridge_dev.c > >> pci_bus.h > >> pci_host.c > >> pci_host.h > >> pci_ids.h > >> pci_regs.h > >> pcie.c > >> pcie.h > >> pcie_aer.c > >> pcie_aer.h > >> pcie_host.c > >> pcie_host.h > >> pcie_port.c > >> pcie_port.h > >> pcie_regs.h > >> shpc.c > >> shpc.h > >> slotid_cap.c > >> slotid_cap.h > >> vfio.c > >> xio3130_downstream.c > >> xio3130_downstream.h > >> xio3130_upstream.c > >> xio3130_upstream.h > >> > >> Paolo > > > > This messes up things. pci core is separate from devices using it, > > and it's important to me. Really just add hw/bridge/ and put all > > kind of bridge devices there. > > Ok, I'll add hw/pci/bridge, and remove the "host-" prefix for host PCI > devices. > > Paolo That's too much nesting I think. hw/bridge/ and we can put isa bridges there as well then.
Il 03/04/2013 16:06, Michael S. Tsirkin ha scritto: > > Ok, I'll add hw/pci/bridge, and remove the "host-" prefix for host PCI > > devices. > > That's too much nesting I think. hw/bridge/ and we can put isa bridges there > as well then. You need to group similar devices for the nesting to be useful. For example, it should be easy to check if something is true of all ISA bridges, or to do the same change in all of them. ISA and PCI bridges have too little in common for that (and why not put I2C and SPI in hw/bridge too :)). Paolo
On Wed, Apr 03, 2013 at 04:08:45PM +0200, Paolo Bonzini wrote: > Il 03/04/2013 16:06, Michael S. Tsirkin ha scritto: > > > Ok, I'll add hw/pci/bridge, and remove the "host-" prefix for host PCI > > > devices. > > > > That's too much nesting I think. hw/bridge/ and we can put isa bridges there > > as well then. > > You need to group similar devices for the nesting to be useful. For > example, it should be easy to check if something is true of all ISA > bridges, or to do the same change in all of them. ISA and PCI bridges > have too little in common for that (and why not put I2C and SPI in > hw/bridge too :)). > > Paolo Yes, why not. What all bridges need to share is their modeling needs to be similar. That's one thing that practically needs to be cleaned up I think.
On Wed, Apr 03, 2013 at 04:08:45PM +0200, Paolo Bonzini wrote: > Il 03/04/2013 16:06, Michael S. Tsirkin ha scritto: > > > Ok, I'll add hw/pci/bridge, and remove the "host-" prefix for host PCI > > > devices. > > > > That's too much nesting I think. hw/bridge/ and we can put isa bridges there > > as well then. > > You need to group similar devices for the nesting to be useful. For > example, it should be easy to check if something is true of all ISA > bridges, or to do the same change in all of them. ISA and PCI bridges > have too little in common for that (and why not put I2C and SPI in > hw/bridge too :)). > > Paolo At some level it might make sense to have pci/ and isa/ with core code at the top level. Specific devices would go into hw/pci/ hw/isa/ ... But will this conflict with how libhw works at the moment? We don't want to rebuild pci for each target ...
On Wed, Apr 03, 2013 at 04:08:45PM +0200, Paolo Bonzini wrote: > Il 03/04/2013 16:06, Michael S. Tsirkin ha scritto: > > > Ok, I'll add hw/pci/bridge, and remove the "host-" prefix for host PCI > > > devices. > > > > That's too much nesting I think. hw/bridge/ and we can put isa bridges there > > as well then. > > You need to group similar devices for the nesting to be useful. For > example, it should be easy to check if something is true of all ISA > bridges, or to do the same change in all of them. ISA and PCI bridges > have too little in common for that (and why not put I2C and SPI in > hw/bridge too :)). > > Paolo Okay after some more thought. hw/pci-host/ hw/pci-bridge/ hw/isa-bridge/ ?
Il 03/04/2013 17:09, Michael S. Tsirkin ha scritto: > Okay after some more thought. > hw/pci-host/ > hw/pci-bridge/ > hw/isa-bridge/ Renaming hw/isa to hw/isa-bridge is easy. For the rest, I would prefer hw/pci/{core,host,bridge}, but whatever you like the most is fine for me too. Paolo
Il 03/04/2013 16:28, Michael S. Tsirkin ha scritto: > > You need to group similar devices for the nesting to be useful. For > > example, it should be easy to check if something is true of all ISA > > bridges, or to do the same change in all of them. ISA and PCI bridges > > have too little in common for that (and why not put I2C and SPI in > > hw/bridge too :)). > > Yes, why not. What all bridges need to share is their modeling > needs to be similar. That's one thing that practically > needs to be cleaned up I think. Bridges are simply devices that expose their own bus, or that derive from a class that does. But bridge is not a universal word, some buses use controller or adapter, it would be weird to have hw/bridge/i2c or hw/bridge/scsi (and leave two files only in hw/scsi). > But will this conflict with how libhw works at the moment? > We don't want to rebuild pci for each target ... No, we won't. In fact, almost everything should be built once only. As far as PCI is concerned, if it's not it is because of some really bad hacks. For unmaintained boards, it's best to stash them in hw/ARCH. If we limit the amount of files that are built per-target, it works nicely. Paolo
On Wed, Apr 03, 2013 at 05:46:00PM +0200, Paolo Bonzini wrote: > Il 03/04/2013 16:28, Michael S. Tsirkin ha scritto: > > > You need to group similar devices for the nesting to be useful. For > > > example, it should be easy to check if something is true of all ISA > > > bridges, or to do the same change in all of them. ISA and PCI bridges > > > have too little in common for that (and why not put I2C and SPI in > > > hw/bridge too :)). > > > > Yes, why not. What all bridges need to share is their modeling > > needs to be similar. That's one thing that practically > > needs to be cleaned up I think. > > Bridges are simply devices that expose their own bus, or that derive > from a class that does. But bridge is not a universal word, some buses > use controller or adapter, it would be weird to have hw/bridge/i2c or > hw/bridge/scsi (and leave two files only in hw/scsi). > > > But will this conflict with how libhw works at the moment? > > We don't want to rebuild pci for each target ... > > No, we won't. In fact, almost everything should be built once only. As > far as PCI is concerned, if it's not it is because of some really bad > hacks. For unmaintained boards, it's best to stash them in hw/ARCH. > > If we limit the amount of files that are built per-target, it works nicely. > > Paolo Well ATM it's part of libhw and is built twice. Not sure what do you propose here.
Il 03/04/2013 19:04, Michael S. Tsirkin ha scritto: > Well ATM it's part of libhw and is built twice. Not sure what > do you propose here. Things haven't been built twice for a few months. hwaddr is unconditionally 64-bit wide. Paolo
On Wed, Apr 03, 2013 at 05:43:40PM +0200, Paolo Bonzini wrote: > Il 03/04/2013 17:09, Michael S. Tsirkin ha scritto: > > Okay after some more thought. > > hw/pci-host/ > > hw/pci-bridge/ > > hw/isa-bridge/ > > Renaming hw/isa to hw/isa-bridge is easy. > > For the rest, I would prefer hw/pci/{core,host,bridge}, but whatever you > like the most is fine for me too. > > Paolo I'd prefer a single level as above, I get lost easily.
----- Messaggio originale ----- > Da: "Michael S. Tsirkin" <mst@redhat.com> > A: "Paolo Bonzini" <pbonzini@redhat.com> > Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org, gleb@redhat.com, mtosatti@redhat.com > Inviato: Mercoledì, 3 aprile 2013 20:39:58 > Oggetto: Re: [PATCH 4/4] pci: add pci test device > > On Wed, Apr 03, 2013 at 05:43:40PM +0200, Paolo Bonzini wrote: > > Il 03/04/2013 17:09, Michael S. Tsirkin ha scritto: > > > Okay after some more thought. > > > hw/pci-host/ > > > hw/pci-bridge/ > > > hw/isa-bridge/ > > > > Renaming hw/isa to hw/isa-bridge is easy. > > > > For the rest, I would prefer hw/pci/{core,host,bridge}, but whatever you > > like the most is fine for me too. > > > > Paolo > > I'd prefer a single level as above, I get lost easily. The reason why I prefer deep structures is that I would like the directories to become the basis for a Kconfig menu. But I'll use the shallow structure, most of those symbols except in pci-bridge would probably be select-ed by the high level "board" symbols anyway. Paolo
diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs index a78c0b2..7497e7a 100644 --- a/hw/i386/Makefile.objs +++ b/hw/i386/Makefile.objs @@ -11,7 +11,7 @@ obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o xen_pt_msi.o obj-y += kvm/ obj-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o -obj-y += pc-testdev.o +obj-y += pc-testdev.o pci-testdev.o obj-y := $(addprefix ../,$(obj-y)) diff --git a/hw/pci-testdev.c b/hw/pci-testdev.c new file mode 100644 index 0000000..9486624 --- /dev/null +++ b/hw/pci-testdev.c @@ -0,0 +1,306 @@ +#include "hw/hw.h" +#include "hw/pci/pci.h" +#include "qemu/event_notifier.h" +#include "qemu/osdep.h" + +typedef struct PCITestDevHdr { + uint8_t test; + uint8_t width; + uint8_t pad0[2]; + uint32_t offset; + uint8_t data; + uint8_t pad1[3]; + uint32_t count; + uint8_t name[]; +} PCITestDevHdr; + +typedef struct IOTest { + MemoryRegion *mr; + EventNotifier notifier; + bool hasnotifier; + unsigned size; + bool match_data; + PCITestDevHdr *hdr; + unsigned bufsize; +} IOTest; + +#define IOTEST_DATAMATCH 0xFA +#define IOTEST_NOMATCH 0xCE + +#define IOTEST_IOSIZE 128 +#define IOTEST_MEMSIZE 2048 + +static const char *iotest_test[] = { + "no-eventfd", + "wildcard-eventfd", + "datamatch-eventfd" +}; + +static const char *iotest_type[] = { + "mmio", + "portio" +}; + +#define IOTEST_TEST(i) (iotest_test[((i) % ARRAY_SIZE(iotest_test))]) +#define IOTEST_TYPE(i) (iotest_type[((i) / ARRAY_SIZE(iotest_test))]) +#define IOTEST_MAX_TEST (ARRAY_SIZE(iotest_test)) +#define IOTEST_MAX_TYPE (ARRAY_SIZE(iotest_type)) +#define IOTEST_MAX (IOTEST_MAX_TEST * IOTEST_MAX_TYPE) + +enum { + IOTEST_ACCESS_NAME, + IOTEST_ACCESS_DATA, + IOTEST_ACCESS_MAX, +}; + +#define IOTEST_ACCESS_TYPE uint8_t +#define IOTEST_ACCESS_WIDTH (sizeof(uint8_t)) + +typedef struct PCITestDevState { + PCIDevice dev; + MemoryRegion mmio; + MemoryRegion portio; + IOTest *tests; + int current; +} PCITestDevState; + +#define IOTEST_IS_MEM(i) (strcmp(IOTEST_TYPE(i), "portio")) +#define IOTEST_REGION(d, i) (IOTEST_IS_MEM(i) ? &(d)->mmio : &(d)->portio) +#define IOTEST_SIZE(i) (IOTEST_IS_MEM(i) ? IOTEST_MEMSIZE : IOTEST_IOSIZE) +#define IOTEST_PCI_BAR(i) (IOTEST_IS_MEM(i) ? PCI_BASE_ADDRESS_SPACE_MEMORY : \ + PCI_BASE_ADDRESS_SPACE_IO) + +static int pci_testdev_start(IOTest *test) +{ + test->hdr->count = 0; + if (!test->hasnotifier) { + return 0; + } + event_notifier_test_and_clear(&test->notifier); + memory_region_add_eventfd(test->mr, + le32_to_cpu(test->hdr->offset), + test->size, + test->match_data, + test->hdr->data, + &test->notifier); + return 0; +} + +static void pci_testdev_stop(IOTest *test) +{ + if (!test->hasnotifier) { + return; + } + memory_region_del_eventfd(test->mr, + le32_to_cpu(test->hdr->offset), + test->size, + test->match_data, + test->hdr->data, + &test->notifier); +} + +static void +pci_testdev_reset(PCITestDevState *d) +{ + if (d->current == -1) { + return; + } + pci_testdev_stop(&d->tests[d->current]); + d->current = -1; +} + +static void pci_testdev_inc(IOTest *test, unsigned inc) +{ + uint32_t c = le32_to_cpu(test->hdr->count); + test->hdr->count = cpu_to_le32(c + inc); +} + +static void +pci_testdev_write(void *opaque, hwaddr addr, uint64_t val, + unsigned size, int type) +{ + PCITestDevState *d = opaque; + IOTest *test; + int t, r; + + if (addr == offsetof(PCITestDevHdr, test)) { + pci_testdev_reset(d); + if (val >= IOTEST_MAX_TEST) { + return; + } + t = type * IOTEST_MAX_TEST + val; + r = pci_testdev_start(&d->tests[t]); + if (r < 0) { + return; + } + d->current = t; + return; + } + if (d->current < 0) { + return; + } + test = &d->tests[d->current]; + if (addr != le32_to_cpu(test->hdr->offset)) { + return; + } + if (test->match_data && test->size != size) { + return; + } + if (test->match_data && val != test->hdr->data) { + return; + } + pci_testdev_inc(test, 1); +} + +static uint64_t +pci_testdev_read(void *opaque, hwaddr addr, unsigned size) +{ + PCITestDevState *d = opaque; + const char *buf; + IOTest *test; + if (d->current < 0) { + return 0; + } + test = &d->tests[d->current]; + buf = (const char *)test->hdr; + if (addr + size >= test->bufsize) { + return 0; + } + if (test->hasnotifier) { + event_notifier_test_and_clear(&test->notifier); + } + return buf[addr]; +} + +static void +pci_testdev_mmio_write(void *opaque, hwaddr addr, uint64_t val, + unsigned size) +{ + pci_testdev_write(opaque, addr, val, size, 0); +} + +static void +pci_testdev_pio_write(void *opaque, hwaddr addr, uint64_t val, + unsigned size) +{ + pci_testdev_write(opaque, addr, val, size, 1); +} + +static const MemoryRegionOps pci_testdev_mmio_ops = { + .read = pci_testdev_read, + .write = pci_testdev_mmio_write, + .endianness = DEVICE_LITTLE_ENDIAN, + .impl = { + .min_access_size = 1, + .max_access_size = 1, + }, +}; + +static const MemoryRegionOps pci_testdev_pio_ops = { + .read = pci_testdev_read, + .write = pci_testdev_pio_write, + .endianness = DEVICE_LITTLE_ENDIAN, + .impl = { + .min_access_size = 1, + .max_access_size = 1, + }, +}; + +static int pci_testdev_init(PCIDevice *pci_dev) +{ + PCITestDevState *d = DO_UPCAST(PCITestDevState, dev, pci_dev); + uint8_t *pci_conf; + char *name; + int r, i; + + pci_conf = d->dev.config; + + pci_conf[PCI_INTERRUPT_PIN] = 0; /* no interrupt pin */ + + memory_region_init_io(&d->mmio, &pci_testdev_mmio_ops, d, + "pci-testdev-mmio", IOTEST_MEMSIZE * 2); + memory_region_init_io(&d->portio, &pci_testdev_pio_ops, d, + "pci-testdev-portio", IOTEST_IOSIZE * 2); + pci_register_bar(&d->dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &d->mmio); + pci_register_bar(&d->dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &d->portio); + + d->current = -1; + d->tests = g_malloc0(IOTEST_MAX * sizeof *d->tests); + for (i = 0; i < IOTEST_MAX; ++i) { + IOTest *test = &d->tests[i]; + name = g_strdup_printf("%s-%s", IOTEST_TYPE(i), IOTEST_TEST(i)); + test->bufsize = sizeof(PCITestDevHdr) + strlen(name) + 1; + test->hdr = g_malloc0(test->bufsize); + memcpy(test->hdr->name, name, strlen(name) + 1); + g_free(name); + test->hdr->offset = cpu_to_le32(IOTEST_SIZE(i) + i * IOTEST_ACCESS_WIDTH); + test->size = IOTEST_ACCESS_WIDTH; + test->match_data = strcmp(IOTEST_TEST(i), "wildcard-eventfd"); + test->hdr->test = i; + test->hdr->data = test->match_data ? IOTEST_DATAMATCH : IOTEST_NOMATCH; + test->hdr->width = IOTEST_ACCESS_WIDTH; + test->mr = IOTEST_REGION(d, i); + if (!strcmp(IOTEST_TEST(i), "no-eventfd")) { + test->hasnotifier = false; + continue; + } + r = event_notifier_init(&test->notifier, 0); + assert(r >= 0); + test->hasnotifier = true; + } + + return 0; +} + +static void +pci_testdev_uninit(PCIDevice *dev) +{ + PCITestDevState *d = DO_UPCAST(PCITestDevState, dev, dev); + int i; + + pci_testdev_reset(d); + for (i = 0; i < IOTEST_MAX; ++i) { + if (d->tests[i].hasnotifier) { + event_notifier_cleanup(&d->tests[i].notifier); + } + g_free(d->tests[i].hdr); + } + g_free(d->tests); + memory_region_destroy(&d->mmio); + memory_region_destroy(&d->portio); +} + +static void qdev_pci_testdev_reset(DeviceState *dev) +{ + PCITestDevState *d = DO_UPCAST(PCITestDevState, dev.qdev, dev); + pci_testdev_reset(d); +} + +static void pci_testdev_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); + + k->init = pci_testdev_init; + k->exit = pci_testdev_uninit; + k->vendor_id = PCI_VENDOR_ID_REDHAT; + k->device_id = PCI_DEVICE_ID_REDHAT_TEST; + k->revision = 0x00; + k->class_id = PCI_CLASS_OTHERS; + dc->desc = "PCI Test Device"; + dc->reset = qdev_pci_testdev_reset; +} + +static const TypeInfo pci_testdev_info = { + .name = "pci-testdev", + .parent = TYPE_PCI_DEVICE, + .instance_size = sizeof(PCITestDevState), + .class_init = pci_testdev_class_init, +}; + +static void pci_testdev_register_types(void) +{ + type_register_static(&pci_testdev_info); +} + +type_init(pci_testdev_register_types) diff --git a/hw/pci/pci.h b/hw/pci/pci.h index 774369c..d81198c 100644 --- a/hw/pci/pci.h +++ b/hw/pci/pci.h @@ -84,6 +84,7 @@ #define PCI_DEVICE_ID_REDHAT_SERIAL 0x0002 #define PCI_DEVICE_ID_REDHAT_SERIAL2 0x0003 #define PCI_DEVICE_ID_REDHAT_SERIAL4 0x0004 +#define PCI_DEVICE_ID_REDHAT_TEST 0x0005 #define PCI_DEVICE_ID_REDHAT_QXL 0x0100 #define FMT_PCIBUS PRIx64
This device is used for kvm unit tests, currently it supports testing performance of ioeventfd. Using updated kvm unittest, here's an example output: mmio-no-eventfd:pci-mem 8796 mmio-wildcard-eventfd:pci-mem 3609 mmio-datamatch-eventfd:pci-mem 3685 portio-no-eventfd:pci-io 5287 portio-wildcard-eventfd:pci-io 1762 portio-datamatch-eventfd:pci-io 1777 Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- hw/i386/Makefile.objs | 2 +- hw/pci-testdev.c | 306 ++++++++++++++++++++++++++++++++++++++++++++++++++ hw/pci/pci.h | 1 + 3 files changed, 308 insertions(+), 1 deletion(-) create mode 100644 hw/pci-testdev.c