* [PULL v1 1/2] xen: mapcache: Fix finding matching entry
2025-05-06 18:26 [PULL v1 0/2] xen: mapcache: Fixes Edgar E. Iglesias
@ 2025-05-06 18:26 ` Edgar E. Iglesias
2025-05-06 18:26 ` [PULL v1 2/2] xen: mapcache: Split mapcache_grants by ro and rw Edgar E. Iglesias
2025-05-10 18:35 ` [PULL v1 0/2] xen: mapcache: Fixes Stefan Hajnoczi
2 siblings, 0 replies; 4+ messages in thread
From: Edgar E. Iglesias @ 2025-05-06 18:26 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, sstabellini, anthony, paul, alex.pentagrid,
peter.maydell, edgar.iglesias, xen-devel, Edgar E. Iglesias
From: Aleksandr Partanen <alex.pentagrid@gmail.com>
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 <alex.pentagrid@gmail.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
---
hw/xen/xen-mapcache.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c
index 698b5c53ed..2c8f861fdb 100644
--- a/hw/xen/xen-mapcache.c
+++ b/hw/xen/xen-mapcache.c
@@ -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;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PULL v1 2/2] xen: mapcache: Split mapcache_grants by ro and rw
2025-05-06 18:26 [PULL v1 0/2] xen: mapcache: Fixes Edgar E. Iglesias
2025-05-06 18:26 ` [PULL v1 1/2] xen: mapcache: Fix finding matching entry Edgar E. Iglesias
@ 2025-05-06 18:26 ` Edgar E. Iglesias
2025-05-10 18:35 ` [PULL v1 0/2] xen: mapcache: Fixes Stefan Hajnoczi
2 siblings, 0 replies; 4+ messages in thread
From: Edgar E. Iglesias @ 2025-05-06 18:26 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, sstabellini, anthony, paul, alex.pentagrid,
peter.maydell, edgar.iglesias, xen-devel,
Philippe Mathieu-Daudé, Edgar E. Iglesias
From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
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é <philmd@linaro.org>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
---
hw/xen/xen-mapcache.c | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c
index 2c8f861fdb..e31d379702 100644
--- a/hw/xen/xen-mapcache.c
+++ b/hw/xen/xen-mapcache.c
@@ -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)
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread