[PATCH v2] tools/libxl: don't allow IOMMU usage with PoD

[PATCH v2] tools/libxl: don't allow IOMMU usage with PoD

[Roger Pau Monne]

Prevent libxl from creating guests that attempts to use PoD together
with an IOMMU, even if no devices are actually assigned.

While the hypervisor could support using PoD together with an IOMMU as
long as no devices are assigned, such usage seems doubtful. There's no
guarantee the guest has PoD no longer be active, and thus a later
assignment of a PCI device to such domain could fail.

Preventing the usage of PoD together with an IOMMU at guest creation
avoids having to add checks for active PoD entries in the device
assignment paths.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
---
Changes since v1:
 - Reword commit message.
---
 tools/libs/light/libxl_create.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
index d7a40d7550..7499922088 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -1160,17 +1160,16 @@ int libxl__domain_config_setdefault(libxl__gc *gc,
     pod_enabled = (d_config->c_info.type != LIBXL_DOMAIN_TYPE_PV) &&
         (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 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.
+    /* We don't support having PoD and an IOMMU at the same time for HVM
+     * guests. An active IOMMU cannot work with PoD because it needs a fully
+     * populated page-table. Prevent PoD usage if the domain has an IOMMU
+     * assigned, even if not active.
      */
     if (d_config->c_info.type != LIBXL_DOMAIN_TYPE_PV &&
-        d_config->num_pcidevs && pod_enabled) {
+        d_config->c_info.passthrough != LIBXL_PASSTHROUGH_DISABLED &&
+        pod_enabled) {
         ret = ERROR_INVAL;
-        LOGD(ERROR, domid,
-             "PCI device assignment for HVM guest failed due to PoD enabled");
+        LOGD(ERROR, domid, "IOMMU not supported together with PoD");
         goto error_out;
     }
 
-- 
2.34.1

Re: [PATCH v2] tools/libxl: don't allow IOMMU usage with PoD

[Anthony PERARD]

On Thu, Feb 03, 2022 at 03:32:11PM +0100, Roger Pau Monne wrote:
>      if (d_config->c_info.type != LIBXL_DOMAIN_TYPE_PV &&
> -        d_config->num_pcidevs && pod_enabled) {
> +        d_config->c_info.passthrough != LIBXL_PASSTHROUGH_DISABLED &&
> +        pod_enabled) {
>          ret = ERROR_INVAL;
> -        LOGD(ERROR, domid,
> -             "PCI device assignment for HVM guest failed due to PoD enabled");
> +        LOGD(ERROR, domid, "IOMMU not supported together with PoD");

I'm not sure that this new error message is going to be good enough to
point out configuration issue for the guest.

One is going to set 'pci=["foo"]' or 'dtdev=["bar"]', which will enable
passthrough. Then they may get en error about IOMMU or PoD.
Should we maybe write something like this instead?

   "IOMMU or device passthrough not supported together with PoD"

Thanks,

-- 
Anthony PERARD

Re: [PATCH v2] tools/libxl: don't allow IOMMU usage with PoD

[Roger Pau Monné]

On Tue, Feb 15, 2022 at 03:08:21PM +0000, Anthony PERARD wrote:
> On Thu, Feb 03, 2022 at 03:32:11PM +0100, Roger Pau Monne wrote:
> >      if (d_config->c_info.type != LIBXL_DOMAIN_TYPE_PV &&
> > -        d_config->num_pcidevs && pod_enabled) {
> > +        d_config->c_info.passthrough != LIBXL_PASSTHROUGH_DISABLED &&
> > +        pod_enabled) {
> >          ret = ERROR_INVAL;
> > -        LOGD(ERROR, domid,
> > -             "PCI device assignment for HVM guest failed due to PoD enabled");
> > +        LOGD(ERROR, domid, "IOMMU not supported together with PoD");
> 
> I'm not sure that this new error message is going to be good enough to
> point out configuration issue for the guest.
> 
> One is going to set 'pci=["foo"]' or 'dtdev=["bar"]', which will enable
> passthrough. Then they may get en error about IOMMU or PoD.
> Should we maybe write something like this instead?
> 
>    "IOMMU or device passthrough not supported together with PoD"

The "or" seems weird to me: IOMMU is mandatory for device passthrough.
Maybe:

"IOMMU required for device passthrough but not support together with PoD"

Would that be OK?

Thanks, Roger.

Re: [PATCH v2] tools/libxl: don't allow IOMMU usage with PoD

[Anthony PERARD]

On Wed, Feb 16, 2022 at 10:12:03AM +0100, Roger Pau Monné wrote:
> On Tue, Feb 15, 2022 at 03:08:21PM +0000, Anthony PERARD wrote:
> > On Thu, Feb 03, 2022 at 03:32:11PM +0100, Roger Pau Monne wrote:
> > >      if (d_config->c_info.type != LIBXL_DOMAIN_TYPE_PV &&
> > > -        d_config->num_pcidevs && pod_enabled) {
> > > +        d_config->c_info.passthrough != LIBXL_PASSTHROUGH_DISABLED &&
> > > +        pod_enabled) {
> > >          ret = ERROR_INVAL;
> > > -        LOGD(ERROR, domid,
> > > -             "PCI device assignment for HVM guest failed due to PoD enabled");
> > > +        LOGD(ERROR, domid, "IOMMU not supported together with PoD");
> > 
> > I'm not sure that this new error message is going to be good enough to
> > point out configuration issue for the guest.
> > 
> > One is going to set 'pci=["foo"]' or 'dtdev=["bar"]', which will enable
> > passthrough. Then they may get en error about IOMMU or PoD.
> > Should we maybe write something like this instead?
> > 
> >    "IOMMU or device passthrough not supported together with PoD"
> 
> The "or" seems weird to me: IOMMU is mandatory for device passthrough.
> Maybe:
> 
> "IOMMU required for device passthrough but not support together with PoD"

                                                 ^ supported ?
> Would that be OK?

Sound good, with that new error message: Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,

-- 
Anthony PERARD