RE: [RFC PATCH] iommu/vt-d: Exclude known RMRRs from reserved ranges

[Tian, Kevin]

> From: Alex Williamson
> Sent: Wednesday, June 6, 2018 3:07 AM
> 
> device_is_rmrr_locked() allows graphics and USB devices to participate
> in the IOMMU API despite, and ignoring their RMRR association, however
> intel_iommu_get_resv_regions() still includes the RMRRs as unavailable
> IOVA space for the device.  Are we ignoring the RMRR for these devices
> or are we not?  If vfio starts consuming reserved regions, perhaps we
> no longer need to consider devices with RMRRs excluded from the IOMMU
> API interface, but we have a transitional problem that these allowed
> devices still impose incompatible IOVA restrictions per the reserved
> region reporting.  Dive further down the rabbit hole by also ignoring
> RMRRs for "known" devices in the reserved region reporting.

intel_iommu_get_resv_regions is used not just for IOMMU API. I'm
afraid doing so will make RMRR completely ignored, even in normal
DMA API path...

> 
> Signed-off-by: Alex Williamson <alex.williamson@xxxxxxxxxx>
> ---
>  drivers/iommu/intel-iommu.c |   35 +++++++++++++++++++++--------------
>  1 file changed, 21 insertions(+), 14 deletions(-)
> 
> If this is the approach we want to take, I could pull this in via the
> vfio tree, along with Shameer's patches which expose an IOVA list and
> enforce it to userspace, otherwise I'm afraid Shameer's patches will
> be blocked a while longer.  Thanks,
> 
> Alex
> 
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 749d8f235346..f312f93199c5 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -2864,19 +2864,24 @@ static bool device_has_rmrr(struct device *dev)
>   * any use of the RMRR regions will be torn down before assigning the
> device
>   * to a guest.
>   */
> -static bool device_is_rmrr_locked(struct device *dev)
> +static bool rmrr_is_ignored(struct device *dev)
>  {
> -	if (!device_has_rmrr(dev))
> -		return false;
> -
>  	if (dev_is_pci(dev)) {
>  		struct pci_dev *pdev = to_pci_dev(dev);
> 
>  		if (IS_USB_DEVICE(pdev) || IS_GFX_DEVICE(pdev))
> -			return false;
> +			return true;
>  	}
> 
> -	return true;
> +	return false;
> +}
> +
> +static bool device_is_rmrr_locked(struct device *dev)
> +{
> +	if (!device_has_rmrr(dev))
> +		return false;
> +
> +	return !rmrr_is_ignored(dev);
>  }
> 
>  static int iommu_should_identity_map(struct device *dev, int startup)
> @@ -5141,17 +5146,19 @@ static void
> intel_iommu_get_resv_regions(struct device *device,
>  	struct device *i_dev;
>  	int i;
> 
> -	rcu_read_lock();
> -	for_each_rmrr_units(rmrr) {
> -		for_each_active_dev_scope(rmrr->devices, rmrr-
> >devices_cnt,
> -					  i, i_dev) {
> -			if (i_dev != device)
> -				continue;
> +	if (!rmrr_is_ignored(device)) {
> +		rcu_read_lock();
> +		for_each_rmrr_units(rmrr) {
> +			for_each_active_dev_scope(rmrr->devices,
> +						  rmrr->devices_cnt, i, i_dev)
> {
> +				if (i_dev != device)
> +					continue;
> 
> -			list_add_tail(&rmrr->resv->list, head);
> +				list_add_tail(&rmrr->resv->list, head);
> +			}
>  		}
> +		rcu_read_unlock();
>  	}
> -	rcu_read_unlock();
> 
>  	reg = iommu_alloc_resv_region(IOAPIC_RANGE_START,
>  				      IOAPIC_RANGE_END -
> IOAPIC_RANGE_START + 1,