2019-05-17 19:31:35

by Isaac J. Manjarres

[permalink] [raw]
Subject: [RFC/PATCH 0/4] Initial support for modular IOMMU drivers

This series adds initial support for being able to use the ARM
SMMU driver as a loadable kernel module. The series also adds
to the IOMMU framework, so that it can defer probing for devices
that depend on an IOMMU driver that may be a loadable module.

The primary reason behind these changes is that having the ARM
SMMU driver as a module allows for the same kernel image to be
used across different platforms. For example, if one platform
contains an IOMMU that implements one version of the ARM SMMU
specification, and another platform simply does not have an
IOMMU, the only way that these platforms can share the same
kernel image is if the ARM SMMU driver is compiled into the
kernel image.

This solution is not scalable, as it will lead to bloating the
kernel image with support for several future versions of the
SMMU specification to maintain a common kernel image that works
across all platforms. Having the ARM SMMU driver as a module allows
for a common kernel image to be supported across all platforms,
while yielding a smaller kernel image size, since the correct
SMMU driver can be loaded at runtime, if necessary.

Patchset Summary:

1. Since the ARM SMMU driver depends on symbols being exported from
several subsystems, the first three patches are dedicated to exporting
the necessary symbols.

2. Similar to how the pinctrl framework handles deferring probes,
the subsequent patch makes it so that the IOMMU framework will defer
probes indefinitely if there is a chance that the IOMMU driver that a
device is waiting for is a module. Otherwise, it upholds the current
behavior of stopping probe deferrals once all of the builtin drivers
have finished probing.

The ARM SMMU driver currently has support for the deprecated
"mmu-masters" binding, which relies on the notion of initcall
ordering for setting the bus ops to ensure that all SMMU devices
have been bound to the driver. This poses a problem with
making the driver a module, as there is no such notion with
loadable modules. Will support for this be completely deprecated?
If not, might it be useful to leverage the device tree ordering,
and assign a property to the last SMMU device, and set the bus ops
at that point? Or perhaps have some deferred timer based approach
to know when to set the bus ops?

Thanks,
Isaac

Isaac J. Manjarres (4):
of: Export of_phandle_iterator_args() to modules
PCI: Export PCI ACS and DMA searching functions to modules
iommu: Export core IOMMU functions to kernel modules
iommu: Add probe deferral support for IOMMU kernel modules

drivers/iommu/iommu-sysfs.c | 3 +++
drivers/iommu/iommu.c | 6 ++++++
drivers/iommu/of_iommu.c | 8 ++++++--
drivers/of/base.c | 1 +
drivers/pci/pci.c | 1 +
drivers/pci/search.c | 1 +
6 files changed, 18 insertions(+), 2 deletions(-)

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


2019-05-17 20:49:10

by Isaac J. Manjarres

[permalink] [raw]
Subject: [RFC/PATCH 2/4] PCI: Export PCI ACS and DMA searching functions to modules

IOMMU drivers that can be compiled as modules may
want to use pci_for_each_dma_alias() and pci_request_acs(),
so export those functions.

Signed-off-by: Isaac J. Manjarres <[email protected]>
---
drivers/pci/pci.c | 1 +
drivers/pci/search.c | 1 +
2 files changed, 2 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 766f577..3f354c1 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3124,6 +3124,7 @@ void pci_request_acs(void)
{
pci_acs_enable = 1;
}
+EXPORT_SYMBOL_GPL(pci_request_acs);

static const char *disable_acs_redir_param;

diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index 2b5f720..cf9ede9 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -111,6 +111,7 @@ int pci_for_each_dma_alias(struct pci_dev *pdev,

return ret;
}
+EXPORT_SYMBOL_GPL(pci_for_each_dma_alias);

static struct pci_bus *pci_do_find_bus(struct pci_bus *bus, unsigned char busnr)
{
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2019-05-21 16:09:23

by Jean-Philippe Brucker

[permalink] [raw]
Subject: Re: [RFC/PATCH 0/4] Initial support for modular IOMMU drivers

Hi Isaac,

On 17/05/2019 19:47, Isaac J. Manjarres wrote:
> This series adds initial support for being able to use the ARM
> SMMU driver as a loadable kernel module. The series also adds
> to the IOMMU framework, so that it can defer probing for devices
> that depend on an IOMMU driver that may be a loadable module.
>
> The primary reason behind these changes is that having the ARM
> SMMU driver as a module allows for the same kernel image to be
> used across different platforms. For example, if one platform
> contains an IOMMU that implements one version of the ARM SMMU
> specification, and another platform simply does not have an
> IOMMU, the only way that these platforms can share the same
> kernel image is if the ARM SMMU driver is compiled into the
> kernel image.
>
> This solution is not scalable, as it will lead to bloating the
> kernel image with support for several future versions of the
> SMMU specification to maintain a common kernel image that works
> across all platforms. Having the ARM SMMU driver as a module allows
> for a common kernel image to be supported across all platforms,
> while yielding a smaller kernel image size, since the correct
> SMMU driver can be loaded at runtime, if necessary.

It can also be useful if IOMMU drivers want to rely on components that
distros usually build as modules. I have that problem with virtio-iommu,
where the whole virtio transport is usually modular.

> Patchset Summary:
>
> 1. Since the ARM SMMU driver depends on symbols being exported from
> several subsystems, the first three patches are dedicated to exporting
> the necessary symbols.
>
> 2. Similar to how the pinctrl framework handles deferring probes,
> the subsequent patch makes it so that the IOMMU framework will defer
> probes indefinitely if there is a chance that the IOMMU driver that a
> device is waiting for is a module. Otherwise, it upholds the current
> behavior of stopping probe deferrals once all of the builtin drivers
> have finished probing.
>
> The ARM SMMU driver currently has support for the deprecated
> "mmu-masters" binding, which relies on the notion of initcall
> ordering for setting the bus ops to ensure that all SMMU devices
> have been bound to the driver. This poses a problem with
> making the driver a module, as there is no such notion with
> loadable modules. Will support for this be completely deprecated?
> If not, might it be useful to leverage the device tree ordering,
> and assign a property to the last SMMU device, and set the bus ops
> at that point? Or perhaps have some deferred timer based approach
> to know when to set the bus ops?

Another problem is module unloading: if the user calls rmmod on an IOMMU
module, we have to ensure that endpoints aren't performing DMA anymore.
It could be solved by declaring consumers of an IOMMU with
device_link_add(), so that device drivers are unbound before the IOMMU
module is unloaded.

Thanks,
Jean

>
> Thanks,
> Isaac
>
> Isaac J. Manjarres (4):
> of: Export of_phandle_iterator_args() to modules
> PCI: Export PCI ACS and DMA searching functions to modules
> iommu: Export core IOMMU functions to kernel modules
> iommu: Add probe deferral support for IOMMU kernel modules
>
> drivers/iommu/iommu-sysfs.c | 3 +++
> drivers/iommu/iommu.c | 6 ++++++
> drivers/iommu/of_iommu.c | 8 ++++++--
> drivers/of/base.c | 1 +
> drivers/pci/pci.c | 1 +
> drivers/pci/search.c | 1 +
> 6 files changed, 18 insertions(+), 2 deletions(-)
>


2019-05-21 16:11:00

by Robin Murphy

[permalink] [raw]
Subject: Re: [RFC/PATCH 0/4] Initial support for modular IOMMU drivers

On 17/05/2019 19:47, Isaac J. Manjarres wrote:
> This series adds initial support for being able to use the ARM
> SMMU driver as a loadable kernel module. The series also adds
> to the IOMMU framework, so that it can defer probing for devices
> that depend on an IOMMU driver that may be a loadable module.
>
> The primary reason behind these changes is that having the ARM
> SMMU driver as a module allows for the same kernel image to be
> used across different platforms. For example, if one platform
> contains an IOMMU that implements one version of the ARM SMMU
> specification, and another platform simply does not have an
> IOMMU, the only way that these platforms can share the same
> kernel image is if the ARM SMMU driver is compiled into the
> kernel image.
>
> This solution is not scalable, as it will lead to bloating the
> kernel image with support for several future versions of the
> SMMU specification to maintain a common kernel image that works
> across all platforms.

There are currently two versions of the SMMU spec: v2 (which forms a
superset of v1), and v3 which is the current architecture. Given how
closely I work with the SMMU architecture team, I'm particularly
interested to hear more about these "future versions"... :)

> Having the ARM SMMU driver as a module allows
> for a common kernel image to be supported across all platforms,
> while yielding a smaller kernel image size, since the correct
> SMMU driver can be loaded at runtime, if necessary.

arm-smmu and arm-smmu-v3 aren't *all* that much bigger than any of the
other IOMMU drivers that are also present in a multiplatform build, and
already share quite a bit of common code, so while I can guess at what
you might really mean, it's a pretty weak argument as stated.

> Patchset Summary:
>
> 1. Since the ARM SMMU driver depends on symbols being exported from
> several subsystems, the first three patches are dedicated to exporting
> the necessary symbols.
>
> 2. Similar to how the pinctrl framework handles deferring probes,
> the subsequent patch makes it so that the IOMMU framework will defer
> probes indefinitely if there is a chance that the IOMMU driver that a
> device is waiting for is a module. Otherwise, it upholds the current
> behavior of stopping probe deferrals once all of the builtin drivers
> have finished probing.
>
> The ARM SMMU driver currently has support for the deprecated
> "mmu-masters" binding, which relies on the notion of initcall
> ordering for setting the bus ops to ensure that all SMMU devices
> have been bound to the driver. This poses a problem with
> making the driver a module, as there is no such notion with
> loadable modules. Will support for this be completely deprecated?
> If not, might it be useful to leverage the device tree ordering,
> and assign a property to the last SMMU device, and set the bus ops
> at that point? Or perhaps have some deferred timer based approach
> to know when to set the bus ops?

Unfortunately, I believe the old binding is still deployed in production
firmwares which may well never get updated, and thus needs to remain
functional (I've already had one report of the default bypass behaviour
breaking it in 5.2 which I need to fix somehow...)

Rather than just the trivial "export a bunch of symbols which won't
actually be needed yet", from the title I was hoping to see some patches
really making drivers modular and proposing solutions to those difficult
problems of making it work robustly. It's very easy to make it 'work' as
a proof-of-concept (locally I still have a patch dated 2016 based on the
original probe-deferral work), but those questions really want answering
to some degree before it's worth doing any of this in mainline.

Robin.

(now starting to wonder whether this might be my own fault for
mentioning it at LPC... :P)

>
> Thanks,
> Isaac
>
> Isaac J. Manjarres (4):
> of: Export of_phandle_iterator_args() to modules
> PCI: Export PCI ACS and DMA searching functions to modules
> iommu: Export core IOMMU functions to kernel modules
> iommu: Add probe deferral support for IOMMU kernel modules
>
> drivers/iommu/iommu-sysfs.c | 3 +++
> drivers/iommu/iommu.c | 6 ++++++
> drivers/iommu/of_iommu.c | 8 ++++++--
> drivers/of/base.c | 1 +
> drivers/pci/pci.c | 1 +
> drivers/pci/search.c | 1 +
> 6 files changed, 18 insertions(+), 2 deletions(-)
>

2019-05-22 21:03:15

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [RFC/PATCH 2/4] PCI: Export PCI ACS and DMA searching functions to modules

On Fri, May 17, 2019 at 11:47:35AM -0700, Isaac J. Manjarres wrote:
> IOMMU drivers that can be compiled as modules may
> want to use pci_for_each_dma_alias() and pci_request_acs(),
> so export those functions.
>
> Signed-off-by: Isaac J. Manjarres <[email protected]>

Acked-by: Bjorn Helgaas <[email protected]>

> ---
> drivers/pci/pci.c | 1 +
> drivers/pci/search.c | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 766f577..3f354c1 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -3124,6 +3124,7 @@ void pci_request_acs(void)
> {
> pci_acs_enable = 1;
> }
> +EXPORT_SYMBOL_GPL(pci_request_acs);
>
> static const char *disable_acs_redir_param;
>
> diff --git a/drivers/pci/search.c b/drivers/pci/search.c
> index 2b5f720..cf9ede9 100644
> --- a/drivers/pci/search.c
> +++ b/drivers/pci/search.c
> @@ -111,6 +111,7 @@ int pci_for_each_dma_alias(struct pci_dev *pdev,
>
> return ret;
> }
> +EXPORT_SYMBOL_GPL(pci_for_each_dma_alias);
>
> static struct pci_bus *pci_do_find_bus(struct pci_bus *bus, unsigned char busnr)
> {
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>