KVM Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2] KVM: PPC: Report single stepping capability
@ 2019-05-29 22:22 Fabiano Rosas
  2019-06-17  6:16 ` Paul Mackerras
  0 siblings, 1 reply; 3+ messages in thread
From: Fabiano Rosas @ 2019-05-29 22:22 UTC (permalink / raw)
  To: kvm-ppc
  Cc: linuxppc-dev, kvm, paulus, benh, mpe, pbonzini, rkrcmar, david, aik

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>
---

v1 -> v2:
 - add capability description to Documentation/virtual/kvm/api.txt

 Documentation/virtual/kvm/api.txt | 3 +++
 arch/powerpc/kvm/powerpc.c        | 5 +++++
 include/uapi/linux/kvm.h          | 1 +
 3 files changed, 9 insertions(+)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index ba6c42c576dd..a77643bfa917 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -2969,6 +2969,9 @@ can be determined by querying the KVM_CAP_GUEST_DEBUG_HW_BPS and
 KVM_CAP_GUEST_DEBUG_HW_WPS capabilities which return a positive number
 indicating the number of supported registers.
 
+For ppc, the KVM_CAP_PPC_GUEST_DEBUG_SSTEP capability indicates whether
+the single-step debug event (KVM_GUESTDBG_SINGLESTEP) is supported.
+
 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/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 3393b166817a..fd7e7d55637e 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 2fe12b40d503..cad9fcd90f39 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -993,6 +993,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_ARM_SVE 170
 #define KVM_CAP_ARM_PTRAUTH_ADDRESS 171
 #define KVM_CAP_ARM_PTRAUTH_GENERIC 172
+#define KVM_CAP_PPC_GUEST_DEBUG_SSTEP 173
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] KVM: PPC: Report single stepping capability
  2019-05-29 22:22 [PATCH v2] KVM: PPC: Report single stepping capability Fabiano Rosas
@ 2019-06-17  6:16 ` Paul Mackerras
  2019-06-17 19:25   ` Fabiano Rosas
  0 siblings, 1 reply; 3+ messages in thread
From: Paul Mackerras @ 2019-06-17  6:16 UTC (permalink / raw)
  To: Fabiano Rosas
  Cc: kvm-ppc, linuxppc-dev, kvm, benh, mpe, pbonzini, rkrcmar, david, aik

On Wed, May 29, 2019 at 07:22:19PM -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.

Comment/question below:

> --- 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

In the !CONFIG_BOOKE case, this will fall through to code which will
return 0 for HV KVM or 1 for PR KVM.  Is that what was intended?
If so, then why do we need the CONFIG_BOOKE case?  Isn't hv_enabled
always 0 on Book E?

In any case, I think this needs at least a /* fall through */ comment
in the code, and something explicit in the patch description to say
that we intend to return 1 on PR KVM.

Paul.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] KVM: PPC: Report single stepping capability
  2019-06-17  6:16 ` Paul Mackerras
@ 2019-06-17 19:25   ` Fabiano Rosas
  0 siblings, 0 replies; 3+ messages in thread
From: Fabiano Rosas @ 2019-06-17 19:25 UTC (permalink / raw)
  To: Paul Mackerras
  Cc: kvm-ppc, linuxppc-dev, kvm, benh, mpe, pbonzini, rkrcmar, david, aik

Paul Mackerras <paulus@ozlabs.org> writes:

> On Wed, May 29, 2019 at 07:22:19PM -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.
>
> Comment/question below:
>
>> --- 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
>
> In the !CONFIG_BOOKE case, this will fall through to code which will
> return 0 for HV KVM or 1 for PR KVM.  Is that what was intended?

Yes. The intention is to return 0 for HV and 1 for everything else.

> If so, then why do we need the CONFIG_BOOKE case?  Isn't hv_enabled
> always 0 on Book E?

Good point. I made a mistake there indeed.

> In any case, I think this needs at least a /* fall through */ comment
> in the code, and something explicit in the patch description to say
> that we intend to return 1 on PR KVM.

I'll send another version.

Thanks


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-29 22:22 [PATCH v2] KVM: PPC: Report single stepping capability Fabiano Rosas
2019-06-17  6:16 ` Paul Mackerras
2019-06-17 19:25   ` Fabiano Rosas

KVM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvm/0 kvm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvm kvm/ https://lore.kernel.org/kvm \
		kvm@vger.kernel.org kvm@archiver.kernel.org
	public-inbox-index kvm


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.kvm


AGPL code for this site: git clone https://public-inbox.org/ public-inbox