qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Philippe Mathieu-Daudé via" <qemu-devel@nongnu.org>
To: qemu-devel@nongnu.org
Cc: "Peter Xu" <peterx@redhat.com>,
	"David Hildenbrand" <david@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	qemu-block@nongnu.org, "Philippe Mathieu-Daudé" <f4bug@amsat.org>
Subject: [PULL 01/19] memory: Directly dispatch alias accesses on origin memory region
Date: Tue, 18 Jan 2022 13:02:11 +0100	[thread overview]
Message-ID: <20220118120229.196337-2-f4bug@amsat.org> (raw)
In-Reply-To: <20220118120229.196337-1-f4bug@amsat.org>

Since commit 2cdfcf272d ("memory: assign MemoryRegionOps to all
regions"), all newly created regions are assigned with
unassigned_mem_ops (which might be then overwritten).

When using aliased container regions, and there is no region mapped
at address 0 in the container, the memory_region_dispatch_read()
and memory_region_dispatch_write() calls incorrectly return the
container unassigned_mem_ops, because the alias offset is not used.

Consider the following setup:

    +--------------------+ < - - - - - - - - - - - +
    |     Container      |  mr
    |  (unassigned_mem)  |                         |
    |                    |
    |                    |                         |
    |                    |  alias_offset
    +                    + <- - - - - - +----------+---------+
    | +----------------+ |              |                    |
    | |  MemoryRegion0 | |              |                    |
    | +----------------+ |              |       Alias        |  addr1
    | |  MemoryRegion1 | | <~ ~  ~  ~ ~ |                    | <~~~~~~
    | +----------------+ |              |                    |
    |                    |              +--------------------+
    |                    |
    |                    |
    |                    |
    |                    |
    | +----------------+ |
    | |  MemoryRegionX | |
    | +----------------+ |
    | |  MemoryRegionY | |
    | +----------------+ |
    | |  MemoryRegionZ | |
    | +----------------+ |
    +--------------------+

The memory_region_init_alias() flow is:

  memory_region_init_alias()
  -> memory_region_init()
     -> object_initialize(TYPE_MEMORY_REGION)
        -> memory_region_initfn()
           -> mr->ops = &unassigned_mem_ops;

Later when accessing offset=addr1 via the alias, we expect to hit
MemoryRegion1. The memory_region_dispatch_read() flow is:

  memory_region_dispatch_read(addr1)
  -> memory_region_access_valid(mr)   <- addr1 offset is ignored
     -> mr->ops->valid.accepts()
        -> unassigned_mem_accepts()
        <- false
     <- false
   <- MEMTX_DECODE_ERROR

The caller gets a MEMTX_DECODE_ERROR while the access is OK.

Fix by dispatching aliases recursively, accessing its origin region
after adding the alias offset.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Peter Xu <peterx@redhat.com>
Message-Id: <20210418055708.820980-1-f4bug@amsat.org>
---
 softmmu/memory.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/softmmu/memory.c b/softmmu/memory.c
index 7340e19ff5e..0c463e0fe53 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -1444,6 +1444,11 @@ MemTxResult memory_region_dispatch_read(MemoryRegion *mr,
     unsigned size = memop_size(op);
     MemTxResult r;
 
+    if (mr->alias) {
+        return memory_region_dispatch_read(mr->alias,
+                                           mr->alias_offset + addr,
+                                           pval, op, attrs);
+    }
     if (!memory_region_access_valid(mr, addr, size, false, attrs)) {
         *pval = unassigned_mem_read(mr, addr, size);
         return MEMTX_DECODE_ERROR;
@@ -1488,6 +1493,11 @@ MemTxResult memory_region_dispatch_write(MemoryRegion *mr,
 {
     unsigned size = memop_size(op);
 
+    if (mr->alias) {
+        return memory_region_dispatch_write(mr->alias,
+                                            mr->alias_offset + addr,
+                                            data, op, attrs);
+    }
     if (!memory_region_access_valid(mr, addr, size, true, attrs)) {
         unassigned_mem_write(mr, addr, data, size);
         return MEMTX_DECODE_ERROR;
-- 
2.34.1



  reply	other threads:[~2022-01-18 12:14 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-18 12:02 [PULL 00/19] Memory API patches for 2022-01-18 Philippe Mathieu-Daudé via
2022-01-18 12:02 ` Philippe Mathieu-Daudé via [this message]
2022-01-18 12:02 ` [PULL 02/19] memory: Split mtree_info() as mtree_info_flatview() + mtree_info_as() Philippe Mathieu-Daudé via
2022-01-18 12:02 ` [PULL 03/19] memory: Have 'info mtree' remove duplicated Address Space information Philippe Mathieu-Daudé via
2022-01-18 12:02 ` [PULL 04/19] machine: Use host_memory_backend_is_mapped() in machine_consume_memdev() Philippe Mathieu-Daudé via
2022-01-18 12:02 ` [PULL 05/19] memory: Make memory_region_is_mapped() succeed when mapped via an alias Philippe Mathieu-Daudé via
2022-01-18 12:02 ` [PULL 06/19] memory: Update description of memory_region_is_mapped() Philippe Mathieu-Daudé via
2022-01-18 12:02 ` [PULL 07/19] memory: Fix incorrect calls of log_global_start/stop Philippe Mathieu-Daudé via
2022-01-18 12:02 ` [PULL 08/19] stubs: Restrict fw_cfg to system emulation Philippe Mathieu-Daudé via
2022-01-18 12:02 ` [PULL 09/19] hw/nvram: Restrict fw_cfg QOM interface to sysemu and tools Philippe Mathieu-Daudé via
2022-01-18 12:02 ` [PULL 10/19] hw/pci: Restrict pci-bus stub to sysemu Philippe Mathieu-Daudé via
2022-01-18 12:02 ` [PULL 11/19] hw/pci: Document pci_dma_map() Philippe Mathieu-Daudé via
2022-01-18 12:02 ` [PULL 12/19] hw/dma: Remove CONFIG_USER_ONLY check Philippe Mathieu-Daudé via
2022-01-18 12:02 ` [PULL 13/19] hw/rdma/rdma_utils: Rename rdma_pci_dma_map 'len' argument Philippe Mathieu-Daudé via
2022-01-18 12:02 ` [PULL 14/19] hw/scsi: Rename SCSIRequest::resid as 'residual' Philippe Mathieu-Daudé via
2022-01-18 12:02 ` [PULL 15/19] hw/dma: Fix format string issues using dma_addr_t Philippe Mathieu-Daudé via
2022-01-18 12:02 ` [PULL 16/19] hw/dma: Move ScatterGatherEntry / QEMUSGList declarations around Philippe Mathieu-Daudé via
2022-01-18 12:02 ` [PULL 17/19] hw/dma: Use dma_addr_t type definition when relevant Philippe Mathieu-Daudé via
2022-01-18 12:02 ` [PULL 18/19] hw/dma: Let dma_buf_read() / dma_buf_write() propagate MemTxResult Philippe Mathieu-Daudé via
2022-01-18 12:02 ` [PULL 19/19] docs/devel: add some clarifying text for aliases Philippe Mathieu-Daudé via
2022-01-18 19:43 ` [PULL 00/19] Memory API patches for 2022-01-18 Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220118120229.196337-2-f4bug@amsat.org \
    --to=qemu-devel@nongnu.org \
    --cc=david@redhat.com \
    --cc=f4bug@amsat.org \
    --cc=pbonzini@redhat.com \
    --cc=peterx@redhat.com \
    --cc=qemu-block@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).