All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm <qemu-arm@nongnu.org>,
	QEMU Developers <qemu-devel@nongnu.org>,
	Patch Tracking <patches@linaro.org>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: Re: [Qemu-arm] [Qemu-devel] [PATCH] hw/arm/virt: Assume EL3 boot rom will handle PSCI if one is provided
Date: Mon, 22 Feb 2016 11:12:09 +0100	[thread overview]
Message-ID: <56CADEF9.3010103@redhat.com> (raw)
In-Reply-To: <CAFEAcA-nzx2JwaC64NfBm2bpWURxezwmZoC+HOXmhrBzWptZqw@mail.gmail.com>

On 02/20/16 12:31, Peter Maydell wrote:
> On 19 February 2016 at 21:58, Laszlo Ersek <lersek@redhat.com> wrote:
>> On 02/19/16 22:41, Ard Biesheuvel wrote:
>>> On 19 February 2016 at 22:03, Laszlo Ersek <lersek@redhat.com> 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
> 


      reply	other threads:[~2016-02-22 10:12 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-19 16:21 [Qemu-arm] [PATCH] hw/arm/virt: Assume EL3 boot rom will handle PSCI if one is provided Peter Maydell
2016-02-19 21:03 ` [Qemu-arm] [Qemu-devel] " Laszlo Ersek
2016-02-19 21:41   ` Ard Biesheuvel
2016-02-19 21:58     ` Laszlo Ersek
2016-02-20 11:31       ` Peter Maydell
2016-02-22 10:12         ` Laszlo Ersek [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56CADEF9.3010103@redhat.com \
    --to=lersek@redhat.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=patches@linaro.org \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.