[Qemu-devel] [PATCHv2 1/3] bwap: add qemu_bswap helper

[Qemu-devel] [PATCHv2 1/3] bwap: add qemu_bswap helper

[Michael S. Tsirkin]

add helper that can swap values of 4, 2, 1 bytes

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 bswap.h |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/bswap.h b/bswap.h
index 4558704..aace9b7 100644
--- a/bswap.h
+++ b/bswap.h
@@ -214,4 +214,10 @@ static inline void cpu_to_be32wu(uint32_t *p, uint32_t v)
 #undef le_bswaps
 #undef be_bswaps
 
+/* len must be one of 1, 2, 4 */
+static inline uint32_t qemu_bswap_len(uint32_t value, int len)
+{
+    return bswap32(value) >> (32 - 8 * len);
+}
+
 #endif /* BSWAP_H */
-- 
1.6.6.144.g5c3af

[Qemu-devel] [PATCHv2 2/3] rwhandler: simplified way to register for mem/io

[Michael S. Tsirkin]

Some users prefer a single callback with length passed as parameter to
using b/w/l callbacks. It would maybe be cleaner to just pass length to
existing callbacks but that's a lot of churn.  So for now add a wrapper.
For convenience use uint64_t for address so a single callback can be
used for ioport/memory.

I did have to resort to preprocessor to reduce code duplication.  It is
however slightly more straightforward, and better contained than what we
had with pci_host_template.h. Again, it would go away if we just passed
len to existing callbacks.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 Makefile.target |    1 +
 rwhandler.c     |   91 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 rwhandler.h     |   27 ++++++++++++++++
 3 files changed, 119 insertions(+), 0 deletions(-)
 create mode 100644 rwhandler.c
 create mode 100644 rwhandler.h

diff --git a/Makefile.target b/Makefile.target
index e661478..f9f4a2e 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -46,6 +46,7 @@ all: $(PROGS)
 # cpu emulator library
 libobj-y = exec.o translate-all.o cpu-exec.o translate.o
 libobj-y += tcg/tcg.o
+libobj-y += rwhandler.o
 libobj-$(CONFIG_SOFTFLOAT) += fpu/softfloat.o
 libobj-$(CONFIG_NOSOFTFLOAT) += fpu/softfloat-native.o
 libobj-y += op_helper.o helper.o
diff --git a/rwhandler.c b/rwhandler.c
new file mode 100644
index 0000000..5e258c1
--- /dev/null
+++ b/rwhandler.c
@@ -0,0 +1,91 @@
+#include "rwhandler.h"
+#include "ioport.h"
+#include "cpu-all.h"
+
+#if !defined(CONFIG_USER_ONLY)
+
+#define RWHANDLER_WRITE(name, len, type) \
+static void name(void *opaque, type addr, uint32_t value) \
+{\
+    struct ReadWriteHandler *handler = opaque;\
+    handler->write(handler, addr, value, len);\
+}
+
+#define RWHANDLER_READ(name, len, type) \
+static uint32_t name(void *opaque, type addr) \
+{ \
+    struct ReadWriteHandler *handler = opaque; \
+    return handler->read(handler, addr, len); \
+}
+
+RWHANDLER_WRITE(cpu_io_memory_simple_writeb, 1, target_phys_addr_t);
+RWHANDLER_READ(cpu_io_memory_simple_readb, 1, target_phys_addr_t);
+RWHANDLER_WRITE(cpu_io_memory_simple_writew, 2, target_phys_addr_t);
+RWHANDLER_READ(cpu_io_memory_simple_readw, 2, target_phys_addr_t);
+RWHANDLER_WRITE(cpu_io_memory_simple_writel, 4, target_phys_addr_t);
+RWHANDLER_READ(cpu_io_memory_simple_readl, 4, target_phys_addr_t);
+
+static CPUWriteMemoryFunc * const cpu_io_memory_simple_write[] = {
+    &cpu_io_memory_simple_writeb,
+    &cpu_io_memory_simple_writew,
+    &cpu_io_memory_simple_writel,
+};
+
+static CPUReadMemoryFunc * const cpu_io_memory_simple_read[] = {
+    &cpu_io_memory_simple_readb,
+    &cpu_io_memory_simple_readw,
+    &cpu_io_memory_simple_readl,
+};
+
+int cpu_register_io_memory_simple(struct ReadWriteHandler *handler)
+{
+    if (!handler->read || !handler->write) {
+        return -1;
+    }
+    return cpu_register_io_memory(cpu_io_memory_simple_read,
+                                  cpu_io_memory_simple_write,
+                                  handler);
+}
+
+RWHANDLER_WRITE(ioport_simple_writeb, 1, uint32_t);
+RWHANDLER_READ(ioport_simple_readb, 1, uint32_t);
+RWHANDLER_WRITE(ioport_simple_writew, 2, uint32_t);
+RWHANDLER_READ(ioport_simple_readw, 2, uint32_t);
+RWHANDLER_WRITE(ioport_simple_writel, 4, uint32_t);
+RWHANDLER_READ(ioport_simple_readl, 4, uint32_t);
+
+int register_ioport_simple(ReadWriteHandler* handler,
+                           pio_addr_t start, int length, int size)
+{
+    IOPortWriteFunc *write;
+    IOPortReadFunc *read;
+    int r;
+    switch (size) {
+    case 1:
+        write = ioport_simple_writeb;
+        read = ioport_simple_readb;
+	break;
+    case 2:
+        write = ioport_simple_writew;
+        read = ioport_simple_readw;
+	break;
+    default:
+        write = ioport_simple_writel;
+        read = ioport_simple_readl;
+    }
+    if (handler->write) {
+        r = register_ioport_write(start, length, size, write, handler);
+        if (r < 0) {
+            return r;
+        }
+    }
+    if (handler->read) {
+        r = register_ioport_read(start, length, size, read, handler);
+        if (r < 0) {
+            return r;
+        }
+    }
+    return 0;
+}
+
+#endif
diff --git a/rwhandler.h b/rwhandler.h
new file mode 100644
index 0000000..aa44b28
--- /dev/null
+++ b/rwhandler.h
@@ -0,0 +1,27 @@
+#ifndef READ_WRITE_HANDLER_H
+#define READ_WRITE_HANDLER_H
+
+#include "qemu-common.h"
+#include "ioport.h"
+
+typedef struct ReadWriteHandler ReadWriteHandler;
+
+/* len is guaranteed to be one of 1, 2 or 4, addr is guaranteed to fit in an
+ * appropriate type (io/memory/etc). They do not need to be range checked. */
+typedef void WriteHandlerFunc(ReadWriteHandler *, uint64_t addr,
+			      uint32_t value, int len);
+typedef uint32_t ReadHandlerFunc(ReadWriteHandler *, uint64_t addr, int len);
+
+struct ReadWriteHandler {
+    WriteHandlerFunc *write;
+    ReadHandlerFunc *read;
+};
+
+/* Helpers for when we want to use a single routine with length. */
+/* CPU memory handler: both read and write must be present. */
+int cpu_register_io_memory_simple(ReadWriteHandler *);
+/* io port handler: can supply only read or write handlers. */
+int register_ioport_simple(ReadWriteHandler *,
+			   pio_addr_t start, int length, int size);
+
+#endif
-- 
1.6.6.144.g5c3af

[Qemu-devel] [PATCHv2 3/3] pci_host: rewrite using rwhandler

[Michael S. Tsirkin]

Save a ton of code by switching pcihost to use rwhandler.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci_host.c          |  172 +++++++++++++++++++----------------------------
 hw/pci_host.h          |    4 +
 hw/pci_host_template.h |  109 ------------------------------
 3 files changed, 74 insertions(+), 211 deletions(-)
 delete mode 100644 hw/pci_host_template.h

diff --git a/hw/pci_host.c b/hw/pci_host.c
index 6289ead..e96f617 100644
--- a/hw/pci_host.c
+++ b/hw/pci_host.c
@@ -79,152 +79,120 @@ uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len)
     return val;
 }
 
-static void pci_host_config_writel(void *opaque, target_phys_addr_t addr,
-                                   uint32_t val)
+static void pci_host_config_write(ReadWriteHandler *handler,
+				  uint64_t addr, uint32_t val, int len)
 {
-    PCIHostState *s = opaque;
+    PCIHostState *s = container_of(handler, PCIHostState, conf_handler);
 
+    PCI_DPRINTF("%s addr %" PRIx64 " %d val %"PRIx32"\n",
+                __func__, addr, len, val);
 #ifdef TARGET_WORDS_BIGENDIAN
-    val = bswap32(val);
+    val = qemu_bswap_len(val, len);
 #endif
-    PCI_DPRINTF("%s addr " TARGET_FMT_plx " val %"PRIx32"\n",
-                __func__, addr, val);
     s->config_reg = val;
 }
 
-static uint32_t pci_host_config_readl(void *opaque, target_phys_addr_t addr)
+static uint32_t pci_host_config_read(ReadWriteHandler *handler,
+                                            uint64_t addr, int len)
 {
-    PCIHostState *s = opaque;
+    PCIHostState *s = container_of(handler, PCIHostState, conf_handler);
     uint32_t val = s->config_reg;
-
 #ifdef TARGET_WORDS_BIGENDIAN
-    val = bswap32(val);
+    val = qemu_bswap_len(val, len);
 #endif
-    PCI_DPRINTF("%s addr " TARGET_FMT_plx " val %"PRIx32"\n",
-                __func__, addr, val);
+    PCI_DPRINTF("%s addr %" PRIx64 " len %d val %"PRIx32"\n",
+                __func__, addr, len, val);
     return val;
 }
 
-static CPUWriteMemoryFunc * const pci_host_config_write[] = {
-    &pci_host_config_writel,
-    &pci_host_config_writel,
-    &pci_host_config_writel,
-};
-
-static CPUReadMemoryFunc * const pci_host_config_read[] = {
-    &pci_host_config_readl,
-    &pci_host_config_readl,
-    &pci_host_config_readl,
-};
-
-int pci_host_conf_register_mmio(PCIHostState *s)
-{
-    return cpu_register_io_memory(pci_host_config_read,
-                                  pci_host_config_write, s);
-}
-
-static void pci_host_config_writel_noswap(void *opaque,
-                                          target_phys_addr_t addr,
-                                          uint32_t val)
+static void pci_host_config_write_noswap(ReadWriteHandler *handler,
+					 uint64_t addr, uint32_t val, int len)
 {
-    PCIHostState *s = opaque;
+    PCIHostState *s = container_of(handler, PCIHostState, conf_noswap_handler);
 
-    PCI_DPRINTF("%s addr " TARGET_FMT_plx " val %"PRIx32"\n",
-                __func__, addr, val);
+    PCI_DPRINTF("%s addr %" PRIx64 " %d val %"PRIx32"\n",
+                __func__, addr, len, val);
     s->config_reg = val;
 }
 
-static uint32_t pci_host_config_readl_noswap(void *opaque,
-                                             target_phys_addr_t addr)
+static uint32_t pci_host_config_read_noswap(ReadWriteHandler *handler,
+                                            uint64_t addr, int len)
 {
-    PCIHostState *s = opaque;
+    PCIHostState *s = container_of(handler, PCIHostState, conf_noswap_handler);
     uint32_t val = s->config_reg;
 
-    PCI_DPRINTF("%s addr " TARGET_FMT_plx " val %"PRIx32"\n",
-                __func__, addr, val);
+    PCI_DPRINTF("%s addr %" PRIx64 " len %d val %"PRIx32"\n",
+                __func__, addr, len, val);
     return val;
 }
 
-static CPUWriteMemoryFunc * const pci_host_config_write_noswap[] = {
-    &pci_host_config_writel_noswap,
-    &pci_host_config_writel_noswap,
-    &pci_host_config_writel_noswap,
-};
-
-static CPUReadMemoryFunc * const pci_host_config_read_noswap[] = {
-    &pci_host_config_readl_noswap,
-    &pci_host_config_readl_noswap,
-    &pci_host_config_readl_noswap,
-};
-
-int pci_host_conf_register_mmio_noswap(PCIHostState *s)
+static void pci_host_data_write(ReadWriteHandler *handler,
+				uint64_t addr, uint32_t val, int len)
 {
-    return cpu_register_io_memory(pci_host_config_read_noswap,
-                                  pci_host_config_write_noswap, s);
+    PCIHostState *s = container_of(handler, PCIHostState, data_handler);
+#ifdef TARGET_WORDS_BIGENDIAN
+    val = qemu_bswap_len(val, len);
+#endif
+    PCI_DPRINTF("write addr %" PRIx64 " len %d val %x\n",
+                addr, len, val);
+    if (s->config_reg & (1u << 31))
+        pci_data_write(s->bus, s->config_reg | (addr & 3), val, len);
 }
 
-static void pci_host_config_writel_ioport(void *opaque,
-                                          uint32_t addr, uint32_t val)
+static uint32_t pci_host_data_read(ReadWriteHandler *handler,
+				   uint64_t addr, int len)
 {
-    PCIHostState *s = opaque;
+    PCIHostState *s = container_of(handler, PCIHostState, data_handler);
+    uint32_t val;
+    if (!(s->config_reg & (1 << 31)))
+        return 0xffffffff;
+    val = pci_data_read(s->bus, s->config_reg | (addr & 3), len);
+    PCI_DPRINTF("read addr %" PRIx64 " len %d val %x\n",
+                addr, len, val);
+#ifdef TARGET_WORDS_BIGENDIAN
+    val = qemu_bswap_len(val, len);
+#endif
+    return val;
+}
 
-    PCI_DPRINTF("%s addr %"PRIx32 " val %"PRIx32"\n", __func__, addr, val);
-    s->config_reg = val;
+static void pci_host_init(PCIHostState *s)
+{
+    s->conf_handler.write = pci_host_config_write;
+    s->conf_handler.read = pci_host_config_read;
+    s->conf_noswap_handler.write = pci_host_config_write_noswap;
+    s->conf_noswap_handler.read = pci_host_config_read_noswap;
+    s->data_handler.write = pci_host_data_write;
+    s->data_handler.read = pci_host_data_read;
 }
 
-static uint32_t pci_host_config_readl_ioport(void *opaque, uint32_t addr)
+int pci_host_conf_register_mmio(PCIHostState *s)
 {
-    PCIHostState *s = opaque;
-    uint32_t val = s->config_reg;
+    pci_host_init(s);
+    return cpu_register_io_memory_simple(&s->conf_handler);
+}
 
-    PCI_DPRINTF("%s addr %"PRIx32" val %"PRIx32"\n", __func__, addr, val);
-    return val;
+int pci_host_conf_register_mmio_noswap(PCIHostState *s)
+{
+    pci_host_init(s);
+    return cpu_register_io_memory_simple(&s->conf_noswap_handler);
 }
 
 void pci_host_conf_register_ioport(pio_addr_t ioport, PCIHostState *s)
 {
-    register_ioport_write(ioport, 4, 4, pci_host_config_writel_ioport, s);
-    register_ioport_read(ioport, 4, 4, pci_host_config_readl_ioport, s);
+    pci_host_init(s);
+    register_ioport_simple(&s->conf_noswap_handler, ioport, 4, 4);
 }
 
-#define PCI_ADDR_T      target_phys_addr_t
-#define PCI_HOST_SUFFIX _mmio
-
-#include "pci_host_template.h"
-
-static CPUWriteMemoryFunc * const pci_host_data_write_mmio[] = {
-    pci_host_data_writeb_mmio,
-    pci_host_data_writew_mmio,
-    pci_host_data_writel_mmio,
-};
-
-static CPUReadMemoryFunc * const pci_host_data_read_mmio[] = {
-    pci_host_data_readb_mmio,
-    pci_host_data_readw_mmio,
-    pci_host_data_readl_mmio,
-};
-
 int pci_host_data_register_mmio(PCIHostState *s)
 {
-    return cpu_register_io_memory(pci_host_data_read_mmio,
-                                  pci_host_data_write_mmio,
-                                  s);
+    pci_host_init(s);
+    return cpu_register_io_memory_simple(&s->data_handler);
 }
 
-#undef PCI_ADDR_T
-#undef PCI_HOST_SUFFIX
-
-#define PCI_ADDR_T      uint32_t
-#define PCI_HOST_SUFFIX _ioport
-
-#include "pci_host_template.h"
-
 void pci_host_data_register_ioport(pio_addr_t ioport, PCIHostState *s)
 {
-    register_ioport_write(ioport, 4, 1, pci_host_data_writeb_ioport, s);
-    register_ioport_write(ioport, 4, 2, pci_host_data_writew_ioport, s);
-    register_ioport_write(ioport, 4, 4, pci_host_data_writel_ioport, s);
-    register_ioport_read(ioport, 4, 1, pci_host_data_readb_ioport, s);
-    register_ioport_read(ioport, 4, 2, pci_host_data_readw_ioport, s);
-    register_ioport_read(ioport, 4, 4, pci_host_data_readl_ioport, s);
+    pci_host_init(s);
+    register_ioport_simple(&s->data_handler, ioport, 4, 1);
+    register_ioport_simple(&s->data_handler, ioport, 4, 2);
+    register_ioport_simple(&s->data_handler, ioport, 4, 4);
 }
diff --git a/hw/pci_host.h b/hw/pci_host.h
index a006687..5ff6443 100644
--- a/hw/pci_host.h
+++ b/hw/pci_host.h
@@ -29,9 +29,13 @@
 #define PCI_HOST_H
 
 #include "sysbus.h"
+#include "rwhandler.h"
 
 struct PCIHostState {
     SysBusDevice busdev;
+    ReadWriteHandler conf_noswap_handler;
+    ReadWriteHandler conf_handler;
+    ReadWriteHandler data_handler;
     uint32_t config_reg;
     PCIBus *bus;
 };
diff --git a/hw/pci_host_template.h b/hw/pci_host_template.h
deleted file mode 100644
index 11e6c88..0000000
--- a/hw/pci_host_template.h
+++ /dev/null
@@ -1,109 +0,0 @@
-/*
- * QEMU Common PCI Host bridge configuration data space access routines.
- *
- * Copyright (c) 2006 Fabrice Bellard
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-
-/* Worker routines for a PCI host controller that uses an {address,data}
-   register pair to access PCI configuration space.  */
-
-static void glue(pci_host_data_writeb, PCI_HOST_SUFFIX)(
-    void* opaque, PCI_ADDR_T addr, uint32_t val)
-{
-    PCIHostState *s = opaque;
-
-    PCI_DPRINTF("writeb addr " TARGET_FMT_plx " val %x\n",
-                (target_phys_addr_t)addr, val);
-    if (s->config_reg & (1u << 31))
-        pci_data_write(s->bus, s->config_reg | (addr & 3), val, 1);
-}
-
-static void glue(pci_host_data_writew, PCI_HOST_SUFFIX)(
-    void* opaque, PCI_ADDR_T addr, uint32_t val)
-{
-    PCIHostState *s = opaque;
-#ifdef TARGET_WORDS_BIGENDIAN
-    val = bswap16(val);
-#endif
-    PCI_DPRINTF("writew addr " TARGET_FMT_plx " val %x\n",
-                (target_phys_addr_t)addr, val);
-    if (s->config_reg & (1u << 31))
-        pci_data_write(s->bus, s->config_reg | (addr & 3), val, 2);
-}
-
-static void glue(pci_host_data_writel, PCI_HOST_SUFFIX)(
-    void* opaque, PCI_ADDR_T addr, uint32_t val)
-{
-    PCIHostState *s = opaque;
-#ifdef TARGET_WORDS_BIGENDIAN
-    val = bswap32(val);
-#endif
-    PCI_DPRINTF("writel addr " TARGET_FMT_plx " val %x\n",
-                (target_phys_addr_t)addr, val);
-    if (s->config_reg & (1u << 31))
-        pci_data_write(s->bus, s->config_reg, val, 4);
-}
-
-static uint32_t glue(pci_host_data_readb, PCI_HOST_SUFFIX)(
-    void* opaque, PCI_ADDR_T addr)
-{
-    PCIHostState *s = opaque;
-    uint32_t val;
-
-    if (!(s->config_reg & (1 << 31)))
-        return 0xff;
-    val = pci_data_read(s->bus, s->config_reg | (addr & 3), 1);
-    PCI_DPRINTF("readb addr " TARGET_FMT_plx " val %x\n",
-                (target_phys_addr_t)addr, val);
-    return val;
-}
-
-static uint32_t glue(pci_host_data_readw, PCI_HOST_SUFFIX)(
-    void* opaque, PCI_ADDR_T addr)
-{
-    PCIHostState *s = opaque;
-    uint32_t val;
-    if (!(s->config_reg & (1 << 31)))
-        return 0xffff;
-    val = pci_data_read(s->bus, s->config_reg | (addr & 3), 2);
-    PCI_DPRINTF("readw addr " TARGET_FMT_plx " val %x\n",
-                (target_phys_addr_t)addr, val);
-#ifdef TARGET_WORDS_BIGENDIAN
-    val = bswap16(val);
-#endif
-    return val;
-}
-
-static uint32_t glue(pci_host_data_readl, PCI_HOST_SUFFIX)(
-    void* opaque, PCI_ADDR_T addr)
-{
-    PCIHostState *s = opaque;
-    uint32_t val;
-    if (!(s->config_reg & (1 << 31)))
-        return 0xffffffff;
-    val = pci_data_read(s->bus, s->config_reg | (addr & 3), 4);
-    PCI_DPRINTF("readl addr " TARGET_FMT_plx " val %x\n",
-                (target_phys_addr_t)addr, val);
-#ifdef TARGET_WORDS_BIGENDIAN
-    val = bswap32(val);
-#endif
-    return val;
-}
-- 
1.6.6.144.g5c3af