diff mbox

[v4,4/4] KVM: Add documentation for KVM_ARM_PREFERRED_TARGET ioctl

Message ID 1380101163-20028-5-git-send-email-anup.patel@linaro.org
State New
Headers show

Commit Message

Anup Patel Sept. 25, 2013, 9:26 a.m. UTC 
To implement CPU=Host we have added KVM_ARM_PREFERRED_TARGET
vm ioctl which provides information to user space required for
creating VCPU matching underlying Host.

This patch adds info related to this new KVM_ARM_PREFERRED_TARGET
vm ioctl in the KVM API documentation.

Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
---
 Documentation/virtual/kvm/api.txt |   27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

Comments

Alexander Graf Sept. 25, 2013, 12:13 p.m. UTC | #1 
On 25.09.2013, at 11:26, Anup Patel wrote:

> To implement CPU=Host we have added KVM_ARM_PREFERRED_TARGET
> vm ioctl which provides information to user space required for
> creating VCPU matching underlying Host.
> 
> This patch adds info related to this new KVM_ARM_PREFERRED_TARGET
> vm ioctl in the KVM API documentation.
> 
> Signed-off-by: Anup Patel <anup.patel@linaro.org>
> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> ---
> Documentation/virtual/kvm/api.txt |   27 +++++++++++++++++++++++----
> 1 file changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index 858aecf..f31e6e8 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -2303,8 +2303,28 @@ Possible features:
> 	- KVM_ARM_VCPU_EL1_32BIT: Starts the CPU in a 32bit mode.
> 	  Depends on KVM_CAP_ARM_EL1_32BIT (arm64 only).
> 
> +4.83 KVM_ARM_PREFERRED_TARGET

Why 4.83 and not 4.86? It feels backwards to rename all these other sections.


Alex
Anup Patel Sept. 25, 2013, 12:28 p.m. UTC | #2 
On Wed, Sep 25, 2013 at 5:43 PM, Alexander Graf <agraf@suse.de> wrote:
>
> On 25.09.2013, at 11:26, Anup Patel wrote:
>
>> To implement CPU=Host we have added KVM_ARM_PREFERRED_TARGET
>> vm ioctl which provides information to user space required for
>> creating VCPU matching underlying Host.
>>
>> This patch adds info related to this new KVM_ARM_PREFERRED_TARGET
>> vm ioctl in the KVM API documentation.
>>
>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>> ---
>> Documentation/virtual/kvm/api.txt |   27 +++++++++++++++++++++++----
>> 1 file changed, 23 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>> index 858aecf..f31e6e8 100644
>> --- a/Documentation/virtual/kvm/api.txt
>> +++ b/Documentation/virtual/kvm/api.txt
>> @@ -2303,8 +2303,28 @@ Possible features:
>>       - KVM_ARM_VCPU_EL1_32BIT: Starts the CPU in a 32bit mode.
>>         Depends on KVM_CAP_ARM_EL1_32BIT (arm64 only).
>>
>> +4.83 KVM_ARM_PREFERRED_TARGET
>
> Why 4.83 and not 4.86? It feels backwards to rename all these other sections.

I wanted to have KVM_ARM_xxxx IOCTLs located nearby hence
placed KVM_ARM_PREFERRED_TARGET after KVM_ARM_VCPU_INIT.

There is no point in keeping KVM_ARM_PREFERRED_TARGET
after KVM_PPC_RTAS_DEFINE_TOKEN and make
KVM_PPC_RTAS_DEFINE_TOKEN dangle in-between documentation
of various KVM_ARM_xxxx IOCTLs.

>
>
> Alex
>
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm

--Anup
Christoffer Dall Sept. 25, 2013, 4:19 p.m. UTC | #3 
On Wed, Sep 25, 2013 at 05:58:07PM +0530, Anup Patel wrote:
> On Wed, Sep 25, 2013 at 5:43 PM, Alexander Graf <agraf@suse.de> wrote:
> >
> > On 25.09.2013, at 11:26, Anup Patel wrote:
> >
> >> To implement CPU=Host we have added KVM_ARM_PREFERRED_TARGET
> >> vm ioctl which provides information to user space required for
> >> creating VCPU matching underlying Host.
> >>
> >> This patch adds info related to this new KVM_ARM_PREFERRED_TARGET
> >> vm ioctl in the KVM API documentation.
> >>
> >> Signed-off-by: Anup Patel <anup.patel@linaro.org>
> >> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> >> ---
> >> Documentation/virtual/kvm/api.txt |   27 +++++++++++++++++++++++----
> >> 1 file changed, 23 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> >> index 858aecf..f31e6e8 100644
> >> --- a/Documentation/virtual/kvm/api.txt
> >> +++ b/Documentation/virtual/kvm/api.txt
> >> @@ -2303,8 +2303,28 @@ Possible features:
> >>       - KVM_ARM_VCPU_EL1_32BIT: Starts the CPU in a 32bit mode.
> >>         Depends on KVM_CAP_ARM_EL1_32BIT (arm64 only).
> >>
> >> +4.83 KVM_ARM_PREFERRED_TARGET
> >
> > Why 4.83 and not 4.86? It feels backwards to rename all these other sections.
> 
> I wanted to have KVM_ARM_xxxx IOCTLs located nearby hence
> placed KVM_ARM_PREFERRED_TARGET after KVM_ARM_VCPU_INIT.
> 
> There is no point in keeping KVM_ARM_PREFERRED_TARGET
> after KVM_PPC_RTAS_DEFINE_TOKEN and make
> KVM_PPC_RTAS_DEFINE_TOKEN dangle in-between documentation
> of various KVM_ARM_xxxx IOCTLs.
> 
I checked the git log and this is not the first time this has happened,
so I think Anup's point here is valid.

-Christoffer
Christoffer Dall Sept. 25, 2013, 8:10 p.m. UTC | #4 
On Wed, Sep 25, 2013 at 02:56:03PM +0530, Anup Patel wrote:
> To implement CPU=Host we have added KVM_ARM_PREFERRED_TARGET
> vm ioctl which provides information to user space required for
> creating VCPU matching underlying Host.
> 
> This patch adds info related to this new KVM_ARM_PREFERRED_TARGET
> vm ioctl in the KVM API documentation.
> 
> Signed-off-by: Anup Patel <anup.patel@linaro.org>
> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> ---
>  Documentation/virtual/kvm/api.txt |   27 +++++++++++++++++++++++----
>  1 file changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index 858aecf..f31e6e8 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -2303,8 +2303,28 @@ Possible features:
>  	- KVM_ARM_VCPU_EL1_32BIT: Starts the CPU in a 32bit mode.
>  	  Depends on KVM_CAP_ARM_EL1_32BIT (arm64 only).
>  
> +4.83 KVM_ARM_PREFERRED_TARGET
>  
> -4.83 KVM_GET_REG_LIST

I think the convention here is to keep two new lines after a Section.

> +Capability: basic
> +Architectures: arm, arm64
> +Type: vm ioctl
> +Parameters: struct struct kvm_vcpu_init (out)
> +Returns: 0 on success; -1 on error
> +Errors:
> +  ENODEV:    no preferred target available for the host
> +
> +This queries KVM for preferred CPU target type which can be emulated
> +by KVM on underlying host.
> +
> +The ioctl returns struct kvm_vcpu_init instance containing information
> +about preferred CPU target type and target specific features available
> +for it.
> +
> +The information returned by this ioctl can be used to prepare instance

an instance

> +of struct kvm_vcpu_init for KVM_ARM_VCPU_INIT ioctl which will result

the KVM_ARM_VCPU_INIT ioctl.
(the last part is not necessarily true is it?  It's only a best bet,
think for example about Big.Little...)

> +in VCPU matching underlying host.

You also need to add something about the features here:

The kvm_vcpu_init->features bitmap will have feature bits set if the
preferred target recommends setting these features, but this is not
required.

> +
> +4.84 KVM_GET_REG_LIST
>  
>  Capability: basic
>  Architectures: arm, arm64
> @@ -2323,8 +2343,7 @@ struct kvm_reg_list {
>  This ioctl returns the guest registers that are supported for the
>  KVM_GET_ONE_REG/KVM_SET_ONE_REG calls.
>  
> -
> -4.84 KVM_ARM_SET_DEVICE_ADDR
> +4.85 KVM_ARM_SET_DEVICE_ADDR
>  
>  Capability: KVM_CAP_ARM_SET_DEVICE_ADDR
>  Architectures: arm, arm64
> @@ -2362,7 +2381,7 @@ must be called after calling KVM_CREATE_IRQCHIP, but before calling
>  KVM_RUN on any of the VCPUs.  Calling this ioctl twice for any of the
>  base addresses will return -EEXIST.
>  
> -4.85 KVM_PPC_RTAS_DEFINE_TOKEN
> +4.86 KVM_PPC_RTAS_DEFINE_TOKEN
>  
>  Capability: KVM_CAP_PPC_RTAS
>  Architectures: ppc
> -- 
> 1.7.9.5
>
Alexander Graf Sept. 25, 2013, 10:26 p.m. UTC | #5 
On 25.09.2013, at 18:19, Christoffer Dall wrote:

> On Wed, Sep 25, 2013 at 05:58:07PM +0530, Anup Patel wrote:
>> On Wed, Sep 25, 2013 at 5:43 PM, Alexander Graf <agraf@suse.de> wrote:
>>> 
>>> On 25.09.2013, at 11:26, Anup Patel wrote:
>>> 
>>>> To implement CPU=Host we have added KVM_ARM_PREFERRED_TARGET
>>>> vm ioctl which provides information to user space required for
>>>> creating VCPU matching underlying Host.
>>>> 
>>>> This patch adds info related to this new KVM_ARM_PREFERRED_TARGET
>>>> vm ioctl in the KVM API documentation.
>>>> 
>>>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>>>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>>>> ---
>>>> Documentation/virtual/kvm/api.txt |   27 +++++++++++++++++++++++----
>>>> 1 file changed, 23 insertions(+), 4 deletions(-)
>>>> 
>>>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>>>> index 858aecf..f31e6e8 100644
>>>> --- a/Documentation/virtual/kvm/api.txt
>>>> +++ b/Documentation/virtual/kvm/api.txt
>>>> @@ -2303,8 +2303,28 @@ Possible features:
>>>>      - KVM_ARM_VCPU_EL1_32BIT: Starts the CPU in a 32bit mode.
>>>>        Depends on KVM_CAP_ARM_EL1_32BIT (arm64 only).
>>>> 
>>>> +4.83 KVM_ARM_PREFERRED_TARGET
>>> 
>>> Why 4.83 and not 4.86? It feels backwards to rename all these other sections.
>> 
>> I wanted to have KVM_ARM_xxxx IOCTLs located nearby hence
>> placed KVM_ARM_PREFERRED_TARGET after KVM_ARM_VCPU_INIT.
>> 
>> There is no point in keeping KVM_ARM_PREFERRED_TARGET
>> after KVM_PPC_RTAS_DEFINE_TOKEN and make
>> KVM_PPC_RTAS_DEFINE_TOKEN dangle in-between documentation
>> of various KVM_ARM_xxxx IOCTLs.
>> 
> I checked the git log and this is not the first time this has happened,
> so I think Anup's point here is valid.

Well, I think it makes sense to be consistent here. Maybe we should add a new section for arch specific ioctls so that we get our own number space? But regardless, all subarchs should try and follow the same scheme - regardless of what the scheme is :).


Alex
Anup Patel Sept. 30, 2013, 4:13 a.m. UTC | #6 
On Thu, Sep 26, 2013 at 1:40 AM, Christoffer Dall
<christoffer.dall@linaro.org> wrote:
> On Wed, Sep 25, 2013 at 02:56:03PM +0530, Anup Patel wrote:
>> To implement CPU=Host we have added KVM_ARM_PREFERRED_TARGET
>> vm ioctl which provides information to user space required for
>> creating VCPU matching underlying Host.
>>
>> This patch adds info related to this new KVM_ARM_PREFERRED_TARGET
>> vm ioctl in the KVM API documentation.
>>
>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>> ---
>>  Documentation/virtual/kvm/api.txt |   27 +++++++++++++++++++++++----
>>  1 file changed, 23 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>> index 858aecf..f31e6e8 100644
>> --- a/Documentation/virtual/kvm/api.txt
>> +++ b/Documentation/virtual/kvm/api.txt
>> @@ -2303,8 +2303,28 @@ Possible features:
>>       - KVM_ARM_VCPU_EL1_32BIT: Starts the CPU in a 32bit mode.
>>         Depends on KVM_CAP_ARM_EL1_32BIT (arm64 only).
>>
>> +4.83 KVM_ARM_PREFERRED_TARGET
>>
>> -4.83 KVM_GET_REG_LIST
>
> I think the convention here is to keep two new lines after a Section.

Well this is not be consistenly followed by everyone.
Please look at the api.txt again.

Anyways, I will change and keep two new lines after a Section.

>
>> +Capability: basic
>> +Architectures: arm, arm64
>> +Type: vm ioctl
>> +Parameters: struct struct kvm_vcpu_init (out)
>> +Returns: 0 on success; -1 on error
>> +Errors:
>> +  ENODEV:    no preferred target available for the host
>> +
>> +This queries KVM for preferred CPU target type which can be emulated
>> +by KVM on underlying host.
>> +
>> +The ioctl returns struct kvm_vcpu_init instance containing information
>> +about preferred CPU target type and target specific features available
>> +for it.
>> +
>> +The information returned by this ioctl can be used to prepare instance
>
> an instance

Ok.

>
>> +of struct kvm_vcpu_init for KVM_ARM_VCPU_INIT ioctl which will result
>
> the KVM_ARM_VCPU_INIT ioctl.
> (the last part is not necessarily true is it?  It's only a best bet,
> think for example about Big.Little...)
>
>> +in VCPU matching underlying host.
>
> You also need to add something about the features here:
>
> The kvm_vcpu_init->features bitmap will have feature bits set if the
> preferred target recommends setting these features, but this is not
> required.

Ok.

>
>> +
>> +4.84 KVM_GET_REG_LIST
>>
>>  Capability: basic
>>  Architectures: arm, arm64
>> @@ -2323,8 +2343,7 @@ struct kvm_reg_list {
>>  This ioctl returns the guest registers that are supported for the
>>  KVM_GET_ONE_REG/KVM_SET_ONE_REG calls.
>>
>> -
>> -4.84 KVM_ARM_SET_DEVICE_ADDR
>> +4.85 KVM_ARM_SET_DEVICE_ADDR
>>
>>  Capability: KVM_CAP_ARM_SET_DEVICE_ADDR
>>  Architectures: arm, arm64
>> @@ -2362,7 +2381,7 @@ must be called after calling KVM_CREATE_IRQCHIP, but before calling
>>  KVM_RUN on any of the VCPUs.  Calling this ioctl twice for any of the
>>  base addresses will return -EEXIST.
>>
>> -4.85 KVM_PPC_RTAS_DEFINE_TOKEN
>> +4.86 KVM_PPC_RTAS_DEFINE_TOKEN
>>
>>  Capability: KVM_CAP_PPC_RTAS
>>  Architectures: ppc
>> --
>> 1.7.9.5
>>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm

--Anup
Alexander Graf Sept. 30, 2013, 9:06 a.m. UTC | #7 
On 26.09.2013, at 00:26, Alexander Graf wrote:

> 
> On 25.09.2013, at 18:19, Christoffer Dall wrote:
> 
>> On Wed, Sep 25, 2013 at 05:58:07PM +0530, Anup Patel wrote:
>>> On Wed, Sep 25, 2013 at 5:43 PM, Alexander Graf <agraf@suse.de> wrote:
>>>> 
>>>> On 25.09.2013, at 11:26, Anup Patel wrote:
>>>> 
>>>>> To implement CPU=Host we have added KVM_ARM_PREFERRED_TARGET
>>>>> vm ioctl which provides information to user space required for
>>>>> creating VCPU matching underlying Host.
>>>>> 
>>>>> This patch adds info related to this new KVM_ARM_PREFERRED_TARGET
>>>>> vm ioctl in the KVM API documentation.
>>>>> 
>>>>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>>>>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>>>>> ---
>>>>> Documentation/virtual/kvm/api.txt |   27 +++++++++++++++++++++++----
>>>>> 1 file changed, 23 insertions(+), 4 deletions(-)
>>>>> 
>>>>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>>>>> index 858aecf..f31e6e8 100644
>>>>> --- a/Documentation/virtual/kvm/api.txt
>>>>> +++ b/Documentation/virtual/kvm/api.txt
>>>>> @@ -2303,8 +2303,28 @@ Possible features:
>>>>>     - KVM_ARM_VCPU_EL1_32BIT: Starts the CPU in a 32bit mode.
>>>>>       Depends on KVM_CAP_ARM_EL1_32BIT (arm64 only).
>>>>> 
>>>>> +4.83 KVM_ARM_PREFERRED_TARGET
>>>> 
>>>> Why 4.83 and not 4.86? It feels backwards to rename all these other sections.
>>> 
>>> I wanted to have KVM_ARM_xxxx IOCTLs located nearby hence
>>> placed KVM_ARM_PREFERRED_TARGET after KVM_ARM_VCPU_INIT.
>>> 
>>> There is no point in keeping KVM_ARM_PREFERRED_TARGET
>>> after KVM_PPC_RTAS_DEFINE_TOKEN and make
>>> KVM_PPC_RTAS_DEFINE_TOKEN dangle in-between documentation
>>> of various KVM_ARM_xxxx IOCTLs.
>>> 
>> I checked the git log and this is not the first time this has happened,
>> so I think Anup's point here is valid.
> 
> Well, I think it makes sense to be consistent here. Maybe we should add a new section for arch specific ioctls so that we get our own number space? But regardless, all subarchs should try and follow the same scheme - regardless of what the scheme is :).

Ok, I think I finally grasped what Anup was trying to say. The ioctl id for this one is 0xaf, in between 0xae (KVM_ARM_VCPU_INIT) and 0xb0 (KVM_GET_REG_LIST), so he wanted to make the documentation follow the ioctl numbering scheme. That makes sense.

However, why do we have this gap there in the first place?


Alex
diff mbox

Patch

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 858aecf..f31e6e8 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -2303,8 +2303,28 @@  Possible features:
 	- KVM_ARM_VCPU_EL1_32BIT: Starts the CPU in a 32bit mode.
 	  Depends on KVM_CAP_ARM_EL1_32BIT (arm64 only).
 
+4.83 KVM_ARM_PREFERRED_TARGET
 
-4.83 KVM_GET_REG_LIST
+Capability: basic
+Architectures: arm, arm64
+Type: vm ioctl
+Parameters: struct struct kvm_vcpu_init (out)
+Returns: 0 on success; -1 on error
+Errors:
+  ENODEV:    no preferred target available for the host
+
+This queries KVM for preferred CPU target type which can be emulated
+by KVM on underlying host.
+
+The ioctl returns struct kvm_vcpu_init instance containing information
+about preferred CPU target type and target specific features available
+for it.
+
+The information returned by this ioctl can be used to prepare instance
+of struct kvm_vcpu_init for KVM_ARM_VCPU_INIT ioctl which will result
+in VCPU matching underlying host.
+
+4.84 KVM_GET_REG_LIST
 
 Capability: basic
 Architectures: arm, arm64
@@ -2323,8 +2343,7 @@  struct kvm_reg_list {
 This ioctl returns the guest registers that are supported for the
 KVM_GET_ONE_REG/KVM_SET_ONE_REG calls.
 
-
-4.84 KVM_ARM_SET_DEVICE_ADDR
+4.85 KVM_ARM_SET_DEVICE_ADDR
 
 Capability: KVM_CAP_ARM_SET_DEVICE_ADDR
 Architectures: arm, arm64
@@ -2362,7 +2381,7 @@  must be called after calling KVM_CREATE_IRQCHIP, but before calling
 KVM_RUN on any of the VCPUs.  Calling this ioctl twice for any of the
 base addresses will return -EEXIST.
 
-4.85 KVM_PPC_RTAS_DEFINE_TOKEN
+4.86 KVM_PPC_RTAS_DEFINE_TOKEN
 
 Capability: KVM_CAP_PPC_RTAS
 Architectures: ppc