RE: [EXT] Re: [PATCH] bus: fsl-mc: Add ACPI support for fsl-mc | linux-arm-kernel

quoted

-----Original Message-----
From: Marc Zyngier <maz@kernel.org>
Sent: Friday, February 14, 2020 9:24 PM
To: Pankaj Bansal <redacted>
Cc: Ard Biesheuvel <redacted>; Lorenzo Pieralisi
[off-list ref]; Makarand Pawagi [off-list ref];
Calvin Johnson [off-list ref]; stuyoder@gmail.com;
nleeder@codeaurora.org; Ioana Ciornei [off-list ref]; Cristi
Sovaiala [off-list ref]; Hanjun Guo [off-list ref];
Will Deacon [off-list ref]; jon@solid-run.com; Russell King
[off-list ref]; ACPI Devel Maling List [off-list ref];
Len Brown [off-list ref]; Jason Cooper [off-list ref]; Andy
Wang [off-list ref]; Varun Sethi [off-list ref]; Thomas
Gleixner [off-list ref]; linux-arm-kernel <linux-arm-
kernel@lists.infradead.org>; Laurentiu Tudor [off-list ref]; Paul
Yang [off-list ref]; netdev@vger.kernel.org; Rafael J. Wysocki
[off-list ref]; Linux Kernel Mailing List [off-list ref];
Shameerali Kolothum Thodi [off-list ref];
Sudeep Holla [off-list ref]; Robin Murphy
[off-list ref]
Subject: Re: [EXT] Re: [PATCH] bus: fsl-mc: Add ACPI support for fsl-mc

On 2020-02-14 15:05, Pankaj Bansal wrote:
-----Original Message-----
From: Ard Biesheuvel <redacted>
Sent: Friday, January 31, 2020 5:32 PM
To: Marc Zyngier <maz@kernel.org>
Cc: Makarand Pawagi <redacted>; Calvin Johnson
[off-list ref]; stuyoder@gmail.com; nleeder@codeaurora.org;
Ioana Ciornei [off-list ref]; Cristi Sovaiala
[off-list ref]; Hanjun Guo [off-list ref]; Will
Deacon [off-list ref]; Lorenzo Pieralisi
[off-list ref]; Pankaj Bansal [off-list ref];
jon@solid-run.com; Russell King [off-list ref]; ACPI Devel
Maling List [off-list ref]; Len Brown
[off-list ref]; Jason Cooper [off-list ref]; Andy Wang
[off-list ref]; Varun Sethi [off-list ref]; Thomas Gleixner
[off-list ref]; linux-arm-kernel <linux-arm-
kernel@lists.infradead.org>; Laurentiu Tudor
[off-list ref]; Paul Yang [off-list ref];
[off-list ref] [off-list ref]; Rafael J. Wysocki
[off-list ref]; Linux Kernel Mailing List
[off-list ref]; Shameerali Kolothum Thodi
[off-list ref]; Sudeep Holla
[off-list ref]; Robin Murphy [off-list ref]
Subject: Re: [EXT] Re: [PATCH] bus: fsl-mc: Add ACPI support for
fsl-mc

On Fri, 31 Jan 2020 at 12:06, Marc Zyngier [off-list ref] wrote:
On 2020-01-31 10:35, Makarand Pawagi wrote:
-----Original Message-----
From: Lorenzo Pieralisi <redacted>
Sent: Tuesday, January 28, 2020 4:39 PM
To: Makarand Pawagi <redacted>
Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
linux-arm- kernel@lists.infradead.org;
linux-acpi@vger.kernel.org; linux@armlinux.org.uk;
jon@solid-run.com; Cristi Sovaiala [off-list ref];
Laurentiu Tudor [off-list ref]; Ioana Ciornei
[off-list ref]; Varun Sethi [off-list ref]; Calvin
Johnson [off-list ref]; Pankaj Bansal
[off-list ref]; guohanjun@huawei.com;
sudeep.holla@arm.com; rjw@rjwysocki.net; lenb@kernel.org;
stuyoder@gmail.com; tglx@linutronix.de; jason@lakedaemon.net;
maz@kernel.org; shameerali.kolothum.thodi@huawei.com;
will@kernel.org; robin.murphy@arm.com; nleeder@codeaurora.org
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 <redacted>
---
 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-syst
ems
&amp;data=02%7C01%7Cpankaj.bansal%40nxp.com%7Cdb56d889d85646277ee
30
8d7a64562fa%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6371606
892
50769265&amp;sdata=C7nCty8%2BVeuq6VhcEUXCwiAinN01rCfe12NRVnXJCIY%
3D
&amp;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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Felix
ir.bootlin.com%2Flinux%2Flatest%2Fsource%2Fdrivers%2Firqchip%2Firq-gic
-v3-its-platform-
msi.c%23L50&amp;data=02%7C01%7Cpankaj.bansal%40nxp.co
m%7Ce18eca3d0494432f73e608d7b1662bdb%7C686ea1d3bc2b4c6fa92cd99c5c
30163
5%7C0%7C1%7C637172924698903527&amp;sdata=N44g3HIK3AL3Gx6%2BlbW
%2B0QnWE
4LPqzrL9uuhRFIy5Lc%3D&amp;reserved=0
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Felix
ir.bootlin.com%2Flinux%2Flatest%2Fsource%2Fdrivers%2Facpi%2Farm64%2Fio
rt.c%23L464&amp;data=02%7C01%7Cpankaj.bansal%40nxp.com%7Ce18eca3d0
4944
32f73e608d7b1662bdb%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7
C63717
2924698903527&amp;sdata=LoWA6lxY4N%2FidPNaDs2DEqETxYGMdFJsDnr%2B
xGjGUB
0%3D&amp;reserved=0
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.
DevID is input ID for PCIe devices. BUT what would be the input ID for platform device? Are we saying that Platform devices can't specify an Input ID ?


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.
IORT is translation between Input IDs <-> Output IDs. The Input ID is still expected to be passed to parse IORT table.


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...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help