All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] pci-testdev: enhance to support new testcases
@ 2016-09-22  6:15 Peter Xu
  2016-09-22 11:18 ` Andrew Jones
  2016-09-22 18:23 ` Michael S. Tsirkin
  0 siblings, 2 replies; 8+ messages in thread
From: Peter Xu @ 2016-09-22  6:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: mst, pbonzini, marcel, peterx

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 <peterx@redhat.com>
---

 This is kind-of a RFC since I am not sure whether this is a good way.
 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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH] pci-testdev: enhance to support new testcases
  2016-09-22  6:15 [Qemu-devel] [PATCH] pci-testdev: enhance to support new testcases Peter Xu
@ 2016-09-22 11:18 ` Andrew Jones
  2016-09-23  3:37   ` Peter Xu
  2016-09-22 18:23 ` Michael S. Tsirkin
  1 sibling, 1 reply; 8+ messages in thread
From: Andrew Jones @ 2016-09-22 11:18 UTC (permalink / raw)
  To: Peter Xu; +Cc: qemu-devel, marcel, pbonzini, mst

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 <peterx@redhat.com>
> ---
> 
>  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
> 
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH] pci-testdev: enhance to support new testcases
  2016-09-22  6:15 [Qemu-devel] [PATCH] pci-testdev: enhance to support new testcases Peter Xu
  2016-09-22 11:18 ` Andrew Jones
@ 2016-09-22 18:23 ` Michael S. Tsirkin
  2016-09-27  6:37   ` Peter Xu
  1 sibling, 1 reply; 8+ messages in thread
From: Michael S. Tsirkin @ 2016-09-22 18:23 UTC (permalink / raw)
  To: Peter Xu; +Cc: qemu-devel, pbonzini, marcel

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.

Instead of a parameter, how about creating a subregion
of the BAR and adding new tests at an offset?

All you need for compatibility is add a 0-filled
entry after existing tests.



> 
> Signed-off-by: Peter Xu <peterx@redhat.com>



> ---
> 
>  This is kind-of a RFC since I am not sure whether this is a good way.
>  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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH] pci-testdev: enhance to support new testcases
  2016-09-22 11:18 ` Andrew Jones
@ 2016-09-23  3:37   ` Peter Xu
  0 siblings, 0 replies; 8+ messages in thread
From: Peter Xu @ 2016-09-23  3:37 UTC (permalink / raw)
  To: Andrew Jones; +Cc: qemu-devel, marcel, pbonzini, mst

On Thu, Sep 22, 2016 at 01:18:24PM +0200, Andrew Jones wrote:
> 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 <peterx@redhat.com>
> > ---
> > 
> >  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

Cool! Will go over. Thanks for the link. :-)

-- peterx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH] pci-testdev: enhance to support new testcases
  2016-09-22 18:23 ` Michael S. Tsirkin
@ 2016-09-27  6:37   ` Peter Xu
  2016-09-27  8:38     ` Peter Xu
  0 siblings, 1 reply; 8+ messages in thread
From: Peter Xu @ 2016-09-27  6:37 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: qemu-devel, pbonzini, marcel

On Thu, Sep 22, 2016 at 09:23:05PM +0300, Michael S. Tsirkin wrote:
> 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.
> 
> Instead of a parameter, how about creating a subregion
> of the BAR and adding new tests at an offset?

Yeah, I can do that as well.

> 
> All you need for compatibility is add a 0-filled
> entry after existing tests.

Could you help explain why we need zero-filled entry? it'll work as
long as tests are using different regions of memory (no overlap),
right?

Thanks,

-- peterx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH] pci-testdev: enhance to support new testcases
  2016-09-27  6:37   ` Peter Xu
@ 2016-09-27  8:38     ` Peter Xu
  2016-09-27 10:13       ` Paolo Bonzini
  0 siblings, 1 reply; 8+ messages in thread
From: Peter Xu @ 2016-09-27  8:38 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: qemu-devel, pbonzini, marcel

On Tue, Sep 27, 2016 at 02:37:44PM +0800, Peter Xu wrote:
> On Thu, Sep 22, 2016 at 09:23:05PM +0300, Michael S. Tsirkin wrote:
> > 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.
> > 
> > Instead of a parameter, how about creating a subregion
> > of the BAR and adding new tests at an offset?
> 
> Yeah, I can do that as well.

Actually what I want to propose is a new "protocol" for pci-testdev.
Currently it only allows a very limited test case, which is ioeventfd
(static device behavior, no parameter can be passed from guest OS,
etc.). IMHO we need something more general, e.g., guest can send cmd
to the testdev, to let it do specific test; along the way, we should
be able to provide parameters from guest OS (maybe we can gather all
kinds of test requirements from the list if people have any idea).

Take my example: IOMMU unit test would want the guest to send DMA/IRQ
request from the device's perspective. In that case, we would like to
"tell" the pci-testdev about where to write the DMA, and what data to
write specifically, or which IRQ to trigger. That's something we
cannot do right now. And I don't want to just add a new test case for
that specifically. I think we can make it more common.

Btw, I just noticed that pci-io and pci-mmio tests are not run by
default by kvm-unit-test. I don't know how many people are using
pci-testdev (guessing there is little since the test is really a
specific one). In that case, I don't know whether it'll be okay that I
cook a patch to refactor the codes without compatibility
considerations. If so, I can provide twin patch for kvm-unit-test as
well.

Thanks,

-- peterx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH] pci-testdev: enhance to support new testcases
  2016-09-27  8:38     ` Peter Xu
@ 2016-09-27 10:13       ` Paolo Bonzini
  2016-09-28  3:04         ` Peter Xu
  0 siblings, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2016-09-27 10:13 UTC (permalink / raw)
  To: Peter Xu; +Cc: Michael S. Tsirkin, qemu-devel, marcel


> Take my example: IOMMU unit test would want the guest to send DMA/IRQ
> request from the device's perspective. In that case, we would like to
> "tell" the pci-testdev about where to write the DMA, and what data to
> write specifically, or which IRQ to trigger. That's something we
> cannot do right now. And I don't want to just add a new test case for
> that specifically. I think we can make it more common.

Do we need to use the pci-testdev?  There's also for example
the edu device, or we could just use virtio-serial with a null
backend.

Paolo

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH] pci-testdev: enhance to support new testcases
  2016-09-27 10:13       ` Paolo Bonzini
@ 2016-09-28  3:04         ` Peter Xu
  0 siblings, 0 replies; 8+ messages in thread
From: Peter Xu @ 2016-09-28  3:04 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Michael S. Tsirkin, qemu-devel, marcel

On Tue, Sep 27, 2016 at 06:13:29AM -0400, Paolo Bonzini wrote:
> 
> > Take my example: IOMMU unit test would want the guest to send DMA/IRQ
> > request from the device's perspective. In that case, we would like to
> > "tell" the pci-testdev about where to write the DMA, and what data to
> > write specifically, or which IRQ to trigger. That's something we
> > cannot do right now. And I don't want to just add a new test case for
> > that specifically. I think we can make it more common.
> 
> Do we need to use the pci-testdev?  There's also for example
> the edu device, or we could just use virtio-serial with a null
> backend.

It's interesting to know that there is such an edu device. So it not
only suits for education, but tailored for IOMMU unit test as well. :)

-- peterx

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-09-28  3:04 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-22  6:15 [Qemu-devel] [PATCH] pci-testdev: enhance to support new testcases Peter Xu
2016-09-22 11:18 ` Andrew Jones
2016-09-23  3:37   ` Peter Xu
2016-09-22 18:23 ` Michael S. Tsirkin
2016-09-27  6:37   ` Peter Xu
2016-09-27  8:38     ` Peter Xu
2016-09-27 10:13       ` Paolo Bonzini
2016-09-28  3:04         ` Peter Xu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.