kvm-ppc-devel — The kvm PowerPC port

Re: [kvm-ppc-devel] [kvm-devel] [PATCH 1 of 4] Add "make sync" to sync need kernel headers to kvm-userspace

[Anthony L. <an...@co...>]

Jerone Young wrote:
> 2 files changed, 25 insertions(+), 4 deletions(-)
> Makefile  |   21 ++++++++++++++++++++-
> configure |    8 +++++---
>
>
> This patch adds ability for kvm-userspace build system to sync needed kernel headers locally without the need of compiled kernel source.
>   

make sync in kernel/ already does this.  Why not just add PPC support there.

Regards,

Anthony Liguori

> Signed-off-by: Jerone Young <jy...@us...>
>
> diff --git a/Makefile b/Makefile
> --- a/Makefile
> +++ b/Makefile
> @@ -7,7 +7,7 @@ rpmrelease = devel
>  
>  .PHONY: kernel user libkvm qemu bios vgabios extboot clean libfdt
>  
> -all: libkvm qemu
> +all: sync libkvm qemu
>  ifneq '$(filter $(ARCH), x86_64 i386 ia64)' ''
>      all: $(if $(WANT_MODULE), kernel) user
>  endif
> @@ -69,6 +69,24 @@ install:
>  	make -C libkvm DESTDIR="$(DESTDIR)" install
>  	make -C qemu DESTDIR="$(DESTDIR)" install
>  
> +
> +ASM_DIR=$(ARCH)
> +ifneq '$(filter $(ARCH), i386 x86_64)' ''
> +	ASM_DIR=x86
> +endif
> +
> +sync:
> +	mkdir -p $(INCLUDES_DIR)
> +	mkdir -p $(INCLUDES_DIR)/asm-$(ASM_DIR)
> +	mkdir -p $(INCLUDES_DIR)/linux
> +	cp -f $(KERNELDIR)/include/asm-$(ASM_DIR)/kvm*.h \
> +		$(INCLUDES_DIR)/asm-$(ASM_DIR)/
> +	cp -f $(KERNELDIR)/include/linux/kvm*.h \
> +		$(KERNELDIR)/include/linux/compiler*.h \
> +		$(INCLUDES_DIR)/linux
> +	ln -sf  $(INCLUDES_DIR)/asm-$(ASM_DIR) $(INCLUDES_DIR)/asm
> +
> +
>  tmpspec = .tmp.kvm.spec
>  RPMTOPDIR = $$(pwd)/rpmtop
>  
> @@ -99,3 +117,4 @@ clean:
>  
>  distclean: clean
>  	rm -f config.mak user/config.mak
> +	rm -rf $(INCLUDES_DIR)
> diff --git a/configure b/configure
> --- a/configure
> +++ b/configure
> @@ -10,6 +10,7 @@ cross_prefix=
>  cross_prefix=
>  arch=`uname -m`
>  target_exec=
> +local_kernel_includes_dir=$PWD/includes
>  
>  usage() {
>      cat <<-EOF
> @@ -108,16 +109,16 @@ fi
>  fi
>  
>  #configure user dir
> -(cd user; ./configure --prefix="$prefix" --kerneldir="$libkvm_kerneldir" \
> +(cd user; ./configure --prefix="$prefix" \
>            --arch="$arch" \
>            ${cross_prefix:+"--cross-prefix=$cross_prefix"})
>  
>  #configure qemu
>  (cd qemu; ./configure --target-list=$target_exec \
>      --disable-kqemu \
> -    --extra-cflags="-I $PWD/../libkvm $qemu_cflags" \
> +    --extra-cflags="-I $PWD/../libkvm $qemu_cflags \
> +               -I $local_kernel_includes_dir" \
>      --extra-ldflags="-L $PWD/../libkvm $qemu_ldflags" \
> -    --kernel-path="$libkvm_kerneldir" \
>      --prefix="$prefix" \
>      ${qemu_cc:+"--cc=$qemu_cc"} \
>      ${cross_prefix:+"--cross-prefix=$cross_prefix"} \
> @@ -131,4 +132,5 @@ KERNELDIR=$kerneldir
>  KERNELDIR=$kerneldir
>  WANT_MODULE=$want_module
>  CROSS_COMPILE=$cross_prefix
> +INCLUDES_DIR=$local_kernel_includes_dir
>  EOF
>
> -------------------------------------------------------------------------
> 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
> _______________________________________________
> kvm-devel mailing list
> kvm...@li...
> https://lists.sourceforge.net/lists/listinfo/kvm-devel
>   

[kvm-ppc-devel] Apps to read & process kvmppc instruction trace data

[Christian E. <ehr...@li...>]

Based on Hollis tlb tracing from http://penguinppc.org/~hollisb/kvm/ this mail contains a c file that can be used to read the instruction traces into a file (binary) and a python script that processes these binary files to a readable table.
@Hollis we might think of merging these and possible following tools of that kind and/or strive to add is to kvm-userspace - what do you think ?

To use the instruction relay stuff, on the host:
	# First build a kernel with "ppc440 instruction emulation tracing" enabled as host kernel
	# then run the guest workload while reading the relay file
        % gcc -Wall -O2 44x_instr.c -o 44x_instr
        % ./44x_instr > instrlog.bin & # can also be invoked later e.g only around some guest workload part
        % qemu-system-ppcemb ...
	% kill the 44x_instr command

        % python decode_instr.py instrlog.bin # can be run on any system

The python script decodes extra informations for some opcodes e.g. sprn for m[ft]spr
An example line:
         time:      instr @         pc          rsval          raval          rbval mnemonic - context info
  0.892007422:   7c700aa6 @         1c          0              0              0        mfspr - sprn 0x030      PID
  0.892007422:   7c8000a6 @         20          0              0              0        mfmsr
  0.892007422:   7c72eba6 @         30          0              0              0        mtspr - sprn 0x3b2    MMUCR
  0.892007422:   7ee02f24 @         40          0              0             3c@r05    tlbsx
  0.892007422:   7c6407a4 @         54          0              1@r04          0        tlbwe - ws ->   PAGEID

- Time helps to find relations what the guest did in that moment
- instruction is the full opcode
- pc the program counter helping developers to analyze where that instr came from e.g. check guest image with objdump and look for these addresses
- r*val - the value in the used register, additionally the number of the used register is printed e.g. 1@r04 means r4 is used in that instruction and contains "1"
- mnemonic is the name of that instruction
- context info decodes special information for some instruction types (can be extended as needed)
	- sprn for m[ft]spr
	- spr name for m[ft]spr
	- dcrn for m[ft]dcr
	- tlbwe action type encoded in "ws"

-- 

Grüsse / regards, 
Christian Ehrhardt
IBM Linux Technology Center, Open Virtualization

Re: [kvm-ppc-devel] First detailed look at instruction emulation using kvm_stat instruction extension

[Christian E. <ehr...@li...>]

Christian Ehrhardt wrote:
> We all saw in prior kvm_stat counters that listed the different exit 
> reasons the high count of instruction emulation. The recent patch to 
> kvm-ppc-devel splits up those into the different instructions. I 
> attached a log of a simple guest boot&halt FYI. E.g. see the high number 
> of m[ft]msr instructions.
[...]
sorry for that type - I meants m[ft]spr activity.
The full instruction tracing allows us to check e.g. which sprn's are accessed, in which sequences they are accessed, with which valued and from which pc of the guest kernel.

I still don't fully trust these tracing or the kvm_stat extension because I get dfferent numbers of instructions (but the ratio between them look the same, so I think it's valid to look at the results). I expect that I increment by more than 1 in the kvm_stat instr patches atm - comments welcome (see the 1/3 in the patch series just posted to kvm-ppc-devel).

I attached only a 2000 line partial log of the later part of the guest execution, because the full log is very big (3.9M).
I can send a full log to everyone interested - please send me a mail (hmmm... maybe I upload that somewhere).

A short check of the ratio between the different sprn's used gave that listing:
grep sprn boot_halt_v4.instr.log | awk '{ printf("%s %s\n",$11,$12) }' | sort | uniq -c| sort -rn
   2461 0x01b SRR1
   2256 0x110 SPRG0
   2245 0x111 SPRG1
   2004 0x01a SRR0
   1538 0x113 SPRG3
   1127 0x114 SPRG4
   1034 0x3b2 MMUCR
    979 0x016 DEC
    597 0x030 PID
    509 0x115 SPRG5
    499 0x117 SPRG7
    475 0x150 TSR
    358 0x03d DEAR
    106 0x03e ESR
      3 0x11f PVR
      1 0x19f IVOR15
      1 0x19e IVOR14
      1 0x19d IVOR13
      1 0x19c IVOR12
      1 0x19b IVOR11
      1 0x19a IVOR10
      1 0x199 IVOR9
      1 0x198 IVOR8
      1 0x197 IVOR7
      1 0x196 IVOR6
      1 0x195 IVOR5
      1 0x194 IVOR4
      1 0x193 IVOR3
      1 0x192 IVOR2
      1 0x191 IVOR1
      1 0x190 IVOR0
      1 0x03f IVPR

While the instruction statistics for all instructions in that log is:
$awk '{ printf("%s\n",$8) }' boot_halt_v4.instr.log | sort | uniq -c | sort -rn
   9157 mtspr
   7494 wrteei
   7051 mfspr
   6073 mfmsr
   4532 wrtee
   1693 rfi
   1647 tlbwe
    943 mtmsr
     89 lbz
     84 stb
     81 tlbsx
     22 mfdcr
     21 lbzx
     20 mtdcr
      7 stbx
      3 sthbrx
      3 iccci


P.S. FYI - atm I think of adding such summary reports as optional part to the post processing script either packing most of the function into awk commands or by extending the python code.

-- 

Grüsse / regards, 
Christian Ehrhardt
IBM Linux Technology Center, Open Virtualization

[kvm-ppc-devel] [PATCH] [1/3] kvmppc: add detailed instruction emulation statistics

[<ehr...@li...>]

From: Christian Ehrhardt <ehr...@li...>

This extends the kvm_stat reports for kvm on embedded powerpc. Since kvmppc is
using emulation (no hardware support yet) this gives people interested in
performance a detailed split of the emul_instruction counter already available.
This statistic does not cover e.g. operants of the commands, but that way it
should have only a small perf impact (never break what you want to measure).
This feature is configurable in .config under the kvmppc virtualization itself.

Signed-off-by: Christian Ehrhardt <ehr...@li...>
---

[diffstat]
 arch/powerpc/kvm/Kconfig       |    8 +++
 arch/powerpc/kvm/emulate.c     |   61 +++++++++++++++++++++++++++++
 arch/powerpc/kvm/powerpc.c     |   84 +++++++++++++++++++++++++++++++++++++++++
 include/asm-powerpc/kvm_host.h |   40 +++++++++++++++++++
 include/asm-powerpc/kvm_ppc.h  |   50 ++++++++++++++++++++++++
 5 files changed, 243 insertions(+)

diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig
--- a/arch/powerpc/kvm/Kconfig
+++ b/arch/powerpc/kvm/Kconfig
@@ -41,6 +41,14 @@ config KVM_POWERPC_440
 	---help---
 	  Provides support for KVM on 440 processors.
 
+config KVM_POWERPC_440_INSTRUCTION_STAT
+	bool "ppc440 instruction emulation statistics"
+	depends on KVM && 44x && KVM_POWERPC_440
+	---help---
+	  Adds the tracking of the different emulated instructions. This is
+	  used to debug performance issues and adds a slight runtime overhead.
+	  If unsure, say N.
+
 config KVM_PPC_VIRTIO
        bool "Virtio Support"
        select VIRTIO
diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
--- a/arch/powerpc/kvm/emulate.c
+++ b/arch/powerpc/kvm/emulate.c
@@ -87,6 +87,14 @@ static inline unsigned int get_d(u32 ins
 static inline unsigned int get_d(u32 inst)
 {
 	return inst & 0xffff;
+}
+
+static inline void emulinstr_stat(struct kvm_vcpu *vcpu,
+			enum kvmppc_emulated_instructions kvmppc_emulinstr)
+{
+#ifdef CONFIG_KVM_POWERPC_440_INSTRUCTION_STAT
+	(*(u32 *)((void *)vcpu + kvmppc_emulinstr_offset[kvmppc_emulinstr]))++;
+#endif
 }
 
 static int tlbe_is_host_safe(const struct kvm_vcpu *vcpu,
@@ -229,6 +237,7 @@ int kvmppc_emulate_instruction(struct kv
 
 	switch (get_op(inst)) {
 	case 3:                                                 /* trap */
+		emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_TRAP);
 		if (get_d(inst) == 1) {
 			/* FIXME port to final hypercall API when defined */
 			printk(KERN_INFO"Guest requested shutdown\n");
@@ -242,6 +251,7 @@ int kvmppc_emulate_instruction(struct kv
 	case 19:
 		switch (get_xop(inst)) {
 		case 50:                                        /* rfi */
+			emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_RFI);
 			kvmppc_emul_rfi(vcpu);
 			advance = 0;
 			break;
@@ -256,32 +266,39 @@ int kvmppc_emulate_instruction(struct kv
 		switch (get_xop(inst)) {
 
 		case 83:                                        /* mfmsr */
+			emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_MFMSR);
 			rt = get_rt(inst);
 			vcpu->arch.gpr[rt] = vcpu->arch.msr;
 			break;
 
 		case 87:                                        /* lbzx */
+			emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_LBZX);
 			rt = get_rt(inst);
 			emulated = kvmppc_handle_load(run, vcpu, rt, 1, 1);
 			break;
 
 		case 131:                                       /* wrtee */
+			emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_WRTEE);
 			rs = get_rs(inst);
 			vcpu->arch.msr = (vcpu->arch.msr & ~MSR_EE)
 			                 | (vcpu->arch.gpr[rs] & MSR_EE);
 			break;
 
 		case 146:                                       /* mtmsr */
+			emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_MTMSR);
 			rs = get_rs(inst);
 			kvmppc_set_msr(vcpu, vcpu->arch.gpr[rs]);
 			break;
 
 		case 163:                                       /* wrteei */
+			emulinstr_stat(vcpu,
+					KVMPPC_EMULATED_INSTRUCTION_WRTEEI);
 			vcpu->arch.msr = (vcpu->arch.msr & ~MSR_EE)
 			                 | (inst & MSR_EE);
 			break;
 
 		case 215:                                       /* stbx */
+			emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_STBX);
 			rs = get_rs(inst);
 			emulated = kvmppc_handle_store(run, vcpu,
 			                               vcpu->arch.gpr[rs],
@@ -289,6 +306,7 @@ int kvmppc_emulate_instruction(struct kv
 			break;
 
 		case 247:                                       /* stbux */
+			emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_STBUX);
 			rs = get_rs(inst);
 			ra = get_ra(inst);
 			rb = get_rb(inst);
@@ -304,11 +322,13 @@ int kvmppc_emulate_instruction(struct kv
 			break;
 
 		case 279:                                       /* lhzx */
+			emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_LHZX);
 			rt = get_rt(inst);
 			emulated = kvmppc_handle_load(run, vcpu, rt, 2, 1);
 			break;
 
 		case 311:                                       /* lhzux */
+			emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_LHZUX);
 			rt = get_rt(inst);
 			ra = get_ra(inst);
 			rb = get_rb(inst);
@@ -322,6 +342,7 @@ int kvmppc_emulate_instruction(struct kv
 			break;
 
 		case 323:                                       /* mfdcr */
+			emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_MFDCR);
 			dcrn = get_dcrn(inst);
 			rt = get_rt(inst);
 
@@ -349,6 +370,7 @@ int kvmppc_emulate_instruction(struct kv
 			break;
 
 		case 339:                                       /* mfspr */
+			emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_MFSPR);
 			sprn = get_sprn(inst);
 			rt = get_rt(inst);
 
@@ -438,6 +460,7 @@ int kvmppc_emulate_instruction(struct kv
 			break;
 
 		case 407:                                       /* sthx */
+			emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_STHX);
 			rs = get_rs(inst);
 			ra = get_ra(inst);
 			rb = get_rb(inst);
@@ -448,6 +471,7 @@ int kvmppc_emulate_instruction(struct kv
 			break;
 
 		case 439:                                       /* sthux */
+			emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_STHUX);
 			rs = get_rs(inst);
 			ra = get_ra(inst);
 			rb = get_rb(inst);
@@ -463,6 +487,7 @@ int kvmppc_emulate_instruction(struct kv
 			break;
 
 		case 451:                                       /* mtdcr */
+			emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_MTDCR);
 			dcrn = get_dcrn(inst);
 			rs = get_rs(inst);
 
@@ -482,6 +507,7 @@ int kvmppc_emulate_instruction(struct kv
 			break;
 
 		case 467:                                       /* mtspr */
+			emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_MTSPR);
 			sprn = get_sprn(inst);
 			rs = get_rs(inst);
 			switch (sprn) {
@@ -588,6 +614,7 @@ int kvmppc_emulate_instruction(struct kv
 			break;
 
 		case 470:                                       /* dcbi */
+			emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_DCBI);
 			/* Do nothing. The guest is performing dcbi because
 			 * hardware DMA is not snooped by the dcache, but
 			 * emulated DMA either goes through the dcache as
@@ -596,14 +623,19 @@ int kvmppc_emulate_instruction(struct kv
 			break;
 
 		case 534:                                       /* lwbrx */
+			emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_LWBRX);
 			rt = get_rt(inst);
 			emulated = kvmppc_handle_load(run, vcpu, rt, 4, 0);
 			break;
 
 		case 566:                                       /* tlbsync */
+			emulinstr_stat(vcpu,
+					KVMPPC_EMULATED_INSTRUCTION_TLBSYNC);
 			break;
 
 		case 662:                                       /* stwbrx */
+			emulinstr_stat(vcpu,
+					KVMPPC_EMULATED_INSTRUCTION_STWBRX);
 			rs = get_rs(inst);
 			ra = get_ra(inst);
 			rb = get_rb(inst);
@@ -614,6 +646,7 @@ int kvmppc_emulate_instruction(struct kv
 			break;
 
 		case 978:                                       /* tlbwe */
+			emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_TLBWE);
 			emulated = kvmppc_emul_tlbwe(vcpu, inst);
 			break;
 
@@ -621,6 +654,7 @@ int kvmppc_emulate_instruction(struct kv
 			int index;
 			unsigned int as = get_mmucr_sts(vcpu);
 			unsigned int pid = get_mmucr_stid(vcpu);
+			emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_TLBSX);
 
 			rt = get_rt(inst);
 			ra = get_ra(inst);
@@ -644,11 +678,14 @@ int kvmppc_emulate_instruction(struct kv
 			break;
 
 		case 790:                                       /* lhbrx */
+			emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_LHBRX);
 			rt = get_rt(inst);
 			emulated = kvmppc_handle_load(run, vcpu, rt, 2, 0);
 			break;
 
 		case 918:                                       /* sthbrx */
+			emulinstr_stat(vcpu,
+					KVMPPC_EMULATED_INSTRUCTION_STHBRX);
 			rs = get_rs(inst);
 			ra = get_ra(inst);
 			rb = get_rb(inst);
@@ -659,6 +696,7 @@ int kvmppc_emulate_instruction(struct kv
 			break;
 
 		case 966:                                       /* iccci */
+			emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_ICCCI);
 			break;
 
 		default:
@@ -670,11 +708,14 @@ int kvmppc_emulate_instruction(struct kv
 		break;
 
 	case 32:                                                /* lwz */
+		emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_LWZ);
 		rt = get_rt(inst);
 		emulated = kvmppc_handle_load(run, vcpu, rt, 4, 1);
 		break;
 
 	case 33:                                                /* lwzu */
+		emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_LWZU);
+		rt = get_rt(inst);
 		ra = get_ra(inst);
 		rt = get_rt(inst);
 		emulated = kvmppc_handle_load(run, vcpu, rt, 4, 1);
@@ -682,11 +723,15 @@ int kvmppc_emulate_instruction(struct kv
 		break;
 
 	case 34:                                                /* lbz */
+		emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_LBZ);
+		rt = get_rt(inst);
 		rt = get_rt(inst);
 		emulated = kvmppc_handle_load(run, vcpu, rt, 1, 1);
 		break;
 
 	case 35:                                                /* lbzu */
+		emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_LBZU);
+		rt = get_rt(inst);
 		ra = get_ra(inst);
 		rt = get_rt(inst);
 		emulated = kvmppc_handle_load(run, vcpu, rt, 1, 1);
@@ -694,12 +739,16 @@ int kvmppc_emulate_instruction(struct kv
 		break;
 
 	case 36:                                                /* stw */
+		emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_STW);
+		rt = get_rt(inst);
 		rs = get_rs(inst);
 		emulated = kvmppc_handle_store(run, vcpu, vcpu->arch.gpr[rs],
 		                               4, 1);
 		break;
 
 	case 37:                                                /* stwu */
+		emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_STWU);
+		rt = get_rt(inst);
 		ra = get_ra(inst);
 		rs = get_rs(inst);
 		emulated = kvmppc_handle_store(run, vcpu, vcpu->arch.gpr[rs],
@@ -708,12 +757,16 @@ int kvmppc_emulate_instruction(struct kv
 		break;
 
 	case 38:                                                /* stb */
+		emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_STB);
+		rt = get_rt(inst);
 		rs = get_rs(inst);
 		emulated = kvmppc_handle_store(run, vcpu, vcpu->arch.gpr[rs],
 		                               1, 1);
 		break;
 
 	case 39:                                                /* stbu */
+		emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_STBU);
+		rt = get_rt(inst);
 		ra = get_ra(inst);
 		rs = get_rs(inst);
 		emulated = kvmppc_handle_store(run, vcpu, vcpu->arch.gpr[rs],
@@ -722,11 +775,15 @@ int kvmppc_emulate_instruction(struct kv
 		break;
 
 	case 40:                                                /* lhz */
+		emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_LHZ);
+		rt = get_rt(inst);
 		rt = get_rt(inst);
 		emulated = kvmppc_handle_load(run, vcpu, rt, 2, 1);
 		break;
 
 	case 41:                                                /* lhzu */
+		emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_LHZU);
+		rt = get_rt(inst);
 		ra = get_ra(inst);
 		rt = get_rt(inst);
 		emulated = kvmppc_handle_load(run, vcpu, rt, 2, 1);
@@ -734,12 +791,16 @@ int kvmppc_emulate_instruction(struct kv
 		break;
 
 	case 44:                                                /* sth */
+		emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_STH);
+		rt = get_rt(inst);
 		rs = get_rs(inst);
 		emulated = kvmppc_handle_store(run, vcpu, vcpu->arch.gpr[rs],
 		                               2, 1);
 		break;
 
 	case 45:                                                /* sthu */
+		emulinstr_stat(vcpu, KVMPPC_EMULATED_INSTRUCTION_STHU);
+		rt = get_rt(inst);
 		ra = get_ra(inst);
 		rs = get_rs(inst);
 		emulated = kvmppc_handle_store(run, vcpu, vcpu->arch.gpr[rs],
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -56,8 +56,92 @@ struct kvm_stats_debugfs_item debugfs_en
 	{ "inst_emu",      VCPU_STAT(emulated_inst_exits) },
 	{ "dec",      VCPU_STAT(dec_exits) },
 	{ "ext_intr",      VCPU_STAT(ext_intr_exits) },
+#ifdef CONFIG_KVM_POWERPC_440_INSTRUCTION_STAT
+	{ "instr_trap", VCPU_STAT(instr_trap) },
+	{ "instr_rfi", VCPU_STAT(instr_rfi) },
+	{ "instr_mfmsr", VCPU_STAT(instr_mfmsr) },
+	{ "instr_lbzx", VCPU_STAT(instr_lbzx) },
+	{ "instr_wrtee", VCPU_STAT(instr_wrtee) },
+	{ "instr_mtmsr", VCPU_STAT(instr_mtmsr) },
+	{ "instr_wrteei", VCPU_STAT(instr_wrteei) },
+	{ "instr_stbx", VCPU_STAT(instr_stbx) },
+	{ "instr_stbux", VCPU_STAT(instr_stbux) },
+	{ "instr_lhzx", VCPU_STAT(instr_lhzx) },
+	{ "instr_lhzux", VCPU_STAT(instr_lhzux) },
+	{ "instr_mfdcr", VCPU_STAT(instr_mfdcr) },
+	{ "instr_mfspr", VCPU_STAT(instr_mfspr) },
+	{ "instr_sthx", VCPU_STAT(instr_sthx) },
+	{ "instr_sthux", VCPU_STAT(instr_sthux) },
+	{ "instr_mtdcr", VCPU_STAT(instr_mtdcr) },
+	{ "instr_mtspr", VCPU_STAT(instr_mtspr) },
+	{ "instr_dcbi", VCPU_STAT(instr_dcbi) },
+	{ "instr_lwbrx", VCPU_STAT(instr_lwbrx) },
+	{ "instr_tlbsync", VCPU_STAT(instr_tlbsync) },
+	{ "instr_stwbrx", VCPU_STAT(instr_stwbrx) },
+	{ "instr_tlbwe", VCPU_STAT(instr_tlbwe) },
+	{ "instr_tlbsx", VCPU_STAT(instr_tlbsx) },
+	{ "instr_lhbrx", VCPU_STAT(instr_lhbrx) },
+	{ "instr_sthbrx", VCPU_STAT(instr_sthbrx) },
+	{ "instr_iccci", VCPU_STAT(instr_iccci) },
+	{ "instr_lwz", VCPU_STAT(instr_lwz) },
+	{ "instr_lwzu", VCPU_STAT(instr_lwzu) },
+	{ "instr_lbz", VCPU_STAT(instr_lbz) },
+	{ "instr_lbzu", VCPU_STAT(instr_lbzu) },
+	{ "instr_stw", VCPU_STAT(instr_stw) },
+	{ "instr_stwu", VCPU_STAT(instr_stwu) },
+	{ "instr_stb", VCPU_STAT(instr_stb) },
+	{ "instr_stbu", VCPU_STAT(instr_stbu) },
+	{ "instr_lhz", VCPU_STAT(instr_lhz) },
+	{ "instr_lhzu", VCPU_STAT(instr_lhzu) },
+	{ "instr_sth", VCPU_STAT(instr_sth) },
+	{ "instr_sthu", VCPU_STAT(instr_sthu) },
+#endif /* CONFIG_KVM_POWERPC_440_INSTRUCTION_STAT */
 	{ NULL }
 };
+
+#ifdef CONFIG_KVM_POWERPC_440_INSTRUCTION_STAT
+#define INST_STAT_OFFSET(x) offsetof(struct kvm_vcpu, stat.x)
+const int kvmppc_emulinstr_offset[] = {
+	[KVMPPC_EMULATED_INSTRUCTION_TRAP]    = INST_STAT_OFFSET(instr_trap),
+	[KVMPPC_EMULATED_INSTRUCTION_RFI]     = INST_STAT_OFFSET(instr_rfi),
+	[KVMPPC_EMULATED_INSTRUCTION_MFMSR]   = INST_STAT_OFFSET(instr_mfmsr),
+	[KVMPPC_EMULATED_INSTRUCTION_LBZX]    = INST_STAT_OFFSET(instr_lbzx),
+	[KVMPPC_EMULATED_INSTRUCTION_WRTEE]   = INST_STAT_OFFSET(instr_wrtee),
+	[KVMPPC_EMULATED_INSTRUCTION_MTMSR]   = INST_STAT_OFFSET(instr_mtmsr),
+	[KVMPPC_EMULATED_INSTRUCTION_WRTEEI]  = INST_STAT_OFFSET(instr_wrteei),
+	[KVMPPC_EMULATED_INSTRUCTION_STBX]    = INST_STAT_OFFSET(instr_stbx),
+	[KVMPPC_EMULATED_INSTRUCTION_STBUX]   = INST_STAT_OFFSET(instr_stbux),
+	[KVMPPC_EMULATED_INSTRUCTION_LHZX]    = INST_STAT_OFFSET(instr_lhzx),
+	[KVMPPC_EMULATED_INSTRUCTION_LHZUX]   = INST_STAT_OFFSET(instr_lhzux),
+	[KVMPPC_EMULATED_INSTRUCTION_MFDCR]   = INST_STAT_OFFSET(instr_mfdcr),
+	[KVMPPC_EMULATED_INSTRUCTION_MFSPR]   = INST_STAT_OFFSET(instr_mfspr),
+	[KVMPPC_EMULATED_INSTRUCTION_STHX]    = INST_STAT_OFFSET(instr_sthx),
+	[KVMPPC_EMULATED_INSTRUCTION_STHUX]   = INST_STAT_OFFSET(instr_sthux),
+	[KVMPPC_EMULATED_INSTRUCTION_MTDCR]   = INST_STAT_OFFSET(instr_mtdcr),
+	[KVMPPC_EMULATED_INSTRUCTION_MTSPR]   = INST_STAT_OFFSET(instr_mtspr),
+	[KVMPPC_EMULATED_INSTRUCTION_DCBI]    = INST_STAT_OFFSET(instr_dcbi),
+	[KVMPPC_EMULATED_INSTRUCTION_LWBRX]   = INST_STAT_OFFSET(instr_lwbrx),
+	[KVMPPC_EMULATED_INSTRUCTION_TLBSYNC] = INST_STAT_OFFSET(instr_tlbsync),
+	[KVMPPC_EMULATED_INSTRUCTION_STWBRX]  = INST_STAT_OFFSET(instr_stwbrx),
+	[KVMPPC_EMULATED_INSTRUCTION_TLBWE]   = INST_STAT_OFFSET(instr_tlbwe),
+	[KVMPPC_EMULATED_INSTRUCTION_TLBSX]   = INST_STAT_OFFSET(instr_tlbsx),
+	[KVMPPC_EMULATED_INSTRUCTION_LHBRX]   = INST_STAT_OFFSET(instr_lhbrx),
+	[KVMPPC_EMULATED_INSTRUCTION_STHBRX]  = INST_STAT_OFFSET(instr_sthbrx),
+	[KVMPPC_EMULATED_INSTRUCTION_ICCCI]   = INST_STAT_OFFSET(instr_iccci),
+	[KVMPPC_EMULATED_INSTRUCTION_LWZ]     = INST_STAT_OFFSET(instr_lwz),
+	[KVMPPC_EMULATED_INSTRUCTION_LWZU]    = INST_STAT_OFFSET(instr_lwzu),
+	[KVMPPC_EMULATED_INSTRUCTION_LBZ]     = INST_STAT_OFFSET(instr_lbz),
+	[KVMPPC_EMULATED_INSTRUCTION_LBZU]    = INST_STAT_OFFSET(instr_lbzu),
+	[KVMPPC_EMULATED_INSTRUCTION_STW]     = INST_STAT_OFFSET(instr_stw),
+	[KVMPPC_EMULATED_INSTRUCTION_STWU]    = INST_STAT_OFFSET(instr_stwu),
+	[KVMPPC_EMULATED_INSTRUCTION_STB]     = INST_STAT_OFFSET(instr_stb),
+	[KVMPPC_EMULATED_INSTRUCTION_STBU]    = INST_STAT_OFFSET(instr_stbu),
+	[KVMPPC_EMULATED_INSTRUCTION_LHZ]     = INST_STAT_OFFSET(instr_lhz),
+	[KVMPPC_EMULATED_INSTRUCTION_LHZU]    = INST_STAT_OFFSET(instr_lhzu),
+	[KVMPPC_EMULATED_INSTRUCTION_STH]     = INST_STAT_OFFSET(instr_sth),
+	[KVMPPC_EMULATED_INSTRUCTION_STHU]    = INST_STAT_OFFSET(instr_sthu)
+};
+#endif /* CONFIG_KVM_POWERPC_440_INSTRUCTION_STAT */
 
 
 static int kvmppc_debug;
diff --git a/include/asm-powerpc/kvm_host.h b/include/asm-powerpc/kvm_host.h
--- a/include/asm-powerpc/kvm_host.h
+++ b/include/asm-powerpc/kvm_host.h
@@ -59,6 +59,46 @@ struct kvm_vcpu_stat {
 	u32 emulated_inst_exits;
 	u32 dec_exits;
 	u32 ext_intr_exits;
+#ifdef CONFIG_KVM_POWERPC_440_INSTRUCTION_STAT
+	u32 instr_trap;
+	u32 instr_rfi;
+	u32 instr_mfmsr;
+	u32 instr_lbzx;
+	u32 instr_wrtee;
+	u32 instr_mtmsr;
+	u32 instr_wrteei;
+	u32 instr_stbx;
+	u32 instr_stbux;
+	u32 instr_lhzx;
+	u32 instr_lhzux;
+	u32 instr_mfdcr;
+	u32 instr_mfspr;
+	u32 instr_sthx;
+	u32 instr_sthux;
+	u32 instr_mtdcr;
+	u32 instr_mtspr;
+	u32 instr_dcbi;
+	u32 instr_lwbrx;
+	u32 instr_tlbsync;
+	u32 instr_stwbrx;
+	u32 instr_tlbwe;
+	u32 instr_tlbsx;
+	u32 instr_lhbrx;
+	u32 instr_sthbrx;
+	u32 instr_iccci;
+	u32 instr_lwz;
+	u32 instr_lwzu;
+	u32 instr_lbz;
+	u32 instr_lbzu;
+	u32 instr_stw;
+	u32 instr_stwu;
+	u32 instr_stb;
+	u32 instr_stbu;
+	u32 instr_lhz;
+	u32 instr_lhzu;
+	u32 instr_sth;
+	u32 instr_sthu;
+#endif /* CONFIG_KVM_POWERPC_440_INSTRUCTION_STAT */
 };
 
 struct tlbe {
diff --git a/include/asm-powerpc/kvm_ppc.h b/include/asm-powerpc/kvm_ppc.h
--- a/include/asm-powerpc/kvm_ppc.h
+++ b/include/asm-powerpc/kvm_ppc.h
@@ -41,6 +41,56 @@ enum emulation_result {
 	EMULATE_FAIL,         /* can't emulate this instruction */
 	EMULATE_SHUTDOWN,     /* shutdown guest requested (no kvm_run data) */
 };
+
+enum kvmppc_emulated_instructions {
+	KVMPPC_EMULATED_INSTRUCTION_TRAP,
+	KVMPPC_EMULATED_INSTRUCTION_RFI,
+	KVMPPC_EMULATED_INSTRUCTION_MFMSR,
+	KVMPPC_EMULATED_INSTRUCTION_LBZX,
+	KVMPPC_EMULATED_INSTRUCTION_WRTEE,
+	KVMPPC_EMULATED_INSTRUCTION_MTMSR,
+	KVMPPC_EMULATED_INSTRUCTION_WRTEEI,
+	KVMPPC_EMULATED_INSTRUCTION_STBX,
+	KVMPPC_EMULATED_INSTRUCTION_STBUX,
+	KVMPPC_EMULATED_INSTRUCTION_LHZX,
+	KVMPPC_EMULATED_INSTRUCTION_LHZUX,
+	KVMPPC_EMULATED_INSTRUCTION_MFDCR,
+	KVMPPC_EMULATED_INSTRUCTION_MFSPR,
+	KVMPPC_EMULATED_INSTRUCTION_STHX,
+	KVMPPC_EMULATED_INSTRUCTION_STHUX,
+	KVMPPC_EMULATED_INSTRUCTION_MTDCR,
+	KVMPPC_EMULATED_INSTRUCTION_MTSPR,
+	KVMPPC_EMULATED_INSTRUCTION_DCBI,
+	KVMPPC_EMULATED_INSTRUCTION_LWBRX,
+	KVMPPC_EMULATED_INSTRUCTION_TLBSYNC,
+	KVMPPC_EMULATED_INSTRUCTION_STWBRX,
+	KVMPPC_EMULATED_INSTRUCTION_TLBWE,
+	KVMPPC_EMULATED_INSTRUCTION_TLBSX,
+	KVMPPC_EMULATED_INSTRUCTION_LHBRX,
+	KVMPPC_EMULATED_INSTRUCTION_STHBRX,
+	KVMPPC_EMULATED_INSTRUCTION_ICCCI,
+	KVMPPC_EMULATED_INSTRUCTION_LWZ,
+	KVMPPC_EMULATED_INSTRUCTION_LWZU,
+	KVMPPC_EMULATED_INSTRUCTION_LBZ,
+	KVMPPC_EMULATED_INSTRUCTION_LBZU,
+	KVMPPC_EMULATED_INSTRUCTION_STW,
+	KVMPPC_EMULATED_INSTRUCTION_STWU,
+	KVMPPC_EMULATED_INSTRUCTION_STB,
+	KVMPPC_EMULATED_INSTRUCTION_STBU,
+	KVMPPC_EMULATED_INSTRUCTION_LHZ,
+	KVMPPC_EMULATED_INSTRUCTION_LHZU,
+	KVMPPC_EMULATED_INSTRUCTION_STH,
+	KVMPPC_EMULATED_INSTRUCTION_STHU,
+};
+
+#ifdef CONFIG_KVM_POWERPC_440_INSTRUCTION_STAT
+/*
+ * offset definitions to map&update counters with low runtime overhead
+ * directly in vcpu->stat.x (no new op/xop -> counter mapping needed, this
+ * uses the already implemented switch/case in emulate_instruction).
+*/
+extern const int kvmppc_emulinstr_offset[];
+#endif /* CONFIG_KVM_POWERPC_440_INSTRUCTION_STAT */
 
 extern const unsigned char exception_priority[];
 extern const unsigned char priority_exception[];

[kvm-ppc-devel] [PATCH] [2/3] kvmppc: full instruction emulation tracing

[<ehr...@li...>]

From: Christian Ehrhardt <ehr...@li...>

This adds a relayfs based kernel interface that allows tracing of all emulated
guest instructions. It is based on Hollis tracing of tlb activities.
A suitable userspace program to read from that interface and a post processing
script that give users a readable output follow.
This feature is configurable in .config under the kvmppc virtualization itself.

Signed-off-by: Christian Ehrhardt <ehr...@li...>
---

[diffstat]
 Kconfig   |    9 ++++++
 emulate.c |   84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 powerpc.c |   11 ++++++++
 3 files changed, 104 insertions(+)

diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig
--- a/arch/powerpc/kvm/Kconfig
+++ b/arch/powerpc/kvm/Kconfig
@@ -49,6 +49,15 @@ config KVM_POWERPC_440_INSTRUCTION_STAT
 	  used to debug performance issues and adds a slight runtime overhead.
 	  If unsure, say N.
 
+config KVM_POWERPC_440_TRACE_INSTRUCTIONS
+	bool "ppc440 instruction emulation tracing"
+	depends on KVM && 44x && KVM_POWERPC_440
+	select RELAY
+	---help---
+	  Adds the complete tracing of the emulated instructions via a relayfs
+	  channel.
+	  If unsure, say N.
+
 config KVM_PPC_VIRTIO
        bool "Virtio Support"
        select VIRTIO
diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
--- a/arch/powerpc/kvm/emulate.c
+++ b/arch/powerpc/kvm/emulate.c
@@ -21,6 +21,7 @@
 #include <linux/timer.h>
 #include <linux/types.h>
 #include <linux/string.h>
+#include <linux/relay.h>
 #include <linux/kvm_host.h>
 
 #include <asm/dcr.h>
@@ -87,6 +88,85 @@ static inline unsigned int get_d(u32 ins
 static inline unsigned int get_d(u32 inst)
 {
 	return inst & 0xffff;
+}
+
+struct rchan *kvmppc_44x_instr_chan;
+
+struct instr_record {
+	u32 time;
+	u32 pc;
+	u32 instr;
+	u32 rsval;
+	u32 raval;
+	u32 rbval;
+};
+
+static void kvmppc_44x_instr_trace(const struct kvm_vcpu *vcpu)
+{
+	u32 inst = vcpu->arch.last_inst;
+	struct instr_record record = {
+		.time = current_kernel_time().tv_nsec,
+		.pc = vcpu->arch.pc,
+		.instr = vcpu->arch.last_inst,
+		.rsval = 0,
+		.raval = 0,
+		.rbval = 0,
+	};
+
+	switch (get_op(inst)) {
+	case 31:
+		switch (get_xop(inst)) {
+		case 131:                                       /* wrtee */
+		case 146:                                       /* mtmsr */
+		case 451:                                       /* mtdcr */
+		case 467:                                       /* mtspr */
+			record.rsval = vcpu->arch.gpr[get_rs(inst)];
+			break;
+		case 279:                                       /* lhzx */
+		case 534:                                       /* lwbrx */
+		case 790:                                       /* lhbrx */
+			record.raval = vcpu->arch.gpr[get_ra(inst)];
+			break;
+		case 311:                                       /* lhzux */
+		case 978:                                       /* tlbwe */
+		case 914:                                       /* tlbsx */
+			record.raval = vcpu->arch.gpr[get_ra(inst)];
+			record.rbval = vcpu->arch.gpr[get_rb(inst)];
+			break;
+		case 215:                                       /* stbx */
+		case 247:                                       /* stbux */
+		case 407:                                       /* sthx */
+		case 439:                                       /* sthux */
+		case 662:                                       /* stwbrx */
+		case 918:                                       /* sthbrx */
+			record.rsval = vcpu->arch.gpr[get_rs(inst)];
+			record.raval = vcpu->arch.gpr[get_ra(inst)];
+			record.rbval = vcpu->arch.gpr[get_rb(inst)];
+			break;
+		}
+		break;
+	case 32:                                                /* lwz */
+	case 33:                                                /* lwzu */
+	case 34:                                                /* lbz */
+	case 35:                                                /* lbzu */
+	case 40:                                                /* lhz */
+	case 41:                                                /* lhzu */
+		record.raval = vcpu->arch.gpr[get_ra(inst)];
+		break;
+	case 36:                                                /* stw */
+	case 38:                                                /* stb */
+	case 44:                                                /* sth */
+		record.rsval = vcpu->arch.gpr[get_rs(inst)];
+		break;
+	case 37:                                                /* stwu */
+	case 39:                                                /* stbu */
+	case 45:                                                /* sthu */
+		record.raval = vcpu->arch.gpr[get_ra(inst)];
+		record.rsval = vcpu->arch.gpr[get_rs(inst)];
+		break;
+	}
+
+	relay_write(kvmppc_44x_instr_chan, &record, sizeof(record));
 }
 
 static inline void emulinstr_stat(struct kvm_vcpu *vcpu,
@@ -234,6 +314,10 @@ int kvmppc_emulate_instruction(struct kv
 	int dcrn;
 	enum emulation_result emulated = EMULATE_DONE;
 	int advance = 1;
+
+#ifdef CONFIG_KVM_POWERPC_440_TRACE_INSTRUCTIONS
+	kvmppc_44x_instr_trace(vcpu);
+#endif
 
 	switch (get_op(inst)) {
 	case 3:                                                 /* trap */
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -1082,6 +1082,7 @@ long kvm_arch_vm_ioctl(struct file *filp
 }
 
 extern struct rchan *kvmppc_44x_tlb_chan;
+extern struct rchan *kvmppc_44x_instr_chan;
 /*
  * create_buf_file() callback.  Creates relay file in debugfs.
  */
@@ -1133,6 +1134,13 @@ int kvm_arch_init(void *opaque)
 	if (!kvmppc_44x_tlb_chan)
 		printk("relay_open failed\n");
 
+#ifdef CONFIG_KVM_POWERPC_440_TRACE_INSTRUCTIONS
+	kvmppc_44x_instr_chan = relay_open("44x_instr", kvm_debugfs_dir, 64,
+					   64, &relay_callbacks, NULL);
+	if (!kvmppc_44x_instr_chan)
+		printk(KERN_ERR"relay_open failed\n");
+#endif
+
 #ifdef CONFIG_MAGIC_SYSRQ
 	printk("Registering kvmppc sysrq handler\n");
 	register_sysrq_key('V', &sysrq_kvm_op);
@@ -1144,6 +1152,9 @@ void kvm_arch_exit(void)
 void kvm_arch_exit(void)
 {
 	relay_close(kvmppc_44x_tlb_chan);
+#ifdef CONFIG_KVM_POWERPC_440_TRACE_INSTRUCTIONS
+	relay_close(kvmppc_44x_instr_chan);
+#endif
 }
 
 static int kvmppc_44x_init(void)

[kvm-ppc-devel] [PATCH] [0/3] kvmppc: instruction emulation statistics

[<ehr...@li...>]

These patch series adds two kinds of instruction emulation tracking.
A low overhead counter based mechanism extending kvm_stat (1/3) and a relayfs
based full tracing of all instructions emulated (2/3). The last patch allows
the already existant tlb tracing to be configurable (3/3) in the same manner.
All together these features help to analyze guest behavior in the virtualized
environment.

 arch/powerpc/kvm/Kconfig         |   17 +++++++
 arch/powerpc/kvm/emulate.c       |   84 +++++++++++++++++++++++++++++++++++++++
 arch/powerpc/kvm/powerpc.c       |   15 ++++++
 b/arch/powerpc/kvm/44x_tlb.c     |    2
 b/arch/powerpc/kvm/Kconfig       |    8 +++
 b/arch/powerpc/kvm/emulate.c     |   61 ++++++++++++++++++++++++++++
 b/arch/powerpc/kvm/powerpc.c     |   84 +++++++++++++++++++++++++++++++++++++++
 b/include/asm-powerpc/kvm_host.h |   40 ++++++++++++++++++
 b/include/asm-powerpc/kvm_ppc.h  |   50 +++++++++++++++++++++++
 9 files changed, 360 insertions(+), 1 deletion(-)

[kvm-ppc-devel] [PATCH] [3/3] kvmppc: make tlb tracing configurable

[<ehr...@li...>]

From: Christian Ehrhardt <ehr...@li...>

This patch adds a config option to allow users to enable/disable the relayfs
base tracing of tlb activities added by Hollis in the patch kvmppc_relay.diff.
This patch aligns the behavior of this debug tracing with the two instruction
tracings I added, all would then be configurable via .config as needed.
This patch is intended to be fold into kvmppc_relay.diff and re-adds the select
RELAY which should be ok now, because this is a separate config symbol.

Signed-off-by: Christian Ehrhardt <ehr...@li...>
---

[diffstat]
 44x_tlb.c |    2 ++
 Kconfig   |    8 ++++++++
 powerpc.c |    4 +++-
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/44x_tlb.c b/arch/powerpc/kvm/44x_tlb.c
--- a/arch/powerpc/kvm/44x_tlb.c
+++ b/arch/powerpc/kvm/44x_tlb.c
@@ -45,6 +45,7 @@ struct tlb_record {
 
 void kvmppc_44x_tlb_trace(int action, int index, struct tlbe *tlbe, int ref)
 {
+#ifdef CONFIG_KVM_POWERPC_440_TRACE_TLB
 	struct tlb_record record = {
 		.action = action,
 		.tlb_index = index,
@@ -56,6 +57,7 @@ void kvmppc_44x_tlb_trace(int action, in
 	};
 
 	relay_write(kvmppc_44x_tlb_chan, &record, sizeof(record));
+#endif
 }
 
 static u32 kvmppc_44x_tlb_shadow_attrib(u32 attrib, int usermode)
diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig
--- a/arch/powerpc/kvm/Kconfig
+++ b/arch/powerpc/kvm/Kconfig
@@ -58,6 +58,14 @@ config KVM_POWERPC_440_TRACE_INSTRUCTION
 	  channel.
 	  If unsure, say N.
 
+config KVM_POWERPC_440_TRACE_TLB
+	bool "ppc440 guest tlb activity tracing"
+	depends on KVM && 44x && KVM_POWERPC_440
+	select RELAY
+	---help---
+	  Adds the complete tracing of the tlb activities via a relayfs channel.
+	  If unsure, say N.
+
 config KVM_PPC_VIRTIO
        bool "Virtio Support"
        select VIRTIO
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -1129,10 +1129,12 @@ static struct sysrq_key_op sysrq_kvm_op 
 
 int kvm_arch_init(void *opaque)
 {
+#ifdef CONFIG_KVM_POWERPC_440_TRACE_TLB
 	kvmppc_44x_tlb_chan = relay_open("44x_tlb", kvm_debugfs_dir, 64, 64,
 	                                 &relay_callbacks, NULL);
 	if (!kvmppc_44x_tlb_chan)
-		printk("relay_open failed\n");
+		printk(KERN_ERR"relay_open failed\n");
+#endif
 
 #ifdef CONFIG_KVM_POWERPC_440_TRACE_INSTRUCTIONS
 	kvmppc_44x_instr_chan = relay_open("44x_instr", kvm_debugfs_dir, 64,

Re: [kvm-ppc-devel] [kvm-devel] [PATCH 1 of 4] Add "make sync" to sync need kernel headers to kvm-userspace

[Avi K. <av...@qu...>]

Christoph Hellwig wrote:
> It would be nice to just be able to build kvm from git without a kernel
> around.  The lack of that is what in fact keeps from hacking kvm
> userspace currently.
>   

It would be nice, but committing all header changes twice is not so nice 
and error prone as well.

Anyone serious about hacking kvm-userspace would not be deterred by the 
additional clone.

[maybe we can provide an automatically-generated git tree that has only 
the kernel headers?]

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

Re: [kvm-ppc-devel] [kvm-devel] [PATCH 1 of 4] Add "make sync" to sync need kernel headers to kvm-userspace

[Christoph H. <hc...@in...>]

On Mon, Apr 14, 2008 at 02:47:16PM +0300, Avi Kivity wrote:
>> Please just keep a copy of the kernel headers in the userspace tree so
>> it can be built standalone.
>>
>>   
>
> The tarballs do contain a copy of the kernel headers; the 'make sync' 
> mechanism is for developers to generate the tarballs without keeping the 
> sources duplicated in git.

It would be nice to just be able to build kvm from git without a kernel
around.  The lack of that is what in fact keeps from hacking kvm
userspace currently.

Re: [kvm-ppc-devel] [kvm-devel] [PATCH 1 of 4] Add "make sync" to sync need kernel headers to kvm-userspace

[Avi K. <av...@qu...>]

Christoph Hellwig wrote:
> On Mon, Apr 14, 2008 at 03:37:04AM -0500, Jerone Young wrote:
>   
>> 2 files changed, 25 insertions(+), 4 deletions(-)
>> Makefile  |   21 ++++++++++++++++++++-
>> configure |    8 +++++---
>>
>>
>> This patch adds ability for kvm-userspace build system to sync needed kernel headers locally without the need of compiled kernel source.
>>     
>
> Please just keep a copy of the kernel headers in the userspace tree so
> it can be built standalone.
>
>   

The tarballs do contain a copy of the kernel headers; the 'make sync' 
mechanism is for developers to generate the tarballs without keeping the 
sources duplicated in git.

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

Re: [kvm-ppc-devel] [kvm-devel] [PATCH 1 of 4] Add "make sync" to sync need kernel headers to kvm-userspace

[Christoph H. <hc...@in...>]

On Mon, Apr 14, 2008 at 03:37:04AM -0500, Jerone Young wrote:
> 2 files changed, 25 insertions(+), 4 deletions(-)
> Makefile  |   21 ++++++++++++++++++++-
> configure |    8 +++++---
> 
> 
> This patch adds ability for kvm-userspace build system to sync needed kernel headers locally without the need of compiled kernel source.

Please just keep a copy of the kernel headers in the userspace tree so
it can be built standalone.

Re: [kvm-ppc-devel] [kvm-devel] [PATCH] [v2] Move kvm_get_pit to libkvm.c common code

[Jerone Y. <jy...@us...>]

I just took the old description that was there before. A much better one
would be:

Remove declarations of kvm_*_pit() on architectures who do not support
not have a PIT.

That is what I was really intending. It removes a lot of compile
warnings, when compiling anything with libkvm.h on platforms that do not
have a pit. It's mainly because of the structures that are used as
arguments to these function declrations.


On Sun, 2008-04-13 at 17:24 +0300, Avi Kivity wrote:
> Jerone Young wrote:
> > 	- I am resending this patch removing ia64. It apprently fell through the cracks.
> >
> > Don't compile kvm_*_pit() on architectures whose currently supported platforms do not contain a PIT.
> >
> >         Signed-off-by: Hollis Blanchard <ho...@us...>
> > 	Signed-off-by: Jerone Young <jy...@us...>
> >
> > diff --git a/libkvm/libkvm.h b/libkvm/libkvm.h
> > --- a/libkvm/libkvm.h
> > +++ b/libkvm/libkvm.h
> > @@ -549,6 +549,7 @@ int kvm_pit_in_kernel(kvm_context_t kvm)
> >  
> >  #ifdef KVM_CAP_PIT
> >  
> > +#if defined(__i386__) || defined(__x86_64__)
> >  /*!
> >   * \brief Get in kernel PIT of the virtual domain
> >   *
> > @@ -569,6 +570,7 @@ int kvm_get_pit(kvm_context_t kvm, struc
> >   * \param s PIT state of the virtual domain
> >   */
> >  int kvm_set_pit(kvm_context_t kvm, struct kvm_pit_state *s);
> > +#endif
> >  
> >  #endif
> 
> Patch is okay, but doesn't match the description at all.  Is this what 
> you intended to send?
> 

Re: [kvm-ppc-devel] [kvm-devel] [PATCH] [v2] Move kvm_get_pit to libkvm.c common code

[Avi K. <av...@qu...>]

Jerone Young wrote:
> 	- I am resending this patch removing ia64. It apprently fell through the cracks.
>
> Don't compile kvm_*_pit() on architectures whose currently supported platforms do not contain a PIT.
>
>         Signed-off-by: Hollis Blanchard <ho...@us...>
> 	Signed-off-by: Jerone Young <jy...@us...>
>
> diff --git a/libkvm/libkvm.h b/libkvm/libkvm.h
> --- a/libkvm/libkvm.h
> +++ b/libkvm/libkvm.h
> @@ -549,6 +549,7 @@ int kvm_pit_in_kernel(kvm_context_t kvm)
>  
>  #ifdef KVM_CAP_PIT
>  
> +#if defined(__i386__) || defined(__x86_64__)
>  /*!
>   * \brief Get in kernel PIT of the virtual domain
>   *
> @@ -569,6 +570,7 @@ int kvm_get_pit(kvm_context_t kvm, struc
>   * \param s PIT state of the virtual domain
>   */
>  int kvm_set_pit(kvm_context_t kvm, struct kvm_pit_state *s);
> +#endif
>  
>  #endif

Patch is okay, but doesn't match the description at all.  Is this what 
you intended to send?

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

[kvm-ppc-devel] [PATCH] [v6] Add idle wait support for 44x platforms

[Jerone Y. <jy...@us...>]

2 files changed, 68 insertions(+), 1 deletion(-)
arch/powerpc/platforms/44x/Makefile |    2 -
arch/powerpc/platforms/44x/idle.c   |   67 +++++++++++++++++++++++++++++++++++


Updates: Shrink code even more from comments made on the list

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 <jy...@us...>

diff --git a/arch/powerpc/platforms/44x/Makefile b/arch/powerpc/platforms/44x/Makefile
--- a/arch/powerpc/platforms/44x/Makefile
+++ b/arch/powerpc/platforms/44x/Makefile
@@ -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
diff --git a/arch/powerpc/platforms/44x/idle.c b/arch/powerpc/platforms/44x/idle.c
new file mode 100644
--- /dev/null
+++ b/arch/powerpc/platforms/44x/idle.c
@@ -0,0 +1,67 @@
+/*
+ * Copyright 2008 IBM Corp. 
+ *
+ * Based on arch/powerpc/platforms/pasemi/idle.c: 
+ * Copyright (C) 2006-2007 PA Semi, Inc
+ *
+ * Added by: Jerone Young <jy...@us...>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
+ *
+ */
+
+#include <linux/of.h>
+#include <linux/kernel.h>
+#include <asm/machdep.h>
+
+static int mode_spin;
+
+static void ppc44x_idle(void)
+{
+	unsigned long msr_save;
+
+	msr_save = mfmsr();
+	/* set wait state MSR */
+	mtmsr(msr_save|MSR_WE|MSR_EE|MSR_CE|MSR_DE);
+	isync();
+	/* return to initial state */
+	mtmsr(msr_save);
+	isync();
+}
+
+int __init ppc44x_idle_init(void)
+{
+	if (!mode_spin) {
+		/* If we are not setting spin mode 
+                   then we set to wait mode */
+		ppc_md.power_save = &ppc44x_idle;
+	}
+
+	return 0;
+}
+
+arch_initcall(ppc44x_idle_init);
+
+static int __init idle_param(char *p)
+{ 
+
+	if (!strcmp("spin", p)) {
+		mode_spin = 1;
+		ppc_md.power_save = NULL;
+	}
+
+	return 0;
+}
+
+early_param("idle", idle_param);

Re: [kvm-ppc-devel] [PATCH] [v5] Add idle wait support for 44x platforms

[Olof J. <ol...@li...>]

On Fri, Apr 11, 2008 at 02:18:22AM +0200, Arnd Bergmann wrote:
> On Thursday 10 April 2008, Jerone Young wrote:
> > Well it could be this simple. But the current code leaves a lot more
> > room to add different type waits or spins if need be (if they are ever
> > needed ... though none off the top of my head at the moment)...but it
> > does allow you to create another wait state for whatever reason a lot
> > easier.
> > 
> > So I really don't think this needs to change. Unless everyone really
> > feels that it just has to be.
> 
> No, it doesn't need to change, the current patch is entirely correct,
> just a little bit more complicated than it needs to be, and I try
> not to have code in anticipation of something getting more complicated
> in the future, you can always add the complexity at the point where you
> need it.

That piece of code came across from the platforms/pasemi version,
where we for a while had more than two kinds of idle loops
(doze/nap/sleep/rvwinkle). Turns out doze does well enough that the
others weren't really needed so I never submitted support for the deeper
sleep modes.


-Olof

Re: [kvm-ppc-devel] [PATCH] [v5] Add idle wait support for 44x platforms

[Josh B. <jw...@li...>]

On Fri, 2008-04-11 at 02:18 +0200, Arnd Bergmann wrote:
> > > static int __init idle_param(char *p)
> > > {
> > >       if (!strcmp(modes[i].name, "spin"))
> > >               ppc_md.power_save = NULL;
> > > }
> > > early_param("idle", idle_param);
> > > 
> > > if you statically initialize the ppc_md.power_save function to ppc44x_idle
> > > in the platform setup files?
> > 
> > The idea is to not statically initialize ppc_md.power_save to
> > ppc44x_idle in each platform setup file.
> > 
> 
> Why not? Unlike the platform_initcall, it wouldn't cost anything and your
> current code has the same effect in the end, but in a less obvious way.

This is likely something that has evolved from the original patch which
added a machine_late_initcall(<machine>, ppc44x_idle_init) to every
platform.c file.  It made the original patch overly complicated as
opposed to adding a single arch_initcall in a common file.  That was
also before we decided to make "wait" the default, which made things
much easier to deal with.

I'll take the blame for the way the minorly suboptimal state the patch
is, as Jerone had to jump through several hoops already at my request.
I agree it's cleaner to do as you suggest in the long run, and may well
make some changes along those lines myself.

josh

Re: [kvm-ppc-devel] [PATCH] [v5] Add idle wait support for 44x platforms

[Arnd B. <ar...@ar...>]

On Thursday 10 April 2008, Jerone Young wrote:
> Well it could be this simple. But the current code leaves a lot more
> room to add different type waits or spins if need be (if they are ever
> needed ... though none off the top of my head at the moment)...but it
> does allow you to create another wait state for whatever reason a lot
> easier.
> 
> So I really don't think this needs to change. Unless everyone really
> feels that it just has to be.

No, it doesn't need to change, the current patch is entirely correct,
just a little bit more complicated than it needs to be, and I try
not to have code in anticipation of something getting more complicated
in the future, you can always add the complexity at the point where you
need it.

> > 
> > static int __init idle_param(char *p)
> > {
> >       if (!strcmp(modes[i].name, "spin"))
> >               ppc_md.power_save = NULL;
> > }
> > early_param("idle", idle_param);
> > 
> > if you statically initialize the ppc_md.power_save function to ppc44x_idle
> > in the platform setup files?
> 
> The idea is to not statically initialize ppc_md.power_save to
> ppc44x_idle in each platform setup file.
> 

Why not? Unlike the platform_initcall, it wouldn't cost anything and your
current code has the same effect in the end, but in a less obvious way.

	Arnd <><

[kvm-ppc-devel] [PATCH 0 of 3] [V2] Fix loading of registers preboot for PowerPC

[Jerone Y. <jy...@us...>]

So this now moves code away from common code to arch specific kvm_qemu_arch_init(). Also added is a removal of duplicate calls to kvm_qemu_init_env() in common code.

Signed-off-by: Jerone Young <jy...@us...>

3 files changed, 6 insertions(+), 4 deletions(-)
qemu/hw/ppc440_bamboo.c |    3 ---
qemu/qemu-kvm-powerpc.c |    6 ++++++
qemu/qemu-kvm.c         |    1 -

[kvm-ppc-devel] [PATCH 1 of 3] Add kvm_load_registers into qemu_arch_init for PowerPC

[Jerone Y. <jy...@us...>]

1 file changed, 6 insertions(+)
qemu/qemu-kvm-powerpc.c |    6 ++++++


This patch adds a call to load_kvm_registers after creation of vcpu. This is required for ppc since we are required to set certain registers before boot.

Signed-off-by: Jerone Young <jy...@us...>

diff --git a/qemu/qemu-kvm-powerpc.c b/qemu/qemu-kvm-powerpc.c
--- a/qemu/qemu-kvm-powerpc.c
+++ b/qemu/qemu-kvm-powerpc.c
@@ -121,6 +121,12 @@ void kvm_arch_save_regs(CPUState *env)
 
 int kvm_arch_qemu_init_env(CPUState *cenv)
 {
+    if (cenv->cpu_index == 0) {
+        /* load any registers set in env into
+           kvm  for the first guest vcpu */
+        kvm_load_registers(cenv);
+    }
+
     return 0;
 }
 

[kvm-ppc-devel] [PATCH 3 of 3] Remove duplicate kvm_qemu_init() calls

[Jerone Y. <jy...@us...>]

1 file changed, 1 deletion(-)
qemu/qemu-kvm.c |    1 -


kvm_qemu_init is called in ap_main_loop() , when ap_main_loop() calls kvm_main_loop_cpu(), kvm_qemu_init() is run for a second time. It should only be called in kvm_main_loop_cpu().

Signed-off-by: Jerone Young <jy...@us...>
~

diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -353,7 +353,6 @@ static void *ap_main_loop(void *_env)
     sigdelset(&signals, SIG_IPI);
     sigprocmask(SIG_BLOCK, &signals, NULL);
     kvm_create_vcpu(kvm_context, env->cpu_index);
-    kvm_qemu_init_env(env);
     if (kvm_irqchip_in_kernel(kvm_context))
 	env->hflags &= ~HF_HALTED_MASK;
     kvm_main_loop_cpu(env);

[kvm-ppc-devel] [PATCH 2 of 3] Remove kvm_load_registers from ppc440_bamboo board model

[Jerone Y. <jy...@us...>]

1 file changed, 3 deletions(-)
qemu/hw/ppc440_bamboo.c |    3 ---


This patch removes the call to kvm_load_registers while in board platform setup code. This must now be done later in vcpu initialization.

Signed-off-by: Jerone Young <jy...@us...>

diff --git a/qemu/hw/ppc440_bamboo.c b/qemu/hw/ppc440_bamboo.c
--- a/qemu/hw/ppc440_bamboo.c
+++ b/qemu/hw/ppc440_bamboo.c
@@ -174,9 +174,6 @@ void bamboo_init(ram_addr_t ram_size, in
 		env->gpr[3] = dt_base;
 #endif
 		env->nip = ep;
-
-		printf("%s: loading kvm registers\n", __func__);
-		kvm_load_registers(env);
 	}
 
 	for (i = 0; i < nb_nics; i++) {

Re: [kvm-ppc-devel] [kvm-devel] [PATCH 1 of 2] Add kvm_load_registers after first vcpu creation

[Jerone Y. <jy...@us...>]

On Thu, 2008-04-10 at 18:35 -0300, Marcelo Tosatti wrote:
> On Thu, Apr 10, 2008 at 04:04:47PM -0500, Jerone Young wrote:
> > 1 file changed, 5 insertions(+)
> > qemu/qemu-kvm.c |    5 +++++
> > 
> >
> > This patch adds a call to load_kvm_registers after creation of
> > vcpu. This is required for ppc since we are required to set certain
> > registers before boot. This should not have any effect on the curren
> > x86 code (though I need to test this to make sure).
> >
> > What I would like though are some comments on the fix. Is this the
> > right place for this? We had this in our platform setup code, but with
> > recent code changes it will not work there anymore).
> >
> > Signed-off-by: Jerone Young <jy...@us...>
> > 
> > diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
> > --- a/qemu/qemu-kvm.c
> > +++ b/qemu/qemu-kvm.c
> > @@ -353,6 +353,11 @@ static void *ap_main_loop(void *_env)
> >      sigdelset(&signals, SIG_IPI);
> >      sigprocmask(SIG_BLOCK, &signals, NULL);
> >      kvm_create_vcpu(kvm_context, env->cpu_index);
> > +    if (env->cpu_index == 0) {
> > +         /* load any registers set in env into 
> > +            kvm  for the first guest vcpu */ 
> > +        kvm_load_registers(env);
> > +    }
> >      kvm_qemu_init_env(env);
> >      if (kvm_irqchip_in_kernel(kvm_context))
> >  	env->hflags &= ~HF_HALTED_MASK;
> 
> Hi Jerone,
> 
> You can hook into PPC's kvm_arch_qemu_init_env().

That would be a much better place. 

I also noticed that kvm_qemu_init_env() is called in ap_main_loop and
the kvm_main_loop_cpu. ap_main_loop calls kvm_main_loop_cpu, so  one
them should be removed. I'll submit another patch for that.

> 

Re: [kvm-ppc-devel] [kvm-devel] [PATCH 1 of 2] Add kvm_load_registers after first vcpu creation

[Marcelo T. <mto...@re...>]

On Thu, Apr 10, 2008 at 04:04:47PM -0500, Jerone Young wrote:
> 1 file changed, 5 insertions(+)
> qemu/qemu-kvm.c |    5 +++++
> 
>
> This patch adds a call to load_kvm_registers after creation of
> vcpu. This is required for ppc since we are required to set certain
> registers before boot. This should not have any effect on the curren
> x86 code (though I need to test this to make sure).
>
> What I would like though are some comments on the fix. Is this the
> right place for this? We had this in our platform setup code, but with
> recent code changes it will not work there anymore).
>
> Signed-off-by: Jerone Young <jy...@us...>
> 
> diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
> --- a/qemu/qemu-kvm.c
> +++ b/qemu/qemu-kvm.c
> @@ -353,6 +353,11 @@ static void *ap_main_loop(void *_env)
>      sigdelset(&signals, SIG_IPI);
>      sigprocmask(SIG_BLOCK, &signals, NULL);
>      kvm_create_vcpu(kvm_context, env->cpu_index);
> +    if (env->cpu_index == 0) {
> +         /* load any registers set in env into 
> +            kvm  for the first guest vcpu */ 
> +        kvm_load_registers(env);
> +    }
>      kvm_qemu_init_env(env);
>      if (kvm_irqchip_in_kernel(kvm_context))
>  	env->hflags &= ~HF_HALTED_MASK;

Hi Jerone,

You can hook into PPC's kvm_arch_qemu_init_env().

[kvm-ppc-devel] [PATCH 0 of 2] Fix loading of registers preboot for PowerPC

[Jerone Y. <jy...@us...>]

These patches fix issue with brining up bamboo board model for PowerPC. We need to load certain registers in the first vcp preboot. 

I have not tested this on x86, but it should work without issue. Does anyone not like this being where it is?

2 files changed, 5 insertions(+), 3 deletions(-)
qemu/hw/ppc440_bamboo.c |    3 ---
qemu/qemu-kvm.c         |    5 +++++

Re: [kvm-ppc-devel] [PATCH] [v5] Add idle wait support for 44x platforms

[Jerone Y. <jy...@us...>]

On Thu, 2008-04-10 at 15:44 +0200, Arnd Bergmann wrote:
> On Tuesday 08 April 2008, Jerone Young wrote:
> > +static struct sleep_mode modes[] = {
> > +       { .name = "wait", .entry = &ppc44x_idle },
> > +       { .name = "spin", .entry = NULL },
> > +};
> > +
> > +int __init ppc44x_idle_init(void)
> > +{
> > +       void *func = modes[current_mode].entry;
> > +       ppc_md.power_save = func;
> > +       return 0;
> > +}
> > +
> > +arch_initcall(ppc44x_idle_init);
> > +
> > +static int __init idle_param(char *p)
> > +{ 
> > +       int i;
> > +
> > +       for (i = 0; i < ARRAY_SIZE(modes); i++) {
> > +               if (!strcmp(modes[i].name, p)) {
> > +                       current_mode = i;
> > +                       break;
> > +               }
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +early_param("idle", idle_param);
> 
> ok, sorry to steal the show again, now that everyone seems to be happy
> with the current code, but isn't this equivalent to the simple

Well it could be this simple. But the current code leaves a lot more
room to add different type waits or spins if need be (if they are ever
needed ... though none off the top of my head at the moment)...but it
does allow you to create another wait state for whatever reason a lot
easier.

So I really don't think this needs to change. Unless everyone really
feels that it just has to be.

> 
> static int __init idle_param(char *p)
> {
> 	if (!strcmp(modes[i].name, "spin"))
> 		ppc_md.power_save = NULL;
> }
> early_param("idle", idle_param);
> 
> if you statically initialize the ppc_md.power_save function to ppc44x_idle
> in the platform setup files?

The idea is to not statically initialize ppc_md.power_save to
ppc44x_idle in each platform setup file.

> 
> 	Arnd <><