Patchwork [1/2] KVM: PPC: epapr: Add idle hcall support for host

login
register
mail settings
Submitter Liu Yu-B13201
Date Dec. 31, 2011, 6:16 a.m.
Message ID <1325312176-17697-1-git-send-email-yu.liu@freescale.com>
Download mbox | patch
Permalink /patch/133731/
State New
Headers show

Comments

Liu Yu-B13201 - Dec. 31, 2011, 6:16 a.m.
Add a new field opt_feature in struct kvm_ppc_pvinfo
to tell userspace whether it support hcall idle.

Signed-off-by: Liu Yu <yu.liu@freescale.com>
---
 arch/powerpc/include/asm/kvm_para.h |   18 ++++++++++++++----
 arch/powerpc/kvm/powerpc.c          |    9 +++++++++
 include/linux/kvm.h                 |    5 ++++-
 3 files changed, 27 insertions(+), 5 deletions(-)
Scott Wood - Jan. 2, 2012, 6:17 p.m.
On 12/31/2011 12:16 AM, Liu Yu wrote:
> Add a new field opt_feature in struct kvm_ppc_pvinfo
> to tell userspace whether it support hcall idle.
> 
> Signed-off-by: Liu Yu <yu.liu@freescale.com>
> ---
>  arch/powerpc/include/asm/kvm_para.h |   18 ++++++++++++++----
>  arch/powerpc/kvm/powerpc.c          |    9 +++++++++
>  include/linux/kvm.h                 |    5 ++++-
>  3 files changed, 27 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/kvm_para.h b/arch/powerpc/include/asm/kvm_para.h
> index 50533f9..0686377 100644
> --- a/arch/powerpc/include/asm/kvm_para.h
> +++ b/arch/powerpc/include/asm/kvm_para.h
> @@ -40,17 +40,27 @@ struct kvm_vcpu_arch_shared {
>  	__u32 sr[16];
>  };
>  
> +#ifdef __KERNEL__
> +
>  #define KVM_SC_MAGIC_R0		0x4b564d21 /* "KVM!" */
> -#define HC_VENDOR_KVM		(42 << 16)
> +
> +#include <asm/epapr_hcalls.h>
> +
> +/* ePAPR Hypercall Vendor ID */
> +#define HC_VENDOR_EPAPR		(EV_EPAPR_VENDOR_ID << 16)
> +#define HC_VENDOR_KVM		(EV_KVM_VENDOR_ID << 16)
> +
> +/* ePAPR Hypercall Token */
> +#define HC_EV_IDLE		EV_IDLE
> +
> +/* ePAPR Hypercall Return Codes */
>  #define HC_EV_SUCCESS		0
> -#define HC_EV_UNIMPLEMENTED	12
> +#define HC_EV_UNIMPLEMENTED	EV_UNIMPLEMENTED
>  
>  #define KVM_FEATURE_MAGIC_PAGE	1
>  
>  #define KVM_MAGIC_FEAT_SR	(1 << 0)
>  
> -#ifdef __KERNEL__
> -

We don't want this stuff to be kernel-only.

> diff --git a/include/linux/kvm.h b/include/linux/kvm.h
> index c107fae..5af21f3 100644
> --- a/include/linux/kvm.h
> +++ b/include/linux/kvm.h
> @@ -426,9 +426,12 @@ struct kvm_ppc_pvinfo {
>  	/* out */
>  	__u32 flags;
>  	__u32 hcall[4];
> -	__u8  pad[108];
> +	__u32 opt_features;
> +	__u8  pad[104];
>  };

If the features weren't "opt"ional, there wouldn't be a bitmap of them. :-)

Just call it "features".  Or maybe just use "flags"?

-Scott

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Avi Kivity - Jan. 3, 2012, 2:01 p.m.
On 12/31/2011 08:16 AM, Liu Yu wrote:
> Add a new field opt_feature in struct kvm_ppc_pvinfo
> to tell userspace whether it support hcall idle.
>
> diff --git a/include/linux/kvm.h b/include/linux/kvm.h
> index c107fae..5af21f3 100644
> --- a/include/linux/kvm.h
> +++ b/include/linux/kvm.h
> @@ -426,9 +426,12 @@ struct kvm_ppc_pvinfo {
>  	/* out */
>  	__u32 flags;
>  	__u32 hcall[4];
> -	__u8  pad[108];
> +	__u32 opt_features;
> +	__u8  pad[104];
>  };
>  
> +#define KVM_PPC_PVINFO_HAS_EV_IDLE   (1<<0)
> +
>

Needs to be documented, plus a KVM_CAP so userspace can discover that
this feature is available,
Alexander Graf - Jan. 3, 2012, 2:13 p.m.
On 03.01.2012, at 15:01, Avi Kivity wrote:

> On 12/31/2011 08:16 AM, Liu Yu wrote:
>> Add a new field opt_feature in struct kvm_ppc_pvinfo
>> to tell userspace whether it support hcall idle.
>> 
>> diff --git a/include/linux/kvm.h b/include/linux/kvm.h
>> index c107fae..5af21f3 100644
>> --- a/include/linux/kvm.h
>> +++ b/include/linux/kvm.h
>> @@ -426,9 +426,12 @@ struct kvm_ppc_pvinfo {
>> 	/* out */
>> 	__u32 flags;
>> 	__u32 hcall[4];
>> -	__u8  pad[108];
>> +	__u32 opt_features;
>> +	__u8  pad[104];
>> };
>> 
>> +#define KVM_PPC_PVINFO_HAS_EV_IDLE   (1<<0)
>> +
>> 
> 
> Needs to be documented, plus a KVM_CAP so userspace can discover that
> this feature is available,

Not if we put the bit into flags. Then user space can just check the flags bitmap and know that it's there regardless of capabilities, because older kernels will set the bit to 0.


Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Avi Kivity - Jan. 4, 2012, 10:33 a.m.
On 01/03/2012 04:13 PM, Alexander Graf wrote:
> On 03.01.2012, at 15:01, Avi Kivity wrote:
>
> > On 12/31/2011 08:16 AM, Liu Yu wrote:
> >> Add a new field opt_feature in struct kvm_ppc_pvinfo
> >> to tell userspace whether it support hcall idle.
> >> 
> >> diff --git a/include/linux/kvm.h b/include/linux/kvm.h
> >> index c107fae..5af21f3 100644
> >> --- a/include/linux/kvm.h
> >> +++ b/include/linux/kvm.h
> >> @@ -426,9 +426,12 @@ struct kvm_ppc_pvinfo {
> >> 	/* out */
> >> 	__u32 flags;
> >> 	__u32 hcall[4];
> >> -	__u8  pad[108];
> >> +	__u32 opt_features;
> >> +	__u8  pad[104];
> >> };
> >> 
> >> +#define KVM_PPC_PVINFO_HAS_EV_IDLE   (1<<0)
> >> +
> >> 
> > 
> > Needs to be documented, plus a KVM_CAP so userspace can discover that
> > this feature is available,
>
> Not if we put the bit into flags. Then user space can just check the flags bitmap and know that it's there regardless of capabilities, because older kernels will set the bit to 0.

It needs to detect that opt_features is available during compile time
(qemu copies headers, but we don't want to force everyone to do that).
Alexander Graf - Jan. 4, 2012, 12:04 p.m.
On 04.01.2012, at 11:33, Avi Kivity <avi@redhat.com> wrote:

> On 01/03/2012 04:13 PM, Alexander Graf wrote:
>> On 03.01.2012, at 15:01, Avi Kivity wrote:
>> 
>>> On 12/31/2011 08:16 AM, Liu Yu wrote:
>>>> Add a new field opt_feature in struct kvm_ppc_pvinfo
>>>> to tell userspace whether it support hcall idle.
>>>> 
>>>> diff --git a/include/linux/kvm.h b/include/linux/kvm.h
>>>> index c107fae..5af21f3 100644
>>>> --- a/include/linux/kvm.h
>>>> +++ b/include/linux/kvm.h
>>>> @@ -426,9 +426,12 @@ struct kvm_ppc_pvinfo {
>>>>    /* out */
>>>>    __u32 flags;
>>>>    __u32 hcall[4];
>>>> -    __u8  pad[108];
>>>> +    __u32 opt_features;
>>>> +    __u8  pad[104];
>>>> };
>>>> 
>>>> +#define KVM_PPC_PVINFO_HAS_EV_IDLE   (1<<0)
>>>> +
>>>> 
>>> 
>>> Needs to be documented, plus a KVM_CAP so userspace can discover that
>>> this feature is available,
>> 
>> Not if we put the bit into flags. Then user space can just check the flags bitmap and know that it's there regardless of capabilities, because older kernels will set the bit to 0.
> 
> It needs to detect that opt_features is available during compile time
> (qemu copies headers, but we don't want to force everyone to do that).

The point is that we don't need opt_features. We already have a fearure bitmap in the struct.

Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Avi Kivity - Jan. 4, 2012, 12:11 p.m.
On 01/04/2012 02:04 PM, Alexander Graf wrote:
> >> Not if we put the bit into flags. Then user space can just check the flags bitmap and know that it's there regardless of capabilities, because older kernels will set the bit to 0.
> > 
> > It needs to detect that opt_features is available during compile time
> > (qemu copies headers, but we don't want to force everyone to do that).
>
> The point is that we don't need opt_features. We already have a fearure bitmap in the struct.

Ah! Okay then.
Scott Wood - Jan. 4, 2012, 7:25 p.m.
On 01/04/2012 06:04 AM, Alexander Graf wrote:
> 
> 
> On 04.01.2012, at 11:33, Avi Kivity <avi@redhat.com> wrote:
> 
>> On 01/03/2012 04:13 PM, Alexander Graf wrote:
>>> On 03.01.2012, at 15:01, Avi Kivity wrote:
>>>
>>>> On 12/31/2011 08:16 AM, Liu Yu wrote:
>>>>> Add a new field opt_feature in struct kvm_ppc_pvinfo
>>>>> to tell userspace whether it support hcall idle.
>>>>>
>>>>> diff --git a/include/linux/kvm.h b/include/linux/kvm.h
>>>>> index c107fae..5af21f3 100644
>>>>> --- a/include/linux/kvm.h
>>>>> +++ b/include/linux/kvm.h
>>>>> @@ -426,9 +426,12 @@ struct kvm_ppc_pvinfo {
>>>>>    /* out */
>>>>>    __u32 flags;
>>>>>    __u32 hcall[4];
>>>>> -    __u8  pad[108];
>>>>> +    __u32 opt_features;
>>>>> +    __u8  pad[104];
>>>>> };
>>>>>
>>>>> +#define KVM_PPC_PVINFO_HAS_EV_IDLE   (1<<0)
>>>>> +
>>>>>
>>>>
>>>> Needs to be documented, plus a KVM_CAP so userspace can discover that
>>>> this feature is available,
>>>
>>> Not if we put the bit into flags. Then user space can just check the flags bitmap and know that it's there regardless of capabilities, because older kernels will set the bit to 0.
>>
>> It needs to detect that opt_features is available during compile time
>> (qemu copies headers, but we don't want to force everyone to do that).
> 
> The point is that we don't need opt_features. We already have a fearure bitmap in the struct.

Even if we did for whatever reason want a new field, the entire struct
is zeroed currently, so we only need a flag if we need to distinguish
"field is zero" from "field is not valid".

-Scott

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/powerpc/include/asm/kvm_para.h b/arch/powerpc/include/asm/kvm_para.h
index 50533f9..0686377 100644
--- a/arch/powerpc/include/asm/kvm_para.h
+++ b/arch/powerpc/include/asm/kvm_para.h
@@ -40,17 +40,27 @@  struct kvm_vcpu_arch_shared {
 	__u32 sr[16];
 };
 
+#ifdef __KERNEL__
+
 #define KVM_SC_MAGIC_R0		0x4b564d21 /* "KVM!" */
-#define HC_VENDOR_KVM		(42 << 16)
+
+#include <asm/epapr_hcalls.h>
+
+/* ePAPR Hypercall Vendor ID */
+#define HC_VENDOR_EPAPR		(EV_EPAPR_VENDOR_ID << 16)
+#define HC_VENDOR_KVM		(EV_KVM_VENDOR_ID << 16)
+
+/* ePAPR Hypercall Token */
+#define HC_EV_IDLE		EV_IDLE
+
+/* ePAPR Hypercall Return Codes */
 #define HC_EV_SUCCESS		0
-#define HC_EV_UNIMPLEMENTED	12
+#define HC_EV_UNIMPLEMENTED	EV_UNIMPLEMENTED
 
 #define KVM_FEATURE_MAGIC_PAGE	1
 
 #define KVM_MAGIC_FEAT_SR	(1 << 0)
 
-#ifdef __KERNEL__
-
 #ifdef CONFIG_KVM_GUEST
 
 #include <linux/of.h>
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index c33f6a7..786b34b 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -81,6 +81,10 @@  int kvmppc_kvm_pv(struct kvm_vcpu *vcpu)
 
 		/* Second return value is in r4 */
 		break;
+	case HC_VENDOR_EPAPR | HC_EV_IDLE:
+		r = HC_EV_SUCCESS;
+		kvm_vcpu_block(vcpu);
+		break;
 	default:
 		r = HC_EV_UNIMPLEMENTED;
 		break;
@@ -772,6 +776,11 @@  static int kvm_vm_ioctl_get_pvinfo(struct kvm_ppc_pvinfo *pvinfo)
 	pvinfo->hcall[2] = inst_sc;
 	pvinfo->hcall[3] = inst_nop;
 
+	pvinfo->opt_features = 0;
+#ifdef CONFIG_BOOKE
+	pvinfo->opt_features |= KVM_PPC_PVINFO_HAS_EV_IDLE;
+#endif
+
 	return 0;
 }
 
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index c107fae..5af21f3 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -426,9 +426,12 @@  struct kvm_ppc_pvinfo {
 	/* out */
 	__u32 flags;
 	__u32 hcall[4];
-	__u8  pad[108];
+	__u32 opt_features;
+	__u8  pad[104];
 };
 
+#define KVM_PPC_PVINFO_HAS_EV_IDLE   (1<<0)
+
 #define KVMIO 0xAE
 
 /*