Re: [PATCH v2] iommu/mediatek: fix NULL pointer dereference when printing dev_name
Hi Yong,
>On Mon, 2022-04-25 at 11:03 +0100, Robin Murphy wrote:
>> On 2022-04-25 09:24, Miles Chen via iommu wrote:
>> > When larbdev is NULL (in the case I hit, the node is incorrectly
>> > set
>> > iommus = < NUM>), it will cause device_link_add() fail and
>>
>> Until the MT8195 infra MMU support lands, is there ever a case where
>> it's actually valid for larbdev to be NULL? If not, I think it would
>> be
>> a lot clearer to explicitly fail the probe here, rather than
>> silently
>> continue and risk fatal errors, hangs, or other weird behaviour if
>> there's no guarantee that the correct LARB is powered up (plus then
>> the
>> release callbacks wouldn't need to worry about it either).
>
>Yes. It should return fail for this case. This issue only happens when
>the dts parameters doesn't respect the definition from the binding[1].
>
>Locally Miles tested with a internal definition that have not send
>upstream to get this KE. In this case, I'm not sure if we should
>request the user use the right ID in dts. Anyway I have no objection to
>modifying this, then something like this: (Avoid invalid input from
>dtb)
>
>@@ -790,6 +790,8 @@ static struct iommu_device
>*mtk_iommu_probe_device(struct device *dev)
>* All the ports in each a device should be in the same larbs.
>*/
> larbid = MTK_M4U_TO_LARB(fwspec->ids[0]);
>+ if (larbid >= MTK_LARB_NR_MAX)
>+ return ERR_PTR(-EINVAL);
> for (i = 1; i < fwspec->num_ids; i++) {
> larbidx = MTK_M4U_TO_LARB(fwspec->ids[i]);
> if (larbid != larbidx) {
>@@ -799,6 +801,8 @@ static struct iommu_device
>*mtk_iommu_probe_device(struct device *dev)
> }
> }
> larbdev = data->larb_imu[larbid].dev;
>+ if (!larbdev)
>+ return ERR_PTR(-EINVAL);
> link = device_link_add(dev, larbdev,
> DL_FLAG_PM_RUNTIME | DL_FLAG_STATELESS);
> if (!link)
Thanks for guilding me, I will put this in patch v2.
Thanks,
Miles
>
>
>[1]
>https://elixir.bootlin.com/linux/v5.18-rc1/source/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml#L116
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/mediatek: fix NULL pointer dereference when printing dev_name
hi Robin,
>> -link = device_link_add(dev, larbdev,
>> - DL_FLAG_PM_RUNTIME | DL_FLAG_STATELESS);
>> -if (!link)
>> -dev_err(dev, "Unable to link %s\n", dev_name(larbdev));
>> +if (larbdev) {
>
>Until the MT8195 infra MMU support lands, is there ever a case where
>it's actually valid for larbdev to be NULL? If not, I think it would be
>a lot clearer to explicitly fail the probe here, rather than silently
>continue and risk fatal errors, hangs, or other weird behaviour if
>there's no guarantee that the correct LARB is powered up (plus then the
>release callbacks wouldn't need to worry about it either).
Thanks, I will do probe fail in patch v3 and remove the release modification.
thanks,
Miles
>
>Robin.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 05/11] iommu/sva: Assign a PASID to mm on PASID allocation and free it on mm exit
On 4/26/22 09:48, Jean-Philippe Brucker wrote: > On Tue, Apr 26, 2022 at 08:27:00AM -0700, Dave Hansen wrote: >> On 4/25/22 09:40, Jean-Philippe Brucker wrote: >>> The problem is that we'd have to request the device driver to stop DMA >>> before we can destroy the context and free the PASID. We did consider >>> doing this in the release() MMU notifier, but there were concerns about >>> blocking mmput() for too long (for example >>> https://lore.kernel.org/linux-iommu/4d68da96-0ad5-b412-5987-2f7a6aa79...@amd.com/ >>> though I think there was a more recent discussion). We also need to drain >>> the PRI and fault queues to get rid of all references to that PASID. >> Is the concern truly about blocking mmput() itself? Or, is it about >> releasing the resources associated with the mm? > The latter I think, this one was about releasing pages as fast as possible > if the process is picked by the OOM killer. We're tying the PASID to the life of the mm itself, not the mm's address space. That means the PASID should be tied to mmgrab()/mmdrop()/mm->mm_count. The address space is what the OOM killer is after. That gets refcounted with mmget()/mmput()/mm->mm_users. The OOM killer is satiated by the page freeing done in __mmput()->exit_mmap(). Also, all the VMAs should be gone after exit_mmap(). So, even if vma->vm_file was holding a reference to a device driver, that reference should be gone by the time __mmdrop() is actually freeing the PASID. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] swiotlb-xen: fix DMA_ATTR_NO_KERNEL_MAPPING on arm
On Sat, 23 Apr 2022, Christoph Hellwig wrote:
> swiotlb-xen uses very different ways to allocate coherent memory on x86
> vs arm. On the former it allocates memory from the page allocator, while
> on the later it reuses the dma-direct allocator the handles the
> complexities of non-coherent DMA on arm platforms.
>
> Unfortunately the complexities of trying to deal with the two cases in
> the swiotlb-xen.c code lead to a bug in the handling of
> DMA_ATTR_NO_KERNEL_MAPPING on arm. With the DMA_ATTR_NO_KERNEL_MAPPING
> flag the coherent memory allocator does not actually allocate coherent
> memory, but just a DMA handle for some memory that is DMA addressable
> by the device, but which does not have to have a kernel mapping. Thus
> dereferencing the return value will lead to kernel crashed and memory
> corruption.
>
> Fix this by using the dma-direct allocator directly for arm, which works
> perfectly fine because on arm swiotlb-xen is only used when the domain is
> 1:1 mapped, and then simplifying the remaining code to only cater for the
> x86 case with DMA coherent device.
>
> Reported-by: Rahul Singh
> Signed-off-by: Christoph Hellwig
Reviewed-by: Stefano Stabellini
> ---
> arch/arm/include/asm/xen/page-coherent.h | 2 -
> arch/arm/xen/mm.c | 17
> arch/arm64/include/asm/xen/page-coherent.h | 2 -
> arch/x86/include/asm/xen/page-coherent.h | 24 -
> arch/x86/include/asm/xen/swiotlb-xen.h | 5 +
> drivers/xen/swiotlb-xen.c | 106 -
> include/xen/arm/page-coherent.h| 20
> include/xen/xen-ops.h | 7 --
> 8 files changed, 45 insertions(+), 138 deletions(-)
> delete mode 100644 arch/arm/include/asm/xen/page-coherent.h
> delete mode 100644 arch/arm64/include/asm/xen/page-coherent.h
> delete mode 100644 arch/x86/include/asm/xen/page-coherent.h
> delete mode 100644 include/xen/arm/page-coherent.h
>
> diff --git a/arch/arm/include/asm/xen/page-coherent.h
> b/arch/arm/include/asm/xen/page-coherent.h
> deleted file mode 100644
> index 27e984977402b..0
> --- a/arch/arm/include/asm/xen/page-coherent.h
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -#include
> diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
> index a7e54a087b802..6e603e5fdebb1 100644
> --- a/arch/arm/xen/mm.c
> +++ b/arch/arm/xen/mm.c
> @@ -118,23 +118,6 @@ bool xen_arch_need_swiotlb(struct device *dev,
> !dev_is_dma_coherent(dev));
> }
>
> -int xen_create_contiguous_region(phys_addr_t pstart, unsigned int order,
> - unsigned int address_bits,
> - dma_addr_t *dma_handle)
> -{
> - if (!xen_initial_domain())
> - return -EINVAL;
> -
> - /* we assume that dom0 is mapped 1:1 for now */
> - *dma_handle = pstart;
> - return 0;
> -}
> -
> -void xen_destroy_contiguous_region(phys_addr_t pstart, unsigned int order)
> -{
> - return;
> -}
> -
> static int __init xen_mm_init(void)
> {
> struct gnttab_cache_flush cflush;
> diff --git a/arch/arm64/include/asm/xen/page-coherent.h
> b/arch/arm64/include/asm/xen/page-coherent.h
> deleted file mode 100644
> index 27e984977402b..0
> --- a/arch/arm64/include/asm/xen/page-coherent.h
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -#include
> diff --git a/arch/x86/include/asm/xen/page-coherent.h
> b/arch/x86/include/asm/xen/page-coherent.h
> deleted file mode 100644
> index 63cd41b2e17ac..0
> --- a/arch/x86/include/asm/xen/page-coherent.h
> +++ /dev/null
> @@ -1,24 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -#ifndef _ASM_X86_XEN_PAGE_COHERENT_H
> -#define _ASM_X86_XEN_PAGE_COHERENT_H
> -
> -#include
> -#include
> -
> -static inline void *xen_alloc_coherent_pages(struct device *hwdev, size_t
> size,
> - dma_addr_t *dma_handle, gfp_t flags,
> - unsigned long attrs)
> -{
> - void *vstart = (void*)__get_free_pages(flags, get_order(size));
> - *dma_handle = virt_to_phys(vstart);
> - return vstart;
> -}
> -
> -static inline void xen_free_coherent_pages(struct device *hwdev, size_t size,
> - void *cpu_addr, dma_addr_t dma_handle,
> - unsigned long attrs)
> -{
> - free_pages((unsigned long) cpu_addr, get_order(size));
> -}
> -
> -#endif /* _ASM_X86_XEN_PAGE_COHERENT_H */
> diff --git a/arch/x86/include/asm/xen/swiotlb-xen.h
> b/arch/x86/include/asm/xen/swiotlb-xen.h
> index 66b4ddde77430..558821387808e 100644
> --- a/arch/x86/include/asm/xen/swiotlb-xen.h
> +++ b/arch/x86/include/asm/xen/swiotlb-xen.h
> @@ -10,4 +10,9 @@ extern int pci_xen_swiotlb_init_late(void);
> static inline int pci_xen_swiotlb_init_late(void) { return -ENXIO; }
> #endif
>
> +int xen_create_contiguous_region(phys_addr_t pstart, unsigned int order,
> + unsigned int address_bits,
> +
Re: [PATCH] iommu/dart: check return value after calling platform_get_resource()
> On 25. Apr 2022, at 10:56, Yang Yingliang wrote:
>
> It will cause null-ptr-deref in resource_size(), if platform_get_resource()
> returns NULL, move calling resource_size() after devm_ioremap_resource() that
> will check 'res' to avoid null-ptr-deref.
> And use devm_platform_get_and_ioremap_resource() to simplify code.
>
> Fixes: 46d1fb072e76 ("iommu/dart: Add DART iommu driver")
> Signed-off-by: Yang Yingliang
Reviewed-by: Sven Peter
Thanks,
Sven
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v7 2/2] PCI: Rename pci_dev->untrusted to pci_dev->untrusted_dma
Rename the field to make it more clear, that the device can execute DMA
attacks on the system, and thus the system may need protection from
such attacks from this device.
No functional change intended.
Signed-off-by: Rajat Jain
Reviewed-by: Mika Westerberg
Reviewed-by: Lu Baolu
Acked-by: Rafael J. Wysocki
---
v7: Added Lu Baolu's "Reviewed by" tag.
v6: No change in this patch, rebased on top of changes in other patch.
v5: Use "untrusted_dma" as property name, based on feedback.
Reorder the patches in the series.
v4: Initial version, created based on comments on other patch
drivers/iommu/dma-iommu.c | 6 +++---
drivers/iommu/intel/iommu.c | 2 +-
drivers/iommu/iommu.c | 2 +-
drivers/pci/ats.c | 2 +-
drivers/pci/pci-acpi.c | 2 +-
drivers/pci/pci.c | 2 +-
drivers/pci/probe.c | 8
drivers/pci/quirks.c| 2 +-
include/linux/pci.h | 5 +++--
9 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 09f6e1c0f9c0..aeee4be7614d 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -497,14 +497,14 @@ static int iova_reserve_iommu_regions(struct device *dev,
return ret;
}
-static bool dev_is_untrusted(struct device *dev)
+static bool dev_has_untrusted_dma(struct device *dev)
{
- return dev_is_pci(dev) && to_pci_dev(dev)->untrusted;
+ return dev_is_pci(dev) && to_pci_dev(dev)->untrusted_dma;
}
static bool dev_use_swiotlb(struct device *dev)
{
- return IS_ENABLED(CONFIG_SWIOTLB) && dev_is_untrusted(dev);
+ return IS_ENABLED(CONFIG_SWIOTLB) && dev_has_untrusted_dma(dev);
}
/**
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index df5c62ecf942..b88f47391140 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4843,7 +4843,7 @@ static bool intel_iommu_is_attach_deferred(struct device
*dev)
*/
static bool risky_device(struct pci_dev *pdev)
{
- if (pdev->untrusted) {
+ if (pdev->untrusted_dma) {
pci_info(pdev,
"Skipping IOMMU quirk for dev [%04X:%04X] on untrusted
PCI link\n",
pdev->vendor, pdev->device);
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index f2c45b85b9fc..d8d3133e2947 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1525,7 +1525,7 @@ static int iommu_get_def_domain_type(struct device *dev)
{
const struct iommu_ops *ops = dev_iommu_ops(dev);
- if (dev_is_pci(dev) && to_pci_dev(dev)->untrusted)
+ if (dev_is_pci(dev) && to_pci_dev(dev)->untrusted_dma)
return IOMMU_DOMAIN_DMA;
if (ops->def_domain_type)
diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index c967ad6e2626..477c16ba9341 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -42,7 +42,7 @@ bool pci_ats_supported(struct pci_dev *dev)
if (!dev->ats_cap)
return false;
- return (dev->untrusted == 0);
+ return (dev->untrusted_dma == 0);
}
EXPORT_SYMBOL_GPL(pci_ats_supported);
diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index 8cb4725d41fa..bf04e873c96a 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -1396,7 +1396,7 @@ void pci_acpi_setup(struct device *dev, struct
acpi_device *adev)
pci_acpi_optimize_delay(pci_dev, adev->handle);
pci_acpi_set_external_facing(pci_dev);
- pci_dev->untrusted |= pci_dev_has_dma_property(pci_dev);
+ pci_dev->untrusted_dma |= pci_dev_has_dma_property(pci_dev);
pci_acpi_add_edr_notifier(pci_dev);
pci_acpi_add_pm_notifier(adev, pci_dev);
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 9ecce435fb3f..1fb0eb8646c8 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -958,7 +958,7 @@ static void pci_std_enable_acs(struct pci_dev *dev)
ctrl |= (cap & PCI_ACS_UF);
/* Enable Translation Blocking for external devices and noats */
- if (pci_ats_disabled() || dev->external_facing || dev->untrusted)
+ if (pci_ats_disabled() || dev->external_facing || dev->untrusted_dma)
ctrl |= (cap & PCI_ACS_TB);
pci_write_config_word(dev, pos + PCI_ACS_CTRL, ctrl);
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 17a969942d37..d2a9b26fcede 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1587,7 +1587,7 @@ static void set_pcie_thunderbolt(struct pci_dev *dev)
dev->is_thunderbolt = 1;
}
-static void set_pcie_untrusted(struct pci_dev *dev)
+static void pci_set_untrusted_dma(struct pci_dev *dev)
{
struct pci_dev *parent;
@@ -1596,8 +1596,8 @@ static void set_pcie_untrusted(struct pci_dev *dev)
* untrusted as well.
*/
parent = pci_upstream_bridge(dev);
- if (parent && (parent->untrusted || parent->external_facing))
- dev->untrusted = true;
+
[PATCH v7 1/2] PCI/ACPI: Support Microsoft's "DmaProperty"
The "DmaProperty" is supported and currently documented and used by
Microsoft [link 1 below], to flag internal PCIe root ports that need
DMA protection [link 2 below]. We have discussed with them and reached
a common understanding that they shall change their MSDN documentation
to say that the same property can be used to protect any PCI device,
and not just internal PCIe root ports (since there is no point
introducing yet another property for arbitrary PCI devices). This helps
with security from internal devices that offer an attack surface for
DMA attacks (e.g. internal network devices).
Support DmaProperty to mark DMA from a PCI device as untrusted.
Link: [1]
https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports#identifying-internal-pcie-ports-accessible-to-users-and-requiring-dma-protection
Link: [2]
https://docs.microsoft.com/en-us/windows/security/information-protection/kernel-dma-protection-for-thunderbolt
Signed-off-by: Rajat Jain
Reviewed-by: Mika Westerberg
Acked-by: Rafael J. Wysocki
---
v7: * Update the comment, based on feedback.
v6: * Take care of Bjorn's comments:
- Update the commit log
- Rename to pci_dev_has_dma_property()
- Use acpi_dev_get_property()
v5: * Reorder the patches in the series
v4: * Add the GUID.
* Update the comment and commitlog.
v3: * Use Microsoft's documented property "DmaProperty"
* Resctrict to ACPI only
drivers/acpi/property.c | 3 +++
drivers/pci/pci-acpi.c | 22 ++
2 files changed, 25 insertions(+)
diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 12bbfe833609..bafe35c301ac 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -48,6 +48,9 @@ static const guid_t prp_guids[] = {
/* Storage device needs D3 GUID: 5025030f-842f-4ab4-a561-99a5189762d0 */
GUID_INIT(0x5025030f, 0x842f, 0x4ab4,
0xa5, 0x61, 0x99, 0xa5, 0x18, 0x97, 0x62, 0xd0),
+ /* DmaProperty for PCI devices GUID:
70d24161-6dd5-4c9e-8070-705531292865 */
+ GUID_INIT(0x70d24161, 0x6dd5, 0x4c9e,
+ 0x80, 0x70, 0x70, 0x55, 0x31, 0x29, 0x28, 0x65),
};
/* ACPI _DSD data subnodes GUID: dbb8e3e6-5886-4ba6-8795-1319f52a966b */
diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index 3ae435beaf0a..8cb4725d41fa 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -1369,12 +1369,34 @@ static void pci_acpi_set_external_facing(struct pci_dev
*dev)
dev->external_facing = 1;
}
+static int pci_dev_has_dma_property(struct pci_dev *dev)
+{
+ struct acpi_device *adev;
+ const union acpi_object *obj;
+
+ adev = ACPI_COMPANION(>dev);
+ if (!adev)
+ return 0;
+
+ /*
+* Property used by Microsoft Windows to enforce IOMMU DMA
+* protection from any device, that the system may not fully trust;
+* we'll honour it the same way.
+*/
+ if (!acpi_dev_get_property(adev, "DmaProperty", ACPI_TYPE_INTEGER,
+ ) && obj->integer.value == 1)
+ return 1;
+
+ return 0;
+}
+
void pci_acpi_setup(struct device *dev, struct acpi_device *adev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
pci_acpi_optimize_delay(pci_dev, adev->handle);
pci_acpi_set_external_facing(pci_dev);
+ pci_dev->untrusted |= pci_dev_has_dma_property(pci_dev);
pci_acpi_add_edr_notifier(pci_dev);
pci_acpi_add_pm_notifier(adev, pci_dev);
--
2.36.0.rc2.479.g8af0fa9b8e-goog
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v6 1/2] PCI/ACPI: Support Microsoft's "DmaProperty"
On Tue, Apr 26, 2022 at 4:15 AM Robin Murphy wrote:
>
> On 2022-04-26 01:06, Rajat Jain via iommu wrote:
> > The "DmaProperty" is supported and currently documented and used by
> > Microsoft [link 1 below], to flag internal PCIe root ports that need
> > DMA protection [link 2 below]. We have discussed with them and reached
> > a common understanding that they shall change their MSDN documentation
> > to say that the same property can be used to protect any PCI device,
> > and not just internal PCIe root ports (since there is no point
> > introducing yet another property for arbitrary PCI devices). This helps
> > with security from internal devices that offer an attack surface for
> > DMA attacks (e.g. internal network devices).
> >
> > Support DmaProperty to mark DMA from a PCI device as untrusted.
> >
> > Link: [1]
> > https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports#identifying-internal-pcie-ports-accessible-to-users-and-requiring-dma-protection
> > Link: [2]
> > https://docs.microsoft.com/en-us/windows/security/information-protection/kernel-dma-protection-for-thunderbolt
> > Signed-off-by: Rajat Jain
> > Reviewed-by: Mika Westerberg
> > Acked-by: Rafael J. Wysocki
> > ---
> > v6: * Take care of Bjorn's comments:
> > - Update the commit log
> > - Rename to pci_dev_has_dma_property()
> > - Use acpi_dev_get_property()
> > v5: * Reorder the patches in the series
> > v4: * Add the GUID.
> > * Update the comment and commitlog.
> > v3: * Use Microsoft's documented property "DmaProperty"
> > * Resctrict to ACPI only
> >
> > drivers/acpi/property.c | 3 +++
> > drivers/pci/pci-acpi.c | 21 +
> > 2 files changed, 24 insertions(+)
> >
> > diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> > index 12bbfe833609..bafe35c301ac 100644
> > --- a/drivers/acpi/property.c
> > +++ b/drivers/acpi/property.c
> > @@ -48,6 +48,9 @@ static const guid_t prp_guids[] = {
> > /* Storage device needs D3 GUID: 5025030f-842f-4ab4-a561-99a5189762d0
> > */
> > GUID_INIT(0x5025030f, 0x842f, 0x4ab4,
> > 0xa5, 0x61, 0x99, 0xa5, 0x18, 0x97, 0x62, 0xd0),
> > + /* DmaProperty for PCI devices GUID:
> > 70d24161-6dd5-4c9e-8070-705531292865 */
> > + GUID_INIT(0x70d24161, 0x6dd5, 0x4c9e,
> > + 0x80, 0x70, 0x70, 0x55, 0x31, 0x29, 0x28, 0x65),
> > };
> >
> > /* ACPI _DSD data subnodes GUID: dbb8e3e6-5886-4ba6-8795-1319f52a966b */
> > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> > index 3ae435beaf0a..d7c6ba48486f 100644
> > --- a/drivers/pci/pci-acpi.c
> > +++ b/drivers/pci/pci-acpi.c
> > @@ -1369,12 +1369,33 @@ static void pci_acpi_set_external_facing(struct
> > pci_dev *dev)
> > dev->external_facing = 1;
> > }
> >
> > +static int pci_dev_has_dma_property(struct pci_dev *dev)
> > +{
> > + struct acpi_device *adev;
> > + const union acpi_object *obj;
> > +
> > + adev = ACPI_COMPANION(>dev);
> > + if (!adev)
> > + return 0;
> > +
> > + /*
> > + * Property also used by Microsoft Windows for same purpose,
> > + * (to implement DMA protection from a device, using the IOMMU).
>
> Nit: there is no context for "same purpose" here, so this comment is
> more confusing than helpful. Might I suggest a rewording like:
>
> /*
> * Property used by Microsoft Windows to enforce IOMMU DMA
> * protection for any device that the system might not fully
> * trust; we'll honour it the same way.
> */
>
> ?
Sure, will do.
>
> Personally I think it would have been more logical to handle this in
> pci_set_dma_untrusted(), but I see some of those implementation aspects
> have already been discussed, and Bjorn's preference definitely wins over
> mine here :)
Yes, this was discussed. The primary reason is that ACPI properties
for PCI devices are not available at the time pci_set_untrusted_dma()
is called.
Thanks & Best Regards,
Rajat
>
> Thanks,
> Robin.
>
> > + */
> > + if (!acpi_dev_get_property(adev, "DmaProperty", ACPI_TYPE_INTEGER,
> > +) && obj->integer.value == 1)
> > + return 1;
> > +
> > + return 0;
> > +}
> > +
> > void pci_acpi_setup(struct device *dev, struct acpi_device *adev)
> > {
> > struct pci_dev *pci_dev = to_pci_dev(dev);
> >
> > pci_acpi_optimize_delay(pci_dev, adev->handle);
> > pci_acpi_set_external_facing(pci_dev);
> > + pci_dev->untrusted |= pci_dev_has_dma_property(pci_dev); >
> > pci_acpi_add_edr_notifier(pci_dev);
> >
> > pci_acpi_add_pm_notifier(adev, pci_dev);
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 05/11] iommu/sva: Assign a PASID to mm on PASID allocation and free it on mm exit
On Tue, Apr 26, 2022 at 08:27:00AM -0700, Dave Hansen wrote: > On 4/25/22 09:40, Jean-Philippe Brucker wrote: > > The problem is that we'd have to request the device driver to stop DMA > > before we can destroy the context and free the PASID. We did consider > > doing this in the release() MMU notifier, but there were concerns about > > blocking mmput() for too long (for example > > https://lore.kernel.org/linux-iommu/4d68da96-0ad5-b412-5987-2f7a6aa79...@amd.com/ > > though I think there was a more recent discussion). We also need to drain > > the PRI and fault queues to get rid of all references to that PASID. > > Is the concern truly about blocking mmput() itself? Or, is it about > releasing the resources associated with the mm? The latter I think, this one was about releasing pages as fast as possible if the process is picked by the OOM killer. Thanks, Jean ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] Documentation: x86: rework IOMMU documentation
Hi Alex, Thanks for doing this, really helps to catch up the current state. Please see my comments inline. On Fri, 22 Apr 2022 16:06:07 -0400, Alex Deucher wrote: > Add preliminary documentation for AMD IOMMU and combine > with the existing Intel IOMMU documentation and clean > up and modernize some of the existing documentation to > align with the current state of the kernel. > > Signed-off-by: Alex Deucher > --- > > V2: Incorporate feedback from Robin to clarify IOMMU vs DMA engine (e.g., > a device) and document proper DMA API. Also correct the fact that > the AMD IOMMU is not limited to managing PCI devices. > v3: Fix spelling and rework text as suggested by Vasant > v4: Combine Intel and AMD documents into a single document as suggested > by Dave Hansen > v5: Clarify that keywords are related to ACPI, grammatical fixes > v6: Make more stuff common based on feedback from Robin > > Documentation/x86/index.rst | 2 +- > Documentation/x86/intel-iommu.rst | 115 > Documentation/x86/iommu.rst | 143 ++ > 3 files changed, 144 insertions(+), 116 deletions(-) > delete mode 100644 Documentation/x86/intel-iommu.rst > create mode 100644 Documentation/x86/iommu.rst > > diff --git a/Documentation/x86/index.rst b/Documentation/x86/index.rst > index f498f1d36cd3..6f8409fe0674 100644 > --- a/Documentation/x86/index.rst > +++ b/Documentation/x86/index.rst > @@ -21,7 +21,7 @@ x86-specific Documentation > tlb > mtrr > pat > - intel-iommu > + iommu > intel_txt > amd-memory-encryption > pti > diff --git a/Documentation/x86/intel-iommu.rst > b/Documentation/x86/intel-iommu.rst deleted file mode 100644 > index 099f13d51d5f.. > --- a/Documentation/x86/intel-iommu.rst > +++ /dev/null > @@ -1,115 +0,0 @@ > -=== > -Linux IOMMU Support > -=== > - > -The architecture spec can be obtained from the below location. > - > -http://www.intel.com/content/dam/www/public/us/en/documents/product-specifications/vt-directed-io-spec.pdf > - > -This guide gives a quick cheat sheet for some basic understanding. > - > -Some Keywords > - > -- DMAR - DMA remapping > -- DRHD - DMA Remapping Hardware Unit Definition > -- RMRR - Reserved memory Region Reporting Structure > -- ZLR - Zero length reads from PCI devices > -- IOVA - IO Virtual address. > - I feel this combined document only focus on IOVA and DMA APIs, it is considered as legacy DMA after scalable mode is introduced by Intel to support DMA with PASID, shared virtual addressing (SVA). Perhaps, we can also combine ./Documentation/x86/sva.rst With scalable mode, it affects boot messages, fault reporting, etc. I am not saying no to this document, just suggesting. I don't know where AMD is at in terms of PASID support but there are lots of things in common between VT-d and ARM's SMMU in terms of PASID/SVA. Should we broaden the purpose of this document even further? > -Basic stuff > > - > -ACPI enumerates and lists the different DMA engines in the platform, and > -device scope relationships between PCI devices and which DMA engine > controls -them. > - > -What is RMRR? > -- > - > -There are some devices the BIOS controls, for e.g USB devices to perform > -PS2 emulation. The regions of memory used for these devices are marked > -reserved in the e820 map. When we turn on DMA translation, DMA to those > -regions will fail. Hence BIOS uses RMRR to specify these regions along > with -devices that need to access these regions. OS is expected to setup > -unity mappings for these regions for these devices to access these > regions. - > -How is IOVA generated? > --- > - > -Well behaved drivers call pci_map_*() calls before sending command to > device -that needs to perform DMA. Once DMA is completed and mapping is > no longer -required, device performs a pci_unmap_*() calls to unmap the > region. - > -The Intel IOMMU driver allocates a virtual address per domain. Each PCIE > -device has its own domain (hence protection). Devices under p2p bridges > -share the virtual address with all devices under the p2p bridge due to > -transaction id aliasing for p2p bridges. > - > -IOVA generation is pretty generic. We used the same technique as > vmalloc() -but these are not global address spaces, but separate for each > domain. -Different DMA engines may support different number of domains. > - > -We also allocate guard pages with each mapping, so we can attempt to > catch -any overflow that might happen. > - > - > -Graphics Problems? > --- > -If you encounter issues with graphics devices, you can try adding > -option intel_iommu=igfx_off to turn off the integrated graphics engine. > -If this fixes anything, please ensure you file a bug reporting the > problem. - > -Some exceptions to IOVA > > -Interrupt ranges are not address translated, (0xfee0 - 0xfeef).
Re: [PATCH v11 4/9] ACPI/IORT: Add support to retrieve IORT RMR reserved regions
On Fri, Apr 22, 2022 at 05:29:02PM +0100, Shameer Kolothum wrote:
> Parse through the IORT RMR nodes and populate the reserve region list
> corresponding to a given IOMMU and device(optional). Also, go through
> the ID mappings of the RMR node and retrieve all the SIDs associated
> with it.
>
> Signed-off-by: Shameer Kolothum
> ---
> drivers/acpi/arm64/iort.c | 290 ++
> include/linux/iommu.h | 8 ++
> 2 files changed, 298 insertions(+)
Reviewed-by: Lorenzo Pieralisi
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index cd5d1d7823cb..5be6e8ecca38 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -788,6 +788,293 @@ void acpi_configure_pmsi_domain(struct device *dev)
> }
>
> #ifdef CONFIG_IOMMU_API
> +static void iort_rmr_free(struct device *dev,
> + struct iommu_resv_region *region)
> +{
> + struct iommu_iort_rmr_data *rmr_data;
> +
> + rmr_data = container_of(region, struct iommu_iort_rmr_data, rr);
> + kfree(rmr_data->sids);
> + kfree(rmr_data);
> +}
> +
> +struct iommu_iort_rmr_data *iort_rmr_alloc(struct acpi_iort_rmr_desc
> *rmr_desc,
> +int prot, enum iommu_resv_type type,
> +u32 *sids, u32 num_sids)
> +{
> + struct iommu_iort_rmr_data *rmr_data;
> + struct iommu_resv_region *region;
> + u32 *sids_copy;
> + u64 addr = rmr_desc->base_address, size = rmr_desc->length;
> +
> + rmr_data = kmalloc(sizeof(*rmr_data), GFP_KERNEL);
> + if (!rmr_data)
> + return NULL;
> +
> + /* Create a copy of SIDs array to associate with this rmr_data */
> + sids_copy = kmemdup(sids, num_sids * sizeof(*sids), GFP_KERNEL);
> + if (!sids_copy) {
> + kfree(rmr_data);
> + return NULL;
> + }
> + rmr_data->sids = sids_copy;
> + rmr_data->num_sids = num_sids;
> +
> + if (!IS_ALIGNED(addr, SZ_64K) || !IS_ALIGNED(size, SZ_64K)) {
> + /* PAGE align base addr and size */
> + addr &= PAGE_MASK;
> + size = PAGE_ALIGN(size +
> offset_in_page(rmr_desc->base_address));
> +
> + pr_err(FW_BUG "RMR descriptor[0x%llx - 0x%llx] not aligned to
> 64K, continue with [0x%llx - 0x%llx]\n",
> +rmr_desc->base_address,
> +rmr_desc->base_address + rmr_desc->length - 1,
> +addr, addr + size - 1);
> + }
> +
> + region = _data->rr;
> + INIT_LIST_HEAD(>list);
> + region->start = addr;
> + region->length = size;
> + region->prot = prot;
> + region->type = type;
> + region->free = iort_rmr_free;
> +
> + return rmr_data;
> +}
> +
> +static void iort_rmr_desc_check_overlap(struct acpi_iort_rmr_desc *desc,
> + u32 count)
> +{
> + int i, j;
> +
> + for (i = 0; i < count; i++) {
> + u64 end, start = desc[i].base_address, length = desc[i].length;
> +
> + if (!length) {
> + pr_err(FW_BUG "RMR descriptor[0x%llx] with zero length,
> continue anyway\n",
> +start);
> + continue;
> + }
> +
> + end = start + length - 1;
> +
> + /* Check for address overlap */
> + for (j = i + 1; j < count; j++) {
> + u64 e_start = desc[j].base_address;
> + u64 e_end = e_start + desc[j].length - 1;
> +
> + if (start <= e_end && end >= e_start)
> + pr_err(FW_BUG "RMR descriptor[0x%llx - 0x%llx]
> overlaps, continue anyway\n",
> +start, end);
> + }
> + }
> +}
> +
> +/*
> + * Please note, we will keep the already allocated RMR reserve
> + * regions in case of a memory allocation failure.
> + */
> +static void iort_get_rmrs(struct acpi_iort_node *node,
> + struct acpi_iort_node *smmu,
> + u32 *sids, u32 num_sids,
> + struct list_head *head)
> +{
> + struct acpi_iort_rmr *rmr = (struct acpi_iort_rmr *)node->node_data;
> + struct acpi_iort_rmr_desc *rmr_desc;
> + int i;
> +
> + rmr_desc = ACPI_ADD_PTR(struct acpi_iort_rmr_desc, node,
> + rmr->rmr_offset);
> +
> + iort_rmr_desc_check_overlap(rmr_desc, rmr->rmr_count);
> +
> + for (i = 0; i < rmr->rmr_count; i++, rmr_desc++) {
> + struct iommu_iort_rmr_data *rmr_data;
> + enum iommu_resv_type type;
> + int prot = IOMMU_READ | IOMMU_WRITE;
> +
> + if (rmr->flags & ACPI_IORT_RMR_REMAP_PERMITTED)
> + type = IOMMU_RESV_DIRECT_RELAXABLE;
> + else
> + type = IOMMU_RESV_DIRECT;
> +
> + if (rmr->flags & ACPI_IORT_RMR_ACCESS_PRIVILEGE)
> +
Re: [PATCH v4 05/11] iommu/sva: Assign a PASID to mm on PASID allocation and free it on mm exit
On 4/25/22 09:40, Jean-Philippe Brucker wrote: > The problem is that we'd have to request the device driver to stop DMA > before we can destroy the context and free the PASID. We did consider > doing this in the release() MMU notifier, but there were concerns about > blocking mmput() for too long (for example > https://lore.kernel.org/linux-iommu/4d68da96-0ad5-b412-5987-2f7a6aa79...@amd.com/ > though I think there was a more recent discussion). We also need to drain > the PRI and fault queues to get rid of all references to that PASID. Is the concern truly about blocking mmput() itself? Or, is it about releasing the resources associated with the mm? ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/arm-smmu-v3-sva: Fix mm use-after-free
On 2022/4/26 下午9:04, Jean-Philippe Brucker wrote:
We currently call arm64_mm_context_put() without holding a reference to
the mm, which can result in use-after-free. Call mmgrab()/mmdrop() to
ensure the mm only gets freed after we unpinned the ASID.
Fixes: 32784a9562fb ("iommu/arm-smmu-v3: Implement iommu_sva_bind/unbind()")
Signed-off-by: Jean-Philippe Brucker
Thanks Jean
Tested-by: Zhangfei Gao
---
v2: Add missing include
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
index 22ddd05bbdcd..5d029e87c8af 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
@@ -6,6 +6,7 @@
#include
#include
#include
+#include
#include
#include "arm-smmu-v3.h"
@@ -96,9 +97,14 @@ static struct arm_smmu_ctx_desc
*arm_smmu_alloc_shared_cd(struct mm_struct *mm)
struct arm_smmu_ctx_desc *cd;
struct arm_smmu_ctx_desc *ret = NULL;
+ /* Don't free the mm until we release the ASID */
+ mmgrab(mm);
+
asid = arm64_mm_context_get(mm);
- if (!asid)
- return ERR_PTR(-ESRCH);
+ if (!asid) {
+ err = -ESRCH;
+ goto out_drop_mm;
+ }
cd = kzalloc(sizeof(*cd), GFP_KERNEL);
if (!cd) {
@@ -165,6 +171,8 @@ static struct arm_smmu_ctx_desc
*arm_smmu_alloc_shared_cd(struct mm_struct *mm)
kfree(cd);
out_put_context:
arm64_mm_context_put(mm);
+out_drop_mm:
+ mmdrop(mm);
return err < 0 ? ERR_PTR(err) : ret;
}
@@ -173,6 +181,7 @@ static void arm_smmu_free_shared_cd(struct arm_smmu_ctx_desc *cd)
if (arm_smmu_free_asid(cd)) {
/* Unpin ASID */
arm64_mm_context_put(cd->mm);
+ mmdrop(cd->mm);
kfree(cd);
}
}
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] swiotlb-xen: fix DMA_ATTR_NO_KERNEL_MAPPING on arm
Hi Christoph,
> On 23 Apr 2022, at 6:14 pm, Christoph Hellwig wrote:
>
> swiotlb-xen uses very different ways to allocate coherent memory on x86
> vs arm. On the former it allocates memory from the page allocator, while
> on the later it reuses the dma-direct allocator the handles the
> complexities of non-coherent DMA on arm platforms.
>
> Unfortunately the complexities of trying to deal with the two cases in
> the swiotlb-xen.c code lead to a bug in the handling of
> DMA_ATTR_NO_KERNEL_MAPPING on arm. With the DMA_ATTR_NO_KERNEL_MAPPING
> flag the coherent memory allocator does not actually allocate coherent
> memory, but just a DMA handle for some memory that is DMA addressable
> by the device, but which does not have to have a kernel mapping. Thus
> dereferencing the return value will lead to kernel crashed and memory
> corruption.
>
> Fix this by using the dma-direct allocator directly for arm, which works
> perfectly fine because on arm swiotlb-xen is only used when the domain is
> 1:1 mapped, and then simplifying the remaining code to only cater for the
> x86 case with DMA coherent device.
>
> Reported-by: Rahul Singh
> Signed-off-by: Christoph Hellwig
Reviewed-by: Rahul Singh
Tested-by: Rahul Singh
Regards,
Rahul
> ---
> arch/arm/include/asm/xen/page-coherent.h | 2 -
> arch/arm/xen/mm.c | 17
> arch/arm64/include/asm/xen/page-coherent.h | 2 -
> arch/x86/include/asm/xen/page-coherent.h | 24 -
> arch/x86/include/asm/xen/swiotlb-xen.h | 5 +
> drivers/xen/swiotlb-xen.c | 106 -
> include/xen/arm/page-coherent.h| 20
> include/xen/xen-ops.h | 7 --
> 8 files changed, 45 insertions(+), 138 deletions(-)
> delete mode 100644 arch/arm/include/asm/xen/page-coherent.h
> delete mode 100644 arch/arm64/include/asm/xen/page-coherent.h
> delete mode 100644 arch/x86/include/asm/xen/page-coherent.h
> delete mode 100644 include/xen/arm/page-coherent.h
>
> diff --git a/arch/arm/include/asm/xen/page-coherent.h
> b/arch/arm/include/asm/xen/page-coherent.h
> deleted file mode 100644
> index 27e984977402b..0
> --- a/arch/arm/include/asm/xen/page-coherent.h
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -#include
> diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
> index a7e54a087b802..6e603e5fdebb1 100644
> --- a/arch/arm/xen/mm.c
> +++ b/arch/arm/xen/mm.c
> @@ -118,23 +118,6 @@ bool xen_arch_need_swiotlb(struct device *dev,
> !dev_is_dma_coherent(dev));
> }
>
> -int xen_create_contiguous_region(phys_addr_t pstart, unsigned int order,
> - unsigned int address_bits,
> - dma_addr_t *dma_handle)
> -{
> - if (!xen_initial_domain())
> - return -EINVAL;
> -
> - /* we assume that dom0 is mapped 1:1 for now */
> - *dma_handle = pstart;
> - return 0;
> -}
> -
> -void xen_destroy_contiguous_region(phys_addr_t pstart, unsigned int order)
> -{
> - return;
> -}
> -
> static int __init xen_mm_init(void)
> {
> struct gnttab_cache_flush cflush;
> diff --git a/arch/arm64/include/asm/xen/page-coherent.h
> b/arch/arm64/include/asm/xen/page-coherent.h
> deleted file mode 100644
> index 27e984977402b..0
> --- a/arch/arm64/include/asm/xen/page-coherent.h
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -#include
> diff --git a/arch/x86/include/asm/xen/page-coherent.h
> b/arch/x86/include/asm/xen/page-coherent.h
> deleted file mode 100644
> index 63cd41b2e17ac..0
> --- a/arch/x86/include/asm/xen/page-coherent.h
> +++ /dev/null
> @@ -1,24 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -#ifndef _ASM_X86_XEN_PAGE_COHERENT_H
> -#define _ASM_X86_XEN_PAGE_COHERENT_H
> -
> -#include
> -#include
> -
> -static inline void *xen_alloc_coherent_pages(struct device *hwdev, size_t
> size,
> - dma_addr_t *dma_handle, gfp_t flags,
> - unsigned long attrs)
> -{
> - void *vstart = (void*)__get_free_pages(flags, get_order(size));
> - *dma_handle = virt_to_phys(vstart);
> - return vstart;
> -}
> -
> -static inline void xen_free_coherent_pages(struct device *hwdev, size_t size,
> - void *cpu_addr, dma_addr_t dma_handle,
> - unsigned long attrs)
> -{
> - free_pages((unsigned long) cpu_addr, get_order(size));
> -}
> -
> -#endif /* _ASM_X86_XEN_PAGE_COHERENT_H */
> diff --git a/arch/x86/include/asm/xen/swiotlb-xen.h
> b/arch/x86/include/asm/xen/swiotlb-xen.h
> index 66b4ddde77430..558821387808e 100644
> --- a/arch/x86/include/asm/xen/swiotlb-xen.h
> +++ b/arch/x86/include/asm/xen/swiotlb-xen.h
> @@ -10,4 +10,9 @@ extern int pci_xen_swiotlb_init_late(void);
> static inline int pci_xen_swiotlb_init_late(void) { return -ENXIO; }
> #endif
>
> +int xen_create_contiguous_region(phys_addr_t pstart, unsigned int order,
> +
[PATCH v2] iommu/arm-smmu-v3-sva: Fix mm use-after-free
We currently call arm64_mm_context_put() without holding a reference to
the mm, which can result in use-after-free. Call mmgrab()/mmdrop() to
ensure the mm only gets freed after we unpinned the ASID.
Fixes: 32784a9562fb ("iommu/arm-smmu-v3: Implement iommu_sva_bind/unbind()")
Signed-off-by: Jean-Philippe Brucker
---
v2: Add missing include
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
index 22ddd05bbdcd..5d029e87c8af 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
@@ -6,6 +6,7 @@
#include
#include
#include
+#include
#include
#include "arm-smmu-v3.h"
@@ -96,9 +97,14 @@ static struct arm_smmu_ctx_desc
*arm_smmu_alloc_shared_cd(struct mm_struct *mm)
struct arm_smmu_ctx_desc *cd;
struct arm_smmu_ctx_desc *ret = NULL;
+ /* Don't free the mm until we release the ASID */
+ mmgrab(mm);
+
asid = arm64_mm_context_get(mm);
- if (!asid)
- return ERR_PTR(-ESRCH);
+ if (!asid) {
+ err = -ESRCH;
+ goto out_drop_mm;
+ }
cd = kzalloc(sizeof(*cd), GFP_KERNEL);
if (!cd) {
@@ -165,6 +171,8 @@ static struct arm_smmu_ctx_desc
*arm_smmu_alloc_shared_cd(struct mm_struct *mm)
kfree(cd);
out_put_context:
arm64_mm_context_put(mm);
+out_drop_mm:
+ mmdrop(mm);
return err < 0 ? ERR_PTR(err) : ret;
}
@@ -173,6 +181,7 @@ static void arm_smmu_free_shared_cd(struct
arm_smmu_ctx_desc *cd)
if (arm_smmu_free_asid(cd)) {
/* Unpin ASID */
arm64_mm_context_put(cd->mm);
+ mmdrop(cd->mm);
kfree(cd);
}
}
--
2.35.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/arm-smmu-v3-sva: Fix mm use-after-free
On Tue, Apr 26, 2022 at 08:20:52PM +0800, zhangfei@foxmail.com wrote:
> Hi, Jean
>
> On 2022/4/26 下午6:04, Jean-Philippe Brucker wrote:
> > We currently call arm64_mm_context_put() without holding a reference to
> > the mm, which can result in use-after-free. Call mmgrab()/mmdrop() to
> > ensure the mm only gets freed after we unpinned the ASID.
> >
> > Fixes: 32784a9562fb ("iommu/arm-smmu-v3: Implement iommu_sva_bind/unbind()")
> > Signed-off-by: Jean-Philippe Brucker
> > ---
> > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 12 ++--
> > 1 file changed, 10 insertions(+), 2 deletions(-)
> Missing +#include for compile.
Ah thanks, I sent the wrong version.
> We still need the fix move mm_pasid_drop from __mmput to __mmdrop, right?
Yes, that's Fenghua's patch
Thanks,
Jean
>
> 1. Test OK with the mm_pasid_drop patch.
>
> 2. Test fail if revert the mm_pasid_drop patch,
> uacce_fops_release
> Unable to handle kernel paging request at virtual address 00083cc6ffc0
>
> By the way, we use mmgrab in bind and mmput in unbind before,
> then the fops_release is not be called if exit without close(fd).
>
> This patch does not have this issue, still not understand why.
>
> Thanks
>
> > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> > b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> > index 582114f94da0..c93477a2d7f1 100644
> > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> > @@ -98,9 +98,14 @@ static struct arm_smmu_ctx_desc
> > *arm_smmu_alloc_shared_cd(struct mm_struct *mm)
> > struct arm_smmu_ctx_desc *cd;
> > struct arm_smmu_ctx_desc *ret = NULL;
> > + /* Don't free the mm until we release the ASID */
> > + mmgrab(mm);
> > +
> > asid = arm64_mm_context_get(mm);
> > - if (!asid)
> > - return ERR_PTR(-ESRCH);
> > + if (!asid) {
> > + err = -ESRCH;
> > + goto out_drop_mm;
> > + }
> > cd = kzalloc(sizeof(*cd), GFP_KERNEL);
> > if (!cd) {
> > @@ -167,6 +172,8 @@ static struct arm_smmu_ctx_desc
> > *arm_smmu_alloc_shared_cd(struct mm_struct *mm)
> > kfree(cd);
> > out_put_context:
> > arm64_mm_context_put(mm);
> > +out_drop_mm:
> > + mmdrop(mm);
> > return err < 0 ? ERR_PTR(err) : ret;
> > }
> > @@ -175,6 +182,7 @@ static void arm_smmu_free_shared_cd(struct
> > arm_smmu_ctx_desc *cd)
> > if (arm_smmu_free_asid(cd)) {
> > /* Unpin ASID */
> > arm64_mm_context_put(cd->mm);
> > + mmdrop(cd->mm);
> > kfree(cd);
> > }
> > }
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/arm-smmu-v3-sva: Fix mm use-after-free
Hi, Jean
On 2022/4/26 下午6:04, Jean-Philippe Brucker wrote:
We currently call arm64_mm_context_put() without holding a reference to
the mm, which can result in use-after-free. Call mmgrab()/mmdrop() to
ensure the mm only gets freed after we unpinned the ASID.
Fixes: 32784a9562fb ("iommu/arm-smmu-v3: Implement iommu_sva_bind/unbind()")
Signed-off-by: Jean-Philippe Brucker
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
Missing +#include for compile.
We still need the fix move mm_pasid_drop from __mmput to __mmdrop, right?
1. Test OK with the mm_pasid_drop patch.
2. Test fail if revert the mm_pasid_drop patch,
uacce_fops_release
Unable to handle kernel paging request at virtual address 00083cc6ffc0
By the way, we use mmgrab in bind and mmput in unbind before,
then the fops_release is not be called if exit without close(fd).
This patch does not have this issue, still not understand why.
Thanks
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
index 582114f94da0..c93477a2d7f1 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
@@ -98,9 +98,14 @@ static struct arm_smmu_ctx_desc
*arm_smmu_alloc_shared_cd(struct mm_struct *mm)
struct arm_smmu_ctx_desc *cd;
struct arm_smmu_ctx_desc *ret = NULL;
+ /* Don't free the mm until we release the ASID */
+ mmgrab(mm);
+
asid = arm64_mm_context_get(mm);
- if (!asid)
- return ERR_PTR(-ESRCH);
+ if (!asid) {
+ err = -ESRCH;
+ goto out_drop_mm;
+ }
cd = kzalloc(sizeof(*cd), GFP_KERNEL);
if (!cd) {
@@ -167,6 +172,8 @@ static struct arm_smmu_ctx_desc
*arm_smmu_alloc_shared_cd(struct mm_struct *mm)
kfree(cd);
out_put_context:
arm64_mm_context_put(mm);
+out_drop_mm:
+ mmdrop(mm);
return err < 0 ? ERR_PTR(err) : ret;
}
@@ -175,6 +182,7 @@ static void arm_smmu_free_shared_cd(struct arm_smmu_ctx_desc *cd)
if (arm_smmu_free_asid(cd)) {
/* Unpin ASID */
arm64_mm_context_put(cd->mm);
+ mmdrop(cd->mm);
kfree(cd);
}
}
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v11 4/9] ACPI/IORT: Add support to retrieve IORT RMR reserved regions
> -Original Message-
> From: kernel test robot [mailto:l...@intel.com]
> Sent: 23 April 2022 10:51
> To: Shameerali Kolothum Thodi ;
> linux-arm-ker...@lists.infradead.org; linux-a...@vger.kernel.org;
> iommu@lists.linux-foundation.org
> Cc: l...@lists.linux.dev; kbuild-...@lists.01.org; Linuxarm
> ; lorenzo.pieral...@arm.com; j...@8bytes.org;
> robin.mur...@arm.com; w...@kernel.org; wanghuiqiang
> ; Guohanjun (Hanjun Guo)
> ; steven.pr...@arm.com; sami.muja...@arm.com;
> j...@solid-run.com; eric.au...@redhat.com; laurentiu.tu...@nxp.com;
> h...@infradead.org
> Subject: Re: [PATCH v11 4/9] ACPI/IORT: Add support to retrieve IORT RMR
> reserved regions
>
[...]
> >> drivers/acpi/arm64/iort.c:801:29: warning: no previous prototype for
> function 'iort_rmr_alloc' [-Wmissing-prototypes]
>struct iommu_iort_rmr_data *iort_rmr_alloc(struct acpi_iort_rmr_desc
> *rmr_desc,
>^
>drivers/acpi/arm64/iort.c:801:1: note: declare 'static' if the function is
> not
> intended to be used outside of this translation unit
>struct iommu_iort_rmr_data *iort_rmr_alloc(struct acpi_iort_rmr_desc
> *rmr_desc,
Oops..missed the 'static' here. The rest of the warnings are because of the
dependency
with ACPICA header patch here[1].
Hi Robin/Lorenzo,
I am planning to send out an updated series soon with that 'static' added and
the R-by
tag received for patch #1 from Christoph. Appreciate if you can take a look and
let me
know if you have any further comments on this series.
Thanks,
Shameer
[1] https://lore.kernel.org/all/44610361.fMDQidcC6G@kreacher/
>^
>static
>drivers/acpi/arm64/iort.c:896:20: error: use of undeclared identifier
> 'ACPI_IORT_RMR_REMAP_PERMITTED'
>if (rmr->flags & ACPI_IORT_RMR_REMAP_PERMITTED)
> ^
>drivers/acpi/arm64/iort.c:901:20: error: use of undeclared identifier
> 'ACPI_IORT_RMR_ACCESS_PRIVILEGE'
>if (rmr->flags & ACPI_IORT_RMR_ACCESS_PRIVILEGE)
> ^
>drivers/acpi/arm64/iort.c:905:7: error: call to undeclared function
> 'ACPI_IORT_RMR_ACCESS_ATTRIBUTES'; ISO C99 and later do not support
> implicit function declarations [-Wimplicit-function-declaration]
>if (ACPI_IORT_RMR_ACCESS_ATTRIBUTES(rmr->flags) <=
>^
>drivers/acpi/arm64/iort.c:906:5: error: use of undeclared identifier
> 'ACPI_IORT_RMR_ATTR_DEVICE_GRE'
>ACPI_IORT_RMR_ATTR_DEVICE_GRE)
>^
>drivers/acpi/arm64/iort.c:909:5: error: use of undeclared identifier
> 'ACPI_IORT_RMR_ATTR_NORMAL_IWB_OWB'
>
> ACPI_IORT_RMR_ATTR_NORMAL_IWB_OWB)
>^
>1 warning and 5 errors generated.
>
>
> vim +/iort_rmr_alloc +801 drivers/acpi/arm64/iort.c
>
>800
> > 801struct iommu_iort_rmr_data *iort_rmr_alloc(struct
> acpi_iort_rmr_desc *rmr_desc,
>802 int prot, enum
> iommu_resv_type type,
>803 u32 *sids, u32
> num_sids)
>804{
>805struct iommu_iort_rmr_data *rmr_data;
>806struct iommu_resv_region *region;
>807u32 *sids_copy;
>808u64 addr = rmr_desc->base_address, size =
> rmr_desc->length;
>809
>810rmr_data = kmalloc(sizeof(*rmr_data), GFP_KERNEL);
>811if (!rmr_data)
>812return NULL;
>813
>814/* Create a copy of SIDs array to associate with this
> rmr_data */
>815sids_copy = kmemdup(sids, num_sids * sizeof(*sids),
> GFP_KERNEL);
>816if (!sids_copy) {
>817kfree(rmr_data);
>818return NULL;
>819}
>820rmr_data->sids = sids_copy;
>821rmr_data->num_sids = num_sids;
>822
>823if (!IS_ALIGNED(addr, SZ_64K) || !IS_ALIGNED(size,
> SZ_64K)) {
>824/* PAGE align base addr and size */
>825addr &= PAGE_MASK;
>826size = PAGE_ALIGN(size +
> offset_in_page(rmr_desc->base_address));
>827
>828pr_err(FW_BUG "RMR descriptor[0x%llx - 0x%llx]
> not
> aligned to 64K, continue with [0x%llx - 0x%llx]\n",
>829 rmr_desc->base_address,
>830 rmr_desc->base_address +
> rmr_desc->length - 1,
>831 addr, addr + size - 1);
>832}
>833
>834region = _data->rr;
>835INIT_LIST_HEAD(>list);
>836region->start = addr;
>837region->length = size;
>838
Re: [PATCH v6 1/2] PCI/ACPI: Support Microsoft's "DmaProperty"
On 2022-04-26 01:06, Rajat Jain via iommu wrote:
The "DmaProperty" is supported and currently documented and used by
Microsoft [link 1 below], to flag internal PCIe root ports that need
DMA protection [link 2 below]. We have discussed with them and reached
a common understanding that they shall change their MSDN documentation
to say that the same property can be used to protect any PCI device,
and not just internal PCIe root ports (since there is no point
introducing yet another property for arbitrary PCI devices). This helps
with security from internal devices that offer an attack surface for
DMA attacks (e.g. internal network devices).
Support DmaProperty to mark DMA from a PCI device as untrusted.
Link: [1]
https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports#identifying-internal-pcie-ports-accessible-to-users-and-requiring-dma-protection
Link: [2]
https://docs.microsoft.com/en-us/windows/security/information-protection/kernel-dma-protection-for-thunderbolt
Signed-off-by: Rajat Jain
Reviewed-by: Mika Westerberg
Acked-by: Rafael J. Wysocki
---
v6: * Take care of Bjorn's comments:
- Update the commit log
- Rename to pci_dev_has_dma_property()
- Use acpi_dev_get_property()
v5: * Reorder the patches in the series
v4: * Add the GUID.
* Update the comment and commitlog.
v3: * Use Microsoft's documented property "DmaProperty"
* Resctrict to ACPI only
drivers/acpi/property.c | 3 +++
drivers/pci/pci-acpi.c | 21 +
2 files changed, 24 insertions(+)
diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 12bbfe833609..bafe35c301ac 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -48,6 +48,9 @@ static const guid_t prp_guids[] = {
/* Storage device needs D3 GUID: 5025030f-842f-4ab4-a561-99a5189762d0 */
GUID_INIT(0x5025030f, 0x842f, 0x4ab4,
0xa5, 0x61, 0x99, 0xa5, 0x18, 0x97, 0x62, 0xd0),
+ /* DmaProperty for PCI devices GUID:
70d24161-6dd5-4c9e-8070-705531292865 */
+ GUID_INIT(0x70d24161, 0x6dd5, 0x4c9e,
+ 0x80, 0x70, 0x70, 0x55, 0x31, 0x29, 0x28, 0x65),
};
/* ACPI _DSD data subnodes GUID: dbb8e3e6-5886-4ba6-8795-1319f52a966b */
diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index 3ae435beaf0a..d7c6ba48486f 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -1369,12 +1369,33 @@ static void pci_acpi_set_external_facing(struct pci_dev
*dev)
dev->external_facing = 1;
}
+static int pci_dev_has_dma_property(struct pci_dev *dev)
+{
+ struct acpi_device *adev;
+ const union acpi_object *obj;
+
+ adev = ACPI_COMPANION(>dev);
+ if (!adev)
+ return 0;
+
+ /*
+* Property also used by Microsoft Windows for same purpose,
+* (to implement DMA protection from a device, using the IOMMU).
Nit: there is no context for "same purpose" here, so this comment is
more confusing than helpful. Might I suggest a rewording like:
/*
* Property used by Microsoft Windows to enforce IOMMU DMA
* protection for any device that the system might not fully
* trust; we'll honour it the same way.
*/
?
Personally I think it would have been more logical to handle this in
pci_set_dma_untrusted(), but I see some of those implementation aspects
have already been discussed, and Bjorn's preference definitely wins over
mine here :)
Thanks,
Robin.
+*/
+ if (!acpi_dev_get_property(adev, "DmaProperty", ACPI_TYPE_INTEGER,
+ ) && obj->integer.value == 1)
+ return 1;
+
+ return 0;
+}
+
void pci_acpi_setup(struct device *dev, struct acpi_device *adev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
pci_acpi_optimize_delay(pci_dev, adev->handle);
pci_acpi_set_external_facing(pci_dev);
+ pci_dev->untrusted |= pci_dev_has_dma_property(pci_dev); >
pci_acpi_add_edr_notifier(pci_dev);
pci_acpi_add_pm_notifier(adev, pci_dev);
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/arm-smmu-v3-sva: Fix mm use-after-free
We currently call arm64_mm_context_put() without holding a reference to
the mm, which can result in use-after-free. Call mmgrab()/mmdrop() to
ensure the mm only gets freed after we unpinned the ASID.
Fixes: 32784a9562fb ("iommu/arm-smmu-v3: Implement iommu_sva_bind/unbind()")
Signed-off-by: Jean-Philippe Brucker
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
index 582114f94da0..c93477a2d7f1 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
@@ -98,9 +98,14 @@ static struct arm_smmu_ctx_desc
*arm_smmu_alloc_shared_cd(struct mm_struct *mm)
struct arm_smmu_ctx_desc *cd;
struct arm_smmu_ctx_desc *ret = NULL;
+ /* Don't free the mm until we release the ASID */
+ mmgrab(mm);
+
asid = arm64_mm_context_get(mm);
- if (!asid)
- return ERR_PTR(-ESRCH);
+ if (!asid) {
+ err = -ESRCH;
+ goto out_drop_mm;
+ }
cd = kzalloc(sizeof(*cd), GFP_KERNEL);
if (!cd) {
@@ -167,6 +172,8 @@ static struct arm_smmu_ctx_desc
*arm_smmu_alloc_shared_cd(struct mm_struct *mm)
kfree(cd);
out_put_context:
arm64_mm_context_put(mm);
+out_drop_mm:
+ mmdrop(mm);
return err < 0 ? ERR_PTR(err) : ret;
}
@@ -175,6 +182,7 @@ static void arm_smmu_free_shared_cd(struct
arm_smmu_ctx_desc *cd)
if (arm_smmu_free_asid(cd)) {
/* Unpin ASID */
arm64_mm_context_put(cd->mm);
+ mmdrop(cd->mm);
kfree(cd);
}
}
--
2.35.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v6 2/2] PCI: Rename pci_dev->untrusted to pci_dev->untrusted_dma
On 2022/4/26 08:06, Rajat Jain wrote:
Rename the field to make it more clear, that the device can execute DMA
attacks on the system, and thus the system may need protection from
such attacks from this device.
No functional change intended.
Signed-off-by: Rajat Jain
Reviewed-by: Mika Westerberg
Acked-by: Rafael J. Wysocki
---
v6: No change in this patch, rebased on top of changes in other patch.
v5: Use "untrusted_dma" as property name, based on feedback.
Reorder the patches in the series.
v4: Initial version, created based on comments on other patch
drivers/iommu/dma-iommu.c | 6 +++---
drivers/iommu/intel/iommu.c | 2 +-
drivers/iommu/iommu.c | 2 +-
drivers/pci/ats.c | 2 +-
drivers/pci/pci-acpi.c | 2 +-
drivers/pci/pci.c | 2 +-
drivers/pci/probe.c | 8
drivers/pci/quirks.c| 2 +-
include/linux/pci.h | 5 +++--
9 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 09f6e1c0f9c0..aeee4be7614d 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -497,14 +497,14 @@ static int iova_reserve_iommu_regions(struct device *dev,
return ret;
}
-static bool dev_is_untrusted(struct device *dev)
+static bool dev_has_untrusted_dma(struct device *dev)
{
- return dev_is_pci(dev) && to_pci_dev(dev)->untrusted;
+ return dev_is_pci(dev) && to_pci_dev(dev)->untrusted_dma;
}
static bool dev_use_swiotlb(struct device *dev)
{
- return IS_ENABLED(CONFIG_SWIOTLB) && dev_is_untrusted(dev);
+ return IS_ENABLED(CONFIG_SWIOTLB) && dev_has_untrusted_dma(dev);
}
/**
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index df5c62ecf942..b88f47391140 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4843,7 +4843,7 @@ static bool intel_iommu_is_attach_deferred(struct device
*dev)
*/
static bool risky_device(struct pci_dev *pdev)
{
- if (pdev->untrusted) {
+ if (pdev->untrusted_dma) {
pci_info(pdev,
"Skipping IOMMU quirk for dev [%04X:%04X] on untrusted PCI
link\n",
pdev->vendor, pdev->device);
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index f2c45b85b9fc..d8d3133e2947 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1525,7 +1525,7 @@ static int iommu_get_def_domain_type(struct device *dev)
{
const struct iommu_ops *ops = dev_iommu_ops(dev);
- if (dev_is_pci(dev) && to_pci_dev(dev)->untrusted)
+ if (dev_is_pci(dev) && to_pci_dev(dev)->untrusted_dma)
return IOMMU_DOMAIN_DMA;
if (ops->def_domain_type)
diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index c967ad6e2626..477c16ba9341 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -42,7 +42,7 @@ bool pci_ats_supported(struct pci_dev *dev)
if (!dev->ats_cap)
return false;
- return (dev->untrusted == 0);
+ return (dev->untrusted_dma == 0);
}
EXPORT_SYMBOL_GPL(pci_ats_supported);
diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index d7c6ba48486f..7c2784e7e954 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -1395,7 +1395,7 @@ void pci_acpi_setup(struct device *dev, struct
acpi_device *adev)
pci_acpi_optimize_delay(pci_dev, adev->handle);
pci_acpi_set_external_facing(pci_dev);
- pci_dev->untrusted |= pci_dev_has_dma_property(pci_dev);
+ pci_dev->untrusted_dma |= pci_dev_has_dma_property(pci_dev);
pci_acpi_add_edr_notifier(pci_dev);
pci_acpi_add_pm_notifier(adev, pci_dev);
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 9ecce435fb3f..1fb0eb8646c8 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -958,7 +958,7 @@ static void pci_std_enable_acs(struct pci_dev *dev)
ctrl |= (cap & PCI_ACS_UF);
/* Enable Translation Blocking for external devices and noats */
- if (pci_ats_disabled() || dev->external_facing || dev->untrusted)
+ if (pci_ats_disabled() || dev->external_facing || dev->untrusted_dma)
ctrl |= (cap & PCI_ACS_TB);
pci_write_config_word(dev, pos + PCI_ACS_CTRL, ctrl);
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 17a969942d37..d2a9b26fcede 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1587,7 +1587,7 @@ static void set_pcie_thunderbolt(struct pci_dev *dev)
dev->is_thunderbolt = 1;
}
-static void set_pcie_untrusted(struct pci_dev *dev)
+static void pci_set_untrusted_dma(struct pci_dev *dev)
{
struct pci_dev *parent;
@@ -1596,8 +1596,8 @@ static void set_pcie_untrusted(struct pci_dev *dev)
* untrusted as well.
*/
parent = pci_upstream_bridge(dev);
- if (parent && (parent->untrusted || parent->external_facing))
- dev->untrusted = true;
+ if
Re: [Patch v2] iommu: arm-smmu: disable large page mappings for Nvidia arm-smmu
Hi Will,
On 22/04/2022 11:55, Will Deacon wrote:
On Thu, 21 Apr 2022 13:45:04 +0530, Ashish Mhetre wrote:
Tegra194 and Tegra234 SoCs have the erratum that causes walk cache
entries to not be invalidated correctly. The problem is that the walk
cache index generated for IOVA is not same across translation and
invalidation requests. This is leading to page faults when PMD entry is
released during unmap and populated with new PTE table during subsequent
map request. Disabling large page mappings avoids the release of PMD
entry and avoid translations seeing stale PMD entry in walk cache.
Fix this by limiting the page mappings to PAGE_SIZE for Tegra194 and
Tegra234 devices. This is recommended fix from Tegra hardware design
team.
[...]
Applied to will (for-joerg/arm-smmu/fixes), thanks!
[1/1] iommu: arm-smmu: disable large page mappings for Nvidia arm-smmu
https://git.kernel.org/will/c/4a25f2ea0e03
Thanks for applying. Sorry to be late to the party, but feel free
to add my ...
Reviewed-by: Jon Hunter
Also any chance we could tag for stable? Probably the most
appropriate fixes-tag would be ...
Fixes: aab5a1c88276 ("iommu/arm-smmu: add NVIDIA implementation for ARM MMU-500
usage")
Thanks!
Jon
--
nvpublic
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
