RE: [PATCH 1/4] VT-d: defer "no DRHD" check when (un)mapping devices

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: Wednesday, September 15, 2021 5:12 PM
> 
> If devices are to be skipped anyway (which is the case in particular for
> host bridges), there's no point complaining about a missing DRHD (and
> hence a missing association with an IOMMU).
> 
> While there convert assignments to initializers and constify "drhd"
> local variables.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>

> 
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -1460,14 +1460,10 @@ static int domain_context_unmap(struct d
>  static int domain_context_mapping(struct domain *domain, u8 devfn,
>                                    struct pci_dev *pdev)
>  {
> -    struct acpi_drhd_unit *drhd;
> +    const struct acpi_drhd_unit *drhd = acpi_find_matched_drhd_unit(pdev);
>      int ret = 0;
>      u8 seg = pdev->seg, bus = pdev->bus, secbus;
> 
> -    drhd = acpi_find_matched_drhd_unit(pdev);
> -    if ( !drhd )
> -        return -ENODEV;
> -
>      /*
>       * Generally we assume only devices from one node to get assigned to a
>       * given guest.  But even if not, by replacing the prior value here we
> @@ -1476,7 +1472,7 @@ static int domain_context_mapping(struct
>       * this or other devices may be penalized then, but some would also be
>       * if we left other than NUMA_NO_NODE untouched here.
>       */
> -    if ( drhd->iommu->node != NUMA_NO_NODE )
> +    if ( drhd && drhd->iommu->node != NUMA_NO_NODE )
>          dom_iommu(domain)->node = drhd->iommu->node;
> 
>      ASSERT(pcidevs_locked());
> @@ -1497,6 +1493,9 @@ static int domain_context_mapping(struct
>          break;
> 
>      case DEV_TYPE_PCIe_ENDPOINT:
> +        if ( !drhd )
> +            return -ENODEV;
> +
>          if ( iommu_debug )
>              printk(VTDPREFIX "%pd:PCIe: map %pp\n",
>                     domain, &PCI_SBDF3(seg, bus, devfn));
> @@ -1508,6 +1507,9 @@ static int domain_context_mapping(struct
>          break;
> 
>      case DEV_TYPE_PCI:
> +        if ( !drhd )
> +            return -ENODEV;
> +
>          if ( iommu_debug )
>              printk(VTDPREFIX "%pd:PCI: map %pp\n",
>                     domain, &PCI_SBDF3(seg, bus, devfn));
> @@ -1651,17 +1653,12 @@ int domain_context_unmap_one(
>  static int domain_context_unmap(struct domain *domain, u8 devfn,
>                                  struct pci_dev *pdev)
>  {
> -    struct acpi_drhd_unit *drhd;
> -    struct vtd_iommu *iommu;
> +    const struct acpi_drhd_unit *drhd = acpi_find_matched_drhd_unit(pdev);
> +    struct vtd_iommu *iommu = drhd ? drhd->iommu : NULL;
>      int ret;
>      u8 seg = pdev->seg, bus = pdev->bus, tmp_bus, tmp_devfn, secbus;
>      int found = 0;
> 
> -    drhd = acpi_find_matched_drhd_unit(pdev);
> -    if ( !drhd )
> -        return -ENODEV;
> -    iommu = drhd->iommu;
> -
>      switch ( pdev->type )
>      {
>      case DEV_TYPE_PCI_HOST_BRIDGE:
> @@ -1676,6 +1673,9 @@ static int domain_context_unmap(struct d
>          return 0;
> 
>      case DEV_TYPE_PCIe_ENDPOINT:
> +        if ( !iommu )
> +            return -ENODEV;
> +
>          if ( iommu_debug )
>              printk(VTDPREFIX "%pd:PCIe: unmap %pp\n",
>                     domain, &PCI_SBDF3(seg, bus, devfn));
> @@ -1686,6 +1686,9 @@ static int domain_context_unmap(struct d
>          break;
> 
>      case DEV_TYPE_PCI:
> +        if ( !iommu )
> +            return -ENODEV;
> +
>          if ( iommu_debug )
>              printk(VTDPREFIX "%pd:PCI: unmap %pp\n",
>                     domain, &PCI_SBDF3(seg, bus, devfn));