[Xen-devel] [PATCH 0/2] VT-d: domain_context_mapping_one() adjustments

[Xen-devel] [PATCH 0/2] VT-d: domain_context_mapping_one() adjustments

[Jan Beulich]

1: don't pass bridge devices to domain_context_mapping_one()
2: adjust log messages in domain_context_mapping_one()

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 1/2] VT-d: don't pass bridge devices to domain_context_mapping_one()

[Jan Beulich]

When passed a non-NULL pdev, the function does an owner check when it
finds an already existing context mapping. Bridges, however, don't get
passed through to guests, and hence their owner is always going to be
Dom0, leading to the assigment of all but one of the function of multi-
function PCI devices behind bridges to fail.

Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Note: This was reported as an apparent regression from XSA-302 / -306.
      So far I haven't been able to figure out how the code would have
      worked before, i.e. to me it looks like a pre-existing problem.
      This leaves the risk of the change here papering over another
      issue.

--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1499,7 +1499,7 @@ static int domain_context_mapping(struct
             break;
 
         ret = domain_context_mapping_one(domain, drhd->iommu, bus, devfn,
-                                         pci_get_pdev(seg, bus, devfn));
+                                         NULL);
 
         /*
          * Devices behind PCIe-to-PCI/PCIx bridge may generate different
@@ -1509,7 +1509,7 @@ static int domain_context_mapping(struct
         if ( !ret && pdev_type(seg, bus, devfn) == DEV_TYPE_PCIe2PCI_BRIDGE &&
              (secbus != pdev->bus || pdev->devfn != 0) )
             ret = domain_context_mapping_one(domain, drhd->iommu, secbus, 0,
-                                             pci_get_pdev(seg, secbus, 0));
+                                             NULL);
 
         break;
 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 2/2] VT-d: adjust log messages in domain_context_mapping_one()

[Jan Beulich]

Add missing newlines, use %pd, and drop exclamation marks.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1308,10 +1308,9 @@ int domain_context_mapping_one(
             if ( pdev->domain != domain )
             {
                 printk(XENLOG_G_INFO VTDPREFIX
-                       "d%d: %04x:%02x:%02x.%u owned by d%d!",
-                       domain->domain_id,
-                       seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
-                       pdev->domain ? pdev->domain->domain_id : -1);
+                       "%pd: %04x:%02x:%02x.%u owned by %pd\n",
+                       domain, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
+                       pdev->domain);
                 res = -EINVAL;
             }
         }
@@ -1323,16 +1322,15 @@ int domain_context_mapping_one(
             if ( cdomain < 0 )
             {
                 printk(XENLOG_G_WARNING VTDPREFIX
-                       "d%d: %04x:%02x:%02x.%u mapped, but can't find owner!\n",
-                       domain->domain_id,
-                       seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
+                       "%pd: %04x:%02x:%02x.%u mapped, but can't find owner\n",
+                       domain, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
                 res = -EINVAL;
             }
             else if ( cdomain != domain->domain_id )
             {
                 printk(XENLOG_G_INFO VTDPREFIX
-                       "d%d: %04x:%02x:%02x.%u already mapped to d%d!",
-                       domain->domain_id,
+                       "%pd: %04x:%02x:%02x.%u already mapped to d%d\n",
+                       domain,
                        seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
                        cdomain);
                 res = -EINVAL;


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 1/2] VT-d: don't pass bridge devices to domain_context_mapping_one()

[Tian, Kevin]

> From: Jan Beulich <jbeulich@suse.com>
> Sent: Tuesday, January 7, 2020 3:38 PM
> 
> When passed a non-NULL pdev, the function does an owner check when it
> finds an already existing context mapping. Bridges, however, don't get
> passed through to guests, and hence their owner is always going to be
> Dom0, leading to the assigment of all but one of the function of multi-
> function PCI devices behind bridges to fail.
> 
> Reported-by: Marek Marczykowski-Górecki
> <marmarek@invisiblethingslab.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> Note: This was reported as an apparent regression from XSA-302 / -306.
>       So far I haven't been able to figure out how the code would have
>       worked before, i.e. to me it looks like a pre-existing problem.
>       This leaves the risk of the change here papering over another
>       issue.
> 
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -1499,7 +1499,7 @@ static int domain_context_mapping(struct
>              break;
> 
>          ret = domain_context_mapping_one(domain, drhd->iommu, bus, devfn,
> -                                         pci_get_pdev(seg, bus, devfn));
> +                                         NULL);

the relationship between NULL and a bridge device is not obvious
by just looking at the code. Can you at least add some comment
here, or possibly be clearer by having a mapping_bridge wrapper?

> 
>          /*
>           * Devices behind PCIe-to-PCI/PCIx bridge may generate different
> @@ -1509,7 +1509,7 @@ static int domain_context_mapping(struct
>          if ( !ret && pdev_type(seg, bus, devfn) == DEV_TYPE_PCIe2PCI_BRIDGE
> &&
>               (secbus != pdev->bus || pdev->devfn != 0) )
>              ret = domain_context_mapping_one(domain, drhd->iommu, secbus,
> 0,
> -                                             pci_get_pdev(seg, secbus, 0));
> +                                             NULL);
> 
>          break;
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 2/2] VT-d: adjust log messages in domain_context_mapping_one()

[Tian, Kevin]

> From: Jan Beulich <jbeulich@suse.com>
> Sent: Tuesday, January 7, 2020 3:38 PM
> 
> Add missing newlines, use %pd, and drop exclamation marks.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel