RE: [v2] powerpc: Fix incorrect PPC32 PAMU dependency
> -Original Message- > From: Linuxppc-dev [mailto:linuxppc-dev- > bounces+varun.sethi=freescale@lists.ozlabs.org] On Behalf Of Madalin- > Cristian Bucur > Sent: Wednesday, March 23, 2016 3:29 PM > To: j...@8bytes.org; Andy Fleming <aflem...@gmail.com> > Cc: iommu@lists.linux-foundation.org; linuxppc-...@lists.ozlabs.org > Subject: Re: [v2] powerpc: Fix incorrect PPC32 PAMU dependency > > > -Original Message- > > From: Andy Fleming <aflem...@gmail.com> > > To: j...@8bytes.org > > Cc: iommu@lists.linux-foundation.org, linuxppc-...@lists.ozlabs.org > > Subject: [v2] powerpc: Fix incorrect PPC32 PAMU dependency > > > > The Freescale PAMU can be enabled on both 32 and 64-bit Power chips. > > Commit 477ab7a19cec8409e4e2dd10e7348e4cac3c06e5 > > (iommu: Make more drivers depend on COMPILE_TEST) restricted PAMU to > > PPC32. PPC covers both. > > > > Signed-off-by: Andy Fleming <aflem...@gmail.com> > > Tested-by: Madalin Bucur <madalin.bu...@freescale.com> > Ackd-by: Varun Sethi <varun.se...@freescale.com> ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [RFC PATCH 0/2] VFIO no-iommu
Hi Alex, Thanks for the patch Alex. This would also require support in Qemu to expose the physical address to the VM. Are you looking at that part as well? Regards Varun -Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu-boun...@lists.linux-foundation.org] On Behalf Of Alex Williamson Sent: Saturday, October 10, 2015 12:11 AM To: alex.william...@redhat.com Cc: a...@scylladb.com; a...@cloudius-systems.com; g...@scylladb.com; m...@redhat.com; bruce.richard...@intel.com; cor...@lwn.net; linux-ker...@vger.kernel.org; alexander.du...@gmail.com; g...@cloudius-systems.com; step...@networkplumber.org; vl...@cloudius-systems.com; iommu@lists.linux-foundation.org; h...@hansjkoch.de; gre...@linuxfoundation.org Subject: [RFC PATCH 0/2] VFIO no-iommu Recent patches for UIO have been attempting to add MSI/X support, which unfortunately implies DMA support, which users have been enabling anyway, but was never intended for UIO. VFIO on the other hand expects an IOMMU to provide isolation of devices, but provides a much more complete device interface, which already supports full MSI/X support. There's really no way to support userspace drivers with DMA capable devices without an IOMMU to protect the host, but we can at least think about doing it in a way that properly taints the kernel and avoids creating new code duplicating existing code, that does have a supportable use case. The diffstat is only so large because I moved vfio.c to vfio_core.c so I could more easily keep the module named vfio.ko while keeping the bulk of the no-iommu support in a separate file that can be optionally compiled. We're really looking at a couple hundred lines of mostly stub code. The VFIO_NOIOMMU_IOMMU could certainly be expanded to do page pinning and virt_to_bus() translation, but I didn't want to complicate anything yet. I've only compiled this and tested loading the module with the new no-iommu mode enabled, I haven't actually tried to port a DPDK driver to it, though it ought to be a pretty obvious mix of the existing UIO and VFIO versions (set the IOMMU, but avoid using it for mapping, use however bus translations are done w/ UIO). The core vfio device file is still /dev/vfio/vfio, but all the groups become /dev/vfio-noiommu/$GROUP. It should be obvious, but I always feel obligated to state that this does not and will not ever enable device assignment to virtual machines on non-IOMMU capable platforms. I'm curious what IOMMU folks think of this. This hack is really only possible because we don't use iommu_ops for regular DMA, so we can hijack it fairly safely. I believe that's intended to change though, so this may not be practical long term. Thanks, Alex --- Alex Williamson (2): vfio: Move vfio.c vfio_core.c vfio: Include no-iommu mode drivers/vfio/Kconfig| 15 drivers/vfio/Makefile |4 drivers/vfio/vfio.c | 1640 -- drivers/vfio/vfio_core.c| 1680 +++ drivers/vfio/vfio_noiommu.c | 185 + drivers/vfio/vfio_private.h | 31 + include/uapi/linux/vfio.h |2 7 files changed, 1917 insertions(+), 1640 deletions(-) delete mode 100644 drivers/vfio/vfio.c create mode 100644 drivers/vfio/vfio_core.c create mode 100644 drivers/vfio/vfio_noiommu.c create mode 100644 drivers/vfio/vfio_private.h ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v2] iommu/fsl: Really fix init section(s) content
Thanks Madalin. Joerg, Can you please pick this patch. Regards Varun Original message From: Bucur Madalin-Cristian-B32716 Date:08/17/2015 8:28 PM (GMT+05:30) To: j...@8bytes.org,iommu@lists.linux-foundation.org,Sethi Varun-B16395 Cc: sta...@vger.kernel.org Subject: RE: [PATCH v2] iommu/fsl: Really fix init section(s) content Patch working on all tested platforms: B4860QDS, P2041RDB, P3041DS, P4080DS, P5020DS, P5040DS, T1024RDB, T1040RDB, T2080RDB, T4240QDS. Tested-by: Madalin Bucur madalin.bu...@freescale.com -Original Message- From: Sethi Varun-B16395 Sent: Monday, August 03, 2015 3:29 PM To: iommu@lists.linux-foundation.org; j...@8bytes.org Cc: sta...@vger.kernel.org; Medve Emilian-EMMEDVE1; Bucur Madalin- Cristian-B32716 Subject: RE: [PATCH v2] iommu/fsl: Really fix init section(s) content Hi Joerg, This patch doesn't seem to be applied to the PAMU driver. Regards Varun -Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Emil Medve Sent: Wednesday, March 25, 2015 10:59 AM To: iommu@lists.linux-foundation.org; j...@8bytes.org Cc: sta...@vger.kernel.org; Medve Emilian-EMMEDVE1 Subject: [PATCH v2] iommu/fsl: Really fix init section(s) content '0f1fb99 iommu/fsl: Fix section mismatch' was intended to address the modpost warning and the potential crash. Crash which is actually easy to trigger with a 'unbind' followed by a 'bind' sequence. The fix is wrong as fsl_of_pamu_driver.driver gets added by bus_add_driver() to a couple of klist(s) which become invalid/corrupted as soon as the init sections are freed. Depending on when/how the init sections storage is reused various/random errors and crashes will happen 'cd70d46 iommu/fsl: Various cleanups' contains annotations that go further down the wrong path laid by '0f1fb99 iommu/fsl: Fix section mismatch' Now remove all the incorrect annotations from the above mentioned patches (not exactly a revert) and those previously existing in the code, This fixes the modpost warning(s), the unbind/bind sequence crashes and the random errors/crashes Fixes: 0f1fb99b62ce (iommu/fsl: Fix section mismatch) Fixes: cd70d4659ff3 (iommu/fsl: Various cleanups) Signed-off-by: Emil Medve emilian.me...@freescale.com Acked-by: Varun Sethi varun.se...@freescale.com Cc: sta...@vger.kernel.org --- drivers/iommu/fsl_pamu.c | 26 +- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c index abeedc9..2570f2a 100644 --- a/drivers/iommu/fsl_pamu.c +++ b/drivers/iommu/fsl_pamu.c @@ -41,7 +41,6 @@ struct pamu_isr_data { static struct paace *ppaact; static struct paace *spaact; -static struct ome *omt __initdata; /* * Table for matching compatible strings, for device tree @@ -50,7 +49,7 @@ static struct ome *omt __initdata; * SOCs. For the older SOCs fsl,qoriq-device-config-1.0 * string would be used. */ -static const struct of_device_id guts_device_ids[] __initconst = { +static const struct of_device_id guts_device_ids[] = { { .compatible = fsl,qoriq-device-config-1.0, }, { .compatible = fsl,qoriq-device-config-2.0, }, {} @@ -599,7 +598,7 @@ found_cpu_node: * Memory accesses to QMAN and BMAN private memory need not be coherent, so * clear the PAACE entry coherency attribute for them. */ -static void __init setup_qbman_paace(struct paace *ppaace, int paace_type) +static void setup_qbman_paace(struct paace *ppaace, int paace_type) { switch (paace_type) { case QMAN_PAACE: @@ -629,7 +628,7 @@ static void __init setup_qbman_paace(struct paace *ppaace, int paace_type) * this table to translate device transaction to appropriate corenet * transaction. */ -static void __init setup_omt(struct ome *omt) +static void setup_omt(struct ome *omt) { struct ome *ome; @@ -666,7 +665,7 @@ static void __init setup_omt(struct ome *omt) * Get the maximum number of PAACT table entries * and subwindows supported by PAMU */ -static void __init get_pamu_cap_values(unsigned long pamu_reg_base) +static void get_pamu_cap_values(unsigned long pamu_reg_base) { u32 pc_val; @@ -676,9 +675,9 @@ static void __init get_pamu_cap_values(unsigned long pamu_reg_base) } /* Setup PAMU registers pointing to PAACT, SPAACT and OMT */ -static int __init setup_one_pamu(unsigned long pamu_reg_base, unsigned long pamu_reg_size, -phys_addr_t ppaact_phys, phys_addr_t spaact_phys, -phys_addr_t omt_phys) +static int setup_one_pamu(unsigned long pamu_reg_base, unsigned long pamu_reg_size, + phys_addr_t ppaact_phys, phys_addr_t spaact_phys, + phys_addr_t omt_phys
RE: [PATCH 2/3] Docs: dt: Add PCI MSI map bindings
Hi Mark, Thanks for the response. Please find my comments inline. Regards Varun -Original Message- From: Mark Rutland [mailto:mark.rutl...@arm.com] Sent: Thursday, August 06, 2015 11:09 PM To: Sethi Varun-B16395 Cc: devicet...@vger.kernel.org; Lorenzo Pieralisi; a...@arndb.de; Marc Zyngier; Will Deacon; linux-ker...@vger.kernel.org; dda...@caviumnetworks.com; iommu@lists.linux-foundation.org; tirumalesh.chalama...@caviumnetworks.com; laurent.pinch...@ideasonboard.com; thunder.leiz...@huawei.com; tred...@nvidia.com; linux-arm-ker...@lists.infradead.org; majun...@huawei.com; Yoder Stuart-B08248 Subject: Re: [PATCH 2/3] Docs: dt: Add PCI MSI map bindings On Wed, Aug 05, 2015 at 05:39:33PM +0100, Varun Sethi wrote: Hi Mark Thanks for the patch. Please find my comment inline. Regards Varun -Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Mark Rutland Sent: Thursday, July 23, 2015 10:23 PM To: devicet...@vger.kernel.org Cc: Mark Rutland; lorenzo.pieral...@arm.com; a...@arndb.de; marc.zyng...@arm.com; will.dea...@arm.com; linux- ker...@vger.kernel.org; dda...@caviumnetworks.com; iommu@lists.linux- foundation.org; tirumalesh.chalama...@caviumnetworks.com; laurent.pinch...@ideasonboard.com; thunder.leiz...@huawei.com; tred...@nvidia.com; linux-arm-ker...@lists.infradead.org; majun...@huawei.com Subject: [PATCH 2/3] Docs: dt: Add PCI MSI map bindings Currently msi-parent is used by a few bindings to describe the relationship between a PCI root complex and a single MSI controller, but this property does not have a generic binding document. Additionally, msi-parent is insufficient to describe more complex relationships between MSI controllers and devices under a root complex, where devices may be able to target multiple MSI controllers, or where MSI controllers use (non-probeable) sideband information to distinguish devices. This patch adds a generic binding for mapping PCI devices to MSI controllers. This document covers msi-parent, and a new msi-map property (specific to PCI*) which may be used to map devices (identified by their Requester ID) to sideband data for each MSI controller that they may target. Signed-off-by: Mark Rutland mark.rutl...@arm.com --- Documentation/devicetree/bindings/pci/pci-msi.txt | 220 ++ 1 file changed, 220 insertions(+) create mode 100644 Documentation/devicetree/bindings/pci/pci-msi.txt diff --git a/Documentation/devicetree/bindings/pci/pci-msi.txt b/Documentation/devicetree/bindings/pci/pci-msi.txt new file mode 100644 index 000..9b3cc81 --- /dev/null +++ b/Documentation/devicetree/bindings/pci/pci-msi.txt @@ -0,0 +1,220 @@ +This document describes the generic device tree binding for +describing the relationship between PCI devices and MSI controllers. + +Each PCI device under a root complex is uniquely identified by its +Requester ID (AKA RID). A Requester ID is a triplet of a Bus +number, Device number, and Function number. + +For the purpose of this document, when treated as a numeric value, +a RID is formatted such that: + +* Bits [15:8] are the Bus number. +* Bits [7:3] are the Device number. +* Bits [2:0] are the Function number. +* Any other bits required for padding must be zero. + +MSIs may be distinguished in part through the use of sideband data +accompanying writes. In the case of PCI devices, this sideband data +may be derived from the Requester ID. A mechanism is required to +associate a device with both the MSI controllers it can address, +and the sideband data that will be associated with its writes to those controllers. + +For generic MSI bindings, see +Documentation/devicetree/bindings/interrupt-controller/msi.txt. + + +PCI root complex + + +Optional properties +--- + +- msi-map: Maps a Requester ID to an MSI controller and associated + msi-specifier data. The property is an arbitrary number of tuples +of + (rid-base,msi-controller,msi-base,length), where: [varun] How would we account for hot plug PCI devices and SR-IOV use cases, with the rid base and length? For hotplug, you simply need the mapping from RID to msi-specifier to be defined in advance in the DT, for the set of RIDs that could possibly occur. For SR-IOV, are you asking about ARI? I should update the description of the RID to describe that for ARI it has the format: * Bits [15:8] are the Bus number * Bits [7:0] are the Identifier Other than that, the handling would be identical to the non-ARI case. What else am I missing? How do we take in to account for a PCIe bridge, while setting up the requestor ID base and length? I'm not sure I follow
RE: [PATCH 2/3] Docs: dt: Add PCI MSI map bindings
Hi Mark Thanks for the patch. Please find my comment inline. Regards Varun -Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Mark Rutland Sent: Thursday, July 23, 2015 10:23 PM To: devicet...@vger.kernel.org Cc: Mark Rutland; lorenzo.pieral...@arm.com; a...@arndb.de; marc.zyng...@arm.com; will.dea...@arm.com; linux- ker...@vger.kernel.org; dda...@caviumnetworks.com; iommu@lists.linux- foundation.org; tirumalesh.chalama...@caviumnetworks.com; laurent.pinch...@ideasonboard.com; thunder.leiz...@huawei.com; tred...@nvidia.com; linux-arm-ker...@lists.infradead.org; majun...@huawei.com Subject: [PATCH 2/3] Docs: dt: Add PCI MSI map bindings Currently msi-parent is used by a few bindings to describe the relationship between a PCI root complex and a single MSI controller, but this property does not have a generic binding document. Additionally, msi-parent is insufficient to describe more complex relationships between MSI controllers and devices under a root complex, where devices may be able to target multiple MSI controllers, or where MSI controllers use (non-probeable) sideband information to distinguish devices. This patch adds a generic binding for mapping PCI devices to MSI controllers. This document covers msi-parent, and a new msi-map property (specific to PCI*) which may be used to map devices (identified by their Requester ID) to sideband data for each MSI controller that they may target. Signed-off-by: Mark Rutland mark.rutl...@arm.com --- Documentation/devicetree/bindings/pci/pci-msi.txt | 220 ++ 1 file changed, 220 insertions(+) create mode 100644 Documentation/devicetree/bindings/pci/pci-msi.txt diff --git a/Documentation/devicetree/bindings/pci/pci-msi.txt b/Documentation/devicetree/bindings/pci/pci-msi.txt new file mode 100644 index 000..9b3cc81 --- /dev/null +++ b/Documentation/devicetree/bindings/pci/pci-msi.txt @@ -0,0 +1,220 @@ +This document describes the generic device tree binding for describing +the relationship between PCI devices and MSI controllers. + +Each PCI device under a root complex is uniquely identified by its +Requester ID (AKA RID). A Requester ID is a triplet of a Bus number, +Device number, and Function number. + +For the purpose of this document, when treated as a numeric value, a +RID is formatted such that: + +* Bits [15:8] are the Bus number. +* Bits [7:3] are the Device number. +* Bits [2:0] are the Function number. +* Any other bits required for padding must be zero. + +MSIs may be distinguished in part through the use of sideband data +accompanying writes. In the case of PCI devices, this sideband data may +be derived from the Requester ID. A mechanism is required to associate +a device with both the MSI controllers it can address, and the sideband +data that will be associated with its writes to those controllers. + +For generic MSI bindings, see +Documentation/devicetree/bindings/interrupt-controller/msi.txt. + + +PCI root complex + + +Optional properties +--- + +- msi-map: Maps a Requester ID to an MSI controller and associated + msi-specifier data. The property is an arbitrary number of tuples of + (rid-base,msi-controller,msi-base,length), where: [varun] How would we account for hot plug PCI devices and SR-IOV use cases, with the rid base and length? How do we take in to account for a PCIe bridge, while setting up the requestor ID base and length? + + * rid-base is a single cell describing the first RID matched by the entry. + + * msi-controller is a single phandle to an MSI controller + + * msi-base is an msi-specifier describing the msi-specifier produced for the +first RID matched by the entry. + + * length is a single cell describing how many consecutive RIDs are matched +following the rid-base. + + Any RID r in the interval [rid-base, rid-base + length) is associated + with the listed msi-controller, with the msi-specifier (r - rid-base + msi- base). + +- msi-map-mask: A mask to be applied to each Requester ID prior to +being mapped + to an msi-specifier per the msi-map property. + [varun] Can you please elaborate on a use case, where this would help. +- msi-parent: Describes the MSI parent of the root complex itself. +Where + the root complex and MSI controller do not pass sideband data with +MSI + writes, this property may be used to describe the MSI controller(s) + used by PCI devices under the root complex, if defined as such in the + binding for the root complex. + + +Example (1) +=== + +/ { + #address-cells = 1; + #size-cells = 1; + + msi: msi-controller@a { + reg = 0xa 0x1; + compatible = vendor,some-controller; + msi-controller; + #msi-cells = 1; + }; + + pci: pci@f { +
RE: [PATCH v2] iommu/fsl: Really fix init section(s) content
Hi Joerg, This patch doesn't seem to be applied to the PAMU driver. Regards Varun -Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Emil Medve Sent: Wednesday, March 25, 2015 10:59 AM To: iommu@lists.linux-foundation.org; j...@8bytes.org Cc: sta...@vger.kernel.org; Medve Emilian-EMMEDVE1 Subject: [PATCH v2] iommu/fsl: Really fix init section(s) content '0f1fb99 iommu/fsl: Fix section mismatch' was intended to address the modpost warning and the potential crash. Crash which is actually easy to trigger with a 'unbind' followed by a 'bind' sequence. The fix is wrong as fsl_of_pamu_driver.driver gets added by bus_add_driver() to a couple of klist(s) which become invalid/corrupted as soon as the init sections are freed. Depending on when/how the init sections storage is reused various/random errors and crashes will happen 'cd70d46 iommu/fsl: Various cleanups' contains annotations that go further down the wrong path laid by '0f1fb99 iommu/fsl: Fix section mismatch' Now remove all the incorrect annotations from the above mentioned patches (not exactly a revert) and those previously existing in the code, This fixes the modpost warning(s), the unbind/bind sequence crashes and the random errors/crashes Fixes: 0f1fb99b62ce (iommu/fsl: Fix section mismatch) Fixes: cd70d4659ff3 (iommu/fsl: Various cleanups) Signed-off-by: Emil Medve emilian.me...@freescale.com Acked-by: Varun Sethi varun.se...@freescale.com Cc: sta...@vger.kernel.org --- drivers/iommu/fsl_pamu.c | 26 +- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c index abeedc9..2570f2a 100644 --- a/drivers/iommu/fsl_pamu.c +++ b/drivers/iommu/fsl_pamu.c @@ -41,7 +41,6 @@ struct pamu_isr_data { static struct paace *ppaact; static struct paace *spaact; -static struct ome *omt __initdata; /* * Table for matching compatible strings, for device tree @@ -50,7 +49,7 @@ static struct ome *omt __initdata; * SOCs. For the older SOCs fsl,qoriq-device-config-1.0 * string would be used. */ -static const struct of_device_id guts_device_ids[] __initconst = { +static const struct of_device_id guts_device_ids[] = { { .compatible = fsl,qoriq-device-config-1.0, }, { .compatible = fsl,qoriq-device-config-2.0, }, {} @@ -599,7 +598,7 @@ found_cpu_node: * Memory accesses to QMAN and BMAN private memory need not be coherent, so * clear the PAACE entry coherency attribute for them. */ -static void __init setup_qbman_paace(struct paace *ppaace, int paace_type) +static void setup_qbman_paace(struct paace *ppaace, int paace_type) { switch (paace_type) { case QMAN_PAACE: @@ -629,7 +628,7 @@ static void __init setup_qbman_paace(struct paace *ppaace, int paace_type) * this table to translate device transaction to appropriate corenet * transaction. */ -static void __init setup_omt(struct ome *omt) +static void setup_omt(struct ome *omt) { struct ome *ome; @@ -666,7 +665,7 @@ static void __init setup_omt(struct ome *omt) * Get the maximum number of PAACT table entries * and subwindows supported by PAMU */ -static void __init get_pamu_cap_values(unsigned long pamu_reg_base) +static void get_pamu_cap_values(unsigned long pamu_reg_base) { u32 pc_val; @@ -676,9 +675,9 @@ static void __init get_pamu_cap_values(unsigned long pamu_reg_base) } /* Setup PAMU registers pointing to PAACT, SPAACT and OMT */ -static int __init setup_one_pamu(unsigned long pamu_reg_base, unsigned long pamu_reg_size, - phys_addr_t ppaact_phys, phys_addr_t spaact_phys, - phys_addr_t omt_phys) +static int setup_one_pamu(unsigned long pamu_reg_base, unsigned long pamu_reg_size, + phys_addr_t ppaact_phys, phys_addr_t spaact_phys, + phys_addr_t omt_phys) { u32 *pc; struct pamu_mmap_regs *pamu_regs; @@ -720,7 +719,7 @@ static int __init setup_one_pamu(unsigned long pamu_reg_base, unsigned long pamu } /* Enable all device LIODNS */ -static void __init setup_liodns(void) +static void setup_liodns(void) { int i, len; struct paace *ppaace; @@ -846,7 +845,7 @@ struct ccsr_law { /* * Create a coherence subdomain for a given memory block. */ -static int __init create_csd(phys_addr_t phys, size_t size, u32 csd_port_id) +static int create_csd(phys_addr_t phys, size_t size, u32 csd_port_id) { struct device_node *np; const __be32 *iprop; @@ -988,7 +987,7 @@ error: static const struct { u32 svr; u32 port_id; -} port_id_map[] __initconst = { +} port_id_map[] = { {(SVR_P2040 8) | 0x10, 0xFF00}, /* P2040 1.0 */ {(SVR_P2040 8) | 0x11, 0xFF00}, /* P2040 1.1
RE: [RFC 0/6] vSMMU initialization
Hi Will, -Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Tuesday, July 14, 2015 4:34 PM To: Sethi Varun-B16395 Cc: Baptiste Reynal; iommu@lists.linux-foundation.org; t...@virtualopensystems.com; qemu-de...@nongnu.org Subject: Re: [RFC 0/6] vSMMU initialization On Tue, Jul 14, 2015 at 03:21:03AM +0100, Varun Sethi wrote: Hi Will, Hi Varun, On Fri, Jun 12, 2015 at 03:20:04PM +0100, Baptiste Reynal wrote: The ARM SMMU has support for 2-stages address translations, allowing a virtual address to be translated at two levels: - Stage 1 translates a virtual address (VA) into an intermediate physical address (IPA) - Stage 2 translates an IPA into a physical address (PA) Will Deacon introduced a virtual SMMU interface for KVM, which gives a virtual machine the possibility to use an IOMMU with native drivers. While the VM will program the first stage of translation (stage 1), the interface will program the second (stage 2) on the physical SMMU. Please note that I have no plans to merge the kernel-side of this at the moment. It was merely an exploratory tool to see what a non-PV vSMMU implementation might look like and certainly not intended to be used in anger. How do you see the context fault reporting work for the PV interface? We could have an interrupt, for the PV IOMMU and have the hypervisor inject that, no? Can you please elaborate on the PV IOMMU interface. I want to understand how context fault information would be communicated to the guest. -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [RFC 0/6] vSMMU initialization
Hi Baptiste, -Original Message- From: Baptiste Reynal [mailto:b.rey...@virtualopensystems.com] Sent: Wednesday, July 15, 2015 7:08 PM To: Will Deacon Cc: Sethi Varun-B16395; iommu@lists.linux-foundation.org; t...@virtualopensystems.com; qemu-de...@nongnu.org Subject: Re: [RFC 0/6] vSMMU initialization On Tue, Jul 14, 2015 at 1:04 PM, Will Deacon will.dea...@arm.com wrote: On Tue, Jul 14, 2015 at 03:21:03AM +0100, Varun Sethi wrote: Hi Will, Hi Varun, On Fri, Jun 12, 2015 at 03:20:04PM +0100, Baptiste Reynal wrote: The ARM SMMU has support for 2-stages address translations, allowing a virtual address to be translated at two levels: - Stage 1 translates a virtual address (VA) into an intermediate physical address (IPA) - Stage 2 translates an IPA into a physical address (PA) Will Deacon introduced a virtual SMMU interface for KVM, which gives a virtual machine the possibility to use an IOMMU with native drivers. While the VM will program the first stage of translation (stage 1), the interface will program the second (stage 2) on the physical SMMU. Please note that I have no plans to merge the kernel-side of this at the moment. It was merely an exploratory tool to see what a non-PV vSMMU implementation might look like and certainly not intended to be used in anger. How do you see the context fault reporting work for the PV interface? We could have an interrupt, for the PV IOMMU and have the hypervisor inject that, no? Currently the vSMMU interface does seem quiet restrictive and it may simplify things by having PV iommu interface. But, do you see this even true in case of SMMUv3? Varun, may I know what do you mean by more restrictive ? Do you have in mind any use case which should apply to the PV interface and not the vSMMU ? [varun] What I meant was that vSMMU allows for setting up of the stage 1 translation, but doesn't specifically allow access to other SMMU hardware functionality. We can certainly extend the vSMMU interface for providing additional functionality to the guest VM. I do agree with Will, that for extending the vSMMU interface prototyping is necessary. We need to come up with specific use cases for that. For controlling stage 1 translation PV interface may be a bit simpler, but then we would need a generic interface to bind to most IOMMUs in the host. -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [RFC 0/6] vSMMU initialization
Hi Will, -Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Will Deacon Sent: Friday, June 12, 2015 7:53 PM To: Baptiste Reynal Cc: iommu@lists.linux-foundation.org; t...@virtualopensystems.com; qemu-de...@nongnu.org Subject: Re: [RFC 0/6] vSMMU initialization On Fri, Jun 12, 2015 at 03:20:04PM +0100, Baptiste Reynal wrote: The ARM SMMU has support for 2-stages address translations, allowing a virtual address to be translated at two levels: - Stage 1 translates a virtual address (VA) into an intermediate physical address (IPA) - Stage 2 translates an IPA into a physical address (PA) Will Deacon introduced a virtual SMMU interface for KVM, which gives a virtual machine the possibility to use an IOMMU with native drivers. While the VM will program the first stage of translation (stage 1), the interface will program the second (stage 2) on the physical SMMU. Please note that I have no plans to merge the kernel-side of this at the moment. It was merely an exploratory tool to see what a non-PV vSMMU implementation might look like and certainly not intended to be used in anger. How do you see the context fault reporting work for the PV interface? Currently the vSMMU interface does seem quiet restrictive and it may simplify things by having PV iommu interface. But, do you see this even true in case of SMMUv3? Just wondering if we can give more control with respect memory transaction attributes to the guest. Also, would it make sense to give guest control of the fault handling attributes i.e. fault/terminate model. -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH] iommu/fsl: Fix the dependency check for PAMU driver.
Hi Scott, -Original Message- From: Wood Scott-B07421 Sent: Thursday, May 14, 2015 11:43 PM To: Sethi Varun-B16395 Cc: linuxppc-...@lists.ozlabs.org; jroe...@suse.de; j...@8bytes.org; iommu@lists.linux-foundation.org Subject: Re: [PATCH] iommu/fsl: Fix the dependency check for PAMU driver. On Thu, 2015-05-14 at 23:11 +0530, Varun Sethi wrote: Fix the build dependency for the PAMU driver. PPC32 build dependecy is incorrect. Add the CORENET_GENERIC build dependency for PAMU driver. Signed-off-by: Varun Sethi varun.se...@freescale.com --- drivers/iommu/Kconfig |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig index 1ae4e54..4ace8db 100644 --- a/drivers/iommu/Kconfig +++ b/drivers/iommu/Kconfig @@ -50,7 +50,7 @@ config OF_IOMMU config FSL_PAMU bool Freescale IOMMU support - depends on PPC32 + depends on CORENET_GENERIC depends on PPC_E500MC || COMPILE_TEST select IOMMU_API select GENERIC_ALLOCATOR CORENET_GENERIC is for board support. There is no guarantee that all corenet boards will use it. You already depend on PPC_E500MC; why do you need anything else (besides probably getting rid of || COMPILE_TEST which is useless if you do add CORENET_GENERIC, because CORENET_GENERIC implies PPC_E500MC)? Yes this is required to counter ||COMPILE_TEST. -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/fsl: Fix the dependency check for PAMU driver.
Fix the build dependency for the PAMU driver. PPC32 build dependecy is incorrect. Add the CORENET_GENERIC build dependency for PAMU driver. Signed-off-by: Varun Sethi varun.se...@freescale.com --- drivers/iommu/Kconfig |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig index 1ae4e54..4ace8db 100644 --- a/drivers/iommu/Kconfig +++ b/drivers/iommu/Kconfig @@ -50,7 +50,7 @@ config OF_IOMMU config FSL_PAMU bool Freescale IOMMU support - depends on PPC32 + depends on CORENET_GENERIC depends on PPC_E500MC || COMPILE_TEST select IOMMU_API select GENERIC_ALLOCATOR -- 1.7.9.5 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [iommu:ppc/pamu 1/1] drivers/iommu/fsl_pamu.h:24:32: fatal error: asm/fsl_pamu_stash.h: No such file or directory
Hi Joerg, -Original Message- From: jroe...@suse.de [mailto:jroe...@suse.de] Sent: Tuesday, May 05, 2015 6:47 PM To: Sethi Varun-B16395 Cc: j...@8bytes.org; iommu@lists.linux-foundation.org; Bucur Madalin- Cristian-B32716 Subject: Re: [iommu:ppc/pamu 1/1] drivers/iommu/fsl_pamu.h:24:32: fatal error: asm/fsl_pamu_stash.h: No such file or directory Hi Varun, On Wed, Apr 08, 2015 at 01:24:16PM +, Varun Sethi wrote: Following issue was observed while building (compile test) for a non-powerpc (e500mc) based platform. The pamu driver includes a file which is available in the architecture include directory. Should we move the header file to general Linux kernel include directory? Before I added the COMPILE_TEST dependency it looked like this: depends on PPC_E500MC Which means the driver was only buildable on e500mc and now you are telling me that this is broken. What am I missing? PPC_E500MC dependency is fine, but with the COMPIL_TEST flag (or condition) the PAMU driver code would be built on any platform if the compile test is selected. -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [iommu:ppc/pamu 1/1] drivers/iommu/fsl_pamu.h:24:32: fatal error: asm/fsl_pamu_stash.h: No such file or directory
Hi Joerg, -Original Message- From: jroe...@suse.de [mailto:jroe...@suse.de] Sent: Tuesday, May 05, 2015 7:13 PM To: Sethi Varun-B16395 Cc: j...@8bytes.org; iommu@lists.linux-foundation.org; Bucur Madalin- Cristian-B32716 Subject: Re: [iommu:ppc/pamu 1/1] drivers/iommu/fsl_pamu.h:24:32: fatal error: asm/fsl_pamu_stash.h: No such file or directory On Tue, May 05, 2015 at 01:22:24PM +, Varun Sethi wrote: Which means the driver was only buildable on e500mc and now you are telling me that this is broken. What am I missing? PPC_E500MC dependency is fine, but with the COMPIL_TEST flag (or condition) the PAMU driver code would be built on any platform if the compile test is selected. You said it is now also selected on non-ppc platforms, but there is a PPC32 dependency, so this shouldn't happen. It should build fine on any ppc platform as the include file is in arch/powerpc/include/asm, no? Actually PPC32 dependency is incorrect, that's what Emil's patch removed. As a result PAMU driver got built on other platforms as well. commit f2fafdd954d743a0e68e5cd76dbef2f2454deefa PAMU driver should depend on PPC_E500MC. -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: SMMU 2-stage support
Hi Will, -Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Will Deacon Sent: Monday, April 13, 2015 4:11 PM To: Baptiste Reynal Cc: Linux IOMMU Subject: Re: SMMU 2-stage support On Fri, Apr 03, 2015 at 10:55:02AM +0100, Baptiste Reynal wrote: We are eventually working on the vSMMU implementation. Relying on the talk Will Deacon gave at the Linux Plumbers IOMMU Microconference on October 2014 (http://linuxplumbersconf.org/2014/ocw/proposals/2019), I tried the vSMMU initialization. My position on the vSMMU still hasn't changed: Anyway, until somebody actually wants this feature I've put it on ice as it adds a whole bunch of complication to the ARM SMMU driver, as well as new user ABI extensions that I don't really want to maintain for fun. So, whilst it's great that you're looking at the code, I'm not very keen on merging anything until we have people committed to using it. Right now, the only feedback I've had has been going in the para-virt direction and I don't think we should do this for fun. Freescale would be interested in using the vSMMU implementation. We have use cases for assigning devices to guest user space. Are you suggesting that you are more inclined to using the para virtualized approach? Thanks, Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 15/16] iommu/fsl: Make use of domain_alloc and domain_free
-iommu_domain; } /* Configure geometry settings for all LIODNs associated with domain */ @@ -499,7 +503,7 @@ static int disable_domain_win(struct fsl_dma_domain *dma_domain, u32 wnd_nr) static void fsl_pamu_window_disable(struct iommu_domain *domain, u32 wnd_nr) { - struct fsl_dma_domain *dma_domain = domain-priv; + struct fsl_dma_domain *dma_domain = to_fsl_dma_domain(domain); unsigned long flags; int ret; @@ -530,7 +534,7 @@ static void fsl_pamu_window_disable(struct iommu_domain *domain, u32 wnd_nr) static int fsl_pamu_window_enable(struct iommu_domain *domain, u32 wnd_nr, phys_addr_t paddr, u64 size, int prot) { - struct fsl_dma_domain *dma_domain = domain-priv; + struct fsl_dma_domain *dma_domain = to_fsl_dma_domain(domain); struct dma_window *wnd; int pamu_prot = 0; int ret; @@ -607,7 +611,7 @@ static int handle_attach_device(struct fsl_dma_domain *dma_domain, int num) { unsigned long flags; - struct iommu_domain *domain = dma_domain-iommu_domain; + struct iommu_domain *domain = dma_domain-iommu_domain; int ret = 0; int i; @@ -653,7 +657,7 @@ static int handle_attach_device(struct fsl_dma_domain *dma_domain, static int fsl_pamu_attach_device(struct iommu_domain *domain, struct device *dev) { - struct fsl_dma_domain *dma_domain = domain-priv; + struct fsl_dma_domain *dma_domain = to_fsl_dma_domain(domain); const u32 *liodn; u32 liodn_cnt; int len, ret = 0; @@ -691,7 +695,7 @@ static int fsl_pamu_attach_device(struct iommu_domain *domain, static void fsl_pamu_detach_device(struct iommu_domain *domain, struct device *dev) { - struct fsl_dma_domain *dma_domain = domain-priv; + struct fsl_dma_domain *dma_domain = to_fsl_dma_domain(domain); const u32 *prop; int len; struct pci_dev *pdev = NULL; @@ -723,7 +727,7 @@ static void fsl_pamu_detach_device(struct iommu_domain *domain, static int configure_domain_geometry(struct iommu_domain *domain, void *data) { struct iommu_domain_geometry *geom_attr = data; - struct fsl_dma_domain *dma_domain = domain-priv; + struct fsl_dma_domain *dma_domain = to_fsl_dma_domain(domain); dma_addr_t geom_size; unsigned long flags; @@ -813,7 +817,7 @@ static int configure_domain_dma_state(struct fsl_dma_domain *dma_domain, bool en static int fsl_pamu_set_domain_attr(struct iommu_domain *domain, enum iommu_attr attr_type, void *data) { - struct fsl_dma_domain *dma_domain = domain-priv; + struct fsl_dma_domain *dma_domain = to_fsl_dma_domain(domain); int ret = 0; switch (attr_type) { @@ -838,7 +842,7 @@ static int fsl_pamu_set_domain_attr(struct iommu_domain *domain, static int fsl_pamu_get_domain_attr(struct iommu_domain *domain, enum iommu_attr attr_type, void *data) { - struct fsl_dma_domain *dma_domain = domain-priv; + struct fsl_dma_domain *dma_domain = to_fsl_dma_domain(domain); int ret = 0; switch (attr_type) { @@ -999,7 +1003,7 @@ static void fsl_pamu_remove_device(struct device *dev) static int fsl_pamu_set_windows(struct iommu_domain *domain, u32 w_count) { - struct fsl_dma_domain *dma_domain = domain-priv; + struct fsl_dma_domain *dma_domain = to_fsl_dma_domain(domain); unsigned long flags; int ret; @@ -1048,15 +1052,15 @@ static int fsl_pamu_set_windows(struct iommu_domain *domain, u32 w_count) static u32 fsl_pamu_get_windows(struct iommu_domain *domain) { - struct fsl_dma_domain *dma_domain = domain-priv; + struct fsl_dma_domain *dma_domain = to_fsl_dma_domain(domain); return dma_domain-win_cnt; } static const struct iommu_ops fsl_pamu_ops = { .capable= fsl_pamu_capable, - .domain_init= fsl_pamu_domain_init, - .domain_destroy = fsl_pamu_domain_destroy, + .domain_alloc = fsl_pamu_domain_alloc, + .domain_free= fsl_pamu_domain_free, .attach_dev = fsl_pamu_attach_device, .detach_dev = fsl_pamu_detach_device, .domain_window_enable = fsl_pamu_window_enable, diff --git a/drivers/iommu/fsl_pamu_domain.h b/drivers/iommu/fsl_pamu_domain.h index c90293f..f2b0f74 100644 --- a/drivers/iommu/fsl_pamu_domain.h +++ b/drivers/iommu/fsl_pamu_domain.h @@ -71,7 +71,7 @@ struct fsl_dma_domain { u32 stash_id; struct pamu_stash_attribute dma_stash; u32 snoop_id; - struct iommu_domain *iommu_domain; + struct iommu_domain iommu_domain; spinlock_t domain_lock; }; Acked-by: Varun Sethi varun.se...@freescale.com
RE: [iommu:ppc/pamu 1/1] drivers/iommu/fsl_pamu.h:24:32: fatal error: asm/fsl_pamu_stash.h: No such file or directory
Hi Joerg, Following issue was observed while building (compile test) for a non-powerpc (e500mc) based platform. The pamu driver includes a file which is available in the architecture include directory. Should we move the header file to general Linux kernel include directory? Regards Varun -Original Message- From: Emil Medve [mailto:emilian.me...@freescale.com] Sent: Wednesday, March 25, 2015 1:07 PM To: Sethi Varun-B16395; j...@8bytes.org; jroe...@suse.de Cc: iommu@lists.linux-foundation.org Subject: Re: [iommu:ppc/pamu 1/1] drivers/iommu/fsl_pamu.h:24:32: fatal error: asm/fsl_pamu_stash.h: No such file or directory Hello Varun, On 03/25/2015 01:25 AM, Sethi Varun-B16395 wrote: Hi Emil, -Original Message- From: Emil Medve [mailto:emilian.me...@freescale.com] Sent: Wednesday, March 25, 2015 11:41 AM To: Sethi Varun-B16395; j...@8bytes.org; jroe...@suse.de Cc: iommu@lists.linux-foundation.org Subject: Re: [iommu:ppc/pamu 1/1] drivers/iommu/fsl_pamu.h:24:32: fatal error: asm/fsl_pamu_stash.h: No such file or directory Hello Varun, On 03/25/2015 12:46 AM, Sethi Varun-B16395 wrote: -Original Message- From: Emil Medve [mailto:emilian.me...@freescale.com] Sent: Tuesday, March 24, 2015 2:10 PM To: linuxppc-...@linux.freescale.net; Sethi Varun-B16395 Subject: Re: [iommu:ppc/pamu 1/1] drivers/iommu/fsl_pamu.h:24:32: fatal error: asm/fsl_pamu_stash.h: No such file or directory Hello Varun, On 03/23/2015 07:02 PM, kbuild test robot wrote: tree: git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git ppc/pamu head: f2fafdd954d743a0e68e5cd76dbef2f2454deefa commit: f2fafdd954d743a0e68e5cd76dbef2f2454deefa [1/1] iommu/fsl: PAMU is also present on 64-bit SoC(s) config: microblaze-allyesconfig (attached as .config) reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp- tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout f2fafdd954d743a0e68e5cd76dbef2f2454deefa # save the attached .config to linux build tree make.cross ARCH=microblaze All error/warnings: In file included from drivers/iommu/fsl_pamu.c:21:0: drivers/iommu/fsl_pamu.h:24:32: fatal error: asm/fsl_pamu_stash.h: No such file or directory #include asm/fsl_pamu_stash.h ^ compilation terminated. vim +24 drivers/iommu/fsl_pamu.h 695093e3 Varun Sethi 2013-07-15 8 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the 695093e3 Varun Sethi 2013-07-15 9 * GNU General Public License for more details. 695093e3 Varun Sethi 2013-07-15 10 * 695093e3 Varun Sethi 2013-07-15 11 * You should have received a copy of the GNU General Public License 695093e3 Varun Sethi 2013-07-15 12 * along with this program; if not, write to the Free Software 695093e3 Varun Sethi 2013-07-15 13 * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. 695093e3 Varun Sethi 2013-07-15 14 * 695093e3 Varun Sethi 2013-07-15 15 * Copyright (C) 2013 Freescale Semiconductor, Inc. 695093e3 Varun Sethi 2013-07-15 16 * 695093e3 Varun Sethi 2013-07-15 17 */ 695093e3 Varun Sethi 2013-07-15 18 695093e3 Varun Sethi 2013-07-15 19 #ifndef __FSL_PAMU_H 695093e3 Varun Sethi 2013-07-15 20 #define __FSL_PAMU_H 695093e3 Varun Sethi 2013-07-15 21 cd70d465 Emil Medve 2015-01-28 22 #include linux/iommu.h cd70d465 Emil Medve 2015-01-28 23 695093e3 Varun Sethi 2013-07-15 @24 #include asm/fsl_pamu_stash.h The patch just triggered the build and for some reason these folks are building the driver on non-e500mc arches/defconfigs Hmm I believe PAMU driver got included due to the COMPILE_TEST dependency. This is an issue, if we add the COMPILE_TEST dependency, then we can't include architecture specific include files? What in fsl_pamu_stash.h is even more arch/platform specific then the content of drivers/iommu/fsl_pamu*? This was discussed during early PAMU driver reviews. Joerg's idea was that PAMU would specific to the Power architecture platform, so we should place the include file in the arch specific directory (originally I had placed it under include/linux). But the only content that ended up in arch/powerpc is one file with two definitions that don't seem particularly specific. The line is a bit blurry and until (if ever) we get different PAMU integration I doubt we can reasonably make it sharper Anyway, I'm unsure what an immediately acceptable answer is. I mean if we're to spend time supporting universal builds of the PAMU driver there is a list of (some non-trivial) changes we ought to do. This particular build issue is just the first Cheers, ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org
RE: [iommu:ppc/pamu 1/1] drivers/iommu/fsl_pamu.h:24:32: fatal error: asm/fsl_pamu_stash.h: No such file or directory
Hi Emil, -Original Message- From: Emil Medve [mailto:emilian.me...@freescale.com] Sent: Wednesday, March 25, 2015 11:41 AM To: Sethi Varun-B16395; j...@8bytes.org; jroe...@suse.de Cc: iommu@lists.linux-foundation.org Subject: Re: [iommu:ppc/pamu 1/1] drivers/iommu/fsl_pamu.h:24:32: fatal error: asm/fsl_pamu_stash.h: No such file or directory Hello Varun, On 03/25/2015 12:46 AM, Sethi Varun-B16395 wrote: -Original Message- From: Emil Medve [mailto:emilian.me...@freescale.com] Sent: Tuesday, March 24, 2015 2:10 PM To: linuxppc-...@linux.freescale.net; Sethi Varun-B16395 Subject: Re: [iommu:ppc/pamu 1/1] drivers/iommu/fsl_pamu.h:24:32: fatal error: asm/fsl_pamu_stash.h: No such file or directory Hello Varun, On 03/23/2015 07:02 PM, kbuild test robot wrote: tree: git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git ppc/pamu head: f2fafdd954d743a0e68e5cd76dbef2f2454deefa commit: f2fafdd954d743a0e68e5cd76dbef2f2454deefa [1/1] iommu/fsl: PAMU is also present on 64-bit SoC(s) config: microblaze-allyesconfig (attached as .config) reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp- tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout f2fafdd954d743a0e68e5cd76dbef2f2454deefa # save the attached .config to linux build tree make.cross ARCH=microblaze All error/warnings: In file included from drivers/iommu/fsl_pamu.c:21:0: drivers/iommu/fsl_pamu.h:24:32: fatal error: asm/fsl_pamu_stash.h: No such file or directory #include asm/fsl_pamu_stash.h ^ compilation terminated. vim +24 drivers/iommu/fsl_pamu.h 695093e3 Varun Sethi 2013-07-15 8 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the 695093e3 Varun Sethi 2013-07-15 9 * GNU General Public License for more details. 695093e3 Varun Sethi 2013-07-15 10 * 695093e3 Varun Sethi 2013-07-15 11 * You should have received a copy of the GNU General Public License 695093e3 Varun Sethi 2013-07-15 12 * along with this program; if not, write to the Free Software 695093e3 Varun Sethi 2013-07-15 13 * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. 695093e3 Varun Sethi 2013-07-15 14 * 695093e3 Varun Sethi 2013-07-15 15 * Copyright (C) 2013 Freescale Semiconductor, Inc. 695093e3 Varun Sethi 2013-07-15 16 * 695093e3 Varun Sethi 2013-07-15 17 */ 695093e3 Varun Sethi 2013-07-15 18 695093e3 Varun Sethi 2013-07-15 19 #ifndef __FSL_PAMU_H 695093e3 Varun Sethi 2013-07-15 20 #define __FSL_PAMU_H 695093e3 Varun Sethi 2013-07-15 21 cd70d465 Emil Medve 2015-01-28 22 #include linux/iommu.h cd70d465 Emil Medve 2015-01-28 23 695093e3 Varun Sethi 2013-07-15 @24 #include asm/fsl_pamu_stash.h The patch just triggered the build and for some reason these folks are building the driver on non-e500mc arches/defconfigs Hmm I believe PAMU driver got included due to the COMPILE_TEST dependency. This is an issue, if we add the COMPILE_TEST dependency, then we can't include architecture specific include files? What in fsl_pamu_stash.h is even more arch/platform specific then the content of drivers/iommu/fsl_pamu*? This was discussed during early PAMU driver reviews. Joerg's idea was that PAMU would specific to the Power architecture platform, so we should place the include file in the arch specific directory (originally I had placed it under include/linux). -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [iommu:ppc/pamu 1/1] drivers/iommu/fsl_pamu.h:24:32: fatal error: asm/fsl_pamu_stash.h: No such file or directory
-Original Message- From: Emil Medve [mailto:emilian.me...@freescale.com] Sent: Tuesday, March 24, 2015 2:10 PM To: linuxppc-...@linux.freescale.net; Sethi Varun-B16395 Subject: Re: [iommu:ppc/pamu 1/1] drivers/iommu/fsl_pamu.h:24:32: fatal error: asm/fsl_pamu_stash.h: No such file or directory Hello Varun, On 03/23/2015 07:02 PM, kbuild test robot wrote: tree: git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git ppc/pamu head: f2fafdd954d743a0e68e5cd76dbef2f2454deefa commit: f2fafdd954d743a0e68e5cd76dbef2f2454deefa [1/1] iommu/fsl: PAMU is also present on 64-bit SoC(s) config: microblaze-allyesconfig (attached as .config) reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp- tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout f2fafdd954d743a0e68e5cd76dbef2f2454deefa # save the attached .config to linux build tree make.cross ARCH=microblaze All error/warnings: In file included from drivers/iommu/fsl_pamu.c:21:0: drivers/iommu/fsl_pamu.h:24:32: fatal error: asm/fsl_pamu_stash.h: No such file or directory #include asm/fsl_pamu_stash.h ^ compilation terminated. vim +24 drivers/iommu/fsl_pamu.h 695093e3 Varun Sethi 2013-07-15 8 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the 695093e3 Varun Sethi 2013-07-15 9 * GNU General Public License for more details. 695093e3 Varun Sethi 2013-07-15 10 * 695093e3 Varun Sethi 2013-07-15 11 * You should have received a copy of the GNU General Public License 695093e3 Varun Sethi 2013-07-15 12 * along with this program; if not, write to the Free Software 695093e3 Varun Sethi 2013-07-15 13 * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. 695093e3 Varun Sethi 2013-07-15 14 * 695093e3 Varun Sethi 2013-07-15 15 * Copyright (C) 2013 Freescale Semiconductor, Inc. 695093e3 Varun Sethi 2013-07-15 16 * 695093e3 Varun Sethi 2013-07-15 17 */ 695093e3 Varun Sethi 2013-07-15 18 695093e3 Varun Sethi 2013-07-15 19 #ifndef __FSL_PAMU_H 695093e3 Varun Sethi 2013-07-15 20 #define __FSL_PAMU_H 695093e3 Varun Sethi 2013-07-15 21 cd70d465 Emil Medve 2015-01-28 22 #include linux/iommu.h cd70d465 Emil Medve 2015-01-28 23 695093e3 Varun Sethi 2013-07-15 @24 #include asm/fsl_pamu_stash.h The patch just triggered the build and for some reason these folks are building the driver on non-e500mc arches/defconfigs Hmm I believe PAMU driver got included due to the COMPILE_TEST dependency. This is an issue, if we add the COMPILE_TEST dependency, then we can't include architecture specific include files? -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH] iommu/fsl: Really fix init section(s) content
device_node *np; const __be32 *iprop; @@ -988,7 +987,7 @@ error: static const struct { u32 svr; u32 port_id; -} port_id_map[] __initconst = { +} port_id_map[] = { {(SVR_P2040 8) | 0x10, 0xFF00}, /* P2040 1.0 */ {(SVR_P2040 8) | 0x11, 0xFF00}, /* P2040 1.1 */ {(SVR_P2041 8) | 0x10, 0xFF00}, /* P2041 1.0 */ @@ -1006,7 +1005,7 @@ static const struct { #define SVR_SECURITY 0x8 /* The Security (E) bit */ -static int __init fsl_pamu_probe(struct platform_device *pdev) +static int fsl_pamu_probe(struct platform_device *pdev) { struct device *dev = pdev-dev; void __iomem *pamu_regs = NULL; @@ -1022,6 +1021,7 @@ static int __init fsl_pamu_probe(struct platform_device *pdev) int irq; phys_addr_t ppaact_phys; phys_addr_t spaact_phys; + struct ome *omt; phys_addr_t omt_phys; size_t mem_size = 0; unsigned int order = 0; @@ -1200,7 +1200,7 @@ error: return ret; } -static struct platform_driver fsl_of_pamu_driver __initdata = { +static struct platform_driver fsl_of_pamu_driver = { .driver = { .name = fsl-of-pamu, }, -- 2.3.0 Acked-by: Varun Sethi varun.se...@freescale.com ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH] iommu/fsl: Really fix init section(s) content
Thanks Emil, Has this also been tested in context of DPAA? -Original Message- From: Emil Medve [mailto:emilian.me...@freescale.com] Sent: Friday, February 13, 2015 3:15 AM To: iommu@lists.linux-foundation.org; j...@8bytes.org; jroe...@suse.de; Sethi Varun-B16395 Cc: Medve Emilian-EMMEDVE1 Subject: [PATCH] iommu/fsl: Really fix init section(s) content '0f1fb99 iommu/fsl: Fix section mismatch' was intended to address the modpost warning and the potential crash. Crash which is actually easy to trigger with a 'unbind' followed by a 'bind' sequence. The fix is/ wrong as fsl_of_pamu_driver.driver gets added by bus_add_driver() to a couple of klist(s) which become invalid/corrupted as soon as the init sections are freed. Depending on when/how the init sections storage is reused various/random errors and crashes will happen 'cd70d46 iommu/fsl: Various cleanups' contains annotations that go further down the wrong path laid by '0f1fb99 iommu/fsl: Fix section mismatch' Now remove all the incorrect annotations from the above mentioned patches (not exactly a revert) and those previously existing in the code, This fixes the modpost warning(s), the unbind/bind sequence crashes and the random errors/crashes Signed-off-by: Emil Medve emilian.me...@freescale.com --- Hello Joerg, This is a critical fix and needs to go into v3.20 Cheers, drivers/iommu/fsl_pamu.c | 26 +- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c index abeedc9..2570f2a 100644 --- a/drivers/iommu/fsl_pamu.c +++ b/drivers/iommu/fsl_pamu.c @@ -41,7 +41,6 @@ struct pamu_isr_data { static struct paace *ppaact; static struct paace *spaact; -static struct ome *omt __initdata; /* * Table for matching compatible strings, for device tree @@ -50,7 +49,7 @@ static struct ome *omt __initdata; * SOCs. For the older SOCs fsl,qoriq-device-config-1.0 * string would be used. */ -static const struct of_device_id guts_device_ids[] __initconst = { +static const struct of_device_id guts_device_ids[] = { { .compatible = fsl,qoriq-device-config-1.0, }, { .compatible = fsl,qoriq-device-config-2.0, }, {} @@ -599,7 +598,7 @@ found_cpu_node: * Memory accesses to QMAN and BMAN private memory need not be coherent, so * clear the PAACE entry coherency attribute for them. */ -static void __init setup_qbman_paace(struct paace *ppaace, int paace_type) +static void setup_qbman_paace(struct paace *ppaace, int paace_type) { switch (paace_type) { case QMAN_PAACE: @@ -629,7 +628,7 @@ static void __init setup_qbman_paace(struct paace *ppaace, int paace_type) * this table to translate device transaction to appropriate corenet * transaction. */ -static void __init setup_omt(struct ome *omt) +static void setup_omt(struct ome *omt) { struct ome *ome; @@ -666,7 +665,7 @@ static void __init setup_omt(struct ome *omt) * Get the maximum number of PAACT table entries * and subwindows supported by PAMU */ -static void __init get_pamu_cap_values(unsigned long pamu_reg_base) +static void get_pamu_cap_values(unsigned long pamu_reg_base) { u32 pc_val; @@ -676,9 +675,9 @@ static void __init get_pamu_cap_values(unsigned long pamu_reg_base) } /* Setup PAMU registers pointing to PAACT, SPAACT and OMT */ -static int __init setup_one_pamu(unsigned long pamu_reg_base, unsigned long pamu_reg_size, - phys_addr_t ppaact_phys, phys_addr_t spaact_phys, - phys_addr_t omt_phys) +static int setup_one_pamu(unsigned long pamu_reg_base, unsigned long pamu_reg_size, + phys_addr_t ppaact_phys, phys_addr_t spaact_phys, + phys_addr_t omt_phys) { u32 *pc; struct pamu_mmap_regs *pamu_regs; @@ -720,7 +719,7 @@ static int __init setup_one_pamu(unsigned long pamu_reg_base, unsigned long pamu } /* Enable all device LIODNS */ -static void __init setup_liodns(void) +static void setup_liodns(void) { int i, len; struct paace *ppaace; @@ -846,7 +845,7 @@ struct ccsr_law { /* * Create a coherence subdomain for a given memory block. */ -static int __init create_csd(phys_addr_t phys, size_t size, u32 csd_port_id) +static int create_csd(phys_addr_t phys, size_t size, u32 csd_port_id) { struct device_node *np; const __be32 *iprop; @@ -988,7 +987,7 @@ error: static const struct { u32 svr; u32 port_id; -} port_id_map[] __initconst = { +} port_id_map[] = { {(SVR_P2040 8) | 0x10, 0xFF00}, /* P2040 1.0 */ {(SVR_P2040 8) | 0x11, 0xFF00}, /* P2040 1.1 */ {(SVR_P2041 8) | 0x10, 0xFF00}, /* P2041 1.0 */ @@ -1006,7 +1005,7 @@ static const struct { #define SVR_SECURITY 0x8 /* The Security (E) bit */ -static int __init
RE: [PATCH v2 01/26] iommu/fsl: Sprinkle some __init* annotations
Hi Emil, -Original Message- From: Emil Medve [mailto:emilian.me...@freescale.com] Sent: Wednesday, February 11, 2015 4:37 PM To: Sethi Varun-B16395; iommu@lists.linux-foundation.org; j...@8bytes.org; jroe...@suse.de Subject: Re: [PATCH v2 01/26] iommu/fsl: Sprinkle some __init* annotations Hello Varun, On 02/09/2015 08:26 PM, Sethi Varun-B16395 wrote: Hi Emil, -Original Message- From: Emil Medve [mailto:emilian.me...@freescale.com] Sent: Tuesday, February 10, 2015 3:17 AM To: iommu@lists.linux-foundation.org; j...@8bytes.org; jroe...@suse.de; Sethi Varun-B16395 Subject: Re: [PATCH v2 01/26] iommu/fsl: Sprinkle some __init* annotations Hello Joerg, On 01/28/2015 08:34 AM, Emil Medve wrote: Signed-off-by: Emil Medve emilian.me...@freescale.com --- drivers/iommu/fsl_pamu.c| 12 ++-- drivers/iommu/fsl_pamu_domain.c | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) Please don't apply this patch as it's wrong. It's based on https://git.kernel.org/cgit/linux/kernel/git/joro/iommu.git/commit/?h =ppc/ pamuid=0f1fb99b62ce226f8d818852f812c5d79071ce58 which is wrong as well and its causing semi-random crashes upon more extensive testing. I will follow-up with details shortly Are you seeing a crash with https://git.kernel.org/cgit/linux/kernel/git/joro/iommu.git/commit/?h= ppc/pamuid=0f1fb99b62ce226f8d818852f812c5d79071ce58 ? At, what point do you see crashes? platform_driver.device_driver gets added to a (platform) bus klist of drivers. (There are also a handful of other pointers that get set to fsl_of_pamu_driver) Once fsl_of_pamu_driver gets released that klist becomes corrupted and, depending on how said memory gets re-used, traversing it for any reason will end up into a crash Anyway, I identified easier ways to produce kernel crashes related to the PAMU driver via sysfs (with or without my __init* annotation patches). For example a unbind, bind sequence will cause a crash with the driver as is in v3.19 BTW, why is the PAMU an early driver? I'll send out some patches to properly fix all these __init* annotations Yes, the fsl_of_pamu_driver structure shouldn't be marked init data, it could actually corrupt the driver list. PAMU driver should initialize before the DPAA driver initialization also, to ensure the default DMA window is setup for all devices. -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v2 01/26] iommu/fsl: Sprinkle some __init* annotations
Hi Emil, -Original Message- From: Sethi Varun-B16395 Sent: Wednesday, February 11, 2015 5:00 PM To: 'Emil Medve'; iommu@lists.linux-foundation.org; j...@8bytes.org; jroe...@suse.de Subject: RE: [PATCH v2 01/26] iommu/fsl: Sprinkle some __init* annotations Hi Emil, -Original Message- From: Emil Medve [mailto:emilian.me...@freescale.com] Sent: Wednesday, February 11, 2015 4:37 PM To: Sethi Varun-B16395; iommu@lists.linux-foundation.org; j...@8bytes.org; jroe...@suse.de Subject: Re: [PATCH v2 01/26] iommu/fsl: Sprinkle some __init* annotations Hello Varun, On 02/09/2015 08:26 PM, Sethi Varun-B16395 wrote: Hi Emil, -Original Message- From: Emil Medve [mailto:emilian.me...@freescale.com] Sent: Tuesday, February 10, 2015 3:17 AM To: iommu@lists.linux-foundation.org; j...@8bytes.org; jroe...@suse.de; Sethi Varun-B16395 Subject: Re: [PATCH v2 01/26] iommu/fsl: Sprinkle some __init* annotations Hello Joerg, On 01/28/2015 08:34 AM, Emil Medve wrote: Signed-off-by: Emil Medve emilian.me...@freescale.com --- drivers/iommu/fsl_pamu.c| 12 ++-- drivers/iommu/fsl_pamu_domain.c | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) Please don't apply this patch as it's wrong. It's based on https://git.kernel.org/cgit/linux/kernel/git/joro/iommu.git/commit/ ?h =ppc/ pamuid=0f1fb99b62ce226f8d818852f812c5d79071ce58 which is wrong as well and its causing semi-random crashes upon more extensive testing. I will follow-up with details shortly Are you seeing a crash with https://git.kernel.org/cgit/linux/kernel/git/joro/iommu.git/commit/? h= ppc/pamuid=0f1fb99b62ce226f8d818852f812c5d79071ce58 ? At, what point do you see crashes? platform_driver.device_driver gets added to a (platform) bus klist of drivers. (There are also a handful of other pointers that get set to fsl_of_pamu_driver) Once fsl_of_pamu_driver gets released that klist becomes corrupted and, depending on how said memory gets re-used, traversing it for any reason will end up into a crash Anyway, I identified easier ways to produce kernel crashes related to the PAMU driver via sysfs (with or without my __init* annotation patches). For example a unbind, bind sequence will cause a crash with the driver as is in v3.19 BTW, why is the PAMU an early driver? I'll send out some patches to properly fix all these __init* annotations Yes, the fsl_of_pamu_driver structure shouldn't be marked init data, it could actually corrupt the driver list. PAMU driver should initialize before the DPAA driver initialization also, to ensure the default DMA window is setup for all devices. I think that we can revert this patch, the section mismatch isn't really an issue. The pamu driver probe routine wouldn't get called during platform bus probe. -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 00/26] iommu/fsl: Various cleanup
Hi Joerg, I have applied the patches on the pamu_next branch at https://github.com/varunfsl/fsl_pamu/tree/pamu_next Please pull commit ids e72d948 to 14241be Regards Varun -Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Joerg Roedel Sent: Friday, January 30, 2015 9:06 PM To: Sethi Varun-B16395 Cc: iommu@lists.linux-foundation.org; jroe...@suse.de; Medve Emilian- EMMEDVE1 Subject: Re: [PATCH 00/26] iommu/fsl: Various cleanup On Fri, Jan 30, 2015 at 02:50:17PM +, Varun Sethi wrote: Acked, all the patches in the series. Can you apply the series or would you prefer a pull request? Yes, please add your acks and send me a pull-request. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 11/26] iommu/fsl: Fix checkpatch type OOM_MESSAGE
Hi Emil, -Original Message- From: Emil Medve [mailto:emilian.me...@freescale.com] Sent: Friday, January 30, 2015 7:49 PM To: Sethi Varun-B16395; iommu@lists.linux-foundation.org; j...@8bytes.org; jroe...@suse.de Subject: Re: [PATCH 11/26] iommu/fsl: Fix checkpatch type OOM_MESSAGE Hello Varun, On 01/30/2015 03:43 AM, Sethi Varun-B16395 wrote: Hi Emil, -Original Message- From: Emil Medve [mailto:emilian.me...@freescale.com] Sent: Wednesday, January 28, 2015 8:05 PM To: iommu@lists.linux-foundation.org; j...@8bytes.org; jroe...@suse.de; Sethi Varun-B16395 Cc: Medve Emilian-EMMEDVE1 Subject: [PATCH 11/26] iommu/fsl: Fix checkpatch type OOM_MESSAGE WARNING:OOM_MESSAGE: Possible unnecessary 'out of memory' message + if (!data) { + dev_err(pdev-dev, PAMU isr data memory allocation + failed\n); Signed-off-by: Emil Medve emilian.me...@freescale.com --- drivers/iommu/fsl_pamu.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c index 4f1926b..6f9c976 100644 --- a/drivers/iommu/fsl_pamu.c +++ b/drivers/iommu/fsl_pamu.c @@ -1041,7 +1041,6 @@ static int __init fsl_pamu_probe(struct platform_device *pdev) data = kzalloc(sizeof(*data), GFP_KERNEL); if (!data) { - dev_err(pdev-dev, PAMU isr data memory allocation failed\n); ret = -ENOMEM; goto error; } I think this is fine, there are other places as well where we have error prints while setting the ENOMEM as the return code. This is a checkpatch report and I quote from the log: Acked-by: Varun Sethi varun.se...@freescale.com ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 00/26] iommu/fsl: Various cleanup
Hi Joerg, -Original Message- From: Joerg Roedel [mailto:j...@8bytes.org] Sent: Friday, January 30, 2015 6:01 PM To: Medve Emilian-EMMEDVE1 Cc: iommu@lists.linux-foundation.org; jroe...@suse.de; Sethi Varun-B16395 Subject: Re: [PATCH 00/26] iommu/fsl: Various cleanup Varun, On Wed, Jan 28, 2015 at 08:34:32AM -0600, Emil Medve wrote: Currently a PAMU driver patch is very likely to receive some checkpatch complaints about the code in the context of the patch. This sequence is an attempt to fix most of that and make the dirver more readable Also fixed a subset of the sparse and coccinelle reported issues Emil Medve (26): iommu/fsl: Sprinkle some __init* annotations iommu/fsl: Use SVR_* instead of magic numbers iommu/fsl: Remove unused/extra includes iommu/fsl: Fix checkpatch type SPACE_BEFORE_TAB iommu/fsl: Fix checkpatch type LINE_SPACING iommu/fsl: Fix checkpatch type LEADING_SPACE iommu/fsl: Fix checkpath type BRACES iommu/fsl: Fix checkpatch type CODE_INDENT iommu/fsl: Fix checkpatch type ALLOC_SIZEOF_STRUCT iommu/fsl: Fix checkpatch type ALLOC_WITH_MULTIPLY iommu/fsl: Fix checkpatch type OOM_MESSAGE iommu/fsl: Fix checkpatch type TYPO_SPELLING iommu/fsl: Fix checkpatch type PREFER_PACKED iommu/fsl: Fix checkpatch type QUOTED_WHITESPACE_BEFORE_NEWLINE iommu/fsl: Fix checkpatch type SPACING iommu/fsl: Use a device pointer to make lines shorter iommu/fsl: Remove pr/dev_*() prefixes iommu/fsl: Fix checkpatch type PARENTHESIS_ALIGNMENT iommu/fsl: Fix some comments alignment iommu/fsl: Fix alignment of some stray lines iommu/fsl: Fix checkpatch type LONG_LINE iommu/fsl: Make local symbols static iommu/fsl: Use NULL instead of zero iommu/fsl: Remove unneeded semicolon iommu/fsl: Don't use integers values with bool type iommu/fsl: Remove extra paranthesis Can you collect the patches you acked and send me a pull-req for them? Acked, all the patches in the series. Can you apply the series or would you prefer a pull request? Acked-by: Varun Sethi varun.se...@freescale.com ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v2 01/26] iommu/fsl: Sprinkle some __init* annotations
Hi Emil, -Original Message- From: Emil Medve [mailto:emilian.me...@freescale.com] Sent: Friday, January 30, 2015 12:26 PM To: Sethi Varun-B16395; iommu@lists.linux-foundation.org; j...@8bytes.org; jroe...@suse.de Subject: Re: [PATCH v2 01/26] iommu/fsl: Sprinkle some __init* annotations Hello Varun, On 01/30/2015 12:46 AM, Sethi Varun-B16395 wrote: Hi Emil, -Original Message- From: Emil Medve [mailto:emilian.me...@freescale.com] Sent: Wednesday, January 28, 2015 8:05 PM To: iommu@lists.linux-foundation.org; j...@8bytes.org; jroe...@suse.de; Sethi Varun-B16395 Cc: Medve Emilian-EMMEDVE1 Subject: [PATCH v2 01/26] iommu/fsl: Sprinkle some __init* annotations Signed-off-by: Emil Medve emilian.me...@freescale.com --- drivers/iommu/fsl_pamu.c| 12 ++-- drivers/iommu/fsl_pamu_domain.c | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c index d958e65..652c34d 100644 --- a/drivers/iommu/fsl_pamu.c +++ b/drivers/iommu/fsl_pamu.c @@ -50,7 +50,7 @@ struct pamu_isr_data { static struct paace *ppaact; static struct paace *spaact; -static struct ome *omt; +static struct ome *omt __initdata; [varun] omt shouldn't be initdata. It seems to be used only from probe()'s call tree: fsl_pamu_probe() and setup_omt(); both __init functions. Am I missing something? Omt corresponds to PAMU operation mapping table, which is pointed to by the PAMU OMT base registers. We are using operation mapping table for operation translation. -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 18/26] iommu/fsl: Fix checkpatch type PARENTHESIS_ALIGNMENT
-by: Varun Sethi varun.se...@freescale.com ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 24/26] iommu/fsl: Remove unneeded semicolon
-Original Message- From: Emil Medve [mailto:emilian.me...@freescale.com] Sent: Wednesday, January 28, 2015 8:05 PM To: iommu@lists.linux-foundation.org; j...@8bytes.org; jroe...@suse.de; Sethi Varun-B16395 Cc: Medve Emilian-EMMEDVE1 Subject: [PATCH 24/26] iommu/fsl: Remove unneeded semicolon drivers/iommu/fsl_pamu_domain.c:859:2-3: Unneeded semicolon drivers/iommu/fsl_pamu_domain.c:833:2-3: Unneeded semicolon Signed-off-by: Emil Medve emilian.me...@freescale.com --- drivers/iommu/fsl_pamu_domain.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c index 38c26be..ebbc396 100644 --- a/drivers/iommu/fsl_pamu_domain.c +++ b/drivers/iommu/fsl_pamu_domain.c @@ -830,7 +830,7 @@ static int fsl_pamu_set_domain_attr(struct iommu_domain *domain, pr_debug(Unsupported attribute type\n); ret = -EINVAL; break; - }; + } return ret; } @@ -856,7 +856,7 @@ static int fsl_pamu_get_domain_attr(struct iommu_domain *domain, pr_debug(Unsupported attribute type\n); ret = -EINVAL; break; - }; + } return ret; } -- 2.2.2 Acked-by: Varun Sethi varun.se...@freescale.com ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 23/26] iommu/fsl: Use NULL instead of zero
-Original Message- From: Emil Medve [mailto:emilian.me...@freescale.com] Sent: Wednesday, January 28, 2015 8:05 PM To: iommu@lists.linux-foundation.org; j...@8bytes.org; jroe...@suse.de; Sethi Varun-B16395 Cc: Medve Emilian-EMMEDVE1 Subject: [PATCH 23/26] iommu/fsl: Use NULL instead of zero drivers/iommu/fsl_pamu.c:532:72: warning: Using plain integer as NULL pointer drivers/iommu/fsl_pamu.c:559:72: warning: Using plain integer as NULL pointer drivers/iommu/fsl_pamu.c:570:66: warning: Using plain integer as NULL pointer Signed-off-by: Emil Medve emilian.me...@freescale.com --- drivers/iommu/fsl_pamu.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c index 1d9273a..7a4665d 100644 --- a/drivers/iommu/fsl_pamu.c +++ b/drivers/iommu/fsl_pamu.c @@ -529,7 +529,7 @@ u32 get_stash_id(u32 stash_dest_hint, u32 vcpu) if (stash_dest_hint == PAMU_ATTR_CACHE_L3) { node = of_find_matching_node(NULL, l3_device_ids); if (node) { - prop = of_get_property(node, cache-stash-id, 0); + prop = of_get_property(node, cache-stash-id, NULL); if (!prop) { pr_debug(missing cache-stash-id at %s\n, node-full_name); @@ -556,7 +556,7 @@ found_cpu_node: /* find the hwnode that represents the cache */ for (cache_level = PAMU_ATTR_CACHE_L1; (cache_level PAMU_ATTR_CACHE_L3) found; cache_level++) { if (stash_dest_hint == cache_level) { - prop = of_get_property(node, cache-stash-id, 0); + prop = of_get_property(node, cache-stash-id, NULL); if (!prop) { pr_debug(missing cache-stash-id at %s\n, node-full_name); @@ -567,7 +567,7 @@ found_cpu_node: return be32_to_cpup(prop); } - prop = of_get_property(node, next-level-cache, 0); + prop = of_get_property(node, next-level-cache, NULL); if (!prop) { pr_debug(can't find next-level-cache at %s\n, node-full_name); -- 2.2.2 Acked-by: Varun Sethi varun.se...@freescale.com ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 20/26] iommu/fsl: Fix alignment of some stray lines
-Original Message- From: Emil Medve [mailto:emilian.me...@freescale.com] Sent: Wednesday, January 28, 2015 8:05 PM To: iommu@lists.linux-foundation.org; j...@8bytes.org; jroe...@suse.de; Sethi Varun-B16395 Cc: Medve Emilian-EMMEDVE1 Subject: [PATCH 20/26] iommu/fsl: Fix alignment of some stray lines Signed-off-by: Emil Medve emilian.me...@freescale.com --- drivers/iommu/fsl_pamu.h| 2 +- drivers/iommu/fsl_pamu_domain.c | 15 ++- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/drivers/iommu/fsl_pamu.h b/drivers/iommu/fsl_pamu.h index 04dcd25..342ac6d 100644 --- a/drivers/iommu/fsl_pamu.h +++ b/drivers/iommu/fsl_pamu.h @@ -67,7 +67,7 @@ struct pamu_mmap_regs { #define PAMU_AVS1_GCV 0x2000 #define PAMU_AVS1_PDV 0x4000 #define PAMU_AV_MASK(PAMU_AVS1_AV | PAMU_AVS1_OTV | PAMU_AVS1_APV | PAMU_AVS1_WAV \ - | PAMU_AVS1_LAV | PAMU_AVS1_GCV | PAMU_AVS1_PDV) + | PAMU_AVS1_LAV | PAMU_AVS1_GCV | PAMU_AVS1_PDV) #define PAMU_AVS1_LIODN_SHIFT 16 #define PAMU_LAV_LIODN_NOT_IN_PPAACT 0x400 diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c index eea2212..ae21305 100644 --- a/drivers/iommu/fsl_pamu_domain.c +++ b/drivers/iommu/fsl_pamu_domain.c @@ -62,8 +62,7 @@ static int __init iommu_init_mempool(void) static phys_addr_t get_phys_addr(struct fsl_dma_domain *dma_domain, dma_addr_t iova) { u32 win_cnt = dma_domain-win_cnt; - struct dma_window *win_ptr = - dma_domain-win_arr[0]; + struct dma_window *win_ptr = dma_domain-win_arr[0]; struct iommu_domain_geometry *geom; geom = dma_domain-iommu_domain-geometry; @@ -92,15 +91,13 @@ static phys_addr_t get_phys_addr(struct fsl_dma_domain *dma_domain, dma_addr_t i static int map_subwins(int liodn, struct fsl_dma_domain *dma_domain) { - struct dma_window *sub_win_ptr = - dma_domain-win_arr[0]; + struct dma_window *sub_win_ptr = dma_domain-win_arr[0]; int i, ret; unsigned long rpn, flags; for (i = 0; i dma_domain-win_cnt; i++) { if (sub_win_ptr[i].valid) { - rpn = sub_win_ptr[i].paddr - PAMU_PAGE_SHIFT; + rpn = sub_win_ptr[i].paddr PAMU_PAGE_SHIFT; spin_lock_irqsave(iommu_lock, flags); ret = pamu_config_spaace(liodn, dma_domain- win_cnt, i, sub_win_ptr[i].size, @@ -180,8 +177,8 @@ static int update_liodn(int liodn, struct fsl_dma_domain *dma_domain, u32 wnd_nr wnd-size, ~(u32)0, wnd-paddr PAMU_PAGE_SHIFT, - dma_domain-snoop_id, dma_domain-stash_id, - 0, wnd-prot); + dma_domain-snoop_id, dma_domain-stash_id, + 0, wnd-prot); if (ret) pr_debug(Window reconfiguration failed for liodn %d\n, liodn); } @@ -679,7 +676,7 @@ static int fsl_pamu_attach_device(struct iommu_domain *domain, } else { pr_debug(missing fsl,liodn property at %s\n, dev-of_node-full_name); - ret = -EINVAL; + ret = -EINVAL; } return ret; -- 2.2.2 Acked-by: Varun Sethi varun.se...@freescale.com ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 26/26] iommu/fsl: Remove extra paranthesis
-Original Message- From: Emil Medve [mailto:emilian.me...@freescale.com] Sent: Wednesday, January 28, 2015 8:05 PM To: iommu@lists.linux-foundation.org; j...@8bytes.org; jroe...@suse.de; Sethi Varun-B16395 Cc: Medve Emilian-EMMEDVE1 Subject: [PATCH 26/26] iommu/fsl: Remove extra paranthesis Signed-off-by: Emil Medve emilian.me...@freescale.com --- drivers/iommu/fsl_pamu.c| 4 ++-- drivers/iommu/fsl_pamu.h| 4 ++-- drivers/iommu/fsl_pamu_domain.c | 8 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c index 7a4665d..ea49d9f 100644 --- a/drivers/iommu/fsl_pamu.c +++ b/drivers/iommu/fsl_pamu.c @@ -160,7 +160,7 @@ int pamu_disable_liodn(int liodn) static unsigned int map_addrspace_size_to_wse(phys_addr_t addrspace_size) { /* Bug if not a power of 2 */ - BUG_ON((addrspace_size (addrspace_size - 1))); + BUG_ON(addrspace_size (addrspace_size - 1)); /* window size is 2^(WSE+1) bytes */ return fls64(addrspace_size) - 2; @@ -801,7 +801,7 @@ static irqreturn_t pamu_av_isr(int irq, void *arg) } /* clear access violation condition */ - out_be32((p + PAMU_AVS1), avs1 PAMU_AV_MASK); + out_be32(p + PAMU_AVS1, avs1 PAMU_AV_MASK); paace = pamu_get_ppaace(avs1 PAMU_AVS1_LIODN_SHIFT); BUG_ON(!paace); /* check if we got a violation for a disabled LIODN */ diff --git a/drivers/iommu/fsl_pamu.h b/drivers/iommu/fsl_pamu.h index 342ac6d..aab723f 100644 --- a/drivers/iommu/fsl_pamu.h +++ b/drivers/iommu/fsl_pamu.h @@ -26,8 +26,8 @@ /* Bit Field macros * v = bit field variable; m = mask, m##_SHIFT = shift, x = value to load */ -#define set_bf(v, m, x) (v = ((v) ~(m)) | (((x) (m##_SHIFT)) (m))) -#define get_bf(v, m) (((v) (m)) (m##_SHIFT)) +#define set_bf(v, m, x) (v = ((v) ~(m)) | (((x) m##_SHIFT) (m))) +#define get_bf(v, m) (((v) (m)) m##_SHIFT) /* PAMU CCSR space */ #define PAMU_PGC 0x /* Allows all peripheral accesses */ diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c index 36622e2..ceebd28 100644 --- a/drivers/iommu/fsl_pamu_domain.c +++ b/drivers/iommu/fsl_pamu_domain.c @@ -84,7 +84,7 @@ static phys_addr_t get_phys_addr(struct fsl_dma_domain *dma_domain, dma_addr_t i } if (win_ptr-valid) - return (win_ptr-paddr + (iova (win_ptr-size - 1))); + return win_ptr-paddr + (iova (win_ptr-size - 1)); return 0; } @@ -386,8 +386,8 @@ static phys_addr_t fsl_pamu_iova_to_phys(struct iommu_domain *domain, { struct fsl_dma_domain *dma_domain = domain-priv; - if ((iova domain-geometry.aperture_start) || - iova (domain-geometry.aperture_end)) + if (iova domain-geometry.aperture_start || + iova domain-geometry.aperture_end) return 0; return get_phys_addr(dma_domain, iova); @@ -1029,7 +1029,7 @@ static int fsl_pamu_set_windows(struct iommu_domain *domain, u32 w_count) } ret = pamu_set_domain_geometry(dma_domain, domain- geometry, -((w_count 1) ? w_count : 0)); +w_count 1 ? w_count : 0); if (!ret) { kfree(dma_domain-win_arr); dma_domain-win_arr = kcalloc(w_count, -- 2.2.2 Acked-by: Varun Sethi varun.se...@freescale.com ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 19/26] iommu/fsl: Fix some comments alignment
; - /* If PCI controller version is = 0x204 we can partition endpoints*/ + /* If PCI controller version is = 0x204 we can partition endpoints */ if (version = 0x204) return 1; -- 2.2.2 Acked-by: Varun Sethi varun.se...@freescale.com ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 25/26] iommu/fsl: Don't use integers values with bool type
-Original Message- From: Emil Medve [mailto:emilian.me...@freescale.com] Sent: Wednesday, January 28, 2015 8:05 PM To: iommu@lists.linux-foundation.org; j...@8bytes.org; jroe...@suse.de; Sethi Varun-B16395 Cc: Medve Emilian-EMMEDVE1 Subject: [PATCH 25/26] iommu/fsl: Don't use integers values with bool type drivers/iommu/fsl_pamu_domain.c:884:9-10: WARNING: return of 0/1 in function 'check_pci_ctl_endpt_part' with return type bool Signed-off-by: Emil Medve emilian.me...@freescale.com --- drivers/iommu/fsl_pamu_domain.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c index ebbc396..36622e2 100644 --- a/drivers/iommu/fsl_pamu_domain.c +++ b/drivers/iommu/fsl_pamu_domain.c @@ -880,10 +880,7 @@ static bool check_pci_ctl_endpt_part(struct pci_controller *pci_ctl) version = in_be32(pci_ctl-cfg_addr + (PCI_FSL_BRR1 2)); version = PCI_FSL_BRR1_VER; /* If PCI controller version is = 0x204 we can partition endpoints */ - if (version = 0x204) - return 1; - - return 0; + return version = 0x204; } /* Get iommu group information from peer devices or devices on the parent bus */ -- 2.2.2 Acked-by: Varun Sethi varun.se...@freescale.com ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 11/26] iommu/fsl: Fix checkpatch type OOM_MESSAGE
Hi Emil, -Original Message- From: Emil Medve [mailto:emilian.me...@freescale.com] Sent: Wednesday, January 28, 2015 8:05 PM To: iommu@lists.linux-foundation.org; j...@8bytes.org; jroe...@suse.de; Sethi Varun-B16395 Cc: Medve Emilian-EMMEDVE1 Subject: [PATCH 11/26] iommu/fsl: Fix checkpatch type OOM_MESSAGE WARNING:OOM_MESSAGE: Possible unnecessary 'out of memory' message + if (!data) { + dev_err(pdev-dev, PAMU isr data memory allocation + failed\n); Signed-off-by: Emil Medve emilian.me...@freescale.com --- drivers/iommu/fsl_pamu.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c index 4f1926b..6f9c976 100644 --- a/drivers/iommu/fsl_pamu.c +++ b/drivers/iommu/fsl_pamu.c @@ -1041,7 +1041,6 @@ static int __init fsl_pamu_probe(struct platform_device *pdev) data = kzalloc(sizeof(*data), GFP_KERNEL); if (!data) { - dev_err(pdev-dev, PAMU isr data memory allocation failed\n); ret = -ENOMEM; goto error; } I think this is fine, there are other places as well where we have error prints while setting the ENOMEM as the return code. -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 06/26] iommu/fsl: Fix checkpatch type LEADING_SPACE
-Original Message- From: Emil Medve [mailto:emilian.me...@freescale.com] Sent: Wednesday, January 28, 2015 8:05 PM To: iommu@lists.linux-foundation.org; j...@8bytes.org; jroe...@suse.de; Sethi Varun-B16395 Cc: Medve Emilian-EMMEDVE1 Subject: [PATCH 06/26] iommu/fsl: Fix checkpatch type LEADING_SPACE WARNING:LEADING_SPACE: please, no spaces at the start of a line + return __ffs(subwindow_cnt) - 1;$ WARNING:LEADING_SPACE: please, no spaces at the start of a line +(PAACE_ATM_WINDOW_XLATE | PAACE_ATM_PAGE_XLATE)$ Signed-off-by: Emil Medve emilian.me...@freescale.com --- drivers/iommu/fsl_pamu.c | 4 ++-- drivers/iommu/fsl_pamu.h | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c index df6007c..ccdc5e5 100644 --- a/drivers/iommu/fsl_pamu.c +++ b/drivers/iommu/fsl_pamu.c @@ -169,8 +169,8 @@ static unsigned int map_addrspace_size_to_wse(phys_addr_t addrspace_size) /* Derive the PAACE window count encoding for the subwindow count */ static unsigned int map_subwindow_cnt_to_wce(u32 subwindow_cnt) { - /* window count is 2^(WCE+1) bytes */ - return __ffs(subwindow_cnt) - 1; + /* window count is 2^(WCE+1) bytes */ + return __ffs(subwindow_cnt) - 1; } /* diff --git a/drivers/iommu/fsl_pamu.h b/drivers/iommu/fsl_pamu.h index a7794b5..52a91af 100644 --- a/drivers/iommu/fsl_pamu.h +++ b/drivers/iommu/fsl_pamu.h @@ -200,8 +200,7 @@ struct pamu_mmap_regs { #define PAACE_ATM_NO_XLATE 0x00 #define PAACE_ATM_WINDOW_XLATE 0x01 #define PAACE_ATM_PAGE_XLATE0x02 -#define PAACE_ATM_WIN_PG_XLATE \ -(PAACE_ATM_WINDOW_XLATE | PAACE_ATM_PAGE_XLATE) +#define PAACE_ATM_WIN_PG_XLATE (PAACE_ATM_WINDOW_XLATE | +PAACE_ATM_PAGE_XLATE) #define PAACE_OTM_NO_XLATE 0x00 #define PAACE_OTM_IMMEDIATE 0x01 #define PAACE_OTM_INDEXED 0x02 -- 2.2.2 Acked-by: Varun Sethi varun.se...@freescale.com ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 16/26] iommu/fsl: Use a device pointer to make lines shorter
, - mem_size, csd_port_id); + dev_dbg(dev, creating coherency subdomain at address %pa, size %zu, port id 0x%08x, + ppaact_phys, mem_size, csd_port_id); ret = create_csd(ppaact_phys, mem_size, csd_port_id); if (ret) { - dev_err(pdev-dev, could not create coherence - subdomain\n); + dev_err(dev, could not create coherence subdomain\n); return ret; } } @@ -1138,7 +1135,7 @@ static int __init fsl_pamu_probe(struct platform_device *pdev) spaace_pool = gen_pool_create(ilog2(sizeof(struct paace)), -1); if (!spaace_pool) { ret = -ENOMEM; - dev_err(pdev-dev, PAMU : failed to allocate spaace gen pool\n); + dev_err(dev, PAMU : failed to allocate spaace gen pool\n); goto error; } -- 2.2.2 Acked-by: Varun Sethi varun.se...@freescale.com ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 17/26] iommu/fsl: Remove pr/dev_*() prefixes
-Original Message- From: Emil Medve [mailto:emilian.me...@freescale.com] Sent: Wednesday, January 28, 2015 8:05 PM To: iommu@lists.linux-foundation.org; j...@8bytes.org; jroe...@suse.de; Sethi Varun-B16395 Cc: Medve Emilian-EMMEDVE1 Subject: [PATCH 17/26] iommu/fsl: Remove pr/dev_*() prefixes pr_fmt() and dev_*() already take care of it Signed-off-by: Emil Medve emilian.me...@freescale.com --- drivers/iommu/fsl_pamu.c| 2 +- drivers/iommu/fsl_pamu_domain.c | 12 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c index 22b72e2..e46c75f 100644 --- a/drivers/iommu/fsl_pamu.c +++ b/drivers/iommu/fsl_pamu.c @@ -1135,7 +1135,7 @@ static int __init fsl_pamu_probe(struct platform_device *pdev) spaace_pool = gen_pool_create(ilog2(sizeof(struct paace)), -1); if (!spaace_pool) { ret = -ENOMEM; - dev_err(dev, PAMU : failed to allocate spaace gen pool\n); + dev_err(dev, Failed to allocate spaace gen pool\n); goto error; } diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c index c7af760..3e7954a 100644 --- a/drivers/iommu/fsl_pamu_domain.c +++ b/drivers/iommu/fsl_pamu_domain.c @@ -113,7 +113,7 @@ static int map_subwins(int liodn, struct fsl_dma_domain *dma_domain) sub_win_ptr[i].prot); spin_unlock_irqrestore(iommu_lock, flags); if (ret) { - pr_debug(PAMU SPAACE configuration failed for liodn %d\n, + pr_debug(SPAACE configuration failed for liodn %d\n, liodn); return ret; } @@ -139,7 +139,7 @@ static int map_win(int liodn, struct fsl_dma_domain *dma_domain) 0, wnd-prot); spin_unlock_irqrestore(iommu_lock, flags); if (ret) - pr_debug(PAMU PAACE configuration failed for liodn %d\n, + pr_debug(PAACE configuration failed for liodn %d\n, liodn); return ret; @@ -250,7 +250,7 @@ static int pamu_set_liodn(int liodn, struct device *dev, dma_domain-stash_id, win_cnt, 0); spin_unlock_irqrestore(iommu_lock, flags); if (ret) { - pr_debug(PAMU PAACE configuration failed for liodn %d, win_cnt =%d\n, liodn, win_cnt); + pr_debug(PAACE configuration failed for liodn %d, win_cnt =%d\n, +liodn, win_cnt); return ret; } @@ -267,7 +267,7 @@ static int pamu_set_liodn(int liodn, struct device *dev, 0, 0); spin_unlock_irqrestore(iommu_lock, flags); if (ret) { - pr_debug(PAMU SPAACE configuration failed for liodn %d\n, liodn); + pr_debug(SPAACE configuration failed for liodn %d\n, liodn); return ret; } } @@ -283,13 +283,13 @@ static int check_size(u64 size, dma_addr_t iova) * to PAMU page size. */ if ((size (size - 1)) || size PAMU_PAGE_SIZE) { - pr_debug(%s: size too small or not a power of two\n, __func__); + pr_debug(Size too small or not a power of two\n); return -EINVAL; } /* iova must be page size aligned*/ if (iova (size - 1)) { - pr_debug(%s: address is not aligned with window size\n, __func__); + pr_debug(Address is not aligned with window size\n); return -EINVAL; } -- 2.2.2 Acked-by: Varun Sethi varun.se...@freescale.com ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH] iommu/fsl: Fix section mismatch
Hi Emil, Thanks for pointing this out. Please find my comment inline. Regards Varun -Original Message- From: Emil Medve [mailto:emilian.me...@freescale.com] Sent: Thursday, January 22, 2015 3:36 AM To: iommu@lists.linux-foundation.org; j...@8bytes.org; jroe...@suse.de; Sethi Varun-B16395 Cc: Medve Emilian-EMMEDVE1 Subject: [PATCH] iommu/fsl: Fix section mismatch Section mismatch in reference from the variable fsl_of_pamu_driver to the function .init.text:fsl_pamu_probe() The variable fsl_of_pamu_driver references the function __init fsl_pamu_probe() If the reference is valid then annotate the variable with __init* or __refdata (see linux/init.h) or name the variable: *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console Signed-off-by: Emil Medve emilian.me...@freescale.com --- drivers/iommu/fsl_pamu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c index 80ac68d..076a951 100644 --- a/drivers/iommu/fsl_pamu.c +++ b/drivers/iommu/fsl_pamu.c @@ -1016,7 +1016,7 @@ static const struct { #define SVR_SECURITY 0x8 /* The Security (E) bit */ -static int __init fsl_pamu_probe(struct platform_device *pdev) +static int fsl_pamu_probe(struct platform_device *pdev) { void __iomem *pamu_regs = NULL; struct ccsr_guts __iomem *guts_regs = NULL; -- 2.2.2 Fsl_pamu_probe is expected to be called during system initialization in order to initialize PAMU before other devices are initialized. We should make fsl_of_pamu_driver as __initdata. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v2 1/3] iommu/fsl: Fix section mismatch
-Original Message- From: Emil Medve [mailto:emilian.me...@freescale.com] Sent: Thursday, January 22, 2015 8:17 PM To: iommu@lists.linux-foundation.org; j...@8bytes.org; jroe...@suse.de; Sethi Varun-B16395 Cc: Medve Emilian-EMMEDVE1 Subject: [PATCH v2 1/3] iommu/fsl: Fix section mismatch Section mismatch in reference from the variable fsl_of_pamu_driver to the function .init.text:fsl_pamu_probe() The variable fsl_of_pamu_driver references the function __init fsl_pamu_probe() If the reference is valid then annotate the variable with __init* or __refdata (see linux/init.h) or name the variable: *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console Signed-off-by: Emil Medve emilian.me...@freescale.com --- v2: Add an __init[data] instead of removing one drivers/iommu/fsl_pamu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c index 80ac68d..d478ad8 100644 --- a/drivers/iommu/fsl_pamu.c +++ b/drivers/iommu/fsl_pamu.c @@ -1224,7 +1224,7 @@ static const struct of_device_id fsl_of_pamu_ids[] = { {}, }; -static struct platform_driver fsl_of_pamu_driver = { +static struct platform_driver fsl_of_pamu_driver __initdata = { .driver = { .name = fsl-of-pamu, }, -- 2.2.2 Acked-by: Varun Sethi varun.se...@freescale.com ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 2/3] iommu/fsl: Remove unused fsl_of_pamu_ids[]
-Original Message- From: Emil Medve [mailto:emilian.me...@freescale.com] Sent: Thursday, January 22, 2015 8:17 PM To: iommu@lists.linux-foundation.org; j...@8bytes.org; jroe...@suse.de; Sethi Varun-B16395 Cc: Medve Emilian-EMMEDVE1 Subject: [PATCH 2/3] iommu/fsl: Remove unused fsl_of_pamu_ids[] Signed-off-by: Emil Medve emilian.me...@freescale.com --- drivers/iommu/fsl_pamu.c | 10 -- 1 file changed, 10 deletions(-) diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c index d478ad8..113fb7b 100644 --- a/drivers/iommu/fsl_pamu.c +++ b/drivers/iommu/fsl_pamu.c @@ -1214,16 +1214,6 @@ error: return ret; } -static const struct of_device_id fsl_of_pamu_ids[] = { - { - .compatible = fsl,p4080-pamu, - }, - { - .compatible = fsl,pamu, - }, - {}, -}; - static struct platform_driver fsl_of_pamu_driver __initdata = { .driver = { .name = fsl-of-pamu, -- 2.2.2 Acked-by: Varun Sethi varun.se...@freescale.com ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 3/3] iommu/fsl: Sprinkle some __init* annotations
Thanks Emil, please find my comments inline. Regards Varun -Original Message- From: Emil Medve [mailto:emilian.me...@freescale.com] Sent: Thursday, January 22, 2015 8:17 PM To: iommu@lists.linux-foundation.org; j...@8bytes.org; jroe...@suse.de; Sethi Varun-B16395 Cc: Medve Emilian-EMMEDVE1 Subject: [PATCH 3/3] iommu/fsl: Sprinkle some __init* annotations Signed-off-by: Emil Medve emilian.me...@freescale.com --- drivers/iommu/fsl_pamu.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c index 113fb7b..db9c1eb 100644 --- a/drivers/iommu/fsl_pamu.c +++ b/drivers/iommu/fsl_pamu.c @@ -59,7 +59,7 @@ static struct ome *omt; * SOCs. For the older SOCs fsl,qoriq-device-config-1.0 * string would be used. */ -static const struct of_device_id guts_device_ids[] = { +static const struct of_device_id guts_device_ids[] __initconst = { { .compatible = fsl,qoriq-device-config-1.0, }, { .compatible = fsl,qoriq-device-config-2.0, }, {} @@ -187,7 +187,7 @@ static unsigned int map_subwindow_cnt_to_wce(u32 subwindow_cnt) * Set the PAACE type as primary and set the coherency required domain * attribute */ -static void pamu_init_ppaace(struct paace *ppaace) +static void __init pamu_init_ppaace(struct paace *ppaace) { [varun] This shouldn't be in the init section. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 2/2] iommu/arm-smmu: remove homebrew PCI dma alias parsing
Hi Will, Please find my query inline. Regards Varun -Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Will Deacon Sent: Friday, January 16, 2015 10:29 PM To: alex.william...@redhat.com Cc: m-kariche...@ti.com; Will Deacon; iommu@lists.linux-foundation.org; a...@arndb.de Subject: [PATCH 2/2] iommu/arm-smmu: remove homebrew PCI dma alias parsing Core code can walk the PCI topology and extract the DMA alias for us whilst retrieving the IOMMU group for a device, so use that instead. Signed-off-by: Will Deacon will.dea...@arm.com --- drivers/iommu/arm-smmu.c | 58 ++--- --- 1 file changed, 31 insertions(+), 27 deletions(-) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 006f006c35e9..779c248a140d 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -1255,12 +1255,6 @@ static bool arm_smmu_capable(enum iommu_cap cap) } } -static int __arm_smmu_get_pci_sid(struct pci_dev *pdev, u16 alias, void *data) -{ - *((u16 *)data) = alias; - return 0; /* Continue walking */ -} - static void __arm_smmu_release_pci_iommudata(void *data) { kfree(data); @@ -1269,56 +1263,66 @@ static void __arm_smmu_release_pci_iommudata(void *data) static int arm_smmu_add_device(struct device *dev) { struct arm_smmu_device *smmu; - struct arm_smmu_master_cfg *cfg; struct iommu_group *group; - void (*releasefn)(void *) = NULL; int ret; smmu = find_smmu_for_device(dev); if (!smmu) return -ENODEV; - group = iommu_group_alloc(); - if (IS_ERR(group)) { - dev_err(dev, Failed to allocate IOMMU group\n); - return PTR_ERR(group); - } - if (dev_is_pci(dev)) { + u16 sid; + struct arm_smmu_master_cfg *cfg; struct pci_dev *pdev = to_pci_dev(dev); + void (*releasefn)(void *) = __arm_smmu_release_pci_iommudata; - cfg = kzalloc(sizeof(*cfg), GFP_KERNEL); + group = iommu_group_get_for_pci_dev(pdev, sid); + if (IS_ERR(group)) + goto out_no_group; + + cfg = iommu_group_get_iommudata(group); if (!cfg) { - ret = -ENOMEM; + cfg = kzalloc(sizeof(*cfg), GFP_KERNEL); + if (!cfg) { + ret = -ENOMEM; + goto out_put_group; + } + + iommu_group_set_iommudata(group, cfg, releasefn); + } + + if (cfg-num_streamids = MAX_MASTER_STREAMIDS) { + ret = -ENOSPC; goto out_put_group; } - cfg-num_streamids = 1; /* * Assume Stream ID == Requester ID for now. * We need a way to describe the ID mappings in FDT. */ - pci_for_each_dma_alias(pdev, __arm_smmu_get_pci_sid, -cfg-streamids[0]); - releasefn = __arm_smmu_release_pci_iommudata; + cfg-streamids[cfg-num_streamids++] = sid; [varun] Couldn't there be an issue, when a single stream id is shared by multiple devices (in case of non transparent bridge). Wouldn't that be an issue here? When devices are attached to the domain SMR would be allocated per stream id, this could lead to duplicate stream ids. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 2/4] iommu: add ARM LPAE page table allocator
Hi Will, -Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Monday, December 15, 2014 9:13 PM To: Sethi Varun-B16395 Cc: linux-arm-ker...@lists.infradead.org; iommu@lists.linux-foundation.org; prem.malla...@broadcom.com; Robin Murphy; lau...@codeaurora.org; mitch...@codeaurora.org; laurent.pinch...@ideasonboard.com; j...@8bytes.org; m.szyprow...@samsung.com Subject: Re: [PATCH 2/4] iommu: add ARM LPAE page table allocator On Mon, Dec 15, 2014 at 01:30:20PM +, Will Deacon wrote: On Sun, Dec 14, 2014 at 05:45:49PM +, Varun Sethi wrote: [varun] ok, but you could potentially end up splitting mapping to the least possible page size e.g. 4K. You, don't seem to take in to account the possibility of using the block size at the next level. For example, take a case where we have a huge page mapping using 1G page size and we have an unmap request for 4K. We could still split maximum part of the mapping using 2M pages at the next level. The entry where we need to unmap the 4K region would potentially go to the next level. Aha, I see what you mean here, thanks. I'll take a look... Scratch that, I think the code is fine as it is. For the case you highlight, we iterate over the 1GB region remapping it using 4k pages, but skipping the one we want to unmap, so I don't think there's a problem (__arm_lpae_map will create the relevant table entries). [[varun]] But you can split 1G in 2M mappings and then split up the unmapped region using 4K pages. In this case you split up the entire region using 4K pages. Thanks, Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 2/4] iommu: add ARM LPAE page table allocator
Hi Will, Please find my response inline. Search for varun. -Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Saturday, December 06, 2014 12:18 AM To: Sethi Varun-B16395 Cc: linux-arm-ker...@lists.infradead.org; iommu@lists.linux-foundation.org; prem.malla...@broadcom.com; Robin Murphy; lau...@codeaurora.org; mitch...@codeaurora.org; laurent.pinch...@ideasonboard.com; j...@8bytes.org; m.szyprow...@samsung.com Subject: Re: [PATCH 2/4] iommu: add ARM LPAE page table allocator On Fri, Dec 05, 2014 at 10:55:11AM +, Varun Sethi wrote: Hi Will, Hi Varun, Thanks for the review! +static int arm_lpae_init_pte(struct arm_lpae_io_pgtable *data, +unsigned long iova, phys_addr_t paddr, +arm_lpae_iopte prot, int lvl, +arm_lpae_iopte *ptep) { + arm_lpae_iopte pte = prot; + + /* We require an unmap first */ + if (iopte_leaf(*ptep, lvl)) + return -EEXIST; [varun] Instead of returning an error, how about displaying a warning and replacing the entry? I'd be ok with displaying a warning, but I don't think we should just continue. It indicates a misuse of the IOMMU API and probably a missing TLBI. [[varun]] May not apply now, but what if we are dealing with a case where memory is not pinned? It may be possible to hookup (without an unmap) an iova to a different physical address. Offcourse, tlb invalidation would be required. Could this scenario be relevant in case of stall mode? +static int __arm_lpae_map(struct arm_lpae_io_pgtable *data, unsigned long iova, + phys_addr_t paddr, size_t size, arm_lpae_iopte prot, + int lvl, arm_lpae_iopte *ptep) { + arm_lpae_iopte *cptep, pte; + void *cookie = data-iop.cookie; + size_t block_size = ARM_LPAE_BLOCK_SIZE(lvl, data); + + /* Find our entry at the current level */ + ptep += ARM_LPAE_LVL_IDX(iova, lvl, data); + + /* If we can install a leaf entry at this level, then do so */ + if (size == block_size (size data-iop.cfg.pgsize_bitmap)) + return arm_lpae_init_pte(data, iova, paddr, prot, lvl, + ptep); + + /* We can't allocate tables at the final level */ + if (WARN_ON(lvl = ARM_LPAE_MAX_LEVELS - 1)) + return -EINVAL; [varun] A warning message would be helpful. Sure, I can add one. +static arm_lpae_iopte arm_lpae_prot_to_pte(struct arm_lpae_io_pgtable *data, + int prot) { + arm_lpae_iopte pte; + + if (data-iop.fmt == ARM_LPAE_S1) { + pte = ARM_LPAE_PTE_AP_UNPRIV | ARM_LPAE_PTE_nG; + + if (!(prot IOMMU_WRITE) (prot IOMMU_READ)) + pte |= ARM_LPAE_PTE_AP_RDONLY; + + if (prot IOMMU_CACHE) + pte |= (ARM_LPAE_MAIR_ATTR_IDX_CACHE +ARM_LPAE_PTE_ATTRINDX_SHIFT); + } else { + pte = ARM_LPAE_PTE_HAP_FAULT; + if (prot IOMMU_READ) + pte |= ARM_LPAE_PTE_HAP_READ; + if (prot IOMMU_WRITE) + pte |= ARM_LPAE_PTE_HAP_WRITE; + if (prot IOMMU_CACHE) + pte |= ARM_LPAE_PTE_MEMATTR_OIWB; + else + pte |= ARM_LPAE_PTE_MEMATTR_NC; + } + + if (prot IOMMU_NOEXEC) + pte |= ARM_LPAE_PTE_XN; + + return pte; +} [[varun]] Do you plan to add a flag to indicate device memory? We had a discussion about this on the patch submitted by me. May be you can include that as a part of this patch. That needs to go in as a separate patch. I think you should continue to push that separately! +static int arm_lpae_split_blk_unmap(struct arm_lpae_io_pgtable *data, + unsigned long iova, size_t size, + arm_lpae_iopte prot, int lvl, + arm_lpae_iopte *ptep, size_t blk_size) { + unsigned long blk_start, blk_end; + phys_addr_t blk_paddr; + arm_lpae_iopte table = 0; + void *cookie = data-iop.cookie; + struct iommu_gather_ops *tlb = data-iop.cfg.tlb; + + blk_start = iova ~(blk_size - 1); + blk_end = blk_start + blk_size; + blk_paddr = iopte_to_pfn(*ptep, data) data-pg_shift; + + for (; blk_start blk_end; blk_start += size, blk_paddr += size) { + arm_lpae_iopte *tablep; + + /* Unmap! */ + if (blk_start == iova) + continue; + + /* __arm_lpae_map expects a pointer to the start of the table */ + tablep = table - ARM_LPAE_LVL_IDX(blk_start, lvl, + data); [[varun]] Not clear what's happening here. May be I am missing something
RE: [PATCH 2/4] iommu: add ARM LPAE page table allocator
Hi Will, Please find my comments inline. Search for varun -Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Thursday, November 27, 2014 5:21 PM To: linux-arm-ker...@lists.infradead.org; iommu@lists.linux-foundation.org Cc: prem.malla...@broadcom.com; robin.mur...@arm.com; lau...@codeaurora.org; mitch...@codeaurora.org; laurent.pinch...@ideasonboard.com; j...@8bytes.org; Sethi Varun-B16395; m.szyprow...@samsung.com; Will Deacon Subject: [PATCH 2/4] iommu: add ARM LPAE page table allocator A number of IOMMUs found in ARM SoCs can walk architecture-compatible page tables. This patch adds a generic allocator for Stage-1 and Stage-2 v7/v8 long-descriptor page tables. 4k, 16k and 64k pages are supported, with up to 4-levels of walk to cover a 48-bit address space. Signed-off-by: Will Deacon will.dea...@arm.com --- MAINTAINERS| 1 + drivers/iommu/Kconfig | 9 + drivers/iommu/Makefile | 1 + drivers/iommu/io-pgtable-arm.c | 735 + drivers/iommu/io-pgtable.c | 7 + drivers/iommu/io-pgtable.h | 12 + 6 files changed, 765 insertions(+) create mode 100644 drivers/iommu/io-pgtable-arm.c diff --git a/MAINTAINERS b/MAINTAINERS index 0ff630de8a6d..d3ca31b7c960 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1562,6 +1562,7 @@ M:Will Deacon will.dea...@arm.com L: linux-arm-ker...@lists.infradead.org (moderated for non-subscribers) S: Maintained F: drivers/iommu/arm-smmu.c +F: drivers/iommu/io-pgtable-arm.c ARM64 PORT (AARCH64 ARCHITECTURE) M: Catalin Marinas catalin.mari...@arm.com diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig index 0f10554e7114..e1742a0146f8 100644 --- a/drivers/iommu/Kconfig +++ b/drivers/iommu/Kconfig @@ -19,6 +19,15 @@ menu Generic IOMMU Pagetable Support config IOMMU_IO_PGTABLE bool +config IOMMU_IO_PGTABLE_LPAE + bool ARMv7/v8 Long Descriptor Format + select IOMMU_IO_PGTABLE + help + Enable support for the ARM long descriptor pagetable format. + This allocator supports 4K/2M/1G, 16K/32M and 64K/512M page + sizes at both stage-1 and stage-2, as well as address spaces + up to 48-bits in size. + endmenu config OF_IOMMU diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile index aff244c78181..269cdd82b672 100644 --- a/drivers/iommu/Makefile +++ b/drivers/iommu/Makefile @@ -2,6 +2,7 @@ obj-$(CONFIG_IOMMU_API) += iommu.o obj-$(CONFIG_IOMMU_API) += iommu-traces.o obj-$(CONFIG_IOMMU_API) += iommu-sysfs.o obj-$(CONFIG_IOMMU_IO_PGTABLE) += io-pgtable.o +obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE) += io-pgtable-arm.o obj-$(CONFIG_OF_IOMMU) += of_iommu.o obj-$(CONFIG_MSM_IOMMU) += msm_iommu.o msm_iommu_dev.o obj-$(CONFIG_AMD_IOMMU) += amd_iommu.o amd_iommu_init.o diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c new file mode 100644 index ..9dbaa2e48424 --- /dev/null +++ b/drivers/iommu/io-pgtable-arm.c @@ -0,0 +1,735 @@ +/* + * CPU-agnostic ARM page table allocator. + * + * 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. + * + * 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. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + * + * Copyright (C) 2014 ARM Limited + * + * Author: Will Deacon will.dea...@arm.com */ + +#define pr_fmt(fmt)arm-lpae io-pgtable: fmt + +#include linux/iommu.h +#include linux/kernel.h +#include linux/sizes.h +#include linux/slab.h +#include linux/types.h + +#include io-pgtable.h + +#define ARM_LPAE_MAX_ADDR_BITS 48 +#define ARM_LPAE_S2_MAX_CONCAT_PAGES 16 +#define ARM_LPAE_MAX_LEVELS4 + +/* Struct accessors */ +#define io_pgtable_to_data(x) \ + container_of((x), struct arm_lpae_io_pgtable, iop) + +#define io_pgtable_ops_to_pgtable(x) \ + container_of((x), struct io_pgtable, ops) + +#define io_pgtable_ops_to_data(x) \ + io_pgtable_to_data(io_pgtable_ops_to_pgtable(x)) + +/* + * For consistency with the architecture, we always consider + * ARM_LPAE_MAX_LEVELS levels, with the walk starting at level n =0 +*/ +#define ARM_LPAE_START_LVL(d) (ARM_LPAE_MAX_LEVELS - (d)-levels) + +/* + * Calculate the right shift amount to get to the portion describing +level l + * in a virtual address mapped by the pagetable in d. + */ +#define ARM_LPAE_LVL_SHIFT(l,d) \ +
[RFC][PATCH] iommu/arm-smmu: Huge page mapping support for ARM SMMU driver.
This patch adds huge page mapping support for the ARM SMMU Driver. Patch allows creation of 1G and 2MB page mappings. It's also possible to create contiguous huge page mappings. The loop in PMD/PUD intialization code has been removed, considering that iommu_map would work on page size granularity (exported by the arm smmu driver). The code doesn't support creation of new mapping, without unmaping an existing mapping. Code does support splitting up of huge page mappings in to smaller mappings, as a part of unmap. Signed-off-by: Varun Sethi varun.se...@freescale.com --- drivers/iommu/arm-smmu.c | 324 -- 1 file changed, 257 insertions(+), 67 deletions(-) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 60558f7..cf05a1f 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -81,16 +81,22 @@ #define ARM_SMMU_PTE_PAGE (((pteval_t)3) 0) #if PAGE_SIZE == SZ_4K -#define ARM_SMMU_PTE_CONT_ENTRIES 16 +#define ARM_SMMU_PGTBL_CONT_ENTRIES16 #elif PAGE_SIZE == SZ_64K -#define ARM_SMMU_PTE_CONT_ENTRIES 32 +#define ARM_SMMU_PGTBL_CONT_ENTRIES32 #else -#define ARM_SMMU_PTE_CONT_ENTRIES 1 +#define ARM_SMMU_PGTBL_CONT_ENTRIES1 #endif -#define ARM_SMMU_PTE_CONT_SIZE (PAGE_SIZE * ARM_SMMU_PTE_CONT_ENTRIES) +#define ARM_SMMU_PTE_CONT_SIZE (PAGE_SIZE * ARM_SMMU_PGTBL_CONT_ENTRIES) #define ARM_SMMU_PTE_CONT_MASK (~(ARM_SMMU_PTE_CONT_SIZE - 1)) +#define ARM_SMMU_PMD_CONT_SIZE (PMD_SIZE * ARM_SMMU_PGTBL_CONT_ENTRIES) +#define ARM_SMMU_PMD_CONT_MASK (~(ARM_SMMU_PMD_CONT_SIZE - 1)) + +#define ARM_SMMU_PUD_CONT_SIZE (PUD_SIZE * ARM_SMMU_PGTBL_CONT_ENTRIES) +#define ARM_SMMU_PUD_CONT_MASK (~(ARM_SMMU_PUD_CONT_SIZE - 1)) + /* Stage-1 PTE */ #define ARM_SMMU_PTE_AP_UNPRIV (((pteval_t)1) 6) #define ARM_SMMU_PTE_AP_RDONLY (((pteval_t)2) 6) @@ -1272,64 +1278,105 @@ static void arm_smmu_detach_dev(struct iommu_domain *domain, struct device *dev) static bool arm_smmu_pte_is_contiguous_range(unsigned long addr, unsigned long end) { - return !(addr ~ARM_SMMU_PTE_CONT_MASK) + return IS_ALIGNED(addr, ARM_SMMU_PTE_CONT_SIZE) (addr + ARM_SMMU_PTE_CONT_SIZE = end); } -static int arm_smmu_alloc_init_pte(struct arm_smmu_device *smmu, pmd_t *pmd, - unsigned long addr, unsigned long end, - unsigned long pfn, int prot, int stage) +static bool arm_smmu_pmd_is_contiguous_range(unsigned long addr, +unsigned long end) { - pte_t *pte, *start; - pteval_t pteval = ARM_SMMU_PTE_PAGE | ARM_SMMU_PTE_AF | ARM_SMMU_PTE_XN; - - if (pmd_none(*pmd)) { - /* Allocate a new set of tables */ - pgtable_t table = alloc_page(GFP_ATOMIC|__GFP_ZERO); + return IS_ALIGNED(addr, ARM_SMMU_PMD_CONT_SIZE) + (addr + ARM_SMMU_PMD_CONT_SIZE = end); +} - if (!table) - return -ENOMEM; +static bool arm_smmu_pud_is_contiguous_range(unsigned long addr, +unsigned long end) +{ + return IS_ALIGNED(addr, ARM_SMMU_PUD_CONT_SIZE) + (addr + ARM_SMMU_PUD_CONT_SIZE = end); +} - arm_smmu_flush_pgtable(smmu, page_address(table), PAGE_SIZE); - pmd_populate(NULL, pmd, table); - arm_smmu_flush_pgtable(smmu, pmd, sizeof(*pmd)); - } +static pteval_t arm_smmu_page_prot(int prot, int stage, u64 type) +{ + pteval_t pgprot = ARM_SMMU_PTE_AF | ARM_SMMU_PTE_XN | type; if (stage == 1) { - pteval |= ARM_SMMU_PTE_AP_UNPRIV | ARM_SMMU_PTE_nG; + pgprot |= ARM_SMMU_PTE_AP_UNPRIV | ARM_SMMU_PTE_nG; if (!(prot IOMMU_WRITE) (prot IOMMU_READ)) - pteval |= ARM_SMMU_PTE_AP_RDONLY; + pgprot |= ARM_SMMU_PTE_AP_RDONLY; if (prot IOMMU_CACHE) - pteval |= (MAIR_ATTR_IDX_CACHE + pgprot |= (MAIR_ATTR_IDX_CACHE ARM_SMMU_PTE_ATTRINDX_SHIFT); } else { - pteval |= ARM_SMMU_PTE_HAP_FAULT; + pgprot |= ARM_SMMU_PTE_HAP_FAULT; if (prot IOMMU_READ) - pteval |= ARM_SMMU_PTE_HAP_READ; + pgprot |= ARM_SMMU_PTE_HAP_READ; if (prot IOMMU_WRITE) - pteval |= ARM_SMMU_PTE_HAP_WRITE; + pgprot |= ARM_SMMU_PTE_HAP_WRITE; if (prot IOMMU_CACHE) - pteval |= ARM_SMMU_PTE_MEMATTR_OIWB; + pgprot |= ARM_SMMU_PTE_MEMATTR_OIWB; else - pteval |= ARM_SMMU_PTE_MEMATTR_NC
[RFC][PATCH 2/2] Add support of the IOMMU_DEVICE flag.
This flag is used for specifying access to device memory. SMMU would apply device memory attributes for a DMA transaction. This is required for setting access to GIC registers, for generating message interrupts. This would ensure that transactions targetting device memory are not gathered or reordered. Signed-off-by: Varun Sethi varun.se...@freescale.com --- drivers/iommu/arm-smmu.c |7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index ca18d6d..f8338d6 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -1263,6 +1263,10 @@ static int arm_smmu_alloc_init_pte(struct arm_smmu_device *smmu, pmd_t *pmd, if (prot IOMMU_CACHE) pteval |= (MAIR_ATTR_IDX_CACHE ARM_SMMU_PTE_ATTRINDX_SHIFT); + + if (prot IOMMU_DEVICE) + pteval |= (MAIR_ATTR_IDX_DEV + ARM_SMMU_PTE_ATTRINDX_SHIFT); } else { pteval |= ARM_SMMU_PTE_HAP_FAULT; if (prot IOMMU_READ) @@ -1273,6 +1277,9 @@ static int arm_smmu_alloc_init_pte(struct arm_smmu_device *smmu, pmd_t *pmd, pteval |= ARM_SMMU_PTE_MEMATTR_OIWB; else pteval |= ARM_SMMU_PTE_MEMATTR_NC; + + if (prot IOMMU_DEVICE) + pteval |= ARM_SMMU_PTE_MEMATTR_DEV; } /* If no access, create a faulting entry to avoid TLB fills */ -- 1.7.9.5 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC][PATCH 1/2] Introduce the IOMMU_DEVICE flag.
This is used for indicating device memory type for a DMA transaction. IOMMU driver would set up attributes indicationg access to device memory. Signed-off-by: Varun Sethi varun.se...@freescale.com --- include/linux/iommu.h |1 + 1 file changed, 1 insertion(+) diff --git a/include/linux/iommu.h b/include/linux/iommu.h index 20f9a52..0599fe1 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -28,6 +28,7 @@ #define IOMMU_WRITE(1 1) #define IOMMU_CACHE(1 2) /* DMA cache coherency */ #define IOMMU_EXEC (1 3) +#define IOMMU_DEVICE (1 4) /* Indicates access to device memory */ struct iommu_ops; struct iommu_group; -- 1.7.9.5 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [RFC][PATCH 2/2] Add support of the IOMMU_DEVICE flag.
-Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Monday, October 06, 2014 4:47 PM To: Sethi Varun-B16395 Cc: iommu@lists.linux-foundation.org Subject: Re: [RFC][PATCH 2/2] Add support of the IOMMU_DEVICE flag. On Mon, Oct 06, 2014 at 11:28:16AM +0100, Varun Sethi wrote: This flag is used for specifying access to device memory. SMMU would apply device memory attributes for a DMA transaction. This is required for setting access to GIC registers, for generating message interrupts. This would ensure that transactions targetting device memory are not gathered or reordered. Signed-off-by: Varun Sethi varun.se...@freescale.com --- drivers/iommu/arm-smmu.c |7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index ca18d6d..f8338d6 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -1263,6 +1263,10 @@ static int arm_smmu_alloc_init_pte(struct arm_smmu_device *smmu, pmd_t *pmd, if (prot IOMMU_CACHE) pteval |= (MAIR_ATTR_IDX_CACHE ARM_SMMU_PTE_ATTRINDX_SHIFT); + + if (prot IOMMU_DEVICE) + pteval |= (MAIR_ATTR_IDX_DEV + ARM_SMMU_PTE_ATTRINDX_SHIFT); This is slightly odd, as I could in theory pass IOMMU_CACHE | IOMMU_DEVICE to iommu_map. Agreed, this should only be relevant for the non-cacheable case. -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [RFC][PATCH 1/2] Introduce the IOMMU_DEVICE flag.
Hi Will, -Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Monday, October 06, 2014 4:34 PM To: Sethi Varun-B16395 Cc: iommu@lists.linux-foundation.org; mitch...@codeaurora.org; ohau...@codeaurora.org Subject: Re: [RFC][PATCH 1/2] Introduce the IOMMU_DEVICE flag. Hi Varun, [adding the Qualcomm guys, as I have an open question below] On Mon, Oct 06, 2014 at 11:28:15AM +0100, Varun Sethi wrote: This is used for indicating device memory type for a DMA transaction. IOMMU driver would set up attributes indicationg access to device memory. Signed-off-by: Varun Sethi varun.se...@freescale.com --- include/linux/iommu.h |1 + 1 file changed, 1 insertion(+) diff --git a/include/linux/iommu.h b/include/linux/iommu.h index 20f9a52..0599fe1 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -28,6 +28,7 @@ #define IOMMU_WRITE(1 1) #define IOMMU_CACHE(1 2) /* DMA cache coherency */ #define IOMMU_EXEC (1 3) +#define IOMMU_DEVICE (1 4) /* Indicates access to device memory */ An alternative to this would be to make device-memory the default type for !IOMMU_CACHE mappings (i.e. MAIR index 0). I'd value feedback either way; the argument comes down to whether we should use normal non-cacheable or device-nGnRE as the default (!IOMMU_CACHE) memory type. The latter is likely to be significantly slower, but provides the ordering guarantees that you need for MSI delivery. If we do go down the route of adding a new IOMMU_* option, it probably needs a better name (IOMMU_MMIO, for example). I think it would be better to have a separate flag for handling DMA access to device memory. There might be cases where we might want DMA accesses to be non cacheable. This may be required for preventing cache evictions (depending on the cache size). -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH] iommu/arm-smmu: fix bug in pmd construction
Hi Mitchel, I have made changes to the arm smmu driver paging support, as a part of the huge page support patch. Will is currently reviewing changes made by me. I should be able to post my patch in the next couple of weeks. Regards Vaun -Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Mitchel Humpherys Sent: Saturday, September 20, 2014 3:29 AM To: linux-arm-ker...@lists.infradead.org; iommu@lists.linux-foundation.org; Will Deacon Subject: [PATCH] iommu/arm-smmu: fix bug in pmd construction We are using the same pfn for every pte we create while constructing the pmd. Fix this by actually updating the pfn on each iteration of the pmd construction loop. It's not clear if we can actually hit this bug right now since iommu_map splits up the calls to .map based on the page size, so we only ever seem to iterate this loop once. However, things might change in the future that might cause us to hit this. Signed-off-by: Mitchel Humpherys mitch...@codeaurora.org --- Will, I was unable to come up with a test case to hit this bug based on what I said in the commit message above. Not sure if my analysis is completely off base, my head is still spinning from all these page tables :). --- drivers/iommu/arm-smmu.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index ca18d6d42a..eba4cb390c 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -1368,6 +1368,7 @@ static int arm_smmu_alloc_init_pmd(struct arm_smmu_device *smmu, pud_t *pud, ret = arm_smmu_alloc_init_pte(smmu, pmd, addr, next, pfn, prot, stage); phys += next - addr; + pfn = __phys_to_pfn(phys); } while (pmd++, addr = next, addr end); return ret; -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [RFC][PATCH] devicetree: Add master-id-bits property to the iommu device
Hi Arnd, -Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Arnd Bergmann Sent: Monday, September 15, 2014 10:27 PM To: Sethi Varun-B16395 Cc: mark.rutl...@arm.com; devicet...@vger.kernel.org; swar...@nvidia.com; will.dea...@arm.com; Yoder Stuart-B08248; robh...@kernel.org; iommu@lists.linux-foundation.org; thierry.red...@gmail.com; linux-arm-ker...@lists.infradead.org Subject: Re: [RFC][PATCH] devicetree: Add master-id-bits property to the iommu device On Monday 15 September 2014, Varun Sethi wrote: This seems rather specific to MMU-500. I don't think that most IOMMUs would use the term 'master ID', 'stream ID' or even the general concept, and you don't expand the acronym 'TBU'. I've seen many IOMMUs and I don't even know what that means. TBU refers to the translation buffer unit, which is responsible for caching page translations. In case of translation miss in the cache, translation request is forwarded to the TCU (Translation control unit). The master id forwarded to TCU would also contain the TBU ID. Using the master-id-bits property we can mask out the additional TBU bits at the TCU. This is a cause of concern when we want to share master id for devices which are connected to different TBUs. We have a hot pluggable bus architecture, where a device group can have multiple devices connected to different TBUs. So, we need a mechanism to mask out additional TBIU bits. Ok, I think I understand now Why do you think this is something that is needed to be known at the global level, rather than a property for some individual drivers? In case of Freescale Layerscape SOCs, number of bits used for defining a stream id are specific to a given platform. Are you suggesting that this property should be added to the master device node, rather than the iommu node? Most importantly, I think this needs to be part of the (iommu) device specific binding, not the generic binding that is used for all iommus that may or may not have this concept. I believe in case of the ARM SMMU, it should actually go into the master node as part of the 'iommus' property, because the mask can be different for each master. If your IOMMU has a fixed mask that is used for all devices, that's fine and you can put it into the iommu node itself but document it in the binding for your IOMMU. For hot-pluggable buses, you probably need to have the 'iommus' property in the node that corresponds to the bus controller, and that will have a mask that is used for all devices plugged into it. Can I add a note to the generic binding about representing the mask as a part of the iommus property. This would similar to the note about the dma-ranges -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [RFC][PATCH] devicetree: Add master-id-bits property to the iommu device
Hi Arnd, -Original Message- From: Arnd Bergmann [mailto:a...@arndb.de] Sent: Monday, September 15, 2014 8:08 AM To: Sethi Varun-B16395 Cc: devicet...@vger.kernel.org; iommu@lists.linux-foundation.org; thierry.red...@gmail.com; mark.rutl...@arm.com; will.dea...@arm.com; hd...@nvidia.com; swar...@nvidia.com; robh...@kernel.org; linux-arm- ker...@lists.infradead.org; Yoder Stuart-B08248 Subject: Re: [RFC][PATCH] devicetree: Add master-id-bits property to the iommu device On Sunday 14 September 2014, Varun Sethi wrote: master-id-bits property added to the IOMMU device node. This property can be used by the IOMMU driver to match relevan bits in the master id expressed by a DMA master. This can be used to mask out certain bits that get added to the device master id due to IOMMU topology. For example, in case of MMU-500 the TBUID gets appended to the master id. This prevents sharing of a stream ID, amongst devices which are connected to different TBUs. Signed-off-by: Varun Sethi varun.se...@freescale.com This seems rather specific to MMU-500. I don't think that most IOMMUs would use the term 'master ID', 'stream ID' or even the general concept, and you don't expand the acronym 'TBU'. I've seen many IOMMUs and I don't even know what that means. TBU refers to the translation buffer unit, which is responsible for caching page translations. In case of translation miss in the cache, translation request is forwarded to the TCU (Translation control unit). The master id forwarded to TCU would also contain the TBU ID. Using the master-id-bits property we can mask out the additional TBU bits at the TCU. This is a cause of concern when we want to share master id for devices which are connected to different TBUs. We have a hot pluggable bus architecture, where a device group can have multiple devices connected to different TBUs. So, we need a mechanism to mask out additional TBIU bits. Why do you think this is something that is needed to be known at the global level, rather than a property for some individual drivers? In case of Freescale Layerscape SOCs, number of bits used for defining a stream id are specific to a given platform. Are you suggesting that this property should be added to the master device node, rather than the iommu node? -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/fsl: Fix warning resulting from adding PCI device twice
iommu_group_get_for_dev determines the iommu group for the PCI device and adds the device to the group. In the PAMU driver we were again adding the device to the same group without checking if the device already had an iommu group. This resulted in the following warning. sysfs: cannot create duplicate filename '/devices/ffe20.pcie/pci:00/:00:00.0/iommu_group' [ cut here ] WARNING: at fs/sysfs/dir.c:31 Modules linked in: CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.17.0-rc3-2-g7505cea-dirty #126 task: c001fe0a ti: c001fe044000 task.ti: c001fe044000 NIP: c018879c LR: c0188798 CTR: c001ea50 REGS: c001fe047040 TRAP: 0700 Not tainted (3.17.0-rc3-2-g7505cea-dirty) MSR: 80029000 CE,EE,ME CR: 24ad8e22 XER: 2000 SOFTE: 1 GPR00: c0188798 c001fe0472c0 c09a52e0 0065 GPR04: 0001 3a30303a 2700 GPR08: 2f696f6d c08d3830 c09b3938 c09bb3d0 GPR12: 28ad8e24 cfff4000 c000205c GPR16: GPR20: c08a4c70 GPR24: c07e9010 c001fe0140a8 ffef 0001 GPR28: c001fe22ebb8 c07e9010 c090bf10 c001fe22 NIP [c018879c] .sysfs_warn_dup+0x74/0xa4 LR [c0188798] .sysfs_warn_dup+0x70/0xa4 Call Trace: [c001fe0472c0] [c0188798] .sysfs_warn_dup+0x70/0xa4 (unreliable) [c001fe047350] [c0188d34] .sysfs_do_create_link_sd.clone.2+0x168/0x174 [c001fe047400] [c04b3cf8] .iommu_group_add_device+0x78/0x244 [c001fe0474b0] [c04b6964] .fsl_pamu_add_device+0x88/0x1a8 [c001fe047570] [c04b3960] .iommu_bus_notifier+0xdc/0x15c [c001fe047600] [c0059848] .notifier_call_chain+0x8c/0xe8 [c001fe0476a0] [c0059d04] .__blocking_notifier_call_chain+0x58/0x84 [c001fe047750] [c036619c] .device_add+0x464/0x5c8 [c001fe047820] [c0300ebc] .pci_device_add+0x14c/0x17c [c001fe0478c0] [c0300fbc] .pci_scan_single_device+0xd0/0xf4 [c001fe047970] [c030104c] .pci_scan_slot+0x6c/0x18c [c001fe047a10] [c030226c] .pci_scan_child_bus+0x40/0x114 [c001fe047ac0] [c0021974] .pcibios_scan_phb+0x240/0x2c8 [c001fe047b70] [c085a970] .pcibios_init+0x64/0xc8 [c001fe047c00] [c0001884] .do_one_initcall+0xbc/0x224 [c001fe047d00] [c0852d50] .kernel_init_freeable+0x14c/0x21c [c001fe047db0] [c0002078] .kernel_init+0x1c/0xfa4 [c001fe047e30] [c884] .ret_from_kernel_thread+0x58/0xd4 Instruction dump: 7c7f1b79 4182001c 7fe4fb78 7f83e378 38a01000 4bffc905 6000 7c641b78 e87e8008 7fa5eb78 48482ff5 6000 0fe0 7fe3fb78 4bf7bd39 6000 Signed-off-by: Varun Sethi varun.se...@freescale.com --- drivers/iommu/fsl_pamu_domain.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c index 61d1daf..af5a548 100644 --- a/drivers/iommu/fsl_pamu_domain.c +++ b/drivers/iommu/fsl_pamu_domain.c @@ -984,7 +984,7 @@ static int fsl_pamu_add_device(struct device *dev) struct iommu_group *group = ERR_PTR(-ENODEV); struct pci_dev *pdev; const u32 *prop; - int ret, len; + int ret = 0, len; /* * For platform devices we allocate a separate group for @@ -1007,7 +1007,13 @@ static int fsl_pamu_add_device(struct device *dev) if (IS_ERR(group)) return PTR_ERR(group); - ret = iommu_group_add_device(group, dev); + /* +* Check if device has already been added to an iommu group. +* Group could have already been created for a PCI device in +* the iommu_group_get_for_dev path. +*/ + if (!iommu_group_get(dev)) + ret = iommu_group_add_device(group, dev); iommu_group_put(group); return ret; -- 1.7.9.5 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2] iommu/fsl: Fix warning resulting from adding PCI device twice
iommu_group_get_for_dev determines the iommu group for the PCI device and adds the device to the group. In the PAMU driver we were again adding the device to the same group without checking if the device already had an iommu group. This resulted in the following warning. sysfs: cannot create duplicate filename '/devices/ffe20.pcie/pci:00/:00:00.0/iommu_group' [ cut here ] WARNING: at fs/sysfs/dir.c:31 Modules linked in: CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.17.0-rc3-2-g7505cea-dirty #126 task: c001fe0a ti: c001fe044000 task.ti: c001fe044000 NIP: c018879c LR: c0188798 CTR: c001ea50 REGS: c001fe047040 TRAP: 0700 Not tainted (3.17.0-rc3-2-g7505cea-dirty) MSR: 80029000 CE,EE,ME CR: 24ad8e22 XER: 2000 SOFTE: 1 GPR00: c0188798 c001fe0472c0 c09a52e0 0065 GPR04: 0001 3a30303a 2700 GPR08: 2f696f6d c08d3830 c09b3938 c09bb3d0 GPR12: 28ad8e24 cfff4000 c000205c GPR16: GPR20: c08a4c70 GPR24: c07e9010 c001fe0140a8 ffef 0001 GPR28: c001fe22ebb8 c07e9010 c090bf10 c001fe22 NIP [c018879c] .sysfs_warn_dup+0x74/0xa4 LR [c0188798] .sysfs_warn_dup+0x70/0xa4 Call Trace: [c001fe0472c0] [c0188798] .sysfs_warn_dup+0x70/0xa4 (unreliable) [c001fe047350] [c0188d34] .sysfs_do_create_link_sd.clone.2+0x168/0x174 [c001fe047400] [c04b3cf8] .iommu_group_add_device+0x78/0x244 [c001fe0474b0] [c04b6964] .fsl_pamu_add_device+0x88/0x1a8 [c001fe047570] [c04b3960] .iommu_bus_notifier+0xdc/0x15c [c001fe047600] [c0059848] .notifier_call_chain+0x8c/0xe8 [c001fe0476a0] [c0059d04] .__blocking_notifier_call_chain+0x58/0x84 [c001fe047750] [c036619c] .device_add+0x464/0x5c8 [c001fe047820] [c0300ebc] .pci_device_add+0x14c/0x17c [c001fe0478c0] [c0300fbc] .pci_scan_single_device+0xd0/0xf4 [c001fe047970] [c030104c] .pci_scan_slot+0x6c/0x18c [c001fe047a10] [c030226c] .pci_scan_child_bus+0x40/0x114 [c001fe047ac0] [c0021974] .pcibios_scan_phb+0x240/0x2c8 [c001fe047b70] [c085a970] .pcibios_init+0x64/0xc8 [c001fe047c00] [c0001884] .do_one_initcall+0xbc/0x224 [c001fe047d00] [c0852d50] .kernel_init_freeable+0x14c/0x21c [c001fe047db0] [c0002078] .kernel_init+0x1c/0xfa4 [c001fe047e30] [c884] .ret_from_kernel_thread+0x58/0xd4 Instruction dump: 7c7f1b79 4182001c 7fe4fb78 7f83e378 38a01000 4bffc905 6000 7c641b78 e87e8008 7fa5eb78 48482ff5 6000 0fe0 7fe3fb78 4bf7bd39 6000 Signed-off-by: Varun Sethi varun.se...@freescale.com --- v2 changes - directly check for the device iommu_group drivers/iommu/fsl_pamu_domain.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c index 61d1daf..56feed7 100644 --- a/drivers/iommu/fsl_pamu_domain.c +++ b/drivers/iommu/fsl_pamu_domain.c @@ -984,7 +984,7 @@ static int fsl_pamu_add_device(struct device *dev) struct iommu_group *group = ERR_PTR(-ENODEV); struct pci_dev *pdev; const u32 *prop; - int ret, len; + int ret = 0, len; /* * For platform devices we allocate a separate group for @@ -1007,7 +1007,13 @@ static int fsl_pamu_add_device(struct device *dev) if (IS_ERR(group)) return PTR_ERR(group); - ret = iommu_group_add_device(group, dev); + /* +* Check if device has already been added to an iommu group. +* Group could have already been created for a PCI device in +* the iommu_group_get_for_dev path. +*/ + if (!dev-iommu_group) + ret = iommu_group_add_device(group, dev); iommu_group_put(group); return ret; -- 1.7.9.5 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v3 3/3] iommu/arm-smmu: add support for DOMAIN_ATTR_NESTING attribute
Hi Will, We would still need a mechanism to distinguish stage1 mapping from stage2 mapping i.e. for nested translation we should be able to specify whether the mapping corresponds to stage1 or stage 2 translation. Also, correspondingly we would require different context banks for stage 1 and stage 2 translation. Regards Varun -Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Will Deacon Sent: Tuesday, September 02, 2014 3:24 PM To: iommu@lists.linux-foundation.org Cc: Will Deacon Subject: [PATCH v3 3/3] iommu/arm-smmu: add support for DOMAIN_ATTR_NESTING attribute When domains are set with the DOMAIN_ATTR_NESTING flag, we must ensure that we allocate them to stage-2 context banks if the hardware permits it. This patch adds support for the attribute to the ARM SMMU driver, with the actual stage being determined depending on the features supported by the hardware. Signed-off-by: Will Deacon will.dea...@arm.com --- drivers/iommu/arm-smmu.c | 110 ++- 1 file changed, 90 insertions(+), 20 deletions(-) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index d2d8cdaf4f0b..c745296b170f 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -404,9 +404,16 @@ struct arm_smmu_cfg { #define ARM_SMMU_CB_ASID(cfg)((cfg)-cbndx) #define ARM_SMMU_CB_VMID(cfg)((cfg)-cbndx + 1) +enum arm_smmu_domain_stage { + ARM_SMMU_DOMAIN_S1 = 0, + ARM_SMMU_DOMAIN_S2, + ARM_SMMU_DOMAIN_NESTED, +}; + struct arm_smmu_domain { struct arm_smmu_device *smmu; struct arm_smmu_cfg cfg; + enum arm_smmu_domain_stage stage; spinlock_t lock; }; @@ -899,19 +906,46 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain, if (smmu_domain-smmu) goto out_unlock; - if (smmu-features ARM_SMMU_FEAT_TRANS_NESTED) { + /* + * Mapping the requested stage onto what we support is surprisingly + * complicated, mainly because the spec allows S1+S2 SMMUs without + * support for nested translation. That means we end up with the + * following table: + * + * RequestedSupportedActual + * S1 N S1 + * S1 S1+S2S1 + * S1 S2 S2 + * S1 S1 S1 + * NN N + * N S1+S2S2 + * NS2 S2 + * NS1 S1 + * + * Note that you can't actually request stage-2 mappings. + */ + if (!(smmu-features ARM_SMMU_FEAT_TRANS_S1)) + smmu_domain-stage = ARM_SMMU_DOMAIN_S2; + if (!(smmu-features ARM_SMMU_FEAT_TRANS_S2)) + smmu_domain-stage = ARM_SMMU_DOMAIN_S1; + + switch (smmu_domain-stage) { + case ARM_SMMU_DOMAIN_S1: + cfg-cbar = CBAR_TYPE_S1_TRANS_S2_BYPASS; + start = smmu-num_s2_context_banks; + break; + case ARM_SMMU_DOMAIN_NESTED: /* * We will likely want to change this if/when KVM gets * involved. */ - cfg-cbar = CBAR_TYPE_S1_TRANS_S2_BYPASS; - start = smmu-num_s2_context_banks; - } else if (smmu-features ARM_SMMU_FEAT_TRANS_S1) { - cfg-cbar = CBAR_TYPE_S1_TRANS_S2_BYPASS; - start = smmu-num_s2_context_banks; - } else { + case ARM_SMMU_DOMAIN_S2: cfg-cbar = CBAR_TYPE_S2_TRANS; start = 0; + break; + default: + ret = -EINVAL; + goto out_unlock; } ret = __arm_smmu_alloc_bitmap(smmu-context_map, start, @@ - 1637,20 +1671,56 @@ static void arm_smmu_remove_device(struct device *dev) iommu_group_remove_device(dev); } +static int arm_smmu_domain_get_attr(struct iommu_domain *domain, + enum iommu_attr attr, void *data) { + struct arm_smmu_domain *smmu_domain = domain-priv; + + switch (attr) { + case DOMAIN_ATTR_NESTING: + *(int *)data = (smmu_domain-stage == ARM_SMMU_DOMAIN_NESTED); + return 0; + default: + return -ENODEV; + } +} + +static int arm_smmu_domain_set_attr(struct iommu_domain *domain, + enum iommu_attr attr, void *data) { + struct arm_smmu_domain *smmu_domain = domain-priv; + + switch (attr) { + case DOMAIN_ATTR_NESTING: + if (smmu_domain-smmu) + return -EPERM; + if (*(int *)data) + smmu_domain-stage =
RE: [RFC PATCH 1/7] iommu: provide early initialisation hook for IOMMU drivers
Hi Will, -Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Friday, August 29, 2014 9:24 PM To: linux-arm-ker...@lists.infradead.org; iommu@lists.linux-foundation.org Cc: a...@arndb.de; dw...@infradead.org; jroe...@suse.de; hd...@nvidia.com; Sethi Varun-B16395; thierry.red...@gmail.com; laurent.pinch...@ideasonboard.com; Will Deacon Subject: [RFC PATCH 1/7] iommu: provide early initialisation hook for IOMMU drivers IOMMU drivers must be initialised before any of their upstream devices, otherwise the relevant iommu_ops won't be configured for the bus in question. To solve this, a number of IOMMU drivers use initcalls to initialise the driver before anything has a chance to be probed. Whilst this solves the immediate problem, it leaves the job of probing the IOMMU completely separate from the iommu_ops to configure the IOMMU, which are called on a per-bus basis and require the driver to figure out exactly which instance of the IOMMU is being requested. In particular, the add_device callback simply passes a struct device to the driver, which then has to parse firmware tables or probe buses to identify the relevant IOMMU instance. This patch takes the first step in addressing this problem by adding an early initialisation pass for IOMMU drivers, giving them the ability to set some per-instance data on their of_node. This data can then be passed back to a new add_device function (added in a later patch) to identify the IOMMU instance in question. Signed-off-by: Will Deacon will.dea...@arm.com --- drivers/iommu/of_iommu.c | 14 ++ include/asm-generic/vmlinux.lds.h | 2 ++ include/linux/of_iommu.h | 21 + 3 files changed, 37 insertions(+) diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c index e550ccb7634e..f9209666157c 100644 --- a/drivers/iommu/of_iommu.c +++ b/drivers/iommu/of_iommu.c @@ -89,3 +89,17 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index, return 0; } EXPORT_SYMBOL_GPL(of_get_dma_window); + +void __init of_iommu_init(void) +{ + struct device_node *np; + const struct of_device_id *match, *matches = __iommu_of_table; + + for_each_matching_node_and_match(np, matches, match) { + const int (*init_fn)(struct device_node *) = match-data; Is the init function also responsible for setting iommu_ops (per bus)? We need to take in to consideration that iommu API (iommu.c) initialization happens during arch_init. When we setup bus iommu ops add_device would be called for devices which have already been probed. Also, as I can see from the code we have of_platform_populate which gets called right after of_iommu_init, which would in turn also lead to add_device invocation (after add_device_master_ids). I believe this would happen before iommu API initialization which would cause issues. -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [RFC PATCH 4/7] iommu: provide helper function to configure an IOMMU for an of master
Hi Will, I am not clear on the functionality we want to achieve with this new API. Is this a way to link devices to a particular IOMMU? Would this be used to filter out add_device invocations i.e. iommu group creations just for the devices attached to a particular IOMMU? What is the purpose of the data pointer, that is being passed to this API? Also, how would this be relevant to PCIe devices (also other hot plug devices). Regards Varun -Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Friday, August 29, 2014 9:24 PM To: linux-arm-ker...@lists.infradead.org; iommu@lists.linux-foundation.org Cc: a...@arndb.de; dw...@infradead.org; jroe...@suse.de; hd...@nvidia.com; Sethi Varun-B16395; thierry.red...@gmail.com; laurent.pinch...@ideasonboard.com; Will Deacon Subject: [RFC PATCH 4/7] iommu: provide helper function to configure an IOMMU for an of master The generic IOMMU device-tree bindings can be used to add arbitrary OF masters to an IOMMU with a compliant binding. This patch introduces of_iommu_configure, which does exactly that. Signed-off-by: Will Deacon will.dea...@arm.com --- drivers/iommu/Kconfig| 2 +- drivers/iommu/of_iommu.c | 36 include/linux/of_iommu.h | 6 ++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig index dd5112265cc9..6d13f962f156 100644 --- a/drivers/iommu/Kconfig +++ b/drivers/iommu/Kconfig @@ -15,7 +15,7 @@ if IOMMU_SUPPORT config OF_IOMMU def_bool y - depends on OF + depends on OF IOMMU_API config FSL_PAMU bool Freescale IOMMU support diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c index f9209666157c..a7d3b3a13b83 100644 --- a/drivers/iommu/of_iommu.c +++ b/drivers/iommu/of_iommu.c @@ -19,6 +19,7 @@ #include linux/export.h #include linux/limits.h +#include linux/iommu.h #include linux/of.h #include linux/of_iommu.h @@ -90,6 +91,41 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index, } EXPORT_SYMBOL_GPL(of_get_dma_window); +int of_iommu_configure(struct device *dev) { + struct of_phandle_args iommu_spec; + struct bus_type *bus = dev-bus; + const struct iommu_ops *ops = bus-iommu_ops; + int ret = -EINVAL, idx = 0; + + if (!iommu_present(bus)) + return -ENODEV; + + /* + * We don't currently walk up the tree looking for a parent IOMMU. + * See the `Notes:' section of + * Documentation/devicetree/bindings/iommu/iommu.txt + */ + while (!of_parse_phandle_with_args(dev-of_node, iommus, +#iommu-cells, idx, +iommu_spec)) { + void *data = of_iommu_get_data(iommu_spec.np); + + of_node_put(iommu_spec.np); + if (!ops-add_device_master_ids) + return -ENODEV; + + ret = ops-add_device_master_ids(dev, iommu_spec.args_count, + iommu_spec.args, data); + if (ret) + break; + + idx++; + } + + return ret; +} + void __init of_iommu_init(void) { struct device_node *np; diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h index 29f2f3f88d6a..85c6d1152624 100644 --- a/include/linux/of_iommu.h +++ b/include/linux/of_iommu.h @@ -1,6 +1,7 @@ #ifndef __OF_IOMMU_H #define __OF_IOMMU_H +#include linux/device.h #include linux/of.h #ifdef CONFIG_OF_IOMMU @@ -10,6 +11,7 @@ extern int of_get_dma_window(struct device_node *dn, const char *prefix, size_t *size); extern void of_iommu_init(void); +extern int of_iommu_configure(struct device *dev); #else @@ -21,6 +23,10 @@ static inline int of_get_dma_window(struct device_node *dn, const char *prefix, } static inline void of_iommu_init(void) { } +static inline int of_iommu_configure(struct device *dev) { + return -ENODEV; +} #endif /* CONFIG_OF_IOMMU */ -- 2.1.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v5] devicetree: Add generic IOMMU device tree bindings
Hi Hiroshi, -Original Message- From: Hiroshi Doyu [mailto:hd...@nvidia.com] Sent: Thursday, August 14, 2014 9:35 PM To: Sethi Varun-B16395 Cc: Hiroshi Doyu; Thierry Reding; Stephen Warren; Arnd Bergmann; Will Deacon; Mark Rutland; devicet...@vger.kernel.org; Olof Johansson; iommu@lists.linux-foundation.org; Rob Herring; linux-te...@vger.kernel.org; linux-arm-ker...@lists.infradead.org Subject: Re: [PATCH v5] devicetree: Add generic IOMMU device tree bindings Hi Varun, Varun Sethi varun.se...@freescale.com writes: -Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Hiroshi Doyu Sent: Thursday, August 14, 2014 12:18 PM To: Thierry Reding; Stephen Warren; Arnd Bergmann; Will Deacon Cc: Mark Rutland; devicet...@vger.kernel.org; Olof Johansson; iommu@lists.linux-foundation.org; Rob Herring; linux-te...@vger.kernel.org; linux-arm-ker...@lists.infradead.org Subject: Re: [PATCH v5] devicetree: Add generic IOMMU device tree bindings Thierry Reding thierry.red...@gmail.com writes: +Multiple-master IOMMU: +-- + + iommu { + /* the specifier represents the ID of the master */ + #iommu-cells = 1; + }; + + master@1 { + /* device has master ID 42 in the IOMMU */ + iommus = {/iommu} 42; + }; + + master@2 { + /* device has master IDs 23 and 24 in the IOMMU */ + iommus = {/iommu} 23, {/iommu} 24; + }; I think that this master ID will be parsed in IOMMU driver. For example, ARM,SMMU expects streamID as master ID, right? If a SoC has a feature to configure to assign streamID to devices at runtime, streamID is not equal to master ID. iommus = {/smmu} soc specific master ID; soc master ID needs to be translated into streamID by SoC SW. It seems that ARM,SMMU kernel driver doesn't expect this kind of ID translation. If ARM,SMMU kernel driver is used as is, soc master ID would be incompatible? ARM,SMMU needs such translation before parsing. Is this my understanding right? If so I think that this master ID configuration/translation may be quite reasonable requirment for SoC using ARM,SMMU. Can we consider this ID translation within ARM,SMMU compatibility? IOW, is it possible to implement some SoC specific hook for ID translation/configuration in ARM,SMMU kernel driver? Can the id translation be done using a SMR mask? No, SoC master ID is completely independenf of SMR. Also, for dynamic stream ID allocation we would need to represent the specific master register (to store the stream ID) in the device tree. I assmue that the above means that iMX has such configuration register to map steramID and a device dynamically. We have per master registers for setting the stream ID on the Layerscape platforms. My point was that we would need the iommu master node to include a reference to the master id register. master@1 { /* device has master ID 42 in the IOMMU */ iommus = {/iommu} 42; master-id-reg = phandle offset }; -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v5] devicetree: Add generic IOMMU device tree bindings
-Original Message- From: Hiroshi Doyu [mailto:hd...@nvidia.com] Sent: Tuesday, August 19, 2014 3:34 PM To: Sethi Varun-B16395; Will Deacon Cc: Hiroshi Doyu; Thierry Reding; Stephen Warren; Arnd Bergmann; Mark Rutland; devicet...@vger.kernel.org; Olof Johansson; iommu@lists.linux- foundation.org; Rob Herring; linux-te...@vger.kernel.org; linux-arm- ker...@lists.infradead.org; Yoder Stuart-B08248 Subject: Re: [PATCH v5] devicetree: Add generic IOMMU device tree bindings Varun Sethi varun.se...@freescale.com writes: Hi Hiroshi, -Original Message- From: Hiroshi Doyu [mailto:hd...@nvidia.com] Sent: Thursday, August 14, 2014 9:35 PM To: Sethi Varun-B16395 Cc: Hiroshi Doyu; Thierry Reding; Stephen Warren; Arnd Bergmann; Will Deacon; Mark Rutland; devicet...@vger.kernel.org; Olof Johansson; iommu@lists.linux-foundation.org; Rob Herring; linux-te...@vger.kernel.org; linux-arm-ker...@lists.infradead.org Subject: Re: [PATCH v5] devicetree: Add generic IOMMU device tree bindings Hi Varun, Varun Sethi varun.se...@freescale.com writes: -Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Hiroshi Doyu Sent: Thursday, August 14, 2014 12:18 PM To: Thierry Reding; Stephen Warren; Arnd Bergmann; Will Deacon Cc: Mark Rutland; devicet...@vger.kernel.org; Olof Johansson; iommu@lists.linux-foundation.org; Rob Herring; linux-te...@vger.kernel.org; linux-arm-ker...@lists.infradead.org Subject: Re: [PATCH v5] devicetree: Add generic IOMMU device tree bindings Thierry Reding thierry.red...@gmail.com writes: +Multiple-master IOMMU: +-- + + iommu { + /* the specifier represents the ID of the master */ + #iommu-cells = 1; + }; + + master@1 { + /* device has master ID 42 in the IOMMU */ + iommus = {/iommu} 42; + }; + + master@2 { + /* device has master IDs 23 and 24 in the IOMMU */ + iommus = {/iommu} 23, {/iommu} 24; + }; I think that this master ID will be parsed in IOMMU driver. For example, ARM,SMMU expects streamID as master ID, right? If a SoC has a feature to configure to assign streamID to devices at runtime, streamID is not equal to master ID. iommus = {/smmu} soc specific master ID; soc master ID needs to be translated into streamID by SoC SW. It seems that ARM,SMMU kernel driver doesn't expect this kind of ID translation. If ARM,SMMU kernel driver is used as is, soc master ID would be incompatible? ARM,SMMU needs such translation before parsing. Is this my understanding right? If so I think that this master ID configuration/translation may be quite reasonable requirment for SoC using ARM,SMMU. Can we consider this ID translation within ARM,SMMU compatibility? IOW, is it possible to implement some SoC specific hook for ID translation/configuration in ARM,SMMU kernel driver? Can the id translation be done using a SMR mask? No, SoC master ID is completely independenf of SMR. Also, for dynamic stream ID allocation we would need to represent the specific master register (to store the stream ID) in the device tree. I assmue that the above means that iMX has such configuration register to map steramID and a device dynamically. We have per master registers for setting the stream ID on the Layerscape platforms. My point was that we would need the iommu master node to include a reference to the master id register. master@1 { /* device has master ID 42 in the IOMMU */ iommus = {/iommu} 42; master-id-reg = phandle offset }; In the above, for iommus= bindings, you wouldn't need to break ARM,SMMU compatibility at all if you set streamID exactly as below. master@1 { /* device has master ID 42 in the IOMMU */ iommus = {/iommu} 'any given streamID'; master-id-reg = phandle offset }; And your SoC needs to register bus_notifier and ADD_DEVICE should configure to map 'any given streamID' to a device via the above register. This wouldn't need any modification from ARM,SMMU driver and keep the iommus bindings as it is. IOW, SoC only needs to register ADD_DEVICE in bus_notifier to map StreamID to a device. This needs to be executed earlier than IOMMU bus's ADD_DEVICE, though. Is my understanding right? I don't think that SOC specific code needs a bus notifier for setting the stream ID. It can be done as a part of SOC specific initialization. The device tree can be updated to reflect the correct stream ID (SMMU driver can get the updated stream ID from device tree). I was thinking more on the lines of updating the device
RE: [PATCH v5] devicetree: Add generic IOMMU device tree bindings
-Original Message- From: Hiroshi Doyu [mailto:hd...@nvidia.com] Sent: Tuesday, August 19, 2014 4:33 PM To: Sethi Varun-B16395; Will Deacon Cc: Hiroshi Doyu; Thierry Reding; Stephen Warren; Arnd Bergmann; Mark Rutland; devicet...@vger.kernel.org; Olof Johansson; iommu@lists.linux- foundation.org; Rob Herring; linux-te...@vger.kernel.org; linux-arm- ker...@lists.infradead.org; Yoder Stuart-B08248 Subject: Re: [PATCH v5] devicetree: Add generic IOMMU device tree bindings Varun Sethi varun.se...@freescale.com writes: Also, for dynamic stream ID allocation we would need to represent the specific master register (to store the stream ID) in the device tree. I assmue that the above means that iMX has such configuration register to map steramID and a device dynamically. We have per master registers for setting the stream ID on the Layerscape platforms. My point was that we would need the iommu master node to include a reference to the master id register. master@1 { /* device has master ID 42 in the IOMMU */ iommus = {/iommu} 42; master-id-reg = phandle offset }; In the above, for iommus= bindings, you wouldn't need to break ARM,SMMU compatibility at all if you set streamID exactly as below. master@1 { /* device has master ID 42 in the IOMMU */ iommus = {/iommu} 'any given streamID'; master-id-reg = phandle offset }; And your SoC needs to register bus_notifier and ADD_DEVICE should configure to map 'any given streamID' to a device via the above register. This wouldn't need any modification from ARM,SMMU driver and keep the iommus bindings as it is. IOW, SoC only needs to register ADD_DEVICE in bus_notifier to map StreamID to a device. This needs to be executed earlier than IOMMU bus's ADD_DEVICE, though. Is my understanding right? I don't think that SOC specific code needs a bus notifier for setting the stream ID. It can be done as a part of SOC specific initialization. The device tree can be updated to reflect the correct stream ID (SMMU driver can get the updated stream ID from device tree). That's possible. I was thinking more on the lines of updating the device stream id while attaching a device to the domain. I thought the same but this would break the ARM,SMMU /compatibility/ since the 1st param of iommus= is always expected as streamID. If streamID can be assigned dynamically like PCIe, not like streamID statically set in DT, how should we describe this dynmaic steramID shifting/assignment in DT? Can you dynamically program the stream ID for the device? If yes, then you require a unique numberspace for allocating a streamID. I am not clear on the stream ID translation requirement. -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v5] devicetree: Add generic IOMMU device tree bindings
Hi Will, -Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Will Deacon Sent: Friday, August 15, 2014 5:21 PM To: Hiroshi Doyu Cc: Mark Rutland; devicet...@vger.kernel.org; Stephen Warren; Arnd Bergmann; Rob Herring; iommu@lists.linux-foundation.org; Thierry Reding; linux-te...@vger.kernel.org; linux-arm-ker...@lists.infradead.org Subject: Re: [PATCH v5] devicetree: Add generic IOMMU device tree bindings On Thu, Aug 14, 2014 at 07:47:42AM +0100, Hiroshi Doyu wrote: Thierry Reding thierry.red...@gmail.com writes: +Multiple-master IOMMU: +-- + + iommu { + /* the specifier represents the ID of the master */ + #iommu-cells = 1; + }; + + master@1 { + /* device has master ID 42 in the IOMMU */ + iommus = {/iommu} 42; + }; + + master@2 { + /* device has master IDs 23 and 24 in the IOMMU */ + iommus = {/iommu} 23, {/iommu} 24; + }; I think that this master ID will be parsed in IOMMU driver. For example, ARM,SMMU expects streamID as master ID, right? If a SoC has a feature to configure to assign streamID to devices at runtime, streamID is not equal to master ID. iommus = {/smmu} soc specific master ID; soc master ID needs to be translated into streamID by SoC SW. It seems that ARM,SMMU kernel driver doesn't expect this kind of ID translation. If ARM,SMMU kernel driver is used as is, soc master ID would be incompatible? ARM,SMMU needs such translation before parsing. Is this my understanding right? If so I think that this master ID configuration/translation may be quite reasonable requirment for SoC using ARM,SMMU. Can we consider this ID translation within ARM,SMMU compatibility? IOW, is it possible to implement some SoC specific hook for ID translation/configuration in ARM,SMMU kernel driver? I think there's some confusion here. The ARM architected SMMU does not perform any StreamID translation -- it sees an incoming ID and uses that to lookup a set of translation tables. I don't completely agree with this. In case of MMU-500 don't we have the TBUID + device assigned stream ID representing the stream ID for matching at the TCU? In case of our platform, we have hot pluggable device objects comprising of devices connected to different TBUs. We have a requirement to use a single stream ID for all the distributed objects. We will have to use the stream ID masking capability to mask out the TBU ID. -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v5] devicetree: Add generic IOMMU device tree bindings
-Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Hiroshi Doyu Sent: Thursday, August 14, 2014 12:18 PM To: Thierry Reding; Stephen Warren; Arnd Bergmann; Will Deacon Cc: Mark Rutland; devicet...@vger.kernel.org; Olof Johansson; iommu@lists.linux-foundation.org; Rob Herring; linux-te...@vger.kernel.org; linux-arm-ker...@lists.infradead.org Subject: Re: [PATCH v5] devicetree: Add generic IOMMU device tree bindings Thierry Reding thierry.red...@gmail.com writes: +Multiple-master IOMMU: +-- + + iommu { + /* the specifier represents the ID of the master */ + #iommu-cells = 1; + }; + + master@1 { + /* device has master ID 42 in the IOMMU */ + iommus = {/iommu} 42; + }; + + master@2 { + /* device has master IDs 23 and 24 in the IOMMU */ + iommus = {/iommu} 23, {/iommu} 24; + }; I think that this master ID will be parsed in IOMMU driver. For example, ARM,SMMU expects streamID as master ID, right? If a SoC has a feature to configure to assign streamID to devices at runtime, streamID is not equal to master ID. iommus = {/smmu} soc specific master ID; soc master ID needs to be translated into streamID by SoC SW. It seems that ARM,SMMU kernel driver doesn't expect this kind of ID translation. If ARM,SMMU kernel driver is used as is, soc master ID would be incompatible? ARM,SMMU needs such translation before parsing. Is this my understanding right? If so I think that this master ID configuration/translation may be quite reasonable requirment for SoC using ARM,SMMU. Can we consider this ID translation within ARM,SMMU compatibility? IOW, is it possible to implement some SoC specific hook for ID translation/configuration in ARM,SMMU kernel driver? Can the id translation be done using a SMR mask? Also, for dynamic stream ID allocation we would need to represent the specific master register (to store the stream ID) in the device tree. -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 2/5] iommu/arm-smmu: add support for PCI master devices
Hi Will, -Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Wednesday, July 09, 2014 6:57 PM To: Sethi Varun-B16395; alex.william...@redhat.com Cc: linux-arm-ker...@lists.infradead.org; iommu@lists.linux- foundation.org; thierry.red...@gmail.com; a...@arndb.de; ohau...@codeaurora.org; j...@8bytes.org; a.mota...@virtualopensystems.com; Marc Zyngier Subject: Re: [PATCH 2/5] iommu/arm-smmu: add support for PCI master devices [Adding Alex; question below] On Thu, Jul 03, 2014 at 03:22:37PM +0100, Varun Sethi wrote: +static int __arm_smmu_get_pci_sid(struct pci_dev *pdev, u16 alias, +void +*data) { + *((u16 *)data) = alias; + return 0; /* Continue walking */ } [...] @@ -1598,15 +1642,36 @@ static int arm_smmu_add_device(struct device *dev) return PTR_ERR(group); } + if (dev_is_pci(dev)) { + struct arm_smmu_master_cfg *cfg; + struct pci_dev *pdev = to_pci_dev(dev); + + cfg = kzalloc(sizeof(*cfg), GFP_KERNEL); + if (!cfg) { + ret = -ENOMEM; + goto out_put_group; + } + + cfg-num_streamids = 1; + pci_for_each_dma_alias(pdev, __arm_smmu_get_pci_sid, +cfg-streamids[0]); [Sethi Varun-B16395] We need to look for upstream DMA device. We should be using pci_find_dma_isolation_root here. Also, this would also imply that there could be multiple devices sharing the same stream ID. So, we should check if a particular stream ID value has already been configured in the SMR registers. pci_find_dma_isolation_root doesn't exist in any of the trees I have. Alex, is this queued anywhere and do I actually need it? The purpose of this code is to find the requester ID of a device as it appears at the host controller. At this point, we can map it (via firmware tables that are TBD) to a Stream ID for the SMMU. It looks to me like pci_for_each_dma_alias walks over non-transparent PCI bridges correctly, so the callback I provide just updates the alias until the walk has completed. I think pci_for_each_dma_alias should work fine here. Isolation would be specifically required while setting up the iommu groups. -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 0/3] iommu/fsl: Fixes for the PAMU driver.
Thanks Joerg. -Original Message- From: Joerg Roedel [mailto:j...@8bytes.org] Sent: Monday, July 07, 2014 2:02 PM To: Sethi Varun-B16395 Cc: iommu@lists.linux-foundation.org; linux-ker...@vger.kernel.org; linuxppc-...@lists.ozlabs.org; alex.william...@redhat.com Subject: Re: [PATCH 0/3] iommu/fsl: Fixes for the PAMU driver. On Tue, Jun 24, 2014 at 07:27:14PM +0530, Varun Sethi wrote: This patch set contains fixes for the PAMU driver. The patches are based on 3.16-rc1. Varun Sethi (3): Fix PAMU window size check. Fix the device domain attach condition. Fix the error condition during iommu group creation. drivers/iommu/fsl_pamu.c|8 drivers/iommu/fsl_pamu_domain.c | 19 +-- 2 files changed, 13 insertions(+), 14 deletions(-) Applied to iommu/fixes, thanks. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v3 02/10] devicetree: Add generic IOMMU device tree bindings
-Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Thierry Reding Sent: Friday, June 27, 2014 2:20 AM To: Rob Herring; Pawel Moll; Mark Rutland; Ian Campbell; Kumar Gala; Stephen Warren; Arnd Bergmann; Will Deacon; Joerg Roedel Cc: Olav Haugan; devicet...@vger.kernel.org; Grant Grundler; Rhyland Klein; iommu@lists.linux-foundation.org; linux-ker...@vger.kernel.org; Marc Zyngier; Allen Martin; Paul Walmsley; linux-te...@vger.kernel.org; Cho KyongHo; Dave Martin; linux-arm-ker...@lists.infradead.org Subject: [PATCH v3 02/10] devicetree: Add generic IOMMU device tree bindings From: Thierry Reding tred...@nvidia.com This commit introduces a generic device tree binding for IOMMU devices. Only a very minimal subset is described here, but it is enough to cover the requirements of both the Exynos System MMU and Tegra SMMU as discussed here: https://lkml.org/lkml/2014/4/27/346 Signed-off-by: Thierry Reding tred...@nvidia.com --- Changes in v3: - use #iommu-cells instead of #address-cells/#size-cells - drop optional iommu-names property Changes in v2: - add notes about dma-ranges property (drop note from commit message) - document priorities of iommus property vs. dma-ranges property - drop #iommu-cells in favour of #address-cells and #size-cells - remove multiple-master device example Documentation/devicetree/bindings/iommu/iommu.txt | 156 ++ 1 file changed, 156 insertions(+) create mode 100644 Documentation/devicetree/bindings/iommu/iommu.txt diff --git a/Documentation/devicetree/bindings/iommu/iommu.txt b/Documentation/devicetree/bindings/iommu/iommu.txt new file mode 100644 index ..f8f03f057156 --- /dev/null +++ b/Documentation/devicetree/bindings/iommu/iommu.txt @@ -0,0 +1,156 @@ +This document describes the generic device tree binding for IOMMUs and +their master(s). + + +IOMMU device node: +== + +An IOMMU can provide the following services: + +* Remap address space to allow devices to access physical memory ranges +that + they otherwise wouldn't be capable of accessing. + + Example: 32-bit DMA to 64-bit physical addresses + +* Implement scatter-gather at page level granularity so that the device +does + not have to. + +* Provide system protection against rogue DMA by forcing all accesses +to go + through the IOMMU and faulting when encountering accesses to unmapped + address regions. + +* Provide address space isolation between multiple contexts. + + Example: Virtualization + +Device nodes compatible with this binding represent hardware with some +of the above capabilities. + +IOMMUs can be single-master or multiple-master. Single-master IOMMU +devices typically have a fixed association to the master device, +whereas multiple- master IOMMU devices can translate accesses from more than one master. + +The device tree node of the IOMMU device's parent bus must contain a +valid dma-ranges property that describes how the physical address +space of the IOMMU maps to memory. An empty dma-ranges property means +that there is a +1:1 mapping from IOMMU to memory. + +Required properties: + +- #iommu-cells: The number of cells in an IOMMU specifier needed to +encode an + address. + +Typical values for the above include: +- #iommu-cells = 0: Single master IOMMU devices are not configurable +and + therefore no additional information needs to be encoded in the specifier. + This may also apply to multiple master IOMMU devices that do not +allow the + association of masters to be configured. +- #iommu-cells = 1: Multiple master IOMMU devices may need to be +configured + in order to enable translation for a given master. In such cases the +single + address cell corresponds to the master device's ID. +- #iommu-cells = 4: Some IOMMU devices allow the DMA window for +masters to + be configured. The first cell of the address in this may contain the +master + device's ID for example, while the second cell could contain the +start of + the DMA window for the given device. The length of the DMA window is +given + by the third and fourth cells. + + +IOMMU master node: +== + +Devices that access memory through an IOMMU are called masters. A +device can have multiple master interfaces (to one or more IOMMU devices). + +Required properties: + +- iommus: A list of phandle and IOMMU specifier pairs that describe the +IOMMU + master interfaces of the device. One entry in the list describes one +master + interface of the device. + +When an iommus property is specified in a device tree node, the IOMMU +will be used for address translation. If a dma-ranges property exists +in the device's parent node it will be ignored. An exception to this +rule is if the referenced IOMMU is disabled,
RE: [PATCH 2/5] iommu/arm-smmu: add support for PCI master devices
Hi Will, -Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Thursday, July 03, 2014 8:14 PM To: Sethi Varun-B16395 Cc: linux-arm-ker...@lists.infradead.org; iommu@lists.linux- foundation.org; thierry.red...@gmail.com; a...@arndb.de; ohau...@codeaurora.org; j...@8bytes.org; a.mota...@virtualopensystems.com; Marc Zyngier Subject: Re: [PATCH 2/5] iommu/arm-smmu: add support for PCI master devices On Thu, Jul 03, 2014 at 03:22:37PM +0100, Varun Sethi wrote: Hi Will, Hi Varun, Thanks for taking a look at this! +static struct arm_smmu_master_cfg * find_smmu_master_cfg(struct +arm_smmu_device *smmu, struct device *dev) { + struct arm_smmu_master *master; + + if (dev_is_pci(dev)) + return dev-archdata.iommu; + + master = find_smmu_master(smmu, + dev_get_master_dev(dev)-of_node); [Sethi Varun-B16395] Why use dev_get_master_dev over here? You can simply use dev. True; we've already done the PCI check above. I'll tidy this up. @@ -1598,15 +1642,36 @@ static int arm_smmu_add_device(struct device *dev) return PTR_ERR(group); } + if (dev_is_pci(dev)) { + struct arm_smmu_master_cfg *cfg; + struct pci_dev *pdev = to_pci_dev(dev); + + cfg = kzalloc(sizeof(*cfg), GFP_KERNEL); + if (!cfg) { + ret = -ENOMEM; + goto out_put_group; + } + + cfg-num_streamids = 1; + pci_for_each_dma_alias(pdev, __arm_smmu_get_pci_sid, +cfg-streamids[0]); [Sethi Varun-B16395] We need to look for upstream DMA device. We should be using pci_find_dma_isolation_root here. Also, this would also imply that there could be multiple devices sharing the same stream ID. So, we should check if a particular stream ID value has already been configured in the SMR registers. That function doesn't seem to appear in mainline or next. I can move to it when it's available, but in the meantime the above is working for me. I'm making the assumption here that the system is configured so that there aren't any duplicate stream IDs. What we actually need is a function here which maps the requester ID to a stream ID using firmware tables (provided by DT or ACPI). In the absence of those tables at the moment, I just assign the ID directly, which happens to work on my platform (1:1 mapping). Once Thierry's generic IOMMU binding is sorted, we should look at adding support for the Stream ID description. Have you looked at that at all? Yes, I have looked at the bindings. Would we need to represent the stream ids for PCI devices in the device tree? Why do we want to depend on the firmware to map the requestor id to the stream id? It can be handled using the APIs proposed by Alex Williamson. This is similar to IOMMU group determination, which is handled by the IOMMU driver. Will BTW: You seem to have a rather strange quoting style on your replies. Is there any way to configure your editor to limit your lines to 80 columns? You also don't need the prefix with your name and number in brackets! Will try to sort out issues with my e-mail client. -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 1/3] iommu/fsl: Fix PAMU window size check.
-Original Message- From: Joerg Roedel [mailto:j...@8bytes.org] Sent: Friday, July 04, 2014 4:15 PM To: Sethi Varun-B16395 Cc: iommu@lists.linux-foundation.org; linux-ker...@vger.kernel.org; linuxppc-...@lists.ozlabs.org; alex.william...@redhat.com Subject: Re: [PATCH 1/3] iommu/fsl: Fix PAMU window size check. On Tue, Jun 24, 2014 at 07:27:15PM +0530, Varun Sethi wrote: /* window size is 2^(WSE+1) bytes */ - return __ffs(addrspace_size) - 1; + return fls64(addrspace_size) - 2; This looks bogus, why do you replace ffs (find-first-bit) by fls (find- last-bit)? Address space size is always a power of 2. This change was required to handle address sizes 32bit width on 32 bit architectures. -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [RFC 01/10] iommu: Add IOMMU device registry
-Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Thierry Reding Sent: Friday, June 27, 2014 12:29 PM To: Rob Herring; Pawel Moll; Mark Rutland; Ian Campbell; Kumar Gala; Stephen Warren; Arnd Bergmann; Will Deacon; Joerg Roedel Cc: Olav Haugan; devicet...@vger.kernel.org; Grant Grundler; Rhyland Klein; iommu@lists.linux-foundation.org; linux-ker...@vger.kernel.org; Marc Zyngier; Allen Martin; Paul Walmsley; linux-te...@vger.kernel.org; Cho KyongHo; Dave Martin; linux-arm-ker...@lists.infradead.org Subject: Re: [RFC 01/10] iommu: Add IOMMU device registry On Thu, Jun 26, 2014 at 10:49:41PM +0200, Thierry Reding wrote: From: Thierry Reding tred...@nvidia.com Add an IOMMU device registry for drivers to register with and implement a method for users of the IOMMU API to attach to an IOMMU device. This allows to support deferred probing and gives the IOMMU API a convenient hook to perform early initialization of a device if necessary. Signed-off-by: Thierry Reding tred...@nvidia.com --- drivers/iommu/iommu.c | 93 +++ include/linux/iommu.h | 27 +++ 2 files changed, 120 insertions(+) I thought that perhaps I should elaborate on this a bit since I have a few ideas on how the API could be enhanced. +static int of_iommu_attach(struct device *dev) { + struct of_phandle_iter iter; + struct iommu *iommu; + + mutex_lock(iommus_lock); + + of_property_for_each_phandle_with_args(iter, dev-of_node, iommus, + #iommu-cells, 0) { + bool found = false; + int err; + + /* skip disabled IOMMUs */ + if (!of_device_is_available(iter.out_args.np)) + continue; + + list_for_each_entry(iommu, iommus, list) { + if (iommu-dev-of_node == iter.out_args.np) { + err = iommu-ops-attach(iommu, dev); + if (err 0) { + } + + found = true; + } + } + + if (!found) { + mutex_unlock(iommus_lock); + return -EPROBE_DEFER; + } + } + + mutex_unlock(iommus_lock); + + return 0; +} + +static int of_iommu_detach(struct device *dev) { + /* TODO: implement */ + return -ENOSYS; +} + +int iommu_attach(struct device *dev) +{ + int err = 0; + + if (IS_ENABLED(CONFIG_OF) dev-of_node) { + err = of_iommu_attach(dev); + if (!err) + return 0; + } + + return err; +} +EXPORT_SYMBOL_GPL(iommu_attach); I think it might make sense to introduce an explicit object for an IOMMU master attachment. Maybe something like: struct iommu_master { struct iommu *iommu; struct device *dev; ... }; iommu_attach() could then return a pointer to that attachment and the IOMMU user driver could subsequently use that as a handle to access other parts of the API. The reason is that if we ever need to support more than a single master interface (and perhaps even multiple master interfaces on different IOMMUs) for a single device, then we need a way for the IOMMU user to differentiate between its master interfaces. diff --git a/include/linux/iommu.h b/include/linux/iommu.h index 284a4683fdc1..ac2ceef194d4 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -43,6 +43,17 @@ struct notifier_block; typedef int (*iommu_fault_handler_t)(struct iommu_domain *, struct device *, unsigned long, int, void *); +struct iommu { + struct device *dev; + + struct list_head list; + + const struct iommu_ops *ops; +}; For reasons explained above, I also think that it would be a good idea to modify the iommu_ops functions to take a struct iommu * as their first argument. This may become important when one driver needs to support multiple IOMMU devices. With the current API drivers have to rely on global variables to track the driver-specific context. As far as I can tell, only .domain_init(), .add_device(), .remove_device() and .device_group(). .domain_init() could set up a pointer to struct iommu in struct iommu_domain so the functions dealing with domains could gain access to the IOMMU device via that pointer. Would the proposed interface be an alternate to the add_device interface? Also, how would the iommu group creation work? We are dependent on device driver initialization to attach device an IOMMU, but the add_device allows creation of iommu_groups during bus probing. Can't the same thing be achieved using the add device interface where an IOMMU driver can determine (in
RE: [PATCH 1/3] iommu/fsl: Fix PAMU window size check.
Thanks Emil. -Original Message- From: Emil Medve [mailto:emilian.me...@freescale.com] Sent: Wednesday, July 02, 2014 1:16 PM To: Sethi Varun-B16395; iommu@lists.linux-foundation.org; j...@8bytes.org; linux-ker...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; alex.william...@redhat.com Subject: Re: [PATCH 1/3] iommu/fsl: Fix PAMU window size check. On 06/24/2014 08:57 AM, Varun Sethi wrote: is_power_of_2 requires an unsigned long parameter which would lead to truncation of 64 bit values on 32 bit architectures. __ffs also expects an unsigned long parameter thus won't work for 64 bit values on 32 bit architectures. Signed-off-by: Varun Sethi varun.se...@freescale.com --- drivers/iommu/fsl_pamu.c|8 drivers/iommu/fsl_pamu_domain.c |2 +- 2 files changed, 5 insertions(+), 5 deletions(-) Tested-by: Emil Medve emilian.me...@freescale.com On a P4080 DS (i.e. 32-bit SoC) Cheers, ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 1/1] iommu/fsl_pamu: Remove unnecessary null test before kfree
-Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Fabian Frederick Sent: Sunday, June 29, 2014 1:31 PM To: linux-ker...@vger.kernel.org Cc: Fabian Frederick; Grant Likely; iommu@lists.linux-foundation.org; devicet...@vger.kernel.org Subject: [PATCH 1/1] iommu/fsl_pamu: Remove unnecessary null test before kfree Fix checkpatch warning: WARNING: kfree(NULL) is safe this check is probably not required Cc: Joerg Roedel j...@8bytes.org Cc: Grant Likely grant.lik...@linaro.org Cc: iommu@lists.linux-foundation.org Cc: devicet...@vger.kernel.org Signed-off-by: Fabian Frederick f...@skynet.be --- drivers/iommu/fsl_pamu_domain.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c index 93072ba..0009dff 100644 --- a/drivers/iommu/fsl_pamu_domain.c +++ b/drivers/iommu/fsl_pamu_domain.c @@ -1118,8 +1118,7 @@ static int fsl_pamu_set_windows(struct iommu_domain *domain, u32 w_count) ret = pamu_set_domain_geometry(dma_domain, domain-geometry, ((w_count 1) ? w_count : 0)); if (!ret) { - if (dma_domain-win_arr) - kfree(dma_domain-win_arr); + kfree(dma_domain-win_arr); dma_domain-win_arr = kzalloc(sizeof(struct dma_window) * w_count, GFP_ATOMIC); if (!dma_domain-win_arr) { Acked-by: Varun Sethi varun.se...@freescale.com ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 0/3] iommu/fsl: Fixes for the PAMU driver.
This patch set contains fixes for the PAMU driver. The patches are based on 3.16-rc1. Varun Sethi (3): Fix PAMU window size check. Fix the device domain attach condition. Fix the error condition during iommu group creation. drivers/iommu/fsl_pamu.c|8 drivers/iommu/fsl_pamu_domain.c | 19 +-- 2 files changed, 13 insertions(+), 14 deletions(-) -- 1.7.9.5 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/3] iommu/fsl: Fix PAMU window size check.
is_power_of_2 requires an unsigned long parameter which would lead to truncation of 64 bit values on 32 bit architectures. __ffs also expects an unsigned long parameter thus won't work for 64 bit values on 32 bit architectures. Signed-off-by: Varun Sethi varun.se...@freescale.com --- drivers/iommu/fsl_pamu.c|8 drivers/iommu/fsl_pamu_domain.c |2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c index b99dd88..bb446d7 100644 --- a/drivers/iommu/fsl_pamu.c +++ b/drivers/iommu/fsl_pamu.c @@ -170,10 +170,10 @@ int pamu_disable_liodn(int liodn) static unsigned int map_addrspace_size_to_wse(phys_addr_t addrspace_size) { /* Bug if not a power of 2 */ - BUG_ON(!is_power_of_2(addrspace_size)); + BUG_ON((addrspace_size (addrspace_size - 1))); /* window size is 2^(WSE+1) bytes */ - return __ffs(addrspace_size) - 1; + return fls64(addrspace_size) - 2; } /* Derive the PAACE window count encoding for the subwindow count */ @@ -351,7 +351,7 @@ int pamu_config_ppaace(int liodn, phys_addr_t win_addr, phys_addr_t win_size, struct paace *ppaace; unsigned long fspi; - if (!is_power_of_2(win_size) || win_size PAMU_PAGE_SIZE) { + if ((win_size (win_size - 1)) || win_size PAMU_PAGE_SIZE) { pr_debug(window size too small or not a power of two %llx\n, win_size); return -EINVAL; } @@ -464,7 +464,7 @@ int pamu_config_spaace(int liodn, u32 subwin_cnt, u32 subwin, return -ENOENT; } - if (!is_power_of_2(subwin_size) || subwin_size PAMU_PAGE_SIZE) { + if ((subwin_size (subwin_size - 1)) || subwin_size PAMU_PAGE_SIZE) { pr_debug(subwindow size out of range, or not a power of 2\n); return -EINVAL; } diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c index 93072ba..3dd0b8e 100644 --- a/drivers/iommu/fsl_pamu_domain.c +++ b/drivers/iommu/fsl_pamu_domain.c @@ -301,7 +301,7 @@ static int check_size(u64 size, dma_addr_t iova) * Size must be a power of two and at least be equal * to PAMU page size. */ - if (!is_power_of_2(size) || size PAMU_PAGE_SIZE) { + if ((size (size - 1)) || size PAMU_PAGE_SIZE) { pr_debug(%s: size too small or not a power of two\n, __func__); return -EINVAL; } -- 1.7.9.5 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 3/3] iommu/fsl: Fix the error condition during iommu group
Earlier PTR_ERR was being returned even if group was set to null. Now, we explicitly set an ERR_PTR value in case the group pointer is NULL. Signed-off-by: Varun Sethi varun.se...@freescale.com --- drivers/iommu/fsl_pamu_domain.c |7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c index 54060d1..af47648 100644 --- a/drivers/iommu/fsl_pamu_domain.c +++ b/drivers/iommu/fsl_pamu_domain.c @@ -1037,12 +1037,15 @@ root_bus: group = get_shared_pci_device_group(pdev); } + if (!group) + group = ERR_PTR(-ENODEV); + return group; } static int fsl_pamu_add_device(struct device *dev) { - struct iommu_group *group = NULL; + struct iommu_group *group = ERR_PTR(-ENODEV); struct pci_dev *pdev; const u32 *prop; int ret, len; @@ -1065,7 +1068,7 @@ static int fsl_pamu_add_device(struct device *dev) group = get_device_iommu_group(dev); } - if (!group || IS_ERR(group)) + if (IS_ERR(group)) return PTR_ERR(group); ret = iommu_group_add_device(group, dev); -- 1.7.9.5 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/3] iommu/fsl: Fix the device domain attach condition.
___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v2] devicetree: Add generic IOMMU device tree bindings
-Original Message- From: Yoder Stuart-B08248 Sent: Tuesday, June 17, 2014 12:24 AM To: Will Deacon Cc: Sethi Varun-B16395; Thierry Reding; Mark Rutland; devicet...@vger.kernel.org; linux-samsung-...@vger.kernel.org; Pawel Moll; Arnd Bergmann; Ian Campbell; Grant Grundler; Stephen Warren; linux- ker...@vger.kernel.org; Marc Zyngier; Linux IOMMU; Rob Herring; Kumar Gala; linux-te...@vger.kernel.org; Cho KyongHo; Dave P Martin; linux-arm- ker...@lists.infradead.org Subject: RE: [PATCH v2] devicetree: Add generic IOMMU device tree bindings -Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Monday, June 16, 2014 12:04 PM To: Yoder Stuart-B08248 Cc: Sethi Varun-B16395; Thierry Reding; Mark Rutland; devicet...@vger.kernel.org; linux-samsung-...@vger.kernel.org; Pawel Moll; Arnd Bergmann; Ian Campbell; Grant Grundler; Stephen Warren; linux- ker...@vger.kernel.org; Marc Zyngier; Linux IOMMU; Rob Herring; Kumar Gala; linux-te...@vger.kernel.org; Cho KyongHo; Dave P Martin; linux-arm- ker...@lists.infradead.org Subject: Re: [PATCH v2] devicetree: Add generic IOMMU device tree bindings Hi Stuart, On Mon, Jun 16, 2014 at 05:56:32PM +0100, Stuart Yoder wrote: Do you have use-cases where you really need to change these mappings dynamically? Yes. In the case of a PCI bus-- you may not know in advance how many PCI devices there are until you probe the bus. We have another FSL proprietary bus we call the fsl-mc bus that is similar. For that case, though, you could still describe an algorithmic transformation from RequesterID to StreamID which corresponds to a fixed mapping. Another thing to consider-- starting with SMMUv2, as you know, there is a new distributed architecture with multiple TBUs and a centralized TCU that walks the SMMU page tables. So instead of sprinkling multiple SMMUs all over an SoC you now have the option a 1 central TCU and sprinkling multiple TBUs around. However, this means that the stream ID namespace is now global and can be pretty limited. In the SMMU implementation we have there are only 64 stream ID total for our Soc. But we have many more masters than that. So we look at stream IDs as really corresponding to an 'isolation context' and not to a bus master. An isolation context is the domain you are trying to isolate with the SMMU. Devices that all belong to the same 'isolation context' can share the same stream ID, since they share the same domain and page tables. Ok, this is more compelling. So, perhaps by default some/most SMMU masters may have a default stream ID of 0x0 that is used by the host...and that could be represented statically in the device tree. But, we absolutely will need to dynamically set new stream IDs into masters when a new IOMMU 'domain' is created and devices are added to it. All the devices in a domain will share the same stream ID. So whatever we do, let's please have an architecture flexible enough to allow for this. What is the software interface to the logic that assigns the StreamIDs? Is it part of the SMMU, or a separate device (or set of devices)? For us at the hardware level there are a few different ways that the streamIDs can be set. It is not part of the SMMU. In the cases where there is simply 1 device to 1 streamID (e.g. USB controller) there is an SoC register where you just program the stream ID. In the case of PCI, our PCI controller has a RequesterID-to-streamID table that you set via some PCI controller registers. The way we generally thought it would work was something like this: -u-boot/bootloader makes any static streamID allocation if needed, sets a default streamID (e.g. 0x0) in device and expresses that in the device tree -device tree would express relationship between devices (including bus controllers) and the SMMU through mmu-masters property -u-boot would express the range of unused (or used) streamIDs via a new device tree property so the kernel SMMU driver knows what streamIDs are free -in the SMMU driver a different vendor specific 'add_device' callback could be used to handle our special cases where we need to set/change the stream ID for devices added to a domain Another possibility, could be to program the stream Id in the device registers (reference for the stream ID register can be obtained from the device tree) during device attach. This could be relevant in case of VFIO, when we are assigning multiple devices to a single VM. All the devices can share the same stream ID. -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [RFC PATCH v6 04/20] iommu/arm-smmu: add capability IOMMU_CAP_INTR_REMAP
-Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Antonios Motakis Sent: Thursday, June 05, 2014 10:33 PM To: alex.william...@redhat.com; kvm...@lists.cs.columbia.edu; iommu@lists.linux-foundation.org Cc: k...@vger.kernel.org; eric.au...@linaro.org; open list; will.dea...@arm.com; a.r...@virtualopensystems.com; Yoder Stuart-B08248; moderated list:ARM SMMU DRIVER; Antonios Motakis; t...@virtualopensystems.com; christoffer.d...@linaro.org Subject: [RFC PATCH v6 04/20] iommu/arm-smmu: add capability IOMMU_CAP_INTR_REMAP With an ARM SMMU, interrupt remapping should always be safe from the SMMU's point of view, as it is properly handled by the GIC. Signed-off-by: Antonios Motakis a.mota...@virtualopensystems.com --- 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 15ab2af..ff29402 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -1544,7 +1544,7 @@ static int arm_smmu_domain_has_cap(struct iommu_domain *domain, if (smmu_domain-root_cfg.smmu-features ARM_SMMU_FEAT_COHERENT_WALK) caps |= IOMMU_CAP_CACHE_COHERENCY; - caps |= IOMMU_CAP_NOEXEC; + caps |= IOMMU_CAP_NOEXEC | IOMMU_CAP_INTR_REMAP; return !!(cap caps); } Why mention this as an IOMMU capability when IOMMU doesn't support it? Also, wouldn't this depend on the GIC IP version? What are we gaining by adding this IOMMU capability list? -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v2] devicetree: Add generic IOMMU device tree bindings
-Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Will Deacon Sent: Wednesday, June 04, 2014 10:12 PM To: Thierry Reding Cc: Mark Rutland; devicet...@vger.kernel.org; linux-samsung- s...@vger.kernel.org; Pawel Moll; Arnd Bergmann; Ian Campbell; Grant Grundler; Stephen Warren; linux-ker...@vger.kernel.org; Marc Zyngier; Linux IOMMU; Rob Herring; Kumar Gala; linux-te...@vger.kernel.org; Cho KyongHo; Dave P Martin; linux-arm-ker...@lists.infradead.org Subject: Re: [PATCH v2] devicetree: Add generic IOMMU device tree bindings On Wed, Jun 04, 2014 at 03:35:10PM +0100, Thierry Reding wrote: On Mon, Jun 02, 2014 at 11:41:04AM +0100, Dave Martin wrote: In the strictest sense, no. But for a large set of sane configurations, this probably works. Small sets of randomly-assigned IDs can just be enumerated one by one. We wouldn't be able to describe folding and bit shuffling, but we probably don't want to encourage that anyway. I'm having some difficulty understanding this. You make it sound like there's a fairly arbitrary number of IDs that the SMMU can handle. So how is the mapping to devices defined? If you say encourage that does make it sound like the assignment of IDs is purely defined by some mechanism in software rather than in hardware. Or they are more or less randomly picked by someone. If that's the case, is that not something that should be dynamically allocated by the kernel rather than put into the device tree? The set of StreamIDs that can be generated by a master is fixed in the hardware. The SMMU can then be programmed to map these incoming IDs onto a context ID (or a set of context IDs), which are the IDs used internally by the SMMU to find the page tables etc. The StreamID - ContextID mapping is dynamic and controlled by software. The Master - StreamIDs mapping is fixed in the hardware. The Master - StreamIDs mapping may not always be fixed in the hardware. At, least in our case we plan to program these via software. PCI devices is one place where this mapping would have to be dynamic (based on the topology i.e. if the devices are connected to a bridge etc). Also, we have a hot plug device architecture where the stream ID is software programmable. Other than that, based on the isolation requirements (iommu domain) software programmability offers greater flexibility. -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [RFC][PATCH] iommu/arm: Add hotplugged devices support for arm-smmu.
Hi Ritesh, We have a hot plug bus architecture for the Layerscape SOCs. I am testing the patch specifically for this architecture on the AFM simulator. Regards Varun -Original Message- From: Ritesh Harjani [mailto:ritesh.harj...@gmail.com] Sent: Wednesday, March 12, 2014 10:00 PM To: Will Deacon Cc: Sethi Varun-B16395; iommu@lists.linux-foundation.org Subject: Re: [RFC][PATCH] iommu/arm: Add hotplugged devices support for arm-smmu. Hi Varun, How are you testing this patch of arm-smmu ? What is your test environment ? On Wed, Mar 12, 2014 at 9:53 PM, Will Deacon will.dea...@arm.com wrote: Hi Varun, On Sat, Mar 08, 2014 at 07:05:40PM +, Varun Sethi wrote: Currently the ARM SMMU driver only considers the bus master devices in the device tree. The master device and stream ID information is maintained per SMMU. Currently there is no mechanism for representing this information in case of PCI or hot plugged devices. This RFC patch proposes a mechanism for representing this information for hot plugged/PCI devices. Patch doesn't contain the add_device callback modification for hot plug devices. This would be bus specific and would be responsible for populating the hot plug devices masters list for the SMMU. Signed-off-by: Varun Sethi varun.se...@freescale.com --- drivers/iommu/arm-smmu.c | 93 -- 1 file changed, 90 insertions(+), 3 deletions(-) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 1d9ab39..6c10df4 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -327,6 +327,9 @@ struct arm_smmu_master { * SMMU chain. */ struct rb_node node; + /* Following fields correspond to the hot plug masters */ + struct list_headhotplug_masters_node; + struct device *dev; I'd much rather have one list for all masters. Distinguishing between masters and `hotplug masters' is unnecessary. +static int is_device_hotplug(struct device *dev) { + return (dev-bus != platform_bus_type) + (dev-bus != amba_bustype); } I'm not fond of this. Why not rework what we currently have so that it can work for other (hotpluggable) buses? Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC][PATCH] iommu/arm: Add hotplugged devices support for arm-smmu.
Currently the ARM SMMU driver only considers the bus master devices in the device tree. The master device and stream ID information is maintained per SMMU. Currently there is no mechanism for representing this information in case of PCI or hot plugged devices. This RFC patch proposes a mechanism for representing this information for hot plugged/PCI devices. Patch doesn't contain the add_device callback modification for hot plug devices. This would be bus specific and would be responsible for populating the hot plug devices masters list for the SMMU. Signed-off-by: Varun Sethi varun.se...@freescale.com --- drivers/iommu/arm-smmu.c | 93 -- 1 file changed, 90 insertions(+), 3 deletions(-) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 1d9ab39..6c10df4 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -327,6 +327,9 @@ struct arm_smmu_master { * SMMU chain. */ struct rb_node node; + /* Following fields correspond to the hot plug masters */ + struct list_headhotplug_masters_node; + struct device *dev; int num_streamids; u16 streamids[MAX_MASTER_STREAMIDS]; @@ -371,6 +374,12 @@ struct arm_smmu_device { struct list_headlist; struct rb_root masters; + /* +* Hot plug master list and lock to +* protect the list. +*/ + struct list_headhotplug_masters_list; + spinlock_t lock; }; struct arm_smmu_cfg { @@ -401,6 +410,31 @@ struct arm_smmu_domain { static DEFINE_SPINLOCK(arm_smmu_devices_lock); static LIST_HEAD(arm_smmu_devices); +static int is_device_hotplug(struct device *dev) +{ + return (dev-bus != platform_bus_type) +(dev-bus != amba_bustype); +} + +static struct arm_smmu_master *find_smmu_hotplug_master(struct arm_smmu_device *smmu, + struct device *dev) +{ + struct arm_smmu_master *master; + unsigned long flags; + bool found = 0; + + spin_lock_irqsave(smmu-lock, flags); + list_for_each_entry(master, smmu-hotplug_masters_list, + hotplug_masters_node) + if (master-dev == dev) { + found = 1; + break; + } + spin_unlock_irqrestore(smmu-lock, flags); + + return found ? master : NULL; +} + static struct arm_smmu_master *find_smmu_master(struct arm_smmu_device *smmu, struct device_node *dev_node) { @@ -421,6 +455,36 @@ static struct arm_smmu_master *find_smmu_master(struct arm_smmu_device *smmu, return NULL; } +static int remove_smmu_hotplug_master(struct arm_smmu_device *smmu, + struct device *dev) +{ + unsigned long flags; + struct arm_smmu_master *master, *tmp; + bool found = 0; + + spin_lock_irqsave(smmu-lock, flags); + list_for_each_entry_safe(master, tmp, smmu-hotplug_masters_list, +hotplug_masters_node) + if (master-dev == dev) { + found = 1; + list_del(master-hotplug_masters_node); + break; + } + spin_unlock_irqrestore(smmu-lock, flags); + + return found ? 0 : -ENODEV; +} + +static void insert_smmu_hotplug_master(struct arm_smmu_device *smmu, + struct arm_smmu_master *master) +{ + unsigned long flags; + + spin_lock_irqsave(smmu-lock, flags); + list_add(master-hotplug_masters_node, smmu-hotplug_masters_list); + spin_unlock_irqrestore(smmu-lock, flags); +} + static int insert_smmu_master(struct arm_smmu_device *smmu, struct arm_smmu_master *master) { @@ -816,6 +880,15 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain) writel_relaxed(reg, cb_base + ARM_SMMU_CB_SCTLR); } +static struct arm_smmu_master *get_smmu_master(struct arm_smmu_device *smmu, + struct device *dev) +{ + if (is_device_hotplug(dev)) + return find_smmu_hotplug_master(smmu, dev); + else + return find_smmu_master(smmu, dev-of_node); +} + static int arm_smmu_init_domain_context(struct iommu_domain *domain, struct device *dev) { @@ -837,7 +910,7 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain, smmu_domain-output_mask = (1ULL smmu-s2_output_size) - 1; } while ((parent = find_parent_smmu(smmu))); - if (!find_smmu_master(smmu, dev-of_node)) { + if (!get_smmu_master
RE: [RFC PATCH] vfio/iommu_type1: Multi-IOMMU domain support
-Original Message- From: Kai Huang [mailto:dev.kai.hu...@gmail.com] Sent: Monday, January 27, 2014 5:50 AM To: Sethi Varun-B16395 Cc: Alex Williamson; iommu@lists.linux-foundation.org; linux- ker...@vger.kernel.org Subject: Re: [RFC PATCH] vfio/iommu_type1: Multi-IOMMU domain support On Tue, Jan 21, 2014 at 2:30 AM, Varun Sethi varun.se...@freescale.com wrote: -Original Message- From: Alex Williamson [mailto:alex.william...@redhat.com] Sent: Monday, January 20, 2014 9:51 PM To: Sethi Varun-B16395 Cc: iommu@lists.linux-foundation.org; linux-ker...@vger.kernel.org Subject: Re: [RFC PATCH] vfio/iommu_type1: Multi-IOMMU domain support On Mon, 2014-01-20 at 14:45 +, Varun Sethi wrote: -Original Message- From: Alex Williamson [mailto:alex.william...@redhat.com] Sent: Saturday, January 18, 2014 2:06 AM To: Sethi Varun-B16395 Cc: iommu@lists.linux-foundation.org; linux-ker...@vger.kernel.org Subject: [RFC PATCH] vfio/iommu_type1: Multi-IOMMU domain support RFC: This is not complete but I want to share with Varun the dirrection I'm thinking about. In particular, I'm really not sure if we want to introduce a v2 interface version with slightly different unmap semantics. QEMU doesn't care about the difference, but other users might. Be warned, I'm not even sure if this code works at the moment. Thanks, Alex We currently have a problem that we cannot support advanced features of an IOMMU domain (ex. IOMMU_CACHE), because we have no guarantee that those features will be supported by all of the hardware units involved with the domain over its lifetime. For instance, the Intel VT-d architecture does not require that all DRHDs support snoop control. If we create a domain based on a device behind a DRHD that does support snoop control and enable SNP support via the IOMMU_CACHE mapping option, we cannot then add a device behind a DRHD which does not support snoop control or we'll get reserved bit faults from the SNP bit in the pagetables. To add to the complexity, we can't know the properties of a domain until a device is attached. [Sethi Varun-B16395] Effectively, it's the same iommu and iommu_ops are common across all bus types. The hardware feature differences are abstracted by the driver. That's a simplifying assumption that is not made anywhere else in the code. The IOMMU API allows entirely independent IOMMU drivers to register per bus_type. There is no guarantee that all devices are backed by the same IOMMU hardware unit or make use of the same iommu_ops. [Sethi Varun-B16395] ok We could pass this problem off to userspace and require that a separate vfio container be used, but we don't know how to handle page accounting in that case. How do we know that a page pinned in one container is the same page as a different container and avoid double billing the user for the page. The solution is therefore to support multiple IOMMU domains per container. In the majority of cases, only one domain will be required since hardware is typically consistent within a system. However, this provides us the ability to validate compatibility of domains and support mixed environments where page table flags can be different between domains. To do this, our DMA tracking needs to change. We currently try to coalesce user mappings into as few tracking entries as possible. The problem then becomes that we lose granularity of user mappings. We've never guaranteed that a user is able to unmap at a finer granularity than the original mapping, but we must honor the granularity of the original mapping. This coalescing code is therefore removed, allowing only unmaps covering complete maps. The change in accounting is fairly small here, a typical QEMU VM will start out with roughly a dozen entries, so it's arguable if this coalescing was ever needed. We also move IOMMU domain creation to the point where a group is attached to the container. An interesting side-effect of this is that we now have access to the device at the time of domain creation and can probe the devices within the group to determine the bus_type. This finally makes vfio_iommu_type1 completely device/bus agnostic. In fact, each IOMMU domain can host devices on different buses managed by different physical IOMMUs, and present a single DMA mapping interface to the user. When a new domain is created, mappings are replayed to bring the IOMMU pagetables up to the state of the current container. And of course, DMA mapping and unmapping automatically traverse all of the configured IOMMU domains. [Sethi Varun-B16395] This code still checks to see that devices being attached to the domain are connected
RE: [PATCH v3 02/11] iommu/arm-smmu: Introduce iommu_group notifier block
-Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Wednesday, January 22, 2014 5:56 PM To: Sethi Varun-B16395 Cc: Andreas Herrmann; iommu@lists.linux-foundation.org; linux-arm- ker...@lists.infradead.org; Andreas Herrmann Subject: Re: [PATCH v3 02/11] iommu/arm-smmu: Introduce iommu_group notifier block Hi Varun, Andreas, On Tue, Jan 21, 2014 at 05:48:02PM +, Varun Sethi wrote: +static int arm_smmu_group_notifier(struct notifier_block *nb, + unsigned long action, void *data) { + struct device *dev = data; + struct dma_iommu_mapping *mapping; + struct arm_smmu_device *smmu; + int ret; + + switch (action) { + case IOMMU_GROUP_NOTIFY_BIND_DRIVER: + + smmu = dev-archdata.iommu; + if (!smmu || !(smmu-options ARM_SMMU_OPT_ISOLATE_DEVICES)) + break; [Sethi Varun-B16395] Should this check be really done here? The Isolate devices property would allow us to set up iommu groups. My understanding is that if we specify the isolate devices property, then each device would have a separate iommu group otherwise all devices connected to the SMMU would share the iommu group. That's not what currently happens (at least, in the patch I have queued for groups). The code queued adds each device to its own group in arm_smmu_add_device, which I think is the right thing to do. With that logic, we should link the mapping to the iommu group. Ok, so are you suggesting that we perform the isolation mapping in arm_smmu_add_device and drop the notifier altogether? I think that should be fine, until we want to delay mapping creation till driver bind time. But the isolate device property should dictate iommu group creation. -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v3 02/11] iommu/arm-smmu: Introduce iommu_group notifier block
-Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Wednesday, January 22, 2014 9:04 PM To: Sethi Varun-B16395 Cc: Andreas Herrmann; iommu@lists.linux-foundation.org; linux-arm- ker...@lists.infradead.org; Andreas Herrmann Subject: Re: [PATCH v3 02/11] iommu/arm-smmu: Introduce iommu_group notifier block On Wed, Jan 22, 2014 at 01:54:11PM +, Varun Sethi wrote: Ok, so are you suggesting that we perform the isolation mapping in arm_smmu_add_device and drop the notifier altogether? I think that should be fine, until we want to delay mapping creation till driver bind time. Is there a hard dependency on that? Not sure, may be Andreas can answer that. Ok. Andreas? I would have thought doing this *earlier* shouldn't be a problem (the DMA ops must be swizzled before the driver is probed). But the isolate device property should dictate iommu group creation. The reason we added automatic group creation (per-device) is for VFIO, which expects all devices to be in a group regardless of the device isolation configuration. IIUC, with the isolate devices property we ensure that there would be independent SMR and S2CR per device. Is that correct? Yes, as part of giving them independent sets of page tables. Initially these tables don't have any valid mappings, but the dma-mapping API will populate them in response to dma_map_*/dma_alloc/etc. Not sure what this has to do with us putting devices into their own groups... [Sethi Varun-B16395] Devices in an iommu group would share the dma mapping, so shouldn't they be sharing the SMR and context registers? In case of the isolate devices property, each device would have its own SMR and context registers, thus allowing the devices to have independent mappings and allowing them to fall in separate iommu groups. -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v3 02/11] iommu/arm-smmu: Introduce iommu_group notifier block
-Original Message- From: Andreas Herrmann [mailto:andreas.herrm...@calxeda.com] Sent: Tuesday, January 21, 2014 3:58 AM To: Will Deacon Cc: iommu@lists.linux-foundation.org; linux-arm- ker...@lists.infradead.org; Andreas Herrmann; Sethi Varun-B16395 Subject: [PATCH v3 02/11] iommu/arm-smmu: Introduce iommu_group notifier block At the moment just handle IOMMU_GROUP_NOTIFY_BIND_DRIVER to conditionally isolate all master devices for an SMMU. Depending on DT information each device is put into its own protection domain (if possible). For configuration with one or just a few masters per SMMU that is easy to achieve. In case of many devices per SMMU (e.g. MMU-500 with it's distributed translation support) isolation of each device might not be possible -- depending on number of available SMR groups and/or context banks. Default is that device isolation is contolled per SMMU with SMMU node property arm,smmu-isolate-devices in a DT. If this property is set for an SMMU node, device isolation is performed. W/o device isolation the driver detects SMMUs but no translation is configured (transactions just bypass translation process). Note that for device isolation dma_base and size are fixed as 0 and SZ_128M at the moment. Additional patches will address this restriction and allow automatic growth of mapping size. Cc: Varun Sethi varun.se...@freescale.com Cc: Andreas Herrmann herrmann.der.u...@googlemail.com Signed-off-by: Andreas Herrmann andreas.herrm...@calxeda.com --- drivers/iommu/arm-smmu.c | 44 1 file changed, 44 insertions(+) Hi Will, This new patch addresses Varun's comments: - use iommu_group notifier instead of bus notifier - remove superfluous call to arm_smmu_add_device in notifier function This patch depends on commit iommu/arm-smmu: add devices attached to the SMMU to an IOMMU group as found in your git tree (e.g. in branch iommu/devel or for-joerg/arm-smmu/updates). Andreas PS: This time with a proper adaption of the notifier function. diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 0a5649f..da19bd6 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -46,6 +46,7 @@ #include linux/amba/bus.h #include asm/pgalloc.h +#include asm/dma-iommu.h /* Driver options */ #define ARM_SMMU_OPT_ISOLATE_DEVICES (1 0) @@ -1517,6 +1518,47 @@ static int arm_smmu_domain_has_cap(struct iommu_domain *domain, return !!(cap caps); } +static int arm_smmu_group_notifier(struct notifier_block *nb, + unsigned long action, void *data) +{ + struct device *dev = data; + struct dma_iommu_mapping *mapping; + struct arm_smmu_device *smmu; + int ret; + + switch (action) { + case IOMMU_GROUP_NOTIFY_BIND_DRIVER: + + smmu = dev-archdata.iommu; + if (!smmu || !(smmu-options ARM_SMMU_OPT_ISOLATE_DEVICES)) + break; [Sethi Varun-B16395] Should this check be really done here? The Isolate devices property would allow us to set up iommu groups. My understanding is that if we specify the isolate devices property, then each device would have a separate iommu group otherwise all devices connected to the SMMU would share the iommu group. With that logic, we should link the mapping to the iommu group. -Varun + + mapping = arm_iommu_create_mapping(platform_bus_type, + 0, SZ_128M, 0); + if (IS_ERR(mapping)) { + ret = PTR_ERR(mapping); + dev_info(dev, arm_iommu_create_mapping failed\n); + break; + } + + ret = arm_iommu_attach_device(dev, mapping); + if (ret 0) { + dev_info(dev, arm_iommu_attach_device failed\n); + arm_iommu_release_mapping(mapping); + } + + break; + default: + break; + } + + return 0; +} + +static struct notifier_block group_nb = { + .notifier_call = arm_smmu_group_notifier, }; + static int arm_smmu_add_device(struct device *dev) { struct arm_smmu_device *child, *parent, *smmu; @@ -1566,6 +1608,8 @@ static int arm_smmu_add_device(struct device *dev) return PTR_ERR(group); } + iommu_group_register_notifier(group, group_nb); + ret = iommu_group_add_device(group, dev); iommu_group_put(group); dev-archdata.iommu = smmu; -- 1.7.9.5 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [RFC PATCH] vfio/iommu_type1: Multi-IOMMU domain support
-Original Message- From: Alex Williamson [mailto:alex.william...@redhat.com] Sent: Saturday, January 18, 2014 2:06 AM To: Sethi Varun-B16395 Cc: iommu@lists.linux-foundation.org; linux-ker...@vger.kernel.org Subject: [RFC PATCH] vfio/iommu_type1: Multi-IOMMU domain support RFC: This is not complete but I want to share with Varun the dirrection I'm thinking about. In particular, I'm really not sure if we want to introduce a v2 interface version with slightly different unmap semantics. QEMU doesn't care about the difference, but other users might. Be warned, I'm not even sure if this code works at the moment. Thanks, Alex We currently have a problem that we cannot support advanced features of an IOMMU domain (ex. IOMMU_CACHE), because we have no guarantee that those features will be supported by all of the hardware units involved with the domain over its lifetime. For instance, the Intel VT-d architecture does not require that all DRHDs support snoop control. If we create a domain based on a device behind a DRHD that does support snoop control and enable SNP support via the IOMMU_CACHE mapping option, we cannot then add a device behind a DRHD which does not support snoop control or we'll get reserved bit faults from the SNP bit in the pagetables. To add to the complexity, we can't know the properties of a domain until a device is attached. [Sethi Varun-B16395] Effectively, it's the same iommu and iommu_ops are common across all bus types. The hardware feature differences are abstracted by the driver. We could pass this problem off to userspace and require that a separate vfio container be used, but we don't know how to handle page accounting in that case. How do we know that a page pinned in one container is the same page as a different container and avoid double billing the user for the page. The solution is therefore to support multiple IOMMU domains per container. In the majority of cases, only one domain will be required since hardware is typically consistent within a system. However, this provides us the ability to validate compatibility of domains and support mixed environments where page table flags can be different between domains. To do this, our DMA tracking needs to change. We currently try to coalesce user mappings into as few tracking entries as possible. The problem then becomes that we lose granularity of user mappings. We've never guaranteed that a user is able to unmap at a finer granularity than the original mapping, but we must honor the granularity of the original mapping. This coalescing code is therefore removed, allowing only unmaps covering complete maps. The change in accounting is fairly small here, a typical QEMU VM will start out with roughly a dozen entries, so it's arguable if this coalescing was ever needed. We also move IOMMU domain creation to the point where a group is attached to the container. An interesting side-effect of this is that we now have access to the device at the time of domain creation and can probe the devices within the group to determine the bus_type. This finally makes vfio_iommu_type1 completely device/bus agnostic. In fact, each IOMMU domain can host devices on different buses managed by different physical IOMMUs, and present a single DMA mapping interface to the user. When a new domain is created, mappings are replayed to bring the IOMMU pagetables up to the state of the current container. And of course, DMA mapping and unmapping automatically traverse all of the configured IOMMU domains. [Sethi Varun-B16395] This code still checks to see that devices being attached to the domain are connected to the same bus type. If we intend to merge devices from different bus types but attached to compatible domains in to a single domain, why can't we avoid the bus check? Why can't we remove the bus dependency from domain allocation? Regards Varun Signed-off-by: Alex Williamson alex.william...@redhat.com --- drivers/vfio/vfio_iommu_type1.c | 631 - -- include/uapi/linux/vfio.h |1 2 files changed, 329 insertions(+), 303 deletions(-) diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c index 4fb7a8f..983aae5 100644 --- a/drivers/vfio/vfio_iommu_type1.c +++ b/drivers/vfio/vfio_iommu_type1.c @@ -30,7 +30,6 @@ #include linux/iommu.h #include linux/module.h #include linux/mm.h -#include linux/pci.h /* pci_bus_type */ #include linux/rbtree.h #include linux/sched.h #include linux/slab.h @@ -55,11 +54,18 @@ MODULE_PARM_DESC(disable_hugepages, Disable VFIO IOMMU support for IOMMU hugepages.); struct vfio_iommu { - struct iommu_domain *domain; + struct list_headdomain_list; struct mutexlock; struct rb_root dma_list; + bool v2; +}; + +struct vfio_domain { + struct
RE: [PATCH 02/11] iommu/arm-smmu: Introduce bus notifier block
-Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Andreas Herrmann Sent: Thursday, January 16, 2014 6:14 PM To: Will Deacon Cc: Andreas Herrmann; iommu@lists.linux-foundation.org; linux-arm- ker...@lists.infradead.org Subject: [PATCH 02/11] iommu/arm-smmu: Introduce bus notifier block At the moment just handle BUS_NOTIFY_BIND_DRIVER to conditionally isolate all master devices for an SMMU. Depending on DT information each device is put into its own protection domain (if possible). For configuration with one or just a few masters per SMMU that is easy to achieve. In case of many devices per SMMU (e.g. MMU-500 with it's distributed translation support) isolation of each device might not be possible -- depending on number of available SMR groups and/or context banks. Default is that device isolation is contolled per SMMU with SMMU node property arm,smmu-isolate-devices in a DT. If this property is set for an SMMU node, device isolation is performed. [Sethi Varun-B16395] What if the devices have to be assigned to different virtual machines? Would the absence of this property indicate that devices can't Be assigned to different virtual machines i.e. devices would be in the same iommu group? W/o device isolation the driver detects SMMUs but no translation is configured (transactions just bypass translation process). [Sethi Varun-B16395] Would SMR and S2CR still be allocated? Note that for device isolation dma_base and size are fixed as 0 and SZ_128M at the moment. Additional patches will address this restriction and allow automatic growth of mapping size. Cc: Andreas Herrmann herrmann.der.u...@googlemail.com Signed-off-by: Andreas Herrmann andreas.herrm...@calxeda.com --- drivers/iommu/arm-smmu.c | 45 + 1 file changed, 45 insertions(+) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 0b97d03..bc81dd0 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -46,6 +46,7 @@ #include linux/amba/bus.h #include asm/pgalloc.h +#include asm/dma-iommu.h /* Driver options */ #define ARM_SMMU_OPT_ISOLATE_DEVICES (1 0) @@ -1964,6 +1965,48 @@ static int arm_smmu_device_remove(struct platform_device *pdev) return 0; } +static int arm_smmu_device_notifier(struct notifier_block *nb, + unsigned long action, void *data) +{ + struct device *dev = data; + struct dma_iommu_mapping *mapping; + struct arm_smmu_device *smmu; + int ret; + + switch (action) { + case BUS_NOTIFY_BIND_DRIVER: + + arm_smmu_add_device(dev); [Sethi Varun-B16395] This would have already happened by virtue of the add_device iommu_ops. + smmu = dev-archdata.iommu; + if (!smmu || !(smmu-options ARM_SMMU_OPT_ISOLATE_DEVICES)) + break; + + mapping = arm_iommu_create_mapping(platform_bus_type, + 0, SZ_128M, 0); + if (IS_ERR(mapping)) { + ret = PTR_ERR(mapping); + dev_info(dev, arm_iommu_create_mapping failed\n); + break; + } + + ret = arm_iommu_attach_device(dev, mapping); + if (ret 0) { + dev_info(dev, arm_iommu_attach_device failed\n); + arm_iommu_release_mapping(mapping); + } + + break; + default: + break; + } + + return 0; +} + +static struct notifier_block device_nb = { + .notifier_call = arm_smmu_device_notifier, }; + #ifdef CONFIG_OF static struct of_device_id arm_smmu_of_match[] = { { .compatible = arm,smmu-v1, }, @@ -2000,6 +2043,8 @@ static int __init arm_smmu_init(void) if (!iommu_present(amba_bustype)) bus_set_iommu(amba_bustype, arm_smmu_ops); + bus_register_notifier(platform_bus_type, device_nb); + [Sethi Varun-B16395] Notifier was already registered for the platform bus via bus_set_iommu. You can actually register a iommu group (once iommu group gets created) notifier (iommu_group_register_notifier) and listen for the IOMMU_GROUP_NOTIFY_BIND_DRIVER event. -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC][PATCH] Remove bus dependency for iommu_domain_alloc.
This patch attempts to remove iommu_domain_alloc function's dependency on the bus type. This dependency is quiet restrictive in case of vfio, where it's possible to bind multiple iommu groups (from different bus types) to the same iommu domain. This patch is based on the assumption, that there is a single iommu for all bus types on the system. We maintain a list of bus types (for which iommu ops are registered). In the iommu_domain_alloc function we ensure that all bus types correspond to the same set of iommu operations. Signed-off-by: Varun Sethi varun.se...@freescale.com --- arch/arm/mm/dma-mapping.c |2 +- drivers/gpu/drm/msm/msm_gpu.c |2 +- drivers/iommu/amd_iommu_v2.c |2 +- drivers/iommu/iommu.c | 32 +--- drivers/media/platform/omap3isp/isp.c |2 +- drivers/remoteproc/remoteproc_core.c |2 +- drivers/vfio/vfio_iommu_type1.c |2 +- include/linux/device.h|2 ++ include/linux/iommu.h |4 ++-- virt/kvm/iommu.c |2 +- 10 files changed, 40 insertions(+), 12 deletions(-) diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index f61a570..0e5a5f5 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -1910,7 +1910,7 @@ arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t size, mapping-order = order; spin_lock_init(mapping-lock); - mapping-domain = iommu_domain_alloc(bus); + mapping-domain = iommu_domain_alloc(); if (!mapping-domain) goto err3; diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index 4583d61..3e8636e 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -428,7 +428,7 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev, * and have separate page tables per context. For now, to keep things * simple and to get something working, just use a single address space: */ - gpu-iommu = iommu_domain_alloc(platform_bus_type); + gpu-iommu = iommu_domain_alloc(); if (!gpu-iommu) { dev_err(drm-dev, failed to allocate IOMMU\n); ret = -ENOMEM; diff --git a/drivers/iommu/amd_iommu_v2.c b/drivers/iommu/amd_iommu_v2.c index 5208828..5c0737b 100644 --- a/drivers/iommu/amd_iommu_v2.c +++ b/drivers/iommu/amd_iommu_v2.c @@ -785,7 +785,7 @@ int amd_iommu_init_device(struct pci_dev *pdev, int pasids) if (dev_state-states == NULL) goto out_free_dev_state; - dev_state-domain = iommu_domain_alloc(pci_bus_type); + dev_state-domain = iommu_domain_alloc(); if (dev_state-domain == NULL) goto out_free_states; diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index efc..799371b 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -35,6 +35,9 @@ static struct kset *iommu_group_kset; static struct ida iommu_group_ida; static struct mutex iommu_group_mutex; +static struct list_head bus_list; +static struct mutex bus_list_mutex; + struct iommu_group { struct kobject kobj; struct kobject *devices_kobj; @@ -611,6 +614,10 @@ int bus_set_iommu(struct bus_type *bus, struct iommu_ops *ops) bus-iommu_ops = ops; + mutex_lock(bus_list_mutex); + list_add(bus-bus_list, bus_list); + mutex_unlock(bus_list_mutex); + /* Do IOMMU specific setup for this bus-type */ iommu_bus_init(bus, ops); @@ -647,19 +654,36 @@ void iommu_set_fault_handler(struct iommu_domain *domain, } EXPORT_SYMBOL_GPL(iommu_set_fault_handler); -struct iommu_domain *iommu_domain_alloc(struct bus_type *bus) +struct iommu_domain *iommu_domain_alloc(void) { struct iommu_domain *domain; + struct iommu_ops *ops = NULL; + struct bus_type *bus; int ret; - if (bus == NULL || bus-iommu_ops == NULL) + /* +* Traverse the bus list and verify that same iommu_ops +* are registered for all the bus types. +*/ + mutex_lock(bus_list_mutex); + list_for_each_entry(bus, bus_list, bus_list) { + if (!bus-iommu_ops || (ops (bus-iommu_ops != ops))) { + mutex_unlock(bus_list_mutex); + return NULL; + } else if (!ops) { + ops = bus-iommu_ops; + } + } + mutex_unlock(bus_list_mutex); + + if (!ops) return NULL; domain = kzalloc(sizeof(*domain), GFP_KERNEL); if (!domain) return NULL; - domain-ops = bus-iommu_ops; + domain-ops = ops; ret = domain-ops-domain_init(domain); if (ret) @@ -924,6 +948,8 @@ static int __init iommu_init(void) NULL, kernel_kobj); ida_init
RE: [PATCH] iommu/fsl_pamu: use physical cpu index to find the matched cpu nodes
For the DSP case again we have to set up the stash attribute. Are you saying that this should be a separate attribute? -Varun -Original Message- From: Wood Scott-B07421 Sent: Tuesday, November 19, 2013 1:07 AM To: Sethi Varun-B16395 Cc: Wang Haiying-R54964; j...@8bytes.org; iommu@lists.linux- foundation.org; linuxppc-...@lists.ozlabs.org Subject: Re: [PATCH] iommu/fsl_pamu: use physical cpu index to find the matched cpu nodes On Thu, 2013-11-14 at 21:16 -0600, Sethi Varun-B16395 wrote: Haiying/Scott, Forgot to mention this, the PAMU driver has to handle stash destination settings both for power and dsp cores (on B4 platform). For the dsp cores we would expect the physical core id (not controlled by Linux). To make the interface consistent, I would expect the caller (for iommu_set_attr) to pass the physical core id. That sounds like you need two different interfaces. -Scott ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH] iommu/fsl_pamu: use physical cpu index to find the matched cpu nodes
-Original Message- From: Wood Scott-B07421 Sent: Tuesday, November 19, 2013 8:34 AM To: Sethi Varun-B16395 Cc: Wang Haiying-R54964; j...@8bytes.org; iommu@lists.linux- foundation.org; linuxppc-...@lists.ozlabs.org Subject: Re: [PATCH] iommu/fsl_pamu: use physical cpu index to find the matched cpu nodes On Mon, 2013-11-18 at 20:42 -0600, Varun Sethi wrote: For the DSP case again we have to set up the stash attribute. Are you saying that this should be a separate attribute? Not necessarily a separate attribute, but there should be some way to distinguish whether you're providing a Linux cpu number or some external stash destination. Yes, the current idea is to use a separate L2 cache type for the DSP cores (PAMU_DSP_L2_CACHE). DSP cores can stash only to the L2 cache. -Varun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH] iommu/fsl_pamu: use physical cpu index to find the matched cpu nodes
Haiying/Scott, Forgot to mention this, the PAMU driver has to handle stash destination settings both for power and dsp cores (on B4 platform). For the dsp cores we would expect the physical core id (not controlled by Linux). To make the interface consistent, I would expect the caller (for iommu_set_attr) to pass the physical core id. Regards Varun -Original Message- From: Wood Scott-B07421 Sent: Friday, November 15, 2013 3:40 AM To: Wang Haiying-R54964 Cc: j...@8bytes.org; iommu@lists.linux-foundation.org; linuxppc- d...@lists.ozlabs.org; Sethi Varun-B16395 Subject: Re: [PATCH] iommu/fsl_pamu: use physical cpu index to find the matched cpu nodes On Thu, 2013-11-14 at 14:30 -0500, Haiying Wang wrote: In the case we miss to bring up some cpus, we need to make sure we can find the correct cpu nodes in the device tree based on the given logical cpu index from the caller. Signed-off-by: Haiying Wang haiying.w...@freescale.com --- drivers/iommu/fsl_pamu.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c index cba0498..a9ab57b 100644 --- a/drivers/iommu/fsl_pamu.c +++ b/drivers/iommu/fsl_pamu.c @@ -539,6 +539,7 @@ u32 get_stash_id(u32 stash_dest_hint, u32 vcpu) Should probably also s/vcpu/cpu/g as vcpu makes no sense outside of virtualization code. u32 cache_level; int len, found = 0; int i; + u32 cpuid = get_hard_smp_processor_id(vcpu); s/cpuid/phys_cpu/ or similar -Scott ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/3 v2] iommu/fsl: Enable default DMA window for PCIe devices
Once the PCIe device assigned to a guest VM (via VFIO) gets detached from the iommu domain (when guest terminates), its PAMU table entry is disabled. So, this would prevent the device from being used once it's assigned back to the host. This patch allows for creation of a default DMA window corresponding to the device and subsequently enabling the PAMU table entry. Before we enable the entry, we ensure that the device's bus master capability is disabled (device quiesced). Signed-off-by: Varun Sethi varun.se...@freescale.com --- drivers/iommu/fsl_pamu.c| 43 drivers/iommu/fsl_pamu.h|1 + drivers/iommu/fsl_pamu_domain.c | 46 --- 3 files changed, 78 insertions(+), 12 deletions(-) diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c index cba0498..fb4a031 100644 --- a/drivers/iommu/fsl_pamu.c +++ b/drivers/iommu/fsl_pamu.c @@ -225,6 +225,21 @@ static struct paace *pamu_get_spaace(struct paace *paace, u32 wnum) return spaace; } +/* + * Defaul PPAACE settings for an LIODN. + */ +static void setup_default_ppaace(struct paace *ppaace) +{ + pamu_init_ppaace(ppaace); + /* window size is 2^(WSE+1) bytes */ + set_bf(ppaace-addr_bitfields, PPAACE_AF_WSE, 35); + ppaace-wbah = 0; + set_bf(ppaace-addr_bitfields, PPAACE_AF_WBAL, 0); + set_bf(ppaace-impl_attr, PAACE_IA_ATM, + PAACE_ATM_NO_XLATE); + set_bf(ppaace-addr_bitfields, PAACE_AF_AP, + PAACE_AP_PERMS_ALL); +} /** * pamu_get_fspi_and_allocate() - Allocates fspi index and reserves subwindows *required for primary PAACE in the secondary @@ -253,6 +268,24 @@ static unsigned long pamu_get_fspi_and_allocate(u32 subwin_cnt) return (spaace_addr - (unsigned long)spaact) / (sizeof(struct paace)); } +/* Reset the PAACE entry to the default state */ +void enable_default_dma_window(int liodn) +{ + struct paace *ppaace; + + ppaace = pamu_get_ppaace(liodn); + if (!ppaace) { + pr_debug(Invalid liodn entry\n); + return; + } + + memset(ppaace, 0, sizeof(struct paace)); + + setup_default_ppaace(ppaace); + mb(); + pamu_enable_liodn(liodn); +} + /* Release the subwindows reserved for a particular LIODN */ void pamu_free_subwins(int liodn) { @@ -752,15 +785,7 @@ static void __init setup_liodns(void) continue; } ppaace = pamu_get_ppaace(liodn); - pamu_init_ppaace(ppaace); - /* window size is 2^(WSE+1) bytes */ - set_bf(ppaace-addr_bitfields, PPAACE_AF_WSE, 35); - ppaace-wbah = 0; - set_bf(ppaace-addr_bitfields, PPAACE_AF_WBAL, 0); - set_bf(ppaace-impl_attr, PAACE_IA_ATM, - PAACE_ATM_NO_XLATE); - set_bf(ppaace-addr_bitfields, PAACE_AF_AP, - PAACE_AP_PERMS_ALL); + setup_default_ppaace(ppaace); if (of_device_is_compatible(node, fsl,qman-portal)) setup_qbman_paace(ppaace, QMAN_PORTAL_PAACE); if (of_device_is_compatible(node, fsl,qman)) diff --git a/drivers/iommu/fsl_pamu.h b/drivers/iommu/fsl_pamu.h index 8fc1a12..0edc 100644 --- a/drivers/iommu/fsl_pamu.h +++ b/drivers/iommu/fsl_pamu.h @@ -406,5 +406,6 @@ void get_ome_index(u32 *omi_index, struct device *dev); int pamu_update_paace_stash(int liodn, u32 subwin, u32 value); int pamu_disable_spaace(int liodn, u32 subwin); u32 pamu_get_max_subwin_cnt(void); +void enable_default_dma_window(int liodn); #endif /* __FSL_PAMU_H */ diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c index 966ae70..dd6cafc 100644 --- a/drivers/iommu/fsl_pamu_domain.c +++ b/drivers/iommu/fsl_pamu_domain.c @@ -340,17 +340,57 @@ static inline struct device_domain_info *find_domain(struct device *dev) return dev-archdata.iommu_domain; } +/* Disable device DMA capability and enable default DMA window */ +static void disable_device_dma(struct device_domain_info *info, + int enable_dma_window) +{ +#ifdef CONFIG_PCI + if (info-dev-bus == pci_bus_type) { + struct pci_dev *pdev = NULL; + pdev = to_pci_dev(info-dev); + if (pci_is_enabled(pdev)) + pci_disable_device(pdev); + } +#endif + + if (enable_dma_window) + enable_default_dma_window(info-liodn); +} + +static int check_for_shared_liodn(struct device_domain_info *info) +{ + struct device_domain_info *tmp; + + /* +* Sanity check, to ensure that this is not a +* shared LIODN. In case of a PCIe controller
[PATCH 0/3 v2] iommu/fsl: PAMU driver fixes.
The first patch fixes a build failure, when we try to build for a Freescale platform without PCI support. The second patch enables a default DMA window for the device, once it's detached from a domain. In case of vfio, once device is detached from a guest it can be again used by the host. The last patch adds the maintainer entry for the Freescale PAMU driver. Varun Sethi (3): iommu/fsl: Factor out PCI specific code. iommu/fsl: Enable default DMA window for PCIe devices once detached Add maintainers entry for the Freescale PAMU driver. MAINTAINERS |7 ++ drivers/iommu/fsl_pamu.c| 43 ++--- drivers/iommu/fsl_pamu.h|1 + drivers/iommu/fsl_pamu_domain.c | 134 +-- 4 files changed, 128 insertions(+), 57 deletions(-) -- 1.7.9.5 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/3 v2] iommu/fsl: Factor out PCI specific code.
Factor out PCI specific code in the PAMU driver. Signed-off-by: Varun Sethi varun.se...@freescale.com --- drivers/iommu/fsl_pamu_domain.c | 88 +++ 1 file changed, 43 insertions(+), 45 deletions(-) diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c index c857c30..966ae70 100644 --- a/drivers/iommu/fsl_pamu_domain.c +++ b/drivers/iommu/fsl_pamu_domain.c @@ -677,21 +677,15 @@ static int handle_attach_device(struct fsl_dma_domain *dma_domain, return ret; } -static int fsl_pamu_attach_device(struct iommu_domain *domain, - struct device *dev) +static struct device *get_dma_device(struct device *dev) { - struct fsl_dma_domain *dma_domain = domain-priv; - const u32 *liodn; - u32 liodn_cnt; - int len, ret = 0; - struct pci_dev *pdev = NULL; - struct pci_controller *pci_ctl; + struct device *dma_dev = dev; +#ifdef CONFIG_PCI - /* -* Use LIODN of the PCI controller while attaching a -* PCI device. -*/ if (dev-bus == pci_bus_type) { + struct pci_controller *pci_ctl; + struct pci_dev *pdev; + pdev = to_pci_dev(dev); pci_ctl = pci_bus_to_host(pdev-bus); /* @@ -699,17 +693,31 @@ static int fsl_pamu_attach_device(struct iommu_domain *domain, * so we can get the LIODN programmed by * u-boot. */ - dev = pci_ctl-parent; + dma_dev = pci_ctl-parent; } +#endif + return dma_dev; +} + +static int fsl_pamu_attach_device(struct iommu_domain *domain, + struct device *dev) +{ + struct fsl_dma_domain *dma_domain = domain-priv; + struct device *dma_dev; + const u32 *liodn; + u32 liodn_cnt; + int len, ret = 0; + + dma_dev = get_dma_device(dev); - liodn = of_get_property(dev-of_node, fsl,liodn, len); + liodn = of_get_property(dma_dev-of_node, fsl,liodn, len); if (liodn) { liodn_cnt = len / sizeof(u32); ret = handle_attach_device(dma_domain, dev, liodn, liodn_cnt); } else { pr_debug(missing fsl,liodn property at %s\n, - dev-of_node-full_name); + dma_dev-of_node-full_name); ret = -EINVAL; } @@ -720,32 +728,18 @@ static void fsl_pamu_detach_device(struct iommu_domain *domain, struct device *dev) { struct fsl_dma_domain *dma_domain = domain-priv; + struct device *dma_dev; const u32 *prop; int len; - struct pci_dev *pdev = NULL; - struct pci_controller *pci_ctl; - /* -* Use LIODN of the PCI controller while detaching a -* PCI device. -*/ - if (dev-bus == pci_bus_type) { - pdev = to_pci_dev(dev); - pci_ctl = pci_bus_to_host(pdev-bus); - /* -* make dev point to pci controller device -* so we can get the LIODN programmed by -* u-boot. -*/ - dev = pci_ctl-parent; - } + dma_dev = get_dma_device(dev); - prop = of_get_property(dev-of_node, fsl,liodn, len); + prop = of_get_property(dma_dev-of_node, fsl,liodn, len); if (prop) detach_device(dev, dma_domain); else pr_debug(missing fsl,liodn property at %s\n, - dev-of_node-full_name); + dma_dev-of_node-full_name); } static int configure_domain_geometry(struct iommu_domain *domain, void *data) @@ -905,6 +899,7 @@ static struct iommu_group *get_device_iommu_group(struct device *dev) return group; } +#ifdef CONFIG_PCI static bool check_pci_ctl_endpt_part(struct pci_controller *pci_ctl) { u32 version; @@ -945,13 +940,18 @@ static struct iommu_group *get_shared_pci_device_group(struct pci_dev *pdev) return NULL; } -static struct iommu_group *get_pci_device_group(struct pci_dev *pdev) +static struct iommu_group *get_pci_device_group(struct device *dev) { struct pci_controller *pci_ctl; bool pci_endpt_partioning; struct iommu_group *group = NULL; - struct pci_dev *bridge, *dma_pdev = NULL; + struct pci_dev *bridge, *pdev; + struct pci_dev *dma_pdev = NULL; + pdev = to_pci_dev(dev); + /* Don't create device groups for virtual PCI bridges */ + if (pdev-subordinate) + return NULL; pci_ctl = pci_bus_to_host(pdev-bus); pci_endpt_partioning = check_pci_ctl_endpt_part(pci_ctl); /* We can partition PCIe devices so assign device group to the device */ @@ -1044,11 +1044,11 @@ root_bus: return
[PATCH 2/3] iommu/fsl: Enable default DMA window for PCIe devices once detached
from domain. Once the PCIe device assigned to a guest VM (via VFIO) gets detached from the iommu domain (when guest terminates), its PAMU table entry is disabled. So, this would prevent the device from being used once it's assigned back to the host. This patch allows for creation of a default DMA window corresponding to the device and subsequently enabling the PAMU table entry. Before we enable the entry, we ensure that the device's bus master capability is disabled (device quiesced). Signed-off-by: Varun Sethi varun.se...@freescale.com --- drivers/iommu/fsl_pamu.c| 43 +++ drivers/iommu/fsl_pamu.h|1 + drivers/iommu/fsl_pamu_domain.c | 35 +++ 3 files changed, 70 insertions(+), 9 deletions(-) diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c index cba0498..fb4a031 100644 --- a/drivers/iommu/fsl_pamu.c +++ b/drivers/iommu/fsl_pamu.c @@ -225,6 +225,21 @@ static struct paace *pamu_get_spaace(struct paace *paace, u32 wnum) return spaace; } +/* + * Defaul PPAACE settings for an LIODN. + */ +static void setup_default_ppaace(struct paace *ppaace) +{ + pamu_init_ppaace(ppaace); + /* window size is 2^(WSE+1) bytes */ + set_bf(ppaace-addr_bitfields, PPAACE_AF_WSE, 35); + ppaace-wbah = 0; + set_bf(ppaace-addr_bitfields, PPAACE_AF_WBAL, 0); + set_bf(ppaace-impl_attr, PAACE_IA_ATM, + PAACE_ATM_NO_XLATE); + set_bf(ppaace-addr_bitfields, PAACE_AF_AP, + PAACE_AP_PERMS_ALL); +} /** * pamu_get_fspi_and_allocate() - Allocates fspi index and reserves subwindows *required for primary PAACE in the secondary @@ -253,6 +268,24 @@ static unsigned long pamu_get_fspi_and_allocate(u32 subwin_cnt) return (spaace_addr - (unsigned long)spaact) / (sizeof(struct paace)); } +/* Reset the PAACE entry to the default state */ +void enable_default_dma_window(int liodn) +{ + struct paace *ppaace; + + ppaace = pamu_get_ppaace(liodn); + if (!ppaace) { + pr_debug(Invalid liodn entry\n); + return; + } + + memset(ppaace, 0, sizeof(struct paace)); + + setup_default_ppaace(ppaace); + mb(); + pamu_enable_liodn(liodn); +} + /* Release the subwindows reserved for a particular LIODN */ void pamu_free_subwins(int liodn) { @@ -752,15 +785,7 @@ static void __init setup_liodns(void) continue; } ppaace = pamu_get_ppaace(liodn); - pamu_init_ppaace(ppaace); - /* window size is 2^(WSE+1) bytes */ - set_bf(ppaace-addr_bitfields, PPAACE_AF_WSE, 35); - ppaace-wbah = 0; - set_bf(ppaace-addr_bitfields, PPAACE_AF_WBAL, 0); - set_bf(ppaace-impl_attr, PAACE_IA_ATM, - PAACE_ATM_NO_XLATE); - set_bf(ppaace-addr_bitfields, PAACE_AF_AP, - PAACE_AP_PERMS_ALL); + setup_default_ppaace(ppaace); if (of_device_is_compatible(node, fsl,qman-portal)) setup_qbman_paace(ppaace, QMAN_PORTAL_PAACE); if (of_device_is_compatible(node, fsl,qman)) diff --git a/drivers/iommu/fsl_pamu.h b/drivers/iommu/fsl_pamu.h index 8fc1a12..0edc 100644 --- a/drivers/iommu/fsl_pamu.h +++ b/drivers/iommu/fsl_pamu.h @@ -406,5 +406,6 @@ void get_ome_index(u32 *omi_index, struct device *dev); int pamu_update_paace_stash(int liodn, u32 subwin, u32 value); int pamu_disable_spaace(int liodn, u32 subwin); u32 pamu_get_max_subwin_cnt(void); +void enable_default_dma_window(int liodn); #endif /* __FSL_PAMU_H */ diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c index e02e1de..553ef3c 100644 --- a/drivers/iommu/fsl_pamu_domain.c +++ b/drivers/iommu/fsl_pamu_domain.c @@ -340,6 +340,40 @@ static inline struct device_domain_info *find_domain(struct device *dev) return dev-archdata.iommu_domain; } +/* Disable device DMA capability and enable default DMA window */ +static void disable_device_dma(struct device_domain_info *info) +{ + struct device_domain_info *tmp; + int enable_dma_window = 1; + +#ifdef CONFIG_PCI + if (info-dev-bus == pci_bus_type) { + struct pci_dev *pdev = NULL; + pdev = to_pci_dev(info-dev); + if (pci_is_enabled(pdev)) + pci_disable_device(pdev); + } +#endif + /* +* Sanity check, to ensure that this is not a +* shared LIODN. In case of a PCIe controller +* it's possible that all PCIe devices share +* the same LIODN. We can't enable the default +* DMA window till all the devices have been +* quiesced (for PCIe
[PATCH 0/3] iommu/fsl: PAMU driver fixes.
The first patch fixes a build failure, when we try to build for a Freescale platform without PCI support. The second patch enables a default DMA window for the device, once it's detached from a domain. In case of vfio, once device is detached from a guest it can be again used by the host. The last patch adds the maintainer entry for the Freescale PAMU driver. Varun Sethi (3): iommu/fsl: Factor out PCI specific code. iommu/fsl: Enable default DMA window for PCIe devices once detached Add maintainers entry for the Freescale PAMU driver. MAINTAINERS |7 +++ drivers/iommu/fsl_pamu.c| 43 --- drivers/iommu/fsl_pamu.h|1 + drivers/iommu/fsl_pamu_domain.c | 116 +-- 4 files changed, 117 insertions(+), 50 deletions(-) -- 1.7.9.5 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/3] iommu/fsl: Factor out PCI specific code.
Factor out PCI specific code in the PAMU driver. Signed-off-by: Varun Sethi varun.se...@freescale.com --- drivers/iommu/fsl_pamu_domain.c | 81 +++ 1 file changed, 40 insertions(+), 41 deletions(-) diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c index c857c30..e02e1de 100644 --- a/drivers/iommu/fsl_pamu_domain.c +++ b/drivers/iommu/fsl_pamu_domain.c @@ -677,13 +677,9 @@ static int handle_attach_device(struct fsl_dma_domain *dma_domain, return ret; } -static int fsl_pamu_attach_device(struct iommu_domain *domain, - struct device *dev) +static void check_for_pci_dma_device(struct device **dev) { - struct fsl_dma_domain *dma_domain = domain-priv; - const u32 *liodn; - u32 liodn_cnt; - int len, ret = 0; +#ifdef CONFIG_PCI struct pci_dev *pdev = NULL; struct pci_controller *pci_ctl; @@ -691,25 +687,38 @@ static int fsl_pamu_attach_device(struct iommu_domain *domain, * Use LIODN of the PCI controller while attaching a * PCI device. */ - if (dev-bus == pci_bus_type) { - pdev = to_pci_dev(dev); + if ((*dev)-bus == pci_bus_type) { + pdev = to_pci_dev(*dev); pci_ctl = pci_bus_to_host(pdev-bus); /* * make dev point to pci controller device * so we can get the LIODN programmed by * u-boot. */ - dev = pci_ctl-parent; + *dev = pci_ctl-parent; } +#endif +} - liodn = of_get_property(dev-of_node, fsl,liodn, len); +static int fsl_pamu_attach_device(struct iommu_domain *domain, + struct device *dev) +{ + struct fsl_dma_domain *dma_domain = domain-priv; + struct device *dma_dev = dev; + const u32 *liodn; + u32 liodn_cnt; + int len, ret = 0; + + check_for_pci_dma_device(dma_dev); + + liodn = of_get_property(dma_dev-of_node, fsl,liodn, len); if (liodn) { liodn_cnt = len / sizeof(u32); ret = handle_attach_device(dma_domain, dev, liodn, liodn_cnt); } else { pr_debug(missing fsl,liodn property at %s\n, - dev-of_node-full_name); + dma_dev-of_node-full_name); ret = -EINVAL; } @@ -720,32 +729,18 @@ static void fsl_pamu_detach_device(struct iommu_domain *domain, struct device *dev) { struct fsl_dma_domain *dma_domain = domain-priv; + struct device *dma_dev = dev; const u32 *prop; int len; - struct pci_dev *pdev = NULL; - struct pci_controller *pci_ctl; - /* -* Use LIODN of the PCI controller while detaching a -* PCI device. -*/ - if (dev-bus == pci_bus_type) { - pdev = to_pci_dev(dev); - pci_ctl = pci_bus_to_host(pdev-bus); - /* -* make dev point to pci controller device -* so we can get the LIODN programmed by -* u-boot. -*/ - dev = pci_ctl-parent; - } + check_for_pci_dma_device(dma_dev); - prop = of_get_property(dev-of_node, fsl,liodn, len); + prop = of_get_property(dma_dev-of_node, fsl,liodn, len); if (prop) detach_device(dev, dma_domain); else pr_debug(missing fsl,liodn property at %s\n, - dev-of_node-full_name); + dma_dev-of_node-full_name); } static int configure_domain_geometry(struct iommu_domain *domain, void *data) @@ -905,6 +900,7 @@ static struct iommu_group *get_device_iommu_group(struct device *dev) return group; } +#ifdef CONFIG_PCI static bool check_pci_ctl_endpt_part(struct pci_controller *pci_ctl) { u32 version; @@ -945,13 +941,18 @@ static struct iommu_group *get_shared_pci_device_group(struct pci_dev *pdev) return NULL; } -static struct iommu_group *get_pci_device_group(struct pci_dev *pdev) +static struct iommu_group *get_pci_device_group(struct device *dev) { struct pci_controller *pci_ctl; bool pci_endpt_partioning; struct iommu_group *group = NULL; - struct pci_dev *bridge, *dma_pdev = NULL; + struct pci_dev *bridge, *pdev; + struct pci_dev *dma_pdev = NULL; + pdev = to_pci_dev(dev); + /* Don't create device groups for virtual PCI bridges */ + if (pdev-subordinate) + return NULL; pci_ctl = pci_bus_to_host(pdev-bus); pci_endpt_partioning = check_pci_ctl_endpt_part(pci_ctl); /* We can partition PCIe devices so assign device group to the device */ @@ -1044,11 +1045,11 @@ root_bus: return
[PATCH 2/2] iommu/fsl: Enable default DMA window for PCIe devices once detached
Once the PCIe device assigned to a guest VM (via VFIO) gets detached from the iommu domain (when guest terminates), its PAMU table entry is disabled. So, this would prevent the device from being used once it's assigned back to the host. This patch allows for creation of a default DMA window corresponding to the device and subsequently enabling the PAMU table entry. Before we enable the entry, we ensure that the device's bus master capability is disabled (device quiesced). Signed-off-by: Varun Sethi varun.se...@freescale.com --- drivers/iommu/fsl_pamu.c| 43 +++ drivers/iommu/fsl_pamu.h|1 + drivers/iommu/fsl_pamu_domain.c | 35 +++ 3 files changed, 70 insertions(+), 9 deletions(-) diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c index cba0498..fb4a031 100644 --- a/drivers/iommu/fsl_pamu.c +++ b/drivers/iommu/fsl_pamu.c @@ -225,6 +225,21 @@ static struct paace *pamu_get_spaace(struct paace *paace, u32 wnum) return spaace; } +/* + * Defaul PPAACE settings for an LIODN. + */ +static void setup_default_ppaace(struct paace *ppaace) +{ + pamu_init_ppaace(ppaace); + /* window size is 2^(WSE+1) bytes */ + set_bf(ppaace-addr_bitfields, PPAACE_AF_WSE, 35); + ppaace-wbah = 0; + set_bf(ppaace-addr_bitfields, PPAACE_AF_WBAL, 0); + set_bf(ppaace-impl_attr, PAACE_IA_ATM, + PAACE_ATM_NO_XLATE); + set_bf(ppaace-addr_bitfields, PAACE_AF_AP, + PAACE_AP_PERMS_ALL); +} /** * pamu_get_fspi_and_allocate() - Allocates fspi index and reserves subwindows *required for primary PAACE in the secondary @@ -253,6 +268,24 @@ static unsigned long pamu_get_fspi_and_allocate(u32 subwin_cnt) return (spaace_addr - (unsigned long)spaact) / (sizeof(struct paace)); } +/* Reset the PAACE entry to the default state */ +void enable_default_dma_window(int liodn) +{ + struct paace *ppaace; + + ppaace = pamu_get_ppaace(liodn); + if (!ppaace) { + pr_debug(Invalid liodn entry\n); + return; + } + + memset(ppaace, 0, sizeof(struct paace)); + + setup_default_ppaace(ppaace); + mb(); + pamu_enable_liodn(liodn); +} + /* Release the subwindows reserved for a particular LIODN */ void pamu_free_subwins(int liodn) { @@ -752,15 +785,7 @@ static void __init setup_liodns(void) continue; } ppaace = pamu_get_ppaace(liodn); - pamu_init_ppaace(ppaace); - /* window size is 2^(WSE+1) bytes */ - set_bf(ppaace-addr_bitfields, PPAACE_AF_WSE, 35); - ppaace-wbah = 0; - set_bf(ppaace-addr_bitfields, PPAACE_AF_WBAL, 0); - set_bf(ppaace-impl_attr, PAACE_IA_ATM, - PAACE_ATM_NO_XLATE); - set_bf(ppaace-addr_bitfields, PAACE_AF_AP, - PAACE_AP_PERMS_ALL); + setup_default_ppaace(ppaace); if (of_device_is_compatible(node, fsl,qman-portal)) setup_qbman_paace(ppaace, QMAN_PORTAL_PAACE); if (of_device_is_compatible(node, fsl,qman)) diff --git a/drivers/iommu/fsl_pamu.h b/drivers/iommu/fsl_pamu.h index 8fc1a12..0edc 100644 --- a/drivers/iommu/fsl_pamu.h +++ b/drivers/iommu/fsl_pamu.h @@ -406,5 +406,6 @@ void get_ome_index(u32 *omi_index, struct device *dev); int pamu_update_paace_stash(int liodn, u32 subwin, u32 value); int pamu_disable_spaace(int liodn, u32 subwin); u32 pamu_get_max_subwin_cnt(void); +void enable_default_dma_window(int liodn); #endif /* __FSL_PAMU_H */ diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c index e02e1de..553ef3c 100644 --- a/drivers/iommu/fsl_pamu_domain.c +++ b/drivers/iommu/fsl_pamu_domain.c @@ -340,6 +340,40 @@ static inline struct device_domain_info *find_domain(struct device *dev) return dev-archdata.iommu_domain; } +/* Disable device DMA capability and enable default DMA window */ +static void disable_device_dma(struct device_domain_info *info) +{ + struct device_domain_info *tmp; + int enable_dma_window = 1; + +#ifdef CONFIG_PCI + if (info-dev-bus == pci_bus_type) { + struct pci_dev *pdev = NULL; + pdev = to_pci_dev(info-dev); + if (pci_is_enabled(pdev)) + pci_disable_device(pdev); + } +#endif + /* +* Sanity check, to ensure that this is not a +* shared LIODN. In case of a PCIe controller +* it's possible that all PCIe devices share +* the same LIODN. We can't enable the default +* DMA window till all the devices have been +* quiesced (for PCIe devices, we
[PATCH 1/2 v15] iommu/fsl: Add additional iommu attributes required by the PAMU driver.
Added the following domain attributes for the FSL PAMU driver: 1. Added new iommu stash attribute, which allows setting of the LIODN specific stash id parameter through IOMMU API. 2. Added an attribute for enabling/disabling DMA to a particular memory window. 3. Added domain attribute to check for PAMUV1 specific constraints. Signed-off-by: Varun Sethi varun.se...@freescale.com --- v15 changes: - Moved fsl_pamu_stash.h under arch/powerpc/include/asm. v14 changes: - Add FSL prefix to PAMU attributes. v13 changes: - created a new file include/linux/fsl_pamu_stash.h for stash attributes. v12 changes: - Moved PAMU specifc stash ids and structures to PAMU header file. - no change in v11. - no change in v10. arch/powerpc/include/asm/fsl_pamu_stash.h | 39 + include/linux/iommu.h | 16 2 files changed, 55 insertions(+), 0 deletions(-) create mode 100644 arch/powerpc/include/asm/fsl_pamu_stash.h diff --git a/arch/powerpc/include/asm/fsl_pamu_stash.h b/arch/powerpc/include/asm/fsl_pamu_stash.h new file mode 100644 index 000..caa1b21 --- /dev/null +++ b/arch/powerpc/include/asm/fsl_pamu_stash.h @@ -0,0 +1,39 @@ +/* + * 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. + * + * 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. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + * + * Copyright (C) 2013 Freescale Semiconductor, Inc. + * + */ + +#ifndef __FSL_PAMU_STASH_H +#define __FSL_PAMU_STASH_H + +/* cache stash targets */ +enum pamu_stash_target { + PAMU_ATTR_CACHE_L1 = 1, + PAMU_ATTR_CACHE_L2, + PAMU_ATTR_CACHE_L3, +}; + +/* + * This attribute allows configuring stashig specific parameters + * in the PAMU hardware. + */ + +struct pamu_stash_attribute { + u32 cpu;/* cpu number */ + u32 cache; /* cache to stash to: L1,L2,L3 */ +}; + +#endif /* __FSL_PAMU_STASH_H */ diff --git a/include/linux/iommu.h b/include/linux/iommu.h index 2727810..313d17a 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -57,10 +57,26 @@ struct iommu_domain { #define IOMMU_CAP_CACHE_COHERENCY 0x1 #define IOMMU_CAP_INTR_REMAP 0x2 /* isolates device intrs */ +/* + * Following constraints are specifc to FSL_PAMUV1: + * -aperture must be power of 2, and naturally aligned + * -number of windows must be power of 2, and address space size + * of each window is determined by aperture size / # of windows + * -the actual size of the mapped region of a window must be power + * of 2 starting with 4KB and physical address must be naturally + * aligned. + * DOMAIN_ATTR_FSL_PAMUV1 corresponds to the above mentioned contraints. + * The caller can invoke iommu_domain_get_attr to check if the underlying + * iommu implementation supports these constraints. + */ + enum iommu_attr { DOMAIN_ATTR_GEOMETRY, DOMAIN_ATTR_PAGING, DOMAIN_ATTR_WINDOWS, + DOMAIN_ATTR_FSL_PAMU_STASH, + DOMAIN_ATTR_FSL_PAMU_ENABLE, + DOMAIN_ATTR_FSL_PAMUV1, DOMAIN_ATTR_MAX, }; -- 1.7.4.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/3 v2] iommu: Move swap_pci_ref function to pci.h.
swap_pci_ref function is used by the IOMMU API code for swapping pci device pointers, while determining the iommu group for the device. Currently this function was being implemented for different IOMMU drivers. This patch moves the function to a new file, drivers/iommu/pci.h so that the implementation can be shared across various IOMMU drivers. Signed-off-by: Varun Sethi varun.se...@freescale.com --- v2 changes: - created a new file drivers/iommu/pci.h. drivers/iommu/amd_iommu.c |7 +-- drivers/iommu/intel-iommu.c |7 +-- drivers/iommu/pci.h | 29 + 3 files changed, 31 insertions(+), 12 deletions(-) create mode 100644 drivers/iommu/pci.h diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c index a7f6b04..2463464 100644 --- a/drivers/iommu/amd_iommu.c +++ b/drivers/iommu/amd_iommu.c @@ -46,6 +46,7 @@ #include amd_iommu_proto.h #include amd_iommu_types.h #include irq_remapping.h +#include pci.h #define CMD_SET_TYPE(cmd, t) ((cmd)-data[1] |= ((t) 28)) @@ -263,12 +264,6 @@ static bool check_device(struct device *dev) return true; } -static void swap_pci_ref(struct pci_dev **from, struct pci_dev *to) -{ - pci_dev_put(*from); - *from = to; -} - static struct pci_bus *find_hosted_bus(struct pci_bus *bus) { while (!bus-self) { diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index 6e0b9ff..81ad7b8 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -47,6 +47,7 @@ #include asm/iommu.h #include irq_remapping.h +#include pci.h #define ROOT_SIZE VTD_PAGE_SIZE #define CONTEXT_SIZE VTD_PAGE_SIZE @@ -4137,12 +4138,6 @@ static int intel_iommu_domain_has_cap(struct iommu_domain *domain, return 0; } -static void swap_pci_ref(struct pci_dev **from, struct pci_dev *to) -{ - pci_dev_put(*from); - *from = to; -} - #define REQ_ACS_FLAGS (PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF) static int intel_iommu_add_device(struct device *dev) diff --git a/drivers/iommu/pci.h b/drivers/iommu/pci.h new file mode 100644 index 000..d460646 --- /dev/null +++ b/drivers/iommu/pci.h @@ -0,0 +1,29 @@ +/* + * 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. + * + * 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. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + * + * Copyright (C) 2013 Red Hat, Inc. + * Copyright (C) 2013 Freescale Semiconductor, Inc. + * + */ +#ifndef __PCI_H +#define __PCI_H + +/* Helper function for swapping pci device reference */ +static inline void swap_pci_ref(struct pci_dev **from, struct pci_dev *to) +{ + pci_dev_put(*from); + *from = to; +} + +#endif /* __PCI_H */ -- 1.7.4.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/3 v14] iommu/fsl: Add additional iommu attributes required by the PAMU driver.
Added the following domain attributes for the FSL PAMU driver: 1. Added new iommu stash attribute, which allows setting of the LIODN specific stash id parameter through IOMMU API. 2. Added an attribute for enabling/disabling DMA to a particular memory window. 3. Added domain attribute to check for PAMUV1 specific constraints. Signed-off-by: Varun Sethi varun.se...@freescale.com --- v14 changes: - Add FSL prefix to PAMU attributes. v13 changes: - created a new file include/linux/fsl_pamu_stash.h for stash attributes. v12 changes: - Moved PAMU specifc stash ids and structures to PAMU header file. - no change in v11. - no change in v10. include/linux/fsl_pamu_stash.h | 39 +++ include/linux/iommu.h | 16 2 files changed, 55 insertions(+), 0 deletions(-) create mode 100644 include/linux/fsl_pamu_stash.h diff --git a/include/linux/fsl_pamu_stash.h b/include/linux/fsl_pamu_stash.h new file mode 100644 index 000..caa1b21 --- /dev/null +++ b/include/linux/fsl_pamu_stash.h @@ -0,0 +1,39 @@ +/* + * 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. + * + * 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. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + * + * Copyright (C) 2013 Freescale Semiconductor, Inc. + * + */ + +#ifndef __FSL_PAMU_STASH_H +#define __FSL_PAMU_STASH_H + +/* cache stash targets */ +enum pamu_stash_target { + PAMU_ATTR_CACHE_L1 = 1, + PAMU_ATTR_CACHE_L2, + PAMU_ATTR_CACHE_L3, +}; + +/* + * This attribute allows configuring stashig specific parameters + * in the PAMU hardware. + */ + +struct pamu_stash_attribute { + u32 cpu;/* cpu number */ + u32 cache; /* cache to stash to: L1,L2,L3 */ +}; + +#endif /* __FSL_PAMU_STASH_H */ diff --git a/include/linux/iommu.h b/include/linux/iommu.h index 2727810..c5dc2b9 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -57,10 +57,26 @@ struct iommu_domain { #define IOMMU_CAP_CACHE_COHERENCY 0x1 #define IOMMU_CAP_INTR_REMAP 0x2 /* isolates device intrs */ +/* + * Following constraints are specifc to FSL_PAMUV1: + * -aperture must be power of 2, and naturally aligned + * -number of windows must be power of 2, and address space size + * of each window is determined by aperture size / # of windows + * -the actual size of the mapped region of a window must be power + * of 2 starting with 4KB and physical address must be naturally + * aligned. + * DOMAIN_ATTR_FSL_PAMUV1 corresponds to the above mentioned contraints. + * The caller can invoke iommu_domain_get_attr to check if the underlying + * iommu implementation supports these constraints. + */ + enum iommu_attr { DOMAIN_ATTR_GEOMETRY, DOMAIN_ATTR_PAGING, DOMAIN_ATTR_WINDOWS, + DOMAIN_ATTR_FSL_PAMU_STASH, + DOMAIN_ATTR_FSL_PAMU_ENABLE, + DOMAIN_ATTR_FSL_PAMUV1, DOMAIN_ATTR_MAX, }; -- 1.7.4.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu