All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] iommu/vt-d: Set ISA bridge reserved region as relaxable
@ 2019-12-11 20:28 Alex Williamson
  2019-12-12  1:16 ` Lu Baolu
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Alex Williamson @ 2019-12-11 20:28 UTC (permalink / raw)
  To: joro, iommu, baolu.lu; +Cc: cprt, eauger

Commit d850c2ee5fe2 ("iommu/vt-d: Expose ISA direct mapping region via
iommu_get_resv_regions") created a direct-mapped reserved memory region
in order to replace the static identity mapping of the ISA address
space, where the latter was then removed in commit df4f3c603aeb
("iommu/vt-d: Remove static identity map code").  According to the
history of this code and the Kconfig option surrounding it, this direct
mapping exists for the benefit of legacy ISA drivers that are not
compatible with the DMA API.

In conjuntion with commit 9b77e5c79840 ("vfio/type1: check dma map
request is within a valid iova range") this change introduced a
regression where the vfio IOMMU backend enforces reserved memory regions
per IOMMU group, preventing userspace from creating IOMMU mappings
conflicting with prescribed reserved regions.  A necessary prerequisite
for the vfio change was the introduction of "relaxable" direct mappings
introduced by commit adfd37382090 ("iommu: Introduce
IOMMU_RESV_DIRECT_RELAXABLE reserved memory regions").  These relaxable
direct mappings provide the same identity mapping support in the default
domain, but also indicate that the reservation is software imposed and
may be relaxed under some conditions, such as device assignment.

Convert the ISA bridge direct-mapped reserved region to relaxable to
reflect that the restriction is self imposed and need not be enforced
by drivers such as vfio.

Fixes: d850c2ee5fe2 ("iommu/vt-d: Expose ISA direct mapping region via iommu_get_resv_regions")
Cc: stable@vger.kernel.org # v5.3+
Link: https://lore.kernel.org/linux-iommu/20191211082304.2d4fab45@x1.home
Reported-by: cprt <cprt@protonmail.com>
Tested-by: cprt <cprt@protonmail.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 drivers/iommu/intel-iommu.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 0c8d81f56a30..6eb0dd7489a1 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5737,7 +5737,7 @@ static void intel_iommu_get_resv_regions(struct device *device,
 
 		if ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA) {
 			reg = iommu_alloc_resv_region(0, 1UL << 24, 0,
-						      IOMMU_RESV_DIRECT);
+						   IOMMU_RESV_DIRECT_RELAXABLE);
 			if (reg)
 				list_add_tail(&reg->list, head);
 		}

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] iommu/vt-d: Set ISA bridge reserved region as relaxable
  2019-12-11 20:28 [PATCH] iommu/vt-d: Set ISA bridge reserved region as relaxable Alex Williamson
@ 2019-12-12  1:16 ` Lu Baolu
  2019-12-12  8:49 ` Auger Eric
  2019-12-13  5:41 ` Jerry Snitselaar
  2 siblings, 0 replies; 6+ messages in thread
From: Lu Baolu @ 2019-12-12  1:16 UTC (permalink / raw)
  To: Alex Williamson, joro, iommu; +Cc: cprt, eauger

Hi,

On 12/12/19 4:28 AM, Alex Williamson wrote:
> Commit d850c2ee5fe2 ("iommu/vt-d: Expose ISA direct mapping region via
> iommu_get_resv_regions") created a direct-mapped reserved memory region
> in order to replace the static identity mapping of the ISA address
> space, where the latter was then removed in commit df4f3c603aeb
> ("iommu/vt-d: Remove static identity map code").  According to the
> history of this code and the Kconfig option surrounding it, this direct
> mapping exists for the benefit of legacy ISA drivers that are not
> compatible with the DMA API.
> 
> In conjuntion with commit 9b77e5c79840 ("vfio/type1: check dma map
> request is within a valid iova range") this change introduced a
> regression where the vfio IOMMU backend enforces reserved memory regions
> per IOMMU group, preventing userspace from creating IOMMU mappings
> conflicting with prescribed reserved regions.  A necessary prerequisite
> for the vfio change was the introduction of "relaxable" direct mappings
> introduced by commit adfd37382090 ("iommu: Introduce
> IOMMU_RESV_DIRECT_RELAXABLE reserved memory regions").  These relaxable
> direct mappings provide the same identity mapping support in the default
> domain, but also indicate that the reservation is software imposed and
> may be relaxed under some conditions, such as device assignment.
> 
> Convert the ISA bridge direct-mapped reserved region to relaxable to
> reflect that the restriction is self imposed and need not be enforced
> by drivers such as vfio.
> 
> Fixes: d850c2ee5fe2 ("iommu/vt-d: Expose ISA direct mapping region via iommu_get_resv_regions")
> Cc: stable@vger.kernel.org # v5.3+
> Link: https://lore.kernel.org/linux-iommu/20191211082304.2d4fab45@x1.home
> Reported-by: cprt <cprt@protonmail.com>
> Tested-by: cprt <cprt@protonmail.com>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>

This fix looks reasonable to me.

Acked-by: Lu Baolu <baolu.lu@linux.intel.com>

Best regards,
baolu

> ---
>   drivers/iommu/intel-iommu.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 0c8d81f56a30..6eb0dd7489a1 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -5737,7 +5737,7 @@ static void intel_iommu_get_resv_regions(struct device *device,
>   
>   		if ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA) {
>   			reg = iommu_alloc_resv_region(0, 1UL << 24, 0,
> -						      IOMMU_RESV_DIRECT);
> +						   IOMMU_RESV_DIRECT_RELAXABLE);
>   			if (reg)
>   				list_add_tail(&reg->list, head);
>   		}
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] iommu/vt-d: Set ISA bridge reserved region as relaxable
  2019-12-11 20:28 [PATCH] iommu/vt-d: Set ISA bridge reserved region as relaxable Alex Williamson
  2019-12-12  1:16 ` Lu Baolu
@ 2019-12-12  8:49 ` Auger Eric
  2019-12-12 19:27   ` Alex Williamson
  2019-12-13  5:41 ` Jerry Snitselaar
  2 siblings, 1 reply; 6+ messages in thread
From: Auger Eric @ 2019-12-12  8:49 UTC (permalink / raw)
  To: Alex Williamson, joro, iommu, baolu.lu; +Cc: cprt, eauger

Hi Alex,

On 12/11/19 9:28 PM, Alex Williamson wrote:
> Commit d850c2ee5fe2 ("iommu/vt-d: Expose ISA direct mapping region via
> iommu_get_resv_regions") created a direct-mapped reserved memory region
> in order to replace the static identity mapping of the ISA address
> space, where the latter was then removed in commit df4f3c603aeb
> ("iommu/vt-d: Remove static identity map code").  According to the
> history of this code and the Kconfig option surrounding it, this direct
> mapping exists for the benefit of legacy ISA drivers that are not
> compatible with the DMA API.
> 
> In conjuntion with commit 9b77e5c79840 ("vfio/type1: check dma map
> request is within a valid iova range") this change introduced a
> regression where the vfio IOMMU backend enforces reserved memory regions
> per IOMMU group, preventing userspace from creating IOMMU mappings
> conflicting with prescribed reserved regions.  A necessary prerequisite
> for the vfio change was the introduction of "relaxable" direct mappings
> introduced by commit adfd37382090 ("iommu: Introduce
> IOMMU_RESV_DIRECT_RELAXABLE reserved memory regions").  These relaxable
> direct mappings provide the same identity mapping support in the default
> domain, but also indicate that the reservation is software imposed and
> may be relaxed under some conditions, such as device assignment.
> 
> Convert the ISA bridge direct-mapped reserved region to relaxable to
> reflect that the restriction is self imposed and need not be enforced
> by drivers such as vfio.
> 
> Fixes: d850c2ee5fe2 ("iommu/vt-d: Expose ISA direct mapping region via iommu_get_resv_regions")
Maybe it is rather a fix of my patch, below, since above patch landed
upstream before the IOMMU_RESV_DIRECT_RELAXABLE availability.

Fixes: 1c5c59fbad20 ("iommu/vt-d: Differentiate relaxable and non
relaxable RMRRs")
> Cc: stable@vger.kernel.org # v5.3+
> Link: https://lore.kernel.org/linux-iommu/20191211082304.2d4fab45@x1.home
> Reported-by: cprt <cprt@protonmail.com>
> Tested-by: cprt <cprt@protonmail.com>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Besides
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric
> ---
>  drivers/iommu/intel-iommu.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 0c8d81f56a30..6eb0dd7489a1 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -5737,7 +5737,7 @@ static void intel_iommu_get_resv_regions(struct device *device,
>  
>  		if ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA) {
>  			reg = iommu_alloc_resv_region(0, 1UL << 24, 0,
> -						      IOMMU_RESV_DIRECT);
> +						   IOMMU_RESV_DIRECT_RELAXABLE);
>  			if (reg)
>  				list_add_tail(&reg->list, head);
>  		}
> 
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
> 

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] iommu/vt-d: Set ISA bridge reserved region as relaxable
  2019-12-12  8:49 ` Auger Eric
@ 2019-12-12 19:27   ` Alex Williamson
  2019-12-17 10:22     ` Joerg Roedel
  0 siblings, 1 reply; 6+ messages in thread
From: Alex Williamson @ 2019-12-12 19:27 UTC (permalink / raw)
  To: Auger Eric; +Cc: eauger, cprt, iommu

On Thu, 12 Dec 2019 09:49:37 +0100
Auger Eric <eric.auger@redhat.com> wrote:

> Hi Alex,
> 
> On 12/11/19 9:28 PM, Alex Williamson wrote:
> > Commit d850c2ee5fe2 ("iommu/vt-d: Expose ISA direct mapping region via
> > iommu_get_resv_regions") created a direct-mapped reserved memory region
> > in order to replace the static identity mapping of the ISA address
> > space, where the latter was then removed in commit df4f3c603aeb
> > ("iommu/vt-d: Remove static identity map code").  According to the
> > history of this code and the Kconfig option surrounding it, this direct
> > mapping exists for the benefit of legacy ISA drivers that are not
> > compatible with the DMA API.
> > 
> > In conjuntion with commit 9b77e5c79840 ("vfio/type1: check dma map
> > request is within a valid iova range") this change introduced a
> > regression where the vfio IOMMU backend enforces reserved memory regions
> > per IOMMU group, preventing userspace from creating IOMMU mappings
> > conflicting with prescribed reserved regions.  A necessary prerequisite
> > for the vfio change was the introduction of "relaxable" direct mappings
> > introduced by commit adfd37382090 ("iommu: Introduce
> > IOMMU_RESV_DIRECT_RELAXABLE reserved memory regions").  These relaxable
> > direct mappings provide the same identity mapping support in the default
> > domain, but also indicate that the reservation is software imposed and
> > may be relaxed under some conditions, such as device assignment.
> > 
> > Convert the ISA bridge direct-mapped reserved region to relaxable to
> > reflect that the restriction is self imposed and need not be enforced
> > by drivers such as vfio.
> > 
> > Fixes: d850c2ee5fe2 ("iommu/vt-d: Expose ISA direct mapping region via iommu_get_resv_regions")  

> Maybe it is rather a fix of my patch, below, since above patch landed
> upstream before the IOMMU_RESV_DIRECT_RELAXABLE availability.
> 
> Fixes: 1c5c59fbad20 ("iommu/vt-d: Differentiate relaxable and non
> relaxable RMRRs")

Sure, if you remember the ordering between these then that might be the
better option.  I checked that they both entered the kernel for
v5.3-rc1 but didn't dig deeper than that.

Joerg, if you'd like a respin for that change let me know, otherwise
just swap my Fixes tag for the one Eric suggests.

> > Cc: stable@vger.kernel.org # v5.3+
> > Link: https://lore.kernel.org/linux-iommu/20191211082304.2d4fab45@x1.home
> > Reported-by: cprt <cprt@protonmail.com>
> > Tested-by: cprt <cprt@protonmail.com>
> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>  
> Besides
> Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks!

Alex

> > ---
> >  drivers/iommu/intel-iommu.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> > index 0c8d81f56a30..6eb0dd7489a1 100644
> > --- a/drivers/iommu/intel-iommu.c
> > +++ b/drivers/iommu/intel-iommu.c
> > @@ -5737,7 +5737,7 @@ static void intel_iommu_get_resv_regions(struct device *device,
> >  
> >  		if ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA) {
> >  			reg = iommu_alloc_resv_region(0, 1UL << 24, 0,
> > -						      IOMMU_RESV_DIRECT);
> > +						   IOMMU_RESV_DIRECT_RELAXABLE);
> >  			if (reg)
> >  				list_add_tail(&reg->list, head);
> >  		}
> > 
> > _______________________________________________
> > iommu mailing list
> > iommu@lists.linux-foundation.org
> > https://lists.linuxfoundation.org/mailman/listinfo/iommu
> >   

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] iommu/vt-d: Set ISA bridge reserved region as relaxable
  2019-12-11 20:28 [PATCH] iommu/vt-d: Set ISA bridge reserved region as relaxable Alex Williamson
  2019-12-12  1:16 ` Lu Baolu
  2019-12-12  8:49 ` Auger Eric
@ 2019-12-13  5:41 ` Jerry Snitselaar
  2 siblings, 0 replies; 6+ messages in thread
From: Jerry Snitselaar @ 2019-12-13  5:41 UTC (permalink / raw)
  To: Alex Williamson; +Cc: eauger, cprt, iommu

On Wed Dec 11 19, Alex Williamson wrote:
>Commit d850c2ee5fe2 ("iommu/vt-d: Expose ISA direct mapping region via
>iommu_get_resv_regions") created a direct-mapped reserved memory region
>in order to replace the static identity mapping of the ISA address
>space, where the latter was then removed in commit df4f3c603aeb
>("iommu/vt-d: Remove static identity map code").  According to the
>history of this code and the Kconfig option surrounding it, this direct
>mapping exists for the benefit of legacy ISA drivers that are not
>compatible with the DMA API.
>
>In conjuntion with commit 9b77e5c79840 ("vfio/type1: check dma map
>request is within a valid iova range") this change introduced a
>regression where the vfio IOMMU backend enforces reserved memory regions
>per IOMMU group, preventing userspace from creating IOMMU mappings
>conflicting with prescribed reserved regions.  A necessary prerequisite
>for the vfio change was the introduction of "relaxable" direct mappings
>introduced by commit adfd37382090 ("iommu: Introduce
>IOMMU_RESV_DIRECT_RELAXABLE reserved memory regions").  These relaxable
>direct mappings provide the same identity mapping support in the default
>domain, but also indicate that the reservation is software imposed and
>may be relaxed under some conditions, such as device assignment.
>
>Convert the ISA bridge direct-mapped reserved region to relaxable to
>reflect that the restriction is self imposed and need not be enforced
>by drivers such as vfio.
>
>Fixes: d850c2ee5fe2 ("iommu/vt-d: Expose ISA direct mapping region via iommu_get_resv_regions")
>Cc: stable@vger.kernel.org # v5.3+
>Link: https://lore.kernel.org/linux-iommu/20191211082304.2d4fab45@x1.home
>Reported-by: cprt <cprt@protonmail.com>
>Tested-by: cprt <cprt@protonmail.com>
>Signed-off-by: Alex Williamson <alex.williamson@redhat.com>

Tested-by: Jerry Snitselaar <jsnitsel@redhat.com>
Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>

>---
> drivers/iommu/intel-iommu.c |    2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
>index 0c8d81f56a30..6eb0dd7489a1 100644
>--- a/drivers/iommu/intel-iommu.c
>+++ b/drivers/iommu/intel-iommu.c
>@@ -5737,7 +5737,7 @@ static void intel_iommu_get_resv_regions(struct device *device,
>
> 		if ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA) {
> 			reg = iommu_alloc_resv_region(0, 1UL << 24, 0,
>-						      IOMMU_RESV_DIRECT);
>+						   IOMMU_RESV_DIRECT_RELAXABLE);
> 			if (reg)
> 				list_add_tail(&reg->list, head);
> 		}
>
>_______________________________________________
>iommu mailing list
>iommu@lists.linux-foundation.org
>https://lists.linuxfoundation.org/mailman/listinfo/iommu
>

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] iommu/vt-d: Set ISA bridge reserved region as relaxable
  2019-12-12 19:27   ` Alex Williamson
@ 2019-12-17 10:22     ` Joerg Roedel
  0 siblings, 0 replies; 6+ messages in thread
From: Joerg Roedel @ 2019-12-17 10:22 UTC (permalink / raw)
  To: Alex Williamson; +Cc: eauger, iommu, cprt

Hi Alex,

On Thu, Dec 12, 2019 at 12:27:11PM -0700, Alex Williamson wrote:
> Sure, if you remember the ordering between these then that might be the
> better option.  I checked that they both entered the kernel for
> v5.3-rc1 but didn't dig deeper than that.
> 
> Joerg, if you'd like a respin for that change let me know, otherwise
> just swap my Fixes tag for the one Eric suggests.

Swapped the Fixes tag and applied for v5.5, thanks.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-12-17 10:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-11 20:28 [PATCH] iommu/vt-d: Set ISA bridge reserved region as relaxable Alex Williamson
2019-12-12  1:16 ` Lu Baolu
2019-12-12  8:49 ` Auger Eric
2019-12-12 19:27   ` Alex Williamson
2019-12-17 10:22     ` Joerg Roedel
2019-12-13  5:41 ` Jerry Snitselaar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.