On 12.08.15 05:12, Pavel Fedin wrote:
> This large region is necessary for some devices like ivshmem and video cards
> 32-bit kernels can be built without LPAE support. In this case such a kernel
> will not be able to use PCI controller which has windows in high addresses.
> In order to work around the problem, "highmem" option is introduced. It
> defaults to on on, but can be manually set to off in order to be able to run
> those old 32-bit guests.
>
> Signed-off-by: Pavel Fedin <p.fe...@samsung.com>
> ---
> v5 => v6:
> - Specify correct FDT_PCI_RANGE_MMIO_64BIT type for the region, the bug
> was discovered by running UEFI
> v4 => v5:
> - Removed machine-dependent "highmem" default, now always ON
> v3 => v4:
> - Added "highmem" option which controls presence of this region. Default
> value is on for 64-bit CPUs and off for 32-bit CPUs.
> - Supply correct min and max address to aml_qword_memory()
> v2 => v3:
> - Region size increased to 512G
> - Added ACPI description
> v1 => v2:
> - Region address changed to 512G, leaving more space for RAM
> ---
> hw/arm/virt-acpi-build.c | 17 +++++++++--
> hw/arm/virt.c | 63
> +++++++++++++++++++++++++++++++++++-----
> include/hw/arm/virt-acpi-build.h | 1 +
> include/hw/arm/virt.h | 1 +
> 4 files changed, 73 insertions(+), 9 deletions(-)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index f365140..9088248 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -159,7 +159,8 @@ static void acpi_dsdt_add_virtio(Aml *scope,
> }
> }
>
> -static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, int irq)
> +static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, int irq,
> + bool use_highmem)
> {
> Aml *method, *crs, *ifctx, *UUID, *ifctx1, *elsectx, *buf;
> int i, bus_no;
> @@ -234,6 +235,17 @@ static void acpi_dsdt_add_pci(Aml *scope, const
> MemMapEntry *memmap, int irq)
> AML_ENTIRE_RANGE, 0x0000, 0x0000, size_pio - 1,
> base_pio,
> size_pio));
>
> + if (use_highmem) {
> + hwaddr base_mmio_high = memmap[VIRT_PCIE_MMIO_HIGH].base;
> + hwaddr size_mmio_high = memmap[VIRT_PCIE_MMIO_HIGH].size;
> +
> + aml_append(rbuf,
> + aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
> + AML_NON_CACHEABLE, AML_READ_WRITE, 0x0000,
> + base_mmio_high, base_mmio_high, 0x0000,
> + size_mmio_high));
> + }
> +
> aml_append(method, aml_name_decl("RBUF", rbuf));
> aml_append(method, aml_return(rbuf));
> aml_append(dev, method);
> @@ -510,7 +522,8 @@ build_dsdt(GArray *table_data, GArray *linker,
> VirtGuestInfo *guest_info)
> acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]);
> acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
> (irqmap[VIRT_MMIO] + ARM_SPI_BASE),
> NUM_VIRTIO_TRANSPORTS);
> - acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] + ARM_SPI_BASE));
> + acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] + ARM_SPI_BASE),
> + guest_info->use_highmem);
>
> aml_append(dsdt, scope);
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 4846892..44dcd0c 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -77,6 +77,7 @@ typedef struct {
> typedef struct {
> MachineState parent;
> bool secure;
> + bool highmem;
> } VirtMachineState;
>
> #define TYPE_VIRT_MACHINE "virt"
> @@ -117,6 +118,7 @@ static const MemMapEntry a15memmap[] = {
> [VIRT_PCIE_PIO] = { 0x3eff0000, 0x00010000 },
> [VIRT_PCIE_ECAM] = { 0x3f000000, 0x01000000 },
> [VIRT_MEM] = { 0x40000000, 30ULL * 1024 * 1024 * 1024 },
> + [VIRT_PCIE_MMIO_HIGH] = { 0x8000000000, 0x8000000000 },
> };
>
> static const int a15irqmap[] = {
> @@ -658,7 +660,8 @@ static void create_pcie_irq_map(const VirtBoardInfo *vbi,
> uint32_t gic_phandle,
> 0x7 /* PCI irq */);
> }
>
> -static void create_pcie(const VirtBoardInfo *vbi, qemu_irq *pic)
> +static void create_pcie(const VirtBoardInfo *vbi, qemu_irq *pic,
> + bool use_highmem)
> {
> hwaddr base_mmio = vbi->memmap[VIRT_PCIE_MMIO].base;
> hwaddr size_mmio = vbi->memmap[VIRT_PCIE_MMIO].size;
> @@ -719,11 +722,33 @@ static void create_pcie(const VirtBoardInfo *vbi,
> qemu_irq *pic)
>
> qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
> 2, base_ecam, 2, size_ecam);
> - qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "ranges",
> - 1, FDT_PCI_RANGE_IOPORT, 2, 0,
> - 2, base_pio, 2, size_pio,
> - 1, FDT_PCI_RANGE_MMIO, 2, base_mmio,
> - 2, base_mmio, 2, size_mmio);
> +
> + if (use_highmem) {
> + /* High MMIO space */
> + hwaddr base_mmio_high = vbi->memmap[VIRT_PCIE_MMIO_HIGH].base;
> + hwaddr size_mmio_high = vbi->memmap[VIRT_PCIE_MMIO_HIGH].size;
> +
> + mmio_alias = g_new0(MemoryRegion, 1);
> + memory_region_init_alias(mmio_alias, OBJECT(dev), "pcie-mmio-high",
> + mmio_reg, base_mmio_high, size_mmio_high);
Meh, some times I wish for the new gmail feature that allows you to not
send emails after you pressed send ;).
So you're just reusing the variable here and create a new alias into
mmio_reg. I think it would be more readable if you actually made that a
new variable as well.
The rest looks good. I can't really review the ACPI parts though.
Alex
