| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <Z3wG6pMbjsldqU/n@lizhi-Precision-Tower-5810>
Date: Mon, 6 Jan 2025 11:38:02 -0500
From: Frank Li <Frank.li@....com>
To: Marc Zyngier <maz@...nel.org>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
Krzysztof Wilczyński <kw@...ux.com>,
Kishon Vijay Abraham I <kishon@...nel.org>,
Bjorn Helgaas <bhelgaas@...gle.com>, Arnd Bergmann <arnd@...db.de>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Thomas Gleixner <tglx@...utronix.de>,
Anup Patel <apatel@...tanamicro.com>, linux-kernel@...r.kernel.org,
linux-pci@...r.kernel.org, imx@...ts.linux.dev,
Niklas Cassel <cassel@...nel.org>, dlemoal@...nel.org,
jdmason@...zu.us, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v13 4/9] irqchip/gic-v3-its: Add
DOMAIN_BUS_DEVICE_PCI_EP_MSI support
On Thu, Dec 19, 2024 at 12:02:02PM -0500, Frank Li wrote:
> On Thu, Dec 19, 2024 at 10:52:30AM +0000, Marc Zyngier wrote:
> > On Wed, 18 Dec 2024 23:08:39 +0000,
> > Frank Li <Frank.Li@....com> wrote:
> > >
> > > ┌────────────────────────────────┐
> > > │ │
> > > │ PCI Endpoint Controller │
> > > │ │
> > > │ ┌─────┐ ┌─────┐ ┌─────┐ │
> > > PCI Bus │ │ │ │ │ │ │ │
> > > ─────────► │ │Func1│ │Func2│ ... │Func │ │
> > > Doorbell │ │ │ │ │ │<n> │ │
> > > │ │ │ │ │ │ │ │
> > > │ └──┬──┘ └──┬──┘ └──┬──┘ │
> > > │ │ │ │ │
> > > └──────┼────────┼───────────┼────┘
> > > │ │ │
> > > ▼ ▼ ▼
> > > ┌────────────────────────┐
> > > │ MSI Controller │
> > > └────────────────────────┘
> > >
> > > Add domain DOMAIN_BUS_DEVICE_PCI_EP_MSI to allocate MSI domain for Endpoint
> > > function in PCI Endpoint (EP) controller, So PCI Root Complex (RC) can
> > > write MSI message to MSI controller to trigger doorbell IRQ for difference
> > > EP functions.
> > >
> > > Signed-off-by: Frank Li <Frank.Li@....com>
> > > ---
> > > change from v12 to v13
> > > - new patch
> >
> > This might be v13, but after all this time, I have no idea what you
> > are trying to do. You keep pasting this non-ASCII drawing in commit
> > messages, but I still have no idea what this PCI Bus Doorbell
> > represents.
>
> PCI Bus/Doorbell is two words. Basic over picture is a PCI EP devices (such
> as imx95), which run linux and PCI Endpoint framework. i.MX95 connect to
> PCI Host, such as PC (x86).
>
> i.MX95 can use standard PCI MSI framework to issue a irq to X86. but there
> are not reverse direction. X86 try write some MMIO register ( mapped PCI
> bar0). But i.MX95 don't know it have been modified. So currently solution
> is create a polling thread to check every 10ms.
>
> So this patches try resolve this problem at the platform, which have MSI
> controller such as ITS.
>
> after this patches, i.MX95 can create a PCI Bar1, which map to MSI
> controller register space, when X86 write data to Bar1 (call as doorbell),
> a irq will be triggered at i.MX95.
>
> Doorbell in diagram means 'push doorbell' (write data to bar<n>).
>
> >
> > I appreciate the knowledge shortage is on my end, but it would
> > definitely help if someone would take the time to explain what this is
> > all about.
>
> I am not sure if diagram in corver letter can help this, or above
> descriptions is enough.
>
>
> ┌────────────┐ ┌───────────────────────────────────┐ ┌────────────────┐
> │ │ │ │ │ │
> │ │ │ PCI Endpoint │ │ PCI Host │
> │ │ │ │ │ │
> │ │◄──┤ 1.platform_msi_domain_alloc_irqs()│ │ │
> │ │ │ │ │ │
> │ MSI ├──►│ 2.write_msi_msg() ├──►├─BAR<n> │
> │ Controller │ │ update doorbell register address│ │ │
> │ │ │ for BAR │ │ │
> │ │ │ │ │ 3. Write BAR<n>│
> │ │◄──┼───────────────────────────────────┼───┤ │
> │ │ │ │ │ │
> │ ├──►│ 4.Irq Handle │ │ │
> │ │ │ │ │ │
> │ │ │ │ │ │
> └────────────┘ └───────────────────────────────────┘ └────────────────┘
> (* some detail have been changed and don't affect understand overall
> picture)
>
> >
> > From what I gather, the ITS is actually on an end-point, and get
> > writes from the host, but that doesn't answer much.
>
> Yes, baisc it is correct. PCI RC -> PCIe Bus TLP -> PCI Endpoint Ctrl ->
> AXI transaction -> ITS MMIO map register -> CPU IRQ.
>
> The major problem how to distingiush from difference PCI Endpoint function
> driver. There are have many EP functions as much as 8, which quite similar
> standard PCI, one PCIe device can have 8 physical functions.
>
> >
> > > ---
> > > drivers/irqchip/irq-gic-v3-its-msi-parent.c | 19 ++++++++++++++++++-
> > > 1 file changed, 18 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/irqchip/irq-gic-v3-its-msi-parent.c b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> > > index b2a4b67545b82..16e7d53f0b133 100644
> > > --- a/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> > > +++ b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> > > @@ -5,6 +5,7 @@
> > > // Copyright (C) 2022 Intel
> > >
> > > #include <linux/acpi_iort.h>
> > > +#include <linux/pci-ep-msi.h>
> > > #include <linux/pci.h>
> > >
> > > #include "irq-gic-common.h"
> > > @@ -173,6 +174,19 @@ static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
> > > return its_pmsi_prepare_devid(domain, dev, nvec, info, dev_id);
> > > }
> > >
> > > +static int its_pci_ep_msi_prepare(struct irq_domain *domain, struct device *dev,
> > > + int nvec, msi_alloc_info_t *info)
> > > +{
> > > + u32 dev_id;
> > > + int ret;
> > > +
> > > + ret = pci_epf_msi_domain_get_msi_rid(dev, &dev_id);
> >
> > What this doesn't express is *how* are the writes conveyed to the ITS.
> > Specifically, the DevID is normally sampled as sideband information at
> > during the write transaction.
>
> Like PCI host, there msi-map in dts file, which descript how map PCI RID
> to DevID, such as
> msi-map = <0 $its 0x80 8>;
>
> This informtion should be descripted in DTS or ACPI ...
>
> >
> > Obviously, you can't do that over PCI. So there is a lot of
> > undisclosed assumption about how the ITS is integrated, and how it
> > samples the DevID.
>
> Yes, it should be platform PCI endpoint ctrl driver jobs. Platform EP
> driver should implement this type of covert. Such as i.MX95, there are
> hardware call LUT in PCI ctrl, which can convert PCI' request ID to DevID
> here.
>
> On going patch may help understand these
> https://lore.kernel.org/linux-pci/20241210-imx95_lut-v8-0-2e730b2e5fde@nxp.com/
>
> If use latest ITS MSI64 should be simple, only need descript it at DTS
> (I have not hardware to test this case yet).
> pci-ep {
> ...
> msi-map = <0 &its, 0x<8_0000, 0xff>;
> ^, ctrl ID.
> msi-mask = <0xff>;
> ...
> }
>
> >
> > My conclusion is that this is not as generic as it seems to be. It is
> > definitely tied to implementation-specific behaviours, none of which
> > are explained.
>
> Compared to standard PCI MSI, which also have implementation-specific
> behaviours, which convert PCI request ID to DevID Or stream ID.
> https://lore.kernel.org/linux-pci/20241210-imx95_lut-v8-0-2e730b2e5fde@nxp.com/
> (I have struggle this for almost one year for this implementation-specific
> part)
>
> Well defined and mature PCI standard, MSI still need two parts, common part
> and "implementation-specific" part.
>
> Common part of standard PCI is at several place, such its driver/msi
> libary/ kernel msi code ...
>
> "implementation-specific" part is in PCI host bridge driver, such as
> drivers/pci/controller/dwc/pcie-qcom.c
>
> This solution already test by Tested-by: Niklas Cassel <cassel@...nel.org>
> who use another dwc controller, which they already implemented
> "implementation-specific" by only update dts to provide hardware
> information.(I guest he use ITS's MSI64)
>
> Because it is new patches, I have not added Niklas's test-by tag. There
> are not big functional change since Nikas test. The major change is make
> msi part better align current MSI framework according to Thomas's
> suggestion.
Thomas Gleixner and Marc Zyngier:
Happy new year! Do you have additioinal comments for this?
Frank
>
> Frank
> >
> > Thanks,
> >
> > M.
> >
> > --
> > Without deviation from the norm, progress is not possible.
Powered by blists - more mailing lists