| Message ID | 1453902715-25304-7-git-send-email-eric.auger@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi Pavel, On 01/28/2016 08:10 AM, Pavel Fedin wrote: > Hello! > >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >> index 3839c68..7eaf8be 100644 >> --- a/hw/arm/virt.c >> +++ b/hw/arm/virt.c >> @@ -125,6 +125,7 @@ static const MemMapEntry a15memmap[] = { >> [VIRT_GPIO] = { 0x09030000, 0x00001000 }, >> [VIRT_SECURE_UART] = { 0x09040000, 0x00001000 }, >> [VIRT_MMIO] = { 0x0a000000, 0x00000200 }, >> + [VIRT_RESERVED] = { 0x0be00000, 0x00100000 }, > > Looks like with this approach we would need to add this to all machine models which make use of PCI. But is it a good idea? Yes that's correct. On the other hand not all platforms do need that feature. As far > as i understand, the only requirement for this region is not to clash with guest RAM addresses. So, can we instead have some code, > which automatically finds some place, based on the size? Maybe we could use the IOVA region already reserved for platform bus which is currently sparsely used (by vfio platform devices). For now we hardcode the size to 0x00100000, but in future we could query > the host for the size, because it's still host's MSI controller. Yes I agree, this is something we currently miss. I need to study how we can go through all domains/groups and get the max number of MSI frames used by any. Thanks for the follow-up! Regards Eric > > Kind regards, > Pavel Fedin > Senior Engineer > Samsung Electronics Research center Russia > >
On 27 January 2016 at 13:51, Eric Auger <eric.auger@linaro.org> wrote: > Registers a 16x64kB reserved iova region. Currently this iova > region is used by the kernel to map host MSI controller frames > (GICv2m, GITS_TRANSLATER). Do you mean the host kernel or the guest kernel? The host kernel should be keeping its paws out of the guest's address space, and the guest kernel can manage the memory and the address space any way it likes, I would have thought. It's not clear to me what this is for. > Signed-off-by: Eric Auger <eric.auger@linaro.org> > --- > hw/arm/virt.c | 10 ++++++++++ > include/hw/arm/virt.h | 1 + > 2 files changed, 11 insertions(+) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index 3839c68..7eaf8be 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -125,6 +125,7 @@ static const MemMapEntry a15memmap[] = { > [VIRT_GPIO] = { 0x09030000, 0x00001000 }, > [VIRT_SECURE_UART] = { 0x09040000, 0x00001000 }, > [VIRT_MMIO] = { 0x0a000000, 0x00000200 }, > + [VIRT_RESERVED] = { 0x0be00000, 0x00100000 }, > /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */ You've put the new entry between the VIRT_MMIO line and the comment that is associated with it. thanks -- PMM
On 01/28/2016 11:10 AM, Peter Maydell wrote: > On 27 January 2016 at 13:51, Eric Auger <eric.auger@linaro.org> wrote: >> Registers a 16x64kB reserved iova region. Currently this iova >> region is used by the kernel to map host MSI controller frames >> (GICv2m, GITS_TRANSLATER). > > Do you mean the host kernel or the guest kernel? The host > kernel should be keeping its paws out of the guest's > address space, and the guest kernel can manage the memory > and the address space any way it likes, I would have thought. > It's not clear to me what this is for. I meant the host kernel. If we do not do anything, the host VFIO-PCI driver programs the PCI device with host GICv2m MSI frame host physical address (as an example). Since it goes through the sMMU and there is no mapping defined, this faults. So the idea of this series is that the guest provides some unused guest PA = IOVA. This window can be used by the host VFIO-PCI driver to transparently create an IOVA/ host GICv2 MSI frame mapping. That way the PCI device is programmed with this IOVA and this eventually reaches the host GICv2m MSI frame physical page. Hope it clarifies. > >> Signed-off-by: Eric Auger <eric.auger@linaro.org> >> --- >> hw/arm/virt.c | 10 ++++++++++ >> include/hw/arm/virt.h | 1 + >> 2 files changed, 11 insertions(+) >> >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >> index 3839c68..7eaf8be 100644 >> --- a/hw/arm/virt.c >> +++ b/hw/arm/virt.c >> @@ -125,6 +125,7 @@ static const MemMapEntry a15memmap[] = { >> [VIRT_GPIO] = { 0x09030000, 0x00001000 }, >> [VIRT_SECURE_UART] = { 0x09040000, 0x00001000 }, >> [VIRT_MMIO] = { 0x0a000000, 0x00000200 }, >> + [VIRT_RESERVED] = { 0x0be00000, 0x00100000 }, >> /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */ > > You've put the new entry between the VIRT_MMIO line and the comment that > is associated with it. sure thanks Eric > > thanks > -- PMM >
diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 3839c68..7eaf8be 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -125,6 +125,7 @@ static const MemMapEntry a15memmap[] = { [VIRT_GPIO] = { 0x09030000, 0x00001000 }, [VIRT_SECURE_UART] = { 0x09040000, 0x00001000 }, [VIRT_MMIO] = { 0x0a000000, 0x00000200 }, + [VIRT_RESERVED] = { 0x0be00000, 0x00100000 }, /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */ [VIRT_PLATFORM_BUS] = { 0x0c000000, 0x02000000 }, [VIRT_PCIE_MMIO] = { 0x10000000, 0x2eff0000 }, @@ -815,6 +816,8 @@ static void create_pcie(const VirtBoardInfo *vbi, qemu_irq *pic, hwaddr size_pio = vbi->memmap[VIRT_PCIE_PIO].size; hwaddr base_ecam = vbi->memmap[VIRT_PCIE_ECAM].base; hwaddr size_ecam = vbi->memmap[VIRT_PCIE_ECAM].size; + hwaddr base_reserved = vbi->memmap[VIRT_RESERVED].base; + hwaddr size_reserved = vbi->memmap[VIRT_RESERVED].size; hwaddr base = base_mmio; int nr_pcie_buses = size_ecam / PCIE_MMCFG_SIZE_MIN; int irq = vbi->irqmap[VIRT_PCIE]; @@ -822,6 +825,7 @@ static void create_pcie(const VirtBoardInfo *vbi, qemu_irq *pic, MemoryRegion *mmio_reg; MemoryRegion *ecam_alias; MemoryRegion *ecam_reg; + MemoryRegion *reserved_reg; DeviceState *dev; char *nodename; int i; @@ -838,6 +842,12 @@ static void create_pcie(const VirtBoardInfo *vbi, qemu_irq *pic, ecam_reg, 0, size_ecam); memory_region_add_subregion(get_system_memory(), base_ecam, ecam_alias); + reserved_reg = g_new0(MemoryRegion, 1); + memory_region_init_reserved_iova(reserved_reg, OBJECT(dev), "reserved-iova", + size_reserved, &error_fatal); + memory_region_add_subregion(get_system_memory(), base_reserved, + reserved_reg); + /* Map the MMIO window into system address space so as to expose * the section of PCI MMIO space which starts at the same base address * (ie 1:1 mapping for that part of PCI MMIO space visible through diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h index 1ce7847..194871b 100644 --- a/include/hw/arm/virt.h +++ b/include/hw/arm/virt.h @@ -61,6 +61,7 @@ enum { VIRT_PCIE_MMIO_HIGH, VIRT_GPIO, VIRT_SECURE_UART, + VIRT_RESERVED, }; typedef struct MemMapEntry {
Registers a 16x64kB reserved iova region. Currently this iova region is used by the kernel to map host MSI controller frames (GICv2m, GITS_TRANSLATER). Signed-off-by: Eric Auger <eric.auger@linaro.org> --- hw/arm/virt.c | 10 ++++++++++ include/hw/arm/virt.h | 1 + 2 files changed, 11 insertions(+) -- 1.9.1