Patches for PCI passthrough with modified E820.

tools/libxc/xc_domain.c +15 -0

     return rc;
 }
+
+int xc_domain_set_memory_map(xc_interface *xch,
+                               uint32_t domid,
+                               struct e820entry entries[],
+                               uint32_t nr_entries)
+{
+    int rc;
+    struct xen_foreign_memory_map fmap = {
+        .domid = domid,
+        .map = { .nr_entries = nr_entries }
+    };
+    DECLARE_HYPERCALL_BOUNCE(entries, nr_entries * sizeof(struct e820entry),
+                             XC_HYPERCALL_BUFFER_BOUNCE_IN);
+
+    if ( !entries || xc_hypercall_bounce_pre(xch, entries) )

Quoted 12 lines · Konrad Rzeszutek Wilk

# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]>
# Date 1302194186 14400
# Node ID decab6c21cc3d7ce4d4dad949d34ba35d4600490
# Parent 97763efc41f9b664cf6f7db653c9c3f51e50b358
tools: Add xc_domain_set_memory_map and xc_get_machine_memory_map calls.

The later retrieves the E820 as seen by the hypervisor (completly unchanged) and the second call sets the E820 for a specific guest.

Signed-off-by Konrad Rzeszutek Wilk <[email protected]>

tools/libxc/xc_domain.c

Quoted 4 lines

diff -r 97763efc41f9 -r decab6c21cc3 tools/libxc/xc_domain.c

tools/libxc/xc_domain.c +0 -0

@@ -510,6 +510,55 @@

Please add:
[diff]
showfunc = True
to your ~/.hgrc

Quoted 24 lines

tools/libxc/xc_domain.c +20 -0

     return rc;
 }
+
+int xc_domain_set_memory_map(xc_interface *xch,
+                               uint32_t domid,
+                               struct e820entry entries[],
+                               uint32_t nr_entries)
+{
+    int rc;
+    struct xen_foreign_memory_map fmap = {
+        .domid = domid,
+        .map = { .nr_entries = nr_entries }
+    };
+    DECLARE_HYPERCALL_BOUNCE(entries, nr_entries * sizeof(struct e820entry),
+                             XC_HYPERCALL_BUFFER_BOUNCE_IN);
+
+    if ( !entries || xc_hypercall_bounce_pre(xch, entries) )
+        return -1;
+
+    set_xen_guest_handle(fmap.map.buffer, entries);
+
+    rc = do_memory_op(xch, XENMEM_set_memory_map, &fmap, sizeof(fmap));

tools/libxc/xc_domain.c line 537

Quoted 5 lines

tools/libxc/xc_domain.c +5 -0

+
+    xc_hypercall_bounce_post(xch, entries);
+
+    return rc;
+}

Should probably reimplement xc_domain_set_memmap_limit() as a wrapper around this new function. And/or nuke it if that's an option.

# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]> # Date 1302202697 14400
# Node ID 01d0d338b97491a3aa816dab43cc709a234214f7 # Parent decab6c21cc3d7ce4d4dad949d34ba35d4600490
x86: make the pv-only e820 array be dynamic.

During creation of the PV domain we allocate the E820 structure to be the E820MAX. This will allow the tool stack to fill the E820 with more than three entries.

Signed-off-by Konrad Rzeszutek Wilk <[email protected]>

diff -r decab6c21cc3 -r 01d0d338b974 xen/arch/x86/domain.c

xen/arch/x86/domain.c +16 -1

@@ -634,14 +634,23 @@
                 d->arch.pirq_emuirq[i] = IRQ_UNBOUND;
             for (i = 0; i < nr_irqs; i++)
                 d->arch.emuirq_pirq[i] = IRQ_UNBOUND;
+        } else
+        {
+          d->arch.pv_domain.e820 = xmalloc_array(struct e820entry, E820MAX);
+
+          if ( !d->arch.pv_domain.e820 )
+            goto fail;
+
+          memset(d->arch.pv_domain.e820, 0,
+                E820MAX * sizeof(*d->arch.pv_domain.e820));
         }
 
-
         if ( (rc = iommu_domain_init(d)) != 0 )
             goto fail;
 
         /* For Guest vMCE MSRs virtualization */
         vmce_init_msr(d);
+
     }
 
     if ( is_hvm_domain(d) )

@@ -668,6 +677,10 @@
  fail:
     d->is_dying = DOMDYING_dead;
     vmce_destroy_msr(d);
+    if ( !is_hvm_domain(d) )
+    {
+      xfree(d->arch.pv_domain.e820);
+    }
     xfree(d->arch.pirq_irq);
     xfree(d->arch.irq_pirq);
     xfree(d->arch.pirq_emuirq);

@@ -696,6 +709,8 @@
 
     if ( is_hvm_domain(d) )
         hvm_domain_destroy(d);
+    else
+      xfree(d->arch.pv_domain.e820);
 
     vmce_destroy_msr(d);
     pci_release_devices(d);

diff -r decab6c21cc3 -r 01d0d338b974 xen/arch/x86/mm.c

xen/arch/x86/mm.c +12 -2

@@ -4710,7 +4710,7 @@
         if ( copy_from_guest(&fmap, arg, 1) )
             return -EFAULT;
 
-        if ( fmap.map.nr_entries > ARRAY_SIZE(d->arch.pv_domain.e820) )
+        if ( fmap.map.nr_entries > E820MAX )
             return -EINVAL;
 
         rc = rcu_lock_target_domain_by_id(fmap.domid, &d);

@@ -4730,9 +4730,16 @@
             return -EPERM;
         }
 
+        if ( d->arch.pv_domain.e820 == NULL )
+        {
+            rcu_unlock_domain(d);
+            return -EINVAL;
+        }
         rc = copy_from_guest(d->arch.pv_domain.e820, fmap.map.buffer,
                              fmap.map.nr_entries) ? -EFAULT : 0;
-        d->arch.pv_domain.nr_e820 = fmap.map.nr_entries;
+
+        if ( rc == 0 )
+          d->arch.pv_domain.nr_e820 = fmap.map.nr_entries;
 
         rcu_unlock_domain(d);
         return rc;

@@ -4747,6 +4754,9 @@
         if ( d->arch.pv_domain.nr_e820 == 0 )
             return -ENOSYS;
 
+        if ( d->arch.pv_domain.e820 == NULL )
+            return -ENOSYS;
+
         if ( copy_from_guest(&map, arg, 1) )
             return -EFAULT;
 

diff -r decab6c21cc3 -r 01d0d338b974 xen/include/asm-x86/domain.h

xen/include/asm-x86/domain.h +1 -1

@@ -238,7 +238,7 @@
     unsigned long pirq_eoi_map_mfn;
 
     /* Pseudophysical e820 map (XENMEM_memory_map).  */
-    struct e820entry e820[3];
+    struct e820entry *e820;
     unsigned int nr_e820;
 };
 

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Quoted 7 lines · Konrad Rzeszutek Wilk

# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]> # Date 1302202697 14400
# Node ID 01d0d338b97491a3aa816dab43cc709a234214f7 # Parent decab6c21cc3d7ce4d4dad949d34ba35d4600490
x86: make the pv-only e820 array be dynamic.

Quoted 3 lines

During creation of the PV domain we allocate the E820 structure to be the E820MAX. This will allow the tool stack to fill the E820 with more than three entries.

Is it possible to defer this allocation to set_memory_map time so we can allocate only the required size? I guess E820MAX (128) entries is only 2.5k but a typical e820 is capped at more like 20 entries in practice (<0.5k).

# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]>
# Date 1302206363 14400
# Node ID 2e464234c94cfd29a98a9011a46d76846b87f7f8
# Parent 546d8a03d5cbe0ceddadf701174f2417a0b72891
libxl: Add support for passing in the machine's E820 for PCI passthrough.

The code (libxl_e820_alloc) calls the xc_get_machine_memory_map to retrieve the systems E820. Then the E820 is sanitized to weed out E820 entries below 16MB, and as well remove any E820_RAM or E820_UNUSED regions as the guest does not need to know abou them. The guest only needs the E820_ACPI, E820_NVS, E820_RESERVED to get an idea of where the PCI I/O space is. Mostly.. The Linux kernel assumes that any gap in the E820 is considered PCI I/O space which means that if we pass in the guest 2GB, and the E820_ACPI, and its friend start at 3GB, the gap between 2GB and 3GB will be considered as PCI I/O space. To guard against that we also create an E820_UNUSABLE between the region of 'target_kb' (called ram_end in the code) up to the first E820_[ACPI,NVS,RESERVED] region.

Memory that is slack or for balloon (so 'maxmem' in guest configuration) is put behind the machine E820. Which in most cases is after the 4GB.

The reason for doing the fetching of the E820 using the hypercall in the toolstack (instead of the guest doing it) is that when a Linux guest would do a hypercall to 'XENMEM_machine_memory_map' it would retrieve an E820 with I/O range caps added in. Meaning that the region after 4GB up to end of possible memory would be marked as unusuable and the Linux kernel would not have any space to allocate a balloon region.

Signed-off-by Konrad Rzeszutek Wilk <[email protected]>

diff -r 546d8a03d5cb -r 2e464234c94c tools/libxl/libxl.h

diff -r 546d8a03d5cb -r 2e464234c94c tools/libxl/libxl_pci.c

tools/libxl/libxl_pci.c Thu Apr 07 15:59:23 2011 -0400 renamed from tools/libxl/libxl_pci.c Thu Apr 07 15:18:46 2011 -0400 +2 -0

@@ -37,6 +37,8 @@
 #include "libxl_internal.h"
 #include "flexarray.h"
 
+#include <xc_e820.h>
+
 #define PCI_BDF                "%04x:%02x:%02x.%01x"
 #define PCI_BDF_SHORT          "%02x:%02x.%01x"
 #define PCI_BDF_VDEVFN         "%04x:%02x:%02x.%01x@%02x"

Are we expecting that libxl users might want to modify the e820? If not then why expose libxl_e820_alloc and libxl_e820_sanitize at all, just add a flag to the libxl interface which says whether or not to provide a host-derived e820.

Ian Campbell writes ("Re: [Xen-devel] [PATCH 4 of 5] libxl: Add support for passing in the machine's E820 for PCI passthrough"):

Quoted 4 lines

Are we expecting that libxl users might want to modify the e820? If not then why expose libxl_e820_alloc and libxl_e820_sanitize at all, just add a flag to the libxl interface which says whether or not to provide a host-derived e820.

Well, also, why do we need that flag at all ? Are we trying to do something different with domains which might get pci passthrough ? If

so then that's what should be specified at the libxl api, surely, rather than some opaque "write this rune to make it work" option.

Quoted 8 lines · Ian Jackson

Ian Campbell writes ("Re: [Xen-devel] [PATCH 4 of 5] libxl: Add support for passing in the machine's E820 for PCI passthrough"):

Quoted 4 lines

Are we expecting that libxl users might want to modify the e820? If not then why expose libxl_e820_alloc and libxl_e820_sanitize at all, just add a flag to the libxl interface which says whether or not to provide a host-derived e820.

Well, also, why do we need that flag at all ? Are we trying to do something different with domains which might get pci passthrough ? If

Yes. Well, it does work OK even if you are _not_ doing PCI passthrough. But the main users for this are the ones doing PCI passthrough.

Quoted 2 lines

so then that's what should be specified at the libxl api, surely, rather than some opaque "write this rune to make it work" option.

OK. If I am understanding you guys right, you are saying, latch it of the 'pci' option instead of this 'pci_hole' option.

And do not expose thse libxl_e820_* functions, but keep them internal to the libxl code.

Quoted 13 lines · Konrad Rzeszutek Wilk

Quoted 8 lines · Ian Jackson

Ian Campbell writes ("Re: [Xen-devel] [PATCH 4 of 5] libxl: Add support for passing in the machine's E820 for PCI passthrough"):

Quoted 4 lines

Are we expecting that libxl users might want to modify the e820? If not then why expose libxl_e820_alloc and libxl_e820_sanitize at all, just add a flag to the libxl interface which says whether or not to provide a host-derived e820.

Well, also, why do we need that flag at all ? Are we trying to do something different with domains which might get pci passthrough ? If

Yes. Well, it does work OK even if you are _not_ doing PCI passthrough. But the main users for this are the ones doing PCI passthrough.

Quoted 5 lines

Quoted 2 lines

so then that's what should be specified at the libxl api, surely, rather than some opaque "write this rune to make it work" option.

OK. If I am understanding you guys right, you are saying, latch it of the 'pci' option instead of this 'pci_hole' option.

ACK. For the pure-hotplug case an empty pci=[] ought to suffice to trigger this functionality.

Quoted 17 lines · Ian Campbell

Quoted 16 lines · Konrad Rzeszutek Wilk

Quoted 8 lines · Ian Jackson

Ian Campbell writes ("Re: [Xen-devel] [PATCH 4 of 5] libxl: Add support for passing in the machine's E820 for PCI passthrough"):

Quoted 4 lines

Are we expecting that libxl users might want to modify the e820? If not then why expose libxl_e820_alloc and libxl_e820_sanitize at all, just add a flag to the libxl interface which says whether or not to provide a host-derived e820.

Well, also, why do we need that flag at all ? Are we trying to do something different with domains which might get pci passthrough ? If

Yes. Well, it does work OK even if you are _not_ doing PCI passthrough. But the main users for this are the ones doing PCI passthrough.

Quoted 2 lines

so then that's what should be specified at the libxl api, surely, rather than some opaque "write this rune to make it work" option.

Quoted 5 lines

Quoted 2 lines

OK. If I am understanding you guys right, you are saying, latch it of the 'pci' option instead of this 'pci_hole' option.

ACK. For the pure-hotplug case an empty pci=[] ought to suffice to trigger this functionality.

That's a _horrible_ config syntax.

We should either do this for all
guests or give it its own option (which can default to 1 for guests with non-empty passthrough lists if you like).

Quoted 1 line

That's a _horrible_ config syntax.

fair enough. I thought it nicely represented the "has an empty PCI bus" myself...

Quoted 3 lines

We should either do this for all
guests or give it its own option (which can default to 1 for guests with non-empty passthrough lists if you like).

Ack, if it's an option it should definitely default to on for non-empty pci passthrough lists. I think we can probably get away with default on always though and allow it to be explicitly disabled. That's really just a case of trying on a few PV OSes and seeing how much they barf...

Quoted 4 lines · Ian Campbell

Quoted 1 line

That's a _horrible_ config syntax.

fair enough. I thought it nicely represented the "has an empty PCI bus" myself...

Sure, but it's really not obvious that that means "has a memory layout that matches the host on which it was last booted".

Quoted 6 lines · Tim Deegan

Quoted 4 lines · Ian Campbell

Quoted 1 line

That's a _horrible_ config syntax.

fair enough. I thought it nicely represented the "has an empty PCI bus" myself...

Quoted 2 lines

Sure, but it's really not obvious that that means "has a memory layout that matches the host on which it was last booted".

Hmm, yes, this proposal doesn't really cover migration does it...

You obviously can't migrate with a device passed through but it should be possible to unplug, migrate and replug. Maybe having a sensible hole

on the first host is sufficient... Especially if the annoying case is GFX passthrough -- you are much less like to want/be able to unplug your gfx over a migration...

Quoted 4 lines · Ian Campbell

Quoted 1 line

That's a _horrible_ config syntax.

fair enough. I thought it nicely represented the "has an empty PCI bus" myself...

Sure, but it's really not obvious that that means "has a memory layout that matches the host on which it was last booted".

Quoted 4 lines

Quoted 3 lines

We should either do this for all
guests or give it its own option (which can default to 1 for guests with non-empty passthrough lists if you like).

Quoted 4 lines

Ack, if it's an option it should definitely default to on for non-empty pci passthrough lists. I think we can probably get away with default on always though and allow it to be explicitly disabled. That's really just a case of trying on a few PV OSes and seeing how much they barf...

Agreed. My guess is there won't be any trouble.

Quoted 18 lines · Tim Deegan

Quoted 4 lines · Ian Campbell

Quoted 1 line

That's a _horrible_ config syntax.

fair enough. I thought it nicely represented the "has an empty PCI bus" myself...

Sure, but it's really not obvious that that means "has a memory layout that matches the host on which it was last booted".

Quoted 8 lines

Quoted 3 lines

We should either do this for all
guests or give it its own option (which can default to 1 for guests with non-empty passthrough lists if you like).

Ack, if it's an option it should definitely default to on for non-empty pci passthrough lists. I think we can probably get away with default on always though and allow it to be explicitly disabled. That's really just a case of trying on a few PV OSes and seeing how much they barf...

Quoted 1 line

Agreed. My guess is there won't be any trouble.

Whoa. I just booted an SLES11 32 bit and it booted so nicely :-) (2.6.27)

Time to try out some RHEL guests.

Quoted 19 lines · Konrad Rzeszutek Wilk

Quoted 18 lines · Tim Deegan

Quoted 4 lines · Ian Campbell

Quoted 1 line

That's a _horrible_ config syntax.

fair enough. I thought it nicely represented the "has an empty PCI bus" myself...

Sure, but it's really not obvious that that means "has a memory layout that matches the host on which it was last booted".

Quoted 8 lines

Quoted 3 lines

We should either do this for all
guests or give it its own option (which can default to 1 for guests with non-empty passthrough lists if you like).

Ack, if it's an option it should definitely default to on for non-empty pci passthrough lists. I think we can probably get away with default on always though and allow it to be explicitly disabled. That's really just a case of trying on a few PV OSes and seeing how much they barf...

Quoted 4 lines

Quoted 1 line

Agreed. My guess is there won't be any trouble.

Whoa. I just booted an SLES11 32 bit and it booted so nicely :-) (2.6.27)

With all of its memory, or did it loose the bits from the holes?

Quoted 4 lines · Ian Campbell

Quoted 1 line

That's a _horrible_ config syntax.

fair enough. I thought it nicely represented the "has an empty PCI bus" myself...

Sure, but it's really not obvious that that means "has a memory layout that matches the host on which it was last booted".

Quoted 39 lines · Konrad Rzeszutek Wilk

# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]>
# Date 1302206363 14400
# Node ID 2e464234c94cfd29a98a9011a46d76846b87f7f8
# Parent 546d8a03d5cbe0ceddadf701174f2417a0b72891
libxl: Add support for passing in the machine's E820 for PCI passthrough.

The code (libxl_e820_alloc) calls the xc_get_machine_memory_map to retrieve the systems E820. Then the E820 is sanitized to weed out E820 entries below 16MB, and as well remove any E820_RAM or E820_UNUSED regions as the guest does not need to know abou them. The guest only needs the E820_ACPI, E820_NVS, E820_RESERVED to get an idea of where the PCI I/O space is. Mostly.. The Linux kernel assumes that any gap in the E820 is considered PCI I/O space which means that if we pass in the guest 2GB, and the E820_ACPI, and its friend start at 3GB, the gap between 2GB and 3GB will be considered as PCI I/O space. To guard against that we also create an E820_UNUSABLE between the region of 'target_kb' (called ram_end in the code) up to the first E820_[ACPI,NVS,RESERVED] region.

Memory that is slack or for balloon (so 'maxmem' in guest configuration) is put behind the machine E820. Which in most cases is after the 4GB.

The reason for doing the fetching of the E820 using the hypercall in the toolstack (instead of the guest doing it) is that when a Linux guest would do a hypercall to 'XENMEM_machine_memory_map' it would retrieve an E820 with I/O range caps added in. Meaning that the region after 4GB up to end of possible memory would be marked as unusuable and the Linux kernel would not have any space to allocate a balloon region.

Signed-off-by Konrad Rzeszutek Wilk <[email protected]>

diff -r 546d8a03d5cb -r 2e464234c94c tools/libxl/libxl.h

tools/libxl/libxl.h Thu Apr 07 15:59:23 2011 -0400 renamed from tools/libxl/libxl.h Thu Apr 07 15:18:46 2011 -0400 +0 -0

@@ -204,6 +204,14 @@
 } libxl_file_reference;
 void libxl_file_reference_destroy(libxl_file_reference *p);
 

tools/libxl/libxl.h Thu Apr 07 15:59:23 2011 -0400 line 207

Quoted 1 line

tools/libxl/libxl.h Thu Apr 07 15:59:23 2011 -0400 renamed from tools/libxl/libxl.h Thu Apr 07 15:18:46 2011 -0400 +1 -0

+#include <xc_e820.h>

We are trying to avoid exposing libxc headers to libxl users. struct e820entry is well defined and unchanging so I don't think there is much harm in duplicating it.

tools/libxl/libxl.h Thu Apr 07 15:59:23 2011 -0400 line 211

Quoted 5 lines

tools/libxl/libxl.h Thu Apr 07 15:59:23 2011 -0400 renamed from tools/libxl/libxl.h Thu Apr 07 15:18:46 2011 -0400 +4 -0

+typedef struct {
+    uint32_t nr_entries;
+    struct e820entry *entry;
+} libxl_e820;

BTW does the e820entry array need to be packed at some point before getting returned to the guest? (I'm not sure what the expected alignment

is, nor how a 20 byte struct containing two u64s and a u32 naturally aligns on x86_64).

tools/libxl/libxl.idl Thu Apr 07 15:59:23 2011 -0400 line 25

Quoted 9 lines

diff -r 546d8a03d5cb -r 2e464234c94c tools/libxl/libxl.idl

tools/libxl/libxl.idl Thu Apr 07 15:59:23 2011 -0400 renamed from tools/libxl/libxl.idl Thu Apr 07 15:18:46 2011 -0400 +1 -0

@@ -22,6 +22,7 @@
 
 libxl_hwcap = Builtin("hwcap")
 
+libxl_e820 = Builtin("e820", destructor_fn="libxl_e820_destroy", passby=PASS_BY_REFERENCE)

Hmmm, We are starting to collect a lot of builtins of the form {len, pointer}. Time to start thinking of a new type class abstraction, I think... (not your problem in this series though)

Quoted 7 lines

diff -r 546d8a03d5cb -r 2e464234c94c tools/libxl/libxl_pci.c

tools/libxl/libxl_pci.c Thu Apr 07 15:59:23 2011 -0400 renamed from tools/libxl/libxl_pci.c Thu Apr 07 15:18:46 2011 -0400 +0 -0

@@ -37,6 +37,8 @@
 #include "libxl_internal.h"
 #include "flexarray.h"
 

tools/libxl/libxl_pci.c Thu Apr 07 15:59:23 2011 -0400 line 1053

Quoted 11 lines

tools/libxl/libxl_pci.c Thu Apr 07 15:59:23 2011 -0400 renamed from tools/libxl/libxl_pci.c Thu Apr 07 15:18:46 2011 -0400 +4 -0

+#include <xc_e820.h>
+
 #define PCI_BDF                "%04x:%02x:%02x.%01x"
 #define PCI_BDF_SHORT          "%02x:%02x.%01x"
 #define PCI_BDF_VDEVFN         "%04x:%02x:%02x.%01x@%02x"

@@ -1047,3 +1049,154 @@
     free(pcidevs);
     return 0;
 }
+
+#define E820MAX (128)

Should be somewhere public for the benefit of libxl users who want to setup an e820? Even if it should be private it should be in a .h.

Are we expecting that libxl users might want to modify the e820? If not then why expose libxl_e820_alloc and libxl_e820_sanitize at all, just add a flag to the libxl interface which says whether or not to provide a host-derived e820.

If users are expected to modify it then I'm in two minds about doing the sanitize step at domain build time instead of in libxl_e820_alloc. If we

provide a default sanitized e820 to the caller and they want to modify it to be non-santized should we let them? I guess the question here is whether we are sanitizing actually invalid e820 maps or if we are also enforcing some higher level policy based on a specific class Linux guest's requirements.

Quoted 9 lines · Ian Campbell

tools/libxl/libxl.h line 211

Quoted 4 lines · Konrad Rzeszutek Wilk

tools/libxl/libxl.h +4 -0

+typedef struct {
+    uint32_t nr_entries;
+    struct e820entry *entry;
+} libxl_e820;

BTW does the e820entry array need to be packed at some point before getting returned to the guest? (I'm not sure what the expected alignment

The struct e820entry has the __packed__ attribute on it.

Quoted 2 lines

is, nor how a 20 byte struct containing two u64s and a u32 naturally aligns on x86_64).

It seems to work OK.. (I booted a 32-bit guest under a 64-bit hypervisor with a 64-bit dom0).

Quoted 4 lines

Are we expecting that libxl users might want to modify the e820? If not then why expose libxl_e820_alloc and libxl_e820_sanitize at all, just add a flag to the libxl interface which says whether or not to provide a host-derived e820.

OK. Where should I put the definitions of these two functions? (They are called from libxl_dom.c and xl_cmdimpl.c) Is there a internal header file?

Quoted 2 lines

If users are expected to modify it then I'm in two minds about doing the sanitize step at domain build time instead of in libxl_e820_alloc. If we

The issue I had was that to construct the e820, I needed the 'target_kb' and 'max_memkb' and 'slack_memkb'. If I can get those two values when the guest config file is parsed (so xl_cmdimpl.c) I think moving the sanitization in there (and not doing it during domain build time) is the right thing to do. Let me see if I can do that.

Quoted 5 lines

provide a default sanitized e820 to the caller and they want to modify it to be non-santized should we let them? I guess the question here is whether we are sanitizing actually invalid e820 maps or if we are also enforcing some higher level policy based on a specific class Linux guest's requirements.

Which might be quite different if the guest is a BSD one. I guess it is time to start launching those NetBSD guests :-)

This set of RFC patches allows a PV domain to see the machine's E820 and figure out where the "PCI I/O" gap is and match it with the reality.

To use this patchset, the guest config file has to have the parameter 'pci_hole=1' enabled (hmm, any ideas for a better name?)

Quoted 3 lines · Konrad Rzeszutek Wilk

This set of RFC patches allows a PV domain to see the machine's E820 and figure out where the "PCI I/O" gap is and match it with the reality.

Does the domain builder obey this memory map at all or is it a PV guests responsibility to take the linear p2m allocation it starts with a move stuff around to fit the map?

Quoted 3 lines

To use this patchset, the guest config file has to have the parameter 'pci_hole=1' enabled (hmm, any ideas for a better name?)

Is there any harm in just doing this for any guest configuration which has a "pci" option specified? (including the empty list "pci=[]" to handle guests which only want hotplug capabilities not an initial set of devices).

Or could we even go so far as to consider always doing this unconditionally?

Will older pvops and/or classic-Xen kernels or other PV OSes misbehave

if we do either of these? is having a default-on option which these users need to force off better or worse than a default-off option which the opposite set of people need to enable?

Ian.