Firmware mitigation options for BMR491 voltage droop defect

[cbiffle]

Flex has proposed a firmware mitigation for existing BMR491 IBCs that have the intermittent voltage droop issue. I'm creating this tracking issue so we can document its implementation in the firmware (or our decision not to implement it, should we decide it's not right!).

The suggestion is, concretely,

• To disable the input undervoltage check by writing VIN_OFF to 0. (All register names in this list are in the PMBus interface.)
• Send VOUT_COMMAND of 0x0060 (sets the ranges for the subsequent commands)
• Override MAX_DUTY to 0.95 / 0xF8EA
• Set VOUT_UV_FAULT_LIMIT to 0x0058
• Set VOUT_UV_FAULT_RESPONSE to 0x80 (disable, do not retry)

They then suggest using STORE_USER_ALL to make this persist.

My interpretation of their doc against the PMBus spec suggests that the fix is plausible. It should effectively disable the input undervoltage monitor, which makes some sense -- it's the input stage for that circuit that has the out-of-tolerance components on the BMR491. To compensate it enables VOUT undervoltage monitoring, which should be less prone to false detection.

We don't appear to do much programming of the BMR491 online right now, so I think we'd need to do this in manufacturing on any defective BMR491s. For systems already in the field, we'd probably want to distribute a firmware update that applies the mitigation at startup. This raises the question of whether we can reliably detect a defective (or potentially defective) '491 in-system, which we need to double check.

@hawkw @ericaasen - did I miss things?

[cbiffle]

I was staring at the "disable + do not retry" part of this recommendation, and I think it's right for our context?

If we applied this mitigation, a droop in the 12V output would trigger an undervoltage shutdown of the BMR491, which Ignition should see as a fault. Cycling at the upstream hotswap controller should still work, so it should be recoverable without manual intervention, unless there's an erratum in this area I've missed.

Thanks to @rmustacc for reminding me of some circuit details here.

[cbiffle]

I have been assured that we ought to be able to detect a potentially-defective unit through PMBus by checking the revision code. So that's lovely.

In that case, the sequencer task is probably the right place to apply the mitigation on the compute sleds. We should be able to verify if it's been applied already or not, since the affected registers are read/write.

[hawkw]

I agree with everything Cliff has written here, with a couple additional notes (which we also discussed on Matrix).

I think that making the sequencer task responsible for applying the mitigation seems like the right thing, provided that the PMBus revision code in the parts we have actually does include the bit we care about (have we checked this?). However, I think the code that actually checks the rev and performs the operation ought to go in a library crate (either drv-i2c-devices or its own thing that depends on that?) since we'll want to have the Cosmo, Gimlet, and Sidecar sequencer tasks do it.

Although we haven't observed any instances of this failure on Sidecar, this is probably just due to a combination of:

• None of Sidecar's Hubris tasks are actually watching for it, unlike Gimlet,
• There aren't any known Sidecar pathologies that we have determined have this as their root cause (there may be sidecar problems that are due to this, but we haven't determined that AFAIK)
• There are just a lot fewer Sidecars in the world than Gimlets

[cbiffle]

Discussed this with @rmustacc -- I think there's an argument for not persisting the mitigation the way Flex described.

The voltage droop issue only affects the 12V rail in practice -- while we have other rails derived from the 12V rail, the droop (at around 8V) is not low enough to affect them. We only begin relying on the 12V rail when we transition out of A2.

This means it should be pretty safe to apply the mitigation any time before we start the A2->A0 transition, and -- because a restart of the SP and kin implies an A0->A2 shutdown -- there is no "race condition" opportunity for the bad settings to cause a problem before we can apply the mitigation.

All of the pieces of the mitigation should be pretty safe to apply with the IBC operational, except the STORE_USER_ALL operation. The PMBus standard has this to say about STORE_USER_ALL:

The STORE_USER_ALL or STORE_USER_CODE commands may generally be issued
when the PMBus device is operating and supplying power to the output. However, this
may result in unpredictable and even catastrophic results. It is recommended that the
output be disabled before issuing a STORE_USER_ALL or STORE_USER_CODE
command.

Flex has not specified the behavior they expect during STORE_USER_ALL, which does not really tell me anything one way or the other.

So, we were toying with the idea of applying the mitigation on every power sequencing start. This has a couple of advantages:

1. It removes a "forked SKU" situation where we'd have a subset of servers with corrected BMR491 flash contents, and a subset without, until the mitigations had been applied globally.
2. If we decide we want to change the mitigation, that's just a firmware update -- any future firmware will be responsible for loading the BMR491 with appropriate parameters, including different or even dynamic values.

That second point is currently compelling, because Flex hasn't been terribly forthcoming with root cause information about the failure, specifics on how they reproduced it, or why they believe the reproduction case is analogous to our production environment. So, I think it's reasonable to assume that this may not be the last word in mitigations.

---

Interestingly, while analyzing the mitigation to write this report, I noticed that it's incorrect. Flex provides fixed bit patterns to write to the VOUT_COMMAND and VOUT_UV_FAULT_LIMIT registers.

Both of these registers use bitwise representations controlled by the VOUT_MODE register. From reverse engineering the undocumented values they provided, I can infer that they're assuming that VOUT_MODE is set for linear representation with an exponent of 3 (meaning, each LSB is 1/8 of a V). But that register is writable, so that assumption is rather dangerous if the mitigation is applied in a running system that might have altered VOUT_MODE.

(Currently, we don't alter VOUT_MODE, but I would like our code to remain correct if we alter it in the future!)