Re: [PATCH v2 06/10] KVM: arm64: guest debug, add SW break point support
On Wed, Apr 29, 2015 at 10:18:18AM +0100, Alex Bennée wrote: Christoffer Dall christoffer.d...@linaro.org writes: On Tue, Apr 28, 2015 at 03:37:01PM +0100, Alex Bennée wrote: Christoffer Dall christoffer.d...@linaro.org writes: On Tue, Apr 28, 2015 at 10:34:12AM +0100, Peter Maydell wrote: On 28 April 2015 at 09:42, Alex Bennée alex.ben...@linaro.org wrote: Peter Maydell peter.mayd...@linaro.org writes: Does the kernel already have a conveniently implemented inject exception into guest lump of code? If so it might be less effort to do it that way round, maybe. So you pointed out we can't just re-inject the exceptions we get as we need to map from things like ESR_ELx_EC_WATCHPT_LOW to ESR_ELx_EC_WATCHPT_CUR before re-injection. Of course if it is as simple as modifying the ESR_EL1 register and returning +ve in the handle_exit path then I can do that but I assumed if any other wrangling needs doing it should be done in userspace. Well, somebody's got to do it, and it's the same amount of work either way (fiddling with ESR, making sure we direct the guest to the right exception vector entry point, maybe a few other things). We already have code in the kernel to inject data/instruction aborts, but not sure how much benefit there is in re-using that. It's up to you really, but I think the kernel code should be clear about what the intention is so that we don't end up in a situation where: (1) The intended behavior is unclear/vague, and (2) it doesn't actually work in practice so nobody can follow the code. Certainly there are some cases where the kernel doesn't have all the information. For example it doesn't know if the soft break was inserted by the guest or the host. That to me favours the let userspace deal with the ugly approach. Not sure I follow. If it's an exception for the guest, then that must be because the guest put in the breakpoint instruction, right? No the host can add breakpoint instructions as well. They both generate the same (redirected) exception to the hypervisor which then has to figure out who planted the breakpoint and where the eventual exception will be handled. I understand this; let's just rewind here. If you've concluded that the exception is for the guest, then the guest must have placed the breakpoint instruction there, correct? Otherwise, the exception is for the hypervisor and the discussion about how to inject an exception for the guest is invalid. Or are you talking about the corner case where the host uses a soft breakpoint to get a breakpoint on an instruction which is also a breakpoint in the guest? However, that's a separate discussion from that of *how* userspace or the kernel then injects an exception to the guest. By using some QEMU TCG functionality or by QEMU calling back into KVM and asking it to inject an exception for it. I don't know if there is explicit TCG functionality to use but QEMU can set the registers and PC up for exception entry and re-enter KVM. I also understand this. I think Peter's point was exactly that if we have existing code somewhere which we can reuse, then we should consider reusing it. Again, I don't care particularly which way, I just want the expected working behavior to be clearly defined. -Christoffer ___ 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
Christoffer Dall christoffer.d...@linaro.org writes: On Tue, Apr 28, 2015 at 03:37:01PM +0100, Alex Bennée wrote: Christoffer Dall christoffer.d...@linaro.org writes: On Tue, Apr 28, 2015 at 10:34:12AM +0100, Peter Maydell wrote: On 28 April 2015 at 09:42, Alex Bennée alex.ben...@linaro.org wrote: Peter Maydell peter.mayd...@linaro.org writes: Does the kernel already have a conveniently implemented inject exception into guest lump of code? If so it might be less effort to do it that way round, maybe. So you pointed out we can't just re-inject the exceptions we get as we need to map from things like ESR_ELx_EC_WATCHPT_LOW to ESR_ELx_EC_WATCHPT_CUR before re-injection. Of course if it is as simple as modifying the ESR_EL1 register and returning +ve in the handle_exit path then I can do that but I assumed if any other wrangling needs doing it should be done in userspace. Well, somebody's got to do it, and it's the same amount of work either way (fiddling with ESR, making sure we direct the guest to the right exception vector entry point, maybe a few other things). We already have code in the kernel to inject data/instruction aborts, but not sure how much benefit there is in re-using that. It's up to you really, but I think the kernel code should be clear about what the intention is so that we don't end up in a situation where: (1) The intended behavior is unclear/vague, and (2) it doesn't actually work in practice so nobody can follow the code. Certainly there are some cases where the kernel doesn't have all the information. For example it doesn't know if the soft break was inserted by the guest or the host. That to me favours the let userspace deal with the ugly approach. Not sure I follow. If it's an exception for the guest, then that must be because the guest put in the breakpoint instruction, right? No the host can add breakpoint instructions as well. They both generate the same (redirected) exception to the hypervisor which then has to figure out who planted the breakpoint and where the eventual exception will be handled. However, that's a separate discussion from that of *how* userspace or the kernel then injects an exception to the guest. By using some QEMU TCG functionality or by QEMU calling back into KVM and asking it to inject an exception for it. I don't know if there is explicit TCG functionality to use but QEMU can set the registers and PC up for exception entry and re-enter KVM. What am I missing? -Christoffer -- 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
Christoffer Dall christoffer.d...@linaro.org writes: On Wed, Apr 29, 2015 at 10:18:18AM +0100, Alex Bennée wrote: Christoffer Dall christoffer.d...@linaro.org writes: On Tue, Apr 28, 2015 at 03:37:01PM +0100, Alex Bennée wrote: Christoffer Dall christoffer.d...@linaro.org writes: On Tue, Apr 28, 2015 at 10:34:12AM +0100, Peter Maydell wrote: On 28 April 2015 at 09:42, Alex Bennée alex.ben...@linaro.org wrote: Peter Maydell peter.mayd...@linaro.org writes: Does the kernel already have a conveniently implemented inject exception into guest lump of code? If so it might be less effort to do it that way round, maybe. snip Certainly there are some cases where the kernel doesn't have all the information. For example it doesn't know if the soft break was inserted by the guest or the host. That to me favours the let userspace deal with the ugly approach. Not sure I follow. If it's an exception for the guest, then that must be because the guest put in the breakpoint instruction, right? No the host can add breakpoint instructions as well. They both generate the same (redirected) exception to the hypervisor which then has to figure out who planted the breakpoint and where the eventual exception will be handled. I understand this; let's just rewind here. If you've concluded that the exception is for the guest, then the guest must have placed the breakpoint instruction there, correct? Otherwise, the exception is for the hypervisor and the discussion about how to inject an exception for the guest is invalid. But only userspace has enough information to make that conclusion (after searching the list of breakpoints it added to the code). So from userspace we can: - re-enter KVM telling it to re-route the exception it just delivered to userspace somehow or - make the changes to deliver the exception in userspace and re-enter KVM as normal. It seems to me if we have already exited into userspace it may as well clean up if it has all the information it needs? Or are you talking about the corner case where the host uses a soft breakpoint to get a breakpoint on an instruction which is also a breakpoint in the guest? I think in this case host debugging just wins. However, that's a separate discussion from that of *how* userspace or the kernel then injects an exception to the guest. By using some QEMU TCG functionality or by QEMU calling back into KVM and asking it to inject an exception for it. I don't know if there is explicit TCG functionality to use but QEMU can set the registers and PC up for exception entry and re-enter KVM. I also understand this. I think Peter's point was exactly that if we have existing code somewhere which we can reuse, then we should consider reusing it. I'm not sure such code exists. The only injection code I know of in KVMs handle_exit code where a +ve return value signals KVM to deliver the exception to the guest. This is used by the hvc and svc handlers after calling kvm_inject_undefined() and the wfx handler which advances the PC first. Again, I don't care particularly which way, I just want the expected working behavior to be clearly defined. I think it makes sense to do it in userspace. I have the kernels inject_fault code for reference for what needs setting up but I'll see if I can find anything in QEMU that already handles this for some other thing (although I don't think it does at first glance). -- 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, Apr 28, 2015 at 10:34:12AM +0100, Peter Maydell wrote: On 28 April 2015 at 09:42, Alex Bennée alex.ben...@linaro.org wrote: Peter Maydell peter.mayd...@linaro.org writes: Does the kernel already have a conveniently implemented inject exception into guest lump of code? If so it might be less effort to do it that way round, maybe. So you pointed out we can't just re-inject the exceptions we get as we need to map from things like ESR_ELx_EC_WATCHPT_LOW to ESR_ELx_EC_WATCHPT_CUR before re-injection. Of course if it is as simple as modifying the ESR_EL1 register and returning +ve in the handle_exit path then I can do that but I assumed if any other wrangling needs doing it should be done in userspace. Well, somebody's got to do it, and it's the same amount of work either way (fiddling with ESR, making sure we direct the guest to the right exception vector entry point, maybe a few other things). We already have code in the kernel to inject data/instruction aborts, but not sure how much benefit there is in re-using that. It's up to you really, but I think the kernel code should be clear about what the intention is so that we don't end up in a situation where: (1) The intended behavior is unclear/vague, and (2) it doesn't actually work in practice so nobody can follow the code. Thanks, -Christoffer ___ 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
Christoffer Dall christoffer.d...@linaro.org writes: On Tue, Apr 28, 2015 at 10:34:12AM +0100, Peter Maydell wrote: On 28 April 2015 at 09:42, Alex Bennée alex.ben...@linaro.org wrote: Peter Maydell peter.mayd...@linaro.org writes: Does the kernel already have a conveniently implemented inject exception into guest lump of code? If so it might be less effort to do it that way round, maybe. So you pointed out we can't just re-inject the exceptions we get as we need to map from things like ESR_ELx_EC_WATCHPT_LOW to ESR_ELx_EC_WATCHPT_CUR before re-injection. Of course if it is as simple as modifying the ESR_EL1 register and returning +ve in the handle_exit path then I can do that but I assumed if any other wrangling needs doing it should be done in userspace. Well, somebody's got to do it, and it's the same amount of work either way (fiddling with ESR, making sure we direct the guest to the right exception vector entry point, maybe a few other things). We already have code in the kernel to inject data/instruction aborts, but not sure how much benefit there is in re-using that. It's up to you really, but I think the kernel code should be clear about what the intention is so that we don't end up in a situation where: (1) The intended behavior is unclear/vague, and (2) it doesn't actually work in practice so nobody can follow the code. Certainly there are some cases where the kernel doesn't have all the information. For example it doesn't know if the soft break was inserted by the guest or the host. That to me favours the let userspace deal with the ugly approach. -- 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
Peter Maydell peter.mayd...@linaro.org writes: On 27 April 2015 at 21:04, Christoffer Dall christoffer.d...@linaro.org wrote: On Thu, Apr 23, 2015 at 03:26:53PM +0100, Alex Bennée wrote: Christoffer Dall christoffer.d...@linaro.org writes: On Tue, Mar 31, 2015 at 04:08:04PM +0100, Alex Bennée wrote: + * 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? I've made it all go through userspace as we may have to translate the hypervisor visible exception code to what the guest was expecting to see. ok, so I think you should re-phrase something like: Userspace may decide that this exception is caused by the guest using debugging itself, and may in that case emulate the guest debug exception in userspace before resuming KVM. But does that really work? Given that we don't support KVM-TCG migration, this sounds a little strange. Did we test it? The QEMU patches have a TODO note at the point where you'd want to do this... Design-wise you can do the reinjection in the kernel or in userspace (ppc QEMU does this in userspace, for instance) because it's pretty much just setting registers to fake up the exception-entry into EL1. Code-wise QEMU's ARM code isn't set up to do it right now, but it shouldn't be too difficult to persuade the TCG exception-entry code to work for this case too. Does the kernel already have a conveniently implemented inject exception into guest lump of code? If so it might be less effort to do it that way round, maybe. So you pointed out we can't just re-inject the exceptions we get as we need to map from things like ESR_ELx_EC_WATCHPT_LOW to ESR_ELx_EC_WATCHPT_CUR before re-injection. Of course if it is as simple as modifying the ESR_EL1 register and returning +ve in the handle_exit path then I can do that but I assumed if any other wrangling needs doing it should be done in userspace. -- PMM -- 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). why should we trap all debug exceptions? The trap for cp14 register r/w seems enough to record relevant informations to context switch the dbg register while neccessary. ___ 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 06/10] KVM: arm64: guest debug, add SW break point support
David Hildenbrand d...@linux.vnet.ibm.com writes:
This adds support for SW breakpoints inserted by userspace.
We do this by trapping all BKPT exceptions in the
hypervisor (MDCR_EL2_TDE). The kvm_debug_exit_arch carries the address
of the exception. 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)
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? */
This should probably go to the earlier patch.
Agreed.
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;
Again, a candidate for clear_debug?
I don't follow. Changes to mdcr_el2 will only get applied as we jump in
through the hyp.S code. We need to ensure the guest can use SW BKPTs if
we are not.
+
}
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
kvm_handle_guest_debug
Sure.
+ *
+ * @vcpu: the vcpu pointer
+ * @run:access to the kvm_run structure for results
+ *
+ * We route all debug exceptions through the same handler as we
+ * 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.
+ */
+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);
Don't you want to fail hard in this case? This might result in many messages.
returning 0 feels wrong.
You mean a BUG_ON()? Although it would be a cock up on the hosts part to
have an un-handled exception enabled allowing the guest to trigger a
host panic seems excessive.
+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 arm_exit_handlers[] = {
[ESR_ELx_EC_SYS64] = kvm_handle_sys_reg,
[ESR_ELx_EC_IABT_LOW] = kvm_handle_guest_abort,
[ESR_ELx_EC_DABT_LOW] = kvm_handle_guest_abort,
+[ESR_ELx_EC_BKPT32] = kvm_handle_guest_debug,
+[ESR_ELx_EC_BRK64] = kvm_handle_guest_debug,
};
[PATCH v2 06/10] KVM: arm64: guest debug, add SW break point support
This adds support for SW breakpoints inserted by userspace.
We do this by trapping all BKPT exceptions in the
hypervisor (MDCR_EL2_TDE). The kvm_debug_exit_arch carries the address
of the exception. 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)
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? */
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;
+
}
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
+ *
+ * @vcpu: the vcpu pointer
+ * @run: access to the kvm_run structure for results
+ *
+ * We route all debug exceptions through the same handler as we
+ * 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.
+ */
+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);
+ 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 arm_exit_handlers[] = {
[ESR_ELx_EC_SYS64] = kvm_handle_sys_reg,
[ESR_ELx_EC_IABT_LOW] = kvm_handle_guest_abort,
[ESR_ELx_EC_DABT_LOW] = kvm_handle_guest_abort,
+ [ESR_ELx_EC_BKPT32] = kvm_handle_guest_debug,
+ [ESR_ELx_EC_BRK64] = kvm_handle_guest_debug,
};
static exit_handle_fn kvm_get_exit_handler(struct kvm_vcpu *vcpu)
--
2.3.4
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
