> -----Original Message-----
> From: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
> Sent: Thursday, September 1, 2022 8:39 PM
> To: Frank Li <frank.li@xxxxxxx>
> Cc: maz@xxxxxxxxxx; tglx@xxxxxxxxxxxxx; robh+dt@xxxxxxxxxx;
> krzysztof.kozlowski+dt@xxxxxxxxxx; shawnguo@xxxxxxxxxx;
> s.hauer@xxxxxxxxxxxxxx; kw@xxxxxxxxx; bhelgaas@xxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; Peng Fan
> <peng.fan@xxxxxxx>; Aisheng Dong <aisheng.dong@xxxxxxx>;
> jdmason@xxxxxxxx; kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx; dl-linux-
> imx <linux-imx@xxxxxxx>; kishon@xxxxxx; lorenzo.pieralisi@xxxxxxx;
> ntb@xxxxxxxxxxxxxxx; lznuaa@xxxxxxxxx
> 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@xxxxxxxxxx>
> > > Sent: Wednesday, August 31, 2022 5:42 AM
> > > To: Frank Li <frank.li@xxxxxxx>
> > > Cc: maz@xxxxxxxxxx; tglx@xxxxxxxxxxxxx; robh+dt@xxxxxxxxxx;
> > > krzysztof.kozlowski+dt@xxxxxxxxxx; shawnguo@xxxxxxxxxx;
> > > s.hauer@xxxxxxxxxxxxxx; kw@xxxxxxxxx; bhelgaas@xxxxxxxxxx; linux-
> > > kernel@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-arm-
> > > kernel@xxxxxxxxxxxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; Peng Fan
> > > <peng.fan@xxxxxxx>; Aisheng Dong <aisheng.dong@xxxxxxx>;
> > > jdmason@xxxxxxxx; kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx; dl-
> linux-
> > > imx <linux-imx@xxxxxxx>; kishon@xxxxxx; lorenzo.pieralisi@xxxxxxx;
> > > ntb@xxxxxxxxxxxxxxx; lznuaa@xxxxxxxxx
> > > 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@xxxxxxx>
> > > > ---
> > > > 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
>
> --
> மணிவண்ணன் சதாசிவம்
