[U-Boot] [PATCH] board: ls1028a: set up integrated PCI stream IDs and cache attributes
Bin Meng
bmeng.cn at gmail.com
Fri Aug 9 13:27:52 UTC 2019
Hi Alex,
On Fri, Aug 9, 2019 at 9:15 PM Alex Marginean <alexm.osslist at gmail.com> wrote:
>
> Hi Bin,
>
> On 8/9/2019 12:58 PM, Bin Meng wrote:
> > Hi Alex,
> >
> > On Fri, Aug 9, 2019 at 3:59 PM Alex Marginean
> > <alexandru.marginean at nxp.com> wrote:
> >>
> >> Configure stream IDs for integrated PCI devices. There are hardware
> >> defaults but unfortunately they are outside the acceptable range for
> >> SMMU, so we need to tune them down. Use values based on Linux device tree
> >> iommu-map or, if missing, start from HW base value shifted down by 4.
> >>
> >> Signed-off-by: Alex Marginean <alexm.osslist at gmail.com>
> >> ---
> >> board/freescale/ls1028a/ls1028a.c | 64 +++++++++++++++++++++++++++++++
> >> 1 file changed, 64 insertions(+)
> >>
> >> diff --git a/board/freescale/ls1028a/ls1028a.c b/board/freescale/ls1028a/ls1028a.c
> >> index 49a9292c31..05eac6f9c4 100644
> >> --- a/board/freescale/ls1028a/ls1028a.c
> >> +++ b/board/freescale/ls1028a/ls1028a.c
> >> @@ -118,6 +118,67 @@ void detail_board_ddr_info(void)
> >> }
> >>
> >> #ifdef CONFIG_OF_BOARD_SETUP
> >> +
> >> +/*
> >> + * Hardware default stream IDs are 0x4000 + PCI function #, but that's outside
> >> + * the acceptable range for SMMU. Use Linux DT values instead or at least
> >> + * smaller defaults.
> >> + */
> >> +#define ECAM_NUM_PFS 7
> >> +#define ECAM_IERB_BASE 0x1F0800000
> >> +#define ECAM_PFAMQ(pf, vf) ((ECAM_IERB_BASE + 0x800 + (pf) * \
> >> + 0x1000 + (vf) * 4))
> >> +/* cache related transaction attributes for PCIe functions */
> >> +#define ECAM_IERB_MSICAR (ECAM_IERB_BASE + 0xa400)
> >> +#define ECAM_IERB_MSICAR_VALUE 0x30
> >> +
> >> +/* number of VFs per PF, VFs have their own AMQ settings */
> >> +static const u8 enetc_vfs[ECAM_NUM_PFS] = { 2, 2 };
> >> +
> >> +void setup_ecam_amq(void *blob)
> >
> > nits: this should be static
>
> I'll send a v2.
>
> >
> >> +{
> >> + int streamid, sid_base, off;
> >> + int pf, vf, vfnn = 1;
> >> + u32 iommu_map[4];
> >> + int err;
> >> +
> >> + /*
> >> + * Look up the stream ID settings in the DT, if found apply the values
> >> + * to HW, otherwise use HW values shifted down by 4.
> >> + */
> >> + off = fdt_node_offset_by_compatible(blob, 0, "pci-host-ecam-generic");
> >> + if (off < 0) {
> >> + debug("ECAM node not found\n");
> >> + return;
> >> + }
> >> +
> >> + err = fdtdec_get_int_array(blob, off, "iommu-map", iommu_map, 4);
> >> + if (err) {
> >> + sid_base = in_le32(ECAM_PFAMQ(0, 0)) >> 4;
> >> + debug("\"iommu-map\" not found, using default SID base %04x\n",
> >> + sid_base);
> >> + } else {
> >> + sid_base = iommu_map[2];
> >> + }
> >> + /* set up AMQs for all integrated PCI functions */
> >> + for (pf = 0; pf < ECAM_NUM_PFS; pf++) {
> >> + streamid = sid_base + pf;
> >> + out_le32(ECAM_PFAMQ(pf, 0), streamid);
> >> +
> >> + /* set up AMQs for VFs, if any */
> >> + for (vf = 0; vf < enetc_vfs[pf]; vf++, vfnn++) {
> >> + streamid = sid_base + ECAM_NUM_PFS + vfnn;
> >> + out_le32(ECAM_PFAMQ(pf, vf + 1), streamid);
> >> + }
> >> + }
> >> +}
> >> +
> >> +void setup_ecam_cacheattr(void)
> >
> > ditto
> >
> >> +{
> >> + /* set MSI cache attributes */
> >> + out_le32(ECAM_IERB_MSICAR, ECAM_IERB_MSICAR_VALUE);
> >> +}
> >> +
> >> int ft_board_setup(void *blob, bd_t *bd)
> >> {
> >> u64 base[CONFIG_NR_DRAM_BANKS];
> >> @@ -143,6 +204,9 @@ int ft_board_setup(void *blob, bd_t *bd)
> >>
> >> fdt_fixup_memory_banks(blob, base, size, 2);
> >>
> >> + setup_ecam_amq(blob);
> >> + setup_ecam_cacheattr();
> >> +
> >> return 0;
> >> }
> >> #endif
> >
> > Not only programming the registers, but also I think we will need fix
> > up the <msi-map> property used by the "pci-host-ecam-generic", just
> > like what it was done in fdt_pcie_set_msi_map_entry() in
> > pcie_layerscape_fixup.c
>
> The DT loaded with the kernel has the two maps (iommu and msi) in sync,
> they are both present and they start from same base ID. For integrated
OK, I checked arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi, and
found pcie at 1f0000000 (ecam) node has <msi-map> and <iommu-map>
properties, both of which have the same ID 0x17.
> PCI we use that as a reference in U-Boot rather than assign the value
> from U-Boot. Layerscape PCI node as far as I remember doesn't have the
> msi/iommu-map properties, they are added by U-Boot using IDs it
> allocates, but we don't do that here. The intent is to try to keep
In the same kernel fsl-ls1028a.dtsi, I did not find any layescape PCI
nodes. I suspect they are not upstreamed yet?
So if the we are doing layersacpe PCI <msi-map> fix in U-Boot, but for
ECAM we are doing the other way around, I don't think it is
consistent. I believe we should either have DT provide <msi-map> and
<iommu-map> for layersacpe PCI nodes and use that as a reference in
U-Boot, or we fix up all in U-Boot.
> fixups to a minimum, in this case the code is needed only because the HW
> defaults are not OK.
Speaking of keeping fixups to a minimum, should we move
setup_ecam_amq() and setup_ecam_cacheattr() to somewhere else? They
are inside ft_board_setup() which is supposed to do board-level DT fix
up but here we are actually not doing any DT fixup.
> For what is worth I tested ping in Linux with this U-Boot change and it
> works.
>
> I suppose one potential issue is to make sure U-Boot won't
> allocate in the same range as the values used in DT for ECAM. The
> values we use now in Linux DT are OK in the sense that they are over the
> range used by U-Boot (FSL_PEX_STREAM_ID_START-FSL_PEX_STREAM_ID_END).
> There is no explicit check for overlap between layerscape PCI stream IDs
> and ECAM stream IDs, I am guessing such a configuration issue would be
> noticeable in Linux though.
>
Regards,
Bin
More information about the U-Boot
mailing list