Re: [PATCH v15 04/15] irqchip/gic-v3-its: Add support for device tree msi-map and msi-mask

[Frank Li <Frank.li@nxp.com>]

On Sat, Mar 01, 2025 at 11:37:14AM +0000, Marc Zyngier wrote:
> On Tue, 11 Feb 2025 19:21:57 +0000,
> Frank Li <Frank.Li@nxp.com> wrote:
> >
> > Some platform devices create child devices dynamically and require the
> > parent device's msi-map to map device IDs to actual sideband information.
> >
> > A typical use case is using ITS as a PCIe Endpoint Controller(EPC)'s
> > doorbell function, where PCI hosts send TLP memory writes to the EP
> > controller. The EP controller converts these writes to AXI transactions
> > and appends platform-specific sideband information.  See below figure.
> >
> >                ┌────────────────────────────────┐
> >                │                                │
> >                │     PCI Endpoint Controller    │
> >                │                                │
> >                │  ┌─────┐   ┌─────┐     ┌─────┐ │
> >     PCI Bus    │  │     │   │     │     │     │ │
> >     ─────────► │  │Func1│   │Func2│ ... │Func │ │
> >     TLP Memory │  │     │   │     │     │<n>  │ │
> >     Write Push │  │     │   │     │     │     │ │
> >     DoorBell   │  └─┬─┬─┘   └──┬──┘     └──┬──┘ │
> >                │    │ │        │           │    │
> >                └────┼─┼────────┼───────────┼────┘
> >         sideband    │ │ Address│           │
> >         information ▼ ▼ /Data  ▼           ▼
> >                    ┌────────────────────────┐
> >                    │    MSI Controller      │
> >                    └────────────────────────┘
> >
>
> Please stop using these figures in commit messages. I don't think they
> help, and they are not in consistent with the way the commit messages
> are managed.

Okay

>
> > EPC's DTS will provide such information by msi-map and msi-mask. A
> > simplified dts as
> >
> > pcie-ep@10000000 {
> > 	...
> > 	msi-map = <0 &its 0xc 8>;
> >                           ^^^ 0xc is implement defined sideband information,
> > 			      which append to AXI write transaction.
> > 	           ^ 0 is function index.
>
> What does this sideband field represent?

ARM ITS use term "streamid" for this sideband field, which indicate which
MSI consumer write to address/data pair on bus. Such as PCI1 or PCI2.

> How is the ITS driver
> supposed to use that data?

ITS use a as devid, or info->scratchpad[0].ul = dev_id;
msi-map actually given a start dev_id (it is 0xc in example) for fuction0.
function1 will use dev_id + 1 ...

> Is it the full devid as presented to the
> ITS?

Yes,

> Something combined with the "function index"? Is the "function
> index" a full RID, as defined in the documentation?

Not a full RID. RID is related with host PCIe's topology. The EP function's
RID may 1:00:01 at PC1, 3:00:01 at the another PC.

So Endpoint driver can't use RID directly. It should related function and
virtual function number only.

PCI define 8 physicla funciton, and 64000 virutal function. Define device
id as vfunc[31:3], pfunc[2:0] as msi-map's input. DTS provide information
how map it to real streamid.

>
> Also, msi-map is so far reserved to a PCIe RC, not this sort of wonky
> contraption. This needs to be documented.

Okay, I can update document.

Frank
>
> >
> > 	msi-mask = <0x7>
> > }
> >
> > Check msi-map if msi-parent missed to keep compatility with existed system.
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > change from v14 to v15
> > - none
> >
> > change from v13 to v14
> > new patch
> > ---
> >  drivers/irqchip/irq-gic-v3-its-msi-parent.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/irqchip/irq-gic-v3-its-msi-parent.c b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> > index e150365fbe892..6c7389bb84a4a 100644
> > --- a/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> > +++ b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> > @@ -118,6 +118,14 @@ static int of_pmsi_get_dev_id(struct irq_domain *domain, struct device *dev,
> >  		index++;
> >  	} while (!ret);
> >
> > +	if (ret) {
> > +		struct device_node *np = NULL;
> > +
> > +		ret = of_map_id(dev->of_node, dev->id, "msi-map", "msi-map-mask", &np, dev_id);
> > +		if (np)
> > +			of_node_put(np);
> > +	}
> > +
> >  	return ret;
> >  }
> >
> >
>
> Thanks,
>
> 	M.
>
> --
> Without deviation from the norm, progress is not possible.