* [PATCH] KVM: arm64: Clarify vcpu reset behaviour @ 2021-04-06 12:58 Marc Zyngier 2021-04-07 15:59 ` Alexandru Elisei 2021-04-07 20:35 ` Will Deacon 0 siblings, 2 replies; 7+ messages in thread From: Marc Zyngier @ 2021-04-06 12:58 UTC (permalink / raw) To: linux-arm-kernel Cc: Will Deacon, kernel-team, ascull, tabba, dbrazdil, James Morse, Suzuki K Poulose, Alexandru Elisei 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. Signed-off-by: Marc Zyngier <maz@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 + * Aarch32: SVC, D, A, I and F bits set + - General Purpose registers, including PC and SP: set to 0 + - FPSIMD/NEON registers: set to 0 + - SVE registers: set to 0 + - System registers: Reset to their architecturally defined + values as for a warm reset to EL1 (resp. SVC) + Note that because some registers reflect machine topology, all vcpus should be created before this ioctl is invoked. -- 2.29.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] KVM: arm64: Clarify vcpu reset behaviour 2021-04-06 12:58 [PATCH] KVM: arm64: Clarify vcpu reset behaviour Marc Zyngier @ 2021-04-07 15:59 ` Alexandru Elisei 2021-04-07 16:26 ` Marc Zyngier 2021-04-07 20:35 ` Will Deacon 1 sibling, 1 reply; 7+ messages in thread From: Alexandru Elisei @ 2021-04-07 15:59 UTC (permalink / raw) To: Marc Zyngier, linux-arm-kernel Cc: Will Deacon, kernel-team, ascull, tabba, dbrazdil, James Morse, Suzuki K Poulose 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. > > Signed-off-by: Marc Zyngier <maz@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). > + - 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. > + - 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@arm.com> Thanks, Alex > + > Note that because some registers reflect machine topology, all vcpus > should be created before this ioctl is invoked. > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] KVM: arm64: Clarify vcpu reset behaviour 2021-04-07 15:59 ` Alexandru Elisei @ 2021-04-07 16:26 ` Marc Zyngier 0 siblings, 0 replies; 7+ messages in thread From: Marc Zyngier @ 2021-04-07 16:26 UTC (permalink / raw) To: Alexandru Elisei Cc: linux-arm-kernel, Will Deacon, kernel-team, ascull, tabba, dbrazdil, James Morse, Suzuki K Poulose On Wed, 07 Apr 2021 16:59:58 +0100, Alexandru Elisei <alexandru.elisei@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@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@arm.com> Thanks for that. I'll send the other fix separately. M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] KVM: arm64: Clarify vcpu reset behaviour 2021-04-06 12:58 [PATCH] KVM: arm64: Clarify vcpu reset behaviour Marc Zyngier 2021-04-07 15:59 ` Alexandru Elisei @ 2021-04-07 20:35 ` Will Deacon 2021-04-08 10:36 ` Marc Zyngier 1 sibling, 1 reply; 7+ messages in thread From: Will Deacon @ 2021-04-07 20:35 UTC (permalink / raw) To: Marc Zyngier Cc: linux-arm-kernel, kernel-team, ascull, tabba, dbrazdil, James Morse, Suzuki K Poulose, Alexandru Elisei On Tue, Apr 06, 2021 at 01:58:41PM +0100, 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. > > Signed-off-by: Marc Zyngier <maz@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 > + * Aarch32: SVC, D, A, I and F bits set nit, but capitalisation should be "AArch32" Is "SVC" sufficient, or do we also need to describe Arm vs Thumb? Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] KVM: arm64: Clarify vcpu reset behaviour 2021-04-07 20:35 ` Will Deacon @ 2021-04-08 10:36 ` Marc Zyngier 2021-04-08 10:53 ` Will Deacon 0 siblings, 1 reply; 7+ messages in thread From: Marc Zyngier @ 2021-04-08 10:36 UTC (permalink / raw) To: Will Deacon Cc: linux-arm-kernel, kernel-team, ascull, tabba, dbrazdil, James Morse, Suzuki K Poulose, Alexandru Elisei On 2021-04-07 21:35, Will Deacon wrote: > On Tue, Apr 06, 2021 at 01:58:41PM +0100, 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. >> >> Signed-off-by: Marc Zyngier <maz@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 >> + * Aarch32: SVC, D, A, I and F bits set > > nit, but capitalisation should be "AArch32" > > Is "SVC" sufficient, or do we also need to describe Arm vs Thumb? SVC doesn't describe the instruction set, but we don't say that PSR.T is set, which implies ARM. I can add something like "All other bits are set to 0", if that makes it clearer. Thanks, M. -- Jazz is not dead. It just smells funny... _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] KVM: arm64: Clarify vcpu reset behaviour 2021-04-08 10:36 ` Marc Zyngier @ 2021-04-08 10:53 ` Will Deacon 2021-04-08 13:14 ` Marc Zyngier 0 siblings, 1 reply; 7+ messages in thread From: Will Deacon @ 2021-04-08 10:53 UTC (permalink / raw) To: Marc Zyngier Cc: linux-arm-kernel, kernel-team, ascull, tabba, dbrazdil, James Morse, Suzuki K Poulose, Alexandru Elisei On Thu, Apr 08, 2021 at 11:36:34AM +0100, Marc Zyngier wrote: > On 2021-04-07 21:35, Will Deacon wrote: > > On Tue, Apr 06, 2021 at 01:58:41PM +0100, 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. > > > > > > Signed-off-by: Marc Zyngier <maz@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 > > > + * Aarch32: SVC, D, A, I and F bits set > > > > nit, but capitalisation should be "AArch32" > > > > Is "SVC" sufficient, or do we also need to describe Arm vs Thumb? > > SVC doesn't describe the instruction set, but we don't say that PSR.T > is set, which implies ARM. Ah yes, thanks. > I can add something like "All other bits are set to 0", if that makes it > clearer. I think that would be worth adding. With that and the minor typo fix: Acked-by: Will Deacon <will@kernel.org> Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] KVM: arm64: Clarify vcpu reset behaviour 2021-04-08 10:53 ` Will Deacon @ 2021-04-08 13:14 ` Marc Zyngier 0 siblings, 0 replies; 7+ messages in thread From: Marc Zyngier @ 2021-04-08 13:14 UTC (permalink / raw) To: Will Deacon Cc: linux-arm-kernel, kernel-team, ascull, tabba, dbrazdil, James Morse, Suzuki K Poulose, Alexandru Elisei On Thu, 08 Apr 2021 11:53:35 +0100, Will Deacon <will@kernel.org> wrote: > > On Thu, Apr 08, 2021 at 11:36:34AM +0100, Marc Zyngier wrote: > > On 2021-04-07 21:35, Will Deacon wrote: > > > On Tue, Apr 06, 2021 at 01:58:41PM +0100, 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. > > > > > > > > Signed-off-by: Marc Zyngier <maz@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 > > > > + * Aarch32: SVC, D, A, I and F bits set > > > > > > nit, but capitalisation should be "AArch32" > > > > > > Is "SVC" sufficient, or do we also need to describe Arm vs Thumb? > > > > SVC doesn't describe the instruction set, but we don't say that PSR.T > > is set, which implies ARM. > > Ah yes, thanks. > > > I can add something like "All other bits are set to 0", if that makes it > > clearer. > > I think that would be worth adding. With that and the minor typo fix: > > Acked-by: Will Deacon <will@kernel.org> Now fixed. Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-04-08 13:16 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-06 12:58 [PATCH] KVM: arm64: Clarify vcpu reset behaviour Marc Zyngier 2021-04-07 15:59 ` Alexandru Elisei 2021-04-07 16:26 ` Marc Zyngier 2021-04-07 20:35 ` Will Deacon 2021-04-08 10:36 ` Marc Zyngier 2021-04-08 10:53 ` Will Deacon 2021-04-08 13:14 ` Marc Zyngier
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.