Skip to contentSkip to conversation

xen: mapcache: Fixes

GIT PULL Applied Edgar E. Iglesias 2 participants 4 messages May 6, 2025 – May 10, 2025

lore
Share & tools
lore

Edgar E. Iglesias

author
#0 lore raw

From: "Edgar E. Iglesias" <[email protected]>

The following changes since commit a9e0c9c0f14e19d23443ac24c8080b4708d2eab8:

Merge tag 'pull-9p-20250505' of https://github.com/cschoenebeck/qemu into staging (2025-05-05 11:26:59 -0400)

are available in the Git repository at:

https://gitlab.com/edgar.iglesias/qemu.git tags/edgar/xen-queue-2025-05-06.for-upstream

for you to fetch changes up to 88fb705600a3b612c571efc9f1a6aed923a18dcc:

xen: mapcache: Split mapcache_grants by ro and rw (2025-05-06 18:39:44 +0200)

----------------------------------------------------------------
Edgars Xen queue

----------------------------------------------------------------
Aleksandr Partanen (1):
xen: mapcache: Fix finding matching entry

Edgar E. Iglesias (1):
xen: mapcache: Split mapcache_grants by ro and rw

 hw/xen/xen-mapcache.c | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)
Signature · 1 line
2.43.0
author
#1 lore raw

[PULL v1 1/2] xen: mapcache: Fix finding matching entry

From: Aleksandr Partanen <[email protected]>

If we have request without lock and hit unlocked or invalid entry during the search, we remap it immediately, even if we have matching entry in next entries in bucket. This leads to duplication of mappings of the same size, and to possibility of selecting the wrong element during invalidation and underflow it's entry->lock counter

Signed-off-by Aleksandr Partanen <[email protected]>
Reviewed-by Stefano Stabellini <[email protected]>
Reviewed-by Edgar E. Iglesias <[email protected]>
Signed-off-by Edgar E. Iglesias <[email protected]>
---
 hw/xen/xen-mapcache.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
hw/xen/xen-mapcache.c +3 -3
index 698b5c53ed..2c8f861fdb 100644
@@ -376,12 +376,12 @@ tryagain:
 
     entry = &mc->entry[address_index % mc->nr_buckets];
 
-    while (entry && (lock || entry->lock) && entry->vaddr_base &&
-            (entry->paddr_index != address_index || entry->size != cache_size ||
+    while (entry && (!entry->vaddr_base ||
+            entry->paddr_index != address_index || entry->size != cache_size ||
              !test_bits(address_offset >> XC_PAGE_SHIFT,
                  test_bit_size >> XC_PAGE_SHIFT,
                  entry->valid_mapping))) {
-        if (!free_entry && !entry->lock) {
+        if (!free_entry && (!entry->lock || !entry->vaddr_base)) {
             free_entry = entry;
             free_pentry = pentry;
         }
Signature · 1 line
2.43.0
author
#2 lore raw

[PULL v1 2/2] xen: mapcache: Split mapcache_grants by ro and rw

In reply to Edgar E. Iglesias

From: "Edgar E. Iglesias" <[email protected]>

Today, we don't track write-abiliy in the cache, if a user requests a readable mapping followed by a writeable mapping on the same page, the second lookup will incorrectly hit the readable entry.

Split mapcache_grants by ro and rw access. Grants will now have separate ways in the cache depending on writeability.

Reviewed-by Philippe Mathieu-Daudé <[email protected]>
Reviewed-by Stefano Stabellini <[email protected]>
Signed-off-by Edgar E. Iglesias <[email protected]>
---
 hw/xen/xen-mapcache.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)
hw/xen/xen-mapcache.c +19 -7
index 2c8f861fdb..e31d379702 100644
@@ -75,7 +75,8 @@ typedef struct MapCache {
 } MapCache;
 
 static MapCache *mapcache;
-static MapCache *mapcache_grants;
+static MapCache *mapcache_grants_ro;
+static MapCache *mapcache_grants_rw;
 static xengnttab_handle *xen_region_gnttabdev;
 
 static inline void mapcache_lock(MapCache *mc)

@@ -176,9 +177,12 @@ void xen_map_cache_init(phys_offset_to_gaddr_t f, void *opaque)
      * Grant mappings must use XC_PAGE_SIZE granularity since we can't
      * map anything beyond the number of pages granted to us.
      */
-    mapcache_grants = xen_map_cache_init_single(f, opaque,
-                                                XC_PAGE_SHIFT,
-                                                max_mcache_size);
+    mapcache_grants_ro = xen_map_cache_init_single(f, opaque,
+                                                   XC_PAGE_SHIFT,
+                                                   max_mcache_size);
+    mapcache_grants_rw = xen_map_cache_init_single(f, opaque,
+                                                   XC_PAGE_SHIFT,
+                                                   max_mcache_size);
 
     setrlimit(RLIMIT_AS, &rlimit_as);
 }

@@ -456,9 +460,13 @@ uint8_t *xen_map_cache(MemoryRegion *mr,
                        bool is_write)
 {
     bool grant = xen_mr_is_grants(mr);
-    MapCache *mc = grant ? mapcache_grants : mapcache;
+    MapCache *mc = mapcache;
     uint8_t *p;
 
+    if (grant) {
+        mc = is_write ? mapcache_grants_rw : mapcache_grants_ro;
+    }
+
     if (grant && !lock) {
         /*
          * Grants are only supported via address_space_map(). Anything

@@ -523,7 +531,10 @@ ram_addr_t xen_ram_addr_from_mapcache(void *ptr)
 
     addr = xen_ram_addr_from_mapcache_single(mapcache, ptr);
     if (addr == RAM_ADDR_INVALID) {
-        addr = xen_ram_addr_from_mapcache_single(mapcache_grants, ptr);
+        addr = xen_ram_addr_from_mapcache_single(mapcache_grants_ro, ptr);
+    }
+    if (addr == RAM_ADDR_INVALID) {
+        addr = xen_ram_addr_from_mapcache_single(mapcache_grants_rw, ptr);
     }
 
     return addr;

@@ -626,7 +637,8 @@ static void xen_invalidate_map_cache_entry_single(MapCache *mc, uint8_t *buffer)
 static void xen_invalidate_map_cache_entry_all(uint8_t *buffer)
 {
     xen_invalidate_map_cache_entry_single(mapcache, buffer);
-    xen_invalidate_map_cache_entry_single(mapcache_grants, buffer);
+    xen_invalidate_map_cache_entry_single(mapcache_grants_ro, buffer);
+    xen_invalidate_map_cache_entry_single(mapcache_grants_rw, buffer);
 }
 
 static void xen_invalidate_map_cache_entry_bh(void *opaque)
Signature · 1 line
2.43.0