From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
To: Will Deacon <will.deacon@arm.com>
Cc: iommu@lists.linux-foundation.org,
Robin Murphy <robin.murphy@arm.com>,
Joerg Roedel <joro@8bytes.org>,
Marc Zyngier <marc.zyngier@arm.com>,
"Rafael J. Wysocki" <rjw@rjwysocki.net>,
Tomasz Nowicki <tn@semihalf.com>,
Hanjun Guo <hanjun.guo@linaro.org>, Jon Masters <jcm@redhat.com>,
Sinan Kaya <okaya@codeaurora.org>,
linux-acpi@vger.kernel.org, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
Subject: Re: [RFC PATCH v2 11/15] drivers: iommu: arm-smmu-v3: add IORT iommu configuration
Date: Wed, 15 Jun 2016 09:52:48 +0100 [thread overview]
Message-ID: <20160615085248.GA32390@red-moon> (raw)
In-Reply-To: <20160614183939.GL16531@arm.com>
On Tue, Jun 14, 2016 at 07:39:39PM +0100, Will Deacon wrote:
> On Tue, Jun 07, 2016 at 02:31:06PM +0100, Lorenzo Pieralisi wrote:
> > In ACPI bases systems, in order to be able to create platform
> > devices and initialize them for arm-smmu-v3 components, the IORT
> > infrastructure requires ARM SMMU drivers to initialize a set of
> > operations that are used by the IORT kernel layer to configure platform
> > devices for ARM SMMU components in turn.
> >
> > This patch adds the IORT IOMMU configuration for the ARM SMMU v3 kernel
> > driver.
> >
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Robin Murphy <robin.murphy@arm.com>
> > Cc: Joerg Roedel <joro@8bytes.org>
> > ---
> > drivers/iommu/arm-smmu-v3.c | 101 ++++++++++++++++++++++++++++++++++++++++++++
> > include/linux/iort.h | 12 +++++-
> > 2 files changed, 112 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> > index 90745a8..7acb6b5 100644
> > --- a/drivers/iommu/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm-smmu-v3.c
> > @@ -2687,6 +2687,107 @@ static int __init arm_smmu_of_init(struct device_node *np)
> > }
> > IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3", arm_smmu_of_init);
> >
> > +#ifdef CONFIG_ACPI
> > +static int acpi_smmu_init(struct acpi_iort_node *node)
> > +{
> > + iort_smmu_set_ops(node, &arm_smmu_ops, NULL);
> > +
> > + return 0;
> > +}
> > +
> > +static void acpi_smmu_register_irq(int hwirq, const char *name,
> > + struct resource *res)
> > +{
> > + int irq = acpi_register_gsi(NULL, hwirq, ACPI_EDGE_SENSITIVE,
> > + ACPI_ACTIVE_HIGH);
> > +
> > + if (irq < 0) {
> > + pr_err("could not register gsi hwirq %d name [%s]\n", hwirq,
> > + name);
> > + return;
> > + }
> > +
> > + res->start = irq;
> > + res->end = irq;
> > + res->flags = IORESOURCE_IRQ;
> > + res->name = name;
> > +}
> > +
> > +static int arm_smmu_count_resources(struct acpi_iort_node *node)
> > +{
> > + struct acpi_iort_smmu_v3 *smmu;
> > + /* Always present mem resource */
> > + int num_res = 1;
> > +
> > + /* Retrieve SMMUv3 specific data */
> > + smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
> > +
> > + if (smmu->event_gsiv)
> > + num_res++;
> > +
> > + if (smmu->pri_gsiv)
> > + num_res++;
> > +
> > + if (smmu->gerr_gsiv)
> > + num_res++;
> > +
> > + if (smmu->sync_gsiv)
> > + num_res++;
> > +
> > + return num_res;
> > +}
> > +
> > +static void arm_smmu_init_resources(struct resource *res,
> > + struct acpi_iort_node *node)
> > +{
> > + struct acpi_iort_smmu_v3 *smmu;
> > + int num_res = 0;
> > +
> > + /* Retrieve SMMUv3 specific data */
> > + smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
> > +
> > + res[num_res].start = smmu->base_address;
> > + res[num_res].end = smmu->base_address + SZ_128K - 1;
> > + res[num_res].flags = IORESOURCE_MEM;
> > +
> > + num_res++;
> > +
> > + if (smmu->event_gsiv)
> > + acpi_smmu_register_irq(smmu->event_gsiv, "eventq",
> > + &res[num_res++]);
> > +
> > + if (smmu->pri_gsiv)
> > + acpi_smmu_register_irq(smmu->pri_gsiv, "priq",
> > + &res[num_res++]);
> > +
> > + if (smmu->gerr_gsiv)
> > + acpi_smmu_register_irq(smmu->gerr_gsiv, "gerror",
> > + &res[num_res++]);
> > +
> > + if (smmu->sync_gsiv)
> > + acpi_smmu_register_irq(smmu->sync_gsiv, "cmdq-sync",
> > + &res[num_res++]);
> > +}
> > +
> > +static bool arm_smmu_is_coherent(struct acpi_iort_node *node)
> > +{
> > + struct acpi_iort_smmu_v3 *smmu;
> > +
> > + /* Retrieve SMMUv3 specific data */
> > + smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
> > +
> > + return smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE;
> > +}
> > +
> > +const struct iort_iommu_config iort_arm_smmu_v3_cfg = {
> > + .name = "arm-smmu-v3",
> > + .iommu_init = acpi_smmu_init,
> > + .iommu_is_coherent = arm_smmu_is_coherent,
> > + .iommu_count_resources = arm_smmu_count_resources,
> > + .iommu_init_resources = arm_smmu_init_resources
> > +};
> > +#endif
> > +
> > MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
> > MODULE_AUTHOR("Will Deacon <will.deacon@arm.com>");
> > MODULE_LICENSE("GPL v2");
> > diff --git a/include/linux/iort.h b/include/linux/iort.h
> > index ced3054..5dcfa09 100644
> > --- a/include/linux/iort.h
> > +++ b/include/linux/iort.h
> > @@ -55,7 +55,17 @@ struct iort_iommu_config {
> > static inline const struct iort_iommu_config *
> > iort_get_iommu_config(struct acpi_iort_node *node)
> > {
> > - return NULL;
> > + switch (node->type) {
> > +#if IS_ENABLED(CONFIG_ARM_SMMU_V3)
> > + case ACPI_IORT_NODE_SMMU_V3: {
> > + extern const struct iort_iommu_config iort_arm_smmu_v3_cfg;
> > +
> > + return &iort_arm_smmu_v3_cfg;
> > + }
> > +#endif
>
> Oh, yuck! I really don't like this mixture of SMMU driver code and IORT
> code over two files using a global structure of largely stateless function
> pointers.
>
> I think you have two options:
>
> (1) Move this all into iort.c (my preference)
> (2) Introduce something like SMMU_ACPI_DECLARE which allows drivers to
> register a callback if they're matched in the IORT.
>
> that at least keeps internal data in one place, rather than spreading it
> around.
(1) is possible for most of the function pointers. Problem is that iort.c
has to know ARM SMMU v3 driver internals (driver name for platform
device matching and IRQ resources names - I wanted to keep the ARM SMMU
v3 probing routine as FW agnostic as possible), it is obviously doable
but I have to make sure I keep them in sync.
We still need (2) to have an IOMMU_OF_DECLARE() equivalent (ie to
make sure that we can carry out the of_xlate() equivalent when
devices are probed), I did not want to add a linker script section
for two components (ARM SMMUv1/v2 and v3) but that's doable too,
I will make changes for v3.
Thanks for having a look !
Lorenzo
next prev parent reply other threads:[~2016-06-15 8:52 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-07 13:30 [RFC PATCH v2 00/15] ACPI IORT ARM SMMU v3 support Lorenzo Pieralisi
2016-06-07 13:30 ` [RFC PATCH v2 01/15] drivers: acpi: iort: fix struct pci_dev compiler warnings Lorenzo Pieralisi
2016-06-07 13:30 ` [RFC PATCH v2 02/15] drivers: irqchip: its: fix its_acpi_probe() prototype Lorenzo Pieralisi
2016-06-07 13:30 ` [RFC PATCH v2 03/15] arm64: mm: change IOMMU notifier action to attach DMA ops Lorenzo Pieralisi
2016-06-17 9:27 ` Robin Murphy
2016-06-17 14:15 ` Lorenzo Pieralisi
2016-06-23 11:32 ` Robin Murphy
2016-06-21 7:53 ` Marek Szyprowski
2016-06-21 16:06 ` Lorenzo Pieralisi
2016-06-23 6:13 ` Marek Szyprowski
2016-06-07 13:30 ` [RFC PATCH v2 04/15] drivers: acpi: iort: add support for IOMMU registration Lorenzo Pieralisi
2016-06-07 13:31 ` [RFC PATCH v2 05/15] drivers: acpi: iort: add support for named component look-up Lorenzo Pieralisi
2016-06-07 13:31 ` [RFC PATCH v2 06/15] drivers: acpi: iort: enhance device identifiers mappings Lorenzo Pieralisi
2016-06-07 13:31 ` [RFC PATCH v2 07/15] drivers: acpi: iort: add node match function Lorenzo Pieralisi
2016-06-07 13:31 ` [RFC PATCH v2 08/15] drivers: acpi: iort: add support for ARM SMMU platform devices creation Lorenzo Pieralisi
2016-06-07 13:31 ` [RFC PATCH v2 09/15] drivers: iommu: arm-smmu-v3: split probe functions into DT/generic portions Lorenzo Pieralisi
2016-06-14 18:09 ` Will Deacon
2016-06-07 13:31 ` [RFC PATCH v2 10/15] drivers: iommu: arm-smmu-v3: enable ACPI driver initialization Lorenzo Pieralisi
2016-06-14 18:12 ` Will Deacon
2016-06-07 13:31 ` [RFC PATCH v2 11/15] drivers: iommu: arm-smmu-v3: add IORT iommu configuration Lorenzo Pieralisi
2016-06-14 18:39 ` Will Deacon
2016-06-15 8:52 ` Lorenzo Pieralisi [this message]
2016-06-07 13:31 ` [RFC PATCH v2 12/15] drivers: acpi: implement acpi_dma_configure Lorenzo Pieralisi
2016-06-10 16:25 ` Bjorn Helgaas
2016-06-07 13:31 ` [RFC PATCH v2 13/15] drivers: acpi: iort: introduce iort_iommu_configure Lorenzo Pieralisi
2016-06-10 12:46 ` Tomasz Nowicki
2016-06-07 13:31 ` [RFC PATCH v2 14/15] drivers: acpi: iort: add function to retrieve IOMMU platform devices Lorenzo Pieralisi
2016-06-07 13:31 ` [RFC PATCH v2 15/15] drivers: iommu: arm-smmu-v3: allow ACPI based streamid translation Lorenzo Pieralisi
2016-06-21 10:37 ` [RFC PATCH v2 00/15] ACPI IORT ARM SMMU v3 support Hanjun Guo
2016-06-21 14:27 ` Lorenzo Pieralisi
2016-06-22 2:45 ` Hanjun Guo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160615085248.GA32390@red-moon \
--to=lorenzo.pieralisi@arm.com \
--cc=hanjun.guo@linaro.org \
--cc=iommu@lists.linux-foundation.org \
--cc=jcm@redhat.com \
--cc=joro@8bytes.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=marc.zyngier@arm.com \
--cc=okaya@codeaurora.org \
--cc=rjw@rjwysocki.net \
--cc=robin.murphy@arm.com \
--cc=tn@semihalf.com \
--cc=will.deacon@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).