diff mbox

hw: Add test device for unittests execution v2

Message ID 1314824769-27740-1-git-send-email-lmr@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lucas Meneghel Rodrigues Aug. 31, 2011, 9:06 p.m. UTC 
Add a test device which supports the kvmctl ioports,
for running the KVM test suite.

Usage:

  qemu
     -chardev file,path=/log/file/some/where,id=testlog
     -device pc-testdev,chardev=testlog

lmr: I've tried to address the comments made by the
reviewers, but I probably left out some things. Please
let me know whether this is on the right track.

Thanks to Marcelo for helping me with questions about
the new memory API.

Changes from v1:
 * Updated description
 * Used the API fw_cfg to retrieve device ram size in k-u-t.git (see
 * separate patch)
 * Use getpagesize() to retrieve page size instead of hard coding it
 * Dropped functions to write to a serial device
 * Updated struct name to be TestDev to comply with CODING_STYLE
 * Now device is called pc-testdev rather than testdev
 * Conversion to MemoryRegion

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
---
 Makefile.target |    1 +
 hw/pc-testdev.c |  121 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 122 insertions(+), 0 deletions(-)
 create mode 100644 hw/pc-testdev.c

Comments

Edgar E. Iglesias Aug. 31, 2011, 9:13 p.m. UTC | #1 
On Wed, Aug 31, 2011 at 06:06:09PM -0300, Lucas Meneghel Rodrigues wrote:
> Add a test device which supports the kvmctl ioports,
> for running the KVM test suite.
> 
> Usage:
> 
>   qemu
>      -chardev file,path=/log/file/some/where,id=testlog
>      -device pc-testdev,chardev=testlog
> 
> lmr: I've tried to address the comments made by the
> reviewers, but I probably left out some things. Please
> let me know whether this is on the right track.
> 
> Thanks to Marcelo for helping me with questions about
> the new memory API.
> 
> Changes from v1:
>  * Updated description
>  * Used the API fw_cfg to retrieve device ram size in k-u-t.git (see
>  * separate patch)
>  * Use getpagesize() to retrieve page size instead of hard coding it
>  * Dropped functions to write to a serial device
>  * Updated struct name to be TestDev to comply with CODING_STYLE
>  * Now device is called pc-testdev rather than testdev
>  * Conversion to MemoryRegion
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Avi Kivity <avi@redhat.com>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
> ---
>  Makefile.target |    1 +
>  hw/pc-testdev.c |  121 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 122 insertions(+), 0 deletions(-)
>  create mode 100644 hw/pc-testdev.c
> 
> diff --git a/Makefile.target b/Makefile.target
> index 07af4d4..450fc18 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -232,6 +232,7 @@ obj-i386-y += debugcon.o multiboot.o
>  obj-i386-y += pc_piix.o
>  obj-i386-$(CONFIG_KVM) += kvmclock.o
>  obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
> +obj-i386-y += pc-testdev.o
>  
>  # shared objects
>  obj-ppc-y = ppc.o
> diff --git a/hw/pc-testdev.c b/hw/pc-testdev.c
> new file mode 100644
> index 0000000..0b6dd96
> --- /dev/null
> +++ b/hw/pc-testdev.c
> @@ -0,0 +1,121 @@
> +#include <sys/mman.h>
> +#include "hw.h"
> +#include "qdev.h"
> +#include "isa.h"
> +#include "fw_cfg.h"
> +#include "exec-memory.h"
> +
> +struct TestDev {
> +    ISADevice dev;
> +    CharDriverState *chr;
> +    MemoryRegion mem;
> +};
> +
> +static void test_device_exit(void *opaque, uint32_t addr, uint32_t data)
> +{
> +    exit(data);
> +}
> +
> +static void test_device_irq_line(void *opaque, uint32_t addr, uint32_t data)
> +{
> +    qemu_set_irq(isa_get_irq(addr - 0x2000), !!data);
> +}
> +
> +static uint32 test_device_ioport_data;
> +
> +static void test_device_ioport_write(void *opaque, uint32_t addr, uint32_t data)
> +{
> +    test_device_ioport_data = data;
> +}
> +
> +static uint32_t test_device_ioport_read(void *opaque, uint32_t addr)
> +{
> +    return test_device_ioport_data;
> +}
> +
> +static void test_device_flush_page(void *opaque, uint32_t addr, uint32_t data)
> +{
> +    target_phys_addr_t len = getpagesize();
> +    void *a = cpu_physical_memory_map(data & ~0xffful, &len, 0);
> +
> +    mprotect(a, 4096, PROT_NONE);
> +    mprotect(a, 4096, PROT_READ|PROT_WRITE);
> +    cpu_physical_memory_unmap(a, len, 0, 0);
> +}
> +
> +static char *iomem_buf;
> +
> +static uint32_t test_iomem_readb(void *opaque, target_phys_addr_t addr)
> +{
> +    return iomem_buf[addr];
> +}
> +
> +static uint32_t test_iomem_readw(void *opaque, target_phys_addr_t addr)
> +{
> +    return *(uint16_t*)(iomem_buf + addr);
> +}
> +
> +static uint32_t test_iomem_readl(void *opaque, target_phys_addr_t addr)
> +{
> +    return *(uint32_t*)(iomem_buf + addr);
> +}
> +
> +static void test_iomem_writeb(void *opaque, target_phys_addr_t addr, uint32_t val)
> +{
> +    iomem_buf[addr] = val;
> +}
> +
> +static void test_iomem_writew(void *opaque, target_phys_addr_t addr, uint32_t val)
> +{
> +    *(uint16_t*)(iomem_buf + addr) = val;
> +}
> +
> +static void test_iomem_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
> +{
> +    *(uint32_t*)(iomem_buf + addr) = val;
> +}
> +
> +static const MemoryRegionOps test_mem_ops = {
> +    .old_mmio = {
> +    .read = { test_iomem_readb, test_iomem_readw, test_iomem_readl },
> +    .write = { test_iomem_writeb, test_iomem_writew, test_iomem_writel },
> +    },
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +
> +static int init_test_device(ISADevice *isa)
> +{
> +    struct TestDev *dev = DO_UPCAST(struct TestDev, dev, isa);
> +
> +    register_ioport_write(0xf4, 1, 4, test_device_exit, dev);
> +    register_ioport_read(0xe0, 1, 1, test_device_ioport_read, dev);
> +    register_ioport_write(0xe0, 1, 1, test_device_ioport_write, dev);
> +    register_ioport_read(0xe0, 1, 2, test_device_ioport_read, dev);
> +    register_ioport_write(0xe0, 1, 2, test_device_ioport_write, dev);
> +    register_ioport_read(0xe0, 1, 4, test_device_ioport_read, dev);
> +    register_ioport_write(0xe0, 1, 4, test_device_ioport_write, dev);
> +    register_ioport_write(0xe4, 1, 4, test_device_flush_page, dev);
> +    register_ioport_write(0x2000, 24, 1, test_device_irq_line, NULL);

Hi,

I think you should address Blue Swirls comment and add indirection
to avoid the need to allocate large number of ports. If the number grows
significantly it will become a problem. Something as simple as an
addr port combined with an access port will do. Then the device can
decode it into the specific operations internally.

And I still don't understand why we cant use MMIO, so that archs without
ioports can use this. Or does that cause other problems?

Cheers
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Kiszka Sept. 1, 2011, 7:22 a.m. UTC | #2 
On 2011-08-31 23:06, Lucas Meneghel Rodrigues wrote:
> Add a test device which supports the kvmctl ioports,
> for running the KVM test suite.
> 
> Usage:
> 
>   qemu
>      -chardev file,path=/log/file/some/where,id=testlog
>      -device pc-testdev,chardev=testlog
> 
> lmr: I've tried to address the comments made by the
> reviewers, but I probably left out some things. Please
> let me know whether this is on the right track.

Quite a few comments were missed (isa_get_irq, static iomem_buf etc.).
Please go through the thread again.

Jan
Paolo Bonzini Sept. 1, 2011, 9:44 a.m. UTC | #3 
On 08/31/2011 11:06 PM, Lucas Meneghel Rodrigues wrote:
> +    CharDriverState *chr;

I think it's unused, no?

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Makefile.target b/Makefile.target
index 07af4d4..450fc18 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -232,6 +232,7 @@  obj-i386-y += debugcon.o multiboot.o
 obj-i386-y += pc_piix.o
 obj-i386-$(CONFIG_KVM) += kvmclock.o
 obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
+obj-i386-y += pc-testdev.o
 
 # shared objects
 obj-ppc-y = ppc.o
diff --git a/hw/pc-testdev.c b/hw/pc-testdev.c
new file mode 100644
index 0000000..0b6dd96
--- /dev/null
+++ b/hw/pc-testdev.c
@@ -0,0 +1,121 @@ 
+#include <sys/mman.h>
+#include "hw.h"
+#include "qdev.h"
+#include "isa.h"
+#include "fw_cfg.h"
+#include "exec-memory.h"
+
+struct TestDev {
+    ISADevice dev;
+    CharDriverState *chr;
+    MemoryRegion mem;
+};
+
+static void test_device_exit(void *opaque, uint32_t addr, uint32_t data)
+{
+    exit(data);
+}
+
+static void test_device_irq_line(void *opaque, uint32_t addr, uint32_t data)
+{
+    qemu_set_irq(isa_get_irq(addr - 0x2000), !!data);
+}
+
+static uint32 test_device_ioport_data;
+
+static void test_device_ioport_write(void *opaque, uint32_t addr, uint32_t data)
+{
+    test_device_ioport_data = data;
+}
+
+static uint32_t test_device_ioport_read(void *opaque, uint32_t addr)
+{
+    return test_device_ioport_data;
+}
+
+static void test_device_flush_page(void *opaque, uint32_t addr, uint32_t data)
+{
+    target_phys_addr_t len = getpagesize();
+    void *a = cpu_physical_memory_map(data & ~0xffful, &len, 0);
+
+    mprotect(a, 4096, PROT_NONE);
+    mprotect(a, 4096, PROT_READ|PROT_WRITE);
+    cpu_physical_memory_unmap(a, len, 0, 0);
+}
+
+static char *iomem_buf;
+
+static uint32_t test_iomem_readb(void *opaque, target_phys_addr_t addr)
+{
+    return iomem_buf[addr];
+}
+
+static uint32_t test_iomem_readw(void *opaque, target_phys_addr_t addr)
+{
+    return *(uint16_t*)(iomem_buf + addr);
+}
+
+static uint32_t test_iomem_readl(void *opaque, target_phys_addr_t addr)
+{
+    return *(uint32_t*)(iomem_buf + addr);
+}
+
+static void test_iomem_writeb(void *opaque, target_phys_addr_t addr, uint32_t val)
+{
+    iomem_buf[addr] = val;
+}
+
+static void test_iomem_writew(void *opaque, target_phys_addr_t addr, uint32_t val)
+{
+    *(uint16_t*)(iomem_buf + addr) = val;
+}
+
+static void test_iomem_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
+{
+    *(uint32_t*)(iomem_buf + addr) = val;
+}
+
+static const MemoryRegionOps test_mem_ops = {
+    .old_mmio = {
+    .read = { test_iomem_readb, test_iomem_readw, test_iomem_readl },
+    .write = { test_iomem_writeb, test_iomem_writew, test_iomem_writel },
+    },
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+
+static int init_test_device(ISADevice *isa)
+{
+    struct TestDev *dev = DO_UPCAST(struct TestDev, dev, isa);
+
+    register_ioport_write(0xf4, 1, 4, test_device_exit, dev);
+    register_ioport_read(0xe0, 1, 1, test_device_ioport_read, dev);
+    register_ioport_write(0xe0, 1, 1, test_device_ioport_write, dev);
+    register_ioport_read(0xe0, 1, 2, test_device_ioport_read, dev);
+    register_ioport_write(0xe0, 1, 2, test_device_ioport_write, dev);
+    register_ioport_read(0xe0, 1, 4, test_device_ioport_read, dev);
+    register_ioport_write(0xe0, 1, 4, test_device_ioport_write, dev);
+    register_ioport_write(0xe4, 1, 4, test_device_flush_page, dev);
+    register_ioport_write(0x2000, 24, 1, test_device_irq_line, NULL);
+    iomem_buf = g_malloc0(0x10000);
+    memory_region_init_io(&dev->mem, &test_mem_ops, NULL, "testdev", 0x10000);
+    memory_region_add_subregion(get_system_memory(), 0xff000000, &dev->mem);
+    return 0;
+}
+
+static ISADeviceInfo testdev_info = {
+    .qdev.name  = "testdev",
+    .qdev.size  = sizeof(struct TestDev),
+    .init       = init_test_device,
+    .qdev.props = (Property[]) {
+        DEFINE_PROP_CHR("chardev", struct TestDev, chr),
+        DEFINE_PROP_END_OF_LIST(),
+    },
+};
+
+static void testdev_register_devices(void)
+{
+    isa_qdev_register(&testdev_info);
+}
+
+device_init(testdev_register_devices)