if_bnxt: add RX timestamping support

[sumit.saxena_broadcom.com]

Summary

Add PHC register mapping, RTC initialization, refclk read with
FW-reset serialization, and sbinuptime-based calibration to convert
the raw hardware timestamp carried in RX completions into an mbuf
timestamp. Advertise IFCAP_HWRXTSTMP, configure the RX timestamp
filter in bnxt_init(), and drain the calibration callout on stop.
Check HWRM_FUNC_QCAPS_OUTPUT_FLAGS_PTP_SUPPORTED in
bnxt_hwrm_func_qcaps() so the capability bit PTP is gated on actually
gets set, without which PTP never armed on real hardware.

[gallatin]

• In addition to the inline comments, please make the code conform to style(9)
• There do not seem to be licenses on the new files

Don't we need to re-calibrate after down/up? Eg, bnxt_stop() drains the callout and clears rx_ts_enabled, but never resets clbr_done or the clbr_points[]. On the next bnxt_init(), bnxt_reset_calibration_callout() sees clbr_done != 0 and takes the callout_reset_sbt_curcpu() branch instead of calibrating so there's a full second during which packets are interpolated against very old ref. points.

sys/dev/bnxt/bnxt_en/bnxt.h

1487

Can you clean this up? There are multiple definitions of this (and readl) in bnxt_re/main/c and bnxt_en/if_bnxt.c
They should all probably just be inlines defined here.

sys/dev/bnxt/bnxt_en/bnxt_ptp.c

38

Does this need a htole64?

52

Are you sure you want getnanotime() and not just nanotime? Note that getnanotime uses a cached less precise, but faster to obtain, time. While nanotime always queries the timecounter for a very accurate time.

86

Why a spin mutex? That's only needed in a hw interrupt context that cannot sleep. Network drivers typically do not run in that context on freebsd and can use a normal mutx.

93

Seems to be a bit of a misnomer, since it doesn't deal with mbufs.

That would be done at the iflib level. Can you please also include me on the review that adds rx timestamp support to iflib? Unless I'm missing something, I don't think iflib supports this now

121

Do we need to worry about wrapping here?

128

If i've got my units correct, we can overflow after 4.3s, which could be triggered by a few missed callouts. Maybe bound hw_clocks against hw_clk_div

129

Is there any way to differentiate an uncalibrated (which is what i think hw_clk_div == 0 means) clock from a valid timestamp of 0?

310

Don't you need to initialize this? bnxt_hwrm_cmd_hdr_init only fills in req_type, cmpl_ring, target_id, resp_addr. Every other field is stack garbage. I'm worried you could misconfigure the mac by accident.

sys/dev/bnxt/bnxt_en/if_bnxt.c

2904

This is kind of weird. If we fail, we're going to return rc, and the attach will fail. But since rc is not checked, all the INIT* stuff below runs. All other failure cases jump to failed. Please either do that, or re-set rc to 0 if failure here is not fatal.

[sumit.saxena_broadcom.com]

sys/dev/bnxt/bnxt_en/bnxt.h

1487

Sure, will clean up.

sys/dev/bnxt/bnxt_en/bnxt_ptp.c

38

Yes, it needs htole64(), will fix up this.

52

I think we can use nanotime().

86

bnxt_calibration_callout (which calls bnxt_refclk_read) can get scheduled from the LAPIC timer interrupt while the CPU is in the idle thread.
That's hard-interrupt/idle-thread context and sleep should not be allowed. I tried replacing the spin-lock with mutex and ran into the panic:

panic: mtx_lock() by idle thread 0xff01000235a96bd0 on mutex 0xffff00fff01000235a96bd0 @ bnxt_ptp.c:119
cpuid = 56
time = 1785859601
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x36/frame 0xfffffe006152a830
vpanic() at vpanic+0x136/frame 0xfffffe006152a960
panic() at panic+0x43/frame 0xfffffe006152a9c0
mtx_lock_flags() at mtx_lock_flags+0x143/frame 0xfffffe006152aa10
bnxt_refclk_read() at bnxt_refclk_read+0x32/frame 0xfffffe006152aa40
bnxt_calibration_callout() at bnxt_calibration_callout+0x56/frame 0xfffffe006152aaa0
softclock_call_cc() at softclock_call_cc+0x1be/frame 0xfffffe006152ab60
callout_process() at callout_process+0x25b/frame 0xfffffe006152ac20
handleevents() at handleevents+0x18a/frame 0xfffffe006152ac80
timercb() at timercb+0x19e/frame 0xfffffe006152ac90
lapic_handle_timer() at lapic_handle_timer+0x8f/frame 0xfffffe006152aca0
Xtimerint() at Xtimerint+0xb1/frame 0xfffffe006152acb0

• interrupt, rip = 0xfffffff804f27c9, rsp = 0xfffffe006152ad70, rbp = 0xfffffe006152adb0 ---

acpi_cpu_idle() at acpi_cpu_idle+0x299/frame 0xfffffe006152adb0
cpu_idle_acpi() at cpu_idle_acpi+0x50/frame 0xfffffe006152add0
cpu_idle() at cpu_idle+0xa6/frame 0xfffffe006152adf0
sched_ule_idletd() at sched_ule_idletd+0x524/frame 0xfffffe006152aef0
fork_exit() at fork_exit+0x82/frame 0xfffffe006152af30
fork_trampoline() at fork_trampoline+0xe/frame 0xfffffe006152af30

• trap 0, rip = 0, rsp = 0, rbp = 0 ---

KDB: enter: panic
[ thread pid 11 tid 100059 ]

Stopped at kdb_enter+0x33: movq $0,0x117e392(%rip)

93

Ok, will fix up this.

128

Ack

310

Ack, will fix up this.

sys/dev/bnxt/bnxt_en/if_bnxt.c

2904

Ack, will fix up this.

[sumit.saxena_broadcom.com]

sys/dev/bnxt/bnxt_en/bnxt_ptp.c

93

Here is the iflib changes to support rx timestamp: https://reviews.freebsd.org/D58638
I've added you to review.

[sumit.saxena_broadcom.com]

Addressed code review feedback.

[gallatin]

sys/dev/bnxt/bnxt_en/bnxt_ptp.c

206

There is only one caller for this. Maybe just call bnxt_ptp_get_current_time() directly

239

This is redundant.. you are checking that above to get into this block

317

is this re-registering the driver? is that intended?

342

FreeBSD error returns should not be negative

sys/dev/bnxt/bnxt_en/bnxt_ptp.h

37–42

Aren't these already in bnxt.h?

79

i think this is unused