[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/2] hw/mips/jazz: Use generic I/O bus via get_system_io()
From: |
Peter Xu |
Subject: |
Re: [PATCH 1/2] hw/mips/jazz: Use generic I/O bus via get_system_io() |
Date: |
Thu, 11 Mar 2021 12:27:10 -0500 |
On Thu, Mar 11, 2021 at 05:21:49PM +0100, Philippe Mathieu-Daudé wrote:
> +Aspeed team
>
> On 3/11/21 1:18 PM, Philippe Mathieu-Daudé wrote:
> > On 3/10/21 9:29 PM, Peter Xu wrote:
>
> >> Yeah no worry - it's just that I feel one memory_region_init_alias() call
> >> is
> >> probably missing in your huge series somewhere, so that you'll take that
> >> alias
> >> MR as subregion rather than the real MR (which is the root of one AS).
> >
> > OK, with your earlier comments start + Mark other comment I start
> > to understand better.
> >
> > So far:
> >
> > (1a) AddressSpace is a physical view, its base address must be zero
> >
> > (1b) AddressSpace aperture is fixed (depends on hardware design,
> > not changeable at runtime
> >
> > Therefore due to (1a):
> > (2) AddressSpace root MemoryRegion is a container and must not be
> > mmio-mapped anywhere (in particular not on SysBus).
> >
> > (3) If hardware has a MMIO view of an AddressSpace, it has to be
> > via a MemoryRegion alias. That way the alias handles paddr offset
> > adjustment to the zero-based AddressSpace root container MR.
> > Aliasing allows resizing the alias size without modifying the AS
> > aperture size (1b).
> >
> > I'll start adding assertions for (1a) and (2) in the code base and
> > see if (3) adjustments are required.
>
> So using:
>
> -- >8 --
> diff --git a/softmmu/memory.c b/softmmu/memory.c
> index 874a8fccdee..8ce2d7f83b9 100644
> --- a/softmmu/memory.c
> +++ b/softmmu/memory.c
> @@ -713,6 +713,12 @@ static MemoryRegion
> *memory_region_get_flatview_root(MemoryRegion *mr)
> continue;
> }
> }
> + if (mr && mr->addr) {
> + error_report("Detected flatview root memory region '%s' with"
> + " non-zero base address (0x%"HWADDR_PRIx"):
> aborting",
> + memory_region_name(mr), mr->addr);
> + abort();
> + }
>
> return mr;
> }
> ---
Maybe it works, but it looks a bit odd to test here. What I meant was
something like attached.
>
> I get:
>
> $ ./qemu-system-arm -M ast2600-evb
> qemu-system-arm: Detected flatview root memory region
> 'aspeed.fmc-ast2600.flash' with non-zero base address (0x20000000): aborting
> Aborted (core dumped)
>
> Indeed:
>
> $ ./qemu-system-arm -M ast2600-evb -S -monitor stdio
> QEMU 5.2.50 monitor - type 'help' for more information
> (qemu) info mtree
> address-space: dma-dram
> 0000000080000000-000000017fffffff (prio 0, i/o): aspeed-ram-container
> 0000000080000000-00000000bfffffff (prio 0, ram): ram
> 00000000c0000000-00000000ffffffff (prio 0, i/o): max_ram
>
> address-space: aspeed.fmc-ast2600-dma-flash
> 0000000020000000-000000002fffffff (prio 0, i/o): aspeed.fmc-ast2600.flash
> 0000000020000000-0000000027ffffff (prio 0, i/o): aspeed.fmc-ast2600.0
> 0000000028000000-000000002fffffff (prio 0, i/o): aspeed.fmc-ast2600.1
>
> address-space: aspeed.fmc-ast2600-dma-dram
> 0000000080000000-000000017fffffff (prio 0, i/o): aspeed-ram-container
> 0000000080000000-00000000bfffffff (prio 0, ram): ram
> 00000000c0000000-00000000ffffffff (prio 0, i/o): max_ram
>
> address-space: aspeed.spi1-ast2600-dma-flash
> 0000000030000000-000000003fffffff (prio 0, i/o): aspeed.spi1-ast2600.flash
> 0000000030000000-0000000037ffffff (prio 0, i/o): aspeed.spi1-ast2600.0
>
> address-space: aspeed.spi1-ast2600-dma-dram
> 0000000080000000-000000017fffffff (prio 0, i/o): aspeed-ram-container
> 0000000080000000-00000000bfffffff (prio 0, ram): ram
> 00000000c0000000-00000000ffffffff (prio 0, i/o): max_ram
>
> address-space: aspeed.spi2-ast2600-dma-flash
> 0000000050000000-000000005fffffff (prio 0, i/o): aspeed.spi2-ast2600.flash
> 0000000050000000-0000000057ffffff (prio 0, i/o): aspeed.spi2-ast2600.0
>
> address-space: aspeed.spi2-ast2600-dma-dram
> 0000000080000000-000000017fffffff (prio 0, i/o): aspeed-ram-container
> 0000000080000000-00000000bfffffff (prio 0, ram): ram
> 00000000c0000000-00000000ffffffff (prio 0, i/o): max_ram
>
> Many address spaces not zero-based...
Maybe it's still legal to make the root mr a subregion of another, so maybe I'm
completely wrong... then the patch attached won't make any sense either. It's
just that in my mind each MR should have a "parent" - for normal MR it's the
container MR, then for root MR it's easier to see the AS as its "parent".
Maybe Paolo could clarify this..
Thanks,
--
Peter Xu
- Re: [PATCH 2/3] memory: Provide 'base address' argument to mtree_print_mr(), (continued)
- Re: [PATCH 2/3] memory: Provide 'base address' argument to mtree_print_mr(), Philippe Mathieu-Daudé, 2021/03/09
- Re: [PATCH 2/3] memory: Provide 'base address' argument to mtree_print_mr(), Peter Xu, 2021/03/10
- Re: [PATCH 2/3] memory: Provide 'base address' argument to mtree_print_mr(), Philippe Mathieu-Daudé, 2021/03/10
- [PATCH 1/2] hw/mips/jazz: Use generic I/O bus via get_system_io(), Philippe Mathieu-Daudé, 2021/03/10
- [PATCH 2/2] NOTFORMERGE memory: Ensure AddressSpace physical base address is zero, Philippe Mathieu-Daudé, 2021/03/10
- Re: [PATCH 1/2] hw/mips/jazz: Use generic I/O bus via get_system_io(), Peter Xu, 2021/03/10
- Re: [PATCH 1/2] hw/mips/jazz: Use generic I/O bus via get_system_io(), Philippe Mathieu-Daudé, 2021/03/10
- Re: [PATCH 1/2] hw/mips/jazz: Use generic I/O bus via get_system_io(), Peter Xu, 2021/03/10
- Re: [PATCH 1/2] hw/mips/jazz: Use generic I/O bus via get_system_io(), Philippe Mathieu-Daudé, 2021/03/11
- Re: [PATCH 1/2] hw/mips/jazz: Use generic I/O bus via get_system_io(), Philippe Mathieu-Daudé, 2021/03/11
- Re: [PATCH 1/2] hw/mips/jazz: Use generic I/O bus via get_system_io(),
Peter Xu <=
- Re: [PATCH 1/2] hw/mips/jazz: Use generic I/O bus via get_system_io(), Philippe Mathieu-Daudé, 2021/03/11
- Re: [PATCH 1/2] hw/mips/jazz: Use generic I/O bus via get_system_io(), Cédric Le Goater, 2021/03/12
- Re: [PATCH 1/2] hw/mips/jazz: Use generic I/O bus via get_system_io(), Peter Xu, 2021/03/11
- Re: [PATCH 1/2] hw/mips/jazz: Use generic I/O bus via get_system_io(), Philippe Mathieu-Daudé, 2021/03/11
- Re: [PATCH 1/2] hw/mips/jazz: Use generic I/O bus via get_system_io(), Jiaxun Yang, 2021/03/10
- Re: [PATCH 2/3] memory: Provide 'base address' argument to mtree_print_mr(), Peter Xu, 2021/03/10
- Re: [PATCH 2/3] memory: Provide 'base address' argument to mtree_print_mr(), Mark Cave-Ayland, 2021/03/10
[PATCH 3/3] memory: Make memory_region_to_absolute_addr() take a const MemoryRegion, Philippe Mathieu-Daudé, 2021/03/05