| 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: <AM9PR04MB87930B4EF3ED6BB5FE88A151887A9@AM9PR04MB8793.eurprd04.prod.outlook.com>
Date: Fri, 2 Sep 2022 01:48:03 +0000
From: Frank Li <frank.li@....com>
To: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
CC: "maz@...nel.org" <maz@...nel.org>,
"tglx@...utronix.de" <tglx@...utronix.de>,
"robh+dt@...nel.org" <robh+dt@...nel.org>,
"krzysztof.kozlowski+dt@...aro.org"
<krzysztof.kozlowski+dt@...aro.org>,
"shawnguo@...nel.org" <shawnguo@...nel.org>,
"s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
"kw@...ux.com" <kw@...ux.com>,
"bhelgaas@...gle.com" <bhelgaas@...gle.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
Peng Fan <peng.fan@....com>,
Aisheng Dong <aisheng.dong@....com>,
"jdmason@...zu.us" <jdmason@...zu.us>,
"kernel@...gutronix.de" <kernel@...gutronix.de>,
"festevam@...il.com" <festevam@...il.com>,
dl-linux-imx <linux-imx@....com>,
"kishon@...com" <kishon@...com>,
"lorenzo.pieralisi@....com" <lorenzo.pieralisi@....com>,
"ntb@...ts.linux.dev" <ntb@...ts.linux.dev>,
"lznuaa@...il.com" <lznuaa@...il.com>
Subject: RE: [EXT] Re: [PATCH v6 4/4] pcie: endpoint: pci-epf-vntb: add
endpoint MSI support
> -----Original Message-----
> From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
> Sent: Thursday, September 1, 2022 8:39 PM
> To: Frank Li <frank.li@....com>
> Cc: maz@...nel.org; tglx@...utronix.de; robh+dt@...nel.org;
> krzysztof.kozlowski+dt@...aro.org; shawnguo@...nel.org;
> s.hauer@...gutronix.de; kw@...ux.com; bhelgaas@...gle.com; linux-
> kernel@...r.kernel.org; devicetree@...r.kernel.org; linux-arm-
> kernel@...ts.infradead.org; linux-pci@...r.kernel.org; Peng Fan
> <peng.fan@....com>; Aisheng Dong <aisheng.dong@....com>;
> jdmason@...zu.us; kernel@...gutronix.de; festevam@...il.com; dl-linux-
> imx <linux-imx@....com>; kishon@...com; lorenzo.pieralisi@....com;
> ntb@...ts.linux.dev; lznuaa@...il.com
> Subject: Re: [EXT] Re: [PATCH v6 4/4] pcie: endpoint: pci-epf-vntb: add
> endpoint MSI support
>
> Caution: EXT Email
>
> On Wed, Aug 31, 2022 at 04:19:17PM +0000, Frank Li wrote:
> >
> >
> > > -----Original Message-----
> > > From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
> > > Sent: Wednesday, August 31, 2022 5:42 AM
> > > To: Frank Li <frank.li@....com>
> > > Cc: maz@...nel.org; tglx@...utronix.de; robh+dt@...nel.org;
> > > krzysztof.kozlowski+dt@...aro.org; shawnguo@...nel.org;
> > > s.hauer@...gutronix.de; kw@...ux.com; bhelgaas@...gle.com; linux-
> > > kernel@...r.kernel.org; devicetree@...r.kernel.org; linux-arm-
> > > kernel@...ts.infradead.org; linux-pci@...r.kernel.org; Peng Fan
> > > <peng.fan@....com>; Aisheng Dong <aisheng.dong@....com>;
> > > jdmason@...zu.us; kernel@...gutronix.de; festevam@...il.com; dl-
> linux-
> > > imx <linux-imx@....com>; kishon@...com; lorenzo.pieralisi@....com;
> > > ntb@...ts.linux.dev; lznuaa@...il.com
> > > Subject: [EXT] Re: [PATCH v6 4/4] pcie: endpoint: pci-epf-vntb: add
> endpoint
> > > MSI support
> > >
> > > Caution: EXT Email
> > >
> > > On Thu, Aug 18, 2022 at 10:11:27AM -0500, Frank Li wrote:
> > > > ┌───────┐ ┌──────────┐
> > > > │ │ │ │
> > > > ┌─────────────┐ │ │ │ PCI Host │
> > > > │ MSI │◄┐ │ │ │ │
> > > > │ Controller │ │ │ │ │ │
> > > > └─────────────┘ └─┼───────┼──
> ─
> > > ───────┼─BAR0 │
> > > > │ PCI │ │ BAR1 │
> > > > │ Func │ │ BAR2 │
> > > > │ │ │ BAR3 │
> > > > │ │ │ BAR4 │
> > > > │ ├─────────►│ │
> > > > └───────┘ └──────────┘
> > > >
> > >
> > > This diagram doesn't say which side is host and which one is endpoint.
> > > And not conveying any useful information.
> >
> > [Frank Li] At V2 version, this diagram is in cover letter. Bjorn suggest move
> to here.
> > I think you have good background knowledge. But it should be helpful for
> new
> > People, who just touch this area.
> >
>
> Having the block diagram always helps but my point is that this diagram
> doesn't
> convey the immediate knowledge that it is supposed to do so. Like there is no
> partition between host and endpoint and you did not add any explanation
> about
> it in the below text. So in v2, please incorporate those.
>
> > I already mark "PCI Func" and "PCI Host".
> >
>
> Sorry, that's not helpful and you need to improve it.
>
> > >
> > > > Linux supports endpoint functions. PCI Host write BAR<n> space like
> write
> > > > to memory. The EP side can't know memory changed by the host driver.
> > > >
> > >
> > > I think you just say, that there is no defined way of raising IRQs by host
> > > to the endpoint.
> > >
> > > > PCI Spec has not defined a standard method to do that. Only define
> MSI(x)
> > > > to let EP notified RC status change.
> > > >
> > >
> > > MSI is from EP, right? Throughout the driver you should call it as
> "doorbell"
> > > and not MSI.
> >
> > [Frank Li] What's I want said is that PCI standard define MSI(x) to let EP
> notify RC.
> > But there are not standard way for reverse direction. MSI should be
> correct here.
> >
>
> Right. But also use "MSI/MSI-X" instead of "MSI(x)"
>
> > >
> > > > The basic idea is to trigger an IRQ when PCI RC writes to a memory
> > > > address. That's what MSI controller provided. EP drivers just need to
> > > > request a platform MSI interrupt, struct msi_msg *msg will pass down a
> > > > memory address and data. EP driver will map such memory address to
> one
> > > of
> > > > PCI BAR<n>. Host just writes such an address to trigger EP side irq.
> > > >
> > >
> > > IIUC (by looking at other patches in the series), the memory assigned for
> BAR
> > > region by the PCI host is mapped to the platform interrupt controller in
> > > PCI Endpoint. Such that, whenever the PCI host writes to the BAR region,
> it
> > > will trigger an IRQ in the Endpoint.
> > >
> > > This kind of setup is available in other platforms like Qualcomm where
> the
> > > mapping of a register region available in BAR0 and interrupt controller is
> > > done in the hardware itself. So whenever the PCI host writes to that
> register
> > > in BAR0, an IRQ will be delivered to the endpoint.
> >
> > [Frank Li] Yes, not all platform have it. And EP driver have not provide a API
> > to get register region. I think platform msi API is pretty good API.
> > Many system have GIC ITS, so EP function driver can use it. Our test
> platform
> > have not ITS yet, so we added a simple MU-MSI driver to do it. I think
> qualcomm
> > platform can use similar method. So all EP function driver can use common
> method
> > to get notification from PCI host.
> >
>
> What is the common method here? If you want to make this doorbell feature
> common across all EPF drivers, then you need to provide EPF APIs.
[Frank Li] Existed MSI API have matched requirement. EPF just reused it.
This patch provided demo method to show how to use platform MSI API to make this
Doorbell.
>
> > >
> > > > Add MSI support for pci-epf-vntb. pci-epf-vntb driver query if system
> > > > have MSI controller. Setup doorbell address according to struct
> msi_msg.
> > > >
> > > > So PCIe host can write this doorbell address to triger EP side's irq.
> > > >
> > > > If no MSI controller exist, fall back to software polling.
> > > >
> > > > Signed-off-by: Frank Li <Frank.Li@....com>
> > > > ---
> > > > drivers/pci/endpoint/functions/pci-epf-vntb.c | 134 +++++++++++++++-
> --
> > > > 1 file changed, 112 insertions(+), 22 deletions(-)
> > > >
> > > > diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > > b/drivers/pci/endpoint/functions/pci-epf-vntb.c
>
> [...]
>
> > > > +static void epf_ntb_epc_msi_init(struct epf_ntb *ntb)
> > > > +{
> > > > + struct device *dev = &ntb->epf->dev;
> > > > + struct irq_domain *domain;
> > > > + int virq;
> > > > + int ret;
> > > > + int i;
> > > > +
> > > > + domain = dev_get_msi_domain(ntb->epf->epc->dev.parent);
> > > > + if (!domain)
> > > > + return;
> > > > +
> > > > + dev_set_msi_domain(dev, domain);
> > > > +
> > > > + if (platform_msi_domain_alloc_irqs(&ntb->epf->dev,
> > > > + ntb->db_count,
> > > > + epf_ntb_write_msi_msg)) {
> > > > + dev_info(dev, "Can't allocate MSI, fall back to poll mode\n");
> > > > + return;
> > > > + }
> > > > +
> > > > + dev_info(dev, "vntb use MSI as doorbell\n");
> > > > +
> > >
> > > Why are you using the interrupt controller as the MSI controller? Why not
> > > just
> > > a plain interrupt controller?
> >
> > [Frank Li] what's your means? I think only MSI controller support write
> memory to trigger irq.
> >
>
> From EPF driver perspective, only the IRQs need to be requested, right? So
> why
> cannot you expose MU as a generic irqchip driver, instead of a MSI controller?
[Frank Li] No. EPF need two information.
1. IRQ number.
2. A physical address, which map such physical address to PCIe Bar, so PCI host can
Write it and trigger EP side IRQ.
>
> Thanks,
> Mani
>
> --
> மணிவண்ணன் சதாசிவம்
Powered by blists - more mailing lists