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

[Paolo Bonzini]

On 02/10/21 22:24, Pedro Tôrres wrote:
> #define APPLESMC_DEFAULT_IOBASE        0x300
> @@ -332,7 +497,27 @@ static void applesmc_isa_realize(DeviceState *dev, Error 
> **errp)
>                          s->iobase + APPLESMC_ERR_PORT);
>
>      if (!s->osk || (strlen(s->osk) != 64)) {
> +#if defined(__APPLE__) && defined(__MACH__)
> +        IOReturn ret;
> +        IOByteCount size = 32;
> +
> +        ret = smc_read_key('OSK0', (uint8_t *) default_osk, &size);
> +        if (ret != kIOReturnSuccess) {
> +            goto failure;
> +        }
> +
> +        ret = smc_read_key('OSK1', (uint8_t *) default_osk + size, &size);
> +        if (ret != kIOReturnSuccess) {
> +            goto failure;
> +        }
> +
> +        warn_report("Using AppleSMC with host key");
> +        goto success;
> +#endif
> +failure:
>          warn_report("Using AppleSMC with invalid key");
> +
> +success:
>          s->osk = default_osk;
>      }
>
> --

I think it is incorrect to use host key if strlen(s->osk) != 64.  So I
would change this code to something like this:

@@ -315,6 +480,7 @@ static const MemoryRegionOps applesmc_err_io_ops = {
 static void applesmc_isa_realize(DeviceState *dev, Error **errp)
 {
     AppleSMCState *s = APPLE_SMC(dev);
+    bool valid_key = false;
 
     memory_region_init_io(&s->io_data, OBJECT(s), &applesmc_data_io_ops, s,
                           "applesmc-data", 1);
@@ -331,7 +497,31 @@ 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)) {
+    if (s->osk) {
+        valid_key = strlen(s->osk) == 64;
+    } else {
+#if defined(__APPLE__) && defined(__MACH__)
+        IOReturn ret;
+        IOByteCount size = 32;
+
+        ret = smc_read_key('OSK0', (uint8_t *) default_osk, &size);
+        if (ret != kIOReturnSuccess) {
+            goto failure;
+        }
+
+        ret = smc_read_key('OSK1', (uint8_t *) default_osk + size, &size);
+        if (ret != kIOReturnSuccess) {
+            goto failure;
+        }
+
+        warn_report("Using AppleSMC with host key");
+        valid_key = true;
+        s->osk = default_osk;
+failure:
+#endif
+    }
+
+    if (!valid_key) {
         warn_report("Using AppleSMC with invalid key");
         s->osk = default_osk;
     }

Otherwise looks good, so I queued it (haven't yet compile-tested it, but I
will before sending out my pull request).

Thanks,

Paolo