Re: [PATCH 0/9] arm64: KVM: debug infrastructure support
On 7 May 2014 20:50, Marc Zyngier marc.zyng...@arm.com wrote: This patch series adds debug support, a key feature missing from the KVM/arm64 port. The main idea is to keep track of whether the debug registers are dirty (changed by the guest) or not. In this case, perform the usual save/restore dance, for one run only. It means we only have a penalty if a guest is actually using the debug registers. The huge amount of registers is properly frightening, but CPUs actually only implement a subset of them. Also, there is a number of registers we don't bother emulating (things having to do with external debug and OSlock). This has been tested on a Cortex-A57 platform, running both 32 and 64bit guests, on top of 3.15-rc4. This code also lives in my tree in the kvm-arm64/debug-trap branch. Marc Zyngier (9): arm64: KVM: rename pm_fake handler to trap_wi_raz arm64: move DBG_MDSCR_* to asm/debug-monitors.h arm64: KVM: add trap handlers for AArch64 debug registers arm64: KVM: common infrastructure for handling AArch32 CP14/CP15 arm64: KVM: use separate tables for AArch32 32 and 64bit traps arm64: KVM: check ordering of all system register tables arm64: KVM: add trap handlers for AArch32 debug registers arm64: KVM: implement lazy world switch for debug registers arm64: KVM: enable trapping of all debug registers arch/arm64/include/asm/debug-monitors.h | 19 +- arch/arm64/include/asm/kvm_asm.h| 39 ++- arch/arm64/include/asm/kvm_coproc.h | 3 +- arch/arm64/include/asm/kvm_host.h | 12 +- arch/arm64/kernel/asm-offsets.c | 1 + arch/arm64/kernel/debug-monitors.c | 9 - arch/arm64/kvm/handle_exit.c| 4 +- arch/arm64/kvm/hyp.S| 457 - arch/arm64/kvm/sys_regs.c | 494 +++- 9 files changed, 940 insertions(+), 98 deletions(-) -- 1.8.3.4 ___ kvmarm mailing list kvm...@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm Hi Marc, Overall the patchset looks good to me. The debug register usage by Guest will be very rare so a lazy save/restore makes lot-of-sense here. The only concern here is that amount of time spend in world-switch will increase for Guest once Guest starts accessing debug registers. I was wondering if it is possible to detect that Guest has stopped using debug HW and we can mark debug state as clean. (or something similar) Regards, Anup -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/9] arm64: KVM: debug infrastructure support
On Mon, May 19 2014 at 10:05:42 am BST, Anup Patel anup.pa...@linaro.org wrote Hi Anup, Overall the patchset looks good to me. The debug register usage by Guest will be very rare so a lazy save/restore makes lot-of-sense here. The only concern here is that amount of time spend in world-switch will increase for Guest once Guest starts accessing debug registers. I was wondering if it is possible to detect that Guest has stopped using debug HW and we can mark debug state as clean. (or something similar) If you look carefully at patch #8 (last hunk of the patch), you'll see that I always reset the debug state to clean at the end of a guest run: @@ -609,6 +1040,12 @@ __kvm_vcpu_return: bl __restore_sysregs bl __restore_fpsimd + + skip_clean_debug_state x3, 1f + // Clear the dirty flag for the next run + str xzr, [x0, #VCPU_DEBUG_FLAGS] + bl __restore_debug +1: restore_host_regs mov x0, x1 This ensures that the guest's debug state will only be reloaded if: - MDSCR_EL1 has either MDE or KDE set (which means the guest is actively using the debug infrastructure) - or the guest has written to a trapped register (which marks the state as dirty). I don't think we can do less work than this. Or can we? Thanks, M. -- Jazz is not dead. It just smells funny. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/9] arm64: KVM: debug infrastructure support
On 19 May 2014 14:58, Marc Zyngier marc.zyng...@arm.com wrote: On Mon, May 19 2014 at 10:05:42 am BST, Anup Patel anup.pa...@linaro.org wrote Hi Anup, Overall the patchset looks good to me. The debug register usage by Guest will be very rare so a lazy save/restore makes lot-of-sense here. The only concern here is that amount of time spend in world-switch will increase for Guest once Guest starts accessing debug registers. I was wondering if it is possible to detect that Guest has stopped using debug HW and we can mark debug state as clean. (or something similar) If you look carefully at patch #8 (last hunk of the patch), you'll see that I always reset the debug state to clean at the end of a guest run: @@ -609,6 +1040,12 @@ __kvm_vcpu_return: bl __restore_sysregs bl __restore_fpsimd + + skip_clean_debug_state x3, 1f + // Clear the dirty flag for the next run + str xzr, [x0, #VCPU_DEBUG_FLAGS] + bl __restore_debug +1: restore_host_regs mov x0, x1 This ensures that the guest's debug state will only be reloaded if: - MDSCR_EL1 has either MDE or KDE set (which means the guest is actively using the debug infrastructure) - or the guest has written to a trapped register (which marks the state as dirty). Thanks for pointing out. Can you add this info as comment in patch#8 where you clear the dirty flag? I don't think we can do less work than this. Or can we? Thanks, M. -- Jazz is not dead. It just smells funny. -- Anup -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/9] arm64: KVM: debug infrastructure support
On Mon, May 19 2014 at 10:35:58 am BST, Anup Patel anup.pa...@linaro.org wrote: On 19 May 2014 14:58, Marc Zyngier marc.zyng...@arm.com wrote: On Mon, May 19 2014 at 10:05:42 am BST, Anup Patel anup.pa...@linaro.org wrote Hi Anup, Overall the patchset looks good to me. The debug register usage by Guest will be very rare so a lazy save/restore makes lot-of-sense here. The only concern here is that amount of time spend in world-switch will increase for Guest once Guest starts accessing debug registers. I was wondering if it is possible to detect that Guest has stopped using debug HW and we can mark debug state as clean. (or something similar) If you look carefully at patch #8 (last hunk of the patch), you'll see that I always reset the debug state to clean at the end of a guest run: @@ -609,6 +1040,12 @@ __kvm_vcpu_return: bl __restore_sysregs bl __restore_fpsimd + + skip_clean_debug_state x3, 1f + // Clear the dirty flag for the next run + str xzr, [x0, #VCPU_DEBUG_FLAGS] + bl __restore_debug +1: restore_host_regs mov x0, x1 This ensures that the guest's debug state will only be reloaded if: - MDSCR_EL1 has either MDE or KDE set (which means the guest is actively using the debug infrastructure) - or the guest has written to a trapped register (which marks the state as dirty). Thanks for pointing out. Can you add this info as comment in patch#8 where you clear the dirty flag? Right. There is already some comments to that effect just above, where we compute the dirty state, but I think it doesn't hurt to repeat it. Thanks, M. -- Jazz is not dead. It just smells funny. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/9] arm64: KVM: debug infrastructure support
On 19 May 2014 10:28, Marc Zyngier marc.zyng...@arm.com wrote: If you look carefully at patch #8 (last hunk of the patch), you'll see that I always reset the debug state to clean at the end of a guest run: @@ -609,6 +1040,12 @@ __kvm_vcpu_return: bl __restore_sysregs bl __restore_fpsimd + + skip_clean_debug_state x3, 1f + // Clear the dirty flag for the next run + str xzr, [x0, #VCPU_DEBUG_FLAGS] + bl __restore_debug +1: restore_host_regs mov x0, x1 This ensures that the guest's debug state will only be reloaded if: - MDSCR_EL1 has either MDE or KDE set (which means the guest is actively using the debug infrastructure) - or the guest has written to a trapped register (which marks the state as dirty). Do we also handle the case where the guest didn't write to the trapped register but userspace did (via the SET_ONE_REG API)? Maybe this just falls out in the wash or is handled already... thanks -- PMM -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/9] arm64: KVM: debug infrastructure support
On Mon, May 19 2014 at 1:32:28 pm BST, Peter Maydell
peter.mayd...@linaro.org wrote:
On 19 May 2014 10:28, Marc Zyngier marc.zyng...@arm.com wrote:
If you look carefully at patch #8 (last hunk of the patch), you'll see
that I always reset the debug state to clean at the end of a guest
run:
@@ -609,6 +1040,12 @@ __kvm_vcpu_return:
bl __restore_sysregs
bl __restore_fpsimd
+
+ skip_clean_debug_state x3, 1f
+ // Clear the dirty flag for the next run
+ str xzr, [x0, #VCPU_DEBUG_FLAGS]
+ bl __restore_debug
+1:
restore_host_regs
mov x0, x1
This ensures that the guest's debug state will only be reloaded if:
- MDSCR_EL1 has either MDE or KDE set (which means the guest is actively
using the debug infrastructure)
- or the guest has written to a trapped register (which marks the state
as dirty).
Do we also handle the case where the guest didn't write to the trapped
register but userspace did (via the SET_ONE_REG API)? Maybe this just
falls out in the wash or is handled already...
This is pretty much handled by the same code:
- Userspace wrote to any register but MDSCR_EL1, and MDSCR_EL1 doesn't
have MDE/KDE set. In this case, we don't need to do anything, as the new
state is not in use yet.
- Userspace has written to MDSCR_EL1.{MDE,KDE}, and this indicates we
must restore the state.
Compared to what the guest does, we don't flag the state as dirty when
we write to any of the debug registers (only MDSCR_EL1 can be used to
enter the dirty state). It is not really a problem, as this is only a
perfermance optimisation (as soon as the guest starts using debug
registers, we want to disable trapping).
Thanks,
M.
--
Jazz is not dead. It just smells funny.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/9] arm64: KVM: debug infrastructure support
On 7 May 2014 16:20, Marc Zyngier marc.zyng...@arm.com wrote: This patch series adds debug support, a key feature missing from the KVM/arm64 port. The main idea is to keep track of whether the debug registers are dirty (changed by the guest) or not. In this case, perform the usual save/restore dance, for one run only. It means we only have a penalty if a guest is actually using the debug registers. The huge amount of registers is properly frightening, but CPUs actually only implement a subset of them. Also, there is a number of registers we don't bother emulating (things having to do with external debug and OSlock). Presumably these registers now appear in the userspace interface too, yes? Did you check that they all cope with the migration reads all register values on the source and then writes them on the destination in arbitrary order semantics without further fiddling? (I have a note that says that at least OSLAR_EL1/OSLSR_EL1 won't work that way, for instance.) thanks -- PMM -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/9] arm64: KVM: debug infrastructure support
On 07/05/14 16:42, Peter Maydell wrote:
On 7 May 2014 16:20, Marc Zyngier marc.zyng...@arm.com wrote:
This patch series adds debug support, a key feature missing from the
KVM/arm64 port.
The main idea is to keep track of whether the debug registers are
dirty (changed by the guest) or not. In this case, perform the usual
save/restore dance, for one run only. It means we only have a penalty
if a guest is actually using the debug registers.
The huge amount of registers is properly frightening, but CPUs
actually only implement a subset of them. Also, there is a number of
registers we don't bother emulating (things having to do with external
debug and OSlock).
Presumably these registers now appear in the userspace
interface too, yes? Did you check that they all cope with
the migration reads all register values on the source and then
writes them on the destination in arbitrary order semantics without
further fiddling? (I have a note that says that at least
OSLAR_EL1/OSLSR_EL1 won't work that way, for instance.)
The only registers that are exported to userspace are MDSCR_EL1,
DBG{BW}{CV}Rn_EL1 and MDCCINT_EL1 (and their 32bit counterparts). They
should be fine being saved/restored in any order, as long as you're not
running the vcpu in between.
The OSL*_EL1 registers are just ignored so far. So far, it is unclear
how they would be supported in a guest, and I don't have any test
environment for them. Should the need arise, support can be added on top
of what we have now.
M.
--
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
