Re: [Qemu-devel] [PATCH] hw: Add test device for unittests execution
Il 04/10/2012 10:02, Peter Maydell ha scritto: > On 4 October 2012 04:49, Lucas Meneghel Rodrigues wrote: >> Add a test device which supports the kvmctl ioports, >> so one can run the KVM unittest suite [1]. >> >> Usage: >> >> qemu -device testdev >> >> 1) Removed port 0xf1, since now kvm-unit-tests use >>serial >> >> 2) Removed exit code port 0xf4, since that can be >>replaced by >> >> -device isa-debugexit,iobase=0xf4,access-size=2 >> >> 3) Removed ram size port 0xd1, since guest memory >>size can be retrieved from firmware, there's a >>patch for kvm-unit-tests including an API to >>retrieve that value. >> >> [1] Preliminary versions of this patch were posted >> to the mailing list about a year ago, I re-read the >> comments of the thread, and had guidance from >> Paolo about which ports to remove from the test >> device. > > General remark: there's no documentation anywhere in > this patch. I don't necessarily mean user-facing docs, > but at least a descriptive comment saying what the > heck this device is and what it does would be helpful. > > >> >> CC: Paolo Bonzini >> Signed-off-by: Gerd Hoffmann >> Signed-off-by: Avi Kivity >> Signed-off-by: Marcelo Tosatti >> Signed-off-by: Lucas Meneghel Rodrigues >> --- >> hw/i386/Makefile.objs | 1 + >> hw/testdev.c | 131 >> ++ >> 2 files changed, 132 insertions(+) >> create mode 100644 hw/testdev.c >> >> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs >> index 8c764bb..64d2787 100644 >> --- a/hw/i386/Makefile.objs >> +++ b/hw/i386/Makefile.objs >> @@ -11,5 +11,6 @@ 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 += testdev.o > > ...the device is useful even in non-KVM configs, then? > >> obj-y := $(addprefix ../,$(obj-y)) >> diff --git a/hw/testdev.c b/hw/testdev.c >> new file mode 100644 >> index 000..44070f2 >> --- /dev/null >> +++ b/hw/testdev.c >> @@ -0,0 +1,131 @@ >> +#include > > This file needs a leading comment with the usual copyright/license/ > description of what the file does. > >> +#include "hw.h" >> +#include "qdev.h" >> +#include "isa.h" >> + >> +struct testdev { >> +ISADevice dev; >> +MemoryRegion iomem; >> +CharDriverState *chr; >> +}; >> + >> +#define TYPE_TESTDEV "testdev" >> +#define TESTDEV(obj) \ >> + OBJECT_CHECK(struct testdev, (obj), TYPE_TESTDEV) >> + >> +static void test_device_irq_line(void *opaque, uint32_t addr, uint32_t data) >> +{ >> +struct testdev *dev = opaque; >> + >> +qemu_set_irq(isa_get_irq(&dev->dev, 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 = 4096; >> +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_iomem_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_LITTLE_ENDIAN, >> +}; >> + >> +static int init_test_device(ISADevice *isa) >> +{ >> +struct testdev *dev = DO_UPCAST(struct testdev, dev, isa); >> + >> +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);
Re: [Qemu-devel] [PATCH] hw: Add test device for unittests execution
On 4 October 2012 04:49, Lucas Meneghel Rodrigues wrote: > Add a test device which supports the kvmctl ioports, > so one can run the KVM unittest suite [1]. > > Usage: > > qemu -device testdev > > 1) Removed port 0xf1, since now kvm-unit-tests use >serial > > 2) Removed exit code port 0xf4, since that can be >replaced by > > -device isa-debugexit,iobase=0xf4,access-size=2 > > 3) Removed ram size port 0xd1, since guest memory >size can be retrieved from firmware, there's a >patch for kvm-unit-tests including an API to >retrieve that value. > > [1] Preliminary versions of this patch were posted > to the mailing list about a year ago, I re-read the > comments of the thread, and had guidance from > Paolo about which ports to remove from the test > device. General remark: there's no documentation anywhere in this patch. I don't necessarily mean user-facing docs, but at least a descriptive comment saying what the heck this device is and what it does would be helpful. > > CC: Paolo Bonzini > Signed-off-by: Gerd Hoffmann > Signed-off-by: Avi Kivity > Signed-off-by: Marcelo Tosatti > Signed-off-by: Lucas Meneghel Rodrigues > --- > hw/i386/Makefile.objs | 1 + > hw/testdev.c | 131 > ++ > 2 files changed, 132 insertions(+) > create mode 100644 hw/testdev.c > > diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs > index 8c764bb..64d2787 100644 > --- a/hw/i386/Makefile.objs > +++ b/hw/i386/Makefile.objs > @@ -11,5 +11,6 @@ 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 += testdev.o ...the device is useful even in non-KVM configs, then? > obj-y := $(addprefix ../,$(obj-y)) > diff --git a/hw/testdev.c b/hw/testdev.c > new file mode 100644 > index 000..44070f2 > --- /dev/null > +++ b/hw/testdev.c > @@ -0,0 +1,131 @@ > +#include This file needs a leading comment with the usual copyright/license/ description of what the file does. > +#include "hw.h" > +#include "qdev.h" > +#include "isa.h" > + > +struct testdev { > +ISADevice dev; > +MemoryRegion iomem; > +CharDriverState *chr; > +}; > + > +#define TYPE_TESTDEV "testdev" > +#define TESTDEV(obj) \ > + OBJECT_CHECK(struct testdev, (obj), TYPE_TESTDEV) > + > +static void test_device_irq_line(void *opaque, uint32_t addr, uint32_t data) > +{ > +struct testdev *dev = opaque; > + > +qemu_set_irq(isa_get_irq(&dev->dev, 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 = 4096; > +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_iomem_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_LITTLE_ENDIAN, > +}; > + > +static int init_test_device(ISADevice *isa) > +{ > +struct testdev *dev = DO_UPCAST(struct testdev, dev, isa); > + > +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); > +regi
Re: [Qemu-devel] [PATCH] hw: Add test device for unittests execution v2
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 > Signed-off-by: Avi Kivity > Signed-off-by: Marcelo Tosatti > Signed-off-by: Lucas Meneghel Rodrigues > --- > 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 000..0b6dd96 > --- /dev/null > +++ b/hw/pc-testdev.c > @@ -0,0 +1,121 @@ > +#include > +#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 nee