* [Qemu-devel] [RFH PATCH 0/2] xen-mapcache: fixes for unlocked memory access @ 2015-01-14 10:20 Paolo Bonzini 2015-01-14 10:20 ` [Qemu-devel] [RFH PATCH 1/2] xen: do not use __-named variables in mapcache Paolo Bonzini 2015-01-14 10:20 ` [Qemu-devel] [RFH PATCH 2/2] xen: add a lock for the mapcache Paolo Bonzini 0 siblings, 2 replies; 5+ messages in thread From: Paolo Bonzini @ 2015-01-14 10:20 UTC (permalink / raw) To: qemu-devel; +Cc: stefano.stabellini These are small preparations to let Xen support memory access outside the BQL. Warning: they are not tested beyond compilation. Paolo Bonzini (2): xen: do not use __-named variables in mapcache xen: add a lock for the mapcache xen-mapcache.c | 94 +++++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 63 insertions(+), 31 deletions(-) -- 1.8.3.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [RFH PATCH 1/2] xen: do not use __-named variables in mapcache 2015-01-14 10:20 [Qemu-devel] [RFH PATCH 0/2] xen-mapcache: fixes for unlocked memory access Paolo Bonzini @ 2015-01-14 10:20 ` Paolo Bonzini 2015-01-14 11:44 ` Stefano Stabellini 2015-01-14 10:20 ` [Qemu-devel] [RFH PATCH 2/2] xen: add a lock for the mapcache Paolo Bonzini 1 sibling, 1 reply; 5+ messages in thread From: Paolo Bonzini @ 2015-01-14 10:20 UTC (permalink / raw) To: qemu-devel; +Cc: stefano.stabellini Keep the namespace clean. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- xen-mapcache.c | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/xen-mapcache.c b/xen-mapcache.c index 66da1a6..458069b 100644 --- a/xen-mapcache.c +++ b/xen-mapcache.c @@ -199,8 +199,8 @@ uint8_t *xen_map_cache(hwaddr phys_addr, hwaddr size, MapCacheEntry *entry, *pentry = NULL; hwaddr address_index; hwaddr address_offset; - hwaddr __size = size; - hwaddr __test_bit_size = size; + hwaddr cache_size = size; + hwaddr test_bit_size; bool translated = false; tryagain: @@ -209,22 +209,22 @@ tryagain: trace_xen_map_cache(phys_addr); - /* __test_bit_size is always a multiple of XC_PAGE_SIZE */ + /* test_bit_size is always a multiple of XC_PAGE_SIZE */ if (size) { - __test_bit_size = size + (phys_addr & (XC_PAGE_SIZE - 1)); + test_bit_size = size + (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); + if (test_bit_size % XC_PAGE_SIZE) { + test_bit_size += XC_PAGE_SIZE - (test_bit_size % XC_PAGE_SIZE); } } else { - __test_bit_size = XC_PAGE_SIZE; + test_bit_size = XC_PAGE_SIZE; } if (mapcache->last_entry != NULL && mapcache->last_entry->paddr_index == address_index && - !lock && !__size && + !lock && !size && test_bits(address_offset >> XC_PAGE_SHIFT, - __test_bit_size >> XC_PAGE_SHIFT, + test_bit_size >> XC_PAGE_SHIFT, mapcache->last_entry->valid_mapping)) { trace_xen_map_cache_return(mapcache->last_entry->vaddr_base + address_offset); return mapcache->last_entry->vaddr_base + address_offset; @@ -232,20 +232,20 @@ tryagain: /* size is always a multiple of MCACHE_BUCKET_SIZE */ if (size) { - __size = size + address_offset; - if (__size % MCACHE_BUCKET_SIZE) { - __size += MCACHE_BUCKET_SIZE - (__size % MCACHE_BUCKET_SIZE); + cache_size = size + address_offset; + if (cache_size % MCACHE_BUCKET_SIZE) { + cache_size += MCACHE_BUCKET_SIZE - (cache_size % MCACHE_BUCKET_SIZE); } } else { - __size = MCACHE_BUCKET_SIZE; + cache_size = MCACHE_BUCKET_SIZE; } entry = &mapcache->entry[address_index % mapcache->nr_buckets]; while (entry && entry->lock && entry->vaddr_base && - (entry->paddr_index != address_index || entry->size != __size || + (entry->paddr_index != address_index || entry->size != cache_size || !test_bits(address_offset >> XC_PAGE_SHIFT, - __test_bit_size >> XC_PAGE_SHIFT, + test_bit_size >> XC_PAGE_SHIFT, entry->valid_mapping))) { pentry = entry; entry = entry->next; @@ -253,19 +253,19 @@ tryagain: if (!entry) { entry = g_malloc0(sizeof (MapCacheEntry)); pentry->next = entry; - xen_remap_bucket(entry, __size, address_index); + xen_remap_bucket(entry, cache_size, address_index); } else if (!entry->lock) { if (!entry->vaddr_base || entry->paddr_index != address_index || - entry->size != __size || + entry->size != cache_size || !test_bits(address_offset >> XC_PAGE_SHIFT, - __test_bit_size >> XC_PAGE_SHIFT, + test_bit_size >> XC_PAGE_SHIFT, entry->valid_mapping)) { - xen_remap_bucket(entry, __size, address_index); + xen_remap_bucket(entry, cache_size, address_index); } } if(!test_bits(address_offset >> XC_PAGE_SHIFT, - __test_bit_size >> XC_PAGE_SHIFT, + test_bit_size >> XC_PAGE_SHIFT, entry->valid_mapping)) { mapcache->last_entry = NULL; if (!translated && mapcache->phys_offset_to_gaddr) { -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [RFH PATCH 1/2] xen: do not use __-named variables in mapcache 2015-01-14 10:20 ` [Qemu-devel] [RFH PATCH 1/2] xen: do not use __-named variables in mapcache Paolo Bonzini @ 2015-01-14 11:44 ` Stefano Stabellini 0 siblings, 0 replies; 5+ messages in thread From: Stefano Stabellini @ 2015-01-14 11:44 UTC (permalink / raw) To: Paolo Bonzini; +Cc: qemu-devel, stefano.stabellini On Wed, 14 Jan 2015, Paolo Bonzini wrote: > Keep the namespace clean. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > xen-mapcache.c | 40 ++++++++++++++++++++-------------------- > 1 file changed, 20 insertions(+), 20 deletions(-) > > diff --git a/xen-mapcache.c b/xen-mapcache.c > index 66da1a6..458069b 100644 > --- a/xen-mapcache.c > +++ b/xen-mapcache.c > @@ -199,8 +199,8 @@ uint8_t *xen_map_cache(hwaddr phys_addr, hwaddr size, > MapCacheEntry *entry, *pentry = NULL; > hwaddr address_index; > hwaddr address_offset; > - hwaddr __size = size; > - hwaddr __test_bit_size = size; > + hwaddr cache_size = size; > + hwaddr test_bit_size; > bool translated = false; > > tryagain: > @@ -209,22 +209,22 @@ tryagain: > > trace_xen_map_cache(phys_addr); > > - /* __test_bit_size is always a multiple of XC_PAGE_SIZE */ > + /* test_bit_size is always a multiple of XC_PAGE_SIZE */ > if (size) { > - __test_bit_size = size + (phys_addr & (XC_PAGE_SIZE - 1)); > + test_bit_size = size + (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); > + if (test_bit_size % XC_PAGE_SIZE) { > + test_bit_size += XC_PAGE_SIZE - (test_bit_size % XC_PAGE_SIZE); > } > } else { > - __test_bit_size = XC_PAGE_SIZE; > + test_bit_size = XC_PAGE_SIZE; > } > > if (mapcache->last_entry != NULL && > mapcache->last_entry->paddr_index == address_index && > - !lock && !__size && > + !lock && !size && > test_bits(address_offset >> XC_PAGE_SHIFT, > - __test_bit_size >> XC_PAGE_SHIFT, > + test_bit_size >> XC_PAGE_SHIFT, > mapcache->last_entry->valid_mapping)) { > trace_xen_map_cache_return(mapcache->last_entry->vaddr_base + address_offset); > return mapcache->last_entry->vaddr_base + address_offset; > @@ -232,20 +232,20 @@ tryagain: > > /* size is always a multiple of MCACHE_BUCKET_SIZE */ > if (size) { > - __size = size + address_offset; > - if (__size % MCACHE_BUCKET_SIZE) { > - __size += MCACHE_BUCKET_SIZE - (__size % MCACHE_BUCKET_SIZE); > + cache_size = size + address_offset; > + if (cache_size % MCACHE_BUCKET_SIZE) { > + cache_size += MCACHE_BUCKET_SIZE - (cache_size % MCACHE_BUCKET_SIZE); > } > } else { > - __size = MCACHE_BUCKET_SIZE; > + cache_size = MCACHE_BUCKET_SIZE; > } > > entry = &mapcache->entry[address_index % mapcache->nr_buckets]; > > while (entry && entry->lock && entry->vaddr_base && > - (entry->paddr_index != address_index || entry->size != __size || > + (entry->paddr_index != address_index || entry->size != cache_size || > !test_bits(address_offset >> XC_PAGE_SHIFT, > - __test_bit_size >> XC_PAGE_SHIFT, > + test_bit_size >> XC_PAGE_SHIFT, > entry->valid_mapping))) { > pentry = entry; > entry = entry->next; > @@ -253,19 +253,19 @@ tryagain: > if (!entry) { > entry = g_malloc0(sizeof (MapCacheEntry)); > pentry->next = entry; > - xen_remap_bucket(entry, __size, address_index); > + xen_remap_bucket(entry, cache_size, address_index); > } else if (!entry->lock) { > if (!entry->vaddr_base || entry->paddr_index != address_index || > - entry->size != __size || > + entry->size != cache_size || > !test_bits(address_offset >> XC_PAGE_SHIFT, > - __test_bit_size >> XC_PAGE_SHIFT, > + test_bit_size >> XC_PAGE_SHIFT, > entry->valid_mapping)) { > - xen_remap_bucket(entry, __size, address_index); > + xen_remap_bucket(entry, cache_size, address_index); > } > } > > if(!test_bits(address_offset >> XC_PAGE_SHIFT, > - __test_bit_size >> XC_PAGE_SHIFT, > + test_bit_size >> XC_PAGE_SHIFT, > entry->valid_mapping)) { > mapcache->last_entry = NULL; > if (!translated && mapcache->phys_offset_to_gaddr) { > -- > 1.8.3.1 > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [RFH PATCH 2/2] xen: add a lock for the mapcache 2015-01-14 10:20 [Qemu-devel] [RFH PATCH 0/2] xen-mapcache: fixes for unlocked memory access Paolo Bonzini 2015-01-14 10:20 ` [Qemu-devel] [RFH PATCH 1/2] xen: do not use __-named variables in mapcache Paolo Bonzini @ 2015-01-14 10:20 ` Paolo Bonzini 2015-01-14 11:52 ` Stefano Stabellini 1 sibling, 1 reply; 5+ messages in thread From: Paolo Bonzini @ 2015-01-14 10:20 UTC (permalink / raw) To: qemu-devel; +Cc: stefano.stabellini Extend the existing dummy mapcache_lock/unlock macros to cover all of xen-mapcache.c. This prepares for unlocked memory access, when parts of exec.c will not be protected by the BQL. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- xen-mapcache.c | 54 +++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 43 insertions(+), 11 deletions(-) diff --git a/xen-mapcache.c b/xen-mapcache.c index 458069b..8cefd0c 100644 --- a/xen-mapcache.c +++ b/xen-mapcache.c @@ -49,9 +49,6 @@ */ #define NON_MCACHE_MEMORY_SIZE (80 * 1024 * 1024) -#define mapcache_lock() ((void)0) -#define mapcache_unlock() ((void)0) - typedef struct MapCacheEntry { hwaddr paddr_index; uint8_t *vaddr_base; @@ -79,11 +76,22 @@ typedef struct MapCache { unsigned int mcache_bucket_shift; phys_offset_to_gaddr_t phys_offset_to_gaddr; + QemuMutex lock; void *opaque; } MapCache; static MapCache *mapcache; +static inline void mapcache_lock(void) +{ + qemu_mutex_lock(&mapcache->lock); +} + +static inline void mapcache_unlock(void) +{ + qemu_mutex_unlock(&mapcache->lock); +} + static inline int test_bits(int nr, int size, const unsigned long *addr) { unsigned long res = find_next_zero_bit(addr, size + nr, nr); @@ -102,6 +110,7 @@ void xen_map_cache_init(phys_offset_to_gaddr_t f, void *opaque) mapcache->phys_offset_to_gaddr = f; mapcache->opaque = opaque; + qemu_mutex_init(&mapcache->lock); QTAILQ_INIT(&mapcache->locked_entries); @@ -193,8 +202,8 @@ static void xen_remap_bucket(MapCacheEntry *entry, g_free(err); } -uint8_t *xen_map_cache(hwaddr phys_addr, hwaddr size, - uint8_t lock) +static uint8_t *xen_map_cache_unlocked(hwaddr phys_addr, hwaddr size, + uint8_t lock) { MapCacheEntry *entry, *pentry = NULL; hwaddr address_index; @@ -291,14 +300,27 @@ tryagain: return mapcache->last_entry->vaddr_base + address_offset; } +uint8_t *xen_map_cache(hwaddr phys_addr, hwaddr size, + uint8_t lock) +{ + uint8_t *p; + + mapcache_lock(); + p = xen_map_cache_unlocked(phys_addr, size, lock); + mapcache_unlock(); + return p; +} + ram_addr_t xen_ram_addr_from_mapcache(void *ptr) { MapCacheEntry *entry = NULL; MapCacheRev *reventry; hwaddr paddr_index; hwaddr size; + ram_addr_t raddr; int found = 0; + mapcache_lock(); QTAILQ_FOREACH(reventry, &mapcache->locked_entries, next) { if (reventry->vaddr_req == ptr) { paddr_index = reventry->paddr_index; @@ -323,13 +345,16 @@ ram_addr_t xen_ram_addr_from_mapcache(void *ptr) } if (!entry) { DPRINTF("Trying to find address %p that is not in the mapcache!\n", ptr); - return 0; + raddr = 0; + } else { + raddr = (reventry->paddr_index << MCACHE_BUCKET_SHIFT) + + ((unsigned long) ptr - (unsigned long) entry->vaddr_base); } - return (reventry->paddr_index << MCACHE_BUCKET_SHIFT) + - ((unsigned long) ptr - (unsigned long) entry->vaddr_base); + mapcache_unlock(); + return raddr; } -void xen_invalidate_map_cache_entry(uint8_t *buffer) +static void xen_invalidate_map_cache_entry_unlocked(uint8_t *buffer) { MapCacheEntry *entry = NULL, *pentry = NULL; MapCacheRev *reventry; @@ -383,6 +408,13 @@ void xen_invalidate_map_cache_entry(uint8_t *buffer) g_free(entry); } +void xen_invalidate_map_cache_entry(uint8_t *buffer) +{ + mapcache_lock(); + xen_invalidate_map_cache_entry_unlocked(buffer); + mapcache_unlock(); +} + void xen_invalidate_map_cache(void) { unsigned long i; @@ -391,14 +423,14 @@ void xen_invalidate_map_cache(void) /* Flush pending AIO before destroying the mapcache */ bdrv_drain_all(); + mapcache_lock(); + QTAILQ_FOREACH(reventry, &mapcache->locked_entries, next) { DPRINTF("There should be no locked mappings at this time, " "but "TARGET_FMT_plx" -> %p is present\n", reventry->paddr_index, reventry->vaddr_req); } - mapcache_lock(); - for (i = 0; i < mapcache->nr_buckets; i++) { MapCacheEntry *entry = &mapcache->entry[i]; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [RFH PATCH 2/2] xen: add a lock for the mapcache 2015-01-14 10:20 ` [Qemu-devel] [RFH PATCH 2/2] xen: add a lock for the mapcache Paolo Bonzini @ 2015-01-14 11:52 ` Stefano Stabellini 0 siblings, 0 replies; 5+ messages in thread From: Stefano Stabellini @ 2015-01-14 11:52 UTC (permalink / raw) To: Paolo Bonzini; +Cc: qemu-devel, stefano.stabellini On Wed, 14 Jan 2015, Paolo Bonzini wrote: > Extend the existing dummy mapcache_lock/unlock macros to cover all of > xen-mapcache.c. This prepares for unlocked memory access, when parts > of exec.c will not be protected by the BQL. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > xen-mapcache.c | 54 +++++++++++++++++++++++++++++++++++++++++++----------- > 1 file changed, 43 insertions(+), 11 deletions(-) > > diff --git a/xen-mapcache.c b/xen-mapcache.c > index 458069b..8cefd0c 100644 > --- a/xen-mapcache.c > +++ b/xen-mapcache.c > @@ -49,9 +49,6 @@ > */ > #define NON_MCACHE_MEMORY_SIZE (80 * 1024 * 1024) > > -#define mapcache_lock() ((void)0) > -#define mapcache_unlock() ((void)0) > - > typedef struct MapCacheEntry { > hwaddr paddr_index; > uint8_t *vaddr_base; > @@ -79,11 +76,22 @@ typedef struct MapCache { > unsigned int mcache_bucket_shift; > > phys_offset_to_gaddr_t phys_offset_to_gaddr; > + QemuMutex lock; > void *opaque; > } MapCache; > > static MapCache *mapcache; > > +static inline void mapcache_lock(void) > +{ > + qemu_mutex_lock(&mapcache->lock); > +} > + > +static inline void mapcache_unlock(void) > +{ > + qemu_mutex_unlock(&mapcache->lock); > +} > + > static inline int test_bits(int nr, int size, const unsigned long *addr) > { > unsigned long res = find_next_zero_bit(addr, size + nr, nr); > @@ -102,6 +110,7 @@ void xen_map_cache_init(phys_offset_to_gaddr_t f, void *opaque) > > mapcache->phys_offset_to_gaddr = f; > mapcache->opaque = opaque; > + qemu_mutex_init(&mapcache->lock); > > QTAILQ_INIT(&mapcache->locked_entries); > > @@ -193,8 +202,8 @@ static void xen_remap_bucket(MapCacheEntry *entry, > g_free(err); > } > > -uint8_t *xen_map_cache(hwaddr phys_addr, hwaddr size, > - uint8_t lock) > +static uint8_t *xen_map_cache_unlocked(hwaddr phys_addr, hwaddr size, > + uint8_t lock) > { > MapCacheEntry *entry, *pentry = NULL; > hwaddr address_index; > @@ -291,14 +300,27 @@ tryagain: > return mapcache->last_entry->vaddr_base + address_offset; > } > > +uint8_t *xen_map_cache(hwaddr phys_addr, hwaddr size, > + uint8_t lock) > +{ > + uint8_t *p; > + > + mapcache_lock(); > + p = xen_map_cache_unlocked(phys_addr, size, lock); > + mapcache_unlock(); > + return p; > +} > + > ram_addr_t xen_ram_addr_from_mapcache(void *ptr) > { > MapCacheEntry *entry = NULL; > MapCacheRev *reventry; > hwaddr paddr_index; > hwaddr size; > + ram_addr_t raddr; > int found = 0; > > + mapcache_lock(); > QTAILQ_FOREACH(reventry, &mapcache->locked_entries, next) { > if (reventry->vaddr_req == ptr) { > paddr_index = reventry->paddr_index; > @@ -323,13 +345,16 @@ ram_addr_t xen_ram_addr_from_mapcache(void *ptr) > } > if (!entry) { > DPRINTF("Trying to find address %p that is not in the mapcache!\n", ptr); > - return 0; > + raddr = 0; > + } else { > + raddr = (reventry->paddr_index << MCACHE_BUCKET_SHIFT) + > + ((unsigned long) ptr - (unsigned long) entry->vaddr_base); > } > - return (reventry->paddr_index << MCACHE_BUCKET_SHIFT) + > - ((unsigned long) ptr - (unsigned long) entry->vaddr_base); > + mapcache_unlock(); > + return raddr; > } > > -void xen_invalidate_map_cache_entry(uint8_t *buffer) > +static void xen_invalidate_map_cache_entry_unlocked(uint8_t *buffer) > { > MapCacheEntry *entry = NULL, *pentry = NULL; > MapCacheRev *reventry; > @@ -383,6 +408,13 @@ void xen_invalidate_map_cache_entry(uint8_t *buffer) > g_free(entry); > } > > +void xen_invalidate_map_cache_entry(uint8_t *buffer) > +{ > + mapcache_lock(); > + xen_invalidate_map_cache_entry_unlocked(buffer); > + mapcache_unlock(); > +} > + > void xen_invalidate_map_cache(void) > { > unsigned long i; > @@ -391,14 +423,14 @@ void xen_invalidate_map_cache(void) > /* Flush pending AIO before destroying the mapcache */ > bdrv_drain_all(); > > + mapcache_lock(); > + > QTAILQ_FOREACH(reventry, &mapcache->locked_entries, next) { > DPRINTF("There should be no locked mappings at this time, " > "but "TARGET_FMT_plx" -> %p is present\n", > reventry->paddr_index, reventry->vaddr_req); > } > > - mapcache_lock(); > - > for (i = 0; i < mapcache->nr_buckets; i++) { > MapCacheEntry *entry = &mapcache->entry[i]; > > -- > 1.8.3.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-01-14 11:52 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-01-14 10:20 [Qemu-devel] [RFH PATCH 0/2] xen-mapcache: fixes for unlocked memory access Paolo Bonzini 2015-01-14 10:20 ` [Qemu-devel] [RFH PATCH 1/2] xen: do not use __-named variables in mapcache Paolo Bonzini 2015-01-14 11:44 ` Stefano Stabellini 2015-01-14 10:20 ` [Qemu-devel] [RFH PATCH 2/2] xen: add a lock for the mapcache Paolo Bonzini 2015-01-14 11:52 ` Stefano Stabellini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).