[RFC PATCH 0/1] KVM: PPC: Inform userspace about singlestep support

[RFC PATCH 0/1] KVM: PPC: Inform userspace about singlestep support

[Fabiano Rosas]

I am looking for a way to inform userspace about the lack of an
implementation in KVM HV for single stepping of instructions
(KVM_GUESTDGB_SINGLESTEP bit from SET_GUEST_DEBUG ioctl).

This will be used by QEMU to decide whether to attempt a call to the
set_guest_debug ioctl (for BookE, KVM PR) or fallback to a QEMU only
implementation (for KVM HV).

QEMU thread:
http://patchwork.ozlabs.org/cover/1049811/

My current proposal is to introduce a ppc-specific capability for
this. However I'm not sure if this would be better as a cap common for
all architectures or even if it should report on all of the possible
set_guest_debug flags to cover for the future.

Please comment. Thanks.


Fabiano Rosas (1):
  KVM: PPC: Report single stepping capability

 arch/powerpc/kvm/powerpc.c | 5 +++++
 include/uapi/linux/kvm.h   | 1 +
 2 files changed, 6 insertions(+)

--
2.20.1

[RFC PATCH 1/1] KVM: PPC: Report single stepping capability

[Fabiano Rosas]

When calling the KVM_SET_GUEST_DEBUG ioctl, userspace might request
the next instruction to be single stepped via the
KVM_GUESTDBG_SINGLESTEP control bit of the kvm_guest_debug structure.

We currently don't have support for guest single stepping implemented
in Book3S HV.

This patch adds the KVM_CAP_PPC_GUEST_DEBUG_SSTEP capability in order
to inform userspace about the state of single stepping support.

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 arch/powerpc/kvm/powerpc.c | 5 +++++
 include/uapi/linux/kvm.h   | 1 +
 2 files changed, 6 insertions(+)

diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 8885377ec3e0..5ba990b0ec74 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -538,6 +538,11 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	case KVM_CAP_IMMEDIATE_EXIT:
 		r = 1;
 		break;
+	case KVM_CAP_PPC_GUEST_DEBUG_SSTEP:
+#ifdef CONFIG_BOOKE
+		r = 1;
+		break;
+#endif
 	case KVM_CAP_PPC_PAIRED_SINGLES:
 	case KVM_CAP_PPC_OSI:
 	case KVM_CAP_PPC_GET_PVINFO:
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 6d4ea4b6c922..33e8a4db867e 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -988,6 +988,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_ARM_VM_IPA_SIZE 165
 #define KVM_CAP_MANUAL_DIRTY_LOG_PROTECT 166
 #define KVM_CAP_HYPERV_CPUID 167
+#define KVM_CAP_PPC_GUEST_DEBUG_SSTEP 168
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
2.20.1

Re: [RFC PATCH 1/1] KVM: PPC: Report single stepping capability

[Alexey Kardashevskiy]

On 21/03/2019 05:39, Fabiano Rosas wrote:
> When calling the KVM_SET_GUEST_DEBUG ioctl, userspace might request
> the next instruction to be single stepped via the
> KVM_GUESTDBG_SINGLESTEP control bit of the kvm_guest_debug structure.
> 
> We currently don't have support for guest single stepping implemented
> in Book3S HV.
> 
> This patch adds the KVM_CAP_PPC_GUEST_DEBUG_SSTEP capability in order
> to inform userspace about the state of single stepping support.
> 
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
>  arch/powerpc/kvm/powerpc.c | 5 +++++
>  include/uapi/linux/kvm.h   | 1 +
>  2 files changed, 6 insertions(+)
> 
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 8885377ec3e0..5ba990b0ec74 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -538,6 +538,11 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>  	case KVM_CAP_IMMEDIATE_EXIT:
>  		r = 1;
>  		break;
> +	case KVM_CAP_PPC_GUEST_DEBUG_SSTEP:
> +#ifdef CONFIG_BOOKE


In the cover letter (which is not really required for a single patch)
you say the capability will be present for BookE and PR KVM (which
Book3s) but here it is BookE only, is that intentional?

Also, you need to update Documentation/virtual/kvm/api.txt for the new
capability. After reading which I started wondering could not we just
use existing KVM_CAP_GUEST_DEBUG_HW_BPS?


> +		r = 1;
> +		break;
> +#endif
>  	case KVM_CAP_PPC_PAIRED_SINGLES:
>  	case KVM_CAP_PPC_OSI:
>  	case KVM_CAP_PPC_GET_PVINFO:
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 6d4ea4b6c922..33e8a4db867e 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -988,6 +988,7 @@ struct kvm_ppc_resize_hpt {
>  #define KVM_CAP_ARM_VM_IPA_SIZE 165
>  #define KVM_CAP_MANUAL_DIRTY_LOG_PROTECT 166
>  #define KVM_CAP_HYPERV_CPUID 167
> +#define KVM_CAP_PPC_GUEST_DEBUG_SSTEP 168
>  
>  #ifdef KVM_CAP_IRQ_ROUTING
>  
> 

-- 
Alexey

Re: [RFC PATCH 1/1] KVM: PPC: Report single stepping capability

[Fabiano Rosas]

Alexey Kardashevskiy <aik@ozlabs.ru> writes:

> In the cover letter (which is not really required for a single patch)
> you say the capability will be present for BookE and PR KVM (which
> Book3s) but here it is BookE only, is that intentional?

A few lines below (falling through) we have:

        /* We support this only for PR */
        r = !hv_enabled;

> Also, you need to update Documentation/virtual/kvm/api.txt for the new
> capability. After reading which I started wondering could not we just
> use existing KVM_CAP_GUEST_DEBUG_HW_BPS?

We _could_, but I think that would conflate two different
concepts. Single stepping does not necessarily makes use of hardware
breakpoints (e.g. Trace Interrupt on Book3s PR).

I also think we should use KVM_CAP_GUEST_DEBUG_HW_BPS in the future to
let QEMU know about: i) the lack of hardware breakpoints in Book3s and
ii) BookE's hardware breakpoints (Instruction Address Compare) that are
currently not being reported via HW_BPS.

Re: [RFC PATCH 1/1] KVM: PPC: Report single stepping capability

[Paul Mackerras]

On Wed, Mar 20, 2019 at 03:39:50PM -0300, Fabiano Rosas wrote:
> When calling the KVM_SET_GUEST_DEBUG ioctl, userspace might request
> the next instruction to be single stepped via the
> KVM_GUESTDBG_SINGLESTEP control bit of the kvm_guest_debug structure.
> 
> We currently don't have support for guest single stepping implemented
> in Book3S HV.
> 
> This patch adds the KVM_CAP_PPC_GUEST_DEBUG_SSTEP capability in order
> to inform userspace about the state of single stepping support.
> 
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
>  arch/powerpc/kvm/powerpc.c | 5 +++++
>  include/uapi/linux/kvm.h   | 1 +
>  2 files changed, 6 insertions(+)

I assume since this is [RFC] that you're not expecting it to go
upstream as-is.  If it were to go upstream it would need to include an
update to Documentation/virtual/kvm/api.txt.

Paul.