qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v1 1/4] physmem: disallow direct access to RAM DEVICE in addr


From: David Hildenbrand
Subject: Re: [PATCH v1 1/4] physmem: disallow direct access to RAM DEVICE in address_space_write_rom()
Date: Wed, 22 Jan 2025 11:18:29 +0100
User-agent: Mozilla Thunderbird

On 22.01.25 11:17, Philippe Mathieu-Daudé wrote:
On 22/1/25 11:13, David Hildenbrand wrote:
On 22.01.25 11:10, David Hildenbrand wrote:
On 22.01.25 11:07, Philippe Mathieu-Daudé wrote:
Hi David,

On 20/1/25 12:14, David Hildenbrand wrote:
As documented in commit 4a2e242bbb306 ("memory: Don't use memcpy for
ram_device regions"), we disallow direct access to RAM DEVICE regions.

Let's factor out the "supports direct access" check from
memory_access_is_direct() so we can reuse it, and make it a bit
easier to
read.

This change implies that address_space_write_rom() and
cpu_memory_rw_debug() won't be able to write to RAM DEVICE regions. It
will also affect cpu_flush_icache_range(), but it's only used by
hw/core/loader.c after writing to ROM, so it is expected to not apply
here with RAM DEVICE.

This fixes direct access to these regions where we don't want direct
access. We'll extend cpu_memory_rw_debug() next to also be able to
write to
these (and IO) regions.

This is a preparation for further changes.

Cc: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
     include/exec/memory.h | 30 ++++++++++++++++++++++++------
     system/physmem.c      |  3 +--
     2 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 3ee1901b52..bd0ddb9cdf 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -2985,15 +2985,33 @@ MemTxResult
address_space_write_cached_slow(MemoryRegionCache *cache,
     int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr);
     bool prepare_mmio_access(MemoryRegion *mr);
+static inline bool
memory_region_supports_direct_access(MemoryRegion *mr)
+{
+    /* ROM DEVICE regions only allow direct access if in ROMD mode. */
+    if (memory_region_is_romd(mr)) {
+        return true;
+    }
+    if (!memory_region_is_ram(mr)) {
+        return false;
+    }
+    /*
+     * RAM DEVICE regions can be accessed directly using memcpy,
but it might
+     * be MMIO and access using mempy can be wrong (e.g., using
instructions not
+     * intended for MMIO access). So we treat this as IO.
+     */
+    return !memory_region_is_ram_device(mr);
+
+}
+
     static inline bool memory_access_is_direct(MemoryRegion *mr,
bool is_write)
     {
-    if (is_write) {
-        return memory_region_is_ram(mr) && !mr->readonly &&
-               !mr->rom_device && !memory_region_is_ram_device(mr);
-    } else {
-        return (memory_region_is_ram(mr) && !
memory_region_is_ram_device(mr)) ||

This patch is doing multiple things at once, and I'm having hard time
reviewing it.

I appreciate the review, but ... really?! :)

25 insertions(+), 8 deletions(-)

FWIW, I'll try to split it up ... I thought the comments added to
memory_region_supports_direct_access() and friends are pretty clear.

No worry, I'll give it another try. (split still welcomed, but not
blocking).

I think unmangling the existing unreadable conditions in memory_access_is_direct() can be done separately; let me see what I can do.

--
Cheers,

David / dhildenb




reply via email to

[Prev in Thread] Current Thread [Next in Thread]