Re: [Xen-devel] [PATCH 2/4] xen/mapcache: add an ability to create dummy mappings

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 3 Jul 2017, Igor Druzhinin wrote:
> On 01/07/17 01:06, Stefano Stabellini wrote:
> > On Fri, 30 Jun 2017, Igor Druzhinin wrote:
> >> Dummys are simple anonymous mappings that are placed instead
> >> of regular foreign mappings in certain situations when we need
> >> to postpone the actual mapping but still have to give a
> >> memory region to QEMU to play with.
> >>
> >> This is planned to be used for restore on Xen.
> >>
> >> Signed-off-by: Igor Druzhinin <igor.druzhinin@xxxxxxxxxx>
> >>
> >> ---
> >>  hw/i386/xen/xen-mapcache.c | 36 ++++++++++++++++++++++++++++--------
> >>  1 file changed, 28 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/hw/i386/xen/xen-mapcache.c b/hw/i386/xen/xen-mapcache.c
> >> index e60156c..05050de 100644
> >> --- a/hw/i386/xen/xen-mapcache.c
> >> +++ b/hw/i386/xen/xen-mapcache.c
> >> @@ -150,7 +150,8 @@ void xen_map_cache_init(phys_offset_to_gaddr_t f, void 
> >> *opaque)
> >>  
> >>  static void xen_remap_bucket(MapCacheEntry *entry,
> >>                               hwaddr size,
> >> -                             hwaddr address_index)
> >> +                             hwaddr address_index,
> >> +                             bool dummy)
> >>  {
> >>      uint8_t *vaddr_base;
> >>      xen_pfn_t *pfns;
> >> @@ -177,11 +178,25 @@ static void xen_remap_bucket(MapCacheEntry *entry,
> >>          pfns[i] = (address_index << (MCACHE_BUCKET_SHIFT-XC_PAGE_SHIFT)) 
> >> + i;
> >>      }
> >>  
> >> -    vaddr_base = xenforeignmemory_map(xen_fmem, xen_domid, 
> >> PROT_READ|PROT_WRITE,
> >> -                                      nb_pfn, pfns, err);
> >> -    if (vaddr_base == NULL) {
> >> -        perror("xenforeignmemory_map");
> >> -        exit(-1);
> >> +    if (!dummy) {
> >> +        vaddr_base = xenforeignmemory_map(xen_fmem, xen_domid,
> >> +                                           PROT_READ|PROT_WRITE,
> >> +                                           nb_pfn, pfns, err);
> >> +        if (vaddr_base == NULL) {
> >> +            perror("xenforeignmemory_map");
> >> +            exit(-1);
> >> +        }
> >> +    } else {
> >> +        /*
> >> +         * 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,
> >> +                          MAP_ANON|MAP_SHARED, -1, 0);
> >> +        if (vaddr_base == NULL) {
> >> +            perror("mmap");
> >> +            exit(-1);
> >> +        }
> > 
> > For our sanity in debugging this in the future, I think it's best if we
> > mark this mapcache entry as "dummy". Since we are at it, we could turn
> > the lock field of MapCacheEntry into a flag field and #define LOCK as
> > (1<<0) and DUMMY as (1<<1). Please do that as a separate patch.
> >
> 
> Unfortunately, lock field is a reference counter (or at least it looks
> like according to the source code). It seems to me that it's technically
> possible to have one region locked from several places in QEMU code. For
> that reason, I'd like to introduce a separate field - something like
> uint8_t flags.

Yes, you are right.


> >>>      }
> >>  
> >>      entry->vaddr_base = vaddr_base;
> >> @@ -211,6 +226,7 @@ static uint8_t *xen_map_cache_unlocked(hwaddr 
> >> phys_addr, hwaddr size,
> >>      hwaddr cache_size = size;
> >>      hwaddr test_bit_size;
> >>      bool translated = false;
> >> +    bool dummy = false;
> >>  
> >>  tryagain:
> >>      address_index  = phys_addr >> MCACHE_BUCKET_SHIFT;
> >> @@ -262,14 +278,14 @@ tryagain:
> >>      if (!entry) {
> >>          entry = g_malloc0(sizeof (MapCacheEntry));
> >>          pentry->next = entry;
> >> -        xen_remap_bucket(entry, cache_size, address_index);
> >> +        xen_remap_bucket(entry, 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);
> >> +            xen_remap_bucket(entry, cache_size, address_index, dummy);
> >>          }
> >>      }
> >>  
> >> @@ -282,6 +298,10 @@ tryagain:
> >>              translated = true;
> >>              goto tryagain;
> >>          }
> >> +        if (!dummy && runstate_check(RUN_STATE_INMIGRATE)) {
> >> +            dummy = true;
> >> +            goto tryagain;
> >> +        }
> >>          trace_xen_map_cache_return(NULL);
> >>          return NULL;
> >>      }
> >> -- 
> >> 2.7.4
> >>
> 

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