RE: [PATCH] PCI: layerscape: Add the SRIOV support in host side

[Xiaowei Bao <xiaowei.bao@nxp.com>]

> -----Original Message-----
> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> Sent: 2019年12月5日 19:11
> To: Xiaowei Bao <xiaowei.bao@nxp.com>; Robin Murphy
> <robin.murphy@arm.com>; Marc Zyngier <maz@kernel.org>
> Cc: Roy Zang <roy.zang@nxp.com>; lorenzo.pieralisi@arm.com;
> devicetree@vger.kernel.org; linux-pci@vger.kernel.org; Z.q. Hou
> <zhiqiang.hou@nxp.com>; linux-kernel@vger.kernel.org; M.h. Lian
> <minghuan.lian@nxp.com>; robh+dt@kernel.org; Mingkai Hu
> <mingkai.hu@nxp.com>; bhelgaas@google.com; andrew.murray@arm.com;
> frowand.list@gmail.com; linux-arm-kernel@lists.infradead.org; Diana
> Madalina Craciun <diana.craciun@nxp.com>
> Subject: RE: [PATCH] PCI: layerscape: Add the SRIOV support in host side
> 
> Hi Xiaowei,
> 
> > -----Original Message-----
> > From: linux-arm-kernel <linux-arm-kernel-bounces@lists.infradead.org>
> > On Behalf Of Xiaowei Bao
> >
> > > -----Original Message-----
> > > From: Robin Murphy <robin.murphy@arm.com>
> > > Sent: 2019年12月3日 23:20
> > > To: Marc Zyngier <maz@kernel.org>; Xiaowei Bao
> <xiaowei.bao@nxp.com>
> > > Cc: Roy Zang <roy.zang@nxp.com>; lorenzo.pieralisi@arm.com;
> > > devicetree@vger.kernel.org; linux-pci@vger.kernel.org; Z.q. Hou
> > > <zhiqiang.hou@nxp.com>; linux-kernel@vger.kernel.org; M.h. Lian
> > > <minghuan.lian@nxp.com>; robh+dt@kernel.org;
> > > linux-arm-kernel@lists.infradead.org; bhelgaas@google.com;
> > > andrew.murray@arm.com; frowand.list@gmail.com; Mingkai Hu
> > > <mingkai.hu@nxp.com>
> > > Subject: Re: [PATCH] PCI: layerscape: Add the SRIOV support in host
> > > side
> > >
> > > On 03/12/2019 11:51 am, Marc Zyngier wrote:
> > > > On 2019-12-03 01:42, Xiaowei Bao wrote:
> > > >>> -----Original Message-----
> > > >>> From: Marc Zyngier <maz@misterjones.org>
> > > >>> Sent: 2019年12月2日 20:48
> > > >>> To: Xiaowei Bao <xiaowei.bao@nxp.com>
> > > >>> Cc: robh+dt@kernel.org; frowand.list@gmail.com; M.h. Lian
> > > >>> <minghuan.lian@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>; Roy
> > > Zang
> > > >>> <roy.zang@nxp.com>; lorenzo.pieralisi@arm.com;
> > > >>> andrew.murray@arm.com; bhelgaas@google.com;
> > > >>> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > >>> linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > >>> Z.q. Hou <zhiqiang.hou@nxp.com>
> > > >>> Subject: Re: [PATCH] PCI: layerscape: Add the SRIOV support in
> > > >>> host side
> > > >>>
> > > >>> On 2019-12-02 10:45, Xiaowei Bao wrote:
> > > >>> > GIC get the map relations of devid and stream id from the
> > > >>> > msi-map property of DTS, our platform add this property in
> > > >>> > u-boot base on the PCIe device in the bus, but if enable the
> > > >>> > vf device in kernel, the vf device msi-map will not set, so
> > > >>> > the vf device can't work, this patch purpose is that manage
> > > >>> > the stream id and device id map relations dynamically in
> > > >>> > kernel, and make the new PCIe device work
> > in
> > > kernel.
> > > >>> >
> > > >>> > Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> > > >>> > ---
> > > >>> >  drivers/of/irq.c                            |  9
> +++
> > > >>> >  drivers/pci/controller/dwc/pci-layerscape.c | 94
> > > >>> > +++++++++++++++++++++++++++++
> > > >>> >  drivers/pci/probe.c                         |  6 ++
> > > >>> >  drivers/pci/remove.c                        |  6 ++
> > > >>> >  4 files changed, 115 insertions(+)
> > > >>> >
> > > >>> > diff --git a/drivers/of/irq.c b/drivers/of/irq.c index
> > > >>> > a296eaf..791e609 100644
> > > >>> > --- a/drivers/of/irq.c
> > > >>> > +++ b/drivers/of/irq.c
> > > >>> > @@ -576,6 +576,11 @@ void __init of_irq_init(const struct
> > > >>> >of_device_id
> > > >>> > *matches)
> > > >>> >      }
> > > >>> >  }
> > > >>> >
> > > >>> > +u32 __weak ls_pcie_streamid_fix(struct device *dev, u32 rid)
> > > >>> > +{
> > > >>> > +    return rid;
> > > >>> > +}
> > > >>> > +
> > > >>> >  static u32 __of_msi_map_rid(struct device *dev, struct
> > > >>> >device_node  **np,
> > > >>> >                  u32 rid_in)
> > > >>> >  {
> > > >>> > @@ -590,6 +595,10 @@ static u32 __of_msi_map_rid(struct device
> > > >>> >*dev,  struct device_node **np,
> > > >>> >          if (!of_map_rid(parent_dev->of_node, rid_in,
> > > >>> >"msi-map",
> > > >>> >                  "msi-map-mask", np, &rid_out))
> > > >>> >              break;
> > > >>> > +
> > > >>> > +    if (rid_out == rid_in)
> > > >>> > +        rid_out = ls_pcie_streamid_fix(parent_dev, rid_in);
> > > >>>
> > > >>> Over my dead body. Get your firmware to properly program the LUT
> > > >>> so that it presents the ITS with a reasonable topology. There is
> > > >>> absolutely no way this kind of change makes it into the kernel.
> > > >>
> > > >> Sorry for this, I know it is not reasonable, but I have no other
> > > >> way, as I know, ARM get the mapping of stream ID to request ID
> > > >> from the msi-map property of DTS, if add a new device which need
> > > >> the stream ID and try to get it from the msi-map of DTS, it will
> > > >> failed and not work, yes? So could you give me a better advice to
> > > >> fix this issue, I would really appreciate any comments or suggestions,
> thanks a lot.
> > > >
> > > > Why can't firmware expose an msi-map/msi-map-mask that has a large
> > > > enough range to ensure mapping of VFs? What are the limitations of
> > > > the LUT that would prevent this from being configured before the
> > > > kernel boots?
> >
> > Thanks for your comments, yes, this is the root cause, we only have 16
> > stream IDs for PCIe domain, this is the hardware limitation, if there
> > have enough stream IDs, we can expose an msi-map/msi-map-mask for all
> > PCIe devices in system, unfortunately, the stream IDs is not enough, I
> > think other ARM vendor have same issue that they don't have enough
> > stream IDs.
> >
> > Thanks
> > Xiaowei
> >
> > >
> > > Furthermore, note that this attempt isn't doing anything for the
> > > SMMU Stream IDs, so the moment anyone tries to assign those VFs
> > > they're still
> > going
> > > to go bang anyway. Any firmware-based fixup for ID mappings, config
> > space
> > > addresses, etc. needs to be SR-IOV-aware and account for all
> > > *possible* BDFs.
> > >
> > > On LS2085 at least, IIRC you can configure a single LUT entry to
> > > just
> > translate
> > > the Bus:Device identifier and pass some or all of the Function bits
> > straight
> > > through as the LSBs of the Stream ID, so I don't believe the
> > > relatively
> > limited
> > > number of LUT registers should be too much of an issue. For example,
> > last
> > > time I hacked on that I apparently had it set up statically like this:
> > >
> > > &pcie3 {
> > > 	/* Squash 8:5:3 BDF down to 2:2:3 */
> > > 	msi-map-mask = <0x031f>;
> > > 	msi-map = <0x000 &its 0x00 0x20>,
> > > 		  <0x100 &its 0x20 0x20>,
> > > 		  <0x200 &its 0x40 0x20>,
> > > 		  <0x300 &its 0x60 0x20>;
> > > };
> >
> > Thanks Robin, this is a effective way, but we only have total 16
> > stream IDs for PCIe domain, and only assign 4 stream IDs for each PCIe
> > controller if the board have 4 PCIe controllers, this is the root
> > cause, I submitted this patch to dynamically manage these stream IDs,
> > so that it looks like each PCIe controller has 16 stream IDs. I can
> > dynamically allocate and release these stream IDs based on the PCIe
> > devices in the current system.
> > If use your method,
> > we support up to 4 PCIe devices(2 PFs and 2 VFs), it will not achieve
> > our purpose.
> >
> 
> We allocate the Stream_IDs in a static fashion in u-boot, see [1], so if a user
> would need a larger range for PCI {s}he could adjust that in there. On most of
> our Layerscape chips the SMMU is configured to 5 bits for TBU_ID plus 10 bits
> for StreamID. Out of these 10 controllable bits we can effectively use 7 bits
> giving us a max range of 127 Stream_IDs.
> 
> [1]
> https://gitlab.denx.de/u-boot/u-boot/blob/master/arch/arm/include/asm/ar
> ch-fsl-layerscape/stream_id_lsch3.h

Thanks for your information, it is not enough even there are 127 Stream_IDs, if a
PCIe device which support SRIOV, but the VFs offset is 128, the VFs will not work.

Thanks 
Xiaowei 

> 
> ---
> Best Regards, Laurentiu