On Wed, Feb 22, 2017 at 6:47 AM, Christoffer Dall <[email protected]> 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 <[email protected]>
>> ---
>> 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 <asm/kvm_mmu.h>
>> #include <asm/kvm_psci.h>
>>
>> +#ifdef CONFIG_KVM_ARM_NESTED_HYP
>> +#include <asm/kvm_nested.h>
>> +#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 <[email protected]>
>> + *
>> + * 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 <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include <linux/kvm.h>
>> +#include <linux/kvm_host.h>
>> +
>> +#include <asm/kvm_emulate.h>
>> +
>> +/* 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.
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?
Thanks,
Jintack
>
> Thanks,
> -Christoffer
>
On Mon, Jun 26, 2017 at 11:21:25AM -0400, Jintack Lim wrote:
> On Wed, Feb 22, 2017 at 6:47 AM, Christoffer Dall <[email protected]> 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 <[email protected]>
> >> ---
> >> 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 <asm/kvm_mmu.h>
> >> #include <asm/kvm_psci.h>
> >>
> >> +#ifdef CONFIG_KVM_ARM_NESTED_HYP
> >> +#include <asm/kvm_nested.h>
> >> +#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 <[email protected]>
> >> + *
> >> + * 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 <http://www.gnu.org/licenses/>.
> >> + */
> >> +
> >> +#include <linux/kvm.h>
> >> +#include <linux/kvm_host.h>
> >> +
> >> +#include <asm/kvm_emulate.h>
> >> +
> >> +/* 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.
>
> 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?
Thanks,
-Christoffer
On Mon, Jul 03, 2017 at 11:08:50AM +0200, 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 <[email protected]> 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 <[email protected]>
> > >> ---
> > >> 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 <asm/kvm_mmu.h>
> > >> #include <asm/kvm_psci.h>
> > >>
> > >> +#ifdef CONFIG_KVM_ARM_NESTED_HYP
> > >> +#include <asm/kvm_nested.h>
> > >> +#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 <[email protected]>
> > >> + *
> > >> + * 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 <http://www.gnu.org/licenses/>.
> > >> + */
> > >> +
> > >> +#include <linux/kvm.h>
> > >> +#include <linux/kvm_host.h>
> > >> +
> > >> +#include <asm/kvm_emulate.h>
> > >> +
> > >> +/* 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.
>
> >
> > 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.
Yes, I've been thinking that a KVM generic vcpu needs to be enlightened,
and to use a hypercall to get the host cpu's errata. If we head down that
road, then even a vcpu emulating EL2 would need to be able to this.
>
> Do we currently have support for the guest to use SMC calls for PSCI
> when it has virtual EL2?
Yup, that's already supported by QEMU and the guest kernel.
Thanks,
drew
On Mon, Jul 03, 2017 at 11:31:56AM +0200, Andrew Jones wrote:
> On Mon, Jul 03, 2017 at 11:08:50AM +0200, 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 <[email protected]> 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 <[email protected]>
> > > >> ---
> > > >> 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 <asm/kvm_mmu.h>
> > > >> #include <asm/kvm_psci.h>
> > > >>
> > > >> +#ifdef CONFIG_KVM_ARM_NESTED_HYP
> > > >> +#include <asm/kvm_nested.h>
> > > >> +#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 <[email protected]>
> > > >> + *
> > > >> + * 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 <http://www.gnu.org/licenses/>.
> > > >> + */
> > > >> +
> > > >> +#include <linux/kvm.h>
> > > >> +#include <linux/kvm_host.h>
> > > >> +
> > > >> +#include <asm/kvm_emulate.h>
> > > >> +
> > > >> +/* 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.
> >
> > >
> > > 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.
>
> Yes, I've been thinking that a KVM generic vcpu needs to be enlightened,
> and to use a hypercall to get the host cpu's errata. If we head down that
> road, then even a vcpu emulating EL2 would need to be able to this.
>
We could use SMC calls here a well, as the "conduit" as I believe the
ARM folks are calling it. We just need to agree somewhere (across
hypervisors preferably), that when you have virtual EL2, everything is
via SMC (even upcalls to a host hypervisor), and otherwise it's via HVC.
> >
> > Do we currently have support for the guest to use SMC calls for PSCI
> > when it has virtual EL2?
>
> Yup, that's already supported by QEMU and the guest kernel.
>
Yes, and the KVM support follows this patch in the series as it turns
out (but given the time since I looked at this series last, I forgot).
Thanks,
-Christoffer
On Mon, Jul 03, 2017 at 11:51:26AM +0200, Christoffer Dall wrote:
> On Mon, Jul 03, 2017 at 11:31:56AM +0200, Andrew Jones wrote:
> > On Mon, Jul 03, 2017 at 11:08:50AM +0200, 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 <[email protected]> wrote:
> > > > > On Mon, Jan 09, 2017 at 01:24:17AM -0500, Jintack Lim wrote:
> > > > >> +/* 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.
> > >
> > > >
> > > > 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.
> >
> > Yes, I've been thinking that a KVM generic vcpu needs to be enlightened,
> > and to use a hypercall to get the host cpu's errata. If we head down that
> > road, then even a vcpu emulating EL2 would need to be able to this.
> >
>
> We could use SMC calls here a well, as the "conduit" as I believe the
> ARM folks are calling it. We just need to agree somewhere (across
> hypervisors preferably), that when you have virtual EL2, everything is
> via SMC (even upcalls to a host hypervisor), and otherwise it's via HVC.
Does that mean you require the CPU to implement EL3 if you want to use
nested virtualisation?
Will
On 03/07/17 13:03, Will Deacon wrote:
> On Mon, Jul 03, 2017 at 11:51:26AM +0200, Christoffer Dall wrote:
>> On Mon, Jul 03, 2017 at 11:31:56AM +0200, Andrew Jones wrote:
>>> On Mon, Jul 03, 2017 at 11:08:50AM +0200, 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 <[email protected]> wrote:
>>>>>> On Mon, Jan 09, 2017 at 01:24:17AM -0500, Jintack Lim wrote:
>>>>>>> +/* 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.
>>>>
>>>>>
>>>>> 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.
>>>
>>> Yes, I've been thinking that a KVM generic vcpu needs to be enlightened,
>>> and to use a hypercall to get the host cpu's errata. If we head down that
>>> road, then even a vcpu emulating EL2 would need to be able to this.
>>>
>>
>> We could use SMC calls here a well, as the "conduit" as I believe the
>> ARM folks are calling it. We just need to agree somewhere (across
>> hypervisors preferably), that when you have virtual EL2, everything is
>> via SMC (even upcalls to a host hypervisor), and otherwise it's via HVC.
>
> Does that mean you require the CPU to implement EL3 if you want to use
> nested virtualisation?
The 8.3 spec has relaxed the use of SMC for the non-root hypervisor,
where the top-level hypervisor can trap SMCs from nested hypervisors,
irrespective of EL3 being implemented. It still cannot SMCs from an EL1
guest if EL3 is not implemented though...
Thanks,
M.
--
Jazz is not dead. It just smells funny...
On Mon, Jul 3, 2017 at 5:08 AM, Christoffer Dall <[email protected]> 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 <[email protected]> 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 <[email protected]>
>> >> ---
>> >> 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 <asm/kvm_mmu.h>
>> >> #include <asm/kvm_psci.h>
>> >>
>> >> +#ifdef CONFIG_KVM_ARM_NESTED_HYP
>> >> +#include <asm/kvm_nested.h>
>> >> +#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 <[email protected]>
>> >> + *
>> >> + * 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 <http://www.gnu.org/licenses/>.
>> >> + */
>> >> +
>> >> +#include <linux/kvm.h>
>> >> +#include <linux/kvm_host.h>
>> >> +
>> >> +#include <asm/kvm_emulate.h>
>> >> +
>> >> +/* 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