All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Jackson <matthew@pq.io>
To: qemu-devel@nongnu.org
Cc: philmd@mailo.com, peter.maydell@linaro.org, pbonzini@redhat.com,
	stefanha@redhat.com, thuth@redhat.com
Subject: [PATCH v3 0/3] hw/misc/applesmc: fix GET_KEY_BY_INDEX iteration and populate Apple SMC key set
Date: Mon,  1 Jun 2026 08:28:44 -0700	[thread overview]
Message-ID: <20260601152847.26916-1-matthew@pq.io> (raw)
In-Reply-To: <20260507152030.48753-1-matthew@pq.io>

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)


  reply	other threads:[~2026-06-01 15:32 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-07  4:01 [PATCH 0/2] hw/misc/applesmc: fix GET_KEY_BY_INDEX iteration and populate Apple SMC key set Matthew Jackson
2026-05-07  4:01 ` [PATCH 1/2] hw/misc/applesmc: fix GET_KEY_BY_INDEX to return real keys, accept WRITE/TYPE commands Matthew Jackson
2026-05-07  4:01 ` [PATCH 2/2] hw/misc/applesmc: populate Apple SMC key table Matthew Jackson
2026-05-07 14:27   ` Peter Maydell
2026-05-07 15:20 ` [PATCH v2 0/2] hw/misc/applesmc: fix GET_KEY_BY_INDEX iteration and populate Apple SMC key set Matthew Jackson
2026-06-01 15:28   ` Matthew Jackson [this message]
2026-06-01 15:28     ` [PATCH v3 1/3] hw/misc/applesmc: fix GET_KEY_BY_INDEX to return real keys, accept WRITE/TYPE commands Matthew Jackson
2026-06-01 15:28     ` [PATCH v3 2/3] hw/misc/applesmc: populate Apple SMC key table Matthew Jackson
2026-07-20  8:02       ` Philippe Mathieu-Daudé
2026-06-01 15:28     ` [PATCH v3 3/3] MAINTAINERS: adopt hw/misc/applesmc.c Matthew Jackson
2026-05-07 15:20 ` [PATCH v2 1/2] hw/misc/applesmc: fix GET_KEY_BY_INDEX to return real keys, accept WRITE/TYPE commands Matthew Jackson
2026-05-07 15:20 ` [PATCH v2 2/2] hw/misc/applesmc: populate Apple SMC key table Matthew Jackson
2026-07-19 15:23 ` [PATCH v3 0/2] hw/misc/applesmc: fix GET_KEY_BY_INDEX iteration and populate Apple SMC key set Matthew Jackson
2026-07-20 16:49   ` [PATCH v4 " Matthew Jackson
2026-07-20 16:49     ` [PATCH v4 1/2] hw/misc/applesmc: implement GET_KEY_BY_INDEX, WRITE and TYPE commands Matthew Jackson
2026-08-19 17:35       ` Daniel Golle
2026-07-20 16:49     ` [PATCH v4 2/2] hw/misc/applesmc: populate Apple SMC key table Matthew Jackson
2026-08-19 17:36       ` Daniel Golle
2026-08-19 17:36     ` [PATCH 0/2] hw/misc/applesmc: SMC shutdown watchdog and trace events Daniel Golle
2026-08-19 17:36       ` [PATCH 1/2] hw/misc/applesmc: Add " Daniel Golle
2026-08-19 17:36       ` [PATCH 2/2] hw/misc/applesmc: Emulate the SMC shutdown watchdog Daniel Golle
2026-07-19 15:23 ` [PATCH v3 1/2] hw/misc/applesmc: fix GET_KEY_BY_INDEX to return real keys, accept WRITE/TYPE commands Matthew Jackson
2026-07-19 15:23 ` [PATCH v3 2/2] hw/misc/applesmc: populate Apple SMC key table Matthew Jackson
2026-07-20  8:04   ` Philippe Mathieu-Daudé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260601152847.26916-1-matthew@pq.io \
    --to=matthew@pq.io \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=philmd@mailo.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.