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@xxxxxxxxxx>
> > ---
> > 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
> >
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
