Re: [PATCH v2 04/10] KVM: arm: guest debug, add stub KVM_SET_GUEST_DEBUG ioctl
On Tue, Mar 31, 2015 at 04:08:02PM +0100, Alex Bennée wrote: This commit adds a stub function to support the KVM_SET_GUEST_DEBUG ioctl. Currently any operation flag will return EINVAL. Actual functionality will be added with further patches. Signed-off-by: Alex Bennée alex.ben...@linaro.org. --- v2 - simplified form of the ioctl (stuff will go into setup_debug) diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt index b112efc..06c5064 100644 --- a/Documentation/virtual/kvm/api.txt +++ b/Documentation/virtual/kvm/api.txt @@ -2604,7 +2604,7 @@ handled. 4.87 KVM_SET_GUEST_DEBUG Capability: KVM_CAP_SET_GUEST_DEBUG -Architectures: x86, s390, ppc +Architectures: x86, s390, ppc, arm64 Type: vcpu ioctl Parameters: struct kvm_guest_debug (in) Returns: 0 on success; -1 on error diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c index 5560f74..445933d 100644 --- a/arch/arm/kvm/arm.c +++ b/arch/arm/kvm/arm.c @@ -183,6 +183,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) case KVM_CAP_ARM_PSCI: case KVM_CAP_ARM_PSCI_0_2: case KVM_CAP_READONLY_MEM: + case KVM_CAP_SET_GUEST_DEBUG: r = 1; break; shouldn't you wait with advertising this capability until you've implemented support for it? I think this would work for now, however it's not very practical - in the end one has to sense which debug flags are actually supported. Question is if he wants to add initial support and extend functionality and flags with each patch or enable the whole set of features in one shot at the end. Doing the latter seems more practicable to me (especially as the debug features are added in the same patch series). David ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 07/10] KVM: arm64: guest debug, add support for single-step
Hi Alex,
On Tue, Mar 31, 2015 at 04:08:05PM +0100, Alex Bennée wrote:
This adds support for single-stepping the guest. As userspace can and
will manipulate guest registers before restarting any tweaking of the
registers has to occur just before control is passed back to the guest.
this sentence is hard to read. Do you mean:
(a) As userspace can and will manipulate guest register, we must ensure
that any tweaking of the registers before restarting the guest happens
immediately before...
or
(b) As userspace manipulates guest registers before restarting the
guest, we must ensure that any tweaking of the register happens
immediately before...
Furthermore while guest debugging is in effect we need to squash the
Furthermore, while guest debugging is in effect,
(commas)
ability of the guest to single-step itself as we have no easy way of
re-entering the guest after the exception has been delivered to the
hypervisor.
I'm not sure I understand this last part of the sentence. Is the point
that if we trap on a guest single-step exception we cannot easily inject
such an exception back into the guest and therefore we trap the guest if
it tries to set itself up for single-stepping?
What is our recourse then? To just ignore the single-step setting of
the guest and execute it as normal (while single-stepping the guest from
the outside)?
Signed-off-by: Alex Bennée alex.ben...@linaro.org
---
v2
- Move pstate/mdscr manipulation into C
- don't export guest_debug to assembly
- add accessor for saved_debug regs
- tweak save/restore of mdscr_el1
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index d3bc8dc..c1ed8cb 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -304,7 +304,21 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
kvm_arm_set_running_vcpu(NULL);
}
-#define KVM_GUESTDBG_VALID (KVM_GUESTDBG_ENABLE|KVM_GUESTDBG_USE_SW_BP)
+#define KVM_GUESTDBG_VALID (KVM_GUESTDBG_ENABLE |\
+ KVM_GUESTDBG_USE_SW_BP | \
+ KVM_GUESTDBG_SINGLESTEP)
+
+/**
+ * kvm_arch_vcpu_ioctl_set_guest_debug - Setup guest debugging
+ * @kvm: pointer to the KVM struct
+ * @kvm_guest_debug: the ioctl data buffer
+ *
+ * This sets up the VM for guest debugging. Care has to be taken when
+ * manipulating guest registers as these will be set/cleared by the
+ * hyper-visor controller, typically before each kvm_run event. As a
which guest registers are set/cleared by userspace exactly?
s/by the hyper-visor controller/by userspace/
+ * result modification of the guest registers needs to take place
As a result, (comma)
s/needs to/must/
+ * after they have been restored in the hyp.S trampoline code.
trampoline code? The trampoline code we are referring to is in
hyp-init.S. Do you mean in EL2? Then just sya in hyp.S or say in EL2
or in hyp mode.
+ */
int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
struct kvm_guest_debug *dbg)
diff --git a/arch/arm64/include/asm/kvm_host.h
b/arch/arm64/include/asm/kvm_host.h
index 0631840..6a33647 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -121,6 +121,13 @@ struct kvm_vcpu_arch {
* here.
*/
+ /* Registers pre any guest debug manipulations */
I couldn't find 'pre' as an independent word in any English
dictionaries. I'm also not entirely sure what you mean? Who modifies
this when, and why do we need to store this?
+ struct {
+ u32 pstate_ss_bit;
+ u32 mdscr_el1_bits;
+
+ } debug_saved_regs;
If I understood this state correctly (see below), then guest_debug_state
is probably a better name for this struct.
+
/* Don't run the guest */
bool pause;
@@ -143,6 +150,7 @@ struct kvm_vcpu_arch {
#define vcpu_gp_regs(v) ((v)-arch.ctxt.gp_regs)
#define vcpu_sys_reg(v,r)((v)-arch.ctxt.sys_regs[(r)])
+#define vcpu_debug_saved_reg(v, r) ((v)-arch.debug_saved_regs.r)
hmm, not sure this is warranted if the 'saved_regs' is not the current
state of the VM, which is sort of what the vcpu_gp_regs() and friends
hint at. Maybe I'm just not getting exactly what piece of state it is.
/*
* CP14 and CP15 live in the same array, as they are backed by the
* same system registers.
diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
index cff0475..b32362c 100644
--- a/arch/arm64/kvm/debug.c
+++ b/arch/arm64/kvm/debug.c
@@ -19,8 +19,16 @@
#include linux/kvm_host.h
+#include asm/debug-monitors.h
+#include asm/kvm_asm.h
#include asm/kvm_arm.h
#include asm/kvm_host.h
+#include asm/kvm_emulate.h
+
+/* These are the bits of MDSCR_EL1 we may mess with */
we may mess with? Can you be more specific?
+#define MDSCR_EL1_DEBUG_BITS (DBG_MDSCR_SS | \
+ DBG_MDSCR_KDE | \
+ DBG_MDSCR_MDE)
Re: [PATCH v2 04/10] KVM: arm: guest debug, add stub KVM_SET_GUEST_DEBUG ioctl
David Hildenbrand d...@linux.vnet.ibm.com writes: On Tue, Mar 31, 2015 at 04:08:02PM +0100, Alex Bennée wrote: This commit adds a stub function to support the KVM_SET_GUEST_DEBUG ioctl. Currently any operation flag will return EINVAL. Actual functionality will be added with further patches. Signed-off-by: Alex Bennée alex.ben...@linaro.org. --- v2 - simplified form of the ioctl (stuff will go into setup_debug) diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt index b112efc..06c5064 100644 --- a/Documentation/virtual/kvm/api.txt +++ b/Documentation/virtual/kvm/api.txt @@ -2604,7 +2604,7 @@ handled. 4.87 KVM_SET_GUEST_DEBUG Capability: KVM_CAP_SET_GUEST_DEBUG -Architectures: x86, s390, ppc +Architectures: x86, s390, ppc, arm64 Type: vcpu ioctl Parameters: struct kvm_guest_debug (in) Returns: 0 on success; -1 on error diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c index 5560f74..445933d 100644 --- a/arch/arm/kvm/arm.c +++ b/arch/arm/kvm/arm.c @@ -183,6 +183,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) case KVM_CAP_ARM_PSCI: case KVM_CAP_ARM_PSCI_0_2: case KVM_CAP_READONLY_MEM: + case KVM_CAP_SET_GUEST_DEBUG: r = 1; break; shouldn't you wait with advertising this capability until you've implemented support for it? I think this would work for now, however it's not very practical - in the end one has to sense which debug flags are actually supported. Question is if he wants to add initial support and extend functionality and flags with each patch or enable the whole set of features in one shot at the end. This is what in effect I was doing. Testing each feature in turn and ensuring it failed gracefully when kernel features where not present (both missing the capability or returning EINVAL). Doing the latter seems more practicable to me (especially as the debug features are added in the same patch series). Well in practice the whole series will go in together so this is only really relevant to what happens when you bisect the series. David -- Alex Bennée ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 06/10] KVM: arm64: guest debug, add SW break point support
On Tue, Mar 31, 2015 at 04:08:04PM +0100, Alex Bennée wrote:
This adds support for SW breakpoints inserted by userspace.
We do this by trapping all BKPT exceptions in the
hypervisor (MDCR_EL2_TDE).
you mean trapping all exceptions in the guest to the hypervisor?
The kvm_debug_exit_arch carries the address
of the exception.
why? can userspace not simply read out the PC using GET_ONE_REG?
If user-space doesn't know of the breakpoint then we
have a guest inserted breakpoint and the hypervisor needs to start again
and deliver the exception to guest.
Signed-off-by: Alex Bennée alex.ben...@linaro.org
---
v2
- update to use new exit struct
- tweak for C setup
- do our setup in debug_setup/clear code
- fixed up comments
diff --git a/Documentation/virtual/kvm/api.txt
b/Documentation/virtual/kvm/api.txt
index 06c5064..17d4f9c 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -2626,7 +2626,7 @@ when running. Common control bits are:
The top 16 bits of the control field are architecture specific control
flags which can include the following:
- - KVM_GUESTDBG_USE_SW_BP: using software breakpoints [x86]
+ - KVM_GUESTDBG_USE_SW_BP: using software breakpoints [x86, arm64]
- KVM_GUESTDBG_USE_HW_BP: using hardware breakpoints [x86, s390]
- KVM_GUESTDBG_INJECT_DB: inject DB type exception [x86]
- KVM_GUESTDBG_INJECT_BP: inject BP type exception [x86]
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 7ea8b0e..d3bc8dc 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -304,7 +304,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
kvm_arm_set_running_vcpu(NULL);
}
-#define KVM_GUESTDBG_VALID (KVM_GUESTDBG_ENABLE)
+#define KVM_GUESTDBG_VALID (KVM_GUESTDBG_ENABLE|KVM_GUESTDBG_USE_SW_BP)
nit: spaces around the operator
int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
struct kvm_guest_debug *dbg)
diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
index 8a29d0b..cff0475 100644
--- a/arch/arm64/kvm/debug.c
+++ b/arch/arm64/kvm/debug.c
@@ -45,11 +45,18 @@ void kvm_arch_setup_debug(struct kvm_vcpu *vcpu)
vcpu-arch.mdcr_el2 |= (MDCR_EL2_TPM | MDCR_EL2_TPMCR);
vcpu-arch.mdcr_el2 |= (MDCR_EL2_TDRA | MDCR_EL2_TDOSA);
+ /* Trap debug register access? */
other patch
if (!vcpu-arch.debug_flags KVM_ARM64_DEBUG_DIRTY)
vcpu-arch.mdcr_el2 |= MDCR_EL2_TDA;
else
vcpu-arch.mdcr_el2 = ~MDCR_EL2_TDA;
+ /* Trap breakpoints? */
+ if (vcpu-guest_debug KVM_GUESTDBG_USE_SW_BP)
+ vcpu-arch.mdcr_el2 |= MDCR_EL2_TDE;
+ else
+ vcpu-arch.mdcr_el2 = ~MDCR_EL2_TDE;
so now you're trapping all debug exceptions, right?
what happens if the guest is using the hardware to debug debug stuff and
generates other kinds of debug exceptions, like a hardware breakpoint,
will we not see an unhandled exception and the guest being forcefully
killed?
+
}
void kvm_arch_clear_debug(struct kvm_vcpu *vcpu)
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 524fa25..ed1bbb4 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -82,6 +82,37 @@ static int kvm_handle_wfx(struct kvm_vcpu *vcpu, struct
kvm_run *run)
return 1;
}
+/**
+ * kvm_handle_debug_exception - handle a debug exception instruction
handle a software breadkpoint exception
+ *
+ * @vcpu:the vcpu pointer
+ * @run: access to the kvm_run structure for results
+ *
+ * We route all debug exceptions through the same handler as we
all debug exceptions? software breakpoints and all? then why the above
shot text?
+ * just need to report the PC and the HSR values to userspace.
+ * Userspace may decide to re-inject the exception and deliver it to
+ * the guest if it wasn't for the host to deal with.
now I'm confused - does userspace setup the guest to receive an
exception or does it tell KVM to emulate an exception for the guest or
do we execute the breakpoint without trapping the debug exception?
+ */
+static int kvm_handle_guest_debug(struct kvm_vcpu *vcpu, struct kvm_run *run)
+{
+ u32 hsr = kvm_vcpu_get_hsr(vcpu);
+
+ run-exit_reason = KVM_EXIT_DEBUG;
+ run-debug.arch.hsr = hsr;
+
+ switch (hsr ESR_ELx_EC_SHIFT) {
+ case ESR_ELx_EC_BKPT32:
+ case ESR_ELx_EC_BRK64:
+ run-debug.arch.pc = *vcpu_pc(vcpu);
+ break;
+ default:
+ kvm_err(%s: un-handled case hsr: %#08x\n,
+ __func__, (unsigned int) hsr);
this should never happen right?
+ break;
+ }
+ return 0;
+}
+
static exit_handle_fn arm_exit_handlers[] = {
[ESR_ELx_EC_WFx]= kvm_handle_wfx,
[ESR_ELx_EC_CP15_32]= kvm_handle_cp15_32,
@@ -96,6 +127,8 @@ static exit_handle_fn
Re: [PATCH v2 02/10] KVM: define common __KVM_GUESTDBG_USE_SW/HW_BP values
On Mon, Apr 13, 2015 at 03:51:33PM +0100, Alex Bennée wrote:
Christoffer Dall christoffer.d...@linaro.org writes:
On Tue, Mar 31, 2015 at 04:08:00PM +0100, Alex Bennée wrote:
Currently x86, powerpc and soon arm64 use the same two architecture
specific bits for guest debug support for software and hardware
breakpoints. This makes the shared values explicit while leaving the
gate open for another architecture to use some other value if they
really really want to.
Signed-off-by: Alex Bennée alex.ben...@linaro.org
diff --git a/arch/powerpc/include/uapi/asm/kvm.h
b/arch/powerpc/include/uapi/asm/kvm.h
index ab4d473..1731569 100644
--- a/arch/powerpc/include/uapi/asm/kvm.h
+++ b/arch/powerpc/include/uapi/asm/kvm.h
@@ -310,8 +310,8 @@ struct kvm_guest_debug_arch {
* and upper 16 bits are architecture specific. Architecture specific
defines
* that ioctl is for setting hardware breakpoint or software breakpoint.
*/
-#define KVM_GUESTDBG_USE_SW_BP0x0001
-#define KVM_GUESTDBG_USE_HW_BP0x0002
+#define KVM_GUESTDBG_USE_SW_BP__KVM_GUESTDBG_USE_SW_BP
+#define KVM_GUESTDBG_USE_HW_BP__KVM_GUESTDBG_USE_HW_BP
/* definition of registers in kvm_run */
struct kvm_sync_regs {
diff --git a/arch/x86/include/uapi/asm/kvm.h
b/arch/x86/include/uapi/asm/kvm.h
index d7dcef5..1438202 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -250,8 +250,8 @@ struct kvm_debug_exit_arch {
__u64 dr7;
};
-#define KVM_GUESTDBG_USE_SW_BP0x0001
-#define KVM_GUESTDBG_USE_HW_BP0x0002
+#define KVM_GUESTDBG_USE_SW_BP__KVM_GUESTDBG_USE_SW_BP
+#define KVM_GUESTDBG_USE_HW_BP__KVM_GUESTDBG_USE_HW_BP
#define KVM_GUESTDBG_INJECT_DB0x0004
#define KVM_GUESTDBG_INJECT_BP0x0008
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 5eedf84..ce2db14 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -525,8 +525,16 @@ struct kvm_s390_irq {
/* for KVM_SET_GUEST_DEBUG */
-#define KVM_GUESTDBG_ENABLE 0x0001
-#define KVM_GUESTDBG_SINGLESTEP 0x0002
+#define KVM_GUESTDBG_ENABLE (1 0)
+#define KVM_GUESTDBG_SINGLESTEP (1 1)
+
+/*
+ * Architecture specific stuff uses the top 16 bits of the field,
can you be more specific than 'stuff' here? features?
+ * however there is some shared commonality for the common cases
I don't like this sentence; shared commonality is a pleonasm and the use
of however makes it sounds like there's some caveat here.
OK I can see that - after I looked it up ;-)
If the top 16 bits are indeed arhictecture specific, then I think they
should just be defined in their architecture specific headers. Unless
the idea here is that there's a fixed set of of flags that architectures
can choose to support, in which case it should simply be defined in the
common header.
Well an architecture might not support some features and want to use
those bits for something else? I didn't want to force the bottom two
of the architecture specific bits to wasted if the features don't exist.
In that case I think the definition is local to each architecture and
should indeed just be duplicated. The __ definitions complicate more
than they help as they are exported to userspace etc. The KVM
maintainers may have a different view on this though.
-Christoffer
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [Linaro-uefi] UEFI on KVM fails to start on juno on cortex-a57 cluster
On 14 April 2015 at 13:07, Christoffer Dall christoffer.d...@linaro.org wrote: On Mon, Apr 13, 2015 at 11:04:00AM +0200, Ard Biesheuvel wrote: On 27 March 2015 at 01:02, Ard Biesheuvel ard.biesheu...@linaro.org wrote: On 26 March 2015 at 09:09, Riku Voipio riku.voi...@linaro.org wrote: On 25 March 2015 at 21:32, Ard Biesheuvel ard.biesheu...@linaro.org wrote: On 25 March 2015 at 17:14, Ard Biesheuvel ard.biesheu...@linaro.org wrote: On 25 March 2015 at 17:14, Ard Biesheuvel ard.biesheu...@linaro.org wrote: On 25 March 2015 at 07:59, Riku Voipio riku.voi...@linaro.org wrote: Hi, It appears on juno, I can start kvm with UEFI only on cortex-a53 cores: taskset -c 0 qemu-system-aarch64 -m 1024 -cpu host -M virt -bios QEMU_EFI.fd -enable-kvm -nographic - works: UEFI Interactive Shell v2.0 taskset -c 1 qemu-system-aarch64 -m 1024 -cpu host -M virt -bios QEMU_EFI.fd -enable-kvm -nographic - hangs at cpu spinning 100% ... I can reproduce the hang, both with your UEFI binary and my own release build. The debug build works fine, unfortunately... Tianocore built from master as of today, that is. OK, it appears that we were missing some cache maintenance. It is not obvious how that should affect A57 only, but with these patches, I can now reliably run the release version on my Seattle A57 https://git.linaro.org/people/ard.biesheuvel/uefi-next.git/shortlog/refs/heads/qemu-xen-cache-maintenance Thanks. Do you know when there would be a new build on releases or snapshots? Let me check with Leif. We have another candidate patch now that he could perhaps apply and kick off a build? I now have independent confirmation (from Laszlo Ersek) that the cache maintenance patches I am proposing fix the issue on Seattle. Which patches are those? For UEFI? Yes, for Tianocore. http://thread.gmane.org/gmane.comp.bios.tianocore.devel/13665 Hopefully this means Juno is fixed as well. I am trying to get a snapshot out asap, today or tomorrow perhaps? Thanks, -Christoffer ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 08/10] KVM: arm64: guest debug, HW assisted debug support
On Fri, Apr 10, 2015 at 02:25:21PM +0200, Andrew Jones wrote: [...] --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -196,16 +196,49 @@ static bool trap_dbgauthstatus_el1(struct kvm_vcpu *vcpu, * - If the dirty bit is set, save guest registers, restore host * registers and clear the dirty bit. This ensure that the host can * now use the debug registers. + * + * We also use this mechanism to set-up the debug registers for guest setup since I'm in this mood: setup: noun or adjective set-up: noun derived from the phrasal verb, example Run! It's a set-up. set up: verb -Christoffer ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH] KVM: arm: irqfd: fix value returned by kvm_irq_map_gsi
On Mon, Apr 13, 2015 at 03:01:59PM +0200, Eric Auger wrote:
irqfd/arm curently does not support routing. kvm_irq_map_gsi is
supposed to return all the routing entries associated with the
provided gsi and return the number of those entries. We should
return 0 at this point.
Signed-off-by: Eric Auger eric.au...@linaro.org
---
virt/kvm/arm/vgic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 9ad074e..9b4f7d4 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -1947,7 +1947,7 @@ int kvm_irq_map_gsi(struct kvm *kvm,
struct kvm_kernel_irq_routing_entry *entries,
int gsi)
{
- return gsi;
+ return 0;
}
int kvm_irq_map_chip_pin(struct kvm *kvm, unsigned irqchip, unsigned pin)
--
Acked-by: Christoffer Dall christoffer.d...@linaro.org
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 08/10] KVM: arm64: guest debug, HW assisted debug support
On Tue, Mar 31, 2015 at 04:08:06PM +0100, Alex Bennée wrote:
This adds support for userspace to control the HW debug registers for
guest debug. We'll only copy the $ARCH defined number across as that is
all that hyp.S will use anyway.
I don't really understand what this sentence means?
I've moved some helper functions into
the hw_breakpoint.h header for re-use.
As with single step we need to tweak the guest registers to enable the
exceptions so we need to save and restore those bits.
Two new capabilities have been added to the KVM_EXTENSION ioctl to allow
userspace to query the number of hardware break and watch points
available on the host hardware.
As QEMU tests for watchpoints based on the address and not the PC we
also need to export the value of far_el2 to userspace.
Signed-off-by: Alex Bennée alex.ben...@linaro.org
---
v2
- switched to C setup
- replace host debug registers directly into context
- minor tweak to api docs
- setup right register for debug
- add FAR_EL2 to debug exit structure
- add support fro trapping debug register access
diff --git a/Documentation/virtual/kvm/api.txt
b/Documentation/virtual/kvm/api.txt
index 17d4f9c..ac34093 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -2627,7 +2627,7 @@ The top 16 bits of the control field are architecture
specific control
flags which can include the following:
- KVM_GUESTDBG_USE_SW_BP: using software breakpoints [x86, arm64]
- - KVM_GUESTDBG_USE_HW_BP: using hardware breakpoints [x86, s390]
+ - KVM_GUESTDBG_USE_HW_BP: using hardware breakpoints [x86, s390, arm64]
- KVM_GUESTDBG_INJECT_DB: inject DB type exception [x86]
- KVM_GUESTDBG_INJECT_BP: inject BP type exception [x86]
- KVM_GUESTDBG_EXIT_PENDING: trigger an immediate guest exit [s390]
@@ -2642,6 +2642,10 @@ updated to the correct (supplied) values.
The second part of the structure is architecture specific and
typically contains a set of debug registers.
+For arm64 the number of debug registers is implementation defined and
+can be determined by querying the KVM_CAP_GUEST_DEBUG_HW_BPS and
+KVM_CAP_GUEST_DEBUG_HW_WPS capabilities.
+
can you document their behavior more specifically? I assume they both
return 0 if HW assisted debugging is not supported and return the number
of implemented hardware registers otherwise?
How does this work on big.LITTLE systems where cores may have a different
number of implemented registers?
When debug events exit the main run loop with the reason
KVM_EXIT_DEBUG with the kvm_debug_exit_arch part of the kvm_run
structure containing architecture specific debug information.
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index c1ed8cb..a286026 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -306,6 +306,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
#define KVM_GUESTDBG_VALID (KVM_GUESTDBG_ENABLE |\
KVM_GUESTDBG_USE_SW_BP | \
+ KVM_GUESTDBG_USE_HW_BP | \
KVM_GUESTDBG_SINGLESTEP)
/**
@@ -328,6 +329,26 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu
*vcpu,
return -EINVAL;
vcpu-guest_debug = dbg-control;
+
+ /* Hardware assisted Break and Watch points */
+ if (vcpu-guest_debug KVM_GUESTDBG_USE_HW_BP) {
+ int nb = get_num_brps();
+ int nw = get_num_wrps();
+
+ /* Copy across up to IMPDEF debug registers to our
+ * shadow copy in the vcpu structure. The debug code
+ * will then set them up before we re-enter the guest.
+ */
+ memcpy(vcpu-arch.guest_debug_regs.dbg_bcr,
+ dbg-arch.dbg_bcr, sizeof(__u64)*nb);
+ memcpy(vcpu-arch.guest_debug_regs.dbg_bvr,
+ dbg-arch.dbg_bvr, sizeof(__u64)*nb);
+ memcpy(vcpu-arch.guest_debug_regs.dbg_wcr,
+ dbg-arch.dbg_wcr, sizeof(__u64)*nw);
+ memcpy(vcpu-arch.guest_debug_regs.dbg_wvr,
+ dbg-arch.dbg_wvr, sizeof(__u64)*nw);
+ }
+
} else {
/* If not enabled clear all flags */
vcpu-guest_debug = 0;
diff --git a/arch/arm64/include/asm/hw_breakpoint.h
b/arch/arm64/include/asm/hw_breakpoint.h
index 52b484b..c450552 100644
--- a/arch/arm64/include/asm/hw_breakpoint.h
+++ b/arch/arm64/include/asm/hw_breakpoint.h
@@ -130,6 +130,18 @@ static inline void ptrace_hw_copy_thread(struct
task_struct *task)
}
#endif
+/* Determine number of BRP registers available. */
+static inline int get_num_brps(void)
+{
+ return ((read_cpuid(ID_AA64DFR0_EL1) 12) 0xf) + 1;
+}
+
+/* Determine
Re: [PATCH v2 09/10] KVM: arm64: trap nested debug register access
On Mon, Apr 13, 2015 at 08:59:21AM +0100, Alex Bennée wrote:
[...]
+ /* MDSCR_EL1 */
+ if (r-reg == MDSCR_EL1) {
+ if (p-is_write)
+ vcpu_debug_saved_reg(vcpu, mdscr_el1) =
+ *vcpu_reg(vcpu, p-Rt);
+ else
+ *vcpu_reg(vcpu, p-Rt) =
+ vcpu_debug_saved_reg(vcpu, mdscr_el1);
With this lines wrapping, {}'s might be nice.
My natural inclination is to wrap in {}'s but I know the kernel is a fan
of the single-statement if forms.
It's accepted to use braces for multi-line single statements - and I
prefer it too :)
-Christoffer
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
