From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:58937) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qy2Lt-0000Fe-39 for qemu-devel@nongnu.org; Mon, 29 Aug 2011 09:58:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Qy2Lr-0007Oo-Kk for qemu-devel@nongnu.org; Mon, 29 Aug 2011 09:58:13 -0400 Received: from mx1.redhat.com ([209.132.183.28]:21026) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qy2Lr-0007OX-CB for qemu-devel@nongnu.org; Mon, 29 Aug 2011 09:58:11 -0400 Message-ID: <4E5B9AED.40805@redhat.com> Date: Mon, 29 Aug 2011 10:58:05 -0300 From: Lucas Meneghel Rodrigues MIME-Version: 1.0 References: <1314389053-30841-1-git-send-email-lmr@redhat.com> <4E580E8E.2060200@codemonkey.ws> In-Reply-To: <4E580E8E.2060200@codemonkey.ws> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] hw: Add test device for unittests execution List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Avi Kivity , Marcelo Tosatti , qemu-devel@nongnu.org, Gerd Hoffmann On 08/26/2011 06:22 PM, Anthony Liguori wrote: > On 08/26/2011 03:04 PM, Lucas Meneghel Rodrigues wrote: >> Add a test device which supports the kvmctl ioports, >> for running the KVM test suite. This is a straight >> port from the latest version of the test device present >> on qemu-kvm, using the APIs currently in use by qemu. >> >> With this we aim for daily execution of >> the KVM unittests to capture any problems with the >> KVM interface. > > I know this has come up before so apologies if this is redundant. > >> >> Usage: >> >> qemu >> -chardev file,path=/log/file/some/where,id=testlog >> -device testdev,chardev=testlog >> >> 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/testdev.c | 140 >> +++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 141 insertions(+), 0 deletions(-) >> create mode 100644 hw/testdev.c >> >> diff --git a/Makefile.target b/Makefile.target >> index e280bf6..e095dd5 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 += testdev.o >> >> # shared objects >> obj-ppc-y = ppc.o >> diff --git a/hw/testdev.c b/hw/testdev.c >> new file mode 100644 >> index 0000000..e38c20e >> --- /dev/null >> +++ b/hw/testdev.c >> @@ -0,0 +1,140 @@ >> +#include >> +#include "hw.h" >> +#include "qdev.h" >> +#include "isa.h" >> + >> +struct testdev { >> + ISADevice dev; >> + CharDriverState *chr; >> +}; >> + >> +static void test_device_serial_write(void *opaque, uint32_t addr, >> uint32_t data) >> +{ >> + struct testdev *dev = opaque; >> + uint8_t buf[1] = { data }; >> + >> + if (dev->chr) { >> + qemu_chr_fe_write(dev->chr, buf, 1); >> + } >> +} > > I think I posted patches at some point for kvm unittests to use a > standard UART. Was there any reason not to do use a UART? I will look for your patches and adapt them to the current conditions. >> +static void test_device_exit(void *opaque, uint32_t addr, uint32_t data) >> +{ >> + exit(data); >> +} > > Port 501 can do this. > >> + >> +static uint32_t test_device_memsize_read(void *opaque, uint32_t addr) >> +{ >> + return ram_size; >> +} > > This can be read through fw_cfg, any reason to do PIO for this? He already explained this, but this was an adaptation of an independent userspace program to run the unittests. I will convert to use fw_cfg then. >> +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; >> +} > > Would be nicer to do this via an opaque. Ok. >> +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); > > This hard codes page size (get it via sysconf). I think mprotect > probably isn't available on windows either. Sure, good point, will look into doing it with getpagesize >> +} >> + >> +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 CPUReadMemoryFunc * const test_iomem_read[3] = { >> + test_iomem_readb, >> + test_iomem_readw, >> + test_iomem_readl, >> +}; >> + >> +static CPUWriteMemoryFunc * const test_iomem_write[3] = { >> + test_iomem_writeb, >> + test_iomem_writew, >> + test_iomem_writel, >> +}; >> + >> +static int init_test_device(ISADevice *isa) >> +{ >> + struct testdev *dev = DO_UPCAST(struct testdev, dev, isa); >> + int iomem; >> + >> + register_ioport_write(0xf1, 1, 1, test_device_serial_write, dev); >> + register_ioport_write(0xf4, 1, 4, test_device_exit, dev); >> + register_ioport_read(0xd1, 1, 4, test_device_memsize_read, 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); >> + iomem = cpu_register_io_memory(test_iomem_read, test_iomem_write, NULL, >> + DEVICE_NATIVE_ENDIAN); >> + cpu_register_physical_memory(0xff000000, 0x10000, iomem); >> + 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(), >> + }, >> +}; > > Should this use MemoryRegion? Yep, will look into converting it. > Regards, Thanks!