RE: [PATCH 0/3 v2] iommu/fsl: PAMU driver fixes.
Hi Joerg, Please consider these patches for 3.12. Regards Varun -Original Message- From: Sethi Varun-B16395 Sent: Wednesday, October 16, 2013 4:53 PM To: j...@8bytes.org; io...@lists.linux-foundation.org; linuxppc- d...@lists.ozlabs.org; linux-ker...@vger.kernel.org; Yoder Stuart-B08248; Wood Scott-B07421; alex.william...@redhat.com; Bhushan Bharat-R65777 Cc: Sethi Varun-B16395 Subject: [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 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/3 v2] iommu/fsl: Enable default DMA window for PCIe devices
-Original Message-
From: Bhushan Bharat-R65777
Sent: Wednesday, October 16, 2013 10:20 PM
To: Sethi Varun-B16395; j...@8bytes.org; iommu@lists.linux-
foundation.org; linuxppc-dev@lists.ozlabs.org; linux-
ker...@vger.kernel.org; Yoder Stuart-B08248; Wood Scott-B07421;
alex.william...@redhat.com
Subject: RE: [PATCH 2/3 v2] iommu/fsl: Enable default DMA window for PCIe
devices
-Original Message-
From: Sethi Varun-B16395
Sent: Wednesday, October 16, 2013 4:53 PM
To: j...@8bytes.org; io...@lists.linux-foundation.org; linuxppc-
d...@lists.ozlabs.org; linux-ker...@vger.kernel.org; Yoder
Stuart-B08248; Wood Scott-B07421; alex.william...@redhat.com; Bhushan
Bharat-R65777
Cc: Sethi Varun-B16395
Subject: [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
RE: [PATCH 2/3 v2] iommu/fsl: Enable default DMA window for PCIe devices
-Original Message-
From: Bhushan Bharat-R65777
Sent: Wednesday, October 16, 2013 10:43 PM
To: Sethi Varun-B16395; j...@8bytes.org; iommu@lists.linux-
foundation.org; linuxppc-dev@lists.ozlabs.org; linux-
ker...@vger.kernel.org; Yoder Stuart-B08248; Wood Scott-B07421;
alex.william...@redhat.com
Subject: RE: [PATCH 2/3 v2] iommu/fsl: Enable default DMA window for PCIe
devices
-Original Message-
From: Sethi Varun-B16395
Sent: Wednesday, October 16, 2013 4:53 PM
To: j...@8bytes.org; io...@lists.linux-foundation.org; linuxppc-
d...@lists.ozlabs.org; linux-ker...@vger.kernel.org; Yoder
Stuart-B08248; Wood Scott-B07421; alex.william...@redhat.com;
Bhushan
Bharat-R65777
Cc: Sethi Varun-B16395
Subject: [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
RE: [PATCH 2/3 v2] iommu/fsl: Enable default DMA window for PCIe devices
-Original Message-
From: Bhushan Bharat-R65777
Sent: Wednesday, October 16, 2013 10:52 PM
To: Sethi Varun-B16395; j...@8bytes.org; iommu@lists.linux-
foundation.org; linuxppc-dev@lists.ozlabs.org; linux-
ker...@vger.kernel.org; Yoder Stuart-B08248; Wood Scott-B07421;
alex.william...@redhat.com
Subject: RE: [PATCH 2/3 v2] iommu/fsl: Enable default DMA window for PCIe
devices
-Original Message-
From: Sethi Varun-B16395
Sent: Wednesday, October 16, 2013 4:53 PM
To: j...@8bytes.org; io...@lists.linux-foundation.org;
linuxppc- d...@lists.ozlabs.org; linux-ker...@vger.kernel.org;
Yoder Stuart-B08248; Wood Scott-B07421;
alex.william...@redhat.com; Bhushan
Bharat-R65777
Cc: Sethi Varun-B16395
Subject: [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
RE: [PATCH 1/3] iommu/fsl: Factor out PCI specific code.
-Original Message-
From: iommu-boun...@lists.linux-foundation.org [mailto:iommu-
boun...@lists.linux-foundation.org] On Behalf Of Bjorn Helgaas
Sent: Tuesday, October 15, 2013 5:46 AM
To: Sethi Varun-B16395
Cc: Yoder Stuart-B08248; linux-ker...@vger.kernel.org; iommu@lists.linux-
foundation.org; Bhushan Bharat-R65777; Wood Scott-B07421; linuxppc-
d...@lists.ozlabs.org
Subject: Re: [PATCH 1/3] iommu/fsl: Factor out PCI specific code.
On Sun, Oct 13, 2013 at 02:02:32AM +0530, Varun Sethi wrote:
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)
check_for_pci_dma_device() doesn't give a good clue about what the
function returns. And why return something via a reference parameter
when you could return it directly?
[Sethi Varun-B16395] I will rename the function to get_dma_device and make it
return a pointer.
{
- 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;
This is sort of a goofy looking function. It would read much better as
something like this:
[Sethi Varun-B16395] Will make the change.
struct device *dma_dev = dev;
#ifdef CONFIG_PCI
if (...) {
dma_dev = ...;
}
#endif
return dma_dev;
Does this need to care about reference counting when you return a pointer
to a different device?
[Sethi Varun-B16395] Reference counting isn't required, as we are just
obtaining the LIODN value from the PCI controller.
-Varun
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/7] iommu: add api to get iommu_domain of a device
-Original Message-
From: Alex Williamson [mailto:alex.william...@redhat.com]
Sent: Friday, October 11, 2013 2:12 AM
To: Sethi Varun-B16395
Cc: Bhushan Bharat-R65777; ag...@suse.de; Wood Scott-B07421; linux-
p...@vger.kernel.org; ga...@kernel.crashing.org; linux-
ker...@vger.kernel.org; io...@lists.linux-foundation.org;
b...@kernel.crashing.org; linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a device
On Thu, 2013-10-10 at 20:09 +, Sethi Varun-B16395 wrote:
-Original Message-
From: iommu-boun...@lists.linux-foundation.org [mailto:iommu-
boun...@lists.linux-foundation.org] On Behalf Of Alex Williamson
Sent: Tuesday, October 08, 2013 8:43 AM
To: Bhushan Bharat-R65777
Cc: ag...@suse.de; Wood Scott-B07421; linux-...@vger.kernel.org;
ga...@kernel.crashing.org; linux-ker...@vger.kernel.org;
io...@lists.linux-foundation.org; b...@kernel.crashing.org;
linuxppc- d...@lists.ozlabs.org
Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a
device
On Mon, 2013-10-07 at 05:46 +, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Alex Williamson [mailto:alex.william...@redhat.com]
Sent: Friday, October 04, 2013 11:42 PM
To: Bhushan Bharat-R65777
Cc: j...@8bytes.org; b...@kernel.crashing.org;
ga...@kernel.crashing.org; linux- ker...@vger.kernel.org;
linuxppc-dev@lists.ozlabs.org; linux- p...@vger.kernel.org;
ag...@suse.de; Wood Scott-B07421; iommu@lists.linux-
foundation.org
Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a
device
On Fri, 2013-10-04 at 17:23 +, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Alex Williamson [mailto:alex.william...@redhat.com]
Sent: Friday, October 04, 2013 10:43 PM
To: Bhushan Bharat-R65777
Cc: j...@8bytes.org; b...@kernel.crashing.org;
ga...@kernel.crashing.org; linux- ker...@vger.kernel.org;
linuxppc-dev@lists.ozlabs.org; linux- p...@vger.kernel.org;
ag...@suse.de; Wood Scott-B07421; iommu@lists.linux-
foundation.org
Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain
of a device
On Fri, 2013-10-04 at 16:47 +, Bhushan Bharat-R65777
wrote:
-Original Message-
From: Alex Williamson
[mailto:alex.william...@redhat.com]
Sent: Friday, October 04, 2013 9:15 PM
To: Bhushan Bharat-R65777
Cc: j...@8bytes.org; b...@kernel.crashing.org;
ga...@kernel.crashing.org; linux-
ker...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
linux- p...@vger.kernel.org; ag...@suse.de; Wood
Scott-B07421; iommu@lists.linux- foundation.org
Subject: Re: [PATCH 2/7] iommu: add api to get
iommu_domain of a device
On Fri, 2013-10-04 at 09:54 +, Bhushan Bharat-R65777
wrote:
-Original Message-
From: linux-pci-ow...@vger.kernel.org
[mailto:linux-pci-ow...@vger.kernel.org]
On Behalf Of Alex Williamson
Sent: Wednesday, September 25, 2013 10:16 PM
To: Bhushan Bharat-R65777
Cc: j...@8bytes.org; b...@kernel.crashing.org;
ga...@kernel.crashing.org; linux-
ker...@vger.kernel.org;
linuxppc-dev@lists.ozlabs.org;
linux- p...@vger.kernel.org; ag...@suse.de; Wood
Scott-B07421; iommu@lists.linux- foundation.org;
Bhushan
Bharat-R65777
Subject: Re: [PATCH 2/7] iommu: add api to get
iommu_domain of a device
On Thu, 2013-09-19 at 12:59 +0530, Bharat Bhushan
wrote:
This api return the iommu domain to which the
device is
attached.
The iommu_domain is required for making API calls
related to
iommu.
Follow up patches which use this API to know iommu
maping.
Signed-off-by: Bharat Bhushan
bharat.bhus...@freescale.com
---
drivers/iommu/iommu.c | 10 ++
include/linux/iommu.h |7 +++
2 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/drivers/iommu/iommu.c
b/drivers/iommu/iommu.c index
fbe9ca7..6ac5f50 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -696,6 +696,16 @@ void
iommu_detach_device(struct iommu_domain *domain,
struct device *dev) }
EXPORT_SYMBOL_GPL(iommu_detach_device);
+struct iommu_domain *iommu_get_dev_domain(struct
device *dev) {
+ struct iommu_ops *ops = dev-bus-iommu_ops;
+
+ if (unlikely(ops == NULL ||
+ops
RE: [PATCH 1/2] iommu/fsl: Factor out PCI specific code.
Hi Joerg,
Please consider these patches for 3.12.
Regards
Varun
-Original Message-
From: Sethi Varun-B16395
Sent: Wednesday, October 09, 2013 11:57 PM
To: j...@8bytes.org; io...@lists.linux-foundation.org; linuxppc-
d...@lists.ozabs.org; linux-ker...@vger.kernel.org; Yoder Stuart-B08248;
alex.william...@redhat.com
Cc: Sethi Varun-B16395
Subject: [PATCH 1/2] 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
RE: [PATCH 2/7] iommu: add api to get iommu_domain of a device
sure that your use of the interface is race free. The interface itself needs to be designed so that it's difficult to use incorrectly. So we can define iommu_get_dev_domain()/iommu_put_dev_domain(); iommu_get_dev_domain() will return domain with the lock held, and iommu_put_dev_domain() will release the lock? And iommu_get_dev_domain() must always be followed by iommu_get_dev_domain(). What lock? get/put are generally used for reference counting, not locking in the kernel. That's not the case here. This is a backdoor to get the iommu domain from the iommu driver regardless of who is using it or how. The iommu domain is created and managed by vfio, so shouldn't we be looking at how to do this through vfio? Let me first describe what we are doing here: During initialization:- - vfio talks to MSI system to know the MSI-page and size - vfio then interacts with iommu to map the MSI-page in iommu (IOVA is decided by userspace and physical address is the MSI-page) - So the IOVA subwindow mapping is created in iommu and yes VFIO know about this mapping. Now do SET_IRQ(MSI/MSIX) ioctl: - calls pci_enable_msix()/pci_enable_msi_block(): which is supposed to set MSI address/data in device. - So in current implementation (this patchset) msi-subsystem gets the IOVA from iommu via this defined interface. - Are you saying that rather than getting this from iommu, we should get this from vfio? What difference does this make? Yes, you just said above that vfio knows the msi to iova mapping, so why go outside of vfio to find it later? The difference is one case you can have a proper reference to data structures to make sure the pointer you get back actually has meaning at the time you're using it vs the code here where you're defining an API that returns a meaningless value With FSL-PAMU we will always get consistant data from iommu or vfio-data structure. Great, but you're trying to add a generic API to the IOMMU subsystem that's difficult to use correctly. The fact that you use it correctly does not justify the API. because you can't check or enforce that an arbitrary caller is using it correctly. I am not sure what is arbitrary caller? pdev is known to vfio, so vfio will only make pci_enable_msix()/pci_enable_msi_block() for this pdev. If anyother code makes then it is some other unexpectedly thing happening in system, no? What's proposed here is a generic IOMMU API. Anybody can call this. What if the host SCSI driver decides to go get the iommu domain for it's device (or any other device)? Does that fit your usage model? It's not maintainable. Thanks, I do not have any issue with this as well, can you also describe the type of API you are envisioning; I can think of defining some function in vfio.c/vfio_iommu*.c, make them global and declare then in include/Linux/vfio.h And include Linux/vfio.h in caller file (arch/powerpc/kernel/msi.c) Do you really want module dependencies between vfio and your core kernel MSI setup? Look at the vfio external user interface that we've already defined. That allows other components of the kernel to get a proper reference to a vfio group. From there you can work out how to get what you want. Another alternative is that vfio could register an MSI to IOVA mapping with architecture code when the mapping is created. The MSI setup path could then do a lookup in architecture code for the mapping. You could even store the MSI to IOVA mapping in VFIO and create an interface where SET_IRQ passes that mapping into setup code. [Sethi Varun-B16395] What you are suggesting is that the MSI setup path queries the vfio subsystem for the mapping, rather than directly querying the iommu subsystem. So, say if we add an interface for getting MSI to IOVA mapping in the msi setup path, wouldn't this again be specific to vfio? I reckon that this interface would again ppc machine specific interface. -Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 1/7] powerpc: Add interface to get msi region information
-Original Message- From: linux-kernel-ow...@vger.kernel.org [mailto:linux-kernel- ow...@vger.kernel.org] On Behalf Of Bhushan Bharat-R65777 Sent: Tuesday, October 08, 2013 10:40 PM To: j...@8bytes.org; Bjorn Helgaas Cc: alex.william...@redhat.com; b...@kernel.crashing.org; ga...@kernel.crashing.org; linux-ker...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; linux-...@vger.kernel.org; ag...@suse.de; Wood Scott-B07421; io...@lists.linux-foundation.org Subject: RE: [PATCH 1/7] powerpc: Add interface to get msi region information -Original Message- From: j...@8bytes.org [mailto:j...@8bytes.org] Sent: Tuesday, October 08, 2013 10:32 PM To: Bjorn Helgaas Cc: Bhushan Bharat-R65777; alex.william...@redhat.com; b...@kernel.crashing.org; ga...@kernel.crashing.org; linux-ker...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; linux-...@vger.kernel.org; ag...@suse.de; Wood Scott- B07421; io...@lists.linux-foundation.org Subject: Re: [PATCH 1/7] powerpc: Add interface to get msi region information On Tue, Oct 08, 2013 at 10:47:49AM -0600, Bjorn Helgaas wrote: I still have no idea what an aperture type IOMMU is, other than that it is different. An aperture based IOMMU is basically any GART-like IOMMU which can only remap a small window (the aperture) of the DMA address space. DMA outside of that window is either blocked completly or passed through untranslated. It is completely blocked for Freescale PAMU. So for this type of iommu what we have to do is to create a MSI mapping just after guest physical address, Example: guest have a 512M of memory then we create window of 1G (because of power of 2 requirement), then we have to FIT MSI just after 512M of guest. [Sethi Varun-B16395] PAMU (FSL IOMMU) has a concept of primary window and subwindows. Primary window corresponds to the complete guest iova address space (including MSI space), with respect to IOMMU_API this is termed as geometry . IOVA Base of subwindow is determined from the number of subwindows (configurable using iommu API). Subwindows allow for handling physically discontiguous memory. PAMU translates device iova accesses to actual physical address. MSI mapping would be addressed by a subwindow, with iova base starting at the end of the guest iova space. VFIO code creates a PAMU window (also defines number of subwindow) to map the guest iova space + msi space. The interface defined by this patch queries the PAMU driver to get the iova mapping for the msi region assigned to the PCIe device (assigned to the guest). -Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 1/3] powerpc: Add iommu domain pointer to device archdata
Thanks Joerg. -Original Message- From: Joerg Roedel [mailto:j...@8bytes.org] Sent: Wednesday, August 14, 2013 9:45 PM To: Sethi Varun-B16395 Cc: io...@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org; linux-ker...@vger.kernel.org; b...@kernel.crashing.org; ga...@kernel.crashing.org; alex.william...@redhat.com; Yoder Stuart- B08248; Wood Scott-B07421 Subject: Re: [PATCH 1/3] powerpc: Add iommu domain pointer to device archdata On Wed, Aug 14, 2013 at 09:56:11AM +, Sethi Varun-B16395 wrote: Please find the .config file attached with this mail. Fantastic, thanks. The build works fine, I'll include the driver into my next-branch. I also have two minor clean-up patches on-top, just if you where wondering. Joerg ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [v2][PATCH 1/7] powerpc/book3e: support CONFIG_RELOCATABLE
-Original Message- From: Linuxppc-dev [mailto:linuxppc-dev- bounces+varun.sethi=freescale@lists.ozlabs.org] On Behalf Of Tiejun Chen Sent: Thursday, June 20, 2013 1:23 PM To: b...@kernel.crashing.org Cc: linuxppc-dev@lists.ozlabs.org; linux-ker...@vger.kernel.org Subject: [v2][PATCH 1/7] powerpc/book3e: support CONFIG_RELOCATABLE book3e is different with book3s since 3s includes the exception vectors code in head_64.S as it relies on absolute addressing which is only possible within this compilation unit. So we have to get that label address with got. And when boot a relocated kernel, we should reset ipvr properly again after .relocate. Signed-off-by: Tiejun Chen tiejun.c...@windriver.com --- arch/powerpc/include/asm/exception-64e.h |8 arch/powerpc/kernel/exceptions-64e.S | 15 ++- arch/powerpc/kernel/head_64.S| 22 ++ arch/powerpc/lib/feature-fixups.c|7 +++ 4 files changed, 51 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/include/asm/exception-64e.h b/arch/powerpc/include/asm/exception-64e.h index 51fa43e..89e940d 100644 --- a/arch/powerpc/include/asm/exception-64e.h +++ b/arch/powerpc/include/asm/exception-64e.h @@ -214,10 +214,18 @@ exc_##label##_book3e: #define TLB_MISS_STATS_SAVE_INFO_BOLTED #endif +#ifndef CONFIG_RELOCATABLE #define SET_IVOR(vector_number, vector_offset) \ li r3,vector_offset@l; \ ori r3,r3,interrupt_base_book3e@l; \ mtspr SPRN_IVOR##vector_number,r3; +#else +#define SET_IVOR(vector_number, vector_offset) \ + LOAD_REG_ADDR(r3,interrupt_base_book3e);\ + rlwinm r3,r3,0,15,0; \ + ori r3,r3,vector_offset@l; \ + mtspr SPRN_IVOR##vector_number,r3; +#endif [Sethi Varun-B16395] Please add a documentation note here. #endif /* _ASM_POWERPC_EXCEPTION_64E_H */ diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S index 645170a..4b23119 100644 --- a/arch/powerpc/kernel/exceptions-64e.S +++ b/arch/powerpc/kernel/exceptions-64e.S @@ -1097,7 +1097,15 @@ skpinv:addir6,r6,1 /* Increment */ * r4 = MAS0 w/TLBSEL ESEL for the temp mapping */ /* Now we branch the new virtual address mapped by this entry */ +#ifdef CONFIG_RELOCATABLE + /* We have to find out address from lr. */ + bl 1f /* Find our address */ +1: mflrr6 + addir6,r6,(2f - 1b) + tovirt(r6,r6) +#else LOAD_REG_IMMEDIATE(r6,2f) +#endif lis r7,MSR_KERNEL@h ori r7,r7,MSR_KERNEL@l mtspr SPRN_SRR0,r6 @@ -1348,9 +1356,14 @@ _GLOBAL(book3e_secondary_thread_init) mflrr28 b 3b -_STATIC(init_core_book3e) +_GLOBAL(init_core_book3e) /* Establish the interrupt vector base */ +#ifdef CONFIG_RELOCATABLE + tovirt(r2,r2) + LOAD_REG_ADDR(r3, interrupt_base_book3e) #else LOAD_REG_IMMEDIATE(r3, interrupt_base_book3e) +#endif mtspr SPRN_IVPR,r3 sync blr [Sethi Varun-B16395] Please add a documentation note here as well. diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S index b61363d..0942f3a 100644 --- a/arch/powerpc/kernel/head_64.S +++ b/arch/powerpc/kernel/head_64.S @@ -414,12 +414,22 @@ _STATIC(__after_prom_start) /* process relocations for the final address of the kernel */ lis r25,PAGE_OFFSET@highest /* compute virtual base of kernel */ sldir25,r25,32 +#if defined(CONFIG_PPC_BOOK3E) + tovirt(r26,r26) /* on booke, we already run at PAGE_OFFSET */ +#endif lwz r7,__run_at_load-_stext(r26) +#if defined(CONFIG_PPC_BOOK3E) + tophys(r26,r26) /* Restore for the remains. */ +#endif cmplwi cr0,r7,1/* flagged to stay where we are ? */ bne 1f add r25,r25,r26 1: mr r3,r25 bl .relocate +#if defined(CONFIG_PPC_BOOK3E) + /* We should set ivpr again after .relocate. */ + bl .init_core_book3e +#endif #endif [Sethi Varun-B16395] A more detailed note over here would be useful. /* @@ -447,12 +457,24 @@ _STATIC(__after_prom_start) * variable __run_at_load, if it is set the kernel is treated as relocatable * kernel, otherwise it will be moved to PHYSICAL_START */ +#if defined(CONFIG_PPC_BOOK3E) + tovirt(r26,r26) /* on booke, we already run at PAGE_OFFSET */ +#endif lwz r7,__run_at_load-_stext(r26) +#if defined(CONFIG_PPC_BOOK3E) + tophys(r26,r26) /* Restore for the remains. */ +#endif cmplwi cr0,r7,1 bne 3f +#ifdef CONFIG_PPC_BOOK3E + LOAD_REG_ADDR(r5, interrupt_end_book3e) + LOAD_REG_ADDR(r11, _stext) + sub r5,r5,r11 +#else /* just copy interrupts
RE: [PATCH 3/3 v18] iommu/fsl: Freescale PAMU driver and iommu implementation.
Thanks Alex. -Original Message- From: Alex Williamson [mailto:alex.william...@redhat.com] Sent: Monday, July 01, 2013 8:05 PM To: Sethi Varun-B16395 Cc: j...@8bytes.org; io...@lists.linux-foundation.org; linuxppc- d...@lists.ozlabs.org; linux-ker...@vger.kernel.org; b...@kernel.crashing.org; ga...@kernel.crashing.org; Yoder Stuart-B08248; Wood Scott-B07421; Timur Tabi Subject: Re: [PATCH 3/3 v18] iommu/fsl: Freescale PAMU driver and iommu implementation. On Mon, 2013-07-01 at 15:41 +0530, Varun Sethi wrote: Following is a brief description of the PAMU hardware: PAMU determines what action to take and whether to authorize the action on the basis of the memory address, a Logical IO Device Number (LIODN), and PAACT table (logically) indexed by LIODN and address. Hardware devices which need to access memory must provide an LIODN in addition to the memory address. Peripheral Access Authorization and Control Tables (PAACTs) are the primary data structures used by PAMU. A PAACT is a table of peripheral access authorization and control entries (PAACE).Each PAACE defines the range of I/O bus address space that is accessible by the LIOD and the associated access capabilities. There are two types of PAACTs: primary PAACT (PPAACT) and secondary PAACT (SPAACT).A given physical I/O device may be able to act as one or more independent logical I/O devices (LIODs). Each such logical I/O device is assigned an identifier called logical I/O device number (LIODN). A LIODN is allocated a contiguous portion of the I/O bus address space called the DSA window for performing DSA operations. The DSA window may optionally be divided into multiple sub-windows, each of which may be used to map to a region in system storage space. The first sub-window is referred to as the primary sub-window and the remaining are called secondary sub-windows. This patch provides the PAMU driver (fsl_pamu.c) and the corresponding IOMMU API implementation (fsl_pamu_domain.c). The PAMU hardware driver (fsl_pamu.c) has been derived from the work done by Ashish Kalra and Timur Tabi. Signed-off-by: Timur Tabi ti...@tabi.org Signed-off-by: Varun Sethi varun.se...@freescale.com --- For iommu group support Acked-by: Alex Williamson alex.william...@redhat.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 3/3 v16] iommu/fsl: Freescale PAMU driver and iommu implementation.
-Original Message-
From: Alex Williamson [mailto:alex.william...@redhat.com]
Sent: Tuesday, June 25, 2013 10:27 AM
To: Sethi Varun-B16395
Cc: j...@8bytes.org; io...@lists.linux-foundation.org; linuxppc-
d...@lists.ozlabs.org; linux-ker...@vger.kernel.org;
b...@kernel.crashing.org; ga...@kernel.crashing.org; Yoder Stuart-B08248;
Wood Scott-B07421; Timur Tabi
Subject: Re: [PATCH 3/3 v16] iommu/fsl: Freescale PAMU driver and iommu
implementation.
On Thu, 2013-06-20 at 21:31 +0530, Varun Sethi wrote:
+#define REQ_ACS_FLAGS (PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR |
PCI_ACS_UF)
+
+static struct iommu_group *get_device_iommu_group(struct device *dev)
+{
+ struct iommu_group *group;
+
+ group = iommu_group_get(dev);
+ if (!group)
+ group = iommu_group_alloc();
+
+ return group;
+}
+
[snip]
+
This really gets parent or peer, right?
+static struct iommu_group *get_peer_pci_device_group(struct pci_dev
+*pdev) {
+ struct iommu_group *group = NULL;
+
+ /* check if this is the first device on the bus*/
+ if (pdev-bus_list.next == pdev-bus_list.prev) {
It's a list_head, use list functions. The list implementation should be
treated as opaque.
if (list_is_singular(pdev-bus_list))
+ struct pci_bus *bus = pdev-bus-parent;
+ /* Traverese the parent bus list to get
+* pdev dev for the sibling device.
+*/
+ while (bus) {
+ if (!list_empty(bus-devices)) {
+ pdev = container_of(bus-devices.next,
+ struct pci_dev, bus_list);
pdev = list_first_entry(bus-devices, struct pci_dev, bus_list);
+ group = iommu_group_get(pdev-dev);
+ break;
+ } else
+ bus = bus-parent;
Is this ever reached? Don't you always have bus-self?
[Sethi Varun-B16395] Not sure I understand. Trying to get the group information
from the parent bus, if there are no sibling devices on the current bus.
+ }
+ } else {
+ /*
+* Get the pdev dev for the sibling device
+*/
+ pdev = container_of(pdev-bus_list.prev,
+ struct pci_dev, bus_list);
How do you know if you're at the head or tail of the list?
struct pci_dev *tmp;
list_for_each_entry(tmp, pdev-bus_list, bus_list) {
if (tmp == pdev)
continue;
group = iommu_group_get(tmp-dev);
break;
}
+ group = iommu_group_get(pdev-dev);
+ }
+
+ return group;
+}
+
+static struct iommu_group *get_pci_device_group(struct pci_dev *pdev)
+{
+ struct iommu_group *group = NULL;
+ struct pci_dev *bridge, *dma_pdev = NULL;
+ struct pci_controller *pci_ctl;
+ bool pci_endpt_partioning;
+
+ 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 */
+ if (pci_endpt_partioning) {
+ bridge = pci_find_upstream_pcie_bridge(pdev);
+ if (bridge) {
+ if (pci_is_pcie(bridge))
+ dma_pdev = pci_get_domain_bus_and_slot(
+ pci_domain_nr(pdev-bus),
+ bridge-subordinate-number, 0);
+ if (!dma_pdev)
+ dma_pdev = pci_dev_get(bridge);
+ } else
+ dma_pdev = pci_dev_get(pdev);
+
+ /* Account for quirked devices */
+ swap_pci_ref(dma_pdev, pci_get_dma_source(dma_pdev));
+
+ /*
+* If it's a multifunction device that does not support our
+* required ACS flags, add to the same group as function 0.
+*/
See c14d2690 in Joerg's next tree, using function 0 was a poor
assumption.
[Sethi Varun-B16395] ok.
+ if (dma_pdev-multifunction
+ !pci_acs_enabled(dma_pdev, REQ_ACS_FLAGS))
+ swap_pci_ref(dma_pdev,
+pci_get_slot(dma_pdev-bus,
+ PCI_DEVFN(PCI_SLOT(dma_pdev-
devfn),
+ 0)));
+
+ group = get_device_iommu_group(pdev-dev);
+ pci_dev_put(pdev);
What was the point of all the above if we use pdev here instead of
dma_pdev? Wrong device and broken reference counting.
[Sethi Varun-B16395] Will fix this
This also isn't
testing ACS all the way up to the root complex or controller.
[Sethi Varun-B16395] In our case the IOMMU can differentiate transactions based
on the LIODN. The PCIe controller can generate a unique LIODN based on the
bus,device
RE: [PATCH 2/5 v11] powerpc: Add iommu domain pointer to device archdata
Hi Joerg, My PAMU driver patches depend on this patch which was Ack by Kumar. Should I resubmit this patch as well? Regards Varun -Original Message- From: Kumar Gala [mailto:ga...@kernel.crashing.org] Sent: Thursday, April 11, 2013 11:46 PM To: Sethi Varun-B16395 Cc: j...@8bytes.org; Yoder Stuart-B08248; Wood Scott-B07421; io...@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org; linux- ker...@vger.kernel.org; b...@kernel.crashing.org Subject: Re: [PATCH 2/5 v11] powerpc: Add iommu domain pointer to device archdata On Mar 28, 2013, at 2:53 PM, Varun Sethi wrote: Add an iommu domain pointer to device (powerpc) archdata. Devices are attached to iommu domains and this pointer provides a mechanism to correlate between a device and the associated iommu domain. This field is set when a device is attached to a domain. Signed-off-by: Varun Sethi varun.se...@freescale.com --- - no change in v11. - no change in v10. - Added CONFIG_IOMMU_API in v9. arch/powerpc/include/asm/device.h |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) Acked-by: Kumar Gala ga...@kernel.crashing.org - k ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] powerpc/pci: Support per-aperture memory offset
-Original Message- From: Benjamin Herrenschmidt [mailto:b...@kernel.crashing.org] Sent: Monday, May 06, 2013 12:20 PM To: linuxppc-dev Cc: Kumar Gala; Wood Scott-B07421; Sethi Varun-B16395; Thomas Petazzoni; Andrew Murray; Bjorn Helgaas; linux-...@vger.kernel.org Subject: [PATCH] powerpc/pci: Support per-aperture memory offset The PCI core supports an offset per aperture nowadays but our arch code still has a single offset per host bridge representing the difference betwen CPU memory addresses and PCI MMIO addresses. This is a problem as new machines and hypervisor versions are coming out where the 64-bit windows will have a different offset (basically mapped 1:1) from the 32-bit windows. This fixes it by using separate offsets. In the long run, we probably want to get rid of that intermediary struct pci_controller and have those directly stored into the pci_host_bridge as they are parsed but this will be a more invasive change. Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org --- Now, this is a big one but I'd like to still merge it for 3.10 because we're having new machine coming up (and new versions of pHyp on existing machines) that are going to expose MMIO windows with different offsets (basically our 64-bit windows are 1:1 and our 32-bit windows remapped). I'm not expecting any major issue with the patch, I've tested it on some of our machines here and will test it more during the next couple of days the notable thing is the removal of a workaround / fallback on 32-bit that I suspect only ever mattered for machines long unsupported (PReP ?) as I don't think we have anything that doesn't populate the bridge resources and expects to work nowadays (other stuff would break anyway). This is also why I'm NAK'ing the patch making pci_process_bridge_OF_ranges() generic since I need to change it for powerpc and this isn't the right long term approach (we should merge pci_controller pci_host_bridge instead and directly populate the pci_host_bridge apertures). If I see no major issue with the patch during the next few days, I'll send it to Linus with my next pull request, probably at -rc1. Kumar, Scott, Sethi, please test on FSL HW. I'll take care of macs and 4xx in addition to the various IBM ppc64 platforms. [Sethi Varun-B16395] Tested patch on FSL T4240, P4080, P5040 and P1020 boards. -Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 1/2 v15] iommu/fsl: Add additional iommu attributes required by the PAMU driver.
-Original Message- From: j...@8bytes.org [mailto:j...@8bytes.org] Sent: Thursday, May 02, 2013 3:46 PM To: Sethi Varun-B16395 Cc: io...@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org; linux-ker...@vger.kernel.org; ga...@kernel.crashing.org; b...@kernel.crashing.org; Yoder Stuart-B08248; Wood Scott-B07421 Subject: Re: [PATCH 1/2 v15] iommu/fsl: Add additional iommu attributes required by the PAMU driver. On Tue, Apr 30, 2013 at 05:09:32PM +, Sethi Varun-B16395 wrote: Would you take this patchset for 3.10 merge? Not this time. The final patch came in very late and is pretty big too. For code of that size I would like to have a few weeks more testing in next and probably also a non-Freescale Reviewed-by. [Sethi Varun-B16395] I would request you and Alex Williamson to review the patch and provide a Reviewed-by. -Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: pci overmapping
-Original Message-
From: Wood Scott-B07421
Sent: Thursday, May 02, 2013 10:44 PM
To: Yoder Stuart-B08248
Cc: ga...@kernel.crashing.org; Sethi Varun-B16395; linuxppc-
d...@lists.ozlabs.org
Subject: Re: pci overmapping
On 05/02/2013 12:05:42 PM, Yoder Stuart-B08248 wrote:
Kumar,
In fsl_pci.c there is a change you made a while back:
powerpc/fsl: Setup PCI inbound window based on actual amount of
memory
...and there is this comment in the code:
/* PCIe can overmap inbound outbound since RX TX are separated
*/
if (early_find_capability(hose, 0, 0, PCI_CAP_ID_EXP)) {
You are implying that PCIe can overmap and PCI can't. Why is
that? (I'm assuming that 'overmap' means that inbound window
can extend beyond the end of ram.)
Shouldn't the concern be whether we're overlapping outbound, not merely
whether we go beyond the end of RAM?
And couldn't inbound/outbound overlap be an issue even on PCIe, if
there's a PCI bridge underneath it?
I believe that the overlap problem would be avoided in case of 36 bit physical
support (outbound window address would be high order 36 bit value), right?
-Varun
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: pci overmapping
-Original Message-
From: Wood Scott-B07421
Sent: Thursday, May 02, 2013 11:50 PM
To: Sethi Varun-B16395
Cc: Wood Scott-B07421; Yoder Stuart-B08248; ga...@kernel.crashing.org;
linuxppc-dev@lists.ozlabs.org
Subject: Re: pci overmapping
On 05/02/2013 01:09:53 PM, Sethi Varun-B16395 wrote:
-Original Message-
From: Wood Scott-B07421
Sent: Thursday, May 02, 2013 10:44 PM
To: Yoder Stuart-B08248
Cc: ga...@kernel.crashing.org; Sethi Varun-B16395; linuxppc-
d...@lists.ozlabs.org
Subject: Re: pci overmapping
On 05/02/2013 12:05:42 PM, Yoder Stuart-B08248 wrote:
Kumar,
In fsl_pci.c there is a change you made a while back:
powerpc/fsl: Setup PCI inbound window based on actual amount of
memory
...and there is this comment in the code:
/* PCIe can overmap inbound outbound since RX TX are
separated
*/
if (early_find_capability(hose, 0, 0, PCI_CAP_ID_EXP)) {
You are implying that PCIe can overmap and PCI can't. Why is
that? (I'm assuming that 'overmap' means that inbound window
can extend beyond the end of ram.)
Shouldn't the concern be whether we're overlapping outbound, not
merely
whether we go beyond the end of RAM?
And couldn't inbound/outbound overlap be an issue even on PCIe, if
there's a PCI bridge underneath it?
I believe that the overlap problem would be avoided in case of 36 bit
physical support (outbound window address would be high order 36 bit
value), right?
Not necessarily -- it depends on what the bus addresses are, not the host
addresses.
The fsl_pci code already checks for the overlap condition. The inbound window
range is set to the minimum of memory end address or the start of pci outbound
address. In our case, the guest memory end address (host physical address
derived from the huge tlb page) is less than the pci outbound start address.
But, it could be the other way round as well. In both cases if the inbound
memory can't map the end of RAM address (for the guest), the fsl_pci driver
would enable swiotlb.
For the first case where end of memory is start of pci outbound address, we
can avoid enabling swiotlb (when end address is not a power of two) by over
committing the inbound address window.
-Varun
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 1/2 v15] iommu/fsl: Add additional iommu attributes required by the PAMU driver.
Hi Joerg,
Would you take this patchset for 3.10 merge?
Regards
Varun
-Original Message-
From: Sethi Varun-B16395
Sent: Wednesday, April 24, 2013 5:06 PM
To: j...@8bytes.org; io...@lists.linux-foundation.org; linuxppc-
d...@lists.ozlabs.org; linux-ker...@vger.kernel.org;
ga...@kernel.crashing.org; b...@kernel.crashing.org; Yoder Stuart-B08248;
Wood Scott-B07421
Cc: Sethi Varun-B16395
Subject: [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_COHERENCY0x1
#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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/3 v14] iommu/fsl: Add additional iommu attributes required by the PAMU driver.
-Original Message- From: Joerg Roedel [mailto:j...@8bytes.org] Sent: Wednesday, April 24, 2013 4:21 PM To: Sethi Varun-B16395 Cc: io...@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org; linux-ker...@vger.kernel.org; ga...@kernel.crashing.org; b...@kernel.crashing.org; Yoder Stuart-B08248; Wood Scott-B07421 Subject: Re: [PATCH 2/3 v14] iommu/fsl: Add additional iommu attributes required by the PAMU driver. On Tue, Apr 23, 2013 at 02:10:25PM +, Sethi Varun-B16395 wrote: I think it's fine to have the header under linux, actually I also the intel-iommu header under linux. Yes, the difference is that VT-d runs on x86 and on ia64. So there is no single arch where the header could be placed. The amd-iommu.h file on the other hand is x86 only and should also be moved to asm/, as I just found out :) And as long as PAMU is only needed on a single architecture the header should also be arch-specific. If that changes someday the header can be moved to a generic place. Ok, I will post the next version of the patch set. -Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 1/3 v2] iommu: Move swap_pci_ref function to pci.h.
Thanks. -Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Joerg Roedel Sent: Tuesday, April 23, 2013 6:32 PM To: Sethi Varun-B16395 Cc: ga...@kernel.crashing.org; b...@kernel.crashing.org; Yoder Stuart- B08248; linux-ker...@vger.kernel.org; io...@lists.linux-foundation.org; Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH 1/3 v2] iommu: Move swap_pci_ref function to pci.h. On Tue, Apr 23, 2013 at 10:05:24AM +0530, Varun Sethi wrote: +#ifndef __PCI_H +#define __PCI_H Using __PCI_H is not a wise choice, it has certainly a high risk of a collision. Anyway, I changed it to __IOMMU_PCI_H and applied the patch. Joerg ___ iommu mailing list io...@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/3 v14] iommu/fsl: Add additional iommu attributes required by the PAMU driver.
-Original Message- From: Joerg Roedel [mailto:j...@8bytes.org] Sent: Tuesday, April 23, 2013 7:09 PM To: Sethi Varun-B16395 Cc: io...@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org; linux-ker...@vger.kernel.org; ga...@kernel.crashing.org; b...@kernel.crashing.org; Yoder Stuart-B08248; Wood Scott-B07421 Subject: Re: [PATCH 2/3 v14] iommu/fsl: Add additional iommu attributes required by the PAMU driver. On Tue, Apr 23, 2013 at 10:05:25AM +0530, Varun Sethi wrote: 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 +++ Isn't asm/ for your architecture a better place for that header? I think it's fine to have the header under linux, actually I also the intel-iommu header under linux. -Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/3 v13] iommu/fsl: Add additional iommu attributes required by the PAMU driver.
-Original Message-
From: Wood Scott-B07421
Sent: Tuesday, April 23, 2013 5:20 AM
To: Sethi Varun-B16395
Cc: j...@8bytes.org; io...@lists.linux-foundation.org; linuxppc-
d...@lists.ozlabs.org; linux-ker...@vger.kernel.org;
ga...@kernel.crashing.org; b...@kernel.crashing.org; Yoder Stuart-B08248;
Sethi Varun-B16395
Subject: Re: [PATCH 2/3 v13] iommu/fsl: Add additional iommu attributes
required by the PAMU driver.
On 04/22/2013 12:31:55 AM, Varun Sethi wrote:
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
---
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 PAMUV1:
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_PAMU_STASH,
+ DOMAIN_ATTR_PAMU_ENABLE,
+ DOMAIN_ATTR_FSL_PAMUV1,
DOMAIN_ATTR_MAX,
Please be consistent on whether PAMU gets an FSL_ namespace prefix
(I'd prefer that it does).
Submitted new version(v14) of the patch with updated attribute names.
-Varun
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 1/3] iommu: Move swap_pci_ref function to pci.h.
Fine, I will move it to iommu.h. -Varun -Original Message- From: Joerg Roedel [mailto:j...@8bytes.org] Sent: Monday, April 15, 2013 8:29 PM To: Sethi Varun-B16395 Cc: Yoder Stuart-B08248; Wood Scott-B07421; iommu@lists.linux- foundation.org; linuxppc-dev@lists.ozlabs.org; linux- ker...@vger.kernel.org; ga...@kernel.crashing.org; b...@kernel.crashing.org; bhelg...@google.com Subject: Re: [PATCH 1/3] iommu: Move swap_pci_ref function to pci.h. On Mon, Apr 15, 2013 at 12:42:00AM +0530, Varun Sethi wrote: 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 pci.h so that the implementation can be shared across various IOMMU drivers. The function is only used in IOMMU code, so I think its fine to keep it there (unless Bjorn disagrees and wants it in PCI code). Joerg ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 5/5 v11] iommu/fsl: Freescale PAMU driver and iommu implementation.
-Original Message- From: Alex Williamson [mailto:alex.william...@redhat.com] Sent: Wednesday, April 03, 2013 11:32 PM To: Joerg Roedel Cc: Sethi Varun-B16395; Yoder Stuart-B08248; Wood Scott-B07421; io...@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org; linux- ker...@vger.kernel.org; ga...@kernel.crashing.org; b...@kernel.crashing.org Subject: Re: [PATCH 5/5 v11] iommu/fsl: Freescale PAMU driver and iommu implementation. On Tue, 2013-04-02 at 18:18 +0200, Joerg Roedel wrote: Cc'ing Alex Williamson Alex, can you please review the iommu-group part of this patch? Sure, it looks pretty reasonable. AIUI, all PCI devices are below some kind of host bridge that is either new and supports partitioning or old and doesn't. I don't know if that's a visibility or isolation requirement, perhaps PCI ACS-ish. In the new host bridge case, each device gets a group. This seems not to have any quirks for multifunction devices though. On AMD and Intel IOMMUs we test multifunction device ACS support to determine whether all the functions should be in the same group. Is there any reason to trust multifunction devices on PAMU? [Sethi Varun-B16395] In the case where we can partition endpoints we can distinguish transactions based on the bus,device,function number combination. This support is available in the PCIe controller (host bridge). I also find it curious what happens to the iommu group of the host bridge. In the partitionable case the host bridge group is removed, in the non-partitionable case the host bridge group becomes the group for the children, removing the host bridge. It's unique to PAMU so far that these host bridges are even in an iommu group (x86 only adds pci devices), but I don't see it as necessarily wrong leaving it in either scenario. Does it solve some problem to remove them from the groups? Thanks, [Sethi Varun-B16395] The PCIe controller isn't a partitionable entity, it would always be owned by the host. -Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 5/5 v11] iommu/fsl: Freescale PAMU driver and iommu implementation.
-Original Message- From: Alex Williamson [mailto:alex.william...@redhat.com] Sent: Thursday, April 04, 2013 8:52 PM To: Sethi Varun-B16395 Cc: Joerg Roedel; Yoder Stuart-B08248; Wood Scott-B07421; io...@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org; linux- ker...@vger.kernel.org; ga...@kernel.crashing.org; b...@kernel.crashing.org Subject: Re: [PATCH 5/5 v11] iommu/fsl: Freescale PAMU driver and iommu implementation. On Thu, 2013-04-04 at 13:00 +, Sethi Varun-B16395 wrote: -Original Message- From: Alex Williamson [mailto:alex.william...@redhat.com] Sent: Wednesday, April 03, 2013 11:32 PM To: Joerg Roedel Cc: Sethi Varun-B16395; Yoder Stuart-B08248; Wood Scott-B07421; io...@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org; linux- ker...@vger.kernel.org; ga...@kernel.crashing.org; b...@kernel.crashing.org Subject: Re: [PATCH 5/5 v11] iommu/fsl: Freescale PAMU driver and iommu implementation. On Tue, 2013-04-02 at 18:18 +0200, Joerg Roedel wrote: Cc'ing Alex Williamson Alex, can you please review the iommu-group part of this patch? Sure, it looks pretty reasonable. AIUI, all PCI devices are below some kind of host bridge that is either new and supports partitioning or old and doesn't. I don't know if that's a visibility or isolation requirement, perhaps PCI ACS-ish. In the new host bridge case, each device gets a group. This seems not to have any quirks for multifunction devices though. On AMD and Intel IOMMUs we test multifunction device ACS support to determine whether all the functions should be in the same group. Is there any reason to trust multifunction devices on PAMU? [Sethi Varun-B16395] In the case where we can partition endpoints we can distinguish transactions based on the bus,device,function number combination. This support is available in the PCIe controller (host bridge). So can x86 IOMMUs, that's the visibility aspect of IOMMU groups. Visibility alone doesn't necessarily imply that a device is isolated though. A multifunction PCI device that doesn't expose ACS support may not isolate functions from each other. For example a peer-to-peer DMA between functions may not be translated by the upstream IOMMU. IOMMU groups should encompass both visibility and isolation. [Sethi Varun-B16395] We can isolate the DMA access to the host based on the to the pci bus,device,function number. I thought that was enough to put devices in to separate iommu groups. This is a PCIe controller property which allows us to partition PCIe devices. But, what I can understand from your point is that we also need to consider isolation at PCIe device level as well. I will check for the case of multifunction devices. I also find it curious what happens to the iommu group of the host bridge. In the partitionable case the host bridge group is removed, in the non-partitionable case the host bridge group becomes the group for the children, removing the host bridge. It's unique to PAMU so far that these host bridges are even in an iommu group (x86 only adds pci devices), but I don't see it as necessarily wrong leaving it in either scenario. Does it solve some problem to remove them from the groups? Thanks, [Sethi Varun-B16395] The PCIe controller isn't a partitionable entity, it would always be owned by the host. Ownership of a device shouldn't play into the group context. An IOMMU group should be defined by it's visibility and isolation from other devices. Whether the PCIe controller is allowed to be handed to userspace is a question for VFIO. [Sethi Varun-B16395] The problem is in the case, where we can't partition PCIe devices. PCIe devices share the same device group as the PCI controller. This becomes a problem while assigning the devices to the guest, as you are required to unbind all the PCIe devices including the controller from the host. PCIe controller can't be unbound from the host, so we simply delete the controller iommu_group. -Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 5/5 v11] iommu/fsl: Freescale PAMU driver and iommu implementation.
-Original Message- From: Alex Williamson [mailto:alex.william...@redhat.com] Sent: Thursday, April 04, 2013 10:14 PM To: Sethi Varun-B16395 Cc: Joerg Roedel; Yoder Stuart-B08248; Wood Scott-B07421; io...@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org; linux- ker...@vger.kernel.org; ga...@kernel.crashing.org; b...@kernel.crashing.org Subject: Re: [PATCH 5/5 v11] iommu/fsl: Freescale PAMU driver and iommu implementation. On Thu, 2013-04-04 at 16:35 +, Sethi Varun-B16395 wrote: -Original Message- From: Alex Williamson [mailto:alex.william...@redhat.com] Sent: Thursday, April 04, 2013 8:52 PM To: Sethi Varun-B16395 Cc: Joerg Roedel; Yoder Stuart-B08248; Wood Scott-B07421; io...@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org; linux- ker...@vger.kernel.org; ga...@kernel.crashing.org; b...@kernel.crashing.org Subject: Re: [PATCH 5/5 v11] iommu/fsl: Freescale PAMU driver and iommu implementation. On Thu, 2013-04-04 at 13:00 +, Sethi Varun-B16395 wrote: -Original Message- From: Alex Williamson [mailto:alex.william...@redhat.com] Sent: Wednesday, April 03, 2013 11:32 PM To: Joerg Roedel Cc: Sethi Varun-B16395; Yoder Stuart-B08248; Wood Scott-B07421; io...@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org; linux- ker...@vger.kernel.org; ga...@kernel.crashing.org; b...@kernel.crashing.org Subject: Re: [PATCH 5/5 v11] iommu/fsl: Freescale PAMU driver and iommu implementation. On Tue, 2013-04-02 at 18:18 +0200, Joerg Roedel wrote: Cc'ing Alex Williamson Alex, can you please review the iommu-group part of this patch? Sure, it looks pretty reasonable. AIUI, all PCI devices are below some kind of host bridge that is either new and supports partitioning or old and doesn't. I don't know if that's a visibility or isolation requirement, perhaps PCI ACS-ish. In the new host bridge case, each device gets a group. This seems not to have any quirks for multifunction devices though. On AMD and Intel IOMMUs we test multifunction device ACS support to determine whether all the functions should be in the same group. Is there any reason to trust multifunction devices on PAMU? [Sethi Varun-B16395] In the case where we can partition endpoints we can distinguish transactions based on the bus,device,function number combination. This support is available in the PCIe controller (host bridge). So can x86 IOMMUs, that's the visibility aspect of IOMMU groups. Visibility alone doesn't necessarily imply that a device is isolated though. A multifunction PCI device that doesn't expose ACS support may not isolate functions from each other. For example a peer-to-peer DMA between functions may not be translated by the upstream IOMMU. IOMMU groups should encompass both visibility and isolation. [Sethi Varun-B16395] We can isolate the DMA access to the host based on the to the pci bus,device,function number. The IOMMU can only isolate DMA that it can see. A multifunction device may never expose peer-to-peer DMA to the upstream device, it's implementation specific. The ACS flags allow that possibility to be controlled and prevented. I thought that was enough to put devices in to separate iommu groups. This is a PCIe controller property which allows us to partition PCIe devices. But, what I can understand from your point is that we also need to consider isolation at PCIe device level as well. I will check for the case of multifunction devices. I also find it curious what happens to the iommu group of the host bridge. In the partitionable case the host bridge group is removed, in the non-partitionable case the host bridge group becomes the group for the children, removing the host bridge. It's unique to PAMU so far that these host bridges are even in an iommu group (x86 only adds pci devices), but I don't see it as necessarily wrong leaving it in either scenario. Does it solve some problem to remove them from the groups? Thanks, [Sethi Varun-B16395] The PCIe controller isn't a partitionable entity, it would always be owned by the host. Ownership of a device shouldn't play into the group context. An IOMMU group should be defined by it's visibility and isolation from other devices. Whether the PCIe controller is allowed to be handed to userspace is a question for VFIO. [Sethi Varun-B16395] The problem is in the case, where we can't partition PCIe devices. PCIe devices share the same device group as the PCI controller. This becomes a problem while assigning the devices to the guest, as you are required to unbind all the PCIe devices including the controller from the host. PCIe controller can't be unbound from the host, so we simply delete
RE: [PATCH 5/5 v11] iommu/fsl: Freescale PAMU driver and iommu implementation.
-Original Message-
From: Joerg Roedel [mailto:j...@8bytes.org]
Sent: Tuesday, April 02, 2013 9:48 PM
To: Sethi Varun-B16395
Cc: Yoder Stuart-B08248; Wood Scott-B07421; iommu@lists.linux-
foundation.org; linuxppc-dev@lists.ozlabs.org; linux-
ker...@vger.kernel.org; ga...@kernel.crashing.org;
b...@kernel.crashing.org; Alex Williamson
Subject: Re: [PATCH 5/5 v11] iommu/fsl: Freescale PAMU driver and iommu
implementation.
Cc'ing Alex Williamson
Alex, can you please review the iommu-group part of this patch?
My comments so far are below:
On Fri, Mar 29, 2013 at 01:24:02AM +0530, Varun Sethi wrote:
+config FSL_PAMU
+ bool Freescale IOMMU support
+ depends on PPC_E500MC
+ select IOMMU_API
+ select GENERIC_ALLOCATOR
+ help
+ Freescale PAMU support.
A bit lame for a help text. Can you elaborate more what PAMU is and when
it should be enabled?
[Sethi Varun-B16395] Will update the description.
+int pamu_enable_liodn(int liodn)
+{
+ struct paace *ppaace;
+
+ ppaace = pamu_get_ppaace(liodn);
+ if (!ppaace) {
+ pr_err(Invalid primary paace entry\n);
+ return -ENOENT;
+ }
+
+ if (!get_bf(ppaace-addr_bitfields, PPAACE_AF_WSE)) {
+ pr_err(liodn %d not configured\n, liodn);
+ return -EINVAL;
+ }
+
+ /* Ensure that all other stores to the ppaace complete first */
+ mb();
+
+ ppaace-addr_bitfields |= PAACE_V_VALID;
+ mb();
Why is it sufficient to set the bit in a variable when enabling liodn but
when disabling it set_bf needs to be called? This looks a bit assymetric.
[Sethi Varun-B16395] Will make it symetric :)
+/* Derive the window size encoding for a particular PAACE entry */
+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)));
Please use is_power_of_2 here.
+
+ /* window size is 2^(WSE+1) bytes */
+ return __ffs(addrspace_size PAMU_PAGE_SHIFT) + PAMU_PAGE_SHIFT -
+1;
The PAMU_PAGE_SHIFT shifting and adding looks redundant.
+ if ((win_size (win_size - 1)) || win_size PAMU_PAGE_SIZE) {
+ pr_err(window size too small or not a power of two %llx\n,
win_size);
+ return -EINVAL;
+ }
+
+ if (win_addr (win_size - 1)) {
+ pr_err(window address is not aligned with window size\n);
+ return -EINVAL;
+ }
Again, use is_power_of_2 instead of hand-coding.
[Sethi Varun-B16395] ok
+ if (~stashid != 0)
+ set_bf(paace-impl_attr, PAACE_IA_CID, stashid);
+
+ smp_wmb();
+
+ if (enable)
+ paace-addr_bitfields |= PAACE_V_VALID;
Havn't you written a helper funtion to set this bit?
[Sethi Varun-B16395] We already have a PAACE entry with us here so we can
directly manipulate it here.
+irqreturn_t pamu_av_isr(int irq, void *arg) {
+ struct pamu_isr_data *data = arg;
+ phys_addr_t phys;
+ unsigned int i, j;
+
+ pr_emerg(fsl-pamu: access violation interrupt\n);
+
+ for (i = 0; i data-count; i++) {
+ void __iomem *p = data-pamu_reg_base + i * PAMU_OFFSET;
+ u32 pics = in_be32(p + PAMU_PICS);
+
+ if (pics PAMU_ACCESS_VIOLATION_STAT) {
+ pr_emerg(POES1=%08x\n, in_be32(p + PAMU_POES1));
+ pr_emerg(POES2=%08x\n, in_be32(p + PAMU_POES2));
+ pr_emerg(AVS1=%08x\n, in_be32(p + PAMU_AVS1));
+ pr_emerg(AVS2=%08x\n, in_be32(p + PAMU_AVS2));
+ pr_emerg(AVA=%016llx\n, make64(in_be32(p +
PAMU_AVAH),
+ in_be32(p + PAMU_AVAL)));
+ pr_emerg(UDAD=%08x\n, in_be32(p + PAMU_UDAD));
+ pr_emerg(POEA=%016llx\n, make64(in_be32(p +
PAMU_POEAH),
+ in_be32(p + PAMU_POEAL)));
+
+ phys = make64(in_be32(p + PAMU_POEAH),
+ in_be32(p + PAMU_POEAL));
+
+ /* Assume that POEA points to a PAACE */
+ if (phys) {
+ u32 *paace = phys_to_virt(phys);
+
+ /* Only the first four words are relevant */
+ for (j = 0; j 4; j++)
+ pr_emerg(PAACE[%u]=%08x\n, j,
in_be32(paace + j));
+ }
+ }
+ }
+
+ panic(\n);
A kernel panic seems like an over-reaction to an access violation.
Besides the device that caused the violation the system should still
work, no?
[Sethi Varun-B16395] Well, if device continues to DMA outside the set aperture
then it's a serious problem. We can run in to an interrupt storm (access
violations).
Alternative could be to disable LIODN, but after that device DMAs would never
go through. So, for the guest device is not functional
RE: [PATCH 0/5 v11] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
-Original Message- From: Joerg Roedel [mailto:j...@8bytes.org] Sent: Tuesday, April 02, 2013 9:53 PM To: Sethi Varun-B16395 Cc: Yoder Stuart-B08248; Wood Scott-B07421; iommu@lists.linux- foundation.org; linuxppc-dev@lists.ozlabs.org; linux- ker...@vger.kernel.org; ga...@kernel.crashing.org; b...@kernel.crashing.org Subject: Re: [PATCH 0/5 v11] iommu/fsl: Freescale PAMU driver and IOMMU API implementation. On Fri, Mar 29, 2013 at 01:23:57AM +0530, Varun Sethi wrote: This patchset provides the Freescale PAMU (Peripheral Access Management Unit) driver and the corresponding IOMMU API implementation. PAMU is the IOMMU present on Freescale QorIQ platforms. PAMU can authorize memory access, remap the memory address, and remap the I/O transaction type. This set consists of the following patches: 1. Make iova dma_addr_t in the iommu_iova_to_phys API. 2. Addition of new field in the device (powerpc) archdata structure for storing iommu domain information pointer. 3. Add window permission flags in the iommu_domain_window_enable API. 4. Add domain attributes for FSL PAMU driver. 5. PAMU driver and IOMMU API implementation. Okay, I am about to apply patches 1 and 3 to a new ppc/pamu branch in my tree. As a general question, how did you test the IOMMU driver and what will you do in the future to avoid regressions? I use a kernel module for testing the iommu_api support. The module allows for dynamic creation and deletion of iommu domains for the devices in the device tree. Also, the vfio support (under development) for Freescale SOCs with APMU hardware would depend on the PAMU driver. -Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 5/5 v11] iommu/fsl: Freescale PAMU driver and iommu implementation.
-Original Message- From: Wood Scott-B07421 Sent: Wednesday, April 03, 2013 7:23 AM To: Timur Tabi Cc: Joerg Roedel; Sethi Varun-B16395; lkml; Kumar Gala; Yoder Stuart- B08248; io...@lists.linux-foundation.org; Benjamin Herrenschmidt; linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH 5/5 v11] iommu/fsl: Freescale PAMU driver and iommu implementation. On 04/02/2013 08:35:54 PM, Timur Tabi wrote: On Tue, Apr 2, 2013 at 11:18 AM, Joerg Roedel j...@8bytes.org wrote: + panic(\n); A kernel panic seems like an over-reaction to an access violation. We have no way to determining what code caused the violation, so we can't just kill the process. I agree it seems like overkill, but what else should we do? Does the IOMMU layer have a way for the IOMMU driver to stop the device that caused the problem? At a minimum, log a message and continue. Probably turn off the LIODN, at least if it continues to be noisy (otherwise we could get stuck in an interrupt storm as you note). Possibly let the user know somehow, especially if it's a VFIO domain. [Sethi Varun-B16395] Can definitely log the message and disable the LIODN (to avoid an interrupt storm), but we definitely need a mechanism to inform vfio subsystem about the error. Also, disabling LIODN may not be a viable option with the new LIODN allocation scheme (where LIODN would be associated with a domain). Don't take down the whole kernel. It's not just overkill; it undermines VFIO's efforts to make it safe for users to control devices. Besides the device that caused the violation the system should still work, no? Not really. The PAMU was designed to add IOMMU support to legacy devices, which have no concept of an MMU. If the PAMU detects an access violation, there's no way for the device to recover, because it has no idea that a violation has occurred. It's going to keep on writing to bad data. I think that's only the case for posted writes (or devices which fail to take a hint and stop even after they see an I/O error). [Sethi Varun-B16395] Even in the case where the guest driver detects a failure, it may not be able to fix the problem without intervention from the VFIO subsystem. -Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/5 v11] powerpc: Add iommu domain pointer to device archdata
Kumar/Ben, Any comments? (Had checked with Ben (on IRC) sometime back, he was fine with this patch) Regards Varun -Original Message- From: Joerg Roedel [mailto:j...@8bytes.org] Sent: Tuesday, April 02, 2013 8:39 PM To: Sethi Varun-B16395 Cc: Yoder Stuart-B08248; Wood Scott-B07421; iommu@lists.linux- foundation.org; linuxppc-dev@lists.ozlabs.org; linux- ker...@vger.kernel.org; ga...@kernel.crashing.org; b...@kernel.crashing.org Subject: Re: [PATCH 2/5 v11] powerpc: Add iommu domain pointer to device archdata On Fri, Mar 29, 2013 at 01:23:59AM +0530, Varun Sethi wrote: Add an iommu domain pointer to device (powerpc) archdata. Devices are attached to iommu domains and this pointer provides a mechanism to correlate between a device and the associated iommu domain. This field is set when a device is attached to a domain. Signed-off-by: Varun Sethi varun.se...@freescale.com This patch needs to be Acked by the PPC maintainers. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 4/5 v11] iommu/fsl: Add additional iommu attributes required by the PAMU driver.
-Original Message-
From: Joerg Roedel [mailto:j...@8bytes.org]
Sent: Tuesday, April 02, 2013 8:40 PM
To: Sethi Varun-B16395
Cc: Yoder Stuart-B08248; Wood Scott-B07421; iommu@lists.linux-
foundation.org; linuxppc-dev@lists.ozlabs.org; linux-
ker...@vger.kernel.org; ga...@kernel.crashing.org;
b...@kernel.crashing.org
Subject: Re: [PATCH 4/5 v11] iommu/fsl: Add additional iommu attributes
required by the PAMU driver.
On Fri, Mar 29, 2013 at 01:24:01AM +0530, Varun Sethi wrote:
+/* cache stash targets */
+enum stash_target {
+ IOMMU_ATTR_CACHE_L1 = 1,
+ IOMMU_ATTR_CACHE_L2,
+ IOMMU_ATTR_CACHE_L3,
+};
+
+/* This attribute corresponds to IOMMUs capable of generating
+ * a stash transaction. A stash transaction is typically a
+ * hardware initiated prefetch of data from memory to cache.
+ * This attribute allows configuring stashig specific parameters
+ * in the IOMMU hardware.
+ */
+
+struct iommu_stash_attribute {
+ u32 cpu;/* cpu number */
+ u32 cache; /* cache to stash to: L1,L2,L3 */
+};
+
I would prefer these PAMU specific enum and struct to be in a pamu-
specific iommu-header.
[Sethi Varun-B16395] But, these would be used by the IOMMU API users (e.g.
VFIO), they shouldn't depend on PAMU specific headers.
-Varun
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 5/5 v9] iommu/fsl: Freescale PAMU driver and iommu implementation.
-Original Message-
From: iommu-boun...@lists.linux-foundation.org [mailto:iommu-
boun...@lists.linux-foundation.org] On Behalf Of Kumar Gala
Sent: Friday, March 15, 2013 1:50 AM
To: Sethi Varun-B16395
Cc: b...@kernel.crashing.org; linux-ker...@vger.kernel.org; Yoder Stuart-
B08248; io...@lists.linux-foundation.org; Wood Scott-B07421; linuxppc-
d...@lists.ozlabs.org
Subject: Re: [PATCH 5/5 v9] iommu/fsl: Freescale PAMU driver and iommu
implementation.
On Mar 13, 2013, at 1:49 PM, Varun Sethi wrote:
+/*
+ * Table of SVRs and the corresponding PORT_ID values.
+ *
+ * All future CoreNet-enabled SOCs will have this erratum fixed, so
+this table
+ * should never need to be updated. SVRs are guaranteed to be
+unique, so
+ * there is no worry that a future SOC will inadvertently have one of
+these
+ * values.
+ */
Maybe add to the comment about what port_id represents
[Sethi Varun-B16395] ok.
+static const struct {
+ u32 svr;
+ u32 port_id;
+} port_id_map[] = {
+ {0x82100010, 0xFF00}, /* P2040 1.0 */
+ {0x82100011, 0xFF00}, /* P2040 1.1 */
+ {0x82100110, 0xFF00}, /* P2041 1.0 */
+ {0x82100111, 0xFF00}, /* P2041 1.1 */
+ {0x82110310, 0xFF00}, /* P3041 1.0 */
+ {0x82110311, 0xFF00}, /* P3041 1.1 */
+ {0x82010020, 0xFFF8}, /* P4040 2.0 */
+ {0x8220, 0xFFF8}, /* P4080 2.0 */
+ {0x82210010, 0xFC00}, /* P5010 1.0 */
+ {0x82210020, 0xFC00}, /* P5010 2.0 */
+ {0x82200010, 0xFC00}, /* P5020 1.0 */
+ {0x82050010, 0xFF80}, /* P5021 1.0 */
+ {0x82040010, 0xFF80}, /* P5040 1.0 */
+};
+
+#define SVR_SECURITY 0x8 /* The Security (E) bit */
+
+static int __init fsl_pamu_probe(struct platform_device *pdev) {
+ void __iomem *pamu_regs = NULL;
+ struct ccsr_guts __iomem *guts_regs = NULL;
+ u32 pamubypenr, pamu_counter;
+ unsigned long pamu_reg_off;
+ unsigned long pamu_reg_base;
+ struct pamu_isr_data *data;
+ struct device_node *guts_node;
+ u64 size;
+ struct page *p;
+ int ret = 0;
+ int irq;
+ phys_addr_t ppaact_phys;
+ phys_addr_t spaact_phys;
+ phys_addr_t omt_phys;
+ size_t mem_size = 0;
+ unsigned int order = 0;
+ u32 csd_port_id = 0;
+ unsigned i;
+ /*
+* enumerate all PAMUs and allocate and setup PAMU tables
+* for each of them,
+* NOTE : All PAMUs share the same LIODN tables.
+*/
+
+ pamu_regs = of_iomap(pdev-dev.of_node, 0);
+ if (!pamu_regs) {
+ dev_err(pdev-dev, ioremap of PAMU node failed\n);
+ return -ENOMEM;
+ }
+ of_get_address(pdev-dev.of_node, 0, size, NULL);
+
+ irq = irq_of_parse_and_map(pdev-dev.of_node, 0);
+ if (irq == NO_IRQ) {
+ dev_warn(pdev-dev, no interrupts listed in PAMU node\n);
+ goto error;
+ }
+
+ data = kzalloc(sizeof(struct pamu_isr_data), GFP_KERNEL);
+ if (!data) {
+ iounmap(pamu_regs);
+ return -ENOMEM;
+ }
+ data-pamu_reg_base = pamu_regs;
+ data-count = size / PAMU_OFFSET;
+
+ /* The ISR needs access to the regs, so we won't iounmap them */
+ ret = request_irq(irq, pamu_av_isr, 0, pamu, data);
+ if (ret 0) {
+ dev_err(pdev-dev, error %i installing ISR for irq %i\n,
+ ret, irq);
+ goto error;
+ }
+
+ guts_node = of_find_compatible_node(NULL, NULL,
+ fsl,qoriq-device-config-1.0);
This doesn't work for T4 or B4 device trees.
[Sethi Varun-B16395]hmm I need to use the dcfg space for this.
+ if (!guts_node) {
+ dev_err(pdev-dev, could not find GUTS node %s\n,
+ pdev-dev.of_node-full_name);
+ ret = -ENODEV;
+ goto error;
+ }
+
+ guts_regs = of_iomap(guts_node, 0);
+ of_node_put(guts_node);
+ if (!guts_regs) {
+ dev_err(pdev-dev, ioremap of GUTS node failed\n);
+ ret = -ENODEV;
+ goto error;
+ }
+
+ /* read in the PAMU capability registers */
+ get_pamu_cap_values((unsigned long)pamu_regs);
+ /*
+* To simplify the allocation of a coherency domain, we allocate
the
+* PAACT and the OMT in the same memory buffer. Unfortunately,
this
+* wastes more memory compared to allocating the buffers
separately.
+*/
+ /* Determine how much memory we need */
+ mem_size = (PAGE_SIZE get_order(PAACT_SIZE)) +
+ (PAGE_SIZE get_order(SPAACT_SIZE)) +
+ (PAGE_SIZE get_order(OMT_SIZE));
+ order = get_order(mem_size);
+
+ p = alloc_pages(GFP_KERNEL | __GFP_ZERO, order);
+ if (!p) {
+ dev_err(pdev-dev, unable to allocate PAACT/SPAACT/OMT
block\n);
+ ret = -ENOMEM;
+ goto error
RE: [PATCH 5/5 v9] iommu/fsl: Freescale PAMU driver and iommu implementation.
-Original Message- From: Kumar Gala [mailto:ga...@kernel.crashing.org] Sent: Friday, March 15, 2013 1:53 AM To: Sethi Varun-B16395 Cc: j...@8bytes.org; linux-ker...@vger.kernel.org; Yoder Stuart-B08248; io...@lists.linux-foundation.org; Wood Scott-B07421; linuxppc- d...@lists.ozlabs.org Subject: Re: [PATCH 5/5 v9] iommu/fsl: Freescale PAMU driver and iommu implementation. On Mar 14, 2013, at 3:20 PM, Kumar Gala wrote: On Mar 13, 2013, at 1:49 PM, Varun Sethi wrote: +/* + * Table of SVRs and the corresponding PORT_ID values. + * + * All future CoreNet-enabled SOCs will have this erratum fixed, so +this table + * should never need to be updated. SVRs are guaranteed to be +unique, so + * there is no worry that a future SOC will inadvertently have one +of these + * values. + */ Maybe add to the comment about what port_id represents also, add reference to the erratum #/id/name [Sethi Varun-B16395] Ok. -Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 5/5 v9] iommu/fsl: Freescale PAMU driver and iommu implementation.
-Original Message- From: Yoder Stuart-B08248 Sent: Friday, March 15, 2013 2:45 AM To: Kumar Gala; Sethi Varun-B16395 Cc: j...@8bytes.org; io...@lists.linux-foundation.org; linuxppc- d...@lists.ozlabs.org; linux-ker...@vger.kernel.org; b...@kernel.crashing.org; Wood Scott-B07421 Subject: RE: [PATCH 5/5 v9] iommu/fsl: Freescale PAMU driver and iommu implementation. -Original Message- From: Kumar Gala [mailto:ga...@kernel.crashing.org] Sent: Thursday, March 14, 2013 3:20 PM To: Sethi Varun-B16395 Cc: j...@8bytes.org; io...@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org; linux- ker...@vger.kernel.org; b...@kernel.crashing.org; Wood Scott-B07421; Yoder Stuart-B08248 Subject: Re: [PATCH 5/5 v9] iommu/fsl: Freescale PAMU driver and iommu implementation. On Mar 13, 2013, at 1:49 PM, Varun Sethi wrote: +/* + * Table of SVRs and the corresponding PORT_ID values. + * + * All future CoreNet-enabled SOCs will have this erratum fixed, so +this table + * should never need to be updated. SVRs are guaranteed to be +unique, so + * there is no worry that a future SOC will inadvertently have one +of these + * values. + */ Maybe add to the comment about what port_id represents When you update the comment, I would also suggest identifying the specific errata here (A-004510) so that it's easy to reference back to the specific issue this code is fixing. [Sethi Varun-B16395] Ok -Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/6] powerpc/fsl_pci: Store the platform device information corresponding to the pci controller.
Hi Joerg, I have to post the next version of my patchset, should I base it on top of 3.9-rc1? By when would you move the iommu git tree to 3.9-rc1? Regards Varun -Original Message- From: Kumar Gala [mailto:ga...@kernel.crashing.org] Sent: Thursday, February 28, 2013 9:15 PM To: Sethi Varun-B16395 Cc: Joerg Roedel; Stuart Yoder; io...@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org; linux-ker...@vger.kernel.org; Wood Scott- B07421; Yoder Stuart-B08248 Subject: Re: [PATCH 2/6] powerpc/fsl_pci: Store the platform device information corresponding to the pci controller. On Feb 27, 2013, at 4:56 AM, Sethi Varun-B16395 wrote: This patch is present in the next branch of linux ppc tree maintained by Kumar Gala. Following is the commit id: 52c5affc545053d37c0b05224bbf70f5336caa20 I am not sure if this would be part of 3.9-rc1. Regards varun This is now in Linus's tree so will be in 3.9-rc1 - k ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 6/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
-Original Message-
From: Stuart Yoder [mailto:b08...@gmail.com]
Sent: Saturday, March 02, 2013 4:58 AM
To: Sethi Varun-B16395
Cc: io...@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org;
linux-ker...@vger.kernel.org; Wood Scott-B07421; Joerg Roedel; Yoder
Stuart-B08248
Subject: Re: [PATCH 6/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU
API implementation.
On Mon, Feb 18, 2013 at 6:52 AM, Varun Sethi varun.se...@freescale.com
wrote:
[cut]
+static phys_addr_t get_phys_addr(struct fsl_dma_domain *dma_domain,
+unsigned long iova) {
+ u32 win_cnt = dma_domain-win_cnt;
+ struct dma_window *win_ptr =
+ dma_domain-win_arr[0];
+ struct iommu_domain_geometry *geom;
+
+ geom = dma_domain-iommu_domain-geometry;
+
+ if (!win_cnt || !dma_domain-geom_size) {
+ pr_err(Number of windows/geometry not configured for
the domain\n);
+ return 0;
+ }
+
+ if (win_cnt 1) {
+ u64 subwin_size;
+ unsigned long subwin_iova;
+ u32 wnd;
+
+ subwin_size = dma_domain-geom_size ilog2(win_cnt);
Could it be just geom_size / win_cnt ??
[Sethi Varun-B16395] You would run in to linking issues with respect to u64
division on 32 bit kernels.
+ subwin_iova = iova ~(subwin_size - 1);
+ wnd = (subwin_iova - geom-aperture_start)
ilog2(subwin_size);
+ win_ptr = dma_domain-win_arr[wnd];
+ }
+
+ if (win_ptr-valid)
+ return (win_ptr-paddr + (iova (win_ptr-size -
+ 1)));
+
+ return 0;
+}
+
+static int map_liodn_subwins(int liodn, struct fsl_dma_domain
+*dma_domain)
Just call it map_subwins(). They are just sub windows, not liodn sub
windows.
[Sethi Varun-B16395] This would be called per liodn.
[cut]
+static int map_liodn_win(int liodn, struct fsl_dma_domain
+*dma_domain)
Call it map_win().
[Sethi Varun-B16395] This would again be called per liodn.
[cut]
+static struct fsl_dma_domain *iommu_alloc_dma_domain(void) {
+ struct fsl_dma_domain *domain;
+
+ domain = kmem_cache_zalloc(fsl_pamu_domain_cache, GFP_KERNEL);
+ if (!domain)
+ return NULL;
+
+ domain-stash_id = ~(u32)0;
+ domain-snoop_id = ~(u32)0;
+ domain-win_cnt = max_subwindow_count;
To align with my previous comments on fsl_pamu.c, I think instead of
referencing a global variable (in fsl_pamu.c) you should be making an
accessor API call here to get the max subwindow _count.
[Sethi Varun-B16395] ok, I will make a accessor API call.
+ domain-geom_size = 0;
+
+ INIT_LIST_HEAD(domain-devices);
+
+ spin_lock_init(domain-domain_lock);
+
+ return domain;
+}
+
+static inline struct device_domain_info *find_domain(struct device
+*dev) {
+ return dev-archdata.iommu_domain; }
+
+static void remove_domain_ref(struct device_domain_info *info, u32
+win_cnt) {
+ list_del(info-link);
+ spin_lock(iommu_lock);
+ if (win_cnt)
+ pamu_free_subwins(info-liodn);
+ pamu_disable_liodn(info-liodn);
+ spin_unlock(iommu_lock);
+ spin_lock(device_domain_lock);
+ info-dev-archdata.iommu_domain = NULL;
+ kmem_cache_free(iommu_devinfo_cache, info);
+ spin_unlock(device_domain_lock); }
The above function is literally removing the _device_ reference from the
domain.
The name implies that it is removing a domain reference. Suggestion
is
to call it remove_device_ref.
Also, the whitespace is messed up there. You have 2 tabs instead of 1.
[Sethi Varun-B16395] ok will make the change.
+static void destroy_domain(struct fsl_dma_domain *dma_domain) {
+ struct device_domain_info *info;
+
+ /* Dissociate all the devices from this domain */
+ while (!list_empty(dma_domain-devices)) {
+ info = list_entry(dma_domain-devices.next,
+ struct device_domain_info, link);
+ remove_domain_ref(info, dma_domain-win_cnt);
+ }
+}
This function is removing all devices from a domain...maybe to be
consistent with my suggestion below on detach_domain(), call this
detach_all_devices().
We have 2 functions
doing almost the same thingone detaches a single device, one detaches
all devices.
The current names destroy_domain and detach_domain are not as clear
to me.
[Sethi Varun-B16395]ok
+static void detach_domain(struct device *dev, struct fsl_dma_domain
+*dma_domain) {
+ struct device_domain_info *info;
+ struct list_head *entry, *tmp;
+ unsigned long flags;
+
+ spin_lock_irqsave(dma_domain-domain_lock, flags);
+ /* Remove
RE: [PATCH 1/6 v8] iommu/fsl: Store iommu domain information pointer in archdata.
-Original Message- From: Yoder Stuart-B08248 Sent: Friday, March 01, 2013 9:52 PM To: Alexey Kardashevskiy; Sethi Varun-B16395 Cc: Kumar Gala; Benjamin Herrenschmidt; io...@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org list; linux-ker...@vger.kernel.org list; Wood Scott-B07421; Joerg Roedel; Paul Mackerras; David Gibson; Alex Williamson Subject: RE: [PATCH 1/6 v8] iommu/fsl: Store iommu domain information pointer in archdata. -Original Message- From: Alexey Kardashevskiy [mailto:a...@ozlabs.ru] Sent: Friday, March 01, 2013 4:07 AM To: Sethi Varun-B16395 Cc: Kumar Gala; Benjamin Herrenschmidt; io...@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org list; linux-ker...@vger.kernel.org list; Wood Scott-B07421; Yoder Stuart-B08248; Joerg Roedel; Paul Mackerras; David Gibson; Alex Williamson Subject: Re: [PATCH 1/6 v8] iommu/fsl: Store iommu domain information pointer in archdata. btw the device struct already has a pointer to its iommu_group, and the iommu_group struct itself has a pointer void *iommu_data which you could use for anything you want (iommu_group_get_iommudata(), iommu_group_set_iommudata()). By design you are expected to add iommu groups to a domain but not devices so I am not so sure that you really need a pointer to domain in the device struct. Well, at the lowest level the IOMMU API does attach devices to domains-- i.e. API attach_dev(). So, it seems to conceptually make sense to have a ptr from the device to the associated domain. When you implement attach_dev() you need to be able to see whether the device is already attached to a domain. Adding a couple of levels of indirection...from device to group to domain...doesn't seems to make things simpler or better IMHO. x86 keeps a pointer to the domain in device archdata and since there is a direct correlation between a device and domain I'd rather see it where this patch has it. As Stuart stated this allows us to maintain the device - domain relationship at the lowest level. -Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 1/6 v8] iommu/fsl: Store iommu domain information pointer in archdata.
Thanks for the clarification Alexey.
Kumar,
We are using this new field (for PAMU) to store the iommu domain (for iommu
API) information for a device.
Regards
Varun
-Original Message-
From: Alexey Kardashevskiy [mailto:a...@ozlabs.ru]
Sent: Friday, March 01, 2013 6:55 AM
To: Kumar Gala
Cc: Sethi Varun-B16395; Benjamin Herrenschmidt; iommu@lists.linux-
foundation.org; linuxppc-dev@lists.ozlabs.org list; linux-
ker...@vger.kernel.org list; Wood Scott-B07421; Yoder Stuart-B08248;
Joerg Roedel; Paul Mackerras; David Gibson; Alex Williamson
Subject: Re: [PATCH 1/6 v8] iommu/fsl: Store iommu domain information
pointer in archdata.
Hi!
On POWERNV we use only the part of IOMMU API which handles devices and
groups. We do not use IOMMU domains as VFIO containers do everything we
need for VFIO and we do not implement iommu_ops as it is not very
relevant to our architecture (does not give dma window properties, etc).
So your work does not overlap with my work :)
On 01/03/13 02:51, Kumar Gala wrote:
On Feb 27, 2013, at 6:04 AM, Sethi Varun-B16395 wrote:
Hi Kumar,Ben,
I am implementing the Freescale PAMU (IOMMU) driver using the Linux
IOMMU API. In this particular patch, I have added a new field to
dev_archdata structure to store the dma domain information.
This field is updated whenever we attach a device to an iommu domain.
Regards
Varun
Would be good to see if this overlaps with Alexey's work for IOMMU
driver for powernv.
- k
-Original Message-
From: Joerg Roedel [mailto:j...@8bytes.org]
Sent: Wednesday, February 27, 2013 5:01 PM
To: Sethi Varun-B16395
Cc: io...@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org;
linux-ker...@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248
Subject: Re: [PATCH 1/6 v8] iommu/fsl: Store iommu domain
information pointer in archdata.
On Mon, Feb 18, 2013 at 06:22:14PM +0530, Varun Sethi wrote:
Add a new field in the device (powerpc) archdata structure for
storing iommu domain information pointer. This pointer is stored
when the device is attached to a particular domain.
Signed-off-by: Varun Sethi varun.se...@freescale.com
---
- no change.
arch/powerpc/include/asm/device.h |4
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/include/asm/device.h
b/arch/powerpc/include/asm/device.h
index 77e97dd..6dc79fe 100644
--- a/arch/powerpc/include/asm/device.h
+++ b/arch/powerpc/include/asm/device.h
@@ -28,6 +28,10 @@ struct dev_archdata {
void*iommu_table_base;
} dma_data;
+/* IOMMU domain information pointer. This would be set
+ * when this device is attached to an iommu_domain.
+ */
+void*iommu_domain;
Please Cc the PowerPC Maintainers on this, so that they can have a
look at it. This also must be put this into an #ifdef
CONFIG_IOMMU_API.
--
Alexey
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/6] powerpc/fsl_pci: Store the platform device information corresponding to the pci controller.
This patch is present in the next branch of linux ppc tree maintained by Kumar Gala. Following is the commit id: 52c5affc545053d37c0b05224bbf70f5336caa20 I am not sure if this would be part of 3.9-rc1. Regards varun -Original Message- From: Joerg Roedel [mailto:j...@8bytes.org] Sent: Wednesday, February 27, 2013 4:15 PM To: Sethi Varun-B16395 Cc: Stuart Yoder; io...@lists.linux-foundation.org; linuxppc- d...@lists.ozlabs.org; linux-ker...@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248 Subject: Re: [PATCH 2/6] powerpc/fsl_pci: Store the platform device information corresponding to the pci controller. On Tue, Feb 26, 2013 at 06:16:10AM +, Sethi Varun-B16395 wrote: This patch is not present in Joerg's tree and the add_device API in the PAMU driver requires this patch. Will this patch be part of v3.9-rc1? Joerg ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 6/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
-Original Message-
From: Stuart Yoder [mailto:b08...@gmail.com]
Sent: Wednesday, February 27, 2013 4:03 AM
To: Sethi Varun-B16395
Cc: io...@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org;
linux-ker...@vger.kernel.org; Wood Scott-B07421; Joerg Roedel; Yoder
Stuart-B08248
Subject: Re: [PATCH 6/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU
API implementation.
Have not got through the entire file, but have a few comments...
+/*
+ * Set the PAACE type as primary and set the coherency required domain
+ * attribute
+ */
+static void pamu_setup_default_xfer_to_host_ppaace(struct paace
+*ppaace) {
+ set_bf(ppaace-addr_bitfields, PAACE_AF_PT, PAACE_PT_PRIMARY);
+
+ set_bf(ppaace-domain_attr.to_host.coherency_required,
PAACE_DA_HOST_CR,
+ PAACE_M_COHERENCE_REQ);
+}
+
+/*
+ * Set the PAACE type as secondary and set the coherency required
+domain
+ * attribute.
+ */
+static void pamu_setup_default_xfer_to_host_spaace(struct paace
+*spaace) {
+ set_bf(spaace-addr_bitfields, PAACE_AF_PT, PAACE_PT_SECONDARY);
+ set_bf(spaace-domain_attr.to_host.coherency_required,
PAACE_DA_HOST_CR,
+ PAACE_M_COHERENCE_REQ);
+}
Can we change the names of the above functions... I know there is some
history
with the name, but xfer_to_host is confusing.
Maybe just call them:
pamu_init_paace()
pamu_init_spaace()
[Sethi Varun-B16395] ok, will change the function names.
+/**
+ * pamu_config_spaace() - Sets up SPAACE entry for specified
+subwindow
+ *
+ * @liodn: Logical IO device number
+ * @subwin_cnt: number of sub-windows associated with dma-window
+ * @subwin_addr: starting address of subwindow
+ * @subwin_size: size of subwindow
+ * @omi: Operation mapping index
+ * @rpn: real (true physical) page number
+ * @snoopid: snoop id for hardware coherency -- if ~snoopid == 0 then
+ * snoopid not defined
+ * @stashid: cache stash id for associated cpu
+ * @enable: enable/disable subwindow after reconfiguration
+ * @prot: sub window permissions
+ *
+ * Returns 0 upon success else error code 0 returned */ int
+pamu_config_spaace(int liodn, u32 subwin_cnt, u32 subwin_addr,
+ phys_addr_t subwin_size, u32 omi, unsigned long
rpn,
+ u32 snoopid, u32 stashid, int enable, int prot)
+{
+ struct paace *paace;
+
+ /* setup sub-windows */
+ if (!subwin_cnt) {
+ pr_err(Invalid subwindow count\n);
+ return -EINVAL;
+ }
+
+ paace = pamu_get_ppaace(liodn);
+ if (subwin_addr 0 subwin_addr subwin_cnt paace) {
Why is the comparison subwin_addr subwin_cnt? Seems wrong...
[Sethi Varun-B16395] It's actually the subwindow index. I will rename the
variable.
+ paace = pamu_get_spaace(paace, subwin_addr - 1);
+
+ if (paace !(paace-addr_bitfields PAACE_V_VALID))
{
+ pamu_setup_default_xfer_to_host_spaace(paace);
+ set_bf(paace-addr_bitfields, SPAACE_AF_LIODN,
liodn);
+ }
+ }
+
+ if (!paace) {
+ pr_err(Invalid liodn entry\n);
+ return -ENOENT;
+ }
+
+ if (!enable prot == PAACE_AP_PERMS_DENIED) {
+ if (subwin_addr 0)
+ set_bf(paace-addr_bitfields, PAACE_AF_V,
+PAACE_V_INVALID);
+ else
+ set_bf(paace-addr_bitfields, PAACE_AF_AP,
+prot);
+ mb();
+ return 0;
+ }
Can you add a comment to the above if statement...when is this function
called with PAACE_AP_PERMS_DENIED?
[Sethi Varun-B16395] Actually, this piece of code is redundant in case of the
window based API. I will remove this. PAACE_AP_PERMS_DENIED is primarily used
for disabling the primary subwindow.
-Varun
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 1/6 v8] iommu/fsl: Store iommu domain information pointer in archdata.
Hi Kumar,Ben,
I am implementing the Freescale PAMU (IOMMU) driver using the Linux IOMMU API.
In this particular patch, I have added a new field to dev_archdata structure to
store the dma domain information.
This field is updated whenever we attach a device to an iommu domain.
Regards
Varun
-Original Message-
From: Joerg Roedel [mailto:j...@8bytes.org]
Sent: Wednesday, February 27, 2013 5:01 PM
To: Sethi Varun-B16395
Cc: io...@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org;
linux-ker...@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248
Subject: Re: [PATCH 1/6 v8] iommu/fsl: Store iommu domain information
pointer in archdata.
On Mon, Feb 18, 2013 at 06:22:14PM +0530, Varun Sethi wrote:
Add a new field in the device (powerpc) archdata structure for storing
iommu domain information pointer. This pointer is stored when the
device is attached to a particular domain.
Signed-off-by: Varun Sethi varun.se...@freescale.com
---
- no change.
arch/powerpc/include/asm/device.h |4
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/include/asm/device.h
b/arch/powerpc/include/asm/device.h
index 77e97dd..6dc79fe 100644
--- a/arch/powerpc/include/asm/device.h
+++ b/arch/powerpc/include/asm/device.h
@@ -28,6 +28,10 @@ struct dev_archdata {
void*iommu_table_base;
} dma_data;
+ /* IOMMU domain information pointer. This would be set
+* when this device is attached to an iommu_domain.
+*/
+ void*iommu_domain;
Please Cc the PowerPC Maintainers on this, so that they can have a look
at it. This also must be put this into an #ifdef CONFIG_IOMMU_API.
Joerg
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 0/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
Hi Joerg, Do you have any comments on the patch set. Regards Varun -Original Message- From: Sethi Varun-B16395 Sent: Monday, February 18, 2013 6:22 PM To: io...@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org; linux-ker...@vger.kernel.org; Wood Scott-B07421; j...@8bytes.org; Yoder Stuart-B08248 Cc: Sethi Varun-B16395 Subject: [PATCH 0/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU API implementation. This patchset provides the Freescale PAMU (Peripheral Access Management Unit) driver and the corresponding IOMMU API implementation. PAMU is the IOMMU present on Freescale QorIQ platforms. PAMU can authorize memory access, remap the memory address, and remap the I/O transaction type. This set consists of the following patches: 1. Addition of new field in the device (powerpc) archdata structure for storing iommu domain information pointer. This pointer is stored when the device is attached to a particular iommu domain. 2. Store PCI controller platform device information in the PCI controller structure. 3. Add defines for FSL PCI controller BRR1 register. 4. Add window permission flags in the iommu_domain_window_enable API. 5. Add domain attributes for FSL PAMU driver. 6. PAMU driver and IOMMU API implementation. This patch set is based on the next branch of the iommu git tree maintained by Joerg. Varun Sethi (6): Store iommu domain information in the device structure. Store the platform device information corresponding to the pci controller. Added defines for the FSL PCI controller BRR1 register. Add window permission flags for iommu_domain_window_enable API. Add addtional attributes specific to the PAMU driver. FSL PAMU driver and IOMMU API implementation. arch/powerpc/include/asm/device.h |4 + arch/powerpc/include/asm/pci-bridge.h |4 + arch/powerpc/sysdev/fsl_pci.c |9 +- arch/powerpc/sysdev/fsl_pci.h |2 +- drivers/iommu/Kconfig |8 + drivers/iommu/Makefile|1 + drivers/iommu/fsl_pamu.c | 1260 + drivers/iommu/fsl_pamu.h | 398 +++ drivers/iommu/fsl_pamu_domain.c | 1135 + drivers/iommu/fsl_pamu_domain.h | 89 +++ drivers/iommu/iommu.c |5 +- include/linux/iommu.h | 40 +- 12 files changed, 2947 insertions(+), 8 deletions(-) create mode 100644 drivers/iommu/fsl_pamu.c create mode 100644 drivers/iommu/fsl_pamu.h create mode 100644 drivers/iommu/fsl_pamu_domain.c create mode 100644 drivers/iommu/fsl_pamu_domain.h -- 1.7.4.1 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/6] powerpc/fsl_pci: Store the platform device information corresponding to the pci controller.
This patch is not present in Joerg's tree and the add_device API in the PAMU
driver requires this patch.
-Varun
-Original Message-
From: Stuart Yoder [mailto:b08...@gmail.com]
Sent: Tuesday, February 26, 2013 5:39 AM
To: Sethi Varun-B16395
Cc: io...@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org;
linux-ker...@vger.kernel.org; Wood Scott-B07421; Joerg Roedel; Yoder
Stuart-B08248
Subject: Re: [PATCH 2/6] powerpc/fsl_pci: Store the platform device
information corresponding to the pci controller.
This patch was submitted separately to linuxppc-dev (and was already
applied). You don't need it in this patch set, right?
Stuart
On Mon, Feb 18, 2013 at 6:52 AM, Varun Sethi varun.se...@freescale.com
wrote:
The pci controller structure has a provision to store the device
strcuture pointer of the corresponding platform device. Currently this
information is not stored during fsl pci controller initialization.
This information is required while dealing with iommu groups for pci
devices connected to the fsl pci controller. For the case where the
pci devices can't be paritioned, they would fall under the same device
group as the pci controller.
This patch stores the platform device information in the pci
controller structure during initialization.
Signed-off-by: Varun Sethi varun.se...@freescale.com
---
arch/powerpc/sysdev/fsl_pci.c |9 +++--
arch/powerpc/sysdev/fsl_pci.h |2 +-
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_pci.c
b/arch/powerpc/sysdev/fsl_pci.c index 92a5915..b393ae7 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -421,13 +421,16 @@ void fsl_pcibios_fixup_bus(struct pci_bus *bus)
}
}
-int __init fsl_add_bridge(struct device_node *dev, int is_primary)
+int __init fsl_add_bridge(struct platform_device *pdev, int
+is_primary)
{
int len;
struct pci_controller *hose;
struct resource rsrc;
const int *bus_range;
u8 hdr_type, progif;
+ struct device_node *dev;
+
+ dev = pdev-dev.of_node;
if (!of_device_is_available(dev)) {
pr_warning(%s: disabled\n, dev-full_name); @@
-453,6 +456,8 @@ int __init fsl_add_bridge(struct device_node *dev, int
is_primary)
if (!hose)
return -ENOMEM;
+ /* set platform device as the parent */
+ hose-parent = pdev-dev;
hose-first_busno = bus_range ? bus_range[0] : 0x0;
hose-last_busno = bus_range ? bus_range[1] : 0xff;
@@ -880,7 +885,7 @@ static int fsl_pci_probe(struct platform_device
*pdev) #endif
node = pdev-dev.of_node;
- ret = fsl_add_bridge(node, fsl_pci_primary == node);
+ ret = fsl_add_bridge(pdev, fsl_pci_primary == node);
#ifdef CONFIG_SWIOTLB
if (ret == 0) {
diff --git a/arch/powerpc/sysdev/fsl_pci.h
b/arch/powerpc/sysdev/fsl_pci.h index d078537..c495c00 100644
--- a/arch/powerpc/sysdev/fsl_pci.h
+++ b/arch/powerpc/sysdev/fsl_pci.h
@@ -91,7 +91,7 @@ struct ccsr_pci {
__be32 pex_err_cap_r3; /* 0x.e34 - PCIE error capture
register 0 */
};
-extern int fsl_add_bridge(struct device_node *dev, int is_primary);
+extern int fsl_add_bridge(struct platform_device *pdev, int
+is_primary);
extern void fsl_pcibios_fixup_bus(struct pci_bus *bus); extern int
mpc83xx_add_bridge(struct device_node *dev);
u64 fsl_pci_immrbar_base(struct pci_controller *hose);
--
1.7.4.1
___
iommu mailing list
io...@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 6/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
-Original Message-
From: Craciun Diana Madalina-STFD002
Sent: Tuesday, February 19, 2013 9:30 PM
To: Sethi Varun-B16395
Cc: io...@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org;
linux-ker...@vger.kernel.org; Wood Scott-B07421; j...@8bytes.org; Yoder
Stuart-B08248
Subject: Re: [PATCH 6/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU
API implementation.
On 02/18/2013 02:52 PM, Varun Sethi wrote:
+/**
+ * pamu_get_ppaace() - Return the primary PACCE
+ * @liodn: liodn PAACT index for desired PAACE
+ *
+ * Returns the ppace pointer upon success else return
+ * null.
+ */
+static struct paace *pamu_get_ppaace(int liodn) {
+ if (!ppaact || liodn PAACE_NUMBER_ENTRIES) {
Shouldn't be liodn = PAACE_NUMBER_ENTRIES ?
Yes, will fix this.
-Varun
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 6/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
-Original Message- From: Craciun Diana Madalina-STFD002 Sent: Tuesday, February 19, 2013 3:34 PM To: Sethi Varun-B16395 Cc: io...@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org; linux-ker...@vger.kernel.org; Wood Scott-B07421; j...@8bytes.org; Yoder Stuart-B08248 Subject: Re: [PATCH 6/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU API implementation. On 02/18/2013 02:52 PM, Varun Sethi wrote: + +#define PAACE_TCEF_FORMAT0_8B 0x00 +#define PAACE_TCEF_FORMAT1_RSVD 0x01 + +#define PAACE_NUMBER_ENTRIES0x1FF Where is this number coming from? This is currently hard coded. We will not require these many entries once we implement the LIODN allocation scheme. -Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 0/4] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
Hi Joerg, It's been a while since I submitted this patch. I have tried to address your comments regarding the subwindow attribute. I would really appreciate if I can get some feedback on this patch. Regards Varun -Original Message- From: Sethi Varun-B16395 Sent: Friday, December 21, 2012 7:17 AM To: 'Joerg Roedel' Cc: Sethi Varun-B16395; joerg.roe...@amd.com; iommu@lists.linux- foundation.org; linuxppc-dev@lists.ozlabs.org; linux- ker...@vger.kernel.org; Tabi Timur-B04825; Wood Scott-B07421 Subject: RE: [PATCH 0/4] iommu/fsl: Freescale PAMU driver and IOMMU API implementation. ping!! -Original Message- From: Sethi Varun-B16395 Sent: Friday, December 14, 2012 7:22 PM To: joerg.roe...@amd.com; io...@lists.linux-foundation.org; linuxppc- d...@lists.ozlabs.org; linux-ker...@vger.kernel.org; Tabi Timur-B04825; Wood Scott-B07421 Cc: Sethi Varun-B16395 Subject: [PATCH 0/4] iommu/fsl: Freescale PAMU driver and IOMMU API implementation. This patchset provides the Freescale PAMU (Peripheral Access Management Unit) driver and the corresponding IOMMU API implementation. PAMU is the IOMMU present on Freescale QorIQ platforms. PAMU can authorize memory access, remap the memory address, and remap the I/O transaction type. This set consists of the following patches: 1. Addition of new field in the device (powerpc) archdata structure for storing iommu domain information pointer. This pointer is stored when the device is attached to a particular iommu domain. 2. Add PAMU bypass enable register to the ccsr_guts structure. 3. Addition of domain attributes required by the PAMU driver IOMMU API. 4. PAMU driver and IOMMU API implementation. This patch set is based on the next branch of the iommu git tree maintained by Joerg. Varun Sethi (4): store iommu domain info in device arch data. add pamu bypass enable register to guts. Add iommu attributes for PAMU FSL PAMU driver. arch/powerpc/include/asm/device.h |4 + arch/powerpc/include/asm/fsl_guts.h |4 +- drivers/iommu/Kconfig |8 + drivers/iommu/Makefile |1 + drivers/iommu/fsl_pamu.c| 1152 +++ drivers/iommu/fsl_pamu.h| 398 drivers/iommu/fsl_pamu_domain.c | 1033 +++ drivers/iommu/fsl_pamu_domain.h | 96 +++ include/linux/iommu.h | 49 ++ 9 files changed, 2744 insertions(+), 1 deletions(-) create mode 100644 drivers/iommu/fsl_pamu.c create mode 100644 drivers/iommu/fsl_pamu.h create mode 100644 drivers/iommu/fsl_pamu_domain.c create mode 100644 drivers/iommu/fsl_pamu_domain.h -- 1.7.4.1 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 0/4] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
Hi Joerg, Do you have any comments on the patchset? Regards Varun -Original Message- From: Sethi Varun-B16395 Sent: Friday, December 21, 2012 7:17 AM To: 'Joerg Roedel' Cc: Sethi Varun-B16395; joerg.roe...@amd.com; iommu@lists.linux- foundation.org; linuxppc-dev@lists.ozlabs.org; linux- ker...@vger.kernel.org; Tabi Timur-B04825; Wood Scott-B07421 Subject: RE: [PATCH 0/4] iommu/fsl: Freescale PAMU driver and IOMMU API implementation. ping!! -Original Message- From: Sethi Varun-B16395 Sent: Friday, December 14, 2012 7:22 PM To: joerg.roe...@amd.com; io...@lists.linux-foundation.org; linuxppc- d...@lists.ozlabs.org; linux-ker...@vger.kernel.org; Tabi Timur-B04825; Wood Scott-B07421 Cc: Sethi Varun-B16395 Subject: [PATCH 0/4] iommu/fsl: Freescale PAMU driver and IOMMU API implementation. This patchset provides the Freescale PAMU (Peripheral Access Management Unit) driver and the corresponding IOMMU API implementation. PAMU is the IOMMU present on Freescale QorIQ platforms. PAMU can authorize memory access, remap the memory address, and remap the I/O transaction type. This set consists of the following patches: 1. Addition of new field in the device (powerpc) archdata structure for storing iommu domain information pointer. This pointer is stored when the device is attached to a particular iommu domain. 2. Add PAMU bypass enable register to the ccsr_guts structure. 3. Addition of domain attributes required by the PAMU driver IOMMU API. 4. PAMU driver and IOMMU API implementation. This patch set is based on the next branch of the iommu git tree maintained by Joerg. Varun Sethi (4): store iommu domain info in device arch data. add pamu bypass enable register to guts. Add iommu attributes for PAMU FSL PAMU driver. arch/powerpc/include/asm/device.h |4 + arch/powerpc/include/asm/fsl_guts.h |4 +- drivers/iommu/Kconfig |8 + drivers/iommu/Makefile |1 + drivers/iommu/fsl_pamu.c| 1152 +++ drivers/iommu/fsl_pamu.h| 398 drivers/iommu/fsl_pamu_domain.c | 1033 +++ drivers/iommu/fsl_pamu_domain.h | 96 +++ include/linux/iommu.h | 49 ++ 9 files changed, 2744 insertions(+), 1 deletions(-) create mode 100644 drivers/iommu/fsl_pamu.c create mode 100644 drivers/iommu/fsl_pamu.h create mode 100644 drivers/iommu/fsl_pamu_domain.c create mode 100644 drivers/iommu/fsl_pamu_domain.h -- 1.7.4.1 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 0/4] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
ping!! -Original Message- From: Sethi Varun-B16395 Sent: Friday, December 14, 2012 7:22 PM To: joerg.roe...@amd.com; io...@lists.linux-foundation.org; linuxppc- d...@lists.ozlabs.org; linux-ker...@vger.kernel.org; Tabi Timur-B04825; Wood Scott-B07421 Cc: Sethi Varun-B16395 Subject: [PATCH 0/4] iommu/fsl: Freescale PAMU driver and IOMMU API implementation. This patchset provides the Freescale PAMU (Peripheral Access Management Unit) driver and the corresponding IOMMU API implementation. PAMU is the IOMMU present on Freescale QorIQ platforms. PAMU can authorize memory access, remap the memory address, and remap the I/O transaction type. This set consists of the following patches: 1. Addition of new field in the device (powerpc) archdata structure for storing iommu domain information pointer. This pointer is stored when the device is attached to a particular iommu domain. 2. Add PAMU bypass enable register to the ccsr_guts structure. 3. Addition of domain attributes required by the PAMU driver IOMMU API. 4. PAMU driver and IOMMU API implementation. This patch set is based on the next branch of the iommu git tree maintained by Joerg. Varun Sethi (4): store iommu domain info in device arch data. add pamu bypass enable register to guts. Add iommu attributes for PAMU FSL PAMU driver. arch/powerpc/include/asm/device.h |4 + arch/powerpc/include/asm/fsl_guts.h |4 +- drivers/iommu/Kconfig |8 + drivers/iommu/Makefile |1 + drivers/iommu/fsl_pamu.c| 1152 +++ drivers/iommu/fsl_pamu.h| 398 drivers/iommu/fsl_pamu_domain.c | 1033 +++ drivers/iommu/fsl_pamu_domain.h | 96 +++ include/linux/iommu.h | 49 ++ 9 files changed, 2744 insertions(+), 1 deletions(-) create mode 100644 drivers/iommu/fsl_pamu.c create mode 100644 drivers/iommu/fsl_pamu.h create mode 100644 drivers/iommu/fsl_pamu_domain.c create mode 100644 drivers/iommu/fsl_pamu_domain.h -- 1.7.4.1 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 3/4 v5] iommu/fsl: Add iommu domain attributes required by fsl PAMU driver.
-Original Message- From: Wood Scott-B07421 Sent: Tuesday, December 04, 2012 11:53 PM To: Sethi Varun-B16395 Cc: Wood Scott-B07421; Joerg Roedel; linux-ker...@vger.kernel.org; io...@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org; Tabi Timur-B04825 Subject: Re: [PATCH 3/4 v5] iommu/fsl: Add iommu domain attributes required by fsl PAMU driver. On 12/04/2012 05:53:33 AM, Sethi Varun-B16395 wrote: -Original Message- From: Wood Scott-B07421 Sent: Monday, December 03, 2012 10:34 PM To: Sethi Varun-B16395 Cc: Joerg Roedel; linux-ker...@vger.kernel.org; iommu@lists.linux- foundation.org; Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org; Tabi Timur-B04825 Subject: Re: [PATCH 3/4 v5] iommu/fsl: Add iommu domain attributes required by fsl PAMU driver. On 12/03/2012 10:57:29 AM, Sethi Varun-B16395 wrote: -Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Joerg Roedel Sent: Sunday, December 02, 2012 7:33 PM To: Sethi Varun-B16395 Cc: linux-ker...@vger.kernel.org; io...@lists.linux-foundation.org; Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org; Tabi Timur-B04825 Subject: Re: [PATCH 3/4 v5] iommu/fsl: Add iommu domain attributes required by fsl PAMU driver. Hmm, we need to work out a good abstraction for this. On Tue, Nov 20, 2012 at 07:24:56PM +0530, Varun Sethi wrote: Added the following domain attributes required by FSL PAMU driver: 1. Subwindows field added to the iommu domain geometry attribute. Are the Subwindows mapped with full size or do you map only parts of the subwindows? [Sethi Varun-B16395] It's possible to map a part of the subwindow i.e. size of the mapping can be less than the sub window size. +* This attribute indicates number of DMA subwindows supported by +* the geometry. If there is a single window that maps the entire +* geometry, attribute must be set to 1. A value of 0 implies +* that this mechanism is not used at all(normal paging is used). +* Value other than* 0 or 1 indicates the actual number of +* subwindows. +*/ This semantic is ugly, how about a feature detection mechanism? [Sethi Varun-B16395] A feature mechanism to query the type of IOMMU? A feature mechanism to determine whether this subwindow mechanism is available, and what the limits are. So, we use the IOMMU capability interface to find out if IOMMU supports sub windows or not, right? But still number of sub windows would be specified as a part of the geometry and the valid value for sub windows would 0,1 or actual number of sub windows. How does a user of the interface find out what values are possible for the actual number of subwindows? How does a user of the interface find out whether there are any limitations on specifying a value of zero (in the case of PAMU, that would be a maximum 1 MiB naturally-aligned aperture to support arbitrary 4KiB mappings)? How about if we say that the default value for subwindows is zero and this what you get when you read the geometry (iommu_get_attr) after initializing the domain? In that case the user would know that implication of setting subwindows to zero with respect to the aperture size. Also, should we introduce an additional API like iommu_check_attr, which the user can use to validate the attribute value. For example in case of geometry, the user can fill up the structure and pass it to the iommu driver in order to verify the aperture and subwindows field. -Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 3/4 v5] iommu/fsl: Add iommu domain attributes required by fsl PAMU driver.
-Original Message- From: Wood Scott-B07421 Sent: Tuesday, December 11, 2012 6:31 AM To: Sethi Varun-B16395 Cc: Wood Scott-B07421; Joerg Roedel; linux-ker...@vger.kernel.org; io...@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org; Tabi Timur-B04825 Subject: Re: [PATCH 3/4 v5] iommu/fsl: Add iommu domain attributes required by fsl PAMU driver. On 12/10/2012 04:10:06 AM, Sethi Varun-B16395 wrote: -Original Message- From: Wood Scott-B07421 Sent: Tuesday, December 04, 2012 11:53 PM To: Sethi Varun-B16395 Cc: Wood Scott-B07421; Joerg Roedel; linux-ker...@vger.kernel.org; io...@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org; Tabi Timur-B04825 Subject: Re: [PATCH 3/4 v5] iommu/fsl: Add iommu domain attributes required by fsl PAMU driver. On 12/04/2012 05:53:33 AM, Sethi Varun-B16395 wrote: -Original Message- From: Wood Scott-B07421 Sent: Monday, December 03, 2012 10:34 PM To: Sethi Varun-B16395 Cc: Joerg Roedel; linux-ker...@vger.kernel.org; iommu@lists.linux- foundation.org; Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org; Tabi Timur-B04825 Subject: Re: [PATCH 3/4 v5] iommu/fsl: Add iommu domain attributes required by fsl PAMU driver. On 12/03/2012 10:57:29 AM, Sethi Varun-B16395 wrote: -Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Joerg Roedel Sent: Sunday, December 02, 2012 7:33 PM To: Sethi Varun-B16395 Cc: linux-ker...@vger.kernel.org; io...@lists.linux-foundation.org; Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org; Tabi Timur-B04825 Subject: Re: [PATCH 3/4 v5] iommu/fsl: Add iommu domain attributes required by fsl PAMU driver. Hmm, we need to work out a good abstraction for this. On Tue, Nov 20, 2012 at 07:24:56PM +0530, Varun Sethi wrote: Added the following domain attributes required by FSL PAMU driver: 1. Subwindows field added to the iommu domain geometry attribute. Are the Subwindows mapped with full size or do you map only parts of the subwindows? [Sethi Varun-B16395] It's possible to map a part of the subwindow i.e. size of the mapping can be less than the sub window size. +* This attribute indicates number of DMA subwindows supported by +* the geometry. If there is a single window that maps the entire +* geometry, attribute must be set to 1. A value of 0 implies +* that this mechanism is not used at all(normal paging is used). +* Value other than* 0 or 1 indicates the actual number of +* subwindows. +*/ This semantic is ugly, how about a feature detection mechanism? [Sethi Varun-B16395] A feature mechanism to query the type of IOMMU? A feature mechanism to determine whether this subwindow mechanism is available, and what the limits are. So, we use the IOMMU capability interface to find out if IOMMU supports sub windows or not, right? But still number of sub windows would be specified as a part of the geometry and the valid value for sub windows would 0,1 or actual number of sub windows. How does a user of the interface find out what values are possible for the actual number of subwindows? How does a user of the interface find out whether there are any limitations on specifying a value of zero (in the case of PAMU, that would be a maximum 1 MiB naturally-aligned aperture to support arbitrary 4KiB mappings)? How about if we say that the default value for subwindows is zero and this what you get when you read the geometry (iommu_get_attr) after initializing the domain? In that case the user would know that implication of setting subwindows to zero with respect to the aperture size. So it would default to the maximum aperture size possible with no subwindows? That might be OK, though is there a way to reset the domain later on to get back to that informational state? [Sethi Varun-B16395] Yes, that can be done via iommu_set_attr API. How about finding out the maximum number of subwindows? [Sethi Varun-B16395] We can introduce an API to determine the permissible range of values for an attribute, but it may be redundant for other IOMMU implementations. For the IOMMU implementations where geometry is just a read only attribute, iommu_get_attr returns the set of permissible values. I think it would be better if we can add a read only field (for the users) max_subwindows to the geometry. -Varun ___ Linuxppc-dev
RE: [PATCH 3/4 v5] iommu/fsl: Add iommu domain attributes required by fsl PAMU driver.
-Original Message- From: Wood Scott-B07421 Sent: Monday, December 03, 2012 10:34 PM To: Sethi Varun-B16395 Cc: Joerg Roedel; linux-ker...@vger.kernel.org; iommu@lists.linux- foundation.org; Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org; Tabi Timur-B04825 Subject: Re: [PATCH 3/4 v5] iommu/fsl: Add iommu domain attributes required by fsl PAMU driver. On 12/03/2012 10:57:29 AM, Sethi Varun-B16395 wrote: -Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Joerg Roedel Sent: Sunday, December 02, 2012 7:33 PM To: Sethi Varun-B16395 Cc: linux-ker...@vger.kernel.org; io...@lists.linux-foundation.org; Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org; Tabi Timur-B04825 Subject: Re: [PATCH 3/4 v5] iommu/fsl: Add iommu domain attributes required by fsl PAMU driver. Hmm, we need to work out a good abstraction for this. On Tue, Nov 20, 2012 at 07:24:56PM +0530, Varun Sethi wrote: Added the following domain attributes required by FSL PAMU driver: 1. Subwindows field added to the iommu domain geometry attribute. Are the Subwindows mapped with full size or do you map only parts of the subwindows? [Sethi Varun-B16395] It's possible to map a part of the subwindow i.e. size of the mapping can be less than the sub window size. +* This attribute indicates number of DMA subwindows supported by +* the geometry. If there is a single window that maps the entire +* geometry, attribute must be set to 1. A value of 0 implies +* that this mechanism is not used at all(normal paging is used). +* Value other than* 0 or 1 indicates the actual number of +* subwindows. +*/ This semantic is ugly, how about a feature detection mechanism? [Sethi Varun-B16395] A feature mechanism to query the type of IOMMU? A feature mechanism to determine whether this subwindow mechanism is available, and what the limits are. So, we use the IOMMU capability interface to find out if IOMMU supports sub windows or not, right? But still number of sub windows would be specified as a part of the geometry and the valid value for sub windows would 0,1 or actual number of sub windows. -Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 4/4 v6] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
-Original Message- From: Tabi Timur-B04825 Sent: Tuesday, December 04, 2012 1:33 AM To: Sethi Varun-B16395 Cc: joerg.roe...@amd.com; io...@lists.linux-foundation.org; linuxppc- d...@lists.ozlabs.org; linux-ker...@vger.kernel.org; Wood Scott-B07421 Subject: Re: [PATCH 4/4 v6] iommu/fsl: Freescale PAMU driver and IOMMU API implementation. Varun Sethi wrote: + out_be32(pamu_regs-ppbah, ((u64)ppaact_phys) 32); + out_be32(pamu_regs-ppbal, ppaact_phys); + ppaact_phys = ppaact_phys + PAACT_SIZE; + out_be32(pamu_regs-pplah, ((u64)ppaact_phys) 32); + out_be32(pamu_regs-pplal, ppaact_phys); Instead of ((u64)ppaact_phys) 32, use upper_32_bits() and lower_32_bits(). +#define PAACE_NUMBER_ENTRIES0xFF This is going to break with large LIODNs. Instead of hard-coding the size of the PPAACT, you need to scan the device tree for the largest LIODN, and make the array dynamically sized. This would in any case change with the new LIODN allocation scheme. I intend on introducing the new scheme as a separate patch. -Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 3/4 v5] iommu/fsl: Add iommu domain attributes required by fsl PAMU driver.
-Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Joerg Roedel Sent: Sunday, December 02, 2012 7:33 PM To: Sethi Varun-B16395 Cc: linux-ker...@vger.kernel.org; io...@lists.linux-foundation.org; Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org; Tabi Timur-B04825 Subject: Re: [PATCH 3/4 v5] iommu/fsl: Add iommu domain attributes required by fsl PAMU driver. Hmm, we need to work out a good abstraction for this. On Tue, Nov 20, 2012 at 07:24:56PM +0530, Varun Sethi wrote: Added the following domain attributes required by FSL PAMU driver: 1. Subwindows field added to the iommu domain geometry attribute. Are the Subwindows mapped with full size or do you map only parts of the subwindows? [Sethi Varun-B16395] It's possible to map a part of the subwindow i.e. size of the mapping can be less than the sub window size. +* This attribute indicates number of DMA subwindows supported by +* the geometry. If there is a single window that maps the entire +* geometry, attribute must be set to 1. A value of 0 implies +* that this mechanism is not used at all(normal paging is used). +* Value other than* 0 or 1 indicates the actual number of +* subwindows. +*/ This semantic is ugly, how about a feature detection mechanism? [Sethi Varun-B16395] A feature mechanism to query the type of IOMMU? -Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 3/4 v5] iommu/fsl: Add iommu domain attributes required by fsl PAMU driver.
Ping!!
-Original Message-
From: Sethi Varun-B16395
Sent: Monday, November 26, 2012 10:55 AM
To: joerg.roe...@amd.com
Cc: Sethi Varun-B16395; io...@lists.linux-foundation.org; linuxppc-
d...@lists.ozlabs.org; linux-ker...@vger.kernel.org; Wood Scott-B07421;
Tabi Timur-B04825
Subject: RE: [PATCH 3/4 v5] iommu/fsl: Add iommu domain attributes
required by fsl PAMU driver.
Hi Joerg,
Any comments? Can we apply this patch?
Regards
Varun
-Original Message-
From: Sethi Varun-B16395
Sent: Tuesday, November 20, 2012 7:25 PM
To: joerg.roe...@amd.com; io...@lists.linux-foundation.org; linuxppc-
d...@lists.ozlabs.org; linux-ker...@vger.kernel.org; Wood Scott-B07421;
Tabi Timur-B04825
Cc: Sethi Varun-B16395
Subject: [PATCH 3/4 v5] iommu/fsl: Add iommu domain attributes
required by fsl PAMU driver.
Added the following domain attributes required by FSL PAMU driver:
1. Subwindows field added to the iommu domain geometry attribute.
2. Added new iommu stash attribute, which allows setting of the
LIODN specific stash id parameter through IOMMU API.
3. Added an attribute for enabling/disabling DMA to a particular
memory window.
Signed-off-by: Varun Sethi varun.se...@freescale.com
---
changes in v5:
- Updated description of the subwindows field.
changes in v4:
- Updated comment explaining subwindows(as mentioned by Scott).
change in v3:
-renamed the stash attribute targets
include/linux/iommu.h | 43
+++
1 files changed, 43 insertions(+), 0 deletions(-)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h index
f3b99e1..7ca1cda 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -44,6 +44,41 @@ struct iommu_domain_geometry {
dma_addr_t aperture_start; /* First address that can be mapped */
dma_addr_t aperture_end; /* Last address that can be mapped
*/
bool force_aperture; /* DMA only allowed in mappable range?
*/
+
+ /*
+* A geometry mapping can be created in one of the following ways
+* for an IOMMU:
+* 1. A single contiguous window
+* 2. Through arbritary paging throughout the aperture.
+* 3. Using multiple subwindows
+*
+* In absence of arbritary paging, subwindows allow for supporting
+* physically discontiguous mappings.
+*
+* This attribute indicates number of DMA subwindows supported by
+* the geometry. If there is a single window that maps the entire
+* geometry, attribute must be set to 1. A value of 0 implies
+* that this mechanism is not used at all(normal paging is used).
+* Value other than* 0 or 1 indicates the actual number of
+* subwindows.
+*/
+ u32 subwindows;
+};
+
+/* cache stash targets */
+#define IOMMU_ATTR_CACHE_L1 1
+#define IOMMU_ATTR_CACHE_L2 2
+#define IOMMU_ATTR_CACHE_L3 3
+
+/* This attribute corresponds to IOMMUs capable of generating
+ * a stash transaction. A stash transaction is typically a
+ * hardware initiated prefetch of data from memory to cache.
+ * This attribute allows configuring stashig specific parameters
+ * in the IOMMU hardware.
+ */
+struct iommu_stash_attribute {
+ u32 cpu;/* cpu number */
+ u32 cache; /* cache to stash to: L1,L2,L3 */
};
struct iommu_domain {
@@ -60,6 +95,14 @@ struct iommu_domain { enum iommu_attr {
DOMAIN_ATTR_MAX,
DOMAIN_ATTR_GEOMETRY,
+ /* Set the IOMMU hardware stashing
+* parameters.
+*/
+ DOMAIN_ATTR_STASH,
+ /* Explicity enable/disable DMA for a
+ * particular memory window.
+ */
+ DOMAIN_ATTR_ENABLE,
};
#ifdef CONFIG_IOMMU_API
--
1.7.4.1
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 3/4 v5] iommu/fsl: Add iommu domain attributes required by fsl PAMU driver.
Hi Joerg,
Any comments? Can we apply this patch?
Regards
Varun
-Original Message-
From: Sethi Varun-B16395
Sent: Tuesday, November 20, 2012 7:25 PM
To: joerg.roe...@amd.com; io...@lists.linux-foundation.org; linuxppc-
d...@lists.ozlabs.org; linux-ker...@vger.kernel.org; Wood Scott-B07421;
Tabi Timur-B04825
Cc: Sethi Varun-B16395
Subject: [PATCH 3/4 v5] iommu/fsl: Add iommu domain attributes required
by fsl PAMU driver.
Added the following domain attributes required by FSL PAMU driver:
1. Subwindows field added to the iommu domain geometry attribute.
2. Added new iommu stash attribute, which allows setting of the
LIODN specific stash id parameter through IOMMU API.
3. Added an attribute for enabling/disabling DMA to a particular
memory window.
Signed-off-by: Varun Sethi varun.se...@freescale.com
---
changes in v5:
- Updated description of the subwindows field.
changes in v4:
- Updated comment explaining subwindows(as mentioned by Scott).
change in v3:
-renamed the stash attribute targets
include/linux/iommu.h | 43 +++
1 files changed, 43 insertions(+), 0 deletions(-)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h index
f3b99e1..7ca1cda 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -44,6 +44,41 @@ struct iommu_domain_geometry {
dma_addr_t aperture_start; /* First address that can be mapped
*/
dma_addr_t aperture_end; /* Last address that can be mapped
*/
bool force_aperture; /* DMA only allowed in mappable range?
*/
+
+ /*
+ * A geometry mapping can be created in one of the following ways
+ * for an IOMMU:
+ * 1. A single contiguous window
+ * 2. Through arbritary paging throughout the aperture.
+ * 3. Using multiple subwindows
+ *
+ * In absence of arbritary paging, subwindows allow for supporting
+ * physically discontiguous mappings.
+ *
+ * This attribute indicates number of DMA subwindows supported by
+ * the geometry. If there is a single window that maps the entire
+ * geometry, attribute must be set to 1. A value of 0 implies
+ * that this mechanism is not used at all(normal paging is used).
+ * Value other than* 0 or 1 indicates the actual number of
+ * subwindows.
+ */
+ u32 subwindows;
+};
+
+/* cache stash targets */
+#define IOMMU_ATTR_CACHE_L1 1
+#define IOMMU_ATTR_CACHE_L2 2
+#define IOMMU_ATTR_CACHE_L3 3
+
+/* This attribute corresponds to IOMMUs capable of generating
+ * a stash transaction. A stash transaction is typically a
+ * hardware initiated prefetch of data from memory to cache.
+ * This attribute allows configuring stashig specific parameters
+ * in the IOMMU hardware.
+ */
+struct iommu_stash_attribute {
+ u32 cpu;/* cpu number */
+ u32 cache; /* cache to stash to: L1,L2,L3 */
};
struct iommu_domain {
@@ -60,6 +95,14 @@ struct iommu_domain { enum iommu_attr {
DOMAIN_ATTR_MAX,
DOMAIN_ATTR_GEOMETRY,
+ /* Set the IOMMU hardware stashing
+ * parameters.
+ */
+ DOMAIN_ATTR_STASH,
+ /* Explicity enable/disable DMA for a
+ * particular memory window.
+ */
+ DOMAIN_ATTR_ENABLE,
};
#ifdef CONFIG_IOMMU_API
--
1.7.4.1
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 1/4 v2] iommu/fsl: Store iommu domain information pointer in archdata.
Hi Kumar,
Can you please apply this patch.
Regards
Varun
-Original Message-
From: Sethi Varun-B16395
Sent: Tuesday, November 20, 2012 7:25 PM
To: joerg.roe...@amd.com; io...@lists.linux-foundation.org; linuxppc-
d...@lists.ozlabs.org; linux-ker...@vger.kernel.org; Wood Scott-B07421;
Tabi Timur-B04825
Cc: Sethi Varun-B16395
Subject: [PATCH 1/4 v2] iommu/fsl: Store iommu domain information pointer
in archdata.
Add a new field in the device (powerpc) archdata structure for storing
iommu domain information pointer. This pointer is stored when the device
is attached to a particular domain.
Signed-off-by: Varun Sethi varun.se...@freescale.com
---
arch/powerpc/include/asm/device.h |4
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/include/asm/device.h
b/arch/powerpc/include/asm/device.h
index 77e97dd..6dc79fe 100644
--- a/arch/powerpc/include/asm/device.h
+++ b/arch/powerpc/include/asm/device.h
@@ -28,6 +28,10 @@ struct dev_archdata {
void*iommu_table_base;
} dma_data;
+ /* IOMMU domain information pointer. This would be set
+ * when this device is attached to an iommu_domain.
+ */
+ void*iommu_domain;
#ifdef CONFIG_SWIOTLB
dma_addr_t max_direct_dma_addr;
#endif
--
1.7.4.1
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] vfio powerpc: enabled and supported on powernv platform
(pages_to_put) {
+ --pages_to_put;
+ put_page(oldpages[pages_to_put]);
+ }
+ }
+ }
+
+ spin_unlock((pool-lock));
+ kfree(oldpages);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_put_tces);
+#endif /* CONFIG_IOMMU_API */
diff --git a/arch/powerpc/platforms/powernv/pci.c
b/arch/powerpc/platforms/powernv/pci.c
index 05205cf..676f4d9 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -8,30 +8,31 @@
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version
* 2 of the License, or (at your option) any later version.
*/
#include linux/kernel.h
#include linux/pci.h
#include linux/delay.h
#include linux/string.h
#include linux/init.h
#include linux/bootmem.h
#include linux/irq.h
#include linux/io.h
#include linux/msi.h
+#include linux/iommu.h
#include asm/sections.h
#include asm/io.h
#include asm/prom.h
#include asm/pci-bridge.h
#include asm/machdep.h
#include asm/ppc-pci.h
#include asm/opal.h
#include asm/iommu.h
#include asm/tce.h
#include asm/abs_addr.h
#include asm/firmware.h
#include powernv.h
#include pci.h
@@ -601,15 +602,149 @@ void __init pnv_pci_init(void)
/* Configure IOMMU DMA hooks */
ppc_md.pci_dma_dev_setup = pnv_pci_dma_dev_setup;
ppc_md.tce_build = pnv_tce_build;
ppc_md.tce_free = pnv_tce_free;
ppc_md.tce_get = pnv_tce_get;
ppc_md.pci_probe_mode = pnv_pci_probe_mode;
set_pci_dma_ops(dma_iommu_ops);
/* Configure MSIs */
#ifdef CONFIG_PCI_MSI
ppc_md.msi_check_device = pnv_msi_check_device;
ppc_md.setup_msi_irqs = pnv_setup_msi_irqs;
ppc_md.teardown_msi_irqs = pnv_teardown_msi_irqs; #endif }
+
+#ifdef CONFIG_IOMMU_API
+/*
+ * IOMMU groups support required by VFIO */ static int
+add_device(struct device *dev) {
+ struct iommu_table *tbl;
+ int ret = 0;
+
+ if (WARN_ON(dev-iommu_group)) {
+ printk(KERN_WARNING tce_vfio: device %s is already in iommu
group %d, skipping\n,
+ dev-kobj.name,
dev_name(dev)
+ iommu_group_id(dev-iommu_group));
+ return -EBUSY;
+ }
+
+ tbl = get_iommu_table_base(dev);
+ if (!tbl) {
+ pr_debug(tce_vfio: skipping device %s with no tbl\n,
+ dev-kobj.name);
+ return 0;
+ }
+
+ pr_debug(tce_vfio: adding %s to iommu group %d\n,
+ dev-kobj.name, iommu_group_id(tbl-it_group));
+
+ ret = iommu_group_add_device(tbl-it_group, dev);
+ if (ret 0)
+ printk(KERN_ERR tce_vfio: %s has not been added, ret=%d\n,
+ dev-kobj.name, ret);
+
+ return ret;
+}
+
+static void del_device(struct device *dev) {
+ iommu_group_remove_device(dev);
+}
+
+static int iommu_bus_notifier(struct notifier_block *nb,
+ unsigned long action, void *data) {
+ struct device *dev = data;
+
+ switch (action) {
+ case BUS_NOTIFY_ADD_DEVICE:
+ return add_device(dev);
+ case BUS_NOTIFY_DEL_DEVICE:
+ del_device(dev);
+ return 0;
+ default:
+ return 0;
+ }
+}
+
+static struct notifier_block tce_iommu_bus_nb = {
+ .notifier_call = iommu_bus_notifier, };
+
+static void group_release(void *iommu_data) {
+ struct iommu_table *tbl = iommu_data;
+ tbl-it_group = NULL;
+}
+
+static int __init tce_iommu_init(void) {
+ struct pci_dev *pdev = NULL;
+ struct iommu_table *tbl;
+ struct iommu_group *grp;
+
+ bus_register_notifier(pci_bus_type, tce_iommu_bus_nb);
There's already a notifier in the iommu code if you were to register an
iommu_ops with the add/remove_device entries. That would allow you to
remove the notifier block and notifier function below and the second loop
below. Are you avoiding that to avoid the rest of iommu_ops?
[Sethi Varun-B16395] Could be one reason, also they are associating the iommu
group with the tce table entry and not the device.
Also, shouldn't this notifier only be registered after the first loop
below? Otherwise ADD_DEVICE could race with setting up groups, which we
assume are present in the add_device() above.
[Sethi Varun-B16395] Isn't this similar to how how the notifier is registered
in iommu_bus_init? First a notifier is registered and then we check for devices
that have already been probed.
-Varun
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 3/3 v3] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
-Original Message-
From: Tabi Timur-B04825
Sent: Tuesday, October 23, 2012 2:48 AM
To: Sethi Varun-B16395
Cc: joerg.roe...@amd.com; io...@lists.linux-foundation.org; linuxppc-
d...@lists.ozlabs.org; linux-ker...@vger.kernel.org
Subject: Re: [PATCH 3/3 v3] iommu/fsl: Freescale PAMU driver and IOMMU
API implementation.
On Wed, Oct 17, 2012 at 12:32 PM, Varun Sethi varun.se...@freescale.com
wrote:
+ * Copyright (C) 2012 Freescale Semiconductor, Inc.
Copyright 2012 Freescale Semiconductor, Inc.
[Sethi Varun-B16395] Have followed similar convention elsewhere i.e. added (C).
+ *
+ */
+
+#include linux/init.h
+#include linux/iommu.h
+#include linux/slab.h
+#include linux/module.h
+#include linux/types.h
+#include linux/mm.h
+#include linux/interrupt.h
+#include linux/device.h
+#include linux/of_platform.h
+#include linux/bootmem.h
+#include linux/genalloc.h
+#include asm/io.h
+#include asm/bitops.h
+
+#include fsl_pamu.h
+
+/* PAMU bypass enbale register contains control bits for
+ * enabling bypass mode on each PAMU.
+ */
/*
* Linux multi-line comments
* look like this.
*/
+#define PAMUBYPENR 0x604
Update struct ccsr_guts instead.
http://patchwork.ozlabs.org/patch/141649/
[Sethi Varun-B16395] Ok.
+
+/* define indexes for each operation mapping scenario */
+#define OMI_QMAN0x00
+#define OMI_FMAN0x01
+#define OMI_QMAN_PRIV 0x02
+#define OMI_CAAM0x03
+
+static paace_t *ppaact;
+static paace_t *spaact;
+static struct ome *omt;
+unsigned int max_subwindow_count;
+
+struct gen_pool *spaace_pool;
+
+static paace_t *pamu_get_ppaace(int liodn) {
+ if (!ppaact) {
+ pr_err(PPAACT doesn't exist\n);
pr_err(fsl-pamu: PPAACT has not been initialized\n);
Make sure ALL pr_xxx() messages in this file start with fsl-pamu:
+ return NULL;
+ }
+
+ return ppaact[liodn];
Bounds checking?
[Sethi Varun-B16395] Ok.
+}
+
+/** Sets validation bit of PACCE
+ *
+ * @parm[in] liodn PAACT index for desired PAACE
+ *
+ * @return Returns 0 upon success else error code 0 returned */
+int pamu_enable_liodn(int liodn) {
+ paace_t *ppaace;
+
+ ppaace = pamu_get_ppaace(liodn);
+ if (!ppaace)
+ return -ENOENT;
error message?
[Sethi Varun-B16395] Have error messages at places from where the function is
invoked.
+
+ if (!get_bf(ppaace-addr_bitfields, PPAACE_AF_WSE)) {
+ pr_err(liodn %d not configured\n, liodn);
+ return -EINVAL;
+ }
+
+ /* Ensure that all other stores to the ppaace complete first */
+ mb();
+
+ ppaace-addr_bitfields |= PAACE_V_VALID;
+ mb();
+
+ return 0;
+}
+
+/** Clears validation bit of PACCE
+ *
+ * @parm[in] liodn PAACT index for desired PAACE
+ *
+ * @return Returns 0 upon success else error code 0 returned
This is not proper kerneldoc format.
[Sethi Varun-B16395] What format must be used? Can you point me to a relevant
file.
+ */
+int pamu_disable_liodn(int liodn)
+{
+ paace_t *ppaace;
+
+ ppaace = pamu_get_ppaace(liodn);
+ if (!ppaace)
+ return -ENOENT;
error message?
[Sethi Varun-B16395] Error message at the point of invocation.
+
+ set_bf(ppaace-addr_bitfields, PAACE_AF_V, PAACE_V_INVALID);
+ mb();
+
+ return 0;
+}
+
+
+static unsigned int map_addrspace_size_to_wse(phys_addr_t
+addrspace_size) {
+ BUG_ON((addrspace_size (addrspace_size - 1)));
+
+ /* window size is 2^(WSE+1) bytes */
+ return __ffs(addrspace_size PAMU_PAGE_SHIFT) +
+PAMU_PAGE_SHIFT - 1; }
+
+static unsigned int map_subwindow_cnt_to_wce(u32 subwindow_cnt) {
+ /* window count is 2^(WCE+1) bytes */
+ return __ffs(subwindow_cnt) - 1; }
+
+static void pamu_setup_default_xfer_to_host_ppaace(paace_t *ppaace) {
+ set_bf(ppaace-addr_bitfields, PAACE_AF_PT, PAACE_PT_PRIMARY);
+
+ set_bf(ppaace-domain_attr.to_host.coherency_required,
PAACE_DA_HOST_CR,
+ PAACE_M_COHERENCE_REQ); }
+
+static void pamu_setup_default_xfer_to_host_spaace(paace_t *spaace) {
+ set_bf(spaace-addr_bitfields, PAACE_AF_PT,
PAACE_PT_SECONDARY);
+ set_bf(spaace-domain_attr.to_host.coherency_required,
PAACE_DA_HOST_CR,
+ PAACE_M_COHERENCE_REQ); }
+
+static paace_t *pamu_get_spaace(u32 fspi_index, u32 wnum) {
+ return spaact[fspi_index + wnum];
bounds checking?
[Sethi Varun-B16395] Ok.
+}
+
+static unsigned long pamu_get_fspi_and_allocate(u32 subwin_cnt) {
subwin_cnt should probably be an unsigned int.
[Sethi Varun-B16395] Why?
This function needs to be documented. What value is being returned?
+ unsigned long spaace_addr
RE: [PATCH 3/3 v3] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
-Original Message-
From: Wood Scott-B07421
Sent: Tuesday, October 23, 2012 5:23 AM
To: Tabi Timur-B04825
Cc: Sethi Varun-B16395; joerg.roe...@amd.com; iommu@lists.linux-
foundation.org; linuxppc-dev@lists.ozlabs.org; linux-
ker...@vger.kernel.org
Subject: Re: [PATCH 3/3 v3] iommu/fsl: Freescale PAMU driver and IOMMU
API implementation.
On 10/22/2012 04:18:07 PM, Tabi Timur-B04825 wrote:
On Wed, Oct 17, 2012 at 12:32 PM, Varun Sethi
varun.se...@freescale.com wrote:
+}
+
+static unsigned long pamu_get_fspi_and_allocate(u32 subwin_cnt) {
subwin_cnt should probably be an unsigned int.
This function needs to be documented. What value is being returned?
spaact offset (yes, this needs to be documented)
[Sethi Varun-B16395] Ok.
+/* This bitmap advertises the page sizes supported by PAMU hardware
+ * to the IOMMU API.
+ */
+#define FSL_PAMU_PGSIZES (~0xFFFUL)
There should be a better way to define this. ~(PAMU_PAGE_SIZE-1)
maybe?
Is it even true? We don't support IOMMU pages larger than the SoC can
address.
The (~0xFFFUL) version also discards some valid IOMMU page sizes on 32-
bit kernels. One use case for windows larger than the CPU virtual
address space is creating one big identity-map window to effectively
disable translation. If we're to support that, the size of pgsize_bitmap
will need to change as well.
[Sethi Varun-B16395] Correct, this needs to be fixed. I will try to address this
In a separate patch (would require changes to iommu_map).
+static int map_liodn(int liodn, struct fsl_dma_domain *dma_domain)
+{
+ u32 subwin_cnt = dma_domain-subwin_cnt;
+ unsigned long rpn;
+ int ret = 0, i;
+
+ if (subwin_cnt) {
+ struct dma_subwindow *sub_win_ptr =
+ dma_domain-sub_win_arr[0];
+ for (i = 0; i subwin_cnt; i++) {
+ if (sub_win_ptr[i].valid) {
+ rpn = sub_win_ptr[i].paddr
+PAMU_PAGE_SHIFT,
+ spin_lock(iommu_lock);
+ ret = pamu_config_spaace(liodn,
subwin_cnt, i,
+
sub_win_ptr[i].size,
+-1,
+rpn,
+
dma_domain-snoop_id,
+
dma_domain-stash_id,
+(i 0) ?
1 : 0,
+
sub_win_ptr[i].prot);
+ spin_unlock(iommu_lock);
+ if (ret) {
+ pr_err(PAMU SPAACE
configuration failed for liodn %d\n,
+liodn);
+ return ret;
+ }
+ }
+ }
Break up that nesting with some subfunctions.
+ while (!list_empty(dma_domain-devices)) {
+ info = list_entry(dma_domain-devices.next,
+ struct device_domain_info, link);
+ remove_domain_ref(info, dma_domain-subwin_cnt);
+ }
I wonder if you should use list_for_each_safe() instead.
The above is simpler if you're destroying the entire list.
+}
+
+static int configure_domain_dma_state(struct fsl_dma_domain
*dma_domain, int enable)
bool enable
Finally, please CC: me on all IOMMU and PAMU patches you post
upstream.
Me too.
[Sethi Varun-B16395] Sure.
-Varun
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/3 v2] iommu/fsl: Add iommu domain attributes required by fsl PAMU driver.
-Original Message-
From: Kumar Gala [mailto:ga...@kernel.crashing.org]
Sent: Thursday, October 04, 2012 6:47 PM
To: Sethi Varun-B16395
Cc: joerg.roe...@amd.com; io...@lists.linux-foundation.org; linuxppc-
d...@lists.ozlabs.org; linux-ker...@vger.kernel.org; Sethi Varun-B16395
Subject: Re: [PATCH 2/3 v2] iommu/fsl: Add iommu domain attributes
required by fsl PAMU driver.
On Oct 4, 2012, at 6:56 AM, b16...@freescale.com b16...@freescale.com
wrote:
From: Varun Sethi varun.se...@freescale.com
Added the following domain attributes required by FSL PAMU driver:
1. Subwindows field added to the iommu domain geometry attribute.
2. Added new iommu stash attribute, which allows setting of the
LIODN specific stash id parameter through IOMMU API.
3. Added an attribute for enabling/disabling DMA to a particular
memory window.
Signed-off-by: Varun Sethi varun.se...@freescale.com
---
include/linux/iommu.h | 35 +++
1 files changed, 35 insertions(+), 0 deletions(-)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h index
f3b99e1..62e22f0 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -44,6 +44,33 @@ struct iommu_domain_geometry {
dma_addr_t aperture_start; /* First address that can be mapped
*/
dma_addr_t aperture_end; /* Last address that can be mapped
*/
bool force_aperture; /* DMA only allowed in mappable range?
*/
+
+ /* The subwindows field indicates number of DMA subwindows
supported
+* by the geometry. Following is the interpretation of
+* values for this field:
+* 0 : This implies that the supported geometry size is 1 MB
+ * with each subwindow size being 4KB. Thus number of
subwindows
+* being = 1MB/4KB = 256.
+* 1 : Only one DMA window i.e. no subwindows.
+* value other than 0 or 1 would indicate actual number of
subwindows.
+*/
+ u32 subwindows;
+};
+
+/* cache stash targets */
+#define L1_CACHE 1
+#define L2_CACHE 2
+#define L3_CACHE 3
These names are way to generic for being exposed to user space
Will fix naming to IOMMU_ATTR_CACHE_L1 etc.
-Varun
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [RFC][PATCH 1/3] iommu/fsl: Store iommu domain information pointer in archdata.
-Original Message- From: Kumar Gala [mailto:ga...@kernel.crashing.org] Sent: Wednesday, September 19, 2012 7:20 PM To: Sethi Varun-B16395 Cc: io...@lists.linux-foundation.org; joerg.roe...@amd.com; linux- ker...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; Sethi Varun-B16395 Subject: Re: [RFC][PATCH 1/3] iommu/fsl: Store iommu domain information pointer in archdata. On Sep 19, 2012, at 8:17 AM, b16...@freescale.com b16...@freescale.com wrote: From: Varun Sethi varun.se...@freescale.com Add a new field in the device (powerpc) archdata structure for storing iommu domain information pointer. This pointer is stored when the device is attached to a particular domain. Signed-off-by: Varun Sethi varun.se...@freescale.com --- arch/powerpc/include/asm/device.h |4 1 files changed, 4 insertions(+), 0 deletions(-) Not too familiar, but what does the IBM Server IOMMU do for iommu_domain? [Sethi Varun-B16395] I am not sure if the IBM iommu driver implements the iommu API. -Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [RFC][PATCH 2/3] iommu/fsl: Add iommu domain attributes required by fsl PAMU driver.
-Original Message-
From: Wood Scott-B07421
Sent: Thursday, September 20, 2012 5:42 AM
To: Kumar Gala
Cc: Sethi Varun-B16395; joerg.roe...@amd.com; iommu@lists.linux-
foundation.org; linuxppc-dev@lists.ozlabs.org; linux-
ker...@vger.kernel.org; Sethi Varun-B16395
Subject: Re: [RFC][PATCH 2/3] iommu/fsl: Add iommu domain attributes
required by fsl PAMU driver.
On 09/19/2012 08:52:27 AM, Kumar Gala wrote:
On Sep 19, 2012, at 8:17 AM, b16...@freescale.com
b16...@freescale.com wrote:
From: Varun Sethi varun.se...@freescale.com
Added the following domain attributes required by FSL PAMU driver:
1. Subwindows field added to the iommu domain geometry attribute.
2. Added new iommu stash attribute, which allows setting of the
LIODN specific stash id parameter through IOMMU API.
3. Added an attribute for enabling/disabling DMA to a particular
memory window.
Signed-off-by: Varun Sethi varun.se...@freescale.com
---
include/linux/iommu.h | 30 ++
1 files changed, 30 insertions(+), 0 deletions(-)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h index
7e83370..eaa40c6 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -44,6 +44,28 @@ struct iommu_domain_geometry {
dma_addr_t aperture_start; /* First address that can be
mapped*/
dma_addr_t aperture_end; /* Last address that can be
mapped */
bool force_aperture; /* DMA only allowed in mappable
range? */
+
+ /* The subwindows field indicates number of DMA subwindows
supported
+ * by the geometry. Following is the interpretation of
+ * values for this field:
+ * 0 : This implies that the supported geometry size is 1 MB
+ * with each subwindow size being 4KB. Thus number of
subwindows
+ * being = 1MB/4KB = 256.
+ * 1 : Only one DMA window i.e. no subwindows.
+ * value other than 0 or 1 would indicate actual number of
subwindows.
+ */
+ u32 subwindows;
+};
+
+/* This attribute corresponds to IOMMUs capable of generating
+ * a stash transaction. A stash transaction is typically a
+ * hardware initiated prefetch of data from memory to cache.
+ * This attribute allows configuring stashig specific parameters
+ * in the IOMMU hardware.
+ */
+struct iommu_stash_attribute {
+ u32 cpu;/* cpu number */
+ u32 cache; /* cache to stash to: L1,L2,L3 */
seems like this should be enum instead of u32 for cache
With enum being something like:
enum iommu_attr_stash_cache {
IOMMU_ATTR_CACHE_L1,
IOMMU_ATTR_CACHE_L2,
IOMMU_ATTR_CACHE_L3,
};
Don't we want these structs to be usable via some VFIO ioctl? In that
case they need to use fixed size types.
Yes, this would be usable via vfio ioctl. But, then the caller should be
aware of supported stash targets. May be I should add an interface for the
caller,
to query supported stash targets.
-Varun
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 3/3 v4] powerpc/mpic: FSL MPIC error interrupt support.
-Original Message- From: Wood Scott-B07421 Sent: Wednesday, August 08, 2012 12:25 AM To: Sethi Varun-B16395 Cc: Kumar Gala; linuxppc-dev@lists.ozlabs.org; Hamciuc Bogdan-BHAMCIU1 Subject: Re: [PATCH 3/3 v4] powerpc/mpic: FSL MPIC error interrupt support. On 08/06/2012 11:22 AM, Sethi Varun-B16395 wrote: -Original Message- From: Kumar Gala [mailto:ga...@kernel.crashing.org] Sent: Monday, August 06, 2012 9:23 PM To: Sethi Varun-B16395 Cc: linuxppc-dev@lists.ozlabs.org; Hamciuc Bogdan-BHAMCIU1 Subject: Re: [PATCH 3/3 v4] powerpc/mpic: FSL MPIC error interrupt support. On Aug 6, 2012, at 7:44 AM, Varun Sethi wrote: All SOC device error interrupts are muxed and delivered to the core as a single MPIC error interrupt. Currently all the device drivers requiring access to device errors have to register for the MPIC error interrupt as a shared interrupt. With this patch we add interrupt demuxing capability in the mpic driver, allowing device drivers to register for their individual error interrupts. This is achieved by handling error interrupts in a cascaded fashion. MPIC error interrupt is handled by the error_int_handler, which subsequently demuxes it using the EISR and delivers it to the respective drivers. The error interrupt capability is dependent on the MPIC EIMR register, which was introduced in FSL MPIC version 4.1 (P4080 rev2). So, error interrupt demuxing capability is dependent on the MPIC version and can be used for versions = 4.1. Signed-off-by: Varun Sethi varun.se...@freescale.com Signed-off-by: Bogdan Hamciuc bogdan.hamc...@freescale.com [In the initial version of the patch we were using handle_simple_irq as the handler for cascaded error interrupts, this resulted in issues in case of threaded isrs (with RT kernel). This issue was debugged by Bogdan and decision was taken to use the handle_level_irq handler] --- arch/powerpc/include/asm/mpic.h| 16 arch/powerpc/sysdev/Makefile |2 +- arch/powerpc/sysdev/fsl_mpic_err.c | 153 arch/powerpc/sysdev/mpic.c | 45 ++- arch/powerpc/sysdev/mpic.h | 22 + 5 files changed, 236 insertions(+), 2 deletions(-) create mode 100644 arch/powerpc/sysdev/fsl_mpic_err.c diff --git a/arch/powerpc/include/asm/mpic.h b/arch/powerpc/include/asm/mpic.h index e14d35d..6c8e53b 100644 --- a/arch/powerpc/include/asm/mpic.h +++ b/arch/powerpc/include/asm/mpic.h @@ -118,6 +118,9 @@ #define MPIC_MAX_CPUS 32 #define MPIC_MAX_ISU 32 +#define MPIC_MAX_ERR 32 +#define MPIC_FSL_ERR_INT 16 + /* * Tsi108 implementation of MPIC has many differences from the original one */ @@ -270,6 +273,7 @@ struct mpic struct irq_chip hc_ipi; #endif struct irq_chip hc_tm; + struct irq_chip hc_err; const char *name; /* Flags */ unsigned intflags; @@ -283,6 +287,8 @@ struct mpic /* vector numbers used for internal sources (ipi/timers) */ unsigned intipi_vecs[4]; unsigned inttimer_vecs[8]; + /* vector numbers used for FSL MPIC error interrupts */ + unsigned interr_int_vecs[MPIC_MAX_ERR]; /* Spurious vector to program into unused sources */ unsigned intspurious_vec; @@ -306,6 +312,11 @@ struct mpic struct mpic_reg_bankcpuregs[MPIC_MAX_CPUS]; struct mpic_reg_bankisus[MPIC_MAX_ISU]; + /* ioremap'ed base for error interrupt registers */ + u32 __iomem *err_regs; + /* error interrupt config */ + u32 err_int_config_done; I thought we were going to remove this as it don't really provide any value. [Sethi Varun-B16395] We need a way to determine that irq handle got registered for Mpic error interrupt, only then can we go ahead and assign individual (cascaded) error interrupts. Initially we were doing the same thing while translating error interrupt specifier, now we are registering the handler in mpic_init. If you register it in mpic_init(), when would you be unsure about whether the registration has happened? It was just a sanity check to ensure that the error interrupt handler actually got registered during mpic_init, so that we can assign individual error interrupts in the xlate function. I agree that this isn't a necessary requirement. Submitted a new version of the patch after removing the check. -Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 3/3 v4] powerpc/mpic: FSL MPIC error interrupt support.
-Original Message- From: Kumar Gala [mailto:ga...@kernel.crashing.org] Sent: Monday, August 06, 2012 9:23 PM To: Sethi Varun-B16395 Cc: linuxppc-dev@lists.ozlabs.org; Hamciuc Bogdan-BHAMCIU1 Subject: Re: [PATCH 3/3 v4] powerpc/mpic: FSL MPIC error interrupt support. On Aug 6, 2012, at 7:44 AM, Varun Sethi wrote: All SOC device error interrupts are muxed and delivered to the core as a single MPIC error interrupt. Currently all the device drivers requiring access to device errors have to register for the MPIC error interrupt as a shared interrupt. With this patch we add interrupt demuxing capability in the mpic driver, allowing device drivers to register for their individual error interrupts. This is achieved by handling error interrupts in a cascaded fashion. MPIC error interrupt is handled by the error_int_handler, which subsequently demuxes it using the EISR and delivers it to the respective drivers. The error interrupt capability is dependent on the MPIC EIMR register, which was introduced in FSL MPIC version 4.1 (P4080 rev2). So, error interrupt demuxing capability is dependent on the MPIC version and can be used for versions = 4.1. Signed-off-by: Varun Sethi varun.se...@freescale.com Signed-off-by: Bogdan Hamciuc bogdan.hamc...@freescale.com [In the initial version of the patch we were using handle_simple_irq as the handler for cascaded error interrupts, this resulted in issues in case of threaded isrs (with RT kernel). This issue was debugged by Bogdan and decision was taken to use the handle_level_irq handler] --- arch/powerpc/include/asm/mpic.h| 16 arch/powerpc/sysdev/Makefile |2 +- arch/powerpc/sysdev/fsl_mpic_err.c | 153 arch/powerpc/sysdev/mpic.c | 45 ++- arch/powerpc/sysdev/mpic.h | 22 + 5 files changed, 236 insertions(+), 2 deletions(-) create mode 100644 arch/powerpc/sysdev/fsl_mpic_err.c diff --git a/arch/powerpc/include/asm/mpic.h b/arch/powerpc/include/asm/mpic.h index e14d35d..6c8e53b 100644 --- a/arch/powerpc/include/asm/mpic.h +++ b/arch/powerpc/include/asm/mpic.h @@ -118,6 +118,9 @@ #define MPIC_MAX_CPUS 32 #define MPIC_MAX_ISU32 +#define MPIC_MAX_ERR 32 +#define MPIC_FSL_ERR_INT 16 + /* * Tsi108 implementation of MPIC has many differences from the original one */ @@ -270,6 +273,7 @@ struct mpic struct irq_chip hc_ipi; #endif struct irq_chip hc_tm; + struct irq_chip hc_err; const char *name; /* Flags */ unsigned intflags; @@ -283,6 +287,8 @@ struct mpic /* vector numbers used for internal sources (ipi/timers) */ unsigned intipi_vecs[4]; unsigned inttimer_vecs[8]; + /* vector numbers used for FSL MPIC error interrupts */ + unsigned interr_int_vecs[MPIC_MAX_ERR]; /* Spurious vector to program into unused sources */ unsigned intspurious_vec; @@ -306,6 +312,11 @@ struct mpic struct mpic_reg_bankcpuregs[MPIC_MAX_CPUS]; struct mpic_reg_bankisus[MPIC_MAX_ISU]; + /* ioremap'ed base for error interrupt registers */ + u32 __iomem *err_regs; + /* error interrupt config */ + u32 err_int_config_done; I thought we were going to remove this as it don't really provide any value. [Sethi Varun-B16395] We need a way to determine that irq handle got registered for Mpic error interrupt, only then can we go ahead and assign individual (cascaded) error interrupts. Initially we were doing the same thing while translating error interrupt specifier, now we are registering the handler in mpic_init. -Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/4] powerpc/booke: Merge the 32 bit e5500/e500mc cpu setup code.
-Original Message- From: Kumar Gala [mailto:ga...@kernel.crashing.org] Sent: Monday, August 06, 2012 9:28 PM To: Sethi Varun-B16395 Cc: ag...@suse.de; b...@kernel.crashing.org; linuxppc- d...@lists.ozlabs.org; kvm-...@vger.kernel.org Subject: Re: [PATCH 2/4] powerpc/booke: Merge the 32 bit e5500/e500mc cpu setup code. On Aug 4, 2012, at 1:31 PM, Sethi Varun-B16395 wrote: -Original Message- From: Kumar Gala [mailto:ga...@kernel.crashing.org] Sent: Friday, August 03, 2012 10:04 PM To: Sethi Varun-B16395 Cc: ag...@suse.de; b...@kernel.crashing.org; linuxppc- d...@lists.ozlabs.org; kvm-...@vger.kernel.org Subject: Re: [PATCH 2/4] powerpc/booke: Merge the 32 bit e5500/e500mc cpu setup code. On Jul 9, 2012, at 7:58 AM, Varun Sethi wrote: Merge the 32 bit cpu setup code for e500mc/e5500 and define the cpu_restore routine (for e5500/e6500) only for the 64 bit case. The cpu_restore routine is used in the 64 bit case for setting up the secondary cores. Signed-off-by: Varun Sethi varun.se...@freescale.com --- arch/powerpc/kernel/cpu_setup_fsl_booke.S |1 + arch/powerpc/kernel/cputable.c|4 2 files changed, 5 insertions(+), 0 deletions(-) diff --git a/arch/powerpc/kernel/cpu_setup_fsl_booke.S b/arch/powerpc/kernel/cpu_setup_fsl_booke.S index a55d028..5e87737 100644 --- a/arch/powerpc/kernel/cpu_setup_fsl_booke.S +++ b/arch/powerpc/kernel/cpu_setup_fsl_booke.S @@ -75,6 +75,7 @@ _GLOBAL(__setup_cpu_e500v2) mtlrr4 blr _GLOBAL(__setup_cpu_e500mc) +_GLOBAL(__setup_cpu_e5500) This is a bit confusing, as we now have duplicated __setup_cpu_e5500() between the ppc32 and ppc64 cases. If you build this patch for corenet32_smp_defconfig it fails. [Sethi Varun-B16395] I am able to build without any issue with the same config. -Varun If you build corenet32_smp_defconfig at commit: commit c5537ef2d672d2cf48d4e4ac754781c8db112843 Author: Varun Sethi varun.se...@freescale.com Date: Mon Jul 9 18:28:21 2012 +0530 powerpc/booke: Merge the 32 bit e5500/e500mc cpu setup code. You get the following build error: arch/powerpc/kernel/cpu_setup_fsl_booke.S: Assembler messages: arch/powerpc/kernel/cpu_setup_fsl_booke.S:110: Error: symbol `__setup_cpu_e5500' is already defined Oh.., didn't realize that. Thanks for fixing this. -Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/4] powerpc/booke: Merge the 32 bit e5500/e500mc cpu setup code.
-Original Message- From: Kumar Gala [mailto:ga...@kernel.crashing.org] Sent: Friday, August 03, 2012 10:04 PM To: Sethi Varun-B16395 Cc: ag...@suse.de; b...@kernel.crashing.org; linuxppc- d...@lists.ozlabs.org; kvm-...@vger.kernel.org Subject: Re: [PATCH 2/4] powerpc/booke: Merge the 32 bit e5500/e500mc cpu setup code. On Jul 9, 2012, at 7:58 AM, Varun Sethi wrote: Merge the 32 bit cpu setup code for e500mc/e5500 and define the cpu_restore routine (for e5500/e6500) only for the 64 bit case. The cpu_restore routine is used in the 64 bit case for setting up the secondary cores. Signed-off-by: Varun Sethi varun.se...@freescale.com --- arch/powerpc/kernel/cpu_setup_fsl_booke.S |1 + arch/powerpc/kernel/cputable.c|4 2 files changed, 5 insertions(+), 0 deletions(-) diff --git a/arch/powerpc/kernel/cpu_setup_fsl_booke.S b/arch/powerpc/kernel/cpu_setup_fsl_booke.S index a55d028..5e87737 100644 --- a/arch/powerpc/kernel/cpu_setup_fsl_booke.S +++ b/arch/powerpc/kernel/cpu_setup_fsl_booke.S @@ -75,6 +75,7 @@ _GLOBAL(__setup_cpu_e500v2) mtlrr4 blr _GLOBAL(__setup_cpu_e500mc) +_GLOBAL(__setup_cpu_e5500) This is a bit confusing, as we now have duplicated __setup_cpu_e5500() between the ppc32 and ppc64 cases. If you build this patch for corenet32_smp_defconfig it fails. [Sethi Varun-B16395] I am able to build without any issue with the same config. -Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 3/3 v3] powerpc/mpic: FSL MPIC error interrupt support.
-Original Message- From: Kumar Gala [mailto:ga...@kernel.crashing.org] Sent: Friday, August 03, 2012 6:49 PM To: Sethi Varun-B16395 Cc: linuxppc-dev@lists.ozlabs.org; Hamciuc Bogdan-BHAMCIU1 Subject: Re: [PATCH 3/3 v3] powerpc/mpic: FSL MPIC error interrupt support. On Jul 31, 2012, at 9:42 AM, Varun Sethi wrote: All SOC device error interrupts are muxed and delivered to the core as a single MPIC error interrupt. Currently all the device drivers requiring access to device errors have to register for the MPIC error interrupt as a shared interrupt. With this patch we add interrupt demuxing capability in the mpic driver, allowing device drivers to register for their individual error interrupts. This is achieved by handling error interrupts in a cascaded fashion. MPIC error interrupt is handled by the error_int_handler, which subsequently demuxes it using the EISR and delivers it to the respective drivers. The error interrupt capability is dependent on the MPIC EIMR register, which was introduced in FSL MPIC version 4.1 (P4080 rev2). So, error interrupt demuxing capability is dependent on the MPIC version and can be used for versions = 4.1. Signed-off-by: Varun Sethi varun.se...@freescale.com Signed-off-by: Bogdan Hamciuc bogdan.hamc...@freescale.com [In the initial version of the patch we were using handle_simple_irq as the handler for cascaded error interrupts, this resulted in issues in case of threaded isrs (with RT kernel). This issue was debugged by Bogdan and decision was taken to use the handle_level_irq handler] [ fix commit message to wrap at 75 char columns ] --- arch/powerpc/include/asm/mpic.h| 13 +++ arch/powerpc/sysdev/Makefile |2 +- arch/powerpc/sysdev/fsl_mpic_err.c | 152 arch/powerpc/sysdev/mpic.c | 41 ++- arch/powerpc/sysdev/mpic.h | 22 + 5 files changed, 228 insertions(+), 2 deletions(-) create mode 100644 arch/powerpc/sysdev/fsl_mpic_err.c diff --git a/arch/powerpc/include/asm/mpic.h b/arch/powerpc/include/asm/mpic.h index e14d35d..5cc8000 100644 --- a/arch/powerpc/include/asm/mpic.h +++ b/arch/powerpc/include/asm/mpic.h @@ -118,6 +118,9 @@ #define MPIC_MAX_CPUS 32 #define MPIC_MAX_ISU32 +#define MPIC_MAX_ERR 32 +#define MPIC_FSL_ERR_INT 16 + /* * Tsi108 implementation of MPIC has many differences from the original one */ @@ -270,6 +273,7 @@ struct mpic struct irq_chip hc_ipi; #endif struct irq_chip hc_tm; + struct irq_chip hc_err; const char *name; /* Flags */ unsigned intflags; @@ -283,6 +287,8 @@ struct mpic /* vector numbers used for internal sources (ipi/timers) */ unsigned intipi_vecs[4]; unsigned inttimer_vecs[8]; + /* vector numbers used for FSL MPIC error interrupts */ + unsigned interr_int_vecs[MPIC_MAX_ERR]; /* Spurious vector to program into unused sources */ unsigned intspurious_vec; @@ -306,6 +312,11 @@ struct mpic struct mpic_reg_bankcpuregs[MPIC_MAX_CPUS]; struct mpic_reg_bankisus[MPIC_MAX_ISU]; + /* ioremap'ed base for error interrupt registers */ + u32 __iomem *err_regs; + /* error interrupt config */ + u32 err_int_config_done; + Is this really needed ? [Sethi Varun-B16395] check for verifying if mpic_err_int_init was successful or not. /* Protected sources */ unsigned long *protected; @@ -370,6 +381,8 @@ struct mpic #define MPIC_NO_RESET 0x4000 /* Freescale MPIC (compatible includes fsl,mpic) */ #define MPIC_FSL0x8000 +/* Freescale MPIC supports EIMR (error interrupt mask register)*/ +#define MPIC_FSL_HAS_EIMR 0x0001 Can't we use BRR for this? [Sethi Varun-B16395] This flag is set based on the BRR check. This is checked at various places. /* MPIC HW modification ID */ #define MPIC_REGSET_MASK0xf000 diff --git a/arch/powerpc/sysdev/Makefile b/arch/powerpc/sysdev/Makefile index 1bd7ecb..a57600b 100644 --- a/arch/powerpc/sysdev/Makefile +++ b/arch/powerpc/sysdev/Makefile @@ -15,7 +15,7 @@ obj-$(CONFIG_PPC_DCR_NATIVE) += dcr-low.o obj-$(CONFIG_PPC_PMI) += pmi.o obj-$(CONFIG_U3_DART) += dart_iommu.o obj-$(CONFIG_MMIO_NVRAM)+= mmio_nvram.o -obj-$(CONFIG_FSL_SOC) += fsl_soc.o +obj-$(CONFIG_FSL_SOC) += fsl_soc.o fsl_mpic_err.o obj-$(CONFIG_FSL_PCI) += fsl_pci.o $(fsl-msi-obj-y) obj-$(CONFIG_FSL_PMC) += fsl_pmc.o obj-$(CONFIG_FSL_LBC) += fsl_lbc.o diff --git a/arch/powerpc/sysdev/fsl_mpic_err.c b/arch/powerpc/sysdev/fsl_mpic_err.c new
RE: [PATCH 3/3 v3] powerpc/mpic: FSL MPIC error interrupt support.
-Original Message- From: Kumar Gala [mailto:ga...@kernel.crashing.org] Sent: Saturday, August 04, 2012 12:43 AM To: Wood Scott-B07421 Cc: Sethi Varun-B16395; Hamciuc Bogdan-BHAMCIU1; linuxppc- d...@lists.ozlabs.org Subject: Re: [PATCH 3/3 v3] powerpc/mpic: FSL MPIC error interrupt support. On Aug 3, 2012, at 11:44 AM, Scott Wood wrote: On 08/03/2012 08:19 AM, Kumar Gala wrote: On Jul 31, 2012, at 9:42 AM, Varun Sethi wrote: + /* ioremap'ed base for error interrupt registers */ + u32 __iomem *err_regs; + /* error interrupt config */ + u32 err_int_config_done; + Is this really needed ? Probably a left over from when it was done on demand. /* Protected sources */ unsigned long *protected; @@ -370,6 +381,8 @@ struct mpic #define MPIC_NO_RESET 0x4000 /* Freescale MPIC (compatible includes fsl,mpic) */ #define MPIC_FSL 0x8000 +/* Freescale MPIC supports EIMR (error interrupt mask register)*/ +#define MPIC_FSL_HAS_EIMR0x0001 Can't we use BRR for this? BRR is used, and this is set as a result. Better than opencoding a BRR check a bunch of places... I'm fine w/that, but didn't see where MPIC_FSL_HAS_EMIR was being set.. Just want to avoid the caller of mpic_alloc() having to pass it in. It's being set in mpic_setup_error_int (called from mpic_alloc) -Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 3/3 v3] powerpc/mpic: FSL MPIC error interrupt support.
-Original Message-
From: Kumar Gala [mailto:ga...@kernel.crashing.org]
Sent: Saturday, August 04, 2012 12:55 AM
To: Sethi Varun-B16395
Cc: linuxppc-dev@lists.ozlabs.org; Hamciuc Bogdan-BHAMCIU1
Subject: Re: [PATCH 3/3 v3] powerpc/mpic: FSL MPIC error interrupt
support.
On Aug 3, 2012, at 1:52 PM, Sethi Varun-B16395 wrote:
-Original Message-
From: Kumar Gala [mailto:ga...@kernel.crashing.org]
Sent: Friday, August 03, 2012 6:49 PM
To: Sethi Varun-B16395
Cc: linuxppc-dev@lists.ozlabs.org; Hamciuc Bogdan-BHAMCIU1
Subject: Re: [PATCH 3/3 v3] powerpc/mpic: FSL MPIC error interrupt
support.
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index 7e32db7..2a0b632 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -1026,6 +1026,9 @@ static int mpic_host_map(struct irq_domain *h,
unsigned int virq,
return 0;
}
+ if (mpic_map_error_int(mpic, virq, hw))
+ return 0;
+
if (hw = mpic-num_sources)
return -EINVAL;
@@ -1085,7 +1088,16 @@ static int mpic_host_xlate(struct irq_domain
*h,
struct device_node *ct,
*/
switch (intspec[2]) {
case 0:
- case 1: /* no EISR/EIMR support for now, treat as shared IRQ
*/
+ break;
+ case 1:
+ if (!mpic-err_int_config_done)
+ break;
+
Under what case would we call mpic_host_xlate and have not called
mpic_init?
[Sethi Varun-B16395] Never, but we shouldn't translate the error
interrupt specifier If mpic_err_int_init failed.
Isnt that true of a 1000 other things. If init failed we shouldn't even
call map for other reasons. I don't think we need a special check here.
[Sethi Varun-B16395] There is no specific check to see if mpic_init failed.
In this particular case if we fail to register the error interrupt handler
we cannot use the error sub interrupts.
-Varun
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 1/3] powerpc/mpic: finish supporting timer group B on Freescale chips
-Original Message- From: Wood Scott-B07421 Sent: Monday, July 09, 2012 11:12 PM To: Kumar Gala Cc: Sethi Varun-B16395; linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH 1/3] powerpc/mpic: finish supporting timer group B on Freescale chips On 07/09/2012 12:36 PM, Kumar Gala wrote: On Jul 9, 2012, at 11:43 AM, Scott Wood wrote: On 07/09/2012 09:12 AM, Kumar Gala wrote: On Jul 9, 2012, at 3:45 AM, Varun Sethi wrote: Previously, these interrupts would be mapped, but the offset calculation was broken, and only the first group was initialized. Signed-off-by: Scott Wood scottw...@freescale.com --- arch/powerpc/include/asm/mpic.h |5 +++ arch/powerpc/sysdev/mpic.c | 58 - -- 2 files changed, 47 insertions(+), 16 deletions(-) Signed-off-by: Varun Sethi varun.se...@freescale.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 0/4] powerpc/booke: Modifications to fsl booke cpu setup code
Hi Kumar, Can you please review this patch set. Regards Varun -Original Message- From: Sethi Varun-B16395 Sent: Monday, July 09, 2012 6:23 PM To: ag...@suse.de; ga...@kernel.crashing.org; b...@kernel.crashing.org; linuxppc-dev@lists.ozlabs.org; kvm-...@vger.kernel.org Cc: Sethi Varun-B16395 Subject: [PATCH 0/4] powerpc/booke: Modifications to fsl booke cpu setup code Modifications are aimed specifically at the e500mc/e5500 cpu setup code. Following modifications are introduced by this patchset: 1. Move the E.HV check to the cpu setup code. Based on this check we manipulte the CPU_FTR_EMB_HV flag (added as a part of e500mc KVM support) in the cpu_spec structure. We determine the presence of this feature based on the MMUCFG[LPIDSIZE] check and decide if we can setup the E.HV mode ivors. 2. Merge the 32 bit cpu setup code for e500mc/e5500 and define the cpu_restore routine (for e5500/e6500) only for the 64 bit case. The cpu_restore routine is used in the 64 bit case for setting up the secondary cores. 3. For the 64 bit case separate out e5500 cpu_setup and cpu_restore functions. The cpu_setup function (for the primary core) is passed the cpu_spec pointer, which is not there in case of the cpu_restore function. Also, in our case we will have to manipulate the CPU_FTR_EMB_HV flag on the the primary core. 4. Added CPU_FTR_EMB_HV check for e5500. Varun Sethi (4): Separate out E.HV check and ivor setup code. Merge the PPC 32 e5500 setup code with e500mc setup and don't define a restore routine for PPC 32. Separate out restore_e5500/setup_e5500 routines and check for E.HV mode before setting ivor setting code. Add CPU_FTR_EMB_HV check for e5500. arch/powerpc/kernel/cpu_setup_fsl_booke.S | 74 + arch/powerpc/kernel/cputable.c|4 ++ arch/powerpc/kernel/exceptions-64e.S | 18 +-- arch/powerpc/kernel/head_fsl_booke.S | 18 ++- 4 files changed, 75 insertions(+), 39 deletions(-) -- 1.7.4.1 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 3/3 v2] powerpc/mpic: FSL MPIC error interrupt support.
+ u32 eisr, eimr;
+ int errint;
+ unsigned int cascade_irq;
+
+ eisr = fsl_mpic_err_read(mpic-err_regs, eisr_offset);
+ eimr = fsl_mpic_err_read(mpic-err_regs, eimr_offset);
+
+ if (!(eisr ~eimr))
+ return IRQ_NONE;
+
+ while (eisr) {
+ errint = __builtin_clz(eisr);
+ cascade_irq = irq_linear_revmap(mpic-irqhost,
+mpic-err_int_vecs[errint]);
+ WARN_ON(cascade_irq == NO_IRQ);
+ if (cascade_irq != NO_IRQ) {
+ generic_handle_irq(cascade_irq);
+ } else {
+ eimr |= 1 (31 - errint);
+ fsl_mpic_err_write(mpic-err_regs, eimr_offset, eimr);
+ }
+ eisr = ~(1 (31 - errint));
+ }
+
+ return IRQ_HANDLED;
+}
+
+int mpic_err_int_init(struct mpic *mpic, irq_hw_number_t irqnum) {
Why can't we do this during mpic_init() time?
[Sethi Varun-B16395] Don't want to hard code the error interrupt number.
+ unsigned int virq;
+ unsigned int offset = MPIC_ERR_INT_EIMR;
remove offset, just use MPIC_ERR_INT_EIMR in mpic_err_write
+ int ret;
+
+ virq = irq_create_mapping(mpic-irqhost, irqnum);
+ if (virq == NO_IRQ) {
+ pr_err(Error interrupt setup failed\n);
+ return -ENOSPC;
+ }
+
+ fsl_mpic_err_write(mpic-err_regs, offset, ~0);
Add a comment about what this line is doing
[Sethi Varun-B16395] We are masking all the error interrupts here. I
Will add a comment for this.
+
+ ret = request_irq(virq, fsl_error_int_handler, IRQF_NO_THREAD,
+ mpic-error-int, mpic);
Hmm, should we be using irq_set_chained_handler() instead of request_irq
+ if (ret) {
+ pr_err(Failed to register error interrupt handler\n);
+ return ret;
+ }
+
+ return 0;
+}
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index 61c7225..7002ef3 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -1026,6 +1026,9 @@ static int mpic_host_map(struct irq_domain *h,
unsigned int virq,
return 0;
}
+ if (mpic_map_error_int(mpic, virq, hw))
+ return 0;
+
if (hw = mpic-num_sources)
return -EINVAL;
@@ -1085,7 +1088,24 @@ static int mpic_host_xlate(struct irq_domain *h,
struct device_node *ct,
*/
switch (intspec[2]) {
case 0:
- case 1: /* no EISR/EIMR support for now, treat as shared IRQ
*/
+ break;
+ case 1:
+ if (!(mpic-flags MPIC_FSL_HAS_EIMR))
+ break;
+
+ if (intspec[3] = ARRAY_SIZE(mpic-err_int_vecs))
+ return -EINVAL;
+
+ if (!mpic-err_int_config_done) {
+ int ret;
+ ret = mpic_err_int_init(mpic, intspec[0]);
+ if (ret)
+ return ret;
+ mpic-err_int_config_done = 1;
+ }
+
+ *out_hwirq = mpic-err_int_vecs[intspec[3]];
+
break;
case 2:
if (intspec[0] = ARRAY_SIZE(mpic-ipi_vecs)) @@ -
1302,6 +1322,8 @@
struct mpic * __init mpic_alloc(struct device_node *node,
mpic_map(mpic, mpic-paddr, mpic-tmregs, MPIC_INFO(TIMER_BASE),
0x1000);
if (mpic-flags MPIC_FSL) {
+ u32 brr1, version;
+
/*
* Yes, Freescale really did put global registers in the
* magic per-cpu area -- and they don't even show up in the
@@
-1309,6 +1331,17 @@ struct mpic * __init mpic_alloc(struct device_node
*node,
*/
mpic_map(mpic, mpic-paddr, mpic-thiscpuregs,
MPIC_CPU_THISBASE, 0x1000);
+
+ brr1 = _mpic_read(mpic-reg_type, mpic-thiscpuregs,
+ MPIC_FSL_BRR1);
+ version = brr1 MPIC_FSL_BRR1_VER;
+
+ /* Error interrupt mask register (EIMR) is required for
+* handling individual device error interrupts. EIMR
+* was added in MPIC version 4.1.
+*/
+ if (version = 0x401)
+ mpic_setup_error_int(mpic, intvec_top - 12);
Would really like not to have this magic 12, but a comment would be nice
if we keep it where the 12 comes from
[Sethi Varun-B16395]Obtaining vector numbers beyond ipi and timers for the
error interrupts.
Will add a comment.
-Varun
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 3/3 v2] powerpc/mpic: FSL MPIC error interrupt support.
-Original Message-
From: Kumar Gala [mailto:ga...@kernel.crashing.org]
Sent: Tuesday, July 10, 2012 7:17 AM
To: Wood Scott-B07421
Cc: Sethi Varun-B16395; Hamciuc Bogdan-BHAMCIU1; linuxppc-
d...@lists.ozlabs.org
Subject: Re: [PATCH 3/3 v2] powerpc/mpic: FSL MPIC error interrupt
support.
On Jul 9, 2012, at 3:22 PM, Scott Wood wrote:
On 07/09/2012 02:03 PM, Kumar Gala wrote:
On Jul 9, 2012, at 3:47 AM, Varun Sethi wrote:
+int mpic_err_int_init(struct mpic *mpic, irq_hw_number_t irqnum) {
Why can't we do this during mpic_init() time?
Are you willing to hardcode that IRQ 16 is the error interrupt,
without waiting to see an intspec?
I'm torn, but the bit of code in mpic_host_xlate that calls
mpic_err_int_init() is just ugly.
We could consider it similar to how we assume IPIs.
[Sethi Varun-B16395] I don't understand this point.
-Varun
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/3 v2] powerpc/mpic: Use the MPIC_LARGE_VECTORS flag for FSL MPIC.
-Original Message- From: Kumar Gala [mailto:ga...@kernel.crashing.org] Sent: Monday, July 09, 2012 7:58 PM To: Sethi Varun-B16395 Cc: linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH 2/3 v2] powerpc/mpic: Use the MPIC_LARGE_VECTORS flag for FSL MPIC. On Jul 9, 2012, at 3:46 AM, Varun Sethi wrote: We should use the MPIC_LARG_VECTORS flag while intializing the MPIC. This prevents us from eating in to hardware vector number space (MSIs) while setting up internal sources. Signed-off-by: Varun Sethi varun.se...@freescale.com --- arch/powerpc/sysdev/mpic.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) Can you remind me the issue here w/running into the MSIs? Vector numbers for internal interrupt sources like timers/ipis are taken from the hardware vector number space. Currently we specify number of interrupt sources as 256 and extract 12 vectors from here for internal interrupt sources. In process we end up eating vector numbers meant for MSIs. With this patch we are basically expanding our vector number space. -Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 4/4] powerpc/mpic: FSL MPIC error interrupt support.
-Original Message- From: Wood Scott-B07421 Sent: Tuesday, June 19, 2012 12:53 AM To: Sethi Varun-B16395 Cc: Wood Scott-B07421; Kumar Gala; Linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH 4/4] powerpc/mpic: FSL MPIC error interrupt support. On 06/18/2012 02:19 PM, Sethi Varun-B16395 wrote: -Original Message- From: Wood Scott-B07421 Sent: Tuesday, June 19, 2012 12:47 AM To: Sethi Varun-B16395 Cc: Kumar Gala; Wood Scott-B07421; Linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH 4/4] powerpc/mpic: FSL MPIC error interrupt support. On 06/18/2012 02:12 PM, Sethi Varun-B16395 wrote: +/* + * Error interrupt registers + */ + +#define MPIC_ERR_INT_BASE0x3900 +#define MPIC_ERR_INT_EISR0x +#define MPIC_ERR_INT_EIMR0x0010 + #define MPIC_MAX_IRQ_SOURCES 2048 #define MPIC_MAX_CPUS 32 #define MPIC_MAX_ISU 32 #define MPIC_MAX_TIMER8 #define MPIC_MAX_IPI 4 +#define MPIC_MAX_ERR 32 Should probably be 64 This patch supports MPIC 4.1 and EISR0. When support is added for EISR1 (didn't realize this was coming until your comment prompted me to check...), this should be updated, but this change alone would not make it work. Would prefer we handle this now rather than later (T4240 is going to need EISR1 support). Hi Kumar, As of now I don't have a proper mechanism to test this functionality. I will submit a follow up patch for EISR1/EIMR1 support once I have a mechanism to test this functionality. You could still write the code in a way that scales to multiple EISRs, and test that it works with EISR0. Yes, but I would like to submit the patch once I have tested it. So test it the way I described, and submit. :-P There just seem to be 32 error interrupts even in case of T4240, that means there is no need to handle multiple EISRs. I have already submitted a revised patch for handling MPIC error interrupts. [PATCH 3/3 v2] powerpc/mpic: FSL MPIC error interrupt support -Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 4/4] powerpc/mpic: FSL MPIC error interrupt support.
+/* + * Error interrupt registers + */ + +#define MPIC_ERR_INT_BASE0x3900 +#define MPIC_ERR_INT_EISR0x +#define MPIC_ERR_INT_EIMR0x0010 + #define MPIC_MAX_IRQ_SOURCES 2048 #define MPIC_MAX_CPUS 32 #define MPIC_MAX_ISU 32 #define MPIC_MAX_TIMER8 #define MPIC_MAX_IPI 4 +#define MPIC_MAX_ERR 32 Should probably be 64 This patch supports MPIC 4.1 and EISR0. When support is added for EISR1 (didn't realize this was coming until your comment prompted me to check...), this should be updated, but this change alone would not make it work. Would prefer we handle this now rather than later (T4240 is going to need EISR1 support). Hi Kumar, As of now I don't have a proper mechanism to test this functionality. I will submit a follow up patch for EISR1/EIMR1 support once I have a mechanism to test this functionality. Regards Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 4/4] powerpc/mpic: FSL MPIC error interrupt support.
-Original Message- From: Wood Scott-B07421 Sent: Tuesday, June 19, 2012 12:47 AM To: Sethi Varun-B16395 Cc: Kumar Gala; Wood Scott-B07421; Linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH 4/4] powerpc/mpic: FSL MPIC error interrupt support. On 06/18/2012 02:12 PM, Sethi Varun-B16395 wrote: +/* + * Error interrupt registers + */ + +#define MPIC_ERR_INT_BASE 0x3900 +#define MPIC_ERR_INT_EISR 0x +#define MPIC_ERR_INT_EIMR 0x0010 + #define MPIC_MAX_IRQ_SOURCES2048 #define MPIC_MAX_CPUS 32 #define MPIC_MAX_ISU32 #define MPIC_MAX_TIMER8 #define MPIC_MAX_IPI 4 +#define MPIC_MAX_ERR 32 Should probably be 64 This patch supports MPIC 4.1 and EISR0. When support is added for EISR1 (didn't realize this was coming until your comment prompted me to check...), this should be updated, but this change alone would not make it work. Would prefer we handle this now rather than later (T4240 is going to need EISR1 support). Hi Kumar, As of now I don't have a proper mechanism to test this functionality. I will submit a follow up patch for EISR1/EIMR1 support once I have a mechanism to test this functionality. You could still write the code in a way that scales to multiple EISRs, and test that it works with EISR0. Yes, but I would like to submit the patch once I have tested it. -Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
FW: [PATCH 0/3] powerpc/mpic: Enhancements for FSL MPIC.
Hi Kumar, If possible could you please review this patch set. Regards Varun -Original Message- From: Sethi Varun-B16395 Sent: Sunday, June 03, 2012 1:11 PM To: linuxppc-dev@lists.ozlabs.org Cc: Sethi Varun-B16395 Subject: [PATCH 0/3] powerpc/mpic: Enhancements for FSL MPIC. This patchset adds/fixes the following functionality specific to the FSL MPIC: 1. Fix support for timer group B interrupts. Previously these were not getting initialized. 2. Use the MPIC_LARGE_VECTORS flag while intializing FSL MPIC. This prevents us from eating in to hardware vector number space (MSIs) while setting up internal sources. 3.Cascaded handling for the MPIC error interrupt. This is possible with FSL MPIC version = 4.1. The patches are based on next branch of Benjamin Herrenschmidt's powerpc linux tree. Varun Sethi (3): Support time group b on freescale chips. Use MPIC_LARGE_VECTORS flag for Freescale MPIC. Add support for cascaded error interrupt handling. arch/powerpc/include/asm/mpic.h | 22 arch/powerpc/sysdev/Makefile |2 +- arch/powerpc/sysdev/fsl_mpic_err.c | 157 ++ arch/powerpc/sysdev/mpic.c | 95 +++ arch/powerpc/sysdev/mpic.h | 22 6 files changed, 338 insertions(+), 19 deletions(-) create mode 100644 arch/powerpc/sysdev/fsl_mpic_err.c -- 1.7.2.2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 3/4] powerpc/mpic: Move internal interrupt source vector allocation to a separate function.
-Original Message- From: Kumar Gala [mailto:ga...@kernel.crashing.org] Sent: Tuesday, March 27, 2012 7:32 PM To: Sethi Varun-B16395 Cc: Linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH 3/4] powerpc/mpic: Move internal interrupt source vector allocation to a separate function. On Mar 27, 2012, at 8:52 AM, Sethi Varun-B16395 wrote: -Original Message- From: Kumar Gala [mailto:ga...@kernel.crashing.org] Sent: Tuesday, March 27, 2012 6:55 PM To: Sethi Varun-B16395 Cc: Linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH 3/4] powerpc/mpic: Move internal interrupt source vector allocation to a separate function. On Mar 27, 2012, at 7:16 AM, Varun Sethi wrote: Allocate vector numbers for MPIC internal interrupt sources (IPIs and Timers) in a separate function. Explain why you are making this change. [Sethi Varun-B16395] With the current code it becomes fairly difficult to add new internal interrupt sources. In my case I had to add 32 additional interrupt sources corresponding to the MPIC error interrupts. It's more convenient doing the internal interrupt source allocation using a loop. I think that is more due to how you added the MPIC error interrupts and issues w/that code. If you are treating the MPIC error interrupts as a cascade than they should have a distinct linux IRQ space from the standard MPIC interrupts. This is how the MSIs work (as an example). In case of error interrupts we are depending on the mpic_host_maps for mapping and interrupt specifier translations. There is no separate initialization as in case of MSIs. That's the reason I am treating error interrupts as mpic internal interrupt sources. -Varun ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
