[PATCH] KVM: arm64: Clarify vcpu reset behaviour
Marc Zyngier
maz at kernel.org
Wed Apr 7 17:26:47 BST 2021
On Wed, 07 Apr 2021 16:59:58 +0100,
Alexandru Elisei <alexandru.elisei at arm.com> wrote:
>
> Hi Marc,
>
> On 4/6/21 1:58 PM, Marc Zyngier wrote:
> > Although the KVM_ARM_VCPU_INIT documentation mention that the
> > registers are reset to their "initial values", it doesn't
> > describe what these values are.
> >
> > Describe this state explicitly.
>
> This is a good idea.
Yeah, it turns out this is something we need for pKVM, because the
reset state is slightly different.
>
> >
> > Signed-off-by: Marc Zyngier <maz at kernel.org>
> > ---
> > Documentation/virt/kvm/api.rst | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> > index 38e327d4b479..e2237e4e10ba 100644
> > --- a/Documentation/virt/kvm/api.rst
> > +++ b/Documentation/virt/kvm/api.rst
> > @@ -3115,6 +3115,16 @@ optional features it should have. This will cause a reset of the cpu
> > registers to their initial values. If this is not called, KVM_RUN will
> > return ENOEXEC for that vcpu.
> >
> > +The initial values are defined as:
> > + - Processor state:
> > + * AArch64: EL1h, D, A, I and F bits set
>
> That value matches the macro VCPU_RESET_STATE_EL1.
>
> > + * Aarch32: SVC, D, A, I and F bits set
> VCPU_RESET_STATE_SVC doesn't have a D bit; according to ARM DDI 0487G.a, page
> G1-5965, CPSR doesn't have a D bit either (I might be reading it wrong).
Meh, copy paste. Thanks for that,
> > + - General Purpose registers, including PC and SP: set to 0
>
> They are zero'ed explicitly in kvm_reset_vcpu().
>
> > + - FPSIMD/NEON registers: set to 0
>
> I haven't been able to find where they are initialized explicitly, I
> assume they aren't. They are zero because the kvm_vcpu struct is
> allocated via kmem_cache_zalloc() in kvm_vm_ioctl_create_vcpu(), so
> this is correct.
Indeed. However, this outlines an interesting buglet. Userspace is
allowed to call KVM_ARM_VCPU_INIT at any point, and get the vcpu back
to the reset state.
The lack of explicit wipeout of the FPSIMD file is on its own a bug
which crept in with e47c2055c6 ("KVM: arm64: Make struct kvm_regs
userspace-only"), when kvm_cpu_context started to use user_pt_regs
directly.
I'll fix that separately, thanks for pushing me to have a closer look.
>
> > + - SVE registers: set to 0
>
> They are zero'ed explicitly in kvm_vcpu_reset_sve().
>
> > + - System registers: Reset to their architecturally defined
> > + values as for a warm reset to EL1 (resp. SVC)
>
> This is done in kvm_reset_vcpu() -> kvm_reset_sys_regs(), which from
> what I can tell does the right thing, but I haven't checked each and
> every register.
>
> Assuming the D bit for CPSR/SPSR was a typo and with it removed:
>
> Reviewed-by: Alexandru Elisei <alexandru.elisei at arm.com>
Thanks for that. I'll send the other fix separately.
M.
--
Without deviation from the norm, progress is not possible.
More information about the linux-arm-kernel
mailing list