From: Jonathan Cameron via <qemu-devel@nongnu.org>
To: "Paolo Bonzini" <pbonzini@redhat.com>,
"Peter Xu" <peterx@redhat.com>,
"David Hildenbrand" <david@redhat.com>,
"Philippe Mathieu-Daudé" <philmd@linaro.org>,
qemu-devel@nongnu.org
Cc: <linuxarm@huawei.com>
Subject: [PATCH v2 4/4] physmem: Fix wrong address in large address_space_read/write_cached_slow()
Date: Thu, 7 Mar 2024 15:37:10 +0000 [thread overview]
Message-ID: <20240307153710.30907-5-Jonathan.Cameron@huawei.com> (raw)
In-Reply-To: <20240307153710.30907-1-Jonathan.Cameron@huawei.com>
If the access is bigger than the MemoryRegion supports,
flatview_read/write_continue() will attempt to update the Memory Region.
but the address passed to flatview_translate() is relative to the cache, not
to the FlatView.
On arm/virt with interleaved CXL memory emulation and virtio-blk-pci this
lead to the first part of descriptor being read from the CXL memory and the
second part from PA 0x8 which happens to be a blank region
of a flash chip and all ffs on this particular configuration.
Note this test requires the out of tree ARM support for CXL, but
the problem is more general.
Avoid this by adding new address_space_read_continue_cached()
and address_space_write_continue_cached() which share all the logic
with the flatview versions except for the MemoryRegion lookup which
is unnecessary as the MemoryRegionCache only covers one MemoryRegion.
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
v2: Review from Peter Xu
- Drop additional lookups of the MemoryRegion via
address_space_translate_cached() as it will always return the same
answer.
- Drop various parameters that are then unused.
- rename addr1 to mr_addr.
- Drop a fuzz_dma_read_cb(). Could put this back but it means
carrying the address into the inner call and the only in tree
fuzzer checks if it is normal RAM and if not does nothing anyway.
We don't hit this path for normal RAM.
---
system/physmem.c | 63 +++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 57 insertions(+), 6 deletions(-)
diff --git a/system/physmem.c b/system/physmem.c
index 1264eab24b..701bea27dd 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -3381,6 +3381,59 @@ static inline MemoryRegion *address_space_translate_cached(
return section.mr;
}
+/* Called within RCU critical section. */
+static MemTxResult address_space_write_continue_cached(MemTxAttrs attrs,
+ const void *ptr,
+ hwaddr len,
+ hwaddr mr_addr,
+ hwaddr l,
+ MemoryRegion *mr)
+{
+ MemTxResult result = MEMTX_OK;
+ const uint8_t *buf = ptr;
+
+ for (;;) {
+ result |= flatview_write_continue_step(attrs, buf, len, mr_addr, &l,
+ mr);
+
+ len -= l;
+ buf += l;
+ mr_addr += l;
+
+ if (!len) {
+ break;
+ }
+
+ l = len;
+ }
+
+ return result;
+}
+
+/* Called within RCU critical section. */
+static MemTxResult address_space_read_continue_cached(MemTxAttrs attrs,
+ void *ptr, hwaddr len,
+ hwaddr mr_addr, hwaddr l,
+ MemoryRegion *mr)
+{
+ MemTxResult result = MEMTX_OK;
+ uint8_t *buf = ptr;
+
+ for (;;) {
+ result |= flatview_read_continue_step(attrs, buf, len, mr_addr, &l, mr);
+ len -= l;
+ buf += l;
+ mr_addr += l;
+
+ if (!len) {
+ break;
+ }
+ l = len;
+ }
+
+ return result;
+}
+
/* Called from RCU critical section. address_space_read_cached uses this
* out of line function when the target is an MMIO or IOMMU region.
*/
@@ -3394,9 +3447,8 @@ address_space_read_cached_slow(MemoryRegionCache *cache, hwaddr addr,
l = len;
mr = address_space_translate_cached(cache, addr, &mr_addr, &l, false,
MEMTXATTRS_UNSPECIFIED);
- return flatview_read_continue(cache->fv,
- addr, MEMTXATTRS_UNSPECIFIED, buf, len,
- mr_addr, l, mr);
+ return address_space_read_continue_cached(MEMTXATTRS_UNSPECIFIED,
+ buf, len, mr_addr, l, mr);
}
/* Called from RCU critical section. address_space_write_cached uses this
@@ -3412,9 +3464,8 @@ address_space_write_cached_slow(MemoryRegionCache *cache, hwaddr addr,
l = len;
mr = address_space_translate_cached(cache, addr, &mr_addr, &l, true,
MEMTXATTRS_UNSPECIFIED);
- return flatview_write_continue(cache->fv,
- addr, MEMTXATTRS_UNSPECIFIED, buf, len,
- mr_addr, l, mr);
+ return address_space_write_continue_cached(MEMTXATTRS_UNSPECIFIED,
+ buf, len, mr_addr, l, mr);
}
#define ARG1_DECL MemoryRegionCache *cache
--
2.39.2
next prev parent reply other threads:[~2024-03-07 15:39 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-07 15:37 [PATCH v2 0/4] physmem: Fix MemoryRegion for second access to cached MMIO Address Space Jonathan Cameron via
2024-03-07 15:37 ` [PATCH v2 1/4] physmem: Rename addr1 to more informative mr_addr in flatview_read/write() and similar Jonathan Cameron via
2024-03-07 16:23 ` David Hildenbrand
2024-03-07 15:37 ` [PATCH v2 2/4] physmem: Reduce local variable scope in flatview_read/write_continue() Jonathan Cameron via
2024-03-07 16:23 ` David Hildenbrand
2024-03-07 16:50 ` Philippe Mathieu-Daudé
2024-03-07 15:37 ` [PATCH v2 3/4] physmem: Factor out body of flatview_read/write_continue() loop Jonathan Cameron via
2024-03-07 16:29 ` David Hildenbrand
2024-03-07 15:37 ` Jonathan Cameron via [this message]
2024-03-08 8:59 ` [PATCH v2 4/4] physmem: Fix wrong address in large address_space_read/write_cached_slow() David Hildenbrand
2024-03-08 7:36 ` [PATCH v2 0/4] physmem: Fix MemoryRegion for second access to cached MMIO Address Space Peter Xu
2024-03-08 8:59 ` David Hildenbrand
2024-03-08 16:16 ` Philippe Mathieu-Daudé
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=20240307153710.30907-5-Jonathan.Cameron@huawei.com \
--to=qemu-devel@nongnu.org \
--cc=Jonathan.Cameron@huawei.com \
--cc=david@redhat.com \
--cc=linuxarm@huawei.com \
--cc=pbonzini@redhat.com \
--cc=peterx@redhat.com \
--cc=philmd@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 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.