* [PATCH v2] memory: Directly dispatch alias accesses on origin memory region
@ 2020-08-22 16:14 Philippe Mathieu-Daudé
2020-08-29 15:46 ` Richard Henderson
2020-08-30 10:20 ` Paolo Bonzini
0 siblings, 2 replies; 3+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-08-22 16:14 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Richard Henderson, Philippe Mathieu-Daudé,
Paolo Bonzini
There is an issue when accessing an alias memory region via the
memory_region_dispatch_read() / memory_region_dispatch_write()
calls.
Since commit 2cdfcf272d ("memory: assign MemoryRegionOps to all
regions") we assign &unassigned_mem_ops to all MemoryRegions.
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;
When accessing the alias using memory_region_dispatch_read(),
flow is:
memory_region_dispatch_read()
-> memory_region_access_valid(mr)
-> 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 directly dispatching aliases accesses to its origin region.
Fixes: 2cdfcf272d ("memory: assign MemoryRegionOps to all regions")
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Since v1:
- reword as not a bugfix (Paolo)
- access the aliases recursively (Paolo)
---
softmmu/memory.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/softmmu/memory.c b/softmmu/memory.c
index 70b93104e8c..3627985bf9f 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -1405,6 +1405,10 @@ MemTxResult memory_region_dispatch_read(MemoryRegion *mr,
unsigned size = memop_size(op);
MemTxResult r;
+ while (mr->alias) {
+ addr += mr->alias_offset;
+ mr = mr->alias;
+ }
if (!memory_region_access_valid(mr, addr, size, false, attrs)) {
*pval = unassigned_mem_read(mr, addr, size);
return MEMTX_DECODE_ERROR;
@@ -1449,6 +1453,10 @@ MemTxResult memory_region_dispatch_write(MemoryRegion *mr,
{
unsigned size = memop_size(op);
+ while (mr->alias) {
+ addr += mr->alias_offset;
+ mr = mr->alias;
+ }
if (!memory_region_access_valid(mr, addr, size, true, attrs)) {
unassigned_mem_write(mr, addr, data, size);
return MEMTX_DECODE_ERROR;
--
2.26.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] memory: Directly dispatch alias accesses on origin memory region
2020-08-22 16:14 [PATCH v2] memory: Directly dispatch alias accesses on origin memory region Philippe Mathieu-Daudé
@ 2020-08-29 15:46 ` Richard Henderson
2020-08-30 10:20 ` Paolo Bonzini
1 sibling, 0 replies; 3+ messages in thread
From: Richard Henderson @ 2020-08-29 15:46 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Peter Maydell, Paolo Bonzini, Richard Henderson
On 8/22/20 9:14 AM, Philippe Mathieu-Daudé wrote:
> There is an issue when accessing an alias memory region via the
> memory_region_dispatch_read() / memory_region_dispatch_write()
> calls.
>
> Since commit 2cdfcf272d ("memory: assign MemoryRegionOps to all
> regions") we assign &unassigned_mem_ops to all MemoryRegions.
>
> 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;
>
> When accessing the alias using memory_region_dispatch_read(),
> flow is:
>
> memory_region_dispatch_read()
> -> memory_region_access_valid(mr)
> -> 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 directly dispatching aliases accesses to its origin region.
>
> Fixes: 2cdfcf272d ("memory: assign MemoryRegionOps to all regions")
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Since v1:
> - reword as not a bugfix (Paolo)
> - access the aliases recursively (Paolo)
> ---
> softmmu/memory.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] memory: Directly dispatch alias accesses on origin memory region
2020-08-22 16:14 [PATCH v2] memory: Directly dispatch alias accesses on origin memory region Philippe Mathieu-Daudé
2020-08-29 15:46 ` Richard Henderson
@ 2020-08-30 10:20 ` Paolo Bonzini
1 sibling, 0 replies; 3+ messages in thread
From: Paolo Bonzini @ 2020-08-30 10:20 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel; +Cc: Peter Maydell, Richard Henderson
On 22/08/20 18:14, Philippe Mathieu-Daudé wrote:
> There is an issue when accessing an alias memory region via the
> memory_region_dispatch_read() / memory_region_dispatch_write()
> calls.
>
> Since commit 2cdfcf272d ("memory: assign MemoryRegionOps to all
> regions") we assign &unassigned_mem_ops to all MemoryRegions.
>
> 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;
>
> When accessing the alias using memory_region_dispatch_read(),
> flow is:
>
> memory_region_dispatch_read()
> -> memory_region_access_valid(mr)
> -> 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 directly dispatching aliases accesses to its origin region.
>
> Fixes: 2cdfcf272d ("memory: assign MemoryRegionOps to all regions")
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
It's still not clear from the commit message what is the case where you
get this. I can integrate it in the commit message myself, but can you
explain it and/or link to a bug report?
Paolo
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-08-30 10:22 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-22 16:14 [PATCH v2] memory: Directly dispatch alias accesses on origin memory region Philippe Mathieu-Daudé
2020-08-29 15:46 ` Richard Henderson
2020-08-30 10:20 ` Paolo Bonzini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.