ACPI support is added in the fsl-mc driver. Driver will parse
MC DSDT table to extract memory and other resorces.
Interrupt (GIC ITS) information will be extracted from MADT table
by drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c.
IORT table will be parsed to configure DMA.
Signed-off-by: Makarand Pawagi <[email protected]>
---
drivers/acpi/arm64/iort.c | 53 +++++++++++++++++++++
drivers/bus/fsl-mc/dprc-driver.c | 3 +-
drivers/bus/fsl-mc/fsl-mc-bus.c | 48 +++++++++++++------
drivers/bus/fsl-mc/fsl-mc-msi.c | 10 +++-
drivers/bus/fsl-mc/fsl-mc-private.h | 4 +-
drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c | 71 ++++++++++++++++++++++++++++-
include/linux/acpi_iort.h | 5 ++
7 files changed, 174 insertions(+), 20 deletions(-)
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 33f7198..beb9cd5 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -15,6 +15,7 @@
#include <linux/kernel.h>
#include <linux/list.h>
#include <linux/pci.h>
+#include <linux/fsl/mc.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
@@ -622,6 +623,29 @@ static int iort_dev_find_its_id(struct device *dev, u32 req_id,
}
/**
+ * iort_get_fsl_mc_device_domain() - Find MSI domain related to a device
+ * @dev: The device.
+ * @mc_icid: ICID for the fsl_mc device.
+ *
+ * Returns: the MSI domain for this device, NULL otherwise
+ */
+struct irq_domain *iort_get_fsl_mc_device_domain(struct device *dev,
+ u32 mc_icid)
+{
+ struct fwnode_handle *handle;
+ int its_id;
+
+ if (iort_dev_find_its_id(dev, mc_icid, 0, &its_id))
+ return NULL;
+
+ handle = iort_find_domain_token(its_id);
+ if (!handle)
+ return NULL;
+
+ return irq_find_matching_fwnode(handle, DOMAIN_BUS_FSL_MC_MSI);
+}
+
+/**
* iort_get_device_domain() - Find MSI domain related to a device
* @dev: The device.
* @req_id: Requester ID for the device.
@@ -924,6 +948,21 @@ static int iort_pci_iommu_init(struct pci_dev *pdev, u16 alias, void *data)
return iort_iommu_xlate(info->dev, parent, streamid);
}
+static int iort_fsl_mc_iommu_init(struct device *dev,
+ struct acpi_iort_node *node, u32 *streamid)
+{
+ struct acpi_iort_node *parent;
+ struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
+
+ parent = iort_node_map_id(node, mc_dev->icid, streamid,
+ IORT_IOMMU_TYPE);
+
+ if (parent)
+ return iort_iommu_xlate(dev, parent, *streamid);
+
+ return 0;
+}
+
/**
* iort_iommu_configure - Set-up IOMMU configuration for a device.
*
@@ -962,6 +1001,20 @@ const struct iommu_ops *iort_iommu_configure(struct device *dev)
if (!err && iort_pci_rc_supports_ats(node))
dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS;
+ } else if (dev_is_fsl_mc(dev)) {
+ struct device *dma_dev = dev;
+
+ if (!(to_acpi_device_node(dma_dev->fwnode))) {
+ while (dev_is_fsl_mc(dma_dev))
+ dma_dev = dma_dev->parent;
+ }
+
+ node = iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
+ iort_match_node_callback, dma_dev);
+ if (!node)
+ return NULL;
+
+ err = iort_fsl_mc_iommu_init(dev, node, &streamid);
} else {
int i = 0;
diff --git a/drivers/bus/fsl-mc/dprc-driver.c b/drivers/bus/fsl-mc/dprc-driver.c
index c8b1c38..31dd790 100644
--- a/drivers/bus/fsl-mc/dprc-driver.c
+++ b/drivers/bus/fsl-mc/dprc-driver.c
@@ -4,6 +4,7 @@
*
* Copyright (C) 2014-2016 Freescale Semiconductor, Inc.
* Author: German Rivera <[email protected]>
+ * Copyright 2018-2020 NXP
*
*/
@@ -638,7 +639,7 @@ static int dprc_probe(struct fsl_mc_device *mc_dev)
return -EINVAL;
error = fsl_mc_find_msi_domain(parent_dev,
- &mc_msi_domain);
+ &mc_msi_domain, mc_dev);
if (error < 0) {
dev_warn(&mc_dev->dev,
"WARNING: MC bus without interrupt support\n");
diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
index a07cc19..5d388e4 100644
--- a/drivers/bus/fsl-mc/fsl-mc-bus.c
+++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
@@ -4,11 +4,13 @@
*
* Copyright (C) 2014-2016 Freescale Semiconductor, Inc.
* Author: German Rivera <[email protected]>
+ * Copyright 2018-2020 NXP
*
*/
#define pr_fmt(fmt) "fsl-mc: " fmt
+#include <linux/acpi.h>
#include <linux/module.h>
#include <linux/of_device.h>
#include <linux/of_address.h>
@@ -129,12 +131,25 @@ static int fsl_mc_bus_uevent(struct device *dev, struct kobj_uevent_env *env)
static int fsl_mc_dma_configure(struct device *dev)
{
+ enum dev_dma_attr attr;
struct device *dma_dev = dev;
+ int err = -ENODEV;
while (dev_is_fsl_mc(dma_dev))
dma_dev = dma_dev->parent;
- return of_dma_configure(dev, dma_dev->of_node, 0);
+ if (dma_dev->of_node)
+ err = of_dma_configure(dev, dma_dev->of_node, 1);
+ else {
+ if (has_acpi_companion(dma_dev)) {
+ attr = acpi_get_dma_attr(to_acpi_device_node
+ (dma_dev->fwnode));
+ if (attr != DEV_DMA_NOT_SUPPORTED)
+ err = acpi_dma_configure(dev, attr);
+ }
+ }
+
+ return err;
}
static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
@@ -863,7 +878,7 @@ static int fsl_mc_bus_probe(struct platform_device *pdev)
phys_addr_t mc_portal_phys_addr;
u32 mc_portal_size;
struct mc_version mc_version;
- struct resource res;
+ struct resource *plat_res;
mc = devm_kzalloc(&pdev->dev, sizeof(*mc), GFP_KERNEL);
if (!mc)
@@ -874,16 +889,9 @@ static int fsl_mc_bus_probe(struct platform_device *pdev)
/*
* Get physical address of MC portal for the root DPRC:
*/
- error = of_address_to_resource(pdev->dev.of_node, 0, &res);
- if (error < 0) {
- dev_err(&pdev->dev,
- "of_address_to_resource() failed for %pOF\n",
- pdev->dev.of_node);
- return error;
- }
-
- mc_portal_phys_addr = res.start;
- mc_portal_size = resource_size(&res);
+ plat_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ mc_portal_phys_addr = plat_res->start;
+ mc_portal_size = resource_size(plat_res);
error = fsl_create_mc_io(&pdev->dev, mc_portal_phys_addr,
mc_portal_size, NULL,
FSL_MC_IO_ATOMIC_CONTEXT_PORTAL, &mc_io);
@@ -900,11 +908,13 @@ static int fsl_mc_bus_probe(struct platform_device *pdev)
dev_info(&pdev->dev, "MC firmware version: %u.%u.%u\n",
mc_version.major, mc_version.minor, mc_version.revision);
- error = get_mc_addr_translation_ranges(&pdev->dev,
+ if (dev_of_node(&pdev->dev)) {
+ error = get_mc_addr_translation_ranges(&pdev->dev,
&mc->translation_ranges,
&mc->num_translation_ranges);
- if (error < 0)
- goto error_cleanup_mc_io;
+ if (error < 0)
+ goto error_cleanup_mc_io;
+ }
error = dprc_get_container_id(mc_io, 0, &container_id);
if (error < 0) {
@@ -931,6 +941,7 @@ static int fsl_mc_bus_probe(struct platform_device *pdev)
goto error_cleanup_mc_io;
mc->root_mc_bus_dev = mc_bus_dev;
+ mc_bus_dev->dev.fwnode = pdev->dev.fwnode;
return 0;
error_cleanup_mc_io:
@@ -964,11 +975,18 @@ static const struct of_device_id fsl_mc_bus_match_table[] = {
MODULE_DEVICE_TABLE(of, fsl_mc_bus_match_table);
+static const struct acpi_device_id fsl_mc_bus_acpi_match_table[] = {
+ {"NXP0008", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, fsl_mc_bus_acpi_match_table);
+
static struct platform_driver fsl_mc_bus_driver = {
.driver = {
.name = "fsl_mc_bus",
.pm = NULL,
.of_match_table = fsl_mc_bus_match_table,
+ .acpi_match_table = fsl_mc_bus_acpi_match_table,
},
.probe = fsl_mc_bus_probe,
.remove = fsl_mc_bus_remove,
diff --git a/drivers/bus/fsl-mc/fsl-mc-msi.c b/drivers/bus/fsl-mc/fsl-mc-msi.c
index 8b9c66d..bd10952 100644
--- a/drivers/bus/fsl-mc/fsl-mc-msi.c
+++ b/drivers/bus/fsl-mc/fsl-mc-msi.c
@@ -4,6 +4,7 @@
*
* Copyright (C) 2015-2016 Freescale Semiconductor, Inc.
* Author: German Rivera <[email protected]>
+ * Copyright 2020 NXP
*
*/
@@ -13,6 +14,7 @@
#include <linux/irq.h>
#include <linux/irqdomain.h>
#include <linux/msi.h>
+#include <linux/acpi_iort.h>
#include "fsl-mc-private.h"
@@ -178,13 +180,19 @@ struct irq_domain *fsl_mc_msi_create_irq_domain(struct fwnode_handle *fwnode,
}
int fsl_mc_find_msi_domain(struct device *mc_platform_dev,
- struct irq_domain **mc_msi_domain)
+ struct irq_domain **mc_msi_domain,
+ struct fsl_mc_device *mc_dev)
{
struct irq_domain *msi_domain;
struct device_node *mc_of_node = mc_platform_dev->of_node;
msi_domain = of_msi_get_domain(mc_platform_dev, mc_of_node,
DOMAIN_BUS_FSL_MC_MSI);
+
+ if (!msi_domain)
+ msi_domain = iort_get_fsl_mc_device_domain(mc_platform_dev,
+ mc_dev->icid);
+
if (!msi_domain) {
pr_err("Unable to find fsl-mc MSI domain for %pOF\n",
mc_of_node);
diff --git a/drivers/bus/fsl-mc/fsl-mc-private.h b/drivers/bus/fsl-mc/fsl-mc-private.h
index 21ca8c756..e8f4c0f 100644
--- a/drivers/bus/fsl-mc/fsl-mc-private.h
+++ b/drivers/bus/fsl-mc/fsl-mc-private.h
@@ -3,6 +3,7 @@
* Freescale Management Complex (MC) bus private declarations
*
* Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Copyright 2020 NXP
*
*/
#ifndef _FSL_MC_PRIVATE_H_
@@ -596,7 +597,8 @@ int fsl_mc_msi_domain_alloc_irqs(struct device *dev,
void fsl_mc_msi_domain_free_irqs(struct device *dev);
int fsl_mc_find_msi_domain(struct device *mc_platform_dev,
- struct irq_domain **mc_msi_domain);
+ struct irq_domain **mc_msi_domain,
+ struct fsl_mc_device *mc_dev);
int fsl_mc_populate_irq_pool(struct fsl_mc_bus *mc_bus,
unsigned int irq_count);
diff --git a/drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c b/drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c
index 606efa6..df99170 100644
--- a/drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c
+++ b/drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c
@@ -4,9 +4,11 @@
*
* Copyright (C) 2015-2016 Freescale Semiconductor, Inc.
* Author: German Rivera <[email protected]>
+ * Copyright 2020 NXP
*
*/
+#include <linux/acpi_iort.h>
#include <linux/of_device.h>
#include <linux/of_address.h>
#include <linux/irq.h>
@@ -66,7 +68,66 @@ static const struct of_device_id its_device_id[] = {
{},
};
-static int __init its_fsl_mc_msi_init(void)
+static int __init its_fsl_mc_msi_init_one(struct fwnode_handle *handle,
+ const char *name)
+{
+ struct irq_domain *parent;
+ struct irq_domain *mc_msi_domain;
+
+ parent = irq_find_matching_fwnode(handle, DOMAIN_BUS_NEXUS);
+ if (!parent || !msi_get_domain_info(parent)) {
+ pr_err("%s: Unable to locate ITS domain\n", name);
+ return -ENXIO;
+ }
+
+ mc_msi_domain = fsl_mc_msi_create_irq_domain(
+ handle,
+ &its_fsl_mc_msi_domain_info,
+ parent);
+ if (!mc_msi_domain)
+ pr_err("ACPIF: unable to create fsl-mc domain\n");
+
+ pr_info("fsl-mc MSI: domain created\n");
+
+ return 0;
+}
+
+static int __init
+its_fsl_mc_msi_parse_madt(union acpi_subtable_headers *header,
+ const unsigned long end)
+{
+ struct acpi_madt_generic_translator *its_entry;
+ struct fwnode_handle *dom_handle;
+ const char *node_name;
+ int err = -ENXIO;
+
+ its_entry = (struct acpi_madt_generic_translator *)header;
+ node_name = kasprintf(GFP_KERNEL, "ITS@0x%lx",
+ (long)its_entry->base_address);
+
+ dom_handle = iort_find_domain_token(its_entry->translation_id);
+ if (!dom_handle) {
+ pr_err("%s: Unable to locate ITS domain handle\n", node_name);
+ goto out;
+ }
+
+ err = its_fsl_mc_msi_init_one(dom_handle, node_name);
+ if (!err)
+ pr_info("fsl-mc MSI: %s domain created\n", node_name);
+
+out:
+ kfree(node_name);
+ return err;
+}
+
+static int __init its_fsl_mc_acpi_msi_init(void)
+{
+ acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
+ its_fsl_mc_msi_parse_madt, 0);
+ return 0;
+}
+
+static int __init its_fsl_mc_of_msi_init(void)
{
struct device_node *np;
struct irq_domain *parent;
@@ -96,8 +157,14 @@ static int __init its_fsl_mc_msi_init(void)
pr_info("fsl-mc MSI: %pOF domain created\n", np);
}
-
return 0;
}
+static int __init its_fsl_mc_msi_init(void)
+{
+ its_fsl_mc_of_msi_init();
+ its_fsl_mc_acpi_msi_init();
+
+ return 0;
+}
early_initcall(its_fsl_mc_msi_init);
diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
index 8e7e2ec..0afc608 100644
--- a/include/linux/acpi_iort.h
+++ b/include/linux/acpi_iort.h
@@ -30,6 +30,8 @@ struct fwnode_handle *iort_find_domain_token(int trans_id);
void acpi_iort_init(void);
u32 iort_msi_map_rid(struct device *dev, u32 req_id);
struct irq_domain *iort_get_device_domain(struct device *dev, u32 req_id);
+struct irq_domain *iort_get_fsl_mc_device_domain(struct device *dev,
+ u32 req_id);
void acpi_configure_pmsi_domain(struct device *dev);
int iort_pmsi_get_dev_id(struct device *dev, u32 *dev_id);
/* IOMMU interface */
@@ -40,6 +42,9 @@ int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head);
static inline void acpi_iort_init(void) { }
static inline u32 iort_msi_map_rid(struct device *dev, u32 req_id)
{ return req_id; }
+static inline struct irq_domain *iort_get_fsl_mc_device_domain
+ (struct device *dev, u32 req_id)
+{ return NULL; }
static inline struct irq_domain *iort_get_device_domain(struct device *dev,
u32 req_id)
{ return NULL; }
--
2.7.4
On 2020-01-28 08:08, Makarand Pawagi wrote:
> ACPI support is added in the fsl-mc driver. Driver will parse
> MC DSDT table to extract memory and other resorces.
>
> Interrupt (GIC ITS) information will be extracted from MADT table
> by drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c.
>
> IORT table will be parsed to configure DMA.
>
> Signed-off-by: Makarand Pawagi <[email protected]>
> ---
> drivers/acpi/arm64/iort.c | 53 +++++++++++++++++++++
> drivers/bus/fsl-mc/dprc-driver.c | 3 +-
> drivers/bus/fsl-mc/fsl-mc-bus.c | 48 +++++++++++++------
> drivers/bus/fsl-mc/fsl-mc-msi.c | 10 +++-
> drivers/bus/fsl-mc/fsl-mc-private.h | 4 +-
> drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c | 71
> ++++++++++++++++++++++++++++-
> include/linux/acpi_iort.h | 5 ++
> 7 files changed, 174 insertions(+), 20 deletions(-)
A general comment when you do this kind of work:
Do not write a single patch that impacts at least three different
subsystems. As it is, it is unmergeable.
Now the real question is *WHY* we need this kind of monstruosity?
ACPI deals with PCI, not with exotic busses and whatnot. If you want
to be creative, DT is your space. ACPI is designed to be plain and
boring, and that's how we like it.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
On Tue, Jan 28, 2020 at 01:38:45PM +0530, Makarand Pawagi wrote:
> ACPI support is added in the fsl-mc driver. Driver will parse
> MC DSDT table to extract memory and other resorces.
>
> Interrupt (GIC ITS) information will be extracted from MADT table
> by drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c.
>
> IORT table will be parsed to configure DMA.
>
> Signed-off-by: Makarand Pawagi <[email protected]>
> ---
> drivers/acpi/arm64/iort.c | 53 +++++++++++++++++++++
> drivers/bus/fsl-mc/dprc-driver.c | 3 +-
> drivers/bus/fsl-mc/fsl-mc-bus.c | 48 +++++++++++++------
> drivers/bus/fsl-mc/fsl-mc-msi.c | 10 +++-
> drivers/bus/fsl-mc/fsl-mc-private.h | 4 +-
> drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c | 71 ++++++++++++++++++++++++++++-
> include/linux/acpi_iort.h | 5 ++
> 7 files changed, 174 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index 33f7198..beb9cd5 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -15,6 +15,7 @@
> #include <linux/kernel.h>
> #include <linux/list.h>
> #include <linux/pci.h>
> +#include <linux/fsl/mc.h>
> #include <linux/platform_device.h>
> #include <linux/slab.h>
>
> @@ -622,6 +623,29 @@ static int iort_dev_find_its_id(struct device *dev, u32 req_id,
> }
>
> /**
> + * iort_get_fsl_mc_device_domain() - Find MSI domain related to a device
> + * @dev: The device.
> + * @mc_icid: ICID for the fsl_mc device.
> + *
> + * Returns: the MSI domain for this device, NULL otherwise
> + */
> +struct irq_domain *iort_get_fsl_mc_device_domain(struct device *dev,
> + u32 mc_icid)
> +{
> + struct fwnode_handle *handle;
> + int its_id;
> +
> + if (iort_dev_find_its_id(dev, mc_icid, 0, &its_id))
> + return NULL;
> +
> + handle = iort_find_domain_token(its_id);
> + if (!handle)
> + return NULL;
> +
> + return irq_find_matching_fwnode(handle, DOMAIN_BUS_FSL_MC_MSI);
> +}
NAK
I am not willing to take platform specific code in the generic IORT
layer.
ACPI on ARM64 works on platforms that comply with SBSA/SBBR guidelines:
https://developer.arm.com/architectures/platform-design/server-systems
Deviating from those requires butchering ACPI specifications (ie IORT)
and related kernel code which goes totally against what ACPI is meant
for on ARM64 systems, so there is no upstream pathway for this code
I am afraid.
Thanks,
Lorenzo
> -----Original Message-----
> From: Lorenzo Pieralisi <[email protected]>
> Sent: Tuesday, January 28, 2020 4:39 PM
> To: Makarand Pawagi <[email protected]>
> Cc: [email protected]; [email protected]; linux-arm-
> [email protected]; [email protected]; [email protected];
> [email protected]; Cristi Sovaiala <[email protected]>; Laurentiu
> Tudor <[email protected]>; Ioana Ciornei <[email protected]>;
> Varun Sethi <[email protected]>; Calvin Johnson <[email protected]>;
> Pankaj Bansal <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: [EXT] Re: [PATCH] bus: fsl-mc: Add ACPI support for fsl-mc
>
> Caution: EXT Email
>
> On Tue, Jan 28, 2020 at 01:38:45PM +0530, Makarand Pawagi wrote:
> > ACPI support is added in the fsl-mc driver. Driver will parse MC DSDT
> > table to extract memory and other resorces.
> >
> > Interrupt (GIC ITS) information will be extracted from MADT table by
> > drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c.
> >
> > IORT table will be parsed to configure DMA.
> >
> > Signed-off-by: Makarand Pawagi <[email protected]>
> > ---
> > drivers/acpi/arm64/iort.c | 53 +++++++++++++++++++++
> > drivers/bus/fsl-mc/dprc-driver.c | 3 +-
> > drivers/bus/fsl-mc/fsl-mc-bus.c | 48 +++++++++++++------
> > drivers/bus/fsl-mc/fsl-mc-msi.c | 10 +++-
> > drivers/bus/fsl-mc/fsl-mc-private.h | 4 +-
> > drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c | 71
> ++++++++++++++++++++++++++++-
> > include/linux/acpi_iort.h | 5 ++
> > 7 files changed, 174 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> > index 33f7198..beb9cd5 100644
> > --- a/drivers/acpi/arm64/iort.c
> > +++ b/drivers/acpi/arm64/iort.c
> > @@ -15,6 +15,7 @@
> > #include <linux/kernel.h>
> > #include <linux/list.h>
> > #include <linux/pci.h>
> > +#include <linux/fsl/mc.h>
> > #include <linux/platform_device.h>
> > #include <linux/slab.h>
> >
> > @@ -622,6 +623,29 @@ static int iort_dev_find_its_id(struct device
> > *dev, u32 req_id, }
> >
> > /**
> > + * iort_get_fsl_mc_device_domain() - Find MSI domain related to a
> > +device
> > + * @dev: The device.
> > + * @mc_icid: ICID for the fsl_mc device.
> > + *
> > + * Returns: the MSI domain for this device, NULL otherwise */ struct
> > +irq_domain *iort_get_fsl_mc_device_domain(struct device *dev,
> > + u32 mc_icid) {
> > + struct fwnode_handle *handle;
> > + int its_id;
> > +
> > + if (iort_dev_find_its_id(dev, mc_icid, 0, &its_id))
> > + return NULL;
> > +
> > + handle = iort_find_domain_token(its_id);
> > + if (!handle)
> > + return NULL;
> > +
> > + return irq_find_matching_fwnode(handle, DOMAIN_BUS_FSL_MC_MSI);
> > +}
>
> NAK
>
> I am not willing to take platform specific code in the generic IORT layer.
>
> ACPI on ARM64 works on platforms that comply with SBSA/SBBR guidelines:
>
>
> https://developer.arm.com/architectures/platform-design/server-systems
>
> Deviating from those requires butchering ACPI specifications (ie IORT) and
> related kernel code which goes totally against what ACPI is meant for on ARM64
> systems, so there is no upstream pathway for this code I am afraid.
>
Reason of adding this platform specific function in the generic IORT layer is
That iort_get_device_domain() only deals with PCI bus (DOMAIN_BUS_PCI_MSI).
fsl-mc objects when probed, need to find irq_domain which is associated with
the fsl-mc bus (DOMAIN_BUS_FSL_MC_MSI). It will not be possible to do that
if we do not add this function because there are no other suitable APIs exported
by IORT layer to do the job.
On Fri, Jan 31, 2020 at 10:35:48AM +0000, Makarand Pawagi wrote:
> > -----Original Message-----
> > From: Lorenzo Pieralisi <[email protected]>
> > Sent: Tuesday, January 28, 2020 4:39 PM
> > To: Makarand Pawagi <[email protected]>
> > Cc: [email protected]; [email protected]; linux-arm-
> > [email protected]; [email protected]; [email protected];
> > [email protected]; Cristi Sovaiala <[email protected]>; Laurentiu
> > Tudor <[email protected]>; Ioana Ciornei <[email protected]>;
> > Varun Sethi <[email protected]>; Calvin Johnson <[email protected]>;
> > Pankaj Bansal <[email protected]>; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]
> > Subject: [EXT] Re: [PATCH] bus: fsl-mc: Add ACPI support for fsl-mc
> >
> > Caution: EXT Email
> >
> > On Tue, Jan 28, 2020 at 01:38:45PM +0530, Makarand Pawagi wrote:
> > > ACPI support is added in the fsl-mc driver. Driver will parse MC DSDT
> > > table to extract memory and other resorces.
> > >
> > > Interrupt (GIC ITS) information will be extracted from MADT table by
> > > drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c.
> > >
> > > IORT table will be parsed to configure DMA.
> > >
> > > Signed-off-by: Makarand Pawagi <[email protected]>
> > > ---
> > > drivers/acpi/arm64/iort.c | 53 +++++++++++++++++++++
> > > drivers/bus/fsl-mc/dprc-driver.c | 3 +-
> > > drivers/bus/fsl-mc/fsl-mc-bus.c | 48 +++++++++++++------
> > > drivers/bus/fsl-mc/fsl-mc-msi.c | 10 +++-
> > > drivers/bus/fsl-mc/fsl-mc-private.h | 4 +-
> > > drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c | 71
> > ++++++++++++++++++++++++++++-
> > > include/linux/acpi_iort.h | 5 ++
> > > 7 files changed, 174 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> > > index 33f7198..beb9cd5 100644
> > > --- a/drivers/acpi/arm64/iort.c
> > > +++ b/drivers/acpi/arm64/iort.c
> > > @@ -15,6 +15,7 @@
> > > #include <linux/kernel.h>
> > > #include <linux/list.h>
> > > #include <linux/pci.h>
> > > +#include <linux/fsl/mc.h>
> > > #include <linux/platform_device.h>
> > > #include <linux/slab.h>
> > >
> > > @@ -622,6 +623,29 @@ static int iort_dev_find_its_id(struct device
> > > *dev, u32 req_id, }
> > >
> > > /**
> > > + * iort_get_fsl_mc_device_domain() - Find MSI domain related to a
> > > +device
> > > + * @dev: The device.
> > > + * @mc_icid: ICID for the fsl_mc device.
> > > + *
> > > + * Returns: the MSI domain for this device, NULL otherwise */ struct
> > > +irq_domain *iort_get_fsl_mc_device_domain(struct device *dev,
> > > + u32 mc_icid) {
> > > + struct fwnode_handle *handle;
> > > + int its_id;
> > > +
> > > + if (iort_dev_find_its_id(dev, mc_icid, 0, &its_id))
> > > + return NULL;
> > > +
> > > + handle = iort_find_domain_token(its_id);
> > > + if (!handle)
> > > + return NULL;
> > > +
> > > + return irq_find_matching_fwnode(handle, DOMAIN_BUS_FSL_MC_MSI);
> > > +}
> >
> > NAK
> >
> > I am not willing to take platform specific code in the generic IORT layer.
> >
> > ACPI on ARM64 works on platforms that comply with SBSA/SBBR guidelines:
> >
> >
> > https://developer.arm.com/architectures/platform-design/server-systems
> >
> > Deviating from those requires butchering ACPI specifications (ie IORT) and
> > related kernel code which goes totally against what ACPI is meant for on ARM64
> > systems, so there is no upstream pathway for this code I am afraid.
> >
> Reason of adding this platform specific function in the generic IORT
> layer is That iort_get_device_domain() only deals with PCI bus
> (DOMAIN_BUS_PCI_MSI).
>
> fsl-mc objects when probed, need to find irq_domain which is
> associated with the fsl-mc bus (DOMAIN_BUS_FSL_MC_MSI). It will not be
> possible to do that if we do not add this function because there are
> no other suitable APIs exported by IORT layer to do the job.
And that's by design.
I don't know what the FSL bus is and I don't want to know, what
I am telling you is that the ACPI code in the mainline is sufficient
to support SBSA compliant HW and that's what we support with ACPI
on ARM64.
We won't hack the kernel (and ACPI tables) up to boot with ACPI on
non-compliant platforms, I don't know how I can be any clearer than
that.
All is needed to configure the (platform dev/PCI->IOMMU->ITS) chain is
in the ACPI/IORT specifications and again, that's by design, adding
DSDT objects and hacking the kernel to make it work "like DT" won't
cut it, you are solving the wrong problem here, boot this platform
with a device tree, it is a problem that has been solved a long time
ago and it is supported in the mainline kernel.
Thanks,
Lorenzo
On 2020-01-31 10:35, Makarand Pawagi wrote:
>> -----Original Message-----
>> From: Lorenzo Pieralisi <[email protected]>
>> Sent: Tuesday, January 28, 2020 4:39 PM
>> To: Makarand Pawagi <[email protected]>
>> Cc: [email protected]; [email protected]; linux-arm-
>> [email protected]; [email protected];
>> [email protected];
>> [email protected]; Cristi Sovaiala <[email protected]>;
>> Laurentiu
>> Tudor <[email protected]>; Ioana Ciornei
>> <[email protected]>;
>> Varun Sethi <[email protected]>; Calvin Johnson
>> <[email protected]>;
>> Pankaj Bansal <[email protected]>; [email protected];
>> [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected]
>> Subject: [EXT] Re: [PATCH] bus: fsl-mc: Add ACPI support for fsl-mc
>>
>> Caution: EXT Email
>>
>> On Tue, Jan 28, 2020 at 01:38:45PM +0530, Makarand Pawagi wrote:
>> > ACPI support is added in the fsl-mc driver. Driver will parse MC DSDT
>> > table to extract memory and other resorces.
>> >
>> > Interrupt (GIC ITS) information will be extracted from MADT table by
>> > drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c.
>> >
>> > IORT table will be parsed to configure DMA.
>> >
>> > Signed-off-by: Makarand Pawagi <[email protected]>
>> > ---
>> > drivers/acpi/arm64/iort.c | 53 +++++++++++++++++++++
>> > drivers/bus/fsl-mc/dprc-driver.c | 3 +-
>> > drivers/bus/fsl-mc/fsl-mc-bus.c | 48 +++++++++++++------
>> > drivers/bus/fsl-mc/fsl-mc-msi.c | 10 +++-
>> > drivers/bus/fsl-mc/fsl-mc-private.h | 4 +-
>> > drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c | 71
>> ++++++++++++++++++++++++++++-
>> > include/linux/acpi_iort.h | 5 ++
>> > 7 files changed, 174 insertions(+), 20 deletions(-)
>> >
>> > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
>> > index 33f7198..beb9cd5 100644
>> > --- a/drivers/acpi/arm64/iort.c
>> > +++ b/drivers/acpi/arm64/iort.c
>> > @@ -15,6 +15,7 @@
>> > #include <linux/kernel.h>
>> > #include <linux/list.h>
>> > #include <linux/pci.h>
>> > +#include <linux/fsl/mc.h>
>> > #include <linux/platform_device.h>
>> > #include <linux/slab.h>
>> >
>> > @@ -622,6 +623,29 @@ static int iort_dev_find_its_id(struct device
>> > *dev, u32 req_id, }
>> >
>> > /**
>> > + * iort_get_fsl_mc_device_domain() - Find MSI domain related to a
>> > +device
>> > + * @dev: The device.
>> > + * @mc_icid: ICID for the fsl_mc device.
>> > + *
>> > + * Returns: the MSI domain for this device, NULL otherwise */ struct
>> > +irq_domain *iort_get_fsl_mc_device_domain(struct device *dev,
>> > + u32 mc_icid) {
>> > + struct fwnode_handle *handle;
>> > + int its_id;
>> > +
>> > + if (iort_dev_find_its_id(dev, mc_icid, 0, &its_id))
>> > + return NULL;
>> > +
>> > + handle = iort_find_domain_token(its_id);
>> > + if (!handle)
>> > + return NULL;
>> > +
>> > + return irq_find_matching_fwnode(handle, DOMAIN_BUS_FSL_MC_MSI);
>> > +}
>>
>> NAK
>>
>> I am not willing to take platform specific code in the generic IORT
>> layer.
>>
>> ACPI on ARM64 works on platforms that comply with SBSA/SBBR
>> guidelines:
>>
>>
>> https://developer.arm.com/architectures/platform-design/server-systems
>>
>> Deviating from those requires butchering ACPI specifications (ie IORT)
>> and
>> related kernel code which goes totally against what ACPI is meant for
>> on ARM64
>> systems, so there is no upstream pathway for this code I am afraid.
>>
> Reason of adding this platform specific function in the generic IORT
> layer is
> That iort_get_device_domain() only deals with PCI bus
> (DOMAIN_BUS_PCI_MSI).
>
> fsl-mc objects when probed, need to find irq_domain which is associated
> with
> the fsl-mc bus (DOMAIN_BUS_FSL_MC_MSI). It will not be possible to do
> that
> if we do not add this function because there are no other suitable APIs
> exported
> by IORT layer to do the job.
I think we all understood the patch. What both Lorenzo and myself are
saying is
that we do not want non-PCI support in IORT.
You have decided to have exotic hardware, and sidestep all the
standardization
efforts. This is your right. But you can't have your cake and eat it.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
On Fri, 31 Jan 2020 at 12:06, Marc Zyngier <[email protected]> wrote:
>
> On 2020-01-31 10:35, Makarand Pawagi wrote:
> >> -----Original Message-----
> >> From: Lorenzo Pieralisi <[email protected]>
> >> Sent: Tuesday, January 28, 2020 4:39 PM
> >> To: Makarand Pawagi <[email protected]>
> >> Cc: [email protected]; [email protected]; linux-arm-
> >> [email protected]; [email protected];
> >> [email protected];
> >> [email protected]; Cristi Sovaiala <[email protected]>;
> >> Laurentiu
> >> Tudor <[email protected]>; Ioana Ciornei
> >> <[email protected]>;
> >> Varun Sethi <[email protected]>; Calvin Johnson
> >> <[email protected]>;
> >> Pankaj Bansal <[email protected]>; [email protected];
> >> [email protected]; [email protected]; [email protected];
> >> [email protected]; [email protected]; [email protected];
> >> [email protected]; [email protected]; [email protected];
> >> [email protected]; [email protected]
> >> Subject: [EXT] Re: [PATCH] bus: fsl-mc: Add ACPI support for fsl-mc
> >>
> >> Caution: EXT Email
> >>
> >> On Tue, Jan 28, 2020 at 01:38:45PM +0530, Makarand Pawagi wrote:
> >> > ACPI support is added in the fsl-mc driver. Driver will parse MC DSDT
> >> > table to extract memory and other resorces.
> >> >
> >> > Interrupt (GIC ITS) information will be extracted from MADT table by
> >> > drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c.
> >> >
> >> > IORT table will be parsed to configure DMA.
> >> >
> >> > Signed-off-by: Makarand Pawagi <[email protected]>
> >> > ---
> >> > drivers/acpi/arm64/iort.c | 53 +++++++++++++++++++++
> >> > drivers/bus/fsl-mc/dprc-driver.c | 3 +-
> >> > drivers/bus/fsl-mc/fsl-mc-bus.c | 48 +++++++++++++------
> >> > drivers/bus/fsl-mc/fsl-mc-msi.c | 10 +++-
> >> > drivers/bus/fsl-mc/fsl-mc-private.h | 4 +-
> >> > drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c | 71
> >> ++++++++++++++++++++++++++++-
> >> > include/linux/acpi_iort.h | 5 ++
> >> > 7 files changed, 174 insertions(+), 20 deletions(-)
> >> >
> >> > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> >> > index 33f7198..beb9cd5 100644
> >> > --- a/drivers/acpi/arm64/iort.c
> >> > +++ b/drivers/acpi/arm64/iort.c
> >> > @@ -15,6 +15,7 @@
> >> > #include <linux/kernel.h>
> >> > #include <linux/list.h>
> >> > #include <linux/pci.h>
> >> > +#include <linux/fsl/mc.h>
> >> > #include <linux/platform_device.h>
> >> > #include <linux/slab.h>
> >> >
> >> > @@ -622,6 +623,29 @@ static int iort_dev_find_its_id(struct device
> >> > *dev, u32 req_id, }
> >> >
> >> > /**
> >> > + * iort_get_fsl_mc_device_domain() - Find MSI domain related to a
> >> > +device
> >> > + * @dev: The device.
> >> > + * @mc_icid: ICID for the fsl_mc device.
> >> > + *
> >> > + * Returns: the MSI domain for this device, NULL otherwise */ struct
> >> > +irq_domain *iort_get_fsl_mc_device_domain(struct device *dev,
> >> > + u32 mc_icid) {
> >> > + struct fwnode_handle *handle;
> >> > + int its_id;
> >> > +
> >> > + if (iort_dev_find_its_id(dev, mc_icid, 0, &its_id))
> >> > + return NULL;
> >> > +
> >> > + handle = iort_find_domain_token(its_id);
> >> > + if (!handle)
> >> > + return NULL;
> >> > +
> >> > + return irq_find_matching_fwnode(handle, DOMAIN_BUS_FSL_MC_MSI);
> >> > +}
> >>
> >> NAK
> >>
> >> I am not willing to take platform specific code in the generic IORT
> >> layer.
> >>
> >> ACPI on ARM64 works on platforms that comply with SBSA/SBBR
> >> guidelines:
> >>
> >>
> >> https://developer.arm.com/architectures/platform-design/server-systems
> >>
> >> Deviating from those requires butchering ACPI specifications (ie IORT)
> >> and
> >> related kernel code which goes totally against what ACPI is meant for
> >> on ARM64
> >> systems, so there is no upstream pathway for this code I am afraid.
> >>
> > Reason of adding this platform specific function in the generic IORT
> > layer is
> > That iort_get_device_domain() only deals with PCI bus
> > (DOMAIN_BUS_PCI_MSI).
> >
> > fsl-mc objects when probed, need to find irq_domain which is associated
> > with
> > the fsl-mc bus (DOMAIN_BUS_FSL_MC_MSI). It will not be possible to do
> > that
> > if we do not add this function because there are no other suitable APIs
> > exported
> > by IORT layer to do the job.
>
> I think we all understood the patch. What both Lorenzo and myself are
> saying is
> that we do not want non-PCI support in IORT.
>
IORT supports platform devices (aka named components) as well, and
there is some support for platform MSIs in the GIC layer.
So it may be possible to hide your exotic bus from the OS entirely,
and make the firmware instantiate a DSDT with device objects and
associated IORT nodes that describe whatever lives on that bus as
named components.
That way, you will not have to change the OS at all, so your hardware
will not only be supported in linux v5.7+, it will also be supported
by OSes that commercial distro vendors are shipping today. *That* is
the whole point of using ACPI.
If you are going to bother and modify the OS, you lose this advantage,
and ACPI gives you no benefit over DT at all.
On Fri, Jan 31, 2020 at 1:02 PM Ard Biesheuvel
<[email protected]> wrote:
>
> On Fri, 31 Jan 2020 at 12:06, Marc Zyngier <[email protected]> wrote:
> >
> > On 2020-01-31 10:35, Makarand Pawagi wrote:
> > >> -----Original Message-----
> > >> From: Lorenzo Pieralisi <[email protected]>
> > >> Sent: Tuesday, January 28, 2020 4:39 PM
> > >> To: Makarand Pawagi <[email protected]>
> > >> Cc: [email protected]; [email protected]; linux-arm-
> > >> [email protected]; [email protected];
> > >> [email protected];
> > >> [email protected]; Cristi Sovaiala <[email protected]>;
> > >> Laurentiu
> > >> Tudor <[email protected]>; Ioana Ciornei
> > >> <[email protected]>;
> > >> Varun Sethi <[email protected]>; Calvin Johnson
> > >> <[email protected]>;
> > >> Pankaj Bansal <[email protected]>; [email protected];
> > >> [email protected]; [email protected]; [email protected];
> > >> [email protected]; [email protected]; [email protected];
> > >> [email protected]; [email protected]; [email protected];
> > >> [email protected]; [email protected]
> > >> Subject: [EXT] Re: [PATCH] bus: fsl-mc: Add ACPI support for fsl-mc
> > >>
> > >> Caution: EXT Email
> > >>
> > >> On Tue, Jan 28, 2020 at 01:38:45PM +0530, Makarand Pawagi wrote:
> > >> > ACPI support is added in the fsl-mc driver. Driver will parse MC DSDT
> > >> > table to extract memory and other resorces.
> > >> >
> > >> > Interrupt (GIC ITS) information will be extracted from MADT table by
> > >> > drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c.
> > >> >
> > >> > IORT table will be parsed to configure DMA.
> > >> >
> > >> > Signed-off-by: Makarand Pawagi <[email protected]>
> > >> > ---
> > >> > drivers/acpi/arm64/iort.c | 53 +++++++++++++++++++++
> > >> > drivers/bus/fsl-mc/dprc-driver.c | 3 +-
> > >> > drivers/bus/fsl-mc/fsl-mc-bus.c | 48 +++++++++++++------
> > >> > drivers/bus/fsl-mc/fsl-mc-msi.c | 10 +++-
> > >> > drivers/bus/fsl-mc/fsl-mc-private.h | 4 +-
> > >> > drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c | 71
> > >> ++++++++++++++++++++++++++++-
> > >> > include/linux/acpi_iort.h | 5 ++
> > >> > 7 files changed, 174 insertions(+), 20 deletions(-)
> > >> >
> > >> > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> > >> > index 33f7198..beb9cd5 100644
> > >> > --- a/drivers/acpi/arm64/iort.c
> > >> > +++ b/drivers/acpi/arm64/iort.c
> > >> > @@ -15,6 +15,7 @@
> > >> > #include <linux/kernel.h>
> > >> > #include <linux/list.h>
> > >> > #include <linux/pci.h>
> > >> > +#include <linux/fsl/mc.h>
> > >> > #include <linux/platform_device.h>
> > >> > #include <linux/slab.h>
> > >> >
> > >> > @@ -622,6 +623,29 @@ static int iort_dev_find_its_id(struct device
> > >> > *dev, u32 req_id, }
> > >> >
> > >> > /**
> > >> > + * iort_get_fsl_mc_device_domain() - Find MSI domain related to a
> > >> > +device
> > >> > + * @dev: The device.
> > >> > + * @mc_icid: ICID for the fsl_mc device.
> > >> > + *
> > >> > + * Returns: the MSI domain for this device, NULL otherwise */ struct
> > >> > +irq_domain *iort_get_fsl_mc_device_domain(struct device *dev,
> > >> > + u32 mc_icid) {
> > >> > + struct fwnode_handle *handle;
> > >> > + int its_id;
> > >> > +
> > >> > + if (iort_dev_find_its_id(dev, mc_icid, 0, &its_id))
> > >> > + return NULL;
> > >> > +
> > >> > + handle = iort_find_domain_token(its_id);
> > >> > + if (!handle)
> > >> > + return NULL;
> > >> > +
> > >> > + return irq_find_matching_fwnode(handle, DOMAIN_BUS_FSL_MC_MSI);
> > >> > +}
> > >>
> > >> NAK
> > >>
> > >> I am not willing to take platform specific code in the generic IORT
> > >> layer.
> > >>
> > >> ACPI on ARM64 works on platforms that comply with SBSA/SBBR
> > >> guidelines:
> > >>
> > >>
> > >> https://developer.arm.com/architectures/platform-design/server-systems
> > >>
> > >> Deviating from those requires butchering ACPI specifications (ie IORT)
> > >> and
> > >> related kernel code which goes totally against what ACPI is meant for
> > >> on ARM64
> > >> systems, so there is no upstream pathway for this code I am afraid.
> > >>
> > > Reason of adding this platform specific function in the generic IORT
> > > layer is
> > > That iort_get_device_domain() only deals with PCI bus
> > > (DOMAIN_BUS_PCI_MSI).
> > >
> > > fsl-mc objects when probed, need to find irq_domain which is associated
> > > with
> > > the fsl-mc bus (DOMAIN_BUS_FSL_MC_MSI). It will not be possible to do
> > > that
> > > if we do not add this function because there are no other suitable APIs
> > > exported
> > > by IORT layer to do the job.
> >
> > I think we all understood the patch. What both Lorenzo and myself are
> > saying is
> > that we do not want non-PCI support in IORT.
> >
>
> IORT supports platform devices (aka named components) as well, and
> there is some support for platform MSIs in the GIC layer.
>
> So it may be possible to hide your exotic bus from the OS entirely,
> and make the firmware instantiate a DSDT with device objects and
> associated IORT nodes that describe whatever lives on that bus as
> named components.
>
> That way, you will not have to change the OS at all, so your hardware
> will not only be supported in linux v5.7+, it will also be supported
> by OSes that commercial distro vendors are shipping today. *That* is
> the whole point of using ACPI.
>
> If you are going to bother and modify the OS, you lose this advantage,
> and ACPI gives you no benefit over DT at all.
You beat me to it, but thanks for the clarification Ard. No where in
the SBSA spec that I have read does it state that only PCIe devices
are supported by the SMMU. It uses PCIe devices as an example, but
the SMMU section is very generic in term and only says "devices".
I feel the SBSA omission of SerDes best practices is an oversight in
the standard and something that probably needs to be revisited.
Forcing high speed networking interfaces to be hung off a bus just for
the sake of having a "standard" PCIe interface seems like a step
backward in this regard. I would much rather have the Spec include a
common standard that could be exposed in a consistent manner. But
this is a conversation for a different place.
I will work with NXP and find a better way to implement this.
-Jon
On 2020-01-31 12:28 pm, Jon Nettleton wrote:
> On Fri, Jan 31, 2020 at 1:02 PM Ard Biesheuvel
> <[email protected]> wrote:
>>
>> On Fri, 31 Jan 2020 at 12:06, Marc Zyngier <[email protected]> wrote:
>>>
>>> On 2020-01-31 10:35, Makarand Pawagi wrote:
>>>>> -----Original Message-----
>>>>> From: Lorenzo Pieralisi <[email protected]>
>>>>> Sent: Tuesday, January 28, 2020 4:39 PM
>>>>> To: Makarand Pawagi <[email protected]>
>>>>> Cc: [email protected]; [email protected]; linux-arm-
>>>>> [email protected]; [email protected];
>>>>> [email protected];
>>>>> [email protected]; Cristi Sovaiala <[email protected]>;
>>>>> Laurentiu
>>>>> Tudor <[email protected]>; Ioana Ciornei
>>>>> <[email protected]>;
>>>>> Varun Sethi <[email protected]>; Calvin Johnson
>>>>> <[email protected]>;
>>>>> Pankaj Bansal <[email protected]>; [email protected];
>>>>> [email protected]; [email protected]; [email protected];
>>>>> [email protected]; [email protected]; [email protected];
>>>>> [email protected]; [email protected]; [email protected];
>>>>> [email protected]; [email protected]
>>>>> Subject: [EXT] Re: [PATCH] bus: fsl-mc: Add ACPI support for fsl-mc
>>>>>
>>>>> Caution: EXT Email
>>>>>
>>>>> On Tue, Jan 28, 2020 at 01:38:45PM +0530, Makarand Pawagi wrote:
>>>>>> ACPI support is added in the fsl-mc driver. Driver will parse MC DSDT
>>>>>> table to extract memory and other resorces.
>>>>>>
>>>>>> Interrupt (GIC ITS) information will be extracted from MADT table by
>>>>>> drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c.
>>>>>>
>>>>>> IORT table will be parsed to configure DMA.
>>>>>>
>>>>>> Signed-off-by: Makarand Pawagi <[email protected]>
>>>>>> ---
>>>>>> drivers/acpi/arm64/iort.c | 53 +++++++++++++++++++++
>>>>>> drivers/bus/fsl-mc/dprc-driver.c | 3 +-
>>>>>> drivers/bus/fsl-mc/fsl-mc-bus.c | 48 +++++++++++++------
>>>>>> drivers/bus/fsl-mc/fsl-mc-msi.c | 10 +++-
>>>>>> drivers/bus/fsl-mc/fsl-mc-private.h | 4 +-
>>>>>> drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c | 71
>>>>> ++++++++++++++++++++++++++++-
>>>>>> include/linux/acpi_iort.h | 5 ++
>>>>>> 7 files changed, 174 insertions(+), 20 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
>>>>>> index 33f7198..beb9cd5 100644
>>>>>> --- a/drivers/acpi/arm64/iort.c
>>>>>> +++ b/drivers/acpi/arm64/iort.c
>>>>>> @@ -15,6 +15,7 @@
>>>>>> #include <linux/kernel.h>
>>>>>> #include <linux/list.h>
>>>>>> #include <linux/pci.h>
>>>>>> +#include <linux/fsl/mc.h>
>>>>>> #include <linux/platform_device.h>
>>>>>> #include <linux/slab.h>
>>>>>>
>>>>>> @@ -622,6 +623,29 @@ static int iort_dev_find_its_id(struct device
>>>>>> *dev, u32 req_id, }
>>>>>>
>>>>>> /**
>>>>>> + * iort_get_fsl_mc_device_domain() - Find MSI domain related to a
>>>>>> +device
>>>>>> + * @dev: The device.
>>>>>> + * @mc_icid: ICID for the fsl_mc device.
>>>>>> + *
>>>>>> + * Returns: the MSI domain for this device, NULL otherwise */ struct
>>>>>> +irq_domain *iort_get_fsl_mc_device_domain(struct device *dev,
>>>>>> + u32 mc_icid) {
>>>>>> + struct fwnode_handle *handle;
>>>>>> + int its_id;
>>>>>> +
>>>>>> + if (iort_dev_find_its_id(dev, mc_icid, 0, &its_id))
>>>>>> + return NULL;
>>>>>> +
>>>>>> + handle = iort_find_domain_token(its_id);
>>>>>> + if (!handle)
>>>>>> + return NULL;
>>>>>> +
>>>>>> + return irq_find_matching_fwnode(handle, DOMAIN_BUS_FSL_MC_MSI);
>>>>>> +}
>>>>>
>>>>> NAK
>>>>>
>>>>> I am not willing to take platform specific code in the generic IORT
>>>>> layer.
>>>>>
>>>>> ACPI on ARM64 works on platforms that comply with SBSA/SBBR
>>>>> guidelines:
>>>>>
>>>>>
>>>>> https://developer.arm.com/architectures/platform-design/server-systems
>>>>>
>>>>> Deviating from those requires butchering ACPI specifications (ie IORT)
>>>>> and
>>>>> related kernel code which goes totally against what ACPI is meant for
>>>>> on ARM64
>>>>> systems, so there is no upstream pathway for this code I am afraid.
>>>>>
>>>> Reason of adding this platform specific function in the generic IORT
>>>> layer is
>>>> That iort_get_device_domain() only deals with PCI bus
>>>> (DOMAIN_BUS_PCI_MSI).
>>>>
>>>> fsl-mc objects when probed, need to find irq_domain which is associated
>>>> with
>>>> the fsl-mc bus (DOMAIN_BUS_FSL_MC_MSI). It will not be possible to do
>>>> that
>>>> if we do not add this function because there are no other suitable APIs
>>>> exported
>>>> by IORT layer to do the job.
>>>
>>> I think we all understood the patch. What both Lorenzo and myself are
>>> saying is
>>> that we do not want non-PCI support in IORT.
>>>
>>
>> IORT supports platform devices (aka named components) as well, and
>> there is some support for platform MSIs in the GIC layer.
>>
>> So it may be possible to hide your exotic bus from the OS entirely,
>> and make the firmware instantiate a DSDT with device objects and
>> associated IORT nodes that describe whatever lives on that bus as
>> named components.
>>
>> That way, you will not have to change the OS at all, so your hardware
>> will not only be supported in linux v5.7+, it will also be supported
>> by OSes that commercial distro vendors are shipping today. *That* is
>> the whole point of using ACPI.
>>
>> If you are going to bother and modify the OS, you lose this advantage,
>> and ACPI gives you no benefit over DT at all.
>
> You beat me to it, but thanks for the clarification Ard. No where in
> the SBSA spec that I have read does it state that only PCIe devices
> are supported by the SMMU. It uses PCIe devices as an example, but
> the SMMU section is very generic in term and only says "devices".
>
> I feel the SBSA omission of SerDes best practices is an oversight in
> the standard and something that probably needs to be revisited.
> Forcing high speed networking interfaces to be hung off a bus just for
> the sake of having a "standard" PCIe interface seems like a step
> backward in this regard. I would much rather have the Spec include a
> common standard that could be exposed in a consistent manner. But
> this is a conversation for a different place.
Just to clarify further, it's not about serdes or high-speed networking
per se - describing a fixed-function network adapter as a named
component is entirely within scope. The issue is when the hardware is
merely a pool of accelerator components that can be dynamically
configured at runtime into something that looks like one or more
'virtual' network adapters - there is no standard interface for *that*
for SBSA to consider.
Robin.
>
> I will work with NXP and find a better way to implement this.
>
> -Jon
>
On Fri, Jan 31, 2020 at 1:48 PM Robin Murphy <[email protected]> wrote:
>
> On 2020-01-31 12:28 pm, Jon Nettleton wrote:
> > On Fri, Jan 31, 2020 at 1:02 PM Ard Biesheuvel
> > <[email protected]> wrote:
> >>
> >> On Fri, 31 Jan 2020 at 12:06, Marc Zyngier <[email protected]> wrote:
> >>>
> >>> On 2020-01-31 10:35, Makarand Pawagi wrote:
> >>>>> -----Original Message-----
> >>>>> From: Lorenzo Pieralisi <[email protected]>
> >>>>> Sent: Tuesday, January 28, 2020 4:39 PM
> >>>>> To: Makarand Pawagi <[email protected]>
> >>>>> Cc: [email protected]; [email protected]; linux-arm-
> >>>>> [email protected]; [email protected];
> >>>>> [email protected];
> >>>>> [email protected]; Cristi Sovaiala <[email protected]>;
> >>>>> Laurentiu
> >>>>> Tudor <[email protected]>; Ioana Ciornei
> >>>>> <[email protected]>;
> >>>>> Varun Sethi <[email protected]>; Calvin Johnson
> >>>>> <[email protected]>;
> >>>>> Pankaj Bansal <[email protected]>; [email protected];
> >>>>> [email protected]; [email protected]; [email protected];
> >>>>> [email protected]; [email protected]; [email protected];
> >>>>> [email protected]; [email protected]; [email protected];
> >>>>> [email protected]; [email protected]
> >>>>> Subject: [EXT] Re: [PATCH] bus: fsl-mc: Add ACPI support for fsl-mc
> >>>>>
> >>>>> Caution: EXT Email
> >>>>>
> >>>>> On Tue, Jan 28, 2020 at 01:38:45PM +0530, Makarand Pawagi wrote:
> >>>>>> ACPI support is added in the fsl-mc driver. Driver will parse MC DSDT
> >>>>>> table to extract memory and other resorces.
> >>>>>>
> >>>>>> Interrupt (GIC ITS) information will be extracted from MADT table by
> >>>>>> drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c.
> >>>>>>
> >>>>>> IORT table will be parsed to configure DMA.
> >>>>>>
> >>>>>> Signed-off-by: Makarand Pawagi <[email protected]>
> >>>>>> ---
> >>>>>> drivers/acpi/arm64/iort.c | 53 +++++++++++++++++++++
> >>>>>> drivers/bus/fsl-mc/dprc-driver.c | 3 +-
> >>>>>> drivers/bus/fsl-mc/fsl-mc-bus.c | 48 +++++++++++++------
> >>>>>> drivers/bus/fsl-mc/fsl-mc-msi.c | 10 +++-
> >>>>>> drivers/bus/fsl-mc/fsl-mc-private.h | 4 +-
> >>>>>> drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c | 71
> >>>>> ++++++++++++++++++++++++++++-
> >>>>>> include/linux/acpi_iort.h | 5 ++
> >>>>>> 7 files changed, 174 insertions(+), 20 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> >>>>>> index 33f7198..beb9cd5 100644
> >>>>>> --- a/drivers/acpi/arm64/iort.c
> >>>>>> +++ b/drivers/acpi/arm64/iort.c
> >>>>>> @@ -15,6 +15,7 @@
> >>>>>> #include <linux/kernel.h>
> >>>>>> #include <linux/list.h>
> >>>>>> #include <linux/pci.h>
> >>>>>> +#include <linux/fsl/mc.h>
> >>>>>> #include <linux/platform_device.h>
> >>>>>> #include <linux/slab.h>
> >>>>>>
> >>>>>> @@ -622,6 +623,29 @@ static int iort_dev_find_its_id(struct device
> >>>>>> *dev, u32 req_id, }
> >>>>>>
> >>>>>> /**
> >>>>>> + * iort_get_fsl_mc_device_domain() - Find MSI domain related to a
> >>>>>> +device
> >>>>>> + * @dev: The device.
> >>>>>> + * @mc_icid: ICID for the fsl_mc device.
> >>>>>> + *
> >>>>>> + * Returns: the MSI domain for this device, NULL otherwise */ struct
> >>>>>> +irq_domain *iort_get_fsl_mc_device_domain(struct device *dev,
> >>>>>> + u32 mc_icid) {
> >>>>>> + struct fwnode_handle *handle;
> >>>>>> + int its_id;
> >>>>>> +
> >>>>>> + if (iort_dev_find_its_id(dev, mc_icid, 0, &its_id))
> >>>>>> + return NULL;
> >>>>>> +
> >>>>>> + handle = iort_find_domain_token(its_id);
> >>>>>> + if (!handle)
> >>>>>> + return NULL;
> >>>>>> +
> >>>>>> + return irq_find_matching_fwnode(handle, DOMAIN_BUS_FSL_MC_MSI);
> >>>>>> +}
> >>>>>
> >>>>> NAK
> >>>>>
> >>>>> I am not willing to take platform specific code in the generic IORT
> >>>>> layer.
> >>>>>
> >>>>> ACPI on ARM64 works on platforms that comply with SBSA/SBBR
> >>>>> guidelines:
> >>>>>
> >>>>>
> >>>>> https://developer.arm.com/architectures/platform-design/server-systems
> >>>>>
> >>>>> Deviating from those requires butchering ACPI specifications (ie IORT)
> >>>>> and
> >>>>> related kernel code which goes totally against what ACPI is meant for
> >>>>> on ARM64
> >>>>> systems, so there is no upstream pathway for this code I am afraid.
> >>>>>
> >>>> Reason of adding this platform specific function in the generic IORT
> >>>> layer is
> >>>> That iort_get_device_domain() only deals with PCI bus
> >>>> (DOMAIN_BUS_PCI_MSI).
> >>>>
> >>>> fsl-mc objects when probed, need to find irq_domain which is associated
> >>>> with
> >>>> the fsl-mc bus (DOMAIN_BUS_FSL_MC_MSI). It will not be possible to do
> >>>> that
> >>>> if we do not add this function because there are no other suitable APIs
> >>>> exported
> >>>> by IORT layer to do the job.
> >>>
> >>> I think we all understood the patch. What both Lorenzo and myself are
> >>> saying is
> >>> that we do not want non-PCI support in IORT.
> >>>
> >>
> >> IORT supports platform devices (aka named components) as well, and
> >> there is some support for platform MSIs in the GIC layer.
> >>
> >> So it may be possible to hide your exotic bus from the OS entirely,
> >> and make the firmware instantiate a DSDT with device objects and
> >> associated IORT nodes that describe whatever lives on that bus as
> >> named components.
> >>
> >> That way, you will not have to change the OS at all, so your hardware
> >> will not only be supported in linux v5.7+, it will also be supported
> >> by OSes that commercial distro vendors are shipping today. *That* is
> >> the whole point of using ACPI.
> >>
> >> If you are going to bother and modify the OS, you lose this advantage,
> >> and ACPI gives you no benefit over DT at all.
> >
> > You beat me to it, but thanks for the clarification Ard. No where in
> > the SBSA spec that I have read does it state that only PCIe devices
> > are supported by the SMMU. It uses PCIe devices as an example, but
> > the SMMU section is very generic in term and only says "devices".
> >
> > I feel the SBSA omission of SerDes best practices is an oversight in
> > the standard and something that probably needs to be revisited.
> > Forcing high speed networking interfaces to be hung off a bus just for
> > the sake of having a "standard" PCIe interface seems like a step
> > backward in this regard. I would much rather have the Spec include a
> > common standard that could be exposed in a consistent manner. But
> > this is a conversation for a different place.
>
> Just to clarify further, it's not about serdes or high-speed networking
> per se - describing a fixed-function network adapter as a named
> component is entirely within scope. The issue is when the hardware is
> merely a pool of accelerator components that can be dynamically
> configured at runtime into something that looks like one or more
> 'virtual' network adapters - there is no standard interface for *that*
> for SBSA to consider.
>
> Robin.
>
> >
> > I will work with NXP and find a better way to implement this.
> >
> > -Jon
> >
But by design SFP, SFP+, and QSFP cages are not fixed function network
adapters. They are physical and logical devices that can adapt to
what is plugged into them. How the devices are exposed should be
irrelevant to this conversation it is about the underlying
connectivity. For instance if this were an accelerator block on a
PCIe card then we wouldn't be having this discussion, even if it did
run a firmware and have a third party driver that exposed virtual
network interfaces.
-Jon
On Fri, 31 Jan 2020 at 14:12, Jon Nettleton <[email protected]> wrote:
>
> On Fri, Jan 31, 2020 at 1:48 PM Robin Murphy <[email protected]> wrote:
> >
> > On 2020-01-31 12:28 pm, Jon Nettleton wrote:
> > > On Fri, Jan 31, 2020 at 1:02 PM Ard Biesheuvel
> > > <[email protected]> wrote:
> > >>
> > >> On Fri, 31 Jan 2020 at 12:06, Marc Zyngier <[email protected]> wrote:
> > >>>
> > >>> On 2020-01-31 10:35, Makarand Pawagi wrote:
> > >>>>> -----Original Message-----
> > >>>>> From: Lorenzo Pieralisi <[email protected]>
> > >>>>> Sent: Tuesday, January 28, 2020 4:39 PM
> > >>>>> To: Makarand Pawagi <[email protected]>
> > >>>>> Cc: [email protected]; [email protected]; linux-arm-
> > >>>>> [email protected]; [email protected];
> > >>>>> [email protected];
> > >>>>> [email protected]; Cristi Sovaiala <[email protected]>;
> > >>>>> Laurentiu
> > >>>>> Tudor <[email protected]>; Ioana Ciornei
> > >>>>> <[email protected]>;
> > >>>>> Varun Sethi <[email protected]>; Calvin Johnson
> > >>>>> <[email protected]>;
> > >>>>> Pankaj Bansal <[email protected]>; [email protected];
> > >>>>> [email protected]; [email protected]; [email protected];
> > >>>>> [email protected]; [email protected]; [email protected];
> > >>>>> [email protected]; [email protected]; [email protected];
> > >>>>> [email protected]; [email protected]
> > >>>>> Subject: [EXT] Re: [PATCH] bus: fsl-mc: Add ACPI support for fsl-mc
> > >>>>>
> > >>>>> Caution: EXT Email
> > >>>>>
> > >>>>> On Tue, Jan 28, 2020 at 01:38:45PM +0530, Makarand Pawagi wrote:
> > >>>>>> ACPI support is added in the fsl-mc driver. Driver will parse MC DSDT
> > >>>>>> table to extract memory and other resorces.
> > >>>>>>
> > >>>>>> Interrupt (GIC ITS) information will be extracted from MADT table by
> > >>>>>> drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c.
> > >>>>>>
> > >>>>>> IORT table will be parsed to configure DMA.
> > >>>>>>
> > >>>>>> Signed-off-by: Makarand Pawagi <[email protected]>
> > >>>>>> ---
> > >>>>>> drivers/acpi/arm64/iort.c | 53 +++++++++++++++++++++
> > >>>>>> drivers/bus/fsl-mc/dprc-driver.c | 3 +-
> > >>>>>> drivers/bus/fsl-mc/fsl-mc-bus.c | 48 +++++++++++++------
> > >>>>>> drivers/bus/fsl-mc/fsl-mc-msi.c | 10 +++-
> > >>>>>> drivers/bus/fsl-mc/fsl-mc-private.h | 4 +-
> > >>>>>> drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c | 71
> > >>>>> ++++++++++++++++++++++++++++-
> > >>>>>> include/linux/acpi_iort.h | 5 ++
> > >>>>>> 7 files changed, 174 insertions(+), 20 deletions(-)
> > >>>>>>
> > >>>>>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> > >>>>>> index 33f7198..beb9cd5 100644
> > >>>>>> --- a/drivers/acpi/arm64/iort.c
> > >>>>>> +++ b/drivers/acpi/arm64/iort.c
> > >>>>>> @@ -15,6 +15,7 @@
> > >>>>>> #include <linux/kernel.h>
> > >>>>>> #include <linux/list.h>
> > >>>>>> #include <linux/pci.h>
> > >>>>>> +#include <linux/fsl/mc.h>
> > >>>>>> #include <linux/platform_device.h>
> > >>>>>> #include <linux/slab.h>
> > >>>>>>
> > >>>>>> @@ -622,6 +623,29 @@ static int iort_dev_find_its_id(struct device
> > >>>>>> *dev, u32 req_id, }
> > >>>>>>
> > >>>>>> /**
> > >>>>>> + * iort_get_fsl_mc_device_domain() - Find MSI domain related to a
> > >>>>>> +device
> > >>>>>> + * @dev: The device.
> > >>>>>> + * @mc_icid: ICID for the fsl_mc device.
> > >>>>>> + *
> > >>>>>> + * Returns: the MSI domain for this device, NULL otherwise */ struct
> > >>>>>> +irq_domain *iort_get_fsl_mc_device_domain(struct device *dev,
> > >>>>>> + u32 mc_icid) {
> > >>>>>> + struct fwnode_handle *handle;
> > >>>>>> + int its_id;
> > >>>>>> +
> > >>>>>> + if (iort_dev_find_its_id(dev, mc_icid, 0, &its_id))
> > >>>>>> + return NULL;
> > >>>>>> +
> > >>>>>> + handle = iort_find_domain_token(its_id);
> > >>>>>> + if (!handle)
> > >>>>>> + return NULL;
> > >>>>>> +
> > >>>>>> + return irq_find_matching_fwnode(handle, DOMAIN_BUS_FSL_MC_MSI);
> > >>>>>> +}
> > >>>>>
> > >>>>> NAK
> > >>>>>
> > >>>>> I am not willing to take platform specific code in the generic IORT
> > >>>>> layer.
> > >>>>>
> > >>>>> ACPI on ARM64 works on platforms that comply with SBSA/SBBR
> > >>>>> guidelines:
> > >>>>>
> > >>>>>
> > >>>>> https://developer.arm.com/architectures/platform-design/server-systems
> > >>>>>
> > >>>>> Deviating from those requires butchering ACPI specifications (ie IORT)
> > >>>>> and
> > >>>>> related kernel code which goes totally against what ACPI is meant for
> > >>>>> on ARM64
> > >>>>> systems, so there is no upstream pathway for this code I am afraid.
> > >>>>>
> > >>>> Reason of adding this platform specific function in the generic IORT
> > >>>> layer is
> > >>>> That iort_get_device_domain() only deals with PCI bus
> > >>>> (DOMAIN_BUS_PCI_MSI).
> > >>>>
> > >>>> fsl-mc objects when probed, need to find irq_domain which is associated
> > >>>> with
> > >>>> the fsl-mc bus (DOMAIN_BUS_FSL_MC_MSI). It will not be possible to do
> > >>>> that
> > >>>> if we do not add this function because there are no other suitable APIs
> > >>>> exported
> > >>>> by IORT layer to do the job.
> > >>>
> > >>> I think we all understood the patch. What both Lorenzo and myself are
> > >>> saying is
> > >>> that we do not want non-PCI support in IORT.
> > >>>
> > >>
> > >> IORT supports platform devices (aka named components) as well, and
> > >> there is some support for platform MSIs in the GIC layer.
> > >>
> > >> So it may be possible to hide your exotic bus from the OS entirely,
> > >> and make the firmware instantiate a DSDT with device objects and
> > >> associated IORT nodes that describe whatever lives on that bus as
> > >> named components.
> > >>
> > >> That way, you will not have to change the OS at all, so your hardware
> > >> will not only be supported in linux v5.7+, it will also be supported
> > >> by OSes that commercial distro vendors are shipping today. *That* is
> > >> the whole point of using ACPI.
> > >>
> > >> If you are going to bother and modify the OS, you lose this advantage,
> > >> and ACPI gives you no benefit over DT at all.
> > >
> > > You beat me to it, but thanks for the clarification Ard. No where in
> > > the SBSA spec that I have read does it state that only PCIe devices
> > > are supported by the SMMU. It uses PCIe devices as an example, but
> > > the SMMU section is very generic in term and only says "devices".
> > >
> > > I feel the SBSA omission of SerDes best practices is an oversight in
> > > the standard and something that probably needs to be revisited.
> > > Forcing high speed networking interfaces to be hung off a bus just for
> > > the sake of having a "standard" PCIe interface seems like a step
> > > backward in this regard. I would much rather have the Spec include a
> > > common standard that could be exposed in a consistent manner. But
> > > this is a conversation for a different place.
> >
> > Just to clarify further, it's not about serdes or high-speed networking
> > per se - describing a fixed-function network adapter as a named
> > component is entirely within scope. The issue is when the hardware is
> > merely a pool of accelerator components that can be dynamically
> > configured at runtime into something that looks like one or more
> > 'virtual' network adapters - there is no standard interface for *that*
> > for SBSA to consider.
> >
> > Robin.
> >
> > >
> > > I will work with NXP and find a better way to implement this.
> > >
> > > -Jon
> > >
>
> But by design SFP, SFP+, and QSFP cages are not fixed function network
> adapters. They are physical and logical devices that can adapt to
> what is plugged into them. How the devices are exposed should be
> irrelevant to this conversation it is about the underlying
> connectivity. For instance if this were an accelerator block on a
> PCIe card then we wouldn't be having this discussion, even if it did
> run a firmware and have a third party driver that exposed virtual
> network interfaces.
>
Again, on an ACPI system, it is the firmware's job to abstract away
from these platform details. Firmware can perform logical hotplug and
hot unplug of virtual devices, and hide the discovery and
initalization of those pluggable gadgets from the OS. If the
configuration changes, one virtual device appears and another one
disappears, and the firmware can signal such a change to the OS.
What is needed here is for people to really buy into the ACPI
paradigm, rather than treat it as MS flavored DT, where every little
soc detail is decribed to, and therefore managed by the OS. As we
know, there are substantial scalability issues around DT, which is why
ACPI aims to provide a higher level of abstraction, with more runtime
firmware to iron out the wrinkles.
In general, simply inventing ACPI _HIDs for every DT node that
describes a given platform, and adding ACPI probe support to the
existing drivers literally gives you the worst of both worlds, and in
this case [where we are getting into interrupt and DMA routing], it is
not even feasible to begin with.
So I'd suggest approaching this from the opposite direction: don't
look at the existing DT description and existing drivers, but look at
what ACPI supports today, and how the hardware can be made to look
like that through the use of firmware magic.
On 2020-01-31 1:11 pm, Jon Nettleton wrote:
> On Fri, Jan 31, 2020 at 1:48 PM Robin Murphy <[email protected]> wrote:
>>
>> On 2020-01-31 12:28 pm, Jon Nettleton wrote:
>>> On Fri, Jan 31, 2020 at 1:02 PM Ard Biesheuvel
>>> <[email protected]> wrote:
>>>>
>>>> On Fri, 31 Jan 2020 at 12:06, Marc Zyngier <[email protected]> wrote:
>>>>>
>>>>> On 2020-01-31 10:35, Makarand Pawagi wrote:
>>>>>>> -----Original Message-----
>>>>>>> From: Lorenzo Pieralisi <[email protected]>
>>>>>>> Sent: Tuesday, January 28, 2020 4:39 PM
>>>>>>> To: Makarand Pawagi <[email protected]>
>>>>>>> Cc: [email protected]; [email protected]; linux-arm-
>>>>>>> [email protected]; [email protected];
>>>>>>> [email protected];
>>>>>>> [email protected]; Cristi Sovaiala <[email protected]>;
>>>>>>> Laurentiu
>>>>>>> Tudor <[email protected]>; Ioana Ciornei
>>>>>>> <[email protected]>;
>>>>>>> Varun Sethi <[email protected]>; Calvin Johnson
>>>>>>> <[email protected]>;
>>>>>>> Pankaj Bansal <[email protected]>; [email protected];
>>>>>>> [email protected]; [email protected]; [email protected];
>>>>>>> [email protected]; [email protected]; [email protected];
>>>>>>> [email protected]; [email protected]; [email protected];
>>>>>>> [email protected]; [email protected]
>>>>>>> Subject: [EXT] Re: [PATCH] bus: fsl-mc: Add ACPI support for fsl-mc
>>>>>>>
>>>>>>> Caution: EXT Email
>>>>>>>
>>>>>>> On Tue, Jan 28, 2020 at 01:38:45PM +0530, Makarand Pawagi wrote:
>>>>>>>> ACPI support is added in the fsl-mc driver. Driver will parse MC DSDT
>>>>>>>> table to extract memory and other resorces.
>>>>>>>>
>>>>>>>> Interrupt (GIC ITS) information will be extracted from MADT table by
>>>>>>>> drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c.
>>>>>>>>
>>>>>>>> IORT table will be parsed to configure DMA.
>>>>>>>>
>>>>>>>> Signed-off-by: Makarand Pawagi <[email protected]>
>>>>>>>> ---
>>>>>>>> drivers/acpi/arm64/iort.c | 53 +++++++++++++++++++++
>>>>>>>> drivers/bus/fsl-mc/dprc-driver.c | 3 +-
>>>>>>>> drivers/bus/fsl-mc/fsl-mc-bus.c | 48 +++++++++++++------
>>>>>>>> drivers/bus/fsl-mc/fsl-mc-msi.c | 10 +++-
>>>>>>>> drivers/bus/fsl-mc/fsl-mc-private.h | 4 +-
>>>>>>>> drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c | 71
>>>>>>> ++++++++++++++++++++++++++++-
>>>>>>>> include/linux/acpi_iort.h | 5 ++
>>>>>>>> 7 files changed, 174 insertions(+), 20 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
>>>>>>>> index 33f7198..beb9cd5 100644
>>>>>>>> --- a/drivers/acpi/arm64/iort.c
>>>>>>>> +++ b/drivers/acpi/arm64/iort.c
>>>>>>>> @@ -15,6 +15,7 @@
>>>>>>>> #include <linux/kernel.h>
>>>>>>>> #include <linux/list.h>
>>>>>>>> #include <linux/pci.h>
>>>>>>>> +#include <linux/fsl/mc.h>
>>>>>>>> #include <linux/platform_device.h>
>>>>>>>> #include <linux/slab.h>
>>>>>>>>
>>>>>>>> @@ -622,6 +623,29 @@ static int iort_dev_find_its_id(struct device
>>>>>>>> *dev, u32 req_id, }
>>>>>>>>
>>>>>>>> /**
>>>>>>>> + * iort_get_fsl_mc_device_domain() - Find MSI domain related to a
>>>>>>>> +device
>>>>>>>> + * @dev: The device.
>>>>>>>> + * @mc_icid: ICID for the fsl_mc device.
>>>>>>>> + *
>>>>>>>> + * Returns: the MSI domain for this device, NULL otherwise */ struct
>>>>>>>> +irq_domain *iort_get_fsl_mc_device_domain(struct device *dev,
>>>>>>>> + u32 mc_icid) {
>>>>>>>> + struct fwnode_handle *handle;
>>>>>>>> + int its_id;
>>>>>>>> +
>>>>>>>> + if (iort_dev_find_its_id(dev, mc_icid, 0, &its_id))
>>>>>>>> + return NULL;
>>>>>>>> +
>>>>>>>> + handle = iort_find_domain_token(its_id);
>>>>>>>> + if (!handle)
>>>>>>>> + return NULL;
>>>>>>>> +
>>>>>>>> + return irq_find_matching_fwnode(handle, DOMAIN_BUS_FSL_MC_MSI);
>>>>>>>> +}
>>>>>>>
>>>>>>> NAK
>>>>>>>
>>>>>>> I am not willing to take platform specific code in the generic IORT
>>>>>>> layer.
>>>>>>>
>>>>>>> ACPI on ARM64 works on platforms that comply with SBSA/SBBR
>>>>>>> guidelines:
>>>>>>>
>>>>>>>
>>>>>>> https://developer.arm.com/architectures/platform-design/server-systems
>>>>>>>
>>>>>>> Deviating from those requires butchering ACPI specifications (ie IORT)
>>>>>>> and
>>>>>>> related kernel code which goes totally against what ACPI is meant for
>>>>>>> on ARM64
>>>>>>> systems, so there is no upstream pathway for this code I am afraid.
>>>>>>>
>>>>>> Reason of adding this platform specific function in the generic IORT
>>>>>> layer is
>>>>>> That iort_get_device_domain() only deals with PCI bus
>>>>>> (DOMAIN_BUS_PCI_MSI).
>>>>>>
>>>>>> fsl-mc objects when probed, need to find irq_domain which is associated
>>>>>> with
>>>>>> the fsl-mc bus (DOMAIN_BUS_FSL_MC_MSI). It will not be possible to do
>>>>>> that
>>>>>> if we do not add this function because there are no other suitable APIs
>>>>>> exported
>>>>>> by IORT layer to do the job.
>>>>>
>>>>> I think we all understood the patch. What both Lorenzo and myself are
>>>>> saying is
>>>>> that we do not want non-PCI support in IORT.
>>>>>
>>>>
>>>> IORT supports platform devices (aka named components) as well, and
>>>> there is some support for platform MSIs in the GIC layer.
>>>>
>>>> So it may be possible to hide your exotic bus from the OS entirely,
>>>> and make the firmware instantiate a DSDT with device objects and
>>>> associated IORT nodes that describe whatever lives on that bus as
>>>> named components.
>>>>
>>>> That way, you will not have to change the OS at all, so your hardware
>>>> will not only be supported in linux v5.7+, it will also be supported
>>>> by OSes that commercial distro vendors are shipping today. *That* is
>>>> the whole point of using ACPI.
>>>>
>>>> If you are going to bother and modify the OS, you lose this advantage,
>>>> and ACPI gives you no benefit over DT at all.
>>>
>>> You beat me to it, but thanks for the clarification Ard. No where in
>>> the SBSA spec that I have read does it state that only PCIe devices
>>> are supported by the SMMU. It uses PCIe devices as an example, but
>>> the SMMU section is very generic in term and only says "devices".
>>>
>>> I feel the SBSA omission of SerDes best practices is an oversight in
>>> the standard and something that probably needs to be revisited.
>>> Forcing high speed networking interfaces to be hung off a bus just for
>>> the sake of having a "standard" PCIe interface seems like a step
>>> backward in this regard. I would much rather have the Spec include a
>>> common standard that could be exposed in a consistent manner. But
>>> this is a conversation for a different place.
>>
>> Just to clarify further, it's not about serdes or high-speed networking
>> per se - describing a fixed-function network adapter as a named
>> component is entirely within scope. The issue is when the hardware is
>> merely a pool of accelerator components that can be dynamically
>> configured at runtime into something that looks like one or more
>> 'virtual' network adapters - there is no standard interface for *that*
>> for SBSA to consider.
>>
>> Robin.
>>
>>>
>>> I will work with NXP and find a better way to implement this.
>>>
>>> -Jon
>>>
>
> But by design SFP, SFP+, and QSFP cages are not fixed function network
> adapters. They are physical and logical devices that can adapt to
> what is plugged into them. How the devices are exposed should be
> irrelevant to this conversation it is about the underlying
> connectivity.
Apologies - I was under the impression that SFP and friends were a
physical-layer thing and that a MAC in the SoC would still be fixed such
that its DMA and interrupt configuration could be statically described
regardless of what transceiver was plugged in (even if some
configurations might not use every interrupt/stream ID/etc.) If that
isn't the case I shall go and educate myself further.
> For instance if this were an accelerator block on a
> PCIe card then we wouldn't be having this discussion, even if it did
> run a firmware and have a third party driver that exposed virtual
> network interfaces.
Right, because in that case the interrupts and DMA have to travel
through the PCIe layer, and thus generic code only needs to worry about
things from the point of the PCI host bridge. That's rather the point of
having an industry-standard interface.
Robin.
> > But by design SFP, SFP+, and QSFP cages are not fixed function network
> > adapters. They are physical and logical devices that can adapt to
> > what is plugged into them. How the devices are exposed should be
> > irrelevant to this conversation it is about the underlying
> > connectivity.
>
> Apologies - I was under the impression that SFP and friends were a
> physical-layer thing and that a MAC in the SoC would still be fixed such
> that its DMA and interrupt configuration could be statically described
> regardless of what transceiver was plugged in (even if some configurations
> might not use every interrupt/stream ID/etc.) If that isn't the case I shall
> go and educate myself further.
Hi Robin
It gets interesting with QSFP cages. The Q is quad, there are 4 SERDES
lanes. You can use them for 1x 40G link, or you can split them into 4x
10G links. So you either need one MAC or 4 MACs connecting to the
cage, and this can change on the fly when a modules is ejected and
replaced with another module. There are only one set of control pins
for i2c, loss of signal, TX disable, module inserted. So where the
interrupt/stream ID/etc are mapped needs some flexibility.
There is also to some degree a conflict with hiding all this inside
firmware. This is complex stuff. It is much better to have one core
implementing in Linux plus some per hardware driver support, than
having X firmware blobs, generally closed source, each with there own
bugs which nobody can fix.
Andrew
On Fri, Jan 31, 2020 at 03:29:06PM +0100, Andrew Lunn wrote:
> > > But by design SFP, SFP+, and QSFP cages are not fixed function network
> > > adapters. They are physical and logical devices that can adapt to
> > > what is plugged into them. How the devices are exposed should be
> > > irrelevant to this conversation it is about the underlying
> > > connectivity.
> >
> > Apologies - I was under the impression that SFP and friends were a
> > physical-layer thing and that a MAC in the SoC would still be fixed such
> > that its DMA and interrupt configuration could be statically described
> > regardless of what transceiver was plugged in (even if some configurations
> > might not use every interrupt/stream ID/etc.) If that isn't the case I shall
> > go and educate myself further.
>
> It gets interesting with QSFP cages. The Q is quad, there are 4 SERDES
> lanes. You can use them for 1x 40G link, or you can split them into 4x
> 10G links. So you either need one MAC or 4 MACs connecting to the
> cage, and this can change on the fly when a modules is ejected and
> replaced with another module. There are only one set of control pins
> for i2c, loss of signal, TX disable, module inserted. So where the
> interrupt/stream ID/etc are mapped needs some flexibility.
>
> There is also to some degree a conflict with hiding all this inside
> firmware. This is complex stuff. It is much better to have one core
> implementing in Linux plus some per hardware driver support, than
> having X firmware blobs, generally closed source, each with there own
> bugs which nobody can fix.
Devicetree to the rescue!
Entertaining the use of ACPI without any firmware abstraction for this
hardware really feels like a square peg / round hole situation, so I'm
assuming somebody's telling you that you need it "FOAR ENTAPRYZE". Who
is it and can you tell them to bog off?
Will
> Devicetree to the rescue!
Yes, exactly. We have good, standardised descriptions for most of this
in device tree. And phylink can handle SFP and SFP+. Nobody has worked
on QSFP yet, since phylink has mostly been pushed by the embedded
world and 40G is not yet popular in the embedded world.
> Entertaining the use of ACPI without any firmware abstraction for this
> hardware really feels like a square peg / round hole situation, so I'm
> assuming somebody's telling you that you need it "FOAR ENTAPRYZE". Who
> is it and can you tell them to bog off?
The issues here is that SFPs are appearing in more and more server
systems, replacing plain old copper Ethernet. If the boxes use off the
shelf Mellanox or Intel PCIe cards, it is not an issue. But silicon
vendors are integrating this into the SoC in the ARM way of doing
things, memory mapped, spread over a number of controllers, not a
single PCIe device.
Maybe we need hybrid systems. Plain, old, simple, boring things like
CPUs, serial ports, SATA, PCIe busses are described in ACPI. Complex
interesting things are in DT. The hard thing is the interface between
the two. DT having a phandle to an ACPI object, e.g a GPIO, interrupt
or an i2c bus.
Andrew
On Fri, Jan 31, 2020 at 4:09 PM Andrew Lunn <[email protected]> wrote:
>
> > Devicetree to the rescue!
>
> Yes, exactly. We have good, standardised descriptions for most of this
> in device tree. And phylink can handle SFP and SFP+. Nobody has worked
> on QSFP yet, since phylink has mostly been pushed by the embedded
> world and 40G is not yet popular in the embedded world.
>
> > Entertaining the use of ACPI without any firmware abstraction for this
> > hardware really feels like a square peg / round hole situation, so I'm
> > assuming somebody's telling you that you need it "FOAR ENTAPRYZE". Who
> > is it and can you tell them to bog off?
>
> The issues here is that SFPs are appearing in more and more server
> systems, replacing plain old copper Ethernet. If the boxes use off the
> shelf Mellanox or Intel PCIe cards, it is not an issue. But silicon
> vendors are integrating this into the SoC in the ARM way of doing
> things, memory mapped, spread over a number of controllers, not a
> single PCIe device.
>
> Maybe we need hybrid systems. Plain, old, simple, boring things like
> CPUs, serial ports, SATA, PCIe busses are described in ACPI. Complex
> interesting things are in DT. The hard thing is the interface between
> the two. DT having a phandle to an ACPI object, e.g a GPIO, interrupt
> or an i2c bus.
>
> Andrew
Just as a review reference, I have found an old attempted
implementation documented here.
https://github.com/CumulusNetworks/apd-tools/wiki
-Jon
On Fri, Jan 31, 2020 at 03:29:06PM +0100, Andrew Lunn wrote:
> > > But by design SFP, SFP+, and QSFP cages are not fixed function network
> > > adapters. They are physical and logical devices that can adapt to
> > > what is plugged into them. How the devices are exposed should be
> > > irrelevant to this conversation it is about the underlying
> > > connectivity.
> >
> > Apologies - I was under the impression that SFP and friends were a
> > physical-layer thing and that a MAC in the SoC would still be fixed such
> > that its DMA and interrupt configuration could be statically described
> > regardless of what transceiver was plugged in (even if some configurations
> > might not use every interrupt/stream ID/etc.) If that isn't the case I shall
> > go and educate myself further.
>
> Hi Robin
>
> It gets interesting with QSFP cages. The Q is quad, there are 4 SERDES
> lanes. You can use them for 1x 40G link, or you can split them into 4x
> 10G links. So you either need one MAC or 4 MACs connecting to the
> cage, and this can change on the fly when a modules is ejected and
> replaced with another module.
I think it's even more complicated than that. If you have a QSFP+
fiber module, that can be connected to four fibers which can either
go to another QSFP+ module, or four separate SFP+ modules.
That means it's a manual configuration decision whether to operate
the QSFP+ module as a single 40G link, or as four separate 10G links.
> There are only one set of control pins
> for i2c, loss of signal, TX disable, module inserted. So where the
> interrupt/stream ID/etc are mapped needs some flexibility.
QSFP changes the way the modules are controlled; gone are many of the
hardware signals, replaced by registers in the I2C space. The
remaining hardware signals are:
ModSelL module select (to enable the I2C bus)
ResetL module reset
SCL/SDA I2C bus
ModPrsL module present
IntL interrupt (but not too useful from what I can see!)
LPMode low power mode (can be overriden via the I2C bus)
> There is also to some degree a conflict with hiding all this inside
> firmware. This is complex stuff. It is much better to have one core
> implementing in Linux plus some per hardware driver support, than
> having X firmware blobs, generally closed source, each with there own
> bugs which nobody can fix.
QSFP and SFP support is not really part of the DPAA2 firmware.
I have some prototype implementation for driving the QSFP+ cage, but
I haven't yet worked out how to sensible deal with the "is it 4x 10G
or 1x 40G" issue you mention above, and how to interface the QSFP+
driver sensibly with one or four network drivers.
I've been concentrating more on the SFP/SFP+ problem on the Honeycomb
board which is what most people will have, working out how to sensibly
drive the hardware so that our existing SFP support in the kernel can
work sensibly. In the last couple of days, I've managed to get
something together which works, switching between 1000base-X and SGMII
on this hardware, using some of the patches I've already pointed to
over the last few weeks. This hardware falls into the "split PCS and
MAC" problem space, so it's relevent to many people - and it's
important that we don't rush into a solution that works for one
implementation and not everyone. This is why I haven't responded to
Jose's proposal - I'm still working out what is required for others,
but what I can say is that it isn't what Jose has proposed. I had
asked Jose to hold off, but he's understandably eager to solve the
problem in front of him at the expense of everyone else.
What I've found is that any attempt to split the current
"phylink_mac_ops" interface between the PCS and MAC blocks results, as
I suspected, in mvneta and mvpp2 suffering very badly; the hardware
does not split along those functional blocks at all well.
My current state of play for this is in my "cex7" branch, pushed out
earlier today. It's a bit hacky right now, and there's various issues
that need to be solved, but it is functional with the right board boot
configuration (basically the DPC file, which is one of the configs for
the MC firmware.)
I'm planning to look at what's required for the faster speeds; there's
other PCS PHYs on this platform that support the other speeds (10G, 25G,
40G, 100G) accessed via Clause 45 cycles.
As for the DSA issue you've raised with DSA links, I don't see any
obvious solution for that - the whole "if no fixed-link is specified,
default to the highest speed" is a real problem; the conversion of DSA
to phylink for the CPU and DSA ports did not take account of that.
phylink has _zero_ information in that case to know how the link should
be configured - there is no PHY, there is no fixed-link specification,
there is absolutely nothing. So it's no surprise when phylink tries to
configure speed=0 duplex=half pause=off on these interfaces when they're
brought up. I notice that this work was contributed by NXP - and in my
mind illustrates that they did not think about what they were doing
there either. They certainly never ran phylink with debugging on and
considered whether the phylink_mac_config() calls contained sensible
information. Did they even have all the information necessary to work
out what was required - I doubt it very much. Did they realise that the
fixed-link specification was optional, did they realise that there
could be a PHY on these links, and did they consider what the behaviour
would be in those cases? And now we have something of a headache
trying to work out how to solve this - one thing is certain, whatever
the fix is, it isn't going to be nice to be backported to stable trees.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
On Fri, Jan 31, 2020 at 04:09:29PM +0100, Andrew Lunn wrote:
> > Devicetree to the rescue!
>
> Yes, exactly. We have good, standardised descriptions for most of this
> in device tree. And phylink can handle SFP and SFP+. Nobody has worked
> on QSFP yet, since phylink has mostly been pushed by the embedded
> world and 40G is not yet popular in the embedded world.
That's incorrect (if you read my previous reply.) It shouldn't come
as any surprise that I have some experimental QSFP code.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
On Fri, 31 Jan 2020 15:15:00 +0000, Russell King - ARM Linux admin
wrote:
> I have some prototype implementation for driving the QSFP+ cage, but
> I haven't yet worked out how to sensible deal with the "is it 4x 10G
> or 1x 40G" issue you mention above, and how to interface the QSFP+
> driver sensibly with one or four network drivers.
I'm pretty sure you know this but just FWIW - vendors who do it in FW
write the current config down in NVM so it doesn't get affected by
reboots and use devlink port splitting to change it.
> Just as a review reference, I have found an old attempted
> implementation documented here.
> https://github.com/CumulusNetworks/apd-tools/wiki
That is interesting, but you need to be careful with it.
Top of rack switches are very different beasts to the majority of
switches supported by Linux, which are SOHO. TOR switches have a lot
of firmware running on them doing most of the work. A typical SOHO
switch with a Linux kernel driver has very little firmware, Linux
really is driving it, not just asking it to do things.
Also, TOR switches typically have a binary blob running in userspace,
meaning different information is probably required compared to kernel
driver.
There may be useful information here, but please take it with a big
pinch of salt. And make sure you have the appropriate networking
people involved in any specification work you do.
Andrew
On Fri, Jan 31, 2020 at 07:40:50AM -0800, Jakub Kicinski wrote:
> On Fri, 31 Jan 2020 15:15:00 +0000, Russell King - ARM Linux admin
> wrote:
> > I have some prototype implementation for driving the QSFP+ cage, but
> > I haven't yet worked out how to sensible deal with the "is it 4x 10G
> > or 1x 40G" issue you mention above, and how to interface the QSFP+
> > driver sensibly with one or four network drivers.
>
> I'm pretty sure you know this but just FWIW - vendors who do it in FW
> write the current config down in NVM so it doesn't get affected by
> reboots and use devlink port splitting to change it.
+Jiri
I wasn't aware of devlink port splitting, so thanks for that. However,
it could do with some better documentation - there's nothing on it
afaics in the Documentation subdirectory, and "man devlink-port"
doesn't give much away either:
devlink port split - split devlink port into more
DEV/PORT_INDEX - specifies the devlink port to operate on.
count COUNT
number of ports to split to.
It's the "into more" that's not clear - presumably more ports, and
presumably each port is a network device, but this isn't explained.
I think what this is trying to say is that, if we have a QSFP+ cage
with 4 serdes lines running at 10G, and they are initially treated as
a single 40G ethernet:
devlink port split device/1 count 4
will then give us four 10G network devices, and:
devlink port unsplit device/1
will recombine them back to a single 40G network device.
What if someone decides to do:
devlink port split device/1 count 2
what do we end up with? Presumably two network devices running with
two serdes lanes each (if supported by the hardware). At that point
can they then do:
devlink port split device/2 count 2
and end up with one network device with two 10G serdes lanes, and two
network devices each with one 10G serdes lane, or can port splitting
only be used on the "master" device/port ?
Unfortunately, I don't think I have any network devices that support
this so I can't experiment to find out how this should work; yes, I
have a Mellanox card, but it supports a single 10G SFP+, and therefore
does not support port splitting.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
On Sat, 1 Feb 2020 11:49:19 +0000, Russell King - ARM Linux admin wrote:
> What if someone decides to do:
>
> devlink port split device/1 count 2
>
> what do we end up with? Presumably two network devices running with
> two serdes lanes each (if supported by the hardware). At that point
> can they then do:
>
> devlink port split device/2 count 2
>
> and end up with one network device with two 10G serdes lanes, and two
> network devices each with one 10G serdes lane,
I think all your guesses are correct, it's a pretty straight forward
API, but it's also pretty thin, and some of the logic is in FW, so
there isn't much in a way of a standard on how things should behave :S
> or can port splitting only be used on the "master" device/port ?
I think both mlxsw and the NFP rejects re-split/further splitting.
Ports have to be unsplit first. So there is only one device for
splitting, and unsplitting can be done on any of the sub-devices.
> Unfortunately, I don't think I have any network devices that support
> this so I can't experiment to find out how this should work; yes, I
> have a Mellanox card, but it supports a single 10G SFP+, and therefore
> does not support port splitting.
I think you'd need a mlxsw or an nfp to play with this.
Maybe Jiri can clarify further :)
> -----Original Message-----
> From: Ard Biesheuvel <[email protected]>
> Sent: Friday, January 31, 2020 5:32 PM
> To: Marc Zyngier <[email protected]>
> Cc: Makarand Pawagi <[email protected]>; Calvin Johnson
> <[email protected]>; [email protected]; [email protected];
> Ioana Ciornei <[email protected]>; Cristi Sovaiala
> <[email protected]>; Hanjun Guo <[email protected]>; Will
> Deacon <[email protected]>; Lorenzo Pieralisi <[email protected]>;
> Pankaj Bansal <[email protected]>; [email protected]; Russell King
> <[email protected]>; ACPI Devel Maling List <[email protected]>;
> Len Brown <[email protected]>; Jason Cooper <[email protected]>; Andy
> Wang <[email protected]>; Varun Sethi <[email protected]>; Thomas
> Gleixner <[email protected]>; linux-arm-kernel <linux-arm-
> [email protected]>; Laurentiu Tudor <[email protected]>; Paul
> Yang <[email protected]>; <[email protected]>
> <[email protected]>; Rafael J. Wysocki <[email protected]>; Linux Kernel
> Mailing List <[email protected]>; Shameerali Kolothum Thodi
> <[email protected]>; Sudeep Holla
> <[email protected]>; Robin Murphy <[email protected]>
> Subject: Re: [EXT] Re: [PATCH] bus: fsl-mc: Add ACPI support for fsl-mc
>
> On Fri, 31 Jan 2020 at 12:06, Marc Zyngier <[email protected]> wrote:
> >
> > On 2020-01-31 10:35, Makarand Pawagi wrote:
> > >> -----Original Message-----
> > >> From: Lorenzo Pieralisi <[email protected]>
> > >> Sent: Tuesday, January 28, 2020 4:39 PM
> > >> To: Makarand Pawagi <[email protected]>
> > >> Cc: [email protected]; [email protected];
> > >> linux-arm- [email protected]; [email protected];
> > >> [email protected]; [email protected]; Cristi Sovaiala
> > >> <[email protected]>; Laurentiu Tudor
> > >> <[email protected]>; Ioana Ciornei <[email protected]>;
> > >> Varun Sethi <[email protected]>; Calvin Johnson
> > >> <[email protected]>; Pankaj Bansal <[email protected]>;
> > >> [email protected]; [email protected]; [email protected];
> > >> [email protected]; [email protected]; [email protected];
> > >> [email protected]; [email protected];
> > >> [email protected]; [email protected];
> > >> [email protected]; [email protected]
> > >> Subject: [EXT] Re: [PATCH] bus: fsl-mc: Add ACPI support for fsl-mc
> > >>
> > >> Caution: EXT Email
> > >>
> > >> On Tue, Jan 28, 2020 at 01:38:45PM +0530, Makarand Pawagi wrote:
> > >> > ACPI support is added in the fsl-mc driver. Driver will parse MC
> > >> > DSDT table to extract memory and other resorces.
> > >> >
> > >> > Interrupt (GIC ITS) information will be extracted from MADT table
> > >> > by drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c.
> > >> >
> > >> > IORT table will be parsed to configure DMA.
> > >> >
> > >> > Signed-off-by: Makarand Pawagi <[email protected]>
> > >> > ---
> > >> > drivers/acpi/arm64/iort.c | 53 +++++++++++++++++++++
> > >> > drivers/bus/fsl-mc/dprc-driver.c | 3 +-
> > >> > drivers/bus/fsl-mc/fsl-mc-bus.c | 48 +++++++++++++------
> > >> > drivers/bus/fsl-mc/fsl-mc-msi.c | 10 +++-
> > >> > drivers/bus/fsl-mc/fsl-mc-private.h | 4 +-
> > >> > drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c | 71
> > >> ++++++++++++++++++++++++++++-
> > >> > include/linux/acpi_iort.h | 5 ++
> > >> > 7 files changed, 174 insertions(+), 20 deletions(-)
> > >> >
> > >> > diff --git a/drivers/acpi/arm64/iort.c
> > >> > b/drivers/acpi/arm64/iort.c index 33f7198..beb9cd5 100644
> > >> > --- a/drivers/acpi/arm64/iort.c
> > >> > +++ b/drivers/acpi/arm64/iort.c
> > >> > @@ -15,6 +15,7 @@
> > >> > #include <linux/kernel.h>
> > >> > #include <linux/list.h>
> > >> > #include <linux/pci.h>
> > >> > +#include <linux/fsl/mc.h>
> > >> > #include <linux/platform_device.h> #include <linux/slab.h>
> > >> >
> > >> > @@ -622,6 +623,29 @@ static int iort_dev_find_its_id(struct
> > >> > device *dev, u32 req_id, }
> > >> >
> > >> > /**
> > >> > + * iort_get_fsl_mc_device_domain() - Find MSI domain related to
> > >> > +a device
> > >> > + * @dev: The device.
> > >> > + * @mc_icid: ICID for the fsl_mc device.
> > >> > + *
> > >> > + * Returns: the MSI domain for this device, NULL otherwise */
> > >> > +struct irq_domain *iort_get_fsl_mc_device_domain(struct device *dev,
> > >> > + u32 mc_icid) {
> > >> > + struct fwnode_handle *handle;
> > >> > + int its_id;
> > >> > +
> > >> > + if (iort_dev_find_its_id(dev, mc_icid, 0, &its_id))
> > >> > + return NULL;
> > >> > +
> > >> > + handle = iort_find_domain_token(its_id);
> > >> > + if (!handle)
> > >> > + return NULL;
> > >> > +
> > >> > + return irq_find_matching_fwnode(handle,
> > >> > +DOMAIN_BUS_FSL_MC_MSI); }
> > >>
> > >> NAK
> > >>
> > >> I am not willing to take platform specific code in the generic IORT
> > >> layer.
> > >>
> > >> ACPI on ARM64 works on platforms that comply with SBSA/SBBR
> > >> guidelines:
> > >>
> > >>
> > >> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fd
> > >> eveloper.arm.com%2Farchitectures%2Fplatform-design%2Fserver-systems
> > >>
> &data=02%7C01%7Cpankaj.bansal%40nxp.com%7Cdb56d889d85646277ee
> 30
> > >>
> 8d7a64562fa%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6371606
> 892
> > >>
> 50769265&sdata=C7nCty8%2BVeuq6VhcEUXCwiAinN01rCfe12NRVnXJCIY%
> 3D
> > >> &reserved=0
> > >>
> > >> Deviating from those requires butchering ACPI specifications (ie
> > >> IORT) and related kernel code which goes totally against what ACPI
> > >> is meant for on ARM64 systems, so there is no upstream pathway for
> > >> this code I am afraid.
> > >>
> > > Reason of adding this platform specific function in the generic IORT
> > > layer is That iort_get_device_domain() only deals with PCI bus
> > > (DOMAIN_BUS_PCI_MSI).
> > >
> > > fsl-mc objects when probed, need to find irq_domain which is
> > > associated with the fsl-mc bus (DOMAIN_BUS_FSL_MC_MSI). It will not
> > > be possible to do that if we do not add this function because there
> > > are no other suitable APIs exported by IORT layer to do the job.
> >
> > I think we all understood the patch. What both Lorenzo and myself are
> > saying is that we do not want non-PCI support in IORT.
> >
>
> IORT supports platform devices (aka named components) as well, and
> there is some support for platform MSIs in the GIC layer.
>
> So it may be possible to hide your exotic bus from the OS entirely,
> and make the firmware instantiate a DSDT with device objects and
> associated IORT nodes that describe whatever lives on that bus as
> named components.
>
> That way, you will not have to change the OS at all, so your hardware
> will not only be supported in linux v5.7+, it will also be supported
> by OSes that commercial distro vendors are shipping today. *That* is
> the whole point of using ACPI.
>
> If you are going to bother and modify the OS, you lose this advantage,
> and ACPI gives you no benefit over DT at all.
I am replying to old message in this conversation, because the discussion got sidetracked from IORT
table to SFP/QSFP/devlink stuff from this point onwards, which is not related to IORT.
I will only focus on representing the MC device in IORT and using the same in linux.
As Ard said:
"IORT supports platform devices (aka named components) as well, and
there is some support for platform MSIs in the GIC layer."
We can represent MC bus as named component in IORT table and use platform MSIs.
The only caveat is that with current implementation of platform MSIs, the Input id of a device is not considered.
https://elixir.bootlin.com/linux/latest/source/drivers/irqchip/irq-gic-v3-its-platform-msi.c#L50
https://elixir.bootlin.com/linux/latest/source/drivers/acpi/arm64/iort.c#L464
While, IORT spec doesn't specify any such limitation.
we can easily update iort.c to remove this limitation.
But, I am not sure how the input id would be passed from platform MSI GIC layer to IORT.
Most obviously, the input id should be supplied by dev itself.
Any thoughts?
If we go by this path, I think we can remove special handling for fsl-mc for of* cases altogether.
https://elixir.bootlin.com/linux/latest/source/drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c
Regards,
Pankaj Bansal
On 2020-02-14 15:05, Pankaj Bansal wrote:
>> -----Original Message-----
>> From: Ard Biesheuvel <[email protected]>
>> Sent: Friday, January 31, 2020 5:32 PM
>> To: Marc Zyngier <[email protected]>
>> Cc: Makarand Pawagi <[email protected]>; Calvin Johnson
>> <[email protected]>; [email protected]; [email protected];
>> Ioana Ciornei <[email protected]>; Cristi Sovaiala
>> <[email protected]>; Hanjun Guo <[email protected]>; Will
>> Deacon <[email protected]>; Lorenzo Pieralisi
>> <[email protected]>;
>> Pankaj Bansal <[email protected]>; [email protected]; Russell King
>> <[email protected]>; ACPI Devel Maling List
>> <[email protected]>;
>> Len Brown <[email protected]>; Jason Cooper <[email protected]>; Andy
>> Wang <[email protected]>; Varun Sethi <[email protected]>; Thomas
>> Gleixner <[email protected]>; linux-arm-kernel <linux-arm-
>> [email protected]>; Laurentiu Tudor
>> <[email protected]>; Paul
>> Yang <[email protected]>; <[email protected]>
>> <[email protected]>; Rafael J. Wysocki <[email protected]>; Linux
>> Kernel
>> Mailing List <[email protected]>; Shameerali Kolothum Thodi
>> <[email protected]>; Sudeep Holla
>> <[email protected]>; Robin Murphy <[email protected]>
>> Subject: Re: [EXT] Re: [PATCH] bus: fsl-mc: Add ACPI support for
>> fsl-mc
>>
>> On Fri, 31 Jan 2020 at 12:06, Marc Zyngier <[email protected]> wrote:
>> >
>> > On 2020-01-31 10:35, Makarand Pawagi wrote:
>> > >> -----Original Message-----
>> > >> From: Lorenzo Pieralisi <[email protected]>
>> > >> Sent: Tuesday, January 28, 2020 4:39 PM
>> > >> To: Makarand Pawagi <[email protected]>
>> > >> Cc: [email protected]; [email protected];
>> > >> linux-arm- [email protected]; [email protected];
>> > >> [email protected]; [email protected]; Cristi Sovaiala
>> > >> <[email protected]>; Laurentiu Tudor
>> > >> <[email protected]>; Ioana Ciornei <[email protected]>;
>> > >> Varun Sethi <[email protected]>; Calvin Johnson
>> > >> <[email protected]>; Pankaj Bansal <[email protected]>;
>> > >> [email protected]; [email protected]; [email protected];
>> > >> [email protected]; [email protected]; [email protected];
>> > >> [email protected]; [email protected];
>> > >> [email protected]; [email protected];
>> > >> [email protected]; [email protected]
>> > >> Subject: [EXT] Re: [PATCH] bus: fsl-mc: Add ACPI support for fsl-mc
>> > >>
>> > >> Caution: EXT Email
>> > >>
>> > >> On Tue, Jan 28, 2020 at 01:38:45PM +0530, Makarand Pawagi wrote:
>> > >> > ACPI support is added in the fsl-mc driver. Driver will parse MC
>> > >> > DSDT table to extract memory and other resorces.
>> > >> >
>> > >> > Interrupt (GIC ITS) information will be extracted from MADT table
>> > >> > by drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c.
>> > >> >
>> > >> > IORT table will be parsed to configure DMA.
>> > >> >
>> > >> > Signed-off-by: Makarand Pawagi <[email protected]>
>> > >> > ---
>> > >> > drivers/acpi/arm64/iort.c | 53 +++++++++++++++++++++
>> > >> > drivers/bus/fsl-mc/dprc-driver.c | 3 +-
>> > >> > drivers/bus/fsl-mc/fsl-mc-bus.c | 48 +++++++++++++------
>> > >> > drivers/bus/fsl-mc/fsl-mc-msi.c | 10 +++-
>> > >> > drivers/bus/fsl-mc/fsl-mc-private.h | 4 +-
>> > >> > drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c | 71
>> > >> ++++++++++++++++++++++++++++-
>> > >> > include/linux/acpi_iort.h | 5 ++
>> > >> > 7 files changed, 174 insertions(+), 20 deletions(-)
>> > >> >
>> > >> > diff --git a/drivers/acpi/arm64/iort.c
>> > >> > b/drivers/acpi/arm64/iort.c index 33f7198..beb9cd5 100644
>> > >> > --- a/drivers/acpi/arm64/iort.c
>> > >> > +++ b/drivers/acpi/arm64/iort.c
>> > >> > @@ -15,6 +15,7 @@
>> > >> > #include <linux/kernel.h>
>> > >> > #include <linux/list.h>
>> > >> > #include <linux/pci.h>
>> > >> > +#include <linux/fsl/mc.h>
>> > >> > #include <linux/platform_device.h> #include <linux/slab.h>
>> > >> >
>> > >> > @@ -622,6 +623,29 @@ static int iort_dev_find_its_id(struct
>> > >> > device *dev, u32 req_id, }
>> > >> >
>> > >> > /**
>> > >> > + * iort_get_fsl_mc_device_domain() - Find MSI domain related to
>> > >> > +a device
>> > >> > + * @dev: The device.
>> > >> > + * @mc_icid: ICID for the fsl_mc device.
>> > >> > + *
>> > >> > + * Returns: the MSI domain for this device, NULL otherwise */
>> > >> > +struct irq_domain *iort_get_fsl_mc_device_domain(struct device *dev,
>> > >> > + u32 mc_icid) {
>> > >> > + struct fwnode_handle *handle;
>> > >> > + int its_id;
>> > >> > +
>> > >> > + if (iort_dev_find_its_id(dev, mc_icid, 0, &its_id))
>> > >> > + return NULL;
>> > >> > +
>> > >> > + handle = iort_find_domain_token(its_id);
>> > >> > + if (!handle)
>> > >> > + return NULL;
>> > >> > +
>> > >> > + return irq_find_matching_fwnode(handle,
>> > >> > +DOMAIN_BUS_FSL_MC_MSI); }
>> > >>
>> > >> NAK
>> > >>
>> > >> I am not willing to take platform specific code in the generic IORT
>> > >> layer.
>> > >>
>> > >> ACPI on ARM64 works on platforms that comply with SBSA/SBBR
>> > >> guidelines:
>> > >>
>> > >>
>> > >> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fd
>> > >> eveloper.arm.com%2Farchitectures%2Fplatform-design%2Fserver-systems
>> > >>
>> &data=02%7C01%7Cpankaj.bansal%40nxp.com%7Cdb56d889d85646277ee
>> 30
>> > >>
>> 8d7a64562fa%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6371606
>> 892
>> > >>
>> 50769265&sdata=C7nCty8%2BVeuq6VhcEUXCwiAinN01rCfe12NRVnXJCIY%
>> 3D
>> > >> &reserved=0
>> > >>
>> > >> Deviating from those requires butchering ACPI specifications (ie
>> > >> IORT) and related kernel code which goes totally against what ACPI
>> > >> is meant for on ARM64 systems, so there is no upstream pathway for
>> > >> this code I am afraid.
>> > >>
>> > > Reason of adding this platform specific function in the generic IORT
>> > > layer is That iort_get_device_domain() only deals with PCI bus
>> > > (DOMAIN_BUS_PCI_MSI).
>> > >
>> > > fsl-mc objects when probed, need to find irq_domain which is
>> > > associated with the fsl-mc bus (DOMAIN_BUS_FSL_MC_MSI). It will not
>> > > be possible to do that if we do not add this function because there
>> > > are no other suitable APIs exported by IORT layer to do the job.
>> >
>> > I think we all understood the patch. What both Lorenzo and myself are
>> > saying is that we do not want non-PCI support in IORT.
>> >
>>
>> IORT supports platform devices (aka named components) as well, and
>> there is some support for platform MSIs in the GIC layer.
>>
>> So it may be possible to hide your exotic bus from the OS entirely,
>> and make the firmware instantiate a DSDT with device objects and
>> associated IORT nodes that describe whatever lives on that bus as
>> named components.
>>
>> That way, you will not have to change the OS at all, so your hardware
>> will not only be supported in linux v5.7+, it will also be supported
>> by OSes that commercial distro vendors are shipping today. *That* is
>> the whole point of using ACPI.
>>
>> If you are going to bother and modify the OS, you lose this advantage,
>> and ACPI gives you no benefit over DT at all.
>
> I am replying to old message in this conversation, because the
> discussion got sidetracked from IORT
> table to SFP/QSFP/devlink stuff from this point onwards, which is not
> related to IORT.
> I will only focus on representing the MC device in IORT and using the
> same in linux.
> As Ard said:
> "IORT supports platform devices (aka named components) as well, and
> there is some support for platform MSIs in the GIC layer."
>
> We can represent MC bus as named component in IORT table and use
> platform MSIs.
> The only caveat is that with current implementation of platform MSIs,
> the Input id of a device is not considered.
> https://elixir.bootlin.com/linux/latest/source/drivers/irqchip/irq-gic-v3-its-platform-msi.c#L50
> https://elixir.bootlin.com/linux/latest/source/drivers/acpi/arm64/iort.c#L464
I don't understand what you mean. Platform MSI using IORT uses the DevID
of end-points. How could the ITS could work without specifying a DevID?
See for example how the SMMUv3 driver uses platform MSI.
> While, IORT spec doesn't specify any such limitation.
>
> we can easily update iort.c to remove this limitation.
> But, I am not sure how the input id would be passed from platform MSI
> GIC layer to IORT.
> Most obviously, the input id should be supplied by dev itself.
Why should the device know about its own ID? That's a bus/interconnect
thing. And nothing should be passed *to* IORT. IORT is the source.
> Any thoughts?
I think that in this thread, we have been fairly explicit about what our
train of though was.
M.
--
Jazz is not dead. It just smells funny...
> -----Original Message-----
> From: Lorenzo Pieralisi <[email protected]>
> Sent: Monday, February 17, 2020 8:55 PM
> To: Pankaj Bansal <[email protected]>
> Cc: Marc Zyngier <[email protected]>; Ard Biesheuvel
> <[email protected]>; Makarand Pawagi <[email protected]>;
> Calvin Johnson <[email protected]>; [email protected];
> [email protected]; Ioana Ciornei <[email protected]>; Cristi
> Sovaiala <[email protected]>; Hanjun Guo <[email protected]>;
> Will Deacon <[email protected]>; [email protected]; Russell King
> <[email protected]>; ACPI Devel Maling List <[email protected]>;
> Len Brown <[email protected]>; Jason Cooper <[email protected]>; Andy
> Wang <[email protected]>; Varun Sethi <[email protected]>; Thomas
> Gleixner <[email protected]>; linux-arm-kernel <linux-arm-
> [email protected]>; Laurentiu Tudor <[email protected]>; Paul
> Yang <[email protected]>; [email protected]; Rafael J. Wysocki
> <[email protected]>; Linux Kernel Mailing List <[email protected]>;
> Shameerali Kolothum Thodi <[email protected]>;
> Sudeep Holla <[email protected]>; Robin Murphy
> <[email protected]>
> Subject: Re: [EXT] Re: [PATCH] bus: fsl-mc: Add ACPI support for fsl-mc
>
> On Mon, Feb 17, 2020 at 12:35:12PM +0000, Pankaj Bansal wrote:
> >
> >
> > > -----Original Message-----
> > > From: Lorenzo Pieralisi <[email protected]>
> > > Sent: Friday, February 14, 2020 11:20 PM
> > > To: Pankaj Bansal <[email protected]>
> > > Cc: Marc Zyngier <[email protected]>; Ard Biesheuvel
> > > <[email protected]>; Makarand Pawagi
> <[email protected]>;
> > > Calvin Johnson <[email protected]>; [email protected];
> > > [email protected]; Ioana Ciornei <[email protected]>; Cristi
> > > Sovaiala <[email protected]>; Hanjun Guo
> <[email protected]>;
> > > Will Deacon <[email protected]>; [email protected]; Russell King
> > > <[email protected]>; ACPI Devel Maling List <linux-
> [email protected]>;
> > > Len Brown <[email protected]>; Jason Cooper <[email protected]>;
> Andy
> > > Wang <[email protected]>; Varun Sethi <[email protected]>; Thomas
> > > Gleixner <[email protected]>; linux-arm-kernel <linux-arm-
> > > [email protected]>; Laurentiu Tudor <[email protected]>;
> Paul
> > > Yang <[email protected]>; [email protected]; Rafael J. Wysocki
> > > <[email protected]>; Linux Kernel Mailing List <linux-
> [email protected]>;
> > > Shameerali Kolothum Thodi <[email protected]>;
> > > Sudeep Holla <[email protected]>; Robin Murphy
> > > <[email protected]>
> > > Subject: Re: [EXT] Re: [PATCH] bus: fsl-mc: Add ACPI support for fsl-mc
> > >
> > > On Fri, Feb 14, 2020 at 04:35:10PM +0000, Pankaj Bansal wrote:
> > >
> > > [...]
> > >
> > > > > -----Original Message-----
> > > > > From: Lorenzo Pieralisi <[email protected]>
> > > > > Sent: Friday, February 14, 2020 9:50 PM
> > > > > To: Pankaj Bansal <[email protected]>
> > > > > Cc: Marc Zyngier <[email protected]>; Ard Biesheuvel
> > > > > <[email protected]>; Makarand Pawagi
> > > <[email protected]>;
> > > > > Calvin Johnson <[email protected]>; [email protected];
> > > > > [email protected]; Ioana Ciornei <[email protected]>; Cristi
> > > > > Sovaiala <[email protected]>; Hanjun Guo
> > > <[email protected]>;
> > > > > Will Deacon <[email protected]>; [email protected]; Russell King
> > > > > <[email protected]>; ACPI Devel Maling List <linux-
> > > [email protected]>;
> > > > > Len Brown <[email protected]>; Jason Cooper <[email protected]>;
> > > Andy
> > > > > Wang <[email protected]>; Varun Sethi <[email protected]>;
> Thomas
> > > > > Gleixner <[email protected]>; linux-arm-kernel <linux-arm-
> > > > > [email protected]>; Laurentiu Tudor
> <[email protected]>;
> > > Paul
> > > > > Yang <[email protected]>; [email protected]; Rafael J. Wysocki
> > > > > <[email protected]>; Linux Kernel Mailing List <linux-
> > > [email protected]>;
> > > > > Shameerali Kolothum Thodi <[email protected]>;
> > > > > Sudeep Holla <[email protected]>; Robin Murphy
> > > > > <[email protected]>
> > > > > Subject: Re: [EXT] Re: [PATCH] bus: fsl-mc: Add ACPI support for fsl-mc
> > > Side note: would you mind removing the email headers (as above) in your
> > > replies please ?
>
> Read the question above please.
>
> [...]
>
> > > > As stated above, in Linux MC is a bus (just like PCI bus, AMBA bus etc)
> > > > There can be multiple devices attached to this bus. Moreover, we can
> > > dynamically create/destroy these devices.
> > > > Now, we want to represent this BUS (not individual devices connected to
> bus)
> > > in IORT table.
> > > > The only possible way right now we see is that we describe it as Named
> > > components having a pool of ID mappings.
> > > > As and when devices are created and attached to bus, we sift through this
> pool
> > > to correctly determine the output ID for the device.
> > > > Now the input ID that we provide, can come from device itself.
> > > > Then we can use the Platform MSI framework for MC bus devices.
> > >
> > > So are you asking me if that's OK ? Or there is something you can't
> > > describe with IORT ?
> >
> > I am asking if that would be acceptable?
> > i.e. we represent MC bus as Named component is IORT table with a pool of IDs
> (without single ID mapping flag)
> > and then we use the Platform MSI framework for all children devices of MC
> bus.
> > Note that it would require the Platform MSI layer to correctly pass an input id
> for a platform device to IORT layer.
>
> How is this solved in DT ? You don't seem to need any DT binding on top
> of the msi-parent property, which is equivalent to IORT single mappings
> AFAICS so I would like to understand the whole DT flow (so that I
> understand how this FSL bus works) before commenting any further.
In DT case, we create the domain DOMAIN_BUS_FSL_MC_MSI for MC bus and it's children.
And then when MC child device is created, we search the "msi-parent" property from the MC
DT node and get the ITS associated with MC bus. Then we search DOMAIN_BUS_FSL_MC_MSI
on that ITS. Once we find the domain, we can call msi_domain_alloc_irqs for that domain.
This is exactly what we tried to do initially with ACPI. But the searching DOMAIN_BUS_FSL_MC_MSI
associated to an ITS, is something that is part of drivers/acpi/arm64/iort.c.
(similar to DOMAIN_BUS_PLATFORM_MSI and DOMAIN_BUS_PCI_MSI)
>
> > And IORT layer ought to retrieve the output id based on single ID mapping flag
> as well as input id.
> >
> > >
> > > Side note: can you explain to me please how the MSI allocation flow
> > > and kernel data structures/drivers are modeled in DT ? I had a quick
> > > look at:
> > >
> > > drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c
> > >
> > > and to start with, does that code imply that we create a
> > > DOMAIN_BUS_FSL_MC_MSI on ALL DT systems with an ITS device node ?
> >
> > Yes. It's being done for all DT systems having ITS node.
>
> This does not seem correct to me, I will let Marc comment on
> the matter.
>
> > The domain creation is handled in drivers/bus/fsl-mc/fsl-mc-msi.c
>
> Thanks,
> Lorenzo
Hi Pankaj,
On 2020/2/18 16:00, Pankaj Bansal (OSS) wrote:
[...]
>>>> Side note: would you mind removing the email headers (as above) in your
>>>> replies please ?
>>
>> Read the question above please.
>>
>> [...]
>>
>>>>> As stated above, in Linux MC is a bus (just like PCI bus, AMBA bus etc)
>>>>> There can be multiple devices attached to this bus. Moreover, we can
>>>> dynamically create/destroy these devices.
>>>>> Now, we want to represent this BUS (not individual devices connected to
>> bus)
>>>> in IORT table.
>>>>> The only possible way right now we see is that we describe it as Named
>>>> components having a pool of ID mappings.
>>>>> As and when devices are created and attached to bus, we sift through this
>> pool
>>>> to correctly determine the output ID for the device.
>>>>> Now the input ID that we provide, can come from device itself.
>>>>> Then we can use the Platform MSI framework for MC bus devices.
>>>>
>>>> So are you asking me if that's OK ? Or there is something you can't
>>>> describe with IORT ?
>>>
>>> I am asking if that would be acceptable?
>>> i.e. we represent MC bus as Named component is IORT table with a pool of IDs
>> (without single ID mapping flag)
>>> and then we use the Platform MSI framework for all children devices of MC
>> bus.
>>> Note that it would require the Platform MSI layer to correctly pass an input id
>> for a platform device to IORT layer.
>>
>> How is this solved in DT ? You don't seem to need any DT binding on top
>> of the msi-parent property, which is equivalent to IORT single mappings
>> AFAICS so I would like to understand the whole DT flow (so that I
>> understand how this FSL bus works) before commenting any further.
>
> In DT case, we create the domain DOMAIN_BUS_FSL_MC_MSI for MC bus and it's children.
> And then when MC child device is created, we search the "msi-parent" property from the MC
> DT node and get the ITS associated with MC bus. Then we search DOMAIN_BUS_FSL_MC_MSI
> on that ITS. Once we find the domain, we can call msi_domain_alloc_irqs for that domain.
>
> This is exactly what we tried to do initially with ACPI. But the searching DOMAIN_BUS_FSL_MC_MSI
> associated to an ITS, is something that is part of drivers/acpi/arm64/iort.c.
> (similar to DOMAIN_BUS_PLATFORM_MSI and DOMAIN_BUS_PCI_MSI)
Can you have a look at mbigen driver (drivers/irqchip/irq-mbigen.c) to see if
it helps you?
mbigen is an irq converter to convert device's wired interrupts into MSI
(connecting to ITS), which will alloc a bunch of MSIs from ITS platform MSI
domain at the setup.
Thanks
Hanjun
> >>>>> As stated above, in Linux MC is a bus (just like PCI bus, AMBA bus etc)
> >>>>> There can be multiple devices attached to this bus. Moreover, we can
> >>>> dynamically create/destroy these devices.
> >>>>> Now, we want to represent this BUS (not individual devices connected to
> >> bus)
> >>>> in IORT table.
> >>>>> The only possible way right now we see is that we describe it as Named
> >>>> components having a pool of ID mappings.
> >>>>> As and when devices are created and attached to bus, we sift through this
> >> pool
> >>>> to correctly determine the output ID for the device.
> >>>>> Now the input ID that we provide, can come from device itself.
> >>>>> Then we can use the Platform MSI framework for MC bus devices.
> >>>>
> >>>> So are you asking me if that's OK ? Or there is something you can't
> >>>> describe with IORT ?
> >>>
> >>> I am asking if that would be acceptable?
> >>> i.e. we represent MC bus as Named component is IORT table with a pool of
> IDs
> >> (without single ID mapping flag)
> >>> and then we use the Platform MSI framework for all children devices of MC
> >> bus.
> >>> Note that it would require the Platform MSI layer to correctly pass an input
> id
> >> for a platform device to IORT layer.
> >>
> >> How is this solved in DT ? You don't seem to need any DT binding on top
> >> of the msi-parent property, which is equivalent to IORT single mappings
> >> AFAICS so I would like to understand the whole DT flow (so that I
> >> understand how this FSL bus works) before commenting any further.
> >
> > In DT case, we create the domain DOMAIN_BUS_FSL_MC_MSI for MC bus and
> it's children.
> > And then when MC child device is created, we search the "msi-parent"
> property from the MC
> > DT node and get the ITS associated with MC bus. Then we search
> DOMAIN_BUS_FSL_MC_MSI
> > on that ITS. Once we find the domain, we can call msi_domain_alloc_irqs for
> that domain.
> >
> > This is exactly what we tried to do initially with ACPI. But the searching
> DOMAIN_BUS_FSL_MC_MSI
> > associated to an ITS, is something that is part of drivers/acpi/arm64/iort.c.
> > (similar to DOMAIN_BUS_PLATFORM_MSI and DOMAIN_BUS_PCI_MSI)
>
> Can you have a look at mbigen driver (drivers/irqchip/irq-mbigen.c) to see if
> it helps you?
>
> mbigen is an irq converter to convert device's wired interrupts into MSI
> (connecting to ITS), which will alloc a bunch of MSIs from ITS platform MSI
> domain at the setup.
Unfortunately this is not the same case as ours. As I see Hisilicon IORT table
Is using single id mapping with named components.
https://github.com/tianocore/edk2-platforms/blob/master/Silicon/Hisilicon/Hi1616/D05AcpiTables/D05Iort.asl#L300
while we are not:
https://source.codeaurora.org/external/qoriq/qoriq-components/edk2-platforms/tree/Platform/NXP/LX2160aRdbPkg/AcpiTables/Iort.aslc?h=LX2160_UEFI_ACPI_EAR1#n290
This is because as I said, we are trying to represent a bus in IORT via named components and
not individual devices connected to that bus.
>
> Thanks
> Hanjun
On Tue, Feb 18, 2020 at 12:48:39PM +0000, Pankaj Bansal (OSS) wrote:
[...]
> > > In DT case, we create the domain DOMAIN_BUS_FSL_MC_MSI for MC bus and
> > it's children.
> > > And then when MC child device is created, we search the "msi-parent"
> > property from the MC
> > > DT node and get the ITS associated with MC bus. Then we search
> > DOMAIN_BUS_FSL_MC_MSI
> > > on that ITS. Once we find the domain, we can call msi_domain_alloc_irqs for
> > that domain.
> > >
> > > This is exactly what we tried to do initially with ACPI. But the searching
> > DOMAIN_BUS_FSL_MC_MSI
> > > associated to an ITS, is something that is part of drivers/acpi/arm64/iort.c.
> > > (similar to DOMAIN_BUS_PLATFORM_MSI and DOMAIN_BUS_PCI_MSI)
> >
> > Can you have a look at mbigen driver (drivers/irqchip/irq-mbigen.c) to see if
> > it helps you?
> >
> > mbigen is an irq converter to convert device's wired interrupts into MSI
> > (connecting to ITS), which will alloc a bunch of MSIs from ITS platform MSI
> > domain at the setup.
>
> Unfortunately this is not the same case as ours. As I see Hisilicon IORT table
> Is using single id mapping with named components.
>
> https://github.com/tianocore/edk2-platforms/blob/master/Silicon/Hisilicon/Hi1616/D05AcpiTables/D05Iort.asl#L300
>
> while we are not:
>
> https://source.codeaurora.org/external/qoriq/qoriq-components/edk2-platforms/tree/Platform/NXP/LX2160aRdbPkg/AcpiTables/Iort.aslc?h=LX2160_UEFI_ACPI_EAR1#n290
>
> This is because as I said, we are trying to represent a bus in IORT
> via named components and not individual devices connected to that bus.
I had a thorough look into this and strictly speaking there is no
*mapping* requirement at all, all you need to know is what ITS the FSL
MC bus is mapping MSIs to. Which brings me to the next question (which
is orthogonal to how to model FSL MC in IORT, that has to be discussed
but I want to have a full picture in mind first).
When you probe the FSL MC as a platform device, the ACPI core,
through IORT (if you add the 1:1 mapping as an array of single
mappings) already link the platform device to ITS platform
device MSI domain (acpi_configure_pmsi_domain()).
The associated fwnode is the *same* (IIUC) as for the
DOMAIN_BUS_FSL_MC_MSI and ITS DOMAIN_BUS_NEXUS, so in practice
you don't need IORT code to retrieve the DOMAIN_BUS_FSL_MC_MSI
domain, the fwnode is the same as the one in the FSL MC platform
device IRQ domain->fwnode pointer and you can use it to
retrieve the DOMAIN_BUS_FSL_MC_MSI domain through it.
Is my reading correct ?
Overall, DOMAIN_BUS_FSL_MC_MSI is just an MSI layer to override the
provide the MSI domain ->prepare hook (ie to stash the MC device id), no
more (ie its_fsl_mc_msi_prepare()).
That's it for the MSI layer - I need to figure out whether we *want* to
extend IORT (and/or ACPI) to defined bindings for "additional busses",
what I write above is a summary of my understanding, I have not made my
mind up yet.
As for the IOMMU code, it seems like the only thing needed is
extending named components configuration to child devices,
hierarchically.
As Marc already mentioned, IOMMU and IRQ code must be separate for
future postings but first we need to find a suitable answer to
the problem at hand.
Lorenzo
On 18/02/2020 2:46 pm, Lorenzo Pieralisi wrote:
> On Tue, Feb 18, 2020 at 12:48:39PM +0000, Pankaj Bansal (OSS) wrote:
>
> [...]
>
>>>> In DT case, we create the domain DOMAIN_BUS_FSL_MC_MSI for MC bus and
>>> it's children.
>>>> And then when MC child device is created, we search the "msi-parent"
>>> property from the MC
>>>> DT node and get the ITS associated with MC bus. Then we search
>>> DOMAIN_BUS_FSL_MC_MSI
>>>> on that ITS. Once we find the domain, we can call msi_domain_alloc_irqs for
>>> that domain.
>>>>
>>>> This is exactly what we tried to do initially with ACPI. But the searching
>>> DOMAIN_BUS_FSL_MC_MSI
>>>> associated to an ITS, is something that is part of drivers/acpi/arm64/iort.c.
>>>> (similar to DOMAIN_BUS_PLATFORM_MSI and DOMAIN_BUS_PCI_MSI)
>>>
>>> Can you have a look at mbigen driver (drivers/irqchip/irq-mbigen.c) to see if
>>> it helps you?
>>>
>>> mbigen is an irq converter to convert device's wired interrupts into MSI
>>> (connecting to ITS), which will alloc a bunch of MSIs from ITS platform MSI
>>> domain at the setup.
>>
>> Unfortunately this is not the same case as ours. As I see Hisilicon IORT table
>> Is using single id mapping with named components.
>>
>> https://github.com/tianocore/edk2-platforms/blob/master/Silicon/Hisilicon/Hi1616/D05AcpiTables/D05Iort.asl#L300
>>
>> while we are not:
>>
>> https://source.codeaurora.org/external/qoriq/qoriq-components/edk2-platforms/tree/Platform/NXP/LX2160aRdbPkg/AcpiTables/Iort.aslc?h=LX2160_UEFI_ACPI_EAR1#n290
>>
>> This is because as I said, we are trying to represent a bus in IORT
>> via named components and not individual devices connected to that bus.
>
> I had a thorough look into this and strictly speaking there is no
> *mapping* requirement at all, all you need to know is what ITS the FSL
> MC bus is mapping MSIs to. Which brings me to the next question (which
> is orthogonal to how to model FSL MC in IORT, that has to be discussed
> but I want to have a full picture in mind first).
>
> When you probe the FSL MC as a platform device, the ACPI core,
> through IORT (if you add the 1:1 mapping as an array of single
> mappings) already link the platform device to ITS platform
> device MSI domain (acpi_configure_pmsi_domain()).
>
> The associated fwnode is the *same* (IIUC) as for the
> DOMAIN_BUS_FSL_MC_MSI and ITS DOMAIN_BUS_NEXUS, so in practice
> you don't need IORT code to retrieve the DOMAIN_BUS_FSL_MC_MSI
> domain, the fwnode is the same as the one in the FSL MC platform
> device IRQ domain->fwnode pointer and you can use it to
> retrieve the DOMAIN_BUS_FSL_MC_MSI domain through it.
>
> Is my reading correct ?
>
> Overall, DOMAIN_BUS_FSL_MC_MSI is just an MSI layer to override the
> provide the MSI domain ->prepare hook (ie to stash the MC device id), no
> more (ie its_fsl_mc_msi_prepare()).
>
> That's it for the MSI layer - I need to figure out whether we *want* to
> extend IORT (and/or ACPI) to defined bindings for "additional busses",
> what I write above is a summary of my understanding, I have not made my
> mind up yet.
I'm really not sure we'd need to go near any bindings - the IORT spec
*can* reasonably describe "giant black box of DPAA2 stuff" as a single
named component, and that's arguably the most accurate abstraction
already, even when it comes to the namespace device. This isn't a bus in
any traditional sense, it's a set of accelerator components with an
interface to dynamically configure them into custom pipelines, and the
expected use-case seems to be for userspace to freely reconfigure
whatever virtual network adapters it wants at any given time. Thus I
don't see that it's logical or even practical for firmware itself to be
involved beyond describing "here's your toolbox", and in particular,
basing any decisions on the particular way that DPAA2 has been
shoehorned into the Linux driver model would almost certainly be a step
in the wrong direction.
IMO the scope of this issue belongs entirely within the
implementation(s) of Linux's own abstraction layers.
Robin.
> As for the IOMMU code, it seems like the only thing needed i
> extending named components configuration to child devices,
> hierarchically.
>
> As Marc already mentioned, IOMMU and IRQ code must be separate for
> future postings but first we need to find a suitable answer to
> the problem at hand.
>
> Lorenzo
>
Hi Lorenzo,
On 2/18/2020 4:46 PM, Lorenzo Pieralisi wrote:
> On Tue, Feb 18, 2020 at 12:48:39PM +0000, Pankaj Bansal (OSS) wrote:
>
> [...]
>
>>>> In DT case, we create the domain DOMAIN_BUS_FSL_MC_MSI for MC bus and
>>> it's children.
>>>> And then when MC child device is created, we search the "msi-parent"
>>> property from the MC
>>>> DT node and get the ITS associated with MC bus. Then we search
>>> DOMAIN_BUS_FSL_MC_MSI
>>>> on that ITS. Once we find the domain, we can call msi_domain_alloc_irqs for
>>> that domain.
>>>> This is exactly what we tried to do initially with ACPI. But the searching
>>> DOMAIN_BUS_FSL_MC_MSI
>>>> associated to an ITS, is something that is part of drivers/acpi/arm64/iort.c.
>>>> (similar to DOMAIN_BUS_PLATFORM_MSI and DOMAIN_BUS_PCI_MSI)
>>> Can you have a look at mbigen driver (drivers/irqchip/irq-mbigen.c) to see if
>>> it helps you?
>>>
>>> mbigen is an irq converter to convert device's wired interrupts into MSI
>>> (connecting to ITS), which will alloc a bunch of MSIs from ITS platform MSI
>>> domain at the setup.
>> Unfortunately this is not the same case as ours. As I see Hisilicon IORT table
>> Is using single id mapping with named components.
>>
>> https://github.com/tianocore/edk2-platforms/blob/master/Silicon/Hisilicon/Hi1616/D05AcpiTables/D05Iort.asl#L300
>>
>> while we are not:
>>
>> https://source.codeaurora.org/external/qoriq/qoriq-components/edk2-platforms/tree/Platform/NXP/LX2160aRdbPkg/AcpiTables/Iort.aslc?h=LX2160_UEFI_ACPI_EAR1#n290
>>
>> This is because as I said, we are trying to represent a bus in IORT
>> via named components and not individual devices connected to that bus.
> I had a thorough look into this and strictly speaking there is no
> *mapping* requirement at all, all you need to know is what ITS the FSL
> MC bus is mapping MSIs to. Which brings me to the next question (which
> is orthogonal to how to model FSL MC in IORT, that has to be discussed
> but I want to have a full picture in mind first).
>
> When you probe the FSL MC as a platform device, the ACPI core,
> through IORT (if you add the 1:1 mapping as an array of single
> mappings) already link the platform device to ITS platform
> device MSI domain (acpi_configure_pmsi_domain()).
>
> The associated fwnode is the *same* (IIUC) as for the
> DOMAIN_BUS_FSL_MC_MSI and ITS DOMAIN_BUS_NEXUS, so in practice
> you don't need IORT code to retrieve the DOMAIN_BUS_FSL_MC_MSI
> domain, the fwnode is the same as the one in the FSL MC platform
> device IRQ domain->fwnode pointer and you can use it to
> retrieve the DOMAIN_BUS_FSL_MC_MSI domain through it.
>
> Is my reading correct ?
Thank you very much for your effort! Really appreciated! Yes, the
understanding is correct. I have prototyped this idea for DT, see below [1].
So, I get the fwnode from the platform device domain (because they are
the same with the devices underneath the MC-BUS bridge) and use the
fwnode to retrieve the MC-BUS domain.
>
> Overall, DOMAIN_BUS_FSL_MC_MSI is just an MSI layer to override the
> provide the MSI domain ->prepare hook (ie to stash the MC device id), no
> more (ie its_fsl_mc_msi_prepare()).
>
> That's it for the MSI layer - I need to figure out whether we *want* to
> extend IORT (and/or ACPI) to defined bindings for "additional busses",
> what I write above is a summary of my understanding, I have not made my
> mind up yet.
>
> As for the IOMMU code, it seems like the only thing needed is
> extending named components configuration to child devices,
> hierarchically.
Laurentiu used a similar approach for DMA configuration (again
prototyped for DT). [2]
It involves wiring up a custom .dma_configure for our devices as anyway,
it made little sense to pretend that these devices are platform devices
and trick the DT or ACPI layers into that. As a nice side effect, this
will allow to get rid of our existing hooks in the DT generic code.
>
> As Marc already mentioned, IOMMU and IRQ code must be separate for
> future postings but first we need to find a suitable answer to
> the problem at hand.
>
> Lorenzo
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
[1] MSI configuration
drivers/bus/fsl-mc/fsl-mc-msi.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/bus/fsl-mc/fsl-mc-msi.c
b/drivers/bus/fsl-mc/fsl-mc-msi.c
index 8b9c66d7c4ff..674f5a60109b 100644
--- a/drivers/bus/fsl-mc/fsl-mc-msi.c
+++ b/drivers/bus/fsl-mc/fsl-mc-msi.c
@@ -182,16 +182,23 @@ int fsl_mc_find_msi_domain(struct device
*mc_platform_dev,
{
struct irq_domain *msi_domain;
struct device_node *mc_of_node = mc_platform_dev->of_node;
+ struct fwnode_handle *fwnode;
- msi_domain = of_msi_get_domain(mc_platform_dev, mc_of_node,
- DOMAIN_BUS_FSL_MC_MSI);
+ msi_domain = dev_get_msi_domain(mc_platform_dev);
if (!msi_domain) {
pr_err("Unable to find fsl-mc MSI domain for %pOF\n",
mc_of_node);
return -ENOENT;
}
+ fwnode = msi_domain->fwnode;
+ msi_domain = irq_find_matching_fwnode(fwnode, DOMAIN_BUS_FSL_MC_MSI);
+ if (!msi_domain) {
+ pr_err("Unable to find fsl-mc MSI domain for %pOF\n",
+ mc_of_node);
+ return -ENOENT;
+ }
*mc_msi_domain = msi_domain;
return 0;
}
--
2.17.1
[2] DMA configuration
drivers/bus/fsl-mc/fsl-mc-bus.c | 42 ++++++++++++++++++++++++++++++++-
1 file changed, 41 insertions(+), 1 deletion(-)
diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c
b/drivers/bus/fsl-mc/fsl-mc-bus.c
index f9bc9c384ab5..5c6021a13612 100644
--- a/drivers/bus/fsl-mc/fsl-mc-bus.c
+++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
@@ -132,11 +132,51 @@ static int fsl_mc_bus_uevent(struct device *dev,
struct kobj_uevent_env *env)
static int fsl_mc_dma_configure(struct device *dev)
{
struct device *dma_dev = dev;
+ struct iommu_fwspec *fwspec;
+ const struct iommu_ops *iommu_ops;
+ struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
+ int ret;
+ u32 icid;
while (dev_is_fsl_mc(dma_dev))
dma_dev = dma_dev->parent;
- return of_dma_configure(dev, dma_dev->of_node, 0);
+ fwspec = dev_iommu_fwspec_get(dma_dev);
+ if (!fwspec) {
+ dev_err(dev, "%s: null fwspec\n", __func__);
+ return -ENODEV;
+ }
+ iommu_ops = iommu_ops_from_fwnode(fwspec->iommu_fwnode);
+ if (!iommu_ops) {
+ dev_err(dev, "%s: null iommu ops\n", __func__);
+ return -ENODEV;
+ }
+
+ ret = iommu_fwspec_init(dev, fwspec->iommu_fwnode, iommu_ops);
+ if (ret) {
+ dev_err(dev, "%s: iommu_fwspec_init failed with %d\n",
__func__, ret);
+ return ret;
+ }
+
+ icid = mc_dev->icid;
+ ret = iommu_fwspec_add_ids(dev, &icid, 1);
+ if (ret) {
+ dev_err(dev, "%s: iommu_fwspec_add_ids failed with %d\n",
__func__, ret);
+ return ret;
+ }
+
+ if (!device_iommu_mapped(dev)) {
+ ret = iommu_probe_device(dev);
+ if (ret) {
+ dev_err(dev, "%s: iommu_fwspec_add_ids failed with %d\n",
__func__, ret);
+ return ret;
+ }
+ }
+
+ arch_setup_dma_ops(dev, 0, *dma_dev->dma_mask + 1,
+ iommu_ops, true);
+
+ return 0;
}
static ssize_t modalias_show(struct device *dev, struct
device_attribute *attr,
--
2.17.1
Regards,
Diana
>
> On 18/02/2020 2:46 pm, Lorenzo Pieralisi wrote:
> > On Tue, Feb 18, 2020 at 12:48:39PM +0000, Pankaj Bansal (OSS) wrote:
> >
> > [...]
> >
> >>>> In DT case, we create the domain DOMAIN_BUS_FSL_MC_MSI for MC bus
> and
> >>> it's children.
> >>>> And then when MC child device is created, we search the "msi-parent"
> >>> property from the MC
> >>>> DT node and get the ITS associated with MC bus. Then we search
> >>> DOMAIN_BUS_FSL_MC_MSI
> >>>> on that ITS. Once we find the domain, we can call msi_domain_alloc_irqs
> for
> >>> that domain.
> >>>>
> >>>> This is exactly what we tried to do initially with ACPI. But the searching
> >>> DOMAIN_BUS_FSL_MC_MSI
> >>>> associated to an ITS, is something that is part of drivers/acpi/arm64/iort.c.
> >>>> (similar to DOMAIN_BUS_PLATFORM_MSI and DOMAIN_BUS_PCI_MSI)
> >>>
> >>> Can you have a look at mbigen driver (drivers/irqchip/irq-mbigen.c) to see if
> >>> it helps you?
> >>>
> >>> mbigen is an irq converter to convert device's wired interrupts into MSI
> >>> (connecting to ITS), which will alloc a bunch of MSIs from ITS platform MSI
> >>> domain at the setup.
> >>
> >> Unfortunately this is not the same case as ours. As I see Hisilicon IORT table
> >> Is using single id mapping with named components.
> >>
> >> https://github.com/tianocore/edk2-
> platforms/blob/master/Silicon/Hisilicon/Hi1616/D05AcpiTables/D05Iort.asl#L30
> 0
> >>
> >> while we are not:
> >>
> >> https://source.codeaurora.org/external/qoriq/qoriq-components/edk2-
> platforms/tree/Platform/NXP/LX2160aRdbPkg/AcpiTables/Iort.aslc?h=LX2160_
> UEFI_ACPI_EAR1#n290
> >>
> >> This is because as I said, we are trying to represent a bus in IORT
> >> via named components and not individual devices connected to that bus.
> >
> > I had a thorough look into this and strictly speaking there is no
> > *mapping* requirement at all, all you need to know is what ITS the FSL
> > MC bus is mapping MSIs to. Which brings me to the next question (which
> > is orthogonal to how to model FSL MC in IORT, that has to be discussed
> > but I want to have a full picture in mind first).
> >
> > When you probe the FSL MC as a platform device, the ACPI core,
> > through IORT (if you add the 1:1 mapping as an array of single
> > mappings) already link the platform device to ITS platform
> > device MSI domain (acpi_configure_pmsi_domain()).
> >
> > The associated fwnode is the *same* (IIUC) as for the
> > DOMAIN_BUS_FSL_MC_MSI and ITS DOMAIN_BUS_NEXUS, so in practice
> > you don't need IORT code to retrieve the DOMAIN_BUS_FSL_MC_MSI
> > domain, the fwnode is the same as the one in the FSL MC platform
> > device IRQ domain->fwnode pointer and you can use it to
> > retrieve the DOMAIN_BUS_FSL_MC_MSI domain through it.
> >
> > Is my reading correct ?
> >
> > Overall, DOMAIN_BUS_FSL_MC_MSI is just an MSI layer to override the
> > provide the MSI domain ->prepare hook (ie to stash the MC device id), no
> > more (ie its_fsl_mc_msi_prepare()).
> >
> > That's it for the MSI layer - I need to figure out whether we *want* to
> > extend IORT (and/or ACPI) to defined bindings for "additional busses",
> > what I write above is a summary of my understanding, I have not made my
> > mind up yet.
>
> I'm really not sure we'd need to go near any bindings - the IORT spec
> *can* reasonably describe "giant black box of DPAA2 stuff" as a single
> named component, and that's arguably the most accurate abstraction
> already, even when it comes to the namespace device. This isn't a bus in
> any traditional sense, it's a set of accelerator components with an
> interface to dynamically configure them into custom pipelines, and the
> expected use-case seems to be for userspace to freely reconfigure
> whatever virtual network adapters it wants at any given time. Thus I
> don't see that it's logical or even practical for firmware itself to be
> involved beyond describing "here's your toolbox", and in particular,
> basing any decisions on the particular way that DPAA2 has been
> shoehorned into the Linux driver model would almost certainly be a step
> in the wrong direction.
>
> IMO the scope of this issue belongs entirely within the
> implementation(s) of Linux's own abstraction layers.
I agree. I think first we ought to get the consensus on how to represent the MC
bus in IORT table. And it should not be based on the fact that "that's how we have
handled IORT in linux". Once this is done, then we can move forward on how to
handle that in linux.
>
> Robin.
>
> > As for the IOMMU code, it seems like the only thing needed i
> > extending named components configuration to child devices,
> > hierarchically.
> >
> > As Marc already mentioned, IOMMU and IRQ code must be separate for
> > future postings but first we need to find a suitable answer to
> > the problem at hand.
> >
> > Lorenzo
> >