From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40618) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bn21U-0000Rk-GN for qemu-devel@nongnu.org; Thu, 22 Sep 2016 07:18:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bn21Q-000547-0U for qemu-devel@nongnu.org; Thu, 22 Sep 2016 07:18:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40808) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bn21P-00053u-Nd for qemu-devel@nongnu.org; Thu, 22 Sep 2016 07:18:31 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id AEBE37F7BB for ; Thu, 22 Sep 2016 11:18:30 +0000 (UTC) Date: Thu, 22 Sep 2016 13:18:24 +0200 From: Andrew Jones Message-ID: <20160922111824.e5ax3zu3lhzjxb47@hawk.localdomain> References: <1474524908-18716-1-git-send-email-peterx@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1474524908-18716-1-git-send-email-peterx@redhat.com> Subject: Re: [Qemu-devel] [PATCH] pci-testdev: enhance to support new testcases List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Xu Cc: qemu-devel@nongnu.org, marcel@redhat.com, pbonzini@redhat.com, mst@redhat.com On Thu, Sep 22, 2016 at 02:15:08PM +0800, Peter Xu wrote: > pci-testdev is used mostly in kvm-unit-test for some eventfd tests. > However I see it a good framework for other tests as well (e.g., the > IOMMU unit test in the future). So enhanced it to support more > testcases. > > The original memory handlers and protocol are strict and not easy to > change (we need to keep the old behavior of pci-testdev). So I added a > new parameter for the device, and memory ops will be dynamically handled > depending on what testcase it is configured. To specify a new test case > for pci-testdev, we use: > > -device pci-testdev,testcase=XXX > > The default will be "eventfd", which is the original behavior for > pci-testdev. In the future, we can just add new testcase for pci-testdev > to achieve different goals. > > Signed-off-by: Peter Xu > --- > > This is kind-of a RFC since I am not sure whether this is a good way. I'm not either :-) I haven't looked too closely at this test device, but I have been involved in reviewing a kvm-unit-tests series[*] that will drive it. Please take a look at that series and maybe test with it as well. Thanks, drew [*] https://www.spinics.net/lists/kvm/msg136892.html > I did run the default kvm-unit-test cases on x86 to make sure it > didn't break anything. In case this is not good, I didn't write any > further test cases (e.g., emulate device DMAR/IR operations) yet. If > we like the idea, I can move on. > > Please kindly review. Thanks. > > hw/misc/pci-testdev.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 76 insertions(+), 4 deletions(-) > > diff --git a/hw/misc/pci-testdev.c b/hw/misc/pci-testdev.c > index 7d59902..b25d673 100644 > --- a/hw/misc/pci-testdev.c > +++ b/hw/misc/pci-testdev.c > @@ -22,6 +22,19 @@ > #include "hw/pci/pci.h" > #include "qemu/event_notifier.h" > #include "sysemu/kvm.h" > +#include "qemu/error-report.h" > + > +/* Type: 0 for MMIO write, 1 for PIO write. */ > +typedef void (*pci_testdev_write_op)(void *opaque, hwaddr addr, > + uint64_t val, unsigned size, int type); > +typedef uint64_t (*pci_testdev_read_op)(void *opaque, hwaddr addr, > + unsigned size); > + > +struct testcase { > + const char *name; > + pci_testdev_write_op write_op; > + pci_testdev_read_op read_op; > +}; > > typedef struct PCITestDevHdr { > uint8_t test; > @@ -85,6 +98,8 @@ typedef struct PCITestDevState { > MemoryRegion portio; > IOTest *tests; > int current; > + char *testcase_name; > + struct testcase *testcase; > } PCITestDevState; > > #define TYPE_PCI_TEST_DEV "pci-testdev" > @@ -200,22 +215,58 @@ pci_testdev_read(void *opaque, hwaddr addr, unsigned size) > return buf[addr]; > } > > +/* > + * To add a new test, we need to implement both write_op and read_op, > + * and add a new "struct testcase" into the global pci_testcases[]. > + */ > +struct testcase pci_testcases[] = { > + {"eventfd", pci_testdev_write, pci_testdev_read}, > + {NULL, NULL, NULL}, > +}; > + > +#define FOREACH_TEST_CASE(n) for (n = &pci_testcases[0]; n->name; n++) > + > +static struct testcase * > +pci_testdev_find_testcase(char *name) > +{ > + struct testcase *test; > + > + FOREACH_TEST_CASE(test) { > + if (!strcmp(test->name, name)) { > + return test; > + } > + } > + return NULL; > +} > + > +static uint64_t > +pci_testdev_common_read(void *opaque, hwaddr addr, unsigned size) > +{ > + PCITestDevState *d = opaque; > + pci_testdev_read_op read_op = d->testcase->read_op; > + return read_op(opaque, addr, size); > +} > + > static void > pci_testdev_mmio_write(void *opaque, hwaddr addr, uint64_t val, > unsigned size) > { > - pci_testdev_write(opaque, addr, val, size, 0); > + PCITestDevState *d = opaque; > + pci_testdev_write_op write_op = d->testcase->write_op; > + write_op(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); > + PCITestDevState *d = opaque; > + pci_testdev_write_op write_op = d->testcase->write_op; > + write_op(opaque, addr, val, size, 1); > } > > static const MemoryRegionOps pci_testdev_mmio_ops = { > - .read = pci_testdev_read, > + .read = pci_testdev_common_read, > .write = pci_testdev_mmio_write, > .endianness = DEVICE_LITTLE_ENDIAN, > .impl = { > @@ -225,7 +276,7 @@ static const MemoryRegionOps pci_testdev_mmio_ops = { > }; > > static const MemoryRegionOps pci_testdev_pio_ops = { > - .read = pci_testdev_read, > + .read = pci_testdev_common_read, > .write = pci_testdev_pio_write, > .endianness = DEVICE_LITTLE_ENDIAN, > .impl = { > @@ -281,6 +332,21 @@ static void pci_testdev_realize(PCIDevice *pci_dev, Error **errp) > assert(r >= 0); > test->hasnotifier = true; > } > + > + if (!d->testcase_name) { > + d->testcase_name = (char *)"eventfd"; > + } > + > + d->testcase = pci_testdev_find_testcase(d->testcase_name); > + if (!d->testcase) { > + struct testcase *test; > + error_report("Invalid test case. Currently support: {"); > + FOREACH_TEST_CASE(test) { > + error_report("\t\"%s\", ", test->name); > + } > + error_report("}"); > + exit(1); > + } > } > > static void > @@ -305,6 +371,11 @@ static void qdev_pci_testdev_reset(DeviceState *dev) > pci_testdev_reset(d); > } > > +static Property pci_testdev_properties[] = { > + DEFINE_PROP_STRING("testcase", PCITestDevState, testcase_name), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > static void pci_testdev_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > @@ -319,6 +390,7 @@ static void pci_testdev_class_init(ObjectClass *klass, void *data) > dc->desc = "PCI Test Device"; > set_bit(DEVICE_CATEGORY_MISC, dc->categories); > dc->reset = qdev_pci_testdev_reset; > + dc->props = pci_testdev_properties; > } > > static const TypeInfo pci_testdev_info = { > -- > 2.7.4 > >