On 21/06/2019 14:24, Julien Thierry wrote: > > > On 21/06/2019 10:37, Marc Zyngier wrote: >> From: Christoffer Dall <christoffer.dall@xxxxxxxxxx> >> >> We were not allowing userspace to set a more privileged mode for the VCPU >> than EL1, but we should allow this when nested virtualization is enabled >> for the VCPU. >> >> Signed-off-by: Christoffer Dall <christoffer.dall@xxxxxxxxxx> >> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx> >> --- >> arch/arm64/kvm/guest.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c >> index 3ae2f82fca46..4c35b5d51e21 100644 >> --- a/arch/arm64/kvm/guest.c >> +++ b/arch/arm64/kvm/guest.c >> @@ -37,6 +37,7 @@ >> #include <asm/kvm_emulate.h> >> #include <asm/kvm_coproc.h> >> #include <asm/kvm_host.h> >> +#include <asm/kvm_nested.h> >> #include <asm/sigcontext.h> >> >> #include "trace.h" >> @@ -194,6 +195,11 @@ static int set_core_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) >> if (vcpu_el1_is_32bit(vcpu)) >> return -EINVAL; >> break; >> + case PSR_MODE_EL2h: >> + case PSR_MODE_EL2t: >> + if (vcpu_el1_is_32bit(vcpu) || !nested_virt_in_use(vcpu)) > > This condition reads a bit weirdly. Why do we care about anything else > than !nested_virt_in_use() ? > > If nested virt is not in use then obviously we return the error. > > If nested virt is in use then why do we care about EL1? Or should this > test read as "highest_el_is_32bit" ? There are multiple things at play here: - MODE_EL2x is not a valid 32bit mode - The architecture forbids nested virt with 32bit EL2 The code above is a simplification of these two conditions. But certainly we can do a bit better, as kvm_reset_cpu() doesn't really check that we don't create a vcpu with both 32bit+NV. These two bits should really be exclusive. M. -- Jazz is not dead. It just smells funny...
