diff mbox series

[v4,07/17] xen: mapcache: Refactor xen_replace_cache_entry_unlocked

Message ID 20240430164939.925307-8-edgar.iglesias@gmail.com (mailing list archive)
State New, archived
Headers show
Series None | expand

Commit Message

Edgar E. Iglesias April 30, 2024, 4:49 p.m. UTC 
From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>

Add MapCache argument to xen_replace_cache_entry_unlocked in
preparation for supporting multiple map caches.

No functional change.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
---
 hw/xen/xen-mapcache.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Stefano Stabellini May 1, 2024, 8:46 p.m. UTC | #1 
On Tue, 30 Apr 2024, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
> 
> Add MapCache argument to xen_replace_cache_entry_unlocked in
> preparation for supporting multiple map caches.
> 
> No functional change.
> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> ---
>  hw/xen/xen-mapcache.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c
> index d2deff70c8..6e758eff94 100644
> --- a/hw/xen/xen-mapcache.c
> +++ b/hw/xen/xen-mapcache.c
> @@ -556,7 +556,8 @@ void xen_invalidate_map_cache(void)
>      mapcache_unlock(mapcache);
>  }
>  
> -static uint8_t *xen_replace_cache_entry_unlocked(hwaddr old_phys_addr,
> +static uint8_t *xen_replace_cache_entry_unlocked(MapCache *mc,
> +                                                 hwaddr old_phys_addr,
>                                                   hwaddr new_phys_addr,
>                                                   hwaddr size)
>  {
> @@ -578,7 +579,7 @@ static uint8_t *xen_replace_cache_entry_unlocked(hwaddr old_phys_addr,
>          cache_size += MCACHE_BUCKET_SIZE - (cache_size % MCACHE_BUCKET_SIZE);
>      }
>  
> -    entry = &mapcache->entry[address_index % mapcache->nr_buckets];
> +    entry = &mc->entry[address_index % mc->nr_buckets];
>      while (entry && !(entry->paddr_index == address_index &&
>                        entry->size == cache_size)) {
>          entry = entry->next;

There is still a global mapcache pointer in use in this function:

  xen_remap_bucket(mapcache, entry, entry->vaddr_base,


> @@ -614,7 +615,8 @@ uint8_t *xen_replace_cache_entry(hwaddr old_phys_addr,
>      uint8_t *p;
>  
>      mapcache_lock(mapcache);
> -    p = xen_replace_cache_entry_unlocked(old_phys_addr, new_phys_addr, size);
> +    p = xen_replace_cache_entry_unlocked(mapcache, old_phys_addr,
> +                                         new_phys_addr, size);
>      mapcache_unlock(mapcache);
>      return p;
>  }
> -- 
> 2.40.1
>
Edgar E. Iglesias May 2, 2024, 6:32 a.m. UTC | #2 
On Wed, May 1, 2024 at 10:46 PM Stefano Stabellini
<sstabellini@kernel.org> wrote:
>
> On Tue, 30 Apr 2024, Edgar E. Iglesias wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
> >
> > Add MapCache argument to xen_replace_cache_entry_unlocked in
> > preparation for supporting multiple map caches.
> >
> > No functional change.
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> > ---
> >  hw/xen/xen-mapcache.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c
> > index d2deff70c8..6e758eff94 100644
> > --- a/hw/xen/xen-mapcache.c
> > +++ b/hw/xen/xen-mapcache.c
> > @@ -556,7 +556,8 @@ void xen_invalidate_map_cache(void)
> >      mapcache_unlock(mapcache);
> >  }
> >
> > -static uint8_t *xen_replace_cache_entry_unlocked(hwaddr old_phys_addr,
> > +static uint8_t *xen_replace_cache_entry_unlocked(MapCache *mc,
> > +                                                 hwaddr old_phys_addr,
> >                                                   hwaddr new_phys_addr,
> >                                                   hwaddr size)
> >  {
> > @@ -578,7 +579,7 @@ static uint8_t *xen_replace_cache_entry_unlocked(hwaddr old_phys_addr,
> >          cache_size += MCACHE_BUCKET_SIZE - (cache_size % MCACHE_BUCKET_SIZE);
> >      }
> >
> > -    entry = &mapcache->entry[address_index % mapcache->nr_buckets];
> > +    entry = &mc->entry[address_index % mc->nr_buckets];
> >      while (entry && !(entry->paddr_index == address_index &&
> >                        entry->size == cache_size)) {
> >          entry = entry->next;
>
> There is still a global mapcache pointer in use in this function:
>
>   xen_remap_bucket(mapcache, entry, entry->vaddr_base,
>


Thanks! I had accidentally put the change to use mc in future patches.
Will fix in v5.

Cheers,
Edgar


>
> > @@ -614,7 +615,8 @@ uint8_t *xen_replace_cache_entry(hwaddr old_phys_addr,
> >      uint8_t *p;
> >
> >      mapcache_lock(mapcache);
> > -    p = xen_replace_cache_entry_unlocked(old_phys_addr, new_phys_addr, size);
> > +    p = xen_replace_cache_entry_unlocked(mapcache, old_phys_addr,
> > +                                         new_phys_addr, size);
> >      mapcache_unlock(mapcache);
> >      return p;
> >  }
> > --
> > 2.40.1
> >
Philippe Mathieu-Daudé May 6, 2024, 10:21 a.m. UTC | #3 
On 2/5/24 08:32, Edgar E. Iglesias wrote:
> On Wed, May 1, 2024 at 10:46 PM Stefano Stabellini
> <sstabellini@kernel.org> wrote:
>>
>> On Tue, 30 Apr 2024, Edgar E. Iglesias wrote:
>>> From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
>>>
>>> Add MapCache argument to xen_replace_cache_entry_unlocked in
>>> preparation for supporting multiple map caches.
>>>
>>> No functional change.
>>>
>>> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
>>> ---
>>>   hw/xen/xen-mapcache.c | 8 +++++---
>>>   1 file changed, 5 insertions(+), 3 deletions(-)


>>> -static uint8_t *xen_replace_cache_entry_unlocked(hwaddr old_phys_addr,
>>> +static uint8_t *xen_replace_cache_entry_unlocked(MapCache *mc,
>>> +                                                 hwaddr old_phys_addr,
>>>                                                    hwaddr new_phys_addr,
>>>                                                    hwaddr size)
>>>   {
>>> @@ -578,7 +579,7 @@ static uint8_t *xen_replace_cache_entry_unlocked(hwaddr old_phys_addr,
>>>           cache_size += MCACHE_BUCKET_SIZE - (cache_size % MCACHE_BUCKET_SIZE);
>>>       }
>>>
>>> -    entry = &mapcache->entry[address_index % mapcache->nr_buckets];
>>> +    entry = &mc->entry[address_index % mc->nr_buckets];
>>>       while (entry && !(entry->paddr_index == address_index &&
>>>                         entry->size == cache_size)) {
>>>           entry = entry->next;
>>
>> There is still a global mapcache pointer in use in this function:
>>
>>    xen_remap_bucket(mapcache, entry, entry->vaddr_base,
>>
> 
> 
> Thanks! I had accidentally put the change to use mc in future patches.
> Will fix in v5.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
diff mbox series

Patch

diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c
index d2deff70c8..6e758eff94 100644
--- a/hw/xen/xen-mapcache.c
+++ b/hw/xen/xen-mapcache.c
@@ -556,7 +556,8 @@  void xen_invalidate_map_cache(void)
     mapcache_unlock(mapcache);
 }
 
-static uint8_t *xen_replace_cache_entry_unlocked(hwaddr old_phys_addr,
+static uint8_t *xen_replace_cache_entry_unlocked(MapCache *mc,
+                                                 hwaddr old_phys_addr,
                                                  hwaddr new_phys_addr,
                                                  hwaddr size)
 {
@@ -578,7 +579,7 @@  static uint8_t *xen_replace_cache_entry_unlocked(hwaddr old_phys_addr,
         cache_size += MCACHE_BUCKET_SIZE - (cache_size % MCACHE_BUCKET_SIZE);
     }
 
-    entry = &mapcache->entry[address_index % mapcache->nr_buckets];
+    entry = &mc->entry[address_index % mc->nr_buckets];
     while (entry && !(entry->paddr_index == address_index &&
                       entry->size == cache_size)) {
         entry = entry->next;
@@ -614,7 +615,8 @@  uint8_t *xen_replace_cache_entry(hwaddr old_phys_addr,
     uint8_t *p;
 
     mapcache_lock(mapcache);
-    p = xen_replace_cache_entry_unlocked(old_phys_addr, new_phys_addr, size);
+    p = xen_replace_cache_entry_unlocked(mapcache, old_phys_addr,
+                                         new_phys_addr, size);
     mapcache_unlock(mapcache);
     return p;
 }