Re: [PATCH v6 09/64] KVM: arm64: nv: Support virtual EL2 exceptions

Marc Zyngier
2022-02-03T17:43:45Z

On Wed, 02 Feb 2022 15:23:20 +0000,
Alexandru Elisei <alexandru.elisei@arm.com> wrote:
>
> Hi,
>
> On Fri, Jan 28, 2022 at 12:18:17PM +0000, Marc Zyngier wrote:
> > diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> > new file mode 100644
> > index 000000000000..f52cd4458947
> > --- /dev/null
> > +++ b/arch/arm64/kvm/emulate-nested.c
> > @@ -0,0 +1,197 @@
>
> Looks like this line:
>
> // SPDX-License-Identifier: GPL-2.0-only
>
> is missing.

Indeed. I should check all the new files, as they are a bit... off.

>
> > +/*
> > + * Copyright (C) 2016 - Linaro and Columbia University
> > + * Author: Jintack Lim <jintack.lim@linaro.org>
> > + */
> > +
> > +#include <linux/kvm.h>
> > +#include <linux/kvm_host.h>
> > +
> > +#include <asm/kvm_emulate.h>
> > +#include <asm/kvm_nested.h>
> > +
> > +#include "hyp/include/hyp/adjust_pc.h"
> > +
> > +#include "trace.h"
> > +
> > +static u64 kvm_check_illegal_exception_return(struct kvm_vcpu *vcpu, u64 spsr)
> > +{
> > + u64 mode = spsr & PSR_MODE_MASK;
> > +
> > + /*
> > + * Possible causes for an Illegal Exception Return from EL2:
> > + * - trying to return to EL3
> > + * - trying to return to a 32bit EL
> > + * - trying to return to EL1 with HCR_EL2.TGE set
> > + */
> > + if (mode == PSR_MODE_EL3t || mode == PSR_MODE_EL3h ||
> > + spsr & PSR_MODE32_BIT ||
>
> I take it KVM will not allow a L1 hypervisor to run EL1 or EL0 in 32
> bit mode?

No, that'd really be a distraction at this stage. I don't expect any
HW supporting NV NV to support AArch32 at EL1, and if someone really
needs EL0 support (the HW support actually exists), they'll have to
revisit this.

>
> > + (vcpu_el2_tge_is_set(vcpu) && (mode == PSR_MODE_EL1t ||
> > + mode == PSR_MODE_EL1h))) {
>
> I think these checks should also be added:
>
> "A return where the value of the saved process state M[4] bit is 0,
> indicating a return to AArch64 state, and one of the following is
> true:
>
> - The M[1] bit is 1.
> - The M[3:0] bits are 0b0001.

Definitely should add these two, probably in the form of a switch
enumerating all the possible exception levels rather than checking for
discrete bits that are hard to reason about.

> - The Exception level being returned to is using AArch32 state, as
> programmed by the SCR_EL3.RW or HCR_EL2.RW bits, or as configured
> from reset."

That's already caught with the SPSR check above.

Thanks,

M.

--
Without deviation from the norm, progress is not possible.