Re: [Xen-devel] [PATCH v2 3/4] xen/mapcache: introduce xen_replace_cache_entry()

[Igor Druzhinin <igor.druzhinin@xxxxxxxxxx>]

On 04/07/17 17:27, Paul Durrant wrote:
>> -----Original Message-----
>> From: Igor Druzhinin
>> Sent: 04 July 2017 16:48
>> To: xen-devel@xxxxxxxxxxxxxxxxxxxx; qemu-devel@xxxxxxxxxx
>> Cc: Igor Druzhinin <igor.druzhinin@xxxxxxxxxx>; sstabellini@xxxxxxxxxx;
>> Anthony Perard <anthony.perard@xxxxxxxxxx>; Paul Durrant
>> <Paul.Durrant@xxxxxxxxxx>; pbonzini@xxxxxxxxxx
>> Subject: [PATCH v2 3/4] xen/mapcache: introduce
>> xen_replace_cache_entry()
>>
>> This new call is trying to update a requested map cache entry
>> according to the changes in the physmap. The call is searching
>> for the entry, unmaps it and maps again at the same place using
>> a new guest address. If the mapping is dummy this call will
>> make it real.
>>
>> This function makes use of a new xenforeignmemory_map2() call
>> with an extended interface that was recently introduced in
>> libxenforeignmemory [1].
> 
> I don't understand how the compat layer works here. If 
> xenforeignmemory_map2() is not available then you can't control the placement 
> in virtual address space.
> 

If it's not 4.10 or newer xenforeignmemory_map2() doesn't exist and is
going to be defined as xenforeignmemory_map(). At the same time
XEN_COMPAT_PHYSMAP is defined and the entry replace function (which
relies on xenforeignmemory_map2 functionality) is never going to be called.

If you mean that I should incorporate this into the description I can do it.

Igor

>   Paul
> 
>>
>> [1] https://www.mail-archive.com/xen-devel@xxxxxxxxxxxxx/msg113007.html
>>
>> Signed-off-by: Igor Druzhinin <igor.druzhinin@xxxxxxxxxx>
>> ---
>>  configure                     | 18 ++++++++++
>>  hw/i386/xen/xen-mapcache.c    | 79
>> ++++++++++++++++++++++++++++++++++++++-----
>>  include/hw/xen/xen_common.h   |  7 ++++
>>  include/sysemu/xen-mapcache.h | 11 +++++-
>>  4 files changed, 106 insertions(+), 9 deletions(-)
>>
>> diff --git a/configure b/configure
>> index c571ad1..ad6156b 100755
>> --- a/configure
>> +++ b/configure
>> @@ -2021,6 +2021,24 @@ EOF
>>      # Xen unstable
>>      elif
>>          cat > $TMPC <<EOF &&
>> +#undef XC_WANT_COMPAT_MAP_FOREIGN_API
>> +#include <xenforeignmemory.h>
>> +int main(void) {
>> +  xenforeignmemory_handle *xfmem;
>> +
>> +  xfmem = xenforeignmemory_open(0, 0);
>> +  xenforeignmemory_map2(xfmem, 0, 0, 0, 0, 0, 0, 0);
>> +
>> +  return 0;
>> +}
>> +EOF
>> +        compile_prog "" "$xen_libs -lxendevicemodel $xen_stable_libs"
>> +      then
>> +      xen_stable_libs="-lxendevicemodel $xen_stable_libs"
>> +      xen_ctrl_version=41000
>> +      xen=yes
>> +    elif
>> +        cat > $TMPC <<EOF &&
>>  #undef XC_WANT_COMPAT_DEVICEMODEL_API
>>  #define __XEN_TOOLS__
>>  #include <xendevicemodel.h>
>> diff --git a/hw/i386/xen/xen-mapcache.c b/hw/i386/xen/xen-mapcache.c
>> index cd4e746..a988be7 100644
>> --- a/hw/i386/xen/xen-mapcache.c
>> +++ b/hw/i386/xen/xen-mapcache.c
>> @@ -151,6 +151,7 @@ void xen_map_cache_init(phys_offset_to_gaddr_t f,
>> void *opaque)
>>  }
>>
>>  static void xen_remap_bucket(MapCacheEntry *entry,
>> +                             void *vaddr,
>>                               hwaddr size,
>>                               hwaddr address_index,
>>                               bool dummy)
>> @@ -167,7 +168,9 @@ static void xen_remap_bucket(MapCacheEntry
>> *entry,
>>      err = g_malloc0(nb_pfn * sizeof (int));
>>
>>      if (entry->vaddr_base != NULL) {
>> -        ram_block_notify_remove(entry->vaddr_base, entry->size);
>> +        if (entry->vaddr_base != vaddr) {
>> +            ram_block_notify_remove(entry->vaddr_base, entry->size);
>> +        }
>>          if (munmap(entry->vaddr_base, entry->size) != 0) {
>>              perror("unmap fails");
>>              exit(-1);
>> @@ -181,11 +184,11 @@ static void xen_remap_bucket(MapCacheEntry
>> *entry,
>>      }
>>
>>      if (!dummy) {
>> -        vaddr_base = xenforeignmemory_map(xen_fmem, xen_domid,
>> -                                           PROT_READ|PROT_WRITE,
>> +        vaddr_base = xenforeignmemory_map2(xen_fmem, xen_domid,
>> vaddr,
>> +                                           PROT_READ|PROT_WRITE, 0,
>>                                             nb_pfn, pfns, err);
>>          if (vaddr_base == NULL) {
>> -            perror("xenforeignmemory_map");
>> +            perror("xenforeignmemory_map2");
>>              exit(-1);
>>          }
>>          entry->flags &= ~(XEN_MAPCACHE_ENTRY_DUMMY);
>> @@ -194,7 +197,7 @@ static void xen_remap_bucket(MapCacheEntry
>> *entry,
>>           * We create dummy mappings where we are unable to create a foreign
>>           * mapping immediately due to certain circumstances (i.e. on resume
>> now)
>>           */
>> -        vaddr_base = mmap(NULL, size, PROT_READ|PROT_WRITE,
>> +        vaddr_base = mmap(vaddr, size, PROT_READ|PROT_WRITE,
>>                            MAP_ANON|MAP_SHARED, -1, 0);
>>          if (vaddr_base == NULL) {
>>              perror("mmap");
>> @@ -203,13 +206,16 @@ static void xen_remap_bucket(MapCacheEntry
>> *entry,
>>          entry->flags |= XEN_MAPCACHE_ENTRY_DUMMY;
>>      }
>>
>> +    if (entry->vaddr_base == NULL || entry->vaddr_base != vaddr) {
>> +        ram_block_notify_add(vaddr_base, size);
>> +    }
>> +
>>      entry->vaddr_base = vaddr_base;
>>      entry->paddr_index = address_index;
>>      entry->size = size;
>>      entry->valid_mapping = (unsigned long *) g_malloc0(sizeof(unsigned long)
>> *
>>              BITS_TO_LONGS(size >> XC_PAGE_SHIFT));
>>
>> -    ram_block_notify_add(entry->vaddr_base, entry->size);
>>      bitmap_zero(entry->valid_mapping, nb_pfn);
>>      for (i = 0; i < nb_pfn; i++) {
>>          if (!err[i]) {
>> @@ -282,14 +288,14 @@ tryagain:
>>      if (!entry) {
>>          entry = g_malloc0(sizeof (MapCacheEntry));
>>          pentry->next = entry;
>> -        xen_remap_bucket(entry, cache_size, address_index, dummy);
>> +        xen_remap_bucket(entry, NULL, cache_size, address_index, dummy);
>>      } else if (!entry->lock) {
>>          if (!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)) {
>> -            xen_remap_bucket(entry, cache_size, address_index, dummy);
>> +            xen_remap_bucket(entry, NULL, cache_size, address_index,
>> dummy);
>>          }
>>      }
>>
>> @@ -486,3 +492,60 @@ void xen_invalidate_map_cache(void)
>>
>>      mapcache_unlock();
>>  }
>> +
>> +static uint8_t *xen_replace_cache_entry_unlocked(hwaddr
>> old_phys_addr,
>> +                                                 hwaddr new_phys_addr,
>> +                                                 hwaddr size)
>> +{
>> +    MapCacheEntry *entry;
>> +    hwaddr address_index;
>> +    hwaddr address_offset;
>> +    hwaddr cache_size = size;
>> +    hwaddr test_bit_size;
>> +
>> +    address_index  = old_phys_addr >> MCACHE_BUCKET_SHIFT;
>> +    address_offset = old_phys_addr & (MCACHE_BUCKET_SIZE - 1);
>> +
>> +    assert(size);
>> +    /* test_bit_size is always a multiple of XC_PAGE_SIZE */
>> +    test_bit_size = size + (old_phys_addr & (XC_PAGE_SIZE - 1));
>> +    if (test_bit_size % XC_PAGE_SIZE) {
>> +        test_bit_size += XC_PAGE_SIZE - (test_bit_size % XC_PAGE_SIZE);
>> +    }
>> +    cache_size = size + address_offset;
>> +    if (cache_size % MCACHE_BUCKET_SIZE) {
>> +        cache_size += MCACHE_BUCKET_SIZE - (cache_size %
>> MCACHE_BUCKET_SIZE);
>> +    }
>> +
>> +    entry = &mapcache->entry[address_index % mapcache->nr_buckets];
>> +    while (entry && !(entry->paddr_index == address_index && entry->size
>> == cache_size)) {
>> +        entry = entry->next;
>> +    }
>> +    if (!entry) {
>> +        DPRINTF("Trying to update an entry for %lx that is not in the
>> mapcache!\n", phys_addr);
>> +        return NULL;
>> +    }
>> +
>> +    address_index  = new_phys_addr >> MCACHE_BUCKET_SHIFT;
>> +    address_offset = new_phys_addr & (MCACHE_BUCKET_SIZE - 1);
>> +
>> +    xen_remap_bucket(entry, entry->vaddr_base, cache_size,
>> address_index, false);
>> +    if(!test_bits(address_offset >> XC_PAGE_SHIFT,
>> +                test_bit_size >> XC_PAGE_SHIFT,
>> +                entry->valid_mapping)) {
>> +        DPRINTF("Unable to update an entry for %lx in the mapcache!\n",
>> phys_addr);
>> +        return NULL;
>> +    }
>> +
>> +    return entry->vaddr_base + address_offset;
>> +}
>> +
>> +uint8_t *xen_replace_cache_entry(hwaddr old_phys_addr, hwaddr
>> new_phys_addr, hwaddr size)
>> +{
>> +    uint8_t *p;
>> +
>> +    mapcache_lock();
>> +    p = xen_replace_cache_entry_unlocked(old_phys_addr,
>> new_phys_addr, size);
>> +    mapcache_unlock();
>> +    return p;
>> +}
>> diff --git a/include/hw/xen/xen_common.h
>> b/include/hw/xen/xen_common.h
>> index e00ddd7..70a5cad 100644
>> --- a/include/hw/xen/xen_common.h
>> +++ b/include/hw/xen/xen_common.h
>> @@ -78,6 +78,13 @@ static inline void
>> *xenforeignmemory_map(xc_interface *h, uint32_t dom,
>>
>>  extern xenforeignmemory_handle *xen_fmem;
>>
>> +#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 41000
>> +
>> +#define xenforeignmemory_map2(h, d, a, p, f, ps, ar, e) \
>> +    xenforeignmemory_map(h, d, p, ps, ar, e)
>> +
>> +#endif
>> +
>>  #if CONFIG_XEN_CTRL_INTERFACE_VERSION < 40900
>>
>>  typedef xc_interface xendevicemodel_handle;
>> diff --git a/include/sysemu/xen-mapcache.h b/include/sysemu/xen-
>> mapcache.h
>> index 01daaad..b38962c 100644
>> --- a/include/sysemu/xen-mapcache.h
>> +++ b/include/sysemu/xen-mapcache.h
>> @@ -21,7 +21,9 @@ uint8_t *xen_map_cache(hwaddr phys_addr, hwaddr
>> size,
>>  ram_addr_t xen_ram_addr_from_mapcache(void *ptr);
>>  void xen_invalidate_map_cache_entry(uint8_t *buffer);
>>  void xen_invalidate_map_cache(void);
>> -
>> +uint8_t *xen_replace_cache_entry(hwaddr old_phys_addr,
>> +                                 hwaddr new_phys_addr,
>> +                                 hwaddr size);
>>  #else
>>
>>  static inline void xen_map_cache_init(phys_offset_to_gaddr_t f,
>> @@ -50,6 +52,13 @@ static inline void xen_invalidate_map_cache(void)
>>  {
>>  }
>>
>> +uint8_t *xen_replace_cache_entry(hwaddr old_phys_addr,
>> +                                 hwaddr new_phys_addr,
>> +                                 hwaddr size)
>> +{
>> +    abort();
>> +}
>> +
>>  #endif
>>
>>  #endif /* XEN_MAPCACHE_H */
>> --
>> 2.7.4
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel