All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] KVM: arm64: Fully zero the vcpu state on reset
@ 2021-04-07 18:13 ` Marc Zyngier
  0 siblings, 0 replies; 9+ messages in thread
From: Marc Zyngier @ 2021-04-07 18:13 UTC (permalink / raw)
  To: linux-arm-kernel, kvm, kvmarm
  Cc: James Morse, Suzuki K Poulose, Alexandru Elisei, kernel-team, stable

On vcpu reset, we expect all the registers to be brought back
to their initial state, which happens to be a bunch of zeroes.

However, some recent commit broke this, and is now leaving a bunch
of registers (such as a FP state) with whatever was left by the
guest. My bad.

Just zero the whole vcpu context on reset. It is more than we
strictly need, but at least we won't miss anything. This also
zeroes the __hyp_running_vcpu pointer, which is always NULL
for a vcpu anyway.

Cc: stable@vger.kernel.org
Fixes: e47c2055c68e ("KVM: arm64: Make struct kvm_regs userspace-only")
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/reset.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index bd354cd45d28..ef1c49a1a3ad 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -240,8 +240,8 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
 		break;
 	}
 
-	/* Reset core registers */
-	memset(vcpu_gp_regs(vcpu), 0, sizeof(*vcpu_gp_regs(vcpu)));
+	/* Zero all registers */
+	memset(&vcpu->arch.ctxt, 0, sizeof(vcpu->arch.ctxt));
 	vcpu_gp_regs(vcpu)->pstate = pstate;
 
 	/* Reset system registers */
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH] KVM: arm64: Fully zero the vcpu state on reset
@ 2021-04-07 18:13 ` Marc Zyngier
  0 siblings, 0 replies; 9+ messages in thread
From: Marc Zyngier @ 2021-04-07 18:13 UTC (permalink / raw)
  To: linux-arm-kernel, kvm, kvmarm; +Cc: kernel-team, stable

On vcpu reset, we expect all the registers to be brought back
to their initial state, which happens to be a bunch of zeroes.

However, some recent commit broke this, and is now leaving a bunch
of registers (such as a FP state) with whatever was left by the
guest. My bad.

Just zero the whole vcpu context on reset. It is more than we
strictly need, but at least we won't miss anything. This also
zeroes the __hyp_running_vcpu pointer, which is always NULL
for a vcpu anyway.

Cc: stable@vger.kernel.org
Fixes: e47c2055c68e ("KVM: arm64: Make struct kvm_regs userspace-only")
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/reset.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index bd354cd45d28..ef1c49a1a3ad 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -240,8 +240,8 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
 		break;
 	}
 
-	/* Reset core registers */
-	memset(vcpu_gp_regs(vcpu), 0, sizeof(*vcpu_gp_regs(vcpu)));
+	/* Zero all registers */
+	memset(&vcpu->arch.ctxt, 0, sizeof(vcpu->arch.ctxt));
 	vcpu_gp_regs(vcpu)->pstate = pstate;
 
 	/* Reset system registers */
-- 
2.30.2

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH] KVM: arm64: Fully zero the vcpu state on reset
@ 2021-04-07 18:13 ` Marc Zyngier
  0 siblings, 0 replies; 9+ messages in thread
From: Marc Zyngier @ 2021-04-07 18:13 UTC (permalink / raw)
  To: linux-arm-kernel, kvm, kvmarm
  Cc: James Morse, Suzuki K Poulose, Alexandru Elisei, kernel-team, stable

On vcpu reset, we expect all the registers to be brought back
to their initial state, which happens to be a bunch of zeroes.

However, some recent commit broke this, and is now leaving a bunch
of registers (such as a FP state) with whatever was left by the
guest. My bad.

Just zero the whole vcpu context on reset. It is more than we
strictly need, but at least we won't miss anything. This also
zeroes the __hyp_running_vcpu pointer, which is always NULL
for a vcpu anyway.

Cc: stable@vger.kernel.org
Fixes: e47c2055c68e ("KVM: arm64: Make struct kvm_regs userspace-only")
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/reset.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index bd354cd45d28..ef1c49a1a3ad 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -240,8 +240,8 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
 		break;
 	}
 
-	/* Reset core registers */
-	memset(vcpu_gp_regs(vcpu), 0, sizeof(*vcpu_gp_regs(vcpu)));
+	/* Zero all registers */
+	memset(&vcpu->arch.ctxt, 0, sizeof(vcpu->arch.ctxt));
 	vcpu_gp_regs(vcpu)->pstate = pstate;
 
 	/* Reset system registers */
-- 
2.30.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] 9+ messages in thread

* Re: [PATCH] KVM: arm64: Fully zero the vcpu state on reset
  2021-04-07 18:13 ` Marc Zyngier
  (?)
@ 2021-04-08 15:36   ` Alexandru Elisei
  -1 siblings, 0 replies; 9+ messages in thread
From: Alexandru Elisei @ 2021-04-08 15:36 UTC (permalink / raw)
  To: Marc Zyngier, linux-arm-kernel, kvm, kvmarm
  Cc: James Morse, Suzuki K Poulose, kernel-team, stable

Hi Marc,

On 4/7/21 7:13 PM, Marc Zyngier wrote:
> On vcpu reset, we expect all the registers to be brought back
> to their initial state, which happens to be a bunch of zeroes.
>
> However, some recent commit broke this, and is now leaving a bunch
> of registers (such as a FP state) with whatever was left by the
> guest. My bad.
>
> Just zero the whole vcpu context on reset. It is more than we
> strictly need, but at least we won't miss anything. This also
> zeroes the __hyp_running_vcpu pointer, which is always NULL
> for a vcpu anyway.

Had a look at struct kvm_cpu_context and indeed the only field which doesn't
represent a guest register is __hyp_running_vcpu. Did a grep for all the places
where __hyp_running_vcpu is used, and indeed the assumption is that for a guest
the pointer is NULL, as __sysreg_restore_el1_state() relies on it.

>
> Cc: stable@vger.kernel.org
> Fixes: e47c2055c68e ("KVM: arm64: Make struct kvm_regs userspace-only")
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/reset.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> index bd354cd45d28..ef1c49a1a3ad 100644
> --- a/arch/arm64/kvm/reset.c
> +++ b/arch/arm64/kvm/reset.c
> @@ -240,8 +240,8 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
>  		break;
>  	}
>  
> -	/* Reset core registers */
> -	memset(vcpu_gp_regs(vcpu), 0, sizeof(*vcpu_gp_regs(vcpu)));
> +	/* Zero all registers */
> +	memset(&vcpu->arch.ctxt, 0, sizeof(vcpu->arch.ctxt));

Checked that code earlier in the function does not touch the guest registers from
vcpu->arch.ctxt, to make sure we're not overwriting other reset values by mistake.
Looks good to me:

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks,

Alex

>  	vcpu_gp_regs(vcpu)->pstate = pstate;
>  
>  	/* Reset system registers */

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] KVM: arm64: Fully zero the vcpu state on reset
@ 2021-04-08 15:36   ` Alexandru Elisei
  0 siblings, 0 replies; 9+ messages in thread
From: Alexandru Elisei @ 2021-04-08 15:36 UTC (permalink / raw)
  To: Marc Zyngier, linux-arm-kernel, kvm, kvmarm; +Cc: kernel-team, stable

Hi Marc,

On 4/7/21 7:13 PM, Marc Zyngier wrote:
> On vcpu reset, we expect all the registers to be brought back
> to their initial state, which happens to be a bunch of zeroes.
>
> However, some recent commit broke this, and is now leaving a bunch
> of registers (such as a FP state) with whatever was left by the
> guest. My bad.
>
> Just zero the whole vcpu context on reset. It is more than we
> strictly need, but at least we won't miss anything. This also
> zeroes the __hyp_running_vcpu pointer, which is always NULL
> for a vcpu anyway.

Had a look at struct kvm_cpu_context and indeed the only field which doesn't
represent a guest register is __hyp_running_vcpu. Did a grep for all the places
where __hyp_running_vcpu is used, and indeed the assumption is that for a guest
the pointer is NULL, as __sysreg_restore_el1_state() relies on it.

>
> Cc: stable@vger.kernel.org
> Fixes: e47c2055c68e ("KVM: arm64: Make struct kvm_regs userspace-only")
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/reset.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> index bd354cd45d28..ef1c49a1a3ad 100644
> --- a/arch/arm64/kvm/reset.c
> +++ b/arch/arm64/kvm/reset.c
> @@ -240,8 +240,8 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
>  		break;
>  	}
>  
> -	/* Reset core registers */
> -	memset(vcpu_gp_regs(vcpu), 0, sizeof(*vcpu_gp_regs(vcpu)));
> +	/* Zero all registers */
> +	memset(&vcpu->arch.ctxt, 0, sizeof(vcpu->arch.ctxt));

Checked that code earlier in the function does not touch the guest registers from
vcpu->arch.ctxt, to make sure we're not overwriting other reset values by mistake.
Looks good to me:

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks,

Alex

>  	vcpu_gp_regs(vcpu)->pstate = pstate;
>  
>  	/* Reset system registers */
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] KVM: arm64: Fully zero the vcpu state on reset
@ 2021-04-08 15:36   ` Alexandru Elisei
  0 siblings, 0 replies; 9+ messages in thread
From: Alexandru Elisei @ 2021-04-08 15:36 UTC (permalink / raw)
  To: Marc Zyngier, linux-arm-kernel, kvm, kvmarm
  Cc: James Morse, Suzuki K Poulose, kernel-team, stable

Hi Marc,

On 4/7/21 7:13 PM, Marc Zyngier wrote:
> On vcpu reset, we expect all the registers to be brought back
> to their initial state, which happens to be a bunch of zeroes.
>
> However, some recent commit broke this, and is now leaving a bunch
> of registers (such as a FP state) with whatever was left by the
> guest. My bad.
>
> Just zero the whole vcpu context on reset. It is more than we
> strictly need, but at least we won't miss anything. This also
> zeroes the __hyp_running_vcpu pointer, which is always NULL
> for a vcpu anyway.

Had a look at struct kvm_cpu_context and indeed the only field which doesn't
represent a guest register is __hyp_running_vcpu. Did a grep for all the places
where __hyp_running_vcpu is used, and indeed the assumption is that for a guest
the pointer is NULL, as __sysreg_restore_el1_state() relies on it.

>
> Cc: stable@vger.kernel.org
> Fixes: e47c2055c68e ("KVM: arm64: Make struct kvm_regs userspace-only")
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/reset.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> index bd354cd45d28..ef1c49a1a3ad 100644
> --- a/arch/arm64/kvm/reset.c
> +++ b/arch/arm64/kvm/reset.c
> @@ -240,8 +240,8 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
>  		break;
>  	}
>  
> -	/* Reset core registers */
> -	memset(vcpu_gp_regs(vcpu), 0, sizeof(*vcpu_gp_regs(vcpu)));
> +	/* Zero all registers */
> +	memset(&vcpu->arch.ctxt, 0, sizeof(vcpu->arch.ctxt));

Checked that code earlier in the function does not touch the guest registers from
vcpu->arch.ctxt, to make sure we're not overwriting other reset values by mistake.
Looks good to me:

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks,

Alex

>  	vcpu_gp_regs(vcpu)->pstate = pstate;
>  
>  	/* Reset system registers */

_______________________________________________
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] 9+ messages in thread

* Re: [PATCH] KVM: arm64: Fully zero the vcpu state on reset
  2021-04-08 15:36   ` Alexandru Elisei
  (?)
@ 2021-04-09 17:18     ` Marc Zyngier
  -1 siblings, 0 replies; 9+ messages in thread
From: Marc Zyngier @ 2021-04-09 17:18 UTC (permalink / raw)
  To: Alexandru Elisei
  Cc: linux-arm-kernel, kvm, kvmarm, James Morse, Suzuki K Poulose,
	kernel-team, stable, Will Deacon

On Thu, 08 Apr 2021 16:36:40 +0100,
Alexandru Elisei <alexandru.elisei@arm.com> wrote:
> 
> Hi Marc,
> 
> On 4/7/21 7:13 PM, Marc Zyngier wrote:
> > On vcpu reset, we expect all the registers to be brought back
> > to their initial state, which happens to be a bunch of zeroes.
> >
> > However, some recent commit broke this, and is now leaving a bunch
> > of registers (such as a FP state) with whatever was left by the
> > guest. My bad.
> >
> > Just zero the whole vcpu context on reset. It is more than we
> > strictly need, but at least we won't miss anything. This also
> > zeroes the __hyp_running_vcpu pointer, which is always NULL
> > for a vcpu anyway.
> 
> Had a look at struct kvm_cpu_context and indeed the only field which doesn't
> represent a guest register is __hyp_running_vcpu. Did a grep for all the places
> where __hyp_running_vcpu is used, and indeed the assumption is that for a guest
> the pointer is NULL, as __sysreg_restore_el1_state() relies on it.
> 
> >
> > Cc: stable@vger.kernel.org
> > Fixes: e47c2055c68e ("KVM: arm64: Make struct kvm_regs userspace-only")
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  arch/arm64/kvm/reset.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> > index bd354cd45d28..ef1c49a1a3ad 100644
> > --- a/arch/arm64/kvm/reset.c
> > +++ b/arch/arm64/kvm/reset.c
> > @@ -240,8 +240,8 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
> >  		break;
> >  	}
> >  
> > -	/* Reset core registers */
> > -	memset(vcpu_gp_regs(vcpu), 0, sizeof(*vcpu_gp_regs(vcpu)));
> > +	/* Zero all registers */
> > +	memset(&vcpu->arch.ctxt, 0, sizeof(vcpu->arch.ctxt));
> 
> Checked that code earlier in the function does not touch the guest
> registers from vcpu->arch.ctxt, to make sure we're not overwriting
> other reset values by mistake.
> Looks good to me:
> 
> Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Scratch that, this is breaks the setting of CNTVOFF, which gets
populated when we create the vcpu. The gotcha is that creating a vcpu
resets CNTVOFF for *all* vcpus:

* If the VMM creates all vcpus, then reset them all, this works
  "fine": all the vcpus have CNTVOFF==0, which is an acceptable
  departure from the current behaviour (where vtime starts at 0).

* If the VMM alternates vcpu creation and reset, then the last vcpu
  ends up with a CNTVOFF set to 0, while all the others have a
  different offset.

QEMU does the former, and kvmtool the latter. Thanks to Will for the
heads up. I'll drop the patch from -next and post a v2 shortly.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] KVM: arm64: Fully zero the vcpu state on reset
@ 2021-04-09 17:18     ` Marc Zyngier
  0 siblings, 0 replies; 9+ messages in thread
From: Marc Zyngier @ 2021-04-09 17:18 UTC (permalink / raw)
  To: Alexandru Elisei
  Cc: kvm, Will Deacon, stable, kernel-team, kvmarm, linux-arm-kernel

On Thu, 08 Apr 2021 16:36:40 +0100,
Alexandru Elisei <alexandru.elisei@arm.com> wrote:
> 
> Hi Marc,
> 
> On 4/7/21 7:13 PM, Marc Zyngier wrote:
> > On vcpu reset, we expect all the registers to be brought back
> > to their initial state, which happens to be a bunch of zeroes.
> >
> > However, some recent commit broke this, and is now leaving a bunch
> > of registers (such as a FP state) with whatever was left by the
> > guest. My bad.
> >
> > Just zero the whole vcpu context on reset. It is more than we
> > strictly need, but at least we won't miss anything. This also
> > zeroes the __hyp_running_vcpu pointer, which is always NULL
> > for a vcpu anyway.
> 
> Had a look at struct kvm_cpu_context and indeed the only field which doesn't
> represent a guest register is __hyp_running_vcpu. Did a grep for all the places
> where __hyp_running_vcpu is used, and indeed the assumption is that for a guest
> the pointer is NULL, as __sysreg_restore_el1_state() relies on it.
> 
> >
> > Cc: stable@vger.kernel.org
> > Fixes: e47c2055c68e ("KVM: arm64: Make struct kvm_regs userspace-only")
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  arch/arm64/kvm/reset.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> > index bd354cd45d28..ef1c49a1a3ad 100644
> > --- a/arch/arm64/kvm/reset.c
> > +++ b/arch/arm64/kvm/reset.c
> > @@ -240,8 +240,8 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
> >  		break;
> >  	}
> >  
> > -	/* Reset core registers */
> > -	memset(vcpu_gp_regs(vcpu), 0, sizeof(*vcpu_gp_regs(vcpu)));
> > +	/* Zero all registers */
> > +	memset(&vcpu->arch.ctxt, 0, sizeof(vcpu->arch.ctxt));
> 
> Checked that code earlier in the function does not touch the guest
> registers from vcpu->arch.ctxt, to make sure we're not overwriting
> other reset values by mistake.
> Looks good to me:
> 
> Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Scratch that, this is breaks the setting of CNTVOFF, which gets
populated when we create the vcpu. The gotcha is that creating a vcpu
resets CNTVOFF for *all* vcpus:

* If the VMM creates all vcpus, then reset them all, this works
  "fine": all the vcpus have CNTVOFF==0, which is an acceptable
  departure from the current behaviour (where vtime starts at 0).

* If the VMM alternates vcpu creation and reset, then the last vcpu
  ends up with a CNTVOFF set to 0, while all the others have a
  different offset.

QEMU does the former, and kvmtool the latter. Thanks to Will for the
heads up. I'll drop the patch from -next and post a v2 shortly.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] KVM: arm64: Fully zero the vcpu state on reset
@ 2021-04-09 17:18     ` Marc Zyngier
  0 siblings, 0 replies; 9+ messages in thread
From: Marc Zyngier @ 2021-04-09 17:18 UTC (permalink / raw)
  To: Alexandru Elisei
  Cc: linux-arm-kernel, kvm, kvmarm, James Morse, Suzuki K Poulose,
	kernel-team, stable, Will Deacon

On Thu, 08 Apr 2021 16:36:40 +0100,
Alexandru Elisei <alexandru.elisei@arm.com> wrote:
> 
> Hi Marc,
> 
> On 4/7/21 7:13 PM, Marc Zyngier wrote:
> > On vcpu reset, we expect all the registers to be brought back
> > to their initial state, which happens to be a bunch of zeroes.
> >
> > However, some recent commit broke this, and is now leaving a bunch
> > of registers (such as a FP state) with whatever was left by the
> > guest. My bad.
> >
> > Just zero the whole vcpu context on reset. It is more than we
> > strictly need, but at least we won't miss anything. This also
> > zeroes the __hyp_running_vcpu pointer, which is always NULL
> > for a vcpu anyway.
> 
> Had a look at struct kvm_cpu_context and indeed the only field which doesn't
> represent a guest register is __hyp_running_vcpu. Did a grep for all the places
> where __hyp_running_vcpu is used, and indeed the assumption is that for a guest
> the pointer is NULL, as __sysreg_restore_el1_state() relies on it.
> 
> >
> > Cc: stable@vger.kernel.org
> > Fixes: e47c2055c68e ("KVM: arm64: Make struct kvm_regs userspace-only")
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  arch/arm64/kvm/reset.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> > index bd354cd45d28..ef1c49a1a3ad 100644
> > --- a/arch/arm64/kvm/reset.c
> > +++ b/arch/arm64/kvm/reset.c
> > @@ -240,8 +240,8 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
> >  		break;
> >  	}
> >  
> > -	/* Reset core registers */
> > -	memset(vcpu_gp_regs(vcpu), 0, sizeof(*vcpu_gp_regs(vcpu)));
> > +	/* Zero all registers */
> > +	memset(&vcpu->arch.ctxt, 0, sizeof(vcpu->arch.ctxt));
> 
> Checked that code earlier in the function does not touch the guest
> registers from vcpu->arch.ctxt, to make sure we're not overwriting
> other reset values by mistake.
> Looks good to me:
> 
> Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Scratch that, this is breaks the setting of CNTVOFF, which gets
populated when we create the vcpu. The gotcha is that creating a vcpu
resets CNTVOFF for *all* vcpus:

* If the VMM creates all vcpus, then reset them all, this works
  "fine": all the vcpus have CNTVOFF==0, which is an acceptable
  departure from the current behaviour (where vtime starts at 0).

* If the VMM alternates vcpu creation and reset, then the last vcpu
  ends up with a CNTVOFF set to 0, while all the others have a
  different offset.

QEMU does the former, and kvmtool the latter. Thanks to Will for the
heads up. I'll drop the patch from -next and post a v2 shortly.

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] 9+ messages in thread

end of thread, other threads:[~2021-04-09 17:20 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-07 18:13 [PATCH] KVM: arm64: Fully zero the vcpu state on reset Marc Zyngier
2021-04-07 18:13 ` Marc Zyngier
2021-04-07 18:13 ` Marc Zyngier
2021-04-08 15:36 ` Alexandru Elisei
2021-04-08 15:36   ` Alexandru Elisei
2021-04-08 15:36   ` Alexandru Elisei
2021-04-09 17:18   ` Marc Zyngier
2021-04-09 17:18     ` Marc Zyngier
2021-04-09 17:18     ` 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.