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-kernel@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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-kernel@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 2/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-...@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-kernel@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 > > > > > > --- > > > > > > 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)
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-...@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-kernel@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 > > > > --- > > > > 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 */ > >
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-...@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-kernel@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 > > --- > > 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")) > &g
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-...@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-kernel@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-...@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-kernel@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-...@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-kernel@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-kernel@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 > > --- > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-kernel@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 2/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-...@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-kernel@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-...@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-...@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-...@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: Bh
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-...@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-kernel@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-...@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-...@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-...@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-...@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-kernel@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 > --- > 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 == _bus_type) { > - pdev = to_pci_dev(dev); > + if ((*dev)->bus == _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", ); > +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(_dev); > + > + liodn = of_get_property(dma_dev->of_node, "fsl,liodn", ); > 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 == _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(_dev); > > - prop = of_get_property(dev->of_node, "fsl,liodn", ); > + prop = of_get_property(dma_dev->of_node, "fsl,liodn", ); > 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 > s
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-kernel@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 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-kernel@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-kernel@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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 2/7] iommu: add api to get iommu_domain of a device
o 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 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 2/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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-kernel@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-kernel@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-...@lists.ozlabs.org; > linux-kernel@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 > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-...@lists.ozlabs.org; linux-kernel@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] iommu/exynos: add devices attached to the System MMU to an IOMMU group
> -Original Message- > From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- > boun...@lists.linux-foundation.org] On Behalf Of Antonios Motakis > Sent: Tuesday, July 23, 2013 3:32 PM > To: linux-arm-ker...@lists.infradead.org; iommu@lists.linux- > foundation.org; linux-samsung-...@vger.kernel.org > Cc: Sachin Kamat; Jiri Kosina; open list; Wei Yongjun; Antonios Motakis; > Cho KyongHo; kvm...@lists.cs.columbia.edu > Subject: [PATCH] iommu/exynos: add devices attached to the System MMU to > an IOMMU group > > IOMMU groups are expected by certain users of the IOMMU API, e.g. VFIO. > Since each device is behind its own System MMU, we can allocate a new > IOMMU group for each device. > > This patch depends on Cho KyongHo's patch series titled "[PATCH v7 00/12] > iommu/exynos: Fixes and Enhancements of System MMU driver with DT", > applied on a Linux 3.10.1 kernel. It has been tested on the Arndale > board. > > Signed-off-by: Antonios Motakis > --- > drivers/iommu/exynos-iommu.c | 24 > 1 file changed, 24 insertions(+) > > diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c > index 51d43bb..9f39eaa 100644 > --- a/drivers/iommu/exynos-iommu.c > +++ b/drivers/iommu/exynos-iommu.c > @@ -1134,6 +1134,28 @@ static phys_addr_t > exynos_iommu_iova_to_phys(struct iommu_domain *domain, > return phys; > } > > +static int exynos_iommu_add_device(struct device *dev) { > + struct iommu_group *group; > + int ret; > + > + group = iommu_group_alloc(); > + if (IS_ERR(group)) { > + dev_err(dev, "Failed to allocate IOMMU group\n"); > + return PTR_ERR(group); > + } > + > + ret = iommu_group_add_device(group, dev); > + iommu_group_put(group); > + > + return ret; > +} > + > +static void exynos_iommu_remove_device(struct device *dev) { > + iommu_group_remove_device(dev); > +} > + > static struct iommu_ops exynos_iommu_ops = { > .domain_init = _iommu_domain_init, > .domain_destroy = _iommu_domain_destroy, @@ -1142,6 +1164,8 > @@ static struct iommu_ops exynos_iommu_ops = { > .map = _iommu_map, > .unmap = _iommu_unmap, > .iova_to_phys = _iommu_iova_to_phys, > + .add_device = exynos_iommu_add_device, > + .remove_device = exynos_iommu_remove_device, > .pgsize_bitmap = SECT_SIZE | LPAGE_SIZE | SPAGE_SIZE, }; > [Sethi Varun-B16395] Do you support PCI devices? This assumption of one iommu group per pci device may not hold true in case of PCI devices. -Varun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] iommu/exynos: add devices attached to the System MMU to an IOMMU group
-Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Antonios Motakis Sent: Tuesday, July 23, 2013 3:32 PM To: linux-arm-ker...@lists.infradead.org; iommu@lists.linux- foundation.org; linux-samsung-...@vger.kernel.org Cc: Sachin Kamat; Jiri Kosina; open list; Wei Yongjun; Antonios Motakis; Cho KyongHo; kvm...@lists.cs.columbia.edu Subject: [PATCH] iommu/exynos: add devices attached to the System MMU to an IOMMU group IOMMU groups are expected by certain users of the IOMMU API, e.g. VFIO. Since each device is behind its own System MMU, we can allocate a new IOMMU group for each device. This patch depends on Cho KyongHo's patch series titled [PATCH v7 00/12] iommu/exynos: Fixes and Enhancements of System MMU driver with DT, applied on a Linux 3.10.1 kernel. It has been tested on the Arndale board. Signed-off-by: Antonios Motakis a.mota...@virtualopensystems.com --- drivers/iommu/exynos-iommu.c | 24 1 file changed, 24 insertions(+) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 51d43bb..9f39eaa 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -1134,6 +1134,28 @@ static phys_addr_t exynos_iommu_iova_to_phys(struct iommu_domain *domain, return phys; } +static int exynos_iommu_add_device(struct device *dev) { + struct iommu_group *group; + int ret; + + group = iommu_group_alloc(); + if (IS_ERR(group)) { + dev_err(dev, Failed to allocate IOMMU group\n); + return PTR_ERR(group); + } + + ret = iommu_group_add_device(group, dev); + iommu_group_put(group); + + return ret; +} + +static void exynos_iommu_remove_device(struct device *dev) { + iommu_group_remove_device(dev); +} + static struct iommu_ops exynos_iommu_ops = { .domain_init = exynos_iommu_domain_init, .domain_destroy = exynos_iommu_domain_destroy, @@ -1142,6 +1164,8 @@ static struct iommu_ops exynos_iommu_ops = { .map = exynos_iommu_map, .unmap = exynos_iommu_unmap, .iova_to_phys = exynos_iommu_iova_to_phys, + .add_device = exynos_iommu_add_device, + .remove_device = exynos_iommu_remove_device, .pgsize_bitmap = SECT_SIZE | LPAGE_SIZE | SPAGE_SIZE, }; [Sethi Varun-B16395] Do you support PCI devices? This assumption of one iommu group per pci device may not hold true in case of PCI devices. -Varun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [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-...@lists.ozlabs.org; linux-kernel@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 > --- > 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(
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-...@lists.ozlabs.org; linux-kernel@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-kernel@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 > > Signed-off-by: Varun Sethi > > --- > > For iommu group support > > Acked-by: Alex Williamson > N�r��yb�X��ǧv�^�)Þº{.n�+{zX����ܨ}ï¿½ï¿½ï¿½Æ z�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jÇ«y�m��@A�a��� 0��h���i
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-kernel@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 N�r��yb�X��ǧv�^�)Þº{.n�+{zX����ܨ}ï¿½ï¿½ï¿½Æ z�j:+v���zZ+��+zf���h���~i���z��w���?��)ߢf��^jÇ«y�m��@A�a��� 0��h���i
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-kernel@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(>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(>devices)) { > > + pdev = container_of(bus->devices.next, > > + struct pci_dev, bus_list); > > pdev = list_first_entry(>devices, struct pci_dev, bus_list); > > > + group = iommu_group_get(>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, >bus_list, bus_list) { > if (tmp == pdev) > continue; > > group = iommu_group_get(>dev); > break; > } > > > + group = iommu_group_get(>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(_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. > > > +
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-kernel@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-...@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 > > --- > > - 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 > > - k -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 2/5 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-...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] iommu: amd/intel: Remove multifunction assumption around grouping
> -Original Message- > From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- > boun...@lists.linux-foundation.org] On Behalf Of Alex Williamson > Sent: Friday, May 31, 2013 12:09 AM > To: io...@lists.linux-foundation.org; dw...@infradead.org; > j...@8bytes.org > Cc: linux-...@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: [PATCH] iommu: amd/intel: Remove multifunction assumption around > grouping > > If a device is multifunction and does not have ACS enabled then we assume > that the entire package lacks ACS and use function 0 as the base of the > group. The PCIe spec however states that components are permitted to > implement ACS on some, none, or all of their applicable functions. It's > therefore conceivable that function 0 may be fully independent and > support ACS while other functions do not. Instead use the lowest > function of the slot that does not have ACS enabled as the base of the > group. This may be the current device, which is intentional. So long as > we use a consistent algorithm, all the non-ACS functions will be grouped > together and ACS functions will get separate groups. > > Signed-off-by: Alex Williamson > --- > drivers/iommu/amd_iommu.c | 25 +++-- > drivers/iommu/intel-iommu.c | 25 +++-- > 2 files changed, 38 insertions(+), 12 deletions(-) > > diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c index > 1d84be1..565c745 100644 > --- a/drivers/iommu/amd_iommu.c > +++ b/drivers/iommu/amd_iommu.c > @@ -287,14 +287,27 @@ static struct pci_dev *get_isolation_root(struct > pci_dev *pdev) > > /* >* If it's a multifunction device that does not support our > - * required ACS flags, add to the same group as function 0. > + * required ACS flags, add to the same group as lowest numbered > + * function that also does not suport the required ACS flags. >*/ > if (dma_pdev->multifunction && > - !pci_acs_enabled(dma_pdev, REQ_ACS_FLAGS)) > - swap_pci_ref(_pdev, > - pci_get_slot(dma_pdev->bus, > - PCI_DEVFN(PCI_SLOT(dma_pdev->devfn), > - 0))); > + !pci_acs_enabled(dma_pdev, REQ_ACS_FLAGS)) { > + u8 i, slot = PCI_SLOT(dma_pdev->devfn); > + > + for (i = 0; i < 8; i++) { > + struct pci_dev *tmp; > + > + tmp = pci_get_slot(dma_pdev->bus, PCI_DEVFN(slot, i)); > + if (!tmp) > + continue; > + > + if (!pci_acs_enabled(tmp, REQ_ACS_FLAGS)) { > + swap_pci_ref(_pdev, tmp); > + break; > + } > + pci_dev_put(tmp); > + } > + } > > /* >* Devices on the root bus go through the iommu. If that's not us, > diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c > index b4f0e28..eec0d3e 100644 > --- a/drivers/iommu/intel-iommu.c > +++ b/drivers/iommu/intel-iommu.c > @@ -4182,14 +4182,27 @@ static int intel_iommu_add_device(struct device > *dev) > > /* >* If it's a multifunction device that does not support our > - * required ACS flags, add to the same group as function 0. > + * required ACS flags, add to the same group as lowest numbered > + * function that also does not suport the required ACS flags. >*/ > if (dma_pdev->multifunction && > - !pci_acs_enabled(dma_pdev, REQ_ACS_FLAGS)) > - swap_pci_ref(_pdev, > - pci_get_slot(dma_pdev->bus, > - PCI_DEVFN(PCI_SLOT(dma_pdev->devfn), > - 0))); > + !pci_acs_enabled(dma_pdev, REQ_ACS_FLAGS)) { > + u8 i, slot = PCI_SLOT(dma_pdev->devfn); > + > + for (i = 0; i < 8; i++) { [Sethi Varun-B16395] A macro like PCI_MAX_FUNCTIONS would improve code readability. > + struct pci_dev *tmp; > + > + tmp = pci_get_slot(dma_pdev->bus, PCI_DEVFN(slot, i)); > + if (!tmp) > + continue; > + > + if (!pci_acs_enabled(tmp, REQ_ACS_FLAGS)) { > + swap_pci_ref(_pdev, tmp); > + break; > + } > + pci_dev_put(tmp); > + } > + } It would be nice if this code could be represented as a function in a common file like iommu/pci.c. -Varun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] iommu: amd/intel: Remove multifunction assumption around grouping
-Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Alex Williamson Sent: Friday, May 31, 2013 12:09 AM To: io...@lists.linux-foundation.org; dw...@infradead.org; j...@8bytes.org Cc: linux-...@vger.kernel.org; linux-kernel@vger.kernel.org Subject: [PATCH] iommu: amd/intel: Remove multifunction assumption around grouping If a device is multifunction and does not have ACS enabled then we assume that the entire package lacks ACS and use function 0 as the base of the group. The PCIe spec however states that components are permitted to implement ACS on some, none, or all of their applicable functions. It's therefore conceivable that function 0 may be fully independent and support ACS while other functions do not. Instead use the lowest function of the slot that does not have ACS enabled as the base of the group. This may be the current device, which is intentional. So long as we use a consistent algorithm, all the non-ACS functions will be grouped together and ACS functions will get separate groups. Signed-off-by: Alex Williamson alex.william...@redhat.com --- drivers/iommu/amd_iommu.c | 25 +++-- drivers/iommu/intel-iommu.c | 25 +++-- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c index 1d84be1..565c745 100644 --- a/drivers/iommu/amd_iommu.c +++ b/drivers/iommu/amd_iommu.c @@ -287,14 +287,27 @@ static struct pci_dev *get_isolation_root(struct pci_dev *pdev) /* * If it's a multifunction device that does not support our - * required ACS flags, add to the same group as function 0. + * required ACS flags, add to the same group as lowest numbered + * function that also does not suport the required ACS flags. */ 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))); + !pci_acs_enabled(dma_pdev, REQ_ACS_FLAGS)) { + u8 i, slot = PCI_SLOT(dma_pdev-devfn); + + for (i = 0; i 8; i++) { + struct pci_dev *tmp; + + tmp = pci_get_slot(dma_pdev-bus, PCI_DEVFN(slot, i)); + if (!tmp) + continue; + + if (!pci_acs_enabled(tmp, REQ_ACS_FLAGS)) { + swap_pci_ref(dma_pdev, tmp); + break; + } + pci_dev_put(tmp); + } + } /* * Devices on the root bus go through the iommu. If that's not us, diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index b4f0e28..eec0d3e 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -4182,14 +4182,27 @@ static int intel_iommu_add_device(struct device *dev) /* * If it's a multifunction device that does not support our - * required ACS flags, add to the same group as function 0. + * required ACS flags, add to the same group as lowest numbered + * function that also does not suport the required ACS flags. */ 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))); + !pci_acs_enabled(dma_pdev, REQ_ACS_FLAGS)) { + u8 i, slot = PCI_SLOT(dma_pdev-devfn); + + for (i = 0; i 8; i++) { [Sethi Varun-B16395] A macro like PCI_MAX_FUNCTIONS would improve code readability. + struct pci_dev *tmp; + + tmp = pci_get_slot(dma_pdev-bus, PCI_DEVFN(slot, i)); + if (!tmp) + continue; + + if (!pci_acs_enabled(tmp, REQ_ACS_FLAGS)) { + swap_pci_ref(dma_pdev, tmp); + break; + } + pci_dev_put(tmp); + } + } It would be nice if this code could be represented as a function in a common file like iommu/pci.c. -Varun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 2/2] vfio: hugepage support for vfio_iommu_type1
> -Original Message- > From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- > boun...@lists.linux-foundation.org] On Behalf Of Alex Williamson > Sent: Friday, May 24, 2013 10:55 PM > To: alex.william...@redhat.com > Cc: io...@lists.linux-foundation.org; chegu_vi...@hp.com; qemu- > de...@nongnu.org; k...@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: [PATCH 2/2] vfio: hugepage support for vfio_iommu_type1 > > We currently send all mappings to the iommu in PAGE_SIZE chunks, which > prevents the iommu from enabling support for larger page sizes. > We still need to pin pages, which means we step through them in PAGE_SIZE > chunks, but we can batch up contiguous physical memory chunks to allow > the iommu the opportunity to use larger pages. The approach here is a > bit different that the one currently used for legacy KVM device > assignment. Rather than looking at the vma page size and using that as > the maximum size to pass to the iommu, we instead simply look at whether > the next page is physically contiguous. This means we might ask the > iommu to map a 4MB region, while legacy KVM might limit itself to a [Sethi Varun-B16395] Wouldn't this depend on the IOMMU page alignment constraints? > maximum of 2MB. > > Splitting our mapping path also allows us to be smarter about locked > memory because we can more easily unwind if the user attempts to exceed > the limit. Therefore, rather than assuming that a mapping will result in > locked memory, we test each page as it is pinned to determine whether it > locks RAM vs an mmap'd MMIO region. This should result in better locking > granularity and less locked page fudge factors in userspace. > > The unmap path uses the same algorithm as legacy KVM. We don't want to > track the pfn for each mapping ourselves, but we need the pfn in order to > unpin pages. We therefore ask the iommu for the iova to physical address > translation, ask it to unpin a page, and see how many pages were actually > unpinned. iommus supporting large pages will often return something > bigger than a page here, which we know will be physically contiguous and > we can unpin a batch of pfns. iommus not supporting large mappings won't > see an improvement in batching here as they only unmap a page at a time. > > With this change, we also make a clarification to the API for mapping and > unmapping DMA. We can only guarantee unmaps at the same granularity as > used for the original mapping. In other words, unmapping a subregion of > a previous mapping is not guaranteed and may result in a larger or > smaller unmapping than requested. The size field in the unmapping > structure is updated to reflect this. > Previously this was unmodified on mapping, always returning the the > requested unmap size. This is now updated to return the actual unmap > size on success, allowing userspace to appropriately track mappings. > [Sethi Varun-B16395] The main problem here is that the user space application is oblivious of the physical memory contiguity, right? > Signed-off-by: Alex Williamson > --- > drivers/vfio/vfio_iommu_type1.c | 523 + > -- > +static long vfio_unpin_pages(unsigned long pfn, long npage, > + int prot, bool do_accounting) > +{ > + unsigned long unlocked = 0; > + long i; > + > + for (i = 0; i < npage; i++) > + unlocked += put_pfn(pfn++, prot); > + > + if (do_accounting) > + vfio_lock_acct(-unlocked); > + > + return unlocked; > +} > + > +static int vfio_unmap_unpin(struct vfio_iommu *iommu, struct vfio_dma > *dma, > + dma_addr_t iova, size_t *size) > +{ > + dma_addr_t start = iova, end = iova + *size; > + long unlocked = 0; > + > + while (iova < end) { > + size_t unmapped; > + phys_addr_t phys; > + > /* > - * Only add actual locked pages to accounting > - * XXX We're effectively marking a page locked for every > - * IOVA page even though it's possible the user could be > - * backing multiple IOVAs with the same vaddr. This over- > - * penalizes the user process, but we currently have no > - * easy way to do this properly. > + * We use the IOMMU to track the physical address. This > + * saves us from having a lot more entries in our mapping > + * tree. The downside is that we don't track the size > + * used to do the mapping. We request unmap of a single > + * page, but expect IOMMUs that support large pages to > + * unmap a larger chun
RE: [PATCH 2/2] vfio: hugepage support for vfio_iommu_type1
-Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Alex Williamson Sent: Friday, May 24, 2013 10:55 PM To: alex.william...@redhat.com Cc: io...@lists.linux-foundation.org; chegu_vi...@hp.com; qemu- de...@nongnu.org; k...@vger.kernel.org; linux-kernel@vger.kernel.org Subject: [PATCH 2/2] vfio: hugepage support for vfio_iommu_type1 We currently send all mappings to the iommu in PAGE_SIZE chunks, which prevents the iommu from enabling support for larger page sizes. We still need to pin pages, which means we step through them in PAGE_SIZE chunks, but we can batch up contiguous physical memory chunks to allow the iommu the opportunity to use larger pages. The approach here is a bit different that the one currently used for legacy KVM device assignment. Rather than looking at the vma page size and using that as the maximum size to pass to the iommu, we instead simply look at whether the next page is physically contiguous. This means we might ask the iommu to map a 4MB region, while legacy KVM might limit itself to a [Sethi Varun-B16395] Wouldn't this depend on the IOMMU page alignment constraints? maximum of 2MB. Splitting our mapping path also allows us to be smarter about locked memory because we can more easily unwind if the user attempts to exceed the limit. Therefore, rather than assuming that a mapping will result in locked memory, we test each page as it is pinned to determine whether it locks RAM vs an mmap'd MMIO region. This should result in better locking granularity and less locked page fudge factors in userspace. The unmap path uses the same algorithm as legacy KVM. We don't want to track the pfn for each mapping ourselves, but we need the pfn in order to unpin pages. We therefore ask the iommu for the iova to physical address translation, ask it to unpin a page, and see how many pages were actually unpinned. iommus supporting large pages will often return something bigger than a page here, which we know will be physically contiguous and we can unpin a batch of pfns. iommus not supporting large mappings won't see an improvement in batching here as they only unmap a page at a time. With this change, we also make a clarification to the API for mapping and unmapping DMA. We can only guarantee unmaps at the same granularity as used for the original mapping. In other words, unmapping a subregion of a previous mapping is not guaranteed and may result in a larger or smaller unmapping than requested. The size field in the unmapping structure is updated to reflect this. Previously this was unmodified on mapping, always returning the the requested unmap size. This is now updated to return the actual unmap size on success, allowing userspace to appropriately track mappings. [Sethi Varun-B16395] The main problem here is that the user space application is oblivious of the physical memory contiguity, right? Signed-off-by: Alex Williamson alex.william...@redhat.com --- drivers/vfio/vfio_iommu_type1.c | 523 + -- +static long vfio_unpin_pages(unsigned long pfn, long npage, + int prot, bool do_accounting) +{ + unsigned long unlocked = 0; + long i; + + for (i = 0; i npage; i++) + unlocked += put_pfn(pfn++, prot); + + if (do_accounting) + vfio_lock_acct(-unlocked); + + return unlocked; +} + +static int vfio_unmap_unpin(struct vfio_iommu *iommu, struct vfio_dma *dma, + dma_addr_t iova, size_t *size) +{ + dma_addr_t start = iova, end = iova + *size; + long unlocked = 0; + + while (iova end) { + size_t unmapped; + phys_addr_t phys; + /* - * Only add actual locked pages to accounting - * XXX We're effectively marking a page locked for every - * IOVA page even though it's possible the user could be - * backing multiple IOVAs with the same vaddr. This over- - * penalizes the user process, but we currently have no - * easy way to do this properly. + * We use the IOMMU to track the physical address. This + * saves us from having a lot more entries in our mapping + * tree. The downside is that we don't track the size + * used to do the mapping. We request unmap of a single + * page, but expect IOMMUs that support large pages to + * unmap a larger chunk. */ - if (!is_invalid_reserved_pfn(pfn)) - locked++; - - ret = iommu_map(iommu-domain, iova, - (phys_addr_t)pfn PAGE_SHIFT, - PAGE_SIZE, prot); - if (ret) { - /* Back out mappings on error
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-...@lists.ozlabs.org; > linux-kernel@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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-...@lists.ozlabs.org; linux-kernel@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-kernel@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 > --- > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-kernel@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 13/21] iommu: tegra: print dma_addr_t using %lld
Hi Arnd, I already submitted a patch to fix this. http://permalink.gmane.org/gmane.linux.kernel.iommu/2036 Regards Varun > -Original Message- > From: Arnd Bergmann [mailto:a...@arndb.de] > Sent: Thursday, April 25, 2013 10:59 PM > To: linux-arm-ker...@lists.infradead.org > Cc: linux-kernel@vger.kernel.org; Arnd Bergmann; Sethi Varun-B16395; > Joerg Roedel > Subject: [PATCH 13/21] iommu: tegra: print dma_addr_t using %lld > > The code was recently changed to work for builds with a 64 bit > dma_addr_t, but the printk unconditionally uses a format string for an > "long" variable, which is always wrong as the dma_add_t is now either > 'unsigned int' or 'unsigned long long' > depending on configuration. > > The easiest solution is to cast the variable to u64 and print it as a 64 > bit value. > > Signed-off-by: Arnd Bergmann > Cc: Varun Sethi > Cc: Joerg Roedel > --- > drivers/iommu/tegra-gart.c | 3 ++- > drivers/iommu/tegra-smmu.c | 2 +- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/iommu/tegra-gart.c b/drivers/iommu/tegra-gart.c > index 4aec8be..ebbdd82 100644 > --- a/drivers/iommu/tegra-gart.c > +++ b/drivers/iommu/tegra-gart.c > @@ -295,7 +295,8 @@ static phys_addr_t gart_iommu_iova_to_phys(struct > iommu_domain *domain, > > pa = (pte & GART_PAGE_MASK); > if (!pfn_valid(__phys_to_pfn(pa))) { > - dev_err(gart->dev, "No entry for %08lx:%08x\n", iova, pa); > + dev_err(gart->dev, "No entry for %08llx:%pa\n", > + (u64)iova, ); > gart_dump_table(gart); > return -EINVAL; > } > diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c > index bc9b599..ef2a120 100644 > --- a/drivers/iommu/tegra-smmu.c > +++ b/drivers/iommu/tegra-smmu.c > @@ -772,7 +772,7 @@ static phys_addr_t smmu_iommu_iova_to_phys(struct > iommu_domain *domain, > pfn = *pte & SMMU_PFN_MASK; > WARN_ON(!pfn_valid(pfn)); > dev_dbg(as->smmu->dev, > - "iova:%08lx pfn:%08lx asid:%d\n", iova, pfn, as->asid); > + "iova:%08llx pfn:%08lx asid:%d\n", (u64)iova, pfn, as->asid); > > spin_unlock_irqrestore(>lock, flags); > return PFN_PHYS(pfn); > -- > 1.8.1.2 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 13/21] iommu: tegra: print dma_addr_t using %lld
Hi Arnd, I already submitted a patch to fix this. http://permalink.gmane.org/gmane.linux.kernel.iommu/2036 Regards Varun -Original Message- From: Arnd Bergmann [mailto:a...@arndb.de] Sent: Thursday, April 25, 2013 10:59 PM To: linux-arm-ker...@lists.infradead.org Cc: linux-kernel@vger.kernel.org; Arnd Bergmann; Sethi Varun-B16395; Joerg Roedel Subject: [PATCH 13/21] iommu: tegra: print dma_addr_t using %lld The code was recently changed to work for builds with a 64 bit dma_addr_t, but the printk unconditionally uses a format string for an long variable, which is always wrong as the dma_add_t is now either 'unsigned int' or 'unsigned long long' depending on configuration. The easiest solution is to cast the variable to u64 and print it as a 64 bit value. Signed-off-by: Arnd Bergmann a...@arndb.de Cc: Varun Sethi varun.se...@freescale.com Cc: Joerg Roedel j...@8bytes.org --- drivers/iommu/tegra-gart.c | 3 ++- drivers/iommu/tegra-smmu.c | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/tegra-gart.c b/drivers/iommu/tegra-gart.c index 4aec8be..ebbdd82 100644 --- a/drivers/iommu/tegra-gart.c +++ b/drivers/iommu/tegra-gart.c @@ -295,7 +295,8 @@ static phys_addr_t gart_iommu_iova_to_phys(struct iommu_domain *domain, pa = (pte GART_PAGE_MASK); if (!pfn_valid(__phys_to_pfn(pa))) { - dev_err(gart-dev, No entry for %08lx:%08x\n, iova, pa); + dev_err(gart-dev, No entry for %08llx:%pa\n, + (u64)iova, pa); gart_dump_table(gart); return -EINVAL; } diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c index bc9b599..ef2a120 100644 --- a/drivers/iommu/tegra-smmu.c +++ b/drivers/iommu/tegra-smmu.c @@ -772,7 +772,7 @@ static phys_addr_t smmu_iommu_iova_to_phys(struct iommu_domain *domain, pfn = *pte SMMU_PFN_MASK; WARN_ON(!pfn_valid(pfn)); dev_dbg(as-smmu-dev, - iova:%08lx pfn:%08lx asid:%d\n, iova, pfn, as-asid); + iova:%08llx pfn:%08lx asid:%d\n, (u64)iova, pfn, as-asid); spin_unlock_irqrestore(as-lock, flags); return PFN_PHYS(pfn); -- 1.8.1.2 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 2/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-...@lists.ozlabs.org; > linux-kernel@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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 2/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-...@lists.ozlabs.org; linux-kernel@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 2/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-...@lists.ozlabs.org; > linux-kernel@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 > > --- > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-kernel@vger.kernel.org; io...@lists.linux-foundation.org; > Wood Scott-B07421; linuxppc-...@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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-kernel@vger.kernel.org; io...@lists.linux-foundation.org; Wood Scott-B07421; linuxppc-...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 2/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-...@lists.ozlabs.org; linux-kernel@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 2/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-kernel@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 > > --- > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 2/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-kernel@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-...@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 > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-...@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-...@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-...@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 > > > > > becom
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-...@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-...@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
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-...@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
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-...@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
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-...@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-...@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
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-...@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-...@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-...@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-...@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_b
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-...@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 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-...@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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 2/5 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-...@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 > > This patch needs to be Acked by the PPC maintainers. > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-...@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 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-...@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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 2/5 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-...@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. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-kernel@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-...@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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-kernel@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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-kernel@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(>dev, "ioremap of PAMU node failed\n"); > > + return -ENOMEM; > > + } > > + of_get_address(pdev->dev.of_node, 0, , NULL); > > + > > + irq = irq_of_parse_and_map(pdev->dev.of_node, 0); > > + if (irq == NO_IRQ) { > > + dev_warn(>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(>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(>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); >
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-kernel@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-kernel@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-kernel@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-...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 2/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-...@lists.ozlabs.org; linux-kernel@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 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 2/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-...@lists.ozlabs.org; linux-kernel@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 6/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-...@lists.ozlabs.org; > linux-kernel@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 > 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 = > > + _domain->win_arr[0]; > > + struct iommu_domain_geometry *geom; > > + > > + geom = _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 = _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(>devices); > > + > > + spin_lock_init(>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(>link); > > + spin_lock(_lock); > > + if (win_cnt) > > + pamu_free_subwins(info->liodn); > > + pamu_disable_liodn(info->liodn); > > + spin_unlock(_lock); > > + spin_lock(_domain_lock); > > + info->dev->archdata.iommu_domain = NULL; > > + kmem_cache_free(iommu_devinfo_cache, info); > > + spin_unlock(_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(_domain->devices
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-...@lists.ozlabs.org; linux-kernel@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-...@lists.ozlabs.org list; linux-kernel@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-...@lists.ozlabs.org list; > > linux-kernel@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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-...@lists.ozlabs.org list; linux-kernel@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-...@lists.ozlabs.org list; linux-kernel@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-...@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-...@lists.ozlabs.org; > >>> linux-kernel@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 > >>>> --- > >>>> - 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-...@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-...@lists.ozlabs.org; linux-kernel@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-...@lists.ozlabs.org; > linux-kernel@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 > > --- > > - 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 > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 6/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-...@lists.ozlabs.org; > linux-kernel@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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 2/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-kernel@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 > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 2/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-kernel@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 6/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-...@lists.ozlabs.org; linux-kernel@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-...@lists.ozlabs.org; linux-kernel@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 2/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-...@lists.ozlabs.org; > linux-kernel@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 > 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 > > --- > > 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 = >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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-...@lists.ozlabs.org; > linux-kernel@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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 2/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-...@lists.ozlabs.org; linux-kernel@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 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-...@lists.ozlabs.org; linux-kernel@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 6/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-...@lists.ozlabs.org; > linux-kernel@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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 6/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-...@lists.ozlabs.org; linux-kernel@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 6/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-...@lists.ozlabs.org; > linux-kernel@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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 6/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-...@lists.ozlabs.org; linux-kernel@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 4/5] iommu: Add domain window handling functions
Thanks Joerg. > -Original Message- > From: Joerg Roedel [mailto:j...@8bytes.org] > Sent: Wednesday, February 06, 2013 3:56 PM > To: Stuart Yoder > Cc: Sethi Varun-B16395; io...@lists.linux-foundation.org; linux- > ker...@vger.kernel.org > Subject: Re: [PATCH 4/5] iommu: Add domain window handling functions > > On Mon, Feb 04, 2013 at 05:31:51PM -0600, Stuart Yoder wrote: > > On Mon, Feb 4, 2013 at 12:56 PM, Joerg Roedel wrote: > > > Okay. So if your architecture supports sizes over 2^32 then size_t > > > probably is already 64bits, right? > > > > No, on a 32-bit platform size_t would generally be 32-bits. But the > PAMU > > is independent of that. I think we should just make it a u64. > > Okay, you can find the patches in the 'core' branch of the iommu-tree. > They are also included in my next branch. You can use them now to base > the PAMU patches on it. > > > Joerg > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 4/5] iommu: Add domain window handling functions
Thanks Joerg. -Original Message- From: Joerg Roedel [mailto:j...@8bytes.org] Sent: Wednesday, February 06, 2013 3:56 PM To: Stuart Yoder Cc: Sethi Varun-B16395; io...@lists.linux-foundation.org; linux- ker...@vger.kernel.org Subject: Re: [PATCH 4/5] iommu: Add domain window handling functions On Mon, Feb 04, 2013 at 05:31:51PM -0600, Stuart Yoder wrote: On Mon, Feb 4, 2013 at 12:56 PM, Joerg Roedel j...@8bytes.org wrote: Okay. So if your architecture supports sizes over 2^32 then size_t probably is already 64bits, right? No, on a 32-bit platform size_t would generally be 32-bits. But the PAMU is independent of that. I think we should just make it a u64. Okay, you can find the patches in the 'core' branch of the iommu-tree. They are also included in my next branch. You can use them now to base the PAMU patches on it. Joerg -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 5/5] iommu: Add DOMAIN_ATTR_WINDOWS domain attribute
> -Original Message- > From: Joerg Roedel [mailto:j...@8bytes.org] > Sent: Tuesday, February 05, 2013 6:36 PM > To: Sethi Varun-B16395 > Cc: Stuart Yoder; io...@lists.linux-foundation.org; linux- > ker...@vger.kernel.org; Wood Scott-B07421 > Subject: Re: [PATCH 5/5] iommu: Add DOMAIN_ATTR_WINDOWS domain attribute > > On Tue, Feb 05, 2013 at 10:52:03AM +, Sethi Varun-B16395 wrote: > > DOMAIN_ATTR_PAMU_ENABLE is required to enable a particular DMA window. > > My point is about the domain geometry, which is incomplete in case of > > PAMU without the number of subwindows. Geometry parameters are used > > for initializing the PAMU window settings. Individual subwindows can > > only be enabled, once the PAMU window has been initialized using the > > geometry settings. > > I understand that. What you do is: > > /* Set geometry */ > set_attr(domain, DOMAIN_ATTR_GEOMETRY); > > /* Set number of PAMU subwindows */ > set_attr(domain, DOMAIN_ATTR_WINDOWS); > > /* Commit changes to hardware and enable the window */ > set_attr(domain, DOMAIN_ATTR_PAMU_ENABLE); > > And I don't see any problem with that. The domain_attr interface was > introduced to cope with device specifics, we don't change global > interface data structures for that. > > [Sethi Varun-B16395] But, then it would be mandatory to set the DOMAIN_ATTR_WINDOWS attribute in case of PAMU. I guess that it would be implementation specific. I am fine with the API. -Varun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 5/5] iommu: Add DOMAIN_ATTR_WINDOWS domain attribute
DOMAIN_ATTR_PAMU_ENABLE is required to enable a particular DMA window. My point is about the domain geometry, which is incomplete in case of PAMU without the number of subwindows. Geometry parameters are used for initializing the PAMU window settings. Individual subwindows can only be enabled, once the PAMU window has been initialized using the geometry settings. Regards Varun > -Original Message- > From: Joerg Roedel [mailto:j...@8bytes.org] > Sent: Tuesday, February 05, 2013 4:12 PM > To: Sethi Varun-B16395 > Cc: Stuart Yoder; io...@lists.linux-foundation.org; linux- > ker...@vger.kernel.org; Wood Scott-B07421 > Subject: Re: [PATCH 5/5] iommu: Add DOMAIN_ATTR_WINDOWS domain attribute > > Hi, > > On Tue, Feb 05, 2013 at 09:14:40AM +, Sethi Varun-B16395 wrote: > > In case of PAMU, geometry would be meaningless without specifying the > > number of subwindows. If the API user specifies a geometry but fails > > to specify the number of required subwindows, we would assume a > > default (max supported) value, which may be incorrect. It's going to > > be really ugly. I think we should add the number of sub windows to the > > domain geometry. This parameter would be used in case of a window > > based iommu. So, a separate set_window API wouldn't be required. > > I think this can all be implemented with the interface proposed here > together with a PAMU-specific DOMAIN_ATTR_PAMU_ENABLE attribute. Stuart > outlined in his RFC mail how this can be done. I don't consider this > ugly, it is rather a clean tradeoff between putting functionality into > the generic part of the IOMMU-API and putting it into the PAMU specific > part. > > Regards, > > Joerg > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 5/5] iommu: Add DOMAIN_ATTR_WINDOWS domain attribute
Hi Joerg, In case of PAMU, geometry would be meaningless without specifying the number of subwindows. If the API user specifies a geometry but fails to specify the number of required subwindows, we would assume a default (max supported) value, which may be incorrect. It's going to be really ugly. I think we should add the number of sub windows to the domain geometry. This parameter would be used in case of a window based iommu. So, a separate set_window API wouldn't be required. We can still implement the get_windows API for getting the maximum number of subwindows supported by the window based IOMMU. Regards Varun > -Original Message- > From: Joerg Roedel [mailto:j...@8bytes.org] > Sent: Monday, February 04, 2013 6:49 PM > To: Sethi Varun-B16395; Stuart Yoder > Cc: io...@lists.linux-foundation.org; linux-kernel@vger.kernel.org; Joerg > Roedel > Subject: [PATCH 5/5] iommu: Add DOMAIN_ATTR_WINDOWS domain attribute > > This attribute can be used to set and get the number of subwindows on > IOMMUs that are window-based. > > Signed-off-by: Joerg Roedel > --- > drivers/iommu/iommu.c | 33 ++--- > include/linux/iommu.h |5 + > 2 files changed, 35 insertions(+), 3 deletions(-) > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index > 0fdb7db..66402f7 100644 > --- a/drivers/iommu/iommu.c > +++ b/drivers/iommu/iommu.c > @@ -891,6 +891,7 @@ int iommu_domain_get_attr(struct iommu_domain > *domain, > struct iommu_domain_geometry *geometry; > bool *paging; > int ret = 0; > + u32 *count; > > switch (attr) { > case DOMAIN_ATTR_GEOMETRY: > @@ -902,6 +903,15 @@ int iommu_domain_get_attr(struct iommu_domain > *domain, > paging = data; > *paging = (domain->ops->pgsize_bitmap != 0UL); > break; > + case DOMAIN_ATTR_WINDOWS: > + count = data; > + > + if (domain->ops->domain_get_windows != NULL) > + *count = domain->ops->domain_get_windows(domain); > + else > + ret = -ENODEV; > + > + break; > default: > if (!domain->ops->domain_get_attr) > return -EINVAL; > @@ -916,9 +926,26 @@ EXPORT_SYMBOL_GPL(iommu_domain_get_attr); > int iommu_domain_set_attr(struct iommu_domain *domain, > enum iommu_attr attr, void *data) > { > - if (!domain->ops->domain_set_attr) > - return -EINVAL; > + int ret = 0; > + u32 *count; > + > + switch (attr) { > + case DOMAIN_ATTR_WINDOWS: > + count = data; > + > + if (domain->ops->domain_set_windows != NULL) > + ret = domain->ops->domain_set_windows(domain, *count); > + else > + ret = -ENODEV; > > - return domain->ops->domain_set_attr(domain, attr, data); > + break; > + default: > + if (domain->ops->domain_set_attr == NULL) > + return -EINVAL; > + > + ret = domain->ops->domain_set_attr(domain, attr, data); > + } > + > + return ret; > } > EXPORT_SYMBOL_GPL(iommu_domain_set_attr); > diff --git a/include/linux/iommu.h b/include/linux/iommu.h index > 0cba2d8..8330df1 100644 > --- a/include/linux/iommu.h > +++ b/include/linux/iommu.h > @@ -60,6 +60,7 @@ struct iommu_domain { > enum iommu_attr { > DOMAIN_ATTR_GEOMETRY, > DOMAIN_ATTR_PAGING, > + DOMAIN_ATTR_WINDOWS, > DOMAIN_ATTR_MAX, > }; > > @@ -106,6 +107,10 @@ struct iommu_ops { > int (*domain_window_enable)(struct iommu_domain *domain, u32 > wnd_nr, > phys_addr_t paddr, size_t size); > void (*domain_window_disable)(struct iommu_domain *domain, u32 > wnd_nr); > + /* Set the numer of window per domain */ > + int (*domain_set_windows)(struct iommu_domain *domain, u32 > w_count); > + /* Get the numer of window per domain */ > + u32 (*domain_get_windows)(struct iommu_domain *domain); > > unsigned long pgsize_bitmap; > }; > -- > 1.7.9.5 > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 5/5] iommu: Add DOMAIN_ATTR_WINDOWS domain attribute
Hi Joerg, In case of PAMU, geometry would be meaningless without specifying the number of subwindows. If the API user specifies a geometry but fails to specify the number of required subwindows, we would assume a default (max supported) value, which may be incorrect. It's going to be really ugly. I think we should add the number of sub windows to the domain geometry. This parameter would be used in case of a window based iommu. So, a separate set_window API wouldn't be required. We can still implement the get_windows API for getting the maximum number of subwindows supported by the window based IOMMU. Regards Varun -Original Message- From: Joerg Roedel [mailto:j...@8bytes.org] Sent: Monday, February 04, 2013 6:49 PM To: Sethi Varun-B16395; Stuart Yoder Cc: io...@lists.linux-foundation.org; linux-kernel@vger.kernel.org; Joerg Roedel Subject: [PATCH 5/5] iommu: Add DOMAIN_ATTR_WINDOWS domain attribute This attribute can be used to set and get the number of subwindows on IOMMUs that are window-based. Signed-off-by: Joerg Roedel j...@8bytes.org --- drivers/iommu/iommu.c | 33 ++--- include/linux/iommu.h |5 + 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 0fdb7db..66402f7 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -891,6 +891,7 @@ int iommu_domain_get_attr(struct iommu_domain *domain, struct iommu_domain_geometry *geometry; bool *paging; int ret = 0; + u32 *count; switch (attr) { case DOMAIN_ATTR_GEOMETRY: @@ -902,6 +903,15 @@ int iommu_domain_get_attr(struct iommu_domain *domain, paging = data; *paging = (domain-ops-pgsize_bitmap != 0UL); break; + case DOMAIN_ATTR_WINDOWS: + count = data; + + if (domain-ops-domain_get_windows != NULL) + *count = domain-ops-domain_get_windows(domain); + else + ret = -ENODEV; + + break; default: if (!domain-ops-domain_get_attr) return -EINVAL; @@ -916,9 +926,26 @@ EXPORT_SYMBOL_GPL(iommu_domain_get_attr); int iommu_domain_set_attr(struct iommu_domain *domain, enum iommu_attr attr, void *data) { - if (!domain-ops-domain_set_attr) - return -EINVAL; + int ret = 0; + u32 *count; + + switch (attr) { + case DOMAIN_ATTR_WINDOWS: + count = data; + + if (domain-ops-domain_set_windows != NULL) + ret = domain-ops-domain_set_windows(domain, *count); + else + ret = -ENODEV; - return domain-ops-domain_set_attr(domain, attr, data); + break; + default: + if (domain-ops-domain_set_attr == NULL) + return -EINVAL; + + ret = domain-ops-domain_set_attr(domain, attr, data); + } + + return ret; } EXPORT_SYMBOL_GPL(iommu_domain_set_attr); diff --git a/include/linux/iommu.h b/include/linux/iommu.h index 0cba2d8..8330df1 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -60,6 +60,7 @@ struct iommu_domain { enum iommu_attr { DOMAIN_ATTR_GEOMETRY, DOMAIN_ATTR_PAGING, + DOMAIN_ATTR_WINDOWS, DOMAIN_ATTR_MAX, }; @@ -106,6 +107,10 @@ struct iommu_ops { int (*domain_window_enable)(struct iommu_domain *domain, u32 wnd_nr, phys_addr_t paddr, size_t size); void (*domain_window_disable)(struct iommu_domain *domain, u32 wnd_nr); + /* Set the numer of window per domain */ + int (*domain_set_windows)(struct iommu_domain *domain, u32 w_count); + /* Get the numer of window per domain */ + u32 (*domain_get_windows)(struct iommu_domain *domain); unsigned long pgsize_bitmap; }; -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/