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

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

[Liu Yu]

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__
-
 #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
 
 /*
-- 
1.6.4

[PATCH 2/2] KVM: PPC: epapr: Install ev_idle hcall for paravirt guest linux

[Liu Yu]

If the guest hypervisor node contains "has-idle" property.

Signed-off-by: Liu Yu <yu.liu@freescale.com>
---
 arch/powerpc/kernel/kvm.c |   29 +++++++++++++++++++++++++++++
 1 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/kernel/kvm.c b/arch/powerpc/kernel/kvm.c
index b06bdae..258f129 100644
--- a/arch/powerpc/kernel/kvm.c
+++ b/arch/powerpc/kernel/kvm.c
@@ -28,6 +28,7 @@
 #include <asm/sections.h>
 #include <asm/cacheflush.h>
 #include <asm/disassemble.h>
+#include <asm/machdep.h>
 
 #define KVM_MAGIC_PAGE		(-4096L)
 #define magic_var(x) KVM_MAGIC_PAGE + offsetof(struct kvm_vcpu_arch_shared, x)
@@ -571,6 +572,30 @@ static __init void kvm_free_tmp(void)
 	}
 }
 
+static void kvm_hcall_ev_idle(void)
+{
+	ulong in[8];
+	ulong out[8];
+
+	current_thread_info()->local_flags |= _TLF_NAPPING;
+	local_irq_enable();
+	kvm_hypercall(in, out, HC_VENDOR_EPAPR | HC_EV_IDLE);
+}
+
+static int kvm_para_ev_has_idle(void)
+{
+	struct device_node *hyper_node;
+
+	hyper_node = of_find_node_by_path("/hypervisor");
+	if (!hyper_node)
+		return 0;
+
+	if (of_get_property(hyper_node, "has-idle", NULL))
+		return 1;
+
+	return 0;
+}
+
 static int __init kvm_guest_init(void)
 {
 	if (!kvm_para_available())
@@ -587,6 +612,10 @@ static int __init kvm_guest_init(void)
 	powersave_nap = 1;
 #endif
 
+	/* Install hcall EV_IDLE based power_save for guest kernel */
+	if (kvm_para_ev_has_idle())
+		ppc_md.power_save = kvm_hcall_ev_idle;
+
 free_tmp:
 	kvm_free_tmp();
 
-- 
1.6.4

Re: [PATCH 2/2] KVM: PPC: epapr: Install ev_idle hcall for paravirt guest linux

[Scott Wood]

On 12/31/2011 12:16 AM, Liu Yu wrote:
> If the guest hypervisor node contains "has-idle" property.
> 
> Signed-off-by: Liu Yu <yu.liu@freescale.com>
> ---
>  arch/powerpc/kernel/kvm.c |   29 +++++++++++++++++++++++++++++
>  1 files changed, 29 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/kvm.c b/arch/powerpc/kernel/kvm.c
> index b06bdae..258f129 100644
> --- a/arch/powerpc/kernel/kvm.c
> +++ b/arch/powerpc/kernel/kvm.c
> @@ -28,6 +28,7 @@
>  #include <asm/sections.h>
>  #include <asm/cacheflush.h>
>  #include <asm/disassemble.h>
> +#include <asm/machdep.h>
>  
>  #define KVM_MAGIC_PAGE		(-4096L)
>  #define magic_var(x) KVM_MAGIC_PAGE + offsetof(struct kvm_vcpu_arch_shared, x)
> @@ -571,6 +572,30 @@ static __init void kvm_free_tmp(void)
>  	}
>  }
>  
> +static void kvm_hcall_ev_idle(void)
> +{
> +	ulong in[8];
> +	ulong out[8];
> +
> +	current_thread_info()->local_flags |= _TLF_NAPPING;
> +	local_irq_enable();
> +	kvm_hypercall(in, out, HC_VENDOR_EPAPR | HC_EV_IDLE);
> +}

I think this has to be done in assembly -- otherwise it's not safe for
the interrupt handler to return directly to LR.

> +static int kvm_para_ev_has_idle(void)
> +{
> +	struct device_node *hyper_node;
> +
> +	hyper_node = of_find_node_by_path("/hypervisor");
> +	if (!hyper_node)
> +		return 0;
> +
> +	if (of_get_property(hyper_node, "has-idle", NULL))
> +		return 1;
> +
> +	return 0;
> +}

Since this is standardized in ePAPR, can we move it out of kvm.c an into
generic code to work with any hypervisor that sets has-idle?  Likewise
with kvm_para_setup().

-Scott

RE: [PATCH 2/2] KVM: PPC: epapr: Install ev_idle hcall for paravirt guest linux

[Liu Yu-B13201]

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, January 03, 2012 2:23 AM
> To: Liu Yu-B13201
> Cc: agraf@suse.de; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
> Subject: Re: [PATCH 2/2] KVM: PPC: epapr: Install ev_idle hcall for
> paravirt guest linux
> 
> On 12/31/2011 12:16 AM, Liu Yu wrote:
> > If the guest hypervisor node contains "has-idle" property.
> >
> > Signed-off-by: Liu Yu <yu.liu@freescale.com>
> > ---
> >  arch/powerpc/kernel/kvm.c |   29 +++++++++++++++++++++++++++++
> >  1 files changed, 29 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/kvm.c b/arch/powerpc/kernel/kvm.c
> > index b06bdae..258f129 100644
> > --- a/arch/powerpc/kernel/kvm.c
> > +++ b/arch/powerpc/kernel/kvm.c
> > @@ -28,6 +28,7 @@
> >  #include <asm/sections.h>
> >  #include <asm/cacheflush.h>
> >  #include <asm/disassemble.h>
> > +#include <asm/machdep.h>
> >
> >  #define KVM_MAGIC_PAGE		(-4096L)
> >  #define magic_var(x) KVM_MAGIC_PAGE + offsetof(struct
> > kvm_vcpu_arch_shared, x) @@ -571,6 +572,30 @@ static __init void
> kvm_free_tmp(void)
> >  	}
> >  }
> >
> > +static void kvm_hcall_ev_idle(void)
> > +{
> > +	ulong in[8];
> > +	ulong out[8];
> > +
> > +	current_thread_info()->local_flags |= _TLF_NAPPING;
> > +	local_irq_enable();
> > +	kvm_hypercall(in, out, HC_VENDOR_EPAPR | HC_EV_IDLE); }
> 
> I think this has to be done in assembly -- otherwise it's not safe for
> the interrupt handler to return directly to LR.
> 
> > +static int kvm_para_ev_has_idle(void) {
> > +	struct device_node *hyper_node;
> > +
> > +	hyper_node = of_find_node_by_path("/hypervisor");
> > +	if (!hyper_node)
> > +		return 0;
> > +
> > +	if (of_get_property(hyper_node, "has-idle", NULL))
> > +		return 1;
> > +
> > +	return 0;
> > +}
> 
> Since this is standardized in ePAPR, can we move it out of kvm.c an into
> generic code to work with any hypervisor that sets has-idle?  Likewise
> with kvm_para_setup().
> 

Do you mean we scan and parse hypervisor node in early_init_devtree()?

Thanks,
Yu

Re: [PATCH 2/2] KVM: PPC: epapr: Install ev_idle hcall for paravirt guest linux

[Scott Wood]

On 01/04/2012 03:23 AM, Liu Yu-B13201 wrote:
> 
> 
>> -----Original Message-----
>> From: Wood Scott-B07421
>> Sent: Tuesday, January 03, 2012 2:23 AM
>> To: Liu Yu-B13201
>> Cc: agraf@suse.de; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
>> Subject: Re: [PATCH 2/2] KVM: PPC: epapr: Install ev_idle hcall for
>> paravirt guest linux
>>
>> Since this is standardized in ePAPR, can we move it out of kvm.c an into
>> generic code to work with any hypervisor that sets has-idle?  Likewise
>> with kvm_para_setup().
>>
> 
> Do you mean we scan and parse hypervisor node in early_init_devtree()?

No, just have a separate initcall for enabling ePAPR paravirt features.

-Scott

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

[Scott Wood]

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

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

[Avi Kivity]

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,

-- 
error compiling committee.c: too many arguments to function

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

[Alexander Graf]

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

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

[Avi Kivity]

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


-- 
error compiling committee.c: too many arguments to function

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

[Alexander Graf]

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

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

[Avi Kivity]

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.

-- 
error compiling committee.c: too many arguments to function

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

[Scott Wood]

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