* [PATCH 1/2] KVM: PPC: Book3S HV: Fix r3 corruption in h_set_dabr()
2019-06-17 7:16 [PATCH 0/2] Fix handling of h_set_dawr Suraj Jitindar Singh
@ 2019-06-17 7:16 ` Suraj Jitindar Singh
2019-06-19 12:36 ` Michael Ellerman
2019-06-17 7:16 ` [PATCH 2/2] KVM: PPC: Book3S HV: Only write DAWR[X] when handling h_set_dawr in real mode Suraj Jitindar Singh
2019-06-17 9:06 ` [PATCH 0/2] Fix handling of h_set_dawr Cédric Le Goater
2 siblings, 1 reply; 6+ messages in thread
From: Suraj Jitindar Singh @ 2019-06-17 7:16 UTC (permalink / raw)
To: linuxppc-dev; +Cc: mikey, clg, kvm-ppc
From: Michael Neuling <mikey@neuling.org>
Commit c1fe190c0672 ("powerpc: Add force enable of DAWR on P9
option") screwed up some assembler and corrupted a pointer in
r3. This resulted in crashes like the below:
[ 44.374746] BUG: Kernel NULL pointer dereference at 0x000013bf
[ 44.374848] Faulting instruction address: 0xc00000000010b044
[ 44.374906] Oops: Kernel access of bad area, sig: 11 [#1]
[ 44.374951] LE PAGE_SIZE=64K MMU=Radix MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
[ 44.375018] Modules linked in: vhost_net vhost tap xt_CHECKSUM iptable_mangle xt_MASQUERADE iptable_nat nf_nat xt_conntrack nf_conntrack nf_defrag_ipv6 libcrc32c nf_defrag_ipv4 ipt_REJECT nf_reject_ipv4 xt_tcpudp bridge stp llc ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter bpfilter vmx_crypto crct10dif_vpmsum crc32c_vpmsum kvm_hv kvm sch_fq_codel ip_tables x_tables autofs4 virtio_net net_failover virtio_scsi failover
[ 44.375401] CPU: 8 PID: 1771 Comm: qemu-system-ppc Kdump: loaded Not tainted 5.2.0-rc4+ #3
[ 44.375500] NIP: c00000000010b044 LR: c0080000089dacf4 CTR: c00000000010aff4
[ 44.375604] REGS: c00000179b397710 TRAP: 0300 Not tainted (5.2.0-rc4+)
[ 44.375691] MSR: 800000000280b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE> CR: 42244842 XER: 00000000
[ 44.375815] CFAR: c00000000010aff8 DAR: 00000000000013bf DSISR: 42000000 IRQMASK: 0
[ 44.375815] GPR00: c0080000089dd6bc c00000179b3979a0 c008000008a04300 ffffffffffffffff
[ 44.375815] GPR04: 0000000000000000 0000000000000003 000000002444b05d c0000017f11c45d0
[ 44.375815] GPR08: 078000003e018dfe 0000000000000028 0000000000000001 0000000000000075
[ 44.375815] GPR12: c00000000010aff4 c000000007ff6300 0000000000000000 0000000000000000
[ 44.375815] GPR16: 0000000000000000 c0000017f11d0000 00000000ffffffff c0000017f11ca7a8
[ 44.375815] GPR20: c0000017f11c42ec ffffffffffffffff 0000000000000000 000000000000000a
[ 44.375815] GPR24: fffffffffffffffc 0000000000000000 c0000017f11c0000 c000000001a77ed8
[ 44.375815] GPR28: c00000179af70000 fffffffffffffffc c0080000089ff170 c00000179ae88540
[ 44.376673] NIP [c00000000010b044] kvmppc_h_set_dabr+0x50/0x68
[ 44.376754] LR [c0080000089dacf4] kvmppc_pseries_do_hcall+0xa3c/0xeb0 [kvm_hv]
[ 44.376849] Call Trace:
[ 44.376886] [c00000179b3979a0] [c0000017f11c0000] 0xc0000017f11c0000 (unreliable)
[ 44.376982] [c00000179b397a10] [c0080000089dd6bc] kvmppc_vcpu_run_hv+0x694/0xec0 [kvm_hv]
[ 44.377084] [c00000179b397ae0] [c0080000093f8bcc] kvmppc_vcpu_run+0x34/0x48 [kvm]
[ 44.377185] [c00000179b397b00] [c0080000093f522c] kvm_arch_vcpu_ioctl_run+0x2f4/0x400 [kvm]
[ 44.377286] [c00000179b397b90] [c0080000093e3618] kvm_vcpu_ioctl+0x460/0x850 [kvm]
[ 44.377384] [c00000179b397d00] [c0000000004ba6c4] do_vfs_ioctl+0xe4/0xb40
[ 44.377464] [c00000179b397db0] [c0000000004bb1e4] ksys_ioctl+0xc4/0x110
[ 44.377547] [c00000179b397e00] [c0000000004bb258] sys_ioctl+0x28/0x80
[ 44.377628] [c00000179b397e20] [c00000000000b888] system_call+0x5c/0x70
[ 44.377712] Instruction dump:
[ 44.377765] 4082fff4 4c00012c 38600000 4e800020 e96280c0 896b0000 2c2b0000 3860ffff
[ 44.377862] 4d820020 50852e74 508516f6 78840724 <f88313c0> f8a313c8 7c942ba6 7cbc2ba6
Fix the bug by only changing r3 when we are returning immediately.
Fixes: c1fe190c0672 ("powerpc: Add force enable of DAWR on P9 option")
Signed-off-by: Michael Neuling <mikey@neuling.org>
Reported-by: Cédric Le Goater <clg@kaod.org>
--
mpe: This is for 5.2 fixes
v2: Review from Christophe Leroy
- De-Mikey/Cedric-ify commit message
- Add "Fixes:"
- Other trivial commit messages changes
- No code change
---
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index d885a5831daa..703cd6cd994d 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -2500,8 +2500,10 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
LOAD_REG_ADDR(r11, dawr_force_enable)
lbz r11, 0(r11)
cmpdi r11, 0
+ bne 3f
li r3, H_HARDWARE
- beqlr
+ blr
+3:
/* Emulate H_SET_DABR/X on P8 for the sake of compat mode guests */
rlwimi r5, r4, 5, DAWRX_DR | DAWRX_DW
rlwimi r5, r4, 2, DAWRX_WT
--
2.13.6
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] KVM: PPC: Book3S HV: Fix r3 corruption in h_set_dabr()
2019-06-17 7:16 ` [PATCH 1/2] KVM: PPC: Book3S HV: Fix r3 corruption in h_set_dabr() Suraj Jitindar Singh
@ 2019-06-19 12:36 ` Michael Ellerman
0 siblings, 0 replies; 6+ messages in thread
From: Michael Ellerman @ 2019-06-19 12:36 UTC (permalink / raw)
To: Suraj Jitindar Singh, linuxppc-dev; +Cc: mikey, clg, kvm-ppc
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 3799 bytes --]
On Mon, 2019-06-17 at 07:16:18 UTC, Suraj Jitindar Singh wrote:
> From: Michael Neuling <mikey@neuling.org>
>
> Commit c1fe190c0672 ("powerpc: Add force enable of DAWR on P9
> option") screwed up some assembler and corrupted a pointer in
> r3. This resulted in crashes like the below:
>
> [ 44.374746] BUG: Kernel NULL pointer dereference at 0x000013bf
> [ 44.374848] Faulting instruction address: 0xc00000000010b044
> [ 44.374906] Oops: Kernel access of bad area, sig: 11 [#1]
> [ 44.374951] LE PAGE_SIZE=64K MMU=Radix MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
> [ 44.375018] Modules linked in: vhost_net vhost tap xt_CHECKSUM iptable_mangle xt_MASQUERADE iptable_nat nf_nat xt_conntrack nf_conntrack nf_defrag_ipv6 libcrc32c nf_defrag_ipv4 ipt_REJECT nf_reject_ipv4 xt_tcpudp bridge stp llc ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter bpfilter vmx_crypto crct10dif_vpmsum crc32c_vpmsum kvm_hv kvm sch_fq_codel ip_tables x_tables autofs4 virtio_net net_failover virtio_scsi failover
> [ 44.375401] CPU: 8 PID: 1771 Comm: qemu-system-ppc Kdump: loaded Not tainted 5.2.0-rc4+ #3
> [ 44.375500] NIP: c00000000010b044 LR: c0080000089dacf4 CTR: c00000000010aff4
> [ 44.375604] REGS: c00000179b397710 TRAP: 0300 Not tainted (5.2.0-rc4+)
> [ 44.375691] MSR: 800000000280b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE> CR: 42244842 XER: 00000000
> [ 44.375815] CFAR: c00000000010aff8 DAR: 00000000000013bf DSISR: 42000000 IRQMASK: 0
> [ 44.375815] GPR00: c0080000089dd6bc c00000179b3979a0 c008000008a04300 ffffffffffffffff
> [ 44.375815] GPR04: 0000000000000000 0000000000000003 000000002444b05d c0000017f11c45d0
> [ 44.375815] GPR08: 078000003e018dfe 0000000000000028 0000000000000001 0000000000000075
> [ 44.375815] GPR12: c00000000010aff4 c000000007ff6300 0000000000000000 0000000000000000
> [ 44.375815] GPR16: 0000000000000000 c0000017f11d0000 00000000ffffffff c0000017f11ca7a8
> [ 44.375815] GPR20: c0000017f11c42ec ffffffffffffffff 0000000000000000 000000000000000a
> [ 44.375815] GPR24: fffffffffffffffc 0000000000000000 c0000017f11c0000 c000000001a77ed8
> [ 44.375815] GPR28: c00000179af70000 fffffffffffffffc c0080000089ff170 c00000179ae88540
> [ 44.376673] NIP [c00000000010b044] kvmppc_h_set_dabr+0x50/0x68
> [ 44.376754] LR [c0080000089dacf4] kvmppc_pseries_do_hcall+0xa3c/0xeb0 [kvm_hv]
> [ 44.376849] Call Trace:
> [ 44.376886] [c00000179b3979a0] [c0000017f11c0000] 0xc0000017f11c0000 (unreliable)
> [ 44.376982] [c00000179b397a10] [c0080000089dd6bc] kvmppc_vcpu_run_hv+0x694/0xec0 [kvm_hv]
> [ 44.377084] [c00000179b397ae0] [c0080000093f8bcc] kvmppc_vcpu_run+0x34/0x48 [kvm]
> [ 44.377185] [c00000179b397b00] [c0080000093f522c] kvm_arch_vcpu_ioctl_run+0x2f4/0x400 [kvm]
> [ 44.377286] [c00000179b397b90] [c0080000093e3618] kvm_vcpu_ioctl+0x460/0x850 [kvm]
> [ 44.377384] [c00000179b397d00] [c0000000004ba6c4] do_vfs_ioctl+0xe4/0xb40
> [ 44.377464] [c00000179b397db0] [c0000000004bb1e4] ksys_ioctl+0xc4/0x110
> [ 44.377547] [c00000179b397e00] [c0000000004bb258] sys_ioctl+0x28/0x80
> [ 44.377628] [c00000179b397e20] [c00000000000b888] system_call+0x5c/0x70
> [ 44.377712] Instruction dump:
> [ 44.377765] 4082fff4 4c00012c 38600000 4e800020 e96280c0 896b0000 2c2b0000 3860ffff
> [ 44.377862] 4d820020 50852e74 508516f6 78840724 <f88313c0> f8a313c8 7c942ba6 7cbc2ba6
>
> Fix the bug by only changing r3 when we are returning immediately.
>
> Fixes: c1fe190c0672 ("powerpc: Add force enable of DAWR on P9 option")
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> Reported-by: Cédric Le Goater <clg@kaod.org>
Series applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/fabb2efcf0846e28b4910fc20bdc203d3d0170af
cheers
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] KVM: PPC: Book3S HV: Only write DAWR[X] when handling h_set_dawr in real mode
2019-06-17 7:16 [PATCH 0/2] Fix handling of h_set_dawr Suraj Jitindar Singh
2019-06-17 7:16 ` [PATCH 1/2] KVM: PPC: Book3S HV: Fix r3 corruption in h_set_dabr() Suraj Jitindar Singh
@ 2019-06-17 7:16 ` Suraj Jitindar Singh
2019-06-17 9:06 ` [PATCH 0/2] Fix handling of h_set_dawr Cédric Le Goater
2 siblings, 0 replies; 6+ messages in thread
From: Suraj Jitindar Singh @ 2019-06-17 7:16 UTC (permalink / raw)
To: linuxppc-dev; +Cc: mikey, kvm-ppc, clg, Suraj Jitindar Singh
The hcall H_SET_DAWR is used by a guest to set the data address
watchpoint register (DAWR). This hcall is handled in the host in
kvmppc_h_set_dawr() which can be called in either real mode on the guest
exit path from hcall_try_real_mode() in book3s_hv_rmhandlers.S, or in
virtual mode when called from kvmppc_pseries_do_hcall() in book3s_hv.c.
The function kvmppc_h_set_dawr updates the dawr and dawrx fields in the
vcpu struct accordingly and then also writes the respective values into
the DAWR and DAWRX registers directly. It is necessary to write the
registers directly here when calling the function in real mode since the
path to re-enter the guest won't do this. However when in virtual mode
the host DAWR and DAWRX values have already been restored, and so writing
the registers would overwrite these. Additionally there is no reason to
write the guest values here as these will be read from the vcpu struct
and written to the registers appropriately the next time the vcpu is
run.
This also avoids the case when handling h_set_dawr for a nested guest
where the guest hypervisor isn't able to write the DAWR and DAWRX
registers directly and must rely on the real hypervisor to do this for
it when it calls H_ENTER_NESTED.
Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index 703cd6cd994d..337e64468d78 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -2510,9 +2510,18 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
clrrdi r4, r4, 3
std r4, VCPU_DAWR(r3)
std r5, VCPU_DAWRX(r3)
+ /*
+ * If came in through the real mode hcall handler then it is necessary
+ * to write the registers since the return path won't. Otherwise it is
+ * sufficient to store then in the vcpu struct as they will be loaded
+ * next time the vcpu is run.
+ */
+ mfmsr r6
+ andi. r6, r6, MSR_DR /* in real mode? */
+ bne 4f
mtspr SPRN_DAWR, r4
mtspr SPRN_DAWRX, r5
- li r3, 0
+4: li r3, 0
blr
_GLOBAL(kvmppc_h_cede) /* r3 = vcpu pointer, r11 = msr, r13 = paca */
--
2.13.6
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] Fix handling of h_set_dawr
2019-06-17 7:16 [PATCH 0/2] Fix handling of h_set_dawr Suraj Jitindar Singh
2019-06-17 7:16 ` [PATCH 1/2] KVM: PPC: Book3S HV: Fix r3 corruption in h_set_dabr() Suraj Jitindar Singh
2019-06-17 7:16 ` [PATCH 2/2] KVM: PPC: Book3S HV: Only write DAWR[X] when handling h_set_dawr in real mode Suraj Jitindar Singh
@ 2019-06-17 9:06 ` Cédric Le Goater
2019-06-20 1:45 ` Suraj Jitindar Singh
2 siblings, 1 reply; 6+ messages in thread
From: Cédric Le Goater @ 2019-06-17 9:06 UTC (permalink / raw)
To: Suraj Jitindar Singh, linuxppc-dev; +Cc: mikey, kvm-ppc
On 17/06/2019 09:16, Suraj Jitindar Singh wrote:
> Series contains 2 patches to fix the host in kernel handling of the hcall
> h_set_dawr.
>
> First patch from Michael Neuling is just a resend added here for clarity.
>
> Michael Neuling (1):
> KVM: PPC: Book3S HV: Fix r3 corruption in h_set_dabr()
>
> Suraj Jitindar Singh (1):
> KVM: PPC: Book3S HV: Only write DAWR[X] when handling h_set_dawr in
> real mode
Reviewed-by: Cédric Le Goater <clg@kaod.org>
and
Tested-by: Cédric Le Goater <clg@kaod.org>
but I see slowdowns in nested as if the IPIs were not delivered. Have we
touch this part in 5.2 ?
Thanks,
C.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] Fix handling of h_set_dawr
2019-06-17 9:06 ` [PATCH 0/2] Fix handling of h_set_dawr Cédric Le Goater
@ 2019-06-20 1:45 ` Suraj Jitindar Singh
0 siblings, 0 replies; 6+ messages in thread
From: Suraj Jitindar Singh @ 2019-06-20 1:45 UTC (permalink / raw)
To: Cédric Le Goater, linuxppc-dev; +Cc: mikey, kvm-ppc
On Mon, 2019-06-17 at 11:06 +0200, Cédric Le Goater wrote:
> On 17/06/2019 09:16, Suraj Jitindar Singh wrote:
> > Series contains 2 patches to fix the host in kernel handling of the
> > hcall
> > h_set_dawr.
> >
> > First patch from Michael Neuling is just a resend added here for
> > clarity.
> >
> > Michael Neuling (1):
> > KVM: PPC: Book3S HV: Fix r3 corruption in h_set_dabr()
> >
> > Suraj Jitindar Singh (1):
> > KVM: PPC: Book3S HV: Only write DAWR[X] when handling h_set_dawr
> > in
> > real mode
>
>
>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
>
> and
>
> Tested-by: Cédric Le Goater <clg@kaod.org>
>
>
> but I see slowdowns in nested as if the IPIs were not delivered. Have
> we
> touch this part in 5.2 ?
Hi,
I've seen the same and tracked it down to decrementer exceptions not
being delivered when the guest is using large decrementer. I've got a
patch I'm about to send so I'll CC you.
Another option is to disable the large decrementer with:
-machine pseries,cap-large-decr=false
Thanks,
Suraj
>
> Thanks,
>
> C.
>
^ permalink raw reply [flat|nested] 6+ messages in thread