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

[Paolo Bonzini <pbonzini@redhat.com>]

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