[PATCH] libxl: disallow PCI device assignment for HVM guest when PoD is enabled

[PATCH] libxl: 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.").

This change 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 do what's necessary. Probably we can revisit it if we need to do
the same thing for PV guest in the future.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc: Xiantao Zhang <xiantao.zhang@intel.com>
---
 tools/libxl/libxl_create.c |   22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index e03bb55..b7adf34 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -706,6 +706,7 @@ static void initiate_domain_create(libxl__egc *egc,
     libxl_ctx *ctx = libxl__gc_owner(gc);
     uint32_t domid;
     int i, ret;
+    bool pod_enabled = false;
 
     /* convenience aliases */
     libxl_domain_config *const d_config = dcs->guest_config;
@@ -714,6 +715,27 @@ static void initiate_domain_create(libxl__egc *egc,
 
     domid = 0;
 
+    /* If target_memkb is smaller than max_memkb, the subsequent call
+     * to libxc when building HVM domain will enable PoD mode.
+     */
+    pod_enabled = (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM) &&
+        (d_config->b_info.target_memkb < d_config->b_info.max_memkb);
+
+    /* We cannot have PoD and PCI device assignment at the same time
+     * for HVM guest. It was reported that VT-d engine cannot
+     * work with PoD enabled because it needs to populated entire page
+     * table for guest. Also a quick grep through AMD IOMMU related
+     * code suggests it has not coped with PoD as well. Just to stay
+     * on the safe side, we disable PCI device assignment with PoD all
+     * together, regardless of the underlying IOMMU in use.
+     */
+    if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM &&
+        d_config->num_pcidevs && pod_enabled) {
+        ret = ERROR_INVAL;
+        LOG(ERROR, "PCI device assignment for HVM guest failed due to PoD enabled");
+        goto error_out;
+    }
+
     ret = libxl__domain_create_info_setdefault(gc, &d_config->c_info);
     if (ret) goto error_out;
 
-- 
1.7.10.4

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

[Jan Beulich]

>>> On 10.01.14 at 12:27, Wei Liu <wei.liu2@citrix.com> wrote:
> +    /* We cannot have PoD and PCI device assignment at the same time
> +     * for HVM guest. It was reported that VT-d engine cannot

There's still a "VT-d" left in here...

Jan

> +     * work with PoD enabled because it needs to populated entire page
> +     * table for guest. Also a quick grep through AMD IOMMU related
> +     * code suggests it has not coped with PoD as well. Just to stay
> +     * on the safe side, we disable PCI device assignment with PoD all
> +     * together, regardless of the underlying IOMMU in use.
> +     */

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

[Wei Liu]

On Fri, Jan 10, 2014 at 11:39:51AM +0000, Jan Beulich wrote:
> >>> On 10.01.14 at 12:27, Wei Liu <wei.liu2@citrix.com> wrote:
> > +    /* We cannot have PoD and PCI device assignment at the same time
> > +     * for HVM guest. It was reported that VT-d engine cannot
> 
> There's still a "VT-d" left in here...
> 

I mentioned AMD as well. Was trying to clarify things a bit more...

Wei.

> Jan
> 
> > +     * work with PoD enabled because it needs to populated entire page
> > +     * table for guest. Also a quick grep through AMD IOMMU related
> > +     * code suggests it has not coped with PoD as well. Just to stay
> > +     * on the safe side, we disable PCI device assignment with PoD all
> > +     * together, regardless of the underlying IOMMU in use.
> > +     */
> 
> 

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

[Andrew Cooper]

On 10/01/2014 11:46, Wei Liu wrote:
> On Fri, Jan 10, 2014 at 11:39:51AM +0000, Jan Beulich wrote:
>>>>> On 10.01.14 at 12:27, Wei Liu <wei.liu2@citrix.com> wrote:
>>> +    /* We cannot have PoD and PCI device assignment at the same time
>>> +     * for HVM guest. It was reported that VT-d engine cannot
>> There's still a "VT-d" left in here...
>>
> I mentioned AMD as well. Was trying to clarify things a bit more...
>
> Wei.

Use "IOMMU"/"PCI Passthrough"/etc as appropriate, which are vendor neutral.

There is no way for for PoD (or Paging for that matter) to work in
combination with PCIPassthrough, as you need all the backing RAM for all
gfns to exist to receive DMA.

~Andrew

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

[Wei Liu]

On Fri, Jan 10, 2014 at 12:16:22PM +0000, Andrew Cooper wrote:
> On 10/01/2014 11:46, Wei Liu wrote:
> > On Fri, Jan 10, 2014 at 11:39:51AM +0000, Jan Beulich wrote:
> >>>>> On 10.01.14 at 12:27, Wei Liu <wei.liu2@citrix.com> wrote:
> >>> +    /* We cannot have PoD and PCI device assignment at the same time
> >>> +     * for HVM guest. It was reported that VT-d engine cannot
> >> There's still a "VT-d" left in here...
> >>
> > I mentioned AMD as well. Was trying to clarify things a bit more...
> >
> > Wei.
> 
> Use "IOMMU"/"PCI Passthrough"/etc as appropriate, which are vendor neutral.
> 
> There is no way for for PoD (or Paging for that matter) to work in
> combination with PCIPassthrough, as you need all the backing RAM for all
> gfns to exist to receive DMA.
> 

Thanks for clarification. I will fix this comment.

Wei.

> ~Andrew

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

[Jan Beulich]

>>> On 10.01.14 at 13:16, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> On 10/01/2014 11:46, Wei Liu wrote:
>> On Fri, Jan 10, 2014 at 11:39:51AM +0000, Jan Beulich wrote:
>>>>>> On 10.01.14 at 12:27, Wei Liu <wei.liu2@citrix.com> wrote:
>>>> +    /* We cannot have PoD and PCI device assignment at the same time
>>>> +     * for HVM guest. It was reported that VT-d engine cannot
>>> There's still a "VT-d" left in here...
>>>
>> I mentioned AMD as well. Was trying to clarify things a bit more...
> 
> Use "IOMMU"/"PCI Passthrough"/etc as appropriate, which are vendor neutral.
> 
> There is no way for for PoD (or Paging for that matter) to work in
> combination with PCIPassthrough, as you need all the backing RAM for all
> gfns to exist to receive DMA.

That's going a little too far: If IOMMU faults were recoverable, dealing
with non-present pages would become possible (with other caveats
of course). So this is not a fundamental attribute of IOMMUs, but
there doesn't seem to be any reason to believe that the current
model would change any time soon for either of the vendors.

Jan

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

[Wei Liu]

On Fri, Jan 10, 2014 at 12:28:52PM +0000, Jan Beulich wrote:
> >>> On 10.01.14 at 13:16, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> > On 10/01/2014 11:46, Wei Liu wrote:
> >> On Fri, Jan 10, 2014 at 11:39:51AM +0000, Jan Beulich wrote:
> >>>>>> On 10.01.14 at 12:27, Wei Liu <wei.liu2@citrix.com> wrote:
> >>>> +    /* We cannot have PoD and PCI device assignment at the same time
> >>>> +     * for HVM guest. It was reported that VT-d engine cannot
> >>> There's still a "VT-d" left in here...
> >>>
> >> I mentioned AMD as well. Was trying to clarify things a bit more...
> > 
> > Use "IOMMU"/"PCI Passthrough"/etc as appropriate, which are vendor neutral.
> > 
> > There is no way for for PoD (or Paging for that matter) to work in
> > combination with PCIPassthrough, as you need all the backing RAM for all
> > gfns to exist to receive DMA.
> 
> That's going a little too far: If IOMMU faults were recoverable, dealing
> with non-present pages would become possible (with other caveats
> of course). So this is not a fundamental attribute of IOMMUs, but
> there doesn't seem to be any reason to believe that the current
> model would change any time soon for either of the vendors.
> 

Do you have suggestion on how this comment should be phrased?

Wei.

> Jan

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

[Ian Campbell]

On Fri, 2014-01-10 at 14:03 +0000, Wei Liu wrote:
> On Fri, Jan 10, 2014 at 12:28:52PM +0000, Jan Beulich wrote:
> > >>> On 10.01.14 at 13:16, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> > > On 10/01/2014 11:46, Wei Liu wrote:
> > >> On Fri, Jan 10, 2014 at 11:39:51AM +0000, Jan Beulich wrote:
> > >>>>>> On 10.01.14 at 12:27, Wei Liu <wei.liu2@citrix.com> wrote:
> > >>>> +    /* We cannot have PoD and PCI device assignment at the same time
> > >>>> +     * for HVM guest. It was reported that VT-d engine cannot
> > >>> There's still a "VT-d" left in here...
> > >>>
> > >> I mentioned AMD as well. Was trying to clarify things a bit more...
> > > 
> > > Use "IOMMU"/"PCI Passthrough"/etc as appropriate, which are vendor neutral.
> > > 
> > > There is no way for for PoD (or Paging for that matter) to work in
> > > combination with PCIPassthrough, as you need all the backing RAM for all
> > > gfns to exist to receive DMA.
> > 
> > That's going a little too far: If IOMMU faults were recoverable, dealing
> > with non-present pages would become possible (with other caveats
> > of course). So this is not a fundamental attribute of IOMMUs, but
> > there doesn't seem to be any reason to believe that the current
> > model would change any time soon for either of the vendors.
> > 
> 
> Do you have suggestion on how this comment should be phrased?

We cannot have PoD and PCI device assignment at the same to for HVM
guests. This is because the IOMMU support in Xen cannot currently cope
with faults due to pages which are not present.

?

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

[Jan Beulich]

>>> On 10.01.14 at 15:03, Wei Liu <wei.liu2@citrix.com> wrote:
> On Fri, Jan 10, 2014 at 12:28:52PM +0000, Jan Beulich wrote:
>> >>> On 10.01.14 at 13:16, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> > On 10/01/2014 11:46, Wei Liu wrote:
>> >> On Fri, Jan 10, 2014 at 11:39:51AM +0000, Jan Beulich wrote:
>> >>>>>> On 10.01.14 at 12:27, Wei Liu <wei.liu2@citrix.com> wrote:
>> >>>> +    /* We cannot have PoD and PCI device assignment at the same time
>> >>>> +     * for HVM guest. It was reported that VT-d engine cannot
>> >>> There's still a "VT-d" left in here...
>> >>>
>> >> I mentioned AMD as well. Was trying to clarify things a bit more...
>> > 
>> > Use "IOMMU"/"PCI Passthrough"/etc as appropriate, which are vendor neutral.
>> > 
>> > There is no way for for PoD (or Paging for that matter) to work in
>> > combination with PCIPassthrough, as you need all the backing RAM for all
>> > gfns to exist to receive DMA.
>> 
>> That's going a little too far: If IOMMU faults were recoverable, dealing
>> with non-present pages would become possible (with other caveats
>> of course). So this is not a fundamental attribute of IOMMUs, but
>> there doesn't seem to be any reason to believe that the current
>> model would change any time soon for either of the vendors.
>> 
> 
> Do you have suggestion on how this comment should be phrased?

Just say "IOMMU" or "DMA remapping" instead of "VT-d".

Jan

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

[Wei Liu]

On Fri, Jan 10, 2014 at 02:09:12PM +0000, Jan Beulich wrote:
> >>> On 10.01.14 at 15:03, Wei Liu <wei.liu2@citrix.com> wrote:
> > On Fri, Jan 10, 2014 at 12:28:52PM +0000, Jan Beulich wrote:
> >> >>> On 10.01.14 at 13:16, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> >> > On 10/01/2014 11:46, Wei Liu wrote:
> >> >> On Fri, Jan 10, 2014 at 11:39:51AM +0000, Jan Beulich wrote:
> >> >>>>>> On 10.01.14 at 12:27, Wei Liu <wei.liu2@citrix.com> wrote:
> >> >>>> +    /* We cannot have PoD and PCI device assignment at the same time
> >> >>>> +     * for HVM guest. It was reported that VT-d engine cannot
> >> >>> There's still a "VT-d" left in here...
> >> >>>
> >> >> I mentioned AMD as well. Was trying to clarify things a bit more...
> >> > 
> >> > Use "IOMMU"/"PCI Passthrough"/etc as appropriate, which are vendor neutral.
> >> > 
> >> > There is no way for for PoD (or Paging for that matter) to work in
> >> > combination with PCIPassthrough, as you need all the backing RAM for all
> >> > gfns to exist to receive DMA.
> >> 
> >> That's going a little too far: If IOMMU faults were recoverable, dealing
> >> with non-present pages would become possible (with other caveats
> >> of course). So this is not a fundamental attribute of IOMMUs, but
> >> there doesn't seem to be any reason to believe that the current
> >> model would change any time soon for either of the vendors.
> >> 
> > 
> > Do you have suggestion on how this comment should be phrased?
> 
> Just say "IOMMU" or "DMA remapping" instead of "VT-d".
> 

OK, like this:

/* We cannot have PoD and PCI device assignment at the same time for HVM
 * guest. It was reported that IOMMU cannot work with PoD enabled
 * because it needs to populated entire page table for guest. To stay on
 * the safe side, we disable PCI device assignment when PoD is enabled.
 */

Wei.

> Jan

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

[Jan Beulich]

>>> On 10.01.14 at 15:15, Wei Liu <wei.liu2@citrix.com> wrote:
> On Fri, Jan 10, 2014 at 02:09:12PM +0000, Jan Beulich wrote:
>> >>> On 10.01.14 at 15:03, Wei Liu <wei.liu2@citrix.com> wrote:
>> > On Fri, Jan 10, 2014 at 12:28:52PM +0000, Jan Beulich wrote:
>> >> >>> On 10.01.14 at 13:16, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> >> > On 10/01/2014 11:46, Wei Liu wrote:
>> >> >> On Fri, Jan 10, 2014 at 11:39:51AM +0000, Jan Beulich wrote:
>> >> >>>>>> On 10.01.14 at 12:27, Wei Liu <wei.liu2@citrix.com> wrote:
>> >> >>>> +    /* We cannot have PoD and PCI device assignment at the same time
>> >> >>>> +     * for HVM guest. It was reported that VT-d engine cannot
>> >> >>> There's still a "VT-d" left in here...
>> >> >>>
>> >> >> I mentioned AMD as well. Was trying to clarify things a bit more...
>> >> > 
>> >> > Use "IOMMU"/"PCI Passthrough"/etc as appropriate, which are vendor 
> neutral.
>> >> > 
>> >> > There is no way for for PoD (or Paging for that matter) to work in
>> >> > combination with PCIPassthrough, as you need all the backing RAM for all
>> >> > gfns to exist to receive DMA.
>> >> 
>> >> That's going a little too far: If IOMMU faults were recoverable, dealing
>> >> with non-present pages would become possible (with other caveats
>> >> of course). So this is not a fundamental attribute of IOMMUs, but
>> >> there doesn't seem to be any reason to believe that the current
>> >> model would change any time soon for either of the vendors.
>> >> 
>> > 
>> > Do you have suggestion on how this comment should be phrased?
>> 
>> Just say "IOMMU" or "DMA remapping" instead of "VT-d".
>> 
> 
> OK, like this:
> 
> /* We cannot have PoD and PCI device assignment at the same time for HVM
>  * guest. It was reported that IOMMU cannot work with PoD enabled
>  * because it needs to populated entire page table for guest. To stay on
>  * the safe side, we disable PCI device assignment when PoD is enabled.
>  */

Sound fine to me. As does Ian's suggestion.

Jan

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

[Wei Liu]

Any more comments on code logic and / or the location of the new
snippet? Should I send a new version with code comment fixed?

Thanks
Wei.

On Fri, Jan 10, 2014 at 11:27:42AM +0000, Wei Liu wrote:
> This replicates a Xend behavior, see ec789523749 ("xend: Dis-allow
> device assignment if PoD is enabled.").
> 
> This change 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 do what's necessary. Probably we can revisit it if we need to do
> the same thing for PV guest in the future.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Cc: Xiantao Zhang <xiantao.zhang@intel.com>
> ---
>  tools/libxl/libxl_create.c |   22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index e03bb55..b7adf34 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -706,6 +706,7 @@ static void initiate_domain_create(libxl__egc *egc,
>      libxl_ctx *ctx = libxl__gc_owner(gc);
>      uint32_t domid;
>      int i, ret;
> +    bool pod_enabled = false;
>  
>      /* convenience aliases */
>      libxl_domain_config *const d_config = dcs->guest_config;
> @@ -714,6 +715,27 @@ static void initiate_domain_create(libxl__egc *egc,
>  
>      domid = 0;
>  
> +    /* If target_memkb is smaller than max_memkb, the subsequent call
> +     * to libxc when building HVM domain will enable PoD mode.
> +     */
> +    pod_enabled = (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM) &&
> +        (d_config->b_info.target_memkb < d_config->b_info.max_memkb);
> +
> +    /* We cannot have PoD and PCI device assignment at the same time
> +     * for HVM guest. It was reported that VT-d engine cannot
> +     * work with PoD enabled because it needs to populated entire page
> +     * table for guest. Also a quick grep through AMD IOMMU related
> +     * code suggests it has not coped with PoD as well. Just to stay
> +     * on the safe side, we disable PCI device assignment with PoD all
> +     * together, regardless of the underlying IOMMU in use.
> +     */
> +    if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM &&
> +        d_config->num_pcidevs && pod_enabled) {
> +        ret = ERROR_INVAL;
> +        LOG(ERROR, "PCI device assignment for HVM guest failed due to PoD enabled");
> +        goto error_out;
> +    }
> +
>      ret = libxl__domain_create_info_setdefault(gc, &d_config->c_info);
>      if (ret) goto error_out;
>  
> -- 
> 1.7.10.4

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

[Ian Campbell]

On Mon, 2014-01-13 at 10:54 +0000, Wei Liu wrote:
> Any more comments on code logic and / or the location of the new
> snippet? Should I send a new version with code comment fixed?
> [...] 
> > +    /* If target_memkb is smaller than max_memkb, the subsequent call
> > +     * to libxc when building HVM domain will enable PoD mode.
> > +     */
> > +    pod_enabled = (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM) &&
> > +        (d_config->b_info.target_memkb < d_config->b_info.max_memkb);

I suppose this corresponds to exactly when PoD would be enabled?

I'm happy with the patch, and it would be better to go ahead now than to
wait for George (with his PoD hat in place) to confirm/deny, so please
resend with the improved comment.

Ian.

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

[Wei Liu]

On Mon, Jan 13, 2014 at 11:11:20AM +0000, Ian Campbell wrote:
> On Mon, 2014-01-13 at 10:54 +0000, Wei Liu wrote:
> > Any more comments on code logic and / or the location of the new
> > snippet? Should I send a new version with code comment fixed?
> > [...] 
> > > +    /* If target_memkb is smaller than max_memkb, the subsequent call
> > > +     * to libxc when building HVM domain will enable PoD mode.
> > > +     */
> > > +    pod_enabled = (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM) &&
> > > +        (d_config->b_info.target_memkb < d_config->b_info.max_memkb);
> 
> I suppose this corresponds to exactly when PoD would be enabled?
> 

Yes. In libxc if maxmem > target_mem then pod_mode is set. See
tools/libxc/xc_hvm_build_x86.c:setup_guest.

> I'm happy with the patch, and it would be better to go ahead now than to
> wait for George (with his PoD hat in place) to confirm/deny, so please
> resend with the improved comment.

Will do.

Wei.

> 
> Ian.
> 

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

[Ian Campbell]

On Mon, 2014-01-13 at 11:45 +0000, Wei Liu wrote:
> On Mon, Jan 13, 2014 at 11:11:20AM +0000, Ian Campbell wrote:
> > On Mon, 2014-01-13 at 10:54 +0000, Wei Liu wrote:
> > > Any more comments on code logic and / or the location of the new
> > > snippet? Should I send a new version with code comment fixed?
> > > [...] 
> > > > +    /* If target_memkb is smaller than max_memkb, the subsequent call
> > > > +     * to libxc when building HVM domain will enable PoD mode.
> > > > +     */
> > > > +    pod_enabled = (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM) &&
> > > > +        (d_config->b_info.target_memkb < d_config->b_info.max_memkb);
> > 
> > I suppose this corresponds to exactly when PoD would be enabled?
> > 
> 
> Yes. In libxc if maxmem > target_mem then pod_mode is set. See
> tools/libxc/xc_hvm_build_x86.c:setup_guest.

Putting this into a common place might be a nice future cleanup...

Ian.