Revert "firmware: QCOM_SCM: Allow qcom_scm driver to be loadable as a permenent module"

Message ID 20201119174149.3860883-1-thierry.reding@gmail.com
State New
Headers show
Series
  • Revert "firmware: QCOM_SCM: Allow qcom_scm driver to be loadable as a permenent module"
Related show

Commit Message

Thierry Reding Nov. 19, 2020, 5:41 p.m.
From: Thierry Reding <treding@nvidia.com>

Commit d0511b5496c0 ("firmware: QCOM_SCM: Allow qcom_scm driver to be
loadable as a permenent module") causes the ARM SMMU driver to be built
as a loadable module when using the Aarch64 default configuration. This
in turn causes problems because if the loadable module is not shipped
in an initial ramdisk, then the deferred probe timeout mechanism will
cause all SMMU masters to probe without SMMU support and fall back to
just plain DMA ops (not IOMMU-backed).

Once the system has mounted the rootfs, the ARM SMMU driver will then
be loaded, but since the ARM SMMU driver faults by default, this causes
a slew of SMMU faults for the SMMU masters that have already been set
up with plain DMA ops and cause these devices to malfunction.

Revert that commit to unbreak things while we look for an alternative
solution.

Reported-by: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/firmware/Kconfig                | 4 ++--
 drivers/firmware/Makefile               | 3 +--
 drivers/firmware/qcom_scm.c             | 4 ----
 drivers/iommu/Kconfig                   | 2 --
 drivers/net/wireless/ath/ath10k/Kconfig | 1 -
 5 files changed, 3 insertions(+), 11 deletions(-)

Comments

Will Deacon Nov. 19, 2020, 7:21 p.m. | #1
On Thu, Nov 19, 2020 at 06:41:49PM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Commit d0511b5496c0 ("firmware: QCOM_SCM: Allow qcom_scm driver to be
> loadable as a permenent module") causes the ARM SMMU driver to be built
> as a loadable module when using the Aarch64 default configuration. This
> in turn causes problems because if the loadable module is not shipped
> in an initial ramdisk, then the deferred probe timeout mechanism will
> cause all SMMU masters to probe without SMMU support and fall back to
> just plain DMA ops (not IOMMU-backed).
> 
> Once the system has mounted the rootfs, the ARM SMMU driver will then
> be loaded, but since the ARM SMMU driver faults by default, this causes
> a slew of SMMU faults for the SMMU masters that have already been set
> up with plain DMA ops and cause these devices to malfunction.
> 
> Revert that commit to unbreak things while we look for an alternative
> solution.
> 
> Reported-by: Jon Hunter <jonathanh@nvidia.com>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/firmware/Kconfig                | 4 ++--
>  drivers/firmware/Makefile               | 3 +--
>  drivers/firmware/qcom_scm.c             | 4 ----
>  drivers/iommu/Kconfig                   | 2 --
>  drivers/net/wireless/ath/ath10k/Kconfig | 1 -
>  5 files changed, 3 insertions(+), 11 deletions(-)

Acked-by: Will Deacon <will@kernel.org>

John previously agreed to this here:

https://lore.kernel.org/r/CALAqxLVDm923WRSB+DVxFacmEtmEPS7Qeq+rW_jbCDMNMWw9PQ@mail.gmail.com

so might be worth putting in a Link: tag when it gets applied.

Will
Bjorn Andersson Nov. 20, 2020, 5:29 a.m. | #2
On Thu, Nov 19, 2020 at 11:42 AM Thierry Reding
<thierry.reding@gmail.com> wrote:
>

> From: Thierry Reding <treding@nvidia.com>

>

> Commit d0511b5496c0 ("firmware: QCOM_SCM: Allow qcom_scm driver to be

> loadable as a permenent module") causes the ARM SMMU driver to be built

> as a loadable module when using the Aarch64 default configuration. This

> in turn causes problems because if the loadable module is not shipped

> in an initial ramdisk, then the deferred probe timeout mechanism will

> cause all SMMU masters to probe without SMMU support and fall back to

> just plain DMA ops (not IOMMU-backed).

>

> Once the system has mounted the rootfs, the ARM SMMU driver will then

> be loaded, but since the ARM SMMU driver faults by default, this causes

> a slew of SMMU faults for the SMMU masters that have already been set

> up with plain DMA ops and cause these devices to malfunction.

>


FWIW I had the same issues on the Qualcomm platform and merged a patch
that turns QCOM_SCM=y in arm64 defconfig earlier today. So this should
hide the problem in next linux-next. (And it really should be =y in
defconfig regardless of this revert or not).

> Revert that commit to unbreak things while we look for an alternative

> solution.

>


I don't fancy the fact that we have a situation where if you're
unlucky to have probe deferrals lingering past late initcall things
will start to just break and this from a growing number of resource
types. But I also don't see any alternatives to fixing the kernel to
handle this gracefully.

Regards,
Bjorn

Patch

diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 5e369928bc56..3315e3c21586 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -235,8 +235,8 @@  config INTEL_STRATIX10_RSU
 	  Say Y here if you want Intel RSU support.
 
 config QCOM_SCM
-	tristate "Qcom SCM driver"
-	depends on (ARM && HAVE_ARM_SMCCC) || ARM64
+	bool
+	depends on ARM || ARM64
 	select RESET_CONTROLLER
 
 config QCOM_SCM_DOWNLOAD_MODE_DEFAULT
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 523173cbff33..5e013b6a3692 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -17,8 +17,7 @@  obj-$(CONFIG_ISCSI_IBFT)	+= iscsi_ibft.o
 obj-$(CONFIG_FIRMWARE_MEMMAP)	+= memmap.o
 obj-$(CONFIG_RASPBERRYPI_FIRMWARE) += raspberrypi.o
 obj-$(CONFIG_FW_CFG_SYSFS)	+= qemu_fw_cfg.o
-obj-$(CONFIG_QCOM_SCM)		+= qcom-scm.o
-qcom-scm-objs += qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o
+obj-$(CONFIG_QCOM_SCM)		+= qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o
 obj-$(CONFIG_TI_SCI_PROTOCOL)	+= ti_sci.o
 obj-$(CONFIG_TRUSTED_FOUNDATIONS) += trusted_foundations.o
 obj-$(CONFIG_TURRIS_MOX_RWTM)	+= turris-mox-rwtm.o
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 6f431b73e617..7be48c1bec96 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -1280,7 +1280,6 @@  static const struct of_device_id qcom_scm_dt_match[] = {
 	{ .compatible = "qcom,scm" },
 	{}
 };
-MODULE_DEVICE_TABLE(of, qcom_scm_dt_match);
 
 static struct platform_driver qcom_scm_driver = {
 	.driver = {
@@ -1296,6 +1295,3 @@  static int __init qcom_scm_init(void)
 	return platform_driver_register(&qcom_scm_driver);
 }
 subsys_initcall(qcom_scm_init);
-
-MODULE_DESCRIPTION("Qualcomm Technologies, Inc. SCM driver");
-MODULE_LICENSE("GPL v2");
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index c64d7a2b6513..04878caf6da4 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -248,7 +248,6 @@  config SPAPR_TCE_IOMMU
 config ARM_SMMU
 	tristate "ARM Ltd. System MMU (SMMU) Support"
 	depends on ARM64 || ARM || (COMPILE_TEST && !GENERIC_ATOMIC64)
-	depends on QCOM_SCM || !QCOM_SCM #if QCOM_SCM=m this can't be =y
 	select IOMMU_API
 	select IOMMU_IO_PGTABLE_LPAE
 	select ARM_DMA_USE_IOMMU if ARM
@@ -376,7 +375,6 @@  config QCOM_IOMMU
 	# Note: iommu drivers cannot (yet?) be built as modules
 	bool "Qualcomm IOMMU Support"
 	depends on ARCH_QCOM || (COMPILE_TEST && !GENERIC_ATOMIC64)
-	depends on QCOM_SCM=y
 	select IOMMU_API
 	select IOMMU_IO_PGTABLE_LPAE
 	select ARM_DMA_USE_IOMMU
diff --git a/drivers/net/wireless/ath/ath10k/Kconfig b/drivers/net/wireless/ath/ath10k/Kconfig
index 741289e385d5..40f91bc8514d 100644
--- a/drivers/net/wireless/ath/ath10k/Kconfig
+++ b/drivers/net/wireless/ath/ath10k/Kconfig
@@ -44,7 +44,6 @@  config ATH10K_SNOC
 	tristate "Qualcomm ath10k SNOC support"
 	depends on ATH10K
 	depends on ARCH_QCOM || COMPILE_TEST
-	depends on QCOM_SCM || !QCOM_SCM #if QCOM_SCM=m this can't be =y
 	select QCOM_QMI_HELPERS
 	help
 	  This module adds support for integrated WCN3990 chip connected