[RFC PATCH 0/1] Add pci_hole_min_size

[RFC PATCH 0/1] Add pci_hole_min_size

[Don Slutz]

This is posted as an RFC because you need the upstream version of qemu with:

http://marc.info/?l=qemu-devel&m=139354036513567&w=2

This allows growing the pci_hole to the size needed.

This may help with using pci passthru and HVM.

Don Slutz (1):
  Add pci_hole_min_size

 tools/firmware/hvmloader/pci.c | 10 ++++++++++
 tools/libxc/xc_hvm_build_x86.c | 22 ++++++++++++++++++++++
 tools/libxc/xenguest.h         | 11 +++++++++++
 tools/libxl/libxl_create.c     |  4 +++-
 tools/libxl/libxl_dm.c         | 13 +++++++++++--
 tools/libxl/libxl_dom.c        |  3 ++-
 tools/libxl/libxl_types.idl    |  1 +
 tools/libxl/xl_cmdimpl.c       |  6 ++++++
 8 files changed, 66 insertions(+), 4 deletions(-)

-- 
1.8.4

[RFC PATCH 1/1] Add pci_hole_min_size

[Don Slutz]

This allows growing the pci_hole to the size needed.

Signed-off-by: Don Slutz <dslutz@verizon.com>
---
 tools/firmware/hvmloader/pci.c | 10 ++++++++++
 tools/libxc/xc_hvm_build_x86.c | 22 ++++++++++++++++++++++
 tools/libxc/xenguest.h         | 11 +++++++++++
 tools/libxl/libxl_create.c     |  4 +++-
 tools/libxl/libxl_dm.c         | 13 +++++++++++--
 tools/libxl/libxl_dom.c        |  3 ++-
 tools/libxl/libxl_types.idl    |  1 +
 tools/libxl/xl_cmdimpl.c       |  6 ++++++
 8 files changed, 66 insertions(+), 4 deletions(-)

diff --git a/tools/firmware/hvmloader/pci.c b/tools/firmware/hvmloader/pci.c
index 627e8cb..b36d00b 100644
--- a/tools/firmware/hvmloader/pci.c
+++ b/tools/firmware/hvmloader/pci.c
@@ -58,6 +58,7 @@ void pci_setup(void)
         uint64_t bar_sz;
     } *bars = (struct bars *)scratch_start;
     unsigned int i, nr_bars = 0;
+    uint64_t pci_hole_min_size;
 
     const char *s;
     /*
@@ -85,6 +86,9 @@ void pci_setup(void)
     printf("Relocating guest memory for lowmem MMIO space %s\n",
            allow_memory_relocate?"enabled":"disabled");
 
+    s = xenstore_read("platform/pci_hole_min_size", "0");
+    pci_hole_min_size = strtoll(s, NULL, 0);
+
     /* Program PCI-ISA bridge with appropriate link routes. */
     isa_irq = 0;
     for ( link = 0; link < 4; link++ )
@@ -236,6 +240,12 @@ void pci_setup(void)
         pci_writew(devfn, PCI_COMMAND, cmd);
     }
 
+    if (pci_hole_min_size) {
+        pci_mem_start = (1ull << 32) - pci_hole_min_size;
+        printf("pci_mem_start=0x%lx (was 0x%x) for pci_hole_min_size=%llu\n",
+               pci_mem_start, PCI_MEM_START, pci_hole_min_size);
+    }
+
     /*
      * 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,
diff --git a/tools/libxc/xc_hvm_build_x86.c b/tools/libxc/xc_hvm_build_x86.c
index dd3b522..4752d58 100644
--- a/tools/libxc/xc_hvm_build_x86.c
+++ b/tools/libxc/xc_hvm_build_x86.c
@@ -603,16 +603,38 @@ int xc_hvm_build_target_mem(xc_interface *xch,
                            int target,
                            const char *image_name)
 {
+    return xc_hvm_build_target_mem_with_hole(xch, domid, memsize, target, image_name, 0);
+}
+
+int xc_hvm_build_target_mem_with_hole(xc_interface *xch,
+                                      uint32_t domid,
+                                      int memsize,
+                                      int target,
+                                      const char *image_name,
+                                      uint64_t pci_hole_min_size)
+{
     struct xc_hvm_build_args args = {};
 
     memset(&args, 0, sizeof(struct xc_hvm_build_args));
     args.mem_size = (uint64_t)memsize << 20;
     args.mem_target = (uint64_t)target << 20;
     args.image_file_name = image_name;
+    if (pci_hole_min_size > HVM_BELOW_4G_MMIO_LENGTH)
+        args.mmio_size = pci_hole_min_size;
 
     return xc_hvm_build(xch, domid, &args);
 }
 
+int xc_hvm_build_with_hole(xc_interface *xch, uint32_t domid,
+                           struct xc_hvm_build_args *args,
+                           uint64_t pci_hole_min_size)
+{
+    if (pci_hole_min_size > HVM_BELOW_4G_MMIO_LENGTH)
+        args->mmio_size = pci_hole_min_size;
+
+    return xc_hvm_build(xch, domid, args);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxc/xenguest.h b/tools/libxc/xenguest.h
index a0e30e1..44290bd 100644
--- a/tools/libxc/xenguest.h
+++ b/tools/libxc/xenguest.h
@@ -248,12 +248,23 @@ struct xc_hvm_build_args {
 int xc_hvm_build(xc_interface *xch, uint32_t domid,
                  struct xc_hvm_build_args *hvm_args);
 
+int xc_hvm_build_with_hole(xc_interface *xch, uint32_t domid,
+                           struct xc_hvm_build_args *args,
+                           uint64_t pci_hole_min_size);
+
 int xc_hvm_build_target_mem(xc_interface *xch,
                             uint32_t domid,
                             int memsize,
                             int target,
                             const char *image_name);
 
+int xc_hvm_build_target_mem_with_hole(xc_interface *xch,
+                                      uint32_t domid,
+                                      int memsize,
+                                      int target,
+                                      const char *image_name,
+                                      uint64_t pci_hole_min_size);
+
 int xc_suspend_evtchn_release(xc_interface *xch, xc_evtchn *xce, int domid, int suspend_evtchn);
 
 int xc_suspend_evtchn_init(xc_interface *xch, xc_evtchn *xce, int domid, int port);
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index a604cd8..24ceac6 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -388,13 +388,15 @@ int libxl__domain_build(libxl__gc *gc,
         vments[4] = "start_time";
         vments[5] = libxl__sprintf(gc, "%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
 
-        localents = libxl__calloc(gc, 7, sizeof(char *));
+        localents = libxl__calloc(gc, 9, sizeof(char *));
         localents[0] = "platform/acpi";
         localents[1] = libxl_defbool_val(info->u.hvm.acpi) ? "1" : "0";
         localents[2] = "platform/acpi_s3";
         localents[3] = libxl_defbool_val(info->u.hvm.acpi_s3) ? "1" : "0";
         localents[4] = "platform/acpi_s4";
         localents[5] = libxl_defbool_val(info->u.hvm.acpi_s4) ? "1" : "0";
+        localents[6] = "platform/pci_hole_min_size";
+        localents[7] = libxl__sprintf(gc, "%llu", (unsigned long long)info->u.hvm.pci_hole_min_size);
 
         break;
     case LIBXL_DOMAIN_TYPE_PV:
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index f6f7bbd..66ba7cd 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -653,17 +653,26 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
             flexarray_append(dm_args, b_info->extra_pv[i]);
         break;
     case LIBXL_DOMAIN_TYPE_HVM:
+    {
+        char *machinearg;
+
         if (!libxl_defbool_val(b_info->u.hvm.xen_platform_pci)) {
             /* Switching here to the machine "pc" which does not add
              * the xen-platform device instead of the default "xenfv" machine.
              */
-            flexarray_append(dm_args, "pc,accel=xen");
+            machinearg = libxl__sprintf(gc, "pc,accel=xen");
         } else {
-            flexarray_append(dm_args, "xenfv");
+            machinearg = libxl__sprintf(gc, "xenfv");
+        }
+        if (b_info->u.hvm.pci_hole_min_size) {
+            machinearg = libxl__sprintf(gc, "%s,pci_hole_min_size=%llu", machinearg,
+                            (unsigned long long) b_info->u.hvm.pci_hole_min_size);
         }
+        flexarray_append(dm_args, machinearg);
         for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
             flexarray_append(dm_args, b_info->extra_hvm[i]);
         break;
+    }
     default:
         abort();
     }
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 55f74b2..6ebc606 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -642,7 +642,8 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
         goto out;
     }
 
-    ret = xc_hvm_build(ctx->xch, domid, &args);
+    ret = xc_hvm_build_with_hole(ctx->xch, domid, &args,
+                                 info->u.hvm.pci_hole_min_size);
     if (ret) {
         LOGEV(ERROR, ret, "hvm building failed");
         goto out;
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 649ce50..febfbb4 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -346,6 +346,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
                                        ("timeoffset",       string),
                                        ("hpet",             libxl_defbool),
                                        ("vpt_align",        libxl_defbool),
+                                       ("pci_hole_min_size",uint64),
                                        ("timer_mode",       libxl_timer_mode),
                                        ("nested_hvm",       libxl_defbool),
                                        ("smbios_firmware",  string),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 4fc46eb..fe247ee 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1025,6 +1025,12 @@ static void parse_config_data(const char *config_source,
         xlu_cfg_get_defbool(config, "hpet", &b_info->u.hvm.hpet, 0);
         xlu_cfg_get_defbool(config, "vpt_align", &b_info->u.hvm.vpt_align, 0);
 
+        if (!xlu_cfg_get_long(config, "pci_hole_min_size", &l, 0)) {
+            b_info->u.hvm.pci_hole_min_size = (uint64_t) l;
+            if (dom_info->debug)
+                fprintf(stderr, "pci_hole_min_size: %llu\n", (unsigned long long) b_info->u.hvm.pci_hole_min_size);
+        }
+
         if (!xlu_cfg_get_long(config, "timer_mode", &l, 1)) {
             const char *s = libxl_timer_mode_to_string(l);
             fprintf(stderr, "WARNING: specifying \"timer_mode\" as an integer is deprecated. "
-- 
1.8.4

Re: [RFC PATCH 1/1] Add pci_hole_min_size

[Boris Ostrovsky]

On 02/28/2014 03:15 PM, Don Slutz wrote:
> This allows growing the pci_hole to the size needed.
>
> Signed-off-by: Don Slutz<dslutz@verizon.com>
> ---
>   tools/firmware/hvmloader/pci.c | 10 ++++++++++
>   tools/libxc/xc_hvm_build_x86.c | 22 ++++++++++++++++++++++
>   tools/libxc/xenguest.h         | 11 +++++++++++
>   tools/libxl/libxl_create.c     |  4 +++-
>   tools/libxl/libxl_dm.c         | 13 +++++++++++--
>   tools/libxl/libxl_dom.c        |  3 ++-
>   tools/libxl/libxl_types.idl    |  1 +
>   tools/libxl/xl_cmdimpl.c       |  6 ++++++
>   8 files changed, 66 insertions(+), 4 deletions(-)
>
> diff --git a/tools/firmware/hvmloader/pci.c b/tools/firmware/hvmloader/pci.c
> index 627e8cb..b36d00b 100644
> --- a/tools/firmware/hvmloader/pci.c
> +++ b/tools/firmware/hvmloader/pci.c
> @@ -58,6 +58,7 @@ void pci_setup(void)
>           uint64_t bar_sz;
>       } *bars = (struct bars *)scratch_start;
>       unsigned int i, nr_bars = 0;
> +    uint64_t pci_hole_min_size;
>   
>       const char *s;
>       /*
> @@ -85,6 +86,9 @@ void pci_setup(void)
>       printf("Relocating guest memory for lowmem MMIO space %s\n",
>              allow_memory_relocate?"enabled":"disabled");
>   
> +    s = xenstore_read("platform/pci_hole_min_size", "0");

'if (s)'

> +    pci_hole_min_size = strtoll(s, NULL, 0);
> +
>       /* Program PCI-ISA bridge with appropriate link routes. */
>       isa_irq = 0;
>       for ( link = 0; link < 4; link++ )
> @@ -236,6 +240,12 @@ void pci_setup(void)
>           pci_writew(devfn, PCI_COMMAND, cmd);
>       }
>   
> +    if (pci_hole_min_size) {
> +        pci_mem_start = (1ull << 32) - pci_hole_min_size;
> +        printf("pci_mem_start=0x%lx (was 0x%x) for pci_hole_min_size=%llu\n",
> +               pci_mem_start, PCI_MEM_START, pci_hole_min_size);
> +    }
> +
>       /*
>        * 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,

...

> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index a604cd8..24ceac6 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -388,13 +388,15 @@ int libxl__domain_build(libxl__gc *gc,
>           vments[4] = "start_time";
>           vments[5] = libxl__sprintf(gc, "%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
>   
> -        localents = libxl__calloc(gc, 7, sizeof(char *));
> +        localents = libxl__calloc(gc, 9, sizeof(char *));
>           localents[0] = "platform/acpi";
>           localents[1] = libxl_defbool_val(info->u.hvm.acpi) ? "1" : "0";
>           localents[2] = "platform/acpi_s3";
>           localents[3] = libxl_defbool_val(info->u.hvm.acpi_s3) ? "1" : "0";
>           localents[4] = "platform/acpi_s4";
>           localents[5] = libxl_defbool_val(info->u.hvm.acpi_s4) ? "1" : "0";
> +        localents[6] = "platform/pci_hole_min_size";
> +        localents[7] = libxl__sprintf(gc, "%llu", (unsigned long long)info->u.hvm.pci_hole_min_size);

Do you want to always store this parameter? There is a default already 
(HVM_BELOW_4G_MMIO_LENGTH) so if it's not set in the config file it may 
be safe to omit it.

...


> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 4fc46eb..fe247ee 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -1025,6 +1025,12 @@ static void parse_config_data(const char *config_source,
>           xlu_cfg_get_defbool(config, "hpet", &b_info->u.hvm.hpet, 0);
>           xlu_cfg_get_defbool(config, "vpt_align", &b_info->u.hvm.vpt_align, 0);
>   
> +        if (!xlu_cfg_get_long(config, "pci_hole_min_size", &l, 0)) {
> +            b_info->u.hvm.pci_hole_min_size = (uint64_t) l;
> +            if (dom_info->debug)
> +                fprintf(stderr, "pci_hole_min_size: %llu\n", (unsigned long long) b_info->u.hvm.pci_hole_min_size);
> +        }

You probably want to set b_info->u.hvm.pci_hole_min_size to 0 (or 
HVM_BELOW_4G_MMIO_LENGTH?) in case it's not specified in 
libxl__domain_build_info_setdefault().

-boris


> +
>           if (!xlu_cfg_get_long(config, "timer_mode", &l, 1)) {
>               const char *s = libxl_timer_mode_to_string(l);
>               fprintf(stderr, "WARNING: specifying \"timer_mode\" as an integer is deprecated. "

Re: [RFC PATCH 1/1] Add pci_hole_min_size

[Don Slutz]

On 02/28/14 17:07, Boris Ostrovsky wrote:
> On 02/28/2014 03:15 PM, Don Slutz wrote:
>> This allows growing the pci_hole to the size needed.
>>
>> Signed-off-by: Don Slutz<dslutz@verizon.com>
>> ---
>>   tools/firmware/hvmloader/pci.c | 10 ++++++++++
>>   tools/libxc/xc_hvm_build_x86.c | 22 ++++++++++++++++++++++
>>   tools/libxc/xenguest.h         | 11 +++++++++++
>>   tools/libxl/libxl_create.c     |  4 +++-
>>   tools/libxl/libxl_dm.c         | 13 +++++++++++--
>>   tools/libxl/libxl_dom.c        |  3 ++-
>>   tools/libxl/libxl_types.idl    |  1 +
>>   tools/libxl/xl_cmdimpl.c       |  6 ++++++
>>   8 files changed, 66 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/firmware/hvmloader/pci.c b/tools/firmware/hvmloader/pci.c
>> index 627e8cb..b36d00b 100644
>> --- a/tools/firmware/hvmloader/pci.c
>> +++ b/tools/firmware/hvmloader/pci.c
>> @@ -58,6 +58,7 @@ void pci_setup(void)
>>           uint64_t bar_sz;
>>       } *bars = (struct bars *)scratch_start;
>>       unsigned int i, nr_bars = 0;
>> +    uint64_t pci_hole_min_size;
>>         const char *s;
>>       /*
>> @@ -85,6 +86,9 @@ void pci_setup(void)
>>       printf("Relocating guest memory for lowmem MMIO space %s\n",
>>              allow_memory_relocate?"enabled":"disabled");
>>   +    s = xenstore_read("platform/pci_hole_min_size", "0");
>
> 'if (s)'
>
Ok, will switch to:

     s = xenstore_read("platform/pci_hole_min_size", NULL);
     if ( s )
>> +    pci_hole_min_size = strtoll(s, NULL, 0);
>> +
>>       /* Program PCI-ISA bridge with appropriate link routes. */
>>       isa_irq = 0;
>>       for ( link = 0; link < 4; link++ )
>> @@ -236,6 +240,12 @@ void pci_setup(void)
>>           pci_writew(devfn, PCI_COMMAND, cmd);
>>       }
>>   +    if (pci_hole_min_size) {
>> +        pci_mem_start = (1ull << 32) - pci_hole_min_size;
>> +        printf("pci_mem_start=0x%lx (was 0x%x) for pci_hole_min_size=%llu\n",
>> +               pci_mem_start, PCI_MEM_START, pci_hole_min_size);
>> +    }
>> +
>>       /*
>>        * 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,
>
> ...
>
>> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
>> index a604cd8..24ceac6 100644
>> --- a/tools/libxl/libxl_create.c
>> +++ b/tools/libxl/libxl_create.c
>> @@ -388,13 +388,15 @@ int libxl__domain_build(libxl__gc *gc,
>>           vments[4] = "start_time";
>>           vments[5] = libxl__sprintf(gc, "%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
>>   -        localents = libxl__calloc(gc, 7, sizeof(char *));
>> +        localents = libxl__calloc(gc, 9, sizeof(char *));
>>           localents[0] = "platform/acpi";
>>           localents[1] = libxl_defbool_val(info->u.hvm.acpi) ? "1" : "0";
>>           localents[2] = "platform/acpi_s3";
>>           localents[3] = libxl_defbool_val(info->u.hvm.acpi_s3) ? "1" : "0";
>>           localents[4] = "platform/acpi_s4";
>>           localents[5] = libxl_defbool_val(info->u.hvm.acpi_s4) ? "1" : "0";
>> +        localents[6] = "platform/pci_hole_min_size";
>> +        localents[7] = libxl__sprintf(gc, "%llu", (unsigned long long)info->u.hvm.pci_hole_min_size);
>
> Do you want to always store this parameter? There is a default already (HVM_BELOW_4G_MMIO_LENGTH) so if it's not set in the config file it may be safe to omit it.
>

I do not always need to store it.  Since none of the rest of these are conditional stores, I just followed them.  Since this is the minimum size, I can add a check on pci_hole_min_size > HVM_BELOW_4G_MMIO_LENGTH here and skip the xenstore write if you want.

> ...
>
>
>> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
>> index 4fc46eb..fe247ee 100644
>> --- a/tools/libxl/xl_cmdimpl.c
>> +++ b/tools/libxl/xl_cmdimpl.c
>> @@ -1025,6 +1025,12 @@ static void parse_config_data(const char *config_source,
>>           xlu_cfg_get_defbool(config, "hpet", &b_info->u.hvm.hpet, 0);
>>           xlu_cfg_get_defbool(config, "vpt_align", &b_info->u.hvm.vpt_align, 0);
>>   +        if (!xlu_cfg_get_long(config, "pci_hole_min_size", &l, 0)) {
>> +            b_info->u.hvm.pci_hole_min_size = (uint64_t) l;
>> +            if (dom_info->debug)
>> +                fprintf(stderr, "pci_hole_min_size: %llu\n", (unsigned long long) b_info->u.hvm.pci_hole_min_size);
>> +        }
>
> You probably want to set b_info->u.hvm.pci_hole_min_size to 0 (or HVM_BELOW_4G_MMIO_LENGTH?) in case it's not specified in libxl__domain_build_info_setdefault().

Since 0 is a valid value, I do not think that libxl__domain_build_info_setdefault() needs to do any thing.  Should I be
specifying a default in the idl of 0?

    -Don Slutz
>
> -boris
>
>
>> +
>>           if (!xlu_cfg_get_long(config, "timer_mode", &l, 1)) {
>>               const char *s = libxl_timer_mode_to_string(l);
>>               fprintf(stderr, "WARNING: specifying \"timer_mode\" as an integer is deprecated. "
>

Re: [RFC PATCH 1/1] Add pci_hole_min_size

[Boris Ostrovsky]

On 03/03/2014 10:30 AM, Don Slutz wrote:
> On 02/28/14 17:07, Boris Ostrovsky wrote:
>>
>>> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
>>> index a604cd8..24ceac6 100644
>>> --- a/tools/libxl/libxl_create.c
>>> +++ b/tools/libxl/libxl_create.c
>>> @@ -388,13 +388,15 @@ int libxl__domain_build(libxl__gc *gc,
>>>           vments[4] = "start_time";
>>>           vments[5] = libxl__sprintf(gc, "%lu.%02d", 
>>> start_time.tv_sec,(int)start_time.tv_usec/10000);
>>>   -        localents = libxl__calloc(gc, 7, sizeof(char *));
>>> +        localents = libxl__calloc(gc, 9, sizeof(char *));
>>>           localents[0] = "platform/acpi";
>>>           localents[1] = libxl_defbool_val(info->u.hvm.acpi) ? "1" : 
>>> "0";
>>>           localents[2] = "platform/acpi_s3";
>>>           localents[3] = libxl_defbool_val(info->u.hvm.acpi_s3) ? 
>>> "1" : "0";
>>>           localents[4] = "platform/acpi_s4";
>>>           localents[5] = libxl_defbool_val(info->u.hvm.acpi_s4) ? 
>>> "1" : "0";
>>> +        localents[6] = "platform/pci_hole_min_size";
>>> +        localents[7] = libxl__sprintf(gc, "%llu", (unsigned long 
>>> long)info->u.hvm.pci_hole_min_size);
>>
>> Do you want to always store this parameter? There is a default 
>> already (HVM_BELOW_4G_MMIO_LENGTH) so if it's not set in the config 
>> file it may be safe to omit it.
>>
>
> I do not always need to store it.  Since none of the rest of these are 
> conditional stores, I just followed them.  Since this is the minimum 
> size, I can add a check on pci_hole_min_size > 
> HVM_BELOW_4G_MMIO_LENGTH here and skip the xenstore write if you want.

If you decide to do this I think the better place may be in 
libxl__build_post().

>
>> ...
>>
>>
>>> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
>>> index 4fc46eb..fe247ee 100644
>>> --- a/tools/libxl/xl_cmdimpl.c
>>> +++ b/tools/libxl/xl_cmdimpl.c
>>> @@ -1025,6 +1025,12 @@ static void parse_config_data(const char 
>>> *config_source,
>>>           xlu_cfg_get_defbool(config, "hpet", &b_info->u.hvm.hpet, 0);
>>>           xlu_cfg_get_defbool(config, "vpt_align", 
>>> &b_info->u.hvm.vpt_align, 0);
>>>   +        if (!xlu_cfg_get_long(config, "pci_hole_min_size", &l, 0)) {
>>> +            b_info->u.hvm.pci_hole_min_size = (uint64_t) l;
>>> +            if (dom_info->debug)
>>> +                fprintf(stderr, "pci_hole_min_size: %llu\n", 
>>> (unsigned long long) b_info->u.hvm.pci_hole_min_size);
>>> +        }
>>
>> You probably want to set b_info->u.hvm.pci_hole_min_size to 0 (or 
>> HVM_BELOW_4G_MMIO_LENGTH?) in case it's not specified in 
>> libxl__domain_build_info_setdefault().
>
> Since 0 is a valid value, I do not think that 
> libxl__domain_build_info_setdefault() needs to do any thing. Should I be
> specifying a default in the idl of 0?


What I meant (but apparently not what I wrote) was that if config file 
doesn't have "pci_hole_min_size" then b_info->u.hvm.pci_hole_min_size 
will be left uninitialized and I don't know whether we can assume that 
it will be zero.

-boris


>
>    -Don Slutz
>>
>> -boris
>>
>>
>>> +
>>>           if (!xlu_cfg_get_long(config, "timer_mode", &l, 1)) {
>>>               const char *s = libxl_timer_mode_to_string(l);
>>>               fprintf(stderr, "WARNING: specifying \"timer_mode\" as 
>>> an integer is deprecated. "
>>
>

Re: [RFC PATCH 1/1] Add pci_hole_min_size

[Don Slutz]

On 03/03/14 11:07, Boris Ostrovsky wrote:
> On 03/03/2014 10:30 AM, Don Slutz wrote:
>> On 02/28/14 17:07, Boris Ostrovsky wrote:
>>>
>>>> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
>>>> index a604cd8..24ceac6 100644
>>>> --- a/tools/libxl/libxl_create.c
>>>> +++ b/tools/libxl/libxl_create.c
>>>> @@ -388,13 +388,15 @@ int libxl__domain_build(libxl__gc *gc,
>>>>           vments[4] = "start_time";
>>>>           vments[5] = libxl__sprintf(gc, "%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
>>>>   -        localents = libxl__calloc(gc, 7, sizeof(char *));
>>>> +        localents = libxl__calloc(gc, 9, sizeof(char *));
>>>>           localents[0] = "platform/acpi";
>>>>           localents[1] = libxl_defbool_val(info->u.hvm.acpi) ? "1" : "0";
>>>>           localents[2] = "platform/acpi_s3";
>>>>           localents[3] = libxl_defbool_val(info->u.hvm.acpi_s3) ? "1" : "0";
>>>>           localents[4] = "platform/acpi_s4";
>>>>           localents[5] = libxl_defbool_val(info->u.hvm.acpi_s4) ? "1" : "0";
>>>> +        localents[6] = "platform/pci_hole_min_size";
>>>> +        localents[7] = libxl__sprintf(gc, "%llu", (unsigned long long)info->u.hvm.pci_hole_min_size);
>>>
>>> Do you want to always store this parameter? There is a default already (HVM_BELOW_4G_MMIO_LENGTH) so if it's not set in the config file it may be safe to omit it.
>>>
>>
>> I do not always need to store it.  Since none of the rest of these are conditional stores, I just followed them.  Since this is the minimum size, I can add a check on pci_hole_min_size > HVM_BELOW_4G_MMIO_LENGTH here and skip the xenstore write if you want.
>
> If you decide to do this I think the better place may be in libxl__build_post().
>

I lean to not doing it.

>>
>>> ...
>>>
>>>
>>>> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
>>>> index 4fc46eb..fe247ee 100644
>>>> --- a/tools/libxl/xl_cmdimpl.c
>>>> +++ b/tools/libxl/xl_cmdimpl.c
>>>> @@ -1025,6 +1025,12 @@ static void parse_config_data(const char *config_source,
>>>>           xlu_cfg_get_defbool(config, "hpet", &b_info->u.hvm.hpet, 0);
>>>>           xlu_cfg_get_defbool(config, "vpt_align", &b_info->u.hvm.vpt_align, 0);
>>>>   +        if (!xlu_cfg_get_long(config, "pci_hole_min_size", &l, 0)) {
>>>> +            b_info->u.hvm.pci_hole_min_size = (uint64_t) l;
>>>> +            if (dom_info->debug)
>>>> +                fprintf(stderr, "pci_hole_min_size: %llu\n", (unsigned long long) b_info->u.hvm.pci_hole_min_size);
>>>> +        }
>>>
>>> You probably want to set b_info->u.hvm.pci_hole_min_size to 0 (or HVM_BELOW_4G_MMIO_LENGTH?) in case it's not specified in libxl__domain_build_info_setdefault().
>>
>> Since 0 is a valid value, I do not think that libxl__domain_build_info_setdefault() needs to do any thing. Should I be
>> specifying a default in the idl of 0?
>
>
> What I meant (but apparently not what I wrote) was that if config file doesn't have "pci_hole_min_size" then b_info->u.hvm.pci_hole_min_size will be left uninitialized and I don't know whether we can assume that it will be zero.
>

As expected, changing the .idl to have an init:

- ("pci_hole_min_size",uint64),
+ ("pci_hole_min_size",UInt(64, init_val = 1)),

The 1 is setup in the generated code routine libxl_domain_build_info_init_type().  I plan to change this to 0.

Since libxl__domain_build_info_setdefault() is called after parse_config_data() it is not simple to code.

If you do not like the .idl to have a 0, (i.e. like LIBXL_MEMKB_DEFAULT) then I will need to add a LIBXL_PCI_HOLE_MIN_DEFAULT define (like #define LIBXL_PCI_HOLE_MIN_DEFAULT ~0ULL). And then use it.

    -Don Slutz

> -boris
>
>
>>
>>    -Don Slutz
>>>
>>> -boris
>>>
>>>
>>>> +
>>>>           if (!xlu_cfg_get_long(config, "timer_mode", &l, 1)) {
>>>>               const char *s = libxl_timer_mode_to_string(l);
>>>>               fprintf(stderr, "WARNING: specifying \"timer_mode\" as an integer is deprecated. "
>>>
>>
>

Re: [RFC PATCH 1/1] Add pci_hole_min_size

[Boris Ostrovsky]

On 03/03/2014 03:43 PM, Don Slutz wrote:
> On 03/03/14 11:07, Boris Ostrovsky wrote:
>> On 03/03/2014 10:30 AM, Don Slutz wrote:
>>> On 02/28/14 17:07, Boris Ostrovsky wrote:
>>>
>>>
>>>> ...
>>>>
>>>>
>>>>> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
>>>>> index 4fc46eb..fe247ee 100644
>>>>> --- a/tools/libxl/xl_cmdimpl.c
>>>>> +++ b/tools/libxl/xl_cmdimpl.c
>>>>> @@ -1025,6 +1025,12 @@ static void parse_config_data(const char 
>>>>> *config_source,
>>>>>           xlu_cfg_get_defbool(config, "hpet", &b_info->u.hvm.hpet, 
>>>>> 0);
>>>>>           xlu_cfg_get_defbool(config, "vpt_align", 
>>>>> &b_info->u.hvm.vpt_align, 0);
>>>>>   +        if (!xlu_cfg_get_long(config, "pci_hole_min_size", &l, 
>>>>> 0)) {
>>>>> +            b_info->u.hvm.pci_hole_min_size = (uint64_t) l;
>>>>> +            if (dom_info->debug)
>>>>> +                fprintf(stderr, "pci_hole_min_size: %llu\n", 
>>>>> (unsigned long long) b_info->u.hvm.pci_hole_min_size);
>>>>> +        }
>>>>
>>>> You probably want to set b_info->u.hvm.pci_hole_min_size to 0 (or 
>>>> HVM_BELOW_4G_MMIO_LENGTH?) in case it's not specified in 
>>>> libxl__domain_build_info_setdefault().
>>>
>>> Since 0 is a valid value, I do not think that 
>>> libxl__domain_build_info_setdefault() needs to do any thing. Should 
>>> I be
>>> specifying a default in the idl of 0?
>>
>>
>> What I meant (but apparently not what I wrote) was that if config 
>> file doesn't have "pci_hole_min_size" then 
>> b_info->u.hvm.pci_hole_min_size will be left uninitialized and I 
>> don't know whether we can assume that it will be zero.
>>
>
> As expected, changing the .idl to have an init:
>
> - ("pci_hole_min_size",uint64),
> + ("pci_hole_min_size",UInt(64, init_val = 1)),
>
> The 1 is setup in the generated code routine 
> libxl_domain_build_info_init_type().  I plan to change this to 0.
>
> Since libxl__domain_build_info_setdefault() is called after 
> parse_config_data() it is not simple to code.
>
> If you do not like the .idl to have a 0, (i.e. like 
> LIBXL_MEMKB_DEFAULT) then I will need to add a 
> LIBXL_PCI_HOLE_MIN_DEFAULT define (like #define 
> LIBXL_PCI_HOLE_MIN_DEFAULT ~0ULL). And then use it.


As long as we know that pci_hole_min_size is initialized either way 
should be fine.

Thanks.
-boris

Re: [RFC PATCH 1/1] Add pci_hole_min_size

[George Dunlap]

On Fri, Feb 28, 2014 at 8:15 PM, Don Slutz <dslutz@verizon.com> wrote:
> This allows growing the pci_hole to the size needed.

You mean, it allows the pci hole size to be specified at boot -- the
pci hole still cannot be enlarged dynamically in hvmloader, correct?
What's your intended use case for this?

 -George

Re: [RFC PATCH 1/1] Add pci_hole_min_size

[Don Slutz]

On 03/04/14 08:25, George Dunlap wrote:
> On Fri, Feb 28, 2014 at 8:15 PM, Don Slutz <dslutz@verizon.com> wrote:
>> This allows growing the pci_hole to the size needed.
> You mean, it allows the pci hole size to be specified at boot

Yes.

>   -- the
> pci hole still cannot be enlarged dynamically in hvmloader, correct?

If I am correctly understanding you, this is in reference to:

/*
      * 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.   */


so the answer is no, however using pci_hole_min_size can mean that allow_memory_relocate is not needed for upstream QEMU.



> What's your intended use case for this?
>
>   -George

If you add enough PCI devices then all mmio may not fit below 4G which may not be the layout the user wanted. This allows you to increase the below 4G address space that PCI devices can use and therefore in more cases not have any mmio that is above 4G.

There are real PCI cards that do not support mmio over 4G, so if you want to emulate them precisely, you may also need to increase the space below 4G for them.  There are drivers for these cards that also do not work if they have their mmio space mapped above 4G.

    -Don Slutz

Re: [RFC PATCH 1/1] Add pci_hole_min_size

[Konrad Rzeszutek Wilk]

On Tue, Mar 04, 2014 at 01:57:26PM -0500, Don Slutz wrote:
> On 03/04/14 08:25, George Dunlap wrote:
> >On Fri, Feb 28, 2014 at 8:15 PM, Don Slutz <dslutz@verizon.com> wrote:
> >>This allows growing the pci_hole to the size needed.
> >You mean, it allows the pci hole size to be specified at boot
> 
> Yes.
> 
> >  -- the
> >pci hole still cannot be enlarged dynamically in hvmloader, correct?
> 
> If I am correctly understanding you, this is in reference to:
> 
> /*
>      * 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.   */
> 
> 
> so the answer is no, however using pci_hole_min_size can mean that 
> allow_memory_relocate is not needed for upstream QEMU.
> 
> 
> 
> >What's your intended use case for this?
> >
> >  -George
> 
> If you add enough PCI devices then all mmio may not fit below 4G which may 
> not be the layout the user wanted. This allows you to increase the below 4G 
> address space that PCI devices can use and therefore in more cases not have 
> any mmio that is above 4G.
> 
> There are real PCI cards that do not support mmio over 4G, so if you want 
> to emulate them precisely, you may also need to increase the space below 4G 
> for them.  There are drivers for these cards that also do not work if they 
> have their mmio space mapped above 4G.

Would it be better if the HVM guests had something similar to what we
manufacture for PV guests with PCI passthrough: an filtered version of 
the host's E820?

That way you don't have to worry about resizing just right and instead
the E820 looks like the hosts one. Granted you can't migrate, but I
don't think that is a problem in your use-case?

Thanks.
> 
>    -Don Slutz
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [RFC PATCH 1/1] Add pci_hole_min_size

[George Dunlap]

On Fri, Mar 7, 2014 at 7:28 PM, Konrad Rzeszutek Wilk <konrad@darnok.org> wrote:
> On Tue, Mar 04, 2014 at 01:57:26PM -0500, Don Slutz wrote:
>> On 03/04/14 08:25, George Dunlap wrote:
>> >On Fri, Feb 28, 2014 at 8:15 PM, Don Slutz <dslutz@verizon.com> wrote:
>> >>This allows growing the pci_hole to the size needed.
>> >You mean, it allows the pci hole size to be specified at boot
>>
>> Yes.
>>
>> >  -- the
>> >pci hole still cannot be enlarged dynamically in hvmloader, correct?
>>
>> If I am correctly understanding you, this is in reference to:
>>
>> /*
>>      * 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.   */
>>
>>
>> so the answer is no, however using pci_hole_min_size can mean that
>> allow_memory_relocate is not needed for upstream QEMU.
>>
>>
>>
>> >What's your intended use case for this?
>> >
>> >  -George
>>
>> If you add enough PCI devices then all mmio may not fit below 4G which may
>> not be the layout the user wanted. This allows you to increase the below 4G
>> address space that PCI devices can use and therefore in more cases not have
>> any mmio that is above 4G.
>>
>> There are real PCI cards that do not support mmio over 4G, so if you want
>> to emulate them precisely, you may also need to increase the space below 4G
>> for them.  There are drivers for these cards that also do not work if they
>> have their mmio space mapped above 4G.
>
> Would it be better if the HVM guests had something similar to what we
> manufacture for PV guests with PCI passthrough: an filtered version of
> the host's E820?
>
> That way you don't have to worry about resizing just right and instead
> the E820 looks like the hosts one. Granted you can't migrate, but I
> don't think that is a problem in your use-case?

Having the guest PCI hole the same size as the host PCI hole also gets
rid of a whole class of (unfortunately very common) bugs in PCI
hardware, such that if guest paddrs collide overlap with device IO
ranges the PCI hardware sends the DMA requests to the wrong place.
(In other words, VT-d as implemented in a very large number of
motherboards is utterly broken -- total fail on someone's part.)

The main disadvantage of this is that it unnecessarily reduces the
amount of lowmem available -- and for 32-bit non-PAE guests, reduces
the total amount of memory available at all.

I think long-term, it would be best to:
* Have the pci hole be small for VMs without devices passed through
* Have the pci hole default to the host pci hole for VMs with devices
passed through
* Have the pci hole size able to be specified, either as a size, or as "host".

As long as the size specification can be extended to this
functionality easily, I think just having a size to begin with is OK.

I think the qemu guys didn't like the term "pci_hole" and wanted
something like "lowmem" instead -- that will need to be sorted out.

 -George

Re: [RFC PATCH 1/1] Add pci_hole_min_size

[Don Slutz]

On 03/11/14 08:54, George Dunlap wrote:
> On Fri, Mar 7, 2014 at 7:28 PM, Konrad Rzeszutek Wilk <konrad@darnok.org> wrote:
>> On Tue, Mar 04, 2014 at 01:57:26PM -0500, Don Slutz wrote:
>>> On 03/04/14 08:25, George Dunlap wrote:
>>>> On Fri, Feb 28, 2014 at 8:15 PM, Don Slutz <dslutz@verizon.com> wrote:
>>>>> This allows growing the pci_hole to the size needed.
>>>> You mean, it allows the pci hole size to be specified at boot
>>> Yes.
>>>
>>>>   -- the
>>>> pci hole still cannot be enlarged dynamically in hvmloader, correct?
>>> If I am correctly understanding you, this is in reference to:
>>>
>>> /*
>>>       * 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.   */
>>>
>>>
>>> so the answer is no, however using pci_hole_min_size can mean that
>>> allow_memory_relocate is not needed for upstream QEMU.
>>>
>>>
>>>
>>>> What's your intended use case for this?
>>>>
>>>>   -George
>>> If you add enough PCI devices then all mmio may not fit below 4G which may
>>> not be the layout the user wanted. This allows you to increase the below 4G
>>> address space that PCI devices can use and therefore in more cases not have
>>> any mmio that is above 4G.
>>>
>>> There are real PCI cards that do not support mmio over 4G, so if you want
>>> to emulate them precisely, you may also need to increase the space below 4G
>>> for them.  There are drivers for these cards that also do not work if they
>>> have their mmio space mapped above 4G.
>> Would it be better if the HVM guests had something similar to what we
>> manufacture for PV guests with PCI passthrough: an filtered version of
>> the host's E820?
>>
>> That way you don't have to worry about resizing just right and instead
>> the E820 looks like the hosts one. Granted you can't migrate, but I
>> don't think that is a problem in your use-case?
> Having the guest PCI hole the same size as the host PCI hole also gets
> rid of a whole class of (unfortunately very common) bugs in PCI
> hardware, such that if guest paddrs collide overlap with device IO
> ranges the PCI hardware sends the DMA requests to the wrong place.
> (In other words, VT-d as implemented in a very large number of
> motherboards is utterly broken -- total fail on someone's part.)
>
> The main disadvantage of this is that it unnecessarily reduces the
> amount of lowmem available -- and for 32-bit non-PAE guests, reduces
> the total amount of memory available at all.
>
> I think long-term, it would be best to:
> * Have the pci hole be small for VMs without devices passed through
> * Have the pci hole default to the host pci hole for VMs with devices
> passed through
> * Have the pci hole size able to be specified, either as a size, or as "host".
>
> As long as the size specification can be extended to this
> functionality easily, I think just having a size to begin with is OK.

I see no problem with extending to add "host".  So I am starting with just a size.  Note: the new QEMU way is simpler to decode from an e820 map.

For example:


Mar 10 13:08:28 dcs-xen-54 kernel: [    0.000000] e820: BIOS-provided physical RAM map:
Mar 10 13:08:28 dcs-xen-54 kernel: [    0.000000] Xen: [mem 0x0000000000000000-0x000000000009afff] usable
Mar 10 13:08:28 dcs-xen-54 kernel: [    0.000000] Xen: [mem 0x000000000009b800-0x00000000000fffff] reserved
Mar 10 13:08:28 dcs-xen-54 kernel: [    0.000000] Xen: [mem 0x0000000000100000-0x00000000bf63efff] usable
Mar 10 13:08:28 dcs-xen-54 kernel: [    0.000000] Xen: [mem 0x00000000bf63f000-0x00000000bf6befff] reserved
Mar 10 13:08:28 dcs-xen-54 kernel: [    0.000000] Xen: [mem 0x00000000bf6bf000-0x00000000bf7befff] ACPI NVS
Mar 10 13:08:28 dcs-xen-54 kernel: [    0.000000] Xen: [mem 0x00000000bf7bf000-0x00000000bf7fefff] ACPI data
Mar 10 13:08:28 dcs-xen-54 kernel: [    0.000000] Xen: [mem 0x00000000bf7ff000-0x00000000bf7fffff] usable
Mar 10 13:08:28 dcs-xen-54 kernel: [    0.000000] Xen: [mem 0x00000000bf800000-0x00000000bfffffff] reserved
Mar 10 13:08:28 dcs-xen-54 kernel: [    0.000000] Xen: [mem 0x00000000e0000000-0x00000000efffffff] reserved
Mar 10 13:08:28 dcs-xen-54 kernel: [    0.000000] Xen: [mem 0x00000000feb00000-0x00000000feb03fff] reserved
Mar 10 13:08:28 dcs-xen-54 kernel: [    0.000000] Xen: [mem 0x00000000fec00000-0x00000000fec00fff] reserved
Mar 10 13:08:28 dcs-xen-54 kernel: [    0.000000] Xen: [mem 0x00000000fed10000-0x00000000fed19fff] reserved
Mar 10 13:08:28 dcs-xen-54 kernel: [    0.000000] Xen: [mem 0x00000000fed1c000-0x00000000fed1ffff] reserved
Mar 10 13:08:28 dcs-xen-54 kernel: [    0.000000] Xen: [mem 0x00000000fee00000-0x00000000feefffff] reserved
Mar 10 13:08:28 dcs-xen-54 kernel: [    0.000000] Xen: [mem 0x00000000ffd80000-0x00000000ffffffff] reserved
Mar 10 13:08:28 dcs-xen-54 kernel: [    0.000000] Xen: [mem 0x0000000100000000-0x00000005c0e16fff] usable
Mar 10 13:08:28 dcs-xen-54 kernel: [    0.000000] Xen: [mem 0x00000005c0e17000-0x000000083fffffff] unusable


pci_hole_min_size =  1082130432 (0x40800000) for 0xbf800000
pci_hole_min_size =  536870912 (0x20000000) for 0xe0000000

Note: It does not leap out at me from the e820 map which is the host one.

> I think the qemu guys didn't like the term "pci_hole" and wanted
> something like "lowmem" instead -- that will need to be sorted out.

Next version of QEMU patch out with new name.  Was not sure what name or size make the most sense.


lowmem == 1 << 32 - pci_hole_min_size.

     -Don Slutz

>   -George

[RFC PATCH 1/1] Add pci_hole_min_size

[Don Slutz]

Add logging of max_ram_below_4g too big.

Signed-off-by: Don Slutz <dslutz@verizon.com>
---
 docs/man/xl.cfg.pod.5          | 10 ++++++++++
 tools/firmware/hvmloader/pci.c | 12 ++++++++++++
 tools/libxc/xc_hvm_build_x86.c | 22 ++++++++++++++++++++++
 tools/libxc/xenguest.h         | 11 +++++++++++
 tools/libxl/libxl_create.c     |  4 +++-
 tools/libxl/libxl_dm.c         | 15 +++++++++++++++
 tools/libxl/libxl_dom.c        |  3 ++-
 tools/libxl/libxl_types.idl    |  1 +
 tools/libxl/xl_cmdimpl.c       |  6 ++++++
 9 files changed, 82 insertions(+), 2 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index c02ad55..7ef7b10 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -1022,6 +1022,16 @@ wallclock (i.e., real) time.
 
 =back
 
+=head3 Memory layout
+
+=over 4
+
+=item B<pci_hole_min_size=BYTES>
+
+Specifies the minimum size the PCI MMIO hole below 4GiB will be.
+
+=back
+
 =head3 Support for Paravirtualisation of HVM Guests
 
 The following options allow Paravirtualised features (such as devices)
diff --git a/tools/firmware/hvmloader/pci.c b/tools/firmware/hvmloader/pci.c
index 627e8cb..4115929 100644
--- a/tools/firmware/hvmloader/pci.c
+++ b/tools/firmware/hvmloader/pci.c
@@ -58,6 +58,7 @@ void pci_setup(void)
         uint64_t bar_sz;
     } *bars = (struct bars *)scratch_start;
     unsigned int i, nr_bars = 0;
+    uint64_t pci_hole_min_size = 0;
 
     const char *s;
     /*
@@ -85,6 +86,10 @@ void pci_setup(void)
     printf("Relocating guest memory for lowmem MMIO space %s\n",
            allow_memory_relocate?"enabled":"disabled");
 
+    s = xenstore_read("platform/pci_hole_min_size", NULL);
+    if ( s )
+        pci_hole_min_size = strtoll(s, NULL, 0);
+
     /* Program PCI-ISA bridge with appropriate link routes. */
     isa_irq = 0;
     for ( link = 0; link < 4; link++ )
@@ -236,6 +241,13 @@ void pci_setup(void)
         pci_writew(devfn, PCI_COMMAND, cmd);
     }
 
+    if (pci_hole_min_size)
+    {
+        pci_mem_start = (1ULL << 32) - pci_hole_min_size;
+        printf("pci_mem_start=0x%lx (was 0x%x) for pci_hole_min_size=%lu\n",
+               pci_mem_start, PCI_MEM_START, (long)pci_hole_min_size);
+    }
+
     /*
      * 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,
diff --git a/tools/libxc/xc_hvm_build_x86.c b/tools/libxc/xc_hvm_build_x86.c
index dd3b522..4752d58 100644
--- a/tools/libxc/xc_hvm_build_x86.c
+++ b/tools/libxc/xc_hvm_build_x86.c
@@ -603,16 +603,38 @@ int xc_hvm_build_target_mem(xc_interface *xch,
                            int target,
                            const char *image_name)
 {
+    return xc_hvm_build_target_mem_with_hole(xch, domid, memsize, target, image_name, 0);
+}
+
+int xc_hvm_build_target_mem_with_hole(xc_interface *xch,
+                                      uint32_t domid,
+                                      int memsize,
+                                      int target,
+                                      const char *image_name,
+                                      uint64_t pci_hole_min_size)
+{
     struct xc_hvm_build_args args = {};
 
     memset(&args, 0, sizeof(struct xc_hvm_build_args));
     args.mem_size = (uint64_t)memsize << 20;
     args.mem_target = (uint64_t)target << 20;
     args.image_file_name = image_name;
+    if (pci_hole_min_size > HVM_BELOW_4G_MMIO_LENGTH)
+        args.mmio_size = pci_hole_min_size;
 
     return xc_hvm_build(xch, domid, &args);
 }
 
+int xc_hvm_build_with_hole(xc_interface *xch, uint32_t domid,
+                           struct xc_hvm_build_args *args,
+                           uint64_t pci_hole_min_size)
+{
+    if (pci_hole_min_size > HVM_BELOW_4G_MMIO_LENGTH)
+        args->mmio_size = pci_hole_min_size;
+
+    return xc_hvm_build(xch, domid, args);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxc/xenguest.h b/tools/libxc/xenguest.h
index a0e30e1..44290bd 100644
--- a/tools/libxc/xenguest.h
+++ b/tools/libxc/xenguest.h
@@ -248,12 +248,23 @@ struct xc_hvm_build_args {
 int xc_hvm_build(xc_interface *xch, uint32_t domid,
                  struct xc_hvm_build_args *hvm_args);
 
+int xc_hvm_build_with_hole(xc_interface *xch, uint32_t domid,
+                           struct xc_hvm_build_args *args,
+                           uint64_t pci_hole_min_size);
+
 int xc_hvm_build_target_mem(xc_interface *xch,
                             uint32_t domid,
                             int memsize,
                             int target,
                             const char *image_name);
 
+int xc_hvm_build_target_mem_with_hole(xc_interface *xch,
+                                      uint32_t domid,
+                                      int memsize,
+                                      int target,
+                                      const char *image_name,
+                                      uint64_t pci_hole_min_size);
+
 int xc_suspend_evtchn_release(xc_interface *xch, xc_evtchn *xce, int domid, int suspend_evtchn);
 
 int xc_suspend_evtchn_init(xc_interface *xch, xc_evtchn *xce, int domid, int port);
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 53e7cb6..4de8260 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -388,13 +388,15 @@ int libxl__domain_build(libxl__gc *gc,
         vments[4] = "start_time";
         vments[5] = libxl__sprintf(gc, "%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
 
-        localents = libxl__calloc(gc, 7, sizeof(char *));
+        localents = libxl__calloc(gc, 9, sizeof(char *));
         localents[0] = "platform/acpi";
         localents[1] = libxl_defbool_val(info->u.hvm.acpi) ? "1" : "0";
         localents[2] = "platform/acpi_s3";
         localents[3] = libxl_defbool_val(info->u.hvm.acpi_s3) ? "1" : "0";
         localents[4] = "platform/acpi_s4";
         localents[5] = libxl_defbool_val(info->u.hvm.acpi_s4) ? "1" : "0";
+        localents[6] = "platform/pci_hole_min_size";
+        localents[7] = libxl__sprintf(gc, "%llu", (unsigned long long)info->u.hvm.pci_hole_min_size);
 
         break;
     case LIBXL_DOMAIN_TYPE_PV:
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 5c06dfa..72842aa 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -656,6 +656,21 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
         } else {
             flexarray_append(dm_args, "xenfv");
         }
+        if (b_info->u.hvm.pci_hole_min_size) {
+            unsigned long long max_ram_below_4g = (1ULL << 32) -
+                b_info->u.hvm.pci_hole_min_size;
+
+            if (max_ram_below_4g > 0xF0000000ULL)
+            {
+                LIBXL__LOG(ctx, LIBXL__LOG_WARNING,
+                           "pci_hole_min_size too big => max_ram_below_4g=%llu > %llu (new adjusted value)\n",
+                           max_ram_below_4g, 0xF0000000ULL);
+                max_ram_below_4g = 0xF0000000ULL;
+            }
+            flexarray_append_pair(dm_args, "-global",
+                           GCSPRINTF("pc-memory-layout.max-ram-below-4g=%llu",
+                                     max_ram_below_4g));
+        }
         for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
             flexarray_append(dm_args, b_info->extra_hvm[i]);
         break;
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 55f74b2..6ebc606 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -642,7 +642,8 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
         goto out;
     }
 
-    ret = xc_hvm_build(ctx->xch, domid, &args);
+    ret = xc_hvm_build_with_hole(ctx->xch, domid, &args,
+                                 info->u.hvm.pci_hole_min_size);
     if (ret) {
         LOGEV(ERROR, ret, "hvm building failed");
         goto out;
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 7d3a62b..bba7076 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -346,6 +346,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
                                        ("timeoffset",       string),
                                        ("hpet",             libxl_defbool),
                                        ("vpt_align",        libxl_defbool),
+                                       ("pci_hole_min_size",UInt(64, init_val = 0)),
                                        ("timer_mode",       libxl_timer_mode),
                                        ("nested_hvm",       libxl_defbool),
                                        ("smbios_firmware",  string),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 5f59bbc..8391824 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1026,6 +1026,12 @@ static void parse_config_data(const char *config_source,
         xlu_cfg_get_defbool(config, "hpet", &b_info->u.hvm.hpet, 0);
         xlu_cfg_get_defbool(config, "vpt_align", &b_info->u.hvm.vpt_align, 0);
 
+        if (!xlu_cfg_get_long(config, "pci_hole_min_size", &l, 0)) {
+            b_info->u.hvm.pci_hole_min_size = (uint64_t) l;
+            if (dom_info->debug)
+                fprintf(stderr, "pci_hole_min_size: %llu\n", (unsigned long long) b_info->u.hvm.pci_hole_min_size);
+        }
+
         if (!xlu_cfg_get_long(config, "timer_mode", &l, 1)) {
             const char *s = libxl_timer_mode_to_string(l);
             fprintf(stderr, "WARNING: specifying \"timer_mode\" as an integer is deprecated. "
-- 
1.8.4