From: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
To: qemu-devel@nongnu.org
Cc: "Paolo Bonzini" <pbonzini@redhat.com>,
"Richard Henderson" <richard.henderson@linaro.org>,
"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
"Peter Xu" <peterx@redhat.com>
Subject: [PATCH v4] memory: Directly dispatch alias accesses on origin memory region
Date: Sun, 18 Apr 2021 07:57:08 +0200 [thread overview]
Message-ID: <20210418055708.820980-1-f4bug@amsat.org> (raw)
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>
---
v4:
- added ASCII schema
v3:
- reworded, mentioning the "alias to container" case
- use recursive call instead of while(), because easier when debugging
therefore reset Richard R-b tag.
v2:
- use while()
---
softmmu/memory.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/softmmu/memory.c b/softmmu/memory.c
index d4493ef9e43..b899ca6a6b7 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -1442,6 +1442,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;
@@ -1486,6 +1491,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.26.3
next reply other threads:[~2021-04-18 5:58 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-18 5:57 Philippe Mathieu-Daudé [this message]
2021-04-20 21:08 ` [PATCH v4] memory: Directly dispatch alias accesses on origin memory region Peter Xu
2021-07-07 13:58 ` Philippe Mathieu-Daudé
2021-08-04 21:11 ` [PATCH-for-6.2 " Philippe Mathieu-Daudé
2021-10-14 23:46 ` [PATCH " Philippe Mathieu-Daudé
2022-01-18 9:35 ` Philippe Mathieu-Daudé via
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=20210418055708.820980-1-f4bug@amsat.org \
--to=f4bug@amsat.org \
--cc=pbonzini@redhat.com \
--cc=peterx@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=richard.henderson@linaro.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).