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:13:55 +0100
User-agent: Mozilla Thunderbird

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.

--
Cheers,

David / dhildenb




reply via email to

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