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

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

[Alex Williamson]

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.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 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,

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

[Jacob Pan]

On Tue, 05 Jun 2018 13:06:54 -0600
Alex Williamson <alex.williamson@redhat.com> wrote:

> 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.
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>  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,
> 
I think this patch makes sense in that it makes IOVA reserved range
check consistent.

Thanks,

Jacob
> 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,
> 
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

[Jacob Pan]

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@redhat.com>
> ---
>  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,

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

[Alex Williamson]

On Wed, 6 Jun 2018 05:29:58 +0000
"Tian, Kevin" <kevin.tian@intel.com> wrote:

> > 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...

Well, I'm a bit stuck then, we have existing IOMMU API users that
ignore these ranges and in fact conflict with these ranges blocking us
from restricting mappings within these ranges.  My impression is that
IOMMU reserved ranges should only be ranges which have some fundamental
limitation in the IOMMU, not simply mappings for which firmware has
requested an identity mapped range.  The latter should simply be a
pre-allocation of the IOVA space, for the cases where we choose to
honor the RMRR.  Thanks,

Alex

> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > ---
> >  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,  
> 

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

[Tian, Kevin]

> From: Alex Williamson [mailto:alex.williamson@redhat.com]
> Sent: Thursday, June 7, 2018 11:02 PM
> 
> On Wed, 6 Jun 2018 05:29:58 +0000
> "Tian, Kevin" <kevin.tian@intel.com> wrote:
> 
> > > 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...
> 
> Well, I'm a bit stuck then, we have existing IOMMU API users that
> ignore these ranges and in fact conflict with these ranges blocking us
> from restricting mappings within these ranges.  My impression is that
> IOMMU reserved ranges should only be ranges which have some
> fundamental
> limitation in the IOMMU, not simply mappings for which firmware has
> requested an identity mapped range.  The latter should simply be a
> pre-allocation of the IOVA space, for the cases where we choose to
> honor the RMRR.  Thanks,
> 

Then possibly need introduce a different interface for pre-allocation
scenario, if above definition of reserved ranges is agreed. Currently
two categories are both called reserved resources, e.g. IOMMU_RESV
_DIRECT for rmrr and IOMMU_RESV_MSI for MSI...

Thanks
Kevin

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

[Shameerali Kolothum Thodi]

Hi Alex,

> -----Original Message-----
> From: Tian, Kevin [mailto:kevin.tian@intel.com]
> Sent: 08 June 2018 03:56
> To: Alex Williamson <alex.williamson@redhat.com>
> Cc: dwmw2@infradead.org; iommu@lists.linux-foundation.org;
> kvm@vger.kernel.org; linux-kernel@vger.kernel.org; Shameerali Kolothum
> Thodi <shameerali.kolothum.thodi@huawei.com>
> Subject: RE: [RFC PATCH] iommu/vt-d: Exclude known RMRRs from reserved
> ranges
> 
> > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > Sent: Thursday, June 7, 2018 11:02 PM
> >
> > On Wed, 6 Jun 2018 05:29:58 +0000
> > "Tian, Kevin" <kevin.tian@intel.com> wrote:
> >
> > > > 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...
> >
> > Well, I'm a bit stuck then, we have existing IOMMU API users that
> > ignore these ranges and in fact conflict with these ranges blocking us
> > from restricting mappings within these ranges.  My impression is that
> > IOMMU reserved ranges should only be ranges which have some
> > fundamental
> > limitation in the IOMMU, not simply mappings for which firmware has
> > requested an identity mapped range.  The latter should simply be a
> > pre-allocation of the IOVA space, for the cases where we choose to
> > honor the RMRR.  Thanks,
> >
> 
> Then possibly need introduce a different interface for pre-allocation
> scenario, if above definition of reserved ranges is agreed. Currently
> two categories are both called reserved resources, e.g. IOMMU_RESV
> _DIRECT for rmrr and IOMMU_RESV_MSI for MSI...

Sorry, but I just thought of checking on the future plans/directions for solving
this issue(Unfortunately vfio iova list series depends on this).

Please let me know if there is any plans for a v2 soon.

Much appreciated,
Shameer

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

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 1336 bytes --]

On Thu, 2018-06-07 at 09:01 -0600, Alex Williamson wrote:
> 
> > 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...
> 
> Well, I'm a bit stuck then, we have existing IOMMU API users that
> ignore these ranges and in fact conflict with these ranges blocking us
> from restricting mappings within these ranges.  My impression is that
> IOMMU reserved ranges should only be ranges which have some fundamental
> limitation in the IOMMU, not simply mappings for which firmware has
> requested an identity mapped range.  The latter should simply be a
> pre-allocation of the IOVA space, for the cases where we choose to
> honor the RMRR.  Thanks,

Unfortunately, Intel likes to encourage vendors to do batshit insane
things in firmware, at runtime, using DMA.

Perhaps the way to handle this is for the device drivers to cancel any
RMRR for their devices, as they take it over. So USB and graphics
drivers would clear the corresponding RMRRs as they take ownership of
the device... but anything without a driver, or things like the SCSI
host controllers which know that there might be some HP insanity going
on in SMM in parallel with what they're doing, would not (and hence
would not be assignable, etc.).


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5213 bytes --]