[Qemu-devel] [PATCH v5 06/11] fw_cfg_mem: introduce the "data_width" property

[Laszlo Ersek]

The "data_width" property is capable of changing the maximum valid access
size to the MMIO data register, and resizes the memory region similarly,
at device realization time.

The default value of "data_memwidth" is set so that we don't yet diverge
from "fw_cfg_data_mem_ops".

Most of the fw_cfg_mem users will stick with the default, and for them we
should continue using the statically allocated "fw_cfg_data_mem_ops". This
is beneficial for debugging because gdb can resolve pointers referencing
static objects to the names of those objects.

Signed-off-by: Laszlo Ersek <address@hidden>
---

Notes:
    v5:
    - the wide data register is restricted to the MMIO mapping now; no
      interference with the I/O port mapping [Laszlo]
    - the wide data ops aren't allocated separately with g_memdup() any
      longer [Alex, Peter]
    - the wide data register handles splitting / combining itself [Paolo,
      Peter]
    
    v4:
    - reject I/O port combining if data register is wider than 1 byte
      [Peter]
    
    v3:
    - new in v3 [Drew Jones]

 hw/nvram/fw_cfg.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 68 insertions(+), 5 deletions(-)

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 5ba49dd..80f846f 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -77,8 +77,10 @@ struct FWCfgMemState {
     /*< public >*/
 
     MemoryRegion ctl_iomem, data_iomem;
     hwaddr ctl_addr, data_addr;
+    uint32_t data_width;
+    MemoryRegionOps wide_data_ops;
 };
 
 #define JPG_FILE 0
 #define BMP_FILE 1
@@ -284,15 +286,60 @@ static uint8_t fw_cfg_read(FWCfgState *s)
 
 static uint64_t fw_cfg_data_mem_read(void *opaque, hwaddr addr,
                                      unsigned size)
 {
-    return fw_cfg_read(opaque);
+    FWCfgState *s = opaque;
+    uint8_t buf[8];
+    unsigned i;
+
+    for (i = 0; i < size; ++i) {
+        buf[i] = fw_cfg_read(s);
+    }
+    switch (size) {
+    case 1:
+        return buf[0];
+    case 2:
+        return lduw_he_p(buf);
+    case 4:
+        return (uint32_t)ldl_he_p(buf);
+    case 8:
+        return ldq_he_p(buf);
+    }
+    abort();
 }
 
 static void fw_cfg_data_mem_write(void *opaque, hwaddr addr,
                                   uint64_t value, unsigned size)
 {
-    fw_cfg_write(opaque, (uint8_t)value);
+    FWCfgState *s = opaque;
+    uint8_t buf[8];
+    unsigned i;
+
+    switch (size) {
+    case 1:
+        buf[0] = value;
+        break;
+    case 2:
+        stw_he_p(buf, value);
+        break;
+    case 4:
+        stl_he_p(buf, value);
+        break;
+    case 8:
+        stq_he_p(buf, value);
+        break;
+    default:
+        abort();
+    }
+    for (i = 0; i < size; ++i) {
+        fw_cfg_write(s, buf[i]);
+    }
+}
+
+static bool fw_cfg_data_mem_valid(void *opaque, hwaddr addr,
+                                  unsigned size, bool is_write)
+{
+    return addr == 0;
 }
 
 static void fw_cfg_ctl_mem_write(void *opaque, hwaddr addr,
                                  uint64_t value, unsigned size)
@@ -343,8 +390,9 @@ static const MemoryRegionOps fw_cfg_data_mem_ops = {
     .endianness = DEVICE_BIG_ENDIAN,
     .valid = {
         .min_access_size = 1,
         .max_access_size = 1,
+        .accepts = fw_cfg_data_mem_valid,
     },
 };
 
 static const MemoryRegionOps fw_cfg_comb_mem_ops = {
@@ -626,8 +674,10 @@ FWCfgState *fw_cfg_init_mem(hwaddr ctl_addr, hwaddr 
data_addr)
 
     dev = qdev_create(NULL, TYPE_FW_CFG_MEM);
     qdev_prop_set_uint64(dev, "ctl_addr", ctl_addr);
     qdev_prop_set_uint64(dev, "data_addr", data_addr);
+    qdev_prop_set_uint32(dev, "data_width",
+                         fw_cfg_data_mem_ops.valid.max_access_size);
 
     return fw_cfg_init1(dev);
 }
 
@@ -686,24 +736,37 @@ static const TypeInfo fw_cfg_io_info = {
 
 static Property fw_cfg_mem_properties[] = {
     DEFINE_PROP_UINT64("ctl_addr", FWCfgMemState, ctl_addr, -1),
     DEFINE_PROP_UINT64("data_addr", FWCfgMemState, data_addr, -1),
+    DEFINE_PROP_UINT32("data_width", FWCfgMemState, data_width, -1),
     DEFINE_PROP_END_OF_LIST(),
 };
 
 static void fw_cfg_mem_realize(DeviceState *dev, Error **errp)
 {
     FWCfgMemState *s = FW_CFG_MEM(dev);
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+    const MemoryRegionOps *data_ops = &fw_cfg_data_mem_ops;
 
     memory_region_init_io(&s->ctl_iomem, OBJECT(s), &fw_cfg_ctl_mem_ops,
                           FW_CFG(s), "fwcfg.ctl", FW_CFG_SIZE);
     sysbus_init_mmio(sbd, &s->ctl_iomem);
     sysbus_mmio_map(sbd, 0, s->ctl_addr);
 
-    memory_region_init_io(&s->data_iomem, OBJECT(s), &fw_cfg_data_mem_ops,
-                          FW_CFG(s), "fwcfg.data",
-                          fw_cfg_data_mem_ops.valid.max_access_size);
+    if (s->data_width > data_ops->valid.max_access_size) {
+        /* memberwise copy because the "old_mmio" member is const */
+        s->wide_data_ops.read       = data_ops->read;
+        s->wide_data_ops.write      = data_ops->write;
+        s->wide_data_ops.endianness = data_ops->endianness;
+        s->wide_data_ops.valid      = data_ops->valid;
+        s->wide_data_ops.impl       = data_ops->impl;
+
+        s->wide_data_ops.valid.max_access_size = s->data_width;
+        s->wide_data_ops.impl.max_access_size  = s->data_width;
+        data_ops = &s->wide_data_ops;
+    }
+    memory_region_init_io(&s->data_iomem, OBJECT(s), data_ops, FW_CFG(s),
+                          "fwcfg.data", data_ops->valid.max_access_size);
     sysbus_init_mmio(sbd, &s->data_iomem);
     sysbus_mmio_map(sbd, 1, s->data_addr);
 }
 
-- 
1.8.3.1