* [Qemu-devel] [PATCH qemu v5 01/18] exec: Explicitly export target AS from address_space_translate_internal
2017-09-21 8:50 [Qemu-devel] [PATCH qemu v5 00/18] memory: Store physical root MR in FlatView Alexey Kardashevskiy
@ 2017-09-21 8:50 ` Alexey Kardashevskiy
2017-09-21 8:50 ` [Qemu-devel] [PATCH qemu v5 02/18] memory: Open code FlatView rendering Alexey Kardashevskiy
` (17 subsequent siblings)
18 siblings, 0 replies; 26+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-21 8:50 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini
This adds an AS** parameter to address_space_do_translate()
to make it easier for the next patch to share FlatViews.
This should cause no behavioural change.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
exec.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/exec.c b/exec.c
index a25a4c6018..fd8994b25d 100644
--- a/exec.c
+++ b/exec.c
@@ -476,7 +476,8 @@ static MemoryRegionSection address_space_do_translate(AddressSpace *as,
hwaddr *xlat,
hwaddr *plen,
bool is_write,
- bool is_mmio)
+ bool is_mmio,
+ AddressSpace **target_as)
{
IOMMUTLBEntry iotlb;
MemoryRegionSection *section;
@@ -503,6 +504,7 @@ static MemoryRegionSection address_space_do_translate(AddressSpace *as,
}
as = iotlb.target_as;
+ *target_as = iotlb.target_as;
}
*xlat = addr;
@@ -525,7 +527,7 @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr,
/* This can never be MMIO. */
section = address_space_do_translate(as, addr, &xlat, &plen,
- is_write, false);
+ is_write, false, &as);
/* Illegal translation */
if (section.mr == &io_mem_unassigned) {
@@ -548,7 +550,7 @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr,
plen -= 1;
return (IOMMUTLBEntry) {
- .target_as = section.address_space,
+ .target_as = as,
.iova = addr & ~plen,
.translated_addr = xlat & ~plen,
.addr_mask = plen,
@@ -569,7 +571,8 @@ MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr,
MemoryRegionSection section;
/* This can be MMIO, so setup MMIO bit. */
- section = address_space_do_translate(as, addr, xlat, plen, is_write, true);
+ section = address_space_do_translate(as, addr, xlat, plen, is_write, true,
+ &as);
mr = section.mr;
if (xen_enabled() && memory_access_is_direct(mr, is_write)) {
--
2.11.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH qemu v5 02/18] memory: Open code FlatView rendering
2017-09-21 8:50 [Qemu-devel] [PATCH qemu v5 00/18] memory: Store physical root MR in FlatView Alexey Kardashevskiy
2017-09-21 8:50 ` [Qemu-devel] [PATCH qemu v5 01/18] exec: Explicitly export target AS from address_space_translate_internal Alexey Kardashevskiy
@ 2017-09-21 8:50 ` Alexey Kardashevskiy
2017-09-21 8:50 ` [Qemu-devel] [PATCH qemu v5 03/18] memory: Move FlatView allocation to a helper Alexey Kardashevskiy
` (16 subsequent siblings)
18 siblings, 0 replies; 26+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-21 8:50 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini
We are going to share FlatView's between AddressSpace's and per-AS
memory listeners won't suit the purpose anymore so open code
the dispatch tree rendering.
Since there is a good chance that dispatch_listener was the only
listener, this avoids address_space_update_topology_pass() if there is
no registered listeners; this should improve starting time.
This should cause no behavioural change.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v4:
* address_space_update_topology_pass() is called now under
if (!QTAILQ_EMPTY(&as->listeners))
v3:
* inlined & simplified address_space_update_flatview
---
include/exec/memory-internal.h | 6 ++++--
include/exec/memory.h | 1 -
exec.c | 27 +++------------------------
memory.c | 19 ++++++++++++++-----
4 files changed, 21 insertions(+), 32 deletions(-)
diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index fb467acdba..9abde2f11c 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -22,8 +22,6 @@
#ifndef CONFIG_USER_ONLY
typedef struct AddressSpaceDispatch AddressSpaceDispatch;
-void address_space_init_dispatch(AddressSpace *as);
-void address_space_unregister(AddressSpace *as);
void address_space_destroy_dispatch(AddressSpace *as);
extern const MemoryRegionOps unassigned_mem_ops;
@@ -31,5 +29,9 @@ extern const MemoryRegionOps unassigned_mem_ops;
bool memory_region_access_valid(MemoryRegion *mr, hwaddr addr,
unsigned size, bool is_write);
+void mem_add(AddressSpace *as, MemoryRegionSection *section);
+void mem_begin(AddressSpace *as);
+void mem_commit(AddressSpace *as);
+
#endif
#endif
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 1dcd3122d7..9581f7a7db 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -318,7 +318,6 @@ struct AddressSpace {
struct MemoryRegionIoeventfd *ioeventfds;
struct AddressSpaceDispatch *dispatch;
struct AddressSpaceDispatch *next_dispatch;
- MemoryListener dispatch_listener;
QTAILQ_HEAD(memory_listeners_as, MemoryListener) listeners;
QTAILQ_ENTRY(AddressSpace) address_spaces_link;
};
diff --git a/exec.c b/exec.c
index fd8994b25d..1626d254bb 100644
--- a/exec.c
+++ b/exec.c
@@ -1347,9 +1347,8 @@ static void register_multipage(AddressSpaceDispatch *d,
phys_page_set(d, start_addr >> TARGET_PAGE_BITS, num_pages, section_index);
}
-static void mem_add(MemoryListener *listener, MemoryRegionSection *section)
+void mem_add(AddressSpace *as, MemoryRegionSection *section)
{
- AddressSpace *as = container_of(listener, AddressSpace, dispatch_listener);
AddressSpaceDispatch *d = as->next_dispatch;
MemoryRegionSection now = *section, remain = *section;
Int128 page_size = int128_make64(TARGET_PAGE_SIZE);
@@ -2673,9 +2672,8 @@ static void io_mem_init(void)
NULL, UINT64_MAX);
}
-static void mem_begin(MemoryListener *listener)
+void mem_begin(AddressSpace *as)
{
- AddressSpace *as = container_of(listener, AddressSpace, dispatch_listener);
AddressSpaceDispatch *d = g_new0(AddressSpaceDispatch, 1);
uint16_t n;
@@ -2699,9 +2697,8 @@ static void address_space_dispatch_free(AddressSpaceDispatch *d)
g_free(d);
}
-static void mem_commit(MemoryListener *listener)
+void mem_commit(AddressSpace *as)
{
- AddressSpace *as = container_of(listener, AddressSpace, dispatch_listener);
AddressSpaceDispatch *cur = as->dispatch;
AddressSpaceDispatch *next = as->next_dispatch;
@@ -2731,24 +2728,6 @@ static void tcg_commit(MemoryListener *listener)
tlb_flush(cpuas->cpu);
}
-void address_space_init_dispatch(AddressSpace *as)
-{
- as->dispatch = NULL;
- as->dispatch_listener = (MemoryListener) {
- .begin = mem_begin,
- .commit = mem_commit,
- .region_add = mem_add,
- .region_nop = mem_add,
- .priority = 0,
- };
- memory_listener_register(&as->dispatch_listener, as);
-}
-
-void address_space_unregister(AddressSpace *as)
-{
- memory_listener_unregister(&as->dispatch_listener);
-}
-
void address_space_destroy_dispatch(AddressSpace *as)
{
AddressSpaceDispatch *d = as->dispatch;
diff --git a/memory.c b/memory.c
index b9920a6540..101d5f4d60 100644
--- a/memory.c
+++ b/memory.c
@@ -879,14 +879,24 @@ static void address_space_update_topology_pass(AddressSpace *as,
}
}
-
static void address_space_update_topology(AddressSpace *as)
{
FlatView *old_view = address_space_get_flatview(as);
FlatView *new_view = generate_memory_topology(as->root);
+ int i;
- address_space_update_topology_pass(as, old_view, new_view, false);
- address_space_update_topology_pass(as, old_view, new_view, true);
+ mem_begin(as);
+ for (i = 0; i < new_view->nr; i++) {
+ MemoryRegionSection mrs =
+ section_from_flat_range(&new_view->ranges[i], as);
+ mem_add(as, &mrs);
+ }
+ mem_commit(as);
+
+ if (!QTAILQ_EMPTY(&as->listeners)) {
+ address_space_update_topology_pass(as, old_view, new_view, false);
+ address_space_update_topology_pass(as, old_view, new_view, true);
+ }
/* Writes are protected by the BQL. */
atomic_rcu_set(&as->current_map, new_view);
@@ -2621,7 +2631,7 @@ void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
QTAILQ_INIT(&as->listeners);
QTAILQ_INSERT_TAIL(&address_spaces, as, address_spaces_link);
as->name = g_strdup(name ? name : "anonymous");
- address_space_init_dispatch(as);
+ as->dispatch = NULL;
memory_region_update_pending |= root->enabled;
memory_region_transaction_commit();
}
@@ -2672,7 +2682,6 @@ void address_space_destroy(AddressSpace *as)
as->root = NULL;
memory_region_transaction_commit();
QTAILQ_REMOVE(&address_spaces, as, address_spaces_link);
- address_space_unregister(as);
/* At this point, as->dispatch and as->current_map are dummy
* entries that the guest should never use. Wait for the old
--
2.11.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH qemu v5 03/18] memory: Move FlatView allocation to a helper
2017-09-21 8:50 [Qemu-devel] [PATCH qemu v5 00/18] memory: Store physical root MR in FlatView Alexey Kardashevskiy
2017-09-21 8:50 ` [Qemu-devel] [PATCH qemu v5 01/18] exec: Explicitly export target AS from address_space_translate_internal Alexey Kardashevskiy
2017-09-21 8:50 ` [Qemu-devel] [PATCH qemu v5 02/18] memory: Open code FlatView rendering Alexey Kardashevskiy
@ 2017-09-21 8:50 ` Alexey Kardashevskiy
2017-09-21 8:50 ` [Qemu-devel] [PATCH qemu v5 04/18] memory: Move AddressSpaceDispatch from AddressSpace to FlatView Alexey Kardashevskiy
` (15 subsequent siblings)
18 siblings, 0 replies; 26+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-21 8:50 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini
This moves a FlatView allocation and initialization to a helper.
While we are nere, replace g_new with g_new0 to not to bother if we add
new fields in the future.
This should cause no behavioural change.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v3:
* s/flatview_alloc/flatview_new/
---
memory.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/memory.c b/memory.c
index 101d5f4d60..9babd7d423 100644
--- a/memory.c
+++ b/memory.c
@@ -258,12 +258,14 @@ static bool flatrange_equal(FlatRange *a, FlatRange *b)
&& a->readonly == b->readonly;
}
-static void flatview_init(FlatView *view)
+static FlatView *flatview_new(void)
{
+ FlatView *view;
+
+ view = g_new0(FlatView, 1);
view->ref = 1;
- view->ranges = NULL;
- view->nr = 0;
- view->nr_allocated = 0;
+
+ return view;
}
/* Insert a range into a given position. Caller is responsible for maintaining
@@ -706,8 +708,7 @@ static FlatView *generate_memory_topology(MemoryRegion *mr)
{
FlatView *view;
- view = g_new(FlatView, 1);
- flatview_init(view);
+ view = flatview_new();
if (mr) {
render_memory_region(view, mr, int128_zero(),
@@ -2624,8 +2625,7 @@ void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
as->ref_count = 1;
as->root = root;
as->malloced = false;
- as->current_map = g_new(FlatView, 1);
- flatview_init(as->current_map);
+ as->current_map = flatview_new();
as->ioeventfd_nb = 0;
as->ioeventfds = NULL;
QTAILQ_INIT(&as->listeners);
--
2.11.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH qemu v5 04/18] memory: Move AddressSpaceDispatch from AddressSpace to FlatView
2017-09-21 8:50 [Qemu-devel] [PATCH qemu v5 00/18] memory: Store physical root MR in FlatView Alexey Kardashevskiy
` (2 preceding siblings ...)
2017-09-21 8:50 ` [Qemu-devel] [PATCH qemu v5 03/18] memory: Move FlatView allocation to a helper Alexey Kardashevskiy
@ 2017-09-21 8:50 ` Alexey Kardashevskiy
2017-09-21 11:51 ` Paolo Bonzini
2017-09-21 8:50 ` [Qemu-devel] [PATCH qemu v5 05/18] memory: Remove AddressSpace pointer from AddressSpaceDispatch Alexey Kardashevskiy
` (14 subsequent siblings)
18 siblings, 1 reply; 26+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-21 8:50 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini
As we are going to share FlatView's between AddressSpace's,
and AddressSpaceDispatch is a structure to perform quick lookup
in FlatView, this moves ASD to FlatView.
After previosly open coded ASD rendering, we can also remove
as->next_dispatch as the new FlatView pointer is stored
on a stack and set to an AS atomically.
flatview_destroy() is executed under RCU instead of
address_space_dispatch_free() now.
This makes mem_begin/mem_commit to work with ASD and mem_add with FV
as later on mem_add will be taking FV as an argument anyway.
This should cause no behavioural change.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v4:
* since FlatView::rcu is used now to dispose FV, call_rcu() in
address_space_update_topology() is replaced with direct call to
flatview_unref()
---
include/exec/memory-internal.h | 12 +++++++-----
include/exec/memory.h | 2 --
exec.c | 41 +++++++++++------------------------------
memory.c | 31 ++++++++++++++++++++++++-------
4 files changed, 42 insertions(+), 44 deletions(-)
diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index 9abde2f11c..6e08eda256 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -22,16 +22,18 @@
#ifndef CONFIG_USER_ONLY
typedef struct AddressSpaceDispatch AddressSpaceDispatch;
-void address_space_destroy_dispatch(AddressSpace *as);
-
extern const MemoryRegionOps unassigned_mem_ops;
bool memory_region_access_valid(MemoryRegion *mr, hwaddr addr,
unsigned size, bool is_write);
-void mem_add(AddressSpace *as, MemoryRegionSection *section);
-void mem_begin(AddressSpace *as);
-void mem_commit(AddressSpace *as);
+void mem_add(AddressSpace *as, FlatView *fv, MemoryRegionSection *section);
+AddressSpaceDispatch *mem_begin(AddressSpace *as);
+void mem_commit(AddressSpaceDispatch *d);
+
+AddressSpaceDispatch *address_space_to_dispatch(AddressSpace *as);
+AddressSpaceDispatch *flatview_to_dispatch(FlatView *fv);
+void address_space_dispatch_free(AddressSpaceDispatch *d);
#endif
#endif
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 9581f7a7db..2346f8b863 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -316,8 +316,6 @@ struct AddressSpace {
int ioeventfd_nb;
struct MemoryRegionIoeventfd *ioeventfds;
- struct AddressSpaceDispatch *dispatch;
- struct AddressSpaceDispatch *next_dispatch;
QTAILQ_HEAD(memory_listeners_as, MemoryListener) listeners;
QTAILQ_ENTRY(AddressSpace) address_spaces_link;
};
diff --git a/exec.c b/exec.c
index 1626d254bb..afd64127e6 100644
--- a/exec.c
+++ b/exec.c
@@ -187,8 +187,6 @@ typedef struct PhysPageMap {
} PhysPageMap;
struct AddressSpaceDispatch {
- struct rcu_head rcu;
-
MemoryRegionSection *mru_section;
/* This is a multi-level map on the physical address space.
* The bottom level has pointers to MemoryRegionSections.
@@ -485,7 +483,7 @@ static MemoryRegionSection address_space_do_translate(AddressSpace *as,
IOMMUMemoryRegionClass *imrc;
for (;;) {
- AddressSpaceDispatch *d = atomic_rcu_read(&as->dispatch);
+ AddressSpaceDispatch *d = address_space_to_dispatch(as);
section = address_space_translate_internal(d, addr, &addr, plen, is_mmio);
iommu_mr = memory_region_get_iommu(section->mr);
@@ -1222,7 +1220,7 @@ hwaddr memory_region_section_get_iotlb(CPUState *cpu,
} else {
AddressSpaceDispatch *d;
- d = atomic_rcu_read(§ion->address_space->dispatch);
+ d = address_space_to_dispatch(section->address_space);
iotlb = section - d->map.sections;
iotlb += xlat;
}
@@ -1347,9 +1345,9 @@ static void register_multipage(AddressSpaceDispatch *d,
phys_page_set(d, start_addr >> TARGET_PAGE_BITS, num_pages, section_index);
}
-void mem_add(AddressSpace *as, MemoryRegionSection *section)
+void mem_add(AddressSpace *as, FlatView *fv, MemoryRegionSection *section)
{
- AddressSpaceDispatch *d = as->next_dispatch;
+ AddressSpaceDispatch *d = flatview_to_dispatch(fv);
MemoryRegionSection now = *section, remain = *section;
Int128 page_size = int128_make64(TARGET_PAGE_SIZE);
@@ -2672,7 +2670,7 @@ static void io_mem_init(void)
NULL, UINT64_MAX);
}
-void mem_begin(AddressSpace *as)
+AddressSpaceDispatch *mem_begin(AddressSpace *as)
{
AddressSpaceDispatch *d = g_new0(AddressSpaceDispatch, 1);
uint16_t n;
@@ -2688,26 +2686,19 @@ void mem_begin(AddressSpace *as)
d->phys_map = (PhysPageEntry) { .ptr = PHYS_MAP_NODE_NIL, .skip = 1 };
d->as = as;
- as->next_dispatch = d;
+
+ return d;
}
-static void address_space_dispatch_free(AddressSpaceDispatch *d)
+void address_space_dispatch_free(AddressSpaceDispatch *d)
{
phys_sections_free(&d->map);
g_free(d);
}
-void mem_commit(AddressSpace *as)
+void mem_commit(AddressSpaceDispatch *d)
{
- AddressSpaceDispatch *cur = as->dispatch;
- AddressSpaceDispatch *next = as->next_dispatch;
-
- phys_page_compact_all(next, next->map.nodes_nb);
-
- atomic_rcu_set(&as->dispatch, next);
- if (cur) {
- call_rcu(cur, address_space_dispatch_free, rcu);
- }
+ phys_page_compact_all(d, d->map.nodes_nb);
}
static void tcg_commit(MemoryListener *listener)
@@ -2723,21 +2714,11 @@ static void tcg_commit(MemoryListener *listener)
* We reload the dispatch pointer now because cpu_reloading_memory_map()
* may have split the RCU critical section.
*/
- d = atomic_rcu_read(&cpuas->as->dispatch);
+ d = address_space_to_dispatch(cpuas->as);
atomic_rcu_set(&cpuas->memory_dispatch, d);
tlb_flush(cpuas->cpu);
}
-void address_space_destroy_dispatch(AddressSpace *as)
-{
- AddressSpaceDispatch *d = as->dispatch;
-
- atomic_rcu_set(&as->dispatch, NULL);
- if (d) {
- call_rcu(d, address_space_dispatch_free, rcu);
- }
-}
-
static void memory_map_init(void)
{
system_memory = g_malloc(sizeof(*system_memory));
diff --git a/memory.c b/memory.c
index 9babd7d423..27d7aeffc2 100644
--- a/memory.c
+++ b/memory.c
@@ -229,6 +229,7 @@ struct FlatView {
FlatRange *ranges;
unsigned nr;
unsigned nr_allocated;
+ struct AddressSpaceDispatch *dispatch;
};
typedef struct AddressSpaceOps AddressSpaceOps;
@@ -289,6 +290,9 @@ static void flatview_destroy(FlatView *view)
{
int i;
+ if (view->dispatch) {
+ address_space_dispatch_free(view->dispatch);
+ }
for (i = 0; i < view->nr; i++) {
memory_region_unref(view->ranges[i].mr);
}
@@ -304,10 +308,25 @@ static void flatview_ref(FlatView *view)
static void flatview_unref(FlatView *view)
{
if (atomic_fetch_dec(&view->ref) == 1) {
- flatview_destroy(view);
+ call_rcu(view, flatview_destroy, rcu);
}
}
+static FlatView *address_space_to_flatview(AddressSpace *as)
+{
+ return atomic_rcu_read(&as->current_map);
+}
+
+AddressSpaceDispatch *flatview_to_dispatch(FlatView *fv)
+{
+ return fv->dispatch;
+}
+
+AddressSpaceDispatch *address_space_to_dispatch(AddressSpace *as)
+{
+ return flatview_to_dispatch(address_space_to_flatview(as));
+}
+
static bool can_merge(FlatRange *r1, FlatRange *r2)
{
return int128_eq(addrrange_end(r1->addr), r2->addr.start)
@@ -886,13 +905,13 @@ static void address_space_update_topology(AddressSpace *as)
FlatView *new_view = generate_memory_topology(as->root);
int i;
- mem_begin(as);
+ new_view->dispatch = mem_begin(as);
for (i = 0; i < new_view->nr; i++) {
MemoryRegionSection mrs =
section_from_flat_range(&new_view->ranges[i], as);
- mem_add(as, &mrs);
+ mem_add(as, new_view, &mrs);
}
- mem_commit(as);
+ mem_commit(new_view->dispatch);
if (!QTAILQ_EMPTY(&as->listeners)) {
address_space_update_topology_pass(as, old_view, new_view, false);
@@ -901,7 +920,7 @@ static void address_space_update_topology(AddressSpace *as)
/* Writes are protected by the BQL. */
atomic_rcu_set(&as->current_map, new_view);
- call_rcu(old_view, flatview_unref, rcu);
+ flatview_unref(old_view);
/* Note that all the old MemoryRegions are still alive up to this
* point. This relieves most MemoryListeners from the need to
@@ -2631,7 +2650,6 @@ void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
QTAILQ_INIT(&as->listeners);
QTAILQ_INSERT_TAIL(&address_spaces, as, address_spaces_link);
as->name = g_strdup(name ? name : "anonymous");
- as->dispatch = NULL;
memory_region_update_pending |= root->enabled;
memory_region_transaction_commit();
}
@@ -2640,7 +2658,6 @@ static void do_address_space_destroy(AddressSpace *as)
{
bool do_free = as->malloced;
- address_space_destroy_dispatch(as);
assert(QTAILQ_EMPTY(&as->listeners));
flatview_unref(as->current_map);
--
2.11.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [Qemu-devel] [PATCH qemu v5 04/18] memory: Move AddressSpaceDispatch from AddressSpace to FlatView
2017-09-21 8:50 ` [Qemu-devel] [PATCH qemu v5 04/18] memory: Move AddressSpaceDispatch from AddressSpace to FlatView Alexey Kardashevskiy
@ 2017-09-21 11:51 ` Paolo Bonzini
2017-09-21 13:44 ` Alexey Kardashevskiy
0 siblings, 1 reply; 26+ messages in thread
From: Paolo Bonzini @ 2017-09-21 11:51 UTC (permalink / raw)
To: Alexey Kardashevskiy, qemu-devel
On 21/09/2017 10:50, Alexey Kardashevskiy wrote:
> * since FlatView::rcu is used now to dispose FV, call_rcu() in
> address_space_update_topology() is replaced with direct call to
> flatview_unref()
Hmm, this is not correct, as you could have
thread 1 thread 2 RCU thread
-------------------------------------------------------------
rcu_read_lock
read as->current_map
set as->current_map
flatview_unref
'--> call_rcu
flatview_ref
rcu_read_unlock
flatview_destroy
I need to think a bit more about this (and possibly ask Paul...).
Paolo
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Qemu-devel] [PATCH qemu v5 04/18] memory: Move AddressSpaceDispatch from AddressSpace to FlatView
2017-09-21 11:51 ` Paolo Bonzini
@ 2017-09-21 13:44 ` Alexey Kardashevskiy
2017-09-21 13:54 ` Paolo Bonzini
0 siblings, 1 reply; 26+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-21 13:44 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel
On 21/09/17 21:51, Paolo Bonzini wrote:
> On 21/09/2017 10:50, Alexey Kardashevskiy wrote:
>> * since FlatView::rcu is used now to dispose FV, call_rcu() in
>> address_space_update_topology() is replaced with direct call to
>> flatview_unref()
>
> Hmm, this is not correct, as you could have
>
>
> thread 1 thread 2 RCU thread
> -------------------------------------------------------------
> rcu_read_lock
> read as->current_map
> set as->current_map
> flatview_unref
> '--> call_rcu
> flatview_ref
> rcu_read_unlock
> flatview_destroy
>
> I need to think a bit more about this (and possibly ask Paul...).
>
> Paolo
>
Nah, you're right, it should be like this:
diff --git a/memory.c b/memory.c
index 35b2fc5f7f..689bf53866 100644
--- a/memory.c
+++ b/memory.c
@@ -317,7 +317,7 @@ static void flatview_ref(FlatView *view)
static void flatview_unref(FlatView *view)
{
if (atomic_fetch_dec(&view->ref) == 1) {
- call_rcu(view, flatview_destroy, rcu);
+ flatview_destroy(view);
}
}
@@ -768,7 +768,7 @@ static FlatView *generate_memory_topology(MemoryRegion *mr)
flatview_simplify(view);
if (!view->nr) {
- flatview_destroy(view);
+ flatview_unref(view);
use_empty = true;
}
}
@@ -1026,7 +1026,7 @@ static void address_space_set_flatview(AddressSpace *as)
/* Writes are protected by the BQL. */
atomic_rcu_set(&as->current_map, new_view);
if (old_view) {
- flatview_unref(old_view);
+ call_rcu(view, flatview_unref, rcu);
}
--
Alexey
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [Qemu-devel] [PATCH qemu v5 04/18] memory: Move AddressSpaceDispatch from AddressSpace to FlatView
2017-09-21 13:44 ` Alexey Kardashevskiy
@ 2017-09-21 13:54 ` Paolo Bonzini
0 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2017-09-21 13:54 UTC (permalink / raw)
To: Alexey Kardashevskiy, qemu-devel
On 21/09/2017 15:44, Alexey Kardashevskiy wrote:
> On 21/09/17 21:51, Paolo Bonzini wrote:
>> On 21/09/2017 10:50, Alexey Kardashevskiy wrote:
>>> * since FlatView::rcu is used now to dispose FV, call_rcu() in
>>> address_space_update_topology() is replaced with direct call to
>>> flatview_unref()
>>
>> Hmm, this is not correct, as you could have
>>
>>
>> thread 1 thread 2 RCU thread
>> -------------------------------------------------------------
>> rcu_read_lock
>> read as->current_map
>> set as->current_map
>> flatview_unref
>> '--> call_rcu
>> flatview_ref
>> rcu_read_unlock
>> flatview_destroy
>>
>> I need to think a bit more about this (and possibly ask Paul...).
>>
>> Paolo
>>
>
> Nah, you're right, it should be like this:
>
>
> diff --git a/memory.c b/memory.c
> index 35b2fc5f7f..689bf53866 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -317,7 +317,7 @@ static void flatview_ref(FlatView *view)
> static void flatview_unref(FlatView *view)
> {
> if (atomic_fetch_dec(&view->ref) == 1) {
> - call_rcu(view, flatview_destroy, rcu);
> + flatview_destroy(view);
> }
> }
>
> @@ -768,7 +768,7 @@ static FlatView *generate_memory_topology(MemoryRegion *mr)
> flatview_simplify(view);
>
> if (!view->nr) {
> - flatview_destroy(view);
> + flatview_unref(view);
> use_empty = true;
> }
> }
> @@ -1026,7 +1026,7 @@ static void address_space_set_flatview(AddressSpace *as)
> /* Writes are protected by the BQL. */
> atomic_rcu_set(&as->current_map, new_view);
> if (old_view) {
> - flatview_unref(old_view);
> + call_rcu(view, flatview_unref, rcu);
> }
This still doesn't cover address_space_get_flatview, i.e. it is a
pre-existing bug.
I found a similar case in Linux, here is how they solved it:
commit 358136532dd29e9ed96e0e523d2d510e71bda003
Author: Paolo Bonzini <pbonzini@redhat.com>
Date: Thu Sep 21 14:32:47 2017 +0200
memory: avoid "resurrection" of dead FlatViews
It's possible for address_space_get_flatview() as it currently stands
to cause a use-after-free for the returned FlatView, if the reference
count is incremented after the FlatView has been replaced by a writer:
thread 1 thread 2 RCU thread
-------------------------------------------------------------
rcu_read_lock
read as->current_map
set as->current_map
flatview_unref
'--> call_rcu
flatview_ref
[ref=1]
rcu_read_unlock
flatview_destroy
<badness>
Since FlatViews are not updated very often, we can just detect the
situation using a new atomic op atomic_fetch_inc_nonzero, similar to
Linux's atomic_inc_not_zero, which performs the refcount increment only if
it hasn't already hit zero. This is similar to Linux commit de09a9771a53
("CRED: Fix get_task_cred() and task_state() to not resurrect dead
credentials", 2010-07-29).
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
diff --git a/docs/devel/atomics.txt b/docs/devel/atomics.txt
index 048e5f23cb..10c5fa37e8 100644
--- a/docs/devel/atomics.txt
+++ b/docs/devel/atomics.txt
@@ -64,6 +64,7 @@ operations:
typeof(*ptr) atomic_fetch_and(ptr, val)
typeof(*ptr) atomic_fetch_or(ptr, val)
typeof(*ptr) atomic_fetch_xor(ptr, val)
+ typeof(*ptr) atomic_fetch_inc_nonzero(ptr)
typeof(*ptr) atomic_xchg(ptr, val)
typeof(*ptr) atomic_cmpxchg(ptr, old, new)
diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index b6b62fb771..44ad1e6c32 100644
--- a/include/qemu/atomic.h
+++ b/include/qemu/atomic.h
@@ -197,6 +197,15 @@
atomic_cmpxchg__nocheck(ptr, old, new); \
})
+#define atomic_fetch_inc_nonzero(ptr) ({ \
+ QEMU_BUILD_BUG_ON(sizeof(*ptr) > ATOMIC_REG_SIZE); \
+ typeof_strip_qual(*ptr) _oldn = atomic_read(ptr); \
+ while (_oldn && atomic_cmpxchg(ptr, _oldn, _oldn + 1) != _oldn) { \
+ _oldn = atomic_read(ptr); \
+ } \
+ _oldn; \
+})
+
/* Provide shorter names for GCC atomic builtins, return old value */
#define atomic_fetch_inc(ptr) __atomic_fetch_add(ptr, 1, __ATOMIC_SEQ_CST)
#define atomic_fetch_dec(ptr) __atomic_fetch_sub(ptr, 1, __ATOMIC_SEQ_CST)
diff --git a/memory.c b/memory.c
index 2b90117c60..51f54ab430 100644
--- a/memory.c
+++ b/memory.c
@@ -294,9 +294,9 @@ static void flatview_destroy(FlatView *view)
g_free(view);
}
-static void flatview_ref(FlatView *view)
+static bool flatview_ref(FlatView *view)
{
- atomic_inc(&view->ref);
+ return atomic_fetch_inc_nonzero(&view->ref) > 0;
}
static void flatview_unref(FlatView *view)
@@ -773,8 +773,12 @@ static FlatView *address_space_get_flatview(AddressSpace *as)
FlatView *view;
rcu_read_lock();
- view = atomic_rcu_read(&as->current_map);
- flatview_ref(view);
+ do {
+ view = atomic_rcu_read(&as->current_map);
+ /* If somebody has replaced as->current_map concurrently,
+ * flatview_ref returns false.
+ */
+ } while (!flatview_ref(view));
rcu_read_unlock();
return view;
}
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH qemu v5 05/18] memory: Remove AddressSpace pointer from AddressSpaceDispatch
2017-09-21 8:50 [Qemu-devel] [PATCH qemu v5 00/18] memory: Store physical root MR in FlatView Alexey Kardashevskiy
` (3 preceding siblings ...)
2017-09-21 8:50 ` [Qemu-devel] [PATCH qemu v5 04/18] memory: Move AddressSpaceDispatch from AddressSpace to FlatView Alexey Kardashevskiy
@ 2017-09-21 8:50 ` Alexey Kardashevskiy
2017-09-21 8:50 ` [Qemu-devel] [PATCH qemu v5 06/18] memory: Switch memory from using AddressSpace to FlatView Alexey Kardashevskiy
` (13 subsequent siblings)
18 siblings, 0 replies; 26+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-21 8:50 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini
AS in ASD is only used to pass AS from mem_begin() to register_subpage()
to store it in MemoryRegionSection, we can do this directly now.
This should cause no behavioural change.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
exec.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/exec.c b/exec.c
index afd64127e6..a54dde7835 100644
--- a/exec.c
+++ b/exec.c
@@ -193,7 +193,6 @@ struct AddressSpaceDispatch {
*/
PhysPageEntry phys_map;
PhysPageMap map;
- AddressSpace *as;
};
#define SUBPAGE_IDX(addr) ((addr) & ~TARGET_PAGE_MASK)
@@ -1303,7 +1302,8 @@ static void phys_sections_free(PhysPageMap *map)
g_free(map->nodes);
}
-static void register_subpage(AddressSpaceDispatch *d, MemoryRegionSection *section)
+static void register_subpage(AddressSpace *as, AddressSpaceDispatch *d,
+ MemoryRegionSection *section)
{
subpage_t *subpage;
hwaddr base = section->offset_within_address_space
@@ -1318,8 +1318,8 @@ static void register_subpage(AddressSpaceDispatch *d, MemoryRegionSection *secti
assert(existing->mr->subpage || existing->mr == &io_mem_unassigned);
if (!(existing->mr->subpage)) {
- subpage = subpage_init(d->as, base);
- subsection.address_space = d->as;
+ subpage = subpage_init(as, base);
+ subsection.address_space = as;
subsection.mr = &subpage->iomem;
phys_page_set(d, base >> TARGET_PAGE_BITS, 1,
phys_section_add(&d->map, &subsection));
@@ -1356,7 +1356,7 @@ void mem_add(AddressSpace *as, FlatView *fv, MemoryRegionSection *section)
- now.offset_within_address_space;
now.size = int128_min(int128_make64(left), now.size);
- register_subpage(d, &now);
+ register_subpage(as, d, &now);
} else {
now.size = int128_zero();
}
@@ -1366,10 +1366,10 @@ void mem_add(AddressSpace *as, FlatView *fv, MemoryRegionSection *section)
remain.offset_within_region += int128_get64(now.size);
now = remain;
if (int128_lt(remain.size, page_size)) {
- register_subpage(d, &now);
+ register_subpage(as, d, &now);
} else if (remain.offset_within_address_space & ~TARGET_PAGE_MASK) {
now.size = page_size;
- register_subpage(d, &now);
+ register_subpage(as, d, &now);
} else {
now.size = int128_and(now.size, int128_neg(page_size));
register_multipage(d, &now);
@@ -2685,7 +2685,6 @@ AddressSpaceDispatch *mem_begin(AddressSpace *as)
assert(n == PHYS_SECTION_WATCH);
d->phys_map = (PhysPageEntry) { .ptr = PHYS_MAP_NODE_NIL, .skip = 1 };
- d->as = as;
return d;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH qemu v5 06/18] memory: Switch memory from using AddressSpace to FlatView
2017-09-21 8:50 [Qemu-devel] [PATCH qemu v5 00/18] memory: Store physical root MR in FlatView Alexey Kardashevskiy
` (4 preceding siblings ...)
2017-09-21 8:50 ` [Qemu-devel] [PATCH qemu v5 05/18] memory: Remove AddressSpace pointer from AddressSpaceDispatch Alexey Kardashevskiy
@ 2017-09-21 8:50 ` Alexey Kardashevskiy
2017-09-21 8:50 ` [Qemu-devel] [PATCH qemu v5 07/18] memory: Cleanup after switching " Alexey Kardashevskiy
` (12 subsequent siblings)
18 siblings, 0 replies; 26+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-21 8:50 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini
FlatView's will be shared between AddressSpace's and subpage_t
and MemoryRegionSection cannot store AS anymore, hence this change.
In particular, for:
typedef struct subpage_t {
MemoryRegion iomem;
- AddressSpace *as;
+ FlatView *fv;
hwaddr base;
uint16_t sub_section[];
} subpage_t;
struct MemoryRegionSection {
MemoryRegion *mr;
- AddressSpace *address_space;
+ FlatView *fv;
hwaddr offset_within_region;
Int128 size;
hwaddr offset_within_address_space;
bool readonly;
};
This should cause no behavioural change.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v4:
* fixed compile warning/error about duplicated "typedef struct FlatView
FlatView"
---
include/exec/memory-internal.h | 2 +-
include/exec/memory.h | 51 ++++++++----
exec.c | 180 ++++++++++++++++++++++++-----------------
hw/intc/openpic_kvm.c | 2 +-
memory.c | 29 +++----
5 files changed, 157 insertions(+), 107 deletions(-)
diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index 6e08eda256..1cf8ad9869 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -27,7 +27,7 @@ extern const MemoryRegionOps unassigned_mem_ops;
bool memory_region_access_valid(MemoryRegion *mr, hwaddr addr,
unsigned size, bool is_write);
-void mem_add(AddressSpace *as, FlatView *fv, MemoryRegionSection *section);
+void mem_add(FlatView *fv, MemoryRegionSection *section);
AddressSpaceDispatch *mem_begin(AddressSpace *as);
void mem_commit(AddressSpaceDispatch *d);
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 2346f8b863..7816e5d655 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -48,6 +48,7 @@
typedef struct MemoryRegionOps MemoryRegionOps;
typedef struct MemoryRegionMmio MemoryRegionMmio;
+typedef struct FlatView FlatView;
struct MemoryRegionMmio {
CPUReadMemoryFunc *read[3];
@@ -320,6 +321,8 @@ struct AddressSpace {
QTAILQ_ENTRY(AddressSpace) address_spaces_link;
};
+FlatView *address_space_to_flatview(AddressSpace *as);
+
/**
* MemoryRegionSection: describes a fragment of a #MemoryRegion
*
@@ -333,7 +336,7 @@ struct AddressSpace {
*/
struct MemoryRegionSection {
MemoryRegion *mr;
- AddressSpace *address_space;
+ FlatView *fv;
hwaddr offset_within_region;
Int128 size;
hwaddr offset_within_address_space;
@@ -1842,9 +1845,17 @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr,
* @len: pointer to length
* @is_write: indicates the transfer direction
*/
-MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr,
- hwaddr *xlat, hwaddr *len,
- bool is_write);
+MemoryRegion *flatview_translate(FlatView *fv,
+ hwaddr addr, hwaddr *xlat,
+ hwaddr *len, bool is_write);
+
+static inline MemoryRegion *address_space_translate(AddressSpace *as,
+ hwaddr addr, hwaddr *xlat,
+ hwaddr *len, bool is_write)
+{
+ return flatview_translate(address_space_to_flatview(as),
+ addr, xlat, len, is_write);
+}
/* address_space_access_valid: check for validity of accessing an address
* space range
@@ -1895,12 +1906,13 @@ void address_space_unmap(AddressSpace *as, void *buffer, hwaddr len,
/* Internal functions, part of the implementation of address_space_read. */
-MemTxResult address_space_read_continue(AddressSpace *as, hwaddr addr,
- MemTxAttrs attrs, uint8_t *buf,
- int len, hwaddr addr1, hwaddr l,
- MemoryRegion *mr);
-MemTxResult address_space_read_full(AddressSpace *as, hwaddr addr,
- MemTxAttrs attrs, uint8_t *buf, int len);
+MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr,
+ MemTxAttrs attrs, uint8_t *buf,
+ int len, hwaddr addr1, hwaddr l,
+ MemoryRegion *mr);
+
+MemTxResult flatview_read_full(FlatView *fv, hwaddr addr,
+ MemTxAttrs attrs, uint8_t *buf, int len);
void *qemu_map_ram_ptr(RAMBlock *ram_block, ram_addr_t addr);
static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
@@ -1927,8 +1939,8 @@ static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
* @buf: buffer with the data transferred
*/
static inline __attribute__((__always_inline__))
-MemTxResult address_space_read(AddressSpace *as, hwaddr addr, MemTxAttrs attrs,
- uint8_t *buf, int len)
+MemTxResult flatview_read(FlatView *fv, hwaddr addr, MemTxAttrs attrs,
+ uint8_t *buf, int len)
{
MemTxResult result = MEMTX_OK;
hwaddr l, addr1;
@@ -1939,22 +1951,29 @@ MemTxResult address_space_read(AddressSpace *as, hwaddr addr, MemTxAttrs attrs,
if (len) {
rcu_read_lock();
l = len;
- mr = address_space_translate(as, addr, &addr1, &l, false);
+ mr = flatview_translate(fv, addr, &addr1, &l, false);
if (len == l && memory_access_is_direct(mr, false)) {
ptr = qemu_map_ram_ptr(mr->ram_block, addr1);
memcpy(buf, ptr, len);
} else {
- result = address_space_read_continue(as, addr, attrs, buf, len,
- addr1, l, mr);
+ result = flatview_read_continue(fv, addr, attrs, buf, len,
+ addr1, l, mr);
}
rcu_read_unlock();
}
} else {
- result = address_space_read_full(as, addr, attrs, buf, len);
+ result = flatview_read_full(fv, addr, attrs, buf, len);
}
return result;
}
+static inline MemTxResult address_space_read(AddressSpace *as, hwaddr addr,
+ MemTxAttrs attrs, uint8_t *buf,
+ int len)
+{
+ return flatview_read(address_space_to_flatview(as), addr, attrs, buf, len);
+}
+
/**
* address_space_read_cached: read from a cached RAM region
*
diff --git a/exec.c b/exec.c
index a54dde7835..d2b9f60494 100644
--- a/exec.c
+++ b/exec.c
@@ -198,7 +198,7 @@ struct AddressSpaceDispatch {
#define SUBPAGE_IDX(addr) ((addr) & ~TARGET_PAGE_MASK)
typedef struct subpage_t {
MemoryRegion iomem;
- AddressSpace *as;
+ FlatView *fv;
hwaddr base;
uint16_t sub_section[];
} subpage_t;
@@ -468,13 +468,13 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x
}
/* Called from RCU critical section */
-static MemoryRegionSection address_space_do_translate(AddressSpace *as,
- hwaddr addr,
- hwaddr *xlat,
- hwaddr *plen,
- bool is_write,
- bool is_mmio,
- AddressSpace **target_as)
+static MemoryRegionSection flatview_do_translate(FlatView *fv,
+ hwaddr addr,
+ hwaddr *xlat,
+ hwaddr *plen,
+ bool is_write,
+ bool is_mmio,
+ AddressSpace **target_as)
{
IOMMUTLBEntry iotlb;
MemoryRegionSection *section;
@@ -482,8 +482,9 @@ static MemoryRegionSection address_space_do_translate(AddressSpace *as,
IOMMUMemoryRegionClass *imrc;
for (;;) {
- AddressSpaceDispatch *d = address_space_to_dispatch(as);
- section = address_space_translate_internal(d, addr, &addr, plen, is_mmio);
+ section = address_space_translate_internal(
+ flatview_to_dispatch(fv), addr, &addr,
+ plen, is_mmio);
iommu_mr = memory_region_get_iommu(section->mr);
if (!iommu_mr) {
@@ -500,7 +501,7 @@ static MemoryRegionSection address_space_do_translate(AddressSpace *as,
goto translate_fail;
}
- as = iotlb.target_as;
+ fv = address_space_to_flatview(iotlb.target_as);
*target_as = iotlb.target_as;
}
@@ -523,8 +524,8 @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr,
plen = (hwaddr)-1;
/* This can never be MMIO. */
- section = address_space_do_translate(as, addr, &xlat, &plen,
- is_write, false, &as);
+ section = flatview_do_translate(address_space_to_flatview(as), addr,
+ &xlat, &plen, is_write, false, &as);
/* Illegal translation */
if (section.mr == &io_mem_unassigned) {
@@ -560,16 +561,15 @@ iotlb_fail:
}
/* Called from RCU critical section */
-MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr,
- hwaddr *xlat, hwaddr *plen,
- bool is_write)
+MemoryRegion *flatview_translate(FlatView *fv, hwaddr addr, hwaddr *xlat,
+ hwaddr *plen, bool is_write)
{
MemoryRegion *mr;
MemoryRegionSection section;
+ AddressSpace *as = NULL;
/* This can be MMIO, so setup MMIO bit. */
- section = address_space_do_translate(as, addr, xlat, plen, is_write, true,
- &as);
+ section = flatview_do_translate(fv, addr, xlat, plen, is_write, true, &as);
mr = section.mr;
if (xen_enabled() && memory_access_is_direct(mr, is_write)) {
@@ -1219,7 +1219,7 @@ hwaddr memory_region_section_get_iotlb(CPUState *cpu,
} else {
AddressSpaceDispatch *d;
- d = address_space_to_dispatch(section->address_space);
+ d = flatview_to_dispatch(section->fv);
iotlb = section - d->map.sections;
iotlb += xlat;
}
@@ -1245,7 +1245,7 @@ hwaddr memory_region_section_get_iotlb(CPUState *cpu,
static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
uint16_t section);
-static subpage_t *subpage_init(AddressSpace *as, hwaddr base);
+static subpage_t *subpage_init(FlatView *fv, hwaddr base);
static void *(*phys_mem_alloc)(size_t size, uint64_t *align) =
qemu_anon_ram_alloc;
@@ -1302,7 +1302,7 @@ static void phys_sections_free(PhysPageMap *map)
g_free(map->nodes);
}
-static void register_subpage(AddressSpace *as, AddressSpaceDispatch *d,
+static void register_subpage(FlatView *fv, AddressSpaceDispatch *d,
MemoryRegionSection *section)
{
subpage_t *subpage;
@@ -1318,8 +1318,8 @@ static void register_subpage(AddressSpace *as, AddressSpaceDispatch *d,
assert(existing->mr->subpage || existing->mr == &io_mem_unassigned);
if (!(existing->mr->subpage)) {
- subpage = subpage_init(as, base);
- subsection.address_space = as;
+ subpage = subpage_init(fv, base);
+ subsection.fv = fv;
subsection.mr = &subpage->iomem;
phys_page_set(d, base >> TARGET_PAGE_BITS, 1,
phys_section_add(&d->map, &subsection));
@@ -1345,7 +1345,7 @@ static void register_multipage(AddressSpaceDispatch *d,
phys_page_set(d, start_addr >> TARGET_PAGE_BITS, num_pages, section_index);
}
-void mem_add(AddressSpace *as, FlatView *fv, MemoryRegionSection *section)
+void mem_add(FlatView *fv, MemoryRegionSection *section)
{
AddressSpaceDispatch *d = flatview_to_dispatch(fv);
MemoryRegionSection now = *section, remain = *section;
@@ -1356,7 +1356,7 @@ void mem_add(AddressSpace *as, FlatView *fv, MemoryRegionSection *section)
- now.offset_within_address_space;
now.size = int128_min(int128_make64(left), now.size);
- register_subpage(as, d, &now);
+ register_subpage(fv, d, &now);
} else {
now.size = int128_zero();
}
@@ -1366,10 +1366,10 @@ void mem_add(AddressSpace *as, FlatView *fv, MemoryRegionSection *section)
remain.offset_within_region += int128_get64(now.size);
now = remain;
if (int128_lt(remain.size, page_size)) {
- register_subpage(as, d, &now);
+ register_subpage(fv, d, &now);
} else if (remain.offset_within_address_space & ~TARGET_PAGE_MASK) {
now.size = page_size;
- register_subpage(as, d, &now);
+ register_subpage(fv, d, &now);
} else {
now.size = int128_and(now.size, int128_neg(page_size));
register_multipage(d, &now);
@@ -2500,6 +2500,11 @@ static const MemoryRegionOps watch_mem_ops = {
.endianness = DEVICE_NATIVE_ENDIAN,
};
+static MemTxResult flatview_write(FlatView *fv, hwaddr addr, MemTxAttrs attrs,
+ const uint8_t *buf, int len);
+static bool flatview_access_valid(FlatView *fv, hwaddr addr, int len,
+ bool is_write);
+
static MemTxResult subpage_read(void *opaque, hwaddr addr, uint64_t *data,
unsigned len, MemTxAttrs attrs)
{
@@ -2511,8 +2516,7 @@ static MemTxResult subpage_read(void *opaque, hwaddr addr, uint64_t *data,
printf("%s: subpage %p len %u addr " TARGET_FMT_plx "\n", __func__,
subpage, len, addr);
#endif
- res = address_space_read(subpage->as, addr + subpage->base,
- attrs, buf, len);
+ res = flatview_read(subpage->fv, addr + subpage->base, attrs, buf, len);
if (res) {
return res;
}
@@ -2561,8 +2565,7 @@ static MemTxResult subpage_write(void *opaque, hwaddr addr,
default:
abort();
}
- return address_space_write(subpage->as, addr + subpage->base,
- attrs, buf, len);
+ return flatview_write(subpage->fv, addr + subpage->base, attrs, buf, len);
}
static bool subpage_accepts(void *opaque, hwaddr addr,
@@ -2574,8 +2577,8 @@ static bool subpage_accepts(void *opaque, hwaddr addr,
__func__, subpage, is_write ? 'w' : 'r', len, addr);
#endif
- return address_space_access_valid(subpage->as, addr + subpage->base,
- len, is_write);
+ return flatview_access_valid(subpage->fv, addr + subpage->base,
+ len, is_write);
}
static const MemoryRegionOps subpage_ops = {
@@ -2609,12 +2612,12 @@ static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
return 0;
}
-static subpage_t *subpage_init(AddressSpace *as, hwaddr base)
+static subpage_t *subpage_init(FlatView *fv, hwaddr base)
{
subpage_t *mmio;
mmio = g_malloc0(sizeof(subpage_t) + TARGET_PAGE_SIZE * sizeof(uint16_t));
- mmio->as = as;
+ mmio->fv = fv;
mmio->base = base;
memory_region_init_io(&mmio->iomem, NULL, &subpage_ops, mmio,
NULL, TARGET_PAGE_SIZE);
@@ -2628,12 +2631,11 @@ static subpage_t *subpage_init(AddressSpace *as, hwaddr base)
return mmio;
}
-static uint16_t dummy_section(PhysPageMap *map, AddressSpace *as,
- MemoryRegion *mr)
+static uint16_t dummy_section(PhysPageMap *map, FlatView *fv, MemoryRegion *mr)
{
- assert(as);
+ assert(fv);
MemoryRegionSection section = {
- .address_space = as,
+ .fv = fv,
.mr = mr,
.offset_within_address_space = 0,
.offset_within_region = 0,
@@ -2672,16 +2674,17 @@ static void io_mem_init(void)
AddressSpaceDispatch *mem_begin(AddressSpace *as)
{
+ FlatView *fv = address_space_to_flatview(as);
AddressSpaceDispatch *d = g_new0(AddressSpaceDispatch, 1);
uint16_t n;
- n = dummy_section(&d->map, as, &io_mem_unassigned);
+ n = dummy_section(&d->map, fv, &io_mem_unassigned);
assert(n == PHYS_SECTION_UNASSIGNED);
- n = dummy_section(&d->map, as, &io_mem_notdirty);
+ n = dummy_section(&d->map, fv, &io_mem_notdirty);
assert(n == PHYS_SECTION_NOTDIRTY);
- n = dummy_section(&d->map, as, &io_mem_rom);
+ n = dummy_section(&d->map, fv, &io_mem_rom);
assert(n == PHYS_SECTION_ROM);
- n = dummy_section(&d->map, as, &io_mem_watch);
+ n = dummy_section(&d->map, fv, &io_mem_watch);
assert(n == PHYS_SECTION_WATCH);
d->phys_map = (PhysPageEntry) { .ptr = PHYS_MAP_NODE_NIL, .skip = 1 };
@@ -2861,11 +2864,11 @@ static bool prepare_mmio_access(MemoryRegion *mr)
}
/* Called within RCU critical section. */
-static MemTxResult address_space_write_continue(AddressSpace *as, hwaddr addr,
- MemTxAttrs attrs,
- const uint8_t *buf,
- int len, hwaddr addr1,
- hwaddr l, MemoryRegion *mr)
+static MemTxResult flatview_write_continue(FlatView *fv, hwaddr addr,
+ MemTxAttrs attrs,
+ const uint8_t *buf,
+ int len, hwaddr addr1,
+ hwaddr l, MemoryRegion *mr)
{
uint8_t *ptr;
uint64_t val;
@@ -2927,14 +2930,14 @@ static MemTxResult address_space_write_continue(AddressSpace *as, hwaddr addr,
}
l = len;
- mr = address_space_translate(as, addr, &addr1, &l, true);
+ mr = flatview_translate(fv, addr, &addr1, &l, true);
}
return result;
}
-MemTxResult address_space_write(AddressSpace *as, hwaddr addr, MemTxAttrs attrs,
- const uint8_t *buf, int len)
+static MemTxResult flatview_write(FlatView *fv, hwaddr addr, MemTxAttrs attrs,
+ const uint8_t *buf, int len)
{
hwaddr l;
hwaddr addr1;
@@ -2944,20 +2947,27 @@ MemTxResult address_space_write(AddressSpace *as, hwaddr addr, MemTxAttrs attrs,
if (len > 0) {
rcu_read_lock();
l = len;
- mr = address_space_translate(as, addr, &addr1, &l, true);
- result = address_space_write_continue(as, addr, attrs, buf, len,
- addr1, l, mr);
+ mr = flatview_translate(fv, addr, &addr1, &l, true);
+ result = flatview_write_continue(fv, addr, attrs, buf, len,
+ addr1, l, mr);
rcu_read_unlock();
}
return result;
}
+MemTxResult address_space_write(AddressSpace *as, hwaddr addr,
+ MemTxAttrs attrs,
+ const uint8_t *buf, int len)
+{
+ return flatview_write(address_space_to_flatview(as), addr, attrs, buf, len);
+}
+
/* Called within RCU critical section. */
-MemTxResult address_space_read_continue(AddressSpace *as, hwaddr addr,
- MemTxAttrs attrs, uint8_t *buf,
- int len, hwaddr addr1, hwaddr l,
- MemoryRegion *mr)
+MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr,
+ MemTxAttrs attrs, uint8_t *buf,
+ int len, hwaddr addr1, hwaddr l,
+ MemoryRegion *mr)
{
uint8_t *ptr;
uint64_t val;
@@ -3017,14 +3027,14 @@ MemTxResult address_space_read_continue(AddressSpace *as, hwaddr addr,
}
l = len;
- mr = address_space_translate(as, addr, &addr1, &l, false);
+ mr = flatview_translate(fv, addr, &addr1, &l, false);
}
return result;
}
-MemTxResult address_space_read_full(AddressSpace *as, hwaddr addr,
- MemTxAttrs attrs, uint8_t *buf, int len)
+MemTxResult flatview_read_full(FlatView *fv, hwaddr addr,
+ MemTxAttrs attrs, uint8_t *buf, int len)
{
hwaddr l;
hwaddr addr1;
@@ -3034,25 +3044,33 @@ MemTxResult address_space_read_full(AddressSpace *as, hwaddr addr,
if (len > 0) {
rcu_read_lock();
l = len;
- mr = address_space_translate(as, addr, &addr1, &l, false);
- result = address_space_read_continue(as, addr, attrs, buf, len,
- addr1, l, mr);
+ mr = flatview_translate(fv, addr, &addr1, &l, false);
+ result = flatview_read_continue(fv, addr, attrs, buf, len,
+ addr1, l, mr);
rcu_read_unlock();
}
return result;
}
-MemTxResult address_space_rw(AddressSpace *as, hwaddr addr, MemTxAttrs attrs,
- uint8_t *buf, int len, bool is_write)
+static MemTxResult flatview_rw(FlatView *fv, hwaddr addr, MemTxAttrs attrs,
+ uint8_t *buf, int len, bool is_write)
{
if (is_write) {
- return address_space_write(as, addr, attrs, (uint8_t *)buf, len);
+ return flatview_write(fv, addr, attrs, (uint8_t *)buf, len);
} else {
- return address_space_read(as, addr, attrs, (uint8_t *)buf, len);
+ return flatview_read(fv, addr, attrs, (uint8_t *)buf, len);
}
}
+MemTxResult address_space_rw(AddressSpace *as, hwaddr addr,
+ MemTxAttrs attrs, uint8_t *buf,
+ int len, bool is_write)
+{
+ return flatview_rw(address_space_to_flatview(as),
+ addr, attrs, buf, len, is_write);
+}
+
void cpu_physical_memory_rw(hwaddr addr, uint8_t *buf,
int len, int is_write)
{
@@ -3210,7 +3228,8 @@ static void cpu_notify_map_clients(void)
qemu_mutex_unlock(&map_client_list_lock);
}
-bool address_space_access_valid(AddressSpace *as, hwaddr addr, int len, bool is_write)
+static bool flatview_access_valid(FlatView *fv, hwaddr addr, int len,
+ bool is_write)
{
MemoryRegion *mr;
hwaddr l, xlat;
@@ -3218,7 +3237,7 @@ bool address_space_access_valid(AddressSpace *as, hwaddr addr, int len, bool is_
rcu_read_lock();
while (len > 0) {
l = len;
- mr = address_space_translate(as, addr, &xlat, &l, is_write);
+ mr = flatview_translate(fv, addr, &xlat, &l, is_write);
if (!memory_access_is_direct(mr, is_write)) {
l = memory_access_size(mr, l, addr);
if (!memory_region_access_valid(mr, xlat, l, is_write)) {
@@ -3234,8 +3253,16 @@ bool address_space_access_valid(AddressSpace *as, hwaddr addr, int len, bool is_
return true;
}
+bool address_space_access_valid(AddressSpace *as, hwaddr addr,
+ int len, bool is_write)
+{
+ return flatview_access_valid(address_space_to_flatview(as),
+ addr, len, is_write);
+}
+
static hwaddr
-address_space_extend_translation(AddressSpace *as, hwaddr addr, hwaddr target_len,
+flatview_extend_translation(FlatView *fv, hwaddr addr,
+ hwaddr target_len,
MemoryRegion *mr, hwaddr base, hwaddr len,
bool is_write)
{
@@ -3252,7 +3279,8 @@ address_space_extend_translation(AddressSpace *as, hwaddr addr, hwaddr target_le
}
len = target_len;
- this_mr = address_space_translate(as, addr, &xlat, &len, is_write);
+ this_mr = flatview_translate(fv, addr, &xlat,
+ &len, is_write);
if (this_mr != mr || xlat != base + done) {
return done;
}
@@ -3275,6 +3303,7 @@ void *address_space_map(AddressSpace *as,
hwaddr l, xlat;
MemoryRegion *mr;
void *ptr;
+ FlatView *fv = address_space_to_flatview(as);
if (len == 0) {
return NULL;
@@ -3282,7 +3311,7 @@ void *address_space_map(AddressSpace *as,
l = len;
rcu_read_lock();
- mr = address_space_translate(as, addr, &xlat, &l, is_write);
+ mr = flatview_translate(fv, addr, &xlat, &l, is_write);
if (!memory_access_is_direct(mr, is_write)) {
if (atomic_xchg(&bounce.in_use, true)) {
@@ -3298,7 +3327,7 @@ void *address_space_map(AddressSpace *as,
memory_region_ref(mr);
bounce.mr = mr;
if (!is_write) {
- address_space_read(as, addr, MEMTXATTRS_UNSPECIFIED,
+ flatview_read(fv, addr, MEMTXATTRS_UNSPECIFIED,
bounce.buffer, l);
}
@@ -3309,7 +3338,8 @@ void *address_space_map(AddressSpace *as,
memory_region_ref(mr);
- *plen = address_space_extend_translation(as, addr, len, mr, xlat, l, is_write);
+ *plen = flatview_extend_translation(fv, addr, len, mr, xlat,
+ l, is_write);
ptr = qemu_ram_ptr_length(mr->ram_block, xlat, plen, true);
rcu_read_unlock();
diff --git a/hw/intc/openpic_kvm.c b/hw/intc/openpic_kvm.c
index 0518e017c4..fa83420254 100644
--- a/hw/intc/openpic_kvm.c
+++ b/hw/intc/openpic_kvm.c
@@ -124,7 +124,7 @@ static void kvm_openpic_region_add(MemoryListener *listener,
uint64_t reg_base;
int ret;
- if (section->address_space != &address_space_memory) {
+ if (section->fv != address_space_to_flatview(&address_space_memory)) {
abort();
}
diff --git a/memory.c b/memory.c
index 27d7aeffc2..1364500eb6 100644
--- a/memory.c
+++ b/memory.c
@@ -154,7 +154,8 @@ enum ListenerDirection { Forward, Reverse };
/* No need to ref/unref .mr, the FlatRange keeps it alive. */
#define MEMORY_LISTENER_UPDATE_REGION(fr, as, dir, callback, _args...) \
do { \
- MemoryRegionSection mrs = section_from_flat_range(fr, as); \
+ MemoryRegionSection mrs = section_from_flat_range(fr, \
+ address_space_to_flatview(as)); \
MEMORY_LISTENER_CALL(as, callback, dir, &mrs, ##_args); \
} while(0)
@@ -208,7 +209,6 @@ static bool memory_region_ioeventfd_equal(MemoryRegionIoeventfd a,
}
typedef struct FlatRange FlatRange;
-typedef struct FlatView FlatView;
/* Range of memory in the global map. Addresses are absolute. */
struct FlatRange {
@@ -238,11 +238,11 @@ typedef struct AddressSpaceOps AddressSpaceOps;
for (var = (view)->ranges; var < (view)->ranges + (view)->nr; ++var)
static inline MemoryRegionSection
-section_from_flat_range(FlatRange *fr, AddressSpace *as)
+section_from_flat_range(FlatRange *fr, FlatView *fv)
{
return (MemoryRegionSection) {
.mr = fr->mr,
- .address_space = as,
+ .fv = fv,
.offset_within_region = fr->offset_in_region,
.size = fr->addr.size,
.offset_within_address_space = int128_get64(fr->addr.start),
@@ -312,7 +312,7 @@ static void flatview_unref(FlatView *view)
}
}
-static FlatView *address_space_to_flatview(AddressSpace *as)
+FlatView *address_space_to_flatview(AddressSpace *as)
{
return atomic_rcu_read(&as->current_map);
}
@@ -760,7 +760,7 @@ static void address_space_add_del_ioeventfds(AddressSpace *as,
fds_new[inew]))) {
fd = &fds_old[iold];
section = (MemoryRegionSection) {
- .address_space = as,
+ .fv = address_space_to_flatview(as),
.offset_within_address_space = int128_get64(fd->addr.start),
.size = fd->addr.size,
};
@@ -773,7 +773,7 @@ static void address_space_add_del_ioeventfds(AddressSpace *as,
fds_old[iold]))) {
fd = &fds_new[inew];
section = (MemoryRegionSection) {
- .address_space = as,
+ .fv = address_space_to_flatview(as),
.offset_within_address_space = int128_get64(fd->addr.start),
.size = fd->addr.size,
};
@@ -908,8 +908,8 @@ static void address_space_update_topology(AddressSpace *as)
new_view->dispatch = mem_begin(as);
for (i = 0; i < new_view->nr; i++) {
MemoryRegionSection mrs =
- section_from_flat_range(&new_view->ranges[i], as);
- mem_add(as, new_view, &mrs);
+ section_from_flat_range(&new_view->ranges[i], new_view);
+ mem_add(new_view, &mrs);
}
mem_commit(new_view->dispatch);
@@ -1865,7 +1865,7 @@ void memory_region_sync_dirty_bitmap(MemoryRegion *mr)
view = address_space_get_flatview(as);
FOR_EACH_FLAT_RANGE(fr, view) {
if (fr->mr == mr) {
- MemoryRegionSection mrs = section_from_flat_range(fr, as);
+ MemoryRegionSection mrs = section_from_flat_range(fr, view);
listener->log_sync(listener, &mrs);
}
}
@@ -1968,7 +1968,7 @@ static void memory_region_update_coalesced_range_as(MemoryRegion *mr, AddressSpa
FOR_EACH_FLAT_RANGE(fr, view) {
if (fr->mr == mr) {
section = (MemoryRegionSection) {
- .address_space = as,
+ .fv = view,
.offset_within_address_space = int128_get64(fr->addr.start),
.size = fr->addr.size,
};
@@ -2330,7 +2330,7 @@ static MemoryRegionSection memory_region_find_rcu(MemoryRegion *mr,
}
ret.mr = fr->mr;
- ret.address_space = as;
+ ret.fv = view;
range = addrrange_intersection(range, fr->addr);
ret.offset_within_region = fr->offset_in_region;
ret.offset_within_region += int128_get64(int128_sub(range.start,
@@ -2379,7 +2379,8 @@ void memory_global_dirty_log_sync(void)
view = address_space_get_flatview(as);
FOR_EACH_FLAT_RANGE(fr, view) {
if (fr->dirty_log_mask) {
- MemoryRegionSection mrs = section_from_flat_range(fr, as);
+ MemoryRegionSection mrs = section_from_flat_range(fr, view);
+
listener->log_sync(listener, &mrs);
}
}
@@ -2464,7 +2465,7 @@ static void listener_add_address_space(MemoryListener *listener,
FOR_EACH_FLAT_RANGE(fr, view) {
MemoryRegionSection section = {
.mr = fr->mr,
- .address_space = as,
+ .fv = view,
.offset_within_region = fr->offset_in_region,
.size = fr->addr.size,
.offset_within_address_space = int128_get64(fr->addr.start),
--
2.11.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH qemu v5 07/18] memory: Cleanup after switching to FlatView
2017-09-21 8:50 [Qemu-devel] [PATCH qemu v5 00/18] memory: Store physical root MR in FlatView Alexey Kardashevskiy
` (5 preceding siblings ...)
2017-09-21 8:50 ` [Qemu-devel] [PATCH qemu v5 06/18] memory: Switch memory from using AddressSpace to FlatView Alexey Kardashevskiy
@ 2017-09-21 8:50 ` Alexey Kardashevskiy
2017-09-21 8:51 ` [Qemu-devel] [PATCH qemu v5 08/18] memory: Rename mem_begin/mem_commit/mem_add helpers Alexey Kardashevskiy
` (11 subsequent siblings)
18 siblings, 0 replies; 26+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-21 8:50 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini
We store AddressSpaceDispatch* in FlatView anyway so there is no need
to carry it from mem_add() to register_subpage/register_multipage.
This should cause no behavioural change.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
exec.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/exec.c b/exec.c
index d2b9f60494..548ec71b4c 100644
--- a/exec.c
+++ b/exec.c
@@ -1302,9 +1302,9 @@ static void phys_sections_free(PhysPageMap *map)
g_free(map->nodes);
}
-static void register_subpage(FlatView *fv, AddressSpaceDispatch *d,
- MemoryRegionSection *section)
+static void register_subpage(FlatView *fv, MemoryRegionSection *section)
{
+ AddressSpaceDispatch *d = flatview_to_dispatch(fv);
subpage_t *subpage;
hwaddr base = section->offset_within_address_space
& TARGET_PAGE_MASK;
@@ -1333,9 +1333,10 @@ static void register_subpage(FlatView *fv, AddressSpaceDispatch *d,
}
-static void register_multipage(AddressSpaceDispatch *d,
+static void register_multipage(FlatView *fv,
MemoryRegionSection *section)
{
+ AddressSpaceDispatch *d = flatview_to_dispatch(fv);
hwaddr start_addr = section->offset_within_address_space;
uint16_t section_index = phys_section_add(&d->map, section);
uint64_t num_pages = int128_get64(int128_rshift(section->size,
@@ -1347,7 +1348,6 @@ static void register_multipage(AddressSpaceDispatch *d,
void mem_add(FlatView *fv, MemoryRegionSection *section)
{
- AddressSpaceDispatch *d = flatview_to_dispatch(fv);
MemoryRegionSection now = *section, remain = *section;
Int128 page_size = int128_make64(TARGET_PAGE_SIZE);
@@ -1356,7 +1356,7 @@ void mem_add(FlatView *fv, MemoryRegionSection *section)
- now.offset_within_address_space;
now.size = int128_min(int128_make64(left), now.size);
- register_subpage(fv, d, &now);
+ register_subpage(fv, &now);
} else {
now.size = int128_zero();
}
@@ -1366,13 +1366,13 @@ void mem_add(FlatView *fv, MemoryRegionSection *section)
remain.offset_within_region += int128_get64(now.size);
now = remain;
if (int128_lt(remain.size, page_size)) {
- register_subpage(fv, d, &now);
+ register_subpage(fv, &now);
} else if (remain.offset_within_address_space & ~TARGET_PAGE_MASK) {
now.size = page_size;
- register_subpage(fv, d, &now);
+ register_subpage(fv, &now);
} else {
now.size = int128_and(now.size, int128_neg(page_size));
- register_multipage(d, &now);
+ register_multipage(fv, &now);
}
}
}
--
2.11.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH qemu v5 08/18] memory: Rename mem_begin/mem_commit/mem_add helpers
2017-09-21 8:50 [Qemu-devel] [PATCH qemu v5 00/18] memory: Store physical root MR in FlatView Alexey Kardashevskiy
` (6 preceding siblings ...)
2017-09-21 8:50 ` [Qemu-devel] [PATCH qemu v5 07/18] memory: Cleanup after switching " Alexey Kardashevskiy
@ 2017-09-21 8:51 ` Alexey Kardashevskiy
2017-09-21 8:51 ` [Qemu-devel] [PATCH qemu v5 09/18] memory: Store physical root MR in FlatView Alexey Kardashevskiy
` (10 subsequent siblings)
18 siblings, 0 replies; 26+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-21 8:51 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini
This renames some helpers to reflect better what they do.
This should cause no behavioural change.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v3:
* s/flatview_mem_add/flatview_add_to_dispatch/
* s/phys_page_compact_all/address_space_dispatch_compact/
* s/address_space_dispatch_alloc/address_space_dispatch_new/
---
include/exec/memory-internal.h | 6 +++---
exec.c | 12 +++---------
memory.c | 6 +++---
3 files changed, 9 insertions(+), 15 deletions(-)
diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index 1cf8ad9869..d4a35c6e96 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -27,9 +27,9 @@ extern const MemoryRegionOps unassigned_mem_ops;
bool memory_region_access_valid(MemoryRegion *mr, hwaddr addr,
unsigned size, bool is_write);
-void mem_add(FlatView *fv, MemoryRegionSection *section);
-AddressSpaceDispatch *mem_begin(AddressSpace *as);
-void mem_commit(AddressSpaceDispatch *d);
+void flatview_add_to_dispatch(FlatView *fv, MemoryRegionSection *section);
+AddressSpaceDispatch *address_space_dispatch_new(FlatView *fv);
+void address_space_dispatch_compact(AddressSpaceDispatch *d);
AddressSpaceDispatch *address_space_to_dispatch(AddressSpace *as);
AddressSpaceDispatch *flatview_to_dispatch(FlatView *fv);
diff --git a/exec.c b/exec.c
index 548ec71b4c..b085f82503 100644
--- a/exec.c
+++ b/exec.c
@@ -358,7 +358,7 @@ static void phys_page_compact(PhysPageEntry *lp, Node *nodes)
}
}
-static void phys_page_compact_all(AddressSpaceDispatch *d, int nodes_nb)
+void address_space_dispatch_compact(AddressSpaceDispatch *d)
{
if (d->phys_map.skip) {
phys_page_compact(&d->phys_map, d->map.nodes);
@@ -1346,7 +1346,7 @@ static void register_multipage(FlatView *fv,
phys_page_set(d, start_addr >> TARGET_PAGE_BITS, num_pages, section_index);
}
-void mem_add(FlatView *fv, MemoryRegionSection *section)
+void flatview_add_to_dispatch(FlatView *fv, MemoryRegionSection *section)
{
MemoryRegionSection now = *section, remain = *section;
Int128 page_size = int128_make64(TARGET_PAGE_SIZE);
@@ -2672,9 +2672,8 @@ static void io_mem_init(void)
NULL, UINT64_MAX);
}
-AddressSpaceDispatch *mem_begin(AddressSpace *as)
+AddressSpaceDispatch *address_space_dispatch_new(FlatView *fv)
{
- FlatView *fv = address_space_to_flatview(as);
AddressSpaceDispatch *d = g_new0(AddressSpaceDispatch, 1);
uint16_t n;
@@ -2698,11 +2697,6 @@ void address_space_dispatch_free(AddressSpaceDispatch *d)
g_free(d);
}
-void mem_commit(AddressSpaceDispatch *d)
-{
- phys_page_compact_all(d, d->map.nodes_nb);
-}
-
static void tcg_commit(MemoryListener *listener)
{
CPUAddressSpace *cpuas;
diff --git a/memory.c b/memory.c
index 1364500eb6..990e9b8478 100644
--- a/memory.c
+++ b/memory.c
@@ -905,13 +905,13 @@ static void address_space_update_topology(AddressSpace *as)
FlatView *new_view = generate_memory_topology(as->root);
int i;
- new_view->dispatch = mem_begin(as);
+ new_view->dispatch = address_space_dispatch_new(new_view);
for (i = 0; i < new_view->nr; i++) {
MemoryRegionSection mrs =
section_from_flat_range(&new_view->ranges[i], new_view);
- mem_add(new_view, &mrs);
+ flatview_add_to_dispatch(new_view, &mrs);
}
- mem_commit(new_view->dispatch);
+ address_space_dispatch_compact(new_view->dispatch);
if (!QTAILQ_EMPTY(&as->listeners)) {
address_space_update_topology_pass(as, old_view, new_view, false);
--
2.11.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH qemu v5 09/18] memory: Store physical root MR in FlatView
2017-09-21 8:50 [Qemu-devel] [PATCH qemu v5 00/18] memory: Store physical root MR in FlatView Alexey Kardashevskiy
` (7 preceding siblings ...)
2017-09-21 8:51 ` [Qemu-devel] [PATCH qemu v5 08/18] memory: Rename mem_begin/mem_commit/mem_add helpers Alexey Kardashevskiy
@ 2017-09-21 8:51 ` Alexey Kardashevskiy
2017-09-21 8:51 ` [Qemu-devel] [PATCH qemu v5 10/18] memory: Alloc dispatch tree where topology is generared Alexey Kardashevskiy
` (9 subsequent siblings)
18 siblings, 0 replies; 26+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-21 8:51 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini
Address spaces get to keep a root MR (alias or not) but FlatView stores
the actual MR as this is going to be used later on to decide whether to
share a particular FlatView or not.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v4:
* s/memory_region_unalias_entire/memory_region_get_flatview_root/
---
memory.c | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/memory.c b/memory.c
index 990e9b8478..98b0b95e6e 100644
--- a/memory.c
+++ b/memory.c
@@ -230,6 +230,7 @@ struct FlatView {
unsigned nr;
unsigned nr_allocated;
struct AddressSpaceDispatch *dispatch;
+ MemoryRegion *root;
};
typedef struct AddressSpaceOps AddressSpaceOps;
@@ -259,12 +260,14 @@ static bool flatrange_equal(FlatRange *a, FlatRange *b)
&& a->readonly == b->readonly;
}
-static FlatView *flatview_new(void)
+static FlatView *flatview_new(MemoryRegion *mr_root)
{
FlatView *view;
view = g_new0(FlatView, 1);
view->ref = 1;
+ view->root = mr_root;
+ memory_region_ref(mr_root);
return view;
}
@@ -297,6 +300,7 @@ static void flatview_destroy(FlatView *view)
memory_region_unref(view->ranges[i].mr);
}
g_free(view->ranges);
+ memory_region_unref(view->root);
g_free(view);
}
@@ -722,12 +726,25 @@ static void render_memory_region(FlatView *view,
}
}
+static MemoryRegion *memory_region_get_flatview_root(MemoryRegion *mr)
+{
+ while (mr->alias && !mr->alias_offset &&
+ int128_ge(mr->size, mr->alias->size)) {
+ /* The alias is included in its entirety. Use it as
+ * the "real" root, so that we can share more FlatViews.
+ */
+ mr = mr->alias;
+ }
+
+ return mr;
+}
+
/* Render a memory topology into a list of disjoint absolute ranges. */
static FlatView *generate_memory_topology(MemoryRegion *mr)
{
FlatView *view;
- view = flatview_new();
+ view = flatview_new(mr);
if (mr) {
render_memory_region(view, mr, int128_zero(),
@@ -902,7 +919,8 @@ static void address_space_update_topology_pass(AddressSpace *as,
static void address_space_update_topology(AddressSpace *as)
{
FlatView *old_view = address_space_get_flatview(as);
- FlatView *new_view = generate_memory_topology(as->root);
+ MemoryRegion *physmr = memory_region_get_flatview_root(old_view->root);
+ FlatView *new_view = generate_memory_topology(physmr);
int i;
new_view->dispatch = address_space_dispatch_new(new_view);
@@ -2645,7 +2663,7 @@ void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
as->ref_count = 1;
as->root = root;
as->malloced = false;
- as->current_map = flatview_new();
+ as->current_map = flatview_new(root);
as->ioeventfd_nb = 0;
as->ioeventfds = NULL;
QTAILQ_INIT(&as->listeners);
--
2.11.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH qemu v5 10/18] memory: Alloc dispatch tree where topology is generared
2017-09-21 8:50 [Qemu-devel] [PATCH qemu v5 00/18] memory: Store physical root MR in FlatView Alexey Kardashevskiy
` (8 preceding siblings ...)
2017-09-21 8:51 ` [Qemu-devel] [PATCH qemu v5 09/18] memory: Store physical root MR in FlatView Alexey Kardashevskiy
@ 2017-09-21 8:51 ` Alexey Kardashevskiy
2017-09-21 8:51 ` [Qemu-devel] [PATCH qemu v5 11/18] memory: Move address_space_update_ioeventfds Alexey Kardashevskiy
` (8 subsequent siblings)
18 siblings, 0 replies; 26+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-21 8:51 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini
This is to make next patches simpler.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
memory.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/memory.c b/memory.c
index 98b0b95e6e..eb730cd619 100644
--- a/memory.c
+++ b/memory.c
@@ -742,6 +742,7 @@ static MemoryRegion *memory_region_get_flatview_root(MemoryRegion *mr)
/* Render a memory topology into a list of disjoint absolute ranges. */
static FlatView *generate_memory_topology(MemoryRegion *mr)
{
+ int i;
FlatView *view;
view = flatview_new(mr);
@@ -752,6 +753,14 @@ static FlatView *generate_memory_topology(MemoryRegion *mr)
}
flatview_simplify(view);
+ view->dispatch = address_space_dispatch_new(view);
+ for (i = 0; i < view->nr; i++) {
+ MemoryRegionSection mrs =
+ section_from_flat_range(&view->ranges[i], view);
+ flatview_add_to_dispatch(view, &mrs);
+ }
+ address_space_dispatch_compact(view->dispatch);
+
return view;
}
@@ -921,15 +930,6 @@ static void address_space_update_topology(AddressSpace *as)
FlatView *old_view = address_space_get_flatview(as);
MemoryRegion *physmr = memory_region_get_flatview_root(old_view->root);
FlatView *new_view = generate_memory_topology(physmr);
- int i;
-
- new_view->dispatch = address_space_dispatch_new(new_view);
- for (i = 0; i < new_view->nr; i++) {
- MemoryRegionSection mrs =
- section_from_flat_range(&new_view->ranges[i], new_view);
- flatview_add_to_dispatch(new_view, &mrs);
- }
- address_space_dispatch_compact(new_view->dispatch);
if (!QTAILQ_EMPTY(&as->listeners)) {
address_space_update_topology_pass(as, old_view, new_view, false);
--
2.11.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH qemu v5 11/18] memory: Move address_space_update_ioeventfds
2017-09-21 8:50 [Qemu-devel] [PATCH qemu v5 00/18] memory: Store physical root MR in FlatView Alexey Kardashevskiy
` (9 preceding siblings ...)
2017-09-21 8:51 ` [Qemu-devel] [PATCH qemu v5 10/18] memory: Alloc dispatch tree where topology is generared Alexey Kardashevskiy
@ 2017-09-21 8:51 ` Alexey Kardashevskiy
2017-09-21 8:51 ` [Qemu-devel] [PATCH qemu v5 12/18] memory: Share FlatView's and dispatch trees between address spaces Alexey Kardashevskiy
` (7 subsequent siblings)
18 siblings, 0 replies; 26+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-21 8:51 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini
So it is called (twice) from the same function. This is to make the next
patches a bit simpler.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
memory.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/memory.c b/memory.c
index eb730cd619..9868d0f57c 100644
--- a/memory.c
+++ b/memory.c
@@ -947,8 +947,6 @@ static void address_space_update_topology(AddressSpace *as)
* counting is necessary.
*/
flatview_unref(old_view);
-
- address_space_update_ioeventfds(as);
}
void memory_region_transaction_begin(void)
@@ -971,6 +969,7 @@ void memory_region_transaction_commit(void)
QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
address_space_update_topology(as);
+ address_space_update_ioeventfds(as);
}
memory_region_update_pending = false;
MEMORY_LISTENER_CALL_GLOBAL(commit, Forward);
--
2.11.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH qemu v5 12/18] memory: Share FlatView's and dispatch trees between address spaces
2017-09-21 8:50 [Qemu-devel] [PATCH qemu v5 00/18] memory: Store physical root MR in FlatView Alexey Kardashevskiy
` (10 preceding siblings ...)
2017-09-21 8:51 ` [Qemu-devel] [PATCH qemu v5 11/18] memory: Move address_space_update_ioeventfds Alexey Kardashevskiy
@ 2017-09-21 8:51 ` Alexey Kardashevskiy
2017-09-21 8:51 ` [Qemu-devel] [PATCH qemu v5 13/18] memory: Do not allocate FlatView in address_space_init Alexey Kardashevskiy
` (6 subsequent siblings)
18 siblings, 0 replies; 26+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-21 8:51 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini
This allows sharing flat views between address spaces (AS) when
the same root memory region is used when creating a new address space.
This is done by walking through all ASes and caching one FlatView per
a physical root MR (i.e. not aliased).
This removes search for duplicates from address_space_init_shareable() as
FlatViews are shared elsewhere and keeping as::ref_count correct seems
an unnecessary and useless complication.
This should cause no change and memory use or boot time yet.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v5:
* s/flatview_set_to_address_space/address_space_set_flatview/g
* generate_memory_topology() does g_hash_table_replace() as well
v4:
* used g_hash_table_new_full() instead of g_hash_table_new() +
g_hash_table_foreach_remove()
* cached last hash table of FV globally
v3:
* got rid of global and per-AS lists of FlatView objects
---
memory.c | 56 +++++++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 45 insertions(+), 11 deletions(-)
diff --git a/memory.c b/memory.c
index 9868d0f57c..56b3dd47ab 100644
--- a/memory.c
+++ b/memory.c
@@ -47,6 +47,8 @@ static QTAILQ_HEAD(memory_listeners, MemoryListener) memory_listeners
static QTAILQ_HEAD(, AddressSpace) address_spaces
= QTAILQ_HEAD_INITIALIZER(address_spaces);
+static GHashTable *flat_views;
+
typedef struct AddrRange AddrRange;
/*
@@ -760,6 +762,7 @@ static FlatView *generate_memory_topology(MemoryRegion *mr)
flatview_add_to_dispatch(view, &mrs);
}
address_space_dispatch_compact(view->dispatch);
+ g_hash_table_replace(flat_views, mr, view);
return view;
}
@@ -925,11 +928,47 @@ static void address_space_update_topology_pass(AddressSpace *as,
}
}
-static void address_space_update_topology(AddressSpace *as)
+static void flatviews_init(void)
+{
+ if (flat_views) {
+ return;
+ }
+
+ flat_views = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL,
+ (GDestroyNotify) flatview_unref);
+}
+
+static void flatviews_reset(void)
+{
+ AddressSpace *as;
+
+ if (flat_views) {
+ g_hash_table_unref(flat_views);
+ flat_views = NULL;
+ }
+ flatviews_init();
+
+ /* Render unique FVs */
+ QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
+ MemoryRegion *physmr = memory_region_get_flatview_root(as->root);
+
+ if (g_hash_table_lookup(flat_views, physmr)) {
+ continue;
+ }
+
+ generate_memory_topology(physmr);
+ }
+}
+
+static void address_space_set_flatview(AddressSpace *as)
{
FlatView *old_view = address_space_get_flatview(as);
- MemoryRegion *physmr = memory_region_get_flatview_root(old_view->root);
- FlatView *new_view = generate_memory_topology(physmr);
+ MemoryRegion *physmr = memory_region_get_flatview_root(as->root);
+ FlatView *new_view = g_hash_table_lookup(flat_views, physmr);
+
+ assert(new_view);
+
+ flatview_ref(new_view);
if (!QTAILQ_EMPTY(&as->listeners)) {
address_space_update_topology_pass(as, old_view, new_view, false);
@@ -965,10 +1004,12 @@ void memory_region_transaction_commit(void)
--memory_region_transaction_depth;
if (!memory_region_transaction_depth) {
if (memory_region_update_pending) {
+ flatviews_reset();
+
MEMORY_LISTENER_CALL_GLOBAL(begin, Forward);
QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
- address_space_update_topology(as);
+ address_space_set_flatview(as);
address_space_update_ioeventfds(as);
}
memory_region_update_pending = false;
@@ -2691,13 +2732,6 @@ AddressSpace *address_space_init_shareable(MemoryRegion *root, const char *name)
{
AddressSpace *as;
- QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
- if (root == as->root && as->malloced) {
- as->ref_count++;
- return as;
- }
- }
-
as = g_malloc0(sizeof *as);
address_space_init(as, root, name);
as->malloced = true;
--
2.11.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH qemu v5 13/18] memory: Do not allocate FlatView in address_space_init
2017-09-21 8:50 [Qemu-devel] [PATCH qemu v5 00/18] memory: Store physical root MR in FlatView Alexey Kardashevskiy
` (11 preceding siblings ...)
2017-09-21 8:51 ` [Qemu-devel] [PATCH qemu v5 12/18] memory: Share FlatView's and dispatch trees between address spaces Alexey Kardashevskiy
@ 2017-09-21 8:51 ` Alexey Kardashevskiy
2017-09-21 8:51 ` [Qemu-devel] [PATCH qemu v5 14/18] memory: Rework "info mtree" to print flat views and dispatch trees Alexey Kardashevskiy
` (5 subsequent siblings)
18 siblings, 0 replies; 26+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-21 8:51 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini
This creates a new AS object without any FlatView as
memory_region_transaction_commit() may want to reuse the empty FV.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
I do not really want to add view!=NULL into
flatview_ref()/flatview_unref() as this is quite special case when
view==NULL.
---
memory.c | 29 +++++++++++++++++++++++------
1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/memory.c b/memory.c
index 56b3dd47ab..c623575dac 100644
--- a/memory.c
+++ b/memory.c
@@ -962,22 +962,37 @@ static void flatviews_reset(void)
static void address_space_set_flatview(AddressSpace *as)
{
- FlatView *old_view = address_space_get_flatview(as);
+ FlatView *old_view = address_space_to_flatview(as);
MemoryRegion *physmr = memory_region_get_flatview_root(as->root);
FlatView *new_view = g_hash_table_lookup(flat_views, physmr);
assert(new_view);
+ if (old_view == new_view) {
+ return;
+ }
+
+ if (old_view) {
+ flatview_ref(old_view);
+ }
+
flatview_ref(new_view);
if (!QTAILQ_EMPTY(&as->listeners)) {
- address_space_update_topology_pass(as, old_view, new_view, false);
- address_space_update_topology_pass(as, old_view, new_view, true);
+ FlatView tmpview = { 0 }, *old_view2 = old_view;
+
+ if (!old_view2) {
+ old_view2 = &tmpview;
+ }
+ address_space_update_topology_pass(as, old_view2, new_view, false);
+ address_space_update_topology_pass(as, old_view2, new_view, true);
}
/* Writes are protected by the BQL. */
atomic_rcu_set(&as->current_map, new_view);
- flatview_unref(old_view);
+ if (old_view) {
+ flatview_unref(old_view);
+ }
/* Note that all the old MemoryRegions are still alive up to this
* point. This relieves most MemoryListeners from the need to
@@ -985,7 +1000,9 @@ static void address_space_set_flatview(AddressSpace *as)
* outside the iothread mutex, in which case precise reference
* counting is necessary.
*/
- flatview_unref(old_view);
+ if (old_view) {
+ flatview_unref(old_view);
+ }
}
void memory_region_transaction_begin(void)
@@ -2703,7 +2720,7 @@ void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
as->ref_count = 1;
as->root = root;
as->malloced = false;
- as->current_map = flatview_new(root);
+ as->current_map = NULL;
as->ioeventfd_nb = 0;
as->ioeventfds = NULL;
QTAILQ_INIT(&as->listeners);
--
2.11.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH qemu v5 14/18] memory: Rework "info mtree" to print flat views and dispatch trees
2017-09-21 8:50 [Qemu-devel] [PATCH qemu v5 00/18] memory: Store physical root MR in FlatView Alexey Kardashevskiy
` (12 preceding siblings ...)
2017-09-21 8:51 ` [Qemu-devel] [PATCH qemu v5 13/18] memory: Do not allocate FlatView in address_space_init Alexey Kardashevskiy
@ 2017-09-21 8:51 ` Alexey Kardashevskiy
2017-09-21 8:51 ` [Qemu-devel] [PATCH qemu v5 15/18] memory: Share special empty FlatView Alexey Kardashevskiy
` (4 subsequent siblings)
18 siblings, 0 replies; 26+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-21 8:51 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini
This adds a new "-d" switch to "info mtree" to print dispatch tree
internals.
This changes the way "-f" is handled - it prints now flat views and
associated address spaces.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v3:
* reimplemented as there is no more global FlatView list
---
Example for a suspended (-S) guest with 8 CPUs, 1 XHCI, 1 virtio, 1 E1000:
aik@fstn1-p1:~$ echo "info mtree -f -d" | nc localhost 30000
QEMU 2.10.50 monitor - type 'help' for more information
(qemu) info mtree -f -d
FlatView #0
AS "pci@800000020000000", root: pci@800000020000000.iommu-root
Root memory region: pci@800000020000000.iommu-root
0000000000000000-000000003fffffff (prio 0, i/o): tce-iommu-80000000
0000040000000000-000004000000ffff (prio 0, i/o): msi
Dispatch
Physical sections
#0 @0000000000000000..ffffffffffffffff (noname) [unassigned]
#1 @0000000000000000..ffffffffffffffff (noname) [not dirty]
#2 @0000000000000000..ffffffffffffffff (noname) [ROM]
#3 @0000000000000000..ffffffffffffffff (noname) [watch]
#4 @0000000000000000..000000003fffffff tce-iommu-80000000 [iommu]
#5 @0000040000000000..000004000000ffff msi
Nodes (9 bits per level, 6 levels) ptr=[2] skip=3
[0]
0 skip=2 ptr=[2]
1..511 skip=1 ptr=NIL
[1]
0 skip=1 ptr=[2]
1..511 skip=1 ptr=NIL
[2]
0 skip=0 ptr=#4
1..7 skip=1 ptr=NIL
8 skip=3 ptr=[6]
9..511 skip=1 ptr=NIL
[3]
0 skip=0 ptr=#4
1..511 skip=1 ptr=NIL
[4]
0 skip=2 ptr=[6]
1..511 skip=1 ptr=NIL
[5]
0 skip=1 ptr=[6]
1..511 skip=1 ptr=NIL
[6]
0..15 skip=0 ptr=#5
16..511 skip=0 ptr=#0
FlatView #1
AS "I/O", root: io
Root memory region: io
0000000000000000-000000000000ffff (prio 0, i/o): io
Dispatch
Physical sections
#0 @0000000000000000..ffffffffffffffff (noname) [unassigned]
#1 @0000000000000000..ffffffffffffffff (noname) [not dirty]
#2 @0000000000000000..ffffffffffffffff (noname) [ROM]
#3 @0000000000000000..ffffffffffffffff (noname) [watch]
#4 @0000000000000000..000000000000ffff io [ROOT]
Nodes (9 bits per level, 6 levels) ptr=[5] skip=6
[0]
0 skip=5 ptr=[5]
1..511 skip=1 ptr=NIL
[1]
0 skip=4 ptr=[5]
1..511 skip=1 ptr=NIL
[2]
0 skip=3 ptr=[5]
1..511 skip=1 ptr=NIL
[3]
0 skip=2 ptr=[5]
1..511 skip=1 ptr=NIL
[4]
0 skip=1 ptr=[5]
1..511 skip=1 ptr=NIL
[5]
0..15 skip=0 ptr=#4
16..511 skip=0 ptr=#0
FlatView #2
AS "memory", root: system
AS "cpu-memory", root: system
AS "cpu-memory", root: system
AS "cpu-memory", root: system
AS "cpu-memory", root: system
AS "cpu-memory", root: system
AS "cpu-memory", root: system
AS "cpu-memory", root: system
AS "cpu-memory", root: system
Root memory region: system
0000000000000000-000000007fffffff (prio 0, ram): ppc_spapr.ram
Dispatch
Physical sections
#0 @0000000000000000..ffffffffffffffff (noname) [unassigned]
#1 @0000000000000000..ffffffffffffffff (noname) [not dirty]
#2 @0000000000000000..ffffffffffffffff (noname) [ROM]
#3 @0000000000000000..ffffffffffffffff (noname) [watch]
#4 @0000000000000000..000000007fffffff ppc_spapr.ram [MRU]
Nodes (9 bits per level, 6 levels) ptr=[3] skip=4
[0]
0 skip=3 ptr=[3]
1..511 skip=1 ptr=NIL
[1]
0 skip=2 ptr=[3]
1..511 skip=1 ptr=NIL
[2]
0 skip=1 ptr=[3]
1..511 skip=1 ptr=NIL
[3]
0..1 skip=0 ptr=#4
2..511 skip=1 ptr=NIL
FlatView #3
AS "virtio-net-pci", root: bus master container
AS "e1000", root: bus master container
AS "nec-usb-xhci", root: bus master container
Root memory region: (none)
No rendered FlatView
FlatView #4
AS "virtio-pci-cfg-as", root: virtio-pci-cfg, alias virtio-pci
Root memory region: virtio-pci
0000000000000000-0000000000000fff (prio 0, i/o): virtio-pci-common
0000000000001000-0000000000001fff (prio 0, i/o): virtio-pci-isr
0000000000002000-0000000000002fff (prio 0, i/o): virtio-pci-device
0000000000003000-0000000000003fff (prio 0, i/o): virtio-pci-notify
Dispatch
Physical sections
#0 @0000000000000000..ffffffffffffffff (noname) [unassigned]
#1 @0000000000000000..ffffffffffffffff (noname) [not dirty]
#2 @0000000000000000..ffffffffffffffff (noname) [ROM]
#3 @0000000000000000..ffffffffffffffff (noname) [watch]
#4 @0000000000000000..0000000000000fff virtio-pci-common
#5 @0000000000001000..0000000000001fff virtio-pci-isr
#6 @0000000000002000..0000000000002fff virtio-pci-device
#7 @0000000000003000..0000000000003fff virtio-pci-notify
Nodes (9 bits per level, 6 levels) ptr=[5] skip=6
[0]
0 skip=5 ptr=[5]
1..511 skip=1 ptr=NIL
[1]
0 skip=4 ptr=[5]
1..511 skip=1 ptr=NIL
[2]
0 skip=3 ptr=[5]
1..511 skip=1 ptr=NIL
[3]
0 skip=2 ptr=[5]
1..511 skip=1 ptr=NIL
[4]
0 skip=1 ptr=[5]
1..511 skip=1 ptr=NIL
[5]
0 skip=0 ptr=#4
1 skip=0 ptr=#5
2 skip=0 ptr=#6
3 skip=0 ptr=#7
4..511 skip=0 ptr=#0
(qemu)
---
include/exec/memory-internal.h | 4 ++
include/exec/memory.h | 3 +-
exec.c | 84 +++++++++++++++++++++++++++++++++++++++
memory.c | 90 +++++++++++++++++++++++++++++++++++++-----
monitor.c | 3 +-
hmp-commands-info.hx | 7 ++--
6 files changed, 176 insertions(+), 15 deletions(-)
diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index d4a35c6e96..647e9bd5c4 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -35,5 +35,9 @@ AddressSpaceDispatch *address_space_to_dispatch(AddressSpace *as);
AddressSpaceDispatch *flatview_to_dispatch(FlatView *fv);
void address_space_dispatch_free(AddressSpaceDispatch *d);
+void mtree_print_dispatch(fprintf_function mon, void *f,
+ struct AddressSpaceDispatch *d,
+ MemoryRegion *root);
+
#endif
#endif
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 7816e5d655..2f4f56cf40 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1515,7 +1515,8 @@ void memory_global_dirty_log_start(void);
*/
void memory_global_dirty_log_stop(void);
-void mtree_info(fprintf_function mon_printf, void *f, bool flatview);
+void mtree_info(fprintf_function mon_printf, void *f, bool flatview,
+ bool dispatch_tree);
/**
* memory_region_request_mmio_ptr: request a pointer to an mmio
diff --git a/exec.c b/exec.c
index b085f82503..7a80460725 100644
--- a/exec.c
+++ b/exec.c
@@ -3616,3 +3616,87 @@ void page_size_init(void)
}
qemu_host_page_mask = -(intptr_t)qemu_host_page_size;
}
+
+#if !defined(CONFIG_USER_ONLY)
+
+static void mtree_print_phys_entries(fprintf_function mon, void *f,
+ int start, int end, int skip, int ptr)
+{
+ if (start == end - 1) {
+ mon(f, "\t%3d ", start);
+ } else {
+ mon(f, "\t%3d..%-3d ", start, end - 1);
+ }
+ mon(f, " skip=%d ", skip);
+ if (ptr == PHYS_MAP_NODE_NIL) {
+ mon(f, " ptr=NIL");
+ } else if (!skip) {
+ mon(f, " ptr=#%d", ptr);
+ } else {
+ mon(f, " ptr=[%d]", ptr);
+ }
+ mon(f, "\n");
+}
+
+#define MR_SIZE(size) (int128_nz(size) ? (hwaddr)int128_get64( \
+ int128_sub((size), int128_one())) : 0)
+
+void mtree_print_dispatch(fprintf_function mon, void *f,
+ AddressSpaceDispatch *d, MemoryRegion *root)
+{
+ int i;
+
+ mon(f, " Dispatch\n");
+ mon(f, " Physical sections\n");
+
+ for (i = 0; i < d->map.sections_nb; ++i) {
+ MemoryRegionSection *s = d->map.sections + i;
+ const char *names[] = { " [unassigned]", " [not dirty]",
+ " [ROM]", " [watch]" };
+
+ mon(f, " #%d @" TARGET_FMT_plx ".." TARGET_FMT_plx " %s%s%s%s%s",
+ i,
+ s->offset_within_address_space,
+ s->offset_within_address_space + MR_SIZE(s->mr->size),
+ s->mr->name ? s->mr->name : "(noname)",
+ i < ARRAY_SIZE(names) ? names[i] : "",
+ s->mr == root ? " [ROOT]" : "",
+ s == d->mru_section ? " [MRU]" : "",
+ s->mr->is_iommu ? " [iommu]" : "");
+
+ if (s->mr->alias) {
+ mon(f, " alias=%s", s->mr->alias->name ?
+ s->mr->alias->name : "noname");
+ }
+ mon(f, "\n");
+ }
+
+ mon(f, " Nodes (%d bits per level, %d levels) ptr=[%d] skip=%d\n",
+ P_L2_BITS, P_L2_LEVELS, d->phys_map.ptr, d->phys_map.skip);
+ for (i = 0; i < d->map.nodes_nb; ++i) {
+ int j, jprev;
+ PhysPageEntry prev;
+ Node *n = d->map.nodes + i;
+
+ mon(f, " [%d]\n", i);
+
+ for (j = 0, jprev = 0, prev = *n[0]; j < ARRAY_SIZE(*n); ++j) {
+ PhysPageEntry *pe = *n + j;
+
+ if (pe->ptr == prev.ptr && pe->skip == prev.skip) {
+ continue;
+ }
+
+ mtree_print_phys_entries(mon, f, jprev, j, prev.skip, prev.ptr);
+
+ jprev = j;
+ prev = *pe;
+ }
+
+ if (jprev != ARRAY_SIZE(*n)) {
+ mtree_print_phys_entries(mon, f, jprev, j, prev.skip, prev.ptr);
+ }
+ }
+}
+
+#endif
diff --git a/memory.c b/memory.c
index c623575dac..8ab953ba24 100644
--- a/memory.c
+++ b/memory.c
@@ -2902,18 +2902,44 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f,
}
}
-static void mtree_print_flatview(fprintf_function p, void *f,
- AddressSpace *as)
+struct FlatViewInfo {
+ fprintf_function mon_printf;
+ void *f;
+ int counter;
+ bool dispatch_tree;
+};
+
+static void mtree_print_flatview(gpointer key, gpointer value,
+ gpointer user_data)
{
- FlatView *view = address_space_get_flatview(as);
+ FlatView *view = key;
+ GArray *fv_address_spaces = value;
+ struct FlatViewInfo *fvi = user_data;
+ fprintf_function p = fvi->mon_printf;
+ void *f = fvi->f;
FlatRange *range = &view->ranges[0];
MemoryRegion *mr;
int n = view->nr;
+ int i;
+ AddressSpace *as;
+
+ p(f, "FlatView #%d\n", fvi->counter);
+ ++fvi->counter;
+
+ for (i = 0; i < fv_address_spaces->len; ++i) {
+ as = g_array_index(fv_address_spaces, AddressSpace*, i);
+ p(f, " AS \"%s\", root: %s", as->name, memory_region_name(as->root));
+ if (as->root->alias) {
+ p(f, ", alias %s", memory_region_name(as->root->alias));
+ }
+ p(f, "\n");
+ }
+
+ p(f, " Root memory region: %s\n",
+ view->root ? memory_region_name(view->root) : "(none)");
if (n <= 0) {
- p(f, MTREE_INDENT "No rendered FlatView for "
- "address space '%s'\n", as->name);
- flatview_unref(view);
+ p(f, MTREE_INDENT "No rendered FlatView\n\n");
return;
}
@@ -2940,21 +2966,65 @@ static void mtree_print_flatview(fprintf_function p, void *f,
range++;
}
+#if !defined(CONFIG_USER_ONLY)
+ if (fvi->dispatch_tree && view->root) {
+ mtree_print_dispatch(p, f, view->dispatch, view->root);
+ }
+#endif
+
+ p(f, "\n");
+}
+
+static gboolean mtree_info_flatview_free(gpointer key, gpointer value,
+ gpointer user_data)
+{
+ FlatView *view = key;
+ GArray *fv_address_spaces = value;
+
+ g_array_unref(fv_address_spaces);
flatview_unref(view);
+
+ return true;
}
-void mtree_info(fprintf_function mon_printf, void *f, bool flatview)
+void mtree_info(fprintf_function mon_printf, void *f, bool flatview,
+ bool dispatch_tree)
{
MemoryRegionListHead ml_head;
MemoryRegionList *ml, *ml2;
AddressSpace *as;
if (flatview) {
+ FlatView *view;
+ struct FlatViewInfo fvi = {
+ .mon_printf = mon_printf,
+ .f = f,
+ .counter = 0,
+ .dispatch_tree = dispatch_tree
+ };
+ GArray *fv_address_spaces;
+ GHashTable *views = g_hash_table_new(g_direct_hash, g_direct_equal);
+
+ /* Gather all FVs in one table */
QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
- mon_printf(f, "address-space (flat view): %s\n", as->name);
- mtree_print_flatview(mon_printf, f, as);
- mon_printf(f, "\n");
+ view = address_space_get_flatview(as);
+
+ fv_address_spaces = g_hash_table_lookup(views, view);
+ if (!fv_address_spaces) {
+ fv_address_spaces = g_array_new(false, false, sizeof(as));
+ g_hash_table_insert(views, view, fv_address_spaces);
+ }
+
+ g_array_append_val(fv_address_spaces, as);
}
+
+ /* Print */
+ g_hash_table_foreach(views, mtree_print_flatview, &fvi);
+
+ /* Free */
+ g_hash_table_foreach_remove(views, mtree_info_flatview_free, 0);
+ g_hash_table_unref(views);
+
return;
}
diff --git a/monitor.c b/monitor.c
index 058045b3cb..f4856b9268 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1703,8 +1703,9 @@ static void hmp_boot_set(Monitor *mon, const QDict *qdict)
static void hmp_info_mtree(Monitor *mon, const QDict *qdict)
{
bool flatview = qdict_get_try_bool(qdict, "flatview", false);
+ bool dispatch_tree = qdict_get_try_bool(qdict, "dispatch_tree", false);
- mtree_info((fprintf_function)monitor_printf, mon, flatview);
+ mtree_info((fprintf_function)monitor_printf, mon, flatview, dispatch_tree);
}
static void hmp_info_numa(Monitor *mon, const QDict *qdict)
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 1c6772597d..4f1ece93e5 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -250,9 +250,10 @@ ETEXI
{
.name = "mtree",
- .args_type = "flatview:-f",
- .params = "[-f]",
- .help = "show memory tree (-f: dump flat view for address spaces)",
+ .args_type = "flatview:-f,dispatch_tree:-d",
+ .params = "[-f][-d]",
+ .help = "show memory tree (-f: dump flat view for address spaces;"
+ "-d: dump dispatch tree, valid with -f only)",
.cmd = hmp_info_mtree,
},
--
2.11.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH qemu v5 15/18] memory: Share special empty FlatView
2017-09-21 8:50 [Qemu-devel] [PATCH qemu v5 00/18] memory: Store physical root MR in FlatView Alexey Kardashevskiy
` (13 preceding siblings ...)
2017-09-21 8:51 ` [Qemu-devel] [PATCH qemu v5 14/18] memory: Rework "info mtree" to print flat views and dispatch trees Alexey Kardashevskiy
@ 2017-09-21 8:51 ` Alexey Kardashevskiy
2017-09-21 8:51 ` [Qemu-devel] [PATCH qemu v5 16/18] memory: Get rid of address_space_init_shareable Alexey Kardashevskiy
` (3 subsequent siblings)
18 siblings, 0 replies; 26+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-21 8:51 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini
This shares an cached empty FlatView among address spaces. The empty
FV is used every time when a root MR renders into a FV without memory
sections which happens when MR or its children are not enabled or
zero-sized. The empty_view is not NULL to keep the rest of memory
API intact; it also has a dispatch tree for the same reason.
On POWER8 with 255 CPUs, 255 virtio-net, 40 PCI bridges guest this halves
the amount of FlatView's in use (557 -> 260) and dispatch tables
(~800000 -> ~370000), however the total memory footprint is pretty much
the same as RCU is holding all these temporary FVs which are created
(and then released) to make sure that they are empty and can be replaced
with @empty_view.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v5:
* generate_memory_topology() now destroys temporary FV directly as
it is not in use anyway
* check for mr->enabled and avoid even temporaty FV allocation
---
memory.c | 44 +++++++++++++++++++++++++++++++++-----------
1 file changed, 33 insertions(+), 11 deletions(-)
diff --git a/memory.c b/memory.c
index 8ab953ba24..4c28b91890 100644
--- a/memory.c
+++ b/memory.c
@@ -48,6 +48,7 @@ static QTAILQ_HEAD(, AddressSpace) address_spaces
= QTAILQ_HEAD_INITIALIZER(address_spaces);
static GHashTable *flat_views;
+static FlatView *empty_view;
typedef struct AddrRange AddrRange;
@@ -746,22 +747,43 @@ static FlatView *generate_memory_topology(MemoryRegion *mr)
{
int i;
FlatView *view;
+ bool use_empty = false;
- view = flatview_new(mr);
+ if (!mr->enabled) {
+ use_empty = true;
+ } else {
+ view = flatview_new(mr);
+ if (mr) {
+ render_memory_region(view, mr, int128_zero(),
+ addrrange_make(int128_zero(), int128_2_64()),
+ false);
+ }
+ flatview_simplify(view);
- if (mr) {
- render_memory_region(view, mr, int128_zero(),
- addrrange_make(int128_zero(), int128_2_64()), false);
+ if (!view->nr) {
+ flatview_destroy(view);
+ use_empty = true;
+ }
}
- flatview_simplify(view);
- view->dispatch = address_space_dispatch_new(view);
- for (i = 0; i < view->nr; i++) {
- MemoryRegionSection mrs =
- section_from_flat_range(&view->ranges[i], view);
- flatview_add_to_dispatch(view, &mrs);
+ if (use_empty) {
+ if (!empty_view) {
+ empty_view = flatview_new(NULL);
+ }
+ view = empty_view;
+ flatview_ref(view);
}
- address_space_dispatch_compact(view->dispatch);
+
+ if (!view->dispatch) {
+ view->dispatch = address_space_dispatch_new(view);
+ for (i = 0; i < view->nr; i++) {
+ MemoryRegionSection mrs =
+ section_from_flat_range(&view->ranges[i], view);
+ flatview_add_to_dispatch(view, &mrs);
+ }
+ address_space_dispatch_compact(view->dispatch);
+ }
+
g_hash_table_replace(flat_views, mr, view);
return view;
--
2.11.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH qemu v5 16/18] memory: Get rid of address_space_init_shareable
2017-09-21 8:50 [Qemu-devel] [PATCH qemu v5 00/18] memory: Store physical root MR in FlatView Alexey Kardashevskiy
` (14 preceding siblings ...)
2017-09-21 8:51 ` [Qemu-devel] [PATCH qemu v5 15/18] memory: Share special empty FlatView Alexey Kardashevskiy
@ 2017-09-21 8:51 ` Alexey Kardashevskiy
2017-09-21 23:06 ` Paolo Bonzini
2017-09-21 8:51 ` [Qemu-devel] [PATCH qemu v5 17/18] memory: Create FlatView directly Alexey Kardashevskiy
` (2 subsequent siblings)
18 siblings, 1 reply; 26+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-21 8:51 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini
Since FlatViews are shared now and ASes not, this gets rid of
address_space_init_shareable().
This should cause no behavioural change.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v3:
* now removes @malloced and @ref_count, used to be in the previos patch
---
include/exec/memory.h | 19 -------------------
include/hw/arm/armv7m.h | 2 +-
cpus.c | 5 +++--
hw/arm/armv7m.c | 9 ++++-----
memory.c | 22 +---------------------
target/arm/cpu.c | 16 ++++++++--------
target/i386/cpu.c | 5 +++--
7 files changed, 20 insertions(+), 58 deletions(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 2f4f56cf40..402824c6f2 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -309,8 +309,6 @@ struct AddressSpace {
struct rcu_head rcu;
char *name;
MemoryRegion *root;
- int ref_count;
- bool malloced;
/* Accessed via RCU. */
struct FlatView *current_map;
@@ -1586,23 +1584,6 @@ MemTxResult memory_region_dispatch_write(MemoryRegion *mr,
void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name);
/**
- * address_space_init_shareable: return an address space for a memory region,
- * creating it if it does not already exist
- *
- * @root: a #MemoryRegion that routes addresses for the address space
- * @name: an address space name. The name is only used for debugging
- * output.
- *
- * This function will return a pointer to an existing AddressSpace
- * which was initialized with the specified MemoryRegion, or it will
- * create and initialize one if it does not already exist. The ASes
- * are reference-counted, so the memory will be freed automatically
- * when the AddressSpace is destroyed via address_space_destroy.
- */
-AddressSpace *address_space_init_shareable(MemoryRegion *root,
- const char *name);
-
-/**
* address_space_destroy: destroy an address space
*
* Releases all resources associated with an address space. After an address space
diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h
index 10eb058027..008000fe11 100644
--- a/include/hw/arm/armv7m.h
+++ b/include/hw/arm/armv7m.h
@@ -21,7 +21,7 @@ typedef struct {
SysBusDevice parent_obj;
/*< public >*/
- AddressSpace *source_as;
+ AddressSpace source_as;
MemoryRegion iomem;
uint32_t base;
MemoryRegion *source_memory;
diff --git a/cpus.c b/cpus.c
index 9bed61eefc..c9a624003a 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1764,8 +1764,9 @@ void qemu_init_vcpu(CPUState *cpu)
/* If the target cpu hasn't set up any address spaces itself,
* give it the default one.
*/
- AddressSpace *as = address_space_init_shareable(cpu->memory,
- "cpu-memory");
+ AddressSpace *as = g_new0(AddressSpace, 1);
+
+ address_space_init(as, cpu->memory, "cpu-memory");
cpu->num_ases = 1;
cpu_address_space_init(cpu, as, 0);
}
diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index b64a409b40..4900339646 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -41,7 +41,7 @@ static MemTxResult bitband_read(void *opaque, hwaddr offset,
/* Find address in underlying memory and round down to multiple of size */
addr = bitband_addr(s, offset) & (-size);
- res = address_space_read(s->source_as, addr, attrs, buf, size);
+ res = address_space_read(&s->source_as, addr, attrs, buf, size);
if (res) {
return res;
}
@@ -66,7 +66,7 @@ static MemTxResult bitband_write(void *opaque, hwaddr offset, uint64_t value,
/* Find address in underlying memory and round down to multiple of size */
addr = bitband_addr(s, offset) & (-size);
- res = address_space_read(s->source_as, addr, attrs, buf, size);
+ res = address_space_read(&s->source_as, addr, attrs, buf, size);
if (res) {
return res;
}
@@ -79,7 +79,7 @@ static MemTxResult bitband_write(void *opaque, hwaddr offset, uint64_t value,
} else {
buf[bitpos >> 3] &= ~bit;
}
- return address_space_write(s->source_as, addr, attrs, buf, size);
+ return address_space_write(&s->source_as, addr, attrs, buf, size);
}
static const MemoryRegionOps bitband_ops = {
@@ -111,8 +111,7 @@ static void bitband_realize(DeviceState *dev, Error **errp)
return;
}
- s->source_as = address_space_init_shareable(s->source_memory,
- "bitband-source");
+ address_space_init(&s->source_as, s->source_memory, "bitband-source");
}
/* Board init. */
diff --git a/memory.c b/memory.c
index 4c28b91890..57e47e990f 100644
--- a/memory.c
+++ b/memory.c
@@ -2739,9 +2739,7 @@ void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
{
memory_region_ref(root);
memory_region_transaction_begin();
- as->ref_count = 1;
as->root = root;
- as->malloced = false;
as->current_map = NULL;
as->ioeventfd_nb = 0;
as->ioeventfds = NULL;
@@ -2754,37 +2752,19 @@ void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
static void do_address_space_destroy(AddressSpace *as)
{
- bool do_free = as->malloced;
-
assert(QTAILQ_EMPTY(&as->listeners));
flatview_unref(as->current_map);
g_free(as->name);
g_free(as->ioeventfds);
memory_region_unref(as->root);
- if (do_free) {
- g_free(as);
- }
-}
-
-AddressSpace *address_space_init_shareable(MemoryRegion *root, const char *name)
-{
- AddressSpace *as;
-
- as = g_malloc0(sizeof *as);
- address_space_init(as, root, name);
- as->malloced = true;
- return as;
+ g_free(as);
}
void address_space_destroy(AddressSpace *as)
{
MemoryRegion *root = as->root;
- as->ref_count--;
- if (as->ref_count) {
- return;
- }
/* Flush out anything from MemoryListeners listening in on this */
memory_region_transaction_begin();
as->root = NULL;
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 412e94c7ad..bba24f4590 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -684,6 +684,9 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
CPUARMState *env = &cpu->env;
int pagebits;
Error *local_err = NULL;
+#ifndef CONFIG_USER_ONLY
+ AddressSpace *as;
+#endif
cpu_exec_realizefn(cs, &local_err);
if (local_err != NULL) {
@@ -874,24 +877,21 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
#ifndef CONFIG_USER_ONLY
if (cpu->has_el3 || arm_feature(env, ARM_FEATURE_M_SECURITY)) {
- AddressSpace *as;
+ as = g_new0(AddressSpace, 1);
cs->num_ases = 2;
if (!cpu->secure_memory) {
cpu->secure_memory = cs->memory;
}
- as = address_space_init_shareable(cpu->secure_memory,
- "cpu-secure-memory");
+ address_space_init(as, cpu->secure_memory, "cpu-secure-memory");
cpu_address_space_init(cs, as, ARMASIdx_S);
} else {
cs->num_ases = 1;
}
-
- cpu_address_space_init(cs,
- address_space_init_shareable(cs->memory,
- "cpu-memory"),
- ARMASIdx_NS);
+ as = g_new0(AddressSpace, 1);
+ address_space_init(as, cs->memory, "cpu-memory");
+ cpu_address_space_init(cs, as, ARMASIdx_NS);
#endif
qemu_init_vcpu(cs);
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 4b0fa0613b..b0b123a571 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3741,10 +3741,11 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
#ifndef CONFIG_USER_ONLY
if (tcg_enabled()) {
- AddressSpace *as_normal = address_space_init_shareable(cs->memory,
- "cpu-memory");
+ AddressSpace *as_normal = g_new0(AddressSpace, 1);
AddressSpace *as_smm = g_new(AddressSpace, 1);
+ address_space_init(as_normal, cs->memory, "cpu-memory");
+
cpu->cpu_as_mem = g_new(MemoryRegion, 1);
cpu->cpu_as_root = g_new(MemoryRegion, 1);
--
2.11.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [Qemu-devel] [PATCH qemu v5 16/18] memory: Get rid of address_space_init_shareable
2017-09-21 8:51 ` [Qemu-devel] [PATCH qemu v5 16/18] memory: Get rid of address_space_init_shareable Alexey Kardashevskiy
@ 2017-09-21 23:06 ` Paolo Bonzini
0 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2017-09-21 23:06 UTC (permalink / raw)
To: Alexey Kardashevskiy, qemu-devel
On 21/09/2017 10:51, Alexey Kardashevskiy wrote:
> Since FlatViews are shared now and ASes not, this gets rid of
> address_space_init_shareable().
>
> This should cause no behavioural change.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> Changes:
> v3:
> * now removes @malloced and @ref_count, used to be in the previos patch
> ---
> include/exec/memory.h | 19 -------------------
> include/hw/arm/armv7m.h | 2 +-
> cpus.c | 5 +++--
> hw/arm/armv7m.c | 9 ++++-----
> memory.c | 22 +---------------------
> target/arm/cpu.c | 16 ++++++++--------
> target/i386/cpu.c | 5 +++--
> 7 files changed, 20 insertions(+), 58 deletions(-)
>
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 2f4f56cf40..402824c6f2 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -309,8 +309,6 @@ struct AddressSpace {
> struct rcu_head rcu;
> char *name;
> MemoryRegion *root;
> - int ref_count;
> - bool malloced;
>
> /* Accessed via RCU. */
> struct FlatView *current_map;
> @@ -1586,23 +1584,6 @@ MemTxResult memory_region_dispatch_write(MemoryRegion *mr,
> void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name);
>
> /**
> - * address_space_init_shareable: return an address space for a memory region,
> - * creating it if it does not already exist
> - *
> - * @root: a #MemoryRegion that routes addresses for the address space
> - * @name: an address space name. The name is only used for debugging
> - * output.
> - *
> - * This function will return a pointer to an existing AddressSpace
> - * which was initialized with the specified MemoryRegion, or it will
> - * create and initialize one if it does not already exist. The ASes
> - * are reference-counted, so the memory will be freed automatically
> - * when the AddressSpace is destroyed via address_space_destroy.
> - */
> -AddressSpace *address_space_init_shareable(MemoryRegion *root,
> - const char *name);
> -
> -/**
> * address_space_destroy: destroy an address space
> *
> * Releases all resources associated with an address space. After an address space
> diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h
> index 10eb058027..008000fe11 100644
> --- a/include/hw/arm/armv7m.h
> +++ b/include/hw/arm/armv7m.h
> @@ -21,7 +21,7 @@ typedef struct {
> SysBusDevice parent_obj;
> /*< public >*/
>
> - AddressSpace *source_as;
> + AddressSpace source_as;
> MemoryRegion iomem;
> uint32_t base;
> MemoryRegion *source_memory;
> diff --git a/cpus.c b/cpus.c
> index 9bed61eefc..c9a624003a 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -1764,8 +1764,9 @@ void qemu_init_vcpu(CPUState *cpu)
> /* If the target cpu hasn't set up any address spaces itself,
> * give it the default one.
> */
> - AddressSpace *as = address_space_init_shareable(cpu->memory,
> - "cpu-memory");
> + AddressSpace *as = g_new0(AddressSpace, 1);
> +
> + address_space_init(as, cpu->memory, "cpu-memory");
> cpu->num_ases = 1;
> cpu_address_space_init(cpu, as, 0);
> }
> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> index b64a409b40..4900339646 100644
> --- a/hw/arm/armv7m.c
> +++ b/hw/arm/armv7m.c
> @@ -41,7 +41,7 @@ static MemTxResult bitband_read(void *opaque, hwaddr offset,
>
> /* Find address in underlying memory and round down to multiple of size */
> addr = bitband_addr(s, offset) & (-size);
> - res = address_space_read(s->source_as, addr, attrs, buf, size);
> + res = address_space_read(&s->source_as, addr, attrs, buf, size);
> if (res) {
> return res;
> }
> @@ -66,7 +66,7 @@ static MemTxResult bitband_write(void *opaque, hwaddr offset, uint64_t value,
>
> /* Find address in underlying memory and round down to multiple of size */
> addr = bitband_addr(s, offset) & (-size);
> - res = address_space_read(s->source_as, addr, attrs, buf, size);
> + res = address_space_read(&s->source_as, addr, attrs, buf, size);
> if (res) {
> return res;
> }
> @@ -79,7 +79,7 @@ static MemTxResult bitband_write(void *opaque, hwaddr offset, uint64_t value,
> } else {
> buf[bitpos >> 3] &= ~bit;
> }
> - return address_space_write(s->source_as, addr, attrs, buf, size);
> + return address_space_write(&s->source_as, addr, attrs, buf, size);
> }
>
> static const MemoryRegionOps bitband_ops = {
> @@ -111,8 +111,7 @@ static void bitband_realize(DeviceState *dev, Error **errp)
> return;
> }
>
> - s->source_as = address_space_init_shareable(s->source_memory,
> - "bitband-source");
> + address_space_init(&s->source_as, s->source_memory, "bitband-source");
> }
>
> /* Board init. */
> diff --git a/memory.c b/memory.c
> index 4c28b91890..57e47e990f 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -2739,9 +2739,7 @@ void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
> {
> memory_region_ref(root);
> memory_region_transaction_begin();
> - as->ref_count = 1;
> as->root = root;
> - as->malloced = false;
> as->current_map = NULL;
> as->ioeventfd_nb = 0;
> as->ioeventfds = NULL;
> @@ -2754,37 +2752,19 @@ void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
>
> static void do_address_space_destroy(AddressSpace *as)
> {
> - bool do_free = as->malloced;
> -
> assert(QTAILQ_EMPTY(&as->listeners));
>
> flatview_unref(as->current_map);
> g_free(as->name);
> g_free(as->ioeventfds);
> memory_region_unref(as->root);
> - if (do_free) {
> - g_free(as);
> - }
> -}
> -
> -AddressSpace *address_space_init_shareable(MemoryRegion *root, const char *name)
> -{
> - AddressSpace *as;
> -
> - as = g_malloc0(sizeof *as);
> - address_space_init(as, root, name);
> - as->malloced = true;
> - return as;
> + g_free(as);
This g_free is wrong; /i386/virtio/blk/pci/hotplug in
tests/virtio-blk-test is a pretty reliable reproducer.
Paolo
> }
>
> void address_space_destroy(AddressSpace *as)
> {
> MemoryRegion *root = as->root;
>
> - as->ref_count--;
> - if (as->ref_count) {
> - return;
> - }
> /* Flush out anything from MemoryListeners listening in on this */
> memory_region_transaction_begin();
> as->root = NULL;
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 412e94c7ad..bba24f4590 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -684,6 +684,9 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
> CPUARMState *env = &cpu->env;
> int pagebits;
> Error *local_err = NULL;
> +#ifndef CONFIG_USER_ONLY
> + AddressSpace *as;
> +#endif
>
> cpu_exec_realizefn(cs, &local_err);
> if (local_err != NULL) {
> @@ -874,24 +877,21 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
>
> #ifndef CONFIG_USER_ONLY
> if (cpu->has_el3 || arm_feature(env, ARM_FEATURE_M_SECURITY)) {
> - AddressSpace *as;
> + as = g_new0(AddressSpace, 1);
>
> cs->num_ases = 2;
>
> if (!cpu->secure_memory) {
> cpu->secure_memory = cs->memory;
> }
> - as = address_space_init_shareable(cpu->secure_memory,
> - "cpu-secure-memory");
> + address_space_init(as, cpu->secure_memory, "cpu-secure-memory");
> cpu_address_space_init(cs, as, ARMASIdx_S);
> } else {
> cs->num_ases = 1;
> }
> -
> - cpu_address_space_init(cs,
> - address_space_init_shareable(cs->memory,
> - "cpu-memory"),
> - ARMASIdx_NS);
> + as = g_new0(AddressSpace, 1);
> + address_space_init(as, cs->memory, "cpu-memory");
> + cpu_address_space_init(cs, as, ARMASIdx_NS);
> #endif
>
> qemu_init_vcpu(cs);
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 4b0fa0613b..b0b123a571 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -3741,10 +3741,11 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>
> #ifndef CONFIG_USER_ONLY
> if (tcg_enabled()) {
> - AddressSpace *as_normal = address_space_init_shareable(cs->memory,
> - "cpu-memory");
> + AddressSpace *as_normal = g_new0(AddressSpace, 1);
> AddressSpace *as_smm = g_new(AddressSpace, 1);
>
> + address_space_init(as_normal, cs->memory, "cpu-memory");
> +
> cpu->cpu_as_mem = g_new(MemoryRegion, 1);
> cpu->cpu_as_root = g_new(MemoryRegion, 1);
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH qemu v5 17/18] memory: Create FlatView directly
2017-09-21 8:50 [Qemu-devel] [PATCH qemu v5 00/18] memory: Store physical root MR in FlatView Alexey Kardashevskiy
` (15 preceding siblings ...)
2017-09-21 8:51 ` [Qemu-devel] [PATCH qemu v5 16/18] memory: Get rid of address_space_init_shareable Alexey Kardashevskiy
@ 2017-09-21 8:51 ` Alexey Kardashevskiy
2017-09-21 8:51 ` [Qemu-devel] [PATCH qemu v5 18/18] memory: Avoid temporary FlatView allocation in a single child case Alexey Kardashevskiy
2017-09-21 10:03 ` [Qemu-devel] [PATCH qemu v5 00/18] memory: Store physical root MR in FlatView Paolo Bonzini
18 siblings, 0 replies; 26+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-21 8:51 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini
This avoids usual memory_region_transaction_commit() which rebuilds
all FVs.
On POWER8 with 255 CPUs, 255 virtio-net, 40 PCI bridges guest this brings
down the boot time from 25s to 20s and reduces the amount of temporary FVs
allocated during machine constructon (~800000 -> ~640000) and amount of
temporary dispatch trees (~370000 -> ~300000), the total memory footprint
goes down (18G -> 17G).
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v5:
* %s/flatview_update_topology_single/address_space_update_topology/g
* fixup after moving g_hash_table_replace() to generate_memory_topology()
---
memory.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/memory.c b/memory.c
index 57e47e990f..5c21563745 100644
--- a/memory.c
+++ b/memory.c
@@ -1027,6 +1027,17 @@ static void address_space_set_flatview(AddressSpace *as)
}
}
+static void address_space_update_topology(AddressSpace *as)
+{
+ MemoryRegion *physmr = memory_region_get_flatview_root(as->root);
+
+ flatviews_init();
+ if (!g_hash_table_lookup(flat_views, physmr)) {
+ generate_memory_topology(physmr);
+ }
+ address_space_set_flatview(as);
+}
+
void memory_region_transaction_begin(void)
{
qemu_flush_coalesced_mmio_buffer();
@@ -2738,7 +2749,6 @@ void memory_region_invalidate_mmio_ptr(MemoryRegion *mr, hwaddr offset,
void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
{
memory_region_ref(root);
- memory_region_transaction_begin();
as->root = root;
as->current_map = NULL;
as->ioeventfd_nb = 0;
@@ -2746,8 +2756,7 @@ void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
QTAILQ_INIT(&as->listeners);
QTAILQ_INSERT_TAIL(&address_spaces, as, address_spaces_link);
as->name = g_strdup(name ? name : "anonymous");
- memory_region_update_pending |= root->enabled;
- memory_region_transaction_commit();
+ address_space_update_topology(as);
}
static void do_address_space_destroy(AddressSpace *as)
--
2.11.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH qemu v5 18/18] memory: Avoid temporary FlatView allocation in a single child case
2017-09-21 8:50 [Qemu-devel] [PATCH qemu v5 00/18] memory: Store physical root MR in FlatView Alexey Kardashevskiy
` (16 preceding siblings ...)
2017-09-21 8:51 ` [Qemu-devel] [PATCH qemu v5 17/18] memory: Create FlatView directly Alexey Kardashevskiy
@ 2017-09-21 8:51 ` Alexey Kardashevskiy
2017-09-21 10:03 ` [Qemu-devel] [PATCH qemu v5 00/18] memory: Store physical root MR in FlatView Paolo Bonzini
18 siblings, 0 replies; 26+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-21 8:51 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini
The root MR may be enabled but the only child may be not (this is
the case for the PCI bus master address space) so check this and avoid
allocating temporary FV if that nested MR is not enabled.
This does not make any difference though.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
memory.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/memory.c b/memory.c
index 5c21563745..bf71f19fec 100644
--- a/memory.c
+++ b/memory.c
@@ -748,9 +748,14 @@ static FlatView *generate_memory_topology(MemoryRegion *mr)
int i;
FlatView *view;
bool use_empty = false;
+ MemoryRegion *child = QTAILQ_FIRST(&mr->subregions);
if (!mr->enabled) {
use_empty = true;
+ } else if (child && !child->enabled &&
+ !QTAILQ_NEXT(child, subregions_link) &&
+ !child->addr && int128_eq(child->size, mr->size)) {
+ use_empty = true;
} else {
view = flatview_new(mr);
if (mr) {
--
2.11.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [Qemu-devel] [PATCH qemu v5 00/18] memory: Store physical root MR in FlatView
2017-09-21 8:50 [Qemu-devel] [PATCH qemu v5 00/18] memory: Store physical root MR in FlatView Alexey Kardashevskiy
` (17 preceding siblings ...)
2017-09-21 8:51 ` [Qemu-devel] [PATCH qemu v5 18/18] memory: Avoid temporary FlatView allocation in a single child case Alexey Kardashevskiy
@ 2017-09-21 10:03 ` Paolo Bonzini
2017-09-21 10:11 ` Alexey Kardashevskiy
18 siblings, 1 reply; 26+ messages in thread
From: Paolo Bonzini @ 2017-09-21 10:03 UTC (permalink / raw)
To: Alexey Kardashevskiy, qemu-devel
On 21/09/2017 10:50, Alexey Kardashevskiy wrote:
> This was inspired by https://bugzilla.redhat.com/show_bug.cgi?id=1481593
> Previous versions:
> v1: https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg01559.html
> v2: https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg04069.html
> v3: https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg04523.html
> v4: https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg05669.html
>
> This patchset tries to reduce amount of memory used by FlatViews
> and tries share as many FVs between address spaces as possible.
Thanks, this looks good! (Patch 18 indeed is not what I had in mind :)).
Paolo
> Changelog:
> v5:
> * removed "memory: Give memory_region_transaction_commit a hint" (broken)
> * added "memory: Avoid temporary FlatView allocation in a single child case"
> but I suggest ditching it, it is just there to demonsrate how I checked
> the suggested idea
> * updated example of "info mtree -f -d" in 14/18 to demonstrate the result
>
> v4:
> * total rework again to provide grounds for new 15/18, 17/18, 18/18
>
> v3:
> * addressed comments from v2, mainly simplified the code
>
> v2:
> * total rework
>
>
> This is based on sha1
> c51700273a Peter Maydell "Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20170919-v2' into staging".
>
> Please comment. Thanks.
>
>
>
> Alexey Kardashevskiy (18):
> exec: Explicitly export target AS from
> address_space_translate_internal
> memory: Open code FlatView rendering
> memory: Move FlatView allocation to a helper
> memory: Move AddressSpaceDispatch from AddressSpace to FlatView
> memory: Remove AddressSpace pointer from AddressSpaceDispatch
> memory: Switch memory from using AddressSpace to FlatView
> memory: Cleanup after switching to FlatView
> memory: Rename mem_begin/mem_commit/mem_add helpers
> memory: Store physical root MR in FlatView
> memory: Alloc dispatch tree where topology is generared
> memory: Move address_space_update_ioeventfds
> memory: Share FlatView's and dispatch trees between address spaces
> memory: Do not allocate FlatView in address_space_init
> memory: Rework "info mtree" to print flat views and dispatch trees
> memory: Share special empty FlatView
> memory: Get rid of address_space_init_shareable
> memory: Create FlatView directly
> memory: Avoid temporary FlatView allocation in a single child case
>
> include/exec/memory-internal.h | 16 +-
> include/exec/memory.h | 76 +++++-----
> include/hw/arm/armv7m.h | 2 +-
> cpus.c | 5 +-
> exec.c | 330 ++++++++++++++++++++++++----------------
> hw/arm/armv7m.c | 9 +-
> hw/intc/openpic_kvm.c | 2 +-
> memory.c | 335 +++++++++++++++++++++++++++++++----------
> monitor.c | 3 +-
> target/arm/cpu.c | 16 +-
> target/i386/cpu.c | 5 +-
> hmp-commands-info.hx | 7 +-
> 12 files changed, 533 insertions(+), 273 deletions(-)
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [Qemu-devel] [PATCH qemu v5 00/18] memory: Store physical root MR in FlatView
2017-09-21 10:03 ` [Qemu-devel] [PATCH qemu v5 00/18] memory: Store physical root MR in FlatView Paolo Bonzini
@ 2017-09-21 10:11 ` Alexey Kardashevskiy
2017-09-21 11:38 ` Paolo Bonzini
0 siblings, 1 reply; 26+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-21 10:11 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel
On 21/09/17 20:03, Paolo Bonzini wrote:
> On 21/09/2017 10:50, Alexey Kardashevskiy wrote:
>> This was inspired by https://bugzilla.redhat.com/show_bug.cgi?id=1481593
>> Previous versions:
>> v1: https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg01559.html
>> v2: https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg04069.html
>> v3: https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg04523.html
>> v4: https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg05669.html
>>
>> This patchset tries to reduce amount of memory used by FlatViews
>> and tries share as many FVs between address spaces as possible.
>
> Thanks, this looks good! (Patch 18 indeed is not what I had in mind :)).
Hmmm, then what did you have in mind then?
When clear my backlog a bit, I'd give it a try.
Thanks for the ideas and review.
ps. this could make a semidecent kvm forum talk, could not it? :)
>
> Paolo
>
>
>> Changelog:
>> v5:
>> * removed "memory: Give memory_region_transaction_commit a hint" (broken)
>> * added "memory: Avoid temporary FlatView allocation in a single child case"
>> but I suggest ditching it, it is just there to demonsrate how I checked
>> the suggested idea
>> * updated example of "info mtree -f -d" in 14/18 to demonstrate the result
>>
>> v4:
>> * total rework again to provide grounds for new 15/18, 17/18, 18/18
>>
>> v3:
>> * addressed comments from v2, mainly simplified the code
>>
>> v2:
>> * total rework
>>
>>
>> This is based on sha1
>> c51700273a Peter Maydell "Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20170919-v2' into staging".
>>
>> Please comment. Thanks.
>>
>>
>>
>> Alexey Kardashevskiy (18):
>> exec: Explicitly export target AS from
>> address_space_translate_internal
>> memory: Open code FlatView rendering
>> memory: Move FlatView allocation to a helper
>> memory: Move AddressSpaceDispatch from AddressSpace to FlatView
>> memory: Remove AddressSpace pointer from AddressSpaceDispatch
>> memory: Switch memory from using AddressSpace to FlatView
>> memory: Cleanup after switching to FlatView
>> memory: Rename mem_begin/mem_commit/mem_add helpers
>> memory: Store physical root MR in FlatView
>> memory: Alloc dispatch tree where topology is generared
>> memory: Move address_space_update_ioeventfds
>> memory: Share FlatView's and dispatch trees between address spaces
>> memory: Do not allocate FlatView in address_space_init
>> memory: Rework "info mtree" to print flat views and dispatch trees
>> memory: Share special empty FlatView
>> memory: Get rid of address_space_init_shareable
>> memory: Create FlatView directly
>> memory: Avoid temporary FlatView allocation in a single child case
>>
>> include/exec/memory-internal.h | 16 +-
>> include/exec/memory.h | 76 +++++-----
>> include/hw/arm/armv7m.h | 2 +-
>> cpus.c | 5 +-
>> exec.c | 330 ++++++++++++++++++++++++----------------
>> hw/arm/armv7m.c | 9 +-
>> hw/intc/openpic_kvm.c | 2 +-
>> memory.c | 335 +++++++++++++++++++++++++++++++----------
>> monitor.c | 3 +-
>> target/arm/cpu.c | 16 +-
>> target/i386/cpu.c | 5 +-
>> hmp-commands-info.hx | 7 +-
>> 12 files changed, 533 insertions(+), 273 deletions(-)
>>
>
--
Alexey
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v5 00/18] memory: Store physical root MR in FlatView
2017-09-21 10:11 ` Alexey Kardashevskiy
@ 2017-09-21 11:38 ` Paolo Bonzini
0 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2017-09-21 11:38 UTC (permalink / raw)
To: Alexey Kardashevskiy, qemu-devel
On 21/09/2017 12:11, Alexey Kardashevskiy wrote:
> On 21/09/17 20:03, Paolo Bonzini wrote:
>> On 21/09/2017 10:50, Alexey Kardashevskiy wrote:
>>> This was inspired by https://bugzilla.redhat.com/show_bug.cgi?id=1481593
>>> Previous versions:
>>> v1: https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg01559.html
>>> v2: https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg04069.html
>>> v3: https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg04523.html
>>> v4: https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg05669.html
>>>
>>> This patchset tries to reduce amount of memory used by FlatViews
>>> and tries share as many FVs between address spaces as possible.
>>
>> Thanks, this looks good! (Patch 18 indeed is not what I had in mind :)).
With further review:
- this is missing from patch 17:
diff --git a/memory.c b/memory.c
index f3db61621c..7d266ec8a2 100644
--- a/memory.c
+++ b/memory.c
@@ -2770,6 +2770,7 @@ void address_space_init(AddressSpace *as,
MemoryRegion *root, const char *name)
QTAILQ_INSERT_TAIL(&address_spaces, as, address_spaces_link);
as->name = g_strdup(name ? name : "anonymous");
address_space_update_topology(as);
+ address_space_update_ioeventfds(as);
}
static void do_address_space_destroy(AddressSpace *as)
- with non-virtio devices 98% (!) of created FlatViews are the empty
ones that are destroyed immediately, so we probably should detect those
in advance
I'll then drop patches 15 and 18, and send some more optimizations on top.
Thanks again for doing the *real* optimization work!
Paolo
> Hmmm, then what did you have in mind then?
> When clear my backlog a bit, I'd give it a try.
>
> Thanks for the ideas and review.
>
>
> ps. this could make a semidecent kvm forum talk, could not it? :)
>
>>
>> Paolo
>>
>>
>>> Changelog:
>>> v5:
>>> * removed "memory: Give memory_region_transaction_commit a hint" (broken)
>>> * added "memory: Avoid temporary FlatView allocation in a single child case"
>>> but I suggest ditching it, it is just there to demonsrate how I checked
>>> the suggested idea
>>> * updated example of "info mtree -f -d" in 14/18 to demonstrate the result
>>>
>>> v4:
>>> * total rework again to provide grounds for new 15/18, 17/18, 18/18
>>>
>>> v3:
>>> * addressed comments from v2, mainly simplified the code
>>>
>>> v2:
>>> * total rework
>>>
>>>
>>> This is based on sha1
>>> c51700273a Peter Maydell "Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20170919-v2' into staging".
>>>
>>> Please comment. Thanks.
>>>
>>>
>>>
>>> Alexey Kardashevskiy (18):
>>> exec: Explicitly export target AS from
>>> address_space_translate_internal
>>> memory: Open code FlatView rendering
>>> memory: Move FlatView allocation to a helper
>>> memory: Move AddressSpaceDispatch from AddressSpace to FlatView
>>> memory: Remove AddressSpace pointer from AddressSpaceDispatch
>>> memory: Switch memory from using AddressSpace to FlatView
>>> memory: Cleanup after switching to FlatView
>>> memory: Rename mem_begin/mem_commit/mem_add helpers
>>> memory: Store physical root MR in FlatView
>>> memory: Alloc dispatch tree where topology is generared
>>> memory: Move address_space_update_ioeventfds
>>> memory: Share FlatView's and dispatch trees between address spaces
>>> memory: Do not allocate FlatView in address_space_init
>>> memory: Rework "info mtree" to print flat views and dispatch trees
>>> memory: Share special empty FlatView
>>> memory: Get rid of address_space_init_shareable
>>> memory: Create FlatView directly
>>> memory: Avoid temporary FlatView allocation in a single child case
>>>
>>> include/exec/memory-internal.h | 16 +-
>>> include/exec/memory.h | 76 +++++-----
>>> include/hw/arm/armv7m.h | 2 +-
>>> cpus.c | 5 +-
>>> exec.c | 330 ++++++++++++++++++++++++----------------
>>> hw/arm/armv7m.c | 9 +-
>>> hw/intc/openpic_kvm.c | 2 +-
>>> memory.c | 335 +++++++++++++++++++++++++++++++----------
>>> monitor.c | 3 +-
>>> target/arm/cpu.c | 16 +-
>>> target/i386/cpu.c | 5 +-
>>> hmp-commands-info.hx | 7 +-
>>> 12 files changed, 533 insertions(+), 273 deletions(-)
>>>
>>
>
>
^ permalink raw reply related [flat|nested] 26+ messages in thread