[Qemu-devel] [PATCH v1 0/6] PMA phase 2 - per CPU address spaces

[Qemu-devel] [PATCH v1 0/6] PMA phase 2 - per CPU address spaces

[Peter Crosthwaite]

Hi All,

This series sets up CPUs with configurable address spaces. This follows
on from Edgars original work and moves towards removal of
address_space_memory and support for arbitrary memory
heirachies/layouts.

Fuller context in RFC:

http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg00483.html

Follow up series' will add the rest of that functionality.

Regards,
Peter

Changed since RFC:
Limit scope to only CPU and Address Space plumbing
Got compiling and tested in fully configured build.


Peter Crosthwaite (6):
  memory: address_space_init: do nothing if no root region given
  memory: AddressSpace: Implement ref counting
  memory: Add address_space_init_shareable()
  qom: Move cpu.o to obj-y.
  qom/cpu: Add Memory Region Property
  cpu: Delay address space init until realize

 Makefile.target       |  1 +
 exec.c                |  1 -
 include/exec/memory.h |  4 ++++
 include/qom/cpu.h     |  1 +
 memory.c              | 34 ++++++++++++++++++++++++++++++++++
 qom/Makefile.objs     |  2 +-
 qom/cpu.c             | 16 ++++++++++++++++
 7 files changed, 57 insertions(+), 2 deletions(-)

-- 
2.1.0.1.g27b9230

[Qemu-devel] [PATCH v1 1/6] memory: address_space_init: do nothing if no root region given

[Peter Crosthwaite]

Just ignore this case. This is needed for bus master device realize
when the machine model doesn't connect an attachment. Then machine
model may decide to not set a memory region for mastering yet the
device will attempt to create itself an address space come realize
time. Gracefully do nothing.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 memory.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/memory.c b/memory.c
index 42317a2..e6014d9 100644
--- a/memory.c
+++ b/memory.c
@@ -1925,6 +1925,10 @@ void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
         memory_init();
     }
 
+    if (!root) {
+        return;
+    }
+
     memory_region_transaction_begin();
     as->root = root;
     as->current_map = g_new(FlatView, 1);
-- 
2.1.0.1.g27b9230

[Qemu-devel] [PATCH v1 2/6] memory: AddressSpace: Implement ref counting

[Peter Crosthwaite]

To allow for sharing of AddressSpaces between multiple masters.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 include/exec/memory.h | 1 +
 memory.c              | 5 +++++
 2 files changed, 6 insertions(+)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index d165b27..18cb6b2 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -207,6 +207,7 @@ struct AddressSpace {
     /* All fields are private. */
     char *name;
     MemoryRegion *root;
+    int ref_count;
     struct FlatView *current_map;
     int ioeventfd_nb;
     struct MemoryRegionIoeventfd *ioeventfds;
diff --git a/memory.c b/memory.c
index e6014d9..27beef1 100644
--- a/memory.c
+++ b/memory.c
@@ -1930,6 +1930,7 @@ void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
     }
 
     memory_region_transaction_begin();
+    as->ref_count = 1;
     as->root = root;
     as->current_map = g_new(FlatView, 1);
     flatview_init(as->current_map);
@@ -1946,6 +1947,10 @@ void address_space_destroy(AddressSpace *as)
 {
     MemoryListener *listener;
 
+    as->ref_count--;
+    if (as->ref_count) {
+        return;
+    }
     /* Flush out anything from MemoryListeners listening in on this */
     memory_region_transaction_begin();
     as->root = NULL;
-- 
2.1.0.1.g27b9230

[Qemu-devel] [PATCH v1 4/6] qom: Move cpu.o to obj-y.

[Peter Crosthwaite]

It's not really a common object and this is needed to give it access to
CONFIG_USER_ONLY definition. Move it to regular obj-y.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 Makefile.target   | 1 +
 qom/Makefile.objs | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile.target b/Makefile.target
index 1e8d7ab..c91c8e5 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -84,6 +84,7 @@ all: $(PROGS) stap
 # cpu emulator library
 obj-y = exec.o translate-all.o cpu-exec.o
 obj-y += tcg/tcg.o tcg/optimize.o
+obj-y += qom/
 obj-$(CONFIG_TCG_INTERPRETER) += tci.o
 obj-$(CONFIG_TCG_INTERPRETER) += disas/tci.o
 obj-y += fpu/softfloat.o
diff --git a/qom/Makefile.objs b/qom/Makefile.objs
index 985003b..3604a4f 100644
--- a/qom/Makefile.objs
+++ b/qom/Makefile.objs
@@ -1,3 +1,3 @@
 common-obj-y = object.o container.o qom-qobject.o
-common-obj-y += cpu.o
 common-obj-y += object_interfaces.o
+obj-y += cpu.o
-- 
2.1.0.1.g27b9230

Re: [Qemu-devel] [PATCH v1 4/6] qom: Move cpu.o to obj-y.

[Peter Maydell]

On 26 August 2014 01:58, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
> It's not really a common object

In what sense isn't it a common object? It's only
compiled once, rather than per-target. We should
avoid moving object files from compiled-once
to compiled-per-target if we can...

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 4/6] qom: Move cpu.o to obj-y.

[Paolo Bonzini]

Il 01/09/2014 19:43, Peter Maydell ha scritto:
> On 26 August 2014 01:58, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
>> It's not really a common object
> 
> In what sense isn't it a common object? It's only
> compiled once, rather than per-target. We should
> avoid moving object files from compiled-once
> to compiled-per-target if we can...

Should we add a softmmu_enabled() function similar to kvm_enabled(),
that returns true for system emulation and false for user-mode emulation?

Paolo

Re: [Qemu-devel] [PATCH v1 4/6] qom: Move cpu.o to obj-y.

[Peter Crosthwaite]

On Tue, Sep 2, 2014 at 3:54 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 01/09/2014 19:43, Peter Maydell ha scritto:
>> On 26 August 2014 01:58, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
>>> It's not really a common object
>>
>> In what sense isn't it a common object? It's only
>> compiled once, rather than per-target. We should
>> avoid moving object files from compiled-once
>> to compiled-per-target if we can...
>
> Should we add a softmmu_enabled() function similar to kvm_enabled(),
> that returns true for system emulation and false for user-mode emulation?
>

Doesn't really help me in this case, as I need conditional compliation
due to use of the maybe-exists &address_space_memory. I would also
need to add get_system_address_space() which return NULL for user-mode
as well.

Regards,
Peter

> Paolo
>
>

Re: [Qemu-devel] [PATCH v1 4/6] qom: Move cpu.o to obj-y.

[Peter Crosthwaite]

On Tue, Sep 2, 2014 at 3:43 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 26 August 2014 01:58, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
>> It's not really a common object
>
> In what sense isn't it a common object? It's only
> compiled once, rather than per-target. We should
> avoid moving object files from compiled-once
> to compiled-per-target if we can...
>

Yeh, I introduce the variation with address_space_memory reliance. CPU
needs to see it for the default AR. It doesn't exist in user-mode
though.

Regards,
Peter

> thanks
> -- PMM
>

Re: [Qemu-devel] [PATCH v1 4/6] qom: Move cpu.o to obj-y.

[Paolo Bonzini]

Il 02/09/2014 00:56, Peter Crosthwaite ha scritto:
> On Tue, Sep 2, 2014 at 3:43 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 26 August 2014 01:58, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
>>> It's not really a common object
>>
>> In what sense isn't it a common object? It's only
>> compiled once, rather than per-target. We should
>> avoid moving object files from compiled-once
>> to compiled-per-target if we can...
>>
> 
> Yeh, I introduce the variation with address_space_memory reliance. CPU
> needs to see it for the default AR. It doesn't exist in user-mode
> though.

cpus.c already contains softmmu-specific of cpu.c.  Perhaps you can add
something in there (and in *-user/main.c).

Paolo

[Qemu-devel] [PATCH v1 5/6] qom/cpu: Add Memory Region Property

[Peter Crosthwaite]

Which is used to construct a per-CPU address space. The Address space
will be created at realize time.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 include/qom/cpu.h |  1 +
 qom/cpu.c         | 11 +++++++++++
 2 files changed, 12 insertions(+)

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 1aafbf5..192d337 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -242,6 +242,7 @@ struct CPUState {
     int64_t icount_extra;
     sigjmp_buf jmp_env;
 
+    MemoryRegion *mr;
     AddressSpace *as;
     MemoryListener *tcg_as_listener;
 
diff --git a/qom/cpu.c b/qom/cpu.c
index b32dd0a..899071e 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -25,6 +25,10 @@
 #include "qemu/log.h"
 #include "qemu/error-report.h"
 #include "sysemu/sysemu.h"
+#include "exec/memory.h"
+#include "qapi/visitor.h"
+#include "exec/address-spaces.h"
+#include "hw/qdev-properties.h"
 
 bool cpu_exists(int64_t id)
 {
@@ -314,6 +318,13 @@ static void cpu_common_initfn(Object *obj)
     CPUClass *cc = CPU_GET_CLASS(obj);
 
     cpu->gdb_num_regs = cpu->gdb_num_g_regs = cc->gdb_num_core_regs;
+#ifndef CONFIG_USER_ONLY
+    object_property_add_link(obj, "mr", TYPE_MEMORY_REGION,
+                             (Object **)&cpu->mr,
+                             qdev_prop_allow_set_link_before_realize,
+                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             &error_abort);
+#endif
 }
 
 static int64_t cpu_common_get_arch_id(CPUState *cpu)
-- 
2.1.0.1.g27b9230

Re: [Qemu-devel] [PATCH v1 5/6] qom/cpu: Add Memory Region Property

[Paolo Bonzini]

Il 26/08/2014 02:59, Peter Crosthwaite ha scritto:
> Which is used to construct a per-CPU address space. The Address space
> will be created at realize time.
> 
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
> 
>  include/qom/cpu.h |  1 +
>  qom/cpu.c         | 11 +++++++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 1aafbf5..192d337 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -242,6 +242,7 @@ struct CPUState {
>      int64_t icount_extra;
>      sigjmp_buf jmp_env;
>  
> +    MemoryRegion *mr;
>      AddressSpace *as;
>      MemoryListener *tcg_as_listener;
>  
> diff --git a/qom/cpu.c b/qom/cpu.c
> index b32dd0a..899071e 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -25,6 +25,10 @@
>  #include "qemu/log.h"
>  #include "qemu/error-report.h"
>  #include "sysemu/sysemu.h"
> +#include "exec/memory.h"
> +#include "qapi/visitor.h"
> +#include "exec/address-spaces.h"
> +#include "hw/qdev-properties.h"
>  
>  bool cpu_exists(int64_t id)
>  {
> @@ -314,6 +318,13 @@ static void cpu_common_initfn(Object *obj)
>      CPUClass *cc = CPU_GET_CLASS(obj);
>  
>      cpu->gdb_num_regs = cpu->gdb_num_g_regs = cc->gdb_num_core_regs;
> +#ifndef CONFIG_USER_ONLY
> +    object_property_add_link(obj, "mr", TYPE_MEMORY_REGION,

Please use a clearer name such as "memory".

Paolo

> +                             (Object **)&cpu->mr,
> +                             qdev_prop_allow_set_link_before_realize,
> +                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
> +                             &error_abort);
> +#endif
>  }
>  
>  static int64_t cpu_common_get_arch_id(CPUState *cpu)
> 

[Qemu-devel] [PATCH v1 6/6] cpu: Delay address space init until realize

[Peter Crosthwaite]

To allow it to be set by the user after init.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 exec.c    | 1 -
 qom/cpu.c | 5 +++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/exec.c b/exec.c
index 5f9857c..15ae6b7 100644
--- a/exec.c
+++ b/exec.c
@@ -492,7 +492,6 @@ void cpu_exec_init(CPUArchState *env)
     QTAILQ_INIT(&cpu->breakpoints);
     QTAILQ_INIT(&cpu->watchpoints);
 #ifndef CONFIG_USER_ONLY
-    cpu->as = &address_space_memory;
     cpu->thread_id = qemu_get_thread_id();
 #endif
     QTAILQ_INSERT_TAIL(&cpus, cpu, node);
diff --git a/qom/cpu.c b/qom/cpu.c
index 899071e..f866d30 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -305,6 +305,11 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp)
 {
     CPUState *cpu = CPU(dev);
 
+#ifndef CONFIG_USER_ONLY
+    cpu->as = cpu->mr ? address_space_init_shareable(cpu->mr, NULL)
+                      : &address_space_memory;
+#endif
+
     if (dev->hotplugged) {
         cpu_synchronize_post_init(cpu);
         notifier_list_notify(&cpu_added_notifiers, dev);
-- 
2.1.0.1.g27b9230

Re: [Qemu-devel] [PATCH v1 0/6] PMA phase 2 - per CPU address spaces

[Peter Maydell]

On 26 August 2014 01:56, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
>
> Hi All,
>
> This series sets up CPUs with configurable address spaces. This follows
> on from Edgars original work and moves towards removal of
> address_space_memory and support for arbitrary memory
> heirachies/layouts.
>
> Fuller context in RFC:
>
> http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg00483.html
>
> Follow up series' will add the rest of that functionality.
>
> Regards,
> Peter
>
> Changed since RFC:
> Limit scope to only CPU and Address Space plumbing
> Got compiling and tested in fully configured build.
>
>
> Peter Crosthwaite (6):
>   memory: address_space_init: do nothing if no root region given
>   memory: AddressSpace: Implement ref counting
>   memory: Add address_space_init_shareable()
>   qom: Move cpu.o to obj-y.
>   qom/cpu: Add Memory Region Property
>   cpu: Delay address space init until realize

It looks like patch 3/6 never reached the list: care to
resend the series?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 0/6] PMA phase 2 - per CPU address spaces

[Peter Crosthwaite]

On Tue, Sep 2, 2014 at 3:40 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 26 August 2014 01:56, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
>>
>> Hi All,
>>
>> This series sets up CPUs with configurable address spaces. This follows
>> on from Edgars original work and moves towards removal of
>> address_space_memory and support for arbitrary memory
>> heirachies/layouts.
>>
>> Fuller context in RFC:
>>
>> http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg00483.html
>>
>> Follow up series' will add the rest of that functionality.
>>
>> Regards,
>> Peter
>>
>> Changed since RFC:
>> Limit scope to only CPU and Address Space plumbing
>> Got compiling and tested in fully configured build.
>>
>>
>> Peter Crosthwaite (6):
>>   memory: address_space_init: do nothing if no root region given
>>   memory: AddressSpace: Implement ref counting
>>   memory: Add address_space_init_shareable()
>>   qom: Move cpu.o to obj-y.
>>   qom/cpu: Add Memory Region Property
>>   cpu: Delay address space init until realize
>
> It looks like patch 3/6 never reached the list: care to
> resend the series?
>

Still have the original handy so sent the missing single. Should
thread properly.

I'll action Paolo's comments and respin V2 shortly.

Regards,
Peter

> thanks
> -- PMM
>

[Qemu-devel] [PATCH v1 3/6] memory: Add address_space_init_shareable()

[Peter Crosthwaite]

This will either create a new AS or return a pointer to an
already existing equivalent one. Both name and root mr must
match.

The motivation is to reuse address spaces as much as possible.
It's going to be quite common that bus masters out in device land
have pointers to the same memory region for their mastering yet
each will need to create its own address space. Let the memory
API implement sharing for them.

Aside from the perf optimisations, this should reduce the amount
of redundant output on info mtree as well.

Thee returned value will be malloced, but the malloc will be
automatically freed when the AS runs out of refs.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
We could change the equivalency test only match mr to support device
specific naming of these shared ASes. The singleton AS can ultimately
only have one name however. So perhaps some strcatting each time a new
sharer is added to the share. That or first-in-best-dressed.

Changed since v1:
Implement ref counting and garbage collection.

 include/exec/memory.h |  3 +++
 memory.c              | 25 +++++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 18cb6b2..cbd7336 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -208,6 +208,7 @@ struct AddressSpace {
     char *name;
     MemoryRegion *root;
     int ref_count;
+    bool malloced;
     struct FlatView *current_map;
     int ioeventfd_nb;
     struct MemoryRegionIoeventfd *ioeventfds;
@@ -977,6 +978,8 @@ void mtree_info(fprintf_function mon_printf, void *f);
  */
 void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name);
 
+AddressSpace *address_space_init_shareable(MemoryRegion *root,
+                                           const char *name);
 
 /**
  * address_space_destroy: destroy an address space
diff --git a/memory.c b/memory.c
index 27beef1..08ab918 100644
--- a/memory.c
+++ b/memory.c
@@ -1932,6 +1932,7 @@ void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
     memory_region_transaction_begin();
     as->ref_count = 1;
     as->root = root;
+    as->malloced = false;
     as->current_map = g_new(FlatView, 1);
     flatview_init(as->current_map);
     as->ioeventfd_nb = 0;
@@ -1943,6 +1944,27 @@ void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
     memory_region_transaction_commit();
 }
 
+AddressSpace *address_space_init_shareable(MemoryRegion *root, const char *name)
+{
+    AddressSpace *as;
+
+    if (!root) {
+        return NULL;
+    }
+
+    QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
+        if (root == as->root && !strcmp(name ? name : "anonymous", as->name)) {
+            as->ref_count++;
+            return as;
+        }
+    }
+
+    as = g_malloc0(sizeof *as);
+    address_space_init(as, root, name);
+    as->malloced = true;
+    return as;
+}
+
 void address_space_destroy(AddressSpace *as)
 {
     MemoryListener *listener;
@@ -1965,6 +1987,9 @@ void address_space_destroy(AddressSpace *as)
     flatview_unref(as->current_map);
     g_free(as->name);
     g_free(as->ioeventfds);
+    if (as->malloced) {
+        g_free(as);
+    }
 }
 
 bool io_mem_read(MemoryRegion *mr, hwaddr addr, uint64_t *pval, unsigned size)
-- 
2.1.0.1.g27b9230

Re: [Qemu-devel] [PATCH v1 0/6] PMA phase 2 - per CPU address spaces

[Peter Maydell]

On 26 August 2014 at 01:56, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> This series sets up CPUs with configurable address spaces. This follows
> on from Edgars original work and moves towards removal of
> address_space_memory and support for arbitrary memory
> heirachies/layouts.
>
> Fuller context in RFC:
>
> http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg00483.html
>
> Follow up series' will add the rest of that functionality.
>
> Regards,
> Peter
>
> Changed since RFC:
> Limit scope to only CPU and Address Space plumbing
> Got compiling and tested in fully configured build.
>
>
> Peter Crosthwaite (6):
>   memory: address_space_init: do nothing if no root region given
>   memory: AddressSpace: Implement ref counting
>   memory: Add address_space_init_shareable()
>   qom: Move cpu.o to obj-y.
>   qom/cpu: Add Memory Region Property
>   cpu: Delay address space init until realize

Just to let you know, I'm taking some of these patches into
a series I'm working on for multiple address-space support
(for ARM trustzone). Basically I'm taking patches 2 and 3,
plus a variant of patch 5 that avoids the need to make cpu.o
an obj-y object. Then I have some more patches of my own which
add an extra QOM property for ARM CPUs for the secure-memory
address space.

Alpha-quality work-in-progress patches currently at
https://git.linaro.org/people/peter.maydell/qemu-arm.git multi-ases
web view:
https://git.linaro.org/people/peter.maydell/qemu-arm.git/shortlog/refs/heads/multi-ases

I'm not completely certain about the AS reference-counting
code right now; will have to come back and think harder
about it later.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 0/6] PMA phase 2 - per CPU address spaces

[Peter Crosthwaite]

On Tue, Oct 20, 2015 at 7:59 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 26 August 2014 at 01:56, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> This series sets up CPUs with configurable address spaces. This follows
>> on from Edgars original work and moves towards removal of
>> address_space_memory and support for arbitrary memory
>> heirachies/layouts.
>>
>> Fuller context in RFC:
>>
>> http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg00483.html
>>
>> Follow up series' will add the rest of that functionality.
>>
>> Regards,
>> Peter
>>
>> Changed since RFC:
>> Limit scope to only CPU and Address Space plumbing
>> Got compiling and tested in fully configured build.
>>
>>
>> Peter Crosthwaite (6):
>>   memory: address_space_init: do nothing if no root region given
>>   memory: AddressSpace: Implement ref counting
>>   memory: Add address_space_init_shareable()
>>   qom: Move cpu.o to obj-y.
>>   qom/cpu: Add Memory Region Property
>>   cpu: Delay address space init until realize
>
> Just to let you know, I'm taking some of these patches into
> a series I'm working on for multiple address-space support
> (for ARM trustzone). Basically I'm taking patches 2 and 3,
> plus a variant of patch 5 that avoids the need to make cpu.o
> an obj-y object. Then I have some more patches of my own which
> add an extra QOM property for ARM CPUs for the secure-memory
> address space.
>

Cool. That is largely orthogonal to the goal of the referenced RFC
(link is still above in quoted text) which was thinking more about the
machine models. See the PetaLogix patches at the end. With the core
work going through in this project, the groundwork for arbitrary
machine AS hierarchy is there and we should think about deprecating
address_space_memory for new SoCs and DMA capable devices.

This might also help Marcin and Christian who were working with some
heterogenous archs and the memory spaces came up on both accounts.

Is info mtree well behaved? I remember needing some changes to mtree
due to verbosity when there were multiple ases and complex aliases at
play. Code is in the Xilinx tree, let me know if you need some
digging.

> Alpha-quality work-in-progress patches currently at
> https://git.linaro.org/people/peter.maydell/qemu-arm.git multi-ases
> web view:
> https://git.linaro.org/people/peter.maydell/qemu-arm.git/shortlog/refs/heads/multi-ases
>
> I'm not completely certain about the AS reference-counting
> code right now; will have to come back and think harder
> about it later.
>

I did once attempt to QOMify address spaces themselves which would
implement this on core layers but I think we decided against that IIRC
(and just use MRs as the QOMified handle for ases).

Regards,
Peter

> thanks
> -- PMM

Re: [Qemu-devel] [PATCH v1 0/6] PMA phase 2 - per CPU address spaces

[Peter Maydell]

On 20 October 2015 at 19:46, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> On Tue, Oct 20, 2015 at 7:59 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> Just to let you know, I'm taking some of these patches into
>> a series I'm working on for multiple address-space support
>> (for ARM trustzone). Basically I'm taking patches 2 and 3,
>> plus a variant of patch 5 that avoids the need to make cpu.o
>> an obj-y object. Then I have some more patches of my own which
>> add an extra QOM property for ARM CPUs for the secure-memory
>> address space.
>>
>
> Cool. That is largely orthogonal to the goal of the referenced RFC
> (link is still above in quoted text) which was thinking more about the
> machine models. See the PetaLogix patches at the end. With the core
> work going through in this project, the groundwork for arbitrary
> machine AS hierarchy is there and we should think about deprecating
> address_space_memory for new SoCs and DMA capable devices.

Yes. We should probably also try to be consistent about what
QOM property names we use for handing the MR to bus masters.
I went for 'memory' and 'secure-memory', but maybe there's a
better scheme?

> This might also help Marcin and Christian who were working with some
> heterogenous archs and the memory spaces came up on both accounts.
>
> Is info mtree well behaved? I remember needing some changes to mtree
> due to verbosity when there were multiple ases and complex aliases at
> play. Code is in the Xilinx tree, let me know if you need some
> digging.

I haven't done any testing beyond "haven't actually broken booting
of something that doesn't use the second AS" yet :-) I'll check
info mtree.

The other area I need to think about is KVM support -- KVM doesn't
really like having different ASes between CPUs (IIRC you could do
it but the performance would be bad). The uses I have in mind would
be ok with this limitation but maybe we need an assertion somewhere
to avoid people hitting it by mistake.

>> Alpha-quality work-in-progress patches currently at
>> https://git.linaro.org/people/peter.maydell/qemu-arm.git multi-ases
>> web view:
>> https://git.linaro.org/people/peter.maydell/qemu-arm.git/shortlog/refs/heads/multi-ases

(I see I messed up some of the patch authorship data; will fix.)

>> I'm not completely certain about the AS reference-counting
>> code right now; will have to come back and think harder
>> about it later.
>>
>
> I did once attempt to QOMify address spaces themselves which would
> implement this on core layers but I think we decided against that IIRC
> (and just use MRs as the QOMified handle for ases).

Why does address_space_init_shareable() insist on the name string
matching as well as the MR, by the way? That seems like a recipe
for having less sharing than we could for no particularly useful
reason...

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 0/6] PMA phase 2 - per CPU address spaces

[Peter Crosthwaite]

On Tue, Oct 20, 2015 at 1:46 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 20 October 2015 at 19:46, Peter Crosthwaite
> <crosthwaitepeter@gmail.com> wrote:
>> On Tue, Oct 20, 2015 at 7:59 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> Just to let you know, I'm taking some of these patches into
>>> a series I'm working on for multiple address-space support
>>> (for ARM trustzone). Basically I'm taking patches 2 and 3,
>>> plus a variant of patch 5 that avoids the need to make cpu.o
>>> an obj-y object. Then I have some more patches of my own which
>>> add an extra QOM property for ARM CPUs for the secure-memory
>>> address space.
>>>
>>
>> Cool. That is largely orthogonal to the goal of the referenced RFC
>> (link is still above in quoted text) which was thinking more about the
>> machine models. See the PetaLogix patches at the end. With the core
>> work going through in this project, the groundwork for arbitrary
>> machine AS hierarchy is there and we should think about deprecating
>> address_space_memory for new SoCs and DMA capable devices.
>
> Yes. We should probably also try to be consistent about what
> QOM property names we use for handing the MR to bus masters.
> I went for 'memory' and 'secure-memory', but maybe there's a
> better scheme?
>

Sometimes the TRM gives a unique symbolic name. E.g. your average
Xilinx soft-IP TRM will name the bus master 'M_AXI" or something like
that. We should follow that if it exists. Food for thought, here is a
diagram of a Microblaze processor with five well-named bus masters
with this kind of naming scheme:

https://reference.digilentinc.com/_media/vivado:mig_37.jpg?w=500&tok=cca517

I'm guessing there is nothing in ARM ARM for CPUs though?

In the absence of a TRM name and for devices with only one bus master,
I used "dma".

>> This might also help Marcin and Christian who were working with some
>> heterogenous archs and the memory spaces came up on both accounts.
>>
>> Is info mtree well behaved? I remember needing some changes to mtree
>> due to verbosity when there were multiple ases and complex aliases at
>> play. Code is in the Xilinx tree, let me know if you need some
>> digging.
>
> I haven't done any testing beyond "haven't actually broken booting
> of something that doesn't use the second AS" yet :-) I'll check
> info mtree.
>
> The other area I need to think about is KVM support -- KVM doesn't
> really like having different ASes between CPUs (IIRC you could do
> it but the performance would be bad). The uses I have in mind would
> be ok with this limitation but maybe we need an assertion somewhere
> to avoid people hitting it by mistake.
>
>>> Alpha-quality work-in-progress patches currently at
>>> https://git.linaro.org/people/peter.maydell/qemu-arm.git multi-ases
>>> web view:
>>> https://git.linaro.org/people/peter.maydell/qemu-arm.git/shortlog/refs/heads/multi-ases
>
> (I see I messed up some of the patch authorship data; will fix.)
>
>>> I'm not completely certain about the AS reference-counting
>>> code right now; will have to come back and think harder
>>> about it later.
>>>
>>
>> I did once attempt to QOMify address spaces themselves which would
>> implement this on core layers but I think we decided against that IIRC
>> (and just use MRs as the QOMified handle for ases).
>
> Why does address_space_init_shareable() insist on the name string
> matching as well as the MR, by the way? That seems like a recipe
> for having less sharing than we could for no particularly useful
> reason...
>

I think it was conservatism on my part to keep alive the possibility
of two different address-spaces sharing a root MR but with different
props on the AS struct level. If no such use case exists then just the
root MR match should be good enough. I guess put another way, is we
can do this if address spaces themselves are guaranteed to be
stateless.

Regards,
Peter

> thanks
> -- PMM