[PATCH v5 5/6] dmaengine: pl330: Make sure microcode is privileged

2016-07-27 Thread Mitchel Humpherys
The PL330 performs privileged instruction fetches.  This can result in
SMMU permission faults on SMMUs that implement the ARMv8 VMSA, which
specifies that mappings that are writeable at one execution level shall
not be executable at any higher-privileged level.  Fix this by using the
DMA_ATTR_PRIVILEGED attribute, which will ensure that the microcode
IOMMU mapping is only accessible to the privileged level.

Cc: Dan Williams 
Cc: Vinod Koul 
Reviewed-by: Robin Murphy 
Tested-by: Robin Murphy 
Signed-off-by: Mitchel Humpherys 
---

Notes:
v3..v4

  - Reworked against the new dma attrs format.

 drivers/dma/pl330.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 4fc3ffbd5ca0..8cd624fc3760 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -1854,14 +1854,16 @@ static int dmac_alloc_resources(struct pl330_dmac 
*pl330)
 {
int chans = pl330->pcfg.num_chan;
int ret;
+   unsigned long dma_attrs = DMA_ATTR_PRIVILEGED;
 
/*
 * Alloc MicroCode buffer for 'chans' Channel threads.
 * A channel's buffer offset is (Channel_Id * MCODE_BUFF_PERCHAN)
 */
-   pl330->mcode_cpu = dma_alloc_coherent(pl330->ddma.dev,
+   pl330->mcode_cpu = dma_alloc_attrs(pl330->ddma.dev,
chans * pl330->mcbufsz,
-   &pl330->mcode_bus, GFP_KERNEL);
+   &pl330->mcode_bus, GFP_KERNEL,
+   dma_attrs);
if (!pl330->mcode_cpu) {
dev_err(pl330->ddma.dev, "%s:%d Can't allocate memory!\n",
__func__, __LINE__);
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v5 6/6] Revert "iommu/arm-smmu: Treat all device transactions as unprivileged"

2016-07-27 Thread Mitchel Humpherys
This reverts commit d346180e70b9 ("iommu/arm-smmu: Treat all device
transactions as unprivileged") since some platforms actually make use of
privileged transactions.

Reviewed-by: Robin Murphy 
Tested-by: Robin Murphy 
Signed-off-by: Mitchel Humpherys 
---

Notes:
v2..v3

  - Moved to the end of the series.

 drivers/iommu/arm-smmu.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 4f49fe29f202..46059b06f48d 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -178,9 +178,6 @@
 #define S2CR_TYPE_BYPASS   (1 << S2CR_TYPE_SHIFT)
 #define S2CR_TYPE_FAULT(2 << S2CR_TYPE_SHIFT)
 
-#define S2CR_PRIVCFG_SHIFT 24
-#define S2CR_PRIVCFG_UNPRIV(2 << S2CR_PRIVCFG_SHIFT)
-
 /* Context bank attribute registers */
 #define ARM_SMMU_GR1_CBAR(n)   (0x0 + ((n) << 2))
 #define CBAR_VMID_SHIFT0
@@ -1175,7 +1172,7 @@ static int arm_smmu_domain_add_master(struct 
arm_smmu_domain *smmu_domain,
u32 idx, s2cr;
 
idx = cfg->smrs ? cfg->smrs[i].idx : cfg->streamids[i];
-   s2cr = S2CR_TYPE_TRANS | S2CR_PRIVCFG_UNPRIV |
+   s2cr = S2CR_TYPE_TRANS |
   (smmu_domain->cfg.cbndx << S2CR_CBNDX_SHIFT);
writel_relaxed(s2cr, gr0_base + ARM_SMMU_GR0_S2CR(idx));
}
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v5 1/6] iommu: add IOMMU_PRIV attribute

2016-07-27 Thread Mitchel Humpherys
Add the IOMMU_PRIV attribute, which is used to indicate privileged
mappings.

Reviewed-by: Robin Murphy 
Tested-by: Robin Murphy 
Signed-off-by: Mitchel Humpherys 
---

Notes:
v2..v3

  - Added comment

 include/linux/iommu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index a35fb8b42e1a..35804abbd6cf 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -31,6 +31,7 @@
 #define IOMMU_CACHE(1 << 2) /* DMA cache coherency */
 #define IOMMU_NOEXEC   (1 << 3)
 #define IOMMU_MMIO (1 << 4) /* e.g. things like MSI doorbells */
+#define IOMMU_PRIV (1 << 5) /* privileged */
 
 struct iommu_ops;
 struct iommu_group;
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v5 2/6] iommu/io-pgtable-arm: add support for the IOMMU_PRIV flag

2016-07-27 Thread Mitchel Humpherys
From: Jeremy Gebben 

Allow the creation of privileged mode mappings, for stage 1 only.

Reviewed-by: Robin Murphy 
Tested-by: Robin Murphy 
Signed-off-by: Jeremy Gebben 
---

Notes:
v2..v3

  - Use existing bit definitions.

 drivers/iommu/io-pgtable-arm.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index f5c90e1366ce..69ba83a135f1 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -350,11 +350,14 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct 
arm_lpae_io_pgtable *data,
 
if (data->iop.fmt == ARM_64_LPAE_S1 ||
data->iop.fmt == ARM_32_LPAE_S1) {
-   pte = ARM_LPAE_PTE_AP_UNPRIV | ARM_LPAE_PTE_nG;
+   pte = ARM_LPAE_PTE_nG;
 
if (!(prot & IOMMU_WRITE) && (prot & IOMMU_READ))
pte |= ARM_LPAE_PTE_AP_RDONLY;
 
+   if (!(prot & IOMMU_PRIV))
+   pte |= ARM_LPAE_PTE_AP_UNPRIV;
+
if (prot & IOMMU_MMIO)
pte |= (ARM_LPAE_MAIR_ATTR_IDX_DEV
<< ARM_LPAE_PTE_ATTRINDX_SHIFT);
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v5 4/6] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED

2016-07-27 Thread Mitchel Humpherys
The newly added DMA_ATTR_PRIVILEGED is useful for creating mappings that
are only accessible to privileged DMA engines.  Implement it in
dma-iommu.c so that the ARM64 DMA IOMMU mapper can make use of it.

Reviewed-by: Robin Murphy 
Tested-by: Robin Murphy 
Signed-off-by: Mitchel Humpherys 
---

Notes:
v4..v5

  - Simplified (suggested by Robin Murphy)

v3..v4

  - Reworked against the new dma attrs format

v2..v3

  - Renamed and redocumented dma_direction_to_prot.
  - Dropped the stuff making all privileged mappings read-only.

 arch/arm64/mm/dma-mapping.c |  6 +++---
 drivers/iommu/dma-iommu.c   | 10 --
 include/linux/dma-iommu.h   |  3 ++-
 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index c4284c432ae8..1c6f85c56115 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -556,7 +556,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t 
size,
 unsigned long attrs)
 {
bool coherent = is_device_dma_coherent(dev);
-   int ioprot = dma_direction_to_prot(DMA_BIDIRECTIONAL, coherent);
+   int ioprot = dma_info_to_prot(DMA_BIDIRECTIONAL, coherent, attrs);
size_t iosize = size;
void *addr;
 
@@ -710,7 +710,7 @@ static dma_addr_t __iommu_map_page(struct device *dev, 
struct page *page,
   unsigned long attrs)
 {
bool coherent = is_device_dma_coherent(dev);
-   int prot = dma_direction_to_prot(dir, coherent);
+   int prot = dma_info_to_prot(dir, coherent, attrs);
dma_addr_t dev_addr = iommu_dma_map_page(dev, page, offset, size, prot);
 
if (!iommu_dma_mapping_error(dev, dev_addr) &&
@@ -768,7 +768,7 @@ static int __iommu_map_sg_attrs(struct device *dev, struct 
scatterlist *sgl,
__iommu_sync_sg_for_device(dev, sgl, nelems, dir);
 
return iommu_dma_map_sg(dev, sgl, nelems,
-   dma_direction_to_prot(dir, coherent));
+   dma_info_to_prot(dir, coherent, attrs));
 }
 
 static void __iommu_unmap_sg_attrs(struct device *dev,
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 08a1e2f3690f..279764305005 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -129,16 +129,22 @@ int iommu_dma_init_domain(struct iommu_domain *domain, 
dma_addr_t base, u64 size
 EXPORT_SYMBOL(iommu_dma_init_domain);
 
 /**
- * dma_direction_to_prot - Translate DMA API directions to IOMMU API page flags
+ * dma_info_to_prot - Translate DMA API directions and attributes to IOMMU API
+ *page flags.
  * @dir: Direction of DMA transfer
  * @coherent: Is the DMA master cache-coherent?
+ * @attrs: DMA attributes for the mapping
  *
  * Return: corresponding IOMMU API page protection flags
  */
-int dma_direction_to_prot(enum dma_data_direction dir, bool coherent)
+int dma_info_to_prot(enum dma_data_direction dir, bool coherent,
+unsigned long attrs)
 {
int prot = coherent ? IOMMU_CACHE : 0;
 
+   if (attrs & DMA_ATTR_PRIVILEGED)
+   prot |= IOMMU_PRIV;
+
switch (dir) {
case DMA_BIDIRECTIONAL:
return prot | IOMMU_READ | IOMMU_WRITE;
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 81c5c8d167ad..b367613d49ba 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -32,7 +32,8 @@ void iommu_put_dma_cookie(struct iommu_domain *domain);
 int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, u64 
size);
 
 /* General helpers for DMA-API <-> IOMMU-API interaction */
-int dma_direction_to_prot(enum dma_data_direction dir, bool coherent);
+int dma_info_to_prot(enum dma_data_direction dir, bool coherent,
+unsigned long attrs);
 
 /*
  * These implement the bulk of the relevant DMA mapping callbacks, but require
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v5 0/6] Add support for privileged mappings

2016-07-27 Thread Mitchel Humpherys
The following patch to the ARM SMMU driver:

commit d346180e70b91b3d5a1ae7e5603e65593d4622bc
Author: Robin Murphy 
Date:   Tue Jan 26 18:06:34 2016 +

iommu/arm-smmu: Treat all device transactions as unprivileged

started forcing all SMMU transactions to come through as "unprivileged".
The rationale given was that:

  (1) There is no way in the IOMMU API to even request privileged mappings.

  (2) It's difficult to implement a DMA mapper that correctly models the
  ARM VMSAv8 behavior of unprivileged-writeable =>
  privileged-execute-never.

This series rectifies (1) by introducing an IOMMU API for privileged
mappings and implements it in io-pgtable-arm.

This series rectifies (2) by introducing a new dma attribute
(DMA_ATTR_PRIVILEGED) for users of the DMA API that need privileged
mappings which are inaccessible to lesser-privileged execution levels, and
implements it in the arm64 IOMMU DMA mapper.  The one known user (pl330.c)
is converted over to the new attribute.

Jordan and Jeremy can provide more info on the use case if needed, but the
high level is that it's a security feature to prevent attacks such as [1].

Joerg, the v3 series was previously acked by Will [2].  He also recommended
that we take all of this through your tree since it's touching multiple
subsystems [3].  Can you please take a look?  Thanks!

It's also worth noting that I will no longer be at QuIC as of this coming
Monday, but the fine folks with codeaurora email addresses Cc'd here can
provide help getting these through once I'm gone.

[1] https://github.com/robclark/kilroy
[2] http://article.gmane.org/gmane.linux.kernel.iommu/14617
[3] http://article.gmane.org/gmane.linux.kernel/2272531

Changelog:

  v4..v5

- Simplified patch 4/6 (suggested by Robin Murphy).

  v3..v4

- Rebased and reworked on linux next due to the dma attrs rework going
  on over there.  Patches changed: 3/6, 4/6, and 5/6.

  v2..v3

- Incorporated feedback from Robin:
  * Various comments and re-wordings.
  * Use existing bit definitions for IOMMU_PRIV implementation
in io-pgtable-arm.
  * Renamed and redocumented dma_direction_to_prot.
  * Don't worry about executability in new DMA attr.

  v1..v2

- Added a new DMA attribute to make executable privileged mappings
  work, and use that in the pl330 driver (suggested by Will).


Jeremy Gebben (1):
  iommu/io-pgtable-arm: add support for the IOMMU_PRIV flag

Mitchel Humpherys (5):
  iommu: add IOMMU_PRIV attribute
  common: DMA-mapping: add DMA_ATTR_PRIVILEGED attribute
  arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED
  dmaengine: pl330: Make sure microcode is privileged
  Revert "iommu/arm-smmu: Treat all device transactions as unprivileged"

 Documentation/DMA-attributes.txt | 10 ++
 arch/arm64/mm/dma-mapping.c  |  6 +++---
 drivers/dma/pl330.c  |  6 --
 drivers/iommu/arm-smmu.c |  5 +
 drivers/iommu/dma-iommu.c| 10 --
 drivers/iommu/io-pgtable-arm.c   |  5 -
 include/linux/dma-iommu.h|  3 ++-
 include/linux/dma-mapping.h  |  6 ++
 include/linux/iommu.h|  1 +
 9 files changed, 39 insertions(+), 13 deletions(-)

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v5 3/6] common: DMA-mapping: add DMA_ATTR_PRIVILEGED attribute

2016-07-27 Thread Mitchel Humpherys
This patch adds the DMA_ATTR_PRIVILEGED attribute to the DMA-mapping
subsystem.

Some advanced peripherals such as remote processors and GPUs perform
accesses to DMA buffers in both privileged "supervisor" and unprivileged
"user" modes.  This attribute is used to indicate to the DMA-mapping
subsystem that the buffer is fully accessible at the elevated privilege
level (and ideally inaccessible or at least read-only at the
lesser-privileged levels).

Cc: linux-...@vger.kernel.org
Reviewed-by: Robin Murphy 
Tested-by: Robin Murphy 
Signed-off-by: Mitchel Humpherys 
---

Notes:
v2..v3

  - Not worrying about executability.

 Documentation/DMA-attributes.txt | 10 ++
 include/linux/dma-mapping.h  |  6 ++
 2 files changed, 16 insertions(+)

diff --git a/Documentation/DMA-attributes.txt b/Documentation/DMA-attributes.txt
index 2d455a5cf671..7728bda278c9 100644
--- a/Documentation/DMA-attributes.txt
+++ b/Documentation/DMA-attributes.txt
@@ -126,3 +126,13 @@ means that we won't try quite as hard to get them.
 
 NOTE: At the moment DMA_ATTR_ALLOC_SINGLE_PAGES is only implemented on ARM,
 though ARM64 patches will likely be posted soon.
+
+DMA_ATTR_PRIVILEGED
+--
+
+Some advanced peripherals such as remote processors and GPUs perform
+accesses to DMA buffers in both privileged "supervisor" and unprivileged
+"user" modes.  This attribute is used to indicate to the DMA-mapping
+subsystem that the buffer is fully accessible at the elevated privilege
+level (and ideally inaccessible or at least read-only at the
+lesser-privileged levels).
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 66533e18276c..73f477609262 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -56,6 +56,12 @@
  * that gives better TLB efficiency.
  */
 #define DMA_ATTR_ALLOC_SINGLE_PAGES(1UL << 7)
+/*
+ * DMA_ATTR_PRIVILEGED: used to indicate that the buffer is fully
+ * accessible at an elevated privilege level (and ideally inaccessible or
+ * at least read-only at lesser-privileged levels).
+ */
+#define DMA_ATTR_PRIVILEGED(1UL << 8)
 
 /*
  * A dma_addr_t can hold any valid DMA or bus address for the platform.
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



Re: [PATCH v4 4/6] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED

2016-07-27 Thread Mitchel Humpherys
On Tue, Jul 26 2016 at 04:05:17 PM, Robin Murphy  wrote:
> + if (attrs & DMA_ATTR_PRIVILEGED)
> + prot |= IOMMU_PRIV;
> +
>
> then drop the rest of the changes to the switch statement below. It's
> taken me an embarrassingly long time to work out why things were blowing
> up in __iommu_sync_single_for_device() all with a VA of phys_to_virt(0) ;)

Ah yes, that is much nicer!  Nice catch...

> With that change, for the whole series:
>
> Reviewed-by: Robin Murphy 
> Tested-by: Robin Murphy 
>
> I guess at this point it may be worth waiting to repost based on -rc1.
> Be sure to CC patch 5 to Vinod as the current dmaengine maintainer, as
> it's his ack we'll need on that.

Thanks for the review and test.  I'll go ahead and send a v5 series
since I'm actually leaving QuIC this Friday and so I won't be around to
send it next week...


-Mitch

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


[PATCH v4 5/6] dmaengine: pl330: Make sure microcode is privileged

2016-07-25 Thread Mitchel Humpherys
The PL330 performs privileged instruction fetches.  This can result in
SMMU permission faults on SMMUs that implement the ARMv8 VMSA, which
specifies that mappings that are writeable at one execution level shall
not be executable at any higher-privileged level.  Fix this by using the
DMA_ATTR_PRIVILEGED attribute, which will ensure that the microcode
IOMMU mapping is only accessible to the privileged level.

Cc: Dan Williams 
Cc: Jassi Brar 
Signed-off-by: Mitchel Humpherys 
---

Notes:
v3..v4

  - Reworked against the new dma attrs format.

 drivers/dma/pl330.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 4fc3ffbd5ca0..8cd624fc3760 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -1854,14 +1854,16 @@ static int dmac_alloc_resources(struct pl330_dmac 
*pl330)
 {
int chans = pl330->pcfg.num_chan;
int ret;
+   unsigned long dma_attrs = DMA_ATTR_PRIVILEGED;
 
/*
 * Alloc MicroCode buffer for 'chans' Channel threads.
 * A channel's buffer offset is (Channel_Id * MCODE_BUFF_PERCHAN)
 */
-   pl330->mcode_cpu = dma_alloc_coherent(pl330->ddma.dev,
+   pl330->mcode_cpu = dma_alloc_attrs(pl330->ddma.dev,
chans * pl330->mcbufsz,
-   &pl330->mcode_bus, GFP_KERNEL);
+   &pl330->mcode_bus, GFP_KERNEL,
+   dma_attrs);
if (!pl330->mcode_cpu) {
dev_err(pl330->ddma.dev, "%s:%d Can't allocate memory!\n",
__func__, __LINE__);
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v4 6/6] Revert "iommu/arm-smmu: Treat all device transactions as unprivileged"

2016-07-25 Thread Mitchel Humpherys
This reverts commit d346180e70b9 ("iommu/arm-smmu: Treat all device
transactions as unprivileged") since some platforms actually make use of
privileged transactions.

Signed-off-by: Mitchel Humpherys 
---

Notes:
v2..v3

  - Moved to the end of the series.

 drivers/iommu/arm-smmu.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 4f49fe29f202..46059b06f48d 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -178,9 +178,6 @@
 #define S2CR_TYPE_BYPASS   (1 << S2CR_TYPE_SHIFT)
 #define S2CR_TYPE_FAULT(2 << S2CR_TYPE_SHIFT)
 
-#define S2CR_PRIVCFG_SHIFT 24
-#define S2CR_PRIVCFG_UNPRIV(2 << S2CR_PRIVCFG_SHIFT)
-
 /* Context bank attribute registers */
 #define ARM_SMMU_GR1_CBAR(n)   (0x0 + ((n) << 2))
 #define CBAR_VMID_SHIFT0
@@ -1175,7 +1172,7 @@ static int arm_smmu_domain_add_master(struct 
arm_smmu_domain *smmu_domain,
u32 idx, s2cr;
 
idx = cfg->smrs ? cfg->smrs[i].idx : cfg->streamids[i];
-   s2cr = S2CR_TYPE_TRANS | S2CR_PRIVCFG_UNPRIV |
+   s2cr = S2CR_TYPE_TRANS |
   (smmu_domain->cfg.cbndx << S2CR_CBNDX_SHIFT);
writel_relaxed(s2cr, gr0_base + ARM_SMMU_GR0_S2CR(idx));
}
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v4 1/6] iommu: add IOMMU_PRIV attribute

2016-07-25 Thread Mitchel Humpherys
Add the IOMMU_PRIV attribute, which is used to indicate privileged
mappings.

Signed-off-by: Mitchel Humpherys 
---

Notes:
v2..v3

  - Added comment

 include/linux/iommu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index a35fb8b42e1a..35804abbd6cf 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -31,6 +31,7 @@
 #define IOMMU_CACHE(1 << 2) /* DMA cache coherency */
 #define IOMMU_NOEXEC   (1 << 3)
 #define IOMMU_MMIO (1 << 4) /* e.g. things like MSI doorbells */
+#define IOMMU_PRIV (1 << 5) /* privileged */
 
 struct iommu_ops;
 struct iommu_group;
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v4 2/6] iommu/io-pgtable-arm: add support for the IOMMU_PRIV flag

2016-07-25 Thread Mitchel Humpherys
From: Jeremy Gebben 

Allow the creation of privileged mode mappings, for stage 1 only.

Signed-off-by: Jeremy Gebben 
---

Notes:
v2..v3

  - Use existing bit definitions.

 drivers/iommu/io-pgtable-arm.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index f5c90e1366ce..69ba83a135f1 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -350,11 +350,14 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct 
arm_lpae_io_pgtable *data,
 
if (data->iop.fmt == ARM_64_LPAE_S1 ||
data->iop.fmt == ARM_32_LPAE_S1) {
-   pte = ARM_LPAE_PTE_AP_UNPRIV | ARM_LPAE_PTE_nG;
+   pte = ARM_LPAE_PTE_nG;
 
if (!(prot & IOMMU_WRITE) && (prot & IOMMU_READ))
pte |= ARM_LPAE_PTE_AP_RDONLY;
 
+   if (!(prot & IOMMU_PRIV))
+   pte |= ARM_LPAE_PTE_AP_UNPRIV;
+
if (prot & IOMMU_MMIO)
pte |= (ARM_LPAE_MAIR_ATTR_IDX_DEV
<< ARM_LPAE_PTE_ATTRINDX_SHIFT);
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v4 4/6] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED

2016-07-25 Thread Mitchel Humpherys
The newly added DMA_ATTR_PRIVILEGED is useful for creating mappings that
are only accessible to privileged DMA engines.  Implement it in
dma-iommu.c so that the ARM64 DMA IOMMU mapper can make use of it.

Signed-off-by: Mitchel Humpherys 
---

Notes:
v3..v4

  - Reworked against the new dma attrs format

v2..v3

  - Renamed and redocumented dma_direction_to_prot.
  - Dropped the stuff making all privileged mappings read-only.

 arch/arm64/mm/dma-mapping.c |  6 +++---
 drivers/iommu/dma-iommu.c   | 16 +++-
 include/linux/dma-iommu.h   |  3 ++-
 3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index c4284c432ae8..1c6f85c56115 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -556,7 +556,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t 
size,
 unsigned long attrs)
 {
bool coherent = is_device_dma_coherent(dev);
-   int ioprot = dma_direction_to_prot(DMA_BIDIRECTIONAL, coherent);
+   int ioprot = dma_info_to_prot(DMA_BIDIRECTIONAL, coherent, attrs);
size_t iosize = size;
void *addr;
 
@@ -710,7 +710,7 @@ static dma_addr_t __iommu_map_page(struct device *dev, 
struct page *page,
   unsigned long attrs)
 {
bool coherent = is_device_dma_coherent(dev);
-   int prot = dma_direction_to_prot(dir, coherent);
+   int prot = dma_info_to_prot(dir, coherent, attrs);
dma_addr_t dev_addr = iommu_dma_map_page(dev, page, offset, size, prot);
 
if (!iommu_dma_mapping_error(dev, dev_addr) &&
@@ -768,7 +768,7 @@ static int __iommu_map_sg_attrs(struct device *dev, struct 
scatterlist *sgl,
__iommu_sync_sg_for_device(dev, sgl, nelems, dir);
 
return iommu_dma_map_sg(dev, sgl, nelems,
-   dma_direction_to_prot(dir, coherent));
+   dma_info_to_prot(dir, coherent, attrs));
 }
 
 static void __iommu_unmap_sg_attrs(struct device *dev,
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 08a1e2f3690f..5e1e495b35f8 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -129,26 +129,32 @@ int iommu_dma_init_domain(struct iommu_domain *domain, 
dma_addr_t base, u64 size
 EXPORT_SYMBOL(iommu_dma_init_domain);
 
 /**
- * dma_direction_to_prot - Translate DMA API directions to IOMMU API page flags
+ * dma_info_to_prot - Translate DMA API directions and attributes to IOMMU API
+ *page flags.
  * @dir: Direction of DMA transfer
  * @coherent: Is the DMA master cache-coherent?
+ * @attrs: DMA attributes for the mapping
  *
  * Return: corresponding IOMMU API page protection flags
  */
-int dma_direction_to_prot(enum dma_data_direction dir, bool coherent)
+int dma_info_to_prot(enum dma_data_direction dir, bool coherent,
+unsigned long attrs)
 {
int prot = coherent ? IOMMU_CACHE : 0;
 
switch (dir) {
case DMA_BIDIRECTIONAL:
-   return prot | IOMMU_READ | IOMMU_WRITE;
+   prot |= IOMMU_READ | IOMMU_WRITE;
case DMA_TO_DEVICE:
-   return prot | IOMMU_READ;
+   prot |= IOMMU_READ;
case DMA_FROM_DEVICE:
-   return prot | IOMMU_WRITE;
+   prot |= IOMMU_WRITE;
default:
return 0;
}
+   if (attrs & DMA_ATTR_PRIVILEGED)
+   prot |= IOMMU_PRIV;
+   return prot;
 }
 
 static struct iova *__alloc_iova(struct iova_domain *iovad, size_t size,
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 81c5c8d167ad..b367613d49ba 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -32,7 +32,8 @@ void iommu_put_dma_cookie(struct iommu_domain *domain);
 int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, u64 
size);
 
 /* General helpers for DMA-API <-> IOMMU-API interaction */
-int dma_direction_to_prot(enum dma_data_direction dir, bool coherent);
+int dma_info_to_prot(enum dma_data_direction dir, bool coherent,
+unsigned long attrs);
 
 /*
  * These implement the bulk of the relevant DMA mapping callbacks, but require
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v4 3/6] common: DMA-mapping: add DMA_ATTR_PRIVILEGED attribute

2016-07-25 Thread Mitchel Humpherys
This patch adds the DMA_ATTR_PRIVILEGED attribute to the DMA-mapping
subsystem.

Some advanced peripherals such as remote processors and GPUs perform
accesses to DMA buffers in both privileged "supervisor" and unprivileged
"user" modes.  This attribute is used to indicate to the DMA-mapping
subsystem that the buffer is fully accessible at the elevated privilege
level (and ideally inaccessible or at least read-only at the
lesser-privileged levels).

Cc: linux-...@vger.kernel.org
Signed-off-by: Mitchel Humpherys 
---

Notes:
v3..v4

  - Reworked against the new dma attrs format

v2..v3

  - Not worrying about executability.

 Documentation/DMA-attributes.txt | 10 ++
 include/linux/dma-mapping.h  |  6 ++
 2 files changed, 16 insertions(+)

diff --git a/Documentation/DMA-attributes.txt b/Documentation/DMA-attributes.txt
index 2d455a5cf671..7728bda278c9 100644
--- a/Documentation/DMA-attributes.txt
+++ b/Documentation/DMA-attributes.txt
@@ -126,3 +126,13 @@ means that we won't try quite as hard to get them.
 
 NOTE: At the moment DMA_ATTR_ALLOC_SINGLE_PAGES is only implemented on ARM,
 though ARM64 patches will likely be posted soon.
+
+DMA_ATTR_PRIVILEGED
+--
+
+Some advanced peripherals such as remote processors and GPUs perform
+accesses to DMA buffers in both privileged "supervisor" and unprivileged
+"user" modes.  This attribute is used to indicate to the DMA-mapping
+subsystem that the buffer is fully accessible at the elevated privilege
+level (and ideally inaccessible or at least read-only at the
+lesser-privileged levels).
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 66533e18276c..73f477609262 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -56,6 +56,12 @@
  * that gives better TLB efficiency.
  */
 #define DMA_ATTR_ALLOC_SINGLE_PAGES(1UL << 7)
+/*
+ * DMA_ATTR_PRIVILEGED: used to indicate that the buffer is fully
+ * accessible at an elevated privilege level (and ideally inaccessible or
+ * at least read-only at lesser-privileged levels).
+ */
+#define DMA_ATTR_PRIVILEGED(1UL << 8)
 
 /*
  * A dma_addr_t can hold any valid DMA or bus address for the platform.
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v4 0/6] Add support for privileged mappings

2016-07-25 Thread Mitchel Humpherys
The following patch to the ARM SMMU driver:

commit d346180e70b91b3d5a1ae7e5603e65593d4622bc
Author: Robin Murphy 
Date:   Tue Jan 26 18:06:34 2016 +

iommu/arm-smmu: Treat all device transactions as unprivileged

started forcing all SMMU transactions to come through as "unprivileged".
The rationale given was that:

  (1) There is no way in the IOMMU API to even request privileged mappings.

  (2) It's difficult to implement a DMA mapper that correctly models the
  ARM VMSAv8 behavior of unprivileged-writeable =>
  privileged-execute-never.

This series rectifies (1) by introducing an IOMMU API for privileged
mappings and implements it in io-pgtable-arm.

This series rectifies (2) by introducing a new dma attribute
(DMA_ATTR_PRIVILEGED) for users of the DMA API that need privileged
mappings which are inaccessible to lesser-privileged execution levels, and
implements it in the arm64 IOMMU DMA mapper.  The one known user (pl330.c)
is converted over to the new attribute.

Jordan and Jeremy can provide more info on the use case if needed, but the
high level is that it's a security feature to prevent attacks such as [1].

Joerg, the v3 series was previously acked by Will [2].  He also recommended
that we take all of this through your tree since it's touching multiple
subsystems [3].  Can you please take a look?  Thanks!

[1] https://github.com/robclark/kilroy
[2] http://article.gmane.org/gmane.linux.kernel.iommu/14617
[3] http://article.gmane.org/gmane.linux.kernel/2272531

Changelog:

  v3..v4

- Rebased and reworked on linux next due to the dma attrs rework going
  on over there.  Patches changed: 3/6, 4/6, and 5/6.

  v2..v3

- Incorporated feedback from Robin:
  * Various comments and re-wordings.
  * Use existing bit definitions for IOMMU_PRIV implementation
in io-pgtable-arm.
  * Renamed and redocumented dma_direction_to_prot.
  * Don't worry about executability in new DMA attr.

  v1..v2

- Added a new DMA attribute to make executable privileged mappings
  work, and use that in the pl330 driver (suggested by Will).


Jeremy Gebben (1):
  iommu/io-pgtable-arm: add support for the IOMMU_PRIV flag

Mitchel Humpherys (5):
  iommu: add IOMMU_PRIV attribute
  common: DMA-mapping: add DMA_ATTR_PRIVILEGED attribute
  arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED
  dmaengine: pl330: Make sure microcode is privileged
  Revert "iommu/arm-smmu: Treat all device transactions as unprivileged"

 Documentation/DMA-attributes.txt | 10 ++
 arch/arm64/mm/dma-mapping.c  |  6 +++---
 drivers/dma/pl330.c  |  6 --
 drivers/iommu/arm-smmu.c |  5 +
 drivers/iommu/dma-iommu.c| 16 +++-
 drivers/iommu/io-pgtable-arm.c   |  5 -
 include/linux/dma-iommu.h|  3 ++-
 include/linux/dma-mapping.h  |  6 ++
 include/linux/iommu.h|  1 +
 9 files changed, 42 insertions(+), 16 deletions(-)

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



Re: [PATCH v3 0/6] Add support for privileged mappings

2016-07-25 Thread Mitchel Humpherys
On Mon, Jul 25 2016 at 10:50:13 AM, Will Deacon  wrote:
> On Fri, Jul 22, 2016 at 01:39:45PM -0700, Mitchel Humpherys wrote:
>> On Fri, Jul 22 2016 at 05:51:07 PM, Will Deacon  wrote:
>> > On Tue, Jul 19, 2016 at 01:36:49PM -0700, Mitchel Humpherys wrote:
>> >> The following patch to the ARM SMMU driver:
>> >> 
>> >> commit d346180e70b91b3d5a1ae7e5603e65593d4622bc
>> >> Author: Robin Murphy 
>> >> Date:   Tue Jan 26 18:06:34 2016 +
>> >> 
>> >> iommu/arm-smmu: Treat all device transactions as unprivileged
>> >> 
>> >> started forcing all SMMU transactions to come through as "unprivileged".
>> >> The rationale given was that:
>> >> 
>> >>   (1) There is no way in the IOMMU API to even request privileged 
>> >> mappings.
>> >> 
>> >>   (2) It's difficult to implement a DMA mapper that correctly models the
>> >>   ARM VMSAv8 behavior of unprivileged-writeable =>
>> >>   privileged-execute-never.
>> >> 
>> >> This series rectifies (1) by introducing an IOMMU API for privileged
>> >> mappings and implements it in io-pgtable-arm.
>> >> 
>> >> This series rectifies (2) by introducing a new dma attribute
>> >> (DMA_ATTR_PRIVILEGED) for users of the DMA API that need privileged
>> >> mappings which are inaccessible to lesser-privileged execution levels, and
>> >> implements it in the arm64 IOMMU DMA mapper.  The one known user (pl330.c)
>> >> is converted over to the new attribute.
>> >> 
>> >> Jordan and Jeremy can provide more info on the use case if needed, but the
>> >> high level is that it's a security feature to prevent attacks such as [1].
>> >
>> > This all looks good to me:
>> >
>> > Acked-by: Will Deacon 
>> >
>> > It looks pretty fiddly to merge, however. How are you planning to get
>> > this upstream?
>> 
>> Fiddly in what way?  Do you mean in relation to "dma-mapping: Use
>> unsigned long for dma_attrs" [1]?  I admit I wasn't aware of that
>> activity until Robin mentioned it.  It looks like it's merged on
>> next/master, shall I rebase/rework on that and resend?
>
> Fiddly in that it touches multiple subsystems. I guess routing it via
> the iommu tree (Joerg) might be the best bet.

Sounds good.  I'm going to rebase on linux-next as well anyways to get
the new dma attrs format and resend.


-Mitch

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v3 0/6] Add support for privileged mappings

2016-07-22 Thread Mitchel Humpherys
On Fri, Jul 22 2016 at 05:51:07 PM, Will Deacon  wrote:
> On Tue, Jul 19, 2016 at 01:36:49PM -0700, Mitchel Humpherys wrote:
>> The following patch to the ARM SMMU driver:
>> 
>> commit d346180e70b91b3d5a1ae7e5603e65593d4622bc
>> Author: Robin Murphy 
>> Date:   Tue Jan 26 18:06:34 2016 +
>> 
>> iommu/arm-smmu: Treat all device transactions as unprivileged
>> 
>> started forcing all SMMU transactions to come through as "unprivileged".
>> The rationale given was that:
>> 
>>   (1) There is no way in the IOMMU API to even request privileged mappings.
>> 
>>   (2) It's difficult to implement a DMA mapper that correctly models the
>>   ARM VMSAv8 behavior of unprivileged-writeable =>
>>   privileged-execute-never.
>> 
>> This series rectifies (1) by introducing an IOMMU API for privileged
>> mappings and implements it in io-pgtable-arm.
>> 
>> This series rectifies (2) by introducing a new dma attribute
>> (DMA_ATTR_PRIVILEGED) for users of the DMA API that need privileged
>> mappings which are inaccessible to lesser-privileged execution levels, and
>> implements it in the arm64 IOMMU DMA mapper.  The one known user (pl330.c)
>> is converted over to the new attribute.
>> 
>> Jordan and Jeremy can provide more info on the use case if needed, but the
>> high level is that it's a security feature to prevent attacks such as [1].
>
> This all looks good to me:
>
> Acked-by: Will Deacon 
>
> It looks pretty fiddly to merge, however. How are you planning to get
> this upstream?

Fiddly in what way?  Do you mean in relation to "dma-mapping: Use
unsigned long for dma_attrs" [1]?  I admit I wasn't aware of that
activity until Robin mentioned it.  It looks like it's merged on
next/master, shall I rebase/rework on that and resend?

[1] https://lkml.org/lkml/2016/7/13/198


-Mitch

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


[PATCH v3 0/6] Add support for privileged mappings

2016-07-19 Thread Mitchel Humpherys
The following patch to the ARM SMMU driver:

commit d346180e70b91b3d5a1ae7e5603e65593d4622bc
Author: Robin Murphy 
Date:   Tue Jan 26 18:06:34 2016 +

iommu/arm-smmu: Treat all device transactions as unprivileged

started forcing all SMMU transactions to come through as "unprivileged".
The rationale given was that:

  (1) There is no way in the IOMMU API to even request privileged mappings.

  (2) It's difficult to implement a DMA mapper that correctly models the
  ARM VMSAv8 behavior of unprivileged-writeable =>
  privileged-execute-never.

This series rectifies (1) by introducing an IOMMU API for privileged
mappings and implements it in io-pgtable-arm.

This series rectifies (2) by introducing a new dma attribute
(DMA_ATTR_PRIVILEGED) for users of the DMA API that need privileged
mappings which are inaccessible to lesser-privileged execution levels, and
implements it in the arm64 IOMMU DMA mapper.  The one known user (pl330.c)
is converted over to the new attribute.

Jordan and Jeremy can provide more info on the use case if needed, but the
high level is that it's a security feature to prevent attacks such as [1].

[1] https://github.com/robclark/kilroy

Changelog:

  v2..v3

- Incorporated feedback from Robin:
  * Various comments and re-wordings.
  * Use existing bit definitions for IOMMU_PRIV implementation
in io-pgtable-arm.
  * Renamed and redocumented dma_direction_to_prot.
  * Don't worry about executability in new DMA attr.

  v1..v2

- Added a new DMA attribute to make executable privileged mappings
  work, and use that in the pl330 driver (suggested by Will).


Jeremy Gebben (1):
  iommu/io-pgtable-arm: add support for the IOMMU_PRIV flag

Mitchel Humpherys (5):
  iommu: add IOMMU_PRIV attribute
  common: DMA-mapping: add DMA_ATTR_PRIVILEGED attribute
  arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED
  dmaengine: pl330: Make sure microcode is privileged
  Revert "iommu/arm-smmu: Treat all device transactions as unprivileged"

 Documentation/DMA-attributes.txt | 10 ++
 arch/arm64/mm/dma-mapping.c  |  6 +++---
 drivers/dma/pl330.c  |  7 +--
 drivers/iommu/arm-smmu.c |  5 +
 drivers/iommu/dma-iommu.c| 16 +++-
 drivers/iommu/io-pgtable-arm.c   |  5 -
 include/linux/dma-attrs.h|  1 +
 include/linux/dma-iommu.h|  3 ++-
 include/linux/iommu.h|  1 +
 9 files changed, 38 insertions(+), 16 deletions(-)

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v3 2/6] iommu/io-pgtable-arm: add support for the IOMMU_PRIV flag

2016-07-19 Thread Mitchel Humpherys
From: Jeremy Gebben 

Allow the creation of privileged mode mappings, for stage 1 only.

Signed-off-by: Jeremy Gebben 
---

Notes:
v2..v3

  - Use existing bit definitions.

 drivers/iommu/io-pgtable-arm.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index a1ed1b73fed4..2a7d28ab8241 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -350,11 +350,14 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct 
arm_lpae_io_pgtable *data,
 
if (data->iop.fmt == ARM_64_LPAE_S1 ||
data->iop.fmt == ARM_32_LPAE_S1) {
-   pte = ARM_LPAE_PTE_AP_UNPRIV | ARM_LPAE_PTE_nG;
+   pte = ARM_LPAE_PTE_nG;
 
if (!(prot & IOMMU_WRITE) && (prot & IOMMU_READ))
pte |= ARM_LPAE_PTE_AP_RDONLY;
 
+   if (!(prot & IOMMU_PRIV))
+   pte |= ARM_LPAE_PTE_AP_UNPRIV;
+
if (prot & IOMMU_MMIO)
pte |= (ARM_LPAE_MAIR_ATTR_IDX_DEV
<< ARM_LPAE_PTE_ATTRINDX_SHIFT);
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v3 5/6] dmaengine: pl330: Make sure microcode is privileged

2016-07-19 Thread Mitchel Humpherys
The PL330 performs privileged instruction fetches.  This can result in
SMMU permission faults on SMMUs that implement the ARMv8 VMSA, which
specifies that mappings that are writeable at one execution level shall
not be executable at any higher-privileged level.  Fix this by using the
DMA_ATTR_PRIVILEGED attribute, which will ensure that the microcode
IOMMU mapping is only accessible to the privileged level.

Cc: Dan Williams 
Cc: Jassi Brar 
Signed-off-by: Mitchel Humpherys 
---
 drivers/dma/pl330.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 372b4359da97..7297cd1d03c8 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -1854,14 +1854,17 @@ static int dmac_alloc_resources(struct pl330_dmac 
*pl330)
 {
int chans = pl330->pcfg.num_chan;
int ret;
+   DEFINE_DMA_ATTRS(attrs);
 
+   dma_set_attr(DMA_ATTR_PRIVILEGED, &attrs);
/*
 * Alloc MicroCode buffer for 'chans' Channel threads.
 * A channel's buffer offset is (Channel_Id * MCODE_BUFF_PERCHAN)
 */
-   pl330->mcode_cpu = dma_alloc_coherent(pl330->ddma.dev,
+   pl330->mcode_cpu = dma_alloc_attrs(pl330->ddma.dev,
chans * pl330->mcbufsz,
-   &pl330->mcode_bus, GFP_KERNEL);
+   &pl330->mcode_bus, GFP_KERNEL,
+   &attrs);
if (!pl330->mcode_cpu) {
dev_err(pl330->ddma.dev, "%s:%d Can't allocate memory!\n",
__func__, __LINE__);
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v3 4/6] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED

2016-07-19 Thread Mitchel Humpherys
The newly added DMA_ATTR_PRIVILEGED is useful for creating mappings that
are only accessible to privileged DMA engines.  Implement it in
dma-iommu.c so that the ARM64 DMA IOMMU mapper can make use of it.

Signed-off-by: Mitchel Humpherys 
---

Notes:
v2..v3

  - Renamed and redocumented dma_direction_to_prot.
  - Dropped the stuff making all privileged mappings read-only.

 arch/arm64/mm/dma-mapping.c |  6 +++---
 drivers/iommu/dma-iommu.c   | 16 +++-
 include/linux/dma-iommu.h   |  3 ++-
 3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index c566ec83719f..5da8946ead9e 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -543,7 +543,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t 
size,
 struct dma_attrs *attrs)
 {
bool coherent = is_device_dma_coherent(dev);
-   int ioprot = dma_direction_to_prot(DMA_BIDIRECTIONAL, coherent);
+   int ioprot = dma_info_to_prot(DMA_BIDIRECTIONAL, coherent, attrs);
size_t iosize = size;
void *addr;
 
@@ -697,7 +697,7 @@ static dma_addr_t __iommu_map_page(struct device *dev, 
struct page *page,
   struct dma_attrs *attrs)
 {
bool coherent = is_device_dma_coherent(dev);
-   int prot = dma_direction_to_prot(dir, coherent);
+   int prot = dma_info_to_prot(dir, coherent, attrs);
dma_addr_t dev_addr = iommu_dma_map_page(dev, page, offset, size, prot);
 
if (!iommu_dma_mapping_error(dev, dev_addr) &&
@@ -755,7 +755,7 @@ static int __iommu_map_sg_attrs(struct device *dev, struct 
scatterlist *sgl,
__iommu_sync_sg_for_device(dev, sgl, nelems, dir);
 
return iommu_dma_map_sg(dev, sgl, nelems,
-   dma_direction_to_prot(dir, coherent));
+   dma_info_to_prot(dir, coherent, attrs));
 }
 
 static void __iommu_unmap_sg_attrs(struct device *dev,
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index ea5a9ebf0f78..cc2c9cccde1f 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -129,26 +129,32 @@ int iommu_dma_init_domain(struct iommu_domain *domain, 
dma_addr_t base, u64 size
 EXPORT_SYMBOL(iommu_dma_init_domain);
 
 /**
- * dma_direction_to_prot - Translate DMA API directions to IOMMU API page flags
+ * dma_info_to_prot - Translate DMA API directions and attributes to IOMMU API
+ *page flags.
  * @dir: Direction of DMA transfer
  * @coherent: Is the DMA master cache-coherent?
+ * @attrs: DMA attributes for the mapping
  *
  * Return: corresponding IOMMU API page protection flags
  */
-int dma_direction_to_prot(enum dma_data_direction dir, bool coherent)
+int dma_info_to_prot(enum dma_data_direction dir, bool coherent,
+struct dma_attrs *attrs)
 {
int prot = coherent ? IOMMU_CACHE : 0;
 
switch (dir) {
case DMA_BIDIRECTIONAL:
-   return prot | IOMMU_READ | IOMMU_WRITE;
+   prot |= IOMMU_READ | IOMMU_WRITE;
case DMA_TO_DEVICE:
-   return prot | IOMMU_READ;
+   prot |= IOMMU_READ;
case DMA_FROM_DEVICE:
-   return prot | IOMMU_WRITE;
+   prot |= IOMMU_WRITE;
default:
return 0;
}
+   if (dma_get_attr(DMA_ATTR_PRIVILEGED, attrs))
+   prot |= IOMMU_PRIV;
+   return prot;
 }
 
 static struct iova *__alloc_iova(struct iova_domain *iovad, size_t size,
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 8443bbb5c071..b0dbcff2d73e 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -32,7 +32,8 @@ void iommu_put_dma_cookie(struct iommu_domain *domain);
 int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, u64 
size);
 
 /* General helpers for DMA-API <-> IOMMU-API interaction */
-int dma_direction_to_prot(enum dma_data_direction dir, bool coherent);
+int dma_info_to_prot(enum dma_data_direction dir, bool coherent,
+struct dma_attrs *attrs);
 
 /*
  * These implement the bulk of the relevant DMA mapping callbacks, but require
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v3 3/6] common: DMA-mapping: add DMA_ATTR_PRIVILEGED attribute

2016-07-19 Thread Mitchel Humpherys
This patch adds the DMA_ATTR_PRIVILEGED attribute to the DMA-mapping
subsystem.

Some advanced peripherals such as remote processors and GPUs perform
accesses to DMA buffers in both privileged "supervisor" and unprivileged
"user" modes.  This attribute is used to indicate to the DMA-mapping
subsystem that the buffer is fully accessible at the elevated privilege
level (and ideally inaccessible or at least read-only at the
lesser-privileged levels).

Cc: linux-...@vger.kernel.org
Signed-off-by: Mitchel Humpherys 
---

Notes:
v2..v3

  - Not worrying about executability.

 Documentation/DMA-attributes.txt | 10 ++
 include/linux/dma-attrs.h|  1 +
 2 files changed, 11 insertions(+)

diff --git a/Documentation/DMA-attributes.txt b/Documentation/DMA-attributes.txt
index e8cf9cf873b3..d985effd0053 100644
--- a/Documentation/DMA-attributes.txt
+++ b/Documentation/DMA-attributes.txt
@@ -126,3 +126,13 @@ means that we won't try quite as hard to get them.
 
 NOTE: At the moment DMA_ATTR_ALLOC_SINGLE_PAGES is only implemented on ARM,
 though ARM64 patches will likely be posted soon.
+
+DMA_ATTR_PRIVILEGED
+--
+
+Some advanced peripherals such as remote processors and GPUs perform
+accesses to DMA buffers in both privileged "supervisor" and unprivileged
+"user" modes.  This attribute is used to indicate to the DMA-mapping
+subsystem that the buffer is fully accessible at the elevated privilege
+level (and ideally inaccessible or at least read-only at the
+lesser-privileged levels).
diff --git a/include/linux/dma-attrs.h b/include/linux/dma-attrs.h
index 5246239a4953..3bc2208e1765 100644
--- a/include/linux/dma-attrs.h
+++ b/include/linux/dma-attrs.h
@@ -19,6 +19,7 @@ enum dma_attr {
DMA_ATTR_SKIP_CPU_SYNC,
DMA_ATTR_FORCE_CONTIGUOUS,
DMA_ATTR_ALLOC_SINGLE_PAGES,
+   DMA_ATTR_PRIVILEGED,
DMA_ATTR_MAX,
 };
 
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v3 6/6] Revert "iommu/arm-smmu: Treat all device transactions as unprivileged"

2016-07-19 Thread Mitchel Humpherys
This reverts commit d346180e70b9 ("iommu/arm-smmu: Treat all device
transactions as unprivileged") since some platforms actually make use of
privileged transactions.

Signed-off-by: Mitchel Humpherys 
---

Notes:
v2..v3

  - Moved to the end of the series.

 drivers/iommu/arm-smmu.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 9345a3fcb706..d0627ef26b05 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -178,9 +178,6 @@
 #define S2CR_TYPE_BYPASS   (1 << S2CR_TYPE_SHIFT)
 #define S2CR_TYPE_FAULT(2 << S2CR_TYPE_SHIFT)
 
-#define S2CR_PRIVCFG_SHIFT 24
-#define S2CR_PRIVCFG_UNPRIV(2 << S2CR_PRIVCFG_SHIFT)
-
 /* Context bank attribute registers */
 #define ARM_SMMU_GR1_CBAR(n)   (0x0 + ((n) << 2))
 #define CBAR_VMID_SHIFT0
@@ -1175,7 +1172,7 @@ static int arm_smmu_domain_add_master(struct 
arm_smmu_domain *smmu_domain,
u32 idx, s2cr;
 
idx = cfg->smrs ? cfg->smrs[i].idx : cfg->streamids[i];
-   s2cr = S2CR_TYPE_TRANS | S2CR_PRIVCFG_UNPRIV |
+   s2cr = S2CR_TYPE_TRANS |
   (smmu_domain->cfg.cbndx << S2CR_CBNDX_SHIFT);
writel_relaxed(s2cr, gr0_base + ARM_SMMU_GR0_S2CR(idx));
}
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v3 1/6] iommu: add IOMMU_PRIV attribute

2016-07-19 Thread Mitchel Humpherys
Add the IOMMU_PRIV attribute, which is used to indicate privileged
mappings.

Signed-off-by: Mitchel Humpherys 
---

Notes:
v2..v3

  - Added comment

 include/linux/iommu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 664683aedcce..4ae46254b915 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -31,6 +31,7 @@
 #define IOMMU_CACHE(1 << 2) /* DMA cache coherency */
 #define IOMMU_NOEXEC   (1 << 3)
 #define IOMMU_MMIO (1 << 4) /* e.g. things like MSI doorbells */
+#define IOMMU_PRIV (1 << 5) /* privileged */
 
 struct iommu_ops;
 struct iommu_group;
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v2 2/6] iommu/io-pgtable-arm: add support for the IOMMU_PRIV flag

2016-07-08 Thread Mitchel Humpherys
From: Jeremy Gebben 

Allow the creation of privileged mode mappings, for stage 1 only.

Signed-off-by: Jeremy Gebben 
---
 drivers/iommu/io-pgtable-arm.c | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index a1ed1b73fed4..e9e7dd179708 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -101,8 +101,10 @@
 ARM_LPAE_PTE_ATTR_HI_MASK)
 
 /* Stage-1 PTE */
-#define ARM_LPAE_PTE_AP_UNPRIV (((arm_lpae_iopte)1) << 6)
-#define ARM_LPAE_PTE_AP_RDONLY (((arm_lpae_iopte)2) << 6)
+#define ARM_LPAE_PTE_AP_PRIV_RW(((arm_lpae_iopte)0) << 6)
+#define ARM_LPAE_PTE_AP_RW (((arm_lpae_iopte)1) << 6)
+#define ARM_LPAE_PTE_AP_PRIV_RO(((arm_lpae_iopte)2) << 6)
+#define ARM_LPAE_PTE_AP_RO (((arm_lpae_iopte)3) << 6)
 #define ARM_LPAE_PTE_ATTRINDX_SHIFT2
 #define ARM_LPAE_PTE_nG(((arm_lpae_iopte)1) << 11)
 
@@ -350,10 +352,14 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct 
arm_lpae_io_pgtable *data,
 
if (data->iop.fmt == ARM_64_LPAE_S1 ||
data->iop.fmt == ARM_32_LPAE_S1) {
-   pte = ARM_LPAE_PTE_AP_UNPRIV | ARM_LPAE_PTE_nG;
+   pte = ARM_LPAE_PTE_nG;
 
-   if (!(prot & IOMMU_WRITE) && (prot & IOMMU_READ))
-   pte |= ARM_LPAE_PTE_AP_RDONLY;
+   if (prot & IOMMU_WRITE)
+   pte |= (prot & IOMMU_PRIV) ? ARM_LPAE_PTE_AP_PRIV_RW
+   : ARM_LPAE_PTE_AP_RW;
+   else
+   pte |= (prot & IOMMU_PRIV) ? ARM_LPAE_PTE_AP_PRIV_RO
+   : ARM_LPAE_PTE_AP_RO;
 
if (prot & IOMMU_MMIO)
pte |= (ARM_LPAE_MAIR_ATTR_IDX_DEV
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v2 3/6] Revert "iommu/arm-smmu: Treat all device transactions as unprivileged"

2016-07-08 Thread Mitchel Humpherys
This reverts commit d346180e70b9 ("iommu/arm-smmu: Treat all device
transactions as unprivileged") since some platforms actually make use of
privileged transactions.

Signed-off-by: Mitchel Humpherys 
---
 drivers/iommu/arm-smmu.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 9345a3fcb706..d0627ef26b05 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -178,9 +178,6 @@
 #define S2CR_TYPE_BYPASS   (1 << S2CR_TYPE_SHIFT)
 #define S2CR_TYPE_FAULT(2 << S2CR_TYPE_SHIFT)
 
-#define S2CR_PRIVCFG_SHIFT 24
-#define S2CR_PRIVCFG_UNPRIV(2 << S2CR_PRIVCFG_SHIFT)
-
 /* Context bank attribute registers */
 #define ARM_SMMU_GR1_CBAR(n)   (0x0 + ((n) << 2))
 #define CBAR_VMID_SHIFT0
@@ -1175,7 +1172,7 @@ static int arm_smmu_domain_add_master(struct 
arm_smmu_domain *smmu_domain,
u32 idx, s2cr;
 
idx = cfg->smrs ? cfg->smrs[i].idx : cfg->streamids[i];
-   s2cr = S2CR_TYPE_TRANS | S2CR_PRIVCFG_UNPRIV |
+   s2cr = S2CR_TYPE_TRANS |
   (smmu_domain->cfg.cbndx << S2CR_CBNDX_SHIFT);
writel_relaxed(s2cr, gr0_base + ARM_SMMU_GR0_S2CR(idx));
}
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v2 5/6] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED_EXECUTABLE

2016-07-08 Thread Mitchel Humpherys
The newly added DMA_ATTR_PRIVILEGED_EXECUTABLE is useful for creating
mappings that are executable by privileged DMA engines.  Implement it in
dma-iommu.c so that the ARM64 DMA IOMMU mapper can make use of it.

Signed-off-by: Mitchel Humpherys 
---
 arch/arm64/mm/dma-mapping.c |  6 +++---
 drivers/iommu/dma-iommu.c   | 15 +++
 include/linux/dma-iommu.h   |  3 ++-
 3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index c566ec83719f..44f676268df6 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -543,7 +543,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t 
size,
 struct dma_attrs *attrs)
 {
bool coherent = is_device_dma_coherent(dev);
-   int ioprot = dma_direction_to_prot(DMA_BIDIRECTIONAL, coherent);
+   int ioprot = dma_direction_to_prot(DMA_BIDIRECTIONAL, coherent, attrs);
size_t iosize = size;
void *addr;
 
@@ -697,7 +697,7 @@ static dma_addr_t __iommu_map_page(struct device *dev, 
struct page *page,
   struct dma_attrs *attrs)
 {
bool coherent = is_device_dma_coherent(dev);
-   int prot = dma_direction_to_prot(dir, coherent);
+   int prot = dma_direction_to_prot(dir, coherent, attrs);
dma_addr_t dev_addr = iommu_dma_map_page(dev, page, offset, size, prot);
 
if (!iommu_dma_mapping_error(dev, dev_addr) &&
@@ -755,7 +755,7 @@ static int __iommu_map_sg_attrs(struct device *dev, struct 
scatterlist *sgl,
__iommu_sync_sg_for_device(dev, sgl, nelems, dir);
 
return iommu_dma_map_sg(dev, sgl, nelems,
-   dma_direction_to_prot(dir, coherent));
+   dma_direction_to_prot(dir, coherent, attrs));
 }
 
 static void __iommu_unmap_sg_attrs(struct device *dev,
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index ea5a9ebf0f78..ccc6219da228 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -132,23 +132,30 @@ EXPORT_SYMBOL(iommu_dma_init_domain);
  * dma_direction_to_prot - Translate DMA API directions to IOMMU API page flags
  * @dir: Direction of DMA transfer
  * @coherent: Is the DMA master cache-coherent?
+ * @attrs: DMA attributes for the mapping
  *
  * Return: corresponding IOMMU API page protection flags
  */
-int dma_direction_to_prot(enum dma_data_direction dir, bool coherent)
+int dma_direction_to_prot(enum dma_data_direction dir, bool coherent,
+ struct dma_attrs *attrs)
 {
int prot = coherent ? IOMMU_CACHE : 0;
 
switch (dir) {
case DMA_BIDIRECTIONAL:
-   return prot | IOMMU_READ | IOMMU_WRITE;
+   prot |= IOMMU_READ | IOMMU_WRITE;
case DMA_TO_DEVICE:
-   return prot | IOMMU_READ;
+   prot |= IOMMU_READ;
case DMA_FROM_DEVICE:
-   return prot | IOMMU_WRITE;
+   prot |= IOMMU_WRITE;
default:
return 0;
}
+   if (dma_get_attr(DMA_ATTR_PRIVILEGED_EXECUTABLE, attrs)) {
+   prot &= ~IOMMU_WRITE;
+   prot |= IOMMU_PRIV;
+   }
+   return prot;
 }
 
 static struct iova *__alloc_iova(struct iova_domain *iovad, size_t size,
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 8443bbb5c071..d5a37e58d29b 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -32,7 +32,8 @@ void iommu_put_dma_cookie(struct iommu_domain *domain);
 int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, u64 
size);
 
 /* General helpers for DMA-API <-> IOMMU-API interaction */
-int dma_direction_to_prot(enum dma_data_direction dir, bool coherent);
+int dma_direction_to_prot(enum dma_data_direction dir, bool coherent,
+ struct dma_attrs *attrs);
 
 /*
  * These implement the bulk of the relevant DMA mapping callbacks, but require
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v2 4/6] common: DMA-mapping: add DMA_ATTR_PRIVILEGED_EXECUTABLE attribute

2016-07-08 Thread Mitchel Humpherys
This patch adds the DMA_ATTR_PRIVILEGED_EXECUTABLE attribute to the
DMA-mapping subsystem.

Some architectures require that writable mappings also be non-executable at
lesser-privileged levels of execution.  This attribute is used to indicate
to the DMA-mapping subsystem that it should do whatever is necessary to
ensure that the buffer is executable at an elevated privilege level (by
making it read-only at the lesser-privileged levels, for example).

Cc: linux-...@vger.kernel.org
Signed-off-by: Mitchel Humpherys 
---
 Documentation/DMA-attributes.txt | 9 +
 include/linux/dma-attrs.h| 1 +
 2 files changed, 10 insertions(+)

diff --git a/Documentation/DMA-attributes.txt b/Documentation/DMA-attributes.txt
index e8cf9cf873b3..6a22d4307008 100644
--- a/Documentation/DMA-attributes.txt
+++ b/Documentation/DMA-attributes.txt
@@ -126,3 +126,12 @@ means that we won't try quite as hard to get them.
 
 NOTE: At the moment DMA_ATTR_ALLOC_SINGLE_PAGES is only implemented on ARM,
 though ARM64 patches will likely be posted soon.
+
+DMA_ATTR_PRIVILEGED_EXECUTABLE
+--
+
+Some architectures require that writable mappings also be non-executable at
+lesser-privileged levels of execution.  This attribute is used to indicate
+to the DMA-mapping subsystem that it should do whatever is necessary to
+ensure that the buffer is executable at an elevated privilege level (by
+making it read-only at the lesser-privileged levels, for example).
diff --git a/include/linux/dma-attrs.h b/include/linux/dma-attrs.h
index 5246239a4953..8cf4dff6185b 100644
--- a/include/linux/dma-attrs.h
+++ b/include/linux/dma-attrs.h
@@ -19,6 +19,7 @@ enum dma_attr {
DMA_ATTR_SKIP_CPU_SYNC,
DMA_ATTR_FORCE_CONTIGUOUS,
DMA_ATTR_ALLOC_SINGLE_PAGES,
+   DMA_ATTR_PRIVILEGED_EXECUTABLE,
DMA_ATTR_MAX,
 };
 
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v2 1/6] iommu: add IOMMU_PRIV attribute

2016-07-08 Thread Mitchel Humpherys
Add the IOMMU_PRIV attribute, which is used to indicate privileged
mappings.

Signed-off-by: Mitchel Humpherys 
---
 include/linux/iommu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 664683aedcce..01c9f2667f2b 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -31,6 +31,7 @@
 #define IOMMU_CACHE(1 << 2) /* DMA cache coherency */
 #define IOMMU_NOEXEC   (1 << 3)
 #define IOMMU_MMIO (1 << 4) /* e.g. things like MSI doorbells */
+#define IOMMU_PRIV (1 << 5)
 
 struct iommu_ops;
 struct iommu_group;
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v2 0/6] Add support for privileged mappings

2016-07-08 Thread Mitchel Humpherys
The following patch to the ARM SMMU driver:

commit d346180e70b91b3d5a1ae7e5603e65593d4622bc
Author: Robin Murphy 
Date:   Tue Jan 26 18:06:34 2016 +

iommu/arm-smmu: Treat all device transactions as unprivileged

started forcing all SMMU transactions to come through as "unprivileged".
The rationale given was that:

  (1) There is no way in the IOMMU API to even request privileged mappings.

  (2) It's difficult to implement a DMA mapper that correctly models the
  ARM VMSAv8 behavior of unprivileged-writeable =>
  privileged-execute-never.

This series rectifies (1) by introducing an IOMMU API for privileged
mappings and implements it in io-pgtable-arm.

This series rectifies (2) by introducing a new dma attribute
(DMA_ATTR_PRIVILEGED_EXECUTABLE) for users of the DMA API that need
privileged, executable mappings, and implements it in the arm64 IOMMU DMA
mapper.  The one known user (pl330.c) is converted over to the new
attribute.

Jordan and Jeremy can provide more info on the use case if needed, but the
high level is that it's a security feature to prevent attacks such as [1].

[1] https://github.com/robclark/kilroy

Changelog:

  v1..v2

- Added a new DMA attribute to make executable privileged mappings
  work, and use that in the pl330 driver (suggested by Will).


Jeremy Gebben (1):
  iommu/io-pgtable-arm: add support for the IOMMU_PRIV flag

Mitchel Humpherys (5):
  iommu: add IOMMU_PRIV attribute
  Revert "iommu/arm-smmu: Treat all device transactions as unprivileged"
  common: DMA-mapping: add DMA_ATTR_PRIVILEGED_EXECUTABLE attribute
  arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED_EXECUTABLE
  dmaengine: pl330: Make sure microcode is privileged-executable

 Documentation/DMA-attributes.txt |  9 +
 arch/arm64/mm/dma-mapping.c  |  6 +++---
 drivers/dma/pl330.c  |  7 +--
 drivers/iommu/arm-smmu.c |  5 +
 drivers/iommu/dma-iommu.c| 15 +++
 drivers/iommu/io-pgtable-arm.c   | 16 +++-
 include/linux/dma-attrs.h|  1 +
 include/linux/dma-iommu.h|  3 ++-
 include/linux/iommu.h|  1 +
 9 files changed, 44 insertions(+), 19 deletions(-)

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v2 6/6] dmaengine: pl330: Make sure microcode is privileged-executable

2016-07-08 Thread Mitchel Humpherys
The PL330 can perform privileged instruction fetches.  This can result
in SMMU permission faults on SMMUs that implement the ARMv8 VMSA, which
specifies that mappings that are writeable at one execution level shall
not be executable at any higher-privileged level.  Fix this by using the
DMA_ATTR_PRIVILEGED_EXECUTABLE attribute, which will ensure that the
microcode IOMMU mapping is not writeable.

Cc: Dan Williams 
Cc: Jassi Brar 
Signed-off-by: Mitchel Humpherys 
---
 drivers/dma/pl330.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 372b4359da97..25bc49d47c45 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -1854,14 +1854,17 @@ static int dmac_alloc_resources(struct pl330_dmac 
*pl330)
 {
int chans = pl330->pcfg.num_chan;
int ret;
+   DEFINE_DMA_ATTRS(attrs);
 
+   dma_set_attr(DMA_ATTR_PRIVILEGED_EXECUTABLE, &attrs);
/*
 * Alloc MicroCode buffer for 'chans' Channel threads.
 * A channel's buffer offset is (Channel_Id * MCODE_BUFF_PERCHAN)
 */
-   pl330->mcode_cpu = dma_alloc_coherent(pl330->ddma.dev,
+   pl330->mcode_cpu = dma_alloc_attrs(pl330->ddma.dev,
chans * pl330->mcbufsz,
-   &pl330->mcode_bus, GFP_KERNEL);
+   &pl330->mcode_bus, GFP_KERNEL,
+   &attrs);
if (!pl330->mcode_cpu) {
dev_err(pl330->ddma.dev, "%s:%d Can't allocate memory!\n",
__func__, __LINE__);
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



Re: [PATCH v2] of: Check for overlap in reserved memory regions

2015-12-04 Thread Mitchel Humpherys
On Thu, Nov 12 2015 at 01:19:59 PM, Michael Ellerman  
wrote:
> On Tue, 2015-09-15 at 18:30 -0700, Mitchel Humpherys wrote:
>
>> Any overlap in the reserved memory regions (those specified in the
>> reserved-memory DT node) is a bug.
>
> Can you expand a bit on why you think it's a bug? I assume it was discussed at
> some point on the list but I didn't see it sorry.

The reason I think it's a bug is because the overlapping memory could be
handed out to multiple firmwares, which generally ends in "random"
firmware crashes.  We've found by sad experience that root-causing such
a crash can be quite difficult.

Is there a valid use case for overlapping regions?  I can't think of
one...

> There's nothing I can see in the binding document[1] about whether regions can
> overlap, or what it would mean if they did.

You're right, the bindings document doesn't say anything about
overlapping memory regions.  I can submit something unless someone comes
up with a reason why we should allow overlapping memory regions.

> If we want to declare that overlapping regions are always a bug then there
> should be some text in the binding explaining that. There's also the
> possibility that we have existing device trees in the wild that contain
> overlapping regions, and whether we think it's OK to retrospectively declare
> that they're incorrect.

I did a quick survey of in-tree users of reserved-memory and couldn't
find any overlapping regions.


-Mitch

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] of: Fix comparison of reserved memory regions

2015-12-04 Thread Mitchel Humpherys
On Wed, Nov 18 2015 at 09:46:38 PM, Michael Ellerman  
wrote:
> In order to check for overlapping reserved memory regions, we first need
> to sort the array of memory regions. This is implemented using sort(),
> and a custom comparison function __rmem_cmp().
>
> Unfortunatley __rmem_cmp() doesn't work in all cases. Because the two
> base values are phys_addr_t, they may be u64 on some platforms, in which
> case subtracting one from the other and then (implicitly) casting to int
> does not give us the -ve/0/+ve value we need.
>
> This leads to incorrect reports about overlaps, eg:
>
>   ibm,slw-image@1ffe60 (0x001ffe60--0x001ffe70) overlaps 
> with
>   ibm,firmware-allocs-memory@10 
> (0x0010--0x001000dc0200)
>
> Fix it by just doing the standard double if and return 0 logic.
>
> Fixes: ae1add247bf8 ("of: Check for overlap in reserved memory regions")
> Signed-off-by: Michael Ellerman 
> ---
>  drivers/of/of_reserved_mem.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)

Woops, thanks.

Tested-by: Mitchel Humpherys 

-Mitch

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC][PATCH 1/2] WIP: Devicetree bindings for Ion

2015-10-20 Thread Mitchel Humpherys
On Tue, Oct 13 2015 at 11:14:23 AM, Andrew  wrote:
> On 2015-10-12 21:39, Mitchel Humpherys wrote:
>> On Tue, Oct 06 2015 at 05:35:41 PM, Rob Herring 
>> wrote:
>>> On Tue, Oct 6, 2015 at 3:47 PM, Laura Abbott 
>>> wrote:
>>
>> [...]
>>
>>>> +Example:
>>>> +
>>>> +   ion {
>>>> +   compatbile = "linux,ion";
>>>> +   #address-cells = <1>;
>>>> +   #size-cells = <0>;
>>>> +
>>>> +   ion-system-heap {
>>>> +   linux,ion-heap-id = <0>;
>>>> +   linux,ion-heap-type = ;
>>>> +   linux,ion-heap-name = "system";
>>>
>>> How does this vary across platforms? Is all of this being pushed down
>>> to DT, because there is no coordination of this at the kernel ABI
>>> level across platforms. In other words, why can't heap 0 be hardcoded
>>> as system heap in the driver. It seems to me any 1 of these 3
>>> properties could be used to derive the other 2.
>>
>> The heap-id<->heap-type mapping isn't necessarily 1:1.  As Laura
>> indicated elsewhere on this thread, a given heap might need to be
>> contiguous on one platform but not on another.  In that case you just
>> swap out the heap-type here and there's no need for userspace to change.
>>
>> The heap-name, OTOH, could be derived from the heap-id, which is what we
>> hackishly do here [1] and here[2].
>
> By the way, since we agreed that heap id and heap type mappings
> are not 1:1 - we have a problem with the current API.
>
> In userspace we currently have this:
>
> int ion_alloc(int fd, size_t len, size_t align, unsigned int heap_mask,
>   unsigned int flags, ion_user_handle_t *handle);
>
> We do not specify here what TYPE of heap we want the allocation to come
> from.
> This may lead to very unpleasant stuff when porting from one platfrom to
> another.

What "unpleasant stuff" are you referring to, exactly?  Abstracting the
heap type away from userspace has actually made our lives easier since
userspace doesn't need to know anything about the properties of the
underlying platform.  It just asks for a buffer from the "camera" heap,
for example.  On some platforms that's contiguous, on others it's not.


-Mitch

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC][PATCH 1/2] WIP: Devicetree bindings for Ion

2015-10-12 Thread Mitchel Humpherys
On Tue, Oct 06 2015 at 05:35:41 PM, Rob Herring  wrote:
> On Tue, Oct 6, 2015 at 3:47 PM, Laura Abbott  
> wrote:

[...]

>> +Example:
>> +
>> +   ion {
>> +   compatbile = "linux,ion";
>> +   #address-cells = <1>;
>> +   #size-cells = <0>;
>> +
>> +   ion-system-heap {
>> +   linux,ion-heap-id = <0>;
>> +   linux,ion-heap-type = ;
>> +   linux,ion-heap-name = "system";
>
> How does this vary across platforms? Is all of this being pushed down
> to DT, because there is no coordination of this at the kernel ABI
> level across platforms. In other words, why can't heap 0 be hardcoded
> as system heap in the driver. It seems to me any 1 of these 3
> properties could be used to derive the other 2.

The heap-id<->heap-type mapping isn't necessarily 1:1.  As Laura
indicated elsewhere on this thread, a given heap might need to be
contiguous on one platform but not on another.  In that case you just
swap out the heap-type here and there's no need for userspace to change.

The heap-name, OTOH, could be derived from the heap-id, which is what we
hackishly do here [1] and here[2].

[1] 
https://www.codeaurora.org/cgit/quic/la/kernel/msm-3.14/tree/drivers/staging/android/ion/msm/msm_ion.c?h=msm-3.14#n53
[2] 
https://www.codeaurora.org/cgit/quic/la/kernel/msm-3.14/tree/drivers/staging/android/ion/msm/msm_ion.c?h=msm-3.14#n398


-Mitch

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2] of: Check for overlap in reserved memory regions

2015-09-15 Thread Mitchel Humpherys
Any overlap in the reserved memory regions (those specified in the
reserved-memory DT node) is a bug.  These bugs might go undetected as
long as the contested region isn't used simultaneously by multiple
software agents, which makes such bugs hard to debug.  Fix this by
printing a scary warning during boot if overlap is detected.

Signed-off-by: Mitchel Humpherys 
---
v1..v2:
  - Suggestions from Rob Herring (remove superfluous array and
print statement)
---
 drivers/of/of_reserved_mem.c | 43 ++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 726ebe792813..62f467b8ccae 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -1,7 +1,7 @@
 /*
  * Device tree based initialization code for reserved memory.
  *
- * Copyright (c) 2013, The Linux Foundation. All Rights Reserved.
+ * Copyright (c) 2013, 2015 The Linux Foundation. All Rights Reserved.
  * Copyright (c) 2013,2014 Samsung Electronics Co., Ltd.
  * http://www.samsung.com
  * Author: Marek Szyprowski 
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define MAX_RESERVED_REGIONS   16
 static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS];
@@ -197,12 +198,52 @@ static int __init __reserved_mem_init_node(struct 
reserved_mem *rmem)
return -ENOENT;
 }
 
+static int __init __rmem_cmp(const void *a, const void *b)
+{
+   const struct reserved_mem *ra = a, *rb = b;
+
+   return ra->base - rb->base;
+}
+
+static void __init __rmem_check_for_overlap(void)
+{
+   int i;
+
+   if (reserved_mem_count < 2)
+   return;
+
+   sort(reserved_mem, reserved_mem_count, sizeof(reserved_mem[0]),
+__rmem_cmp, NULL);
+   for (i = 0; i < reserved_mem_count - 1; i++) {
+   struct reserved_mem *this, *next;
+
+   this = &reserved_mem[i];
+   next = &reserved_mem[i + 1];
+   if (!(this->base && next->base))
+   continue;
+   if (this->base + this->size > next->base) {
+   phys_addr_t this_end, next_end;
+
+   this_end = this->base + this->size;
+   next_end = next->base + next->size;
+   WARN(1,
+"Reserved memory: OVERLAP DETECTED!\n%s (%pa--%pa) 
overlaps with %s (%pa--%pa)\n",
+this->name, &this->base, &this_end,
+next->name, &next->base, &next_end);
+   }
+   }
+}
+
 /**
  * fdt_init_reserved_mem - allocate and init all saved reserved memory regions
  */
 void __init fdt_init_reserved_mem(void)
 {
int i;
+
+   /* check for overlapping reserved regions */
+   __rmem_check_for_overlap();
+
for (i = 0; i < reserved_mem_count; i++) {
struct reserved_mem *rmem = &reserved_mem[i];
unsigned long node = rmem->fdt_node;
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] of: Check for overlap in reserved memory regions

2015-09-14 Thread Mitchel Humpherys
On Mon, Sep 14 2015 at 02:21:04 PM, Rob Herring  wrote:
> On Fri, Sep 11, 2015 at 12:31 PM, Mitchel Humpherys
>  wrote:
>> Any overlap in the reserved memory regions (those specified in the
>> reserved-memory DT node) is a bug.  These bugs might go undetected as
>> long as the contested region isn't used simultaneously by multiple
>> software agents, which makes such bugs hard to debug.  Fix this by
>> printing a scary warning during boot if overlap is detected.

[...]

>> +
>> +static void __init __rmem_check_for_overlap(void)
>> +{
>> +   int i;
>> +
>> +   if (reserved_mem_count < 2)
>> +   return;
>> +
>> +   memcpy(sorted_reserved_mem, reserved_mem, 
>> sizeof(sorted_reserved_mem));
>> +   sort(sorted_reserved_mem, reserved_mem_count,
>> +sizeof(sorted_reserved_mem[0]), __rmem_cmp, NULL);
>
> Why not just sort reserved_mem?

Yeah, I considered that but wasn't sure if it would break things in the
few places where the ordering of reserved_mem matters (like
__find_rmem).  Looking closer I think we're safe to sort reserved_mem
array directly to avoid the memcpy.  Will update in v2.

>
>> +   for (i = 0; i < reserved_mem_count - 1; i++) {
>> +   struct reserved_mem *this, *next;
>> +
>> +   this = &sorted_reserved_mem[i];
>> +   next = &sorted_reserved_mem[i + 1];
>> +   if (!(this->base && next->base))
>> +   continue;
>> +   if (this->base + this->size > next->base) {
>> +   phys_addr_t this_end, next_end;
>> +
>> +   this_end = this->base + this->size;
>> +   next_end = next->base + next->size;
>> +   WARN(1, "Reserved mem: OVERLAP DETECTED!\n");
>> +   pr_err("%s (%pa--%pa) overlaps with %s (%pa--%pa)\n",
>
> This seems overly verbose having both WARN and pr_err. I'd combine
> these. I don't think the stack trace from a WARN is too useful here
> given it is the DT file that users will need to go look at.

Yeah the reason for the WARN is to make it harder for this to slip by,
and I thought a BUG might be too heavy-handed.  I should be able to
squeeze the content of the pr_err into the WARN in v2.


-Mitch

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] of: Check for overlap in reserved memory regions

2015-09-11 Thread Mitchel Humpherys
Any overlap in the reserved memory regions (those specified in the
reserved-memory DT node) is a bug.  These bugs might go undetected as
long as the contested region isn't used simultaneously by multiple
software agents, which makes such bugs hard to debug.  Fix this by
printing a scary warning during boot if overlap is detected.

Signed-off-by: Mitchel Humpherys 
---
 drivers/of/of_reserved_mem.c | 45 +++-
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 726ebe792813..5246d346cee0 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -1,7 +1,7 @@
 /*
  * Device tree based initialization code for reserved memory.
  *
- * Copyright (c) 2013, The Linux Foundation. All Rights Reserved.
+ * Copyright (c) 2013, 2015 The Linux Foundation. All Rights Reserved.
  * Copyright (c) 2013,2014 Samsung Electronics Co., Ltd.
  * http://www.samsung.com
  * Author: Marek Szyprowski 
@@ -20,9 +20,11 @@
 #include 
 #include 
 #include 
+#include 
 
 #define MAX_RESERVED_REGIONS   16
 static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS];
+static struct reserved_mem sorted_reserved_mem[MAX_RESERVED_REGIONS] 
__initdata;
 static int reserved_mem_count;
 
 #if defined(CONFIG_HAVE_MEMBLOCK)
@@ -197,12 +199,53 @@ static int __init __reserved_mem_init_node(struct 
reserved_mem *rmem)
return -ENOENT;
 }
 
+static int __init __rmem_cmp(const void *a, const void *b)
+{
+   const struct reserved_mem *ra = a, *rb = b;
+
+   return ra->base - rb->base;
+}
+
+static void __init __rmem_check_for_overlap(void)
+{
+   int i;
+
+   if (reserved_mem_count < 2)
+   return;
+
+   memcpy(sorted_reserved_mem, reserved_mem, sizeof(sorted_reserved_mem));
+   sort(sorted_reserved_mem, reserved_mem_count,
+sizeof(sorted_reserved_mem[0]), __rmem_cmp, NULL);
+   for (i = 0; i < reserved_mem_count - 1; i++) {
+   struct reserved_mem *this, *next;
+
+   this = &sorted_reserved_mem[i];
+   next = &sorted_reserved_mem[i + 1];
+   if (!(this->base && next->base))
+   continue;
+   if (this->base + this->size > next->base) {
+   phys_addr_t this_end, next_end;
+
+   this_end = this->base + this->size;
+   next_end = next->base + next->size;
+   WARN(1, "Reserved mem: OVERLAP DETECTED!\n");
+   pr_err("%s (%pa--%pa) overlaps with %s (%pa--%pa)\n",
+  this->name, &this->base, &this_end,
+  next->name, &next->base, &next_end);
+   }
+   }
+}
+
 /**
  * fdt_init_reserved_mem - allocate and init all saved reserved memory regions
  */
 void __init fdt_init_reserved_mem(void)
 {
int i;
+
+   /* check for overlapping reserved regions */
+   __rmem_check_for_overlap();
+
for (i = 0; i < reserved_mem_count; i++) {
struct reserved_mem *rmem = &reserved_mem[i];
unsigned long node = rmem->fdt_node;
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 2/2] staging: ion: chunk_heap: use %pad for printing dma_addr_t's

2015-05-04 Thread Mitchel Humpherys
On Mon, May 04 2015 at 01:05:50 PM, Colin Cross  wrote:
> On Mon, May 4, 2015 at 1:22 AM, Dan Carpenter  
> wrote:
>> On Thu, Apr 09, 2015 at 06:10:04PM -0700, Mitchel Humpherys wrote:
>>> We're currently using %lu and %ld to print some variables of type
>>> dma_addr_t, which results in the following warning when dma_addr_t is
>>> 64-bits wide:
>>>
>>> drivers/staging/android/ion/ion_chunk_heap.c: In function 
>>> 'ion_chunk_heap_create':
>>> drivers/staging/android/ion/ion_chunk_heap.c:176:2: warning: format 
>>> '%lu' expects argument of type 'long unsigned int', but argument 3 has type 
>>> 'dma_addr_t' [-Wformat=]
>>>   pr_info("%s: base %lu size %zu align %ld\n", __func__, 
>>> chunk_heap->base,
>>>   ^
>>> drivers/staging/android/ion/ion_chunk_heap.c:176:2: warning: format 
>>> '%ld' expects argument of type 'long int', but argument 5 has type 
>>> 'dma_addr_t' [-Wformat=]
>>>
>>> Fix this by using %pad as instructed in printk-formats.txt.
>>>
>>> Signed-off-by: Mitchel Humpherys 
>>
>> This one was just merged and I was about to email you that it introduces
>> some new Smatch warnings, but actually looking at it, it's just wrong.
>>
>> We want to print "chunk_heap->base" and not "&chunk_heap->base".
>
> This would be correct if base was a dma_addr_t...
>
>> And anyway "&chunk_heap->base" is a regular pointer, not a dma_addr_t.
>
> But it is actually an ion_phys_addr_t, which is currently typedef'd to
> unsigned long.  Are you using a local patch that replaces
> ion_phys_addr_t with dma_addr_t?
>
>> So please send a new patch that removes the &.
>
> Removing the & is not correct, lib/vsprintf.c will dereference the arg
> for %pad or %pap.  I think this patch should just be dropped, the old
> %lu was correct for what is in Linus' tree.

Ah, you're absolutely correct.  We have a local patch that makes
ion_phys_addr_t a dma_addr_t (needed for LPAE), which is why I needed to
convert the printk format...

Greg, can you please drop this patch from your tree?  We'll only need
this if/when mainline Ion gets LPAE support...


-Mitch

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] ion: improve ion_phys error message

2015-03-10 Thread Mitchel Humpherys
Clients often get confused when ion_phys errors out due to some heap
being used that they didn't expect.  Add the heap name and heap type to
the error message to make it more obvious.

Signed-off-by: Mitchel Humpherys 
---
 drivers/staging/android/ion/ion.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c 
b/drivers/staging/android/ion/ion.c
index 296d347660fc..966b7fdc9ecf 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -566,8 +566,8 @@ int ion_phys(struct ion_client *client, struct ion_handle 
*handle,
buffer = handle->buffer;
 
if (!buffer->heap->ops->phys) {
-   pr_err("%s: ion_phys is not implemented by this heap.\n",
-  __func__);
+   pr_err("%s: ion_phys is not implemented by this heap (name=%s, 
type=%d).\n",
+   __func__, buffer->heap->name, buffer->heap->type);
mutex_unlock(&client->lock);
return -ENODEV;
}
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/5] iommu/mediatek: Add mt8173 IOMMU driver

2015-03-09 Thread Mitchel Humpherys
On Mon, Mar 09 2015 at 05:16:26 AM, Yong Wu  wrote:
> Dear Mitchel,
>  Thanks very much for your review.
>
> On Fri, 2015-03-06 at 09:15 -0800, Mitchel Humpherys wrote:
>> On Fri, Mar 06 2015 at 02:48:17 AM,  wrote:
>> > From: Yong Wu 
>> >
>> > This patch adds support for mediatek m4u (MultiMedia Memory Management 
>> > Unit).
>> > Currently this only supports m4u gen 2 with 2 levels of page table on 
>> > mt8173.
>> 
>> [...]
>> 
>> > +static int mtk_iommu_invalidate_tlb(const struct mtk_iommu_info *piommu,
>> > +  int isinvall, unsigned int iova_start,
>> > +  unsigned int iova_end)
>> > +{
>> > +  void __iomem *m4u_base = piommu->m4u_base;
>> > +  u32 val;
>> > +  u64 start, end;
>> > +
>> > +  start = sched_clock();
>> > +
>> > +  if (!isinvall) {
>> > +  iova_start = round_down(iova_start, SZ_4K);
>> > +  iova_end = round_up(iova_end, SZ_4K);
>> > +  }
>> > +
>> > +  val = F_MMU_INV_EN_L2 | F_MMU_INV_EN_L1;
>> > +
>> > +  writel(val, m4u_base + REG_INVLID_SEL);
>> > +
>> > +  if (isinvall) {
>> > +  writel(F_MMU_INV_ALL, m4u_base + REG_MMU_INVLD);
>> > +  } else {
>> > +  writel(iova_start, m4u_base + REG_MMU_INVLD_SA);
>> > +  writel(iova_end, m4u_base + REG_MMU_INVLD_EA);
>> > +  writel(F_MMU_INV_RANGE, m4u_base + REG_MMU_INVLD);
>> > +
>> > +  while (!readl(m4u_base + REG_MMU_CPE_DONE)) {
>> > +  end = sched_clock();
>> > +  if (end - start >= 1ULL) {
>> > +  dev_warn(piommu->dev, "invalid don't done\n");
>> > +  writel(F_MMU_INV_ALL, m4u_base + REG_MMU_INVLD);
>> > +  }
>> > +  };
>> 
>> Superfluous `;'.  Also, maybe you should be using readl_poll_timeout?
>Thanks.
>For the "readl_poll_timeout", My base is 3.19-rc7 and robin's patch.
> it don't have this interface.  I will try to add it in the next version.

Yeah it was merged in v4.0-rc1.

-Mitch

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/5] iommu/mediatek: Add mt8173 IOMMU driver

2015-03-06 Thread Mitchel Humpherys
On Fri, Mar 06 2015 at 02:48:17 AM,  wrote:
> From: Yong Wu 
>
> This patch adds support for mediatek m4u (MultiMedia Memory Management Unit).
> Currently this only supports m4u gen 2 with 2 levels of page table on mt8173.

[...]

> +static int mtk_iommu_invalidate_tlb(const struct mtk_iommu_info *piommu,
> + int isinvall, unsigned int iova_start,
> + unsigned int iova_end)
> +{
> + void __iomem *m4u_base = piommu->m4u_base;
> + u32 val;
> + u64 start, end;
> +
> + start = sched_clock();
> +
> + if (!isinvall) {
> + iova_start = round_down(iova_start, SZ_4K);
> + iova_end = round_up(iova_end, SZ_4K);
> + }
> +
> + val = F_MMU_INV_EN_L2 | F_MMU_INV_EN_L1;
> +
> + writel(val, m4u_base + REG_INVLID_SEL);
> +
> + if (isinvall) {
> + writel(F_MMU_INV_ALL, m4u_base + REG_MMU_INVLD);
> + } else {
> + writel(iova_start, m4u_base + REG_MMU_INVLD_SA);
> + writel(iova_end, m4u_base + REG_MMU_INVLD_EA);
> + writel(F_MMU_INV_RANGE, m4u_base + REG_MMU_INVLD);
> +
> + while (!readl(m4u_base + REG_MMU_CPE_DONE)) {
> + end = sched_clock();
> + if (end - start >= 1ULL) {
> + dev_warn(piommu->dev, "invalid don't done\n");
> + writel(F_MMU_INV_ALL, m4u_base + REG_MMU_INVLD);
> + }
> + };

Superfluous `;'.  Also, maybe you should be using readl_poll_timeout?

> + writel(0, m4u_base + REG_MMU_CPE_DONE);
> + }
> +
> + return 0;
> +}



-Mitch

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] ion: improve ion_phys error message

2015-02-13 Thread Mitchel Humpherys
Clients often get confused when ion_phys errors out due to some heap
being used that they didn't expect.  Add the heap name and heap type to
the error message to make it more obvious.

Signed-off-by: Mitchel Humpherys 
---
 drivers/staging/android/ion/ion.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c 
b/drivers/staging/android/ion/ion.c
index 296d347660fc..966b7fdc9ecf 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -566,8 +566,8 @@ int ion_phys(struct ion_client *client, struct ion_handle 
*handle,
buffer = handle->buffer;
 
if (!buffer->heap->ops->phys) {
-   pr_err("%s: ion_phys is not implemented by this heap.\n",
-  __func__);
+   pr_err("%s: ion_phys is not implemented by this heap (name=%s, 
type=%d).\n",
+   __func__, buffer->heap->name, buffer->heap->type);
mutex_unlock(&client->lock);
return -ENODEV;
}
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v10] iopoll: Introduce memory-mapped IO polling macros

2015-01-14 Thread Mitchel Humpherys
On Tue, Dec 16 2014 at 01:45:27 AM, Will Deacon  wrote:
> On Mon, Dec 15, 2014 at 11:47:23PM +0000, Mitchel Humpherys wrote:
>> From: Matt Wagantall 
>> 
>> It is sometimes necessary to poll a memory-mapped register until its value
>> satisfies some condition. Introduce a family of convenience macros that do
>> this. Tight-looping, sleeping, and timing out can all be accomplished using
>> these macros.
>> 
>> Cc: Thierry Reding 
>> Cc: Will Deacon 
>> Cc: Arnd Bergmann 
>> Cc: Andrew Morton 
>> Cc: Robert Elliott 
>> Signed-off-by: Matt Wagantall 
>> Signed-off-by: Mitchel Humpherys 
>> ---
>> v9..10:
>>   - Actually added the comments mentioned in v8..v9 (doh!)
>> 
>> v8..v9:
>>   - Added note in comments about max sleep time (Rob Elliott)
>> 
>> v7..v8:
>>   - sorted helper macros by size (b, w, l, q)
>>   - removed some of the more esoteric (or flat-out bogus) helper macros
>> 
>> This patch was originally part of a series [1] for adding support for IOMMU
>> address translations through an ARM SMMU hardware register.  The other
>> patch in the series (the one that actually uses these macros and implements
>> said hardware address translations) was Ack'd by the driver maintainer
>> there (Will Deacon) so I've pulled this patch out to avoid resending an
>> already Ack'd patch over and over again.
>> 
>> In short, please see [1] for previous discussion and the first user of
>> these macros.
>> 
>> Will also acked this patch in [2].  I didn't retain his Ack here because I
>> added to the macro comments.
>
> You can keep the ack, it still looks good to me and I'm not really fussed
> about the comments.
>
> Will

This hasn't gotten any further comments.  Would someone be willing to
take it?

Joerg, maybe you could take this through the IOMMU tree since the first
user is an IOMMU driver?  Currently we can't move [1] forward because of
this dependency...

[1] http://thread.gmane.org/gmane.linux.kernel.iommu/7837


-Mitch

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] staging: ion: always initialize the free list parameters

2015-01-08 Thread Mitchel Humpherys
Currently we initialize the heap free_lock and free list size in
ion_heap_init_deferred_free, which is only called when the
ION_HEAP_FLAG_DEFER_FREE heap flag is given.  However, the lock and size
are used in the shrinker path as well as the deferred free path, and we
can register a shrinker *without* enabling deferred freeing.  So, if a
heap provides a shrinker but *doesn't* set the DEFER_FREE flag we will
use these parameters uninitialized (resulting in a spinlock bug and
broken shrinker accounting).

Fix these problems by initializing the free list parameters directly in
ion_device_add_heap, which is always called no matter which heap
features are being used.

Signed-off-by: Mitchel Humpherys 
---
 drivers/staging/android/ion/ion.c  | 3 +++
 drivers/staging/android/ion/ion_heap.c | 2 --
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c 
b/drivers/staging/android/ion/ion.c
index 296d347660fc..b8f1c491553e 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -1508,6 +1508,9 @@ void ion_device_add_heap(struct ion_device *dev, struct 
ion_heap *heap)
pr_err("%s: can not add heap with invalid ops struct.\n",
   __func__);
 
+   spin_lock_init(&heap->free_lock);
+   heap->free_list_size = 0;
+
if (heap->flags & ION_HEAP_FLAG_DEFER_FREE)
ion_heap_init_deferred_free(heap);
 
diff --git a/drivers/staging/android/ion/ion_heap.c 
b/drivers/staging/android/ion/ion_heap.c
index 4605e04712aa..fd13d05b538a 100644
--- a/drivers/staging/android/ion/ion_heap.c
+++ b/drivers/staging/android/ion/ion_heap.c
@@ -253,8 +253,6 @@ int ion_heap_init_deferred_free(struct ion_heap *heap)
struct sched_param param = { .sched_priority = 0 };
 
INIT_LIST_HEAD(&heap->free_list);
-   heap->free_list_size = 0;
-   spin_lock_init(&heap->free_lock);
init_waitqueue_head(&heap->waitqueue);
heap->task = kthread_run(ion_heap_deferred_free, heap,
 "%s", heap->name);
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v10] iopoll: Introduce memory-mapped IO polling macros

2014-12-15 Thread Mitchel Humpherys
From: Matt Wagantall 

It is sometimes necessary to poll a memory-mapped register until its value
satisfies some condition. Introduce a family of convenience macros that do
this. Tight-looping, sleeping, and timing out can all be accomplished using
these macros.

Cc: Thierry Reding 
Cc: Will Deacon 
Cc: Arnd Bergmann 
Cc: Andrew Morton 
Cc: Robert Elliott 
Signed-off-by: Matt Wagantall 
Signed-off-by: Mitchel Humpherys 
---
v9..10:
  - Actually added the comments mentioned in v8..v9 (doh!)

v8..v9:
  - Added note in comments about max sleep time (Rob Elliott)

v7..v8:
  - sorted helper macros by size (b, w, l, q)
  - removed some of the more esoteric (or flat-out bogus) helper macros

This patch was originally part of a series [1] for adding support for IOMMU
address translations through an ARM SMMU hardware register.  The other
patch in the series (the one that actually uses these macros and implements
said hardware address translations) was Ack'd by the driver maintainer
there (Will Deacon) so I've pulled this patch out to avoid resending an
already Ack'd patch over and over again.

In short, please see [1] for previous discussion and the first user of
these macros.

Will also acked this patch in [2].  I didn't retain his Ack here because I
added to the macro comments.

[1] http://thread.gmane.org/gmane.linux.kernel.iommu/7140
[2] http://thread.gmane.org/gmane.linux.kernel.iommu/7449

---
 include/linux/iopoll.h | 144 +
 1 file changed, 144 insertions(+)
 create mode 100644 include/linux/iopoll.h

diff --git a/include/linux/iopoll.h b/include/linux/iopoll.h
new file mode 100644
index ..08fd52cdb5a0
--- /dev/null
+++ b/include/linux/iopoll.h
@@ -0,0 +1,144 @@
+/*
+ * Copyright (c) 2012-2014 The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef _LINUX_IOPOLL_H
+#define _LINUX_IOPOLL_H
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/**
+ * readx_poll_timeout - Periodically poll an address until a condition is met 
or a timeout occurs
+ * @op: accessor function (takes @addr as its only argument)
+ * @addr: Address to poll
+ * @val: Variable to read the value into
+ * @cond: Break condition (usually involving @val)
+ * @sleep_us: Maximum time to sleep between reads in us (0
+ *tight-loops).  Should be less than ~20ms since usleep_range
+ *is used (see Documentation/timers/timers-howto.txt).
+ * @timeout_us: Timeout in us, 0 means never timeout
+ *
+ * Returns 0 on success and -ETIMEDOUT upon a timeout. In either
+ * case, the last read value at @addr is stored in @val. Must not
+ * be called from atomic context if sleep_us or timeout_us are used.
+ *
+ * When available, you'll probably want to use one of the specialized
+ * macros defined below rather than this macro directly.
+ */
+#define readx_poll_timeout(op, addr, val, cond, sleep_us, timeout_us)  \
+({ \
+   ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
+   might_sleep_if(sleep_us); \
+   for (;;) { \
+   (val) = op(addr); \
+   if (cond) \
+   break; \
+   if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \
+   (val) = op(addr); \
+   break; \
+   } \
+   if (sleep_us) \
+   usleep_range((sleep_us >> 2) + 1, sleep_us); \
+   } \
+   (cond) ? 0 : -ETIMEDOUT; \
+})
+
+/**
+ * readx_poll_timeout_atomic - Periodically poll an address until a condition 
is met or a timeout occurs
+ * @op: accessor function (takes @addr as its only argument)
+ * @addr: Address to poll
+ * @val: Variable to read the value into
+ * @cond: Break condition (usually involving @val)
+ * @delay_us: Time to udelay between reads in us (0 tight-loops).  Should
+ *be less than ~10us since udelay is used (see
+ *Documentation/timers/timers-howto.txt).
+ * @timeout_us: Timeout in us, 0 means never timeout
+ *
+ * Returns 0 on success and -ETIMEDOUT upon a timeout. In either
+ * case, the last read value at @addr is stored in @val.
+ *
+ * When available, you'll probably want to use one of the specialized
+ * macros defined below rather than this macro directly.
+ */
+#define readx_poll_timeout_atomic(op, addr, val, cond, delay_us, timeout_us) \
+({ \
+   ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
+   for (;;) { \
+

Re: [PATCH v9] iopoll: Introduce memory-mapped IO polling macros

2014-12-15 Thread Mitchel Humpherys
On Mon, Dec 15 2014 at 03:31:20 PM, Mitchel Humpherys  
wrote:
> From: Matt Wagantall 
>
> It is sometimes necessary to poll a memory-mapped register until its value
> satisfies some condition. Introduce a family of convenience macros that do
> this. Tight-looping, sleeping, and timing out can all be accomplished using
> these macros.
>
> Cc: Thierry Reding 
> Cc: Will Deacon 
> Cc: Arnd Bergmann 
> Cc: Andrew Morton 
> Cc: Robert Elliott 
> Signed-off-by: Matt Wagantall 
> Signed-off-by: Mitchel Humpherys 
> ---
> v8..v9:
>   - Added note in comments about max sleep time (Rob Elliott)

Sorry, just noticed that I somehow dropped these additional comments
that Rob requested...  Let me send a v10 that actually includes them.
Please ignore this version.


-Mitch

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v9] iopoll: Introduce memory-mapped IO polling macros

2014-12-15 Thread Mitchel Humpherys
From: Matt Wagantall 

It is sometimes necessary to poll a memory-mapped register until its value
satisfies some condition. Introduce a family of convenience macros that do
this. Tight-looping, sleeping, and timing out can all be accomplished using
these macros.

Cc: Thierry Reding 
Cc: Will Deacon 
Cc: Arnd Bergmann 
Cc: Andrew Morton 
Cc: Robert Elliott 
Signed-off-by: Matt Wagantall 
Signed-off-by: Mitchel Humpherys 
---
v8..v9:
  - Added note in comments about max sleep time (Rob Elliott)

v7..v8:
  - sorted helper macros by size (b, w, l, q)
  - removed some of the more esoteric (or flat-out bogus) helper macros

This patch was originally part of a series [1] for adding support for IOMMU
address translations through an ARM SMMU hardware register.  The other
patch in the series (the one that actually uses these macros and implements
said hardware address translations) was Ack'd by the driver maintainer
there (Will Deacon) so I've pulled this patch out to avoid resending an
already Ack'd patch over and over again.

In short, please see [1] for previous discussion and the first user of
these macros.

Will also acked this patch in [2].  I didn't retain his Ack here because I
added to the macro comments.

[1] http://thread.gmane.org/gmane.linux.kernel.iommu/7140
[2] http://thread.gmane.org/gmane.linux.kernel.iommu/7449

---
 include/linux/iopoll.h | 140 +
 1 file changed, 140 insertions(+)
 create mode 100644 include/linux/iopoll.h

diff --git a/include/linux/iopoll.h b/include/linux/iopoll.h
new file mode 100644
index ..bd161dae2d40
--- /dev/null
+++ b/include/linux/iopoll.h
@@ -0,0 +1,140 @@
+/*
+ * Copyright (c) 2012-2014 The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef _LINUX_IOPOLL_H
+#define _LINUX_IOPOLL_H
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/**
+ * readx_poll_timeout - Periodically poll an address until a condition is met 
or a timeout occurs
+ * @op: accessor function (takes @addr as its only argument)
+ * @addr: Address to poll
+ * @val: Variable to read the value into
+ * @cond: Break condition (usually involving @val)
+ * @sleep_us: Maximum time to sleep between reads in us (0 tight-loops)
+ * @timeout_us: Timeout in us, 0 means never timeout
+ *
+ * Returns 0 on success and -ETIMEDOUT upon a timeout. In either
+ * case, the last read value at @addr is stored in @val. Must not
+ * be called from atomic context if sleep_us or timeout_us are used.
+ *
+ * When available, you'll probably want to use one of the specialized
+ * macros defined below rather than this macro directly.
+ */
+#define readx_poll_timeout(op, addr, val, cond, sleep_us, timeout_us)  \
+({ \
+   ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
+   might_sleep_if(sleep_us); \
+   for (;;) { \
+   (val) = op(addr); \
+   if (cond) \
+   break; \
+   if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \
+   (val) = op(addr); \
+   break; \
+   } \
+   if (sleep_us) \
+   usleep_range((sleep_us >> 2) + 1, sleep_us); \
+   } \
+   (cond) ? 0 : -ETIMEDOUT; \
+})
+
+/**
+ * readx_poll_timeout_atomic - Periodically poll an address until a condition 
is met or a timeout occurs
+ * @op: accessor function (takes @addr as its only argument)
+ * @addr: Address to poll
+ * @val: Variable to read the value into
+ * @cond: Break condition (usually involving @val)
+ * @delay_us: Time to udelay between reads in us (0 tight-loops)
+ * @timeout_us: Timeout in us, 0 means never timeout
+ *
+ * Returns 0 on success and -ETIMEDOUT upon a timeout. In either
+ * case, the last read value at @addr is stored in @val.
+ *
+ * When available, you'll probably want to use one of the specialized
+ * macros defined below rather than this macro directly.
+ */
+#define readx_poll_timeout_atomic(op, addr, val, cond, delay_us, timeout_us) \
+({ \
+   ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
+   for (;;) { \
+   (val) = op(addr); \
+   if (cond) \
+   break; \
+   if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \
+   (val) = op(addr); \
+   break; \
+   } \
+   if (delay_us) \
+ 

Re: [PATCH RESEND v8] iopoll: Introduce memory-mapped IO polling macros

2014-11-24 Thread Mitchel Humpherys
On Mon, Nov 24 2014 at 04:53:19 PM, "Elliott, Robert (Server Storage)" 
 wrote:
>> -Original Message-
>> From: linux-kernel-ow...@vger.kernel.org [mailto:linux-kernel-
>> ow...@vger.kernel.org] On Behalf Of Mitchel Humpherys
>> Sent: Monday, 24 November, 2014 2:15 PM
> ...
>> From: Matt Wagantall 
>> 
>> It is sometimes necessary to poll a memory-mapped register until its value
>> satisfies some condition. Introduce a family of convenience macros that do
>> this. Tight-looping, sleeping, and timing out can all be accomplished
>> using these macros.
>> 
> ...
>> +#define readx_poll_timeout(op, addr, val, cond, sleep_us, timeout_us)   
>> \
>> +({ \
>> +ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
>> +might_sleep_if(sleep_us); \
>> +for (;;) { \
>> +(val) = op(addr); \
>> +if (cond) \
>> +break; \
>> +if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) {
>> \
>> +(val) = op(addr); \
>> +break; \
>> +} \
>> +if (sleep_us) \
>> +usleep_range((sleep_us >> 2) + 1, sleep_us); \
>
> The hpsa SCSI driver used to use usleep_range in a loop like 
> that, but we found that it caused scheduler problems during
> boots because it uses TASK_UNINTERRUPTIBLE:
> [9.260668] [sched_delayed] sched: RT throttling activated
>
> msleep() worked much better.

Hmm, maybe you were just sleeping for too long?  According to
Documentation/timers/timers-howto.txt, usleep_range is what should be
used for non-atomic sleeps in the range [10us, 20ms].  Plus we need
microsecond granularity anyways, so msleep wouldn't cut it.

If there are any potential users of these macros that would want to
sleep for more than 20ms I guess we could add a special case here to use
msleep when sleep_us exceeds 20,000 or so.


-Mitch

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH RESEND v8] iopoll: Introduce memory-mapped IO polling macros

2014-11-24 Thread Mitchel Humpherys
From: Matt Wagantall 

It is sometimes necessary to poll a memory-mapped register until its value
satisfies some condition. Introduce a family of convenience macros that do
this. Tight-looping, sleeping, and timing out can all be accomplished using
these macros.

Cc: Thierry Reding 
Cc: Will Deacon 
Cc: Arnd Bergmann 
Cc: Andrew Morton 
Signed-off-by: Matt Wagantall 
Signed-off-by: Mitchel Humpherys 
---
This patch was originally part of a series [1] for adding support for IOMMU
address translations through an ARM SMMU hardware register.  The other
patch in the series (the one that actually uses these macros and implements
said hardware address translations) was Ack'd by the driver maintainer
there (Will Deacon) so I've pulled this patch out to avoid resending an
already Ack'd patch over and over again.

In short, please see [1] for previous discussion and the first user of
these macros.

This patch has been resent previously here [2], here [3], and here [4] on
2014-10-30, 2014-11-06, and 2014-11-17, respectively.  It has not changed
since [2] and has not received any comments since [1] on 2014-10-19.
Thanks to everyone who has taken a look at this.

[1] http://thread.gmane.org/gmane.linux.kernel.iommu/7140
[2] http://thread.gmane.org/gmane.linux.kernel/1818213
[3] http://thread.gmane.org/gmane.linux.kernel/1823422
[4] http://thread.gmane.org/gmane.linux.kernel.iommu/7394


Changes since v7:
  - sorted helper macros by size (b, w, l, q)
  - removed some of the more esoteric (or flat-out bogus) helper macros
---
 include/linux/iopoll.h | 140 +
 1 file changed, 140 insertions(+)
 create mode 100644 include/linux/iopoll.h

diff --git a/include/linux/iopoll.h b/include/linux/iopoll.h
new file mode 100644
index 00..bd161dae2d
--- /dev/null
+++ b/include/linux/iopoll.h
@@ -0,0 +1,140 @@
+/*
+ * Copyright (c) 2012-2014 The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef _LINUX_IOPOLL_H
+#define _LINUX_IOPOLL_H
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/**
+ * readx_poll_timeout - Periodically poll an address until a condition is met 
or a timeout occurs
+ * @op: accessor function (takes @addr as its only argument)
+ * @addr: Address to poll
+ * @val: Variable to read the value into
+ * @cond: Break condition (usually involving @val)
+ * @sleep_us: Maximum time to sleep between reads in us (0 tight-loops)
+ * @timeout_us: Timeout in us, 0 means never timeout
+ *
+ * Returns 0 on success and -ETIMEDOUT upon a timeout. In either
+ * case, the last read value at @addr is stored in @val. Must not
+ * be called from atomic context if sleep_us or timeout_us are used.
+ *
+ * When available, you'll probably want to use one of the specialized
+ * macros defined below rather than this macro directly.
+ */
+#define readx_poll_timeout(op, addr, val, cond, sleep_us, timeout_us)  \
+({ \
+   ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
+   might_sleep_if(sleep_us); \
+   for (;;) { \
+   (val) = op(addr); \
+   if (cond) \
+   break; \
+   if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \
+   (val) = op(addr); \
+   break; \
+   } \
+   if (sleep_us) \
+   usleep_range((sleep_us >> 2) + 1, sleep_us); \
+   } \
+   (cond) ? 0 : -ETIMEDOUT; \
+})
+
+/**
+ * readx_poll_timeout_atomic - Periodically poll an address until a condition 
is met or a timeout occurs
+ * @op: accessor function (takes @addr as its only argument)
+ * @addr: Address to poll
+ * @val: Variable to read the value into
+ * @cond: Break condition (usually involving @val)
+ * @delay_us: Time to udelay between reads in us (0 tight-loops)
+ * @timeout_us: Timeout in us, 0 means never timeout
+ *
+ * Returns 0 on success and -ETIMEDOUT upon a timeout. In either
+ * case, the last read value at @addr is stored in @val.
+ *
+ * When available, you'll probably want to use one of the specialized
+ * macros defined below rather than this macro directly.
+ */
+#define readx_poll_timeout_atomic(op, addr, val, cond, delay_us, timeout_us) \
+({ \
+   ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
+   for (;;) { \
+   (val) = op(addr); \
+   if (cond) \
+   break; \
+   if (timeout_us && ktime_compare(ktime_get

Re: [RFC] add a struct page* parameter to dma_map_ops.unmap_page

2014-11-21 Thread Mitchel Humpherys
On Fri, Nov 21 2014 at 03:48:33 AM, Stefano Stabellini 
 wrote:
> On Mon, 17 Nov 2014, Stefano Stabellini wrote:
>> Hi all,
>> I am writing this email to ask for your advice.
>> 
>> On architectures where dma addresses are different from physical
>> addresses, it can be difficult to retrieve the physical address of a
>> page from its dma address.
>> 
>> Specifically this is the case for Xen on arm and arm64 but I think that
>> other architectures might have the same issue.
>> 
>> Knowing the physical address is necessary to be able to issue any
>> required cache maintenance operations when unmap_page,
>> sync_single_for_cpu and sync_single_for_device are called.
>> 
>> Adding a struct page* parameter to unmap_page, sync_single_for_cpu and
>> sync_single_for_device would make Linux dma handling on Xen on arm and
>> arm64 much easier and quicker.
>> 
>> I think that other drivers have similar problems, such as the Intel
>> IOMMU driver having to call find_iova and walking down an rbtree to get
>> the physical address in its implementation of unmap_page.
>> 
>> Callers have the struct page* in their hands already from the previous
>> map_page call so it shouldn't be an issue for them.  A problem does
>> exist however: there are about 280 callers of dma_unmap_page and
>> pci_unmap_page. We have even more callers of the dma_sync_single_for_*
>> functions.
>> 
>> 
>> 
>> Is such a change even conceivable? How would one go about it?
>> 
>> I think that Xen would not be the only one to gain from it, but I would
>> like to have a confirmation from others: given the magnitude of the
>> changes involved I would actually prefer to avoid them unless multiple
>> drivers/archs/subsystems could really benefit from them.
>
> Given the lack of interest from the community, I am going to drop this
> idea.

Actually it sounds like the right API design to me.  As a bonus it
should help performance a bit as well.  For example, the current
implementations of dma_sync_single_for_{cpu,device} and dma_unmap_page
on ARM while using the IOMMU mapper
(arm_iommu_sync_single_for_{cpu,device}, arm_iommu_unmap_page) all call
iommu_iova_to_phys which generally results in a page table walk or a
hardware register write/poll/read.

The problem, as you mentioned, is that there are a ton of callers of the
existing APIs.  I think David Vrabel had a good suggestion for dealing
with this:

On Mon, Nov 17 2014 at 06:43:46 AM, David Vrabel  
wrote:
> You may need to consider a parallel set of map/unmap API calls that
> return/accept a handle, and then converting drivers one-by-one as
> required, instead of trying to convert every single driver at once.

However, I'm not sure whether the costs of having a parallel set of APIs
outweigh the benefits of a cleaner API and a slight performance boost...
But I hope the idea isn't completely abandoned without some profiling or
other evidence of its benefits (e.g. patches showing how drivers could
be simplified with the new APIs).


-Mitch

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH RESEND v8] iopoll: Introduce memory-mapped IO polling macros

2014-11-17 Thread Mitchel Humpherys
From: Matt Wagantall 

It is sometimes necessary to poll a memory-mapped register until its value
satisfies some condition. Introduce a family of convenience macros that do
this. Tight-looping, sleeping, and timing out can all be accomplished using
these macros.

Cc: Thierry Reding 
Cc: Will Deacon 
Cc: Arnd Bergmann 
Signed-off-by: Matt Wagantall 
Signed-off-by: Mitchel Humpherys 
---
Sorry for any confusion regarding the genesis of this patch.  Let me try to
clarify the history here.  This patch was originally part of a series [1]
for adding support for IOMMU address translations through an ARM SMMU
hardware register.  The other patch in the series (the one that actually
uses these macros and implements said hardware address translations) was
Ack'd by the driver maintainer there (Will Deacon) so I've pulled this
patch out to avoid resending an already Ack'd patch over and over again.

In short, please see [1] for previous discussion and the first user of
these macros.

[1] http://thread.gmane.org/gmane.linux.kernel.iommu/7140

Changes since v7:
  - sorted helper macros by size (b, w, l, q)
  - removed some of the more esoteric (or flat-out bogus) helper macros
---
 include/linux/iopoll.h | 140 +
 1 file changed, 140 insertions(+)
 create mode 100644 include/linux/iopoll.h

diff --git a/include/linux/iopoll.h b/include/linux/iopoll.h
new file mode 100644
index 00..bd161dae2d
--- /dev/null
+++ b/include/linux/iopoll.h
@@ -0,0 +1,140 @@
+/*
+ * Copyright (c) 2012-2014 The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef _LINUX_IOPOLL_H
+#define _LINUX_IOPOLL_H
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/**
+ * readx_poll_timeout - Periodically poll an address until a condition is met 
or a timeout occurs
+ * @op: accessor function (takes @addr as its only argument)
+ * @addr: Address to poll
+ * @val: Variable to read the value into
+ * @cond: Break condition (usually involving @val)
+ * @sleep_us: Maximum time to sleep between reads in us (0 tight-loops)
+ * @timeout_us: Timeout in us, 0 means never timeout
+ *
+ * Returns 0 on success and -ETIMEDOUT upon a timeout. In either
+ * case, the last read value at @addr is stored in @val. Must not
+ * be called from atomic context if sleep_us or timeout_us are used.
+ *
+ * When available, you'll probably want to use one of the specialized
+ * macros defined below rather than this macro directly.
+ */
+#define readx_poll_timeout(op, addr, val, cond, sleep_us, timeout_us)  \
+({ \
+   ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
+   might_sleep_if(sleep_us); \
+   for (;;) { \
+   (val) = op(addr); \
+   if (cond) \
+   break; \
+   if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \
+   (val) = op(addr); \
+   break; \
+   } \
+   if (sleep_us) \
+   usleep_range((sleep_us >> 2) + 1, sleep_us); \
+   } \
+   (cond) ? 0 : -ETIMEDOUT; \
+})
+
+/**
+ * readx_poll_timeout_atomic - Periodically poll an address until a condition 
is met or a timeout occurs
+ * @op: accessor function (takes @addr as its only argument)
+ * @addr: Address to poll
+ * @val: Variable to read the value into
+ * @cond: Break condition (usually involving @val)
+ * @delay_us: Time to udelay between reads in us (0 tight-loops)
+ * @timeout_us: Timeout in us, 0 means never timeout
+ *
+ * Returns 0 on success and -ETIMEDOUT upon a timeout. In either
+ * case, the last read value at @addr is stored in @val.
+ *
+ * When available, you'll probably want to use one of the specialized
+ * macros defined below rather than this macro directly.
+ */
+#define readx_poll_timeout_atomic(op, addr, val, cond, delay_us, timeout_us) \
+({ \
+   ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
+   for (;;) { \
+   (val) = op(addr); \
+   if (cond) \
+   break; \
+   if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \
+   (val) = op(addr); \
+   break; \
+   } \
+   if (delay_us) \
+   udelay(delay_us);   \
+   } \
+   (cond) ? 0 : -ETIMEDOUT; \
+})
+
+
+#define readb_poll_timeout(addr, val, cond, delay_us, timeout_us) \
+   readx_poll

[PATCH RESEND v8] iopoll: Introduce memory-mapped IO polling macros

2014-11-06 Thread Mitchel Humpherys
From: Matt Wagantall 

It is sometimes necessary to poll a memory-mapped register until its value
satisfies some condition. Introduce a family of convenience macros that do
this. Tight-looping, sleeping, and timing out can all be accomplished using
these macros.

Cc: Thierry Reding 
Cc: Will Deacon 
Cc: Arnd Bergmann 
Signed-off-by: Matt Wagantall 
Signed-off-by: Mitchel Humpherys 
---
Changes since v7:
  - sorted helper macros by size (b, w, l, q)
  - removed some of the more esoteric (or flat-out bogus) helper macros
---
 include/linux/iopoll.h | 140 +
 1 file changed, 140 insertions(+)
 create mode 100644 include/linux/iopoll.h

diff --git a/include/linux/iopoll.h b/include/linux/iopoll.h
new file mode 100644
index 00..bd161dae2d
--- /dev/null
+++ b/include/linux/iopoll.h
@@ -0,0 +1,140 @@
+/*
+ * Copyright (c) 2012-2014 The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef _LINUX_IOPOLL_H
+#define _LINUX_IOPOLL_H
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/**
+ * readx_poll_timeout - Periodically poll an address until a condition is met 
or a timeout occurs
+ * @op: accessor function (takes @addr as its only argument)
+ * @addr: Address to poll
+ * @val: Variable to read the value into
+ * @cond: Break condition (usually involving @val)
+ * @sleep_us: Maximum time to sleep between reads in us (0 tight-loops)
+ * @timeout_us: Timeout in us, 0 means never timeout
+ *
+ * Returns 0 on success and -ETIMEDOUT upon a timeout. In either
+ * case, the last read value at @addr is stored in @val. Must not
+ * be called from atomic context if sleep_us or timeout_us are used.
+ *
+ * When available, you'll probably want to use one of the specialized
+ * macros defined below rather than this macro directly.
+ */
+#define readx_poll_timeout(op, addr, val, cond, sleep_us, timeout_us)  \
+({ \
+   ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
+   might_sleep_if(sleep_us); \
+   for (;;) { \
+   (val) = op(addr); \
+   if (cond) \
+   break; \
+   if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \
+   (val) = op(addr); \
+   break; \
+   } \
+   if (sleep_us) \
+   usleep_range((sleep_us >> 2) + 1, sleep_us); \
+   } \
+   (cond) ? 0 : -ETIMEDOUT; \
+})
+
+/**
+ * readx_poll_timeout_atomic - Periodically poll an address until a condition 
is met or a timeout occurs
+ * @op: accessor function (takes @addr as its only argument)
+ * @addr: Address to poll
+ * @val: Variable to read the value into
+ * @cond: Break condition (usually involving @val)
+ * @delay_us: Time to udelay between reads in us (0 tight-loops)
+ * @timeout_us: Timeout in us, 0 means never timeout
+ *
+ * Returns 0 on success and -ETIMEDOUT upon a timeout. In either
+ * case, the last read value at @addr is stored in @val.
+ *
+ * When available, you'll probably want to use one of the specialized
+ * macros defined below rather than this macro directly.
+ */
+#define readx_poll_timeout_atomic(op, addr, val, cond, delay_us, timeout_us) \
+({ \
+   ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
+   for (;;) { \
+   (val) = op(addr); \
+   if (cond) \
+   break; \
+   if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \
+   (val) = op(addr); \
+   break; \
+   } \
+   if (delay_us) \
+   udelay(delay_us);   \
+   } \
+   (cond) ? 0 : -ETIMEDOUT; \
+})
+
+
+#define readb_poll_timeout(addr, val, cond, delay_us, timeout_us) \
+   readx_poll_timeout(readb, addr, val, cond, delay_us, timeout_us)
+
+#define readb_poll_timeout_atomic(addr, val, cond, delay_us, timeout_us) \
+   readx_poll_timeout_atomic(readb, addr, val, cond, delay_us, timeout_us)
+
+#define readw_poll_timeout(addr, val, cond, delay_us, timeout_us) \
+   readx_poll_timeout(readw, addr, val, cond, delay_us, timeout_us)
+
+#define readw_poll_timeout_atomic(addr, val, cond, delay_us, timeout_us) \
+   readx_poll_timeout_atomic(readw, addr, val, cond, delay_us, timeout_us)
+
+#define readl_poll_timeout(addr, val, cond, delay_us, timeout_us) \
+   readx_poll_timeout(readl, addr, val, cond, delay_us, timeout_us)
+
+#define rea

[PATCH v8] iopoll: Introduce memory-mapped IO polling macros

2014-10-30 Thread Mitchel Humpherys
From: Matt Wagantall 

It is sometimes necessary to poll a memory-mapped register until its value
satisfies some condition. Introduce a family of convenience macros that do
this. Tight-looping, sleeping, and timing out can all be accomplished using
these macros.

Cc: Thierry Reding 
Cc: Will Deacon 
Cc: Arnd Bergmann 
Signed-off-by: Matt Wagantall 
Signed-off-by: Mitchel Humpherys 
---
Changes since v7:
  - sorted helper macros by size (b, w, l, q)
  - removed some of the more esoteric (or flat-out bogus) helper macros
---
 include/linux/iopoll.h | 140 +
 1 file changed, 140 insertions(+)
 create mode 100644 include/linux/iopoll.h

diff --git a/include/linux/iopoll.h b/include/linux/iopoll.h
new file mode 100644
index 00..bd161dae2d
--- /dev/null
+++ b/include/linux/iopoll.h
@@ -0,0 +1,140 @@
+/*
+ * Copyright (c) 2012-2014 The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef _LINUX_IOPOLL_H
+#define _LINUX_IOPOLL_H
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/**
+ * readx_poll_timeout - Periodically poll an address until a condition is met 
or a timeout occurs
+ * @op: accessor function (takes @addr as its only argument)
+ * @addr: Address to poll
+ * @val: Variable to read the value into
+ * @cond: Break condition (usually involving @val)
+ * @sleep_us: Maximum time to sleep between reads in us (0 tight-loops)
+ * @timeout_us: Timeout in us, 0 means never timeout
+ *
+ * Returns 0 on success and -ETIMEDOUT upon a timeout. In either
+ * case, the last read value at @addr is stored in @val. Must not
+ * be called from atomic context if sleep_us or timeout_us are used.
+ *
+ * When available, you'll probably want to use one of the specialized
+ * macros defined below rather than this macro directly.
+ */
+#define readx_poll_timeout(op, addr, val, cond, sleep_us, timeout_us)  \
+({ \
+   ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
+   might_sleep_if(sleep_us); \
+   for (;;) { \
+   (val) = op(addr); \
+   if (cond) \
+   break; \
+   if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \
+   (val) = op(addr); \
+   break; \
+   } \
+   if (sleep_us) \
+   usleep_range((sleep_us >> 2) + 1, sleep_us); \
+   } \
+   (cond) ? 0 : -ETIMEDOUT; \
+})
+
+/**
+ * readx_poll_timeout_atomic - Periodically poll an address until a condition 
is met or a timeout occurs
+ * @op: accessor function (takes @addr as its only argument)
+ * @addr: Address to poll
+ * @val: Variable to read the value into
+ * @cond: Break condition (usually involving @val)
+ * @delay_us: Time to udelay between reads in us (0 tight-loops)
+ * @timeout_us: Timeout in us, 0 means never timeout
+ *
+ * Returns 0 on success and -ETIMEDOUT upon a timeout. In either
+ * case, the last read value at @addr is stored in @val.
+ *
+ * When available, you'll probably want to use one of the specialized
+ * macros defined below rather than this macro directly.
+ */
+#define readx_poll_timeout_atomic(op, addr, val, cond, delay_us, timeout_us) \
+({ \
+   ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
+   for (;;) { \
+   (val) = op(addr); \
+   if (cond) \
+   break; \
+   if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \
+   (val) = op(addr); \
+   break; \
+   } \
+   if (delay_us) \
+   udelay(delay_us);   \
+   } \
+   (cond) ? 0 : -ETIMEDOUT; \
+})
+
+
+#define readb_poll_timeout(addr, val, cond, delay_us, timeout_us) \
+   readx_poll_timeout(readb, addr, val, cond, delay_us, timeout_us)
+
+#define readb_poll_timeout_atomic(addr, val, cond, delay_us, timeout_us) \
+   readx_poll_timeout_atomic(readb, addr, val, cond, delay_us, timeout_us)
+
+#define readw_poll_timeout(addr, val, cond, delay_us, timeout_us) \
+   readx_poll_timeout(readw, addr, val, cond, delay_us, timeout_us)
+
+#define readw_poll_timeout_atomic(addr, val, cond, delay_us, timeout_us) \
+   readx_poll_timeout_atomic(readw, addr, val, cond, delay_us, timeout_us)
+
+#define readl_poll_timeout(addr, val, cond, delay_us, timeout_us) \
+   readx_poll_timeout(readl, addr, val, cond, delay_us, timeout_us)
+
+#define rea

Re: [PATCH v2 0/9] staging: ion: system heap and page pool fixes

2014-05-28 Thread Mitchel Humpherys
On Tue, May 27 2014 at 11:52:51 PM, Heesub Shin  wrote:
> Hi,
>
> Here is my patchset with some modification, hoping reviews or comments
> from you guys.
>
> v2:
>  o No changes in the code, just reworded changelog
>  o Reorder patch 

Some very nice cleanup. And I learned a new trick with page.lru :).

Reviewed-by: Mitchel Humpherys 

>
> Heesub Shin (9):
>   staging: ion: tidy up a bit
>   staging: ion: simplify ion_page_pool_total()
>   staging: ion: remove struct ion_page_pool_item
>   staging: ion: use compound pages on high order pages for system heap
>   staging: ion: remove order from struct page_info
>   staging: ion: remove struct page_info
>   staging: ion: remove order argument from free_buffer_page()
>   staging: ion: shrink highmem pages on kswapd
>   staging: ion: optimize struct ion_system_heap
>
>  drivers/staging/android/ion/ion_page_pool.c   |  49 ---
>  drivers/staging/android/ion/ion_priv.h|   2 +-
>  drivers/staging/android/ion/ion_system_heap.c | 121 
> ++
>  3 files changed, 67 insertions(+), 105 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 7/9] staging: ion: remove order argument from free_buffer_page()

2014-05-27 Thread Mitchel Humpherys
On Mon, May 26 2014 at 03:04:59 AM, Heesub Shin  wrote:
> Not that the pages returned from the pool are compound pages, we do not
> need to pass the order information to free_buffer_page().

s/Not/Now/


-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 6/9] staging: ion: remove struct page_info

2014-05-27 Thread Mitchel Humpherys
On Mon, May 26 2014 at 03:04:58 AM, Heesub Shin  wrote:
> ION system heap uses a temporary list holding meta data on pages
> allocated to build scatter/gather table. Now that the pages are compound
> pages, we do not need to introduce a new data type redundantly.
>
> Signed-off-by: Heesub Shin 
> ---
>  drivers/staging/android/ion/ion_system_heap.c | 47 
> +--
>  1 file changed, 15 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/staging/android/ion/ion_system_heap.c 
> b/drivers/staging/android/ion/ion_system_heap.c
> index 73a2e67..f0ae210 100644
> --- a/drivers/staging/android/ion/ion_system_heap.c
> +++ b/drivers/staging/android/ion/ion_system_heap.c
> @@ -51,11 +51,6 @@ struct ion_system_heap {
>   struct ion_page_pool **pools;
>  };
>  
> -struct page_info {
> - struct page *page;
> - struct list_head list;
> -};
> -
>  static struct page *alloc_buffer_page(struct ion_system_heap *heap,
> struct ion_buffer *buffer,
> unsigned long order)
> @@ -96,19 +91,14 @@ static void free_buffer_page(struct ion_system_heap *heap,
>  }
>  
>  
> -static struct page_info *alloc_largest_available(struct ion_system_heap 
> *heap,
> -  struct ion_buffer *buffer,
> -  unsigned long size,
> -  unsigned int max_order)
> +static struct page *alloc_largest_available(struct ion_system_heap *heap,
> + struct ion_buffer *buffer,
> + unsigned long size,
> + unsigned int max_order)

Was this whitespace-only change intentional?


-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[RESEND PATCH] staging: ion: WARN when the handle kmap_cnt is going to wrap around

2014-05-23 Thread Mitchel Humpherys
There are certain client bugs (double unmap, for example) that can cause
the handle->kmap_cnt (an unsigned int) to wrap around from zero. This
causes problems when the handle is destroyed because we have:

while (handle->kmap_cnt)
ion_handle_kmap_put(handle);

which takes a long time to complete when kmap_cnt starts at ~0 and can
result in a watchdog timeout.

WARN and bail when kmap_cnt is about to wrap around from zero.

Signed-off-by: Mitchel Humpherys 
Acked-by: Colin Cross 
---
Resending since I missed a few folks on the original. Also retaining
Colin's Acked-by.
---
 drivers/staging/android/ion/ion.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/staging/android/ion/ion.c 
b/drivers/staging/android/ion/ion.c
index 3d5bf14722..f55f61a4cc 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -626,6 +626,10 @@ static void ion_handle_kmap_put(struct ion_handle *handle)
 {
struct ion_buffer *buffer = handle->buffer;
 
+   if (!handle->kmap_cnt) {
+   WARN(1, "%s: Double unmap detected! bailing...\n", __func__);
+   return;
+   }
handle->kmap_cnt--;
if (!handle->kmap_cnt)
ion_buffer_kmap_put(buffer);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] staging: ion: WARN when the handle kmap_cnt is going to wrap around

2014-05-23 Thread Mitchel Humpherys
++greg-kh and de...@driverdev.osuosl.org
(my bad for missing you the first time around)

On Thu, May 22 2014 at 06:09:11 PM, Colin Cross  wrote:
> On Thu, May 22, 2014 at 5:51 PM, Mitchel Humpherys
>  wrote:
>> There are certain client bugs (double unmap, for example) that can cause
>> the handle->kmap_cnt (an unsigned int) to wrap around from zero. This
>> causes problems when the handle is destroyed because we have:
>>
>> while (handle->kmap_cnt)
>> ion_handle_kmap_put(handle);
>>
>> which takes a long time to complete when kmap_cnt starts at ~0 and can
>> result in a watchdog timeout.
>>
>> WARN and bail when kmap_cnt is about to wrap around from zero.
>>
>> Signed-off-by: Mitchel Humpherys 
>> ---
>>  drivers/staging/android/ion/ion.c | 4 
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/staging/android/ion/ion.c 
>> b/drivers/staging/android/ion/ion.c
>> index 3d5bf14722..f55f61a4cc 100644
>> --- a/drivers/staging/android/ion/ion.c
>> +++ b/drivers/staging/android/ion/ion.c
>> @@ -626,6 +626,10 @@ static void ion_handle_kmap_put(struct ion_handle 
>> *handle)
>>  {
>> struct ion_buffer *buffer = handle->buffer;
>>
>> +   if (!handle->kmap_cnt) {
>> +   WARN(1, "%s: Double unmap detected! bailing...\n", __func__);
>> +   return;
>> +   }
>> handle->kmap_cnt--;
>> if (!handle->kmap_cnt)
>> ion_buffer_kmap_put(buffer);
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>> hosted by The Linux Foundation
>>
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to kernel-team+unsubscr...@android.com.
>
> Acked-by: Colin Cross 

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] staging: ion: WARN when the handle kmap_cnt is going to wrap around

2014-05-22 Thread Mitchel Humpherys
There are certain client bugs (double unmap, for example) that can cause
the handle->kmap_cnt (an unsigned int) to wrap around from zero. This
causes problems when the handle is destroyed because we have:

while (handle->kmap_cnt)
ion_handle_kmap_put(handle);

which takes a long time to complete when kmap_cnt starts at ~0 and can
result in a watchdog timeout.

WARN and bail when kmap_cnt is about to wrap around from zero.

Signed-off-by: Mitchel Humpherys 
---
 drivers/staging/android/ion/ion.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/staging/android/ion/ion.c 
b/drivers/staging/android/ion/ion.c
index 3d5bf14722..f55f61a4cc 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -626,6 +626,10 @@ static void ion_handle_kmap_put(struct ion_handle *handle)
 {
struct ion_buffer *buffer = handle->buffer;
 
+   if (!handle->kmap_cnt) {
+   WARN(1, "%s: Double unmap detected! bailing...\n", __func__);
+   return;
+   }
handle->kmap_cnt--;
if (!handle->kmap_cnt)
ion_buffer_kmap_put(buffer);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 1/7] arm64: Use pr_* instead of printk

2014-04-29 Thread Mitchel Humpherys
On Mon, Apr 28 2014 at 09:59:14 PM, Jungseok Lee  wrote:
> This patch fixed the following checkpatch complaint as using pr_*
> instead of printk.
>
> WARNING: printk() should include KERN_ facility level
>
> Cc: Catalin Marinas 
> Signed-off-by: Jungseok Lee 
> Reviewed-by: Sungjinn Chung 
> ---
>  arch/arm64/kernel/traps.c |   14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> index 7ffaddd..0484e81 100644
> --- a/arch/arm64/kernel/traps.c
> +++ b/arch/arm64/kernel/traps.c
> @@ -65,7 +65,7 @@ static void dump_mem(const char *lvl, const char *str, 
> unsigned long bottom,
>   fs = get_fs();
>   set_fs(KERNEL_DS);
>  
> - printk("%s%s(0x%016lx to 0x%016lx)\n", lvl, str, bottom, top);
> + pr_emerg("%s%s(0x%016lx to 0x%016lx)\n", lvl, str, bottom, top);

Currently this printk is being called with lvl=KERN_EMERG or lvl="". In
the case of lvl=KERN_EMERG leaving lvl in is redundant. In the case of
lvl="" this is a behavioral change (printing to a different log
level). Was this intended?

>  
>   for (first = bottom & ~31; first < top; first += 32) {
>   unsigned long p;
> @@ -83,7 +83,7 @@ static void dump_mem(const char *lvl, const char *str, 
> unsigned long bottom,
>   sprintf(str + i * 9, " ");
>   }
>   }
> - printk("%s%04lx:%s\n", lvl, first & 0x, str);
> + pr_emerg("%s%04lx:%s\n", lvl, first & 0x, str);

Ditto

>   }
>  
>   set_fs(fs);
> @@ -124,7 +124,7 @@ static void dump_instr(const char *lvl, struct pt_regs 
> *regs)
>   break;
>   }
>   }
> - printk("%sCode: %s\n", lvl, str);
> + pr_emerg("%sCode: %s\n", lvl, str);

Ditto. Also called with with lvl=KERN_INFO.


Mitch

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] ion: only use the CMA heap when CONFIG_CMA is enabled

2014-04-17 Thread Mitchel Humpherys
The CMA heap is intended to be used with CMA (as the name
suggests). Don't compile or use it if CONFIG_CMA is not
enabled.

Currently, if CONFIG_CMA=n and someone creates and uses a CMA heap, some
of their allocations might actually succeed (since the CMA heap is just
using generic DMA API routines) but the fact that the memory isn't
coming from CMA is confusing.

Signed-off-by: Mitchel Humpherys 
---
 drivers/staging/android/ion/Makefile   | 3 ++-
 drivers/staging/android/ion/ion_heap.c | 4 
 drivers/staging/android/ion/ion_priv.h | 3 +++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/android/ion/Makefile 
b/drivers/staging/android/ion/Makefile
index b56fd2bf2b..83923eac97 100644
--- a/drivers/staging/android/ion/Makefile
+++ b/drivers/staging/android/ion/Makefile
@@ -1,5 +1,6 @@
 obj-$(CONFIG_ION) +=   ion.o ion_heap.o ion_page_pool.o ion_system_heap.o \
-   ion_carveout_heap.o ion_chunk_heap.o ion_cma_heap.o
+   ion_carveout_heap.o ion_chunk_heap.o
+obj-$(CONFIG_CMA) += ion_cma_heap.o
 obj-$(CONFIG_ION_TEST) += ion_test.o
 ifdef CONFIG_COMPAT
 obj-$(CONFIG_ION) += compat_ion.o
diff --git a/drivers/staging/android/ion/ion_heap.c 
b/drivers/staging/android/ion/ion_heap.c
index bdc6a28ba8..d72940e631 100644
--- a/drivers/staging/android/ion/ion_heap.c
+++ b/drivers/staging/android/ion/ion_heap.c
@@ -332,9 +332,11 @@ struct ion_heap *ion_heap_create(struct ion_platform_heap 
*heap_data)
case ION_HEAP_TYPE_CHUNK:
heap = ion_chunk_heap_create(heap_data);
break;
+#ifdef CONFIG_CMA
case ION_HEAP_TYPE_DMA:
heap = ion_cma_heap_create(heap_data);
break;
+#endif
default:
pr_err("%s: Invalid heap type %d\n", __func__,
   heap_data->type);
@@ -371,9 +373,11 @@ void ion_heap_destroy(struct ion_heap *heap)
case ION_HEAP_TYPE_CHUNK:
ion_chunk_heap_destroy(heap);
break;
+#ifdef CONFIG_CMA
case ION_HEAP_TYPE_DMA:
ion_cma_heap_destroy(heap);
break;
+#endif
default:
pr_err("%s: Invalid heap type %d\n", __func__,
   heap->type);
diff --git a/drivers/staging/android/ion/ion_priv.h 
b/drivers/staging/android/ion/ion_priv.h
index 1eba3f2076..42e541e961 100644
--- a/drivers/staging/android/ion/ion_priv.h
+++ b/drivers/staging/android/ion/ion_priv.h
@@ -323,8 +323,11 @@ void ion_carveout_heap_destroy(struct ion_heap *);
 
 struct ion_heap *ion_chunk_heap_create(struct ion_platform_heap *);
 void ion_chunk_heap_destroy(struct ion_heap *);
+
+#ifdef CONFIG_CMA
 struct ion_heap *ion_cma_heap_create(struct ion_platform_heap *);
 void ion_cma_heap_destroy(struct ion_heap *);
+#endif
 
 /**
  * kernel api to allocate/free from carveout -- used when carveout is
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v4] Some printk cleanup in mm

2014-04-15 Thread Mitchel Humpherys
This series cleans up some printks in the mm code that were missing
log levels.

Changelog:

  - v4: Remove redundant prefixes due to pr_fmt, improve commit
message (suggested by Andrew Morton)

  - v3: Leaving slub.c alone. It's using hand-tagged printk's
correctly so it's probably just churn to convert everything to the
pr_ macros.

  - v2: Suggestions by Joe Perches (pr_fmt, pr_cont, pr_err, __func__,
    missing \n)

Mitchel Humpherys (1):
  mm: convert some level-less printks to pr_*

 mm/bounce.c|  7 +--
 mm/mempolicy.c |  5 -
 mm/mmap.c  | 21 -
 mm/nommu.c |  5 -
 mm/vmscan.c|  5 -
 5 files changed, 29 insertions(+), 14 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v4] mm: convert some level-less printks to pr_*

2014-04-15 Thread Mitchel Humpherys
printk is meant to be used with an associated log level. There are some
instances of printk scattered around the mm code where the log level is
missing. Add a log level and adhere to suggestions by
scripts/checkpatch.pl by moving to the pr_* macros.

Also add the typical pr_fmt definition so that print statements can be
easily traced back to the modules where they occur, correlated one with
another, etc. This will require the removal of some (now redundant)
prefixes on a few print statements.

Signed-off-by: Mitchel Humpherys 
---
 mm/bounce.c|  7 +--
 mm/mempolicy.c |  5 -
 mm/mmap.c  | 21 -
 mm/nommu.c |  5 -
 mm/vmscan.c|  5 -
 5 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/mm/bounce.c b/mm/bounce.c
index 523918b8c6..ab21ba203d 100644
--- a/mm/bounce.c
+++ b/mm/bounce.c
@@ -3,6 +3,8 @@
  * - Split from highmem.c
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include 
 #include 
 #include 
@@ -15,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -34,7 +37,7 @@ static __init int init_emergency_pool(void)
 
page_pool = mempool_create_page_pool(POOL_SIZE, 0);
BUG_ON(!page_pool);
-   printk("bounce pool size: %d pages\n", POOL_SIZE);
+   pr_info("pool size: %d pages\n", POOL_SIZE);
 
return 0;
 }
@@ -86,7 +89,7 @@ int init_emergency_isa_pool(void)
   mempool_free_pages, (void *) 0);
BUG_ON(!isa_page_pool);
 
-   printk("isa bounce pool size: %d pages\n", ISA_POOL_SIZE);
+   pr_info("isa pool size: %d pages\n", ISA_POOL_SIZE);
return 0;
 }
 
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 78e1472933..d7c2e8fe01 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -65,6 +65,8 @@
kernel is not always grateful with that.
 */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include 
 #include 
 #include 
@@ -91,6 +93,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -2645,7 +2648,7 @@ void __init numa_policy_init(void)
node_set(prefer, interleave_nodes);
 
if (do_set_mempolicy(MPOL_INTERLEAVE, 0, &interleave_nodes))
-   printk("numa_policy_init: interleaving failed\n");
+   pr_err("%s: interleaving failed\n", __func__);
 
check_numabalancing_enable();
 }
diff --git a/mm/mmap.c b/mm/mmap.c
index b1202cf81f..6bdf81669f 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -6,6 +6,8 @@
  * Address space accounting code   
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include 
 #include 
 #include 
@@ -37,6 +39,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -361,20 +364,20 @@ static int browse_rb(struct rb_root *root)
struct vm_area_struct *vma;
vma = rb_entry(nd, struct vm_area_struct, vm_rb);
if (vma->vm_start < prev) {
-   printk("vm_start %lx prev %lx\n", vma->vm_start, prev);
+   pr_info("vm_start %lx prev %lx\n", vma->vm_start, prev);
bug = 1;
}
if (vma->vm_start < pend) {
-   printk("vm_start %lx pend %lx\n", vma->vm_start, pend);
+   pr_info("vm_start %lx pend %lx\n", vma->vm_start, pend);
bug = 1;
}
if (vma->vm_start > vma->vm_end) {
-   printk("vm_end %lx < vm_start %lx\n",
+   pr_info("vm_end %lx < vm_start %lx\n",
vma->vm_end, vma->vm_start);
bug = 1;
}
if (vma->rb_subtree_gap != vma_compute_subtree_gap(vma)) {
-   printk("free gap %lx, correct %lx\n",
+   pr_info("free gap %lx, correct %lx\n",
   vma->rb_subtree_gap,
   vma_compute_subtree_gap(vma));
bug = 1;
@@ -388,7 +391,7 @@ static int browse_rb(struct rb_root *root)
for (nd = pn; nd; nd = rb_prev(nd))
j++;
if (i != j) {
-   printk("backwards %d, forwards %d\n", j, i);
+   pr_info("backwards %d, forwards %d\n", j, i);
bug = 1;
}
return bug ? -1 : i;
@@ -423,17 +426,17 @@ static void validate_mm(struct mm_struct *mm)
i++;
}
if (i != mm->map_count) {
-   printk("map_count %d vm_next %d\n", mm->map_count, i);
+   pr_info("map_count %d vm_next %d\n", mm->map_count, i);
bug = 1;
}
if (highest_address != m

Re: [PATCH v2] mm: convert some level-less printks to pr_*

2014-04-15 Thread Mitchel Humpherys
On Tue, Apr 15 2014 at 04:58:21 PM, Mitchel Humpherys  
wrote:
> On Mon, Apr 14 2014 at 03:55:26 PM, Andrew Morton  
> wrote:
>> And all of this should be described and justified in the changelog,
>> please.
>
> Will send a v3 shortly. Thanks for your comments.

Make that a v4, I actually already sent a v3. You'd think I could get a
printk change right on v1 :). We'll get there.

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2] mm: convert some level-less printks to pr_*

2014-04-15 Thread Mitchel Humpherys
On Mon, Apr 14 2014 at 03:55:26 PM, Andrew Morton  
wrote:
> On Thu, 27 Mar 2014 10:54:19 -0700 Mitchel Humpherys 
>  wrote:
>>  #include 
>>  #include 
>>  #include 
>> @@ -15,6 +17,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  
>>  #include 
>> @@ -34,7 +37,7 @@ static __init int init_emergency_pool(void)
>>  
>>  page_pool = mempool_create_page_pool(POOL_SIZE, 0);
>>  BUG_ON(!page_pool);
>> -printk("bounce pool size: %d pages\n", POOL_SIZE);
>> +pr_info("bounce pool size: %d pages\n", POOL_SIZE);
>
> This used to print "bounce pool size: N pages" but will now print
> "bounce: bounce pool size: N pages".
>
> It isn't necessarily a *bad* change but perhaps a little more thought
> could be put into it.  In this example it would be better remove the
> redundancy by using 
>
>   pr_info("pool size: %d pages\n"...);

Yes I noticed this in my boot-test... I'll change it to remove the
redundancy. The others all seem okay.

>
> And all of this should be described and justified in the changelog,
> please.

Will send a v3 shortly. Thanks for your comments.

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v3] mm: convert some level-less printks to pr_*

2014-04-07 Thread Mitchel Humpherys
printk is meant to be used with an associated log level. There are some
instances of printk scattered around the mm code where the log level is
missing. Add a log level and adhere to suggestions by
scripts/checkpatch.pl by moving to the pr_* macros.

Signed-off-by: Mitchel Humpherys 
---
 mm/bounce.c|  7 +--
 mm/mempolicy.c |  5 -
 mm/mmap.c  | 21 -
 mm/nommu.c |  5 -
 mm/vmscan.c|  5 -
 5 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/mm/bounce.c b/mm/bounce.c
index 523918b8c6..d35850895b 100644
--- a/mm/bounce.c
+++ b/mm/bounce.c
@@ -3,6 +3,8 @@
  * - Split from highmem.c
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include 
 #include 
 #include 
@@ -15,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -34,7 +37,7 @@ static __init int init_emergency_pool(void)
 
page_pool = mempool_create_page_pool(POOL_SIZE, 0);
BUG_ON(!page_pool);
-   printk("bounce pool size: %d pages\n", POOL_SIZE);
+   pr_info("bounce pool size: %d pages\n", POOL_SIZE);
 
return 0;
 }
@@ -86,7 +89,7 @@ int init_emergency_isa_pool(void)
   mempool_free_pages, (void *) 0);
BUG_ON(!isa_page_pool);
 
-   printk("isa bounce pool size: %d pages\n", ISA_POOL_SIZE);
+   pr_info("isa bounce pool size: %d pages\n", ISA_POOL_SIZE);
return 0;
 }
 
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index e3ab028227..ec6c90fc51 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -65,6 +65,8 @@
kernel is not always grateful with that.
 */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include 
 #include 
 #include 
@@ -91,6 +93,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -2679,7 +2682,7 @@ void __init numa_policy_init(void)
node_set(prefer, interleave_nodes);
 
if (do_set_mempolicy(MPOL_INTERLEAVE, 0, &interleave_nodes))
-   printk("numa_policy_init: interleaving failed\n");
+   pr_err("%s: interleaving failed\n", __func__);
 
check_numabalancing_enable();
 }
diff --git a/mm/mmap.c b/mm/mmap.c
index 46433e137a..7cb79eb2fc 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -6,6 +6,8 @@
  * Address space accounting code   
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include 
 #include 
 #include 
@@ -36,6 +38,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -360,20 +363,20 @@ static int browse_rb(struct rb_root *root)
struct vm_area_struct *vma;
vma = rb_entry(nd, struct vm_area_struct, vm_rb);
if (vma->vm_start < prev) {
-   printk("vm_start %lx prev %lx\n", vma->vm_start, prev);
+   pr_info("vm_start %lx prev %lx\n", vma->vm_start, prev);
bug = 1;
}
if (vma->vm_start < pend) {
-   printk("vm_start %lx pend %lx\n", vma->vm_start, pend);
+   pr_info("vm_start %lx pend %lx\n", vma->vm_start, pend);
bug = 1;
}
if (vma->vm_start > vma->vm_end) {
-   printk("vm_end %lx < vm_start %lx\n",
+   pr_info("vm_end %lx < vm_start %lx\n",
vma->vm_end, vma->vm_start);
bug = 1;
}
if (vma->rb_subtree_gap != vma_compute_subtree_gap(vma)) {
-   printk("free gap %lx, correct %lx\n",
+   pr_info("free gap %lx, correct %lx\n",
   vma->rb_subtree_gap,
   vma_compute_subtree_gap(vma));
bug = 1;
@@ -387,7 +390,7 @@ static int browse_rb(struct rb_root *root)
for (nd = pn; nd; nd = rb_prev(nd))
j++;
if (i != j) {
-   printk("backwards %d, forwards %d\n", j, i);
+   pr_info("backwards %d, forwards %d\n", j, i);
bug = 1;
}
return bug ? -1 : i;
@@ -422,17 +425,17 @@ static void validate_mm(struct mm_struct *mm)
i++;
}
if (i != mm->map_count) {
-   printk("map_count %d vm_next %d\n", mm->map_count, i);
+   pr_info("map_count %d vm_next %d\n", mm->map_count, i);
bug = 1;
}
if (highest_address != mm->highest_vm_end) {
-   printk("mm->highest_vm_end %lx, found %lx\n",
+   pr_info("mm->highest_vm_end %lx, found %lx\n",
   mm->highest_vm_end, highest_address);
 

[PATCH v3] Some printk cleanup in mm

2014-04-07 Thread Mitchel Humpherys
This series cleans up some printks in the mm code that were missing
log levels.

Changelog:

  - v3: Leaving slub.c alone. It's using hand-tagged printk's
correctly so it's probably just churn to convert everything to the
pr_ macros.

  - v2: Suggestions by Joe Perches (pr_fmt, pr_cont, pr_err, __func__,
    missing \n)

Mitchel Humpherys (1):
  mm: convert some level-less printks to pr_*

 mm/bounce.c|  7 +--
 mm/mempolicy.c |  5 -
 mm/mmap.c  | 21 -
 mm/nommu.c |  5 -
 mm/vmscan.c|  5 -
 5 files changed, 29 insertions(+), 14 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2] mm: convert some level-less printks to pr_*

2014-04-03 Thread Mitchel Humpherys
On Thu, Apr 03 2014 at 09:33:15 AM, Christoph Lameter  wrote:
> On Mon, 31 Mar 2014, Joe Perches wrote:
>
>> On Mon, 2014-03-31 at 13:35 -0500, Christoph Lameter wrote:
>> > On Thu, 27 Mar 2014, Mitchel Humpherys wrote:
>> >
>> > > diff --git a/mm/slub.c b/mm/slub.c
>> []
>> > > @@ -9,6 +9,8 @@
>> > >   * (C) 2011 Linux Foundation, Christoph Lameter
>> > >   */
>> > >
>> > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> >
>> > This is implicitly used by some macros? If so then please define this
>> > elsewhere. I do not see any use in slub.c of this one.
>>
>> Hi Christoph
>>
>> All the pr_ macros use it.
>>
>> from include/linux/printk.h:
>
> Ok then why do you add the definition to slub.c?

Ah that was an oversight on my part after changing to pr_cont. I'll send
a v3 that removes the pr_fmt. Or I could send a v3 that leaves the
pr_fmt but changes the printk that the pr_cont's are continuing (at the
top of note_cmpxchg_failure) to pr_info, but that wouldn't be consistent
with the rest of the file, which is using hand-tagged printk's.

I don't know if it's worthwhile to convert all of the hand-tagged
printk's to the pr_ macros, but if so I can do that in a separate patch.

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] arm: ftrace: work with CONFIG_DEBUG_SET_MODULE_RONX

2014-04-03 Thread Mitchel Humpherys
On Wed, Apr 02 2014 at 06:04:29 PM, Laura Abbott  wrote:
> I think Mitch tested this on our internal targets. I'll let him reply with 
> his Tested-by
>
> Laura

Indeed:

Tested-by: Mitchel Humpherys 

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2] mm: convert some level-less printks to pr_*

2014-03-27 Thread Mitchel Humpherys
printk is meant to be used with an associated log level. There are some
instances of printk scattered around the mm code where the log level is
missing. Add a log level and adhere to suggestions by
scripts/checkpatch.pl by moving to the pr_* macros.

Signed-off-by: Mitchel Humpherys 
---
 mm/bounce.c|  7 +--
 mm/mempolicy.c |  5 -
 mm/mmap.c  | 21 -
 mm/nommu.c |  5 -
 mm/slub.c  |  9 ++---
 mm/vmscan.c|  5 -
 6 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/mm/bounce.c b/mm/bounce.c
index 523918b8c6..d35850895b 100644
--- a/mm/bounce.c
+++ b/mm/bounce.c
@@ -3,6 +3,8 @@
  * - Split from highmem.c
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include 
 #include 
 #include 
@@ -15,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -34,7 +37,7 @@ static __init int init_emergency_pool(void)
 
page_pool = mempool_create_page_pool(POOL_SIZE, 0);
BUG_ON(!page_pool);
-   printk("bounce pool size: %d pages\n", POOL_SIZE);
+   pr_info("bounce pool size: %d pages\n", POOL_SIZE);
 
return 0;
 }
@@ -86,7 +89,7 @@ int init_emergency_isa_pool(void)
   mempool_free_pages, (void *) 0);
BUG_ON(!isa_page_pool);
 
-   printk("isa bounce pool size: %d pages\n", ISA_POOL_SIZE);
+   pr_info("isa bounce pool size: %d pages\n", ISA_POOL_SIZE);
return 0;
 }
 
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index ae3c8f3595..aec6220485 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -65,6 +65,8 @@
kernel is not always grateful with that.
 */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include 
 #include 
 #include 
@@ -91,6 +93,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -2751,7 +2754,7 @@ void __init numa_policy_init(void)
node_set(prefer, interleave_nodes);
 
if (do_set_mempolicy(MPOL_INTERLEAVE, 0, &interleave_nodes))
-   printk("numa_policy_init: interleaving failed\n");
+   pr_err("%s: interleaving failed\n", __func__);
 
check_numabalancing_enable();
 }
diff --git a/mm/mmap.c b/mm/mmap.c
index 20ff0c3327..c7023516da 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -6,6 +6,8 @@
  * Address space accounting code   
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include 
 #include 
 #include 
@@ -36,6 +38,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -360,20 +363,20 @@ static int browse_rb(struct rb_root *root)
struct vm_area_struct *vma;
vma = rb_entry(nd, struct vm_area_struct, vm_rb);
if (vma->vm_start < prev) {
-   printk("vm_start %lx prev %lx\n", vma->vm_start, prev);
+   pr_info("vm_start %lx prev %lx\n", vma->vm_start, prev);
bug = 1;
}
if (vma->vm_start < pend) {
-   printk("vm_start %lx pend %lx\n", vma->vm_start, pend);
+   pr_info("vm_start %lx pend %lx\n", vma->vm_start, pend);
bug = 1;
}
if (vma->vm_start > vma->vm_end) {
-   printk("vm_end %lx < vm_start %lx\n",
+   pr_info("vm_end %lx < vm_start %lx\n",
vma->vm_end, vma->vm_start);
bug = 1;
}
if (vma->rb_subtree_gap != vma_compute_subtree_gap(vma)) {
-   printk("free gap %lx, correct %lx\n",
+   pr_info("free gap %lx, correct %lx\n",
   vma->rb_subtree_gap,
   vma_compute_subtree_gap(vma));
bug = 1;
@@ -387,7 +390,7 @@ static int browse_rb(struct rb_root *root)
for (nd = pn; nd; nd = rb_prev(nd))
j++;
if (i != j) {
-   printk("backwards %d, forwards %d\n", j, i);
+   pr_info("backwards %d, forwards %d\n", j, i);
bug = 1;
}
return bug ? -1 : i;
@@ -422,17 +425,17 @@ void validate_mm(struct mm_struct *mm)
i++;
}
if (i != mm->map_count) {
-   printk("map_count %d vm_next %d\n", mm->map_count, i);
+   pr_info("map_count %d vm_next %d\n", mm->map_count, i);
bug = 1;
}
if (highest_address != mm->highest_vm_end) {
-   printk("mm->highest_vm_end %lx, found %lx\n",
+   pr_info("mm->highest_vm_end %lx, found %lx\n",
   mm->highest_vm_end

[PATCH v2] Some printk cleanup in mm

2014-03-27 Thread Mitchel Humpherys
This series cleans up some printks in the mm code that were missing
log levels.

Changelog:

  - v2: Suggestions by Joe Perches (pr_fmt, pr_cont, pr_err, __func__,
missing \n)

Mitchel Humpherys (1):
  mm: convert some level-less printks to pr_*

 mm/bounce.c|  7 +--
 mm/mempolicy.c |  5 -
 mm/mmap.c  | 21 -
 mm/nommu.c |  5 -
 mm/slub.c  |  9 ++---
 mm/vmscan.c|  5 -
 6 files changed, 35 insertions(+), 17 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] mm: convert some level-less printks to pr_*

2014-03-26 Thread Mitchel Humpherys
printk is meant to be used with an associated log level. There are some
instances of printk scattered around the mm code where the log level is
missing. Add a log level and adhere to suggestions by
scripts/checkpatch.pl by moving to the pr_* macros.

Signed-off-by: Mitchel Humpherys 
---
 mm/bounce.c|  5 +++--
 mm/mempolicy.c |  3 ++-
 mm/mmap.c  | 19 ++-
 mm/nommu.c |  3 ++-
 mm/slub.c  |  7 ---
 mm/vmscan.c|  3 ++-
 6 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/mm/bounce.c b/mm/bounce.c
index 523918b8c6..7fca8ef660 100644
--- a/mm/bounce.c
+++ b/mm/bounce.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -34,7 +35,7 @@ static __init int init_emergency_pool(void)
 
page_pool = mempool_create_page_pool(POOL_SIZE, 0);
BUG_ON(!page_pool);
-   printk("bounce pool size: %d pages\n", POOL_SIZE);
+   pr_info("bounce pool size: %d pages\n", POOL_SIZE);
 
return 0;
 }
@@ -86,7 +87,7 @@ int init_emergency_isa_pool(void)
   mempool_free_pages, (void *) 0);
BUG_ON(!isa_page_pool);
 
-   printk("isa bounce pool size: %d pages\n", ISA_POOL_SIZE);
+   pr_info("isa bounce pool size: %d pages\n", ISA_POOL_SIZE);
return 0;
 }
 
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index ae3c8f3595..81ae17cf92 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -91,6 +91,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -2751,7 +2752,7 @@ void __init numa_policy_init(void)
node_set(prefer, interleave_nodes);
 
if (do_set_mempolicy(MPOL_INTERLEAVE, 0, &interleave_nodes))
-   printk("numa_policy_init: interleaving failed\n");
+   pr_warn("numa_policy_init: interleaving failed\n");
 
check_numabalancing_enable();
 }
diff --git a/mm/mmap.c b/mm/mmap.c
index 20ff0c3327..52f85997e0 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -36,6 +36,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -360,20 +361,20 @@ static int browse_rb(struct rb_root *root)
struct vm_area_struct *vma;
vma = rb_entry(nd, struct vm_area_struct, vm_rb);
if (vma->vm_start < prev) {
-   printk("vm_start %lx prev %lx\n", vma->vm_start, prev);
+   pr_info("vm_start %lx prev %lx\n", vma->vm_start, prev);
bug = 1;
}
if (vma->vm_start < pend) {
-   printk("vm_start %lx pend %lx\n", vma->vm_start, pend);
+   pr_info("vm_start %lx pend %lx\n", vma->vm_start, pend);
bug = 1;
}
if (vma->vm_start > vma->vm_end) {
-   printk("vm_end %lx < vm_start %lx\n",
+   pr_info("vm_end %lx < vm_start %lx\n",
vma->vm_end, vma->vm_start);
bug = 1;
}
if (vma->rb_subtree_gap != vma_compute_subtree_gap(vma)) {
-   printk("free gap %lx, correct %lx\n",
+   pr_info("free gap %lx, correct %lx\n",
   vma->rb_subtree_gap,
   vma_compute_subtree_gap(vma));
bug = 1;
@@ -387,7 +388,7 @@ static int browse_rb(struct rb_root *root)
for (nd = pn; nd; nd = rb_prev(nd))
j++;
if (i != j) {
-   printk("backwards %d, forwards %d\n", j, i);
+   pr_info("backwards %d, forwards %d\n", j, i);
bug = 1;
}
return bug ? -1 : i;
@@ -422,17 +423,17 @@ void validate_mm(struct mm_struct *mm)
i++;
}
if (i != mm->map_count) {
-   printk("map_count %d vm_next %d\n", mm->map_count, i);
+   pr_info("map_count %d vm_next %d\n", mm->map_count, i);
bug = 1;
}
if (highest_address != mm->highest_vm_end) {
-   printk("mm->highest_vm_end %lx, found %lx\n",
+   pr_info("mm->highest_vm_end %lx, found %lx\n",
   mm->highest_vm_end, highest_address);
bug = 1;
}
i = browse_rb(&mm->mm_rb);
if (i != mm->map_count) {
-   printk("map_count %d rb %d\n", mm->map_count, i);
+   pr_info("map_count %d rb %d\n", mm->map_count, i);
bug = 1;
}
BUG_ON(bug);
@@ -3237,7 +3238,7 @@ static struct notifier_block reserve_mem_nb = {
 static int __meminit init_reserve_notifier

[PATCH] Some printk cleanup in mm

2014-03-26 Thread Mitchel Humpherys
This series cleans up some printks in the mm code that were missing
log levels.

Mitchel Humpherys (1):
  mm: convert some level-less printks to pr_*

 mm/bounce.c|  5 +++--
 mm/mempolicy.c |  3 ++-
 mm/mmap.c  | 19 ++-
 mm/nommu.c |  3 ++-
 mm/slub.c  |  7 ---
 mm/vmscan.c|  3 ++-
 6 files changed, 23 insertions(+), 17 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 1/2] Documentation/SubmittingPatches: update some dead URLs

2014-03-25 Thread Mitchel Humpherys
The links to "The perfect patch" and "NO No more huge patch
bombs..." have gone stale. Update them to some working locations.

Signed-off-by: Mitchel Humpherys 
---
 Documentation/SubmittingPatches   | 4 ++--
 Documentation/ja_JP/SubmittingPatches | 4 ++--
 Documentation/zh_CN/SubmittingPatches | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 26b1e31d5a..af80689517 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -725,7 +725,7 @@ SECTION 3 - REFERENCES
 --
 
 Andrew Morton, "The perfect patch" (tpp).
-  <http://userweb.kernel.org/~akpm/stuff/tpp.txt>
+  <http://www.ozlabs.org/~akpm/stuff/tpp.txt>
 
 Jeff Garzik, "Linux kernel patch submission format".
   <http://linux.yyz.us/patch-format.html>
@@ -738,7 +738,7 @@ Greg Kroah-Hartman, "How to piss off a kernel subsystem 
maintainer".
   <http://www.kroah.com/log/linux/maintainer-05.html>
 
 NO No more huge patch bombs to linux-kernel@vger.kernel.org people!
-  <http://marc.theaimsgroup.com/?l=linux-kernel&m=112112749912944&w=2>
+  <https://lkml.org/lkml/2005/7/11/336>
 
 Kernel Documentation/CodingStyle:
   <http://users.sosdg.org/~qiyong/lxr/source/Documentation/CodingStyle>
diff --git a/Documentation/ja_JP/SubmittingPatches 
b/Documentation/ja_JP/SubmittingPatches
index 97f78dd0c0..ff6cb5729c 100644
--- a/Documentation/ja_JP/SubmittingPatches
+++ b/Documentation/ja_JP/SubmittingPatches
@@ -695,7 +695,7 @@ gcc においては、マクロと同じくらい軽いです。
 --
 
 Andrew Morton, "The perfect patch" (tpp).
-  <http://userweb.kernel.org/~akpm/stuff/tpp.txt>
+  <http://www.ozlabs.org/~akpm/stuff/tpp.txt>
 
 Jeff Garzik, "Linux kernel patch submission format".
   <http://linux.yyz.us/patch-format.html>
@@ -707,7 +707,7 @@ Greg Kroah-Hartman, "How to piss off a kernel subsystem 
maintainer".
   <http://www.kroah.com/log/2006/01/11/>
 
 NO No more huge patch bombs to linux-kernel@vger.kernel.org people!
-  <http://marc.theaimsgroup.com/?l=linux-kernel&m=112112749912944&w=2>
+  <https://lkml.org/lkml/2005/7/11/336>
 
 Kernel Documentation/CodingStyle:
   <http://users.sosdg.org/~qiyong/lxr/source/Documentation/CodingStyle>
diff --git a/Documentation/zh_CN/SubmittingPatches 
b/Documentation/zh_CN/SubmittingPatches
index be0bd47250..788ab47303 100644
--- a/Documentation/zh_CN/SubmittingPatches
+++ b/Documentation/zh_CN/SubmittingPatches
@@ -394,7 +394,7 @@ Static inline 函数相比宏来说,是好得多的选择。Static inline 函
 
 
 Andrew Morton, "The perfect patch" (tpp).
-  <http://userweb.kernel.org/~akpm/stuff/tpp.txt>
+  <http://www.ozlabs.org/~akpm/stuff/tpp.txt>
 
 Jeff Garzik, "Linux kernel patch submission format".
   <http://linux.yyz.us/patch-format.html>
@@ -406,7 +406,7 @@ Greg Kroah-Hartman, "How to piss off a kernel subsystem 
maintainer".
   <http://www.kroah.com/log/2006/01/11/>
 
 NO No more huge patch bombs to linux-kernel@vger.kernel.org people!
-  <http://marc.theaimsgroup.com/?l=linux-kernel&m=112112749912944&w=2>
+  <https://lkml.org/lkml/2005/7/11/336>
 
 Kernel Documentation/CodingStyle:
   <http://sosdg.org/~coywolf/lxr/source/Documentation/CodingStyle>
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 2/2] Documentation/SubmittingPatches: remove references to patch-scripts

2014-03-25 Thread Mitchel Humpherys
The link to the tarball for Andrew Morton's patch scripts is dead. These
scripts don't seem to be used for kernel development these days anyways
so just rip out all references to them.

Signed-off-by: Mitchel Humpherys 
---
 Documentation/SubmittingPatches   | 5 -
 Documentation/ja_JP/SubmittingPatches | 5 -
 Documentation/zh_CN/SubmittingPatches | 4 
 3 files changed, 14 deletions(-)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index af80689517..3294469efd 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -75,11 +75,6 @@ There are a number of scripts which can aid in this:
 Quilt:
 http://savannah.nongnu.org/projects/quilt
 
-Andrew Morton's patch scripts:
-http://userweb.kernel.org/~akpm/stuff/patch-scripts.tar.gz
-Instead of these scripts, quilt is the recommended patch management
-tool (see above).
-
 
 
 2) Describe your changes.
diff --git a/Documentation/ja_JP/SubmittingPatches 
b/Documentation/ja_JP/SubmittingPatches
index ff6cb5729c..5d6ae639bf 100644
--- a/Documentation/ja_JP/SubmittingPatches
+++ b/Documentation/ja_JP/SubmittingPatches
@@ -98,11 +98,6 @@ dontdiff ファイルには Linux カーネルのビルドプロセスの過程
 Quilt:
 http://savannah.nongnu.org/projects/quilt
 
-Andrew Morton's patch scripts:
-http://userweb.kernel.org/~akpm/stuff/patch-scripts.tar.gz
-このリンクの先のスクリプトの代わりとして、quilt がパッチマネジメント
-ツールとして推奨されています(上のリンクを見てください)。
-
 2) パッチに対する説明
 
 パッチの中の変更点に対する技術的な詳細について説明してください。
diff --git a/Documentation/zh_CN/SubmittingPatches 
b/Documentation/zh_CN/SubmittingPatches
index 788ab47303..1d3a10f874 100644
--- a/Documentation/zh_CN/SubmittingPatches
+++ b/Documentation/zh_CN/SubmittingPatches
@@ -82,10 +82,6 @@ Documentation/SubmittingDrivers 。
 Quilt:
 http://savannah.nongnu.org/projects/quilt
 
-Andrew Morton 的补丁脚本:
-http://userweb.kernel.org/~akpm/stuff/patch-scripts.tar.gz
-作为这些脚本的替代,quilt 是值得推荐的补丁管理工具(看上面的链接)。
-
 2)描述你的改动。
 描述你的改动包含的技术细节。
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 0/2] URL cleanup in Documentation/SubmittingPatches

2014-03-25 Thread Mitchel Humpherys
Some URLs in Documentation/SubmittingPatches have gone stale. This
series cleans some of them up by removing or updating them.

Mitchel Humpherys (2):
  Documentation/SubmittingPatches: update some dead URLs
  Documentation/SubmittingPatches: remove references to patch-scripts

 Documentation/SubmittingPatches   | 9 ++---
 Documentation/ja_JP/SubmittingPatches | 9 ++---
 Documentation/zh_CN/SubmittingPatches | 8 ++--
 3 files changed, 6 insertions(+), 20 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/