Per Master Addressing (PMA)
cover Per Master Addressing (PMA)
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(-)
Signature · 1 line
2.0.0
RFC PATCH 1/11 microblaze: ml605: Get rid of useless ddr_base variable
It's a constant based on a macro. Just use the macro in place.
---
hw/microblaze/petalogix_ml605_mmu.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
hw/microblaze/petalogix_ml605_mmu.c +2 -3
@@ -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);
Signature · 1 line
2.0.0
RFC PATCH 2/11 dma: axidma: Variablise repeated s->streams[i] sub-expr
In reply to Peter CrosthwaiteThis have 5 usages. Make it a bit more readable by using a local variable.
---
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(-)
hw/dma/xilinx_axidma.c +5 -4
@@ -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 ;
Signature · 1 line
2.0.0
RFC PATCH 3/11 exec: Parent root MRs to the machine
In reply to Peter CrosthwaiteParent the root MemoryRegions for Memory and IO to the machine. This gives them a QOM path.
---
exec.c | 4 ++++ 1 file changed, 4 insertions(+)
exec.c +4 -0
@@ -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);
Signature · 1 line
2.0.0
RFC PATCH 4/11 exec: dummy_section: Pass address space through.
In reply to Peter CrosthwaiteRather than use the global singleton.
---
exec.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
exec.c +8 -6
@@ -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 };
Signature · 1 line
2.0.0
RFC PATCH 5/11 memory: Parent Memory Regions to their registered owners
In reply to Peter CrosthwaiteAs 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.
---
memory.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
memory.c +19 -0
@@ -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 ();
Signature · 1 line
2.0.0
RFC PATCH 6/11 memory: address_space_init: do nothing if no root region given
In reply to Peter CrosthwaiteJust 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.
---
memory.c | 4 ++++ 1 file changed, 4 insertions(+)
memory.c +4 -0
@@ -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 );
Signature · 1 line
2.0.0
RFC PATCH 7/11 memory.c: Add address_space_init_shareable()
In reply to Peter CrosthwaiteThis 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.
---
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(+)
include/exec/memory.h +2 -0
@@ -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
memory.c +24 -0
@@ -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 */
Signature · 1 line
2.0.0
Re: [Qemu-devel] [RFC PATCH v1 07/11] memory.c: Add address_space_init_shareable()
Quoted 16 lines · 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 <[email protected]>---
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.
Quoted 4 lines
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()
Quoted 17 lines · Peter Maydell
Quoted 16 lines · 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 <[email protected]>---
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.
Quoted 9 lines
Quoted 4 lines
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
Quoted 3 lines
thanks
-- PMM
RFC PATCH 8/11 qom/cpu: Add Memory Region Property
In reply to Peter CrosthwaiteWhich is used to construct a per-CPU address space. The Address space is constructed ASAP from the MemoryRegion property setter itself.
---
include/qom/cpu.h | 1 + qom/cpu.c | 30 ++++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+)
include/qom/cpu.h +1 -0
@@ -237,6 +237,7 @@ struct CPUState {int64_t icount_extra;sigjmp_buf jmp_env; +MemoryRegion *mr;AddressSpace *as;MemoryListener *tcg_as_listener;
qom/cpu.c +30 -0
@@ -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)
Signature · 1 line
2.0.0
RFC PATCH 9/11 exec: use per-cpu address-spaces for cpuisms
In reply to Peter CrosthwaiteRather than address_space_memory.
Needs a few random callsites with hardcoded address_space_memory to be patched. Use first_cpu->as in these cases.
---
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(-)
exec.c +3 -2
@@ -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 {
hw/core/loader.c +1 -1
@@ -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) {
Signature · 1 line
2.0.0
Re: [Qemu-devel] [RFC PATCH v1 09/11] exec: use per-cpu address-spaces for cpuisms
Quoted 8 lines · Peter Crosthwaite
Rather than
address_space_memory.Needs a few random callsites with hardcoded
address_space_memoryto be patched. Usefirst_cpu->asin these cases.Signed-off-by Peter Crosthwaite <[email protected]>---
Long term,cpu_physical_memory_rwand 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
Quoted 6 lines · Peter Maydell
Quoted 5 lines · Peter Crosthwaite
Rather than
address_space_memory.Needs a few random callsites with hardcoded
address_space_memoryto be patched. Usefirst_cpu->asin these cases.
Quoted 7 lines
Quoted 3 lines
Signed-off-by Peter Crosthwaite <[email protected]>---
Long term,cpu_physical_memory_rwand friends needs to be replaced.Yeah. The other two are not so bad, but
cpu_physical_memory_rw()really shouldn't have afirst_cpureference 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
Quoted 4 lines
thanks
-- PMM
RFC PATCH 10/11 microblaze: ml605: Convert to PMA
In reply to Peter CrosthwaiteGet 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.
---
hw/microblaze/petalogix_ml605_mmu.c | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-)
hw/microblaze/petalogix_ml605_mmu.c +26 -9
@@ -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" );
Signature · 1 line
2.0.0
RFC PATCH 11/11 dma: axidma: Convert to PMA
In reply to Peter CrosthwaiteAdd 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.
---
hw/dma/xilinx_axidma.c | 38 ++++++++++++++++++++++++++++++++----- hw/microblaze/petalogix_ml605_mmu.c | 4 ++++ 2 files changed, 37 insertions(+), 5 deletions(-)
hw/dma/xilinx_axidma.c +33 -5
@@ -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[] = {
hw/microblaze/petalogix_ml605_mmu.c +4 -0
@@ -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 );
Signature · 1 line
2.0.0
Il 03/06/2014 04:06, Peter Crosthwaite ha scritto:
Quoted 62 lines
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 spacesAF+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,
PeterPeter Crosthwaite (11):
microblaze: ml605: Get rid of uselessddr_basevariable
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 givenmemory.c: Addaddress_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
Quoted 10 lines
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
Quoted 2 lines · Peter Crosthwaite
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