[PATCH] tools/xl: disallow PCI device assignment for HVM guest when PoD is enabled

[PATCH] tools/xl: disallow PCI device assignment for HVM guest when PoD is enabled

[Wei Liu]

This replicates a Xend behavior, see ec789523749 ("xend: Dis-allow
device assignment if PoD is enabled.").

Allegedly-reported-by: Konrad Wilk <konrad.wilk@oracle.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrx.com>
Cc: Ian Jackson <ian.jackson@eu.citrx.com>
---
This was listed in 4.4 development update. A quick skim through
hypervisor vtd changesets suggests the situation stays unchanged since 3
years ago -- at least I didn't find any log message related to "PoD".

Rune: git log --since="2010-01-21" xen/drivers/passthrough/vtd
(It was first reported on 2010-01-21)

This patch is tested with setting memory=, maxmem= and pci=[] parameters
in both HVM and PV guests. In HVM guest's case I need to have
claim_mode=0 in /etc/xen/xl.conf to make xl actually create HVM with PoD
mode enabled.
---
 tools/libxl/xl_cmdimpl.c |   29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index c30f495..59aba7d 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -738,6 +738,7 @@ static void parse_config_data(const char *config_source,
     int pci_msitranslate = 0;
     int pci_permissive = 0;
     int i, e;
+    bool pod_enabled = false;
 
     libxl_domain_create_info *c_info = &d_config->c_info;
     libxl_domain_build_info *b_info = &d_config->b_info;
@@ -916,6 +917,12 @@ static void parse_config_data(const char *config_source,
     if (!xlu_cfg_get_long (config, "maxmem", &l, 0))
         b_info->max_memkb = l * 1024;
 
+    /* If target_memkb is smaller than max_memkb, the subsequent call
+     * to libxc when building HVM omain will enable PoD mode.
+     */
+    pod_enabled = (c_info->type == LIBXL_DOMAIN_TYPE_HVM) &&
+        (b_info->target_memkb < b_info->max_memkb);
+
     libxl_defbool_set(&b_info->claim_mode, claim_mode);
 
     if (xlu_cfg_get_string (config, "on_poweroff", &buf, 0))
@@ -1468,9 +1475,9 @@ skip_vfb:
         xlu_cfg_get_defbool(config, "e820_host", &b_info->u.pv.e820_host, 0);
     }
 
+    d_config->num_pcidevs = 0;
+    d_config->pcidevs = NULL;
     if (!xlu_cfg_get_list (config, "pci", &pcis, 0, 0)) {
-        d_config->num_pcidevs = 0;
-        d_config->pcidevs = NULL;
         for(i = 0; (buf = xlu_cfg_get_listitem (pcis, i)) != NULL; i++) {
             libxl_device_pci *pcidev;
 
@@ -1488,6 +1495,24 @@ skip_vfb:
             libxl_defbool_set(&b_info->u.pv.e820_host, true);
     }
 
+    /* We cannot have PoD and PCI device assignment at the same
+     * time. VT-d engine needs to set up the entire page table for
+     * the domain. However if PoD is enabled, un-populated memory is
+     * marked as populate_on_demand, and VT-d engine won't set up page
+     * tables for them. Therefore any DMA towards those memory may
+     * cause DMA fault.
+     *
+     * This is restricted to HVM guest, as only VT-d is relevant
+     * in the counterpart in Xend. We're late in release cycle so the change
+     * should only does what's necessary. Probably we can revisit if
+     * we need to do the same thing for PV guest in the future.
+     */
+    if (c_info->type == LIBXL_DOMAIN_TYPE_HVM &&
+        d_config->num_pcidevs && pod_enabled) {
+        fprintf(stderr, "PCI device assignment for HVM guest failed due to Paging-on-Demand enabled\n");
+        exit(1);
+    }
+
     switch (xlu_cfg_get_list(config, "cpuid", &cpuids, 0, 1)) {
     case 0:
         {
-- 
1.7.10.4

Re: [PATCH] tools/xl: disallow PCI device assignment for HVM guest when PoD is enabled

[Wei Liu]

On Thu, Jan 09, 2014 at 07:10:44PM +0000, Wei Liu wrote:
> This replicates a Xend behavior, see ec789523749 ("xend: Dis-allow
> device assignment if PoD is enabled.").
> 
> Allegedly-reported-by: Konrad Wilk <konrad.wilk@oracle.com>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrx.com>
> Cc: Ian Jackson <ian.jackson@eu.citrx.com>

Oops... Missing "i". Will resend...

[PATCH] tools/xl: disallow PCI device assignment for HVM guest when PoD is enabled

[Wei Liu]

This replicates a Xend behavior, see ec789523749 ("xend: Dis-allow
device assignment if PoD is enabled.").

Allegedly-reported-by: Konrad Wilk <konrad.wilk@oracle.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
---
This was listed in 4.4 development update. A quick skim through
hypervisor vtd changesets suggests the situation stays unchanged since 3
years ago -- at least I didn't find any log message related to "PoD".

Rune: git log --since="2010-01-21" xen/drivers/passthrough/vtd
(It was first reported on 2010-01-21)

This patch is tested with setting memory=, maxmem= and pci=[] parameters
in both HVM and PV guests. In HVM guest's case I need to have
claim_mode=0 in /etc/xen/xl.conf to make xl actually create HVM with PoD
mode enabled.
---
 tools/libxl/xl_cmdimpl.c |   29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index c30f495..59aba7d 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -738,6 +738,7 @@ static void parse_config_data(const char *config_source,
     int pci_msitranslate = 0;
     int pci_permissive = 0;
     int i, e;
+    bool pod_enabled = false;
 
     libxl_domain_create_info *c_info = &d_config->c_info;
     libxl_domain_build_info *b_info = &d_config->b_info;
@@ -916,6 +917,12 @@ static void parse_config_data(const char *config_source,
     if (!xlu_cfg_get_long (config, "maxmem", &l, 0))
         b_info->max_memkb = l * 1024;
 
+    /* If target_memkb is smaller than max_memkb, the subsequent call
+     * to libxc when building HVM omain will enable PoD mode.
+     */
+    pod_enabled = (c_info->type == LIBXL_DOMAIN_TYPE_HVM) &&
+        (b_info->target_memkb < b_info->max_memkb);
+
     libxl_defbool_set(&b_info->claim_mode, claim_mode);
 
     if (xlu_cfg_get_string (config, "on_poweroff", &buf, 0))
@@ -1468,9 +1475,9 @@ skip_vfb:
         xlu_cfg_get_defbool(config, "e820_host", &b_info->u.pv.e820_host, 0);
     }
 
+    d_config->num_pcidevs = 0;
+    d_config->pcidevs = NULL;
     if (!xlu_cfg_get_list (config, "pci", &pcis, 0, 0)) {
-        d_config->num_pcidevs = 0;
-        d_config->pcidevs = NULL;
         for(i = 0; (buf = xlu_cfg_get_listitem (pcis, i)) != NULL; i++) {
             libxl_device_pci *pcidev;
 
@@ -1488,6 +1495,24 @@ skip_vfb:
             libxl_defbool_set(&b_info->u.pv.e820_host, true);
     }
 
+    /* We cannot have PoD and PCI device assignment at the same
+     * time. VT-d engine needs to set up the entire page table for
+     * the domain. However if PoD is enabled, un-populated memory is
+     * marked as populate_on_demand, and VT-d engine won't set up page
+     * tables for them. Therefore any DMA towards those memory may
+     * cause DMA fault.
+     *
+     * This is restricted to HVM guest, as only VT-d is relevant
+     * in the counterpart in Xend. We're late in release cycle so the change
+     * should only does what's necessary. Probably we can revisit if
+     * we need to do the same thing for PV guest in the future.
+     */
+    if (c_info->type == LIBXL_DOMAIN_TYPE_HVM &&
+        d_config->num_pcidevs && pod_enabled) {
+        fprintf(stderr, "PCI device assignment for HVM guest failed due to Paging-on-Demand enabled\n");
+        exit(1);
+    }
+
     switch (xlu_cfg_get_list(config, "cpuid", &cpuids, 0, 1)) {
     case 0:
         {
-- 
1.7.10.4

Re: [PATCH] tools/xl: disallow PCI device assignment for HVM guest when PoD is enabled

[Konrad Rzeszutek Wilk]

On Thu, Jan 09, 2014 at 07:17:03PM +0000, Wei Liu wrote:
> This replicates a Xend behavior, see ec789523749 ("xend: Dis-allow
> device assignment if PoD is enabled.").
> 
> Allegedly-reported-by: Konrad Wilk <konrad.wilk@oracle.com>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> ---
> This was listed in 4.4 development update. A quick skim through
> hypervisor vtd changesets suggests the situation stays unchanged since 3
> years ago -- at least I didn't find any log message related to "PoD".
> 
> Rune: git log --since="2010-01-21" xen/drivers/passthrough/vtd
> (It was first reported on 2010-01-21)
> 
> This patch is tested with setting memory=, maxmem= and pci=[] parameters
> in both HVM and PV guests. In HVM guest's case I need to have
> claim_mode=0 in /etc/xen/xl.conf to make xl actually create HVM with PoD
> mode enabled.

Which implies something is amiss with the PoD memory usage >= nr_pages
for the domain. Which implies that it allocates more memory than it asked
for.

We should track that as a bug I think.
> ---
>  tools/libxl/xl_cmdimpl.c |   29 +++++++++++++++++++++++++++--
>  1 file changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index c30f495..59aba7d 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -738,6 +738,7 @@ static void parse_config_data(const char *config_source,
>      int pci_msitranslate = 0;
>      int pci_permissive = 0;
>      int i, e;
> +    bool pod_enabled = false;
>  
>      libxl_domain_create_info *c_info = &d_config->c_info;
>      libxl_domain_build_info *b_info = &d_config->b_info;
> @@ -916,6 +917,12 @@ static void parse_config_data(const char *config_source,
>      if (!xlu_cfg_get_long (config, "maxmem", &l, 0))
>          b_info->max_memkb = l * 1024;
>  
> +    /* If target_memkb is smaller than max_memkb, the subsequent call
> +     * to libxc when building HVM omain will enable PoD mode.
> +     */
> +    pod_enabled = (c_info->type == LIBXL_DOMAIN_TYPE_HVM) &&
> +        (b_info->target_memkb < b_info->max_memkb);
> +
>      libxl_defbool_set(&b_info->claim_mode, claim_mode);
>  
>      if (xlu_cfg_get_string (config, "on_poweroff", &buf, 0))
> @@ -1468,9 +1475,9 @@ skip_vfb:
>          xlu_cfg_get_defbool(config, "e820_host", &b_info->u.pv.e820_host, 0);
>      }
>  
> +    d_config->num_pcidevs = 0;
> +    d_config->pcidevs = NULL;
>      if (!xlu_cfg_get_list (config, "pci", &pcis, 0, 0)) {
> -        d_config->num_pcidevs = 0;
> -        d_config->pcidevs = NULL;
>          for(i = 0; (buf = xlu_cfg_get_listitem (pcis, i)) != NULL; i++) {
>              libxl_device_pci *pcidev;
>  
> @@ -1488,6 +1495,24 @@ skip_vfb:
>              libxl_defbool_set(&b_info->u.pv.e820_host, true);
>      }
>  
> +    /* We cannot have PoD and PCI device assignment at the same
> +     * time. VT-d engine needs to set up the entire page table for
> +     * the domain. However if PoD is enabled, un-populated memory is
> +     * marked as populate_on_demand, and VT-d engine won't set up page
> +     * tables for them. Therefore any DMA towards those memory may
> +     * cause DMA fault.
> +     *
> +     * This is restricted to HVM guest, as only VT-d is relevant
> +     * in the counterpart in Xend. We're late in release cycle so the change
> +     * should only does what's necessary. Probably we can revisit if
> +     * we need to do the same thing for PV guest in the future.
> +     */
> +    if (c_info->type == LIBXL_DOMAIN_TYPE_HVM &&
> +        d_config->num_pcidevs && pod_enabled) {
> +        fprintf(stderr, "PCI device assignment for HVM guest failed due to Paging-on-Demand enabled\n");
> +        exit(1);
> +    }
> +
>      switch (xlu_cfg_get_list(config, "cpuid", &cpuids, 0, 1)) {
>      case 0:
>          {
> -- 
> 1.7.10.4
> 

Re: [PATCH] tools/xl: disallow PCI device assignment for HVM guest when PoD is enabled

[Ian Campbell]

On Thu, 2014-01-09 at 19:10 +0000, Wei Liu wrote:
> This replicates a Xend behavior, see ec789523749 ("xend: Dis-allow
> device assignment if PoD is enabled.").

Thanks, but I think this should be handled in libxl for the benefit of
all toolstacks. I think it has all the necessary inputs in
libxl_domain_create to make the decision and log + return ERROR_INVAL?

Re: [PATCH] tools/xl: disallow PCI device assignment for HVM guest when PoD is enabled

[Jan Beulich]

>>> On 09.01.14 at 20:10, Wei Liu <wei.liu2@citrix.com> wrote:
> +    /* We cannot have PoD and PCI device assignment at the same
> +     * time. VT-d engine needs to set up the entire page table for
> +     * the domain. However if PoD is enabled, un-populated memory is
> +     * marked as populate_on_demand, and VT-d engine won't set up page
> +     * tables for them. Therefore any DMA towards those memory may
> +     * cause DMA fault.
> +     *
> +     * This is restricted to HVM guest, as only VT-d is relevant
> +     * in the counterpart in Xend. We're late in release cycle so the change
> +     * should only does what's necessary. Probably we can revisit if
> +     * we need to do the same thing for PV guest in the future.
> +     */

The comment shouldn't say VT-d as long as the code isn't special
casing VT-d.

Jan

Re: [PATCH] tools/xl: disallow PCI device assignment for HVM guest when PoD is enabled

[Ian Campbell]

On Fri, 2014-01-10 at 09:19 +0000, Jan Beulich wrote:
> >>> On 09.01.14 at 20:10, Wei Liu <wei.liu2@citrix.com> wrote:
> > +    /* We cannot have PoD and PCI device assignment at the same
> > +     * time. VT-d engine needs to set up the entire page table for
> > +     * the domain. However if PoD is enabled, un-populated memory is
> > +     * marked as populate_on_demand, and VT-d engine won't set up page
> > +     * tables for them. Therefore any DMA towards those memory may
> > +     * cause DMA fault.
> > +     *
> > +     * This is restricted to HVM guest, as only VT-d is relevant
> > +     * in the counterpart in Xend. We're late in release cycle so the change
> > +     * should only does what's necessary. Probably we can revisit if
> > +     * we need to do the same thing for PV guest in the future.
> > +     */
> 
> The comment shouldn't say VT-d as long as the code isn't special
> casing VT-d.

Also this second paragraph looks more suitable to the commit message to
me.

Ian.

Re: [PATCH] tools/xl: disallow PCI device assignment for HVM guest when PoD is enabled

[Wei Liu]

On Thu, Jan 09, 2014 at 02:19:14PM -0500, Konrad Rzeszutek Wilk wrote:
> On Thu, Jan 09, 2014 at 07:17:03PM +0000, Wei Liu wrote:
> > This replicates a Xend behavior, see ec789523749 ("xend: Dis-allow
> > device assignment if PoD is enabled.").
> > 
> > Allegedly-reported-by: Konrad Wilk <konrad.wilk@oracle.com>
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > Cc: Ian Campbell <ian.campbell@citrix.com>
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > ---
> > This was listed in 4.4 development update. A quick skim through
> > hypervisor vtd changesets suggests the situation stays unchanged since 3
> > years ago -- at least I didn't find any log message related to "PoD".
> > 
> > Rune: git log --since="2010-01-21" xen/drivers/passthrough/vtd
> > (It was first reported on 2010-01-21)
> > 
> > This patch is tested with setting memory=, maxmem= and pci=[] parameters
> > in both HVM and PV guests. In HVM guest's case I need to have
> > claim_mode=0 in /etc/xen/xl.conf to make xl actually create HVM with PoD
> > mode enabled.
> 
> Which implies something is amiss with the PoD memory usage >= nr_pages
> for the domain. Which implies that it allocates more memory than it asked
> for.
> 
> We should track that as a bug I think.

Ian, can you track this in Xen bug tracker?

Claim mode doesn't work for HVM with PoD enabled.

Wei.