Re: [PATCH 06/59] KVM: arm64: nv: Allow userspace to set PSR_MODE_EL2x

Marc Zyngier
2019-06-21T13:50:19Z

On 21/06/2019 14:24, Julien Thierry wrote:
>
>
> On 21/06/2019 10:37, Marc Zyngier wrote:
>> From: Christoffer Dall <christoffer.dall@linaro.org>
>>
>> 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@linaro.org>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>> 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...