hvmloader - pci - relevance of qemu-traditional workaround

[Neowutran <xen@xxxxxxxxxxxxx>]

Hello, 

In QubesOS I had the following issue 
(https://github.com/QubesOS/qubes-issues/issues/4321):

When I create a HVM and passthrough a GPU (pci device), it seems to
trash the memory of the guest and either prevent the HVM to be started
or prevent the GPU to be used. 

Looking more at the details, it is a memory corruption issue provoked
by the hvmloader of xen. 
This specific issue is well documented in the xen source code here: 

https://github.com/xen-project/xen/blob/master/tools/firmware/hvmloader/pci.c#L105

"
   /*
     * Do we allow hvmloader to relocate guest memory in order to
     * increase the size of the lowmem MMIO hole?  Defaulting to 1
     * here will
 mean that non-libxl toolstacks (including xend and
     * home-grown ones) means that those using qemu-xen will still
     * experience the memory relocation bug described below; but it
     * also means that those using qemu-traditional will *not*
     * experience any change; and it also means that there is a
     * work-around for those using qemu-xen, namely switching to
     * qemu-traditional.
     *
     * If we defaulted to 0, and failing to resize the hole caused any
     * problems with qemu-traditional, then there is no work-around.
     *
     * Since xend can only use qemu-traditional, I think this is the
     * option that will have the least impact.
     */
    bool allow_memory_relocate = 1;
"

and the corruption happen here: 

https://github.com/xen-project/xen/blob/master/tools/firmware/hvmloader/pci.c#L355

"
        /*
         * 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 t
he 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)) )
            pci_mem_start <<= 1;
"

I wrote a small patch (https://github.com/QubesOS/qubes-vmm-xen/pull/172/files) 
to force the value of "allow_memory_relocate" to be 0.
It fixed the issue I was having, I saw no drawback,
 so I am suggesting this patch to the QubesOS project. 

But I have some questions: 

- In the xen source code, the default value of "allow_memory_relocate" have been
defined to 1 for qemu-traditional support. As of today, should this default 
value still be "1" ? 

- Does something have changed regarding "At the moment qemu-xen can't deal with
relocated memory regions." ? / something else should be modified ?



Many thanks, 
Neowutran