[PATCH 0 of 3] Fix guest eating 100% cpu when guest is idle on PowerPC

Jerone Young

author

2008-04-25 05:56 UTC #0 lore raw

PATCH 1/1 Fix guest eating 100% cpu when guest is idle on PowerPC

This set of patches fixes 100% CPU usage when a guest is idle on PowerPC. This ti me it uses common kvm functions to sleep the guest.

Signed-off-by Jerone Young <[email protected]>

5 files changed, 91 insertions(+), 4 deletions(-)
arch/powerpc/kvm/booke_guest.c      |    6 +++
arch/powerpc/kvm/emulate.c          |    5 ++
arch/powerpc/kvm/powerpc.c          |   15 ++++++-
arch/powerpc/platforms/44x/Makefile |    2 -
arch/powerpc/platforms/44x/idle.c   |   67 +++++++++++++++++++++++++++++++++++

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Jerone Young

author

2008-04-25 05:56 UTC #1 lore raw

PATCH 1/1 Add idle wait support for 44x platforms

2 files changed, 68 insertions(+), 1 deletion(-)

arch/powerpc/platforms/44x/Makefile | 2 -
arch/powerpc/platforms/44x/idle.c | 67 +++++++++++++++++++++++++++++++++++

This patch has been accepted upstream and will be in 2.6.26. So it will eventually need to be removed when we move to 2.6.26rc.

This patch adds the ability for the CPU to go into wait state while in cpu_idle loop. This helps virtulization solutions know when the guest Linux kernel is in an idle state. There are two ways to do it.

Command line options:
idle=spin <-- CPU will spin

By default will go into wait mode.

Signed-off-by Jerone Young <[email protected]>

arch/powerpc/platforms/44x/Makefile +1 -1

@@ -1,4 +1,4 @@ obj-$(CONFIG_44x)	:= misc_44x.o
-obj-$(CONFIG_44x)	:= misc_44x.o
+obj-$(CONFIG_44x)	:= misc_44x.o idle.o
 obj-$(CONFIG_EBONY)	+= ebony.o
 obj-$(CONFIG_TAISHAN)	+= taishan.o
 obj-$(CONFIG_BAMBOO)	+= bamboo.o

arch/powerpc/platforms/44x/idle.c new file +67 -0

67 lines not on this page · raw message

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Jerone Young

author

2008-04-25 05:56 UTC #2 lore raw

PATCH 1/1 Add PowerPC KVM guest wait handling support

In reply to Jerone Young

2 files changed, 7 insertions(+), 3 deletions(-)
arch/powerpc/kvm/emulate.c |    5 +++++
arch/powerpc/kvm/powerpc.c |    5 ++---

This patch handles a guest that is in a wait state. This ensures that the guest is not allways eating up 100% cpu when it is idle.

Signed-off-by Jerone Young <[email protected]>

arch/powerpc/kvm/emulate.c +5 -0

@@ -265,6 +265,11 @@ int kvmppc_emulate_instruction(struct kv
 		case 146:                                       /* mtmsr */
 			rs = get_rs(inst);
 			kvmppc_set_msr(vcpu, vcpu->arch.gpr[rs]);
+			
+			/* handle guest vcpu that is in wait state */
+			if (vcpu->arch.msr & MSR_WE) {
+				kvm_vcpu_block(vcpu);
+			}
 			break;
 
 		case 163:                                       /* wrteei */

arch/powerpc/kvm/powerpc.c +2 -3

@@ -36,13 +36,12 @@ gfn_t unalias_gfn(struct kvm *kvm, gfn_t
 
 int kvm_cpu_has_interrupt(struct kvm_vcpu *v)
 {
-	/* XXX implement me */
-	return 0;
+	return !!(v->arch.pending_exceptions);
 }
 
 int kvm_arch_vcpu_runnable(struct kvm_vcpu *v)
 {
-	return 1;
+	return !(v->arch.msr & MSR_WE);
 }

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Jerone Young

author

2008-04-25 05:56 UTC #4 lore raw

PATCH 1/1 Add premption handlers & properly wake sleeping guest

In reply to Jerone Young

2 files changed, 16 insertions(+)

arch/powerpc/kvm/booke_guest.c | 6 ++++++
arch/powerpc/kvm/powerpc.c | 10 ++++++++++

This patch adds vcpu_put & vpu_load in strategic places (as x86 does it), for use of premption. This patch also adds a very critial bit need to wake up guest that end up going to being rescheduled and go to sleep.

Signed-off-by Jerone Young <[email protected]>

arch/powerpc/kvm/booke_guest.c +6 -0

@@ -514,6 +514,8 @@ int kvm_arch_vcpu_ioctl_get_regs(struct 
 {
 	int i;
 
+	vcpu_load(vcpu);
+
 	regs->pc = vcpu->arch.pc;
 	regs->cr = vcpu->arch.cr;
 	regs->ctr = vcpu->arch.ctr;

@@ -533,6 +535,8 @@ int kvm_arch_vcpu_ioctl_get_regs(struct 
 
 	for (i = 0; i < ARRAY_SIZE(regs->gpr); i++)
 		regs->gpr[i] = vcpu->arch.gpr[i];
+
+	vcpu_put(vcpu);	
 
 	return 0;
 }

@@ -595,6 +599,7 @@ int kvm_arch_vcpu_ioctl_translate(struct
 	u8 pid;
 	u8 as;
 
+	vcpu_load(vcpu);
 	eaddr = tr->linear_address;
 	pid = (tr->linear_address >> 32) & 0xff;
 	as = (tr->linear_address >> 40) & 0x1;

@@ -610,6 +615,7 @@ int kvm_arch_vcpu_ioctl_translate(struct
 	tr->physical_address = tlb_xlate(gtlbe, eaddr);
 	/* XXX what does "writeable" and "usermode" even mean? */
 	tr->valid = 1;
+	vcpu_put(vcpu);
 
 	return 0;
 }

arch/powerpc/kvm/powerpc.c +10 -0

@@ -212,6 +212,9 @@ static void kvmppc_decrementer_func(unsi
 {
 	struct kvm_vcpu *vcpu = (struct kvm_vcpu *)data;
 
+	if (waitqueue_active(&vcpu->wq))
+		wake_up_interruptible(&vcpu->wq);
+
 	kvmppc_queue_exception(vcpu, BOOKE_INTERRUPT_DECREMENTER);
 }

@@ -338,6 +341,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_v
 	int r;
 	sigset_t sigsaved;
 
+	vcpu_load(vcpu);
+
 	if (vcpu->sigset_active)
 		sigprocmask(SIG_SETMASK, &vcpu->sigset, &sigsaved);

@@ -362,12 +367,17 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_v
 	if (vcpu->sigset_active)
 		sigprocmask(SIG_SETMASK, &sigsaved, NULL);
 
+	vcpu_put(vcpu);
+
 	return r;
 }
 
 int kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu, struct kvm_interrupt *irq)
 {
+	vcpu_load(vcpu);
 	kvmppc_queue_exception(vcpu, BOOKE_INTERRUPT_EXTERNAL);
+	vcpu_put(vcpu);
+
 	return 0;
 }

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Hollis Blanchard

2008-04-25 13:52 UTC #5 lore raw

Re: [kvm-ppc-devel] [PATCH 3 of 3] Add premption handlers & properly wake sleeping guest

arch/powerpc/kvm/powerpc.c static void kvmppc_decrementer_func(unsi · line 219

Quoted 12 lines · On Friday 25 April 2008 00:56:04 Jerone Young wrote:

arch/powerpc/kvm/powerpc.c +3 -0

@@ -212,6 +212,9 @@ static void kvmppc_decrementer_func(unsi
 {
 	struct kvm_vcpu *vcpu = (struct kvm_vcpu *)data;
 
+	if (waitqueue_active(&vcpu->wq))
+		wake_up_interruptible(&vcpu->wq);
+
 	kvmppc_queue_exception(vcpu, BOOKE_INTERRUPT_DECREMENTER);
 }

Hooray!

arch/powerpc/kvm/powerpc.c static void kvmppc_decrementer_func(unsi · line 220

Quoted 1 line

arch/powerpc/kvm/powerpc.c +0 -0

 int kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu, struct kvm_interrupt

*irq)

arch/powerpc/kvm/powerpc.c int kvm_arch_vcpu_ioctl_run(struct kvm_v · line 382

Quoted 7 lines

{

arch/powerpc/kvm/powerpc.c +3 -0

+	vcpu_load(vcpu);
 	kvmppc_queue_exception(vcpu, BOOKE_INTERRUPT_EXTERNAL);
+	vcpu_put(vcpu);
+
 	return 0;
 }

load/put here is definitely unnecessary.

That makes me question how necessary it is in other parts of this patch too. I think the (hardware) TLB is the only state we really need to worry about, because there is no other state that our guest can load into the hardware that is not handled by a regular context switch.

If that's true, we should only need vcpu_load/put() on paths where we muck with the TLB behind the host's back, and that is only in the run path.

Signature · 8 lines

Hollis Blanchard
IBM Linux Technology Center

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Hollis Blanchard

2008-04-25 13:56 UTC #3 lore raw

Re: [kvm-ppc-devel] [PATCH 2 of 3] Add PowerPC KVM guest wait handling support

In reply to Jerone Young

Quoted 1 line · On Friday 25 April 2008 00:56:03 Jerone Young wrote:

This patch handles a guest that is in a wait state. This ensures that the

guest is not allways eating up 100% cpu when it is idle.

Quoted 8 lines

Signed-off-by Jerone Young <[email protected]>

arch/powerpc/kvm/emulate.c +0 -0

@@ -265,6 +265,11 @@ int kvmppc_emulate_instruction(struct kv
 		case 146:                                       /* mtmsr */

arch/powerpc/kvm/emulate.c int kvmppc_emulate_instruction(struct kv · line 275

Quoted 10 lines

arch/powerpc/kvm/emulate.c +5 -0

 			rs = get_rs(inst);
 			kvmppc_set_msr(vcpu, vcpu->arch.gpr[rs]);
+			
+			/* handle guest vcpu that is in wait state */
+			if (vcpu->arch.msr & MSR_WE) {
+				kvm_vcpu_block(vcpu);
+			}
 			break;
 
 		case 163:                                       /* wrteei */

So if I apply this patch and not #3, the guest will put itself to sleep and never wake up? You need to combine patches 2 and 3.

Also, for completeness, you should add the same test to the rfi emulation, which could (theoretically) also set MSR[WE].

Signature · 8 lines

Hollis Blanchard
IBM Linux Technology Center

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Hollis Blanchard

2008-04-25 14:00 UTC #6 lore raw

In reply to Jerone Young

Quoted 1 line · On Friday 25 April 2008 00:56:01 Jerone Young wrote:

This set of patches fixes 100% CPU usage when a guest is idle on PowerPC.

This time it uses common kvm functions to sleep the guest.

Looking much better now, with just a few minor issues to correct. With these patches applied, about how much CPU *does* an idling guest consume?

By the way, you don't explicitly *unset* MSR[WE]. I think this works implicitly because of the way we deliver interrupts; could you add a comment explaining that?

Signature · 8 lines

Hollis Blanchard
IBM Linux Technology Center

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Jerone Young

author

2008-04-25 16:34 UTC #7 lore raw

Quoted 6 lines · Hollis Blanchard

Quoted 1 line · On Friday 25 April 2008 00:56:01 Jerone Young wrote:
This set of patches fixes 100% CPU usage when a guest is idle on PowerPC.
This time it uses common kvm functions to sleep the guest.
Looking much better now, with just a few minor issues to correct. With these patches applied, about how much CPU *does* an idling guest consume?

With the current patch *as is* idle guest are eating about 16% CPU. Better then 100%, but more then the other patch. I'll see if by removing the vcpu_loads & vcpu_puts if that goes down.

Quoted 4 lines

By the way, you don't explicitly *unset* MSR[WE]. I think this works implicitly because of the way we deliver interrupts; could you add a comment explaining that?

Yes it is unset implicity. I can add a comment on this.

Quoted 1 line

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone