[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 3/4] xen/mapcache: introduce xen_replace_cach
From: |
Igor Druzhinin |
Subject: |
Re: [Qemu-devel] [PATCH v2 3/4] xen/mapcache: introduce xen_replace_cache_entry() |
Date: |
Tue, 4 Jul 2017 17:46:58 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 |
On 04/07/17 17:42, Paul Durrant wrote:
>> -----Original Message-----
>> From: Igor Druzhinin
>> Sent: 04 July 2017 17:34
>> To: Paul Durrant <address@hidden>; address@hidden;
>> address@hidden
>> Cc: address@hidden; Anthony Perard <address@hidden>;
>> address@hidden
>> 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: address@hidden; address@hidden
>>>> Cc: Igor Druzhinin <address@hidden>; address@hidden;
>>>> Anthony Perard <address@hidden>; Paul Durrant
>>>> <address@hidden>; address@hidden
>>>> 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-
>> address@hidden/msg113007.html
>>>>
>>>> Signed-off-by: Igor Druzhinin <address@hidden>
>>>> ---
>>>> 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
>>>
- [Qemu-devel] [PATCH v2 1/4] xen: move physmap saving into a separate function, (continued)
- [Qemu-devel] [PATCH v2 1/4] xen: move physmap saving into a separate function, Igor Druzhinin, 2017/07/04
- [Qemu-devel] [PATCH v2 2/4] xen/mapcache: add an ability to create dummy mappings, Igor Druzhinin, 2017/07/04
- [Qemu-devel] [PATCH v2 3/4] xen/mapcache: introduce xen_replace_cache_entry(), Igor Druzhinin, 2017/07/04
- Re: [Qemu-devel] [PATCH v2 3/4] xen/mapcache: introduce xen_replace_cache_entry(), Paul Durrant, 2017/07/04
- Re: [Qemu-devel] [PATCH v2 3/4] xen/mapcache: introduce xen_replace_cache_entry(), Igor Druzhinin, 2017/07/04
- Re: [Qemu-devel] [PATCH v2 3/4] xen/mapcache: introduce xen_replace_cache_entry(), Paul Durrant, 2017/07/04
- Re: [Qemu-devel] [PATCH v2 3/4] xen/mapcache: introduce xen_replace_cache_entry(),
Igor Druzhinin <=
- Re: [Qemu-devel] [PATCH v2 3/4] xen/mapcache: introduce xen_replace_cache_entry(), Paul Durrant, 2017/07/05
- Re: [Qemu-devel] [PATCH v2 3/4] xen/mapcache: introduce xen_replace_cache_entry(), Stefano Stabellini, 2017/07/05
- Re: [Qemu-devel] [PATCH v2 3/4] xen/mapcache: introduce xen_replace_cache_entry(), Paul Durrant, 2017/07/06
Re: [Qemu-devel] [PATCH v2 3/4] xen/mapcache: introduce xen_replace_cache_entry(), Stefano Stabellini, 2017/07/05
[Qemu-devel] [PATCH v2 4/4] xen: don't use xenstore to save/restore physmap anymore, Igor Druzhinin, 2017/07/04
Re: [Qemu-devel] [PATCH v2 0/4] xen: don't save/restore the physmap on VM save/restore, no-reply, 2017/07/06