Re: [PATCH qemu v2] spapr: Kill SLOF

[Cédric Le Goater]

>>>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>>>> index e62c89b3dd40..1c97534a0aea 100644
>>>>> --- a/hw/ppc/spapr.c
>>>>> +++ b/hw/ppc/spapr.c
>>>>> @@ -896,6 +896,51 @@ out:
>>>>>       return ret;
>>>>>   }
>>>>>   +/*
>>>>> + * Below is a compiled version of RTAS blob and OF client interface 
>>>>> entry point.
>>>>> + *
>>>>> + * gcc -nostdlib  -mbig -o spapr-rtas.img spapr-rtas.S
>>>>> + * objcopy  -O binary -j .text  spapr-rtas.img spapr-rtas.bin
>>>>> + *
>>>>> + *   .globl  _start
>>>>> + *   _start:
>>>>> + *           mr      4,3
>>>>> + *           lis     3,KVMPPC_H_RTAS@h
>>>>> + *           ori     3,3,KVMPPC_H_RTAS@l
>>>>> + *           sc      1
>>>>> + *           blr
>>>>> + *           mr      4,3
>>>>> + *           lis     3,KVMPPC_H_CLIENT@h
>>>>> + *           ori     3,3,KVMPPC_H_CLIENT@l
>>>>> + *           sc      1
>>>>> + *           blr
>>>>> + */
>>>>> +static struct {
>>>>> +    uint8_t rtas[20], client[20];
>>>>> +} QEMU_PACKED rtas_client_blob = {
>>>>> +    .rtas = {
>>>>> +        0x7c, 0x64, 0x1b, 0x78,
>>>>> +        0x3c, 0x60, 0x00, 0x00,
>>>>> +        0x60, 0x63, 0xf0, 0x00,
>>>>> +        0x44, 0x00, 0x00, 0x22,
>>>>> +        0x4e, 0x80, 0x00, 0x20
>>>>> +    },
>>>>> +    .client = {
>>>>> +        0x7c, 0x64, 0x1b, 0x78,
>>>>> +        0x3c, 0x60, 0x00, 0x00,
>>>>> +        0x60, 0x63, 0xf0, 0x05,
>>>>> +        0x44, 0x00, 0x00, 0x22,
>>>>> +        0x4e, 0x80, 0x00, 0x20
>>>>> +    }
>>>>> +};
>>>>
>>>> I'd really prefer to read this in from a file (or files) which we
>>>> assemble (as with the existing spapr-rtas.img), rather than having a
>>>> magic array in qemu.
>>>
>>> Two considerations here:
>>> 1. This blob is not going to change (at least often)
>>
>> True, but I'd still prefer to build it from a .S rather than have the
>> binary written out.  We already had this until recently (when we moved
>> the RTAS image into SLOF), and it's really not too hard to manage.
> 
> 
> But what is exactly the benefit? It is not going to change. I can look into 
> making it a .S, compile it to a binary an include this binary into 
> qemu-system-ppc64 as a byte array, just need to refresh my gcc/ldd memories 
> but a separate file is too much for this imho.

ARM bootloaders are also embedded in QEMU's code. See hw/arm/boot.c.
You could improve a bit the definition though.

C.

> 
>>> 2. A new file which needs to be packaged/signed/copied.
>>
>> Eh, again, we had this for RTAS until very recently and it really
>> wasn't that much hassle.
> 
> 
> I keep secure VMs in mind. And SLOF is a separate package, why was not RTAS?