From: drjones@redhat.com (Andrew Jones)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 11/37] KVM: arm64: Improve debug register save/restore flow
Date: Sun, 3 Dec 2017 14:49:58 +0100 [thread overview]
Message-ID: <20171203134958.b5zfdv2mbu6lr6gj@hawk.localdomain> (raw)
In-Reply-To: <20171201175203.GD6615@lvm>
On Fri, Dec 01, 2017 at 06:52:03PM +0100, Christoffer Dall wrote:
> On Tue, Nov 07, 2017 at 03:48:57PM +0100, Andrew Jones wrote:
> > On Thu, Oct 12, 2017 at 12:41:15PM +0200, Christoffer Dall wrote:
> > > Instead of having multiple calls from the world switch path to the debug
> > > logic, each figuring out if the dirty bit is set and if we should
> > > save/restore the debug registes, let's just provide two hooks to the
> > > debug save/restore functionality, one for switching to the guest
> > > context, and one for switching to the host context, and we get the
> > > benefit of only having to evaluate the dirty flag once on each path,
> > > plus we give the compiler some more room to inline some of this
> > > functionality.
> > >
> > > Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> > > ---
> > > arch/arm64/include/asm/kvm_hyp.h | 10 ++-----
> > > arch/arm64/kvm/hyp/debug-sr.c | 56 +++++++++++++++++++++++++++-------------
> > > arch/arm64/kvm/hyp/switch.c | 6 ++---
> > > 3 files changed, 42 insertions(+), 30 deletions(-)
> > >
> > > diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
> > > index 08d3bb6..a0e5a70 100644
> > > --- a/arch/arm64/include/asm/kvm_hyp.h
> > > +++ b/arch/arm64/include/asm/kvm_hyp.h
> > > @@ -139,14 +139,8 @@ void __sysreg_restore_guest_state(struct kvm_cpu_context *ctxt);
> > > void __sysreg32_save_state(struct kvm_vcpu *vcpu);
> > > void __sysreg32_restore_state(struct kvm_vcpu *vcpu);
> > >
> > > -void __debug_save_state(struct kvm_vcpu *vcpu,
> > > - struct kvm_guest_debug_arch *dbg,
> > > - struct kvm_cpu_context *ctxt);
> > > -void __debug_restore_state(struct kvm_vcpu *vcpu,
> > > - struct kvm_guest_debug_arch *dbg,
> > > - struct kvm_cpu_context *ctxt);
> > > -void __debug_cond_save_host_state(struct kvm_vcpu *vcpu);
> > > -void __debug_cond_restore_host_state(struct kvm_vcpu *vcpu);
> > > +void __debug_switch_to_guest(struct kvm_vcpu *vcpu);
> > > +void __debug_switch_to_host(struct kvm_vcpu *vcpu);
> > >
> > > void __fpsimd_save_state(struct user_fpsimd_state *fp_regs);
> > > void __fpsimd_restore_state(struct user_fpsimd_state *fp_regs);
> > > diff --git a/arch/arm64/kvm/hyp/debug-sr.c b/arch/arm64/kvm/hyp/debug-sr.c
> > > index a2291b6..b4cd8e0 100644
> > > --- a/arch/arm64/kvm/hyp/debug-sr.c
> > > +++ b/arch/arm64/kvm/hyp/debug-sr.c
> > > @@ -116,16 +116,13 @@ static void __hyp_text __debug_restore_spe(u64 pmscr_el1)
> > > write_sysreg_s(pmscr_el1, PMSCR_EL1);
> > > }
> > >
> > > -void __hyp_text __debug_save_state(struct kvm_vcpu *vcpu,
> > > - struct kvm_guest_debug_arch *dbg,
> > > - struct kvm_cpu_context *ctxt)
> > > +static void __hyp_text __debug_save_state(struct kvm_vcpu *vcpu,
> > > + struct kvm_guest_debug_arch *dbg,
> > > + struct kvm_cpu_context *ctxt)
> > > {
> > > u64 aa64dfr0;
> > > int brps, wrps;
> > >
> > > - if (!(vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY))
> > > - return;
> > > -
> > > aa64dfr0 = read_sysreg(id_aa64dfr0_el1);
> > > brps = (aa64dfr0 >> 12) & 0xf;
> > > wrps = (aa64dfr0 >> 20) & 0xf;
> > > @@ -138,16 +135,13 @@ void __hyp_text __debug_save_state(struct kvm_vcpu *vcpu,
> > > ctxt->sys_regs[MDCCINT_EL1] = read_sysreg(mdccint_el1);
> > > }
> > >
> > > -void __hyp_text __debug_restore_state(struct kvm_vcpu *vcpu,
> > > - struct kvm_guest_debug_arch *dbg,
> > > - struct kvm_cpu_context *ctxt)
> > > +static void __hyp_text __debug_restore_state(struct kvm_vcpu *vcpu,
> > > + struct kvm_guest_debug_arch *dbg,
> > > + struct kvm_cpu_context *ctxt)
> > > {
> > > u64 aa64dfr0;
> > > int brps, wrps;
> > >
> > > - if (!(vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY))
> > > - return;
> > > -
> > > aa64dfr0 = read_sysreg(id_aa64dfr0_el1);
> > >
> > > brps = (aa64dfr0 >> 12) & 0xf;
> > > @@ -161,24 +155,50 @@ void __hyp_text __debug_restore_state(struct kvm_vcpu *vcpu,
> > > write_sysreg(ctxt->sys_regs[MDCCINT_EL1], mdccint_el1);
> > > }
> > >
> > > -void __hyp_text __debug_cond_save_host_state(struct kvm_vcpu *vcpu)
> > > +void __hyp_text __debug_switch_to_guest(struct kvm_vcpu *vcpu)
> > > {
> > > - __debug_save_state(vcpu, &vcpu->arch.host_debug_state.regs,
> > > - kern_hyp_va(vcpu->arch.host_cpu_context));
> > > + struct kvm_cpu_context *__host_ctxt;
> > > + struct kvm_cpu_context *__guest_ctxt;
> > > + struct kvm_guest_debug_arch *__host_dbg;
> > > + struct kvm_guest_debug_arch *__guest_dbg;
> >
> > I caught in your reply to Marc that the __ prefix here is for hyp mode
> > accessible code and data, but do we also need to use it for stack data?
> > No big deal, but it's not very pretty.
> >
>
> sure.
>
> > >
> > > /* Non-VHE: Disable and flush SPE data generation
> > > * VHE: The vcpu can run. but it can't hide. */
> > > if (!has_vhe())
> > > __debug_save_spe_nvhe(&vcpu->arch.host_debug_state.pmscr_el1);
> > > +
> > > + if (!(vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY))
> > > + return;
> > > +
> > > + __host_ctxt = kern_hyp_va(vcpu->arch.host_cpu_context);
> > > + __guest_ctxt = &vcpu->arch.ctxt;
> > > + __host_dbg = &vcpu->arch.host_debug_state.regs;
> > > + __guest_dbg = kern_hyp_va(vcpu->arch.debug_ptr);
> > > +
> > > + __debug_save_state(vcpu, __host_dbg, __host_ctxt);
> > > + __debug_restore_state(vcpu, __guest_dbg, __guest_ctxt);
> > > }
> > >
> > > -void __hyp_text __debug_cond_restore_host_state(struct kvm_vcpu *vcpu)
> > > +void __hyp_text __debug_switch_to_host(struct kvm_vcpu *vcpu)
> > > {
> > > + struct kvm_cpu_context *__host_ctxt;
> > > + struct kvm_cpu_context *__guest_ctxt;
> > > + struct kvm_guest_debug_arch *__host_dbg;
> > > + struct kvm_guest_debug_arch *__guest_dbg;
> > > +
> > > if (!has_vhe())
> > > __debug_restore_spe(vcpu->arch.host_debug_state.pmscr_el1);
> > >
> > > - __debug_restore_state(vcpu, &vcpu->arch.host_debug_state.regs,
> > > - kern_hyp_va(vcpu->arch.host_cpu_context));
> > > + if (!(vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY))
> > > + return;
> > > +
> > > + __host_ctxt = kern_hyp_va(vcpu->arch.host_cpu_context);
> > > + __guest_ctxt = &vcpu->arch.ctxt;
> > > + __host_dbg = &vcpu->arch.host_debug_state.regs;
> > > + __guest_dbg = kern_hyp_va(vcpu->arch.debug_ptr);
> > > +
> > > + __debug_save_state(vcpu, __guest_dbg, __guest_ctxt);
> > > + __debug_restore_state(vcpu, __host_dbg, __host_ctxt);
> > >
> > > vcpu->arch.debug_flags &= ~KVM_ARM64_DEBUG_DIRTY;
> > > }
> > > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> > > index ef05c59..e270cba 100644
> > > --- a/arch/arm64/kvm/hyp/switch.c
> > > +++ b/arch/arm64/kvm/hyp/switch.c
> > > @@ -271,7 +271,6 @@ int __hyp_text __kvm_vcpu_run(struct kvm_vcpu *vcpu)
> > > guest_ctxt = &vcpu->arch.ctxt;
> > >
> > > __sysreg_save_host_state(host_ctxt);
> > > - __debug_cond_save_host_state(vcpu);
> > >
> > > __activate_traps(vcpu);
> > > __activate_vm(vcpu);
> > > @@ -285,7 +284,7 @@ int __hyp_text __kvm_vcpu_run(struct kvm_vcpu *vcpu)
> > > */
> > > __sysreg32_restore_state(vcpu);
> > > __sysreg_restore_guest_state(guest_ctxt);
> > > - __debug_restore_state(vcpu, kern_hyp_va(vcpu->arch.debug_ptr), guest_ctxt);
> > > + __debug_switch_to_guest(vcpu);
> > >
> > > /* Jump in the fire! */
> > > again:
> > > @@ -353,12 +352,11 @@ int __hyp_text __kvm_vcpu_run(struct kvm_vcpu *vcpu)
> > >
> > > __sysreg_restore_host_state(host_ctxt);
> > >
> > > - __debug_save_state(vcpu, kern_hyp_va(vcpu->arch.debug_ptr), guest_ctxt);
> > > /*
> > > * This must come after restoring the host sysregs, since a non-VHE
> > > * system may enable SPE here and make use of the TTBRs.
> > > */
> > > - __debug_cond_restore_host_state(vcpu);
> > > + __debug_switch_to_host(vcpu);
> > >
> > > return exit_code;
> > > }
> > > --
> > > 2.9.0
> > >
> >
> > This looks like a nice cleanup, but can you please add a note to the
> > commit message about why we don't need to use the
> >
> > save-host-state
> > activate-traps-and-vm
> > restore-guest-state
> >
> > and the reverse, patterns for the debug registers?
>
> I think the current commit message motivates the change fine.
>
The motivation is obvious, the justification for an additional change
is not. How does it justify changing the sequence
save-debug-host-state
activate-debug-traps /* and other stuff in between */
restore-debug-guest-state
to
activate-debug-traps /* and other stuff in between */
save-debug-host-state
restore-debug-guest-state
Thanks,
drew
next prev parent reply other threads:[~2017-12-03 13:49 UTC|newest]
Thread overview: 127+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-12 10:41 [PATCH 00/37] Optimize KVM/ARM for VHE systems Christoffer Dall
2017-10-12 10:41 ` [PATCH 01/37] KVM: arm64: Avoid storing the vcpu pointer on the stack Christoffer Dall
2017-10-12 15:49 ` Marc Zyngier
2017-10-12 17:02 ` Christoffer Dall
2017-10-13 11:31 ` Marc Zyngier
2017-11-23 20:59 ` Christoffer Dall
2017-11-27 11:11 ` James Morse
2017-11-29 18:20 ` Christoffer Dall
2017-11-06 17:22 ` Andrew Jones
2017-11-07 8:24 ` Christoffer Dall
2017-10-12 10:41 ` [PATCH 02/37] KVM: arm64: Rework hyp_panic for VHE and non-VHE Christoffer Dall
2017-10-12 15:55 ` Marc Zyngier
2017-10-12 17:06 ` Christoffer Dall
2017-10-12 10:41 ` [PATCH 03/37] KVM: arm64: Move HCR_INT_OVERRIDE to default HCR_EL2 guest flag Christoffer Dall
2017-10-12 16:20 ` Marc Zyngier
2017-10-12 10:41 ` [PATCH 04/37] KVM: arm/arm64: Get rid of vcpu->arch.irq_lines Christoffer Dall
2017-10-12 16:24 ` Marc Zyngier
2017-11-06 17:58 ` Andrew Jones
2017-11-14 12:17 ` Julien Thierry
2017-11-16 16:11 ` Julien Thierry
2017-11-26 16:04 ` Christoffer Dall
2017-10-12 10:41 ` [PATCH 05/37] KVM: Record the executing ioctl number on the vcpu struct Christoffer Dall
2017-10-13 17:13 ` Radim Krčmář
2017-10-13 17:31 ` Christoffer Dall
2017-10-13 18:38 ` Radim Krčmář
2017-10-13 18:51 ` Christoffer Dall
2017-11-07 10:45 ` Andrew Jones
2017-11-22 20:28 ` Christoffer Dall
2017-10-12 10:41 ` [PATCH 06/37] KVM: arm/arm64: Only load/put VCPU state for KVM_RUN Christoffer Dall
2017-10-12 10:41 ` [PATCH 07/37] KVM: arm/arm64: Add kvm_vcpu_load_sysregs and kvm_vcpu_put_sysregs Christoffer Dall
2017-11-07 10:56 ` Andrew Jones
2017-11-07 11:10 ` Andrew Jones
2017-11-22 20:34 ` Christoffer Dall
2017-11-23 11:08 ` Andrew Jones
2017-10-12 10:41 ` [PATCH 08/37] KVM: arm64: Defer restoring host VFP state to vcpu_put Christoffer Dall
2017-11-07 13:15 ` Andrew Jones
2017-11-26 16:24 ` Christoffer Dall
2017-11-15 16:04 ` Andrew Jones
2017-11-26 16:17 ` Christoffer Dall
2017-11-27 8:32 ` Andrew Jones
2017-11-25 7:52 ` Yury Norov
2017-11-26 16:17 ` Christoffer Dall
2017-11-26 18:58 ` Yury Norov
2017-11-26 19:18 ` Christoffer Dall
2017-11-27 6:25 ` Yury Norov
2017-11-30 19:07 ` Marc Zyngier
2017-10-12 10:41 ` [PATCH 09/37] KVM: arm64: Move debug dirty flag calculation out of world switch Christoffer Dall
2017-11-07 14:09 ` Andrew Jones
2017-11-25 8:09 ` Yury Norov
2017-12-01 17:25 ` Christoffer Dall
2017-12-03 13:17 ` Andrew Jones
2017-10-12 10:41 ` [PATCH 10/37] KVM: arm64: Slightly improve debug save/restore functions Christoffer Dall
2017-11-07 14:22 ` Andrew Jones
2017-12-01 17:51 ` Christoffer Dall
2017-11-14 16:42 ` Julien Thierry
2017-12-01 15:19 ` Christoffer Dall
2017-12-06 15:38 ` Julien Thierry
2017-10-12 10:41 ` [PATCH 11/37] KVM: arm64: Improve debug register save/restore flow Christoffer Dall
2017-11-07 14:48 ` Andrew Jones
2017-12-01 17:52 ` Christoffer Dall
2017-12-03 13:49 ` Andrew Jones [this message]
2017-12-03 20:47 ` Christoffer Dall
2017-10-12 10:41 ` [PATCH 12/37] KVM: arm64: Factor out fault info population and gic workarounds Christoffer Dall
2017-11-07 15:12 ` Andrew Jones
2017-10-12 10:41 ` [PATCH 13/37] KVM: arm64: Introduce VHE-specific kvm_vcpu_run Christoffer Dall
2017-11-07 15:25 ` Andrew Jones
2017-12-01 18:10 ` Christoffer Dall
2017-10-12 10:41 ` [PATCH 14/37] KVM: arm64: Remove kern_hyp_va() use in VHE switch function Christoffer Dall
2017-11-07 16:07 ` Andrew Jones
2017-10-12 10:41 ` [PATCH 15/37] KVM: arm64: Don't deactivate VM on VHE systems Christoffer Dall
2017-11-07 16:14 ` Andrew Jones
2017-12-03 19:27 ` Christoffer Dall
2017-10-12 10:41 ` [PATCH 16/37] KVM: arm64: Remove noop calls to timer save/restore from VHE switch Christoffer Dall
2017-11-07 16:25 ` Andrew Jones
2017-12-03 19:27 ` Christoffer Dall
2017-10-12 10:41 ` [PATCH 17/37] KVM: arm64: Move userspace system registers into separate function Christoffer Dall
2017-11-08 9:32 ` Andrew Jones
2017-12-03 19:36 ` Christoffer Dall
2017-10-12 10:41 ` [PATCH 18/37] KVM: arm64: Rewrite sysreg alternatives to static keys Christoffer Dall
2017-10-12 10:41 ` [PATCH 19/37] KVM: arm64: Introduce separate VHE/non-VHE sysreg save/restore functions Christoffer Dall
2017-11-08 10:31 ` Andrew Jones
2017-10-12 10:41 ` [PATCH 20/37] KVM: arm64: Unify non-VHE host/guest sysreg save and restore functions Christoffer Dall
2017-11-08 10:39 ` Andrew Jones
2017-12-03 19:41 ` Christoffer Dall
2017-10-12 10:41 ` [PATCH 21/37] KVM: arm64: Don't save the host ELR_EL2 and SPSR_EL2 on VHE systems Christoffer Dall
2017-11-08 17:03 ` Andrew Jones
2017-12-03 19:45 ` Christoffer Dall
2017-10-12 10:41 ` [PATCH 22/37] KVM: arm64: Change 32-bit handling of VM system registers Christoffer Dall
2017-11-13 16:25 ` Andrew Jones
2017-10-12 10:41 ` [PATCH 23/37] KVM: arm64: Prepare to handle traps on deferred VM sysregs Christoffer Dall
2017-11-13 17:54 ` Andrew Jones
2017-12-03 19:50 ` Christoffer Dall
2017-12-04 10:05 ` Andrew Jones
2017-10-12 10:41 ` [PATCH 24/37] KVM: arm64: Prepare to handle traps on deferred EL0 sysregs Christoffer Dall
2017-11-15 9:25 ` Julien Thierry
2017-12-03 19:51 ` Christoffer Dall
2017-10-12 10:41 ` [PATCH 25/37] KVM: arm64: Prepare to handle traps on remaining deferred EL1 sysregs Christoffer Dall
2017-11-13 18:56 ` Andrew Jones
2017-12-03 20:29 ` Christoffer Dall
2017-10-12 10:41 ` [PATCH 26/37] KVM: arm64: Prepare to handle traps on deferred AArch32 sysregs Christoffer Dall
2017-11-13 19:07 ` Andrew Jones
2017-12-03 20:35 ` Christoffer Dall
2017-10-12 10:41 ` [PATCH 27/37] KVM: arm64: Defer saving/restoring system registers to vcpu load/put on VHE Christoffer Dall
2017-10-12 10:41 ` [PATCH 28/37] KVM: arm64: Move common VHE/non-VHE trap config in separate functions Christoffer Dall
2017-11-25 10:43 ` Yury Norov
2017-11-25 10:49 ` Russell King - ARM Linux
2017-10-12 10:41 ` [PATCH 29/37] KVM: arm64: Configure FPSIMD traps on vcpu load/put for VHE Christoffer Dall
2017-10-12 10:41 ` [PATCH 30/37] KVM: arm64: Configure c15, PMU, and debug register traps on cpu " Christoffer Dall
2017-10-12 10:41 ` [PATCH 31/37] KVM: arm64: Separate activate_traps and deactive_traps for VHE and non-VHE Christoffer Dall
2017-10-12 10:41 ` [PATCH 32/37] KVM: arm/arm64: Handle VGICv2 save/restore from the main VGIC code Christoffer Dall
2017-11-15 17:50 ` Andre Przywara
2017-11-26 10:29 ` Yury Norov
2017-11-26 19:46 ` Christoffer Dall
2017-11-30 12:09 ` Yury Norov
2017-11-26 19:37 ` Christoffer Dall
2017-10-12 10:41 ` [PATCH 33/37] KVM: arm/arm64: Move arm64-only vgic-v2-sr.c file to arm64 Christoffer Dall
2017-11-15 17:52 ` Andre Przywara
2017-10-12 10:41 ` [PATCH 34/37] KVM: arm/arm64: Handle VGICv3 save/restore from the main VGIC code on VHE Christoffer Dall
2017-10-12 10:41 ` [PATCH 35/37] KVM: arm/arm64: Get rid of vgic_elrsr Christoffer Dall
2017-11-26 14:39 ` Yury Norov
2017-11-26 19:53 ` Christoffer Dall
2017-10-12 10:41 ` [PATCH 36/37] KVM: arm/arm64: Move VGIC APR save/restore to vgic put/load Christoffer Dall
2017-11-26 15:09 ` Yury Norov
2017-11-26 19:55 ` Christoffer Dall
2017-10-12 10:41 ` [PATCH 37/37] KVM: arm/arm64: Avoid VGICv3 save/restore on VHE with no IRQs Christoffer Dall
2017-11-30 18:33 ` Yury Norov
2017-12-03 20:38 ` Christoffer Dall
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171203134958.b5zfdv2mbu6lr6gj@hawk.localdomain \
--to=drjones@redhat.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox