Re: [PATCH] iommu/arm-smmu-v3: Set GBPA to abort all transactions

[Marc Zyngier]

On 2018-03-28 15:39, Timur Tabi wrote:

From: Sameer Goel 

Set SMMU_GBPA to abort all incoming translations during the SMMU 
reset

when SMMUEN==0.

This prevents a race condition where a stray DMA from the crashed 
primary
kernel can try to access an IOVA address as an invalid PA when SMMU 
is

disabled during reset in the crash kernel.

Signed-off-by: Sameer Goel 
---
 drivers/iommu/arm-smmu-v3.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/iommu/arm-smmu-v3.c 
b/drivers/iommu/arm-smmu-v3.c

index 3f2f1fc68b52..c04a89310c59 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2458,6 +2458,18 @@ static int arm_smmu_device_reset(struct
arm_smmu_device *smmu, bool bypass)
if (reg & CR0_SMMUEN)
dev_warn(smmu->dev, "SMMU currently enabled! Resetting...\n");

+   /*
+* Abort all incoming translations. This can happen in a kdump case
+* where SMMU is initialized when a prior DMA is pending. Just
+	 * disabling the SMMU in this case might result in writes to 
invalid

+* PAs.
+*/
+   ret = arm_smmu_update_gbpa(smmu, 1, GBPA_ABORT);
+   if (ret) {
+   dev_err(smmu->dev, "GBPA not responding to update\n");
+   return ret;
+   }
+
ret = arm_smmu_device_disable(smmu);
if (ret)
return ret;


A tangential question: can we reliably detect that the SMMU already has 
valid mappings, which would indicate that we're in a pretty bad shape 
already by the time we set that bit? For all we know, memory could have 
been corrupted long before we hit this point, and this patch barely 
narrows the window of opportunity.


At the very least, we should emit a warning and taint the kernel (we've 
recently added such measures to the GICv3 driver).


Thanks,

M.
--
Fast, cheap, reliable. Pick two.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] dma-mapping: don't clear GFP_ZERO in dma_alloc_attrs

[Evgeniy Didin]

Adding linux-snps and linux-arch mailing lists.

> Revert the clearing of __GFP_ZERO in dma_alloc_attrs and move it to
> dma_direct_alloc for now.  While most common architectures always zero dma
> cohereny allocations (and x86 did so since day one) this is not documented
> and at least arc and s390 do not zero without the explicit __GFP_ZERO
> argument.
This patch fixed Ethernet issues on ARC HSDK.
https://www.spinics.net/lists/kernel/msg2762054.html

Tested-by: Evgeniy Didin 
> Fixes: 57bf5a8963f8 ("dma-mapping: clear harmful GFP_* flags in common code")
> Reported-by: Evgeniy Didin 
> Reported-by: Sebastian Ott 
> Signed-off-by: Christoph Hellwig 
> ---
>  include/linux/dma-mapping.h | 8 ++--
>  lib/dma-direct.c| 3 +++
>  2 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index eb9eab4ecd6d..12fedcba9a9a 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -518,12 +518,8 @@ static inline void *dma_alloc_attrs(struct device *dev, 
> size_t size,
>   if (dma_alloc_from_dev_coherent(dev, size, dma_handle, _addr))
>   return cpu_addr;
>  
> - /*
> -  * Let the implementation decide on the zone to allocate from, and
> -  * decide on the way of zeroing the memory given that the memory
> -  * returned should always be zeroed.
> -  */
> - flag &= ~(__GFP_DMA | __GFP_DMA32 | __GFP_HIGHMEM | __GFP_ZERO);
> + /* let the implementation decide on the zone to allocate from: */
> + flag &= ~(__GFP_DMA | __GFP_DMA32 | __GFP_HIGHMEM);
> 
>   if (!arch_dma_alloc_attrs(, ))
>   return NULL;
> diff --git a/lib/dma-direct.c b/lib/dma-direct.c
> index 1277d293d4da..c0bba30fef0a 100644
> --- a/lib/dma-direct.c
> +++ b/lib/dma-direct.c
> @@ -59,6 +59,9 @@ void *dma_direct_alloc(struct device *dev, size_t size, 
> dma_addr_t *dma_handle,
>   struct page *page = NULL;
>   void *ret;
>  
> + /* we always manually zero the memory once we are done: */
> + gfp &= ~__GFP_ZERO;
> +
>   /* GFP_DMA32 and GFP_DMA are no ops without the corresponding zones: */
>   if (dev->coherent_dma_mask <= DMA_BIT_MASK(ARCH_ZONE_DMA_BITS))
>   gfp |= GFP_DMA;
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH] iommu/arm-smmu-v3: Set GBPA to abort all transactions

[Timur Tabi]

From: Sameer Goel 

Set SMMU_GBPA to abort all incoming translations during the SMMU reset
when SMMUEN==0.

This prevents a race condition where a stray DMA from the crashed primary
kernel can try to access an IOVA address as an invalid PA when SMMU is
disabled during reset in the crash kernel.

Signed-off-by: Sameer Goel 
---
 drivers/iommu/arm-smmu-v3.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 3f2f1fc68b52..c04a89310c59 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2458,6 +2458,18 @@ static int arm_smmu_device_reset(struct arm_smmu_device 
*smmu, bool bypass)
if (reg & CR0_SMMUEN)
dev_warn(smmu->dev, "SMMU currently enabled! Resetting...\n");
 
+   /*
+* Abort all incoming translations. This can happen in a kdump case
+* where SMMU is initialized when a prior DMA is pending. Just
+* disabling the SMMU in this case might result in writes to invalid
+* PAs.
+*/
+   ret = arm_smmu_update_gbpa(smmu, 1, GBPA_ABORT);
+   if (ret) {
+   dev_err(smmu->dev, "GBPA not responding to update\n");
+   return ret;
+   }
+
ret = arm_smmu_device_disable(smmu);
if (ret)
return ret;
-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

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

RE: [PATCH v5 7/7] iommu/dma: Move PCI window region reservation back into dma specific path.

[Shameerali Kolothum Thodi]

Hi Joerg,

> -Original Message-
> From: Linuxarm [mailto:linuxarm-boun...@huawei.com] On Behalf Of
> Shameerali Kolothum Thodi
> Sent: Friday, March 23, 2018 8:57 AM
> To: Robin Murphy ; Alex Williamson
> 
> Cc: k...@vger.kernel.org; Joerg Roedel ;
> pmo...@linux.vnet.ibm.com; linux-ker...@vger.kernel.org; Linuxarm
> ; eric.au...@redhat.com; iommu@lists.linux-
> foundation.org; xuwei (O) 
> Subject: RE: [PATCH v5 7/7] iommu/dma: Move PCI window region reservation
> back into dma specific path.
> 
> 
> 
> > -Original Message-
> > From: Robin Murphy [mailto:robin.mur...@arm.com]
> > Sent: Thursday, March 22, 2018 5:22 PM
> > To: Alex Williamson ; Shameerali Kolothum
> > Thodi 
> > Cc: eric.au...@redhat.com; pmo...@linux.vnet.ibm.com;
> > k...@vger.kernel.org; linux-ker...@vger.kernel.org; iommu@lists.linux-
> > foundation.org; Linuxarm ; John Garry
> > ; xuwei (O) ; Joerg Roedel
> > 
> > Subject: Re: [PATCH v5 7/7] iommu/dma: Move PCI window region
> > reservation back into dma specific path.
> >
> > On 22/03/18 16:21, Alex Williamson wrote:
> > > On Thu, 15 Mar 2018 16:35:09 +
> > > Shameer Kolothum  wrote:
> > >
> > >> This pretty much reverts commit 273df9635385 ("iommu/dma: Make PCI
> > >> window reservation generic")  by moving the PCI window region
> > >> reservation back into the dma specific path so that these regions
> > >> doesn't get exposed via the IOMMU API interface. With this change,
> > >> the vfio interface will report only iommu specific reserved regions
> > >> to the user space.
> > >>
> > >> Cc: Robin Murphy 
> > >> Cc: Joerg Roedel 
> > >> Signed-off-by: Shameer Kolothum
> > 
> > >> ---
> > >
> > > As currently ordered, we expose the iova list to the user in 5/7
> > > with the PCI window reservations still intact.  Isn't that a
> > > bisection problem?  This patch should come before the iova list is
> > > expose to the user.  This is otherwise independent, so I can pop it
> > > up in the stack on commit, but I'd need an ack from Joerg and Robin
> > > to take it via my tree.  Thanks,
> >
> > If it counts, the changes look right, so:
> >
> > Acked-by: Robin Murphy 
> 
> Thanks Robin.
> 
> > but it does look like there's a hard dependency on Joerg's core branch
> > where Shameer's ITS workaround patches are currently queued.
> > Otherwise, though, I don't think there's anything else due to be
> > touching iommu-dma just yet.
> 
> True, I have mentioned this dependency[1] in the cover letter.

Just a gentle ping on this. 

If you are Ok with this one, as mentioned by Alex above, he might be able to
take this via his tree or please advise the best option to pull this 
considering the
dependency with the ITS workaround patches.

Thanks,
Shameer

> 1.
> https://kernel.googlesource.com/pub/scm/linux/kernel/git/joro/iommu/+log/c
> ore
> 
> 
> > Robin.
> >
> > >
> > > Alex
> > >
> > >>   drivers/iommu/dma-iommu.c | 54
> > >> ++-
> > 
> > >>   1 file changed, 25 insertions(+), 29 deletions(-)
> > >>
> > >> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> > >> index f05f3cf..ddcbbdb 100644
> > >> --- a/drivers/iommu/dma-iommu.c
> > >> +++ b/drivers/iommu/dma-iommu.c
> > >> @@ -167,40 +167,16 @@ EXPORT_SYMBOL(iommu_put_dma_cookie);
> > >>* @list: Reserved region list from iommu_get_resv_regions()
> > >>*
> > >>* IOMMU drivers can use this to implement their
> > >> .get_resv_regions
> > callback
> > >> - * for general non-IOMMU-specific reservations. Currently, this
> > >> covers host
> > >> - * bridge windows for PCI devices and GICv3 ITS region reservation
> > >> on ACPI
> > >> - * based ARM platforms that may require HW MSI reservation.
> > >> + * for general non-IOMMU-specific reservations. Currently, this
> > >> + covers
> > GICv3
> > >> + * ITS region reservation on ACPI based ARM platforms that may
> > >> + require
> > HW MSI
> > >> + * reservation.
> > >>*/
> > >>   void iommu_dma_get_resv_regions(struct device *dev, struct
> > >> list_head
> > *list)
> > >>   {
> > >> -struct pci_host_bridge *bridge;
> > >> -struct resource_entry *window;
> > >> -
> > >> -if (!is_of_node(dev->iommu_fwspec->iommu_fwnode) &&
> > >> -iort_iommu_msi_get_resv_regions(dev, list) < 0)
> > >> -return;
> > >> -
> > >> -if (!dev_is_pci(dev))
> > >> -return;
> > >> -
> > >> -bridge = pci_find_host_bridge(to_pci_dev(dev)->bus);
> > >> -resource_list_for_each_entry(window, >windows) {
> > >> -struct iommu_resv_region *region;
> > 

tip/x86/dma fix for arc and s390 (at least)

[Christoph Hellwig]

Hi all,

this restores previous __GFP_ZERO passthrough behavior for now as arc and
s390 rely on it.  Needs more work to sort out the API mess in the long run.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH] dma-mapping: don't clear GFP_ZERO in dma_alloc_attrs

[Christoph Hellwig]

Revert the clearing of __GFP_ZERO in dma_alloc_attrs and move it to
dma_direct_alloc for now.  While most common architectures always zero dma
cohereny allocations (and x86 did so since day one) this is not documented
and at least arc and s390 do not zero without the explicit __GFP_ZERO
argument.

Fixes: 57bf5a8963f8 ("dma-mapping: clear harmful GFP_* flags in common code")
Reported-by: Evgeniy Didin 
Reported-by: Sebastian Ott 
Signed-off-by: Christoph Hellwig 
---
 include/linux/dma-mapping.h | 8 ++--
 lib/dma-direct.c| 3 +++
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index eb9eab4ecd6d..12fedcba9a9a 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -518,12 +518,8 @@ static inline void *dma_alloc_attrs(struct device *dev, 
size_t size,
if (dma_alloc_from_dev_coherent(dev, size, dma_handle, _addr))
return cpu_addr;
 
-   /*
-* Let the implementation decide on the zone to allocate from, and
-* decide on the way of zeroing the memory given that the memory
-* returned should always be zeroed.
-*/
-   flag &= ~(__GFP_DMA | __GFP_DMA32 | __GFP_HIGHMEM | __GFP_ZERO);
+   /* let the implementation decide on the zone to allocate from: */
+   flag &= ~(__GFP_DMA | __GFP_DMA32 | __GFP_HIGHMEM);
 
if (!arch_dma_alloc_attrs(, ))
return NULL;
diff --git a/lib/dma-direct.c b/lib/dma-direct.c
index 1277d293d4da..c0bba30fef0a 100644
--- a/lib/dma-direct.c
+++ b/lib/dma-direct.c
@@ -59,6 +59,9 @@ void *dma_direct_alloc(struct device *dev, size_t size, 
dma_addr_t *dma_handle,
struct page *page = NULL;
void *ret;
 
+   /* we always manually zero the memory once we are done: */
+   gfp &= ~__GFP_ZERO;
+
/* GFP_DMA32 and GFP_DMA are no ops without the corresponding zones: */
if (dev->coherent_dma_mask <= DMA_BIT_MASK(ARCH_ZONE_DMA_BITS))
gfp |= GFP_DMA;
-- 
2.14.2

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

[PATCH RFC 1/1] iommu/of: Deconfigure iommu on driver detach

[Vivek Gautam]

As part of dma_deconfigure, lets deconfigure the iommu too
on driver detach, so that we clear the iommu domain and
related group.

Signed-off-by: Vivek Gautam 
---

As a part of dma_deconfigure, shouldn't we deconfigure the iommu
as well? This should reverse all that we do in of_iommu_configure.
So that we call the .remove_device ops for iommu and eventually
clear all the iommu domain, related group infrastructure.
I am seeing that the loadable modules get the same iommu configurations
after re-loading them, i.e. iommu domain, and iova_domain configurations,
as we didn't cleared them.
So should we be clearing these configurations, and therefore do a
of_iommu_deconfigure() or sort?

 drivers/iommu/of_iommu.c | 11 +++
 drivers/of/device.c  |  1 +
 include/linux/of_iommu.h |  5 +
 3 files changed, 17 insertions(+)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 5c36a8b7656a..1a23e6204ade 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -160,6 +160,17 @@ static int of_pci_iommu_init(struct pci_dev *pdev, u16 
alias, void *data)
return err;
 }
 
+void of_iommu_deconfigure(struct device *dev)
+{
+   const struct iommu_ops *ops = NULL;
+
+   if (dev->iommu_fwspec && dev->iommu_fwspec->ops)
+   ops = dev->iommu_fwspec->ops;
+
+   if (ops && ops->remove_device && dev->iommu_group)
+   ops->remove_device(dev);
+}
+
 const struct iommu_ops *of_iommu_configure(struct device *dev,
   struct device_node *master_np)
 {
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 064c818105bd..e13cb7914dbe 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -177,6 +177,7 @@ EXPORT_SYMBOL_GPL(of_dma_configure);
 void of_dma_deconfigure(struct device *dev)
 {
arch_teardown_dma_ops(dev);
+   of_iommu_deconfigure(dev);
 }
 
 int of_device_register(struct platform_device *pdev)
diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
index 4fa654e4b5a9..3d4c22e95c0c 100644
--- a/include/linux/of_iommu.h
+++ b/include/linux/of_iommu.h
@@ -15,6 +15,8 @@ extern int of_get_dma_window(struct device_node *dn, const 
char *prefix,
 extern const struct iommu_ops *of_iommu_configure(struct device *dev,
struct device_node *master_np);
 
+extern void of_iommu_deconfigure(struct device *dev);
+
 #else
 
 static inline int of_get_dma_window(struct device_node *dn, const char *prefix,
@@ -30,6 +32,9 @@ static inline const struct iommu_ops 
*of_iommu_configure(struct device *dev,
return NULL;
 }
 
+static inline void of_iommu_deconfigure(struct device *dev)
+{ }
+
 #endif /* CONFIG_OF_IOMMU */
 
 extern struct of_device_id __iommu_of_table;
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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

Re: dma-mapping: clearing GFP_ZERO flag caused crashes of Ethernet on arc/hsdk board.

[h...@lst.de]

> > The logical question is why?
> 
> 1. See that's another platform with ARC core so maybe in case of ARM
>DMA allocator already zeroes pages regardless provided flags -
>personally I didn't check that.

Yes, most architectures always clear memory returned by dma_alloc*.
Looks like a few don't and my commit got them in trouble.  As usual
I'd prefer to match x86 semantics for now to avoid problems.

I'll send patches for arc and s390 which seem to be actually used
holdouts, and will look if anyone else is also affected.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v9 5/5] iommu/arm-smmu: Add support for qcom,smmu-v2 variant

[Yisheng Xie]

Hi Vivek,

On 2018/3/28 12:37, Vivek Gautam wrote:
> Hi Yisheng
> 
> 
> On 3/28/2018 6:54 AM, Yisheng Xie wrote:
>> Hi Vivek,
>>
>> On 2018/3/13 16:55, Vivek Gautam wrote:
>>> +- power-domains:  Specifiers for power domains required to be powered on 
>>> for
>>> +  the SMMU to operate, as per generic power domain 
>>> bindings.
>>> +
>> In this patchset, power-domains is not used right? And you just do the clock 
>> gating,
>> but not power gating, right?
> 
> We are handling the power-domains too. Please see the example in this binding 
> doc.

I see, but I do not find the point in code of these patchset, do you mean 
PMIC(e.g mmcc)
will gate the power domain of SMMU(e.g. MDSS_GDSC of mmcc) when PMIC suspend?

> 
>>
>> Another question is if smmu do power gating, it will reset some of its 
>> registers, so
>> it need save at suspend and restore at resume, right?
> 
> Qualcomm implementation of the arm-smmu has the retenetion enabled. So the 
> smmu doesn't
> loose state when power is pulled out of it.
> And now we are just selectively enabling the runtime pm. So only the 
> platforms that can really
> support runtime pm can enable it.

Get it, thanks for your explain.

Thanks
Yisheng
> 
> Thanks
> Vivek
>>
>> Thanks
>> Yisheng
>>
> 
> 
> 

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