[PATCH 0/2] KVM API documentation patches

[PATCH 0/2] KVM API documentation patches

[Alex Bennée]

Hi,

I'm preparing to add ARM KVM GDB support and I went to read the API
documentation and found it surprisingly mute on the subject ;-)

The first patch documents the "new" KVM_SET_GUEST_DEBUG ioctl based on
reviewing the code. I've included a long CC list of people who've
actually done the various implementations who I hope can sanity check
the write-up. The second is a trivial formatting fix for what looks
like a minor merge trip-up.

Alex Bennée (2):
  KVM: document KVM_SET_GUEST_DEBUG api
  KVM: fix api documentation of KVM_GET_EMULATED_CPUID

 Documentation/virtual/kvm/api.txt | 184 +++++++++++++++++++++++---------------
 1 file changed, 114 insertions(+), 70 deletions(-)

-- 
2.1.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/2] KVM: document KVM_SET_GUEST_DEBUG api

[Alex Bennée]

In preparation for working on the ARM implementation I noticed the debug
interface was missing from the API document. I've pieced together the
expected behaviour from the code and commit messages written it up as
best I can.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index d3dde61..723d3f3 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -2575,6 +2575,50 @@ associated with the service will be forgotten, and subsequent RTAS
 calls by the guest for that service will be passed to userspace to be
 handled.
 
+4.87 KVM_SET_GUEST_DEBUG
+
+Capability: KVM_CAP_SET_GUEST_DEBUG
+Architectures: x86, s390, ppc
+Type: vcpu ioctl
+Parameters: struct kvm_guest_debug (in)
+Returns: 0 on success; -1 on error
+
+struct kvm_guest_debug {
+       __u32 control;
+       __u32 pad;
+       struct kvm_guest_debug_arch arch;
+};
+
+Set up the processor specific debug registers and configure vcpu for
+handling guest debug events. There are two parts to the structure, the
+first a control bitfield indicates the type of debug events to handle
+when running. Common control bits are:
+
+  - KVM_GUESTDBG_ENABLE:        guest debugging is enabled
+  - KVM_GUESTDBG_SINGLESTEP:    the next run should single-step
+
+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_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]
+  - KVM_GUESTDBG_EXIT_PENDING:  trigger an immediate guest exit [s390]
+
+For example KVM_GUESTDBG_USE_SW_BP indicates that software breakpoints
+are enabled in memory so we need to ensure breakpoint exceptions are
+correctly trapped and the KVM run loop exits at the breakpoint and not
+running off into the normal guest vector. For KVM_GUESTDBG_USE_HW_BP
+we need to ensure the guest vCPUs architecture specific registers are
+updated to the correct (supplied) values.
+
+The second part of the structure is architecture specific and
+typically contains a set of debug 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.
 
 5. The kvm_run structure
 ------------------------
-- 
2.1.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] KVM: document KVM_SET_GUEST_DEBUG api

[Christoffer Dall]

On Tue, Sep 09, 2014 at 05:27:18PM +0100, Alex Bennée wrote:
> In preparation for working on the ARM implementation I noticed the debug
> interface was missing from the API document. I've pieced together the
> expected behaviour from the code and commit messages written it up as
> best I can.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> 
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index d3dde61..723d3f3 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -2575,6 +2575,50 @@ associated with the service will be forgotten, and subsequent RTAS
>  calls by the guest for that service will be passed to userspace to be
>  handled.
>  
> +4.87 KVM_SET_GUEST_DEBUG
> +
> +Capability: KVM_CAP_SET_GUEST_DEBUG
> +Architectures: x86, s390, ppc
> +Type: vcpu ioctl
> +Parameters: struct kvm_guest_debug (in)
> +Returns: 0 on success; -1 on error

Any specific error codes that need explaining here?

> +
> +struct kvm_guest_debug {
> +       __u32 control;
> +       __u32 pad;
> +       struct kvm_guest_debug_arch arch;
> +};
> +
> +Set up the processor specific debug registers and configure vcpu for

configure *the* vcpu?

> +handling guest debug events. There are two parts to the structure, the

handling guest debug events> does this mean whatever user space needs to
configure so that the guest can deug things or is this for userspace to
debug the guest execution, we could probably be more specific.

> +first a control bitfield indicates the type of debug events to handle
> +when running. Common control bits are:

'when running'? 'when running the vcpu'?

> +
> +  - KVM_GUESTDBG_ENABLE:        guest debugging is enabled
> +  - KVM_GUESTDBG_SINGLESTEP:    the next run should single-step
> +
> +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_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]
> +  - KVM_GUESTDBG_EXIT_PENDING:  trigger an immediate guest exit [s390]
> +
> +For example KVM_GUESTDBG_USE_SW_BP indicates that software breakpoints
> +are enabled in memory so we need to ensure breakpoint exceptions are
> +correctly trapped and the KVM run loop exits at the breakpoint and not
> +running off into the normal guest vector. For KVM_GUESTDBG_USE_HW_BP

I didn't quite understand this bit, can you clarify slightly?  For
example, I don't know what it means that a software breakpoint is
'enabled in memory' and I'm not quite sure what the goal you are arguing
for here is; is this about if this ioctl is used to set a specific
breakpoint then we want to make sure that the breakpoint exception goes
to KVM and not to the guest?

also, are you not missing a 'that' before 'the KVM run loop...'

> +we need to ensure the guest vCPUs architecture specific registers are

please be consistent about the use of vcpu, VCPUs, vCPUs, etc.  The
document seems to prefer lowercase vcpus most places.

> +updated to the correct (supplied) values.
> +
> +The second part of the structure is architecture specific and
> +typically contains a set of debug registers.
> +
> +When debug events exit the main run loop with the reason

I think you should just talk about then the VCPU exits the guest and not
be specific about whether we have a main run loop or not.

> +KVM_EXIT_DEBUG with the kvm_debug_exit_arch part of the kvm_run
> +structure containing architecture specific debug information.

I feel like this sentence leaves me hanging; When they exit with the
debug information, then what?  Or did you mean to say that when they
exit, then the arch struct is filled with the debug info?

>  
>  5. The kvm_run structure
>  ------------------------
> -- 
> 2.1.0
> 

Thanks for taking care of this!
-Christoffer

[PATCH 2/2] KVM: fix api documentation of KVM_GET_EMULATED_CPUID

[Alex Bennée]

It looks like when this was initially merged it got accidentally included
in the following section. I've just moved it back in the correct section
and re-numbered it as other ioctls have been added since.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 723d3f3..60c1582 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -2620,6 +2620,76 @@ 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.
 
+4.88 KVM_GET_EMULATED_CPUID
+
+Capability: KVM_CAP_EXT_EMUL_CPUID
+Architectures: x86
+Type: system ioctl
+Parameters: struct kvm_cpuid2 (in/out)
+Returns: 0 on success, -1 on error
+
+struct kvm_cpuid2 {
+	__u32 nent;
+	__u32 flags;
+	struct kvm_cpuid_entry2 entries[0];
+};
+
+The member 'flags' is used for passing flags from userspace.
+
+#define KVM_CPUID_FLAG_SIGNIFCANT_INDEX		BIT(0)
+#define KVM_CPUID_FLAG_STATEFUL_FUNC		BIT(1)
+#define KVM_CPUID_FLAG_STATE_READ_NEXT		BIT(2)
+
+struct kvm_cpuid_entry2 {
+	__u32 function;
+	__u32 index;
+	__u32 flags;
+	__u32 eax;
+	__u32 ebx;
+	__u32 ecx;
+	__u32 edx;
+	__u32 padding[3];
+};
+
+This ioctl returns x86 cpuid features which are emulated by
+kvm.Userspace can use the information returned by this ioctl to query
+which features are emulated by kvm instead of being present natively.
+
+Userspace invokes KVM_GET_EMULATED_CPUID by passing a kvm_cpuid2
+structure with the 'nent' field indicating the number of entries in
+the variable-size array 'entries'. If the number of entries is too low
+to describe the cpu capabilities, an error (E2BIG) is returned. If the
+number is too high, the 'nent' field is adjusted and an error (ENOMEM)
+is returned. If the number is just right, the 'nent' field is adjusted
+to the number of valid entries in the 'entries' array, which is then
+filled.
+
+The entries returned are the set CPUID bits of the respective features
+which kvm emulates, as returned by the CPUID instruction, with unknown
+or unsupported feature bits cleared.
+
+Features like x2apic, for example, may not be present in the host cpu
+but are exposed by kvm in KVM_GET_SUPPORTED_CPUID because they can be
+emulated efficiently and thus not included here.
+
+The fields in each entry are defined as follows:
+
+  function: the eax value used to obtain the entry
+  index: the ecx value used to obtain the entry (for entries that are
+         affected by ecx)
+  flags: an OR of zero or more of the following:
+        KVM_CPUID_FLAG_SIGNIFCANT_INDEX:
+           if the index field is valid
+        KVM_CPUID_FLAG_STATEFUL_FUNC:
+           if cpuid for this function returns different values for successive
+           invocations; there will be several entries with the same function,
+           all with this flag set
+        KVM_CPUID_FLAG_STATE_READ_NEXT:
+           for KVM_CPUID_FLAG_STATEFUL_FUNC entries, set if this entry is
+           the first entry to be read by a cpu
+   eax, ebx, ecx, edx: the values returned by the cpuid instruction for
+         this function/index combination
+
 5. The kvm_run structure
 ------------------------
 
@@ -2918,76 +2988,6 @@ and usually define the validity of a groups of registers. (e.g. one bit
 };
 
 
-4.81 KVM_GET_EMULATED_CPUID
-
-Capability: KVM_CAP_EXT_EMUL_CPUID
-Architectures: x86
-Type: system ioctl
-Parameters: struct kvm_cpuid2 (in/out)
-Returns: 0 on success, -1 on error
-
-struct kvm_cpuid2 {
-	__u32 nent;
-	__u32 flags;
-	struct kvm_cpuid_entry2 entries[0];
-};
-
-The member 'flags' is used for passing flags from userspace.
-
-#define KVM_CPUID_FLAG_SIGNIFCANT_INDEX		BIT(0)
-#define KVM_CPUID_FLAG_STATEFUL_FUNC		BIT(1)
-#define KVM_CPUID_FLAG_STATE_READ_NEXT		BIT(2)
-
-struct kvm_cpuid_entry2 {
-	__u32 function;
-	__u32 index;
-	__u32 flags;
-	__u32 eax;
-	__u32 ebx;
-	__u32 ecx;
-	__u32 edx;
-	__u32 padding[3];
-};
-
-This ioctl returns x86 cpuid features which are emulated by
-kvm.Userspace can use the information returned by this ioctl to query
-which features are emulated by kvm instead of being present natively.
-
-Userspace invokes KVM_GET_EMULATED_CPUID by passing a kvm_cpuid2
-structure with the 'nent' field indicating the number of entries in
-the variable-size array 'entries'. If the number of entries is too low
-to describe the cpu capabilities, an error (E2BIG) is returned. If the
-number is too high, the 'nent' field is adjusted and an error (ENOMEM)
-is returned. If the number is just right, the 'nent' field is adjusted
-to the number of valid entries in the 'entries' array, which is then
-filled.
-
-The entries returned are the set CPUID bits of the respective features
-which kvm emulates, as returned by the CPUID instruction, with unknown
-or unsupported feature bits cleared.
-
-Features like x2apic, for example, may not be present in the host cpu
-but are exposed by kvm in KVM_GET_SUPPORTED_CPUID because they can be
-emulated efficiently and thus not included here.
-
-The fields in each entry are defined as follows:
-
-  function: the eax value used to obtain the entry
-  index: the ecx value used to obtain the entry (for entries that are
-         affected by ecx)
-  flags: an OR of zero or more of the following:
-        KVM_CPUID_FLAG_SIGNIFCANT_INDEX:
-           if the index field is valid
-        KVM_CPUID_FLAG_STATEFUL_FUNC:
-           if cpuid for this function returns different values for successive
-           invocations; there will be several entries with the same function,
-           all with this flag set
-        KVM_CPUID_FLAG_STATE_READ_NEXT:
-           for KVM_CPUID_FLAG_STATEFUL_FUNC entries, set if this entry is
-           the first entry to be read by a cpu
-   eax, ebx, ecx, edx: the values returned by the cpuid instruction for
-         this function/index combination
-
 
 6. Capabilities that can be enabled on vCPUs
 --------------------------------------------
-- 
2.1.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] KVM: fix api documentation of KVM_GET_EMULATED_CPUID

[Borislav Petkov]

On Tue, Sep 09, 2014 at 05:27:19PM +0100, Alex Bennée wrote:
> It looks like when this was initially merged it got accidentally included
> in the following section.

Whoops.

> I've just moved it back in the correct section
> and re-numbered it as other ioctls have been added since.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

Acked-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.
--

Re: [PATCH 0/2] KVM API documentation patches

[Paolo Bonzini]

Il 09/09/2014 18:27, Alex Bennée ha scritto:
> Hi,
> 
> I'm preparing to add ARM KVM GDB support and I went to read the API
> documentation and found it surprisingly mute on the subject ;-)
> 
> The first patch documents the "new" KVM_SET_GUEST_DEBUG ioctl based on
> reviewing the code. I've included a long CC list of people who've
> actually done the various implementations who I hope can sanity check
> the write-up. The second is a trivial formatting fix for what looks
> like a minor merge trip-up.
> 
> Alex Bennée (2):
>   KVM: document KVM_SET_GUEST_DEBUG api
>   KVM: fix api documentation of KVM_GET_EMULATED_CPUID
> 
>  Documentation/virtual/kvm/api.txt | 184 +++++++++++++++++++++++---------------
>  1 file changed, 114 insertions(+), 70 deletions(-)
> 

Thanks, applying both!

Paolo

Re: [PATCH 0/2] KVM API documentation patches

[Christoffer Dall]

On Wed, Sep 10, 2014 at 11:34:53AM +0200, Paolo Bonzini wrote:
> Il 09/09/2014 18:27, Alex Bennée ha scritto:
> > Hi,
> > 
> > I'm preparing to add ARM KVM GDB support and I went to read the API
> > documentation and found it surprisingly mute on the subject ;-)
> > 
> > The first patch documents the "new" KVM_SET_GUEST_DEBUG ioctl based on
> > reviewing the code. I've included a long CC list of people who've
> > actually done the various implementations who I hope can sanity check
> > the write-up. The second is a trivial formatting fix for what looks
> > like a minor merge trip-up.
> > 
> > Alex Bennée (2):
> >   KVM: document KVM_SET_GUEST_DEBUG api
> >   KVM: fix api documentation of KVM_GET_EMULATED_CPUID
> > 
> >  Documentation/virtual/kvm/api.txt | 184 +++++++++++++++++++++++---------------
> >  1 file changed, 114 insertions(+), 70 deletions(-)
> > 
> 
> Thanks, applying both!
> 
ha, read this after reviewing his text.  Was I too nit-picky? ;)

-Christoffer

Re: [PATCH 0/2] KVM API documentation patches

[Alex Bennée]

Christoffer Dall writes:

> On Wed, Sep 10, 2014 at 11:34:53AM +0200, Paolo Bonzini wrote:
>> Il 09/09/2014 18:27, Alex Bennée ha scritto:
>> > Hi,
>> > 
>> > I'm preparing to add ARM KVM GDB support and I went to read the API
>> > documentation and found it surprisingly mute on the subject ;-)
>> > 
>> > The first patch documents the "new" KVM_SET_GUEST_DEBUG ioctl based on
>> > reviewing the code.
>> 
> ha, read this after reviewing his text.  Was I too nit-picky? ;)

Well I can send in a clean-up patch for the docs when I submit some
working code ;-)

-- 
Alex Bennée