[Qemu-devel] [PATCH v2 0/3] Update AppleSMC for OS X Sierra 10.12.4 guests

[Qemu-devel] [PATCH v2 0/3] Update AppleSMC for OS X Sierra 10.12.4 guests

[Gabriel L. Somlo]

As of 10.12.4 (currently the latest Sierra update), OS X refuses to boot
unless the AppleSMC supports a third I/O port, which provides the current
error status when read.

New since v1:

	- 1/3: don't touch the default OSK string, as it's unnecessary
		at this time

	- 2/3: don't consolidate I/O regions, leave as-is for data
		and cmd; This patch now implements the error-code state
		machine, AND adds an i/o region dedicaded to the error
		status port, complete with read/write access methods.

	- 3/3: optional patch setting access width to 1-byte on data and
		cmd i/o regions. Tested on OS X versions 10.[6..12].

> This series consists of three patches:
>
>	- 1/3: indentation/whitespace cleanup for applesmc.c to the point
>		where it now passes scripts/checkpatc.pl, and allows
>		subsequent changes to look nice in diff-patch format :)
>
>	- 2/3: consolidate Port I/O into a single region, and invoke
>		appropriate read/write methods based on the offset being
>		accessed
>
>	- 3/3: implement read-only error/status port, and update
>		data and command read/write methods to correctly
>		maintain the state machine for keeping the status_1e
>		value up to date.

Gabriel L. Somlo (3):
  applesmc: cosmetic whitespace and indentation cleanup
  applesmc: implement error status port
  applesmc: fix port i/o access width

 hw/misc/applesmc.c | 219 +++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 155 insertions(+), 64 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH v2 1/3] applesmc: cosmetic whitespace and indentation cleanup

[Gabriel L. Somlo]

Signed-off-by: Gabriel Somlo <gsomlo@gmail.com>
Reviewed-by: Alexander Graf <agraf@suse.de>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/misc/applesmc.c | 98 ++++++++++++++++++++++++++++--------------------------
 1 file changed, 50 insertions(+), 48 deletions(-)

diff --git a/hw/misc/applesmc.c b/hw/misc/applesmc.c
index 77fab5b..6381197 100644
--- a/hw/misc/applesmc.c
+++ b/hw/misc/applesmc.c
@@ -39,21 +39,24 @@
 /* #define DEBUG_SMC */
 
 #define APPLESMC_DEFAULT_IOBASE        0x300
-/* data port used by Apple SMC */
-#define APPLESMC_DATA_PORT             0x0
-/* command/status port used by Apple SMC */
-#define APPLESMC_CMD_PORT              0x4
-#define APPLESMC_NR_PORTS              32
 
-#define APPLESMC_READ_CMD              0x10
-#define APPLESMC_WRITE_CMD             0x11
-#define APPLESMC_GET_KEY_BY_INDEX_CMD  0x12
-#define APPLESMC_GET_KEY_TYPE_CMD      0x13
+enum {
+    APPLESMC_DATA_PORT               = 0x00,
+    APPLESMC_CMD_PORT                = 0x04,
+    APPLESMC_NUM_PORTS               = 0x20,
+};
+
+enum {
+    APPLESMC_READ_CMD                = 0x10,
+    APPLESMC_WRITE_CMD               = 0x11,
+    APPLESMC_GET_KEY_BY_INDEX_CMD    = 0x12,
+    APPLESMC_GET_KEY_TYPE_CMD        = 0x13,
+};
 
 #ifdef DEBUG_SMC
 #define smc_debug(...) fprintf(stderr, "AppleSMC: " __VA_ARGS__)
 #else
-#define smc_debug(...) do { } while(0)
+#define smc_debug(...) do { } while (0)
 #endif
 
 static char default_osk[64] = "This is a dummy key. Enter the real key "
@@ -77,12 +80,11 @@ struct AppleSMCState {
     uint32_t iobase;
     uint8_t cmd;
     uint8_t status;
-    uint8_t key[4];
+    char key[4];
     uint8_t read_pos;
     uint8_t data_len;
     uint8_t data_pos;
     uint8_t data[255];
-    uint8_t charactic[4];
     char *osk;
     QLIST_HEAD(, AppleSMCData) data_def;
 };
@@ -93,10 +95,10 @@ static void applesmc_io_cmd_write(void *opaque, hwaddr addr, uint64_t val,
     AppleSMCState *s = opaque;
 
     smc_debug("CMD Write B: %#x = %#x\n", addr, val);
-    switch(val) {
-        case APPLESMC_READ_CMD:
-            s->status = 0x0c;
-            break;
+    switch (val) {
+    case APPLESMC_READ_CMD:
+        s->status = 0x0c;
+        break;
     }
     s->cmd = val;
     s->read_pos = 0;
@@ -123,54 +125,54 @@ static void applesmc_io_data_write(void *opaque, hwaddr addr, uint64_t val,
     AppleSMCState *s = opaque;
 
     smc_debug("DATA Write B: %#x = %#x\n", addr, val);
-    switch(s->cmd) {
-        case APPLESMC_READ_CMD:
-            if(s->read_pos < 4) {
-                s->key[s->read_pos] = val;
-                s->status = 0x04;
-            } else if(s->read_pos == 4) {
-                s->data_len = val;
-                s->status = 0x05;
-                s->data_pos = 0;
-                smc_debug("Key = %c%c%c%c Len = %d\n", s->key[0],
-                          s->key[1], s->key[2], s->key[3], val);
-                applesmc_fill_data(s);
-            }
-            s->read_pos++;
-            break;
+    switch (s->cmd) {
+    case APPLESMC_READ_CMD:
+        if (s->read_pos < 4) {
+            s->key[s->read_pos] = val;
+            s->status = 0x04;
+        } else if (s->read_pos == 4) {
+            s->data_len = val;
+            s->status = 0x05;
+            s->data_pos = 0;
+            smc_debug("Key = %c%c%c%c Len = %d\n", s->key[0],
+                      s->key[1], s->key[2], s->key[3], val);
+            applesmc_fill_data(s);
+        }
+        s->read_pos++;
+        break;
     }
 }
 
-static uint64_t applesmc_io_data_read(void *opaque, hwaddr addr1,
-                                      unsigned size)
+static uint64_t applesmc_io_data_read(void *opaque, hwaddr addr, unsigned size)
 {
     AppleSMCState *s = opaque;
     uint8_t retval = 0;
 
-    switch(s->cmd) {
-        case APPLESMC_READ_CMD:
-            if(s->data_pos < s->data_len) {
-                retval = s->data[s->data_pos];
-                smc_debug("READ_DATA[%d] = %#hhx\n", s->data_pos,
-                          retval);
-                s->data_pos++;
-                if(s->data_pos == s->data_len) {
-                    s->status = 0x00;
-                    smc_debug("EOF\n");
-                } else
-                    s->status = 0x05;
+    switch (s->cmd) {
+    case APPLESMC_READ_CMD:
+        if (s->data_pos < s->data_len) {
+            retval = s->data[s->data_pos];
+            smc_debug("READ_DATA[%d] = %#hhx\n", s->data_pos,
+                      retval);
+            s->data_pos++;
+            if (s->data_pos == s->data_len) {
+                s->status = 0x00;
+                smc_debug("EOF\n");
+            } else {
+                s->status = 0x05;
             }
+        }
     }
-    smc_debug("DATA Read b: %#x = %#x\n", addr1, retval);
+    smc_debug("DATA Read b: %#x = %#x\n", addr, retval);
 
     return retval;
 }
 
-static uint64_t applesmc_io_cmd_read(void *opaque, hwaddr addr1, unsigned size)
+static uint64_t applesmc_io_cmd_read(void *opaque, hwaddr addr, unsigned size)
 {
     AppleSMCState *s = opaque;
 
-    smc_debug("CMD Read B: %#x\n", addr1);
+    smc_debug("CMD Read B: %#x\n", addr);
     return s->status;
 }
 
-- 
2.7.4

[Qemu-devel] [PATCH v2 2/3] applesmc: implement error status port

[Gabriel L. Somlo]

As of release 10.12.4, OS X (Sierra) refuses to boot unless the
AppleSMC supports an additional I/O port, expected to provide an
error status code.

Update the [cmd|data]_write() and data_read() methods to implement
the required state machine, and add I/O region & methods to handle
access to the error port.

Originally proposed by Eric Shelton <eshelton@pobox.com>

Signed-off-by: Gabriel Somlo <gsomlo@gmail.com>
---
 hw/misc/applesmc.c | 141 +++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 115 insertions(+), 26 deletions(-)

diff --git a/hw/misc/applesmc.c b/hw/misc/applesmc.c
index 6381197..0d882e8 100644
--- a/hw/misc/applesmc.c
+++ b/hw/misc/applesmc.c
@@ -43,6 +43,7 @@
 enum {
     APPLESMC_DATA_PORT               = 0x00,
     APPLESMC_CMD_PORT                = 0x04,
+    APPLESMC_ERR_PORT                = 0x1e,
     APPLESMC_NUM_PORTS               = 0x20,
 };
 
@@ -53,6 +54,24 @@ enum {
     APPLESMC_GET_KEY_TYPE_CMD        = 0x13,
 };
 
+enum {
+    APPLESMC_ST_CMD_DONE             = 0x00,
+    APPLESMC_ST_DATA_READY           = 0x01,
+    APPLESMC_ST_BUSY                 = 0x02,
+    APPLESMC_ST_ACK                  = 0x04,
+    APPLESMC_ST_NEW_CMD              = 0x08,
+};
+
+enum {
+    APPLESMC_ST_1E_CMD_INTRUPTED     = 0x80,
+    APPLESMC_ST_1E_STILL_BAD_CMD     = 0x81,
+    APPLESMC_ST_1E_BAD_CMD           = 0x82,
+    APPLESMC_ST_1E_NOEXIST           = 0x84,
+    APPLESMC_ST_1E_WRITEONLY         = 0x85,
+    APPLESMC_ST_1E_READONLY          = 0x86,
+    APPLESMC_ST_1E_BAD_INDEX         = 0xb8,
+};
+
 #ifdef DEBUG_SMC
 #define smc_debug(...) fprintf(stderr, "AppleSMC: " __VA_ARGS__)
 #else
@@ -77,9 +96,12 @@ struct AppleSMCState {
 
     MemoryRegion io_data;
     MemoryRegion io_cmd;
+    MemoryRegion io_err;
     uint32_t iobase;
     uint8_t cmd;
     uint8_t status;
+    uint8_t status_1e;
+    uint8_t last_ret;
     char key[4];
     uint8_t read_pos;
     uint8_t data_len;
@@ -93,89 +115,138 @@ static void applesmc_io_cmd_write(void *opaque, hwaddr addr, uint64_t val,
                                   unsigned size)
 {
     AppleSMCState *s = opaque;
+    uint8_t status = s->status & 0x0f;
 
-    smc_debug("CMD Write B: %#x = %#x\n", addr, val);
+    smc_debug("CMD received: 0x%02x\n", (uint8_t)val);
     switch (val) {
     case APPLESMC_READ_CMD:
-        s->status = 0x0c;
+        /* did last command run through OK? */
+        if (status == APPLESMC_ST_CMD_DONE || status == APPLESMC_ST_NEW_CMD) {
+            s->cmd = val;
+            s->status = APPLESMC_ST_NEW_CMD | APPLESMC_ST_ACK;
+        } else {
+            smc_debug("ERROR: previous command interrupted!\n");
+            s->status = APPLESMC_ST_NEW_CMD;
+            s->status_1e = APPLESMC_ST_1E_CMD_INTRUPTED;
+        }
         break;
+    default:
+        smc_debug("UNEXPECTED CMD 0x%02x\n", (uint8_t)val);
+        s->status = APPLESMC_ST_NEW_CMD;
+        s->status_1e = APPLESMC_ST_1E_BAD_CMD;
     }
-    s->cmd = val;
     s->read_pos = 0;
     s->data_pos = 0;
 }
 
-static void applesmc_fill_data(AppleSMCState *s)
+static struct AppleSMCData *applesmc_find_key(AppleSMCState *s)
 {
     struct AppleSMCData *d;
 
     QLIST_FOREACH(d, &s->data_def, node) {
         if (!memcmp(d->key, s->key, 4)) {
-            smc_debug("Key matched (%s Len=%d Data=%s)\n", d->key,
-                      d->len, d->data);
-            memcpy(s->data, d->data, d->len);
-            return;
+            return d;
         }
     }
+    return NULL;
 }
 
 static void applesmc_io_data_write(void *opaque, hwaddr addr, uint64_t val,
                                    unsigned size)
 {
     AppleSMCState *s = opaque;
+    struct AppleSMCData *d;
 
-    smc_debug("DATA Write B: %#x = %#x\n", addr, val);
+    smc_debug("DATA received: 0x%02x\n", (uint8_t)val);
     switch (s->cmd) {
     case APPLESMC_READ_CMD:
+        if ((s->status & 0x0f) == APPLESMC_ST_CMD_DONE) {
+            break;
+        }
         if (s->read_pos < 4) {
             s->key[s->read_pos] = val;
-            s->status = 0x04;
+            s->status = APPLESMC_ST_ACK;
         } else if (s->read_pos == 4) {
-            s->data_len = val;
-            s->status = 0x05;
-            s->data_pos = 0;
-            smc_debug("Key = %c%c%c%c Len = %d\n", s->key[0],
-                      s->key[1], s->key[2], s->key[3], val);
-            applesmc_fill_data(s);
+            d = applesmc_find_key(s);
+            if (d != NULL) {
+                memcpy(s->data, d->data, d->len);
+                s->data_len = d->len;
+                s->data_pos = 0;
+                s->status = APPLESMC_ST_ACK | APPLESMC_ST_DATA_READY;
+                s->status_1e = APPLESMC_ST_CMD_DONE;  /* clear on valid key */
+            } 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]);
+                s->status = APPLESMC_ST_CMD_DONE;
+                s->status_1e = APPLESMC_ST_1E_NOEXIST;
+            }
         }
         s->read_pos++;
         break;
+    default:
+        s->status = APPLESMC_ST_CMD_DONE;
+        s->status_1e = APPLESMC_ST_1E_STILL_BAD_CMD;
     }
 }
 
+static void applesmc_io_err_write(void *opaque, hwaddr addr, uint64_t val,
+                                  unsigned size)
+{
+    smc_debug("ERR_CODE received: 0x%02x, ignoring!\n", (uint8_t)val);
+    /* NOTE: writing to the error port not supported! */
+}
+
 static uint64_t applesmc_io_data_read(void *opaque, hwaddr addr, unsigned size)
 {
     AppleSMCState *s = opaque;
-    uint8_t retval = 0;
 
     switch (s->cmd) {
     case APPLESMC_READ_CMD:
+        if (!(s->status & APPLESMC_ST_DATA_READY)) {
+            break;
+        }
         if (s->data_pos < s->data_len) {
-            retval = s->data[s->data_pos];
-            smc_debug("READ_DATA[%d] = %#hhx\n", s->data_pos,
-                      retval);
+            s->last_ret = s->data[s->data_pos];
+            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);
             s->data_pos++;
             if (s->data_pos == s->data_len) {
-                s->status = 0x00;
-                smc_debug("EOF\n");
+                s->status = APPLESMC_ST_CMD_DONE;
+                smc_debug("READ '%c%c%c%c' Len=%d complete!\n",
+                          s->key[0], s->key[1], s->key[2], s->key[3],
+                          s->data_len);
             } else {
-                s->status = 0x05;
+                s->status = APPLESMC_ST_ACK | APPLESMC_ST_DATA_READY;
             }
         }
+        break;
+    default:
+        s->status = APPLESMC_ST_CMD_DONE;
+        s->status_1e = APPLESMC_ST_1E_STILL_BAD_CMD;
     }
-    smc_debug("DATA Read b: %#x = %#x\n", addr, retval);
+    smc_debug("DATA sent: 0x%02x\n", s->last_ret);
 
-    return retval;
+    return s->last_ret;
 }
 
 static uint64_t applesmc_io_cmd_read(void *opaque, hwaddr addr, unsigned size)
 {
     AppleSMCState *s = opaque;
 
-    smc_debug("CMD Read B: %#x\n", addr);
+    smc_debug("CMD sent: 0x%02x\n", s->status);
     return s->status;
 }
 
+static uint64_t applesmc_io_err_read(void *opaque, hwaddr addr, unsigned size)
+{
+    AppleSMCState *s = opaque;
+
+    /* NOTE: read does not clear the 1e status */
+    smc_debug("ERR_CODE sent: 0x%02x\n", s->status_1e);
+    return s->status_1e;
+}
+
 static void applesmc_add_key(AppleSMCState *s, const char *key,
                              int len, const char *data)
 {
@@ -198,6 +269,9 @@ static void qdev_applesmc_isa_reset(DeviceState *dev)
     QLIST_FOREACH_SAFE(d, &s->data_def, node, next) {
         QLIST_REMOVE(d, node);
     }
+    s->status = 0x00;
+    s->status_1e = 0x00;
+    s->last_ret = 0x00;
 
     applesmc_add_key(s, "REV ", 6, "\x01\x13\x0f\x00\x00\x03");
     applesmc_add_key(s, "OSK0", 32, s->osk);
@@ -227,6 +301,16 @@ static const MemoryRegionOps applesmc_cmd_io_ops = {
     },
 };
 
+static const MemoryRegionOps applesmc_err_io_ops = {
+    .write = applesmc_io_err_write,
+    .read = applesmc_io_err_read,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
+
 static void applesmc_isa_realize(DeviceState *dev, Error **errp)
 {
     AppleSMCState *s = APPLE_SMC(dev);
@@ -241,6 +325,11 @@ static void applesmc_isa_realize(DeviceState *dev, Error **errp)
     isa_register_ioport(&s->parent_obj, &s->io_cmd,
                         s->iobase + APPLESMC_CMD_PORT);
 
+    memory_region_init_io(&s->io_err, OBJECT(s), &applesmc_err_io_ops, s,
+                          "applesmc-err", 1);
+    isa_register_ioport(&s->parent_obj, &s->io_err,
+                        s->iobase + APPLESMC_ERR_PORT);
+
     if (!s->osk || (strlen(s->osk) != 64)) {
         fprintf(stderr, "WARNING: Using AppleSMC with invalid key\n");
         s->osk = default_osk;
-- 
2.7.4

[Qemu-devel] [PATCH v2 3/3] applesmc: fix port i/o access width

[Gabriel L. Somlo]

Set width of the two i/o regions dedicated to the AppleSMC's 8-bit
data and command ports to 1 byte.

Signed-off-by: Gabriel Somlo <gsomlo@gmail.com>
---

Setting these to 1-byte width works fine on any OS X version I could find
to test on: 10.(6-12), inclusive.

On linux, the applesmc kernel module tries *hard* to avoid loading on
anything that's not a Mac, by checking DMI board vendor and product
strings. If I force it using:

   -smbios type=1,manufacturer='Apple Inc.',product='iMac2',family='iMac' \
   -smbios type=2,manufacturer='Apple Inc.',version='iMac' \
   -device isa-applesmc

the module fails both before and after this whole series, suggesting it's
not (just) the access width but rather the overall incomplete emulation
that's the issue.

If we decide to go for implementing a more complete emulation, beyond
simply the minimum necessary to satisfy OS X, we should definitely ensure
that Linux is also happily able to initialize its applesmc driver...

 hw/misc/applesmc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/misc/applesmc.c b/hw/misc/applesmc.c
index 0d882e8..7896812 100644
--- a/hw/misc/applesmc.c
+++ b/hw/misc/applesmc.c
@@ -316,12 +316,12 @@ static void applesmc_isa_realize(DeviceState *dev, Error **errp)
     AppleSMCState *s = APPLE_SMC(dev);
 
     memory_region_init_io(&s->io_data, OBJECT(s), &applesmc_data_io_ops, s,
-                          "applesmc-data", 4);
+                          "applesmc-data", 1);
     isa_register_ioport(&s->parent_obj, &s->io_data,
                         s->iobase + APPLESMC_DATA_PORT);
 
     memory_region_init_io(&s->io_cmd, OBJECT(s), &applesmc_cmd_io_ops, s,
-                          "applesmc-cmd", 4);
+                          "applesmc-cmd", 1);
     isa_register_ioport(&s->parent_obj, &s->io_cmd,
                         s->iobase + APPLESMC_CMD_PORT);
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 0/3] Update AppleSMC for OS X Sierra 10.12.4 guests

[Alexander Graf]

On 04/04/2017 07:01 PM, Gabriel L. Somlo wrote:
> As of 10.12.4 (currently the latest Sierra update), OS X refuses to boot
> unless the AppleSMC supports a third I/O port, which provides the current
> error status when read.

Looks much nicer after this series :). Thanks a lot!

Reviewed-by: Alexander Graf <agraf@suse.de>


Alex

Re: [Qemu-devel] [PATCH v2 0/3] Update AppleSMC for OS X Sierra 10.12.4 guests

[Gabriel L. Somlo]

On Tue, Apr 04, 2017 at 09:35:00PM +0200, Alexander Graf wrote:
> On 04/04/2017 07:01 PM, Gabriel L. Somlo wrote:
> > As of 10.12.4 (currently the latest Sierra update), OS X refuses to boot
> > unless the AppleSMC supports a third I/O port, which provides the current
> > error status when read.
> 
> Looks much nicer after this series :). Thanks a lot!
> 
> Reviewed-by: Alexander Graf <agraf@suse.de>

Thanks! Thinking about it, we should probably drop the last (3/3)
patch, once again to minimize useless churn -- Doing that part (or
not, as the case may turn out to be) should be part of a future
series implementing more accurate emulation, including key write
support...

Thanks,
--Gabriel

Re: [Qemu-devel] [PATCH v2 1/3] applesmc: cosmetic whitespace and indentation cleanup

[Phil Dennis-Jordan]

Looks good to me.

Reviewed-by: Phil Dennis-Jordan <phil@philjordan.eu>


On Wed, Apr 5, 2017 at 5:01 AM, Gabriel L. Somlo <gsomlo@gmail.com> wrote:
> Signed-off-by: Gabriel Somlo <gsomlo@gmail.com>
> Reviewed-by: Alexander Graf <agraf@suse.de>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/misc/applesmc.c | 98 ++++++++++++++++++++++++++++--------------------------
>  1 file changed, 50 insertions(+), 48 deletions(-)
>
> diff --git a/hw/misc/applesmc.c b/hw/misc/applesmc.c
> index 77fab5b..6381197 100644
> --- a/hw/misc/applesmc.c
> +++ b/hw/misc/applesmc.c
> @@ -39,21 +39,24 @@
>  /* #define DEBUG_SMC */
>
>  #define APPLESMC_DEFAULT_IOBASE        0x300
> -/* data port used by Apple SMC */
> -#define APPLESMC_DATA_PORT             0x0
> -/* command/status port used by Apple SMC */
> -#define APPLESMC_CMD_PORT              0x4
> -#define APPLESMC_NR_PORTS              32
>
> -#define APPLESMC_READ_CMD              0x10
> -#define APPLESMC_WRITE_CMD             0x11
> -#define APPLESMC_GET_KEY_BY_INDEX_CMD  0x12
> -#define APPLESMC_GET_KEY_TYPE_CMD      0x13
> +enum {
> +    APPLESMC_DATA_PORT               = 0x00,
> +    APPLESMC_CMD_PORT                = 0x04,
> +    APPLESMC_NUM_PORTS               = 0x20,
> +};
> +
> +enum {
> +    APPLESMC_READ_CMD                = 0x10,
> +    APPLESMC_WRITE_CMD               = 0x11,
> +    APPLESMC_GET_KEY_BY_INDEX_CMD    = 0x12,
> +    APPLESMC_GET_KEY_TYPE_CMD        = 0x13,
> +};
>
>  #ifdef DEBUG_SMC
>  #define smc_debug(...) fprintf(stderr, "AppleSMC: " __VA_ARGS__)
>  #else
> -#define smc_debug(...) do { } while(0)
> +#define smc_debug(...) do { } while (0)
>  #endif
>
>  static char default_osk[64] = "This is a dummy key. Enter the real key "
> @@ -77,12 +80,11 @@ struct AppleSMCState {
>      uint32_t iobase;
>      uint8_t cmd;
>      uint8_t status;
> -    uint8_t key[4];
> +    char key[4];
>      uint8_t read_pos;
>      uint8_t data_len;
>      uint8_t data_pos;
>      uint8_t data[255];
> -    uint8_t charactic[4];
>      char *osk;
>      QLIST_HEAD(, AppleSMCData) data_def;
>  };
> @@ -93,10 +95,10 @@ static void applesmc_io_cmd_write(void *opaque, hwaddr addr, uint64_t val,
>      AppleSMCState *s = opaque;
>
>      smc_debug("CMD Write B: %#x = %#x\n", addr, val);
> -    switch(val) {
> -        case APPLESMC_READ_CMD:
> -            s->status = 0x0c;
> -            break;
> +    switch (val) {
> +    case APPLESMC_READ_CMD:
> +        s->status = 0x0c;
> +        break;
>      }
>      s->cmd = val;
>      s->read_pos = 0;
> @@ -123,54 +125,54 @@ static void applesmc_io_data_write(void *opaque, hwaddr addr, uint64_t val,
>      AppleSMCState *s = opaque;
>
>      smc_debug("DATA Write B: %#x = %#x\n", addr, val);
> -    switch(s->cmd) {
> -        case APPLESMC_READ_CMD:
> -            if(s->read_pos < 4) {
> -                s->key[s->read_pos] = val;
> -                s->status = 0x04;
> -            } else if(s->read_pos == 4) {
> -                s->data_len = val;
> -                s->status = 0x05;
> -                s->data_pos = 0;
> -                smc_debug("Key = %c%c%c%c Len = %d\n", s->key[0],
> -                          s->key[1], s->key[2], s->key[3], val);
> -                applesmc_fill_data(s);
> -            }
> -            s->read_pos++;
> -            break;
> +    switch (s->cmd) {
> +    case APPLESMC_READ_CMD:
> +        if (s->read_pos < 4) {
> +            s->key[s->read_pos] = val;
> +            s->status = 0x04;
> +        } else if (s->read_pos == 4) {
> +            s->data_len = val;
> +            s->status = 0x05;
> +            s->data_pos = 0;
> +            smc_debug("Key = %c%c%c%c Len = %d\n", s->key[0],
> +                      s->key[1], s->key[2], s->key[3], val);
> +            applesmc_fill_data(s);
> +        }
> +        s->read_pos++;
> +        break;
>      }
>  }
>
> -static uint64_t applesmc_io_data_read(void *opaque, hwaddr addr1,
> -                                      unsigned size)
> +static uint64_t applesmc_io_data_read(void *opaque, hwaddr addr, unsigned size)
>  {
>      AppleSMCState *s = opaque;
>      uint8_t retval = 0;
>
> -    switch(s->cmd) {
> -        case APPLESMC_READ_CMD:
> -            if(s->data_pos < s->data_len) {
> -                retval = s->data[s->data_pos];
> -                smc_debug("READ_DATA[%d] = %#hhx\n", s->data_pos,
> -                          retval);
> -                s->data_pos++;
> -                if(s->data_pos == s->data_len) {
> -                    s->status = 0x00;
> -                    smc_debug("EOF\n");
> -                } else
> -                    s->status = 0x05;
> +    switch (s->cmd) {
> +    case APPLESMC_READ_CMD:
> +        if (s->data_pos < s->data_len) {
> +            retval = s->data[s->data_pos];
> +            smc_debug("READ_DATA[%d] = %#hhx\n", s->data_pos,
> +                      retval);
> +            s->data_pos++;
> +            if (s->data_pos == s->data_len) {
> +                s->status = 0x00;
> +                smc_debug("EOF\n");
> +            } else {
> +                s->status = 0x05;
>              }
> +        }
>      }
> -    smc_debug("DATA Read b: %#x = %#x\n", addr1, retval);
> +    smc_debug("DATA Read b: %#x = %#x\n", addr, retval);
>
>      return retval;
>  }
>
> -static uint64_t applesmc_io_cmd_read(void *opaque, hwaddr addr1, unsigned size)
> +static uint64_t applesmc_io_cmd_read(void *opaque, hwaddr addr, unsigned size)
>  {
>      AppleSMCState *s = opaque;
>
> -    smc_debug("CMD Read B: %#x\n", addr1);
> +    smc_debug("CMD Read B: %#x\n", addr);
>      return s->status;
>  }
>
> --
> 2.7.4
>
>

Re: [Qemu-devel] [PATCH v2 2/3] applesmc: implement error status port

[Phil Dennis-Jordan]

I can confirm this solves the boot-time kernel panic on macOS 10.12.4
guests. No apparent regressions for earlier OS versions I tested,
either. Code looks sane to me. I guess the only thing is I assume this
was backed by some kind of investigation into what the real hardware
does. If that investigation is documented somewhere (FakeSMC mailing
list/forum perhaps?) then it might be worth linking to that in the
commit message, assuming that's in line with Qemu policy.

Thanks for implementing this!

Reviewed-by: Phil Dennis-Jordan <phil@philjordan.eu>


On Wed, Apr 5, 2017 at 5:01 AM, Gabriel L. Somlo <gsomlo@gmail.com> wrote:
> As of release 10.12.4, OS X (Sierra) refuses to boot unless the
> AppleSMC supports an additional I/O port, expected to provide an
> error status code.
>
> Update the [cmd|data]_write() and data_read() methods to implement
> the required state machine, and add I/O region & methods to handle
> access to the error port.
>
> Originally proposed by Eric Shelton <eshelton@pobox.com>
>
> Signed-off-by: Gabriel Somlo <gsomlo@gmail.com>
> ---
>  hw/misc/applesmc.c | 141 +++++++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 115 insertions(+), 26 deletions(-)
>
> diff --git a/hw/misc/applesmc.c b/hw/misc/applesmc.c
> index 6381197..0d882e8 100644
> --- a/hw/misc/applesmc.c
> +++ b/hw/misc/applesmc.c
> @@ -43,6 +43,7 @@
>  enum {
>      APPLESMC_DATA_PORT               = 0x00,
>      APPLESMC_CMD_PORT                = 0x04,
> +    APPLESMC_ERR_PORT                = 0x1e,
>      APPLESMC_NUM_PORTS               = 0x20,
>  };
>
> @@ -53,6 +54,24 @@ enum {
>      APPLESMC_GET_KEY_TYPE_CMD        = 0x13,
>  };
>
> +enum {
> +    APPLESMC_ST_CMD_DONE             = 0x00,
> +    APPLESMC_ST_DATA_READY           = 0x01,
> +    APPLESMC_ST_BUSY                 = 0x02,
> +    APPLESMC_ST_ACK                  = 0x04,
> +    APPLESMC_ST_NEW_CMD              = 0x08,
> +};
> +
> +enum {
> +    APPLESMC_ST_1E_CMD_INTRUPTED     = 0x80,
> +    APPLESMC_ST_1E_STILL_BAD_CMD     = 0x81,
> +    APPLESMC_ST_1E_BAD_CMD           = 0x82,
> +    APPLESMC_ST_1E_NOEXIST           = 0x84,
> +    APPLESMC_ST_1E_WRITEONLY         = 0x85,
> +    APPLESMC_ST_1E_READONLY          = 0x86,
> +    APPLESMC_ST_1E_BAD_INDEX         = 0xb8,
> +};
> +
>  #ifdef DEBUG_SMC
>  #define smc_debug(...) fprintf(stderr, "AppleSMC: " __VA_ARGS__)
>  #else
> @@ -77,9 +96,12 @@ struct AppleSMCState {
>
>      MemoryRegion io_data;
>      MemoryRegion io_cmd;
> +    MemoryRegion io_err;
>      uint32_t iobase;
>      uint8_t cmd;
>      uint8_t status;
> +    uint8_t status_1e;
> +    uint8_t last_ret;
>      char key[4];
>      uint8_t read_pos;
>      uint8_t data_len;
> @@ -93,89 +115,138 @@ static void applesmc_io_cmd_write(void *opaque, hwaddr addr, uint64_t val,
>                                    unsigned size)
>  {
>      AppleSMCState *s = opaque;
> +    uint8_t status = s->status & 0x0f;
>
> -    smc_debug("CMD Write B: %#x = %#x\n", addr, val);
> +    smc_debug("CMD received: 0x%02x\n", (uint8_t)val);
>      switch (val) {
>      case APPLESMC_READ_CMD:
> -        s->status = 0x0c;
> +        /* did last command run through OK? */
> +        if (status == APPLESMC_ST_CMD_DONE || status == APPLESMC_ST_NEW_CMD) {
> +            s->cmd = val;
> +            s->status = APPLESMC_ST_NEW_CMD | APPLESMC_ST_ACK;
> +        } else {
> +            smc_debug("ERROR: previous command interrupted!\n");
> +            s->status = APPLESMC_ST_NEW_CMD;
> +            s->status_1e = APPLESMC_ST_1E_CMD_INTRUPTED;
> +        }
>          break;
> +    default:
> +        smc_debug("UNEXPECTED CMD 0x%02x\n", (uint8_t)val);
> +        s->status = APPLESMC_ST_NEW_CMD;
> +        s->status_1e = APPLESMC_ST_1E_BAD_CMD;
>      }
> -    s->cmd = val;
>      s->read_pos = 0;
>      s->data_pos = 0;
>  }
>
> -static void applesmc_fill_data(AppleSMCState *s)
> +static struct AppleSMCData *applesmc_find_key(AppleSMCState *s)
>  {
>      struct AppleSMCData *d;
>
>      QLIST_FOREACH(d, &s->data_def, node) {
>          if (!memcmp(d->key, s->key, 4)) {
> -            smc_debug("Key matched (%s Len=%d Data=%s)\n", d->key,
> -                      d->len, d->data);
> -            memcpy(s->data, d->data, d->len);
> -            return;
> +            return d;
>          }
>      }
> +    return NULL;
>  }
>
>  static void applesmc_io_data_write(void *opaque, hwaddr addr, uint64_t val,
>                                     unsigned size)
>  {
>      AppleSMCState *s = opaque;
> +    struct AppleSMCData *d;
>
> -    smc_debug("DATA Write B: %#x = %#x\n", addr, val);
> +    smc_debug("DATA received: 0x%02x\n", (uint8_t)val);
>      switch (s->cmd) {
>      case APPLESMC_READ_CMD:
> +        if ((s->status & 0x0f) == APPLESMC_ST_CMD_DONE) {
> +            break;
> +        }
>          if (s->read_pos < 4) {
>              s->key[s->read_pos] = val;
> -            s->status = 0x04;
> +            s->status = APPLESMC_ST_ACK;
>          } else if (s->read_pos == 4) {
> -            s->data_len = val;
> -            s->status = 0x05;
> -            s->data_pos = 0;
> -            smc_debug("Key = %c%c%c%c Len = %d\n", s->key[0],
> -                      s->key[1], s->key[2], s->key[3], val);
> -            applesmc_fill_data(s);
> +            d = applesmc_find_key(s);
> +            if (d != NULL) {
> +                memcpy(s->data, d->data, d->len);
> +                s->data_len = d->len;
> +                s->data_pos = 0;
> +                s->status = APPLESMC_ST_ACK | APPLESMC_ST_DATA_READY;
> +                s->status_1e = APPLESMC_ST_CMD_DONE;  /* clear on valid key */
> +            } 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]);
> +                s->status = APPLESMC_ST_CMD_DONE;
> +                s->status_1e = APPLESMC_ST_1E_NOEXIST;
> +            }
>          }
>          s->read_pos++;
>          break;
> +    default:
> +        s->status = APPLESMC_ST_CMD_DONE;
> +        s->status_1e = APPLESMC_ST_1E_STILL_BAD_CMD;
>      }
>  }
>
> +static void applesmc_io_err_write(void *opaque, hwaddr addr, uint64_t val,
> +                                  unsigned size)
> +{
> +    smc_debug("ERR_CODE received: 0x%02x, ignoring!\n", (uint8_t)val);
> +    /* NOTE: writing to the error port not supported! */
> +}
> +
>  static uint64_t applesmc_io_data_read(void *opaque, hwaddr addr, unsigned size)
>  {
>      AppleSMCState *s = opaque;
> -    uint8_t retval = 0;
>
>      switch (s->cmd) {
>      case APPLESMC_READ_CMD:
> +        if (!(s->status & APPLESMC_ST_DATA_READY)) {
> +            break;
> +        }
>          if (s->data_pos < s->data_len) {
> -            retval = s->data[s->data_pos];
> -            smc_debug("READ_DATA[%d] = %#hhx\n", s->data_pos,
> -                      retval);
> +            s->last_ret = s->data[s->data_pos];
> +            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);
>              s->data_pos++;
>              if (s->data_pos == s->data_len) {
> -                s->status = 0x00;
> -                smc_debug("EOF\n");
> +                s->status = APPLESMC_ST_CMD_DONE;
> +                smc_debug("READ '%c%c%c%c' Len=%d complete!\n",
> +                          s->key[0], s->key[1], s->key[2], s->key[3],
> +                          s->data_len);
>              } else {
> -                s->status = 0x05;
> +                s->status = APPLESMC_ST_ACK | APPLESMC_ST_DATA_READY;
>              }
>          }
> +        break;
> +    default:
> +        s->status = APPLESMC_ST_CMD_DONE;
> +        s->status_1e = APPLESMC_ST_1E_STILL_BAD_CMD;
>      }
> -    smc_debug("DATA Read b: %#x = %#x\n", addr, retval);
> +    smc_debug("DATA sent: 0x%02x\n", s->last_ret);
>
> -    return retval;
> +    return s->last_ret;
>  }
>
>  static uint64_t applesmc_io_cmd_read(void *opaque, hwaddr addr, unsigned size)
>  {
>      AppleSMCState *s = opaque;
>
> -    smc_debug("CMD Read B: %#x\n", addr);
> +    smc_debug("CMD sent: 0x%02x\n", s->status);
>      return s->status;
>  }
>
> +static uint64_t applesmc_io_err_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    AppleSMCState *s = opaque;
> +
> +    /* NOTE: read does not clear the 1e status */
> +    smc_debug("ERR_CODE sent: 0x%02x\n", s->status_1e);
> +    return s->status_1e;
> +}
> +
>  static void applesmc_add_key(AppleSMCState *s, const char *key,
>                               int len, const char *data)
>  {
> @@ -198,6 +269,9 @@ static void qdev_applesmc_isa_reset(DeviceState *dev)
>      QLIST_FOREACH_SAFE(d, &s->data_def, node, next) {
>          QLIST_REMOVE(d, node);
>      }
> +    s->status = 0x00;
> +    s->status_1e = 0x00;
> +    s->last_ret = 0x00;
>
>      applesmc_add_key(s, "REV ", 6, "\x01\x13\x0f\x00\x00\x03");
>      applesmc_add_key(s, "OSK0", 32, s->osk);
> @@ -227,6 +301,16 @@ static const MemoryRegionOps applesmc_cmd_io_ops = {
>      },
>  };
>
> +static const MemoryRegionOps applesmc_err_io_ops = {
> +    .write = applesmc_io_err_write,
> +    .read = applesmc_io_err_read,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .impl = {
> +        .min_access_size = 1,
> +        .max_access_size = 1,
> +    },
> +};
> +
>  static void applesmc_isa_realize(DeviceState *dev, Error **errp)
>  {
>      AppleSMCState *s = APPLE_SMC(dev);
> @@ -241,6 +325,11 @@ static void applesmc_isa_realize(DeviceState *dev, Error **errp)
>      isa_register_ioport(&s->parent_obj, &s->io_cmd,
>                          s->iobase + APPLESMC_CMD_PORT);
>
> +    memory_region_init_io(&s->io_err, OBJECT(s), &applesmc_err_io_ops, s,
> +                          "applesmc-err", 1);
> +    isa_register_ioport(&s->parent_obj, &s->io_err,
> +                        s->iobase + APPLESMC_ERR_PORT);
> +
>      if (!s->osk || (strlen(s->osk) != 64)) {
>          fprintf(stderr, "WARNING: Using AppleSMC with invalid key\n");
>          s->osk = default_osk;
> --
> 2.7.4
>
>

Re: [Qemu-devel] [PATCH v2 2/3] applesmc: implement error status port

[Philippe Mathieu-Daudé]

On 04/04/2017 02:01 PM, Gabriel L. Somlo wrote:
> As of release 10.12.4, OS X (Sierra) refuses to boot unless the
> AppleSMC supports an additional I/O port, expected to provide an
> error status code.
>
> Update the [cmd|data]_write() and data_read() methods to implement
> the required state machine, and add I/O region & methods to handle
> access to the error port.
>
> Originally proposed by Eric Shelton <eshelton@pobox.com>
>
> Signed-off-by: Gabriel Somlo <gsomlo@gmail.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/misc/applesmc.c | 141 +++++++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 115 insertions(+), 26 deletions(-)
>
> diff --git a/hw/misc/applesmc.c b/hw/misc/applesmc.c
> index 6381197..0d882e8 100644
> --- a/hw/misc/applesmc.c
> +++ b/hw/misc/applesmc.c
> @@ -43,6 +43,7 @@
>  enum {
>      APPLESMC_DATA_PORT               = 0x00,
>      APPLESMC_CMD_PORT                = 0x04,
> +    APPLESMC_ERR_PORT                = 0x1e,
>      APPLESMC_NUM_PORTS               = 0x20,
>  };
>
> @@ -53,6 +54,24 @@ enum {
>      APPLESMC_GET_KEY_TYPE_CMD        = 0x13,
>  };
>
> +enum {
> +    APPLESMC_ST_CMD_DONE             = 0x00,
> +    APPLESMC_ST_DATA_READY           = 0x01,
> +    APPLESMC_ST_BUSY                 = 0x02,
> +    APPLESMC_ST_ACK                  = 0x04,
> +    APPLESMC_ST_NEW_CMD              = 0x08,
> +};
> +
> +enum {
> +    APPLESMC_ST_1E_CMD_INTRUPTED     = 0x80,
> +    APPLESMC_ST_1E_STILL_BAD_CMD     = 0x81,
> +    APPLESMC_ST_1E_BAD_CMD           = 0x82,
> +    APPLESMC_ST_1E_NOEXIST           = 0x84,
> +    APPLESMC_ST_1E_WRITEONLY         = 0x85,
> +    APPLESMC_ST_1E_READONLY          = 0x86,
> +    APPLESMC_ST_1E_BAD_INDEX         = 0xb8,
> +};
> +
>  #ifdef DEBUG_SMC
>  #define smc_debug(...) fprintf(stderr, "AppleSMC: " __VA_ARGS__)
>  #else
> @@ -77,9 +96,12 @@ struct AppleSMCState {
>
>      MemoryRegion io_data;
>      MemoryRegion io_cmd;
> +    MemoryRegion io_err;
>      uint32_t iobase;
>      uint8_t cmd;
>      uint8_t status;
> +    uint8_t status_1e;
> +    uint8_t last_ret;
>      char key[4];
>      uint8_t read_pos;
>      uint8_t data_len;
> @@ -93,89 +115,138 @@ static void applesmc_io_cmd_write(void *opaque, hwaddr addr, uint64_t val,
>                                    unsigned size)
>  {
>      AppleSMCState *s = opaque;
> +    uint8_t status = s->status & 0x0f;
>
> -    smc_debug("CMD Write B: %#x = %#x\n", addr, val);
> +    smc_debug("CMD received: 0x%02x\n", (uint8_t)val);
>      switch (val) {
>      case APPLESMC_READ_CMD:
> -        s->status = 0x0c;
> +        /* did last command run through OK? */
> +        if (status == APPLESMC_ST_CMD_DONE || status == APPLESMC_ST_NEW_CMD) {
> +            s->cmd = val;
> +            s->status = APPLESMC_ST_NEW_CMD | APPLESMC_ST_ACK;
> +        } else {
> +            smc_debug("ERROR: previous command interrupted!\n");
> +            s->status = APPLESMC_ST_NEW_CMD;
> +            s->status_1e = APPLESMC_ST_1E_CMD_INTRUPTED;
> +        }
>          break;
> +    default:
> +        smc_debug("UNEXPECTED CMD 0x%02x\n", (uint8_t)val);
> +        s->status = APPLESMC_ST_NEW_CMD;
> +        s->status_1e = APPLESMC_ST_1E_BAD_CMD;
>      }
> -    s->cmd = val;
>      s->read_pos = 0;
>      s->data_pos = 0;
>  }
>
> -static void applesmc_fill_data(AppleSMCState *s)
> +static struct AppleSMCData *applesmc_find_key(AppleSMCState *s)
>  {
>      struct AppleSMCData *d;
>
>      QLIST_FOREACH(d, &s->data_def, node) {
>          if (!memcmp(d->key, s->key, 4)) {
> -            smc_debug("Key matched (%s Len=%d Data=%s)\n", d->key,
> -                      d->len, d->data);
> -            memcpy(s->data, d->data, d->len);
> -            return;
> +            return d;
>          }
>      }
> +    return NULL;
>  }
>
>  static void applesmc_io_data_write(void *opaque, hwaddr addr, uint64_t val,
>                                     unsigned size)
>  {
>      AppleSMCState *s = opaque;
> +    struct AppleSMCData *d;
>
> -    smc_debug("DATA Write B: %#x = %#x\n", addr, val);
> +    smc_debug("DATA received: 0x%02x\n", (uint8_t)val);
>      switch (s->cmd) {
>      case APPLESMC_READ_CMD:
> +        if ((s->status & 0x0f) == APPLESMC_ST_CMD_DONE) {
> +            break;
> +        }
>          if (s->read_pos < 4) {
>              s->key[s->read_pos] = val;
> -            s->status = 0x04;
> +            s->status = APPLESMC_ST_ACK;
>          } else if (s->read_pos == 4) {
> -            s->data_len = val;
> -            s->status = 0x05;
> -            s->data_pos = 0;
> -            smc_debug("Key = %c%c%c%c Len = %d\n", s->key[0],
> -                      s->key[1], s->key[2], s->key[3], val);
> -            applesmc_fill_data(s);
> +            d = applesmc_find_key(s);
> +            if (d != NULL) {
> +                memcpy(s->data, d->data, d->len);
> +                s->data_len = d->len;
> +                s->data_pos = 0;
> +                s->status = APPLESMC_ST_ACK | APPLESMC_ST_DATA_READY;
> +                s->status_1e = APPLESMC_ST_CMD_DONE;  /* clear on valid key */
> +            } 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]);
> +                s->status = APPLESMC_ST_CMD_DONE;
> +                s->status_1e = APPLESMC_ST_1E_NOEXIST;
> +            }
>          }
>          s->read_pos++;
>          break;
> +    default:
> +        s->status = APPLESMC_ST_CMD_DONE;
> +        s->status_1e = APPLESMC_ST_1E_STILL_BAD_CMD;
>      }
>  }
>
> +static void applesmc_io_err_write(void *opaque, hwaddr addr, uint64_t val,
> +                                  unsigned size)
> +{
> +    smc_debug("ERR_CODE received: 0x%02x, ignoring!\n", (uint8_t)val);
> +    /* NOTE: writing to the error port not supported! */
> +}
> +
>  static uint64_t applesmc_io_data_read(void *opaque, hwaddr addr, unsigned size)
>  {
>      AppleSMCState *s = opaque;
> -    uint8_t retval = 0;
>
>      switch (s->cmd) {
>      case APPLESMC_READ_CMD:
> +        if (!(s->status & APPLESMC_ST_DATA_READY)) {
> +            break;
> +        }
>          if (s->data_pos < s->data_len) {
> -            retval = s->data[s->data_pos];
> -            smc_debug("READ_DATA[%d] = %#hhx\n", s->data_pos,
> -                      retval);
> +            s->last_ret = s->data[s->data_pos];
> +            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);
>              s->data_pos++;
>              if (s->data_pos == s->data_len) {
> -                s->status = 0x00;
> -                smc_debug("EOF\n");
> +                s->status = APPLESMC_ST_CMD_DONE;
> +                smc_debug("READ '%c%c%c%c' Len=%d complete!\n",
> +                          s->key[0], s->key[1], s->key[2], s->key[3],
> +                          s->data_len);
>              } else {
> -                s->status = 0x05;
> +                s->status = APPLESMC_ST_ACK | APPLESMC_ST_DATA_READY;
>              }
>          }
> +        break;
> +    default:
> +        s->status = APPLESMC_ST_CMD_DONE;
> +        s->status_1e = APPLESMC_ST_1E_STILL_BAD_CMD;
>      }
> -    smc_debug("DATA Read b: %#x = %#x\n", addr, retval);
> +    smc_debug("DATA sent: 0x%02x\n", s->last_ret);
>
> -    return retval;
> +    return s->last_ret;
>  }
>
>  static uint64_t applesmc_io_cmd_read(void *opaque, hwaddr addr, unsigned size)
>  {
>      AppleSMCState *s = opaque;
>
> -    smc_debug("CMD Read B: %#x\n", addr);
> +    smc_debug("CMD sent: 0x%02x\n", s->status);
>      return s->status;
>  }
>
> +static uint64_t applesmc_io_err_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    AppleSMCState *s = opaque;
> +
> +    /* NOTE: read does not clear the 1e status */
> +    smc_debug("ERR_CODE sent: 0x%02x\n", s->status_1e);
> +    return s->status_1e;
> +}
> +
>  static void applesmc_add_key(AppleSMCState *s, const char *key,
>                               int len, const char *data)
>  {
> @@ -198,6 +269,9 @@ static void qdev_applesmc_isa_reset(DeviceState *dev)
>      QLIST_FOREACH_SAFE(d, &s->data_def, node, next) {
>          QLIST_REMOVE(d, node);
>      }
> +    s->status = 0x00;
> +    s->status_1e = 0x00;
> +    s->last_ret = 0x00;
>
>      applesmc_add_key(s, "REV ", 6, "\x01\x13\x0f\x00\x00\x03");
>      applesmc_add_key(s, "OSK0", 32, s->osk);
> @@ -227,6 +301,16 @@ static const MemoryRegionOps applesmc_cmd_io_ops = {
>      },
>  };
>
> +static const MemoryRegionOps applesmc_err_io_ops = {
> +    .write = applesmc_io_err_write,
> +    .read = applesmc_io_err_read,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .impl = {
> +        .min_access_size = 1,
> +        .max_access_size = 1,
> +    },
> +};
> +
>  static void applesmc_isa_realize(DeviceState *dev, Error **errp)
>  {
>      AppleSMCState *s = APPLE_SMC(dev);
> @@ -241,6 +325,11 @@ static void applesmc_isa_realize(DeviceState *dev, Error **errp)
>      isa_register_ioport(&s->parent_obj, &s->io_cmd,
>                          s->iobase + APPLESMC_CMD_PORT);
>
> +    memory_region_init_io(&s->io_err, OBJECT(s), &applesmc_err_io_ops, s,
> +                          "applesmc-err", 1);
> +    isa_register_ioport(&s->parent_obj, &s->io_err,
> +                        s->iobase + APPLESMC_ERR_PORT);
> +
>      if (!s->osk || (strlen(s->osk) != 64)) {
>          fprintf(stderr, "WARNING: Using AppleSMC with invalid key\n");
>          s->osk = default_osk;
>