* 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(®->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(®->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(®->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
* 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(®->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