[PATCH v3] hw/misc: applesmc: use host osk as default on macs

[PATCH v3] hw/misc: applesmc: use host osk as default on macs

[Pedro Tôrres]

From: Pedro Tôrres <t0rr3sp3dr0@gmail.com>

When running on a Mac, QEMU is able to get the host OSK and use it as
the default value for the AppleSMC device. The OSK query operation
doesn't require administrator privileges and can be executed by any user
on the system. This patch is based on Phil Dennis-Jordan's description
of the process for reading OSK from SCM on macOS:
https://lists.nongnu.org/archive/html/qemu-devel/2021-10/msg02843.html

In the future, this could also be extended to work on Linux and Windows
when running on Macs. Just implement the applesmc_read_osk function for
those platforms.

The Apple SMC driver for Linux is currently being rewritten by Hector
Martin as part of the effort to bring Linux to Macs with Apple Silicon
(Asahi Linux). When the new driver gets merged to the Linux Kernel, it
will be a good time to extend this to work with it.

Signed-off-by: Pedro Tôrres <t0rr3sp3dr0@gmail.com>
---
 hw/misc/applesmc.c | 75 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 73 insertions(+), 2 deletions(-)

diff --git a/hw/misc/applesmc.c b/hw/misc/applesmc.c
index 81cd6b6423..c95e038bd2 100644
--- a/hw/misc/applesmc.c
+++ b/hw/misc/applesmc.c
@@ -5,6 +5,7 @@
  *
  *  Authors: Alexander Graf <agraf@suse.de>
  *           Susanne Graf <suse@csgraf.de>
+ *           Pedro Tôrres <t0rr3sp3dr0@gmail.com>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -28,8 +29,16 @@
  * This driver was mostly created by looking at the Linux AppleSMC driver
  * implementation and does not support IRQ.
  *
+ * Reading OSK from SCM on macOS was implemented based on Phil Dennis-Jordan's
+ * description of the process:
+ * https://lists.nongnu.org/archive/html/qemu-devel/2021-10/msg02843.html
+ *
  */
 
+#if defined(__APPLE__) && defined(__MACH__)
+#include <IOKit/IOKitLib.h>
+#endif
+
 #include "qemu/osdep.h"
 #include "hw/isa/isa.h"
 #include "hw/qdev-properties.h"
@@ -312,9 +321,62 @@ static const MemoryRegionOps applesmc_err_io_ops = {
     },
 };
 
+static bool applesmc_read_osk(uint8_t *osk)
+{
+#if defined(__APPLE__) && defined(__MACH__)
+    struct AppleSMCParams {
+        uint32_t key;
+        uint8_t __pad0[16];
+        uint8_t result;
+        uint8_t __pad1[7];
+        uint32_t size;
+        uint8_t __pad2[10];
+        uint8_t data8;
+        uint8_t __pad3[5];
+        uint8_t output[32];
+    };
+
+    io_service_t svc;
+    io_connect_t conn;
+    kern_return_t ret;
+    size_t size = sizeof(struct AppleSMCParams);
+    struct AppleSMCParams params_in = { .size = 32, .data8 = 5 };
+    struct AppleSMCParams params_out = {};
+
+    svc = IOServiceGetMatchingService(0, IOServiceMatching("AppleSMC"));
+    if (svc == 0) {
+        return false;
+    }
+
+    ret = IOServiceOpen(svc, mach_task_self(), 0, &conn);
+    if (ret != 0) {
+        return false;
+    }
+
+    for (params_in.key = 'OSK0'; params_in.key <= 'OSK1'; ++params_in.key) {
+        ret = IOConnectCallStructMethod(conn, 2, &params_in, size, &params_out, &size);
+        if (ret != 0) {
+            return false;
+        }
+
+        if (params_out.result != 0) {
+            return false;
+        }
+        memcpy(osk, params_out.output, params_in.size);
+
+        osk += params_in.size;
+    }
+
+    return true;
+#else
+    return false;
+#endif
+}
+
 static void applesmc_isa_realize(DeviceState *dev, Error **errp)
 {
     AppleSMCState *s = APPLE_SMC(dev);
+    bool valid_osk = false;
 
     memory_region_init_io(&s->io_data, OBJECT(s), &applesmc_data_io_ops, s,
                           "applesmc-data", 1);
@@ -331,8 +393,17 @@ static void applesmc_isa_realize(DeviceState *dev, Error **errp)
     isa_register_ioport(&s->parent_obj, &s->io_err,
                         s->iobase + APPLESMC_ERR_PORT);
 
-    if (!s->osk || (strlen(s->osk) != 64)) {
-        warn_report("Using AppleSMC with invalid key");
+    if (s->osk) {
+        valid_osk = strlen(s->osk) == 64;
+    } else {
+        valid_osk = applesmc_read_osk((uint8_t *) default_osk);
+        if (valid_osk) {
+            warn_report("Using AppleSMC with host OSK");
+            s->osk = default_osk;
+        }
+    }
+    if (!valid_osk) {
+        warn_report("Using AppleSMC with invalid OSK");
         s->osk = default_osk;
     }
 
-- 
2.32.0 (Apple Git-132)

Re: [PATCH v3] hw/misc: applesmc: use host osk as default on macs

[Vladislav Yaroshchuk]

Hi Pedro Torres,

Please note this threads
https://patchew.org/QEMU/20211022161448.81579-1-yaroshchuk2000@gmail.com/
https://patchew.org/QEMU/20220113152836.60398-1-yaroshchuk2000@gmail.com/

There was a discussion about how to preserve
backward compatibility of emulated AppleSMC
behaviour. The discussion has stopped, but
if you're intended to see this feature working
within QEMU - it'd be good to cc all the
participans of previous threads :)


Thanks,

Vladislav Yaroshchuk 

Re: [PATCH v3] hw/misc: applesmc: use host osk as default on macs

[Vladislav Yaroshchuk]

[-- Attachment #1: Type: text/plain, Size: 4608 bytes --]

I've CCed all the people from previous threads.


> [...]
> +static bool applesmc_read_osk(uint8_t *osk)
> +{
> +#if defined(__APPLE__) && defined(__MACH__)
> +    struct AppleSMCParams {
> +        uint32_t key;
> +        uint8_t __pad0[16];
> +        uint8_t result;
> +        uint8_t __pad1[7];
> +        uint32_t size;
> +        uint8_t __pad2[10];
> +        uint8_t data8;
> +        uint8_t __pad3[5];
> +        uint8_t output[32];
> +    };
> +
> +    io_service_t svc;
> +    io_connect_t conn;
> +    kern_return_t ret;
> +    size_t size = sizeof(struct AppleSMCParams);
> +    struct AppleSMCParams params_in = { .size = 32, .data8 = 5 };

Maybe it's better to name this magic number '5'

> +    struct AppleSMCParams params_out = {};
> +

params_in and params_out can be the same variable, see
https://patchew.org/QEMU/20211022161448.81579-1-yaroshchuk2000@gmail.com/

> +    svc = IOServiceGetMatchingService(0, IOServiceMatching("AppleSMC"));
> +    if (svc == 0) {
> +        return false;
> +    }
> +
> +    ret = IOServiceOpen(svc, mach_task_self(), 0, &conn);
> +    if (ret != 0) {
> +        return false;
> +    }
> +
> +    for (params_in.key = 'OSK0'; params_in.key <= 'OSK1';
++params_in.key) {
> +        ret = IOConnectCallStructMethod(conn, 2, &params_in, size,
&params_out, &size);

Same about this magic number '2'.

> +        if (ret != 0) {
> +            return false;
> +        }
> +
> +        if (params_out.result != 0) {
> +            return false;
> +        }
> +        memcpy(osk, params_out.output, params_in.size);
> +
> +        osk += params_in.size;
> +     }
> +

Cleanup IOServiceClose and IOObjectReturn are not called at the
end of the procedure.

This is also mentioned in Phil Dennis-Jordan's instruction you noted (stage
5):
https://lists.nongnu.org/archive/html/qemu-devel/2021-10/msg02843.html

> +    return true;
> +#else
> +    return false;
> +#endif
> +}
> +
> [...]
>
> static void applesmc_isa_realize(DeviceState *dev, Error **errp)
>  {
>      AppleSMCState *s = APPLE_SMC(dev);
> +    bool valid_osk = false;
>
>      memory_region_init_io(&s->io_data, OBJECT(s), &applesmc_data_io_ops,
s,
>                            "applesmc-data", 1);
> @@ -331,8 +393,17 @@ static void applesmc_isa_realize(DeviceState *dev,
Error **errp)
>      isa_register_ioport(&s->parent_obj, &s->io_err,
>                          s->iobase + APPLESMC_ERR_PORT);
>
> -    if (!s->osk || (strlen(s->osk) != 64)) {
> -        warn_report("Using AppleSMC with invalid key");
> +    if (s->osk) {
> +        valid_osk = strlen(s->osk) == 64;
> +    } else {
> +        valid_osk = applesmc_read_osk((uint8_t *) default_osk);
> +        if (valid_osk) {
> +            warn_report("Using AppleSMC with host OSK");
> +            s->osk = default_osk;
> +        }
> +    }
> +    if (!valid_osk) {
> +        warn_report("Using AppleSMC with invalid OSK");
>          s->osk = default_osk;
>      }
> [...]

After the previous discussion we've decided (if i don't confuse anything)
to have a way to enable/disable host OSK reading with new property:
1. properties osk=$key and hostosk=on cannot be used together (fail hard)
2. for QEMU machine > 7.0 - hostosk=on by default.
    If unable to read - fail hard with error_setg.
3. for QEMU machine <= 7.0 - hostosk=off by default,
   the dummy OSK is used (as previously).

BTW since my patches lost 7.0, I planned to wait until compat machines
for 7.1 are added (after 7.0 release) and then rebase the patches,
adding required changes into `hw/core/machine.c`

Now we have two versions of host OSK forwarding implementations,
Pedro's (this one) and mine (
https://patchew.org/QEMU/20220113152836.60398-1-yaroshchuk2000@gmail.com/#)

Do we still want to add this feature? If yes - whose version is preferred?
(I'm still ready to work on this)

Regards,
Vlad

вс, 17 апр. 2022 г. в 04:36, Vladislav Yaroshchuk <yaroshchuk2000@gmail.com
>:

> Hi Pedro Torres,
>
> Please note this threads
> https://patchew.org/QEMU/20211022161448.81579-1-yaroshchuk2000@gmail.com/
> https://patchew.org/QEMU/20220113152836.60398-1-yaroshchuk2000@gmail.com/
>
> There was a discussion about how to preserve
> backward compatibility of emulated AppleSMC
> behaviour. The discussion has stopped, but
> if you're intended to see this feature working
> within QEMU - it'd be good to cc all the
> participans of previous threads :)
>
>
> Thanks,
>
> Vladislav Yaroshchuk
>
>

[-- Attachment #2: Type: text/html, Size: 6425 bytes --]

Re: [PATCH v3] hw/misc: applesmc: use host osk as default on macs

[Daniel P. Berrangé]

On Sun, Apr 17, 2022 at 04:43:14PM +0300, Vladislav Yaroshchuk wrote:
> I've CCed all the people from previous threads.
> 
> 
> > [...]
> > +static bool applesmc_read_osk(uint8_t *osk)
> > +{
> > +#if defined(__APPLE__) && defined(__MACH__)
> > +    struct AppleSMCParams {
> > +        uint32_t key;
> > +        uint8_t __pad0[16];
> > +        uint8_t result;
> > +        uint8_t __pad1[7];
> > +        uint32_t size;
> > +        uint8_t __pad2[10];
> > +        uint8_t data8;
> > +        uint8_t __pad3[5];
> > +        uint8_t output[32];
> > +    };
> > +
> > +    io_service_t svc;
> > +    io_connect_t conn;
> > +    kern_return_t ret;
> > +    size_t size = sizeof(struct AppleSMCParams);
> > +    struct AppleSMCParams params_in = { .size = 32, .data8 = 5 };
> 
> Maybe it's better to name this magic number '5'
> 
> > +    struct AppleSMCParams params_out = {};
> > +
> 
> params_in and params_out can be the same variable, see
> https://patchew.org/QEMU/20211022161448.81579-1-yaroshchuk2000@gmail.com/
> 
> > +    svc = IOServiceGetMatchingService(0, IOServiceMatching("AppleSMC"));
> > +    if (svc == 0) {
> > +        return false;
> > +    }
> > +
> > +    ret = IOServiceOpen(svc, mach_task_self(), 0, &conn);
> > +    if (ret != 0) {
> > +        return false;
> > +    }
> > +
> > +    for (params_in.key = 'OSK0'; params_in.key <= 'OSK1';
> ++params_in.key) {
> > +        ret = IOConnectCallStructMethod(conn, 2, &params_in, size,
> &params_out, &size);
> 
> Same about this magic number '2'.
> 
> > +        if (ret != 0) {
> > +            return false;
> > +        }
> > +
> > +        if (params_out.result != 0) {
> > +            return false;
> > +        }
> > +        memcpy(osk, params_out.output, params_in.size);
> > +
> > +        osk += params_in.size;
> > +     }
> > +
> 
> Cleanup IOServiceClose and IOObjectReturn are not called at the
> end of the procedure.
> 
> This is also mentioned in Phil Dennis-Jordan's instruction you noted (stage
> 5):
> https://lists.nongnu.org/archive/html/qemu-devel/2021-10/msg02843.html
> 
> > +    return true;
> > +#else
> > +    return false;
> > +#endif
> > +}
> > +
> > [...]
> >
> > static void applesmc_isa_realize(DeviceState *dev, Error **errp)
> >  {
> >      AppleSMCState *s = APPLE_SMC(dev);
> > +    bool valid_osk = false;
> >
> >      memory_region_init_io(&s->io_data, OBJECT(s), &applesmc_data_io_ops,
> s,
> >                            "applesmc-data", 1);
> > @@ -331,8 +393,17 @@ static void applesmc_isa_realize(DeviceState *dev,
> Error **errp)
> >      isa_register_ioport(&s->parent_obj, &s->io_err,
> >                          s->iobase + APPLESMC_ERR_PORT);
> >
> > -    if (!s->osk || (strlen(s->osk) != 64)) {
> > -        warn_report("Using AppleSMC with invalid key");
> > +    if (s->osk) {
> > +        valid_osk = strlen(s->osk) == 64;
> > +    } else {
> > +        valid_osk = applesmc_read_osk((uint8_t *) default_osk);
> > +        if (valid_osk) {
> > +            warn_report("Using AppleSMC with host OSK");
> > +            s->osk = default_osk;
> > +        }
> > +    }
> > +    if (!valid_osk) {
> > +        warn_report("Using AppleSMC with invalid OSK");
> >          s->osk = default_osk;
> >      }
> > [...]
> 
> After the previous discussion we've decided (if i don't confuse anything)
> to have a way to enable/disable host OSK reading with new property:
> 1. properties osk=$key and hostosk=on cannot be used together (fail hard)
> 2. for QEMU machine > 7.0 - hostosk=on by default.
>     If unable to read - fail hard with error_setg.
> 3. for QEMU machine <= 7.0 - hostosk=off by default,
>    the dummy OSK is used (as previously).
> 
> BTW since my patches lost 7.0, I planned to wait until compat machines
> for 7.1 are added (after 7.0 release) and then rebase the patches,
> adding required changes into `hw/core/machine.c`
> 
> Now we have two versions of host OSK forwarding implementations,
> Pedro's (this one) and mine (
> https://patchew.org/QEMU/20220113152836.60398-1-yaroshchuk2000@gmail.com/#)
> 
> Do we still want to add this feature? If yes - whose version is preferred?
> (I'm still ready to work on this)

I prefer yours, since the feature is introspectable by mgmt apps,
given the existance of the 'hostosk' property

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v3] hw/misc: applesmc: use host osk as default on macs

[Vladislav Yaroshchuk]

[-- Attachment #1: Type: text/plain, Size: 5186 bytes --]

Hi, Daniel!

Ok, thank you - then I'll wait until compat machines
for 7.1 are added (after release) and send a new
patch.

Regards,
Vladislav

вт, 19 апр. 2022 г. в 19:03, Daniel P. Berrangé <berrange@redhat.com>:

> On Sun, Apr 17, 2022 at 04:43:14PM +0300, Vladislav Yaroshchuk wrote:
> > I've CCed all the people from previous threads.
> >
> >
> > > [...]
> > > +static bool applesmc_read_osk(uint8_t *osk)
> > > +{
> > > +#if defined(__APPLE__) && defined(__MACH__)
> > > +    struct AppleSMCParams {
> > > +        uint32_t key;
> > > +        uint8_t __pad0[16];
> > > +        uint8_t result;
> > > +        uint8_t __pad1[7];
> > > +        uint32_t size;
> > > +        uint8_t __pad2[10];
> > > +        uint8_t data8;
> > > +        uint8_t __pad3[5];
> > > +        uint8_t output[32];
> > > +    };
> > > +
> > > +    io_service_t svc;
> > > +    io_connect_t conn;
> > > +    kern_return_t ret;
> > > +    size_t size = sizeof(struct AppleSMCParams);
> > > +    struct AppleSMCParams params_in = { .size = 32, .data8 = 5 };
> >
> > Maybe it's better to name this magic number '5'
> >
> > > +    struct AppleSMCParams params_out = {};
> > > +
> >
> > params_in and params_out can be the same variable, see
> >
> https://patchew.org/QEMU/20211022161448.81579-1-yaroshchuk2000@gmail.com/
> >
> > > +    svc = IOServiceGetMatchingService(0,
> IOServiceMatching("AppleSMC"));
> > > +    if (svc == 0) {
> > > +        return false;
> > > +    }
> > > +
> > > +    ret = IOServiceOpen(svc, mach_task_self(), 0, &conn);
> > > +    if (ret != 0) {
> > > +        return false;
> > > +    }
> > > +
> > > +    for (params_in.key = 'OSK0'; params_in.key <= 'OSK1';
> > ++params_in.key) {
> > > +        ret = IOConnectCallStructMethod(conn, 2, &params_in, size,
> > &params_out, &size);
> >
> > Same about this magic number '2'.
> >
> > > +        if (ret != 0) {
> > > +            return false;
> > > +        }
> > > +
> > > +        if (params_out.result != 0) {
> > > +            return false;
> > > +        }
> > > +        memcpy(osk, params_out.output, params_in.size);
> > > +
> > > +        osk += params_in.size;
> > > +     }
> > > +
> >
> > Cleanup IOServiceClose and IOObjectReturn are not called at the
> > end of the procedure.
> >
> > This is also mentioned in Phil Dennis-Jordan's instruction you noted
> (stage
> > 5):
> > https://lists.nongnu.org/archive/html/qemu-devel/2021-10/msg02843.html
> >
> > > +    return true;
> > > +#else
> > > +    return false;
> > > +#endif
> > > +}
> > > +
> > > [...]
> > >
> > > static void applesmc_isa_realize(DeviceState *dev, Error **errp)
> > >  {
> > >      AppleSMCState *s = APPLE_SMC(dev);
> > > +    bool valid_osk = false;
> > >
> > >      memory_region_init_io(&s->io_data, OBJECT(s),
> &applesmc_data_io_ops,
> > s,
> > >                            "applesmc-data", 1);
> > > @@ -331,8 +393,17 @@ static void applesmc_isa_realize(DeviceState *dev,
> > Error **errp)
> > >      isa_register_ioport(&s->parent_obj, &s->io_err,
> > >                          s->iobase + APPLESMC_ERR_PORT);
> > >
> > > -    if (!s->osk || (strlen(s->osk) != 64)) {
> > > -        warn_report("Using AppleSMC with invalid key");
> > > +    if (s->osk) {
> > > +        valid_osk = strlen(s->osk) == 64;
> > > +    } else {
> > > +        valid_osk = applesmc_read_osk((uint8_t *) default_osk);
> > > +        if (valid_osk) {
> > > +            warn_report("Using AppleSMC with host OSK");
> > > +            s->osk = default_osk;
> > > +        }
> > > +    }
> > > +    if (!valid_osk) {
> > > +        warn_report("Using AppleSMC with invalid OSK");
> > >          s->osk = default_osk;
> > >      }
> > > [...]
> >
> > After the previous discussion we've decided (if i don't confuse anything)
> > to have a way to enable/disable host OSK reading with new property:
> > 1. properties osk=$key and hostosk=on cannot be used together (fail hard)
> > 2. for QEMU machine > 7.0 - hostosk=on by default.
> >     If unable to read - fail hard with error_setg.
> > 3. for QEMU machine <= 7.0 - hostosk=off by default,
> >    the dummy OSK is used (as previously).
> >
> > BTW since my patches lost 7.0, I planned to wait until compat machines
> > for 7.1 are added (after 7.0 release) and then rebase the patches,
> > adding required changes into `hw/core/machine.c`
> >
> > Now we have two versions of host OSK forwarding implementations,
> > Pedro's (this one) and mine (
> >
> https://patchew.org/QEMU/20220113152836.60398-1-yaroshchuk2000@gmail.com/#
> )
> >
> > Do we still want to add this feature? If yes - whose version is
> preferred?
> > (I'm still ready to work on this)
>
> I prefer yours, since the feature is introspectable by mgmt apps,
> given the existance of the 'hostosk' property
>
> With regards,
> Daniel
> --
> |: https://berrange.com      -o-
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-
> https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-
> https://www.instagram.com/dberrange :|
>
>

[-- Attachment #2: Type: text/html, Size: 7654 bytes --]