Re: [PATCH v2] cxl/mce: Avoid alias page retirement for corrected errors

Confirmed, and I dug into this thoroughly before proposing a fix -
here's the full trail, in case it's useful to anyone else hitting
this.

Starting point: the full, current comment above
amd_mce_usable_address() (arch/x86/kernel/cpu/mce/amd.c):

  /*
   * Some AMD systems have an explicit indicator that the value in MCA_ADDR is a
   * system physical address. Individual cases though, need to be detected for
   * other systems. Future cases will be added as needed.
   *
   * 1) General case
   *      a) Assume address is not usable.
   * 2) Poison errors
   *      a) Indicated by MCA_STATUS[43]: poison. Defined for all banks except legacy
   *         northbridge (bank 4).
   *      b) Refers to poison consumption in the core. Does not include "no action",
   *         "action optional", or "deferred" error severities.
   *      c) Will include a usable address so that immediate action can be taken.
   * 3) Northbridge DRAM ECC errors
   *      a) Reported in legacy bank 4 with extended error code (XEC) 8.
   *      b) MCA_STATUS[43] is *not* defined as poison in legacy bank 4. Therefore,
   *         this bit should not be checked.
   * 4) MCI_STATUS_PADDRVAL is set
   *      a) Will provide a valid system physical address.
   *
   * NOTE: SMCA UMC memory errors fall into case #1.
   */

Case 2 in that comment is what confirms your original finding. Here's
the failure, broken into three steps:

1. Poison is defined for every bank except legacy northbridge - so
   poison-consumption reported from Load/Store or Data Fabric banks
   (non-UMC) is a legitimate, address-usable error, just like poison
   from a UMC bank would be.

2. mce_is_memory_error() only accepts UMC banks. Load/Store, Data
   Fabric, and every other non-UMC bank type are rejected here,
   regardless of poison status.

3. So a CXL poison-consumption record reported from one of those
   non-UMC banks never gets the chance to reach
   mce_usable_address() - which would correctly recognize it via
   case 2 - because mce_is_memory_error() already rejected it one
   step earlier.

Net effect: a real, address-usable error is silently dropped, and
the alias page is never offlined.

To make sure I wasn't missing context, I cross-referenced the AMD64
APM Vol 2 (24593, Rev 3.45): Sec 9.3.2.4 (MCi_STATUS/Poison, p.311)
and Sec 9.3.3.2 (MCA_IPID bank-type ID, p.318) describe these as two
structurally separate registers, with poison's definition making no
reference to bank type:
https://docs.amd.com/v/u/en-US/24593_3.45_APM_Vol2

Then I traced the comment's own history, since the NOTE ("SMCA UMC
falls into case #1") initially seemed to conflict with case 1 being
the "not usable" default - i.e. how do ordinary DRAM ECC errors ever
validate at all?

- Introduced in commit 48da1ad8ba95 ("x86/mce: Define
  amd_mce_usable_address()", Jun 2023), three cases only - no case 4
  existed yet. The commit's justification for the function:

    "Currently, all valid MCA_ADDR values are assumed to be usable on
    AMD systems. However, this is not correct in most cases.
    Notifiers expecting usable addresses may then operate on
    inappropriate values."

  And its design note on the incomplete case list:

    "AMD systems do not have an explicit indicator that the value in
    MCA_ADDR is a system physical address. Therefore, individual
    cases need to be detected. Future cases and checks will be added
    as needed."

- Case 4 (PADDRV) added in commit 821f5fe4dbcb ("x86/mce: Add support
  for physical address valid bit", Nov 2025) - exactly the promised
  future case, arriving 2.5 years later. Explicitly scoped to
  "Starting with Zen6" hardware: two new bits, MCA_CONFIG[11]
  (PhysAddrValidSupported, per-bank capability) and MCA_STATUS[54]
  (PhysAddrV, per-record validity). Confirmed at the register-probing
  level too - mce_amd_feature_init() reads MCA_CONFIG and MCA_IPID
  via two separate rdmsr calls per bank at boot, populating
  independent fields with no relationship between them.

That resolves the apparent conflict, walked through concretely:

Before PADDRV existed (pre-Zen6, i.e. everything currently deployed),
amd_mce_usable_address() for a UMC bank had exactly two paths:

  if (m->status & MCI_STATUS_POISON)   // case 2
          return true;
  return false;                         // case 1, the only remaining option

A UMC bank error with POISON clear - an ordinary DRAM ECC correctable
or uncorrected-but-not-consumed error, which is the vast majority of
real DRAM ECC traffic - has nowhere else to go. It falls straight to
case 1 and returns false. So the NOTE ("SMCA UMC memory errors fall
into case #1") isn't a loose generalization - for any UMC error
without poison set, it's the *only* possible outcome, because no
other branch in the function could have caught it.

PADDRV, once it exists (Zen6+), inserts a new branch ahead of poison:

  if (this_cpu_ptr(smca_banks)[m->bank].paddrv)   // case 4
          return m->status & MCI_STATUS_PADDRV;

Now a UMC bank that supports PADDRV gets validated (or rejected)
here, before ever reaching the poison check or the case-1 fallback.
This is what actually gives UMC errors a validated-address path for
the first time - not by changing what "case #1" means, but by adding
an earlier branch that intercepts UMC records before they'd otherwise
fall through to it.

So the "conflict" was never a real contradiction in the comment - it
was a timing gap in my own understanding. The NOTE was accurate for
the hardware it was written against (2023, no PADDRV yet). PADDRV
didn't rewrite case #1's meaning; it simply gave certain banks (UMC,
on Zen6+) a way to be resolved by a different case entirely, before
they'd ever reach case #1.

This also points to a broader lesson: mce_is_memory_error() answers
"what type of bank reported this," not "can I trust this address."
Those are different questions, and only mce_usable_address() answers
the second one - through poison (all hardware) or PADDRV (Zen6+),
never through bank type. Even for the one case mce_is_memory_error()
does accept (UMC), it still doesn't tell you whether the address is
usable; that determination happens entirely inside
mce_usable_address(), independent of what mce_is_memory_error() said.

On nfit_handle_mce(): it carries the same mce_is_memory_error() +
mce_is_correctable() gate my v2 had, unchanged since commit
5d96c9342c23 ("acpi/nfit, x86/mce: Handle only uncorrectable machine
checks", 2018) - five years before amd_mce_usable_address() existed,
so it isn't independent evidence about this specific AMD gap, just
the same bug-fix pattern applied earlier for a different reason.
Might be worth a separate heads-up to the nfit maintainers.

Fix for v3: drop mce_is_memory_error() entirely, since
amd_mce_usable_address() already correctly and completely implements
the address-usability logic this handler needs:

  if (mce_is_correctable(mce))
          return NOTIFY_DONE;
  if (!mce_usable_address(mce))
          return NOTIFY_DONE;

Still setting up an AMD SMCA reproduction before posting v3