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

[Alexander Graf]

On 25.10.21 16:21, 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,osk=host`
>
> 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.
>
> Enable this feature by default on Apple devices
>
> Based on 
> https://web.archive.org/web/20200103161737/osxbook.com/book/bonus/chapter7/tpmdrmmyth/
>
> Signed-off-by: Vladislav Yaroshchuk <yaroshchuk2000@gmail.com>
> ---
>   hw/misc/applesmc.c | 121 +++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 121 insertions(+)
>
> diff --git a/hw/misc/applesmc.c b/hw/misc/applesmc.c
> index 1b9acaf1d3..6bd2584ca0 100644
> --- a/hw/misc/applesmc.c
> +++ b/hw/misc/applesmc.c
> @@ -37,6 +37,11 @@
>   #include "qemu/module.h"
>   #include "qemu/timer.h"
>   #include "qom/object.h"
> +#include "qapi/error.h"
> +
> +#if defined(__APPLE__) && defined(__MACH__)
> +#include <IOKit/IOKitLib.h>
> +#endif
>   
>   /* #define DEBUG_SMC */
>   
> @@ -312,9 +317,106 @@ static const MemoryRegionOps applesmc_err_io_ops = {
>       },
>   };
>   
> +#if defined(__APPLE__) && defined(__MACH__)
> +/*
> + * Based on
> + * 
> https://web.archive.org/web/20200103161737/osxbook.com/book/bonus/chapter7/tpmdrmmyth/
> + */
> +enum {
> +    SMC_HANDLE_EVENT     = 2,
> +    SMC_READ_KEY         = 5


Anonymous enums are not great. Would you mind to just make them 
#define's instead?


> +};
> +
> +struct AppleSMCParam {
> +    uint32_t    key;
> +    uint8_t     pad0[22];
> +    IOByteCount data_size;
> +    uint8_t     pad1[10];
> +    uint8_t     command;
> +    uint32_t    pad2;
> +    uint8_t     bytes[32];
> +};
> +
> +static bool applesmc_read_host_osk(char *host_osk, Error **errp)
> +{
> +    assert(host_osk != NULL);
> +
> +    io_service_t    hostsmc_service = IO_OBJECT_NULL;
> +    io_connect_t    hostsmc_connect = IO_OBJECT_NULL;
> +    size_t          smc_param_size = sizeof(struct AppleSMCParam);
> +    IOReturn        status = kIOReturnError;
> +    int             i;


No need to align the variable names. If anything, better follow the 
Reverse Christmas Tree model :)

Also, let's wait for v6 until Daniel and me agreed on the parameter.


Alex