Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 04/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_prepare()
From: Hanjun Guo @ 2017-01-02 13:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483363905-2806-1-git-send-email-hanjun.guo@linaro.org>

Adding ACPI support for platform MSI, we need to retrieve the
dev id in ACPI way instead of device tree, we already have
a well formed function its_pmsi_prepare() to get the dev id
but it's OF dependent, so collect OF related code and put them
into a single function to make its_pmsi_prepare() more friendly
to ACPI later.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Tested-by: Sinan Kaya <okaya@codeaurora.org>
Tested-by: Majun <majun258@huawei.com>
Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Tomasz Nowicki <tn@semihalf.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/irqchip/irq-gic-v3-its-platform-msi.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its-platform-msi.c b/drivers/irqchip/irq-gic-v3-its-platform-msi.c
index 470b4aa..3c94278 100644
--- a/drivers/irqchip/irq-gic-v3-its-platform-msi.c
+++ b/drivers/irqchip/irq-gic-v3-its-platform-msi.c
@@ -24,15 +24,11 @@
 	.name			= "ITS-pMSI",
 };
 
-static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
-			    int nvec, msi_alloc_info_t *info)
+static int of_pmsi_get_dev_id(struct irq_domain *domain, struct device *dev,
+				  u32 *dev_id)
 {
-	struct msi_domain_info *msi_info;
-	u32 dev_id;
 	int ret, index = 0;
 
-	msi_info = msi_get_domain_info(domain->parent);
-
 	/* Suck the DeviceID out of the msi-parent property */
 	do {
 		struct of_phandle_args args;
@@ -43,11 +39,24 @@ static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
 		if (args.np == irq_domain_get_of_node(domain)) {
 			if (WARN_ON(args.args_count != 1))
 				return -EINVAL;
-			dev_id = args.args[0];
+			*dev_id = args.args[0];
 			break;
 		}
 	} while (!ret);
 
+	return ret;
+}
+
+static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
+			    int nvec, msi_alloc_info_t *info)
+{
+	struct msi_domain_info *msi_info;
+	u32 dev_id;
+	int ret;
+
+	msi_info = msi_get_domain_info(domain->parent);
+
+	ret = of_pmsi_get_dev_id(domain, dev, &dev_id);
 	if (ret)
 		return ret;
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH v6 03/14] ACPI: ARM64: IORT: add missing comment for iort_dev_find_its_id()
From: Hanjun Guo @ 2017-01-02 13:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483363905-2806-1-git-send-email-hanjun.guo@linaro.org>

We are missing req_id's comment for iort_dev_find_its_id(),
add it back.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Tested-by: Majun <majun258@huawei.com>
Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Tomasz Nowicki <tn@semihalf.com>
---
 drivers/acpi/arm64/iort.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 46e2d82..174e983 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -446,6 +446,7 @@ u32 iort_msi_map_rid(struct device *dev, u32 req_id)
 /**
  * iort_dev_find_its_id() - Find the ITS identifier for a device
  * @dev: The device.
+ * @req_id: Device's Requster ID
  * @idx: Index of the ITS identifier list.
  * @its_id: ITS identifier.
  *
-- 
1.9.1

^ permalink raw reply related

* [PATCH v6 02/14] irqchip: gic-v3-its: keep the head file include in alphabetic order
From: Hanjun Guo @ 2017-01-02 13:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483363905-2806-1-git-send-email-hanjun.guo@linaro.org>

The head file is strictly in alphabetic order now, so let's
be the rule breaker. As acpi_iort.h includes acpi.h so remove
the duplidate acpi.h inclusion as well.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Tested-by: Majun <majun258@huawei.com>
Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Tomasz Nowicki <tn@semihalf.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 69b040f..f471939 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -15,14 +15,13 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include <linux/acpi.h>
+#include <linux/acpi_iort.h>
 #include <linux/bitmap.h>
 #include <linux/cpu.h>
 #include <linux/delay.h>
 #include <linux/dma-iommu.h>
 #include <linux/interrupt.h>
 #include <linux/irqdomain.h>
-#include <linux/acpi_iort.h>
 #include <linux/log2.h>
 #include <linux/mm.h>
 #include <linux/msi.h>
-- 
1.9.1

^ permalink raw reply related

* [PATCH v6 01/14] ACPI: ARM64: IORT: minor cleanup for iort_match_node_callback()
From: Hanjun Guo @ 2017-01-02 13:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483363905-2806-1-git-send-email-hanjun.guo@linaro.org>

Cleanup iort_match_node_callback() a little bit to reduce
some lines of code, aslo fix the indentation in iort_scan_node().

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Tested-by: Majun <majun258@huawei.com>
Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Tomasz Nowicki <tn@semihalf.com>
---
 drivers/acpi/arm64/iort.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index e0d2e6e..46e2d82 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -225,7 +225,7 @@ static struct acpi_iort_node *iort_scan_node(enum acpi_iort_node_type type,
 
 		if (iort_node->type == type &&
 		    ACPI_SUCCESS(callback(iort_node, context)))
-				return iort_node;
+			return iort_node;
 
 		iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort_node,
 					 iort_node->length);
@@ -253,17 +253,15 @@ static acpi_status iort_match_node_callback(struct acpi_iort_node *node,
 					    void *context)
 {
 	struct device *dev = context;
-	acpi_status status;
+	acpi_status status = AE_NOT_FOUND;
 
 	if (node->type == ACPI_IORT_NODE_NAMED_COMPONENT) {
 		struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
 		struct acpi_device *adev = to_acpi_device_node(dev->fwnode);
 		struct acpi_iort_named_component *ncomp;
 
-		if (!adev) {
-			status = AE_NOT_FOUND;
+		if (!adev)
 			goto out;
-		}
 
 		status = acpi_get_name(adev->handle, ACPI_FULL_PATHNAME, &buf);
 		if (ACPI_FAILURE(status)) {
@@ -289,8 +287,6 @@ static acpi_status iort_match_node_callback(struct acpi_iort_node *node,
 		 */
 		status = pci_rc->pci_segment_number == pci_domain_nr(bus) ?
 							AE_OK : AE_NOT_FOUND;
-	} else {
-		status = AE_NOT_FOUND;
 	}
 out:
 	return status;
-- 
1.9.1

^ permalink raw reply related

* [PATCH v6 00/14] ACPI platform MSI support and its example mbigen
From: Hanjun Guo @ 2017-01-02 13:31 UTC (permalink / raw)
  To: linux-arm-kernel

v5 -> v6:
	- Call acpi_configure_pmsi_domain() for platform devices in
	  acpi_platform_notify() as it's cleaner (suggested by Rafael)
	- Remove the "u8 type" for iort_id_map() because it's unused
	- Rebase on top of 4.10-rc2
	- Collect test and review tags

v4 -> v5:
	- Add mbigen support back with tested on with Agustin's patchset,
	  and it's a good example of how ACPI platform MSI works
	- rebased on top of lastest Linus tree (commit 52bce91 splice: reinstate SIGPIPE/EPIPE handling)

v3 -> v4:
        - Drop mbi-gen patches to just submit platform msi support because
          will rebase mbi-gen patches on top of Agustin's patchset, and discusion
          is going there.
        - Add a patch to support device topology such as NC(named componant, paltform device)
          ->SMMU->ITS which suggested by Lorenzo;
        - rebased on top of Lorenzo's v9 of ACPI IORT ARM SMMU support;
        - rebased on top of 4.9-rc7

v2 -> v3:
        - Drop RFC tag
        - Rebase against v4.9-rc2 and Lorenzo's v6 of ACPI IORT ARM SMMU support [1]
        - Add 3 cleanup patches (patch 1, 2, 3)
        - Drop arch_init call patch from last version
        - Introduce a callback for platform device to set msi domain
        - Introduce a new API to get paltform device's domain instead of
          reusing the PCI one in previous version
        - Add a patch to rework iort_node_get_id()

[1]: http://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1251993.html

v1 -> v2:
        - Fix the bug of if multi Interrupt() resoures in single _PRS,
          we need to calculate all the irq numbers (I missed it in previous
          version);
        - Rebased on Marc's irq/irqchip-4.9 branch and Lorenzo's v5
          SMMU patches (also Robin's SMMu patches)
        - Add patch irqchip: mbigen: promote mbigen init.

With platform msi support landed in the kernel, and the introduction
of IORT for GICv3 ITS (PCI MSI) and SMMU, the framework for platform msi
is ready, this patch set add few patches to enable the ACPI platform
msi support.

For platform device connecting to ITS on arm platform, we have IORT
table with the named componant node to describe the mappings of paltform
device and ITS, so we can retrieve the dev id and find its parent
irqdomain (ITS) from IORT table (simlar with the ACPI ITS support).

The fisrt 3 patches are cleanups;

Patch 4,5 are refactoring its_pmsi_prepare() for both DT and ACPI
then retrieve the dev id from iort;

Patch 6,7 to create platform msi domain to ACPI case which scanned
the MADT table;

Patch 8,9,10,11 to setup the msi domain for platform device based
on IORT table.

Patch 12,13,14 convert DT based mbigen driver to support ACPI/DT.

Thanks
Hanjun

Hanjun Guo (12):
  ACPI: ARM64: IORT: minor cleanup for iort_match_node_callback()
  irqchip: gic-v3-its: keep the head file include in alphabetic order
  ACPI: ARM64: IORT: add missing comment for iort_dev_find_its_id()
  irqchip: gicv3-its: platform-msi: refactor its_pmsi_prepare()
  ACPI: platform-msi: retrieve dev id from IORT
  irqchip: gicv3-its: platform-msi: refactor its_pmsi_init() to prepare
    for ACPI
  irqchip: gicv3-its: platform-msi: scan MADT to create platform msi
    domain
  ACPI: ARM64: IORT: rework iort_node_get_id()
  ACPI: platform: setup MSI domain for ACPI based platform device
  ACPI: ARM64: IORT: rework iort_node_get_id() for NC->SMMU->ITS case
  msi: platform: make platform_msi_create_device_domain() ACPI aware
  irqchip: mbigen: Add ACPI support

Kefeng Wang (2):
  irqchip: mbigen: drop module owner
  irqchip: mbigen: introduce mbigen_of_create_domain()

 drivers/acpi/arm64/iort.c                     | 140 ++++++++++++++++++++------
 drivers/acpi/glue.c                           |   6 ++
 drivers/base/platform-msi.c                   |   3 +-
 drivers/irqchip/irq-gic-v3-its-platform-msi.c | 106 ++++++++++++++-----
 drivers/irqchip/irq-gic-v3-its.c              |   3 +-
 drivers/irqchip/irq-mbigen.c                  | 109 ++++++++++++++++----
 include/linux/acpi_iort.h                     |  11 ++
 7 files changed, 299 insertions(+), 79 deletions(-)

-- 
1.9.1

^ permalink raw reply

* [PATCH v7 2/5] i2c: Add STM32F4 I2C driver
From: M'boumba Cedric Madianga @ 2017-01-02 13:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAOAejn1ocTSvdPWnZ-+DwMfJj45cXXEDc1+=B5d3zCy2Pw+0-A@mail.gmail.com>

Hello Uwe,

>> Is it possible to make it more obvious by doing:
>>
>>         status = read_from_status_register() & read_from_interrupt_enable_register();
>>
>> at a single place?
Contrary to what I said previously I have to keep possible_status
variable as for one irq enabled we allow several events.
For example, ITBUFEN allows to generate an irq for RXNE and for TXE events.
So, using status = read_from_status_register() &
read_from_interrupt_enable_register(); is not possible.

Best regards,

Cedric

^ permalink raw reply

* [PATCH] serial: 8250: use initializer instead of memset to clear local struct
From: Russell King - ARM Linux @ 2017-01-02 13:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161223072026.GA23895@kroah.com>

On Fri, Dec 23, 2016 at 08:20:26AM +0100, Greg Kroah-Hartman wrote:
> On Fri, Dec 23, 2016 at 12:21:48PM +0900, Masahiro Yamada wrote:
> > Leave the way of zero-out to the compiler's decision; the compiler
> > may know a more optimized way than calling memset().
> 
> But no, it doesn't, it will leave "blank" areas in the structure with
> bad data in it, which is why we do memset.  See the tree-wide fixups we
> made about a year ago for this very issue.  Are you sure none of these
> structures get copied to userspace?
> 
> > It may end up with memset() for big structures like this after all,
> > but the code will be cleaner at least.
> 
> Please leave it as-is, unless you see a measured speedup.

We can probably have both... we have an "optimisation" in ARM for
zero-based memset()s which was beneficial with older compilers, but
I suspect GCC 4 does a much better job itself of optimising
memset().  arch/arm/include/asm/string.h:

#define memset(p,v,n)                                                   \
        ({                                                              \
                void *__p = (p); size_t __n = n;                        \
                if ((__n) != 0) {                                       \
                        if (__builtin_constant_p((v)) && (v) == 0)      \
                                __memzero((__p),(__n));                 \
                        else                                            \
                                memset((__p),(v),(__n));                \
                }                                                       \
                (__p);                                                  \
        })

I suspect we should get rid of that with GCC >= 4.

I also suspect that it'll make no difference for uart_8250_port, as
it's rather large, but for smaller structures (eg, up to a cache line)
GCC can probably optimise to inline initialisation.

So, probably something for resulting code and performance analysis...

It's worth noting that 32-bit x86 always uses __builtin_memset() for
memset() on GCC >= 4, so GCC's memset() optimisations must be safe for
structures copied to userspace, or if not, 32-bit x86 is probably
rather buggy.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply

* [PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED
From: Sricharan @ 2017-01-02 13:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <0f67b88d-5b93-f9a8-7c70-900170a4e9c2@arm.com>

Hi Robin,

>>
>> <snip..>
>>
>>>>>>>  		return prot | IOMMU_READ | IOMMU_WRITE;
>>>>>>
>>>>>> ...and applying against -next now also needs this hunk:
>>>>>>
>>>>>> @@ -639,7 +639,7 @@ dma_addr_t iommu_dma_map_resource(struct device
>>>>>> *dev, phys_addr_t phys,
>>>>>> 		size_t size, enum dma_data_direction dir, unsigned long attrs)
>>>>>> {
>>>>>> 	return __iommu_dma_map(dev, phys, size,
>>>>>> -			dma_direction_to_prot(dir, false) | IOMMU_MMIO);
>>>>>> +			dma_info_to_prot(dir, false, attrs) | IOMMU_MMIO);
>>>>>> }
>>>>>>
>>>>>> void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,
>>>>>>
>>>>>> With those two issues fixed up, I've given the series (applied to
>>>>>> next-20161213) a spin on a SMMUv3/PL330 fast model and it still checks out.
>>>>>>
>>>>>
>>>>> oops, sorry that i missed this in rebase. I can repost now with this fixed,
>>>>> 'checks out' you mean something is not working correct ?
>>>>
>>>> No, I mean it _is_ still correct - I guess that's more of an idiom than
>>>> I thought :)
>>>>
>>>
>>> ha ok, thanks for the testing as well. I will just send a v8 with those two fixed now.
>>
>> Just while checking that i have not missed anything else, realized that the
>> dma-mapping apis in arm as to be modified to pass the PRIVILIGED attributes
>> as well. While my testing path was using the iommu_map directly i was not
>> seeing this, but then i did a patch like below. I will just figure out another
>> other codebase where the master uses the dma apis, test and add it in the
>> V8 that i would send.
>
>True, adding support to 32-bit as well can't hurt, and I guess it's
>equally relevant to QC's GPU use-case. I haven't considered it myself
>because AArch32 is immune to the specific PL330 problem which caught me
>out - that subtle corner of VMSAv8 is unique to AArch64.
>
>> From: Sricharan R <sricharan@codeaurora.org>
>> Date: Tue, 13 Dec 2016 23:25:01 +0530
>> Subject: [PATCH V8 6/9] arm/dma-mapping: Implement DMA_ATTR_PRIVILEGED
>>
>> The newly added DMA_ATTR_PRIVILEGED is useful for creating mappings that
>> are only accessible to privileged DMA engines.  Implementing it in dma-mapping
>> for it to get used from the dma mappings apis.
>>
>> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
>> ---
>>  arch/arm/mm/dma-mapping.c | 24 +++++++++++++++---------
>>  1 file changed, 15 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
>> index ab77100..e0d9923 100644
>> --- a/arch/arm/mm/dma-mapping.c
>> +++ b/arch/arm/mm/dma-mapping.c
>> @@ -1394,7 +1394,8 @@ static int __iommu_free_buffer(struct device *dev, struct page **pages,
>>   * Create a mapping in device IO address space for specified pages
>>   */
>>  static dma_addr_t
>> -__iommu_create_mapping(struct device *dev, struct page **pages, size_t size)
>> +__iommu_create_mapping(struct device *dev, struct page **pages, size_t size,
>> +		       unsigned long attrs)
>>  {
>>  	struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev);
>>  	unsigned int count = PAGE_ALIGN(size) >> PAGE_SHIFT;
>> @@ -1419,7 +1420,7 @@ static int __iommu_free_buffer(struct device *dev, struct page **pages,
>>
>>  		len = (j - i) << PAGE_SHIFT;
>>  		ret = iommu_map(mapping->domain, iova, phys, len,
>> -				IOMMU_READ|IOMMU_WRITE);
>> +				__dma_info_to_prot(DMA_BIRECTIONAL, attrs));
>>  		if (ret < 0)
>>  			goto fail;
>>  		iova += len;
>> @@ -1476,7 +1477,8 @@ static struct page **__iommu_get_pages(void *cpu_addr, unsigned long attrs)
>>  }
>>
>>  static void *__iommu_alloc_simple(struct device *dev, size_t size, gfp_t gfp,
>> -				  dma_addr_t *handle, int coherent_flag)
>> +				  dma_addr_t *handle, int coherent_flag,
>> +				  unsigned long attrs)
>>  {
>>  	struct page *page;
>>  	void *addr;
>> @@ -1488,7 +1490,7 @@ static void *__iommu_alloc_simple(struct device *dev, size_t size, gfp_t gfp,
>>  	if (!addr)
>>  		return NULL;
>>
>> -	*handle = __iommu_create_mapping(dev, &page, size);
>> +	*handle = __iommu_create_mapping(dev, &page, size, attrs);
>>  	if (*handle == DMA_ERROR_CODE)
>>  		goto err_mapping;
>>
>> @@ -1522,7 +1524,8 @@ static void *__arm_iommu_alloc_attrs(struct device *dev, size_t size,
>>
>>  	if (coherent_flag  == COHERENT || !gfpflags_allow_blocking(gfp))
>>  		return __iommu_alloc_simple(dev, size, gfp, handle,
>> -					    coherent_flag);
>> +					    coherent_flag,
>> +					    attrs);
>
>Super-nit: unnecessary line break.
>
>>
>>  	/*
>>  	 * Following is a work-around (a.k.a. hack) to prevent pages
>> @@ -1672,10 +1675,13 @@ static int arm_iommu_get_sgtable(struct device *dev, struct sg_table *sgt,
>>  					 GFP_KERNEL);
>>  }
>>
>> -static int __dma_direction_to_prot(enum dma_data_direction dir)
>> +static int __dma_info_to_prot(enum dma_data_direction dir, unsigned long attrs)
>>  {
>>  	int prot;
>>
>> +	if (attrs & DMA_ATTR_PRIVILEGED)
>> +		prot |= IOMMU_PRIV;
>> +
>>  	switch (dir) {
>>  	case DMA_BIDIRECTIONAL:
>>  		prot = IOMMU_READ | IOMMU_WRITE;
>> @@ -1722,7 +1728,7 @@ static int __map_sg_chunk(struct device *dev, struct scatterlist *sg,
>>  		if (!is_coherent && (attrs & DMA_ATTR_SKIP_CPU_SYNC) == 0)
>>  			__dma_page_cpu_to_dev(sg_page(s), s->offset, s->length, dir);
>>
>> -		prot = __dma_direction_to_prot(dir);
>> +		prot = __dma_info_to_prot(dir, attrs);
>>
>>  		ret = iommu_map(mapping->domain, iova, phys, len, prot);
>>  		if (ret < 0)
>> @@ -1930,7 +1936,7 @@ static dma_addr_t arm_coherent_iommu_map_page(struct device *dev, struct page *p
>>  	if (dma_addr == DMA_ERROR_CODE)
>>  		return dma_addr;
>>
>> -	prot = __dma_direction_to_prot(dir);
>> +	prot = __dma_info_to_prot(dir, attrs);
>>
>>  	ret = iommu_map(mapping->domain, dma_addr, page_to_phys(page), len, prot);
>>  	if (ret < 0)
>> @@ -2036,7 +2042,7 @@ static dma_addr_t arm_iommu_map_resource(struct device *dev,
>>  	if (dma_addr == DMA_ERROR_CODE)
>>  		return dma_addr;
>>
>> -	prot = __dma_direction_to_prot(dir) | IOMMU_MMIO;
>> +	prot = __dma_info_to_prot(dir, attrs) | IOMMU_MMIO;
>>
>>  	ret = iommu_map(mapping->domain, dma_addr, addr, len, prot);
>>  	if (ret < 0)
>>
>
>Looks reasonable to me. Assuming it survives testing:
>
>Acked-by: Robin Murphy <robin.murphy@arm.com>

Posted V8 [1]. I changed a few more things after the testing,
though functionally the same. So have not taken your ack and
would be nice to have it once again

[1] https://lkml.org/lkml/2017/1/2/224

Regards,
 Sricharan

^ permalink raw reply

* [PATCH V8 9/9] Revert "iommu/arm-smmu: Set PRIVCFG in stage 1 STEs"
From: Sricharan R @ 2017-01-02 13:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483362764-11990-1-git-send-email-sricharan@codeaurora.org>

From: Robin Murphy <robin.murphy@arm.com>

This reverts commit df5e1a0f2a2d779ad467a691203bcbc74d75690e.

Now that proper privileged mappings can be requested via IOMMU_PRIV,
unconditionally overriding the incoming PRIVCFG becomes the wrong thing
to do, so stop it.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/arm-smmu-v3.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 4d6ec44..7d45d8b 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -269,9 +269,6 @@
 #define STRTAB_STE_1_SHCFG_INCOMING	1UL
 #define STRTAB_STE_1_SHCFG_SHIFT	44
 
-#define STRTAB_STE_1_PRIVCFG_UNPRIV	2UL
-#define STRTAB_STE_1_PRIVCFG_SHIFT	48
-
 #define STRTAB_STE_2_S2VMID_SHIFT	0
 #define STRTAB_STE_2_S2VMID_MASK	0xffffUL
 #define STRTAB_STE_2_VTCR_SHIFT		32
@@ -1073,9 +1070,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_device *smmu, u32 sid,
 #ifdef CONFIG_PCI_ATS
 			 STRTAB_STE_1_EATS_TRANS << STRTAB_STE_1_EATS_SHIFT |
 #endif
-			 STRTAB_STE_1_STRW_NSEL1 << STRTAB_STE_1_STRW_SHIFT |
-			 STRTAB_STE_1_PRIVCFG_UNPRIV <<
-			 STRTAB_STE_1_PRIVCFG_SHIFT);
+			 STRTAB_STE_1_STRW_NSEL1 << STRTAB_STE_1_STRW_SHIFT);
 
 		if (smmu->features & ARM_SMMU_FEAT_STALLS)
 			dst[1] |= cpu_to_le64(STRTAB_STE_1_S1STALLD);
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply related

* [PATCH V8 8/9] iommu/arm-smmu: Set privileged attribute to 'default' instead of 'unprivileged'
From: Sricharan R @ 2017-01-02 13:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483362764-11990-1-git-send-email-sricharan@codeaurora.org>

Currently the driver sets all the device transactions privileges
to UNPRIVILEGED, but there are cases where the iommu masters wants
to isolate privileged supervisor and unprivileged user.
So don't override the privileged setting to unprivileged, instead
set it to default as incoming and let it be controlled by the pagetable
settings.

Acked-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
 drivers/iommu/arm-smmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index a60cded..73a0a25 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1214,7 +1214,7 @@ static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
 			continue;
 
 		s2cr[idx].type = type;
-		s2cr[idx].privcfg = S2CR_PRIVCFG_UNPRIV;
+		s2cr[idx].privcfg = S2CR_PRIVCFG_DEFAULT;
 		s2cr[idx].cbndx = cbndx;
 		arm_smmu_write_s2cr(smmu, idx);
 	}
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply related

* [PATCH V8 7/9] dmaengine: pl330: Make sure microcode is privileged
From: Sricharan R @ 2017-01-02 13:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483362764-11990-1-git-send-email-sricharan@codeaurora.org>

From: Mitchel Humpherys <mitchelh@codeaurora.org>

The PL330 is hard-wired such that instruction fetches on both the
manager and channel threads go out onto the bus with the "privileged"
bit set. This can become troublesome once there is an IOMMU or other
form of memory protection downstream, since those will typically be
programmed by the DMA mapping subsystem in the expectation of normal
unprivileged transactions (such as the PL330 channel threads' own data
accesses as currently configured by this driver).

To avoid the case of, say, an IOMMU blocking an unexpected privileged
transaction with a permission fault, use the newly-introduced
DMA_ATTR_PRIVILEGED attribute for the mapping of our microcode buffer.
That way the DMA layer can do whatever it needs to do to make things
continue to work as expected on more complex systems.

Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Vinod Koul <vinod.koul@intel.com>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Tested-by: Robin Murphy <robin.murphy@arm.com>
Acked-by: Will Deacon <will.deacon@arm.com>
Acked-by: Vinod Koul <vinod.koul@intel.com>
Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
[rm: remove now-redundant local variable, clarify commit message]
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/dma/pl330.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 87fd015..5a90d0c 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -1864,9 +1864,10 @@ static int dmac_alloc_resources(struct pl330_dmac *pl330)
 	 * 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_ATTR_PRIVILEGED);
 	if (!pl330->mcode_cpu) {
 		dev_err(pl330->ddma.dev, "%s:%d Can't allocate memory!\n",
 			__func__, __LINE__);
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply related

* [PATCH V8 6/9] arm/dma-mapping: Implement DMA_ATTR_PRIVILEGED
From: Sricharan R @ 2017-01-02 13:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483362764-11990-1-git-send-email-sricharan@codeaurora.org>

The newly added DMA_ATTR_PRIVILEGED is useful for creating mappings that
are only accessible to privileged DMA engines.  Adding it to the
arm dma-mapping.c so that the ARM32 DMA IOMMU mapper can make use of it.

Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
 arch/arm/mm/dma-mapping.c | 60 +++++++++++++++++++++++------------------------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index ab77100..82d3e79 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1171,6 +1171,25 @@ static int __init dma_debug_do_init(void)
 
 #ifdef CONFIG_ARM_DMA_USE_IOMMU
 
+static int __dma_info_to_prot(enum dma_data_direction dir, unsigned long attrs)
+{
+	int prot = 0;
+
+	if (attrs & DMA_ATTR_PRIVILEGED)
+		prot |= IOMMU_PRIV;
+
+	switch (dir) {
+	case DMA_BIDIRECTIONAL:
+		return prot | IOMMU_READ | IOMMU_WRITE;
+	case DMA_TO_DEVICE:
+		return prot | IOMMU_READ;
+	case DMA_FROM_DEVICE:
+		return prot | IOMMU_WRITE;
+	default:
+		return prot;
+	}
+}
+
 /* IOMMU */
 
 static int extend_iommu_mapping(struct dma_iommu_mapping *mapping);
@@ -1394,7 +1413,8 @@ static int __iommu_free_buffer(struct device *dev, struct page **pages,
  * Create a mapping in device IO address space for specified pages
  */
 static dma_addr_t
-__iommu_create_mapping(struct device *dev, struct page **pages, size_t size)
+__iommu_create_mapping(struct device *dev, struct page **pages, size_t size,
+		       unsigned long attrs)
 {
 	struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev);
 	unsigned int count = PAGE_ALIGN(size) >> PAGE_SHIFT;
@@ -1419,7 +1439,7 @@ static int __iommu_free_buffer(struct device *dev, struct page **pages,
 
 		len = (j - i) << PAGE_SHIFT;
 		ret = iommu_map(mapping->domain, iova, phys, len,
-				IOMMU_READ|IOMMU_WRITE);
+				__dma_info_to_prot(DMA_BIDIRECTIONAL, attrs));
 		if (ret < 0)
 			goto fail;
 		iova += len;
@@ -1476,7 +1496,8 @@ static struct page **__iommu_get_pages(void *cpu_addr, unsigned long attrs)
 }
 
 static void *__iommu_alloc_simple(struct device *dev, size_t size, gfp_t gfp,
-				  dma_addr_t *handle, int coherent_flag)
+				  dma_addr_t *handle, int coherent_flag,
+				  unsigned long attrs)
 {
 	struct page *page;
 	void *addr;
@@ -1488,7 +1509,7 @@ static void *__iommu_alloc_simple(struct device *dev, size_t size, gfp_t gfp,
 	if (!addr)
 		return NULL;
 
-	*handle = __iommu_create_mapping(dev, &page, size);
+	*handle = __iommu_create_mapping(dev, &page, size, attrs);
 	if (*handle == DMA_ERROR_CODE)
 		goto err_mapping;
 
@@ -1522,7 +1543,7 @@ static void *__arm_iommu_alloc_attrs(struct device *dev, size_t size,
 
 	if (coherent_flag  == COHERENT || !gfpflags_allow_blocking(gfp))
 		return __iommu_alloc_simple(dev, size, gfp, handle,
-					    coherent_flag);
+					    coherent_flag, attrs);
 
 	/*
 	 * Following is a work-around (a.k.a. hack) to prevent pages
@@ -1537,7 +1558,7 @@ static void *__arm_iommu_alloc_attrs(struct device *dev, size_t size,
 	if (!pages)
 		return NULL;
 
-	*handle = __iommu_create_mapping(dev, pages, size);
+	*handle = __iommu_create_mapping(dev, pages, size, attrs);
 	if (*handle == DMA_ERROR_CODE)
 		goto err_buffer;
 
@@ -1672,27 +1693,6 @@ static int arm_iommu_get_sgtable(struct device *dev, struct sg_table *sgt,
 					 GFP_KERNEL);
 }
 
-static int __dma_direction_to_prot(enum dma_data_direction dir)
-{
-	int prot;
-
-	switch (dir) {
-	case DMA_BIDIRECTIONAL:
-		prot = IOMMU_READ | IOMMU_WRITE;
-		break;
-	case DMA_TO_DEVICE:
-		prot = IOMMU_READ;
-		break;
-	case DMA_FROM_DEVICE:
-		prot = IOMMU_WRITE;
-		break;
-	default:
-		prot = 0;
-	}
-
-	return prot;
-}
-
 /*
  * Map a part of the scatter-gather list into contiguous io address space
  */
@@ -1722,7 +1722,7 @@ static int __map_sg_chunk(struct device *dev, struct scatterlist *sg,
 		if (!is_coherent && (attrs & DMA_ATTR_SKIP_CPU_SYNC) == 0)
 			__dma_page_cpu_to_dev(sg_page(s), s->offset, s->length, dir);
 
-		prot = __dma_direction_to_prot(dir);
+		prot = __dma_info_to_prot(dir, attrs);
 
 		ret = iommu_map(mapping->domain, iova, phys, len, prot);
 		if (ret < 0)
@@ -1930,7 +1930,7 @@ static dma_addr_t arm_coherent_iommu_map_page(struct device *dev, struct page *p
 	if (dma_addr == DMA_ERROR_CODE)
 		return dma_addr;
 
-	prot = __dma_direction_to_prot(dir);
+	prot = __dma_info_to_prot(dir, attrs);
 
 	ret = iommu_map(mapping->domain, dma_addr, page_to_phys(page), len, prot);
 	if (ret < 0)
@@ -2036,7 +2036,7 @@ static dma_addr_t arm_iommu_map_resource(struct device *dev,
 	if (dma_addr == DMA_ERROR_CODE)
 		return dma_addr;
 
-	prot = __dma_direction_to_prot(dir) | IOMMU_MMIO;
+	prot = __dma_info_to_prot(dir, attrs) | IOMMU_MMIO;
 
 	ret = iommu_map(mapping->domain, dma_addr, addr, len, prot);
 	if (ret < 0)
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply related

* [PATCH V8 5/9] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED
From: Sricharan R @ 2017-01-02 13:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483362764-11990-1-git-send-email-sricharan@codeaurora.org>

From: Mitchel Humpherys <mitchelh@codeaurora.org>

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 <robin.murphy@arm.com>
Tested-by: Robin Murphy <robin.murphy@arm.com>
Acked-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
---
 arch/arm64/mm/dma-mapping.c |  6 +++---
 drivers/iommu/dma-iommu.c   | 12 +++++++++---
 include/linux/dma-iommu.h   |  3 ++-
 3 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index 290a84f..49c6f75 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -557,7 +557,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;
 
@@ -711,7 +711,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) &&
@@ -769,7 +769,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 2db0d64..3006eee 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -181,16 +181,22 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
 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;
@@ -633,7 +639,7 @@ dma_addr_t iommu_dma_map_resource(struct device *dev, phys_addr_t phys,
 		size_t size, enum dma_data_direction dir, unsigned long attrs)
 {
 	return __iommu_dma_map(dev, phys, size,
-			dma_direction_to_prot(dir, false) | IOMMU_MMIO);
+			dma_info_to_prot(dir, false, attrs) | IOMMU_MMIO);
 }
 
 void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 7f7e9a7..c5511e1 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -34,7 +34,8 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
 		u64 size, struct device *dev);
 
 /* 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 INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply related

* [PATCH V8 4/9] common: DMA-mapping: add DMA_ATTR_PRIVILEGED attribute
From: Sricharan R @ 2017-01-02 13:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483362764-11990-1-git-send-email-sricharan@codeaurora.org>

From: Mitchel Humpherys <mitchelh@codeaurora.org>

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-doc at vger.kernel.org
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Tested-by: Robin Murphy <robin.murphy@arm.com>
Acked-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
---
 Documentation/DMA-attributes.txt | 10 ++++++++++
 include/linux/dma-mapping.h      |  7 +++++++
 2 files changed, 17 insertions(+)

diff --git a/Documentation/DMA-attributes.txt b/Documentation/DMA-attributes.txt
index 98bf7ac..44c6bc4 100644
--- a/Documentation/DMA-attributes.txt
+++ b/Documentation/DMA-attributes.txt
@@ -143,3 +143,13 @@ So, this provides a way for drivers to avoid those error messages on calls
 where allocation failures are not a problem, and shouldn't bother the logs.
 
 NOTE: At the moment DMA_ATTR_NO_WARN is only implemented on PowerPC.
+
+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 10c5a17b1..c24721a 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -63,6 +63,13 @@
 #define DMA_ATTR_NO_WARN	(1UL << 8)
 
 /*
+ * 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@lesser-privileged levels).
+ */
+#define DMA_ATTR_PRIVILEGED		(1UL << 9)
+
+/*
  * A dma_addr_t can hold any valid DMA or bus address for the platform.
  * It can be given to a device to use as a DMA source or target.  A CPU cannot
  * reference a dma_addr_t directly because there may be translation between
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply related

* [PATCH V8 3/9] iommu/io-pgtable-arm-v7s: Add support for the IOMMU_PRIV flag
From: Sricharan R @ 2017-01-02 13:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483362764-11990-1-git-send-email-sricharan@codeaurora.org>

From: Robin Murphy <robin.murphy@arm.com>

The short-descriptor format also allows privileged-only mappings, so
let's wire it up.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Tested-by: Sricharan R <sricharan@codeaurora.org>
---
 drivers/iommu/io-pgtable-arm-v7s.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/io-pgtable-arm-v7s.c b/drivers/iommu/io-pgtable-arm-v7s.c
index 0769276..1c049e2 100644
--- a/drivers/iommu/io-pgtable-arm-v7s.c
+++ b/drivers/iommu/io-pgtable-arm-v7s.c
@@ -265,7 +265,9 @@ static arm_v7s_iopte arm_v7s_prot_to_pte(int prot, int lvl,
 	if (!(prot & IOMMU_MMIO))
 		pte |= ARM_V7S_ATTR_TEX(1);
 	if (ap) {
-		pte |= ARM_V7S_PTE_AF | ARM_V7S_PTE_AP_UNPRIV;
+		pte |= ARM_V7S_PTE_AF;
+		if (!(prot & IOMMU_PRIV))
+			pte |= ARM_V7S_PTE_AP_UNPRIV;
 		if (!(prot & IOMMU_WRITE))
 			pte |= ARM_V7S_PTE_AP_RDONLY;
 	}
@@ -288,6 +290,8 @@ static int arm_v7s_pte_to_prot(arm_v7s_iopte pte, int lvl)
 
 	if (!(attr & ARM_V7S_PTE_AP_RDONLY))
 		prot |= IOMMU_WRITE;
+	if (!(attr & ARM_V7S_PTE_AP_UNPRIV))
+		prot |= IOMMU_PRIV;
 	if ((attr & (ARM_V7S_TEX_MASK << ARM_V7S_TEX_SHIFT)) == 0)
 		prot |= IOMMU_MMIO;
 	else if (pte & ARM_V7S_ATTR_C)
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply related

* [PATCH V8 2/9] iommu/io-pgtable-arm: add support for the IOMMU_PRIV flag
From: Sricharan R @ 2017-01-02 13:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483362764-11990-1-git-send-email-sricharan@codeaurora.org>

From: Jeremy Gebben <jgebben@codeaurora.org>

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

Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Tested-by: Robin Murphy <robin.murphy@arm.com>
Acked-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Jeremy Gebben <jgebben@codeaurora.org>
---
 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 a40ce34..feacc54 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 INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply related

* [PATCH V8 1/9] iommu: add IOMMU_PRIV attribute
From: Sricharan R @ 2017-01-02 13:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483362764-11990-1-git-send-email-sricharan@codeaurora.org>

From: Mitchel Humpherys <mitchelh@codeaurora.org>

Add the IOMMU_PRIV attribute, which is used to indicate privileged
mappings.

Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Tested-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
Acked-by: Will Deacon <will.deacon@arm.com>
---
 include/linux/iommu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 0ff5111..8c15ada 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 INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply related

* [PATCH V8 0/9] Add support for privileged mappings
From: Sricharan R @ 2017-01-02 13:12 UTC (permalink / raw)
  To: linux-arm-kernel

This series is a resend of the V5 that Mitch sent sometime back [2]
All the patches are the same and i have just rebased. Redid patch [3],
as it does not apply in this code base. Added a couple of more patches
[4], [5] from Robin for adding the privileged attributes to armv7s format
and arm-smmuv3 revert. Added a patch for passing in the privileged
attributes from arm32 dma-mapping apis as well.

The following patch to the ARM SMMU driver:

    commit d346180e70b91b3d5a1ae7e5603e65593d4622bc
    Author: Robin Murphy <robin.murphy@arm.com>
    Date:   Tue Jan 26 18:06:34 2016 +0000
    
        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].

Note that, i tested this on arm64 with arm-smmuv2, short descriptor changes,
tested this on arm32 platform as well and do not have an platform to test
this with arm-smmuv3.

[1] https://github.com/robclark/kilroy
[2] https://lkml.org/lkml/2016/7/27/590
[3] https://patchwork.kernel.org/patch/9250493/
[4] http://www.linux-arm.org/git?p=linux-rm.git;a=commit;h=1291bd74f05d31da1dab3df02987cba5bd25849b
[5] http://www.linux-arm.org/git?p=linux-rm.git;a=commit;h=a79c1c6333f26849dba418cd92de26b60f5954f3

Changelog:
 v7..v8
    - Added a patch for passing in the privileged attributes from arm32
      dma-mapping apis as well.

 v6..v7
    - Added couple of more patches, picked up acks, updated commit log

 v5..v6
    - Rebased all the patches and redid 6/6 as it does not apply in
      this code base. 

 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 (4):
  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

Robin Murphy (2):
  iommu/io-pgtable-arm-v7s: Add support for the IOMMU_PRIV flag
  Revert "iommu/arm-smmu: Set PRIVCFG in stage 1 STEs"

Sricharan R (2):
  arm/dma-mapping: Implement DMA_ATTR_PRIVILEGED
  iommu/arm-smmu: Set privileged attribute to 'default' instead of
    'unprivileged'

 Documentation/DMA-attributes.txt   | 10 +++++++
 arch/arm/mm/dma-mapping.c          | 60 +++++++++++++++++++-------------------
 arch/arm64/mm/dma-mapping.c        |  6 ++--
 drivers/dma/pl330.c                |  5 ++--
 drivers/iommu/arm-smmu-v3.c        |  7 +----
 drivers/iommu/arm-smmu.c           |  2 +-
 drivers/iommu/dma-iommu.c          | 12 ++++++--
 drivers/iommu/io-pgtable-arm-v7s.c |  6 +++-
 drivers/iommu/io-pgtable-arm.c     |  5 +++-
 include/linux/dma-iommu.h          |  3 +-
 include/linux/dma-mapping.h        |  7 +++++
 include/linux/iommu.h              |  1 +
 12 files changed, 76 insertions(+), 48 deletions(-)

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply

* [PATCH] DTS: MCCMON6: IMX: Provide support for iMX6Q based Liebherr mccmon6 board
From: Vladimir Zapolskiy @ 2017-01-02 12:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1482794396-16153-1-git-send-email-l.majewski@majess.pl>

Hello Lukasz,

On 12/27/2016 01:19 AM, Lukasz Majewski wrote:
> Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>

please add a commit message with a short description of the change.

Also change subject line to "ARM: dts: imx6q: Add mccmon6 board support".

> ---
> MCCMON6 board support depends on following patches:
> 
> 1. "video: backlight: pwm_bl: Initialize fb_bl_on[x] and use_count during pwm_backlight_probe()"
> 	http://patchwork.ozlabs.org/patch/708844/
> 
> 2. "pwm: imx: Provide atomic operation for IMX PWM driver"
> 	http://patchwork.ozlabs.org/patch/708847/ - http://patchwork.ozlabs.org/patch/708843/
> 
> 
> ---
>  arch/arm/boot/dts/Makefile          |   1 +
>  arch/arm/boot/dts/imx6q-mccmon6.dts | 469 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 470 insertions(+)
>  create mode 100644 arch/arm/boot/dts/imx6q-mccmon6.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index c558ba7..7ce1080 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -382,6 +382,7 @@ dtb-$(CONFIG_SOC_IMX6Q) += \
>  	imx6q-h100.dtb \
>  	imx6q-hummingboard.dtb \
>  	imx6q-icore-rqs.dtb \
> +	imx6q-mccmon6.dtb \
>  	imx6q-marsboard.dtb \

Please add a new line preserving alphabetical order.

>  	imx6q-nitrogen6x.dtb \
>  	imx6q-nitrogen6_max.dtb \
> diff --git a/arch/arm/boot/dts/imx6q-mccmon6.dts b/arch/arm/boot/dts/imx6q-mccmon6.dts
> new file mode 100644
> index 0000000..7445d01
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6q-mccmon6.dts
> @@ -0,0 +1,469 @@
> +/*
> + * Copyright 2016

Copyright holder is missing.

> + *
> + * Author: Lukasz Majewski <l.majewski@majess.pl>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */

Please add an empty line here to improve readability.

> +/dts-v1/;

Please add an empty line here to improve readability.

> +#include "imx6q.dtsi"
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/pwm/pwm.h>
> +
> +/ {
> +	model = "Monitor6 i.MX6 Quad Board";

Missing hardware vendor name.

> +	compatible = "mccmon6", "fsl,imx6q";

Missing hardware vendor prefix before "mccmon6".

> +
> +	memory {
> +		reg = <0x10000000 0x80000000>;
> +	};
> +
> +	ethernet0 {
> +		status = "okay";
> +	};

It looks like a useless device node, you have a description of &fec already.

> +
> +	backlight_lvds: backlight {
> +		compatible = "pwm-backlight";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_display>;

I would recommend to rename "pinctrl_display" to "pinctrl_backlight".

> +		pwms = <&pwm2 0 5000000 PWM_POLARITY_INVERTED>;

This should work when extension to the i.MX PWM driver is merged.

> +		brightness-levels = <  0   1   2   3   4   5   6   7   8   9
> +				      10  11  12  13  14  15  16  17  18  19
> +				      20  21  22  23  24  25  26  27  28  29
> +				      30  31  32  33  34  35  36  37  38  39
> +				      40  41  42  43  44  45  46  47  48  49
> +				      50  51  52  53  54  55  56  57  58  59
> +				      60  61  62  63  64  65  66  67  68  69
> +				      70  71  72  73  74  75  76  77  78  79
> +				      80  81  82  83  84  85  86  87  88  89
> +				      90  91  92  93  94  95  96  97  98  99
> +				     100 101 102 103 104 105 106 107 108 109
> +				     110 111 112 113 114 115 116 117 118 119
> +				     120 121 122 123 124 125 126 127 128 129
> +				     130 131 132 133 134 135 136 137 138 139
> +				     140 141 142 143 144 145 146 147 148 149
> +				     150 151 152 153 154 155 156 157 158 159
> +				     160 161 162 163 164 165 166 167 168 169
> +				     170 171 172 173 174 175 176 177 178 179
> +				     180 181 182 183 184 185 186 187 188 189
> +				     190 191 192 193 194 195 196 197 198 199
> +				     200 201 202 203 204 205 206 207 208 209
> +				     210 211 212 213 214 215 216 217 218 219
> +				     220 221 222 223 224 225 226 227 228 229
> +				     230 231 232 233 234 235 236 237 238 239
> +				     240 241 242 243 244 245 246 247 248 249
> +				     250 251 252 253 254 255>;

I'm not sure that actually need such a long list of brightness levels.

> +		default-brightness-level = <50>;
> +		enable-gpios = <&gpio1 2 GPIO_ACTIVE_LOW>;
> +	};
> +
> +	reg_lvds: regulator-lvds {
> +		compatible = "regulator-fixed";
> +		regulator-name = "lvds_ppen";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		regulator-boot-on;
> +		gpio = <&gpio1 19 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +
> +	panel-lvds0 {
> +		compatible = "innolux,g121x1-l03";
> +		backlight = <&backlight_lvds>;
> +		power-supply = <&reg_lvds>;
> +
> +		port {
> +			panel_in_lvds0: endpoint {
> +				remote-endpoint = <&lvds0_out>;
> +			};
> +		};
> +	};
> +};
> +
> +&i2c1 {
> +	clock-frequency = <100000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c1>;
> +	status = "okay";
> +};
> +
> +&i2c2 {
> +	clock-frequency = <100000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c2>;
> +	status = "okay";
> +
> +	pmic: pfuze100 at 08 {
> +		compatible = "fsl,pfuze100";
> +		reg = <0x08>;
> +
> +		regulators {
> +			sw1a_reg: sw1ab {
> +				regulator-min-microvolt = <300000>;
> +				regulator-max-microvolt = <1875000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +				regulator-ramp-delay = <6250>;
> +			};
> +
> +			sw1c_reg: sw1c {
> +				regulator-min-microvolt = <300000>;
> +				regulator-max-microvolt = <1875000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +				regulator-ramp-delay = <6250>;
> +			};
> +
> +			sw2_reg: sw2 {
> +				regulator-min-microvolt = <800000>;
> +				regulator-max-microvolt = <3950000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			sw3a_reg: sw3a {
> +				regulator-min-microvolt = <400000>;
> +				regulator-max-microvolt = <1975000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			sw3b_reg: sw3b {
> +				regulator-min-microvolt = <400000>;
> +				regulator-max-microvolt = <1975000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			sw4_reg: sw4 {
> +				regulator-min-microvolt = <800000>;
> +				regulator-max-microvolt = <3300000>;
> +			};
> +
> +			swbst_reg: swbst {
> +				regulator-min-microvolt = <5000000>;
> +				regulator-max-microvolt = <5150000>;
> +			};
> +
> +			snvs_reg: vsnvs {
> +				regulator-min-microvolt = <1000000>;
> +				regulator-max-microvolt = <3000000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			vref_reg: vrefddr {
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			vgen1_reg: vgen1 {
> +				regulator-min-microvolt = <800000>;
> +				regulator-max-microvolt = <1550000>;
> +			};
> +
> +			vgen2_reg: vgen2 {
> +				regulator-min-microvolt = <800000>;
> +				regulator-max-microvolt = <1550000>;
> +			};
> +
> +			vgen3_reg: vgen3 {
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <3300000>;
> +			};
> +
> +			vgen4_reg: vgen4 {
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-always-on;
> +			};
> +
> +			vgen5_reg: vgen5 {
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-always-on;
> +			};
> +
> +			vgen6_reg: vgen6 {
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-always-on;
> +			};
> +		};
> +	};
> +};
> +
> +&iomuxc {
> +	pinctrl-names = "default";
> +
> +	imx6q-mccmon6 {
> +

Please drop the empty line above.

> +		pinctrl_enet: enetgrp {
> +			fsl,pins = <
> +				MX6QDL_PAD_ENET_MDIO__ENET_MDIO		0x1b0b0
> +				MX6QDL_PAD_ENET_MDC__ENET_MDC		0x1b0b0
> +				MX6QDL_PAD_RGMII_TXC__RGMII_TXC		0x1b0b0
> +				MX6QDL_PAD_RGMII_TD0__RGMII_TD0		0x1b0b0
> +				MX6QDL_PAD_RGMII_TD1__RGMII_TD1		0x1b0b0
> +				MX6QDL_PAD_RGMII_TD2__RGMII_TD2		0x1b0b0
> +				MX6QDL_PAD_RGMII_TD3__RGMII_TD3		0x1b0b0
> +				MX6QDL_PAD_RGMII_TX_CTL__RGMII_TX_CTL	0x1b0b0
> +				MX6QDL_PAD_ENET_REF_CLK__ENET_TX_CLK	0x1b0b0
> +				MX6QDL_PAD_RGMII_RXC__RGMII_RXC		0x1b0b0
> +				MX6QDL_PAD_RGMII_RD0__RGMII_RD0		0x1b0b0
> +				MX6QDL_PAD_RGMII_RD1__RGMII_RD1		0x1b0b0
> +				MX6QDL_PAD_RGMII_RD2__RGMII_RD2		0x1b0b0
> +				MX6QDL_PAD_RGMII_RD3__RGMII_RD3		0x1b0b0
> +				MX6QDL_PAD_RGMII_RX_CTL__RGMII_RX_CTL	0x1b0b0
> +				MX6QDL_PAD_GPIO_16__ENET_REF_CLK 0x4001b0a8
> +				MX6QDL_PAD_GPIO_6__ENET_IRQ		0x000b1
> +			>;
> +		};
> +
> +		pinctrl_i2c1: i2c1grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_CSI0_DAT9__I2C1_SCL	0x4001b8b1
> +				MX6QDL_PAD_CSI0_DAT8__I2C1_SDA	0x4001b8b1
> +			>;
> +		};
> +
> +		pinctrl_i2c2: i2c2grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_KEY_COL3__I2C2_SCL	0x4001b8b1
> +				MX6QDL_PAD_KEY_ROW3__I2C2_SDA	0x4001b8b1
> +			>;
> +		};
> +
> +		pinctrl_uart1: uart1grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_CSI0_DAT10__UART1_TX_DATA	0x1b0b1
> +				MX6QDL_PAD_CSI0_DAT11__UART1_RX_DATA	0x1b0b1
> +			>;
> +		};
> +
> +		pinctrl_usdhc2: usdhc2grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_SD2_CMD__SD2_CMD		0x17059
> +				MX6QDL_PAD_SD2_CLK__SD2_CLK		0x10059
> +				MX6QDL_PAD_SD2_DAT0__SD2_DATA0		0x17059
> +				MX6QDL_PAD_SD2_DAT1__SD2_DATA1		0x17059
> +				MX6QDL_PAD_SD2_DAT2__SD2_DATA2		0x17059
> +				MX6QDL_PAD_SD2_DAT3__SD2_DATA3		0x17059
> +			>;
> +		};
> +
> +		pinctrl_usdhc3: usdhc3grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_SD3_CMD__SD3_CMD		0x17059
> +				MX6QDL_PAD_SD3_CLK__SD3_CLK		0x10059
> +				MX6QDL_PAD_SD3_DAT0__SD3_DATA0		0x17059
> +				MX6QDL_PAD_SD3_DAT1__SD3_DATA1		0x17059
> +				MX6QDL_PAD_SD3_DAT2__SD3_DATA2		0x17059
> +				MX6QDL_PAD_SD3_DAT3__SD3_DATA3		0x17059
> +				MX6QDL_PAD_SD3_DAT4__SD3_DATA4		0x17059
> +				MX6QDL_PAD_SD3_DAT5__SD3_DATA5		0x17059
> +				MX6QDL_PAD_SD3_DAT6__SD3_DATA6		0x17059
> +				MX6QDL_PAD_SD3_DAT7__SD3_DATA7		0x17059
> +				MX6QDL_PAD_SD3_RST__SD3_RESET		0x17059
> +			>;
> +		};
> +
> +		pinctrl_weim_cs0: weimcs0grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_EIM_CS0__EIM_CS0_B		0xb0b1
> +			>;
> +		};
> +
> +		pinctrl_weim_nor: weimnorgrp {
> +			fsl,pins = <
> +				MX6QDL_PAD_EIM_OE__EIM_OE_B		0xb0b1
> +				MX6QDL_PAD_EIM_RW__EIM_RW		0xb0b1
> +				MX6QDL_PAD_EIM_WAIT__EIM_WAIT_B	0xb060
> +				MX6QDL_PAD_EIM_D16__EIM_DATA16		0x1b0b0
> +				MX6QDL_PAD_EIM_D17__EIM_DATA17		0x1b0b0
> +				MX6QDL_PAD_EIM_D18__EIM_DATA18		0x1b0b0
> +				MX6QDL_PAD_EIM_D19__EIM_DATA19		0x1b0b0
> +				MX6QDL_PAD_EIM_D20__EIM_DATA20		0x1b0b0
> +				MX6QDL_PAD_EIM_D21__EIM_DATA21		0x1b0b0
> +				MX6QDL_PAD_EIM_D22__EIM_DATA22		0x1b0b0
> +				MX6QDL_PAD_EIM_D23__EIM_DATA23		0x1b0b0
> +				MX6QDL_PAD_EIM_D24__EIM_DATA24		0x1b0b0
> +				MX6QDL_PAD_EIM_D25__EIM_DATA25		0x1b0b0
> +				MX6QDL_PAD_EIM_D26__EIM_DATA26		0x1b0b0
> +				MX6QDL_PAD_EIM_D27__EIM_DATA27		0x1b0b0
> +				MX6QDL_PAD_EIM_D28__EIM_DATA28		0x1b0b0
> +				MX6QDL_PAD_EIM_D29__EIM_DATA29		0x1b0b0
> +				MX6QDL_PAD_EIM_D30__EIM_DATA30		0x1b0b0
> +				MX6QDL_PAD_EIM_D31__EIM_DATA31		0x1b0b0
> +				MX6QDL_PAD_EIM_A23__EIM_ADDR23		0xb0b1
> +				MX6QDL_PAD_EIM_A22__EIM_ADDR22		0xb0b1
> +				MX6QDL_PAD_EIM_A21__EIM_ADDR21		0xb0b1
> +				MX6QDL_PAD_EIM_A20__EIM_ADDR20		0xb0b1
> +				MX6QDL_PAD_EIM_A19__EIM_ADDR19		0xb0b1
> +				MX6QDL_PAD_EIM_A18__EIM_ADDR18		0xb0b1
> +				MX6QDL_PAD_EIM_A17__EIM_ADDR17		0xb0b1
> +				MX6QDL_PAD_EIM_A16__EIM_ADDR16		0xb0b1
> +				MX6QDL_PAD_EIM_DA15__EIM_AD15		0xb0b1
> +				MX6QDL_PAD_EIM_DA14__EIM_AD14		0xb0b1
> +				MX6QDL_PAD_EIM_DA13__EIM_AD13		0xb0b1
> +				MX6QDL_PAD_EIM_DA12__EIM_AD12		0xb0b1
> +				MX6QDL_PAD_EIM_DA11__EIM_AD11		0xb0b1
> +				MX6QDL_PAD_EIM_DA10__EIM_AD10		0xb0b1
> +				MX6QDL_PAD_EIM_DA9__EIM_AD09		0xb0b1
> +				MX6QDL_PAD_EIM_DA8__EIM_AD08		0xb0b1
> +				MX6QDL_PAD_EIM_DA7__EIM_AD07		0xb0b1
> +				MX6QDL_PAD_EIM_DA6__EIM_AD06		0xb0b1
> +				MX6QDL_PAD_EIM_DA5__EIM_AD05		0xb0b1
> +				MX6QDL_PAD_EIM_DA4__EIM_AD04		0xb0b1
> +				MX6QDL_PAD_EIM_DA3__EIM_AD03		0xb0b1
> +				MX6QDL_PAD_EIM_DA2__EIM_AD02		0xb0b1
> +				MX6QDL_PAD_EIM_DA1__EIM_AD01		0xb0b1
> +				MX6QDL_PAD_EIM_DA0__EIM_AD00		0xb0b1
> +			>;
> +		};
> +
> +		pinctrl_ecspi3: ecspi3grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_DISP0_DAT2__ECSPI3_MISO	0x100b1
> +				MX6QDL_PAD_DISP0_DAT1__ECSPI3_MOSI	0x100b1
> +				MX6QDL_PAD_DISP0_DAT0__ECSPI3_SCLK	0x100b1
> +			>;
> +		};
> +
> +		pinctrl_ecspi3_cs: ecspi3cs {
> +			fsl,pins = <
> +				MX6QDL_PAD_DISP0_DAT3__GPIO4_IO24 0x80000000
> +			>;
> +		};
> +		pinctrl_ecspi3_flwp: ecspi3flwp {
> +			fsl,pins = <
> +				MX6QDL_PAD_DISP0_DAT6__GPIO4_IO27 0x80000000
> +			>;
> +		};
> +
> +		pinctrl_uart4: uart4grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_KEY_COL0__UART4_TX_DATA	0x1b0b1
> +				MX6QDL_PAD_KEY_ROW0__UART4_RX_DATA	0x1b0b1
> +				MX6QDL_PAD_CSI0_DAT16__UART4_RTS_B	0x1b0b1
> +				MX6QDL_PAD_CSI0_DAT17__UART4_CTS_B	0x1b0b1
> +			>;
> +		};
> +
> +		pinctrl_display: dispgrp {
> +			fsl,pins = <
> +				/* BLEN_OUT */
> +				MX6QDL_PAD_GPIO_2__GPIO1_IO02    0x1b0b0
> +				/* LVDS_PPEN_OUT */
> +				MX6QDL_PAD_SD1_DAT2__GPIO1_IO19   0x1b0b0

This GPIO should be moved to a pinctrl group of regulator-lvds device node.

> +			>;
> +		};
> +
> +		pinctrl_pwm2: pwm2grp {
> +			fsl,pins = <
> +				 MX6QDL_PAD_GPIO_1__PWM2_OUT	0x1b0b1
> +			>;
> +		};

Please sort out all pinctrl_* nodes alphabetically.

> +	};
> +};
> +
> +&fec {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_enet>;
> +	phy-mode = "rgmii";
> +	phy-reset-gpios = <&gpio1 27 0>;

GPIO1_27 has no pad configuration in pinctrl_enet.

> +	interrupts-extended = <&gpio1 6 IRQ_TYPE_LEVEL_HIGH>,
> +			      <&intc 0 119 IRQ_TYPE_LEVEL_HIGH>;
> +	status = "okay";
> +};
> +
> +&uart1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart1>;

Should you add "uart-has-rtscts" property?

> +	status = "okay";
> +};
> +
> +&usdhc2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_usdhc2>;
> +	cd-gpios = <&gpio1 4 GPIO_ACTIVE_LOW>;

bus-width = <4>;

You should consider to add the GPIO1_4 into pinctrl_usdhc2 group.

> +	status = "okay";
> +};
> +
> +&usdhc3 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_usdhc3>;
> +	bus-width = <8>;
> +	status = "okay";

No "cd-gpios" property, should you add "non-removable" property then?

> +};
> +
> +&weim {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_weim_nor &pinctrl_weim_cs0>;
> +	#address-cells = <2>;
> +	#size-cells = <1>;
> +	ranges = <0 0 0x08000000 0x08000000>;
> +	status = "okay";
> +
> +	nor at 0,0 {
> +		compatible = "cfi-flash";
> +		reg = <0 0 0x02000000>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		bank-width = <2>;
> +		use-advanced-sector-protection;
> +		fsl,weim-cs-timing = <0x00620081 0x00000001 0x1c022000
> +				0x0000c000 0x1404a38e 0x00000000>;
> +	};
> +};
> +
> +&ecspi3 {
> +	fsl,spi-num-chipselects = <1>;

This property is obsoleted, please remove it.

> +	cs-gpios = <&gpio4 24 0>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_ecspi3 &pinctrl_ecspi3_cs &pinctrl_ecspi3_flwp>;
> +	status = "okay";
> +
> +	flash: s25sl032p at 0 {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "spansion,s25sl032p", "jedec,spi-nor";
> +		spi-max-frequency = <40000000>;
> +		reg = <0>;
> +	};
> +};
> +
> +&uart4 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart4>;
> +	status = "okay";

Should you add "uart-has-rtscts" property?

> +};
> +
> +&ldb {
> +	status = "okay";
> +
> +	lvds0: lvds-channel at 0 {
> +		fsl,data-mapping = "spwg";
> +		fsl,data-width = <24>;
> +		status = "okay";
> +
> +		port at 4 {
> +			reg = <4>;
> +
> +			lvds0_out: endpoint {
> +				remote-endpoint = <&panel_in_lvds0>;
> +			};
> +		};
> +	};
> +};
> +
> +&pwm2 {
> +	#pwm-cells = <3>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_pwm2>;
> +	status = "okay";
> +};
> 

Please sort out all device nodes but &iomuxc alphabetically:

* iomuxc
* ecspi3
* fec
* i2c1
* i2c2
* ldb
* pwm2
* uart1
* uart4
* usdhc2
* usdhc3
* weim

--
With best wishes,
Vladimir

^ permalink raw reply

* [PATCH 1/2] ARM: hyp-stub: improve ABI
From: Russell King - ARM Linux @ 2017-01-02 12:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161217120707.GA72718@MBP.local>

On Sat, Dec 17, 2016 at 12:07:11PM +0000, Catalin Marinas wrote:
> On Thu, Dec 15, 2016 at 06:57:18PM +0000, Russell King - ARM Linux wrote:
> > On Thu, Dec 15, 2016 at 03:37:15PM +0000, Marc Zyngier wrote:
> > > And this interface exists for the sole purpose of enabling KVM. Call it
> > > a hypervisor if you wish, but its usefulness is doubtful on its own.
> 
> IMO, the interface between EL1 and EL2 _is_ ABI. However, it should not
> be confused with a *stable* ABI like the one we expose to user. Since
> all users of the EL2 ABI live inside the kernel (either on the EL1 or
> EL2 side), we are free to change it as long as everything is updated
> simultaneously. I don't see this any different from other in-kernel ABI
> like that exposed to loadable modules (for the latter, we have the
> advantage of a partly self-documenting ABI as part of the C language).
> 
> I would welcome some documentation for the EL2 ABI as well, especially
> since head.S and the KVM counterpart is maintained by different people.

Well, it seems that different people within ARM have different opinions
on this subject - that makes it extremely hard to make progress on this.

As you're all local to each other, please can you hash it out amongst
yourselves and come to some sort of concensus!

> > What's also coming clear is that there's very few people who understand
> > all the interactions here, and the whole thing seems to be an undocumented
> > mess.  Something needs to change there - people need to stop shovelling
> > half baked crap into the kernel.  KVM have the privilege of being able to
> > push ARM stuff directly, I now see that was a very big mistake.
> > 
> > So, I want KVM further changes to come through my tree once this merge
> > window is over -
> 
> I'm afraid I strongly disagree. There are a few reasons neither you nor
> me gatekeep the KVM port: (a) the ARM KVM maintainers know a lot more
> about KVM than either of us, (b) the KVM code under arch/arm is reused
> by arm64 and (c) you or I would certainly become a bottleneck. There is
> a lot more to KVM support on ARM than the hyp stub and realistically you
> won't have the time to review all the stuff that comes your way.

I don't think so - in general the statistics for arch/arm/kvm are very
quiet - there's a bigger update once in a while:

v4.1:  8 files changed, 281 insertions(+), 150 deletions(-)
v4.2:  7 files changed, 56 insertions(+), 38 deletions(-)
v4.3:  8 files changed, 59 insertions(+), 41 deletions(-)
v4.4:  6 files changed, 95 insertions(+), 50 deletions(-)
v4.5:  6 files changed, 97 insertions(+), 50 deletions(-)
v4.6:  22 files changed, 1203 insertions(+), 1307 deletions(-)
v4.7:  5 files changed, 351 insertions(+), 262 deletions(-)
v4.8:  9 files changed, 134 insertions(+), 163 deletions(-)
v4.9:  12 files changed, 209 insertions(+), 159 deletions(-)

In terms of number of commits:

v4.1: 17
v4.2: 14
v4.3: 15
v4.4: 13
v4.5: 9
v4.6: 48
v4.7: 18
v4.8: 22
v4.9: 17

So there isn't much going on there.

> > Let's start with some documentation on the KVM hypervisor interfaces on
> > 32-bit ARM.  Documentation/virtual/kvm/hypercalls.txt already contains
> > some for x86, s390 and PPC, so that would be a good place to document
> > the ARM side.  Arguably, that should have already been done.
> 
> I'm all for documenting the interface.
> 
> (even though Will and I co-maintain arch/arm64, I deliberately kept his
> name out of the above as I haven't had the chance to ask for his
> opinion on this matter, which may as well differ)

Well, I started discussing this with Will before I produced the patch,
and Will thought that the ABI was entirely localised within hyp-stub.S.

This is exactly my point: very few people seem to have a proper
understanding of this (as illustrated by the number of different
opinions people appear to hold over various parts of the code), which
makes it very difficult for problems to get fixed.

Either we need more people to have an understanding (so if one of them
gets run over by a bus, we're not left floundering around) or we need
it to be documented - even if it's just a simple comment "the ABI in
this file is shared with XYZ, if you change the ABI here, also update
XYZ too."

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply

* [PATCH v5 09/14] ACPI: platform: setup MSI domain for ACPI based platform device
From: Hanjun Guo @ 2017-01-02 12:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAJZ5v0h3d5ZUxJXhgz74fSDR=AKTg_Of2knaONF_e2dr6Q-PsA@mail.gmail.com>

On 01/01/2017 04:45 AM, Rafael J. Wysocki wrote:
> On Fri, Dec 30, 2016 at 11:50 AM, Hanjun Guo <guohanjun@huawei.com> wrote:
[...]
>>
>> So how about just add the code as below?
>
> Works for me.

OK, will send out the updated patch set soon.

>
>> diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
>> index 11e63dd..37a8dfe 100644
>> --- a/drivers/acpi/glue.c
>> +++ b/drivers/acpi/glue.c
>> @@ -316,7 +316,8 @@ static int acpi_platform_notify(struct device *dev)
>>          if (!adev)
>>                  goto out;
>>
>> + if (dev->bus == &platform_bus_type)
>> +         acpi_configure_pmsi_domain(dev);
>>
>>          if (type && type->setup)
>>                  type->setup(dev);

Thanks for your comments.

Hanjun

^ permalink raw reply

* [PATCH v2 2/3] dmaeninge: xilinx_dma: Fix bug in multiple frame stores scenario in vdma
From: Appana Durga Kedareswara Rao @ 2017-01-02 11:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <a269409d-103e-1650-5df6-cc8743ed62cf@synopsys.com>

Hi Jose Miguel Abreu,
	
	Thanks for the review....
Sorry for the delay in the reply please see comments inline...

> >  	if (chan->has_sg) {
> >  		dma_ctrl_write(chan, XILINX_DMA_REG_TAILDESC,
> >  				tail_segment->phys);
> > +		list_splice_tail_init(&chan->pending_list, &chan->active_list);
> > +		chan->desc_pendingcount = 0;
> >  	} else {
> >  		struct xilinx_vdma_tx_segment *segment, *last = NULL;
> > -		int i = 0;
> > +		int i = 0, j = 0;
> >
> >  		if (chan->desc_submitcount < chan->num_frms)
> >  			i = chan->desc_submitcount;
> >
> > -		list_for_each_entry(segment, &desc->segments, node) {
> > -			if (chan->ext_addr)
> > -				vdma_desc_write_64(chan,
> > -
> 	XILINX_VDMA_REG_START_ADDRESS_64(i++),
> > -					segment->hw.buf_addr,
> > -					segment->hw.buf_addr_msb);
> > -			else
> > -				vdma_desc_write(chan,
> > -
> 	XILINX_VDMA_REG_START_ADDRESS(i++),
> > -					segment->hw.buf_addr);
> > -
> > -			last = segment;
> > +		for (j = 0; j < chan->num_frms; ) {
> > +			list_for_each_entry(segment, &desc->segments, node)
> {
> > +				if (chan->ext_addr)
> > +					vdma_desc_write_64(chan,
> > +
> XILINX_VDMA_REG_START_ADDRESS_64(i++),
> > +					  segment->hw.buf_addr,
> > +					  segment->hw.buf_addr_msb);
> > +				else
> > +					vdma_desc_write(chan,
> > +
> XILINX_VDMA_REG_START_ADDRESS(i++),
> > +					    segment->hw.buf_addr);
> > +
> > +				last = segment;
> > +			}
> > +			list_del(&desc->node);
> > +			list_add_tail(&desc->node, &chan->active_list);
> > +			j++;
> > +			if (list_empty(&chan->pending_list) ||
> > +			    (i == chan->num_frms))
> > +				break;
> > +			desc = list_first_entry(&chan->pending_list,
> > +						struct
> xilinx_dma_tx_descriptor,
> > +						node);
> >  		}
> >
> >  		if (!last)
> > @@ -1081,20 +1094,14 @@ static void xilinx_vdma_start_transfer(struct
> xilinx_dma_chan *chan)
> >  		vdma_desc_write(chan, XILINX_DMA_REG_FRMDLY_STRIDE,
> >  				last->hw.stride);
> >  		vdma_desc_write(chan, XILINX_DMA_REG_VSIZE, last-
> >hw.vsize);
> 
> What if we reach here and j < num_frms? It can happen because if
> the pending list is empty the loop breaks. Then VDMA will start
> with framebuffers having address 0x0. You should only program
> vsize when j > num_frms or when we have already previously set
> the framebuffers (i.e. when submitcount has already been
> incremented num_frms at least once).

In case of j < num_frms VDMA won't start the frame buffer having address 0
As we are programming the VDMA buffer address based on the desc_submitcount value only i.e. i.

Code snippet in the driver doing this:
                         list_for_each_entry(segment, &desc->segments, node) {
                                if (chan->ext_addr)
                                        vdma_desc_write_64(chan,
                                          XILINX_VDMA_REG_START_ADDRESS_64(i++),
                                          segment->hw.buf_addr,
                                          segment->hw.buf_addr_msb);
                                else
                                        vdma_desc_write(chan,
                                            XILINX_VDMA_REG_START_ADDRESS(i++),
                                            segment->hw.buf_addr);

                                last = segment;
                        } 

Initially desc_submitcount will be zero.
Let's assume h/w is configured for 10 frames and user submitted only 5 frames.
And triggered the VDMA h/w using issue_pending in this case desc_submitcount will be 5.
desc_submitcount will become zero again only when
User submits more frames than h/w capable or user submit frames up to h/w capable.

If my understanding is wrong could you please elaborate the race condition that you are talking above?
	
Regards,
Kedar.

> 
> Best regards,
> Jose Miguel Abreu
> 

^ permalink raw reply

* [RFC PATCH net-next v4 1/2] macb: Add 1588 support in Cadence GEM.
From: Harini Katakam @ 2017-01-02 11:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170102113155.GA16373@localhost.localdomain>

Hi Richard,

On Mon, Jan 2, 2017 at 5:01 PM, Richard Cochran
<richardcochran@gmail.com> wrote:
> On Mon, Jan 02, 2017 at 09:36:10AM +0000, Rafal Ozieblo wrote:
>> According Cadence Hardware team:
>> "It is just that some customers prefer to have the time in the descriptors as that is provided per frame.
>> The registers are simply overwritten when a new event frame is transmitted/received and so software could miss it."
>> The question is are you sure that you read timestamp for current frame? (not for the next frame).
>
> AFAICT, having the time stamp in the descriptor is not universally
> supported.  Looking at the Xilinx Zynq 7000 TRM, I can't find any
> mention of this.
>
> This Cadence IP core is a complete disaster.
>
> Unless someone can tell us how this IP works in all of its
> incarnations, this series is going nowhere.

>From the revision history of Cadence spec, all versions starting
r1p02 have ability to include timestamp in descriptors.
For previous versions the event register is the only option.
But yes, there have been multiple enhancements and
bug fixes in this IP w.r.t PTP making each implementation
different.

Regards,
Harini

^ permalink raw reply

* [RFC PATCH net-next v4 1/2] macb: Add 1588 support in Cadence GEM.
From: Richard Cochran @ 2017-01-02 11:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <BN3PR07MB2516815EDE69ACDCE27F81A3C96F0@BN3PR07MB2516.namprd07.prod.outlook.com>

On Mon, Jan 02, 2017 at 09:36:10AM +0000, Rafal Ozieblo wrote:
> According Cadence Hardware team:
> "It is just that some customers prefer to have the time in the descriptors as that is provided per frame.
> The registers are simply overwritten when a new event frame is transmitted/received and so software could miss it."
> The question is are you sure that you read timestamp for current frame? (not for the next frame).

AFAICT, having the time stamp in the descriptor is not universally
supported.  Looking at the Xilinx Zynq 7000 TRM, I can't find any
mention of this.

This Cadence IP core is a complete disaster.

Unless someone can tell us how this IP works in all of its
incarnations, this series is going nowhere.

Thanks,
Richard

^ permalink raw reply

* [PATCH v7 5/5] ARM: dts: da850: specify the maximum pixel clock rate for tilcdc
From: Sekhar Nori @ 2017-01-02 11:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1481623759-12786-6-git-send-email-bgolaszewski@baylibre.com>

On Tuesday 13 December 2016 03:39 PM, Bartosz Golaszewski wrote:
> At maximum CPU frequency of 300 MHz the maximum pixel clock frequency
> is 37.5 MHz[1]. We must filter out any mode for which the calculated
> pixel clock rate would exceed this value.
> 
> Specify the max-pixelclock property for the display node for
> da850-lcdk.
> 
> [1] http://processors.wiki.ti.com/index.php/OMAP-L1x/C674x/AM1x_LCD_Controller_(LCDC)_Throughput_and_Optimization_Techniques

This wiki page is not really a TI datasheet and can change without
notice. I changed this to refer to
http://www.ti.com/lit/ds/symlink/am1808.pdf (SPRS653E, revised March
2014), table 6-110.

Applied to v4.11/dt

Thanks,
Sekhar

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox