[Qemu-devel] [PATCH 0/8] Add missed read callback for some memory region

[Qemu-devel] [PATCH 0/8] Add missed read callback for some memory region

[Li Qiang]

From: Li Qiang <liq3ea@gmail.com>

This patch set try to add the missed read callback for memory region.
Without this patchset, when the guest reads the IO port/memory, it will
cause an NULL-dereference issue. For example, add 
"-device isa-debug-exit" to command, then read the 0x501 port, it causes a 
SIGSEGV.

The only exception is 'readonly_mem_ops' as its read is directly 
access the underlying host ram as the comments says.

These missed read callback is mostly pointed by Laszlo Ersek.



Li Qiang (8):
  fw_cfg_mem: add read memory region callback
  hw: debugexit: add read callback
  hw: hyperv_testdev: add read callback
  hw: pc-testdev: add read memory region callback
  hw: designware: add read memory region callback
  hw: pvrdma: add read memory region callback
  hw: sun4c: add read memory region callback
  exec: add read callback for notdirty memory region

 exec.c                    |  7 +++++++
 hw/misc/debugexit.c       |  6 ++++++
 hw/misc/hyperv_testdev.c  | 10 ++++++++--
 hw/misc/pc-testdev.c      | 20 ++++++++++++++++----
 hw/nvram/fw_cfg.c         |  6 ++++++
 hw/pci-host/designware.c  |  7 +++++++
 hw/rdma/vmw/pvrdma_main.c |  6 ++++++
 hw/sparc64/sun4u.c        |  6 ++++++
 8 files changed, 62 insertions(+), 6 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH 1/8] fw_cfg_mem: add read memory region callback

[Li Qiang]

From: Li Qiang <liq3ea@gmail.com>

Signed-off-by: Li Qiang <liq3ea@gmail.com>
---
 hw/nvram/fw_cfg.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index d79a568f54..6de7809f1a 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -434,6 +434,11 @@ static bool fw_cfg_data_mem_valid(void *opaque, hwaddr addr,
     return addr == 0;
 }
 
+static uint64_t fw_cfg_ctl_mem_read(void *opaque, hwaddr addr, unsigned size)
+{
+    return 0;
+}
+
 static void fw_cfg_ctl_mem_write(void *opaque, hwaddr addr,
                                  uint64_t value, unsigned size)
 {
@@ -468,6 +473,7 @@ static bool fw_cfg_comb_valid(void *opaque, hwaddr addr,
 }
 
 static const MemoryRegionOps fw_cfg_ctl_mem_ops = {
+    .read = fw_cfg_ctl_mem_read,
     .write = fw_cfg_ctl_mem_write,
     .endianness = DEVICE_BIG_ENDIAN,
     .valid.accepts = fw_cfg_ctl_mem_valid,
-- 
2.17.1

[Qemu-devel] [PATCH 2/8] hw: debugexit: add read callback

[Li Qiang]

From: Li Qiang <liq3ea@gmail.com>

Signed-off-by: Li Qiang <liq3ea@gmail.com>
---
 hw/misc/debugexit.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/misc/debugexit.c b/hw/misc/debugexit.c
index 84fa1a5b9d..bed293247e 100644
--- a/hw/misc/debugexit.c
+++ b/hw/misc/debugexit.c
@@ -23,6 +23,11 @@ typedef struct ISADebugExitState {
     MemoryRegion io;
 } ISADebugExitState;
 
+static uint64_t debug_exit_read(void *opaque, hwaddr addr, unsigned size)
+{
+    return 0;
+}
+
 static void debug_exit_write(void *opaque, hwaddr addr, uint64_t val,
                              unsigned width)
 {
@@ -30,6 +35,7 @@ static void debug_exit_write(void *opaque, hwaddr addr, uint64_t val,
 }
 
 static const MemoryRegionOps debug_exit_ops = {
+    .read = debug_exit_read,
     .write = debug_exit_write,
     .valid.min_access_size = 1,
     .valid.max_access_size = 4,
-- 
2.17.1

[Qemu-devel] [PATCH 3/8] hw: hyperv_testdev: add read callback

[Li Qiang]

From: Li Qiang <liq3ea@gmail.com>

Signed-off-by: Li Qiang <liq3ea@gmail.com>
---
 hw/misc/hyperv_testdev.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/misc/hyperv_testdev.c b/hw/misc/hyperv_testdev.c
index bf6bbfa8cf..7549f470b1 100644
--- a/hw/misc/hyperv_testdev.c
+++ b/hw/misc/hyperv_testdev.c
@@ -105,7 +105,12 @@ static void hv_synic_test_dev_control(HypervTestDev *dev, uint32_t ctl,
     }
 }
 
-static void hv_test_dev_control(void *opaque, hwaddr addr, uint64_t data,
+static uint64_t hv_test_dev_read(void *opaque, hwaddr addr, unsigned size)
+{
+    return 0;
+}
+
+static void hv_test_dev_write(void *opaque, hwaddr addr, uint64_t data,
                                 uint32_t len)
 {
     HypervTestDev *dev = HYPERV_TEST_DEV(opaque);
@@ -127,7 +132,8 @@ static void hv_test_dev_control(void *opaque, hwaddr addr, uint64_t data,
 }
 
 static const MemoryRegionOps synic_test_sint_ops = {
-    .write = hv_test_dev_control,
+    .read = hv_test_dev_read,
+    .write = hv_test_dev_write,
     .valid.min_access_size = 4,
     .valid.max_access_size = 4,
     .endianness = DEVICE_LITTLE_ENDIAN,
-- 
2.17.1

[Qemu-devel] [PATCH 4/8] hw: pc-testdev: add read memory region callback

[Li Qiang]

From: Li Qiang <liq3ea@gmail.com>

Also change the write callback name.

Signed-off-by: Li Qiang <liq3ea@gmail.com>
---
 hw/misc/pc-testdev.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/hw/misc/pc-testdev.c b/hw/misc/pc-testdev.c
index b81d820084..697eb88c97 100644
--- a/hw/misc/pc-testdev.c
+++ b/hw/misc/pc-testdev.c
@@ -58,7 +58,12 @@ typedef struct PCTestdev {
 #define TESTDEV(obj) \
      OBJECT_CHECK(PCTestdev, (obj), TYPE_TESTDEV)
 
-static void test_irq_line(void *opaque, hwaddr addr, uint64_t data,
+static uint64_t test_irq_line_read(void *opaque, hwaddr addr, unsigned size)
+{
+    return 0;
+}
+
+static void test_irq_line_write(void *opaque, hwaddr addr, uint64_t data,
                           unsigned len)
 {
     PCTestdev *dev = opaque;
@@ -68,7 +73,8 @@ static void test_irq_line(void *opaque, hwaddr addr, uint64_t data,
 }
 
 static const MemoryRegionOps test_irq_ops = {
-    .write = test_irq_line,
+    .read = test_irq_line_read,
+    .write = test_irq_line_write,
     .valid.min_access_size = 1,
     .valid.max_access_size = 1,
     .endianness = DEVICE_LITTLE_ENDIAN,
@@ -110,7 +116,12 @@ static const MemoryRegionOps test_ioport_byte_ops = {
     .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
-static void test_flush_page(void *opaque, hwaddr addr, uint64_t data,
+static uint64_t test_flush_page_read(void *opaque, hwaddr addr, unsigned size)
+{
+    return 0;
+}
+
+static void test_flush_page_write(void *opaque, hwaddr addr, uint64_t data,
                             unsigned len)
 {
     hwaddr page = 4096;
@@ -126,7 +137,8 @@ static void test_flush_page(void *opaque, hwaddr addr, uint64_t data,
 }
 
 static const MemoryRegionOps test_flush_ops = {
-    .write = test_flush_page,
+    .read = test_flush_page_read,
+    .write = test_flush_page_write,
     .valid.min_access_size = 4,
     .valid.max_access_size = 4,
     .endianness = DEVICE_LITTLE_ENDIAN,
-- 
2.17.1

[Qemu-devel] [PATCH 5/8] hw: designware: add read memory region callback

[Li Qiang]

From: Li Qiang <liq3ea@gmail.com>

Signed-off-by: Li Qiang <liq3ea@gmail.com>
---
 hw/pci-host/designware.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
index 29ea313798..f5641b5c8c 100644
--- a/hw/pci-host/designware.c
+++ b/hw/pci-host/designware.c
@@ -57,6 +57,12 @@ designware_pcie_root_to_host(DesignwarePCIERoot *root)
     return DESIGNWARE_PCIE_HOST(bus->parent);
 }
 
+static uint64_t designware_pcie_root_msi_read(void *opaque, hwaddr addr,
+                                              unsigned size)
+{
+    return 0;
+}
+
 static void designware_pcie_root_msi_write(void *opaque, hwaddr addr,
                                            uint64_t val, unsigned len)
 {
@@ -71,6 +77,7 @@ static void designware_pcie_root_msi_write(void *opaque, hwaddr addr,
 }
 
 static const MemoryRegionOps designware_pci_host_msi_ops = {
+    .read = designware_pcie_root_msi_read,
     .write = designware_pcie_root_msi_write,
     .endianness = DEVICE_LITTLE_ENDIAN,
     .valid = {
-- 
2.17.1

[Qemu-devel] [PATCH 6/8] hw: pvrdma: add read memory region callback

[Li Qiang]

From: Li Qiang <liq3ea@gmail.com>

Signed-off-by: Li Qiang <liq3ea@gmail.com>
---
 hw/rdma/vmw/pvrdma_main.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
index ca5fa8d981..a6211d416d 100644
--- a/hw/rdma/vmw/pvrdma_main.c
+++ b/hw/rdma/vmw/pvrdma_main.c
@@ -455,6 +455,11 @@ static const MemoryRegionOps regs_ops = {
     },
 };
 
+static uint64_t uar_read(void *opaque, hwaddr addr, unsigned size)
+{
+    return 0;
+}
+
 static void uar_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
 {
     PVRDMADev *dev = opaque;
@@ -496,6 +501,7 @@ static void uar_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
 }
 
 static const MemoryRegionOps uar_ops = {
+    .read = uar_read,
     .write = uar_write,
     .endianness = DEVICE_LITTLE_ENDIAN,
     .impl = {
-- 
2.17.1

[Qemu-devel] [PATCH 7/8] hw: sun4c: add read memory region callback

[Li Qiang]

From: Li Qiang <liq3ea@gmail.com>

Signed-off-by: Li Qiang <liq3ea@gmail.com>
---
 hw/sparc64/sun4u.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index d16843b30e..74c55a82f4 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -212,6 +212,11 @@ typedef struct PowerDevice {
     MemoryRegion power_mmio;
 } PowerDevice;
 
+static uint64_t power_mem_read(void *opaque, hwaddr addr, unsigned size)
+{
+    return 0;
+}
+
 /* Power */
 static void power_mem_write(void *opaque, hwaddr addr,
                             uint64_t val, unsigned size)
@@ -223,6 +228,7 @@ static void power_mem_write(void *opaque, hwaddr addr,
 }
 
 static const MemoryRegionOps power_mem_ops = {
+    .read = power_mem_read,
     .write = power_mem_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
     .valid = {
-- 
2.17.1

[Qemu-devel] [PATCH 8/8] exec: add read callback for notdirty memory region

[Li Qiang]

From: Li Qiang <liq3ea@gmail.com>

Signed-off-by: Li Qiang <liq3ea@gmail.com>
---
 exec.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/exec.c b/exec.c
index 6826c8337d..3cd5ad2cae 100644
--- a/exec.c
+++ b/exec.c
@@ -2681,6 +2681,11 @@ void memory_notdirty_write_complete(NotDirtyInfo *ndi)
     }
 }
 
+static uint64_t notdirty_mem_read(void *opaque, hwaddr addr, unsigned size)
+{
+    return 0;
+}
+
 /* Called within RCU critical section.  */
 static void notdirty_mem_write(void *opaque, hwaddr ram_addr,
                                uint64_t val, unsigned size)
@@ -2702,6 +2707,7 @@ static bool notdirty_mem_accepts(void *opaque, hwaddr addr,
 }
 
 static const MemoryRegionOps notdirty_mem_ops = {
+    .read = notdirty_mem_read,
     .write = notdirty_mem_write,
     .valid.accepts = notdirty_mem_accepts,
     .endianness = DEVICE_NATIVE_ENDIAN,
@@ -2965,6 +2971,7 @@ static uint16_t dummy_section(PhysPageMap *map, FlatView *fv, MemoryRegion *mr)
     return phys_section_add(map, &section);
 }
 
+
 static void readonly_mem_write(void *opaque, hwaddr addr,
                                uint64_t val, unsigned size)
 {
-- 
2.17.1

Re: [Qemu-devel] [PATCH 0/8] Add missed read callback for some memory region

[Philippe Mathieu-Daudé]

Hi Li,

On 9/12/18 6:01 PM, Li Qiang wrote:
> From: Li Qiang <liq3ea@gmail.com>
> 
> This patch set try to add the missed read callback for memory region.
> Without this patchset, when the guest reads the IO port/memory, it will
> cause an NULL-dereference issue. For example, add 
> "-device isa-debug-exit" to command, then read the 0x501 port, it causes a 
> SIGSEGV.
> 
> The only exception is 'readonly_mem_ops' as its read is directly 
> access the underlying host ram as the comments says.
> 
> These missed read callback is mostly pointed by Laszlo Ersek.
> 
> 
> 
> Li Qiang (8):
>   fw_cfg_mem: add read memory region callback
>   hw: debugexit: add read callback
>   hw: hyperv_testdev: add read callback
>   hw: pc-testdev: add read memory region callback
>   hw: designware: add read memory region callback
>   hw: pvrdma: add read memory region callback
>   hw: sun4c: add read memory region callback
>   exec: add read callback for notdirty memory region

Why not rather simply add a check in
memory_region_oldmmio_read_accessor() instead?

Eventually:

{
    uint64_t tmp;
    int idx = ctz32(size);

    if (unlikely(mr->ops->old_mmio.write[idx]
                 && !mr->ops->old_mmio.read[idx])) {
        tmp = 0; /* XXX is 0 the expected value??? */
    } else {
        tmp = mr->ops->old_mmio.read[idx](mr->opaque, addr);
    }
    ...

Re: [Qemu-devel] [PATCH 1/8] fw_cfg_mem: add read memory region callback

[Eric Blake]

On 9/12/18 11:01 AM, Li Qiang wrote:
> From: Li Qiang <liq3ea@gmail.com>
> 
> Signed-off-by: Li Qiang <liq3ea@gmail.com>

This commit message doesn't state why. The cover letter does, but that 
doesn't get checked into git. A year from now, if someone lands on this 
commit during a bisect, it would help if the commit message told them 
why you are adding a no-op callback here, instead of trying to fix the 
caller to treat a NULL callback as acceptable.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 7/8] hw: sun4c: add read memory region callback

[Artyom Tarasenko]

Please correct the typo in the subject. I was scared that someone dug
out the sun4c zombie emulation.

In the particular case of sun4u, I think the proper way is not to have
the read callback, but throw an 'Unassigned memory access' exception

Regards,
Artyom
On Wed, Sep 12, 2018 at 6:09 PM Li Qiang <liq3ea@163.com> wrote:
>
> From: Li Qiang <liq3ea@gmail.com>
>
> Signed-off-by: Li Qiang <liq3ea@gmail.com>
> ---
>  hw/sparc64/sun4u.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index d16843b30e..74c55a82f4 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -212,6 +212,11 @@ typedef struct PowerDevice {
>      MemoryRegion power_mmio;
>  } PowerDevice;
>
> +static uint64_t power_mem_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    return 0;
> +}
> +
>  /* Power */
>  static void power_mem_write(void *opaque, hwaddr addr,
>                              uint64_t val, unsigned size)
> @@ -223,6 +228,7 @@ static void power_mem_write(void *opaque, hwaddr addr,
>  }
>
>  static const MemoryRegionOps power_mem_ops = {
> +    .read = power_mem_read,
>      .write = power_mem_write,
>      .endianness = DEVICE_NATIVE_ENDIAN,
>      .valid = {
> --
> 2.17.1
>
>
>


-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PATCH 0/8] Add missed read callback for some memory region

[Li Qiang]

Philippe Mathieu-Daudé <f4bug@amsat.org> 于2018年9月13日周四 上午1:12写道：

> Hi Li,
>
> On 9/12/18 6:01 PM, Li Qiang wrote:
> > From: Li Qiang <liq3ea@gmail.com>
> >
> > This patch set try to add the missed read callback for memory region.
> > Without this patchset, when the guest reads the IO port/memory, it will
> > cause an NULL-dereference issue. For example, add
> > "-device isa-debug-exit" to command, then read the 0x501 port, it causes
> a
> > SIGSEGV.
> >
> > The only exception is 'readonly_mem_ops' as its read is directly
> > access the underlying host ram as the comments says.
> >
> > These missed read callback is mostly pointed by Laszlo Ersek.
> >
> >
> >
> > Li Qiang (8):
> >   fw_cfg_mem: add read memory region callback
> >   hw: debugexit: add read callback
> >   hw: hyperv_testdev: add read callback
> >   hw: pc-testdev: add read memory region callback
> >   hw: designware: add read memory region callback
> >   hw: pvrdma: add read memory region callback
> >   hw: sun4c: add read memory region callback
> >   exec: add read callback for notdirty memory region
>
> Why not rather simply add a check in
> memory_region_oldmmio_read_accessor() instead?
>
> Eventually:
>
> {
>     uint64_t tmp;
>     int idx = ctz32(size);
>
>     if (unlikely(mr->ops->old_mmio.write[idx]
>                  && !mr->ops->old_mmio.read[idx])) {
>         tmp = 0; /* XXX is 0 the expected value??? */
>     } else {
>         tmp = mr->ops->old_mmio.read[idx](mr->opaque, addr);
>     }
>     ...
>

Hi, I have sent this patch. But...


We have discussed in another thread:
-->https://lists.gnu.org/archive/html/qemu-devel/2018-09/msg01391.html

Thanks,
Li Qiang

Re: [Qemu-devel] [PATCH 8/8] exec: add read callback for notdirty memory region

[Paolo Bonzini]

On 12/09/2018 18:01, Li Qiang wrote:
> From: Li Qiang <liq3ea@gmail.com>
> 
> Signed-off-by: Li Qiang <liq3ea@gmail.com>

This cannot happen, since TLB_NOTDIRTY is only added to the addr_write
member (see accel/tcg/cputlb.c).

Paolo

> ---
>  exec.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/exec.c b/exec.c
> index 6826c8337d..3cd5ad2cae 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2681,6 +2681,11 @@ void memory_notdirty_write_complete(NotDirtyInfo *ndi)
>      }
>  }
>  
> +static uint64_t notdirty_mem_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    return 0;
> +}
> +
>  /* Called within RCU critical section.  */
>  static void notdirty_mem_write(void *opaque, hwaddr ram_addr,
>                                 uint64_t val, unsigned size)
> @@ -2702,6 +2707,7 @@ static bool notdirty_mem_accepts(void *opaque, hwaddr addr,
>  }
>  
>  static const MemoryRegionOps notdirty_mem_ops = {
> +    .read = notdirty_mem_read,
>      .write = notdirty_mem_write,
>      .valid.accepts = notdirty_mem_accepts,
>      .endianness = DEVICE_NATIVE_ENDIAN,
> @@ -2965,6 +2971,7 @@ static uint16_t dummy_section(PhysPageMap *map, FlatView *fv, MemoryRegion *mr)
>      return phys_section_add(map, &section);
>  }
>  
> +
>  static void readonly_mem_write(void *opaque, hwaddr addr,
>                                 uint64_t val, unsigned size)
>  {
> 

Re: [Qemu-devel] [PATCH 5/8] hw: designware: add read memory region callback

[Paolo Bonzini]

On 12/09/2018 18:01, Li Qiang wrote:
> From: Li Qiang <liq3ea@gmail.com>
> 
> Signed-off-by: Li Qiang <liq3ea@gmail.com>
> ---
>  hw/pci-host/designware.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
> index 29ea313798..f5641b5c8c 100644
> --- a/hw/pci-host/designware.c
> +++ b/hw/pci-host/designware.c
> @@ -57,6 +57,12 @@ designware_pcie_root_to_host(DesignwarePCIERoot *root)
>      return DESIGNWARE_PCIE_HOST(bus->parent);
>  }
>  
> +static uint64_t designware_pcie_root_msi_read(void *opaque, hwaddr addr,
> +                                              unsigned size)
> +{
> +    return 0;
> +}
> +
>  static void designware_pcie_root_msi_write(void *opaque, hwaddr addr,
>                                             uint64_t val, unsigned len)
>  {
> @@ -71,6 +77,7 @@ static void designware_pcie_root_msi_write(void *opaque, hwaddr addr,
>  }
>  
>  static const MemoryRegionOps designware_pci_host_msi_ops = {
> +    .read = designware_pcie_root_msi_read,
>      .write = designware_pcie_root_msi_write,
>      .endianness = DEVICE_LITTLE_ENDIAN,
>      .valid = {
> 

This probably needs to generate an unassigned access too, because the
datasheet says that the device basically traps memory writes.

Generating an unassigned access is probably a good idea for the memory
core; devices can then override the behavior to return zero.  I'm
queuing patches 1-4, with slightly expanded commit messages.

Paolo

Re: [Qemu-devel] [PATCH 5/8] hw: designware: add read memory region callback

[Li Qiang]

Paolo Bonzini <pbonzini@redhat.com> 于2018年9月13日周四 下午11:12写道：

> On 12/09/2018 18:01, Li Qiang wrote:
> > From: Li Qiang <liq3ea@gmail.com>
> >
> > Signed-off-by: Li Qiang <liq3ea@gmail.com>
> > ---
> >  hw/pci-host/designware.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
> > index 29ea313798..f5641b5c8c 100644
> > --- a/hw/pci-host/designware.c
> > +++ b/hw/pci-host/designware.c
> > @@ -57,6 +57,12 @@ designware_pcie_root_to_host(DesignwarePCIERoot *root)
> >      return DESIGNWARE_PCIE_HOST(bus->parent);
> >  }
> >
> > +static uint64_t designware_pcie_root_msi_read(void *opaque, hwaddr addr,
> > +                                              unsigned size)
> > +{
> > +    return 0;
> > +}
> > +
> >  static void designware_pcie_root_msi_write(void *opaque, hwaddr addr,
> >                                             uint64_t val, unsigned len)
> >  {
> > @@ -71,6 +77,7 @@ static void designware_pcie_root_msi_write(void
> *opaque, hwaddr addr,
> >  }
> >
> >  static const MemoryRegionOps designware_pci_host_msi_ops = {
> > +    .read = designware_pcie_root_msi_read,
> >      .write = designware_pcie_root_msi_write,
> >      .endianness = DEVICE_LITTLE_ENDIAN,
> >      .valid = {
> >
>
> This probably needs to generate an unassigned access too, because the
> datasheet says that the device basically traps memory writes.
>
> Generating an unassigned access is probably a good idea for the memory
> core; devices can then override the behavior to return zero.  I'm
> queuing patches 1-4, with slightly expanded commit messages.
>
>
Hi Paolo,
Thanks for your review!

These patches has been got a deep discuss, I'm not sure these patches is
necessory.
The discussion can be found in this thread.
-->https://lists.gnu.org/archive/html/qemu-devel/2018-09/msg01391.html


Thanks,
Li Qiang


> Paolo
>