Re: [PATCH] hw/vfio: let readonly flag take effect for mmaped regions

[Philippe Mathieu-Daudé]

Hi Yan,

On 3/27/20 12:19 PM, yan.y.z...@intel.com wrote:
From: Yan Zhao <yan.y.z...@intel.com>
currently, vfio regions without VFIO_REGION_INFO_FLAG_WRITE are only
read-only when VFIO_REGION_INFO_FLAG_MMAP is not set.

regions with flag VFIO_REGION_INFO_FLAG_READ | VFIO_REGION_INFO_FLAG_MMAP
are only read-only in host page table for qemu.

This patch sets corresponding ept page entries read-only for regions
with flag VFIO_REGION_INFO_FLAG_READ | VFIO_REGION_INFO_FLAG_MMAP.

accordingly, it ignores guest write when guest writes to the read-only
regions are trapped.

Signed-off-by: Yan Zhao <yan.y.z...@intel.com>
Signed-off-by: Xin Zeng <xin.z...@intel.com>
---
  hw/vfio/common.c | 4 ++++
  memory.c         | 3 +++
  2 files changed, 7 insertions(+)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 0b3593b3c0..e901621ca0 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -971,6 +971,10 @@ int vfio_region_mmap(VFIORegion *region)
                                            name, region->mmaps[i].size,
                                            region->mmaps[i].mmap);
          g_free(name);
+
+        if (!(region->flags & VFIO_REGION_INFO_FLAG_WRITE)) {
+            memory_region_set_readonly(&region->mmaps[i].mem, true);
+        }
          memory_region_add_subregion(region->mem, region->mmaps[i].offset,
                                      &region->mmaps[i].mem);
  
diff --git a/memory.c b/memory.c
index 601b749906..4b1071dc74 100644
--- a/memory.c
+++ b/memory.c
@@ -1313,6 +1313,9 @@ static void memory_region_ram_device_write(void *opaque, 
hwaddr addr,
      MemoryRegion *mr = opaque;
  
      trace_memory_region_ram_device_write(get_cpu_index(), mr, addr, data, size);
+    if (mr->readonly) {
+        return;
+    }

Shouldn't this be in memory_region_dispatch_write()?

Please split this patch in 2, this (generic) hunk as first patch, then 
the VFIO more specific change.

  
      switch (size) {
      case 1: