| Message ID | 1499183267-28623-4-git-send-email-igor.druzhinin@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> -----Original Message----- > From: Igor Druzhinin > Sent: 04 July 2017 16:48 > To: xen-devel@lists.xenproject.org; qemu-devel@nongnu.org > Cc: Igor Druzhinin <igor.druzhinin@citrix.com>; sstabellini@kernel.org; > Anthony Perard <anthony.perard@citrix.com>; Paul Durrant > <Paul.Durrant@citrix.com>; pbonzini@redhat.com > 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. Paul > > [1] https://www.mail-archive.com/xen-devel@lists.xen.org/msg113007.html > > Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com> > --- > 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
On 04/07/17 17:27, Paul Durrant wrote: >> -----Original Message----- >> From: Igor Druzhinin >> Sent: 04 July 2017 16:48 >> To: xen-devel@lists.xenproject.org; qemu-devel@nongnu.org >> Cc: Igor Druzhinin <igor.druzhinin@citrix.com>; sstabellini@kernel.org; >> Anthony Perard <anthony.perard@citrix.com>; Paul Durrant >> <Paul.Durrant@citrix.com>; pbonzini@redhat.com >> 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@lists.xen.org/msg113007.html >> >> Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com> >> --- >> 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 >
> -----Original Message----- > From: Igor Druzhinin > Sent: 04 July 2017 17:34 > To: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org; > qemu-devel@nongnu.org > Cc: sstabellini@kernel.org; Anthony Perard <anthony.perard@citrix.com>; > pbonzini@redhat.com > Subject: Re: [PATCH v2 3/4] xen/mapcache: introduce > xen_replace_cache_entry() > > On 04/07/17 17:27, Paul Durrant wrote: > >> -----Original Message----- > >> From: Igor Druzhinin > >> Sent: 04 July 2017 16:48 > >> To: xen-devel@lists.xenproject.org; qemu-devel@nongnu.org > >> Cc: Igor Druzhinin <igor.druzhinin@citrix.com>; sstabellini@kernel.org; > >> Anthony Perard <anthony.perard@citrix.com>; Paul Durrant > >> <Paul.Durrant@citrix.com>; pbonzini@redhat.com > >> 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. AFAICT XEN_COMPAT_PHYSMAP is not introduced until patch #4 though. The problem really comes down to defining xenforeignmemory_map2() in terms of xenforeignmemory_map(). It basically can't be safely done. Could you define xenforeignmemory_map2() as abort() in the compat case instead? Paul > > Igor > > > Paul > > > >> > >> [1] https://www.mail-archive.com/xen- > devel@lists.xen.org/msg113007.html > >> > >> Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com> > >> --- > >> 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 > >
On 04/07/17 17:42, Paul Durrant wrote: >> -----Original Message----- >> From: Igor Druzhinin >> Sent: 04 July 2017 17:34 >> To: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org; >> qemu-devel@nongnu.org >> Cc: sstabellini@kernel.org; Anthony Perard <anthony.perard@citrix.com>; >> pbonzini@redhat.com >> Subject: Re: [PATCH v2 3/4] xen/mapcache: introduce >> xen_replace_cache_entry() >> >> On 04/07/17 17:27, Paul Durrant wrote: >>>> -----Original Message----- >>>> From: Igor Druzhinin >>>> Sent: 04 July 2017 16:48 >>>> To: xen-devel@lists.xenproject.org; qemu-devel@nongnu.org >>>> Cc: Igor Druzhinin <igor.druzhinin@citrix.com>; sstabellini@kernel.org; >>>> Anthony Perard <anthony.perard@citrix.com>; Paul Durrant >>>> <Paul.Durrant@citrix.com>; pbonzini@redhat.com >>>> 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. > > AFAICT XEN_COMPAT_PHYSMAP is not introduced until patch #4 though. > > The problem really comes down to defining xenforeignmemory_map2() in terms of xenforeignmemory_map(). It basically can't be safely done. Could you define xenforeignmemory_map2() as abort() in the compat case instead? > xen_replace_cache_entry() is not called in patch #3. Which means it's safe to use a fallback version (xenforeignmemory_map) in xen_remap_bucket here. Igor > Paul > >> >> Igor >> >>> Paul >>> >>>> >>>> [1] https://www.mail-archive.com/xen- >> devel@lists.xen.org/msg113007.html >>>> >>>> Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com> >>>> --- >>>> 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 >>>
> -----Original Message----- > From: Igor Druzhinin > Sent: 04 July 2017 17:47 > To: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org; > qemu-devel@nongnu.org > Cc: sstabellini@kernel.org; Anthony Perard <anthony.perard@citrix.com>; > pbonzini@redhat.com > Subject: Re: [PATCH v2 3/4] xen/mapcache: introduce > xen_replace_cache_entry() > > On 04/07/17 17:42, Paul Durrant wrote: > >> -----Original Message----- > >> From: Igor Druzhinin > >> Sent: 04 July 2017 17:34 > >> To: Paul Durrant <Paul.Durrant@citrix.com>; xen- > devel@lists.xenproject.org; > >> qemu-devel@nongnu.org > >> Cc: sstabellini@kernel.org; Anthony Perard > <anthony.perard@citrix.com>; > >> pbonzini@redhat.com > >> Subject: Re: [PATCH v2 3/4] xen/mapcache: introduce > >> xen_replace_cache_entry() > >> > >> On 04/07/17 17:27, Paul Durrant wrote: > >>>> -----Original Message----- > >>>> From: Igor Druzhinin > >>>> Sent: 04 July 2017 16:48 > >>>> To: xen-devel@lists.xenproject.org; qemu-devel@nongnu.org > >>>> Cc: Igor Druzhinin <igor.druzhinin@citrix.com>; sstabellini@kernel.org; > >>>> Anthony Perard <anthony.perard@citrix.com>; Paul Durrant > >>>> <Paul.Durrant@citrix.com>; pbonzini@redhat.com > >>>> 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. > > > > AFAICT XEN_COMPAT_PHYSMAP is not introduced until patch #4 though. > > > > The problem really comes down to defining xenforeignmemory_map2() in > terms of xenforeignmemory_map(). It basically can't be safely done. Could > you define xenforeignmemory_map2() as abort() in the compat case > instead? > > > > xen_replace_cache_entry() is not called in patch #3. Which means it's > safe to use a fallback version (xenforeignmemory_map) in > xen_remap_bucket here. I still don't like the fact that the compat definition of xenforeignmemory_map2() loses the extra argument. That's going to catch someone out one day. Is there any way you could re-work it so that xenforeignmemory_map() is uses in the cases where the memory placement does not matter? Paul > > Igor > > > Paul > > > >> > >> Igor > >> > >>> Paul > >>> > >>>> > >>>> [1] https://www.mail-archive.com/xen- > >> devel@lists.xen.org/msg113007.html > >>>> > >>>> Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com> > >>>> --- > >>>> 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 > >>>
On Tue, 4 Jul 2017, Igor Druzhinin wrote: > 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]. > > [1] https://www.mail-archive.com/xen-devel@lists.xen.org/msg113007.html > > Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com> > --- > 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); > + } I would prefer to see checks based on the dummy flag, rather than entry->vaddr_base != vaddr. > 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); > } Can we print a warning if (!dummy && vaddr != NULL)? > 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); > + } Please also check (or check instead) on the dummy flag. > 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 >
On Wed, 5 Jul 2017, Paul Durrant wrote: > > -----Original Message----- > > From: Igor Druzhinin > > Sent: 04 July 2017 17:47 > > To: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org; > > qemu-devel@nongnu.org > > Cc: sstabellini@kernel.org; Anthony Perard <anthony.perard@citrix.com>; > > pbonzini@redhat.com > > Subject: Re: [PATCH v2 3/4] xen/mapcache: introduce > > xen_replace_cache_entry() > > > > On 04/07/17 17:42, Paul Durrant wrote: > > >> -----Original Message----- > > >> From: Igor Druzhinin > > >> Sent: 04 July 2017 17:34 > > >> To: Paul Durrant <Paul.Durrant@citrix.com>; xen- > > devel@lists.xenproject.org; > > >> qemu-devel@nongnu.org > > >> Cc: sstabellini@kernel.org; Anthony Perard > > <anthony.perard@citrix.com>; > > >> pbonzini@redhat.com > > >> Subject: Re: [PATCH v2 3/4] xen/mapcache: introduce > > >> xen_replace_cache_entry() > > >> > > >> On 04/07/17 17:27, Paul Durrant wrote: > > >>>> -----Original Message----- > > >>>> From: Igor Druzhinin > > >>>> Sent: 04 July 2017 16:48 > > >>>> To: xen-devel@lists.xenproject.org; qemu-devel@nongnu.org > > >>>> Cc: Igor Druzhinin <igor.druzhinin@citrix.com>; sstabellini@kernel.org; > > >>>> Anthony Perard <anthony.perard@citrix.com>; Paul Durrant > > >>>> <Paul.Durrant@citrix.com>; pbonzini@redhat.com > > >>>> 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. > > > > > > AFAICT XEN_COMPAT_PHYSMAP is not introduced until patch #4 though. > > > > > > The problem really comes down to defining xenforeignmemory_map2() in > > terms of xenforeignmemory_map(). It basically can't be safely done. Could > > you define xenforeignmemory_map2() as abort() in the compat case > > instead? > > > > > > > xen_replace_cache_entry() is not called in patch #3. Which means it's > > safe to use a fallback version (xenforeignmemory_map) in > > xen_remap_bucket here. > > I still don't like the fact that the compat definition of xenforeignmemory_map2() loses the extra argument. That's going to catch someone out one day. Is there any way you could re-work it so that xenforeignmemory_map() is uses in the cases where the memory placement does not matter? We could assert(vaddr == NULL) in the compat implementation of xenforeignmemory_map2. Would that work? > > Igor > > > > > Paul > > > > > >> > > >> Igor > > >> > > >>> Paul > > >>> > > >>>> > > >>>> [1] https://www.mail-archive.com/xen- > > >> devel@lists.xen.org/msg113007.html > > >>>> > > >>>> Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com> > > >>>> --- > > >>>> 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 > > >>> >
> -----Original Message----- > > > > > > > > The problem really comes down to defining > xenforeignmemory_map2() in > > > terms of xenforeignmemory_map(). It basically can't be safely done. > Could > > > you define xenforeignmemory_map2() as abort() in the compat case > > > instead? > > > > > > > > > > xen_replace_cache_entry() is not called in patch #3. Which means it's > > > safe to use a fallback version (xenforeignmemory_map) in > > > xen_remap_bucket here. > > > > I still don't like the fact that the compat definition of > xenforeignmemory_map2() loses the extra argument. That's going to catch > someone out one day. Is there any way you could re-work it so that > xenforeignmemory_map() is uses in the cases where the memory > placement does not matter? > > We could assert(vaddr == NULL) in the compat implementation of > xenforeignmemory_map2. Would that work? > Yes, if the patch was changed from being a straight #define as it is now to an inline that had such an assertion then that would be ok. Cheers, Paul
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 */
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]. [1] https://www.mail-archive.com/xen-devel@lists.xen.org/msg113007.html Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com> --- 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(-)