|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 5/5] libxl, hvmloader: Don't relocate memory for MMIO hole
On Tue, 18 Jun 2013, George Dunlap wrote:
> At the moment, qemu-xen can't handle memory being relocated by
> hvmloader. This may happen if a device with a large enough memory
> region is passed through to the guest. At the moment, if this
> happens, then at some point in the future qemu will crash and the
> domain will hang. (qemu-traditional is fine.)
>
> It's too late in the release to do a proper fix, so we try to do
> damage control.
>
> hvmloader already has mechanisms to relocate memory to 64-bit space
> if it can't make a big enough MMIO hole. By default this is 2GiB; if
> we just refuse to make the hole bigger if it will overlap with guest
> memory, then the relocation will happen by default.
>
> v2:
> - style fixes
> - fix and expand comment on the MMIO hole loop
> - use "%d" rather than "%s" -> (...)?"1":"0"
> - use bool instead of uint8_t
> - Move 64-bit bar relocate detection to another patch
> - Add more diagnostic messages
>
> Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> CC: Ian Campbell <ian.campbell@xxxxxxxxxx>
> CC: Ian Jackson <ian.jackson@xxxxxxxxxx>
> CC: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
> CC: Hanweidong <hanweidong@xxxxxxxxxx>
> ---
> tools/firmware/hvmloader/pci.c | 41
> ++++++++++++++++++++++++++++---
> tools/libxl/libxl_dm.c | 6 +++++
> xen/include/public/hvm/hvm_xs_strings.h | 1 +
> 3 files changed, 45 insertions(+), 3 deletions(-)
>
> diff --git a/tools/firmware/hvmloader/pci.c b/tools/firmware/hvmloader/pci.c
> index 63d79a2..1ab5124 100644
> --- a/tools/firmware/hvmloader/pci.c
> +++ b/tools/firmware/hvmloader/pci.c
> @@ -27,6 +27,8 @@
>
> #include <xen/memory.h>
> #include <xen/hvm/ioreq.h>
> +#include <xen/hvm/hvm_xs_strings.h>
> +#include <stdbool.h>
>
> unsigned long pci_mem_start = PCI_MEM_START;
> unsigned long pci_mem_end = PCI_MEM_END;
> @@ -58,6 +60,15 @@ void pci_setup(void)
> } *bars = (struct bars *)scratch_start;
> unsigned int i, nr_bars = 0;
>
> + const char *s;
> + bool allow_memory_relocate = 1;
Arguably the default should be 0, given that the default device model is
qemu-xen that cannot cope with memory relocation.
> + s = xenstore_read(HVM_XS_ALLOW_MEMORY_RELOCATE, NULL);
> + if ( s )
> + allow_memory_relocate = (bool)strtoll(s, NULL, 0);
> + printf("Relocating guest memory for lowmem MMIO space %s\n",
> + allow_memory_relocate?"enabled":"disabled");
It doesn't take a strtoll to parse a boolean.
> /* Program PCI-ISA bridge with appropriate link routes. */
> isa_irq = 0;
> for ( link = 0; link < 4; link++ )
> @@ -209,14 +220,38 @@ void pci_setup(void)
> pci_writew(devfn, PCI_COMMAND, cmd);
> }
>
> - while ( (mmio_total > (pci_mem_end - pci_mem_start)) &&
> - ((pci_mem_start << 1) != 0) )
> + /*
> + * At the moment qemu-xen can't deal with relocated memory regions.
> + * It's too close to the release to make a proper fix; for now,
> + * only allow the MMIO hole to grow large enough to move guest memory
> + * if we're running qemu-traditional. Items that don't fit will be
> + * relocated into the 64-bit address space.
I would avoid mentioning release issues in a comment within the code.
> + * This loop now does the following:
> + * - If allow_memory_relocate, increase the MMIO hole until it's
> + * big enough, or until it's 2GiB
> + * - If !allow_memory_relocate, increase the MMIO hole until it's
> + * big enough, or until it's 2GiB, or until it overlaps guest
> + * memory
> + */
> + while ( (mmio_total > (pci_mem_end - pci_mem_start))
> + && ((pci_mem_start << 1) != 0)
> + && (allow_memory_relocate
> + || (((pci_mem_start << 1) >> PAGE_SHIFT)
> + < hvm_info->low_mem_pgend)) )
Isn't this last condition inverted? It should be '>=' ?
> pci_mem_start <<= 1;
>
> - if ( mmio_total > (pci_mem_end - pci_mem_start) )
> + if ( mmio_total > (pci_mem_end - pci_mem_start) ) {
> + printf("<4GiB MMIO hole not large enough,"
> + " relocating some BARs to 64-bit\n");
> bar64_relocate = 1;
> + }
>
> /* Relocate RAM that overlaps PCI space (in 64k-page chunks). */
> + if ( (pci_mem_start >> PAGE_SHIFT) < hvm_info->low_mem_pgend )
> + printf("Relocating 0x%lx pages to highmem for lowmem MMIO hole\n",
> + hvm_info->low_mem_pgend - (pci_mem_start >> PAGE_SHIFT));
> +
> while ( (pci_mem_start >> PAGE_SHIFT) < hvm_info->low_mem_pgend )
> {
> struct xen_add_to_physmap xatp;
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index ac1f90e..342d2ce 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -1154,6 +1154,12 @@ void libxl__spawn_local_dm(libxl__egc *egc,
> libxl__dm_spawn_state *dmss)
> libxl__xs_write(gc, XBT_NULL,
> libxl__sprintf(gc, "%s/hvmloader/bios", path),
> "%s", libxl_bios_type_to_string(b_info->u.hvm.bios));
> + /* Disable relocating memory to make the MMIO hole larger
> + * unless we're running qemu-traditional */
> + libxl__xs_write(gc, XBT_NULL,
> + libxl__sprintf(gc,
> "%s/hvmloader/allow-memory-relocate", path),
> + "%d",
> +
> b_info->device_model_version==LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL);
> free(path);
> }
>
> diff --git a/xen/include/public/hvm/hvm_xs_strings.h
> b/xen/include/public/hvm/hvm_xs_strings.h
> index 9042303..4de5881 100644
> --- a/xen/include/public/hvm/hvm_xs_strings.h
> +++ b/xen/include/public/hvm/hvm_xs_strings.h
> @@ -28,6 +28,7 @@
> #define HVM_XS_HVMLOADER "hvmloader"
> #define HVM_XS_BIOS "hvmloader/bios"
> #define HVM_XS_GENERATION_ID_ADDRESS "hvmloader/generation-id-address"
> +#define HVM_XS_ALLOW_MEMORY_RELOCATE "hvmloader/allow-memory-relocate"
>
> /* The following values allow additional ACPI tables to be added to the
> * virtual ACPI BIOS that hvmloader constructs. The values specify the guest
> --
> 1.7.9.5
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |