[Qemu-devel] [RFC PATCH v1 00/11] Per Master Addressing (PMA)

[Qemu-devel] [RFC PATCH v1 00/11] Per Master Addressing (PMA)

[Peter Crosthwaite]

Hi All,

This series implements Per-bus-Master Address spaces (PMA). It builds
on the merged components of Edgar's original series:

https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg01346.html

And my pending MemoryRegion QOMification:

https://lists.nongnu.org/archive/html/qemu-devel/2014-06/msg00029.html

The approach is based on PMA as discussed KVM call 2014-05-13. MOM:

http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg02430.html

"
PM: Address space is flattended memory regions, needed for actual memory
access when constructing machines, we deal with memory regions, not
address spaces

AF+PM: Each CPU should have its own address space

...

Do busmasters get their own address space, too?
PM: Yes
"

Patches 3-9 patch memory, exec and CPU to enable this capability
generally. The basic idea is MemoryRegions are QOM/linkable, and masters
create their own AddressSpace as needed from their given MRs.

The PetaLogix ml605 board is converted to use the new framework as the
lead example P10/11. It is tested as working. For a diagram of the real
hardware setup WRT to DMA and bus master visibility:

http://www.xilinx.com/support/documentation/ip_documentation/axi_dma/v6_03_a/pg021_axi_dma.pdf

page 49.

This series is work-in-progress and has numerous FIXMEs in it.
Looking for conceptual level commentary.

2 trivials at the front of the series. Easily skipped by reviewers.

Regards,
Peter


Peter Crosthwaite (11):
  microblaze: ml605: Get rid of useless ddr_base variable
  dma: axidma: Variablise repeated s->streams[i] sub-expr
  exec: Parent root MRs to the machine
  exec: dummy_section: Pass address space through.
  memory: Parent Memory Regions to their registered owners
  memory: address_space_init: do nothing if no root region given
  memory.c: Add address_space_init_shareable()
  qom/cpu: Add Memory Region Property
  exec: use per-cpu address-spaces for cpuisms
  microblaze: ml605: Convert to PMA
  dma: axidma: Convert to PMA

 exec.c                              | 23 +++++++++++-------
 hw/core/loader.c                    |  2 +-
 hw/dma/xilinx_axidma.c              | 47 ++++++++++++++++++++++++++++++-------
 hw/microblaze/petalogix_ml605_mmu.c | 42 ++++++++++++++++++++++++---------
 include/exec/memory.h               |  2 ++
 include/qom/cpu.h                   |  1 +
 memory.c                            | 47 +++++++++++++++++++++++++++++++++++++
 qom/cpu.c                           | 30 +++++++++++++++++++++++
 8 files changed, 165 insertions(+), 29 deletions(-)

-- 
2.0.0

[Qemu-devel] [RFC PATCH v1 01/11] microblaze: ml605: Get rid of useless ddr_base variable

[Peter Crosthwaite]

It's a constant based on a macro. Just use the macro in place.

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

 hw/microblaze/petalogix_ml605_mmu.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c
index aea9c5b..6843abf 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -89,7 +89,6 @@ petalogix_ml605_init(MachineState *machine)
     SysBusDevice *busdev;
     DriveInfo *dinfo;
     int i;
-    hwaddr ddr_base = MEMORY_BASEADDR;
     MemoryRegion *phys_lmb_bram = g_new(MemoryRegion, 1);
     MemoryRegion *phys_ram = g_new(MemoryRegion, 1);
     qemu_irq irq[32];
@@ -106,7 +105,7 @@ petalogix_ml605_init(MachineState *machine)
 
     memory_region_init_ram(phys_ram, NULL, "petalogix_ml605.ram", ram_size);
     vmstate_register_ram_global(phys_ram);
-    memory_region_add_subregion(address_space_mem, ddr_base, phys_ram);
+    memory_region_add_subregion(address_space_mem, MEMORY_BASEADDR, phys_ram);
 
     dinfo = drive_get(IF_PFLASH, 0, 0);
     /* 5th parameter 2 means bank-width
@@ -201,7 +200,7 @@ petalogix_ml605_init(MachineState *machine)
         }
     }
 
-    microblaze_load_kernel(cpu, ddr_base, ram_size,
+    microblaze_load_kernel(cpu, MEMORY_BASEADDR, ram_size,
                            machine->initrd_filename,
                            BINARY_DEVICE_TREE_FILE,
                            machine_cpu_reset);
-- 
2.0.0

[Qemu-devel] [RFC PATCH v1 02/11] dma: axidma: Variablise repeated s->streams[i] sub-expr

[Peter Crosthwaite]

This have 5 usages. Make it a bit more readable by using a local
variable.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
Theres three more usages of this in coming patches, that would push
the number of usages higher

 hw/dma/xilinx_axidma.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
index cc90eb5..fb741b8 100644
--- a/hw/dma/xilinx_axidma.c
+++ b/hw/dma/xilinx_axidma.c
@@ -558,10 +558,11 @@ static void xilinx_axidma_realize(DeviceState *dev, Error **errp)
     int i;
 
     for (i = 0; i < 2; i++) {
-        s->streams[i].nr = i;
-        s->streams[i].bh = qemu_bh_new(timer_hit, &s->streams[i]);
-        s->streams[i].ptimer = ptimer_init(s->streams[i].bh);
-        ptimer_set_freq(s->streams[i].ptimer, s->freqhz);
+        struct Stream *st = &s->streams[i];
+        st->nr = i;
+        st->bh = qemu_bh_new(timer_hit, st);
+        st->ptimer = ptimer_init(st->bh);
+        ptimer_set_freq(st->ptimer, s->freqhz);
     }
     return;
 
-- 
2.0.0

[Qemu-devel] [RFC PATCH v1 03/11] exec: Parent root MRs to the machine

[Peter Crosthwaite]

Parent the root MemoryRegions for Memory and IO to the machine. This
gives them a QOM path.

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

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

diff --git a/exec.c b/exec.c
index 4e179a6..32cc099 100644
--- a/exec.c
+++ b/exec.c
@@ -1885,11 +1885,15 @@ static void memory_map_init(void)
     system_memory = g_malloc(sizeof(*system_memory));
 
     memory_region_init(system_memory, NULL, "system", UINT64_MAX);
+    object_property_add_child(qdev_get_machine(), "sysmem",
+                              OBJECT(system_memory), &error_abort);
     address_space_init(&address_space_memory, system_memory, "memory");
 
     system_io = g_malloc(sizeof(*system_io));
     memory_region_init_io(system_io, NULL, &unassigned_io_ops, NULL, "io",
                           65536);
+    object_property_add_child(qdev_get_machine(), "sysio",
+                              OBJECT(system_io), &error_abort);
     address_space_init(&address_space_io, system_io, "I/O");
 
     memory_listener_register(&core_memory_listener, &address_space_memory);
-- 
2.0.0

[Qemu-devel] [RFC PATCH v1 04/11] exec: dummy_section: Pass address space through.

[Peter Crosthwaite]

Rather than use the global singleton.

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

 exec.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/exec.c b/exec.c
index 32cc099..aa39f6c 100644
--- a/exec.c
+++ b/exec.c
@@ -1760,10 +1760,12 @@ static subpage_t *subpage_init(AddressSpace *as, hwaddr base)
     return mmio;
 }
 
-static uint16_t dummy_section(PhysPageMap *map, MemoryRegion *mr)
+static uint16_t dummy_section(PhysPageMap *map, AddressSpace *as,
+                              MemoryRegion *mr)
 {
+    assert(as);
     MemoryRegionSection section = {
-        .address_space = &address_space_memory,
+        .address_space = as,
         .mr = mr,
         .offset_within_address_space = 0,
         .offset_within_region = 0,
@@ -1795,13 +1797,13 @@ static void mem_begin(MemoryListener *listener)
     AddressSpaceDispatch *d = g_new0(AddressSpaceDispatch, 1);
     uint16_t n;
 
-    n = dummy_section(&d->map, &io_mem_unassigned);
+    n = dummy_section(&d->map, as, &io_mem_unassigned);
     assert(n == PHYS_SECTION_UNASSIGNED);
-    n = dummy_section(&d->map, &io_mem_notdirty);
+    n = dummy_section(&d->map, as, &io_mem_notdirty);
     assert(n == PHYS_SECTION_NOTDIRTY);
-    n = dummy_section(&d->map, &io_mem_rom);
+    n = dummy_section(&d->map, as, &io_mem_rom);
     assert(n == PHYS_SECTION_ROM);
-    n = dummy_section(&d->map, &io_mem_watch);
+    n = dummy_section(&d->map, as, &io_mem_watch);
     assert(n == PHYS_SECTION_WATCH);
 
     d->phys_map  = (PhysPageEntry) { .ptr = PHYS_MAP_NODE_NIL, .skip = 1 };
-- 
2.0.0

[Qemu-devel] [RFC PATCH v1 05/11] memory: Parent Memory Regions to their registered owners

[Peter Crosthwaite]

As the QOM child. This gives Memory regions a canon path without
machines having to do it manually.

Unfortunately, there are no assurances against multiple memory
regions having the same name even when parented to the same owner.

We could munge names, but the canon paths will then get messy
and far less usable.

So we forbid naming ambiguity when initing mmio. We cant do this
forcefully, so we just ignore the error when subsequent inits
come along with the same name.

The consequences of a memory_region not getting parent will be an
assertion later when someone tries to link with it. This will only
happen with super modern Memory QOMified code. All legacy Memory
API users are unaffected until they are converted to QOM.

To aid with that conversion there's a macro you can define to
force an assertion on naming ambiguity. I'm hoping that make check
with this turned on will find the bulk of the violators.

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

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

diff --git a/memory.c b/memory.c
index 9e030ad..9192941 100644
--- a/memory.c
+++ b/memory.c
@@ -846,6 +846,21 @@ static void memory_region_readd_subregion(MemoryRegion *mr)
     }
 }
 
+/* It's going to take some time and patience to weed out tree-wide user's
+ * where a single owner of multiple memory regions adds them with the
+ * same name. This is not supported when using QOM APIs with memory
+ * regions due to canonical path ambiguity. But things will continue to
+ * work for non QOMified Memory API users when the error is simply
+ * ignored. So we ignore it by default for the moment.
+ *
+ * When QOMifying Memomy API users, switch this on to trap this failure
+ * rather than get an obscure segfault later than the point of error.
+ */
+
+#ifndef TRAP_DUP_MR_NAME
+#define TRAP_DUP_MR_NAME 0
+#endif
+
 void memory_region_init(MemoryRegion *mr,
                         Object *owner,
                         const char *name,
@@ -854,6 +869,10 @@ void memory_region_init(MemoryRegion *mr,
     object_initialize(mr, sizeof(*mr), TYPE_MEMORY_REGION);
 
     mr->owner = owner;
+    if (owner) {
+        object_property_add_child(owner, name, OBJECT(mr),
+                                  TRAP_DUP_MR_NAME ? &error_abort : NULL);
+    }
     mr->size = int128_make64(size);
     if (size == UINT64_MAX) {
         mr->size = int128_2_64();
-- 
2.0.0

[Qemu-devel] [RFC PATCH v1 06/11] memory: address_space_init: do nothing if no root region given

[Peter Crosthwaite]

Just ignore this case. This is need 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 9192941..6f5e0e5 100644
--- a/memory.c
+++ b/memory.c
@@ -1936,6 +1936,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.0.0

[Qemu-devel] [RFC PATCH v1 07/11] memory.c: 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.
Its 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.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
I know this leaks memory. I'll fix that post RFC. I think we need
AS ref counters to do it properly if anyone has any input on how
that should be done.

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.

 include/exec/memory.h |  2 ++
 memory.c              | 24 ++++++++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 117c0d3..ae902b3 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -945,6 +945,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 6f5e0e5..33dde7b 100644
--- a/memory.c
+++ b/memory.c
@@ -1953,6 +1953,30 @@ 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)) {
+            return as;
+        }
+    }
+
+    /* FIMXE: this leaks */
+    as = g_malloc0(sizeof *as);
+    address_space_init(as, root, name);
+    return as;
+}
+
+/* FIXME: patch this to be a ref decrementer, and when the AS runs out of
+ * refs do garbage collection
+ */
+
 void address_space_destroy(AddressSpace *as)
 {
     /* Flush out anything from MemoryListeners listening in on this */
-- 
2.0.0

Re: [Qemu-devel] [RFC PATCH v1 07/11] memory.c: Add address_space_init_shareable()

[Peter Maydell]

On 3 June 2014 03:10, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
> 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.
> Its 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.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
> I know this leaks memory. I'll fix that post RFC. I think we need
> AS ref counters to do it properly if anyone has any input on how
> that should be done.
>
> 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.

Is this here because it looked like it would be really easy
to do, or because you tried this series without shared ASes
and found it was too inefficient?

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH v1 07/11] memory.c: Add address_space_init_shareable()

[Peter Crosthwaite]

On Wed, Jun 4, 2014 at 1:30 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 3 June 2014 03:10, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
>> 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.
>> Its 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.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> ---
>> I know this leaks memory. I'll fix that post RFC. I think we need
>> AS ref counters to do it properly if anyone has any input on how
>> that should be done.
>>
>> 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.
>
> Is this here because it looked like it would be really easy
> to do, or because you tried this series without shared ASes
> and found it was too inefficient?
>

Because it look easy. Although less actual as's means less info mtree
output. I would suspect that tool is un-usable without shared AS.

Regards,
Peter

> thanks
> -- PMM
>

[Qemu-devel] [RFC PATCH v1 08/11] qom/cpu: Add Memory Region Property

[Peter Crosthwaite]

Which is used to construct a per-CPU address space. The Address space
is constructed ASAP from the MemoryRegion property setter itself.

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

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

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index df977c8..4f6a4ee 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -237,6 +237,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 fada2d4..433e627 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -25,6 +25,8 @@
 #include "qemu/log.h"
 #include "qemu/error-report.h"
 #include "sysemu/sysemu.h"
+#include "exec/memory.h"
+#include "qapi/visitor.h"
 
 bool cpu_exists(int64_t id)
 {
@@ -303,12 +305,40 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp)
     }
 }
 
+static void cpu_set_mr(Object *obj, Visitor *v, void *opaque,
+                       const char *name, Error **errp)
+{
+    CPUState *cpu = CPU(obj);
+    Error *local_err = NULL;
+    char *path = NULL;
+
+    visit_type_str(v, &path, name, &local_err);
+
+    if (!local_err && strcmp(path, "") != 0) {
+        cpu->mr = MEMORY_REGION(object_resolve_link(obj, name, path,
+                                &local_err));
+    }
+
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    object_ref(OBJECT(cpu->mr));
+    cpu->as = address_space_init_shareable(cpu->mr, NULL);
+}
+
 static void cpu_common_initfn(Object *obj)
 {
     CPUState *cpu = CPU(obj);
     CPUClass *cc = CPU_GET_CLASS(obj);
 
     cpu->gdb_num_regs = cpu->gdb_num_g_regs = cc->gdb_num_core_regs;
+    object_property_add(obj, "mr", "link<" TYPE_MEMORY_REGION ">",
+                        NULL, /* FIXME: Implement the getter */
+                        cpu_set_mr,
+                        NULL, /* FIXME: Implement the cleanup */
+                        NULL, &error_abort);
 }
 
 static int64_t cpu_common_get_arch_id(CPUState *cpu)
-- 
2.0.0

[Qemu-devel] [RFC PATCH v1 09/11] exec: use per-cpu address-spaces for cpuisms

[Peter Crosthwaite]

Rather than address_space_memory.

Needs a few random callsites with hardcoded address_space_memory to be
patched. Use first_cpu->as in these cases.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
Long term, cpu_physical_memory_rw and friends needs to be replaced.

 exec.c           | 5 +++--
 hw/core/loader.c | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/exec.c b/exec.c
index aa39f6c..2535b32 100644
--- a/exec.c
+++ b/exec.c
@@ -487,7 +487,8 @@ void cpu_exec_init(CPUArchState *env)
     QTAILQ_INIT(&cpu->breakpoints);
     QTAILQ_INIT(&cpu->watchpoints);
 #ifndef CONFIG_USER_ONLY
-    cpu->as = &address_space_memory;
+    object_property_set_link(OBJECT(cpu), OBJECT(system_memory), "mr",
+                             &error_abort);
     cpu->thread_id = qemu_get_thread_id();
 #endif
     QTAILQ_INSERT_TAIL(&cpus, cpu, node);
@@ -2105,7 +2106,7 @@ bool address_space_read(AddressSpace *as, hwaddr addr, uint8_t *buf, int len)
 void cpu_physical_memory_rw(hwaddr addr, uint8_t *buf,
                             int len, int is_write)
 {
-    address_space_rw(&address_space_memory, addr, buf, len, is_write);
+    address_space_rw(first_cpu->as, addr, buf, len, is_write);
 }
 
 enum write_rom_type {
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 2bf6b8f..16473a9 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -798,7 +798,7 @@ static void rom_reset(void *unused)
             void *host = memory_region_get_ram_ptr(rom->mr);
             memcpy(host, rom->data, rom->datasize);
         } else {
-            cpu_physical_memory_write_rom(&address_space_memory,
+            cpu_physical_memory_write_rom(first_cpu->as,
                                           rom->addr, rom->data, rom->datasize);
         }
         if (rom->isrom) {
-- 
2.0.0

Re: [Qemu-devel] [RFC PATCH v1 09/11] exec: use per-cpu address-spaces for cpuisms

[Peter Maydell]

On 3 June 2014 03:11, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
> Rather than address_space_memory.
>
> Needs a few random callsites with hardcoded address_space_memory to be
> patched. Use first_cpu->as in these cases.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
> Long term, cpu_physical_memory_rw and friends needs to be replaced.

Yeah. The other two are not so bad, but cpu_physical_memory_rw()
really shouldn't have a first_cpu reference in it.


thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH v1 09/11] exec: use per-cpu address-spaces for cpuisms

[Peter Crosthwaite]

On Wed, Jun 4, 2014 at 1:28 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 3 June 2014 03:11, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
>> Rather than address_space_memory.
>>
>> Needs a few random callsites with hardcoded address_space_memory to be
>> patched. Use first_cpu->as in these cases.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> ---
>> Long term, cpu_physical_memory_rw and friends needs to be replaced.
>
> Yeah. The other two are not so bad, but cpu_physical_memory_rw()
> really shouldn't have a first_cpu reference in it.
>

Dropping that. I guess the plan is to convert users on
cpu_physical_memory_rw to address_space_rw anyways. Can fix them on a
demand basis.

Regards,
Peter

>
> thanks
> -- PMM
>

[Qemu-devel] [RFC PATCH v1 10/11] microblaze: ml605: Convert to PMA

[Peter Crosthwaite]

Get rid of reliance on the global AddressSpace and implement the bus
hierachy properly.

This board design has two distinct masterable bus views.

* DDR - Used by DMA capable peripherals, contains only DDR memory.
* CPU - Used by CPU, contains everything.

CPU view is a superset of DDR view. Use overlapping regions to
implement.

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

 hw/microblaze/petalogix_ml605_mmu.c | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c
index 6843abf..baf330f 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -82,7 +82,6 @@ static void
 petalogix_ml605_init(MachineState *machine)
 {
     ram_addr_t ram_size = machine->ram_size;
-    MemoryRegion *address_space_mem = get_system_memory();
     DeviceState *dev, *dma, *eth0;
     Object *ds, *cs;
     MicroBlazeCPU *cpu;
@@ -91,21 +90,33 @@ petalogix_ml605_init(MachineState *machine)
     int i;
     MemoryRegion *phys_lmb_bram = g_new(MemoryRegion, 1);
     MemoryRegion *phys_ram = g_new(MemoryRegion, 1);
+
+    MemoryRegion *cpu_mr = g_new(MemoryRegion, 1);
+    MemoryRegion *ddr_mr = g_new(MemoryRegion, 1);
+
     qemu_irq irq[32];
 
+    /* 32 bit system */
+    memory_region_init(cpu_mr, qdev_get_machine(), "cpu-mr", 1ull << 32);
+    memory_region_init(ddr_mr, qdev_get_machine(), "ddr-mr", 1ull << 32);
+    memory_region_add_subregion_overlap(cpu_mr, 0, ddr_mr, -1);
+
     /* init CPUs */
     cpu = MICROBLAZE_CPU(object_new(TYPE_MICROBLAZE_CPU));
+    object_property_add_child(qdev_get_machine(), "cpu", OBJECT(cpu),
+                              &error_abort);
     object_property_set_bool(OBJECT(cpu), true, "realized", &error_abort);
+    object_property_set_link(OBJECT(cpu), OBJECT(cpu_mr), "mr", &error_abort);
 
     /* Attach emulated BRAM through the LMB.  */
     memory_region_init_ram(phys_lmb_bram, NULL, "petalogix_ml605.lmb_bram",
                            LMB_BRAM_SIZE);
     vmstate_register_ram_global(phys_lmb_bram);
-    memory_region_add_subregion(address_space_mem, 0x00000000, phys_lmb_bram);
+    memory_region_add_subregion(cpu_mr, 0x00000000, phys_lmb_bram);
 
     memory_region_init_ram(phys_ram, NULL, "petalogix_ml605.ram", ram_size);
     vmstate_register_ram_global(phys_ram);
-    memory_region_add_subregion(address_space_mem, MEMORY_BASEADDR, phys_ram);
+    memory_region_add_subregion(ddr_mr, MEMORY_BASEADDR, phys_ram);
 
     dinfo = drive_get(IF_PFLASH, 0, 0);
     /* 5th parameter 2 means bank-width
@@ -120,14 +131,15 @@ petalogix_ml605_init(MachineState *machine)
     dev = qdev_create(NULL, "xlnx.xps-intc");
     qdev_prop_set_uint32(dev, "kind-of-intr", 1 << TIMER_IRQ);
     qdev_init_nofail(dev);
-    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, INTC_BASEADDR);
+    memory_region_add_subregion(cpu_mr, INTC_BASEADDR,
+                                sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0));
     sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0,
                        qdev_get_gpio_in(DEVICE(cpu), MB_CPU_IRQ));
     for (i = 0; i < 32; i++) {
         irq[i] = qdev_get_gpio_in(dev, i);
     }
 
-    serial_mm_init(address_space_mem, UART16550_BASEADDR + 0x1000, 2,
+    serial_mm_init(cpu_mr, UART16550_BASEADDR + 0x1000, 2,
                    irq[UART16550_IRQ], 115200, serial_hds[0],
                    DEVICE_LITTLE_ENDIAN);
 
@@ -136,7 +148,8 @@ petalogix_ml605_init(MachineState *machine)
     qdev_prop_set_uint32(dev, "one-timer-only", 0);
     qdev_prop_set_uint32(dev, "clock-frequency", 100 * 1000000);
     qdev_init_nofail(dev);
-    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, TIMER_BASEADDR);
+    memory_region_add_subregion(cpu_mr, TIMER_BASEADDR,
+                                sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0));
     sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[TIMER_IRQ]);
 
     /* axi ethernet and dma initialization. */
@@ -162,7 +175,9 @@ petalogix_ml605_init(MachineState *machine)
     object_property_set_link(OBJECT(eth0), OBJECT(cs),
                              "axistream-control-connected", &error_abort);
     qdev_init_nofail(eth0);
-    sysbus_mmio_map(SYS_BUS_DEVICE(eth0), 0, AXIENET_BASEADDR);
+    memory_region_add_subregion(cpu_mr, AXIENET_BASEADDR,
+                                sysbus_mmio_get_region(SYS_BUS_DEVICE(eth0),
+                                                       0));
     sysbus_connect_irq(SYS_BUS_DEVICE(eth0), 0, irq[AXIENET_IRQ]);
 
     ds = object_property_get_link(OBJECT(eth0),
@@ -175,7 +190,8 @@ petalogix_ml605_init(MachineState *machine)
     object_property_set_link(OBJECT(dma), OBJECT(cs),
                              "axistream-control-connected", &error_abort);
     qdev_init_nofail(dma);
-    sysbus_mmio_map(SYS_BUS_DEVICE(dma), 0, AXIDMA_BASEADDR);
+    memory_region_add_subregion(cpu_mr, AXIDMA_BASEADDR,
+                                sysbus_mmio_get_region(SYS_BUS_DEVICE(dma), 0));
     sysbus_connect_irq(SYS_BUS_DEVICE(dma), 0, irq[AXIDMA_IRQ0]);
     sysbus_connect_irq(SYS_BUS_DEVICE(dma), 1, irq[AXIDMA_IRQ1]);
 
@@ -186,7 +202,8 @@ petalogix_ml605_init(MachineState *machine)
         qdev_prop_set_uint8(dev, "num-ss-bits", NUM_SPI_FLASHES);
         qdev_init_nofail(dev);
         busdev = SYS_BUS_DEVICE(dev);
-        sysbus_mmio_map(busdev, 0, SPI_BASEADDR);
+        memory_region_add_subregion(cpu_mr, SPI_BASEADDR,
+                                    sysbus_mmio_get_region(busdev, 0));
         sysbus_connect_irq(busdev, 0, irq[SPI_IRQ]);
 
         spi = (SSIBus *)qdev_get_child_bus(dev, "spi");
-- 
2.0.0

[Qemu-devel] [RFC PATCH v1 11/11] dma: axidma: Convert to PMA

[Peter Crosthwaite]

Add MemoryRegion and AddressSpace's for the master attachments of
the DMA engine. The MemoryRegions are exposed as links to be set by
the machine model (or whatever). At realize time, the memory regions
are used to construct the masterable AddressSpace's.

Could have been done simpler as just one AS, but we make an attempt
here to be true to real hardware. The core does physically have three
AXI bus master attachments that could in theory be attached to
different buses. We facilitate that, even though the only user
as of now, connects them all to the one place (the DDR controller
in petalogix_ml605).

Update petalogix_ml605 to make it's connection properly. The changes
to axidma do not support a fallback, so we must make both the
device and machine changes concurrently.

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

 hw/dma/xilinx_axidma.c              | 38 ++++++++++++++++++++++++++++++++-----
 hw/microblaze/petalogix_ml605_mmu.c |  4 ++++
 2 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
index fb741b8..ab5e1f2 100644
--- a/hw/dma/xilinx_axidma.c
+++ b/hw/dma/xilinx_axidma.c
@@ -29,6 +29,7 @@
 #include "qapi/qmp/qerror.h"
 #include "qemu/main-loop.h"
 
+#include "sysemu/dma.h"
 #include "hw/stream.h"
 
 #define D(x)
@@ -110,6 +111,10 @@ struct Stream {
     unsigned int complete_cnt;
     uint32_t regs[R_MAX];
     uint8_t app[20];
+
+    MemoryRegion *data_mr;
+    AddressSpace *data_as;
+    AddressSpace *sg_as;
 };
 
 struct XilinxAXIDMAStreamSlave {
@@ -129,6 +134,8 @@ struct XilinxAXIDMA {
 
     struct Stream streams[2];
 
+    MemoryRegion *sg_mr;
+
     StreamCanPushNotifyFn notify;
     void *notify_opaque;
 };
@@ -197,7 +204,7 @@ static void stream_desc_load(struct Stream *s, hwaddr addr)
 {
     struct SDesc *d = &s->desc;
 
-    cpu_physical_memory_read(addr, d, sizeof *d);
+    dma_memory_read(s->sg_as, addr, d, sizeof *d);
 
     /* Convert from LE into host endianness.  */
     d->buffer_address = le64_to_cpu(d->buffer_address);
@@ -215,7 +222,7 @@ static void stream_desc_store(struct Stream *s, hwaddr addr)
     d->nxtdesc = cpu_to_le64(d->nxtdesc);
     d->control = cpu_to_le32(d->control);
     d->status = cpu_to_le32(d->status);
-    cpu_physical_memory_write(addr, d, sizeof *d);
+    dma_memory_write(s->sg_as, addr, d, sizeof *d);
 }
 
 static void stream_update_irq(struct Stream *s)
@@ -296,8 +303,8 @@ static void stream_process_mem2s(struct Stream *s, StreamSlave *tx_data_dev,
                      txlen + s->pos);
         }
 
-        cpu_physical_memory_read(s->desc.buffer_address,
-                                 txbuf + s->pos, txlen);
+        dma_memory_read(s->data_as, s->desc.buffer_address,
+                        txbuf + s->pos, txlen);
         s->pos += txlen;
 
         if (stream_desc_eof(&s->desc)) {
@@ -346,7 +353,7 @@ static size_t stream_process_s2mem(struct Stream *s, unsigned char *buf,
             rxlen = len;
         }
 
-        cpu_physical_memory_write(s->desc.buffer_address, buf + pos, rxlen);
+        dma_memory_write(s->data_as, s->desc.buffer_address, buf + pos, rxlen);
         len -= rxlen;
         pos += rxlen;
 
@@ -535,6 +542,7 @@ static void xilinx_axidma_realize(DeviceState *dev, Error **errp)
     XilinxAXIDMAStreamSlave *cs = XILINX_AXI_DMA_CONTROL_STREAM(
                                                             &s->rx_control_dev);
     Error *local_err = NULL;
+    AddressSpace *sg_as;
 
     object_property_add_link(OBJECT(ds), "dma", TYPE_XILINX_AXI_DMA,
                              (Object **)&ds->dma,
@@ -557,12 +565,16 @@ static void xilinx_axidma_realize(DeviceState *dev, Error **errp)
 
     int i;
 
+    sg_as = address_space_init_shareable(s->sg_mr, NULL);
+
     for (i = 0; i < 2; i++) {
         struct Stream *st = &s->streams[i];
         st->nr = i;
         st->bh = qemu_bh_new(timer_hit, st);
         st->ptimer = ptimer_init(st->bh);
         ptimer_set_freq(st->ptimer, s->freqhz);
+        st->data_as = address_space_init_shareable(st->data_mr, NULL);
+        st->sg_as = sg_as;
     }
     return;
 
@@ -604,6 +616,22 @@ static void xilinx_axidma_init(Object *obj)
     memory_region_init_io(&s->iomem, obj, &axidma_ops, s,
                           "xlnx.axi-dma", R_MAX * 4 * 2);
     sysbus_init_mmio(sbd, &s->iomem);
+
+    object_property_add_link(obj, "mm2s", TYPE_MEMORY_REGION,
+                             (Object **)&s->streams[0].data_mr,
+                             qdev_prop_allow_set_link_before_realize,
+                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             &error_abort);
+    object_property_add_link(obj, "s2mm", TYPE_MEMORY_REGION,
+                             (Object **)&s->streams[1].data_mr,
+                             qdev_prop_allow_set_link_before_realize,
+                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             &error_abort);
+    object_property_add_link(obj, "sg", TYPE_MEMORY_REGION,
+                             (Object **)&s->sg_mr,
+                             qdev_prop_allow_set_link_before_realize,
+                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             &error_abort);
 }
 
 static Property axidma_properties[] = {
diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c
index baf330f..aa755f7 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -157,6 +157,10 @@ petalogix_ml605_init(MachineState *machine)
     eth0 = qdev_create(NULL, "xlnx.axi-ethernet");
     dma = qdev_create(NULL, "xlnx.axi-dma");
 
+    object_property_set_link(OBJECT(dma), OBJECT(ddr_mr), "sg", &error_abort);
+    object_property_set_link(OBJECT(dma), OBJECT(ddr_mr), "s2mm", &error_abort);
+    object_property_set_link(OBJECT(dma), OBJECT(ddr_mr), "mm2s", &error_abort);
+
     /* FIXME: attach to the sysbus instead */
     object_property_add_child(qdev_get_machine(), "xilinx-eth", OBJECT(eth0),
                               NULL);
-- 
2.0.0

Re: [Qemu-devel] [RFC PATCH v1 00/11] Per Master Addressing (PMA)

[Paolo Bonzini]

Il 03/06/2014 04:06, Peter Crosthwaite ha scritto:
> Hi All,
>
> This series implements Per-bus-Master Address spaces (PMA). It builds
> on the merged components of Edgar's original series:
>
> https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg01346.html
>
> And my pending MemoryRegion QOMification:
>
> https://lists.nongnu.org/archive/html/qemu-devel/2014-06/msg00029.html
>
> The approach is based on PMA as discussed KVM call 2014-05-13. MOM:
>
> http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg02430.html
>
> "
> PM: Address space is flattended memory regions, needed for actual memory
> access when constructing machines, we deal with memory regions, not
> address spaces
>
> AF+PM: Each CPU should have its own address space
>
> ...
>
> Do busmasters get their own address space, too?
> PM: Yes
> "
>
> Patches 3-9 patch memory, exec and CPU to enable this capability
> generally. The basic idea is MemoryRegions are QOM/linkable, and masters
> create their own AddressSpace as needed from their given MRs.
>
> The PetaLogix ml605 board is converted to use the new framework as the
> lead example P10/11. It is tested as working. For a diagram of the real
> hardware setup WRT to DMA and bus master visibility:
>
> http://www.xilinx.com/support/documentation/ip_documentation/axi_dma/v6_03_a/pg021_axi_dma.pdf
>
> page 49.
>
> This series is work-in-progress and has numerous FIXMEs in it.
> Looking for conceptual level commentary.
>
> 2 trivials at the front of the series. Easily skipped by reviewers.
>
> Regards,
> Peter
>
>
> Peter Crosthwaite (11):
>   microblaze: ml605: Get rid of useless ddr_base variable
>   dma: axidma: Variablise repeated s->streams[i] sub-expr
>   exec: Parent root MRs to the machine
>   exec: dummy_section: Pass address space through.
>   memory: Parent Memory Regions to their registered owners
>   memory: address_space_init: do nothing if no root region given
>   memory.c: Add address_space_init_shareable()
>   qom/cpu: Add Memory Region Property
>   exec: use per-cpu address-spaces for cpuisms
>   microblaze: ml605: Convert to PMA
>   dma: axidma: Convert to PMA
>
>  exec.c                              | 23 +++++++++++-------
>  hw/core/loader.c                    |  2 +-
>  hw/dma/xilinx_axidma.c              | 47 ++++++++++++++++++++++++++++++-------
>  hw/microblaze/petalogix_ml605_mmu.c | 42 ++++++++++++++++++++++++---------
>  include/exec/memory.h               |  2 ++
>  include/qom/cpu.h                   |  1 +
>  memory.c                            | 47 +++++++++++++++++++++++++++++++++++++
>  qom/cpu.c                           | 30 +++++++++++++++++++++++
>  8 files changed, 165 insertions(+), 29 deletions(-)
>

Makes sense.  Patch 4 is an obvious cleanup/bugfix and I'm applying it 
now to my tree.

Paolo

Re: [Qemu-devel] [RFC PATCH v1 00/11] Per Master Addressing (PMA)

[Peter Maydell]

On 3 June 2014 03:06, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
> This series is work-in-progress and has numerous FIXMEs in it.
> Looking for conceptual level commentary

Looks good and like the right thing to me.

To check I understand how the QOM stuff is working here,
for a transaction master we expose an MR link property,
which the board sets to be a pointer to some appropriate
MR (which is owned by the board).
Conversely, for a transaction slave (ie a device), the
device owns its own memory regions. The board can map
those via the traditional APIs for putting MRs in
containers, or it could choose to set the MR 'container'
link property to point at the container MR (plus the
addr property).

The transaction master properties/API actually make
much more sense to me than the slave ones...

thanks
-- PMM