From: marc.zyngier@arm•com (Marc Zyngier)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v9 07/12] ARM/ARM64: KVM: Emulate PSCI v0.2 SYSTEM_OFF and SYSTEM_RESET
Date: Tue, 15 Apr 2014 13:28:10 +0100 [thread overview]
Message-ID: <87r44yydyd.fsf@approximate.cambridge.arm.com> (raw)
In-Reply-To: <CAAhSdy0XkoOaCid+OmU_Z0VRhs_4nzf3YodLd5LH8X+8HRjAdw@mail.gmail.com> (Anup Patel's message of "Tue, 15 Apr 2014 12:26:06 +0100")
On Tue, Apr 15 2014 at 12:26:06 pm BST, Anup Patel <anup@brainfault•org> wrote:
> On Tue, Apr 15, 2014 at 4:04 PM, Marc Zyngier <marc.zyngier@arm•com> wrote:
>> On Tue, Apr 15 2014 at 7:14:10 am BST, Anup Patel <anup.patel@linaro•org> wrote:
>>> The PSCI v0.2 SYSTEM_OFF and SYSTEM_RESET functions are system-level
>>> functions hence cannot be fully emulated by in-kernel PSCI emulation code.
>>>
>>> To tackle this, we forward PSCI v0.2 SYSTEM_OFF and SYSTEM_RESET function
>>> calls from vcpu to user space (i.e. QEMU or KVMTOOL) via kvm_run structure
>>> using KVM_EXIT_SYSTEM_EVENT exit reasons.
>>>
>>> Signed-off-by: Anup Patel <anup.patel@linaro•org>
>>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro•org>
>>> Reviewed-by: Christoffer Dall <christoffer.dall@linaro•org>
>>> ---
>>> arch/arm/kvm/psci.c | 32 +++++++++++++++++++++++++++++---
>>> 1 file changed, 29 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
>>> index 14e6fa6..b964aa4 100644
>>> --- a/arch/arm/kvm/psci.c
>>> +++ b/arch/arm/kvm/psci.c
>>> @@ -85,6 +85,23 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>>> return PSCI_RET_SUCCESS;
>>> }
>>>
>>> +static inline void kvm_prepare_system_event(struct kvm_vcpu *vcpu, u32 type)
>>
>> Loose the "inline". This is not performance critical, and the compiler
>> does a pretty good job doing that for you.
>
> OK, I will drop the "inline" attribute.
>
>>
>>> +{
>>> + memset(&vcpu->run->system_event, 0, sizeof(vcpu->run->system_event));
>>> + vcpu->run->system_event.type = type;
>>> + vcpu->run->exit_reason = KVM_EXIT_SYSTEM_EVENT;
>>> +}
>>> +
>>> +static void kvm_psci_system_off(struct kvm_vcpu *vcpu)
>>> +{
>>> + kvm_prepare_system_event(vcpu, KVM_SYSTEM_EVENT_SHUTDOWN);
>>> +}
>>> +
>>> +static void kvm_psci_system_reset(struct kvm_vcpu *vcpu)
>>> +{
>>> + kvm_prepare_system_event(vcpu, KVM_SYSTEM_EVENT_RESET);
>>> +}
>>> +
>>> int kvm_psci_version(struct kvm_vcpu *vcpu)
>>> {
>>> if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features))
>>> @@ -95,6 +112,7 @@ int kvm_psci_version(struct kvm_vcpu *vcpu)
>>>
>>> static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>>> {
>>> + int ret = 1;
>>> unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
>>> unsigned long val;
>>>
>>> @@ -114,13 +132,21 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>>> case PSCI_0_2_FN64_CPU_ON:
>>> val = kvm_psci_vcpu_on(vcpu);
>>> break;
>>> + case PSCI_0_2_FN_SYSTEM_OFF:
>>> + kvm_psci_system_off(vcpu);
>>> + val = PSCI_RET_SUCCESS;
>>> + ret = 0;
>>> + break;
>>> + case PSCI_0_2_FN_SYSTEM_RESET:
>>> + kvm_psci_system_reset(vcpu);
>>> + val = PSCI_RET_SUCCESS;
>>> + ret = 0;
>>> + break;
>>
>> What is the significance of setting val to PSCI_RET_SUCCESS here? We're
>> exiting to userspace, so surely only the platform emulation can set
>> that. Am I missing something?
>
> Actually, return value is undefined for SYSTEM_OFF and SYSTEM_RESET
> because these functions are not expected to return. Currently, we are updating
> r0 (or x0) for all PSCI functions.
>
> Are you suggesting that we update r0 (or x0) only when there are no error
> (i.e. ret == 0) ?
What I'm suggesting is that the only valid return value should be
indicative of a failure. If you're coming back to the guest after a
SYSTEM_OFF or a SYSTEM_RESET, it means you've failed to perform the
operation.
So userspace either should report the failure (by putting it in x0), and
there is no need to pre-load x0 with SUCCESS, or you start by storing
FAILURE in x0, before exiting to userspace.
M.
--
Jazz is not dead. It just smells funny.
next prev parent reply other threads:[~2014-04-15 12:28 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-15 6:14 [PATCH v9 00/12] In-kernel PSCI v0.2 emulation for KVM ARM/ARM64 Anup Patel
2014-04-15 6:14 ` [PATCH v9 01/12] KVM: Add capability to advertise PSCI v0.2 support Anup Patel
2014-04-15 9:49 ` Marc Zyngier
2014-04-15 6:14 ` [PATCH v9 02/12] ARM/ARM64: KVM: Add common header for PSCI related defines Anup Patel
2014-04-15 10:06 ` Marc Zyngier
2014-04-15 11:10 ` Anup Patel
2014-04-15 12:18 ` Marc Zyngier
2014-04-15 6:14 ` [PATCH v9 03/12] ARM/ARM64: KVM: Add base for PSCI v0.2 emulation Anup Patel
2014-04-15 10:19 ` Marc Zyngier
2014-04-15 6:14 ` [PATCH v9 04/12] KVM: Documentation: Add info regarding KVM_ARM_VCPU_PSCI_0_2 feature Anup Patel
2014-04-15 10:20 ` Marc Zyngier
2014-04-15 6:14 ` [PATCH v9 05/12] ARM/ARM64: KVM: Make kvm_psci_call() return convention more flexible Anup Patel
2014-04-15 10:21 ` Marc Zyngier
2014-04-15 11:13 ` Anup Patel
2014-04-15 12:23 ` Marc Zyngier
2014-04-15 13:07 ` Peter Maydell
2014-04-15 6:14 ` [PATCH v9 06/12] KVM: Add KVM_EXIT_SYSTEM_EVENT to user space API header Anup Patel
2014-04-15 10:25 ` Marc Zyngier
2014-04-15 6:14 ` [PATCH v9 07/12] ARM/ARM64: KVM: Emulate PSCI v0.2 SYSTEM_OFF and SYSTEM_RESET Anup Patel
2014-04-15 10:34 ` Marc Zyngier
2014-04-15 11:26 ` Anup Patel
2014-04-15 12:28 ` Marc Zyngier [this message]
2014-04-15 6:14 ` [PATCH v9 08/12] ARM/ARM64: KVM: Emulate PSCI v0.2 AFFINITY_INFO Anup Patel
2014-04-15 10:55 ` Marc Zyngier
2014-04-15 11:15 ` Anup Patel
2014-04-15 6:14 ` [PATCH v9 09/12] ARM/ARM64: KVM: Emulate PSCI v0.2 MIGRATE_INFO_TYPE and related functions Anup Patel
2014-04-15 12:05 ` Marc Zyngier
2014-04-15 6:14 ` [PATCH v9 10/12] ARM/ARM64: KVM: Fix CPU_ON emulation for PSCI v0.2 Anup Patel
2014-04-15 12:10 ` Marc Zyngier
2014-04-15 6:14 ` [PATCH v9 11/12] ARM/ARM64: KVM: Emulate PSCI v0.2 CPU_SUSPEND Anup Patel
2014-04-15 12:14 ` Marc Zyngier
2014-04-15 6:14 ` [PATCH v9 12/12] ARM/ARM64: KVM: Advertise KVM_CAP_ARM_PSCI_0_2 to user space Anup Patel
2014-04-15 12:16 ` Marc Zyngier
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=87r44yydyd.fsf@approximate.cambridge.arm.com \
--to=marc.zyngier@arm$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox