[PATCH v3 0/3] hw/misc/applesmc: fix GET_KEY_BY_INDEX iteration and populate Apple SMC key set

Matthew Jackson posted 3 patches 6 days, 12 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260601152847.26916-1-matthew@pq.io
Maintainers: Matthew Jackson <matthew@pq.io>
MAINTAINERS        |   5 +
hw/misc/applesmc.c | 348 +++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 345 insertions(+), 8 deletions(-)
[PATCH v3 0/3] hw/misc/applesmc: fix GET_KEY_BY_INDEX iteration and populate Apple SMC key set
Posted by Matthew Jackson 6 days, 12 hours ago
v1: https://lore.kernel.org/qemu-devel/20260507040153.14565-1-matthew@pq.io/
v2: https://lore.kernel.org/qemu-devel/20260507152030.48753-1-matthew@pq.io/

This is a rebase/respin of v2, which had no outstanding review
comments after Peter's two style nits were addressed. Resending on
current master with checkpatch cleared, plus a new patch 3/3 that
gives the file a MAINTAINERS entry.

Changes in v3:

  * New patch 3/3: add a MAINTAINERS entry for hw/misc/applesmc.c.
    The file currently has no entry, so get_maintainer.pl resolves
    only the open list and these patches had no designated reviewer.
    This is easy to drop if a maintainer would rather route it
    elsewhere; patches 1-2 do not depend on it.

  * Rebased on current master. hw/misc/applesmc.c is byte-identical
    upstream to the v2 base (blob fd96f5f), so the rebase is a clean
    no-op with no functional change to patches 1-2 from v2.

  * checkpatch cleanups in patches 1-2, no functional change:
      - patch 1/2: reformat the GET_KEY_TYPE 4th-byte block comment to
        the leading-/* and trailing-*/-on-their-own-line house style.
      - patch 2/2: shorten the F0Ac fan-key comment to stay under the
        80-column limit.
    Both were pre-existing checkpatch warnings; neither was raised in
    the v1/v2 review. The series is now checkpatch-clean.

Changes in v2 (carried into v3, all in patch 2/2, #KEY block):

  * Add braces around the QLIST_FOREACH() count loop body
    (qemu coding style: loops always need braces, even single-line).
    Reported-by: Peter Maydell <peter.maydell@linaro.org>

  * Replace the manual 4-byte big-endian byte-shift packing of
    `count` into `numkey_buf` with a single stl_be_p() call.
    Reported-by: Peter Maydell <peter.maydell@linaro.org>

Patch 1/2 is otherwise unchanged since v1.

On Peter's v2 remark that applesmc_add_key()'s "pointer must remain
valid forever" contract is awkward: agreed it's not ideal, but the
fix (e.g. converting the key table to a glib hashtable that copies
values) is orthogonal to this protocol bug fix and isn't reported as
noticeable in profiles. Happy to send that conversion as a separate
follow-up series if the maintainers think it's worth doing.

Original v1 cover letter follows below.

---

The QEMU applesmc device implements just enough of the Apple SMC PMIO
protocol to satisfy the OSK boot check on older macOS versions. On
modern macOS guests (x86 10.14+, all of the 15.x series) the real
AppleSMC kext enumerates the SMC key space at boot via
APPLESMC_GET_KEY_BY_INDEX_CMD (0x12). The current device only
acknowledges APPLESMC_READ_CMD (0x10) at the command port; every
other command falls through to the default arm of the switch and
sets ST_1E_BAD_CMD.

The macOS driver interprets the resulting 0x82 reply as "spurious
data" and enters a retry loop that floods the kernel log with
kSMCSpuriousData (0x81) / kSMCKeyNotFound errors at roughly 1800
events per second, pegging kernel_task at ~70% CPU and WindowServer
at ~509% CPU. This reproduces reliably on any recent macOS 15 guest
booted with -device isa-applesmc,osk=<valid-OSK>.

This series fixes the protocol-level bug and rounds out the SMC key
table to a complete iMac20,1 profile.

  Patch 1: protocol-level fix
    - Accept WRITE_CMD, GET_KEY_BY_INDEX_CMD, GET_KEY_TYPE_CMD at the
      command port (in addition to READ_CMD).
    - Implement the indexed-iteration walker (returns real key names
      from s->data_def, or APPLESMC_ST_1E_BAD_INDEX 0xb8 once the
      index is past the end so the guest stops iterating).
    - Implement GET_KEY_TYPE returning a 6-byte type/size/attr
      response matching VirtualSMC's kern_pmio.cpp behaviour.
    - Accept and log WRITE_CMD silently.
    - Replace the unknown-key NOEXIST (0x84) reply with a zeroed
      payload of the requested length, logged at LOG_UNIMP.
    - Route the BAD_CMD path through qemu_log_mask(LOG_GUEST_ERROR).
    - Fix MSSD initialiser typo ("\0x3" -> "\x03"). The original
      literal was three bytes ('\0', 'x', '3') truncated to one
      ('\0') by the size argument, so MSSD has been silently
      returning 0 since the device was introduced; the corrected
      value matches what a real iMac20,1 SMC reports.

  Patch 2: populate the key table
    - Add 94 keys covering the categories macOS queries on a Sequoia
      15.7.5 guest: 28 temperature sensors (sp78), 4 fan keys (fpe2),
      12 power-rail keys, 6 DIMM keys, 11 SMC-internal bookkeeping,
      13 motion-sensor / wireless, 3 write targets (HE0N/MSDW/NTOK),
      2 power-management gates (HE2N/WDTC), 8 platform-identity /
      probe keys, plus the Apple-canonical #KEY total-count.
    - Sensor values match a real iMac20,1 idle probe published at
      https://linux-hardware.org/?probe=999fc708a4&log=sensors:
      CPU 40-51 C, GPU 36-42 C, fan at 1200 RPM (= F0Mn idle), etc.

  Patch 3: MAINTAINERS entry for the device (see "Changes in v3").

Measured impact (macOS 15.7.5 guest, iMac20,1 profile):

   Metric           | Before   | After
   -----------------|---------:|------:
   SMC errors / 5s  |   9,225  |     2
   kernel_task CPU  |    70 %  |  ~2 %
   WindowServer CPU |   509 %  |  ~6 %

A note for review on the zero-valued keys in patch 2: the 26 keys
covering DIMM / SMC bookkeeping / motion-sensor / wireless rails are
registered with present-with-zero values rather than omitted. macOS
distinguishes "absent" (NOEXIST reply, retry-poll) from "broken"
(present, value 0, accepted-and-ignored). Registering these keys
present-with-zero stops the retry-poll behaviour without asserting
any specific value. If the maintainer prefers a tighter scope for
this series I am happy to drop any subset and follow up; the
present-with-zero approach was driven by which keys macOS observed
querying during boot.

Backwards compatibility: legacy macOS guests (10.11-10.13) which do
not iterate the key space via GET_KEY_BY_INDEX boot unchanged. The
original six keys (REV/OSK0/OSK1/NATJ/MSSP/MSSD) are still present
and respond with the same values, modulo the MSSD typo fix in patch
1 which corrects MSSD to the value a real iMac20,1 SMC reports.

Tested against current master. Builds clean on gcc-13 / clang-17
with --enable-werror. Sequoia 15.7.5 guest boots to login screen
with the SMC retry storm absent from the kernel log.

Matthew Jackson (3):
  hw/misc/applesmc: fix GET_KEY_BY_INDEX to return real keys, accept
    WRITE/TYPE commands
  hw/misc/applesmc: populate Apple SMC key table
  MAINTAINERS: adopt hw/misc/applesmc.c

 MAINTAINERS        |   5 +
 hw/misc/applesmc.c | 348 +++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 345 insertions(+), 8 deletions(-)

--
2.50.1 (Apple Git-155)