Page MenuHomeFreeBSD

Initial DPAA2 support
ClosedPublic

Authored by dsl on Sep 20 2022, 3:28 PM.
Tags
Referenced Files
Unknown Object (File)
Thu, Dec 5, 8:45 AM
Unknown Object (File)
Tue, Dec 3, 9:36 PM
Unknown Object (File)
Mon, Dec 2, 2:27 PM
Unknown Object (File)
Fri, Nov 29, 3:42 PM
Unknown Object (File)
Fri, Nov 29, 7:12 AM
Unknown Object (File)
Thu, Nov 28, 7:51 PM
Unknown Object (File)
Wed, Nov 27, 8:30 AM
Unknown Object (File)
Mon, Nov 25, 12:01 PM

Details

Summary

DPAA2 is a hardware-level networking architecture found in some NXP SoCs which contain hardware blocks including Management Complex (MC, a command interface to manipulate DPAA2 objects), Wire Rate I/O processor (WRIOP, packets distribution, queuing, drop decisions), Queues and Buffers Manager (QBMan, Rx/Tx queues control, Rx buffer pools) and the others.

The Management Complex runs NXP-supplied firmware which provides DPAA2 objects as an abstraction layer over those blocks to simplify an access to the underlying hardware. Each DPAA2 object has its own driver (to perform an initialization at least) and will be visible as a separate device in the device tree.

Two new drivers (dpaa2_mc and dpaa2_rc) act like firmware buses in order to form a hierarchy of the DPAA2 devices:

	acpiX (or simplebusX)
	  dpaa2_mcX
	    dpaa2_rcX
	      dpaa2_mcp0
	      ...
	      dpaa2_mcpN
	      dpaa2_bpX
	      dpaa2_macX
	      dpaa2_io0
	      ...
	      dpaa2_ioM
	      dpaa2_niX

dpaa2_mc is suppossed to be a root of the hierarchy, comes in ACPI and FDT flavours and implements helper interfaces to allocate and assign bus resources, MSI and "managed" DPAA2 devices (NXP treats some of the objects as resources for the other DPAA2 objects to let them function properly). Almost all of the DPAA2 objects are assigned to the resource containers (dpaa2_rc) to implement isolation.

The initial implementation focuses on the DPAA2 network interface to be operational. It is the most complex object in terms of dependencies which uses I/O objects to transmit/receive packets.

Test Plan

Honeycomb (from SolidRun): build kernel with DPAA2 drivers from the latest CURRENT, boot it and configure "dpni" interface.

Ten64 (from Traverse): I used to network-boot it, but recently discovered that buildworld/buildkernel with several threads fails for me. Kernel panics in the different places during network stress test which I can't explain at the moment. I wouldn't recommend to use it for tests until those issues fixed. It appeared that SoC on Ten64 was overheated during my build processes and the stress test with the latest firmware (0.8.10) flashed. Ten64 was shipped (in my case, at least) with Cortex-A53 cores running at 1600 MHz, a basic/small radiator and a single fan installed. It wasn't enough to cool it properly, I think. My thermocouple showed ~64 °C on radiator under load. It's possible to down-clock Ten64 to 1200 MHz (and reduced bus frequencies) by flashing modified firmware: https://forum.traverse.com.au/t/cpu-temperature-and-fan-speed/182/4?u=dsl. It helped me to solve all of the unexpected panics during world/kernel builds and the stress test. Maximum temperature on radiator observed was ~54 °C.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

dsl requested review of this revision.Sep 20 2022, 3:28 PM
sys/arm64/conf/std.nxp
27

probably keep these sorted alphabetically

sys/conf/files.arm64
351–353

looks like these blocks are also typically supported alphabetically

sys/modules/Makefile
649

also alpha

I'm using it for a few months now on my Honeycomb and it works great, never had any issue.
Reviewing all that is a bit too big for me but you can at least add me as a Tested by: entry :)

Thanks a lot for working on this.

sys/dev/dpaa2/dpaa2_bp.h
5

Oh if you like you could use © now

Copyright © 2022 Dmitry Salychev

sys/modules/dpaa2/Makefile
2

This $FreeBSD$ isn't needed any longer in new files

In D36638#831758, @manu wrote:

I'm using it for a few months now on my Honeycomb and it works great, never had any issue.
Reviewing all that is a bit too big for me but you can at least add me as a Tested by: entry :)

Thanks a lot for working on this.

I'm glad you didn't have any issues :) Btw, have you tried to load it, i.e. stream something maybe?

In D36638#831761, @dsl wrote:
In D36638#831758, @manu wrote:

I'm using it for a few months now on my Honeycomb and it works great, never had any issue.
Reviewing all that is a bit too big for me but you can at least add me as a Tested by: entry :)

Thanks a lot for working on this.

I'm glad you didn't have any issues :) Btw, have you tried to load it, i.e. stream something maybe?

The only network workload that I have on this machine is syncthing for my packages/pkgbase set.

sys/dev/acpica/acpi.c
1975 ↗(On Diff #110742)

this should be split out to a separate ACPI change.

sys/dev/dpaa2/dpaa2_swp.c
1018

When is _STANDALONE needed?

sys/dev/dpaa2/memac_mdio.c
75 ↗(On Diff #110742)

It would pay to split this into struct memacphy_softc with the common parts, and an ACPI and FDT softc with the bus-specific parts.

598 ↗(On Diff #110742)

..?

cosmetic changes, alphabetical re-ordering and ACPI changes removed (to be reviewed separately)

dsl marked 5 inline comments as done.Sep 21 2022, 4:44 PM
dsl added inline comments.
sys/arm64/conf/std.nxp
27

I've arranged more space to add a comment as well

sys/conf/files.arm64
351–353

done

sys/dev/dpaa2/dpaa2_bp.h
5

OK, it looks neat with ©

sys/dev/dpaa2/dpaa2_swp.c
1018

I'm not sure, but this is how KASSERT is defined in sys/sys/kassert.h:

#if (defined(_KERNEL) && defined(INVARIANTS)) || defined(_STANDALONE)
#define	KASSERT(exp,msg) do {						\
	if (__predict_false(!(exp)))					\
		kassert_panic msg;					\
} while (0)
#else /* !(KERNEL && INVARIANTS) && !_STANDALONE */
#define	KASSERT(exp,msg) do { \
} while (0)
#endif /* (_KERNEL && INVARIANTS) || _STANDALONE */

I wanted to be sure that there will be no errors later (line 1050). I don't need _STANDALONE, it seems.

sys/dev/dpaa2/memac_mdio.c
75 ↗(On Diff #110742)

No problem, it's on my way.

dsl marked 3 inline comments as done.

_STANDALONE test removed

sys/conf/files.arm64
351

I believe we spell the soc_* in lower case in this file?

sys/dev/acpica/acpi.c
1975 ↗(On Diff #110742)

Yes indeed. Do I need to open a review for those bits and sort them out or will you @dsl ?

sys/dev/dpaa2/memac_mdio.c
75 ↗(On Diff #110742)

That would also require to duplicate some other functions between ACPI or FDT. Is there a functional incentive to split them or is it just preference?

598 ↗(On Diff #110742)

@andrew want to name it?

sys/modules/dpaa2/Makefile
27

I believe we don't need amd64 and i386 checks. This is an ARM only device?

dsl marked an inline comment as not done.Sep 22 2022, 7:35 AM
dsl added inline comments.
sys/dev/acpica/acpi.c
1975 ↗(On Diff #110742)

Let me open it.

sys/dev/dpaa2/memac_mdio.c
75 ↗(On Diff #110742)

Even if it won't be required from the functional point of view, I'd separate ACPI and FDT parts as it is done for dpaa2_mc. I'd look uniform.

598 ↗(On Diff #110742)

Changed already to the "Bus interface"

dsl edited the test plan for this revision. (Show Details)
sys/modules/dpaa2/Makefile
27

Yes in sys/modules/Makefile we include dpaa2 only for aarch64, so I am not sure why this condition is here at all

sys/dev/dpaa2/memac_mdio.c
75 ↗(On Diff #110742)

@andrew and @dsl

here's a change completely splitting up the memac parts into ACPI, FDT, and "common". This includes changes to the build infrastructure as well as two s/acpi/fdt/ fixes. I booted on both a Ten64 and a HoneyComb. It seems indeed cleaner, though being more files, more (duplicate) lines of code.

https://people.freebsd.org/~bz/tmp/20220928-01-dpaa-memac-split.diff

@dsl if you and/or @andrew agree that this is better, can you apply this and upload a new revision of the review?

  • memac bits are re-worked by @bz to separate FDT/ACPI parts into the different drivers (thanks!);
  • frame queues aren't blocked anymore on Rx now (not necessary due to the QBMan's processing of the Volatile Dequeue commands where frames are pulled from the only frame queue in response to the command; it fixes LOR reported by @bz as well)

Details:

Invoking IPv6 network device address event may sleep with the following non-sleepable locks held:
exclusive sleep mutex dpaa2_ni0 (dpaa2_rx_fq) r = 0 (0xffff0000fb609110) locked @ sys/dev/dpaa2/dpaa2_ni.c:2905
stack backtrace:
#0 0xffff0000004c8a80 at witness_debugger+0x5c
#1 0xffff0000004c9c7c at witness_warn+0x400
#2 0xffff000000616df4 at in6_update_ifa+0x9b4
#3 0xffff000000640b94 at in6_ifadd+0x1d0
#4 0xffff00000063d248 at nd6_ra_input+0xdf8
#5 0xffff000000612348 at icmp6_input+0xae8
#6 0xffff00000062938c at ip6_input+0xd84
#7 0xffff00000059c9c4 at netisr_dispatch_src+0xe0
#8 0xffff000000580328 at ether_demux+0x174
#9 0xffff00000058198c at ether_nh_input+0x3ec
#10 0xffff00000059c9c4 at netisr_dispatch_src+0xe0
#11 0xffff0000005807e8 at ether_input+0x80
#12 0xffff0000007cdd9c at dpaa2_ni_rx+0x194
#13 0xffff0000007cd824 at dpaa2_ni_poll+0x108
#14 0xffff0000007c39e0 at dpaa2_io_intr+0x13c
#15 0xffff000000414694 at ithread_loop+0x2a4
#16 0xffff000000410b80 at fork_exit+0x74
#17 0xffff000000772a2c at fork_trampoline+0x14
lock order reversal: (sleepable after non-sleepable)
 1st 0xffff0000fb609110 dpaa2_ni0 (dpaa2_rx_fq, sleep mutex) @ sys/dev/dpaa2/dpaa2_ni.c:2905
 2nd 0xffff000000f2b878 in6_multi_sx (in6_multi_sx, sx) @ sys/netinet6/in6_mcast.c:1193
lock order dpaa2_rx_fq -> in6_multi_sx attempted at:
#0 0xffff0000004c8634 at witness_checkorder+0xadc
#1 0xffff0000004644c8 at _sx_xlock+0x7c
#2 0xffff00000061f6e4 at in6_joingroup+0x44
#3 0xffff000000617000 at in6_update_ifa+0xbc0
#4 0xffff000000640b94 at in6_ifadd+0x1d0
#5 0xffff00000063d248 at nd6_ra_input+0xdf8
#6 0xffff000000612348 at icmp6_input+0xae8
#7 0xffff00000062938c at ip6_input+0xd84
#8 0xffff00000059c9c4 at netisr_dispatch_src+0xe0
#9 0xffff000000580328 at ether_demux+0x174
#10 0xffff00000058198c at ether_nh_input+0x3ec
#11 0xffff00000059c9c4 at netisr_dispatch_src+0xe0
#12 0xffff0000005807e8 at ether_input+0x80
#13 0xffff0000007cdd9c at dpaa2_ni_rx+0x194
#14 0xffff0000007cd824 at dpaa2_ni_poll+0x108
#15 0xffff0000007c39e0 at dpaa2_io_intr+0x13c
#16 0xffff000000414694 at ithread_loop+0x2a4
#17 0xffff000000410b80 at fork_exit+0x74
dsl marked an inline comment as done.Sep 30 2022, 6:43 AM
dsl added inline comments.
sys/modules/dpaa2/Makefile
27

done

Obtain DEVICE_PROP_HANDLE properties using new API (as @bz suggested in https://reviews.freebsd.org/D36793#837246)

SOC_NXP_LS to lower case

dsl marked an inline comment as done.Oct 6 2022, 4:07 PM
sys/arm64/conf/std.nxp
19

The above are all unrelated whitespace changes. Do you have to rebase or are these local?

27

Also unrelated whitespace change.

sys/conf/files.arm64
351–353

dpaa2 goes between dev/cpufreq/ and dev/dwc

sys/arm64/conf/std.nxp
19

These are local. I've aligned the others with the DPAA2 comment.

  • DPAA2 files were placed between dev/cpufreq and dev/dwc in sys/conf/files.arm64;
  • Copyrights were added to sys/dev/dpaa2/dpaa2_ni_dpkg.h (drivers/net/ethernet/freescale/dpaa2/dpkg.h) and sys/dev/dpaa2/dpaa2_swp.c (drivers/soc/fsl/dpio/qbman-portal.c) as their significant parts were copy-pasted from the Linux drivers.
dsl marked an inline comment as done.Oct 10 2022, 8:38 AM

Whitespace changes in sys/arm64/conf/std.nxp removed

dsl edited the summary of this revision. (Show Details)
  • dpaa2_mc.c patched to fix LINT-FDT build (discovered and fixed by @bz, thanks!);
  • copyrights of dpaa2_ni_dpkg.h and dpaa2_swc.c updated to include copyright notices from the original files together with the updated SPDX license identifiers.
sys/conf/files.arm64
202

You only need optional soc_nxp_ls dpaa2 for memac_mdio_common.c and memac_mdio_if.m. It's safe to assume either ACPI or FDT is present in the kernel config.

sys/dev/dpaa2/dpaa2_mc.c
752

When are thee NULL?

754

The normal way to handle acpi & fdt differences is to either create an interface (_if.m file) and implement as needed in each attachment or as an ivar.

sys/dev/dpaa2/dpaa2_mc.h
85

This should move to a FDT specific softc in a FDT specific file, e.g.

struct dpaa2_mc_fdt_softc {
    struct dpaa2_mc_softc base;
    phandle_t ofw_node;
};

Thanks @andrew ; I came across some of that the other day when I tried to make LINT-FDT compile but I postponed "fixing proper" to after the import. That okay with you?

sys/arm64/conf/std.nxp
19

The whitespace changes shouldn't be in the same change. Can we do them afterwards if needed?

This revision was not accepted when it landed; it landed in state Needs Review.Oct 14 2022, 8:53 PM
Closed by commit rGba7319e9091b: Add initial DPAA2 support (authored by dsl). · Explain Why
This revision was automatically updated to reflect the committed changes.