Re: [PATCH] xen: mapcache: Fix finding matching entry

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 10 Apr 2025, Aleksandr Partanen wrote:
> 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@xxxxxxxxx>

Hi Aleksandr, thanks for the patch, it looks correct to me.

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


Edgar, would you be able to give it a look as well to make sure?


> ---
>  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.39.5
>