The first patch fixes a build failure, when we try to build for a Freescale
platform without PCI support.
The second patch enables a default DMA window for the device, once it's
detached from a domain. In case of vfio, once device is detached from a
guest it can be again used by the host.
The last patch adds the maintainer entry for the Freescale PAMU driver.
Varun Sethi (3):
iommu/fsl: Factor out PCI specific code.
iommu/fsl: Enable default DMA window for PCIe devices once detached
Add maintainers entry for the Freescale PAMU driver.
MAINTAINERS | 7 +++
drivers/iommu/fsl_pamu.c | 43 ++++++++++++---
drivers/iommu/fsl_pamu.h | 1 +
drivers/iommu/fsl_pamu_domain.c | 116 +++++++++++++++++++++++++--------------
4 files changed, 117 insertions(+), 50 deletions(-)
--
1.7.9.5
Factor out PCI specific code in the PAMU driver.
Signed-off-by: Varun Sethi <[email protected]>
---
drivers/iommu/fsl_pamu_domain.c | 81 +++++++++++++++++++--------------------
1 file changed, 40 insertions(+), 41 deletions(-)
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index c857c30..e02e1de 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -677,13 +677,9 @@ static int handle_attach_device(struct fsl_dma_domain *dma_domain,
return ret;
}
-static int fsl_pamu_attach_device(struct iommu_domain *domain,
- struct device *dev)
+static void check_for_pci_dma_device(struct device **dev)
{
- struct fsl_dma_domain *dma_domain = domain->priv;
- const u32 *liodn;
- u32 liodn_cnt;
- int len, ret = 0;
+#ifdef CONFIG_PCI
struct pci_dev *pdev = NULL;
struct pci_controller *pci_ctl;
@@ -691,25 +687,38 @@ static int fsl_pamu_attach_device(struct iommu_domain *domain,
* Use LIODN of the PCI controller while attaching a
* PCI device.
*/
- if (dev->bus == &pci_bus_type) {
- pdev = to_pci_dev(dev);
+ if ((*dev)->bus == &pci_bus_type) {
+ pdev = to_pci_dev(*dev);
pci_ctl = pci_bus_to_host(pdev->bus);
/*
* make dev point to pci controller device
* so we can get the LIODN programmed by
* u-boot.
*/
- dev = pci_ctl->parent;
+ *dev = pci_ctl->parent;
}
+#endif
+}
- liodn = of_get_property(dev->of_node, "fsl,liodn", &len);
+static int fsl_pamu_attach_device(struct iommu_domain *domain,
+ struct device *dev)
+{
+ struct fsl_dma_domain *dma_domain = domain->priv;
+ struct device *dma_dev = dev;
+ const u32 *liodn;
+ u32 liodn_cnt;
+ int len, ret = 0;
+
+ check_for_pci_dma_device(&dma_dev);
+
+ liodn = of_get_property(dma_dev->of_node, "fsl,liodn", &len);
if (liodn) {
liodn_cnt = len / sizeof(u32);
ret = handle_attach_device(dma_domain, dev,
liodn, liodn_cnt);
} else {
pr_debug("missing fsl,liodn property at %s\n",
- dev->of_node->full_name);
+ dma_dev->of_node->full_name);
ret = -EINVAL;
}
@@ -720,32 +729,18 @@ static void fsl_pamu_detach_device(struct iommu_domain *domain,
struct device *dev)
{
struct fsl_dma_domain *dma_domain = domain->priv;
+ struct device *dma_dev = dev;
const u32 *prop;
int len;
- struct pci_dev *pdev = NULL;
- struct pci_controller *pci_ctl;
- /*
- * Use LIODN of the PCI controller while detaching a
- * PCI device.
- */
- if (dev->bus == &pci_bus_type) {
- pdev = to_pci_dev(dev);
- pci_ctl = pci_bus_to_host(pdev->bus);
- /*
- * make dev point to pci controller device
- * so we can get the LIODN programmed by
- * u-boot.
- */
- dev = pci_ctl->parent;
- }
+ check_for_pci_dma_device(&dma_dev);
- prop = of_get_property(dev->of_node, "fsl,liodn", &len);
+ prop = of_get_property(dma_dev->of_node, "fsl,liodn", &len);
if (prop)
detach_device(dev, dma_domain);
else
pr_debug("missing fsl,liodn property at %s\n",
- dev->of_node->full_name);
+ dma_dev->of_node->full_name);
}
static int configure_domain_geometry(struct iommu_domain *domain, void *data)
@@ -905,6 +900,7 @@ static struct iommu_group *get_device_iommu_group(struct device *dev)
return group;
}
+#ifdef CONFIG_PCI
static bool check_pci_ctl_endpt_part(struct pci_controller *pci_ctl)
{
u32 version;
@@ -945,13 +941,18 @@ static struct iommu_group *get_shared_pci_device_group(struct pci_dev *pdev)
return NULL;
}
-static struct iommu_group *get_pci_device_group(struct pci_dev *pdev)
+static struct iommu_group *get_pci_device_group(struct device *dev)
{
struct pci_controller *pci_ctl;
bool pci_endpt_partioning;
struct iommu_group *group = NULL;
- struct pci_dev *bridge, *dma_pdev = NULL;
+ struct pci_dev *bridge, *pdev;
+ struct pci_dev *dma_pdev = NULL;
+ pdev = to_pci_dev(dev);
+ /* Don't create device groups for virtual PCI bridges */
+ if (pdev->subordinate)
+ return NULL;
pci_ctl = pci_bus_to_host(pdev->bus);
pci_endpt_partioning = check_pci_ctl_endpt_part(pci_ctl);
/* We can partition PCIe devices so assign device group to the device */
@@ -1044,11 +1045,11 @@ root_bus:
return group;
}
+#endif
static int fsl_pamu_add_device(struct device *dev)
{
struct iommu_group *group = NULL;
- struct pci_dev *pdev;
const u32 *prop;
int ret, len;
@@ -1056,19 +1057,15 @@ static int fsl_pamu_add_device(struct device *dev)
* For platform devices we allocate a separate group for
* each of the devices.
*/
- if (dev->bus == &pci_bus_type) {
- pdev = to_pci_dev(dev);
- /* Don't create device groups for virtual PCI bridges */
- if (pdev->subordinate)
- return 0;
-
- group = get_pci_device_group(pdev);
-
- } else {
+ if (dev->bus == &platform_bus_type) {
prop = of_get_property(dev->of_node, "fsl,liodn", &len);
if (prop)
group = get_device_iommu_group(dev);
}
+#ifdef CONFIG_PCI
+ else
+ group = get_pci_device_group(dev);
+#endif
if (!group || IS_ERR(group))
return PTR_ERR(group);
@@ -1166,7 +1163,9 @@ int pamu_domain_init()
return ret;
bus_set_iommu(&platform_bus_type, &fsl_pamu_ops);
+#ifdef CONFIG_PCI
bus_set_iommu(&pci_bus_type, &fsl_pamu_ops);
+#endif
return ret;
}
--
1.7.9.5
from domain.
Once the PCIe device assigned to a guest VM (via VFIO) gets detached from the iommu domain
(when guest terminates), its PAMU table entry is disabled. So, this would prevent the device
from being used once it's assigned back to the host.
This patch allows for creation of a default DMA window corresponding to the device
and subsequently enabling the PAMU table entry. Before we enable the entry, we ensure that
the device's bus master capability is disabled (device quiesced).
Signed-off-by: Varun Sethi <[email protected]>
---
drivers/iommu/fsl_pamu.c | 43 +++++++++++++++++++++++++++++++--------
drivers/iommu/fsl_pamu.h | 1 +
drivers/iommu/fsl_pamu_domain.c | 35 +++++++++++++++++++++++++++++++
3 files changed, 70 insertions(+), 9 deletions(-)
diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c
index cba0498..fb4a031 100644
--- a/drivers/iommu/fsl_pamu.c
+++ b/drivers/iommu/fsl_pamu.c
@@ -225,6 +225,21 @@ static struct paace *pamu_get_spaace(struct paace *paace, u32 wnum)
return spaace;
}
+/*
+ * Defaul PPAACE settings for an LIODN.
+ */
+static void setup_default_ppaace(struct paace *ppaace)
+{
+ pamu_init_ppaace(ppaace);
+ /* window size is 2^(WSE+1) bytes */
+ set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE, 35);
+ ppaace->wbah = 0;
+ set_bf(ppaace->addr_bitfields, PPAACE_AF_WBAL, 0);
+ set_bf(ppaace->impl_attr, PAACE_IA_ATM,
+ PAACE_ATM_NO_XLATE);
+ set_bf(ppaace->addr_bitfields, PAACE_AF_AP,
+ PAACE_AP_PERMS_ALL);
+}
/**
* pamu_get_fspi_and_allocate() - Allocates fspi index and reserves subwindows
* required for primary PAACE in the secondary
@@ -253,6 +268,24 @@ static unsigned long pamu_get_fspi_and_allocate(u32 subwin_cnt)
return (spaace_addr - (unsigned long)spaact) / (sizeof(struct paace));
}
+/* Reset the PAACE entry to the default state */
+void enable_default_dma_window(int liodn)
+{
+ struct paace *ppaace;
+
+ ppaace = pamu_get_ppaace(liodn);
+ if (!ppaace) {
+ pr_debug("Invalid liodn entry\n");
+ return;
+ }
+
+ memset(ppaace, 0, sizeof(struct paace));
+
+ setup_default_ppaace(ppaace);
+ mb();
+ pamu_enable_liodn(liodn);
+}
+
/* Release the subwindows reserved for a particular LIODN */
void pamu_free_subwins(int liodn)
{
@@ -752,15 +785,7 @@ static void __init setup_liodns(void)
continue;
}
ppaace = pamu_get_ppaace(liodn);
- pamu_init_ppaace(ppaace);
- /* window size is 2^(WSE+1) bytes */
- set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE, 35);
- ppaace->wbah = 0;
- set_bf(ppaace->addr_bitfields, PPAACE_AF_WBAL, 0);
- set_bf(ppaace->impl_attr, PAACE_IA_ATM,
- PAACE_ATM_NO_XLATE);
- set_bf(ppaace->addr_bitfields, PAACE_AF_AP,
- PAACE_AP_PERMS_ALL);
+ setup_default_ppaace(ppaace);
if (of_device_is_compatible(node, "fsl,qman-portal"))
setup_qbman_paace(ppaace, QMAN_PORTAL_PAACE);
if (of_device_is_compatible(node, "fsl,qman"))
diff --git a/drivers/iommu/fsl_pamu.h b/drivers/iommu/fsl_pamu.h
index 8fc1a12..0edcbbbb 100644
--- a/drivers/iommu/fsl_pamu.h
+++ b/drivers/iommu/fsl_pamu.h
@@ -406,5 +406,6 @@ void get_ome_index(u32 *omi_index, struct device *dev);
int pamu_update_paace_stash(int liodn, u32 subwin, u32 value);
int pamu_disable_spaace(int liodn, u32 subwin);
u32 pamu_get_max_subwin_cnt(void);
+void enable_default_dma_window(int liodn);
#endif /* __FSL_PAMU_H */
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index e02e1de..553ef3c 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -340,6 +340,40 @@ static inline struct device_domain_info *find_domain(struct device *dev)
return dev->archdata.iommu_domain;
}
+/* Disable device DMA capability and enable default DMA window */
+static void disable_device_dma(struct device_domain_info *info)
+{
+ struct device_domain_info *tmp;
+ int enable_dma_window = 1;
+
+#ifdef CONFIG_PCI
+ if (info->dev->bus == &pci_bus_type) {
+ struct pci_dev *pdev = NULL;
+ pdev = to_pci_dev(info->dev);
+ if (pci_is_enabled(pdev))
+ pci_disable_device(pdev);
+ }
+#endif
+ /*
+ * Sanity check, to ensure that this is not a
+ * shared LIODN. In case of a PCIe controller
+ * it's possible that all PCIe devices share
+ * the same LIODN. We can't enable the default
+ * DMA window till all the devices have been
+ * quiesced (for PCIe devices, we explicitly
+ * disable the bus master capability).
+ */
+ list_for_each_entry(tmp, &info->domain->devices, link) {
+ if (info->dev->iommu_group == tmp->dev->iommu_group) {
+ enable_dma_window = 0;
+ break;
+ }
+ }
+
+ if (enable_dma_window)
+ enable_default_dma_window(info->liodn);
+}
+
static void remove_device_ref(struct device_domain_info *info, u32 win_cnt)
{
unsigned long flags;
@@ -351,6 +385,7 @@ static void remove_device_ref(struct device_domain_info *info, u32 win_cnt)
pamu_disable_liodn(info->liodn);
spin_unlock_irqrestore(&iommu_lock, flags);
spin_lock_irqsave(&device_domain_lock, flags);
+ disable_device_dma(info);
info->dev->archdata.iommu_domain = NULL;
kmem_cache_free(iommu_devinfo_cache, info);
spin_unlock_irqrestore(&device_domain_lock, flags);
--
1.7.9.5
Signed-off-by: Varun Sethi <[email protected]>
---
MAINTAINERS | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 8a0cbf3..5b6ea5c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3511,6 +3511,13 @@ S: Maintained
F: drivers/net/ethernet/freescale/fs_enet/
F: include/linux/fs_enet_pd.h
+FREESCALE PAMU DRIVER
+M: Varun Sethi <[email protected]>
+L: [email protected]
+L: [email protected]
+S: Maintained
+F: drivers/iommu/fsl_pamu*
+
FREESCALE QUICC ENGINE LIBRARY
L: [email protected]
S: Orphan
--
1.7.9.5
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 <[email protected]>
> ---
> 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?
> {
> - 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:
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?
Bjorn
>
> @@ -691,25 +687,38 @@ static int fsl_pamu_attach_device(struct iommu_domain *domain,
> * Use LIODN of the PCI controller while attaching a
> * PCI device.
> */
> - if (dev->bus == &pci_bus_type) {
> - pdev = to_pci_dev(dev);
> + if ((*dev)->bus == &pci_bus_type) {
> + pdev = to_pci_dev(*dev);
> pci_ctl = pci_bus_to_host(pdev->bus);
> /*
> * make dev point to pci controller device
> * so we can get the LIODN programmed by
> * u-boot.
> */
> - dev = pci_ctl->parent;
> + *dev = pci_ctl->parent;
> }
> +#endif
> +}
>
> - liodn = of_get_property(dev->of_node, "fsl,liodn", &len);
> +static int fsl_pamu_attach_device(struct iommu_domain *domain,
> + struct device *dev)
> +{
> + struct fsl_dma_domain *dma_domain = domain->priv;
> + struct device *dma_dev = dev;
> + const u32 *liodn;
> + u32 liodn_cnt;
> + int len, ret = 0;
> +
> + check_for_pci_dma_device(&dma_dev);
> +
> + liodn = of_get_property(dma_dev->of_node, "fsl,liodn", &len);
> if (liodn) {
> liodn_cnt = len / sizeof(u32);
> ret = handle_attach_device(dma_domain, dev,
> liodn, liodn_cnt);
> } else {
> pr_debug("missing fsl,liodn property at %s\n",
> - dev->of_node->full_name);
> + dma_dev->of_node->full_name);
> ret = -EINVAL;
> }
>
> @@ -720,32 +729,18 @@ static void fsl_pamu_detach_device(struct iommu_domain *domain,
> struct device *dev)
> {
> struct fsl_dma_domain *dma_domain = domain->priv;
> + struct device *dma_dev = dev;
> const u32 *prop;
> int len;
> - struct pci_dev *pdev = NULL;
> - struct pci_controller *pci_ctl;
>
> - /*
> - * Use LIODN of the PCI controller while detaching a
> - * PCI device.
> - */
> - if (dev->bus == &pci_bus_type) {
> - pdev = to_pci_dev(dev);
> - pci_ctl = pci_bus_to_host(pdev->bus);
> - /*
> - * make dev point to pci controller device
> - * so we can get the LIODN programmed by
> - * u-boot.
> - */
> - dev = pci_ctl->parent;
> - }
> + check_for_pci_dma_device(&dma_dev);
>
> - prop = of_get_property(dev->of_node, "fsl,liodn", &len);
> + prop = of_get_property(dma_dev->of_node, "fsl,liodn", &len);
> if (prop)
> detach_device(dev, dma_domain);
> else
> pr_debug("missing fsl,liodn property at %s\n",
> - dev->of_node->full_name);
> + dma_dev->of_node->full_name);
> }
>
> static int configure_domain_geometry(struct iommu_domain *domain, void *data)
> @@ -905,6 +900,7 @@ static struct iommu_group *get_device_iommu_group(struct device *dev)
> return group;
> }
>
> +#ifdef CONFIG_PCI
> static bool check_pci_ctl_endpt_part(struct pci_controller *pci_ctl)
> {
> u32 version;
> @@ -945,13 +941,18 @@ static struct iommu_group *get_shared_pci_device_group(struct pci_dev *pdev)
> return NULL;
> }
>
> -static struct iommu_group *get_pci_device_group(struct pci_dev *pdev)
> +static struct iommu_group *get_pci_device_group(struct device *dev)
> {
> struct pci_controller *pci_ctl;
> bool pci_endpt_partioning;
> struct iommu_group *group = NULL;
> - struct pci_dev *bridge, *dma_pdev = NULL;
> + struct pci_dev *bridge, *pdev;
> + struct pci_dev *dma_pdev = NULL;
>
> + pdev = to_pci_dev(dev);
> + /* Don't create device groups for virtual PCI bridges */
> + if (pdev->subordinate)
> + return NULL;
> pci_ctl = pci_bus_to_host(pdev->bus);
> pci_endpt_partioning = check_pci_ctl_endpt_part(pci_ctl);
> /* We can partition PCIe devices so assign device group to the device */
> @@ -1044,11 +1045,11 @@ root_bus:
>
> return group;
> }
> +#endif
>
> static int fsl_pamu_add_device(struct device *dev)
> {
> struct iommu_group *group = NULL;
> - struct pci_dev *pdev;
> const u32 *prop;
> int ret, len;
>
> @@ -1056,19 +1057,15 @@ static int fsl_pamu_add_device(struct device *dev)
> * For platform devices we allocate a separate group for
> * each of the devices.
> */
> - if (dev->bus == &pci_bus_type) {
> - pdev = to_pci_dev(dev);
> - /* Don't create device groups for virtual PCI bridges */
> - if (pdev->subordinate)
> - return 0;
> -
> - group = get_pci_device_group(pdev);
> -
> - } else {
> + if (dev->bus == &platform_bus_type) {
> prop = of_get_property(dev->of_node, "fsl,liodn", &len);
> if (prop)
> group = get_device_iommu_group(dev);
> }
> +#ifdef CONFIG_PCI
> + else
> + group = get_pci_device_group(dev);
> +#endif
>
> if (!group || IS_ERR(group))
> return PTR_ERR(group);
> @@ -1166,7 +1163,9 @@ int pamu_domain_init()
> return ret;
>
> bus_set_iommu(&platform_bus_type, &fsl_pamu_ops);
> +#ifdef CONFIG_PCI
> bus_set_iommu(&pci_bus_type, &fsl_pamu_ops);
> +#endif
>
> return ret;
> }
> --
> 1.7.9.5
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
> -----Original Message-----
> From: [email protected] [mailto:iommu-
> [email protected]] On Behalf Of Bjorn Helgaas
> Sent: Tuesday, October 15, 2013 5:46 AM
> To: Sethi Varun-B16395
> Cc: Yoder Stuart-B08248; [email protected]; [email protected]
> foundation.org; Bhushan Bharat-R65777; Wood Scott-B07421; linuxppc-
> [email protected]
> 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 <[email protected]>
> > ---
> > 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