[PATCH v3 0/4] Target CPU=Host implementation for KVM ARM/ARM64

[PATCH v3 0/4] Target CPU=Host implementation for KVM ARM/ARM64

[Anup Patel]

It will be very useful for user space (QEMU/KVMTOOL) if it has a
method of querying VCPU target type matching to underlying Host. 
We can use such querying mechanism and implement machine models
in user space where VCPU target type is always same-as/similar-to
underlying Host (i.e. Target CPU=Host).

This patch series implements KVM_ARM_PREFERRED_TARGET vm ioclt for
querying VCPU target type matching underlying host. Using this new
ioctl we can implement VCPU target CPU=Host in user space.

Also, it is not mandatory to call KVM_ARM_PREFERRED_TARGET vm ioctl
and the old method of trying all possible target types using the
KVM_ARM_VCPU_INIT ioctl to initialize VCPU works fine.

V3:
 - Return -ENODEV if no preferred target available for host
 - Make KVM_ARM_PREFERRED_TARGET ioctl as vm ioctl

V2:
 - Renamed the ioclt to KVM_ARM_PREFERRED_TARGET
 - Return struct kvm_vcpu_init instace instead of just target type

V1:
 - Initial patch-set with ioctl named as KVM_ARM_SUITABLE_TARGET

Anup Patel (4):
  ARM: KVM: Implement kvm_vcpu_preferred_target() function
  ARM64: KVM: Implement kvm_vcpu_preferred_target() function
  ARM/ARM64: KVM: Implement KVM_ARM_PREFERRED_TARGET ioctl
  KVM: Add documentation for KVM_ARM_PREFERRED_TARGET ioctl

 Documentation/virtual/kvm/api.txt |   26 ++++++++++++++++++++++----
 arch/arm/include/asm/kvm_host.h   |    1 +
 arch/arm/kvm/arm.c                |   13 +++++++++++++
 arch/arm/kvm/guest.c              |   20 ++++++++++++++++++++
 arch/arm64/include/asm/kvm_host.h |    1 +
 arch/arm64/kvm/guest.c            |   21 +++++++++++++++++++++
 include/uapi/linux/kvm.h          |    1 +
 7 files changed, 79 insertions(+), 4 deletions(-)

-- 
1.7.9.5

[PATCH v3 1/4] ARM: KVM: Implement kvm_vcpu_preferred_target() function

[Anup Patel]

This patch implements kvm_vcpu_preferred_target() function for
KVM ARM which will help us implement KVM_ARM_PREFERRED_TARGET ioctl
for user space.

Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
---
 arch/arm/kvm/guest.c              |   20 ++++++++++++++++++++
 arch/arm64/include/asm/kvm_host.h |    1 +
 2 files changed, 21 insertions(+)

diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c
index 152d036..b407e6c 100644
--- a/arch/arm/kvm/guest.c
+++ b/arch/arm/kvm/guest.c
@@ -222,6 +222,26 @@ int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
 	return kvm_reset_vcpu(vcpu);
 }
 
+int kvm_vcpu_preferred_target(struct kvm_vcpu_init *init)
+{
+	int target = kvm_target_cpu();
+
+	if (target < 0)
+		return -ENODEV;
+
+	memset(init, 0, sizeof(*init));
+
+	/* 
+	 * For now, we return all optional features are available
+	 * for preferred target. In future, we might have features
+	 * available based on underlying host.
+	 */
+	init->target = (__u32)target;
+	init->features[0] |= (1 << KVM_ARM_VCPU_POWER_OFF);
+
+	return 0;
+}
+
 int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)
 {
 	return -EINVAL;
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index f318c43..b609db3 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -156,6 +156,7 @@ struct kvm_vcpu_stat {
 struct kvm_vcpu_init;
 int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
 			const struct kvm_vcpu_init *init);
+int kvm_vcpu_preferred_target(struct kvm_vcpu_init *init);
 unsigned long kvm_arm_num_regs(struct kvm_vcpu *vcpu);
 int kvm_arm_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *indices);
 struct kvm_one_reg;
-- 
1.7.9.5

[PATCH v3 1/4] ARM: KVM: Implement kvm_vcpu_preferred_target() function

[Marc Zyngier]

On 19/09/13 14:11, Anup Patel wrote:
> This patch implements kvm_vcpu_preferred_target() function for
> KVM ARM which will help us implement KVM_ARM_PREFERRED_TARGET ioctl
> for user space.
> 
> Signed-off-by: Anup Patel <anup.patel@linaro.org>
> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> ---
>  arch/arm/kvm/guest.c              |   20 ++++++++++++++++++++
>  arch/arm64/include/asm/kvm_host.h |    1 +
>  2 files changed, 21 insertions(+)
> 
> diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c
> index 152d036..b407e6c 100644
> --- a/arch/arm/kvm/guest.c
> +++ b/arch/arm/kvm/guest.c
> @@ -222,6 +222,26 @@ int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
>  	return kvm_reset_vcpu(vcpu);
>  }
>  
> +int kvm_vcpu_preferred_target(struct kvm_vcpu_init *init)
> +{
> +	int target = kvm_target_cpu();
> +
> +	if (target < 0)
> +		return -ENODEV;
> +
> +	memset(init, 0, sizeof(*init));
> +
> +	/* 
> +	 * For now, we return all optional features are available
> +	 * for preferred target. In future, we might have features
> +	 * available based on underlying host.
> +	 */
> +	init->target = (__u32)target;
> +	init->features[0] |= (1 << KVM_ARM_VCPU_POWER_OFF);

I'm in two minds about this feature reporting. I see they serve a
purpose, but they also duplicate capabilities, which is the standard way
to advertise what KVM can do.

It means we end up having to sync two reporting mechanism, and I feel
this is in general a bad idea.

Furthermore, KVM_ARM_VCPU_POWER_OFF is hardly a feature of the HW, but
rather a firmware emulation thing.

Peter, Christoffer: Thoughts?

	M.

> +	return 0;
> +}
> +
>  int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)
>  {
>  	return -EINVAL;
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index f318c43..b609db3 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -156,6 +156,7 @@ struct kvm_vcpu_stat {
>  struct kvm_vcpu_init;
>  int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
>  			const struct kvm_vcpu_init *init);
> +int kvm_vcpu_preferred_target(struct kvm_vcpu_init *init);
>  unsigned long kvm_arm_num_regs(struct kvm_vcpu *vcpu);
>  int kvm_arm_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *indices);
>  struct kvm_one_reg;
> 


-- 
Jazz is not dead. It just smells funny...

[PATCH v3 1/4] ARM: KVM: Implement kvm_vcpu_preferred_target() function

[Christoffer Dall]

On Thu, Sep 19, 2013 at 03:27:54PM +0100, Marc Zyngier wrote:
> On 19/09/13 14:11, Anup Patel wrote:
> > This patch implements kvm_vcpu_preferred_target() function for
> > KVM ARM which will help us implement KVM_ARM_PREFERRED_TARGET ioctl
> > for user space.
> > 
> > Signed-off-by: Anup Patel <anup.patel@linaro.org>
> > Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> > ---
> >  arch/arm/kvm/guest.c              |   20 ++++++++++++++++++++
> >  arch/arm64/include/asm/kvm_host.h |    1 +
> >  2 files changed, 21 insertions(+)
> > 
> > diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c
> > index 152d036..b407e6c 100644
> > --- a/arch/arm/kvm/guest.c
> > +++ b/arch/arm/kvm/guest.c
> > @@ -222,6 +222,26 @@ int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
> >  	return kvm_reset_vcpu(vcpu);
> >  }
> >  
> > +int kvm_vcpu_preferred_target(struct kvm_vcpu_init *init)
> > +{
> > +	int target = kvm_target_cpu();
> > +
> > +	if (target < 0)
> > +		return -ENODEV;
> > +
> > +	memset(init, 0, sizeof(*init));
> > +
> > +	/* 
> > +	 * For now, we return all optional features are available
> > +	 * for preferred target. In future, we might have features
> > +	 * available based on underlying host.
> > +	 */
> > +	init->target = (__u32)target;
> > +	init->features[0] |= (1 << KVM_ARM_VCPU_POWER_OFF);
> 
> I'm in two minds about this feature reporting. I see they serve a
> purpose, but they also duplicate capabilities, which is the standard way
> to advertise what KVM can do.
> 
> It means we end up having to sync two reporting mechanism, and I feel
> this is in general a bad idea.
> 
> Furthermore, KVM_ARM_VCPU_POWER_OFF is hardly a feature of the HW, but
> rather a firmware emulation thing.
> 
> Peter, Christoffer: Thoughts?
> 
I wanted to return the full kvm_vcpu_init instead of just a target int,
so we did not have to come up with yet another ioctl if we need to
return more information about the capabilities of the host CPU in the
future.

But perhaps we can formulate the API, to say only the (currently empty)
following list of features can only be enabled if the corresponding bit
is enabled, or something along those lines.

-Christoffer

[PATCH v3 1/4] ARM: KVM: Implement kvm_vcpu_preferred_target() function

[Anup Patel]

Hi Christoffer/Marc,

On Fri, Sep 20, 2013 at 12:50 AM, Christoffer Dall
<christoffer.dall@linaro.org> wrote:
> On Thu, Sep 19, 2013 at 03:27:54PM +0100, Marc Zyngier wrote:
>> On 19/09/13 14:11, Anup Patel wrote:
>> > This patch implements kvm_vcpu_preferred_target() function for
>> > KVM ARM which will help us implement KVM_ARM_PREFERRED_TARGET ioctl
>> > for user space.
>> >
>> > Signed-off-by: Anup Patel <anup.patel@linaro.org>
>> > Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>> > ---
>> >  arch/arm/kvm/guest.c              |   20 ++++++++++++++++++++
>> >  arch/arm64/include/asm/kvm_host.h |    1 +
>> >  2 files changed, 21 insertions(+)
>> >
>> > diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c
>> > index 152d036..b407e6c 100644
>> > --- a/arch/arm/kvm/guest.c
>> > +++ b/arch/arm/kvm/guest.c
>> > @@ -222,6 +222,26 @@ int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
>> >     return kvm_reset_vcpu(vcpu);
>> >  }
>> >
>> > +int kvm_vcpu_preferred_target(struct kvm_vcpu_init *init)
>> > +{
>> > +   int target = kvm_target_cpu();
>> > +
>> > +   if (target < 0)
>> > +           return -ENODEV;
>> > +
>> > +   memset(init, 0, sizeof(*init));
>> > +
>> > +   /*
>> > +    * For now, we return all optional features are available
>> > +    * for preferred target. In future, we might have features
>> > +    * available based on underlying host.
>> > +    */
>> > +   init->target = (__u32)target;
>> > +   init->features[0] |= (1 << KVM_ARM_VCPU_POWER_OFF);
>>
>> I'm in two minds about this feature reporting. I see they serve a
>> purpose, but they also duplicate capabilities, which is the standard way
>> to advertise what KVM can do.
>>
>> It means we end up having to sync two reporting mechanism, and I feel
>> this is in general a bad idea.
>>
>> Furthermore, KVM_ARM_VCPU_POWER_OFF is hardly a feature of the HW, but
>> rather a firmware emulation thing.
>>
>> Peter, Christoffer: Thoughts?
>>
> I wanted to return the full kvm_vcpu_init instead of just a target int,
> so we did not have to come up with yet another ioctl if we need to
> return more information about the capabilities of the host CPU in the
> future.
>
> But perhaps we can formulate the API, to say only the (currently empty)
> following list of features can only be enabled if the corresponding bit
> is enabled, or something along those lines.
>
> -Christoffer
> _______________________________________________
> kvmarm mailing list
> kvmarm at lists.cs.columbia.edu
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm

Do we stick with current implementation of returning struct kvm_vcpu_init ?
OR
Do we return struct kvm_vcpu_init with all features set to zero ?

Regards,
Anup

[PATCH v3 1/4] ARM: KVM: Implement kvm_vcpu_preferred_target() function

[Christoffer Dall]

On Mon, Sep 23, 2013 at 01:35:20PM +0530, Anup Patel wrote:
> Hi Christoffer/Marc,
> 
> On Fri, Sep 20, 2013 at 12:50 AM, Christoffer Dall
> <christoffer.dall@linaro.org> wrote:
> > On Thu, Sep 19, 2013 at 03:27:54PM +0100, Marc Zyngier wrote:
> >> On 19/09/13 14:11, Anup Patel wrote:
> >> > This patch implements kvm_vcpu_preferred_target() function for
> >> > KVM ARM which will help us implement KVM_ARM_PREFERRED_TARGET ioctl
> >> > for user space.
> >> >
> >> > Signed-off-by: Anup Patel <anup.patel@linaro.org>
> >> > Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> >> > ---
> >> >  arch/arm/kvm/guest.c              |   20 ++++++++++++++++++++
> >> >  arch/arm64/include/asm/kvm_host.h |    1 +
> >> >  2 files changed, 21 insertions(+)
> >> >
> >> > diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c
> >> > index 152d036..b407e6c 100644
> >> > --- a/arch/arm/kvm/guest.c
> >> > +++ b/arch/arm/kvm/guest.c
> >> > @@ -222,6 +222,26 @@ int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
> >> >     return kvm_reset_vcpu(vcpu);
> >> >  }
> >> >
> >> > +int kvm_vcpu_preferred_target(struct kvm_vcpu_init *init)
> >> > +{
> >> > +   int target = kvm_target_cpu();
> >> > +
> >> > +   if (target < 0)
> >> > +           return -ENODEV;
> >> > +
> >> > +   memset(init, 0, sizeof(*init));
> >> > +
> >> > +   /*
> >> > +    * For now, we return all optional features are available
> >> > +    * for preferred target. In future, we might have features
> >> > +    * available based on underlying host.
> >> > +    */
> >> > +   init->target = (__u32)target;
> >> > +   init->features[0] |= (1 << KVM_ARM_VCPU_POWER_OFF);
> >>
> >> I'm in two minds about this feature reporting. I see they serve a
> >> purpose, but they also duplicate capabilities, which is the standard way
> >> to advertise what KVM can do.
> >>
> >> It means we end up having to sync two reporting mechanism, and I feel
> >> this is in general a bad idea.
> >>
> >> Furthermore, KVM_ARM_VCPU_POWER_OFF is hardly a feature of the HW, but
> >> rather a firmware emulation thing.
> >>
> >> Peter, Christoffer: Thoughts?
> >>
> > I wanted to return the full kvm_vcpu_init instead of just a target int,
> > so we did not have to come up with yet another ioctl if we need to
> > return more information about the capabilities of the host CPU in the
> > future.
> >
> > But perhaps we can formulate the API, to say only the (currently empty)
> > following list of features can only be enabled if the corresponding bit
> > is enabled, or something along those lines.
> >
> > -Christoffer
> > _______________________________________________
> > kvmarm mailing list
> > kvmarm at lists.cs.columbia.edu
> > https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
> 
> Do we stick with current implementation of returning struct kvm_vcpu_init ?
> OR
> Do we return struct kvm_vcpu_init with all features set to zero ?
> 
Let's give Marc a day or two to respond to this one ;)

-Christoffer

[PATCH v3 1/4] ARM: KVM: Implement kvm_vcpu_preferred_target() function

[Marc Zyngier]

On 23/09/13 16:31, Christoffer Dall wrote:
> On Mon, Sep 23, 2013 at 01:35:20PM +0530, Anup Patel wrote:
>> Hi Christoffer/Marc,
>>
>> On Fri, Sep 20, 2013 at 12:50 AM, Christoffer Dall
>> <christoffer.dall@linaro.org> wrote:
>>> On Thu, Sep 19, 2013 at 03:27:54PM +0100, Marc Zyngier wrote:
>>>> On 19/09/13 14:11, Anup Patel wrote:
>>>>> This patch implements kvm_vcpu_preferred_target() function for
>>>>> KVM ARM which will help us implement KVM_ARM_PREFERRED_TARGET ioctl
>>>>> for user space.
>>>>>
>>>>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>>>>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>>>>> ---
>>>>>  arch/arm/kvm/guest.c              |   20 ++++++++++++++++++++
>>>>>  arch/arm64/include/asm/kvm_host.h |    1 +
>>>>>  2 files changed, 21 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c
>>>>> index 152d036..b407e6c 100644
>>>>> --- a/arch/arm/kvm/guest.c
>>>>> +++ b/arch/arm/kvm/guest.c
>>>>> @@ -222,6 +222,26 @@ int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
>>>>>     return kvm_reset_vcpu(vcpu);
>>>>>  }
>>>>>
>>>>> +int kvm_vcpu_preferred_target(struct kvm_vcpu_init *init)
>>>>> +{
>>>>> +   int target = kvm_target_cpu();
>>>>> +
>>>>> +   if (target < 0)
>>>>> +           return -ENODEV;
>>>>> +
>>>>> +   memset(init, 0, sizeof(*init));
>>>>> +
>>>>> +   /*
>>>>> +    * For now, we return all optional features are available
>>>>> +    * for preferred target. In future, we might have features
>>>>> +    * available based on underlying host.
>>>>> +    */
>>>>> +   init->target = (__u32)target;
>>>>> +   init->features[0] |= (1 << KVM_ARM_VCPU_POWER_OFF);
>>>>
>>>> I'm in two minds about this feature reporting. I see they serve a
>>>> purpose, but they also duplicate capabilities, which is the standard way
>>>> to advertise what KVM can do.
>>>>
>>>> It means we end up having to sync two reporting mechanism, and I feel
>>>> this is in general a bad idea.
>>>>
>>>> Furthermore, KVM_ARM_VCPU_POWER_OFF is hardly a feature of the HW, but
>>>> rather a firmware emulation thing.
>>>>
>>>> Peter, Christoffer: Thoughts?
>>>>
>>> I wanted to return the full kvm_vcpu_init instead of just a target int,
>>> so we did not have to come up with yet another ioctl if we need to
>>> return more information about the capabilities of the host CPU in the
>>> future.
>>>
>>> But perhaps we can formulate the API, to say only the (currently empty)
>>> following list of features can only be enabled if the corresponding bit
>>> is enabled, or something along those lines.
>>>
>>> -Christoffer
>>> _______________________________________________
>>> kvmarm mailing list
>>> kvmarm at lists.cs.columbia.edu
>>> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
>>
>> Do we stick with current implementation of returning struct kvm_vcpu_init ?
>> OR
>> Do we return struct kvm_vcpu_init with all features set to zero ?
>>
> Let's give Marc a day or two to respond to this one ;)

Are you implying I'm getting slow? ;-)

To answer Anup's question, I would tend to be cautious, and not expose
things for which we already have another API in place. So far, we've
stuck with the KVM approach of having a capability bit for each feature
we enable, and I'm quite happy with that.

So I'm in favour of Christoffer's proposal to return an empty feature
set, and start adding stuff if/when the need arises.

Cheers,

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH v3 1/4] ARM: KVM: Implement kvm_vcpu_preferred_target() function

[Anup Patel]

On Mon, Sep 23, 2013 at 9:14 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 23/09/13 16:31, Christoffer Dall wrote:
>> On Mon, Sep 23, 2013 at 01:35:20PM +0530, Anup Patel wrote:
>>> Hi Christoffer/Marc,
>>>
>>> On Fri, Sep 20, 2013 at 12:50 AM, Christoffer Dall
>>> <christoffer.dall@linaro.org> wrote:
>>>> On Thu, Sep 19, 2013 at 03:27:54PM +0100, Marc Zyngier wrote:
>>>>> On 19/09/13 14:11, Anup Patel wrote:
>>>>>> This patch implements kvm_vcpu_preferred_target() function for
>>>>>> KVM ARM which will help us implement KVM_ARM_PREFERRED_TARGET ioctl
>>>>>> for user space.
>>>>>>
>>>>>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>>>>>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>>>>>> ---
>>>>>>  arch/arm/kvm/guest.c              |   20 ++++++++++++++++++++
>>>>>>  arch/arm64/include/asm/kvm_host.h |    1 +
>>>>>>  2 files changed, 21 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c
>>>>>> index 152d036..b407e6c 100644
>>>>>> --- a/arch/arm/kvm/guest.c
>>>>>> +++ b/arch/arm/kvm/guest.c
>>>>>> @@ -222,6 +222,26 @@ int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
>>>>>>     return kvm_reset_vcpu(vcpu);
>>>>>>  }
>>>>>>
>>>>>> +int kvm_vcpu_preferred_target(struct kvm_vcpu_init *init)
>>>>>> +{
>>>>>> +   int target = kvm_target_cpu();
>>>>>> +
>>>>>> +   if (target < 0)
>>>>>> +           return -ENODEV;
>>>>>> +
>>>>>> +   memset(init, 0, sizeof(*init));
>>>>>> +
>>>>>> +   /*
>>>>>> +    * For now, we return all optional features are available
>>>>>> +    * for preferred target. In future, we might have features
>>>>>> +    * available based on underlying host.
>>>>>> +    */
>>>>>> +   init->target = (__u32)target;
>>>>>> +   init->features[0] |= (1 << KVM_ARM_VCPU_POWER_OFF);
>>>>>
>>>>> I'm in two minds about this feature reporting. I see they serve a
>>>>> purpose, but they also duplicate capabilities, which is the standard way
>>>>> to advertise what KVM can do.
>>>>>
>>>>> It means we end up having to sync two reporting mechanism, and I feel
>>>>> this is in general a bad idea.
>>>>>
>>>>> Furthermore, KVM_ARM_VCPU_POWER_OFF is hardly a feature of the HW, but
>>>>> rather a firmware emulation thing.
>>>>>
>>>>> Peter, Christoffer: Thoughts?
>>>>>
>>>> I wanted to return the full kvm_vcpu_init instead of just a target int,
>>>> so we did not have to come up with yet another ioctl if we need to
>>>> return more information about the capabilities of the host CPU in the
>>>> future.
>>>>
>>>> But perhaps we can formulate the API, to say only the (currently empty)
>>>> following list of features can only be enabled if the corresponding bit
>>>> is enabled, or something along those lines.
>>>>
>>>> -Christoffer
>>>> _______________________________________________
>>>> kvmarm mailing list
>>>> kvmarm at lists.cs.columbia.edu
>>>> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
>>>
>>> Do we stick with current implementation of returning struct kvm_vcpu_init ?
>>> OR
>>> Do we return struct kvm_vcpu_init with all features set to zero ?
>>>
>> Let's give Marc a day or two to respond to this one ;)
>
> Are you implying I'm getting slow? ;-)
>
> To answer Anup's question, I would tend to be cautious, and not expose
> things for which we already have another API in place. So far, we've
> stuck with the KVM approach of having a capability bit for each feature
> we enable, and I'm quite happy with that.
>
> So I'm in favour of Christoffer's proposal to return an empty feature
> set, and start adding stuff if/when the need arises.

Agreed, I will send revised patch where we return zeroed-out features
in struct kvm_vcpu_init (for now).

>
> Cheers,
>
>         M.
> --
> Jazz is not dead. It just smells funny...
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Regards,
Anup

[PATCH v3 1/4] ARM: KVM: Implement kvm_vcpu_preferred_target() function

[Christoffer Dall]

On Mon, Sep 23, 2013 at 09:24:06PM +0530, Anup Patel wrote:
> On Mon, Sep 23, 2013 at 9:14 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> > On 23/09/13 16:31, Christoffer Dall wrote:
> >> On Mon, Sep 23, 2013 at 01:35:20PM +0530, Anup Patel wrote:
> >>> Hi Christoffer/Marc,
> >>>
> >>> On Fri, Sep 20, 2013 at 12:50 AM, Christoffer Dall
> >>> <christoffer.dall@linaro.org> wrote:
> >>>> On Thu, Sep 19, 2013 at 03:27:54PM +0100, Marc Zyngier wrote:
> >>>>> On 19/09/13 14:11, Anup Patel wrote:
> >>>>>> This patch implements kvm_vcpu_preferred_target() function for
> >>>>>> KVM ARM which will help us implement KVM_ARM_PREFERRED_TARGET ioctl
> >>>>>> for user space.
> >>>>>>
> >>>>>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
> >>>>>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> >>>>>> ---
> >>>>>>  arch/arm/kvm/guest.c              |   20 ++++++++++++++++++++
> >>>>>>  arch/arm64/include/asm/kvm_host.h |    1 +
> >>>>>>  2 files changed, 21 insertions(+)
> >>>>>>
> >>>>>> diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c
> >>>>>> index 152d036..b407e6c 100644
> >>>>>> --- a/arch/arm/kvm/guest.c
> >>>>>> +++ b/arch/arm/kvm/guest.c
> >>>>>> @@ -222,6 +222,26 @@ int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
> >>>>>>     return kvm_reset_vcpu(vcpu);
> >>>>>>  }
> >>>>>>
> >>>>>> +int kvm_vcpu_preferred_target(struct kvm_vcpu_init *init)
> >>>>>> +{
> >>>>>> +   int target = kvm_target_cpu();
> >>>>>> +
> >>>>>> +   if (target < 0)
> >>>>>> +           return -ENODEV;
> >>>>>> +
> >>>>>> +   memset(init, 0, sizeof(*init));
> >>>>>> +
> >>>>>> +   /*
> >>>>>> +    * For now, we return all optional features are available
> >>>>>> +    * for preferred target. In future, we might have features
> >>>>>> +    * available based on underlying host.
> >>>>>> +    */
> >>>>>> +   init->target = (__u32)target;
> >>>>>> +   init->features[0] |= (1 << KVM_ARM_VCPU_POWER_OFF);
> >>>>>
> >>>>> I'm in two minds about this feature reporting. I see they serve a
> >>>>> purpose, but they also duplicate capabilities, which is the standard way
> >>>>> to advertise what KVM can do.
> >>>>>
> >>>>> It means we end up having to sync two reporting mechanism, and I feel
> >>>>> this is in general a bad idea.
> >>>>>
> >>>>> Furthermore, KVM_ARM_VCPU_POWER_OFF is hardly a feature of the HW, but
> >>>>> rather a firmware emulation thing.
> >>>>>
> >>>>> Peter, Christoffer: Thoughts?
> >>>>>
> >>>> I wanted to return the full kvm_vcpu_init instead of just a target int,
> >>>> so we did not have to come up with yet another ioctl if we need to
> >>>> return more information about the capabilities of the host CPU in the
> >>>> future.
> >>>>
> >>>> But perhaps we can formulate the API, to say only the (currently empty)
> >>>> following list of features can only be enabled if the corresponding bit
> >>>> is enabled, or something along those lines.
> >>>>
> >>>> -Christoffer
> >>>> _______________________________________________
> >>>> kvmarm mailing list
> >>>> kvmarm at lists.cs.columbia.edu
> >>>> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
> >>>
> >>> Do we stick with current implementation of returning struct kvm_vcpu_init ?
> >>> OR
> >>> Do we return struct kvm_vcpu_init with all features set to zero ?
> >>>
> >> Let's give Marc a day or two to respond to this one ;)
> >
> > Are you implying I'm getting slow? ;-)

Wouldn't dream of it.

> >
> > To answer Anup's question, I would tend to be cautious, and not expose
> > things for which we already have another API in place. So far, we've
> > stuck with the KVM approach of having a capability bit for each feature
> > we enable, and I'm quite happy with that.
> >
> > So I'm in favour of Christoffer's proposal to return an empty feature
> > set, and start adding stuff if/when the need arises.
> 
> Agreed, I will send revised patch where we return zeroed-out features
> in struct kvm_vcpu_init (for now).
> 
Sounds good, also remember to address this specific item in the API
documentation.

-Christoffer

[PATCH v3 2/4] ARM64: KVM: Implement kvm_vcpu_preferred_target() function

[Anup Patel]

This patch implements kvm_vcpu_preferred_target() function for
KVM ARM64 which will help us implement KVM_ARM_PREFERRED_TARGET
ioctl for user space.

Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
---
 arch/arm/include/asm/kvm_host.h |    1 +
 arch/arm64/kvm/guest.c          |   21 +++++++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 7d22517..76f3c19 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -154,6 +154,7 @@ struct kvm_vcpu_stat {
 struct kvm_vcpu_init;
 int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
 			const struct kvm_vcpu_init *init);
+int kvm_vcpu_preferred_target(struct kvm_vcpu_init *init);
 unsigned long kvm_arm_num_regs(struct kvm_vcpu *vcpu);
 int kvm_arm_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *indices);
 struct kvm_one_reg;
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index d7bf7d6..4da7882 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -254,6 +254,27 @@ int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
 	return kvm_reset_vcpu(vcpu);
 }
 
+int kvm_vcpu_preferred_target(struct kvm_vcpu_init *init)
+{
+	int target = kvm_target_cpu();
+
+	if (target < 0)
+		return -ENODEV;
+
+	memset(init, 0, sizeof(*init));
+
+	/* 
+	 * For now, we return all optional features are available
+	 * for preferred target. In future, we might have features
+	 * available based on underlying host.
+	 */
+	init->target = (__u32)target;
+	init->features[0] |= (1 << KVM_ARM_VCPU_POWER_OFF);
+	init->features[0] |= (1 << KVM_ARM_VCPU_EL1_32BIT);
+
+	return 0;
+}
+
 int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)
 {
 	return -EINVAL;
-- 
1.7.9.5

[PATCH v3 2/4] ARM64: KVM: Implement kvm_vcpu_preferred_target() function

[Marc Zyngier]

On 19/09/13 14:11, Anup Patel wrote:
> This patch implements kvm_vcpu_preferred_target() function for
> KVM ARM64 which will help us implement KVM_ARM_PREFERRED_TARGET
> ioctl for user space.
> 
> Signed-off-by: Anup Patel <anup.patel@linaro.org>
> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> ---
>  arch/arm/include/asm/kvm_host.h |    1 +

This hunk belongs to patch #1, and the similar hunk in #1 belongs here.

	M.

>  arch/arm64/kvm/guest.c          |   21 +++++++++++++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 7d22517..76f3c19 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -154,6 +154,7 @@ struct kvm_vcpu_stat {
>  struct kvm_vcpu_init;
>  int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
>  			const struct kvm_vcpu_init *init);
> +int kvm_vcpu_preferred_target(struct kvm_vcpu_init *init);
>  unsigned long kvm_arm_num_regs(struct kvm_vcpu *vcpu);
>  int kvm_arm_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *indices);
>  struct kvm_one_reg;
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index d7bf7d6..4da7882 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -254,6 +254,27 @@ int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
>  	return kvm_reset_vcpu(vcpu);
>  }
>  
> +int kvm_vcpu_preferred_target(struct kvm_vcpu_init *init)
> +{
> +	int target = kvm_target_cpu();
> +
> +	if (target < 0)
> +		return -ENODEV;
> +
> +	memset(init, 0, sizeof(*init));
> +
> +	/* 
> +	 * For now, we return all optional features are available
> +	 * for preferred target. In future, we might have features
> +	 * available based on underlying host.
> +	 */
> +	init->target = (__u32)target;
> +	init->features[0] |= (1 << KVM_ARM_VCPU_POWER_OFF);
> +	init->features[0] |= (1 << KVM_ARM_VCPU_EL1_32BIT);
> +
> +	return 0;
> +}
> +
>  int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)
>  {
>  	return -EINVAL;
> 


-- 
Jazz is not dead. It just smells funny...

[PATCH v3 2/4] ARM64: KVM: Implement kvm_vcpu_preferred_target() function

[Anup Patel]

On Thu, Sep 19, 2013 at 7:45 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 19/09/13 14:11, Anup Patel wrote:
>> This patch implements kvm_vcpu_preferred_target() function for
>> KVM ARM64 which will help us implement KVM_ARM_PREFERRED_TARGET
>> ioctl for user space.
>>
>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>> ---
>>  arch/arm/include/asm/kvm_host.h |    1 +
>
> This hunk belongs to patch #1, and the similar hunk in #1 belongs here.

UHH, How did I miss this one !!!

Thanks for pointing.

--Anup

>
>         M.
>
>>  arch/arm64/kvm/guest.c          |   21 +++++++++++++++++++++
>>  2 files changed, 22 insertions(+)
>>
>> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
>> index 7d22517..76f3c19 100644
>> --- a/arch/arm/include/asm/kvm_host.h
>> +++ b/arch/arm/include/asm/kvm_host.h
>> @@ -154,6 +154,7 @@ struct kvm_vcpu_stat {
>>  struct kvm_vcpu_init;
>>  int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
>>                       const struct kvm_vcpu_init *init);
>> +int kvm_vcpu_preferred_target(struct kvm_vcpu_init *init);
>>  unsigned long kvm_arm_num_regs(struct kvm_vcpu *vcpu);
>>  int kvm_arm_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *indices);
>>  struct kvm_one_reg;
>> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
>> index d7bf7d6..4da7882 100644
>> --- a/arch/arm64/kvm/guest.c
>> +++ b/arch/arm64/kvm/guest.c
>> @@ -254,6 +254,27 @@ int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
>>       return kvm_reset_vcpu(vcpu);
>>  }
>>
>> +int kvm_vcpu_preferred_target(struct kvm_vcpu_init *init)
>> +{
>> +     int target = kvm_target_cpu();
>> +
>> +     if (target < 0)
>> +             return -ENODEV;
>> +
>> +     memset(init, 0, sizeof(*init));
>> +
>> +     /*
>> +      * For now, we return all optional features are available
>> +      * for preferred target. In future, we might have features
>> +      * available based on underlying host.
>> +      */
>> +     init->target = (__u32)target;
>> +     init->features[0] |= (1 << KVM_ARM_VCPU_POWER_OFF);
>> +     init->features[0] |= (1 << KVM_ARM_VCPU_EL1_32BIT);
>> +
>> +     return 0;
>> +}
>> +
>>  int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)
>>  {
>>       return -EINVAL;
>>
>
>
> --
> Jazz is not dead. It just smells funny...
>
>
> _______________________________________________
> kvmarm mailing list
> kvmarm at lists.cs.columbia.edu
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm

[PATCH v3 3/4] ARM/ARM64: KVM: Implement KVM_ARM_PREFERRED_TARGET ioctl

[Anup Patel]

For implementing CPU=host, we need a mechanism for querying
preferred VCPU target type on underlying Host.

This patch implements KVM_ARM_PREFERRED_TARGET vm ioctl which
returns struct kvm_vcpu_init instance containing information
about preferred VCPU target type and available optional VCPU
features to user space.

Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
---
 arch/arm/kvm/arm.c       |   13 +++++++++++++
 include/uapi/linux/kvm.h |    1 +
 2 files changed, 14 insertions(+)

diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 9c697db..cc5adb9 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -797,6 +797,19 @@ long kvm_arch_vm_ioctl(struct file *filp,
 			return -EFAULT;
 		return kvm_vm_ioctl_set_device_addr(kvm, &dev_addr);
 	}
+	case KVM_ARM_PREFERRED_TARGET: {
+		int err;
+		struct kvm_vcpu_init init;
+
+		err = kvm_vcpu_preferred_target(&init);
+		if (err)
+			return err;
+
+		if (copy_to_user(argp, &init, sizeof(init)))
+			return -EFAULT;
+
+		return 0;
+	}
 	default:
 		return -EINVAL;
 	}
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 99c2533..e32e776 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1012,6 +1012,7 @@ struct kvm_s390_ucas_mapping {
 /* VM is being stopped by host */
 #define KVM_KVMCLOCK_CTRL	  _IO(KVMIO,   0xad)
 #define KVM_ARM_VCPU_INIT	  _IOW(KVMIO,  0xae, struct kvm_vcpu_init)
+#define KVM_ARM_PREFERRED_TARGET  _IOR(KVMIO,  0xaf, struct kvm_vcpu_init)
 #define KVM_GET_REG_LIST	  _IOWR(KVMIO, 0xb0, struct kvm_reg_list)
 
 #define KVM_DEV_ASSIGN_ENABLE_IOMMU	(1 << 0)
-- 
1.7.9.5

[PATCH v3 4/4] KVM: Add documentation for KVM_ARM_PREFERRED_TARGET ioctl

[Anup Patel]

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 |   26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 858aecf..d513cc5 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -2303,8 +2303,27 @@ 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 optional 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 +2342,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 +2380,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