Page MenuHomeFreeBSD

aq(4): expose the PHY temperature sensor as a sysctl
Needs ReviewPublic

Authored by nick_spun.io on Mon, Jul 20, 1:39 AM.
Tags
None
Referenced Files
F163544621: D58353.diff
Fri, Jul 24, 8:05 AM
F163524211: D58353.id182267.diff
Fri, Jul 24, 3:05 AM
F163504618: D58353.diff
Thu, Jul 23, 9:53 PM
Unknown Object (File)
Wed, Jul 22, 11:48 PM
Unknown Object (File)
Wed, Jul 22, 5:43 PM
Unknown Object (File)
Wed, Jul 22, 12:04 AM
Subscribers
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

Both Atlantic generations report a PHY die temperature that the driver
never surfaced, so there was no way to tell a card that is running hot
from one that is merely dropping packets.

Add a get_temp method to the firmware ops, reporting millidegrees
Celsius, and publish it per instance as dev.aq.N.temperature in the
usual decikelvin "IK" encoding, so sysctl renders it as degrees.

Atlantic 1 v2 firmware answers through the mailbox: toggle the
temperature bit in the MPI control register, wait for the F/W to echo
it back in the state register, then read phy_temperature, which is a
signed 1/256 degree value. The capability bit is checked first and
ENOTSUP returned when the F/W lacks it. Atlantic 2 keeps a
phy_health_monitor block in the OUT window holding a whole-degree
value. Its address is not derived anywhere in the driver, so compute
it the way the shared-memory layout does and check the result against
the two offsets the driver already knows: link_status at 0x13014 and
statistics at 0x13700 fix the block at 0x13620, which the hardware
confirms by reporting a ready bit, a plausible temperature and a
ticking heartbeat there.

Atlantic 1 v1 firmware has no sensor, so its ops leave get_temp NULL
and no node appears.

The F/W zeroes the Atlantic 2 block until the PHY reports in, which is
still the case when attach runs, so return ENXIO while the ready bit
is clear rather than publishing a false zero. Registration probes
once and only skips the node on ENOTSUP, so a cold PHY still gets a
node that starts answering once the sensor is live.

This is the driver's first firmware accessor that iflib does not
serialise, and it forces a lock. Every other path into the firmware
runs from an ifdi method, and iflib holds its exclusive context sx
across all of them, so the v2 read-modify-write of the 64-bit MPI
control register at 0x368 in set_mode() and get_stats() could not
previously interleave. A sysctl handler is not an ifdi method: iflib
wraps exactly one sysctl of its own in the context lock and leaves
driver-registered oids entirely unserialised, so a temperature read
can land in the middle of the statistics toggle and either write can
discard the other's bit. e1000 hit this same edge and had to take the
iflib context lock explicitly in its NVM sysctl to stop a KASSERT.

Add a per-instance mutex to struct aq_hw and take it across the
read-modify-write in set_mode() and get_stats(), across the paired
register reads in get_mode(), and across the toggle, echo wait and
mailbox read in get_temp(). Locking in the firmware ops rather than
in the sysctl handler follows bnxt and ice, which serialise at the
firmware transport and leave their sysctls bare. A regular MTX_DEF
mutex is sufficient: every firmware path runs in a sleepable context,
and the only interrupt handler, aq_linkstat_isr(), just acknowledges
the interrupt and defers to the admin task. None of the firmware ops
call one another, so the mutex cannot recurse. Destroy it on the
attach_pre failure path as well, since iflib skips ifdi_detach when
ifdi_attach_pre fails.

The v1 and Atlantic 2 ops do not take it. v1 writes its control
register whole rather than modifying it, and while Atlantic 2 does
read-modify-write its IN window, it only does so from ifdi paths;
its sysctl-reachable read touches the OUT window, which the firmware
brackets with a transaction counter. Both carry a comment saying so,
because the asymmetry otherwise reads as an omission.

Tested on a host with an AQC107 and an AQC113 direct-attached at
10Gbase-T. The AQC107 reads 66.6C and the AQC113 52.1C at idle,
rising to 67.9C and 53.1C after thirty seconds of iperf3 moving
20.7 GB at 5.92 Gbit/s, and the AQC113 climbs from 32C to 52C as the
PHY warms up after link establishment. 2400 concurrent reads issued
against both cards while that traffic ran produced no torn values and
no WITNESS reports on a kernel with WITNESS and INVARIANTS.

Signed-off-by: Nick Price <nick@spun.io>

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 74943
Build 71826: arc lint + arc unit

Event Timeline

Given the rather hot nature of these chips perhaps add some kind of temperature warning or possibly drop link speed (if it helps reducing temperature) once it reaches a threshold rather than it completely wedges itself?

Given the rather hot nature of these chips perhaps add some kind of temperature warning or possibly drop link speed (if it helps reducing temperature) once it reaches a threshold rather than it completely wedges itself?

Exploring now but it does look like we can at least configure the PHY to deactivate if it gets too hot so that would be pretty lightweight, will see what other options we have as well.

Beyond that I feel like we need some idea of a sensor/health framework so we can use a more standard approach instead of each driver exposing and managing this sort of stuff inconsistently and behaving inconsistently 🤔

Looks like by default aq1 and aq2 PHYs will shut down at 108C and set a bit until they recover to a specified threshold

The thermal shutdown latches the values though so changing the shutdown/recovery values during a shutdown will not bring the card back even if the card is within those thresholds - it has to cool off to the values that were present at the time the shutdown initiated

It looks like there are some warning thresholds that can be set and bits asserting "in warning mode" as well however that's not as well-documented for aq1 and I'm still exploring there

Thanks for looking into this!
I'm asking because I've seen countless of people reporting these controllers (especially on motherboards) as being notoriously unstable/flakey likely due to heat and the cause might not be obvious for the end user given that you normally have no issues at least in terms of heat with integrated NICs. I've also seen some reports on forums of the controller completely die as in drop off PCIe bus and never come back again but I'm not sure if that's only due to overheating. I agree that it probably should go into some kind of sensor/health framework in the end but it would be nice to some kind of warning for diagnostic purposes. While we can discuss thresholds later I'm not sure if I would consider it "safe" for it to go all up to 108C before shutting down/throttling, perhaps warn around 70C and kick in around 80C.

Thanks for looking into this!
I'm asking because I've seen countless of people reporting these controllers (especially on motherboards) as being notoriously unstable/flakey likely due to heat and the cause might not be obvious for the end user given that you normally have no issues at least in terms of heat with integrated NICs. I've also seen some reports on forums of the controller completely die as in drop off PCIe bus and never come back again but I'm not sure if that's only due to overheating. I agree that it probably should go into some kind of sensor/health framework in the end but it would be nice to some kind of warning for diagnostic purposes. While we can discuss thresholds later I'm not sure if I would consider it "safe" for it to go all up to 108C before shutting down/throttling, perhaps warn around 70C and kick in around 80C.

Might bump those up a bit - my AQC107 with good ventilation is idling at 70C (AQC113C is around 55C)

Interestingly enough, the hotter a1 cards also seem to ship with the thermal shutdown disabled by default - curious if it's because they're so warm to begin with lol

Thanks for looking into this!
I'm asking because I've seen countless of people reporting these controllers (especially on motherboards) as being notoriously unstable/flakey likely due to heat and the cause might not be obvious for the end user given that you normally have no issues at least in terms of heat with integrated NICs. I've also seen some reports on forums of the controller completely die as in drop off PCIe bus and never come back again but I'm not sure if that's only due to overheating. I agree that it probably should go into some kind of sensor/health framework in the end but it would be nice to some kind of warning for diagnostic purposes. While we can discuss thresholds later I'm not sure if I would consider it "safe" for it to go all up to 108C before shutting down/throttling, perhaps warn around 70C and kick in around 80C.

Inbound diff coming to enable a thermal shutdown and recovery path for all of these - it's actually disabled on Atlantic 1 chips by default and I think it's just because nobody ever actually defined a recovery path and all of the obvious ones wedge the MAC in a way that requires a full power-down cycle to recover from - it's always fun to get to a point where we can say we have more full-featured support than Linux :D

Thanks for suggesting this, it led me down a really interesting path that surfaced an iflib use-after-free as well (because I was wedging my card so much lol)

Sounds great, thanks for all the work you've put into this!