On 29 January 2016 at 16:53, Eric Auger <eric.au...@linaro.org> wrote:
> Registers a 16x64kB reserved iova region. Currently this iova
> region is used by the host kernel to map host MSI controller frames
> (GICv2m, GITS_TRANSLATER). The host kernel needs this iova window
> since it cannot program the PCIe device with MSI frame physical
> address (as opposed to x86) since the MSI write transactions go
> through the IOMMU.
>
> The reserved region is mapped on the platform bus.
I guess that keeps it neatly out of the way of everybody else :-)
> Signed-off-by: Eric Auger <eric.au...@linaro.org>
>
> ---
>
> RFC v1 -> RFC v2:
> - use the platform bus to map the reserved iova region
> ---
> hw/arm/virt.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 3839c68..4b2a891 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -805,7 +805,7 @@ static void create_pcie_irq_map(const VirtBoardInfo *vbi,
> uint32_t gic_phandle,
> }
>
> static void create_pcie(const VirtBoardInfo *vbi, qemu_irq *pic,
> - bool use_highmem)
> + bool use_highmem, MemoryRegion **reserved_reg)
> {
> hwaddr base_mmio = vbi->memmap[VIRT_PCIE_MMIO].base;
> hwaddr size_mmio = vbi->memmap[VIRT_PCIE_MMIO].size;
> @@ -920,10 +920,16 @@ static void create_pcie(const VirtBoardInfo *vbi,
> qemu_irq *pic,
> qemu_fdt_setprop_cell(vbi->fdt, nodename, "#interrupt-cells", 1);
> create_pcie_irq_map(vbi, vbi->gic_phandle, irq, nodename);
>
> + /* initialize the reserved iova region for MSI binding (16 x 64kb) */
> + *reserved_reg = g_new0(MemoryRegion, 1);
> + memory_region_init_reserved_iova(*reserved_reg, OBJECT(dev),
> + "reserved-iova",
> + 0x100000, &error_fatal);
So the only reason this is here is because we need to have a pointer to
the PCIe controller DeviceState, right? I think it would be better to
make create_pcie() return the DeviceState* instead of NULL. Then you
can either (a) pass the pcie controller pointer into create_platform_bus()
and have that create and map the reserved iova region, or (b) have a
separate function to create the reserved iova region. In any case I
think it fits more naturally with the rest of the platform bus code
rather than in the PCIe controller creation function.
thanks
-- PMM
