qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: peterx@redhat.com
To: qemu-devel@nongnu.org, Peter Maydell <peter.maydell@linaro.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	peterx@redhat.com, Fabiano Rosas <farosas@suse.de>,
	David Hildenbrand <david@redhat.com>,
	Prasad Pandit <ppandit@redhat.com>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>
Subject: [PULL 14/34] physmem: Fix wrong address in large address_space_read/write_cached_slow()
Date: Mon, 11 Mar 2024 17:59:05 -0400	[thread overview]
Message-ID: <20240311215925.40618-15-peterx@redhat.com> (raw)
In-Reply-To: <20240311215925.40618-1-peterx@redhat.com>

From: Jonathan Cameron <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>
Link: https://lore.kernel.org/r/20240307153710.30907-5-Jonathan.Cameron@huawei.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 system/physmem.c | 63 +++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 57 insertions(+), 6 deletions(-)

diff --git a/system/physmem.c b/system/physmem.c
index 737869a3f5..6cfb7a80ab 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -3370,6 +3370,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.
  */
@@ -3383,9 +3436,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
@@ -3401,9 +3453,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.44.0



  parent reply	other threads:[~2024-03-11 22:00 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-11 21:58 [PULL 00/34] Migration 20240311 patches peterx
2024-03-11 21:58 ` [PULL 01/34] migration: Don't serialize devices in qemu_savevm_state_iterate() peterx
2024-03-11 21:58 ` [PULL 02/34] vfio/migration: Refactor vfio_save_state() return value peterx
2024-03-11 21:58 ` [PULL 03/34] vfio/migration: Add a note about migration rate limiting peterx
2024-03-11 21:58 ` [PULL 04/34] migration/ram: add additional check peterx
2024-03-11 21:58 ` [PULL 05/34] migration: Report error when shutdown fails peterx
2024-03-11 21:58 ` [PULL 06/34] migration: Remove SaveStateHandler and LoadStateHandler typedefs peterx
2024-03-11 21:58 ` [PULL 07/34] migration: Add documentation for SaveVMHandlers peterx
2024-03-11 21:58 ` [PULL 08/34] migration: Do not call PRECOPY_NOTIFY_SETUP notifiers in case of error peterx
2024-03-11 21:59 ` [PULL 09/34] migration/multifd: Don't fsync when closing QIOChannelFile peterx
2024-03-11 21:59 ` [PULL 10/34] migration/rdma: Fix a memory issue for migration peterx
2024-03-11 21:59 ` [PULL 11/34] physmem: Rename addr1 to more informative mr_addr in flatview_read/write() and similar peterx
2024-03-11 21:59 ` [PULL 12/34] physmem: Reduce local variable scope in flatview_read/write_continue() peterx
2024-03-11 21:59 ` [PULL 13/34] physmem: Factor out body of flatview_read/write_continue() loop peterx
2024-03-11 21:59 ` peterx [this message]
2024-03-11 21:59 ` [PULL 15/34] migration: Fix format in error message peterx
2024-03-11 21:59 ` [PULL 16/34] migration: export fewer options peterx
2024-03-11 21:59 ` [PULL 17/34] migration: remove migration.h references peterx
2024-03-11 21:59 ` [PULL 18/34] migration: export migration_is_setup_or_active peterx
2024-03-11 21:59 ` [PULL 19/34] migration: export migration_is_active peterx
2024-03-11 21:59 ` [PULL 20/34] migration: export migration_is_running peterx
2024-03-11 21:59 ` [PULL 21/34] migration: export vcpu_dirty_limit_period peterx
2024-03-11 21:59 ` [PULL 22/34] migration: migration_thread_is_self peterx
2024-03-11 21:59 ` [PULL 23/34] migration: migration_is_device peterx
2024-03-11 21:59 ` [PULL 24/34] migration: migration_file_set_error peterx
2024-03-11 21:59 ` [PULL 25/34] migration: privatize colo interfaces peterx
2024-03-11 21:59 ` [PULL 26/34] migration: delete unused accessors peterx
2024-03-11 21:59 ` [PULL 27/34] migration: purge MigrationState from public interface peterx
2024-03-11 21:59 ` [PULL 28/34] migration/multifd: Allow zero pages in file migration peterx
2024-03-11 21:59 ` [PULL 29/34] migration/multifd: Allow clearing of the file_bmap from multifd peterx
2024-03-11 21:59 ` [PULL 30/34] migration/multifd: Add new migration option zero-page-detection peterx
2024-03-11 21:59 ` [PULL 31/34] migration/multifd: Implement zero page transmission on the multifd thread peterx
2024-03-11 21:59 ` [PULL 32/34] migration/multifd: Implement ram_save_target_page_multifd to handle multifd version of MigrationOps::ram_save_target_page peterx
2024-03-11 21:59 ` [PULL 33/34] migration/multifd: Enable multifd zero page checking by default peterx
2024-03-11 21:59 ` [PULL 34/34] migration/multifd: Add new migration test cases for legacy zero page checking peterx
2024-03-12 13:07 ` [PULL 00/34] Migration 20240311 patches 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=20240311215925.40618-15-peterx@redhat.com \
    --to=peterx@redhat.com \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=david@redhat.com \
    --cc=farosas@suse.de \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=ppandit@redhat.com \
    --cc=qemu-devel@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).