* [PATCH v2 0/9] iommu: Permit modular builds of ARM SMMU[v3] drivers
@ 2019-11-08 15:15 Will Deacon
2019-11-08 15:16 ` [PATCH v2 1/9] drivers/iommu: Export core IOMMU API symbols to permit modular drivers Will Deacon
` (8 more replies)
0 siblings, 9 replies; 18+ messages in thread
From: Will Deacon @ 2019-11-08 15:15 UTC (permalink / raw)
To: iommu, linux-kernel
Cc: Isaac J. Manjarres, Jean-Philippe Brucker, Saravana Kannan,
Robin Murphy, Bjorn Helgaas, Will Deacon
Hi all,
This is version two of the patches I previously posted here:
https://lore.kernel.org/lkml/20191030145112.19738-1-will@kernel.org/
Changes since v1 include:
* Build single "arm-smmu-mod.ko" module for the Arm SMMU driver
* Hold a reference to the IOMMU driver module across {add,remove}_device()
* Take a reference to the IOMMU driver module during of_xlate()
* Added Bjorn's ack on the PCI export patch
Please note that I haven't been able to test this properly, since I don't
currently have access to any Arm SMMU hardware.
Cheers,
Will
Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
Cc: Jordan Crouse <jcrouse@codeaurora.org>
Cc: John Garry <john.garry@huawei.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Saravana Kannan <saravanak@google.com>
Cc: "Isaac J. Manjarres" <isaacm@codeaurora.org>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Joerg Roedel <joro@8bytes.org>
--->8
Will Deacon (9):
drivers/iommu: Export core IOMMU API symbols to permit modular drivers
iommu/of: Request ACS from the PCI core when configuring IOMMU linkage
PCI: Export pci_ats_disabled() as a GPL symbol to modules
drivers/iommu: Take a ref to the IOMMU driver prior to ->add_device()
iommu/of: Take a ref to the IOMMU driver during ->of_xlate()
Revert "iommu/arm-smmu: Make arm-smmu-v3 explicitly non-modular"
iommu/arm-smmu-v3: Allow building as a module
Revert "iommu/arm-smmu: Make arm-smmu explicitly non-modular"
iommu/arm-smmu: Allow building as a module
drivers/iommu/Kconfig | 16 ++++++-
drivers/iommu/Makefile | 3 +-
drivers/iommu/arm-smmu-v3.c | 27 +++++++-----
drivers/iommu/arm-smmu.c | 87 ++++++++++++++++++++++---------------
drivers/iommu/iommu-sysfs.c | 5 +++
drivers/iommu/iommu.c | 27 +++++++++++-
drivers/iommu/of_iommu.c | 17 +++++---
drivers/pci/pci.c | 1 +
include/linux/iommu.h | 2 +
9 files changed, 130 insertions(+), 55 deletions(-)
--
2.24.0.rc1.363.gb1bccd3e3d-goog
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 1/9] drivers/iommu: Export core IOMMU API symbols to permit modular drivers
2019-11-08 15:15 [PATCH v2 0/9] iommu: Permit modular builds of ARM SMMU[v3] drivers Will Deacon
@ 2019-11-08 15:16 ` Will Deacon
2019-11-08 15:16 ` [PATCH v2 2/9] iommu/of: Request ACS from the PCI core when configuring IOMMU linkage Will Deacon
` (7 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Will Deacon @ 2019-11-08 15:16 UTC (permalink / raw)
To: iommu, linux-kernel
Cc: Isaac J. Manjarres, Jean-Philippe Brucker, Saravana Kannan,
Robin Murphy, Bjorn Helgaas, Will Deacon
Building IOMMU drivers as modules requires that the core IOMMU API
symbols are exported as GPL symbols.
Signed-off-by: Will Deacon <will@kernel.org>
---
drivers/iommu/iommu-sysfs.c | 5 +++++
drivers/iommu/iommu.c | 8 ++++++++
2 files changed, 13 insertions(+)
diff --git a/drivers/iommu/iommu-sysfs.c b/drivers/iommu/iommu-sysfs.c
index e436ff813e7e..99869217fbec 100644
--- a/drivers/iommu/iommu-sysfs.c
+++ b/drivers/iommu/iommu-sysfs.c
@@ -87,6 +87,7 @@ int iommu_device_sysfs_add(struct iommu_device *iommu,
put_device(iommu->dev);
return ret;
}
+EXPORT_SYMBOL_GPL(iommu_device_sysfs_add);
void iommu_device_sysfs_remove(struct iommu_device *iommu)
{
@@ -94,6 +95,8 @@ void iommu_device_sysfs_remove(struct iommu_device *iommu)
device_unregister(iommu->dev);
iommu->dev = NULL;
}
+EXPORT_SYMBOL_GPL(iommu_device_sysfs_remove);
+
/*
* IOMMU drivers can indicate a device is managed by a given IOMMU using
* this interface. A link to the device will be created in the "devices"
@@ -119,6 +122,7 @@ int iommu_device_link(struct iommu_device *iommu, struct device *link)
return ret;
}
+EXPORT_SYMBOL_GPL(iommu_device_link);
void iommu_device_unlink(struct iommu_device *iommu, struct device *link)
{
@@ -128,3 +132,4 @@ void iommu_device_unlink(struct iommu_device *iommu, struct device *link)
sysfs_remove_link(&link->kobj, "iommu");
sysfs_remove_link_from_group(&iommu->dev->kobj, "devices", dev_name(link));
}
+EXPORT_SYMBOL_GPL(iommu_device_unlink);
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index d658c7c6a2ab..c1aadb570145 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -141,6 +141,7 @@ int iommu_device_register(struct iommu_device *iommu)
spin_unlock(&iommu_device_lock);
return 0;
}
+EXPORT_SYMBOL_GPL(iommu_device_register);
void iommu_device_unregister(struct iommu_device *iommu)
{
@@ -148,6 +149,7 @@ void iommu_device_unregister(struct iommu_device *iommu)
list_del(&iommu->list);
spin_unlock(&iommu_device_lock);
}
+EXPORT_SYMBOL_GPL(iommu_device_unregister);
static struct iommu_param *iommu_get_dev_param(struct device *dev)
{
@@ -886,6 +888,7 @@ struct iommu_group *iommu_group_ref_get(struct iommu_group *group)
kobject_get(group->devices_kobj);
return group;
}
+EXPORT_SYMBOL_GPL(iommu_group_ref_get);
/**
* iommu_group_put - Decrement group reference
@@ -1259,6 +1262,7 @@ struct iommu_group *generic_device_group(struct device *dev)
{
return iommu_group_alloc();
}
+EXPORT_SYMBOL_GPL(generic_device_group);
/*
* Use standard PCI bus topology, isolation features, and DMA alias quirks
@@ -1326,6 +1330,7 @@ struct iommu_group *pci_device_group(struct device *dev)
/* No shared group found, allocate new */
return iommu_group_alloc();
}
+EXPORT_SYMBOL_GPL(pci_device_group);
/* Get the IOMMU group for device on fsl-mc bus */
struct iommu_group *fsl_mc_device_group(struct device *dev)
@@ -1338,6 +1343,7 @@ struct iommu_group *fsl_mc_device_group(struct device *dev)
group = iommu_group_alloc();
return group;
}
+EXPORT_SYMBOL_GPL(fsl_mc_device_group);
/**
* iommu_group_get_for_dev - Find or create the IOMMU group for a device
@@ -1406,6 +1412,7 @@ struct iommu_group *iommu_group_get_for_dev(struct device *dev)
return group;
}
+EXPORT_SYMBOL(iommu_group_get_for_dev);
struct iommu_domain *iommu_group_default_domain(struct iommu_group *group)
{
@@ -2185,6 +2192,7 @@ struct iommu_resv_region *iommu_alloc_resv_region(phys_addr_t start,
region->type = type;
return region;
}
+EXPORT_SYMBOL_GPL(iommu_alloc_resv_region);
static int
request_default_domain_for_dev(struct device *dev, unsigned long type)
--
2.24.0.rc1.363.gb1bccd3e3d-goog
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 2/9] iommu/of: Request ACS from the PCI core when configuring IOMMU linkage
2019-11-08 15:15 [PATCH v2 0/9] iommu: Permit modular builds of ARM SMMU[v3] drivers Will Deacon
2019-11-08 15:16 ` [PATCH v2 1/9] drivers/iommu: Export core IOMMU API symbols to permit modular drivers Will Deacon
@ 2019-11-08 15:16 ` Will Deacon
2019-11-08 15:16 ` [PATCH v2 3/9] PCI: Export pci_ats_disabled() as a GPL symbol to modules Will Deacon
` (6 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Will Deacon @ 2019-11-08 15:16 UTC (permalink / raw)
To: iommu, linux-kernel
Cc: Isaac J. Manjarres, Jean-Philippe Brucker, Saravana Kannan,
Robin Murphy, Bjorn Helgaas, Will Deacon
To avoid having to export 'pci_request_acs()' to modular IOMMU drivers,
move the call into the 'of_dma_configure()' path in a similar manner to
the way in which ACS is configured when probing via ACPI/IORT.
Signed-off-by: Will Deacon <will@kernel.org>
---
drivers/iommu/of_iommu.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 614a93aa5305..78faa9f73a91 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -177,6 +177,7 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
.np = master_np,
};
+ pci_request_acs();
err = pci_for_each_dma_alias(to_pci_dev(dev),
of_pci_iommu_init, &info);
} else if (dev_is_fsl_mc(dev)) {
--
2.24.0.rc1.363.gb1bccd3e3d-goog
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 3/9] PCI: Export pci_ats_disabled() as a GPL symbol to modules
2019-11-08 15:15 [PATCH v2 0/9] iommu: Permit modular builds of ARM SMMU[v3] drivers Will Deacon
2019-11-08 15:16 ` [PATCH v2 1/9] drivers/iommu: Export core IOMMU API symbols to permit modular drivers Will Deacon
2019-11-08 15:16 ` [PATCH v2 2/9] iommu/of: Request ACS from the PCI core when configuring IOMMU linkage Will Deacon
@ 2019-11-08 15:16 ` Will Deacon
2019-11-08 15:16 ` [PATCH v2 4/9] drivers/iommu: Take a ref to the IOMMU driver prior to ->add_device() Will Deacon
` (5 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Will Deacon @ 2019-11-08 15:16 UTC (permalink / raw)
To: iommu, linux-kernel
Cc: Isaac J. Manjarres, Jean-Philippe Brucker, Saravana Kannan,
Robin Murphy, Bjorn Helgaas, Will Deacon
Building drivers for ATS-aware IOMMUs as modules requires access to
pci_ats_disabled(). Export it as a GPL symbol to get things working.
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
drivers/pci/pci.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index a97e2571a527..4fbe5b576dd8 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -123,6 +123,7 @@ bool pci_ats_disabled(void)
{
return pcie_ats_disabled;
}
+EXPORT_SYMBOL_GPL(pci_ats_disabled);
/* Disable bridge_d3 for all PCIe ports */
static bool pci_bridge_d3_disable;
--
2.24.0.rc1.363.gb1bccd3e3d-goog
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 4/9] drivers/iommu: Take a ref to the IOMMU driver prior to ->add_device()
2019-11-08 15:15 [PATCH v2 0/9] iommu: Permit modular builds of ARM SMMU[v3] drivers Will Deacon
` (2 preceding siblings ...)
2019-11-08 15:16 ` [PATCH v2 3/9] PCI: Export pci_ats_disabled() as a GPL symbol to modules Will Deacon
@ 2019-11-08 15:16 ` Will Deacon
2019-11-08 15:16 ` [PATCH v2 5/9] iommu/of: Take a ref to the IOMMU driver during ->of_xlate() Will Deacon
` (4 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Will Deacon @ 2019-11-08 15:16 UTC (permalink / raw)
To: iommu, linux-kernel
Cc: Isaac J. Manjarres, Jean-Philippe Brucker, Saravana Kannan,
Robin Murphy, Bjorn Helgaas, Will Deacon
To avoid accidental removal of an active IOMMU driver module, take a
reference to the driver module in 'iommu_probe_device()' immediately
prior to invoking the '->add_device()' callback and hold it until the
after the device has been removed by '->remove_device()'.
Suggested-by: Joerg Roedel <joro@8bytes.org>
Signed-off-by: Will Deacon <will@kernel.org>
---
drivers/iommu/iommu.c | 19 +++++++++++++++++--
include/linux/iommu.h | 2 ++
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index c1aadb570145..4bfecfbbe2cf 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -22,6 +22,7 @@
#include <linux/bitops.h>
#include <linux/property.h>
#include <linux/fsl/mc.h>
+#include <linux/module.h>
#include <trace/events/iommu.h>
static struct kset *iommu_group_kset;
@@ -185,10 +186,21 @@ int iommu_probe_device(struct device *dev)
if (!iommu_get_dev_param(dev))
return -ENOMEM;
+ if (!try_module_get(ops->owner)) {
+ ret = -EINVAL;
+ goto err_free_dev_param;
+ }
+
ret = ops->add_device(dev);
if (ret)
- iommu_free_dev_param(dev);
+ goto err_module_put;
+
+ return 0;
+err_module_put:
+ module_put(ops->owner);
+err_free_dev_param:
+ iommu_free_dev_param(dev);
return ret;
}
@@ -199,7 +211,10 @@ void iommu_release_device(struct device *dev)
if (dev->iommu_group)
ops->remove_device(dev);
- iommu_free_dev_param(dev);
+ if (dev->iommu_param) {
+ module_put(ops->owner);
+ iommu_free_dev_param(dev);
+ }
}
static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 29bac5345563..d9dab5a3e912 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -245,6 +245,7 @@ struct iommu_iotlb_gather {
* @sva_get_pasid: Get PASID associated to a SVA handle
* @page_response: handle page request response
* @pgsize_bitmap: bitmap of all possible supported page sizes
+ * @owner: Driver module providing these ops
*/
struct iommu_ops {
bool (*capable)(enum iommu_cap);
@@ -308,6 +309,7 @@ struct iommu_ops {
struct iommu_page_response *msg);
unsigned long pgsize_bitmap;
+ struct module *owner;
};
/**
--
2.24.0.rc1.363.gb1bccd3e3d-goog
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 5/9] iommu/of: Take a ref to the IOMMU driver during ->of_xlate()
2019-11-08 15:15 [PATCH v2 0/9] iommu: Permit modular builds of ARM SMMU[v3] drivers Will Deacon
` (3 preceding siblings ...)
2019-11-08 15:16 ` [PATCH v2 4/9] drivers/iommu: Take a ref to the IOMMU driver prior to ->add_device() Will Deacon
@ 2019-11-08 15:16 ` Will Deacon
2019-11-08 15:16 ` [PATCH v2 6/9] Revert "iommu/arm-smmu: Make arm-smmu-v3 explicitly non-modular" Will Deacon
` (3 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Will Deacon @ 2019-11-08 15:16 UTC (permalink / raw)
To: iommu, linux-kernel
Cc: Isaac J. Manjarres, Jean-Philippe Brucker, Saravana Kannan,
Robin Murphy, Bjorn Helgaas, Will Deacon
Ensure that we hold a reference to the IOMMU driver module while calling
the '->of_xlate()' callback during early device probing.
Signed-off-by: Will Deacon <will@kernel.org>
---
drivers/iommu/of_iommu.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 78faa9f73a91..25491403a0bd 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -8,6 +8,7 @@
#include <linux/export.h>
#include <linux/iommu.h>
#include <linux/limits.h>
+#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_iommu.h>
#include <linux/of_pci.h>
@@ -89,16 +90,16 @@ static int of_iommu_xlate(struct device *dev,
{
const struct iommu_ops *ops;
struct fwnode_handle *fwnode = &iommu_spec->np->fwnode;
- int err;
+ int ret;
ops = iommu_ops_from_fwnode(fwnode);
if ((ops && !ops->of_xlate) ||
!of_device_is_available(iommu_spec->np))
return NO_IOMMU;
- err = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops);
- if (err)
- return err;
+ ret = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops);
+ if (ret)
+ return ret;
/*
* The otherwise-empty fwspec handily serves to indicate the specific
* IOMMU device we're waiting for, which will be useful if we ever get
@@ -107,7 +108,12 @@ static int of_iommu_xlate(struct device *dev,
if (!ops)
return driver_deferred_probe_check_state(dev);
- return ops->of_xlate(dev, iommu_spec);
+ if (!try_module_get(ops->owner))
+ return -ENODEV;
+
+ ret = ops->of_xlate(dev, iommu_spec);
+ module_put(ops->owner);
+ return ret;
}
struct of_pci_iommu_alias_info {
--
2.24.0.rc1.363.gb1bccd3e3d-goog
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 6/9] Revert "iommu/arm-smmu: Make arm-smmu-v3 explicitly non-modular"
2019-11-08 15:15 [PATCH v2 0/9] iommu: Permit modular builds of ARM SMMU[v3] drivers Will Deacon
` (4 preceding siblings ...)
2019-11-08 15:16 ` [PATCH v2 5/9] iommu/of: Take a ref to the IOMMU driver during ->of_xlate() Will Deacon
@ 2019-11-08 15:16 ` Will Deacon
2019-11-08 16:17 ` John Garry
2019-11-08 15:16 ` [PATCH v2 7/9] iommu/arm-smmu-v3: Allow building as a module Will Deacon
` (2 subsequent siblings)
8 siblings, 1 reply; 18+ messages in thread
From: Will Deacon @ 2019-11-08 15:16 UTC (permalink / raw)
To: iommu, linux-kernel
Cc: Isaac J. Manjarres, Jean-Philippe Brucker, Saravana Kannan,
Robin Murphy, Bjorn Helgaas, Will Deacon
This reverts commit c07b6426df922d21a13a959cf785d46e9c531941.
Let's get the SMMUv3 driver building as a module, which means putting
back some dead code that we used to carry.
Signed-off-by: Will Deacon <will@kernel.org>
---
drivers/iommu/arm-smmu-v3.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 8da93e730d6f..2ad8e2ca0583 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -21,8 +21,7 @@
#include <linux/io-pgtable.h>
#include <linux/iommu.h>
#include <linux/iopoll.h>
-#include <linux/init.h>
-#include <linux/moduleparam.h>
+#include <linux/module.h>
#include <linux/msi.h>
#include <linux/of.h>
#include <linux/of_address.h>
@@ -384,10 +383,6 @@
#define MSI_IOVA_BASE 0x8000000
#define MSI_IOVA_LENGTH 0x100000
-/*
- * not really modular, but the easiest way to keep compat with existing
- * bootargs behaviour is to continue using module_param_named here.
- */
static bool disable_bypass = 1;
module_param_named(disable_bypass, disable_bypass, bool, S_IRUGO);
MODULE_PARM_DESC(disable_bypass,
@@ -3683,25 +3678,37 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
return 0;
}
-static void arm_smmu_device_shutdown(struct platform_device *pdev)
+static int arm_smmu_device_remove(struct platform_device *pdev)
{
struct arm_smmu_device *smmu = platform_get_drvdata(pdev);
arm_smmu_device_disable(smmu);
+
+ return 0;
+}
+
+static void arm_smmu_device_shutdown(struct platform_device *pdev)
+{
+ arm_smmu_device_remove(pdev);
}
static const struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "arm,smmu-v3", },
{ },
};
+MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
static struct platform_driver arm_smmu_driver = {
.driver = {
.name = "arm-smmu-v3",
.of_match_table = of_match_ptr(arm_smmu_of_match),
- .suppress_bind_attrs = true,
},
.probe = arm_smmu_device_probe,
+ .remove = arm_smmu_device_remove,
.shutdown = arm_smmu_device_shutdown,
};
-builtin_platform_driver(arm_smmu_driver);
+module_platform_driver(arm_smmu_driver);
+
+MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
+MODULE_AUTHOR("Will Deacon <will.deacon@arm.com>");
+MODULE_LICENSE("GPL v2");
--
2.24.0.rc1.363.gb1bccd3e3d-goog
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 7/9] iommu/arm-smmu-v3: Allow building as a module
2019-11-08 15:15 [PATCH v2 0/9] iommu: Permit modular builds of ARM SMMU[v3] drivers Will Deacon
` (5 preceding siblings ...)
2019-11-08 15:16 ` [PATCH v2 6/9] Revert "iommu/arm-smmu: Make arm-smmu-v3 explicitly non-modular" Will Deacon
@ 2019-11-08 15:16 ` Will Deacon
2019-11-08 15:16 ` [PATCH v2 8/9] Revert "iommu/arm-smmu: Make arm-smmu explicitly non-modular" Will Deacon
2019-11-08 15:16 ` [PATCH v2 9/9] iommu/arm-smmu: Allow building as a module Will Deacon
8 siblings, 0 replies; 18+ messages in thread
From: Will Deacon @ 2019-11-08 15:16 UTC (permalink / raw)
To: iommu, linux-kernel
Cc: Isaac J. Manjarres, Jean-Philippe Brucker, Saravana Kannan,
Robin Murphy, Bjorn Helgaas, Will Deacon
By removing the redundant call to 'pci_request_acs()' we can allow the
ARM SMMUv3 driver to be built as a module.
Signed-off-by: Will Deacon <will@kernel.org>
---
drivers/iommu/Kconfig | 2 +-
drivers/iommu/arm-smmu-v3.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index e3842eabcfdd..7583d47fc4d5 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -388,7 +388,7 @@ config ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT
config.
config ARM_SMMU_V3
- bool "ARM Ltd. System MMU Version 3 (SMMUv3) Support"
+ tristate "ARM Ltd. System MMU Version 3 (SMMUv3) Support"
depends on ARM64
select IOMMU_API
select IOMMU_IO_PGTABLE_LPAE
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 2ad8e2ca0583..d54ceb80c28d 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2733,6 +2733,7 @@ static struct iommu_ops arm_smmu_ops = {
.get_resv_regions = arm_smmu_get_resv_regions,
.put_resv_regions = arm_smmu_put_resv_regions,
.pgsize_bitmap = -1UL, /* Restricted during device attach */
+ .owner = THIS_MODULE,
};
/* Probing and initialisation functions */
@@ -3657,7 +3658,6 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
#ifdef CONFIG_PCI
if (pci_bus_type.iommu_ops != &arm_smmu_ops) {
- pci_request_acs();
ret = bus_set_iommu(&pci_bus_type, &arm_smmu_ops);
if (ret)
return ret;
--
2.24.0.rc1.363.gb1bccd3e3d-goog
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 8/9] Revert "iommu/arm-smmu: Make arm-smmu explicitly non-modular"
2019-11-08 15:15 [PATCH v2 0/9] iommu: Permit modular builds of ARM SMMU[v3] drivers Will Deacon
` (6 preceding siblings ...)
2019-11-08 15:16 ` [PATCH v2 7/9] iommu/arm-smmu-v3: Allow building as a module Will Deacon
@ 2019-11-08 15:16 ` Will Deacon
2019-11-08 15:16 ` [PATCH v2 9/9] iommu/arm-smmu: Allow building as a module Will Deacon
8 siblings, 0 replies; 18+ messages in thread
From: Will Deacon @ 2019-11-08 15:16 UTC (permalink / raw)
To: iommu, linux-kernel
Cc: Isaac J. Manjarres, Jean-Philippe Brucker, Saravana Kannan,
Robin Murphy, Bjorn Helgaas, Will Deacon
This reverts commit addb672f200f4e99368270da205320b83efe01a0.
Let's get the SMMU driver building as a module, which means putting
back some dead code that we used to carry.
Signed-off-by: Will Deacon <will@kernel.org>
---
drivers/iommu/arm-smmu.c | 32 +++++++++++++++++++-------------
1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 7c503a6bc585..53bbe0663b9e 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -27,8 +27,7 @@
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/iopoll.h>
-#include <linux/init.h>
-#include <linux/moduleparam.h>
+#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/of_device.h>
@@ -59,10 +58,6 @@
#define MSI_IOVA_LENGTH 0x100000
static int force_stage;
-/*
- * not really modular, but the easiest way to keep compat with existing
- * bootargs behaviour is to continue using module_param() here.
- */
module_param(force_stage, int, S_IRUGO);
MODULE_PARM_DESC(force_stage,
"Force SMMU mappings to be installed at a particular stage of translation. A value of '1' or '2' forces the corresponding stage. All other values are ignored (i.e. no stage is forced). Note that selecting a specific stage will disable support for nested translation.");
@@ -1878,6 +1873,7 @@ static const struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "qcom,smmu-v2", .data = &qcom_smmuv2 },
{ },
};
+MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
#ifdef CONFIG_ACPI
static int acpi_smmu_get_data(u32 model, struct arm_smmu_device *smmu)
@@ -2165,12 +2161,12 @@ static int arm_smmu_legacy_bus_init(void)
}
device_initcall_sync(arm_smmu_legacy_bus_init);
-static void arm_smmu_device_shutdown(struct platform_device *pdev)
+static int arm_smmu_device_remove(struct platform_device *pdev)
{
struct arm_smmu_device *smmu = platform_get_drvdata(pdev);
if (!smmu)
- return;
+ return -ENODEV;
if (!bitmap_empty(smmu->context_map, ARM_SMMU_MAX_CBS))
dev_err(&pdev->dev, "removing device with active domains!\n");
@@ -2186,6 +2182,12 @@ static void arm_smmu_device_shutdown(struct platform_device *pdev)
clk_bulk_disable(smmu->num_clks, smmu->clks);
clk_bulk_unprepare(smmu->num_clks, smmu->clks);
+ return 0;
+}
+
+static void arm_smmu_device_shutdown(struct platform_device *pdev)
+{
+ arm_smmu_device_remove(pdev);
}
static int __maybe_unused arm_smmu_runtime_resume(struct device *dev)
@@ -2235,12 +2237,16 @@ static const struct dev_pm_ops arm_smmu_pm_ops = {
static struct platform_driver arm_smmu_driver = {
.driver = {
- .name = "arm-smmu",
- .of_match_table = of_match_ptr(arm_smmu_of_match),
- .pm = &arm_smmu_pm_ops,
- .suppress_bind_attrs = true,
+ .name = "arm-smmu",
+ .of_match_table = of_match_ptr(arm_smmu_of_match),
+ .pm = &arm_smmu_pm_ops,
},
.probe = arm_smmu_device_probe,
+ .remove = arm_smmu_device_remove,
.shutdown = arm_smmu_device_shutdown,
};
-builtin_platform_driver(arm_smmu_driver);
+module_platform_driver(arm_smmu_driver);
+
+MODULE_DESCRIPTION("IOMMU API for ARM architected SMMU implementations");
+MODULE_AUTHOR("Will Deacon <will.deacon@arm.com>");
+MODULE_LICENSE("GPL v2");
--
2.24.0.rc1.363.gb1bccd3e3d-goog
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 9/9] iommu/arm-smmu: Allow building as a module
2019-11-08 15:15 [PATCH v2 0/9] iommu: Permit modular builds of ARM SMMU[v3] drivers Will Deacon
` (7 preceding siblings ...)
2019-11-08 15:16 ` [PATCH v2 8/9] Revert "iommu/arm-smmu: Make arm-smmu explicitly non-modular" Will Deacon
@ 2019-11-08 15:16 ` Will Deacon
8 siblings, 0 replies; 18+ messages in thread
From: Will Deacon @ 2019-11-08 15:16 UTC (permalink / raw)
To: iommu, linux-kernel
Cc: Isaac J. Manjarres, Jean-Philippe Brucker, Saravana Kannan,
Robin Murphy, Bjorn Helgaas, Will Deacon
By conditionally dropping support for the legacy binding and exporting
the newly introduced 'arm_smmu_impl_init()' function we can allow the
ARM SMMU driver to be built as a module.
Signed-off-by: Will Deacon <will@kernel.org>
---
drivers/iommu/Kconfig | 14 +++++++++-
drivers/iommu/Makefile | 3 ++-
drivers/iommu/arm-smmu.c | 55 ++++++++++++++++++++++++----------------
3 files changed, 48 insertions(+), 24 deletions(-)
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 7583d47fc4d5..fc55f7ba0d18 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -350,7 +350,7 @@ config SPAPR_TCE_IOMMU
# ARM IOMMU support
config ARM_SMMU
- bool "ARM Ltd. System MMU (SMMU) Support"
+ tristate "ARM Ltd. System MMU (SMMU) Support"
depends on (ARM64 || ARM) && MMU
select IOMMU_API
select IOMMU_IO_PGTABLE_LPAE
@@ -362,6 +362,18 @@ config ARM_SMMU
Say Y here if your SoC includes an IOMMU device implementing
the ARM SMMU architecture.
+config ARM_SMMU_LEGACY_DT_BINDINGS
+ bool "Support the legacy \"mmu-masters\" devicetree bindings"
+ depends on ARM_SMMU=y && OF
+ help
+ Support for the badly designed and deprecated "mmu-masters"
+ devicetree bindings. This allows some DMA masters to attach
+ to the SMMU but does not provide any support via the DMA API.
+ If you're lucky, you might be able to get VFIO up and running.
+
+ If you say Y here then you'll make me very sad. Instead, say N
+ and move your firmware to the utopian future that was 2016.
+
config ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT
bool "Default to disabling bypass on ARM SMMU v1 and v2"
depends on ARM_SMMU
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 4f405f926e73..b52a03d87fc3 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -13,7 +13,8 @@ obj-$(CONFIG_MSM_IOMMU) += msm_iommu.o
obj-$(CONFIG_AMD_IOMMU) += amd_iommu.o amd_iommu_init.o amd_iommu_quirks.o
obj-$(CONFIG_AMD_IOMMU_DEBUGFS) += amd_iommu_debugfs.o
obj-$(CONFIG_AMD_IOMMU_V2) += amd_iommu_v2.o
-obj-$(CONFIG_ARM_SMMU) += arm-smmu.o arm-smmu-impl.o
+obj-$(CONFIG_ARM_SMMU) += arm-smmu-mod.o
+arm-smmu-mod-objs += arm-smmu.o arm-smmu-impl.o
obj-$(CONFIG_ARM_SMMU_V3) += arm-smmu-v3.o
obj-$(CONFIG_DMAR_TABLE) += dmar.o
obj-$(CONFIG_INTEL_IOMMU) += intel-iommu.o intel-pasid.o
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 53bbe0663b9e..9eb52410d016 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -125,6 +125,12 @@ static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
return container_of(dom, struct arm_smmu_domain, domain);
}
+static struct platform_driver arm_smmu_driver;
+static struct iommu_ops arm_smmu_ops;
+
+#ifdef CONFIG_ARM_SMMU_LEGACY_DT_BINDINGS
+static void arm_smmu_bus_init(void);
+
static struct device_node *dev_get_dev_node(struct device *dev)
{
if (dev_is_pci(dev)) {
@@ -160,9 +166,6 @@ static int __find_legacy_master_phandle(struct device *dev, void *data)
return err == -ENOENT ? 0 : err;
}
-static struct platform_driver arm_smmu_driver;
-static struct iommu_ops arm_smmu_ops;
-
static int arm_smmu_register_legacy_master(struct device *dev,
struct arm_smmu_device **smmu)
{
@@ -214,6 +217,27 @@ static int arm_smmu_register_legacy_master(struct device *dev,
return err;
}
+/*
+ * With the legacy DT binding in play, we have no guarantees about
+ * probe order, but then we're also not doing default domains, so we can
+ * delay setting bus ops until we're sure every possible SMMU is ready,
+ * and that way ensure that no add_device() calls get missed.
+ */
+static int arm_smmu_legacy_bus_init(void)
+{
+ if (using_legacy_binding)
+ arm_smmu_bus_init();
+ return 0;
+}
+device_initcall_sync(arm_smmu_legacy_bus_init);
+#else
+static int arm_smmu_register_legacy_master(struct device *dev,
+ struct arm_smmu_device **smmu)
+{
+ return -ENODEV;
+}
+#endif /* CONFIG_ARM_SMMU_LEGACY_DT_BINDINGS */
+
static int __arm_smmu_alloc_bitmap(unsigned long *map, int start, int end)
{
int idx;
@@ -1566,6 +1590,7 @@ static struct iommu_ops arm_smmu_ops = {
.get_resv_regions = arm_smmu_get_resv_regions,
.put_resv_regions = arm_smmu_put_resv_regions,
.pgsize_bitmap = -1UL, /* Restricted during device attach */
+ .owner = THIS_MODULE,
};
static void arm_smmu_device_reset(struct arm_smmu_device *smmu)
@@ -1960,8 +1985,10 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev,
legacy_binding = of_find_property(dev->of_node, "mmu-masters", NULL);
if (legacy_binding && !using_generic_binding) {
- if (!using_legacy_binding)
- pr_notice("deprecated \"mmu-masters\" DT property in use; DMA API support unavailable\n");
+ if (!using_legacy_binding) {
+ pr_notice("deprecated \"mmu-masters\" DT property in use; %s support unavailable\n",
+ IS_ENABLED(CONFIG_ARM_SMMU_LEGACY_DT_BINDINGS) ? "DMA API" : "SMMU");
+ }
using_legacy_binding = true;
} else if (!legacy_binding && !using_legacy_binding) {
using_generic_binding = true;
@@ -1986,10 +2013,8 @@ static void arm_smmu_bus_init(void)
bus_set_iommu(&amba_bustype, &arm_smmu_ops);
#endif
#ifdef CONFIG_PCI
- if (!iommu_present(&pci_bus_type)) {
- pci_request_acs();
+ if (!iommu_present(&pci_bus_type))
bus_set_iommu(&pci_bus_type, &arm_smmu_ops);
- }
#endif
#ifdef CONFIG_FSL_MC_BUS
if (!iommu_present(&fsl_mc_bus_type))
@@ -2147,20 +2172,6 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
return 0;
}
-/*
- * With the legacy DT binding in play, though, we have no guarantees about
- * probe order, but then we're also not doing default domains, so we can
- * delay setting bus ops until we're sure every possible SMMU is ready,
- * and that way ensure that no add_device() calls get missed.
- */
-static int arm_smmu_legacy_bus_init(void)
-{
- if (using_legacy_binding)
- arm_smmu_bus_init();
- return 0;
-}
-device_initcall_sync(arm_smmu_legacy_bus_init);
-
static int arm_smmu_device_remove(struct platform_device *pdev)
{
struct arm_smmu_device *smmu = platform_get_drvdata(pdev);
--
2.24.0.rc1.363.gb1bccd3e3d-goog
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2 6/9] Revert "iommu/arm-smmu: Make arm-smmu-v3 explicitly non-modular"
2019-11-08 15:16 ` [PATCH v2 6/9] Revert "iommu/arm-smmu: Make arm-smmu-v3 explicitly non-modular" Will Deacon
@ 2019-11-08 16:17 ` John Garry
2019-11-08 16:44 ` John Garry
0 siblings, 1 reply; 18+ messages in thread
From: John Garry @ 2019-11-08 16:17 UTC (permalink / raw)
To: Will Deacon, iommu, linux-kernel
Cc: Isaac J. Manjarres, Jean-Philippe Brucker, Saravana Kannan,
Bjorn Helgaas, Robin Murphy
On 08/11/2019 15:16, Will Deacon wrote:
> +MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
Hi Will,
>
> static struct platform_driver arm_smmu_driver = {
> .driver = {
> .name = "arm-smmu-v3",
> .of_match_table = of_match_ptr(arm_smmu_of_match),
> - .suppress_bind_attrs = true,
Does this mean that we can now manually unbind this driver from the SMMU
device?
Seems dangerous. Here's what happens for me:
root@ubuntu:/sys# cd ./bus/platform/drivers/arm-smmu-v3
ind @ubuntu:/sys/bus/platform/drivers/arm-smmu-v3# echo
arm-smmu-v3.0.auto > unbind
[ 77.580351] hisi_sas_v2_hw HISI0162:01: CQE_AXI_W_ERR (0x800) found!
ho [ 78.635473] platform arm-smmu-v3.0.auto: CMD_SYNC timeout at
0x00000146 [hwprod 0x00000146, hwcons 0x00000000]
> },
> .probe = arm_smmu_device_probe,
> + .remove = arm_smmu_device_remove,
> .shutdown = arm_smmu_device_shutdown,
> };
> -builtin_platform_driver(arm_smmu_driver);
> +module_platform_driver(arm_smmu_driver);
> +
Thanks,
John
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 6/9] Revert "iommu/arm-smmu: Make arm-smmu-v3 explicitly non-modular"
2019-11-08 16:17 ` John Garry
@ 2019-11-08 16:44 ` John Garry
2019-11-08 16:47 ` Will Deacon
0 siblings, 1 reply; 18+ messages in thread
From: John Garry @ 2019-11-08 16:44 UTC (permalink / raw)
To: Will Deacon, iommu, linux-kernel
Cc: Isaac J. Manjarres, Jean-Philippe Brucker, Bjorn Helgaas,
Robin Murphy, Saravana Kannan
On 08/11/2019 16:17, John Garry wrote:
> On 08/11/2019 15:16, Will Deacon wrote:
>> +MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
>
> Hi Will,
>
>> static struct platform_driver arm_smmu_driver = {
>> .driver = {
>> .name = "arm-smmu-v3",
>> .of_match_table = of_match_ptr(arm_smmu_of_match),
>> - .suppress_bind_attrs = true,
>
> Does this mean that we can now manually unbind this driver from the SMMU
> device?
>
> Seems dangerous. Here's what happens for me:
>
> root@ubuntu:/sys# cd ./bus/platform/drivers/arm-smmu-v3
> ind @ubuntu:/sys/bus/platform/drivers/arm-smmu-v3# echo
> arm-smmu-v3.0.auto > unbind
> [ 77.580351] hisi_sas_v2_hw HISI0162:01: CQE_AXI_W_ERR (0x800) found!
> ho [ 78.635473] platform arm-smmu-v3.0.auto: CMD_SYNC timeout at
> 0x00000146 [hwprod 0x00000146, hwcons 0x00000000]
>
>> },
>> .probe = arm_smmu_device_probe,
>> + .remove = arm_smmu_device_remove,
>> .shutdown = arm_smmu_device_shutdown,
>> };
>> -builtin_platform_driver(arm_smmu_driver);
>> +module_platform_driver(arm_smmu_driver);
>> +
BTW, it now looks like it was your v1 series I was testing there, on
your branch iommu/module. It would be helpful to update for ease of testing.
Thanks,
John
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 6/9] Revert "iommu/arm-smmu: Make arm-smmu-v3 explicitly non-modular"
2019-11-08 16:44 ` John Garry
@ 2019-11-08 16:47 ` Will Deacon
2019-11-08 17:25 ` John Garry
0 siblings, 1 reply; 18+ messages in thread
From: Will Deacon @ 2019-11-08 16:47 UTC (permalink / raw)
To: John Garry
Cc: Isaac J. Manjarres, Jean-Philippe Brucker, Saravana Kannan,
linux-kernel, iommu, Bjorn Helgaas, Robin Murphy
On Fri, Nov 08, 2019 at 04:44:25PM +0000, John Garry wrote:
> On 08/11/2019 16:17, John Garry wrote:
> > On 08/11/2019 15:16, Will Deacon wrote:
> > > +MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
> >
> > Hi Will,
> >
> > > static struct platform_driver arm_smmu_driver = {
> > > .driver = {
> > > .name = "arm-smmu-v3",
> > > .of_match_table = of_match_ptr(arm_smmu_of_match),
> > > - .suppress_bind_attrs = true,
> >
> > Does this mean that we can now manually unbind this driver from the SMMU
> > device?
> >
> > Seems dangerous. Here's what happens for me:
> >
> > root@ubuntu:/sys# cd ./bus/platform/drivers/arm-smmu-v3
> > ind @ubuntu:/sys/bus/platform/drivers/arm-smmu-v3# echo
> > arm-smmu-v3.0.auto > unbind
> > [ 77.580351] hisi_sas_v2_hw HISI0162:01: CQE_AXI_W_ERR (0x800) found!
> > ho [ 78.635473] platform arm-smmu-v3.0.auto: CMD_SYNC timeout at
> > 0x00000146 [hwprod 0x00000146, hwcons 0x00000000]
> >
> > > },
> > > .probe = arm_smmu_device_probe,
> > > + .remove = arm_smmu_device_remove,
> > > .shutdown = arm_smmu_device_shutdown,
> > > };
> > > -builtin_platform_driver(arm_smmu_driver);
> > > +module_platform_driver(arm_smmu_driver);
> > > +
>
> BTW, it now looks like it was your v1 series I was testing there, on your
> branch iommu/module. It would be helpful to update for ease of testing.
Yes, sorry about that. I'll update it now (although I'm not sure it will
help with this -- I was going to see what happens with other devices such
as the intel-iommu or storage controllers)
Will
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 6/9] Revert "iommu/arm-smmu: Make arm-smmu-v3 explicitly non-modular"
2019-11-08 16:47 ` Will Deacon
@ 2019-11-08 17:25 ` John Garry
2019-11-08 17:32 ` Will Deacon
0 siblings, 1 reply; 18+ messages in thread
From: John Garry @ 2019-11-08 17:25 UTC (permalink / raw)
To: Will Deacon
Cc: Isaac J. Manjarres, Jean-Philippe Brucker, Saravana Kannan,
linux-kernel, iommu, Bjorn Helgaas, Robin Murphy
On 08/11/2019 16:47, Will Deacon wrote:
> On Fri, Nov 08, 2019 at 04:44:25PM +0000, John Garry wrote:
>> On 08/11/2019 16:17, John Garry wrote:
>>> On 08/11/2019 15:16, Will Deacon wrote:
>>>> +MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
>>>
>>> Hi Will,
>>>
>>>> static struct platform_driver arm_smmu_driver = {
>>>> .driver = {
>>>> .name = "arm-smmu-v3",
>>>> .of_match_table = of_match_ptr(arm_smmu_of_match),
>>>> - .suppress_bind_attrs = true,
>>>
>>> Does this mean that we can now manually unbind this driver from the SMMU
>>> device?
>>>
>>> Seems dangerous. Here's what happens for me:
>>>
>>> root@ubuntu:/sys# cd ./bus/platform/drivers/arm-smmu-v3
>>> ind @ubuntu:/sys/bus/platform/drivers/arm-smmu-v3# echo
>>> arm-smmu-v3.0.auto > unbind
>>> [ 77.580351] hisi_sas_v2_hw HISI0162:01: CQE_AXI_W_ERR (0x800) found!
>>> ho [ 78.635473] platform arm-smmu-v3.0.auto: CMD_SYNC timeout at
>>> 0x00000146 [hwprod 0x00000146, hwcons 0x00000000]
>>>
>>>> },
>>>> .probe = arm_smmu_device_probe,
>>>> + .remove = arm_smmu_device_remove,
>>>> .shutdown = arm_smmu_device_shutdown,
>>>> };
>>>> -builtin_platform_driver(arm_smmu_driver);
>>>> +module_platform_driver(arm_smmu_driver);
>>>> +
>>
>> BTW, it now looks like it was your v1 series I was testing there, on your
>> branch iommu/module. It would be helpful to update for ease of testing.
>
Hi Will,
> Yes, sorry about that. I'll update it now (although I'm not sure it will
> help with this -- I was going to see what happens with other devices such
> as the intel-iommu or storage controllers)
So I tried your v2 series for this - it has the same issue, as I
anticipated.
It seems that some iommu drivers do call iommu_device_register(), so
maybe a decent reference. Or simply stop the driver being unbound.
Cheers,
John
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 6/9] Revert "iommu/arm-smmu: Make arm-smmu-v3 explicitly non-modular"
2019-11-08 17:25 ` John Garry
@ 2019-11-08 17:32 ` Will Deacon
2019-11-08 17:48 ` Will Deacon
2019-11-08 17:49 ` John Garry
0 siblings, 2 replies; 18+ messages in thread
From: Will Deacon @ 2019-11-08 17:32 UTC (permalink / raw)
To: John Garry
Cc: Isaac J. Manjarres, Jean-Philippe Brucker, Saravana Kannan,
linux-kernel, iommu, Bjorn Helgaas, Robin Murphy
On Fri, Nov 08, 2019 at 05:25:09PM +0000, John Garry wrote:
> On 08/11/2019 16:47, Will Deacon wrote:
> > On Fri, Nov 08, 2019 at 04:44:25PM +0000, John Garry wrote:
> > > BTW, it now looks like it was your v1 series I was testing there, on your
> > > branch iommu/module. It would be helpful to update for ease of testing.
> >
> > Yes, sorry about that. I'll update it now (although I'm not sure it will
> > help with this -- I was going to see what happens with other devices such
> > as the intel-iommu or storage controllers)
>
> So I tried your v2 series for this - it has the same issue, as I
> anticipated.
Right, I'm just not sure how resilient drivers are expected to be to force
unbinding like this. You can break lots of stuff with root...
> It seems that some iommu drivers do call iommu_device_register(), so maybe a
> decent reference. Or simply stop the driver being unbound.
I'm not sure what you mean about iommu_device_register() (we call that
already), but I guess we can keep the '.suppress_bind_attrs = true' if
necessary. I'll have a play on my laptop and see how well that works if
you start unbinding stuff.
Will
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 6/9] Revert "iommu/arm-smmu: Make arm-smmu-v3 explicitly non-modular"
2019-11-08 17:32 ` Will Deacon
@ 2019-11-08 17:48 ` Will Deacon
2019-11-08 18:00 ` John Garry
2019-11-08 17:49 ` John Garry
1 sibling, 1 reply; 18+ messages in thread
From: Will Deacon @ 2019-11-08 17:48 UTC (permalink / raw)
To: John Garry
Cc: Isaac J. Manjarres, Jean-Philippe Brucker, Saravana Kannan,
linux-kernel, iommu, Bjorn Helgaas, Robin Murphy
On Fri, Nov 08, 2019 at 05:32:48PM +0000, Will Deacon wrote:
> On Fri, Nov 08, 2019 at 05:25:09PM +0000, John Garry wrote:
> > On 08/11/2019 16:47, Will Deacon wrote:
> > > On Fri, Nov 08, 2019 at 04:44:25PM +0000, John Garry wrote:
> > > > BTW, it now looks like it was your v1 series I was testing there, on your
> > > > branch iommu/module. It would be helpful to update for ease of testing.
> > >
> > > Yes, sorry about that. I'll update it now (although I'm not sure it will
> > > help with this -- I was going to see what happens with other devices such
> > > as the intel-iommu or storage controllers)
> >
> > So I tried your v2 series for this - it has the same issue, as I
> > anticipated.
>
> Right, I'm just not sure how resilient drivers are expected to be to force
> unbinding like this. You can break lots of stuff with root...
>
> > It seems that some iommu drivers do call iommu_device_register(), so maybe a
> > decent reference. Or simply stop the driver being unbound.
>
> I'm not sure what you mean about iommu_device_register() (we call that
> already), but I guess we can keep the '.suppress_bind_attrs = true' if
> necessary. I'll have a play on my laptop and see how well that works if
> you start unbinding stuff.
So unbinding the nvme driver goes bang:
[90139.090158] nvme nvme0: failed to set APST feature (-19)
[90141.966780] Aborting journal on device dm-1-8.
[90141.967124] Buffer I/O error on dev dm-1, logical block 26247168, lost sync page write
[90141.967169] JBD2: Error -5 detected when updating journal superblock for dm-1-8.
[90141.967403] Buffer I/O error on dev dm-1, logical block 0, lost sync page write
[90141.967454] EXT4-fs (dm-1): I/O error while writing superblock
[90141.967467] EXT4-fs error (device dm-1): ext4_journal_check_start:61: Detected aborted journal
[90141.967473] EXT4-fs (dm-1): Remounting filesystem read-only
[90141.967569] Buffer I/O error on dev dm-1, logical block 0, lost sync page write
[90141.967682] EXT4-fs (dm-1): I/O error while writing superblock
and I've not managed to recover the thing yet (it's stuck trying to reboot.)
What state was your system in after unbinding the SMMU?
Will
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 6/9] Revert "iommu/arm-smmu: Make arm-smmu-v3 explicitly non-modular"
2019-11-08 17:32 ` Will Deacon
2019-11-08 17:48 ` Will Deacon
@ 2019-11-08 17:49 ` John Garry
1 sibling, 0 replies; 18+ messages in thread
From: John Garry @ 2019-11-08 17:49 UTC (permalink / raw)
To: Will Deacon
Cc: Isaac J. Manjarres, Jean-Philippe Brucker, Saravana Kannan,
linux-kernel, iommu, Bjorn Helgaas, Robin Murphy
On 08/11/2019 17:32, Will Deacon wrote:
> On Fri, Nov 08, 2019 at 05:25:09PM +0000, John Garry wrote:
>> On 08/11/2019 16:47, Will Deacon wrote:
>>> On Fri, Nov 08, 2019 at 04:44:25PM +0000, John Garry wrote:
>>>> BTW, it now looks like it was your v1 series I was testing there, on your
>>>> branch iommu/module. It would be helpful to update for ease of testing.
>>>
>>> Yes, sorry about that. I'll update it now (although I'm not sure it will
>>> help with this -- I was going to see what happens with other devices such
>>> as the intel-iommu or storage controllers)
>>
>> So I tried your v2 series for this - it has the same issue, as I
>> anticipated.
>
> Right, I'm just not sure how resilient drivers are expected to be to force
> unbinding like this. You can break lots of stuff with root...
For sure, but it is good practice to limit that.
I had to fix something like this recently, so know about it... another
potential problem is use-after-frees, where your device managed memory
is freed at removal but still registered somewhere.
>
>> It seems that some iommu drivers do call iommu_device_register(), so maybe a
>> decent reference. Or simply stop the driver being unbound.
>
> I'm not sure what you mean about iommu_device_register() (we call that
> already),
Sorry, I meant to say iommu_device_unregister().
but I guess we can keep the '.suppress_bind_attrs = true' if
> necessary.
It may be good to add it to older stable kernels also, pre c07b6426df92.
I'll have a play on my laptop and see how well that works if
> you start unbinding stuff.
Cheers,
John
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 6/9] Revert "iommu/arm-smmu: Make arm-smmu-v3 explicitly non-modular"
2019-11-08 17:48 ` Will Deacon
@ 2019-11-08 18:00 ` John Garry
0 siblings, 0 replies; 18+ messages in thread
From: John Garry @ 2019-11-08 18:00 UTC (permalink / raw)
To: Will Deacon
Cc: Isaac J. Manjarres, Jean-Philippe Brucker, Saravana Kannan,
linux-kernel, iommu, Bjorn Helgaas, Robin Murphy
On 08/11/2019 17:48, Will Deacon wrote:
> On Fri, Nov 08, 2019 at 05:32:48PM +0000, Will Deacon wrote:
>> On Fri, Nov 08, 2019 at 05:25:09PM +0000, John Garry wrote:
>>> On 08/11/2019 16:47, Will Deacon wrote:
>>>> On Fri, Nov 08, 2019 at 04:44:25PM +0000, John Garry wrote:
>>>>> BTW, it now looks like it was your v1 series I was testing there, on your
>>>>> branch iommu/module. It would be helpful to update for ease of testing.
>>>>
>>>> Yes, sorry about that. I'll update it now (although I'm not sure it will
>>>> help with this -- I was going to see what happens with other devices such
>>>> as the intel-iommu or storage controllers)
>>>
>>> So I tried your v2 series for this - it has the same issue, as I
>>> anticipated.
>>
>> Right, I'm just not sure how resilient drivers are expected to be to force
>> unbinding like this. You can break lots of stuff with root...
>>
>>> It seems that some iommu drivers do call iommu_device_register(), so maybe a
>>> decent reference. Or simply stop the driver being unbound.
>>
>> I'm not sure what you mean about iommu_device_register() (we call that
>> already), but I guess we can keep the '.suppress_bind_attrs = true' if
>> necessary. I'll have a play on my laptop and see how well that works if
>> you start unbinding stuff.
>
> So unbinding the nvme driver goes bang:
>
> [90139.090158] nvme nvme0: failed to set APST feature (-19)
> [90141.966780] Aborting journal on device dm-1-8.
> [90141.967124] Buffer I/O error on dev dm-1, logical block 26247168, lost sync page write
> [90141.967169] JBD2: Error -5 detected when updating journal superblock for dm-1-8.
> [90141.967403] Buffer I/O error on dev dm-1, logical block 0, lost sync page write
> [90141.967454] EXT4-fs (dm-1): I/O error while writing superblock
> [90141.967467] EXT4-fs error (device dm-1): ext4_journal_check_start:61: Detected aborted journal
> [90141.967473] EXT4-fs (dm-1): Remounting filesystem read-only
> [90141.967569] Buffer I/O error on dev dm-1, logical block 0, lost sync page write
> [90141.967682] EXT4-fs (dm-1): I/O error while writing superblock
>
> and I've not managed to recover the thing yet (it's stuck trying to reboot.)
>
Not surprised. I guess the device backing your root directory disappeared.
> What state was your system in after unbinding the SMMU?
Unusable again. For me the storage controller backing the root directory
is compromised by disabling the SMMU unsafely.
Thanks,
John
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2019-11-08 18:00 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-08 15:15 [PATCH v2 0/9] iommu: Permit modular builds of ARM SMMU[v3] drivers Will Deacon
2019-11-08 15:16 ` [PATCH v2 1/9] drivers/iommu: Export core IOMMU API symbols to permit modular drivers Will Deacon
2019-11-08 15:16 ` [PATCH v2 2/9] iommu/of: Request ACS from the PCI core when configuring IOMMU linkage Will Deacon
2019-11-08 15:16 ` [PATCH v2 3/9] PCI: Export pci_ats_disabled() as a GPL symbol to modules Will Deacon
2019-11-08 15:16 ` [PATCH v2 4/9] drivers/iommu: Take a ref to the IOMMU driver prior to ->add_device() Will Deacon
2019-11-08 15:16 ` [PATCH v2 5/9] iommu/of: Take a ref to the IOMMU driver during ->of_xlate() Will Deacon
2019-11-08 15:16 ` [PATCH v2 6/9] Revert "iommu/arm-smmu: Make arm-smmu-v3 explicitly non-modular" Will Deacon
2019-11-08 16:17 ` John Garry
2019-11-08 16:44 ` John Garry
2019-11-08 16:47 ` Will Deacon
2019-11-08 17:25 ` John Garry
2019-11-08 17:32 ` Will Deacon
2019-11-08 17:48 ` Will Deacon
2019-11-08 18:00 ` John Garry
2019-11-08 17:49 ` John Garry
2019-11-08 15:16 ` [PATCH v2 7/9] iommu/arm-smmu-v3: Allow building as a module Will Deacon
2019-11-08 15:16 ` [PATCH v2 8/9] Revert "iommu/arm-smmu: Make arm-smmu explicitly non-modular" Will Deacon
2019-11-08 15:16 ` [PATCH v2 9/9] iommu/arm-smmu: Allow building as a module Will Deacon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).