Re: [PATCH v3 1/1] platform/x86/intel/pmt: Notify userspace when crashlogs complete

[David Box <david.e.box@linux.intel.com>]

Sashiko's report [1] is right, and the underlying problem isn't one the
crashlog driver can fix on its own. Please hold off on this version.

The problem

On BMG and CRI, PMT features share one MMIO window, and a SoC remapper
index selects which region it shows. That is why PMT has callbacks: the
parent switches the window and performs the access under its lock
(xe->pmt.lock). On CRI the read_reg()/write_reg() callbacks cover the
discovery status and control areas.

For CRI, xe also holds pmt.lock across intel_vsec_register() with the
window parked on discovery, because the PMT drivers read their
discovery headers directly rather than through a callback. But a PMT
driver's probe also needs its own regions: crashlog reads its status
through read_reg(), which must take pmt.lock to switch the window. v2
deadlocked on exactly that, and v3 hit the same lock through an ABBA
with control_mutex (what Sashiko found). Remove has the same problem,
and the lock doesn't protect a PMT driver loaded after registration.

What it takes to fix

1. Document the contract in include/linux/intel_vsec.h: callbacks may
   be called from the client's probe and remove, so a parent must not
   hold a lock its callbacks take across intel_vsec_register() or
   device removal. I'll send that patch.

2. Have xe switch the window only at the time of each access, as
   xe_pmt_telem_read() already does, and stop holding pmt.lock across
   registration. The only thing that requires holding it today is the
   PMT drivers' direct discovery-header reads. Since the headers are
   static, xe can copy the discovery tables under the lock before
   registering and pass the copy in, and the PMT drivers can decode
   from it. Live status and control stay on read_reg()/write_reg().
   I'll can do the PMT side.

3. Then v4 of this patch returns to the simpler v2 design: check for a
   completed crashlog directly in probe, with no work item. The other
   v3 fixes stay.

Michael, does switching the window only at access time work for CRI,
and are there other direct reads of the window I'm missing? Ilpo, are
you OK with the documented contract and with the PMT drivers decoding
discovery headers from a copy the parent provides?

[1] https://lore.kernel.org/all/sashiko-outbox-210176@kernel.org/

David