mbox series

[PULL,v8] KVM: arm64: Optimise FPSIMD context switching

Message ID 20180516104942.GS7753@e103592.cambridge.arm.com
State New
Headers show
Series [PULL,v8] KVM: arm64: Optimise FPSIMD context switching | expand

Pull-request

git://linux-arm.org/linux-dm.git

Message

Dave Martin May 16, 2018, 10:49 a.m. UTC 
Hi Marc,

This is a trivial update to the previously posted v7 [1].  The only
changes are a couple of minor cosmetic changes requested by reviewers,
on-list and the addition of Acked-by/Reviewed-by tags received since the
series was posted.

Let me know if you need anything else on this.

Cheers
---Dave

[1] [PATCH v7 00/16] KVM: arm64: Optimise FPSIMD context switching
http://lists.infradead.org/pipermail/linux-arm-kernel/2018-May/576595.html


The following changes since commit 6da6c0db5316275015e8cc2959f12a17584aeb64:

  Linux v4.17-rc3 (2018-04-29 14:17:42 -0700)

are available in the git repository at:

  git://linux-arm.org/linux-dm.git 

for you to fetch changes up to 90255dd95fe71a9bd1c4ce728abad4b0eec240d9:

  KVM: arm64: Invoke FPSIMD context switch trap from C (2018-05-16 10:44:00 +0100)

----------------------------------------------------------------
Christoffer Dall (1):
      KVM: arm/arm64: Introduce kvm_arch_vcpu_run_pid_change

Dave Martin (15):
      thread_info: Add update_thread_flag() helpers
      arm64: Use update{,_tsk}_thread_flag()
      KVM: arm64: Convert lazy FPSIMD context switch trap to C
      arm64: fpsimd: Generalise context saving for non-task contexts
      arm64/sve: Refactor user SVE trap maintenance for external use
      KVM: arm64: Repurpose vcpu_arch.debug_flags for general-purpose flags
      KVM: arm64: Optimise FPSIMD handling to reduce guest/host thrashing
      arm64/sve: Move read_zcr_features() out of cpufeature.h
      arm64/sve: Switch sve_pffr() argument from task to thread
      arm64/sve: Move sve_pffr() to fpsimd.h and make inline
      KVM: arm64: Save host SVE context as appropriate
      KVM: arm64: Remove eager host SVE state saving
      KVM: arm64: Remove redundant *exit_code changes in fpsimd_guest_exit()
      KVM: arm64: Fold redundant exit code checks out of fixup_guest_exit()
      KVM: arm64: Invoke FPSIMD context switch trap from C

 arch/arm/include/asm/kvm_host.h     |   9 ++-
 arch/arm64/Kconfig                  |   7 ++
 arch/arm64/include/asm/cpufeature.h |  29 -------
 arch/arm64/include/asm/fpsimd.h     |  21 ++++++
 arch/arm64/include/asm/kvm_asm.h    |   3 -
 arch/arm64/include/asm/kvm_host.h   |  32 +++++---
 arch/arm64/include/asm/processor.h  |   2 +
 arch/arm64/kernel/fpsimd.c          | 147 +++++++++++++++++++-----------------
 arch/arm64/kernel/ptrace.c          |   1 +
 arch/arm64/kvm/Kconfig              |   1 +
 arch/arm64/kvm/Makefile             |   2 +-
 arch/arm64/kvm/debug.c              |   8 +-
 arch/arm64/kvm/fpsimd.c             | 111 +++++++++++++++++++++++++++
 arch/arm64/kvm/hyp/debug-sr.c       |   6 +-
 arch/arm64/kvm/hyp/entry.S          |  43 -----------
 arch/arm64/kvm/hyp/hyp-entry.S      |  19 -----
 arch/arm64/kvm/hyp/switch.c         | 124 ++++++++++++++++++++----------
 arch/arm64/kvm/hyp/sysreg-sr.c      |   4 +-
 arch/arm64/kvm/sys_regs.c           |   9 +--
 include/linux/kvm_host.h            |   9 +++
 include/linux/sched.h               |   6 ++
 include/linux/thread_info.h         |  11 +++
 virt/kvm/Kconfig                    |   3 +
 virt/kvm/arm/arm.c                  |  25 +++++-
 virt/kvm/kvm_main.c                 |   7 +-
 25 files changed, 406 insertions(+), 233 deletions(-)
 create mode 100644 arch/arm64/kvm/fpsimd.c

Comments

Marc Zyngier May 20, 2018, 1:14 p.m. UTC | #1 
On Wed, 16 May 2018 11:49:42 +0100
Dave Martin <Dave.Martin@arm.com> wrote:

Hi Dave,

> Hi Marc,
> 
> This is a trivial update to the previously posted v7 [1].  The only
> changes are a couple of minor cosmetic changes requested by reviewers,
> on-list and the addition of Acked-by/Reviewed-by tags received since the
> series was posted.
> 
> Let me know if you need anything else on this.

So I've taken this, merged in Linus' top of tree, started a guest on a
dual A53 board, and immediately hit the following:

root@sy-borg:~# [  287.226184] Unable to handle kernel NULL pointer dereference at virtual address 00000000
[  287.231672] Mem abort info:
[  287.234537]   ESR = 0x96000044
[  287.237674]   Exception class = DABT (current EL), IL = 32 bits
[  287.243765]   SET = 0, FnV = 0
[  287.246900]   EA = 0, S1PTW = 0
[  287.250126] Data abort info:
[  287.253083]   ISV = 0, ISS = 0x00000044
[  287.257025]   CM = 0, WnR = 1
[  287.260076] user pgtable: 4k pages, 48-bit VAs, pgdp = 00000000b8483f75
[  287.266882] [0000000000000000] pgd=0000000000000000
[  287.271903] Internal error: Oops: 96000044 [#1] PREEMPT SMP
[  287.277636] Modules linked in:
[  287.280776] CPU: 1 PID: 3098 Comm: kworker/u4:3 Not tainted 4.17.0-rc5-00166-gd84e81cca249 #136
[  287.289730] Hardware name: Globalscale Marvell ESPRESSOBin Board (DT)
[  287.296364] pstate: 40000085 (nZcv daIf -PAN -UAO)
[  287.301301] pc : fpsimd_save_state+0x0/0x54
[  287.305595] lr : fpsimd_save+0x50/0x100
[  287.309531] sp : ffff00000dde3af0
[  287.312936] x29: ffff00000dde3af0 x28: ffff000008cd565c 
[  287.318401] x27: ffff800078ee9c80 x26: ffff80007b207628 
[  287.323867] x25: ffff0000093f9000 x24: 0000000000000001 
[  287.329333] x23: ffff0000093d4000 x22: ffff80007b207000 
[  287.334798] x21: ffff80007efd7d80 x20: ffff80007b207000 
[  287.340264] x19: 0000000000000000 x18: 0000000000040f0b 
[  287.345729] x17: 0000ffffb70752b8 x16: 0000ffffb708e008 
[  287.351195] x15: 0000000000000000 x14: 0000000000000400 
[  287.356661] x13: 0000000000000001 x12: 0000000000000001 
[  287.362127] x11: 0000000000000001 x10: 0000000000000000 
[  287.367592] x9 : 0000000000000253 x8 : ffff80007b207200 
[  287.373057] x7 : ffff80007b207100 x6 : ffff80007c378f18 
[  287.378523] x5 : 00000042c2094c00 x4 : 0000000000000000 
[  287.383990] x3 : 00000042e0033450 x2 : 0000000000000000 
[  287.389454] x1 : 0000800075bf6000 x0 : 0000000000000000 
[  287.394922] Process kworker/u4:3 (pid: 3098, stack limit = 0x00000000ca0dd8c6)
[  287.402358] Call trace:
[  287.404873]  fpsimd_save_state+0x0/0x54
[  287.408813]  fpsimd_thread_switch+0x28/0xa0
[  287.413114]  __switch_to+0x1c/0xd0
[  287.416609]  __schedule+0x1b8/0x730
[  287.420191]  preempt_schedule_common+0x24/0x48
[  287.424760]  preempt_schedule.part.23+0x1c/0x28
[  287.429419]  preempt_schedule+0x1c/0x28
[  287.433363]  _raw_spin_unlock+0x34/0x48
[  287.437308]  flush_old_exec+0x45c/0x6a0
[  287.441250]  load_elf_binary+0x324/0x1198
[  287.445372]  search_binary_handler+0xac/0x230
[  287.449851]  do_execveat_common.isra.14+0x508/0x6e0
[  287.454867]  do_execve+0x28/0x30
[  287.458185]  call_usermodehelper_exec_async+0xdc/0x140
[  287.463468]  ret_from_fork+0x10/0x18
[  287.467143] Code: a9425bf5 a8c37bfd d65f03c0 d65f03c0 (ad000400) 
[  287.473414] ---[ end trace c4346b99cc877f8e ]---

It happened just after having loaded the guest kernel, so I presume
we're missing some kind of initialization. I couldn't subsequently
reproduce it  on the same machine, and the same kernel is doing
absolutely fine on a Seattle box.

I can't immediately see how st would be NULL, unless we somehow are
missing some state tracking somewhere...

Any idea?

	M.
Dave Martin May 21, 2018, 10:02 a.m. UTC | #2 
On Sun, May 20, 2018 at 02:14:41PM +0100, Marc Zyngier wrote:
> On Wed, 16 May 2018 11:49:42 +0100
> Dave Martin <Dave.Martin@arm.com> wrote:
> 
> Hi Dave,
> 
> > Hi Marc,
> > 
> > This is a trivial update to the previously posted v7 [1].  The only
> > changes are a couple of minor cosmetic changes requested by reviewers,
> > on-list and the addition of Acked-by/Reviewed-by tags received since the
> > series was posted.
> > 
> > Let me know if you need anything else on this.
> 
> So I've taken this, merged in Linus' top of tree, started a guest on a
> dual A53 board, and immediately hit the following:
> 
> root@sy-borg:~# [  287.226184] Unable to handle kernel NULL pointer dereference at virtual address 00000000
> [  287.231672] Mem abort info:
> [  287.234537]   ESR = 0x96000044
> [  287.237674]   Exception class = DABT (current EL), IL = 32 bits
> [  287.243765]   SET = 0, FnV = 0
> [  287.246900]   EA = 0, S1PTW = 0
> [  287.250126] Data abort info:
> [  287.253083]   ISV = 0, ISS = 0x00000044
> [  287.257025]   CM = 0, WnR = 1
> [  287.260076] user pgtable: 4k pages, 48-bit VAs, pgdp = 00000000b8483f75
> [  287.266882] [0000000000000000] pgd=0000000000000000
> [  287.271903] Internal error: Oops: 96000044 [#1] PREEMPT SMP
> [  287.277636] Modules linked in:
> [  287.280776] CPU: 1 PID: 3098 Comm: kworker/u4:3 Not tainted 4.17.0-rc5-00166-gd84e81cca249 #136
> [  287.289730] Hardware name: Globalscale Marvell ESPRESSOBin Board (DT)
> [  287.296364] pstate: 40000085 (nZcv daIf -PAN -UAO)
> [  287.301301] pc : fpsimd_save_state+0x0/0x54
> [  287.305595] lr : fpsimd_save+0x50/0x100
> [  287.309531] sp : ffff00000dde3af0
> [  287.312936] x29: ffff00000dde3af0 x28: ffff000008cd565c 
> [  287.318401] x27: ffff800078ee9c80 x26: ffff80007b207628 
> [  287.323867] x25: ffff0000093f9000 x24: 0000000000000001 
> [  287.329333] x23: ffff0000093d4000 x22: ffff80007b207000 
> [  287.334798] x21: ffff80007efd7d80 x20: ffff80007b207000 
> [  287.340264] x19: 0000000000000000 x18: 0000000000040f0b 
> [  287.345729] x17: 0000ffffb70752b8 x16: 0000ffffb708e008 
> [  287.351195] x15: 0000000000000000 x14: 0000000000000400 
> [  287.356661] x13: 0000000000000001 x12: 0000000000000001 
> [  287.362127] x11: 0000000000000001 x10: 0000000000000000 
> [  287.367592] x9 : 0000000000000253 x8 : ffff80007b207200 
> [  287.373057] x7 : ffff80007b207100 x6 : ffff80007c378f18 
> [  287.378523] x5 : 00000042c2094c00 x4 : 0000000000000000 
> [  287.383990] x3 : 00000042e0033450 x2 : 0000000000000000 
> [  287.389454] x1 : 0000800075bf6000 x0 : 0000000000000000 
> [  287.394922] Process kworker/u4:3 (pid: 3098, stack limit = 0x00000000ca0dd8c6)
> [  287.402358] Call trace:
> [  287.404873]  fpsimd_save_state+0x0/0x54
> [  287.408813]  fpsimd_thread_switch+0x28/0xa0
> [  287.413114]  __switch_to+0x1c/0xd0
> [  287.416609]  __schedule+0x1b8/0x730
> [  287.420191]  preempt_schedule_common+0x24/0x48
> [  287.424760]  preempt_schedule.part.23+0x1c/0x28
> [  287.429419]  preempt_schedule+0x1c/0x28
> [  287.433363]  _raw_spin_unlock+0x34/0x48
> [  287.437308]  flush_old_exec+0x45c/0x6a0
> [  287.441250]  load_elf_binary+0x324/0x1198
> [  287.445372]  search_binary_handler+0xac/0x230
> [  287.449851]  do_execveat_common.isra.14+0x508/0x6e0
> [  287.454867]  do_execve+0x28/0x30
> [  287.458185]  call_usermodehelper_exec_async+0xdc/0x140
> [  287.463468]  ret_from_fork+0x10/0x18
> [  287.467143] Code: a9425bf5 a8c37bfd d65f03c0 d65f03c0 (ad000400) 
> [  287.473414] ---[ end trace c4346b99cc877f8e ]---
> 
> It happened just after having loaded the guest kernel, so I presume
> we're missing some kind of initialization. I couldn't subsequently
> reproduce it  on the same machine, and the same kernel is doing
> absolutely fine on a Seattle box.

Curious, this is a kernel thread, which shouldn't have any fpsimd state
of its own.  It would be interesting to know what ran before, but tricky
to find that out now unless we can find a way to repoduce...

Maybe a thread flag has ended up in the wrong state.

> I can't immediately see how st would be NULL, unless we somehow are
> missing some state tracking somewhere...

This might be a race, or a missing piece of logic somewhere... I'll
take a look.


Cheers
---Dave