Re: [Qemu-arm] [Qemu-devel] [PATCH] hw/arm/virt: Assume EL3 boot rom will handle PSCI if one is provided

[Laszlo Ersek]

On 02/20/16 12:31, Peter Maydell wrote:
> On 19 February 2016 at 21:58, Laszlo Ersek <address@hidden> wrote:
>> On 02/19/16 22:41, Ard Biesheuvel wrote:
>>> On 19 February 2016 at 22:03, Laszlo Ersek <address@hidden> wrote:
>>>> Ard, any opinion on this?
>>>>
>>>
>>> I agree with Peter. Note that this is strictly about emulation, under
>>> KVM we always run at EL1 or below and PSCI calls are handled by the
>>> host kernel, not QEMU
>>
>> Great, that's all I wanted to hear -- out of scope for me. :)
>>
>> Actually, I have now read the patch even, and I have the following comments:
>>
>> - As long as "using_psci" is true, the behavior doesn't change. Great.
>>
>> - The only place where using_psci *changes* to false is reachable only
>> with (vms->secure && firmware_loaded). That's what wasn't immediately
>> obvious from the patch -- when vms->secure is true (-machine secure=on),
>> it's out of scope for me. :)
>>
>> - However, I think I might have noticed a bug -- or rather missed
>> something trivial --, namely, where is "using_psci" *initially* set to
>> true? The "machines" static array is not touched.
> 
> Derp. I changed at the last minute from having using_psci be a
> local bool in the machvirt_init() function to putting it in the
> vbi struct, and forgot that when I deleted the "bool using_psci = true;"

Independently, I think that may be why some projects (edk2 for example,
and perhaps even some of the BSDs?) forbid initialization of auto
variables in their coding styles. Could be a little heavy-handed, so I'm
certainly not suggesting that QEMU adopt that rule; it's just
interesting to see that it really occurs sometimes.

Thanks!
Laszlo

> I needed to add something to init vbi->using_psci to true.
> 
> thanks
> -- PMM
>