Re: [PATCH] powerpc/kvm: Fix kvmppc_vcore->in_guest value in kvmhv_switch_to_host
On Fri, 2019-10-04 at 02:53:17 UTC, Jordan Niethe wrote:
> kvmhv_switch_to_host() in arch/powerpc/kvm/book3s_hv_rmhandlers.S needs
> to set kvmppc_vcore->in_guest to 0 to signal secondary CPUs to continue.
> This happens after resetting the PCR. Before commit 13c7bb3c57dc
> ("powerpc/64s: Set reserved PCR bits"), r0 would always be 0 before it
> was stored to kvmppc_vcore->in_guest. However because of this change in
> the commit:
>
> /* Reset PCR */
> ld r0, VCORE_PCR(r5)
> - cmpdi r0, 0
> + LOAD_REG_IMMEDIATE(r6, PCR_MASK)
> + cmpld r0, r6
> beq 18f
> - li r0, 0
> - mtspr SPRN_PCR, r0
> + mtspr SPRN_PCR, r6
> 18:
> /* Signal secondary CPUs to continue */
> stb r0,VCORE_IN_GUEST(r5)
>
> We are no longer comparing r0 against 0 and loading it with 0 if it
> contains something else. Hence when we store r0 to
> kvmppc_vcore->in_guest, it might not be 0. This means that secondary
> CPUs will not be signalled to continue. Those CPUs get stuck and errors
> like the following are logged:
>
> KVM: CPU 1 seems to be stuck
> KVM: CPU 2 seems to be stuck
> KVM: CPU 3 seems to be stuck
> KVM: CPU 4 seems to be stuck
> KVM: CPU 5 seems to be stuck
> KVM: CPU 6 seems to be stuck
> KVM: CPU 7 seems to be stuck
>
> This can be reproduced with:
> $ for i in `seq 1 7` ; do chcpu -d $i ; done ;
> $ taskset -c 0 qemu-system-ppc64 -smp 8,threads=8 \
>-M pseries,accel=kvm,kvm-type=HV -m 1G -nographic -vga none \
>-kernel vmlinux -initrd initrd.cpio.xz
>
> Fix by making sure r0 is 0 before storing it to kvmppc_vcore->in_guest.
>
> Fixes: 13c7bb3c57dc ("powerpc/64s: Set reserved PCR bits")
> Reported-by: Alexey Kardashevskiy
> Signed-off-by: Jordan Niethe
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/7fe4e1176dfe47a243d8edd98d26abd11f91b042
cheers
Re: [PATCH] powerpc/kvm: Fix kvmppc_vcore->in_guest value in kvmhv_switch_to_host
On Friday, 4 October 2019 12:53:17 PM AEDT Jordan Niethe wrote:
> kvmhv_switch_to_host() in arch/powerpc/kvm/book3s_hv_rmhandlers.S needs
> to set kvmppc_vcore->in_guest to 0 to signal secondary CPUs to continue.
> This happens after resetting the PCR. Before commit 13c7bb3c57dc
> ("powerpc/64s: Set reserved PCR bits"), r0 would always be 0 before it
> was stored to kvmppc_vcore->in_guest. However because of this change in
> the commit:
>
> /* Reset PCR */
> ld r0, VCORE_PCR(r5)
> - cmpdi r0, 0
> + LOAD_REG_IMMEDIATE(r6, PCR_MASK)
> + cmpld r0, r6
> beq 18f
> - li r0, 0
> - mtspr SPRN_PCR, r0
> + mtspr SPRN_PCR, r6
> 18:
> /* Signal secondary CPUs to continue */
> stb r0,VCORE_IN_GUEST(r5)
Easy to understand how that was missed :-)
Reviewed-by: Alistair Popple
> We are no longer comparing r0 against 0 and loading it with 0 if it
> contains something else. Hence when we store r0 to
> kvmppc_vcore->in_guest, it might not be 0. This means that secondary
> CPUs will not be signalled to continue. Those CPUs get stuck and errors
> like the following are logged:
>
> KVM: CPU 1 seems to be stuck
> KVM: CPU 2 seems to be stuck
> KVM: CPU 3 seems to be stuck
> KVM: CPU 4 seems to be stuck
> KVM: CPU 5 seems to be stuck
> KVM: CPU 6 seems to be stuck
> KVM: CPU 7 seems to be stuck
>
> This can be reproduced with:
> $ for i in `seq 1 7` ; do chcpu -d $i ; done ;
> $ taskset -c 0 qemu-system-ppc64 -smp 8,threads=8 \
>-M pseries,accel=kvm,kvm-type=HV -m 1G -nographic -vga none \
>-kernel vmlinux -initrd initrd.cpio.xz
>
> Fix by making sure r0 is 0 before storing it to kvmppc_vcore->in_guest.
>
> Fixes: 13c7bb3c57dc ("powerpc/64s: Set reserved PCR bits")
> Reported-by: Alexey Kardashevskiy
> Signed-off-by: Jordan Niethe
> ---
> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> index 74a9cfe84aee..faebcbb8c4db 100644
> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> @@ -1921,6 +1921,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
> mtspr SPRN_PCR, r6
> 18:
> /* Signal secondary CPUs to continue */
> + li r0, 0
> stb r0,VCORE_IN_GUEST(r5)
> 19: lis r8,0x7fff /* MAX_INT@h */
> mtspr SPRN_HDEC,r8
>
Re: [PATCH] powerpc/kvm: Fix kvmppc_vcore->in_guest value in kvmhv_switch_to_host
On 04/10/2019 12:53, Jordan Niethe wrote:
> kvmhv_switch_to_host() in arch/powerpc/kvm/book3s_hv_rmhandlers.S needs
> to set kvmppc_vcore->in_guest to 0 to signal secondary CPUs to continue.
> This happens after resetting the PCR. Before commit 13c7bb3c57dc
> ("powerpc/64s: Set reserved PCR bits"), r0 would always be 0 before it
> was stored to kvmppc_vcore->in_guest. However because of this change in
> the commit:
>
> /* Reset PCR */
> ld r0, VCORE_PCR(r5)
> - cmpdi r0, 0
> + LOAD_REG_IMMEDIATE(r6, PCR_MASK)
> + cmpld r0, r6
> beq 18f
> - li r0, 0
> - mtspr SPRN_PCR, r0
> + mtspr SPRN_PCR, r6
> 18:
> /* Signal secondary CPUs to continue */
> stb r0,VCORE_IN_GUEST(r5)
>
> We are no longer comparing r0 against 0 and loading it with 0 if it
> contains something else. Hence when we store r0 to
> kvmppc_vcore->in_guest, it might not be 0. This means that secondary
> CPUs will not be signalled to continue. Those CPUs get stuck and errors
> like the following are logged:
>
> KVM: CPU 1 seems to be stuck
> KVM: CPU 2 seems to be stuck
> KVM: CPU 3 seems to be stuck
> KVM: CPU 4 seems to be stuck
> KVM: CPU 5 seems to be stuck
> KVM: CPU 6 seems to be stuck
> KVM: CPU 7 seems to be stuck
>
> This can be reproduced with:
> $ for i in `seq 1 7` ; do chcpu -d $i ; done ;
> $ taskset -c 0 qemu-system-ppc64 -smp 8,threads=8 \
>-M pseries,accel=kvm,kvm-type=HV -m 1G -nographic -vga none \
>-kernel vmlinux -initrd initrd.cpio.xz
>
> Fix by making sure r0 is 0 before storing it to kvmppc_vcore->in_guest.
>
> Fixes: 13c7bb3c57dc ("powerpc/64s: Set reserved PCR bits")
> Reported-by: Alexey Kardashevskiy
> Signed-off-by: Jordan Niethe
Tested-by: Alexey Kardashevskiy
> ---
> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> index 74a9cfe84aee..faebcbb8c4db 100644
> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> @@ -1921,6 +1921,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
> mtspr SPRN_PCR, r6
> 18:
> /* Signal secondary CPUs to continue */
> + li r0, 0
> stb r0,VCORE_IN_GUEST(r5)
> 19: lis r8,0x7fff /* MAX_INT@h */
> mtspr SPRN_HDEC,r8
>
--
Alexey
Re: [PATCH] powerpc/kvm: Fix kvmppc_vcore->in_guest value in kvmhv_switch_to_host
Reviewed-by: Alistair Popple
On Friday, 4 October 2019 12:53:17 PM AEST Jordan Niethe wrote:
> kvmhv_switch_to_host() in arch/powerpc/kvm/book3s_hv_rmhandlers.S needs
> to set kvmppc_vcore->in_guest to 0 to signal secondary CPUs to continue.
> This happens after resetting the PCR. Before commit 13c7bb3c57dc
> ("powerpc/64s: Set reserved PCR bits"), r0 would always be 0 before it
> was stored to kvmppc_vcore->in_guest. However because of this change in
> the commit:
>
> /* Reset PCR */
> ld r0, VCORE_PCR(r5)
> - cmpdi r0, 0
> + LOAD_REG_IMMEDIATE(r6, PCR_MASK)
> + cmpld r0, r6
> beq 18f
> - li r0, 0
> - mtspr SPRN_PCR, r0
> + mtspr SPRN_PCR, r6
> 18:
> /* Signal secondary CPUs to continue */
> stb r0,VCORE_IN_GUEST(r5)
>
> We are no longer comparing r0 against 0 and loading it with 0 if it
> contains something else. Hence when we store r0 to
> kvmppc_vcore->in_guest, it might not be 0. This means that secondary
> CPUs will not be signalled to continue. Those CPUs get stuck and errors
> like the following are logged:
>
> KVM: CPU 1 seems to be stuck
> KVM: CPU 2 seems to be stuck
> KVM: CPU 3 seems to be stuck
> KVM: CPU 4 seems to be stuck
> KVM: CPU 5 seems to be stuck
> KVM: CPU 6 seems to be stuck
> KVM: CPU 7 seems to be stuck
>
> This can be reproduced with:
> $ for i in `seq 1 7` ; do chcpu -d $i ; done ;
> $ taskset -c 0 qemu-system-ppc64 -smp 8,threads=8 \
>-M pseries,accel=kvm,kvm-type=HV -m 1G -nographic -vga none \
>-kernel vmlinux -initrd initrd.cpio.xz
>
> Fix by making sure r0 is 0 before storing it to kvmppc_vcore->in_guest.
>
> Fixes: 13c7bb3c57dc ("powerpc/64s: Set reserved PCR bits")
> Reported-by: Alexey Kardashevskiy
> Signed-off-by: Jordan Niethe
> ---
> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> index 74a9cfe84aee..faebcbb8c4db 100644
> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> @@ -1921,6 +1921,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
> mtspr SPRN_PCR, r6
> 18:
> /* Signal secondary CPUs to continue */
> + li r0, 0
> stb r0,VCORE_IN_GUEST(r5)
> 19: lis r8,0x7fff /* MAX_INT@h */
> mtspr SPRN_HDEC,r8
>
[PATCH] powerpc/kvm: Fix kvmppc_vcore->in_guest value in kvmhv_switch_to_host
kvmhv_switch_to_host() in arch/powerpc/kvm/book3s_hv_rmhandlers.S needs
to set kvmppc_vcore->in_guest to 0 to signal secondary CPUs to continue.
This happens after resetting the PCR. Before commit 13c7bb3c57dc
("powerpc/64s: Set reserved PCR bits"), r0 would always be 0 before it
was stored to kvmppc_vcore->in_guest. However because of this change in
the commit:
/* Reset PCR */
ld r0, VCORE_PCR(r5)
- cmpdi r0, 0
+ LOAD_REG_IMMEDIATE(r6, PCR_MASK)
+ cmpld r0, r6
beq 18f
- li r0, 0
- mtspr SPRN_PCR, r0
+ mtspr SPRN_PCR, r6
18:
/* Signal secondary CPUs to continue */
stb r0,VCORE_IN_GUEST(r5)
We are no longer comparing r0 against 0 and loading it with 0 if it
contains something else. Hence when we store r0 to
kvmppc_vcore->in_guest, it might not be 0. This means that secondary
CPUs will not be signalled to continue. Those CPUs get stuck and errors
like the following are logged:
KVM: CPU 1 seems to be stuck
KVM: CPU 2 seems to be stuck
KVM: CPU 3 seems to be stuck
KVM: CPU 4 seems to be stuck
KVM: CPU 5 seems to be stuck
KVM: CPU 6 seems to be stuck
KVM: CPU 7 seems to be stuck
This can be reproduced with:
$ for i in `seq 1 7` ; do chcpu -d $i ; done ;
$ taskset -c 0 qemu-system-ppc64 -smp 8,threads=8 \
-M pseries,accel=kvm,kvm-type=HV -m 1G -nographic -vga none \
-kernel vmlinux -initrd initrd.cpio.xz
Fix by making sure r0 is 0 before storing it to kvmppc_vcore->in_guest.
Fixes: 13c7bb3c57dc ("powerpc/64s: Set reserved PCR bits")
Reported-by: Alexey Kardashevskiy
Signed-off-by: Jordan Niethe
---
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index 74a9cfe84aee..faebcbb8c4db 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -1921,6 +1921,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
mtspr SPRN_PCR, r6
18:
/* Signal secondary CPUs to continue */
+ li r0, 0
stb r0,VCORE_IN_GUEST(r5)
19:lis r8,0x7fff /* MAX_INT@h */
mtspr SPRN_HDEC,r8
--
2.20.1
