On 17.10.2013, at 13:30, Anup Patel <anup@xxxxxxxxxxxxxx> wrote:
> On Thu, Oct 17, 2013 at 4:51 PM, Marc Zyngier <marc.zyngier@xxxxxxx> wrote:
>> On 17/10/13 12:10, Anup Patel wrote:
>>> On Thu, Oct 17, 2013 at 2:17 PM, Marc Zyngier <marc.zyngier@xxxxxxx> wrote:
>>>> On 17/10/13 07:45, Anup Patel wrote:
>>>>> On Thu, Oct 17, 2013 at 3:41 AM, Christoffer Dall
>>>>> <christoffer.dall@xxxxxxxxxx> wrote:
>>>>>> On Wed, Oct 16, 2013 at 10:32:30PM +0530, Anup Patel wrote:
>>>>>>> Update user space API interface headers for providing information to
>>>>>>> user space needed to emulate PSCI function calls in user space (i.e.
>>>>>>> QEMU or KVMTOOL).
>>>>>>>
>>>>>>> Signed-off-by: Anup Patel <anup.patel@xxxxxxxxxx>
>>>>>>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@xxxxxxxxxx>
>>>>>>> ---
>>>>>>> include/uapi/linux/kvm.h | 7 +++++++
>>>>>>> 1 file changed, 7 insertions(+)
>>>>>>>
>>>>>>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>>>>>>> index e32e776..dae2664 100644
>>>>>>> --- a/include/uapi/linux/kvm.h
>>>>>>> +++ b/include/uapi/linux/kvm.h
>>>>>>> @@ -171,6 +171,7 @@ struct kvm_pit_config {
>>>>>>> #define KVM_EXIT_WATCHDOG 21
>>>>>>> #define KVM_EXIT_S390_TSCH 22
>>>>>>> #define KVM_EXIT_EPR 23
>>>>>>> +#define KVM_EXIT_PSCI 24
>>>>>>>
>>>>>>> /* For KVM_EXIT_INTERNAL_ERROR */
>>>>>>> /* Emulate instruction failed. */
>>>>>>> @@ -301,6 +302,12 @@ struct kvm_run {
>>>>>>> struct {
>>>>>>> __u32 epr;
>>>>>>> } epr;
>>>>>>> + /* KVM_EXIT_PSCI */
>>>>>>> + struct {
>>>>>>> + __u32 fn;
>>>>>>> + __u64 args[7];
>>>>>>> + __u64 ret[4];
>>>>>>> + } psci;
>>>>>>> /* Fix the size of the union. */
>>>>>>> char padding[256];
>>>>>>> };
>>>>>>> --
>>>>>>> 1.7.9.5
>>>>>>>
>>>>>> I am also wondering if this is not solving a very specific need without
>>>>>> thinking a little more carefully about this problem.
>>>>>
>>>>> No, its not solving a specific problem.
>>>>>
>>>>> In fact, its more general because we pass complete info required to
>>>>> emulate a PSCI call in user space.
>>>>> (Please refer PSCI calling convention)
>>>>>
>>>>>>
>>>>>> We have previously discussed the need for some secure side emulation
>>>>>> in QEMU, and I think perhaps we need something more generic which allows
>>>>>> user space to handle SMC calls and/or allows user space to "inject" some
>>>>>> secure world runtime that the kernel can run in a partially or fully
>>>>>> isolated container to handle SMC calls.
>>>>>>
>>>>>> Peter raised this issue previously and pointed to a proposal he had as
>>>>>> well.
>>>>>
>>>>> If required we can have an additional field in kvm_run->psci which tells
>>>>> whether the PSCI call is an SMC call or HVC call.
>>>>>
>>>>>>
>>>>>> Is there a technical reason why we need something specifically directed
>>>>>> to PSCI?
>>>>>
>>>>> Its quite natural to add this to PSCI emulation in KVM ARM/ARM64 instead
>>>>> of adding a separate VirtIO device for System reboot and System poweroff.
>>>>>
>>>>> Also in the process of implementing SYSTEM_OFF and SYSTEM_RESET
>>>>> emulation in user space we would also have an infrastructure for adding
>>>>> emulation of new PSCI calls in user space.
>>>>
>>>> And I strongly oppose to that. It creates consistency issues (what if
>>>> userspace implements one version of PSCI, and the kernel another?), and
>>>> also some really horrible situations: Imagine you implement the SUSPEND
>>>> operation in userspace, and want to wake the vcpu up with an interrupt.
>>>> You'd end-up having to keep track of the state in the kernel, having to
>>>> forward the interrupt event to userspace...
>>>
>>> It is not about emulating all PSCI functions in user space. Its about forwarding
>>> system-level PSCI functions or PSCI functions which cannot be emulated in
>>> kernel to user space.
>>
>> The CPU parts of PSCI can perfectly be implemented in the kernel.
>
> Agreed. This patches does the same.
>
>>
>> Then you can return something to userspace indicating what just
>> happened. And it doesn't have to be PSCI specific.
>
> Are you suggesting that everytime we want to emulate some new
> PSCI call with help from user space (e.g. SYSTEM_OFF and
> SYSTEM_RESET), we add new exit reasons and just keep on
> increasing KVM exit reasons ?
>
> Why can't the exit reason and exit info in struct kvm_run be
> PSCI specific ?
>
> On the contrary, it will be good to have exit reason and exit info
> PSCI specific because we have PSCI specification which tells
> how it is to be emulated ?
I completely agree with Marc that split-brain ownership of any address space (and PSCI is basically one) is a very bad idea.
However, so far the only solution I've seen mentioned is that the kernel owns PSCI (read: decodes it) and then drives user space with explicit commands.
Couldn't we reverse this logic? User space owns PSCI. By default all PSCI calls go to user space. If a PSCI call makes more sense to be executed by kvm, it can explicitly route it to be handled by kvm instead.
That way the owner is still at a single spot and we can fast path the few cases that may be performance critical or a lot easier to handle in kvm.
The good part about this is that we get consistency in QEMU with the TCG PSCI handlers along the way.
Alex
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
