[Xen-devel] Re: [Qemu-devel] [PATCH RFC V3 07/12] xen: Introduce the Xen mapcache

[Anthony PERARD <anthony@xxxxxxxxxx>]

On Fri, 17 Sep 2010, Blue Swirl wrote:

> On Fri, Sep 17, 2010 at 11:15 AM,  <anthony.perard@xxxxxxxxxx> wrote:
> > From: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> >
> > The mapcache maps chucks of guest memory on demand, unmaps them when
> > they are not needed anymore.
> >
> > Each call to qemu_get_ram_ptr makes a call to qemu_map_cache with the
> > lock option, so mapcache will not unmap these ram_ptr.
> >
> > Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> > ---
> >  Makefile.target |    2 +-
> >  exec.c          |   36 ++++++-
> >  hw/xen.h        |    4 +
> >  xen-all.c       |   63 ++++++++++++
> >  xen-stub.c      |    4 +
> >  xen_mapcache.c  |  302 
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  xen_mapcache.h  |   26 +++++
> >  7 files changed, 432 insertions(+), 5 deletions(-)
> >  create mode 100644 xen_mapcache.c
> >  create mode 100644 xen_mapcache.h
> >

[...]

> > +        while (j > 0) {
> > +            word = (word << 1) | !err[i + --j];
>
> You are mixing bitwise OR with logical NOT, is this correct?

Yes, this is correct.

> > +        }
> > +        entry->valid_mapping[i / BITS_PER_LONG] = word;
> > +    }
> > +
> > +    qemu_free(pfns);
> > +    qemu_free(err);
> > +}
> > +
> > +uint8_t *qemu_map_cache(target_phys_addr_t phys_addr, target_phys_addr_t 
> > size, uint8_t lock)
> > +{
> > +    MapCacheEntry *entry, *pentry = NULL;
> > +    unsigned long address_index  = phys_addr >> MCACHE_BUCKET_SHIFT;
> > +    unsigned long address_offset = phys_addr & (MCACHE_BUCKET_SIZE-1);
>
> unsigned long will not be long enough on 32 bit host (or 32 bit user
> space) for a 64 bit target. I can't remember if this was a supported
> case for Xen anyway.

Xen can do that, so I change unsigned long to target_phys_addr_t.


[...]

> > diff --git a/xen_mapcache.h b/xen_mapcache.h
> > new file mode 100644
> > index 0000000..5a6730f
> > --- /dev/null
> > +++ b/xen_mapcache.h
> > @@ -0,0 +1,26 @@
> > +#ifndef XEN_MAPCACHE_H
> > +#define XEN_MAPCACHE_H
> > +
> > +#if (defined(__i386__) || defined(__x86_64__))
> > +#  define MAPCACHE
>
> xen_mapcache.c could be split into two files, xen-mapcache-stub.c and
> xen-mapcache.c. configure could perform the check for i386 or x86_64
> host and define CONFIG_XEN_MAPCACHE=y appropriately. Then
> Makefile.target would compile the correct file based on that.

Ok, I will do that.

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel