Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20191030145112.19738-1-will@kernel.org> <6e457227-ca06-2998-4ffa-a58ab171ce32@arm.com>
 <20191030155444.GC19096@willie-the-truck>
In-Reply-To: <20191030155444.GC19096@willie-the-truck>
From:   Saravana Kannan <saravanak@google.com>
Date:   Wed, 30 Oct 2019 17:57:44 -0700
Message-ID: <CAGETcx9ogWQC1ZtnS_4xC3ShqBpuRSKudWEEWC22UZUEhdEU4A@mail.gmail.com>
Subject: Re: [PATCH 0/7] iommu: Permit modular builds of ARM SMMU[v3] drivers
To:     Will Deacon <will@kernel.org>
Cc:     Robin Murphy <robin.murphy@arm.com>,
        iommu@lists.linux-foundation.org,
        LKML <linux-kernel@vger.kernel.org>,
        Joerg Roedel <joro@8bytes.org>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Oct 30, 2019 at 8:54 AM Will Deacon <will@kernel.org> wrote:
>
> Hi Robin,
>
> On Wed, Oct 30, 2019 at 03:35:55PM +0000, Robin Murphy wrote:
> > On 30/10/2019 14:51, Will Deacon wrote:
> > > As part of the work to enable a "Generic Kernel Image" across multiple
> > > Android devices, there is a need to seperate shared, core kernel code
> > > from modular driver code that may not be needed by all SoCs. This means
> > > building IOMMU drivers as modules.
> > >
> > > It turns out that most of the groundwork has already been done to enable
> > > the ARM SMMU drivers to be 'tristate' options in drivers/iommu/Kconfig;
> > > with a few symbols exported from the IOMMU/PCI core, everything builds
> > > nicely out of the box. The one exception is support for the legacy SMMU
> > > DT binding, which is not in widespread use and has never worked with
> > > modules, so we can simply remove that when building as a module rather
> > > than try to paper over it with even more hacks.
> > >
> > > Obviously you need to be careful about using IOMMU drivers as modules,
> > > since late loading of the driver for an IOMMU serving active DMA masters
> > > is going to end badly in many cases. On Android, we're using device links
> > > to ensure that the IOMMU probes first.
> >
> > Out of curiosity, which device links are those? Clearly not the RPM links
> > created by the IOMMU drivers themselves... Is this some special Android
> > magic, or is there actually a chance of replacing all the
> > of_iommu_configure() machinery with something more generic?
>
> I'll admit that I haven't used them personally yet, but I'm referring to
> this series from Saravana [CC'd]:
>
> https://lore.kernel.org/linux-acpi/20190904211126.47518-1-saravanak@google.com/
>
> which is currently sitting in linux-next now that we're upstreaming the
> "special Android magic" ;)

Hi Robin,

Actually, none of this is special Android magic. Will is talking about
the of_devlink feature that's been merged into driver-core-next.

A one line summary of of_devlink: the driver core + firmware (DT in
this case) automatically add the device links during device addition
based on the firmware properties of each device. The link that Will
gave has more details.

Wrt IOMMUs, the only missing piece in upstream is a trivial change
that does something like this in drivers/of/property.c

+static struct device_node *parse_iommus(struct device_node *np,
+                                        const char *prop_name, int index)
+{
+        return parse_prop_cells(np, prop_name, index, "iommus",
+                                "#iommu-cells");
+}

static const struct supplier_bindings of_supplier_bindings[] = {
        { .parse_prop = parse_clocks, },
        { .parse_prop = parse_interconnects, },
        { .parse_prop = parse_regulators, },
+        { .parse_prop = parse_iommus, },
        {},
};

I plan to upstream this pretty soon, but I have other patches in
flight that touch the same file and I'm waiting for those to get
accepted. I also want to clean up the code a bit to reduce some
repetition before I add support for more bindings.

Thanks,
Saravana