From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH v2 5/5] libxl, hvmloader: Don't relocate memory for MMIO hole Date: Tue, 18 Jun 2013 18:16:49 +0100 Message-ID: <51C09601.1020105@eu.citrix.com> References: <1371573984-28514-1-git-send-email-george.dunlap@eu.citrix.com> <1371573984-28514-5-git-send-email-george.dunlap@eu.citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1371573984-28514-5-git-send-email-george.dunlap@eu.citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: George Dunlap Cc: Ian Jackson , Hanweidong , Stefano Stabellini , Ian Campbell , xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 06/18/2013 05:46 PM, 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 > CC: Ian Campbell > CC: Ian Jackson > CC: Stefano Stabellini > CC: Hanweidong > --- > 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 > #include > +#include > +#include > > 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; > + > + 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"); > + > /* 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. > + * > + * 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)) ) Hmm, the polarity on the comparison here is wrong -- it should be >=, not <. I've hacked up qemu so that it presents a 256MiB xen platform pci device. I'll re-send when I've done some more testing. But I have, I think, reproduced the original problem; the above error means that for qemu-xen, it will resize the MMIO hole only if it *can* move guest memory. :-) -George