From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754542AbdGCN3h (ORCPT ); Mon, 3 Jul 2017 09:29:37 -0400 Received: from mail-oi0-f48.google.com ([209.85.218.48]:34341 "EHLO mail-oi0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753447AbdGCN3e (ORCPT ); Mon, 3 Jul 2017 09:29:34 -0400 MIME-Version: 1.0 In-Reply-To: <20170703090850.GG4066@cbox> References: <1483943091-1364-1-git-send-email-jintack@cs.columbia.edu> <1483943091-1364-22-git-send-email-jintack@cs.columbia.edu> <20170222114758.GM26976@cbox> <20170703090850.GG4066@cbox> From: Jintack Lim Date: Mon, 3 Jul 2017 09:29:29 -0400 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [RFC 21/55] KVM: arm64: Forward HVC instruction to the guest hypervisor To: Christoffer Dall Cc: Christoffer Dall , Marc Zyngier , Paolo Bonzini , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , linux@armlinux.org.uk, Catalin Marinas , Will Deacon , vladimir.murzin@arm.com, Suzuki K Poulose , mark.rutland@arm.com, james.morse@arm.com, lorenzo.pieralisi@arm.com, kevin.brodsky@arm.com, wcohen@redhat.com, shankerd@codeaurora.org, geoff@infradead.org, Andre Przywara , Eric Auger , anna-maria@linutronix.de, Shih-Wei Li , arm-mail-list , kvmarm@lists.cs.columbia.edu, KVM General , lkml - Kernel Mailing List Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jul 3, 2017 at 5:08 AM, Christoffer Dall wrote: > On Mon, Jun 26, 2017 at 11:21:25AM -0400, Jintack Lim wrote: >> On Wed, Feb 22, 2017 at 6:47 AM, Christoffer Dall wrote: >> > On Mon, Jan 09, 2017 at 01:24:17AM -0500, Jintack Lim wrote: >> >> Forward exceptions due to hvc instruction to the guest hypervisor. >> >> >> >> Signed-off-by: Jintack Lim >> >> --- >> >> arch/arm64/include/asm/kvm_nested.h | 5 +++++ >> >> arch/arm64/kvm/Makefile | 1 + >> >> arch/arm64/kvm/handle_exit.c | 11 +++++++++++ >> >> arch/arm64/kvm/handle_exit_nested.c | 27 +++++++++++++++++++++++++++ >> >> 4 files changed, 44 insertions(+) >> >> create mode 100644 arch/arm64/include/asm/kvm_nested.h >> >> create mode 100644 arch/arm64/kvm/handle_exit_nested.c >> >> >> >> diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h >> >> new file mode 100644 >> >> index 0000000..620b4d3 >> >> --- /dev/null >> >> +++ b/arch/arm64/include/asm/kvm_nested.h >> >> @@ -0,0 +1,5 @@ >> >> +#ifndef __ARM64_KVM_NESTED_H__ >> >> +#define __ARM64_KVM_NESTED_H__ >> >> + >> >> +int handle_hvc_nested(struct kvm_vcpu *vcpu); >> >> +#endif >> >> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile >> >> index b342bdd..9c35e9a 100644 >> >> --- a/arch/arm64/kvm/Makefile >> >> +++ b/arch/arm64/kvm/Makefile >> >> @@ -35,4 +35,5 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/irqchip.o >> >> kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/arch_timer.o >> >> kvm-$(CONFIG_KVM_ARM_PMU) += $(KVM)/arm/pmu.o >> >> >> >> +kvm-$(CONFIG_KVM_ARM_NESTED_HYP) += handle_exit_nested.o >> >> kvm-$(CONFIG_KVM_ARM_NESTED_HYP) += emulate-nested.o >> >> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c >> >> index a891684..208be16 100644 >> >> --- a/arch/arm64/kvm/handle_exit.c >> >> +++ b/arch/arm64/kvm/handle_exit.c >> >> @@ -29,6 +29,10 @@ >> >> #include >> >> #include >> >> >> >> +#ifdef CONFIG_KVM_ARM_NESTED_HYP >> >> +#include >> >> +#endif >> >> + >> >> #define CREATE_TRACE_POINTS >> >> #include "trace.h" >> >> >> >> @@ -42,6 +46,13 @@ static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run) >> >> kvm_vcpu_hvc_get_imm(vcpu)); >> >> vcpu->stat.hvc_exit_stat++; >> >> >> >> +#ifdef CONFIG_KVM_ARM_NESTED_HYP >> >> + ret = handle_hvc_nested(vcpu); >> >> + if (ret < 0 && ret != -EINVAL) >> >> + return ret; >> >> + else if (ret >= 0) >> >> + return ret; >> >> +#endif >> >> ret = kvm_psci_call(vcpu); >> >> if (ret < 0) { >> >> kvm_inject_undefined(vcpu); >> >> diff --git a/arch/arm64/kvm/handle_exit_nested.c b/arch/arm64/kvm/handle_exit_nested.c >> >> new file mode 100644 >> >> index 0000000..a6ce23b >> >> --- /dev/null >> >> +++ b/arch/arm64/kvm/handle_exit_nested.c >> >> @@ -0,0 +1,27 @@ >> >> +/* >> >> + * Copyright (C) 2016 - Columbia University >> >> + * Author: Jintack Lim >> >> + * >> >> + * This program is free software; you can redistribute it and/or modify >> >> + * it under the terms of the GNU General Public License version 2 as >> >> + * published by the Free Software Foundation. >> >> + * >> >> + * This program is distributed in the hope that it will be useful, >> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> >> + * GNU General Public License for more details. >> >> + * >> >> + * You should have received a copy of the GNU General Public License >> >> + * along with this program. If not, see . >> >> + */ >> >> + >> >> +#include >> >> +#include >> >> + >> >> +#include >> >> + >> >> +/* We forward all hvc instruction to the guest hypervisor. */ >> >> +int handle_hvc_nested(struct kvm_vcpu *vcpu) >> >> +{ >> >> + return kvm_inject_nested_sync(vcpu, kvm_vcpu_get_hsr(vcpu)); >> >> +} >> > >> > I don't understand the logic here or in the caller above. Do we really >> > forward *all" hvc calls to the guest hypervisor now, so that we no >> > longer support any hypercalls from the VM? That seems a little rough >> > and probably requires some more discussions. >> >> So I think if we run a VM with the EL2 support, then all hvc calls >> from the VM should be forwarded to the virtual EL2. > > But do we actually check if the guest has EL2 here? It seems you cann > handle_hvc_nested unconditionally when you have > OCNFIG_KVM_ARM_NESTED_HYP. I think that's what threw me off when first > reading your patch. You're right. We should check it first. > >> >> I may miss something obvious, so can you (or anyone) come up with some >> cases that the host hypervisor needs to directly handle hvc from the >> VM with the EL2 support? >> > > So I'm a little unsure what to say here. On one hand you are absolutely > correct, that architecturally if we emulated virtual EL2, then all > hypercalls are handled by the virtual EL2 (even hypercalls from virtual > EL2 which should become self-hypercalls). > > On the other hand, an enlightened guest may want to use hypercalls to > the hypervisor for some reason, but that would require some numbering > scheme to separate the two concepts. > > Do we currently have support for the guest to use SMC calls for PSCI > when it has virtual EL2? Yes, we do in "[RFC,22/55] KVM: arm64: Handle PSCI call from the guest" as you figured out. > > Thanks, > -Christoffer From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jintack Lim Subject: Re: [RFC 21/55] KVM: arm64: Forward HVC instruction to the guest hypervisor Date: Mon, 3 Jul 2017 09:29:29 -0400 Message-ID: References: <1483943091-1364-1-git-send-email-jintack@cs.columbia.edu> <1483943091-1364-22-git-send-email-jintack@cs.columbia.edu> <20170222114758.GM26976@cbox> <20170703090850.GG4066@cbox> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Cc: Christoffer Dall , Marc Zyngier , Paolo Bonzini , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , linux@armlinux.org.uk, Catalin Marinas , Will Deacon , vladimir.murzin@arm.com, Suzuki K Poulose , mark.rutland@arm.com, james.morse@arm.com, lorenzo.pieralisi@arm.com, kevin.brodsky@arm.com, wcohen@redhat.com, shankerd@codeaurora.org, geoff@infradead.org, Andre Przywara , Eric Auger , anna-maria@linutronix.de, Shih-Wei Li , arm-mail-list , kvmarm@lists.cs.columbia.edu, KVM General , lkml - Kern To: Christoffer Dall Return-path: Received: from mail-oi0-f53.google.com ([209.85.218.53]:34342 "EHLO mail-oi0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754150AbdGCN3e (ORCPT ); Mon, 3 Jul 2017 09:29:34 -0400 Received: by mail-oi0-f53.google.com with SMTP id l130so81344734oib.1 for ; Mon, 03 Jul 2017 06:29:34 -0700 (PDT) In-Reply-To: <20170703090850.GG4066@cbox> Sender: kvm-owner@vger.kernel.org List-ID: On Mon, Jul 3, 2017 at 5:08 AM, Christoffer Dall wrote: > On Mon, Jun 26, 2017 at 11:21:25AM -0400, Jintack Lim wrote: >> On Wed, Feb 22, 2017 at 6:47 AM, Christoffer Dall wrote: >> > On Mon, Jan 09, 2017 at 01:24:17AM -0500, Jintack Lim wrote: >> >> Forward exceptions due to hvc instruction to the guest hypervisor. >> >> >> >> Signed-off-by: Jintack Lim >> >> --- >> >> arch/arm64/include/asm/kvm_nested.h | 5 +++++ >> >> arch/arm64/kvm/Makefile | 1 + >> >> arch/arm64/kvm/handle_exit.c | 11 +++++++++++ >> >> arch/arm64/kvm/handle_exit_nested.c | 27 +++++++++++++++++++++++++++ >> >> 4 files changed, 44 insertions(+) >> >> create mode 100644 arch/arm64/include/asm/kvm_nested.h >> >> create mode 100644 arch/arm64/kvm/handle_exit_nested.c >> >> >> >> diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h >> >> new file mode 100644 >> >> index 0000000..620b4d3 >> >> --- /dev/null >> >> +++ b/arch/arm64/include/asm/kvm_nested.h >> >> @@ -0,0 +1,5 @@ >> >> +#ifndef __ARM64_KVM_NESTED_H__ >> >> +#define __ARM64_KVM_NESTED_H__ >> >> + >> >> +int handle_hvc_nested(struct kvm_vcpu *vcpu); >> >> +#endif >> >> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile >> >> index b342bdd..9c35e9a 100644 >> >> --- a/arch/arm64/kvm/Makefile >> >> +++ b/arch/arm64/kvm/Makefile >> >> @@ -35,4 +35,5 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/irqchip.o >> >> kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/arch_timer.o >> >> kvm-$(CONFIG_KVM_ARM_PMU) += $(KVM)/arm/pmu.o >> >> >> >> +kvm-$(CONFIG_KVM_ARM_NESTED_HYP) += handle_exit_nested.o >> >> kvm-$(CONFIG_KVM_ARM_NESTED_HYP) += emulate-nested.o >> >> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c >> >> index a891684..208be16 100644 >> >> --- a/arch/arm64/kvm/handle_exit.c >> >> +++ b/arch/arm64/kvm/handle_exit.c >> >> @@ -29,6 +29,10 @@ >> >> #include >> >> #include >> >> >> >> +#ifdef CONFIG_KVM_ARM_NESTED_HYP >> >> +#include >> >> +#endif >> >> + >> >> #define CREATE_TRACE_POINTS >> >> #include "trace.h" >> >> >> >> @@ -42,6 +46,13 @@ static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run) >> >> kvm_vcpu_hvc_get_imm(vcpu)); >> >> vcpu->stat.hvc_exit_stat++; >> >> >> >> +#ifdef CONFIG_KVM_ARM_NESTED_HYP >> >> + ret = handle_hvc_nested(vcpu); >> >> + if (ret < 0 && ret != -EINVAL) >> >> + return ret; >> >> + else if (ret >= 0) >> >> + return ret; >> >> +#endif >> >> ret = kvm_psci_call(vcpu); >> >> if (ret < 0) { >> >> kvm_inject_undefined(vcpu); >> >> diff --git a/arch/arm64/kvm/handle_exit_nested.c b/arch/arm64/kvm/handle_exit_nested.c >> >> new file mode 100644 >> >> index 0000000..a6ce23b >> >> --- /dev/null >> >> +++ b/arch/arm64/kvm/handle_exit_nested.c >> >> @@ -0,0 +1,27 @@ >> >> +/* >> >> + * Copyright (C) 2016 - Columbia University >> >> + * Author: Jintack Lim >> >> + * >> >> + * This program is free software; you can redistribute it and/or modify >> >> + * it under the terms of the GNU General Public License version 2 as >> >> + * published by the Free Software Foundation. >> >> + * >> >> + * This program is distributed in the hope that it will be useful, >> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> >> + * GNU General Public License for more details. >> >> + * >> >> + * You should have received a copy of the GNU General Public License >> >> + * along with this program. If not, see . >> >> + */ >> >> + >> >> +#include >> >> +#include >> >> + >> >> +#include >> >> + >> >> +/* We forward all hvc instruction to the guest hypervisor. */ >> >> +int handle_hvc_nested(struct kvm_vcpu *vcpu) >> >> +{ >> >> + return kvm_inject_nested_sync(vcpu, kvm_vcpu_get_hsr(vcpu)); >> >> +} >> > >> > I don't understand the logic here or in the caller above. Do we really >> > forward *all" hvc calls to the guest hypervisor now, so that we no >> > longer support any hypercalls from the VM? That seems a little rough >> > and probably requires some more discussions. >> >> So I think if we run a VM with the EL2 support, then all hvc calls >> from the VM should be forwarded to the virtual EL2. > > But do we actually check if the guest has EL2 here? It seems you cann > handle_hvc_nested unconditionally when you have > OCNFIG_KVM_ARM_NESTED_HYP. I think that's what threw me off when first > reading your patch. You're right. We should check it first. > >> >> I may miss something obvious, so can you (or anyone) come up with some >> cases that the host hypervisor needs to directly handle hvc from the >> VM with the EL2 support? >> > > So I'm a little unsure what to say here. On one hand you are absolutely > correct, that architecturally if we emulated virtual EL2, then all > hypercalls are handled by the virtual EL2 (even hypercalls from virtual > EL2 which should become self-hypercalls). > > On the other hand, an enlightened guest may want to use hypercalls to > the hypervisor for some reason, but that would require some numbering > scheme to separate the two concepts. > > Do we currently have support for the guest to use SMC calls for PSCI > when it has virtual EL2? Yes, we do in "[RFC,22/55] KVM: arm64: Handle PSCI call from the guest" as you figured out. > > Thanks, > -Christoffer From mboxrd@z Thu Jan 1 00:00:00 1970 From: jintack.lim@linaro.org (Jintack Lim) Date: Mon, 3 Jul 2017 09:29:29 -0400 Subject: [RFC 21/55] KVM: arm64: Forward HVC instruction to the guest hypervisor In-Reply-To: <20170703090850.GG4066@cbox> References: <1483943091-1364-1-git-send-email-jintack@cs.columbia.edu> <1483943091-1364-22-git-send-email-jintack@cs.columbia.edu> <20170222114758.GM26976@cbox> <20170703090850.GG4066@cbox> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Mon, Jul 3, 2017 at 5:08 AM, Christoffer Dall wrote: > On Mon, Jun 26, 2017 at 11:21:25AM -0400, Jintack Lim wrote: >> On Wed, Feb 22, 2017 at 6:47 AM, Christoffer Dall wrote: >> > On Mon, Jan 09, 2017 at 01:24:17AM -0500, Jintack Lim wrote: >> >> Forward exceptions due to hvc instruction to the guest hypervisor. >> >> >> >> Signed-off-by: Jintack Lim >> >> --- >> >> arch/arm64/include/asm/kvm_nested.h | 5 +++++ >> >> arch/arm64/kvm/Makefile | 1 + >> >> arch/arm64/kvm/handle_exit.c | 11 +++++++++++ >> >> arch/arm64/kvm/handle_exit_nested.c | 27 +++++++++++++++++++++++++++ >> >> 4 files changed, 44 insertions(+) >> >> create mode 100644 arch/arm64/include/asm/kvm_nested.h >> >> create mode 100644 arch/arm64/kvm/handle_exit_nested.c >> >> >> >> diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h >> >> new file mode 100644 >> >> index 0000000..620b4d3 >> >> --- /dev/null >> >> +++ b/arch/arm64/include/asm/kvm_nested.h >> >> @@ -0,0 +1,5 @@ >> >> +#ifndef __ARM64_KVM_NESTED_H__ >> >> +#define __ARM64_KVM_NESTED_H__ >> >> + >> >> +int handle_hvc_nested(struct kvm_vcpu *vcpu); >> >> +#endif >> >> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile >> >> index b342bdd..9c35e9a 100644 >> >> --- a/arch/arm64/kvm/Makefile >> >> +++ b/arch/arm64/kvm/Makefile >> >> @@ -35,4 +35,5 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/irqchip.o >> >> kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/arch_timer.o >> >> kvm-$(CONFIG_KVM_ARM_PMU) += $(KVM)/arm/pmu.o >> >> >> >> +kvm-$(CONFIG_KVM_ARM_NESTED_HYP) += handle_exit_nested.o >> >> kvm-$(CONFIG_KVM_ARM_NESTED_HYP) += emulate-nested.o >> >> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c >> >> index a891684..208be16 100644 >> >> --- a/arch/arm64/kvm/handle_exit.c >> >> +++ b/arch/arm64/kvm/handle_exit.c >> >> @@ -29,6 +29,10 @@ >> >> #include >> >> #include >> >> >> >> +#ifdef CONFIG_KVM_ARM_NESTED_HYP >> >> +#include >> >> +#endif >> >> + >> >> #define CREATE_TRACE_POINTS >> >> #include "trace.h" >> >> >> >> @@ -42,6 +46,13 @@ static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run) >> >> kvm_vcpu_hvc_get_imm(vcpu)); >> >> vcpu->stat.hvc_exit_stat++; >> >> >> >> +#ifdef CONFIG_KVM_ARM_NESTED_HYP >> >> + ret = handle_hvc_nested(vcpu); >> >> + if (ret < 0 && ret != -EINVAL) >> >> + return ret; >> >> + else if (ret >= 0) >> >> + return ret; >> >> +#endif >> >> ret = kvm_psci_call(vcpu); >> >> if (ret < 0) { >> >> kvm_inject_undefined(vcpu); >> >> diff --git a/arch/arm64/kvm/handle_exit_nested.c b/arch/arm64/kvm/handle_exit_nested.c >> >> new file mode 100644 >> >> index 0000000..a6ce23b >> >> --- /dev/null >> >> +++ b/arch/arm64/kvm/handle_exit_nested.c >> >> @@ -0,0 +1,27 @@ >> >> +/* >> >> + * Copyright (C) 2016 - Columbia University >> >> + * Author: Jintack Lim >> >> + * >> >> + * This program is free software; you can redistribute it and/or modify >> >> + * it under the terms of the GNU General Public License version 2 as >> >> + * published by the Free Software Foundation. >> >> + * >> >> + * This program is distributed in the hope that it will be useful, >> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> >> + * GNU General Public License for more details. >> >> + * >> >> + * You should have received a copy of the GNU General Public License >> >> + * along with this program. If not, see . >> >> + */ >> >> + >> >> +#include >> >> +#include >> >> + >> >> +#include >> >> + >> >> +/* We forward all hvc instruction to the guest hypervisor. */ >> >> +int handle_hvc_nested(struct kvm_vcpu *vcpu) >> >> +{ >> >> + return kvm_inject_nested_sync(vcpu, kvm_vcpu_get_hsr(vcpu)); >> >> +} >> > >> > I don't understand the logic here or in the caller above. Do we really >> > forward *all" hvc calls to the guest hypervisor now, so that we no >> > longer support any hypercalls from the VM? That seems a little rough >> > and probably requires some more discussions. >> >> So I think if we run a VM with the EL2 support, then all hvc calls >> from the VM should be forwarded to the virtual EL2. > > But do we actually check if the guest has EL2 here? It seems you cann > handle_hvc_nested unconditionally when you have > OCNFIG_KVM_ARM_NESTED_HYP. I think that's what threw me off when first > reading your patch. You're right. We should check it first. > >> >> I may miss something obvious, so can you (or anyone) come up with some >> cases that the host hypervisor needs to directly handle hvc from the >> VM with the EL2 support? >> > > So I'm a little unsure what to say here. On one hand you are absolutely > correct, that architecturally if we emulated virtual EL2, then all > hypercalls are handled by the virtual EL2 (even hypercalls from virtual > EL2 which should become self-hypercalls). > > On the other hand, an enlightened guest may want to use hypercalls to > the hypervisor for some reason, but that would require some numbering > scheme to separate the two concepts. > > Do we currently have support for the guest to use SMC calls for PSCI > when it has virtual EL2? Yes, we do in "[RFC,22/55] KVM: arm64: Handle PSCI call from the guest" as you figured out. > > Thanks, > -Christoffer