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

["Edgar E. Iglesias" <edgar.iglesias@xxxxxxxxx>]

On Fri, Apr 18, 2025 at 04:39:04PM -0700, Stefano Stabellini wrote:
> 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?


Looks good to me too, thanks!

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xxxxxxx>




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