Hi Marc,
On Thu, Feb 03, 2022 at 05:43:36PM +0000, Marc Zyngier wrote:
> On Wed, 02 Feb 2022 15:23:20 +0000,
> Alexandru Elisei <alexandru.elisei@xxxxxxx> 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@xxxxxxxxxx>
> > > + */
> > > +
> > > +#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.
Hmm... I don't think so. The illegal condition, according to the snippet, should
be:
!(mode & PSR_MODE32_BIT) && !(__vcpu_sys_reg(vcpu, HCR_EL2) & HCR_RW)
Or, perhaps better, KVM could add an HCR_EL2 accessor that treated the
HCR_EL2.RW bit as RA0/WI as per the architecture when the EL1 is not capable of
AArch32. That would make the above situation impossible.
Thanks,
Alex
>
> Thanks,
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.
