Re: [PATCH 3/4] iommu/vt-d: Handle RMRR with PCI bridge device scopes

[Jacob Pan]

On Mon, 13 May 2019 09:13:01 +0200
Eric Auger <eric.auger@xxxxxxxxxx> wrote:

> When reading the vtd specification and especially the
> Reserved Memory Region Reporting Structure chapter,
> it is not obvious a device scope element cannot be a
> PCI-PCI bridge, in which case all downstream ports are
> likely to access the reserved memory region. Let's handle
> this case in device_has_rmrr.
> 
> Fixes: ea2447f700ca ("intel-iommu: Prevent devices with RMRRs from
> being placed into SI Domain")
> 
> Signed-off-by: Eric Auger <eric.auger@xxxxxxxxxx>
> ---
>  drivers/iommu/intel-iommu.c | 32 +++++++++++++++++++++++---------
>  1 file changed, 23 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index e2134b13c9ae..89d82a1d50b1 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -736,12 +736,31 @@ static int iommu_dummy(struct device *dev)
>  	return dev->archdata.iommu == DUMMY_DEVICE_DOMAIN_INFO;
>  }
>  
> +static bool
> +is_downstream_to_pci_bridge(struct device *deva, struct device *devb)
> +{
> +	struct pci_dev *pdeva, *pdevb;
> +
is there a more illustrative name for these. i guess deva is is the
bridge dev?
> +	if (!dev_is_pci(deva) || !dev_is_pci(devb))
> +		return false;
> +
> +	pdeva = to_pci_dev(deva);
> +	pdevb = to_pci_dev(devb);
> +
> +	if (pdevb->subordinate &&
> +	    pdevb->subordinate->number <= pdeva->bus->number &&
> +	    pdevb->subordinate->busn_res.end >= pdeva->bus->number)
> +		return true;
> +
> +	return false;
> +}
> +
this seems to be a separate cleanup patch.
>  static struct intel_iommu *device_to_iommu(struct device *dev, u8
> *bus, u8 *devfn) {
>  	struct dmar_drhd_unit *drhd = NULL;
>  	struct intel_iommu *iommu;
>  	struct device *tmp;
> -	struct pci_dev *ptmp, *pdev = NULL;
> +	struct pci_dev *pdev = NULL;
>  	u16 segment = 0;
>  	int i;
>  
> @@ -787,13 +806,7 @@ static struct intel_iommu
> *device_to_iommu(struct device *dev, u8 *bus, u8 *devf goto out;
>  			}
>  
> -			if (!pdev || !dev_is_pci(tmp))
> -				continue;
> -
> -			ptmp = to_pci_dev(tmp);
> -			if (ptmp->subordinate &&
> -			    ptmp->subordinate->number <=
> pdev->bus->number &&
> -			    ptmp->subordinate->busn_res.end >=
> pdev->bus->number)
> +			if (is_downstream_to_pci_bridge(dev, tmp))
>  				goto got_pdev;
>  		}
>  
> @@ -2886,7 +2899,8 @@ static bool device_has_rmrr(struct device *dev)
>  		 */
>  		for_each_active_dev_scope(rmrr->devices,
>  					  rmrr->devices_cnt, i, tmp)
> -			if (tmp == dev) {
> +			if (tmp == dev ||
> +			    is_downstream_to_pci_bridge(dev, tmp)) {
>  				rcu_read_unlock();
>  				return true;
>  			}

[Jacob Pan]