All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexander Graf <agraf@csgraf.de>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: "Marc Zyngier" <maz@kernel.org>,
	"Eduardo Habkost" <ehabkost@redhat.com>,
	"Sergio Lopez" <slp@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"QEMU Developers" <qemu-devel@nongnu.org>,
	"Cameron Esfahani" <dirty@apple.com>,
	"Roman Bolshakov" <r.bolshakov@yadro.com>,
	qemu-arm <qemu-arm@nongnu.org>, "Frank Yang" <lfy@google.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Peter Collingbourne" <pcc@google.com>
Subject: Re: [PATCH v9 07/11] hvf: arm: Implement PSCI handling
Date: Mon, 13 Sep 2021 13:07:17 +0200	[thread overview]
Message-ID: <ad707e0d-8d4a-0248-80f4-a02a7226ce0a@csgraf.de> (raw)
In-Reply-To: <CAFEAcA8BybrfxNxkWbEjxji4DvDumr6Otb_RK_f84Dt_TWXfpA@mail.gmail.com>


On 13.09.21 10:54, Peter Maydell wrote:
> On Mon, 13 Sept 2021 at 00:08, Alexander Graf <agraf@csgraf.de> wrote:
>> We need to handle PSCI calls. Most of the TCG code works for us,
>> but we can simplify it to only handle aa64 mode and we need to
>> handle SUSPEND differently.
>>
>> This patch takes the TCG code as template and duplicates it in HVF.
>>
>> To tell the guest that we support PSCI 0.2 now, update the check in
>> arm_cpu_initfn() as well.
>>
>> Signed-off-by: Alexander Graf <agraf@csgraf.de>
>> Reviewed-by: Sergio Lopez <slp@redhat.com>
>>
>> ---
>>
>> v6 -> v7:
>>
>>   - This patch integrates "arm: Set PSCI to 0.2 for HVF"
>>
>> v7 -> v8:
>>
>>   - Do not advance for HVC, PC is already updated by hvf
>>   - Fix checkpatch error
>>
>> v8 -> v9:
>>
>>   - Use new hvf_raise_exception() prototype
>>   - Make cpu_off function void
>>   - Add comment about return value, use -1 for "not found"
>>   - Remove cpu_synchronize_state() when halted
>> ---
>>  target/arm/cpu.c            |   4 +-
>>  target/arm/hvf/hvf.c        | 127 ++++++++++++++++++++++++++++++++++--
>>  target/arm/hvf/trace-events |   1 +
>>  3 files changed, 126 insertions(+), 6 deletions(-)
> Something in here should be checking whether the insn the guest
> used matches the PSCI conduit configured for the VM, ie
> what arm_is_psci_call() does after your patch 10.


It's yet another case where I believe we are both reading the spec
differently :)

  https://documentation-service.arm.com/static/6013e5faeee5236980d08619

Section 2.5.3 speaks about the conduits. It says

    Service calls are expected to be invoked through SMC instructions,
except
    for Standard Hypervisor Calls and Vendor Specific Hypervisor Calls. On
    some platforms, however, SMC instructions are not available, and the
    services can be accessed through HVC instructions. The method that
    is used to invoke the service is referred to as the conduit.

To me, that reads like "Use SMC whenever you can. If your hardware does
not give you a way to handle SMC calls, falling back to HVC is ok. In
that case, indicate that mandate to the OS".

In hvf, we can very easily trap for SMC calls and handle them. Why are
we making OSs implement HVC call paths when SMC would work just as well
for everyone?

To keep your train of thought though, what would you do if we encounter
a conduit that is different from the chosen one? Today, I am aware of 2
different implementations: TCG injects #UD [1] while KVM sets x0 to -1 [2].

IMHO the best way to resolve all of this mess is to consolidate to SMC
as default PSCI handler and for now treat HVC as if it was an SMC call
as well for virtual environments. Once we get nested virtualization, we
will need to move to SMC as default anyway.


Alex

[1]
https://git.qemu.org/?p=qemu.git;a=blob;f=target/arm/op_helper.c;hb=HEAD#l813
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/kvm/handle_exit.c#n52



  reply	other threads:[~2021-09-13 11:09 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-12 23:07 [PATCH v9 00/11] hvf: Implement Apple Silicon Support Alexander Graf
2021-09-12 23:07 ` [PATCH v9 01/11] arm: Move PMC register definitions to cpu.h Alexander Graf
2021-09-13  8:49   ` Peter Maydell
2021-09-12 23:07 ` [PATCH v9 02/11] hvf: Add execute to dirty log permission bitmap Alexander Graf
2021-09-12 23:07 ` [PATCH v9 03/11] hvf: Introduce hvf_arch_init() callback Alexander Graf
2021-09-12 23:07 ` [PATCH v9 04/11] hvf: Add Apple Silicon support Alexander Graf
2021-09-12 23:07 ` [PATCH v9 05/11] arm/hvf: Add a WFI handler Alexander Graf
2021-09-12 23:07 ` [PATCH v9 06/11] hvf: arm: Implement -cpu host Alexander Graf
2021-09-13  8:54   ` Philippe Mathieu-Daudé
2021-09-12 23:07 ` [PATCH v9 07/11] hvf: arm: Implement PSCI handling Alexander Graf
2021-09-13  8:54   ` Peter Maydell
2021-09-13 11:07     ` Alexander Graf [this message]
2021-09-13 11:44       ` Peter Maydell
2021-09-13 12:02         ` Alexander Graf
2021-09-13 12:30           ` Peter Maydell
2021-09-13 21:29             ` Alexander Graf
2021-09-15  9:46             ` Marc Zyngier
2021-09-15 10:58               ` Alexander Graf
2021-09-15 15:07                 ` Marc Zyngier
2021-09-12 23:07 ` [PATCH v9 08/11] arm: Add Hypervisor.framework build target Alexander Graf
2021-09-12 23:07 ` [PATCH v9 09/11] hvf: arm: Add rudimentary PMC support Alexander Graf
2021-09-12 23:07 ` [PATCH v9 10/11] arm: tcg: Adhere to SMCCC 1.3 section 5.2 Alexander Graf
2021-09-13  8:46   ` Peter Maydell
2021-09-12 23:07 ` [PATCH v9 11/11] hvf: arm: " Alexander Graf
2021-09-13  8:52   ` Peter Maydell

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=ad707e0d-8d4a-0248-80f4-a02a7226ce0a@csgraf.de \
    --to=agraf@csgraf.de \
    --cc=dirty@apple.com \
    --cc=ehabkost@redhat.com \
    --cc=lfy@google.com \
    --cc=maz@kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=pcc@google.com \
    --cc=peter.maydell@linaro.org \
    --cc=philmd@redhat.com \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=r.bolshakov@yadro.com \
    --cc=richard.henderson@linaro.org \
    --cc=slp@redhat.com \
    /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.