[PATCH 0/4] hw/misc/applesmc: Implement writes, the shutdown watchdog and get-key-type

[PATCH 0/4] hw/misc/applesmc: Implement writes, the shutdown watchdog and get-key-type

[Daniel Golle]

QEMU's AppleSMC device only implements the SMC read command, so every
other command a guest issues fails with kSMCBadCommand. Tracing a macOS
guest shows it relies on two more: the write command (0x11) and
get-key-type (0x13). The missing write command delays boot (macOS retries
failed boot-time key writes) and produces a stream of
SMCWDT::setWatchdogTimer errors at shutdown; the missing get-key-type is
rejected dozens of times during boot.

This series implements what the guest actually uses, one command per
patch for reviewability:

  1/4  add trace events, so the SMC conversation can be observed with
       -trace 'applesmc*' (used to find exactly which commands and keys
       macOS issues).
  2/4  implement the write command (0x11): accept writes and return
       success, removing the boot delay and the shutdown errors.
  3/4  model the "NATi"/"NATJ"/"OSWD" keys as the SMC shutdown watchdog
       they represent, rather than silently accepting the writes: a write
       arms, refreshes or disarms a QEMUTimer whose expiry forces the
       machine down.
  4/4  implement get-key-type (0x13): give each key an SMC type and answer
       the type queries instead of rejecting them.

Traced against a macOS guest before and after: the boot-time key writes
(NTOK, MSDW, QENA, HE0N) now succeed, the shutdown "OSWD" write (a
watchdog disarm) is handled, and the get-key-type rejections become
correct replies (for example REV is reported as {rev, six bytes). The
traced guest only disarms the watchdog; the arm-and-fire path in patch 3
is the documented behaviour for a guest that arms it and was not observed
being triggered. get-key-by-index (0x12) is deliberately not implemented,
as macOS never issues it.

Daniel Golle (4):
  hw/misc/applesmc: Add trace events
  hw/misc/applesmc: Implement the write command
  hw/misc/applesmc: Emulate the SMC shutdown watchdog
  hw/misc/applesmc: Implement the get-key-type command

 hw/misc/applesmc.c   | 174 +++++++++++++++++++++++++++++++++++++++++--
 hw/misc/trace-events |  16 ++++
 2 files changed, 183 insertions(+), 7 deletions(-)


-- 
2.55.0

[PATCH 1/4] hw/misc/applesmc: Add trace events

[Daniel Golle]

The device only offered the compile-time DEBUG_SMC fprintf() macro for
introspection. Add trace events for the command and data port
transactions: the incoming command byte and whether it was accepted or
rejected, each four-character key as it is assembled, key lookups that
miss, and every data byte handed back. The SMC conversation a guest
performs can then be observed at runtime, for example with
-trace 'applesmc*', which is helpful for understanding which keys and
commands a macOS guest actually issues.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 hw/misc/applesmc.c   | 13 +++++++++++++
 hw/misc/trace-events |  9 +++++++++
 2 files changed, 22 insertions(+)

diff --git a/hw/misc/applesmc.c b/hw/misc/applesmc.c
index d004e8b443..6f9d0a590d 100644
--- a/hw/misc/applesmc.c
+++ b/hw/misc/applesmc.c
@@ -39,6 +39,7 @@
 #include "qemu/timer.h"
 #include "qom/object.h"
 #include "hw/acpi/acpi_aml_interface.h"
+#include "trace.h"
 
 /* #define DEBUG_SMC */
 
@@ -123,6 +124,7 @@ static void applesmc_io_cmd_write(void *opaque, hwaddr addr, uint64_t val,
     AppleSMCState *s = opaque;
     uint8_t status = s->status & 0x0f;
 
+    trace_applesmc_cmd_write((uint8_t)val, s->status);
     smc_debug("CMD received: 0x%02x\n", (uint8_t)val);
     switch (val) {
     case APPLESMC_READ_CMD:
@@ -130,6 +132,7 @@ static void applesmc_io_cmd_write(void *opaque, hwaddr addr, uint64_t val,
         if (status == APPLESMC_ST_CMD_DONE || status == APPLESMC_ST_NEW_CMD) {
             s->cmd = val;
             s->status = APPLESMC_ST_NEW_CMD | APPLESMC_ST_ACK;
+            trace_applesmc_cmd_accepted((uint8_t)val);
         } else {
             smc_debug("ERROR: previous command interrupted!\n");
             s->status = APPLESMC_ST_NEW_CMD;
@@ -140,6 +143,7 @@ static void applesmc_io_cmd_write(void *opaque, hwaddr addr, uint64_t val,
         smc_debug("UNEXPECTED CMD 0x%02x\n", (uint8_t)val);
         s->status = APPLESMC_ST_NEW_CMD;
         s->status_1e = APPLESMC_ST_1E_BAD_CMD;
+        trace_applesmc_cmd_rejected((uint8_t)val, s->status_1e);
     }
     s->read_pos = 0;
     s->data_pos = 0;
@@ -164,6 +168,7 @@ static void applesmc_io_data_write(void *opaque, hwaddr addr, uint64_t val,
     const struct AppleSMCData *d;
 
     smc_debug("DATA received: 0x%02x\n", (uint8_t)val);
+    trace_applesmc_data_write(s->cmd, s->read_pos, (uint8_t)val);
     switch (s->cmd) {
     case APPLESMC_READ_CMD:
         if ((s->status & 0x0f) == APPLESMC_ST_CMD_DONE) {
@@ -172,6 +177,10 @@ static void applesmc_io_data_write(void *opaque, hwaddr addr, uint64_t val,
         if (s->read_pos < 4) {
             s->key[s->read_pos] = val;
             s->status = APPLESMC_ST_ACK;
+            if (s->read_pos == 3) {
+                trace_applesmc_key_selected(s->key[0], s->key[1],
+                                            s->key[2], s->key[3]);
+            }
         } else if (s->read_pos == 4) {
             d = applesmc_find_key(s);
             if (d != NULL) {
@@ -183,6 +192,8 @@ static void applesmc_io_data_write(void *opaque, hwaddr addr, uint64_t val,
             } else {
                 smc_debug("READ_CMD: key '%c%c%c%c' not found!\n",
                           s->key[0], s->key[1], s->key[2], s->key[3]);
+                trace_applesmc_key_not_found(s->key[0], s->key[1],
+                                             s->key[2], s->key[3]);
                 s->status = APPLESMC_ST_CMD_DONE;
                 s->status_1e = APPLESMC_ST_1E_NOEXIST;
             }
@@ -213,6 +224,8 @@ static uint64_t applesmc_io_data_read(void *opaque, hwaddr addr, unsigned size)
         }
         if (s->data_pos < s->data_len) {
             s->last_ret = s->data[s->data_pos];
+            trace_applesmc_data_read(s->key[0], s->key[1], s->key[2],
+                                     s->key[3], s->data_pos, s->last_ret);
             smc_debug("READ '%c%c%c%c'[%d] = %02x\n",
                       s->key[0], s->key[1], s->key[2], s->key[3],
                       s->data_pos, s->last_ret);
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index c9a868b3ef..b924b4398e 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -37,6 +37,15 @@ allwinner_sid_write(uint64_t offset, uint64_t data, unsigned size) "offset 0x%"
 allwinner_sramc_read(uint64_t offset, uint64_t data) "offset 0x%" PRIx64 " data 0x%" PRIx64
 allwinner_sramc_write(uint64_t offset, uint64_t data) "offset 0x%" PRIx64 " data 0x%" PRIx64
 
+# applesmc.c
+applesmc_cmd_write(uint8_t cmd, uint8_t status) "cmd 0x%02x (status 0x%02x)"
+applesmc_cmd_accepted(uint8_t cmd) "cmd 0x%02x accepted"
+applesmc_cmd_rejected(uint8_t cmd, uint8_t status_1e) "cmd 0x%02x rejected (status_1e 0x%02x)"
+applesmc_data_write(uint8_t cmd, uint8_t pos, uint8_t val) "cmd 0x%02x pos %u data 0x%02x"
+applesmc_key_selected(uint8_t c0, uint8_t c1, uint8_t c2, uint8_t c3) "key %c%c%c%c"
+applesmc_key_not_found(uint8_t c0, uint8_t c1, uint8_t c2, uint8_t c3) "key %c%c%c%c not found"
+applesmc_data_read(uint8_t c0, uint8_t c1, uint8_t c2, uint8_t c3, uint8_t pos, uint8_t val) "read key %c%c%c%c [%u] 0x%02x"
+
 # avr_power.c
 avr_power_read(uint8_t value) "power_reduc read value:%u"
 avr_power_write(uint8_t value) "power_reduc write value:%u"
-- 
2.55.0

[PATCH 2/4] hw/misc/applesmc: Implement the write command

[Daniel Golle]

The device implemented only the SMC read command (0x10). The write
command (0x11) fell through to the default case and returned
kSMCBadCommand, so every SMC key write a guest performs failed.

A macOS guest writes keys through this command in normal operation: at
boot it writes "NTOK", "MSDW", "QENA" and "HE0N", and retrying those
failed writes delays boot; at shutdown it writes "OSWD". All of them
showed up as a stream of kSMCBadCommand errors.

Accept the write command, collect the key, the declared length and the
payload, and return success. The payload is not interpreted yet, so the
writes are accepted and discarded, which is enough to remove the boot
delay and the shutdown errors; the following patch gives the watchdog
keys meaning.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 hw/misc/applesmc.c   | 39 +++++++++++++++++++++++++++++++++++++++
 hw/misc/trace-events |  3 +++
 2 files changed, 42 insertions(+)

diff --git a/hw/misc/applesmc.c b/hw/misc/applesmc.c
index 6f9d0a590d..bc44b0f1d8 100644
--- a/hw/misc/applesmc.c
+++ b/hw/misc/applesmc.c
@@ -128,6 +128,7 @@ static void applesmc_io_cmd_write(void *opaque, hwaddr addr, uint64_t val,
     smc_debug("CMD received: 0x%02x\n", (uint8_t)val);
     switch (val) {
     case APPLESMC_READ_CMD:
+    case APPLESMC_WRITE_CMD:
         /* did last command run through OK? */
         if (status == APPLESMC_ST_CMD_DONE || status == APPLESMC_ST_NEW_CMD) {
             s->cmd = val;
@@ -200,6 +201,44 @@ static void applesmc_io_data_write(void *opaque, hwaddr addr, uint64_t val,
         }
         s->read_pos++;
         break;
+    case APPLESMC_WRITE_CMD:
+        if (s->read_pos < 4) {
+            s->key[s->read_pos] = val;
+            s->status = APPLESMC_ST_ACK;
+            if (s->read_pos == 3) {
+                trace_applesmc_key_selected(s->key[0], s->key[1],
+                                            s->key[2], s->key[3]);
+            }
+            s->read_pos++;
+        } else if (s->read_pos == 4) {
+            s->data_len = val;
+            s->data_pos = 0;
+            s->read_pos++;
+            s->status = APPLESMC_ST_ACK;
+            trace_applesmc_write_len(s->key[0], s->key[1], s->key[2],
+                                     s->key[3], s->data_len);
+            if (s->data_len == 0) {
+                s->status = APPLESMC_ST_CMD_DONE;
+                s->status_1e = APPLESMC_ST_CMD_DONE;
+            }
+        } else {
+            if (s->data_pos < s->data_len) {
+                s->data[s->data_pos] = val;
+                trace_applesmc_write_data(s->key[0], s->key[1], s->key[2],
+                                          s->key[3], s->data_pos, (uint8_t)val);
+                s->data_pos++;
+            }
+            if (s->data_pos >= s->data_len) {
+                trace_applesmc_write_complete(s->key[0], s->key[1],
+                                              s->key[2], s->key[3],
+                                              s->data_len);
+                s->status = APPLESMC_ST_CMD_DONE;
+                s->status_1e = APPLESMC_ST_CMD_DONE;
+            } else {
+                s->status = APPLESMC_ST_ACK;
+            }
+        }
+        break;
     default:
         s->status = APPLESMC_ST_CMD_DONE;
         s->status_1e = APPLESMC_ST_1E_STILL_BAD_CMD;
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index b924b4398e..fc01cc9b7d 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -45,6 +45,9 @@ applesmc_data_write(uint8_t cmd, uint8_t pos, uint8_t val) "cmd 0x%02x pos %u da
 applesmc_key_selected(uint8_t c0, uint8_t c1, uint8_t c2, uint8_t c3) "key %c%c%c%c"
 applesmc_key_not_found(uint8_t c0, uint8_t c1, uint8_t c2, uint8_t c3) "key %c%c%c%c not found"
 applesmc_data_read(uint8_t c0, uint8_t c1, uint8_t c2, uint8_t c3, uint8_t pos, uint8_t val) "read key %c%c%c%c [%u] 0x%02x"
+applesmc_write_len(uint8_t c0, uint8_t c1, uint8_t c2, uint8_t c3, uint8_t len) "write key %c%c%c%c len %u"
+applesmc_write_data(uint8_t c0, uint8_t c1, uint8_t c2, uint8_t c3, uint8_t pos, uint8_t val) "write key %c%c%c%c [%u] 0x%02x"
+applesmc_write_complete(uint8_t c0, uint8_t c1, uint8_t c2, uint8_t c3, uint8_t len) "write key %c%c%c%c complete (%u bytes)"
 
 # avr_power.c
 avr_power_read(uint8_t value) "power_reduc read value:%u"
-- 
2.55.0

[PATCH 3/4] hw/misc/applesmc: Emulate the SMC shutdown watchdog

[Daniel Golle]

"NATi", "NATJ" and "OSWD" are the SMC shutdown watchdog keys. On real
Intel Macs the OS arms this watchdog so that, if the machine wedges, the
SMC forces it off or reset. "NATi" and "OSWD" hold a countdown in seconds
and "NATJ" a job code (1 shut down, 2 restart); a non-zero countdown arms
or refreshes the watchdog and zero disarms it.

The previous patch already lets writes to these keys succeed, but treats
them like any other write. Model them as the watchdog they are instead of
silently accepting them: back them with a QEMUTimer that a write arms,
refreshes or disarms, and whose expiry requests a guest shutdown or reset
through the main loop with no guest cooperation, matching the hardware
backstop. Add "NATi" and "OSWD" so the guest can read them back.

A traced macOS guest writes "OSWD" with a zero countdown to disarm the
watchdog during shutdown, which this handles; the arm-and-expiry path is
the documented behaviour for a guest that arms it.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 hw/misc/applesmc.c   | 74 ++++++++++++++++++++++++++++++++++++++++++++
 hw/misc/trace-events |  3 ++
 2 files changed, 77 insertions(+)

diff --git a/hw/misc/applesmc.c b/hw/misc/applesmc.c
index bc44b0f1d8..8d5bf74373 100644
--- a/hw/misc/applesmc.c
+++ b/hw/misc/applesmc.c
@@ -39,6 +39,7 @@
 #include "qemu/timer.h"
 #include "qom/object.h"
 #include "hw/acpi/acpi_aml_interface.h"
+#include "system/runstate.h"
 #include "trace.h"
 
 /* #define DEBUG_SMC */
@@ -80,6 +81,17 @@ enum {
     APPLESMC_ST_1E_BAD_INDEX         = 0xb8,
 };
 
+/*
+ * Job codes written to the "NATJ" key, and implied by "OSWD": the action the
+ * SMC watchdog takes when its countdown (seeded from "NATi"/"OSWD") elapses
+ * because the guest failed to power down in time.
+ */
+enum {
+    APPLESMC_WDT_DISABLE             = 0,
+    APPLESMC_WDT_SHUTDOWN            = 1,
+    APPLESMC_WDT_RESTART             = 2,
+};
+
 #ifdef DEBUG_SMC
 #define smc_debug(...) fprintf(stderr, "AppleSMC: " __VA_ARGS__)
 #else
@@ -116,6 +128,10 @@ struct AppleSMCState {
     uint8_t data[255];
     char *osk;
     QLIST_HEAD(, AppleSMCData) data_def;
+
+    QEMUTimer *wdt_timer;   /* shutdown watchdog, armed via NATi/NATJ/OSWD */
+    uint16_t wdt_timeout;   /* countdown in seconds */
+    uint8_t wdt_job;        /* action on expiry (APPLESMC_WDT_*) */
 };
 
 static void applesmc_io_cmd_write(void *opaque, hwaddr addr, uint64_t val,
@@ -162,6 +178,48 @@ static const struct AppleSMCData *applesmc_find_key(AppleSMCState *s)
     return NULL;
 }
 
+static void applesmc_wdt_expired(void *opaque)
+{
+    AppleSMCState *s = opaque;
+
+    trace_applesmc_wdt_expired(s->wdt_job);
+    warn_report("applesmc: watchdog expired, forcing the guest down");
+    if (s->wdt_job == APPLESMC_WDT_SHUTDOWN) {
+        qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
+    } else {
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
+    }
+}
+
+static void applesmc_wdt_set(AppleSMCState *s, uint8_t job, uint16_t seconds)
+{
+    s->wdt_job = job;
+    if (job == APPLESMC_WDT_DISABLE || seconds == 0) {
+        timer_del(s->wdt_timer);
+        trace_applesmc_wdt_disarm();
+        return;
+    }
+    timer_mod(s->wdt_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
+              (int64_t)seconds * NANOSECONDS_PER_SECOND);
+    trace_applesmc_wdt_arm(seconds, job);
+}
+
+/* Act on a guest write once its full payload has been received. */
+static void applesmc_write_key(AppleSMCState *s)
+{
+    if (!memcmp(s->key, "NATi", 4) && s->data_len >= 2) {
+        /* Big-endian seconds; stored only, the "NATJ" write arms the timer. */
+        s->wdt_timeout = (s->data[0] << 8) | s->data[1];
+    } else if (!memcmp(s->key, "NATJ", 4) && s->data_len >= 1) {
+        applesmc_wdt_set(s, s->data[0], s->wdt_timeout);
+    } else if (!memcmp(s->key, "OSWD", 4) && s->data_len >= 2) {
+        uint16_t seconds = (s->data[0] << 8) | s->data[1];
+        uint8_t job = seconds ? APPLESMC_WDT_RESTART : APPLESMC_WDT_DISABLE;
+
+        applesmc_wdt_set(s, job, seconds);
+    }
+}
+
 static void applesmc_io_data_write(void *opaque, hwaddr addr, uint64_t val,
                                    unsigned size)
 {
@@ -218,6 +276,7 @@ static void applesmc_io_data_write(void *opaque, hwaddr addr, uint64_t val,
             trace_applesmc_write_len(s->key[0], s->key[1], s->key[2],
                                      s->key[3], s->data_len);
             if (s->data_len == 0) {
+                applesmc_write_key(s);
                 s->status = APPLESMC_ST_CMD_DONE;
                 s->status_1e = APPLESMC_ST_CMD_DONE;
             }
@@ -229,6 +288,7 @@ static void applesmc_io_data_write(void *opaque, hwaddr addr, uint64_t val,
                 s->data_pos++;
             }
             if (s->data_pos >= s->data_len) {
+                applesmc_write_key(s);
                 trace_applesmc_write_complete(s->key[0], s->key[1],
                                               s->key[2], s->key[3],
                                               s->data_len);
@@ -325,6 +385,12 @@ static void qdev_applesmc_isa_reset(DeviceState *dev)
     s->status = 0x00;
     s->status_1e = 0x00;
     s->last_ret = 0x00;
+
+    if (s->wdt_timer) {
+        timer_del(s->wdt_timer);
+    }
+    s->wdt_job = APPLESMC_WDT_DISABLE;
+    s->wdt_timeout = 0;
 }
 
 static const MemoryRegionOps applesmc_data_io_ops = {
@@ -388,6 +454,10 @@ static void applesmc_isa_realize(DeviceState *dev, Error **errp)
     applesmc_add_key(s, "NATJ", 1, "\x00");
     applesmc_add_key(s, "MSSP", 1, "\x00");
     applesmc_add_key(s, "MSSD", 1, "\x03");
+    applesmc_add_key(s, "NATi", 2, "\x00\x00");
+    applesmc_add_key(s, "OSWD", 2, "\x00\x00");
+
+    s->wdt_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, applesmc_wdt_expired, s);
 }
 
 static void applesmc_unrealize(DeviceState *dev)
@@ -395,6 +465,10 @@ static void applesmc_unrealize(DeviceState *dev)
     AppleSMCState *s = APPLE_SMC(dev);
     struct AppleSMCData *d, *next;
 
+    if (s->wdt_timer) {
+        timer_free(s->wdt_timer);
+    }
+
     /* Remove existing entries */
     QLIST_FOREACH_SAFE(d, &s->data_def, node, next) {
         QLIST_REMOVE(d, node);
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index fc01cc9b7d..d94a5fba8a 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -48,6 +48,9 @@ applesmc_data_read(uint8_t c0, uint8_t c1, uint8_t c2, uint8_t c3, uint8_t pos,
 applesmc_write_len(uint8_t c0, uint8_t c1, uint8_t c2, uint8_t c3, uint8_t len) "write key %c%c%c%c len %u"
 applesmc_write_data(uint8_t c0, uint8_t c1, uint8_t c2, uint8_t c3, uint8_t pos, uint8_t val) "write key %c%c%c%c [%u] 0x%02x"
 applesmc_write_complete(uint8_t c0, uint8_t c1, uint8_t c2, uint8_t c3, uint8_t len) "write key %c%c%c%c complete (%u bytes)"
+applesmc_wdt_arm(uint16_t seconds, uint8_t job) "watchdog armed %u s, job %u"
+applesmc_wdt_disarm(void) "watchdog disarmed"
+applesmc_wdt_expired(uint8_t job) "watchdog expired, job %u"
 
 # avr_power.c
 avr_power_read(uint8_t value) "power_reduc read value:%u"
-- 
2.55.0

[PATCH 4/4] hw/misc/applesmc: Implement the get-key-type command

[Daniel Golle]

macOS probes the type of many keys through the SMC get-key-type command
(0x13), dozens of times during boot. The device did not implement it, so
each probe fell through to the default case and returned kSMCBadCommand.

Implement it. Give each key a four-character SMC type ("ui8 ", "ui16",
"ch8*", "{rev", ...) and answer the command with the standard key
information structure: the one-byte data size, the four-byte type and a
one-byte attributes field (reported as zero, as the device does not model
key attributes). Unknown keys return kSMCKeyNotFound, consistent with a
read of a missing key. macOS tolerates the probe failing, so this is not
a functional fix, but it replies to the queries the guest makes instead
of rejecting them.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 hw/misc/applesmc.c   | 52 ++++++++++++++++++++++++++++++++++++--------
 hw/misc/trace-events |  1 +
 2 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/hw/misc/applesmc.c b/hw/misc/applesmc.c
index 8d5bf74373..47e7e3f4be 100644
--- a/hw/misc/applesmc.c
+++ b/hw/misc/applesmc.c
@@ -104,6 +104,7 @@ static char default_osk[64] = "This is a dummy key. Enter the real key "
 struct AppleSMCData {
     uint8_t len;
     const char *key;
+    const char *type;
     const char *data;
     QLIST_ENTRY(AppleSMCData) node;
 };
@@ -145,6 +146,7 @@ static void applesmc_io_cmd_write(void *opaque, hwaddr addr, uint64_t val,
     switch (val) {
     case APPLESMC_READ_CMD:
     case APPLESMC_WRITE_CMD:
+    case APPLESMC_GET_KEY_TYPE_CMD:
         /* did last command run through OK? */
         if (status == APPLESMC_ST_CMD_DONE || status == APPLESMC_ST_NEW_CMD) {
             s->cmd = val;
@@ -299,6 +301,36 @@ static void applesmc_io_data_write(void *opaque, hwaddr addr, uint64_t val,
             }
         }
         break;
+    case APPLESMC_GET_KEY_TYPE_CMD:
+        /* Unlike a read, the guest sends only the 4 key bytes, no length. */
+        if (s->read_pos < 4) {
+            s->key[s->read_pos] = val;
+            s->status = APPLESMC_ST_ACK;
+            if (++s->read_pos == 4) {
+                trace_applesmc_key_selected(s->key[0], s->key[1],
+                                            s->key[2], s->key[3]);
+                d = applesmc_find_key(s);
+                if (d != NULL) {
+                    /* key info: 1-byte size, 4-byte type, 1-byte attributes */
+                    s->data[0] = d->len;
+                    memcpy(&s->data[1], d->type, 4);
+                    s->data[5] = 0;
+                    s->data_len = 6;
+                    s->data_pos = 0;
+                    s->status = APPLESMC_ST_ACK | APPLESMC_ST_DATA_READY;
+                    s->status_1e = APPLESMC_ST_CMD_DONE;
+                    trace_applesmc_key_type(s->key[0], s->key[1], s->key[2],
+                                            s->key[3], d->type[0], d->type[1],
+                                            d->type[2], d->type[3], d->len);
+                } else {
+                    trace_applesmc_key_not_found(s->key[0], s->key[1],
+                                                 s->key[2], s->key[3]);
+                    s->status = APPLESMC_ST_CMD_DONE;
+                    s->status_1e = APPLESMC_ST_1E_NOEXIST;
+                }
+            }
+        }
+        break;
     default:
         s->status = APPLESMC_ST_CMD_DONE;
         s->status_1e = APPLESMC_ST_1E_STILL_BAD_CMD;
@@ -318,6 +350,7 @@ static uint64_t applesmc_io_data_read(void *opaque, hwaddr addr, unsigned size)
 
     switch (s->cmd) {
     case APPLESMC_READ_CMD:
+    case APPLESMC_GET_KEY_TYPE_CMD:
         if (!(s->status & APPLESMC_ST_DATA_READY)) {
             break;
         }
@@ -366,12 +399,13 @@ static uint64_t applesmc_io_err_read(void *opaque, hwaddr addr, unsigned size)
 }
 
 static void applesmc_add_key(AppleSMCState *s, const char *key,
-                             int len, const char *data)
+                             const char *type, int len, const char *data)
 {
     struct AppleSMCData *def;
 
     def = g_new0(struct AppleSMCData, 1);
     def->key = key;
+    def->type = type;
     def->len = len;
     def->data = data;
 
@@ -448,14 +482,14 @@ static void applesmc_isa_realize(DeviceState *dev, Error **errp)
     }
 
     QLIST_INIT(&s->data_def);
-    applesmc_add_key(s, "REV ", 6, "\x01\x13\x0f\x00\x00\x03");
-    applesmc_add_key(s, "OSK0", 32, s->osk);
-    applesmc_add_key(s, "OSK1", 32, s->osk + 32);
-    applesmc_add_key(s, "NATJ", 1, "\x00");
-    applesmc_add_key(s, "MSSP", 1, "\x00");
-    applesmc_add_key(s, "MSSD", 1, "\x03");
-    applesmc_add_key(s, "NATi", 2, "\x00\x00");
-    applesmc_add_key(s, "OSWD", 2, "\x00\x00");
+    applesmc_add_key(s, "REV ", "{rev", 6, "\x01\x13\x0f\x00\x00\x03");
+    applesmc_add_key(s, "OSK0", "ch8*", 32, s->osk);
+    applesmc_add_key(s, "OSK1", "ch8*", 32, s->osk + 32);
+    applesmc_add_key(s, "NATJ", "ui8 ", 1, "\x00");
+    applesmc_add_key(s, "MSSP", "ui8 ", 1, "\x00");
+    applesmc_add_key(s, "MSSD", "si8 ", 1, "\x03");
+    applesmc_add_key(s, "NATi", "ui16", 2, "\x00\x00");
+    applesmc_add_key(s, "OSWD", "ui16", 2, "\x00\x00");
 
     s->wdt_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, applesmc_wdt_expired, s);
 }
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index d94a5fba8a..bd79fa45e1 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -44,6 +44,7 @@ applesmc_cmd_rejected(uint8_t cmd, uint8_t status_1e) "cmd 0x%02x rejected (stat
 applesmc_data_write(uint8_t cmd, uint8_t pos, uint8_t val) "cmd 0x%02x pos %u data 0x%02x"
 applesmc_key_selected(uint8_t c0, uint8_t c1, uint8_t c2, uint8_t c3) "key %c%c%c%c"
 applesmc_key_not_found(uint8_t c0, uint8_t c1, uint8_t c2, uint8_t c3) "key %c%c%c%c not found"
+applesmc_key_type(uint8_t c0, uint8_t c1, uint8_t c2, uint8_t c3, uint8_t t0, uint8_t t1, uint8_t t2, uint8_t t3, uint8_t len) "type key %c%c%c%c = %c%c%c%c[%u]"
 applesmc_data_read(uint8_t c0, uint8_t c1, uint8_t c2, uint8_t c3, uint8_t pos, uint8_t val) "read key %c%c%c%c [%u] 0x%02x"
 applesmc_write_len(uint8_t c0, uint8_t c1, uint8_t c2, uint8_t c3, uint8_t len) "write key %c%c%c%c len %u"
 applesmc_write_data(uint8_t c0, uint8_t c1, uint8_t c2, uint8_t c3, uint8_t pos, uint8_t val) "write key %c%c%c%c [%u] 0x%02x"
-- 
2.55.0

Re: [PATCH 0/4] hw/misc/applesmc: Implement writes, the shutdown watchdog and get-key-type

[Peter Maydell]

On Wed, 19 Aug 2026 at 07:41, Daniel Golle <daniel@makrotopia.org> wrote:
>
> QEMU's AppleSMC device only implements the SMC read command, so every
> other command a guest issues fails with kSMCBadCommand. Tracing a macOS
> guest shows it relies on two more: the write command (0x11) and
> get-key-type (0x13). The missing write command delays boot (macOS retries
> failed boot-time key writes) and produces a stream of
> SMCWDT::setWatchdogTimer errors at shutdown; the missing get-key-type is
> rejected dozens of times during boot.

How does this series fit in with Matthew Jackson's set of patches
https://patchew.org/QEMU/20260720164930.68383-1-matthew@pq.io/

?

thanks
-- PMM

Re: [PATCH 0/4] hw/misc/applesmc: Implement writes, the shutdown watchdog and get-key-type

[Daniel Golle]

On Wed, 19 Aug 2026 at 14:29, Peter Maydell <peter.maydell@linaro.org> wrote:
> How does this series fit in with Matthew Jackson's set of patches
> https://patchew.org/QEMU/20260720164930.68383-1-matthew@pq.io/

They do overlap, and Matthew's is the more complete of the two, so I
am dropping the overlapping parts of mine in favour of his.

We independently implemented the same two missing SMC commands (write
and get-key-type). On top of that Matthew's v4 also implements
get-key-by-index and populates a full key table, whereas my series
left get-key-by-index out (the macOS 26 guest I traced doesn't seem to
emitted them). So his series supersedes patches 2 and 4 of mine.

The only parts of my series not covered by his are the SMC shutdown
watchdog and the trace events. I have rebased those two patches on top
of his v4 and will send them as a short follow-up ("[PATCH 0/2]
hw/misc/ applesmc: SMC shutdown watchdog and trace events").
Reviewed-by/Tested-by for his series are in my replies to it.


Cheers and thanks for connecting the two threads


Daniel