Re: [PATCH v4] isa-applesmc: provide OSK forwarding on Apple hosts

[Alexander Graf <agraf@csgraf.de>]

On 25.10.21 16:47, Daniel P. Berrangé wrote:
> On Mon, Oct 25, 2021 at 04:42:22PM +0200, Alexander Graf wrote:
>> On 25.10.21 16:22, Daniel P. Berrangé wrote:
>>> On Mon, Oct 25, 2021 at 12:13:32PM +0200, Alexander Graf wrote:
>>>> On 22.10.21 18:14, Vladislav Yaroshchuk wrote:
>>>>> On Apple hosts we can read AppleSMC OSK key directly from host's
>>>>> SMC and forward this value to QEMU Guest.
>>>>>
>>>>> Usage:
>>>>> `-device isa-applesmc,hostosk=on`
>>>>>
>>>>> Apple licence allows use and run up to two additional copies
>>>>> or instances of macOS operating within virtual operating system
>>>>> environments on each Apple-branded computer that is already running
>>>>> the Apple Software, for purposes of:
>>>>> - software development
>>>>> - testing during software development
>>>>> - using macOS Server
>>>>> - personal, non-commercial use
>>>>>
>>>>> Guest macOS requires AppleSMC with correct OSK. The most legal
>>>>> way to pass it to the Guest is to forward the key from host SMC
>>>>> without any value exposion.
>>>>>
>>>>> Based on https://web.archive.org/web/20200103161737/osxbook.com/book/bonus/chapter7/tpmdrmmyth/
>>>>>
>>>>> Signed-off-by: Vladislav Yaroshchuk <yaroshchuk2000@gmail.com>
>>>>> @@ -331,6 +464,25 @@ 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->hostosk_flag) {
>>>>> +        /*
>>>>> +         * Property 'hostosk' has higher priority than 'osk'
>>>>> +         * and shadows it.
>>>>> +         * Free user-provided 'osk' property value
>>>>> +         */
>>>>> +        if (s->osk) {
>>>>> +            warn_report("isa-applesmc.osk is shadowed "
>>>>> +                        "by isa-applesmc.hostosk");
>>>>> +            g_free(s->osk);
>>>>> +        }
>>>>> +
>>>>> +        if (!applesmc_read_host_osk(&s->osk, &err)) {
>>>>> +            /* On host OSK retrieval error report a warning */
>>>>> +            error_report_err(err);
>>>>> +            s->osk = default_osk;
>>>>> +        }
>>>>> +    }
>>>> This part is yucky. A few things:
>>>>
>>>> 1) QEMU in general does not fail user requested operations silently. If the
>>>> user explicitly asked to read the host OSK and we couldn't, it must
>>>> propagate that error.
>>>> 2) In tandem to the above, I think the only consistent CX is to make both
>>>> options mutually exclusive. The easiest way to achieve that IMHO would be to
>>>> overload the "osk" property. If it is "host", then use the host one.
>>>> 3) Should we make "osk"="host" the default on macOS as well then? Of course,
>>>> that one should *not* fail hard when it can't read the key, because it's an
>>>> implicit request rather than an explicit one.
>>> The problem with using a magic string value for the existing "osk"
>>> parameter is that this is not introspectable by management apps.
>>
>> What introspectability would you like to have?
> Essentially to answer the question
>
>    "Does this QEMU support OSK passthrough from the host"
>
> Mgmt apps like libvirt introspect using various query-XXX QMP commands.
> For devices, the typical approach is to ask for the list of properties
> the device supports. If we're just accepting a new magic value on an
> existing property there is no way to query for existance of that feature.
> If we add a "host-osk=bool" parameter introspectability is trivially
> satisfied.


Ok, the only flow that remains sensible in that case to me sounds like 
the following:

if (s->osk) {
     /* Use osk */
} else if (s->use_host_osk) {
     /* Use host OSK. Fail hard if we can't find it */
} else if (can_use_host_osk) {
     /* See if we can extract the key from the host. If not, fall back 
to old behavior */
} else {
     /* Old fallback behavior */
}


Alex