[PATCH 1/2] KVM: PPC: Fix kvm_exit_names array

[PATCH 1/2] KVM: PPC: Fix kvm_exit_names array

[Mihai Caraman]

Some exit ids where left out from kvm_exit_names array.

Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
---
 arch/powerpc/kvm/timing.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/kvm/timing.c b/arch/powerpc/kvm/timing.c
index 07b6110..c392d26 100644
--- a/arch/powerpc/kvm/timing.c
+++ b/arch/powerpc/kvm/timing.c
@@ -135,7 +135,9 @@ static const char *kvm_exit_names[__NUMBER_OF_KVM_EXIT_TYPES] = {
 	[USR_PR_INST] =             "USR_PR_INST",
 	[FP_UNAVAIL] =              "FP_UNAVAIL",
 	[DEBUG_EXITS] =             "DEBUG",
-	[TIMEINGUEST] =             "TIMEINGUEST"
+	[TIMEINGUEST] =             "TIMEINGUEST",
+	[DBELL_EXITS] =             "DBELL",
+	[GDBELL_EXITS] =            "GDBELL"
 };
 
 static int kvmppc_exit_timing_show(struct seq_file *m, void *private)
-- 
1.7.3.4

[PATCH 1/2] KVM: PPC: Fix kvm_exit_names array

[Mihai Caraman]

Some exit ids where left out from kvm_exit_names array.

Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
---
 arch/powerpc/kvm/timing.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/kvm/timing.c b/arch/powerpc/kvm/timing.c
index 07b6110..c392d26 100644
--- a/arch/powerpc/kvm/timing.c
+++ b/arch/powerpc/kvm/timing.c
@@ -135,7 +135,9 @@ static const char *kvm_exit_names[__NUMBER_OF_KVM_EXIT_TYPES] = {
 	[USR_PR_INST] =             "USR_PR_INST",
 	[FP_UNAVAIL] =              "FP_UNAVAIL",
 	[DEBUG_EXITS] =             "DEBUG",
-	[TIMEINGUEST] =             "TIMEINGUEST"
+	[TIMEINGUEST] =             "TIMEINGUEST",
+	[DBELL_EXITS] =             "DBELL",
+	[GDBELL_EXITS] =            "GDBELL"
 };
 
 static int kvmppc_exit_timing_show(struct seq_file *m, void *private)
-- 
1.7.3.4

[PATCH 1/2] KVM: PPC: Fix kvm_exit_names array

[Mihai Caraman]

Some exit ids where left out from kvm_exit_names array.

Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
---
 arch/powerpc/kvm/timing.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/kvm/timing.c b/arch/powerpc/kvm/timing.c
index 07b6110..c392d26 100644
--- a/arch/powerpc/kvm/timing.c
+++ b/arch/powerpc/kvm/timing.c
@@ -135,7 +135,9 @@ static const char *kvm_exit_names[__NUMBER_OF_KVM_EXIT_TYPES] = {
 	[USR_PR_INST] =             "USR_PR_INST",
 	[FP_UNAVAIL] =              "FP_UNAVAIL",
 	[DEBUG_EXITS] =             "DEBUG",
-	[TIMEINGUEST] =             "TIMEINGUEST"
+	[TIMEINGUEST] =             "TIMEINGUEST",
+	[DBELL_EXITS] =             "DBELL",
+	[GDBELL_EXITS] =            "GDBELL"
 };
 
 static int kvmppc_exit_timing_show(struct seq_file *m, void *private)
-- 
1.7.3.4

[PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Mihai Caraman]

Some guests are making use of return from machine check instruction
to do crazy things even though the 64-bit kernel doesn't handle yet
this interrupt. Emulate MCSRR0/1 SPR and rfmci instruction accordingly.

Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
---
 arch/powerpc/include/asm/kvm_host.h |    1 +
 arch/powerpc/kvm/booke_emulate.c    |   25 +++++++++++++++++++++++++
 arch/powerpc/kvm/timing.c           |    1 +
 3 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index af326cd..0466789 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -148,6 +148,7 @@ enum kvm_exit_types {
 	EMULATED_TLBWE_EXITS,
 	EMULATED_RFI_EXITS,
 	EMULATED_RFCI_EXITS,
+	EMULATED_RFMCI_EXITS,
 	DEC_EXITS,
 	EXT_INTR_EXITS,
 	HALT_WAKEUP,
diff --git a/arch/powerpc/kvm/booke_emulate.c b/arch/powerpc/kvm/booke_emulate.c
index 27a4b28..aaff1b7 100644
--- a/arch/powerpc/kvm/booke_emulate.c
+++ b/arch/powerpc/kvm/booke_emulate.c
@@ -23,6 +23,7 @@
 
 #include "booke.h"
 
+#define OP_19_XOP_RFMCI   38
 #define OP_19_XOP_RFI     50
 #define OP_19_XOP_RFCI    51
 
@@ -43,6 +44,12 @@ static void kvmppc_emul_rfci(struct kvm_vcpu *vcpu)
 	kvmppc_set_msr(vcpu, vcpu->arch.csrr1);
 }
 
+static void kvmppc_emul_rfmci(struct kvm_vcpu *vcpu)
+{
+	vcpu->arch.pc = vcpu->arch.mcsrr0;
+	kvmppc_set_msr(vcpu, vcpu->arch.mcsrr1);
+}
+
 int kvmppc_booke_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
                             unsigned int inst, int *advance)
 {
@@ -65,6 +72,12 @@ int kvmppc_booke_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
 			*advance = 0;
 			break;
 
+		case OP_19_XOP_RFMCI:
+			kvmppc_emul_rfmci(vcpu);
+			kvmppc_set_exit_type(vcpu, EMULATED_RFMCI_EXITS);
+			*advance = 0;
+			break;
+
 		default:
 			emulated = EMULATE_FAIL;
 			break;
@@ -138,6 +151,12 @@ int kvmppc_booke_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val)
 	case SPRN_DBCR1:
 		vcpu->arch.dbg_reg.dbcr1 = spr_val;
 		break;
+	case SPRN_MCSRR0:
+		vcpu->arch.mcsrr0 = spr_val;
+		break;
+	case SPRN_MCSRR1:
+		vcpu->arch.mcsrr1 = spr_val;
+		break;
 	case SPRN_DBSR:
 		vcpu->arch.dbsr &= ~spr_val;
 		break;
@@ -284,6 +303,12 @@ int kvmppc_booke_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val)
 	case SPRN_DBCR1:
 		*spr_val = vcpu->arch.dbg_reg.dbcr1;
 		break;
+	case SPRN_MCSRR0:
+		*spr_val = vcpu->arch.mcsrr0;
+		break;
+	case SPRN_MCSRR1:
+		*spr_val = vcpu->arch.mcsrr1;
+		break;
 	case SPRN_DBSR:
 		*spr_val = vcpu->arch.dbsr;
 		break;
diff --git a/arch/powerpc/kvm/timing.c b/arch/powerpc/kvm/timing.c
index c392d26..670f63d 100644
--- a/arch/powerpc/kvm/timing.c
+++ b/arch/powerpc/kvm/timing.c
@@ -129,6 +129,7 @@ static const char *kvm_exit_names[__NUMBER_OF_KVM_EXIT_TYPES] = {
 	[EMULATED_TLBSX_EXITS] =    "EMUL_TLBSX",
 	[EMULATED_TLBWE_EXITS] =    "EMUL_TLBWE",
 	[EMULATED_RFI_EXITS] =      "EMUL_RFI",
+	[EMULATED_RFMCI_EXITS] =    "EMUL_RFMCI",
 	[DEC_EXITS] =               "DEC",
 	[EXT_INTR_EXITS] =          "EXTINT",
 	[HALT_WAKEUP] =             "HALT",
-- 
1.7.3.4

[PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Mihai Caraman]

Some guests are making use of return from machine check instruction
to do crazy things even though the 64-bit kernel doesn't handle yet
this interrupt. Emulate MCSRR0/1 SPR and rfmci instruction accordingly.

Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
---
 arch/powerpc/include/asm/kvm_host.h |    1 +
 arch/powerpc/kvm/booke_emulate.c    |   25 +++++++++++++++++++++++++
 arch/powerpc/kvm/timing.c           |    1 +
 3 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index af326cd..0466789 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -148,6 +148,7 @@ enum kvm_exit_types {
 	EMULATED_TLBWE_EXITS,
 	EMULATED_RFI_EXITS,
 	EMULATED_RFCI_EXITS,
+	EMULATED_RFMCI_EXITS,
 	DEC_EXITS,
 	EXT_INTR_EXITS,
 	HALT_WAKEUP,
diff --git a/arch/powerpc/kvm/booke_emulate.c b/arch/powerpc/kvm/booke_emulate.c
index 27a4b28..aaff1b7 100644
--- a/arch/powerpc/kvm/booke_emulate.c
+++ b/arch/powerpc/kvm/booke_emulate.c
@@ -23,6 +23,7 @@
 
 #include "booke.h"
 
+#define OP_19_XOP_RFMCI   38
 #define OP_19_XOP_RFI     50
 #define OP_19_XOP_RFCI    51
 
@@ -43,6 +44,12 @@ static void kvmppc_emul_rfci(struct kvm_vcpu *vcpu)
 	kvmppc_set_msr(vcpu, vcpu->arch.csrr1);
 }
 
+static void kvmppc_emul_rfmci(struct kvm_vcpu *vcpu)
+{
+	vcpu->arch.pc = vcpu->arch.mcsrr0;
+	kvmppc_set_msr(vcpu, vcpu->arch.mcsrr1);
+}
+
 int kvmppc_booke_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
                             unsigned int inst, int *advance)
 {
@@ -65,6 +72,12 @@ int kvmppc_booke_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
 			*advance = 0;
 			break;
 
+		case OP_19_XOP_RFMCI:
+			kvmppc_emul_rfmci(vcpu);
+			kvmppc_set_exit_type(vcpu, EMULATED_RFMCI_EXITS);
+			*advance = 0;
+			break;
+
 		default:
 			emulated = EMULATE_FAIL;
 			break;
@@ -138,6 +151,12 @@ int kvmppc_booke_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val)
 	case SPRN_DBCR1:
 		vcpu->arch.dbg_reg.dbcr1 = spr_val;
 		break;
+	case SPRN_MCSRR0:
+		vcpu->arch.mcsrr0 = spr_val;
+		break;
+	case SPRN_MCSRR1:
+		vcpu->arch.mcsrr1 = spr_val;
+		break;
 	case SPRN_DBSR:
 		vcpu->arch.dbsr &= ~spr_val;
 		break;
@@ -284,6 +303,12 @@ int kvmppc_booke_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val)
 	case SPRN_DBCR1:
 		*spr_val = vcpu->arch.dbg_reg.dbcr1;
 		break;
+	case SPRN_MCSRR0:
+		*spr_val = vcpu->arch.mcsrr0;
+		break;
+	case SPRN_MCSRR1:
+		*spr_val = vcpu->arch.mcsrr1;
+		break;
 	case SPRN_DBSR:
 		*spr_val = vcpu->arch.dbsr;
 		break;
diff --git a/arch/powerpc/kvm/timing.c b/arch/powerpc/kvm/timing.c
index c392d26..670f63d 100644
--- a/arch/powerpc/kvm/timing.c
+++ b/arch/powerpc/kvm/timing.c
@@ -129,6 +129,7 @@ static const char *kvm_exit_names[__NUMBER_OF_KVM_EXIT_TYPES] = {
 	[EMULATED_TLBSX_EXITS] =    "EMUL_TLBSX",
 	[EMULATED_TLBWE_EXITS] =    "EMUL_TLBWE",
 	[EMULATED_RFI_EXITS] =      "EMUL_RFI",
+	[EMULATED_RFMCI_EXITS] =    "EMUL_RFMCI",
 	[DEC_EXITS] =               "DEC",
 	[EXT_INTR_EXITS] =          "EXTINT",
 	[HALT_WAKEUP] =             "HALT",
-- 
1.7.3.4

[PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Mihai Caraman]

Some guests are making use of return from machine check instruction
to do crazy things even though the 64-bit kernel doesn't handle yet
this interrupt. Emulate MCSRR0/1 SPR and rfmci instruction accordingly.

Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
---
 arch/powerpc/include/asm/kvm_host.h |    1 +
 arch/powerpc/kvm/booke_emulate.c    |   25 +++++++++++++++++++++++++
 arch/powerpc/kvm/timing.c           |    1 +
 3 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index af326cd..0466789 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -148,6 +148,7 @@ enum kvm_exit_types {
 	EMULATED_TLBWE_EXITS,
 	EMULATED_RFI_EXITS,
 	EMULATED_RFCI_EXITS,
+	EMULATED_RFMCI_EXITS,
 	DEC_EXITS,
 	EXT_INTR_EXITS,
 	HALT_WAKEUP,
diff --git a/arch/powerpc/kvm/booke_emulate.c b/arch/powerpc/kvm/booke_emulate.c
index 27a4b28..aaff1b7 100644
--- a/arch/powerpc/kvm/booke_emulate.c
+++ b/arch/powerpc/kvm/booke_emulate.c
@@ -23,6 +23,7 @@
 
 #include "booke.h"
 
+#define OP_19_XOP_RFMCI   38
 #define OP_19_XOP_RFI     50
 #define OP_19_XOP_RFCI    51
 
@@ -43,6 +44,12 @@ static void kvmppc_emul_rfci(struct kvm_vcpu *vcpu)
 	kvmppc_set_msr(vcpu, vcpu->arch.csrr1);
 }
 
+static void kvmppc_emul_rfmci(struct kvm_vcpu *vcpu)
+{
+	vcpu->arch.pc = vcpu->arch.mcsrr0;
+	kvmppc_set_msr(vcpu, vcpu->arch.mcsrr1);
+}
+
 int kvmppc_booke_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
                             unsigned int inst, int *advance)
 {
@@ -65,6 +72,12 @@ int kvmppc_booke_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
 			*advance = 0;
 			break;
 
+		case OP_19_XOP_RFMCI:
+			kvmppc_emul_rfmci(vcpu);
+			kvmppc_set_exit_type(vcpu, EMULATED_RFMCI_EXITS);
+			*advance = 0;
+			break;
+
 		default:
 			emulated = EMULATE_FAIL;
 			break;
@@ -138,6 +151,12 @@ int kvmppc_booke_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val)
 	case SPRN_DBCR1:
 		vcpu->arch.dbg_reg.dbcr1 = spr_val;
 		break;
+	case SPRN_MCSRR0:
+		vcpu->arch.mcsrr0 = spr_val;
+		break;
+	case SPRN_MCSRR1:
+		vcpu->arch.mcsrr1 = spr_val;
+		break;
 	case SPRN_DBSR:
 		vcpu->arch.dbsr &= ~spr_val;
 		break;
@@ -284,6 +303,12 @@ int kvmppc_booke_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val)
 	case SPRN_DBCR1:
 		*spr_val = vcpu->arch.dbg_reg.dbcr1;
 		break;
+	case SPRN_MCSRR0:
+		*spr_val = vcpu->arch.mcsrr0;
+		break;
+	case SPRN_MCSRR1:
+		*spr_val = vcpu->arch.mcsrr1;
+		break;
 	case SPRN_DBSR:
 		*spr_val = vcpu->arch.dbsr;
 		break;
diff --git a/arch/powerpc/kvm/timing.c b/arch/powerpc/kvm/timing.c
index c392d26..670f63d 100644
--- a/arch/powerpc/kvm/timing.c
+++ b/arch/powerpc/kvm/timing.c
@@ -129,6 +129,7 @@ static const char *kvm_exit_names[__NUMBER_OF_KVM_EXIT_TYPES] = {
 	[EMULATED_TLBSX_EXITS] =    "EMUL_TLBSX",
 	[EMULATED_TLBWE_EXITS] =    "EMUL_TLBWE",
 	[EMULATED_RFI_EXITS] =      "EMUL_RFI",
+	[EMULATED_RFMCI_EXITS] =    "EMUL_RFMCI",
 	[DEC_EXITS] =               "DEC",
 	[EXT_INTR_EXITS] =          "EXTINT",
 	[HALT_WAKEUP] =             "HALT",
-- 
1.7.3.4

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Alexander Graf]

On 03.07.2013, at 15:30, Mihai Caraman wrote:

> Some guests are making use of return from machine check instruction
> to do crazy things even though the 64-bit kernel doesn't handle yet
> this interrupt. Emulate MCSRR0/1 SPR and rfmci instruction accordingly.
> 
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> ---
> arch/powerpc/include/asm/kvm_host.h |    1 +
> arch/powerpc/kvm/booke_emulate.c    |   25 +++++++++++++++++++++++++
> arch/powerpc/kvm/timing.c           |    1 +
> 3 files changed, 27 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
> index af326cd..0466789 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -148,6 +148,7 @@ enum kvm_exit_types {
> 	EMULATED_TLBWE_EXITS,
> 	EMULATED_RFI_EXITS,
> 	EMULATED_RFCI_EXITS,
> +	EMULATED_RFMCI_EXITS,

I would quite frankly prefer to see us abandon the whole exit timing framework in the kernel and instead use trace points. Then we don't have to maintain all of this randomly exercised code.

FWIW I think in this case however, treating RFMCI the same as RFI or random "instruction emulation" shouldn't hurt. This whole table is only about timing measurements. If you want to know for real what's going on, use trace points.

Otherwise looks good.


Alex

> 	DEC_EXITS,
> 	EXT_INTR_EXITS,
> 	HALT_WAKEUP,
> diff --git a/arch/powerpc/kvm/booke_emulate.c b/arch/powerpc/kvm/booke_emulate.c
> index 27a4b28..aaff1b7 100644
> --- a/arch/powerpc/kvm/booke_emulate.c
> +++ b/arch/powerpc/kvm/booke_emulate.c
> @@ -23,6 +23,7 @@
> 
> #include "booke.h"
> 
> +#define OP_19_XOP_RFMCI   38
> #define OP_19_XOP_RFI     50
> #define OP_19_XOP_RFCI    51
> 
> @@ -43,6 +44,12 @@ static void kvmppc_emul_rfci(struct kvm_vcpu *vcpu)
> 	kvmppc_set_msr(vcpu, vcpu->arch.csrr1);
> }
> 
> +static void kvmppc_emul_rfmci(struct kvm_vcpu *vcpu)
> +{
> +	vcpu->arch.pc = vcpu->arch.mcsrr0;
> +	kvmppc_set_msr(vcpu, vcpu->arch.mcsrr1);
> +}
> +
> int kvmppc_booke_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
>                             unsigned int inst, int *advance)
> {
> @@ -65,6 +72,12 @@ int kvmppc_booke_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
> 			*advance = 0;
> 			break;
> 
> +		case OP_19_XOP_RFMCI:
> +			kvmppc_emul_rfmci(vcpu);
> +			kvmppc_set_exit_type(vcpu, EMULATED_RFMCI_EXITS);
> +			*advance = 0;
> +			break;
> +
> 		default:
> 			emulated = EMULATE_FAIL;
> 			break;
> @@ -138,6 +151,12 @@ int kvmppc_booke_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val)
> 	case SPRN_DBCR1:
> 		vcpu->arch.dbg_reg.dbcr1 = spr_val;
> 		break;
> +	case SPRN_MCSRR0:
> +		vcpu->arch.mcsrr0 = spr_val;
> +		break;
> +	case SPRN_MCSRR1:
> +		vcpu->arch.mcsrr1 = spr_val;
> +		break;
> 	case SPRN_DBSR:
> 		vcpu->arch.dbsr &= ~spr_val;
> 		break;
> @@ -284,6 +303,12 @@ int kvmppc_booke_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val)
> 	case SPRN_DBCR1:
> 		*spr_val = vcpu->arch.dbg_reg.dbcr1;
> 		break;
> +	case SPRN_MCSRR0:
> +		*spr_val = vcpu->arch.mcsrr0;
> +		break;
> +	case SPRN_MCSRR1:
> +		*spr_val = vcpu->arch.mcsrr1;
> +		break;
> 	case SPRN_DBSR:
> 		*spr_val = vcpu->arch.dbsr;
> 		break;
> diff --git a/arch/powerpc/kvm/timing.c b/arch/powerpc/kvm/timing.c
> index c392d26..670f63d 100644
> --- a/arch/powerpc/kvm/timing.c
> +++ b/arch/powerpc/kvm/timing.c
> @@ -129,6 +129,7 @@ static const char *kvm_exit_names[__NUMBER_OF_KVM_EXIT_TYPES] = {
> 	[EMULATED_TLBSX_EXITS] =    "EMUL_TLBSX",
> 	[EMULATED_TLBWE_EXITS] =    "EMUL_TLBWE",
> 	[EMULATED_RFI_EXITS] =      "EMUL_RFI",
> +	[EMULATED_RFMCI_EXITS] =    "EMUL_RFMCI",
> 	[DEC_EXITS] =               "DEC",
> 	[EXT_INTR_EXITS] =          "EXTINT",
> 	[HALT_WAKEUP] =             "HALT",
> -- 
> 1.7.3.4
> 
> 
> --
> 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

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Alexander Graf]

On 03.07.2013, at 15:30, Mihai Caraman wrote:

> Some guests are making use of return from machine check instruction
> to do crazy things even though the 64-bit kernel doesn't handle yet
> this interrupt. Emulate MCSRR0/1 SPR and rfmci instruction accordingly.
> 
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> ---
> arch/powerpc/include/asm/kvm_host.h |    1 +
> arch/powerpc/kvm/booke_emulate.c    |   25 +++++++++++++++++++++++++
> arch/powerpc/kvm/timing.c           |    1 +
> 3 files changed, 27 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
> index af326cd..0466789 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -148,6 +148,7 @@ enum kvm_exit_types {
> 	EMULATED_TLBWE_EXITS,
> 	EMULATED_RFI_EXITS,
> 	EMULATED_RFCI_EXITS,
> +	EMULATED_RFMCI_EXITS,

I would quite frankly prefer to see us abandon the whole exit timing framework in the kernel and instead use trace points. Then we don't have to maintain all of this randomly exercised code.

FWIW I think in this case however, treating RFMCI the same as RFI or random "instruction emulation" shouldn't hurt. This whole table is only about timing measurements. If you want to know for real what's going on, use trace points.

Otherwise looks good.


Alex

> 	DEC_EXITS,
> 	EXT_INTR_EXITS,
> 	HALT_WAKEUP,
> diff --git a/arch/powerpc/kvm/booke_emulate.c b/arch/powerpc/kvm/booke_emulate.c
> index 27a4b28..aaff1b7 100644
> --- a/arch/powerpc/kvm/booke_emulate.c
> +++ b/arch/powerpc/kvm/booke_emulate.c
> @@ -23,6 +23,7 @@
> 
> #include "booke.h"
> 
> +#define OP_19_XOP_RFMCI   38
> #define OP_19_XOP_RFI     50
> #define OP_19_XOP_RFCI    51
> 
> @@ -43,6 +44,12 @@ static void kvmppc_emul_rfci(struct kvm_vcpu *vcpu)
> 	kvmppc_set_msr(vcpu, vcpu->arch.csrr1);
> }
> 
> +static void kvmppc_emul_rfmci(struct kvm_vcpu *vcpu)
> +{
> +	vcpu->arch.pc = vcpu->arch.mcsrr0;
> +	kvmppc_set_msr(vcpu, vcpu->arch.mcsrr1);
> +}
> +
> int kvmppc_booke_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
>                             unsigned int inst, int *advance)
> {
> @@ -65,6 +72,12 @@ int kvmppc_booke_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
> 			*advance = 0;
> 			break;
> 
> +		case OP_19_XOP_RFMCI:
> +			kvmppc_emul_rfmci(vcpu);
> +			kvmppc_set_exit_type(vcpu, EMULATED_RFMCI_EXITS);
> +			*advance = 0;
> +			break;
> +
> 		default:
> 			emulated = EMULATE_FAIL;
> 			break;
> @@ -138,6 +151,12 @@ int kvmppc_booke_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val)
> 	case SPRN_DBCR1:
> 		vcpu->arch.dbg_reg.dbcr1 = spr_val;
> 		break;
> +	case SPRN_MCSRR0:
> +		vcpu->arch.mcsrr0 = spr_val;
> +		break;
> +	case SPRN_MCSRR1:
> +		vcpu->arch.mcsrr1 = spr_val;
> +		break;
> 	case SPRN_DBSR:
> 		vcpu->arch.dbsr &= ~spr_val;
> 		break;
> @@ -284,6 +303,12 @@ int kvmppc_booke_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val)
> 	case SPRN_DBCR1:
> 		*spr_val = vcpu->arch.dbg_reg.dbcr1;
> 		break;
> +	case SPRN_MCSRR0:
> +		*spr_val = vcpu->arch.mcsrr0;
> +		break;
> +	case SPRN_MCSRR1:
> +		*spr_val = vcpu->arch.mcsrr1;
> +		break;
> 	case SPRN_DBSR:
> 		*spr_val = vcpu->arch.dbsr;
> 		break;
> diff --git a/arch/powerpc/kvm/timing.c b/arch/powerpc/kvm/timing.c
> index c392d26..670f63d 100644
> --- a/arch/powerpc/kvm/timing.c
> +++ b/arch/powerpc/kvm/timing.c
> @@ -129,6 +129,7 @@ static const char *kvm_exit_names[__NUMBER_OF_KVM_EXIT_TYPES] = {
> 	[EMULATED_TLBSX_EXITS] =    "EMUL_TLBSX",
> 	[EMULATED_TLBWE_EXITS] =    "EMUL_TLBWE",
> 	[EMULATED_RFI_EXITS] =      "EMUL_RFI",
> +	[EMULATED_RFMCI_EXITS] =    "EMUL_RFMCI",
> 	[DEC_EXITS] =               "DEC",
> 	[EXT_INTR_EXITS] =          "EXTINT",
> 	[HALT_WAKEUP] =             "HALT",
> -- 
> 1.7.3.4
> 
> 
> --
> 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

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Alexander Graf]

On 03.07.2013, at 15:30, Mihai Caraman wrote:

> Some guests are making use of return from machine check instruction
> to do crazy things even though the 64-bit kernel doesn't handle yet
> this interrupt. Emulate MCSRR0/1 SPR and rfmci instruction =
accordingly.
>=20
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> ---
> arch/powerpc/include/asm/kvm_host.h |    1 +
> arch/powerpc/kvm/booke_emulate.c    |   25 +++++++++++++++++++++++++
> arch/powerpc/kvm/timing.c           |    1 +
> 3 files changed, 27 insertions(+), 0 deletions(-)
>=20
> diff --git a/arch/powerpc/include/asm/kvm_host.h =
b/arch/powerpc/include/asm/kvm_host.h
> index af326cd..0466789 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -148,6 +148,7 @@ enum kvm_exit_types {
> 	EMULATED_TLBWE_EXITS,
> 	EMULATED_RFI_EXITS,
> 	EMULATED_RFCI_EXITS,
> +	EMULATED_RFMCI_EXITS,

I would quite frankly prefer to see us abandon the whole exit timing =
framework in the kernel and instead use trace points. Then we don't have =
to maintain all of this randomly exercised code.

FWIW I think in this case however, treating RFMCI the same as RFI or =
random "instruction emulation" shouldn't hurt. This whole table is only =
about timing measurements. If you want to know for real what's going on, =
use trace points.

Otherwise looks good.


Alex

> 	DEC_EXITS,
> 	EXT_INTR_EXITS,
> 	HALT_WAKEUP,
> diff --git a/arch/powerpc/kvm/booke_emulate.c =
b/arch/powerpc/kvm/booke_emulate.c
> index 27a4b28..aaff1b7 100644
> --- a/arch/powerpc/kvm/booke_emulate.c
> +++ b/arch/powerpc/kvm/booke_emulate.c
> @@ -23,6 +23,7 @@
>=20
> #include "booke.h"
>=20
> +#define OP_19_XOP_RFMCI   38
> #define OP_19_XOP_RFI     50
> #define OP_19_XOP_RFCI    51
>=20
> @@ -43,6 +44,12 @@ static void kvmppc_emul_rfci(struct kvm_vcpu *vcpu)
> 	kvmppc_set_msr(vcpu, vcpu->arch.csrr1);
> }
>=20
> +static void kvmppc_emul_rfmci(struct kvm_vcpu *vcpu)
> +{
> +	vcpu->arch.pc =3D vcpu->arch.mcsrr0;
> +	kvmppc_set_msr(vcpu, vcpu->arch.mcsrr1);
> +}
> +
> int kvmppc_booke_emulate_op(struct kvm_run *run, struct kvm_vcpu =
*vcpu,
>                             unsigned int inst, int *advance)
> {
> @@ -65,6 +72,12 @@ int kvmppc_booke_emulate_op(struct kvm_run *run, =
struct kvm_vcpu *vcpu,
> 			*advance =3D 0;
> 			break;
>=20
> +		case OP_19_XOP_RFMCI:
> +			kvmppc_emul_rfmci(vcpu);
> +			kvmppc_set_exit_type(vcpu, =
EMULATED_RFMCI_EXITS);
> +			*advance =3D 0;
> +			break;
> +
> 		default:
> 			emulated =3D EMULATE_FAIL;
> 			break;
> @@ -138,6 +151,12 @@ int kvmppc_booke_emulate_mtspr(struct kvm_vcpu =
*vcpu, int sprn, ulong spr_val)
> 	case SPRN_DBCR1:
> 		vcpu->arch.dbg_reg.dbcr1 =3D spr_val;
> 		break;
> +	case SPRN_MCSRR0:
> +		vcpu->arch.mcsrr0 =3D spr_val;
> +		break;
> +	case SPRN_MCSRR1:
> +		vcpu->arch.mcsrr1 =3D spr_val;
> +		break;
> 	case SPRN_DBSR:
> 		vcpu->arch.dbsr &=3D ~spr_val;
> 		break;
> @@ -284,6 +303,12 @@ int kvmppc_booke_emulate_mfspr(struct kvm_vcpu =
*vcpu, int sprn, ulong *spr_val)
> 	case SPRN_DBCR1:
> 		*spr_val =3D vcpu->arch.dbg_reg.dbcr1;
> 		break;
> +	case SPRN_MCSRR0:
> +		*spr_val =3D vcpu->arch.mcsrr0;
> +		break;
> +	case SPRN_MCSRR1:
> +		*spr_val =3D vcpu->arch.mcsrr1;
> +		break;
> 	case SPRN_DBSR:
> 		*spr_val =3D vcpu->arch.dbsr;
> 		break;
> diff --git a/arch/powerpc/kvm/timing.c b/arch/powerpc/kvm/timing.c
> index c392d26..670f63d 100644
> --- a/arch/powerpc/kvm/timing.c
> +++ b/arch/powerpc/kvm/timing.c
> @@ -129,6 +129,7 @@ static const char =
*kvm_exit_names[__NUMBER_OF_KVM_EXIT_TYPES] =3D {
> 	[EMULATED_TLBSX_EXITS] =3D    "EMUL_TLBSX",
> 	[EMULATED_TLBWE_EXITS] =3D    "EMUL_TLBWE",
> 	[EMULATED_RFI_EXITS] =3D      "EMUL_RFI",
> +	[EMULATED_RFMCI_EXITS] =3D    "EMUL_RFMCI",
> 	[DEC_EXITS] =3D               "DEC",
> 	[EXT_INTR_EXITS] =3D          "EXTINT",
> 	[HALT_WAKEUP] =3D             "HALT",
> --=20
> 1.7.3.4
>=20
>=20
> --
> 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

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Scott Wood]

On 07/08/2013 01:45:58 PM, Alexander Graf wrote:
> 
> On 03.07.2013, at 15:30, Mihai Caraman wrote:
> 
> > Some guests are making use of return from machine check instruction
> > to do crazy things even though the 64-bit kernel doesn't handle yet
> > this interrupt. Emulate MCSRR0/1 SPR and rfmci instruction  
> accordingly.
> >
> > Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> > ---
> > arch/powerpc/include/asm/kvm_host.h |    1 +
> > arch/powerpc/kvm/booke_emulate.c    |   25 +++++++++++++++++++++++++
> > arch/powerpc/kvm/timing.c           |    1 +
> > 3 files changed, 27 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/kvm_host.h  
> b/arch/powerpc/include/asm/kvm_host.h
> > index af326cd..0466789 100644
> > --- a/arch/powerpc/include/asm/kvm_host.h
> > +++ b/arch/powerpc/include/asm/kvm_host.h
> > @@ -148,6 +148,7 @@ enum kvm_exit_types {
> > 	EMULATED_TLBWE_EXITS,
> > 	EMULATED_RFI_EXITS,
> > 	EMULATED_RFCI_EXITS,
> > +	EMULATED_RFMCI_EXITS,
> 
> I would quite frankly prefer to see us abandon the whole exit timing  
> framework in the kernel and instead use trace points. Then we don't  
> have to maintain all of this randomly exercised code.

Would this map well to tracepoints?  We're not trying to track discrete  
events, so much as accumulated time spent in different areas.

-Scott

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Scott Wood]

On 07/08/2013 01:45:58 PM, Alexander Graf wrote:
> 
> On 03.07.2013, at 15:30, Mihai Caraman wrote:
> 
> > Some guests are making use of return from machine check instruction
> > to do crazy things even though the 64-bit kernel doesn't handle yet
> > this interrupt. Emulate MCSRR0/1 SPR and rfmci instruction  
> accordingly.
> >
> > Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> > ---
> > arch/powerpc/include/asm/kvm_host.h |    1 +
> > arch/powerpc/kvm/booke_emulate.c    |   25 +++++++++++++++++++++++++
> > arch/powerpc/kvm/timing.c           |    1 +
> > 3 files changed, 27 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/kvm_host.h  
> b/arch/powerpc/include/asm/kvm_host.h
> > index af326cd..0466789 100644
> > --- a/arch/powerpc/include/asm/kvm_host.h
> > +++ b/arch/powerpc/include/asm/kvm_host.h
> > @@ -148,6 +148,7 @@ enum kvm_exit_types {
> > 	EMULATED_TLBWE_EXITS,
> > 	EMULATED_RFI_EXITS,
> > 	EMULATED_RFCI_EXITS,
> > +	EMULATED_RFMCI_EXITS,
> 
> I would quite frankly prefer to see us abandon the whole exit timing  
> framework in the kernel and instead use trace points. Then we don't  
> have to maintain all of this randomly exercised code.

Would this map well to tracepoints?  We're not trying to track discrete  
events, so much as accumulated time spent in different areas.

-Scott

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Scott Wood]

On 07/08/2013 01:45:58 PM, Alexander Graf wrote:
>=20
> On 03.07.2013, at 15:30, Mihai Caraman wrote:
>=20
> > Some guests are making use of return from machine check instruction
> > to do crazy things even though the 64-bit kernel doesn't handle yet
> > this interrupt. Emulate MCSRR0/1 SPR and rfmci instruction =20
> accordingly.
> >
> > Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> > ---
> > arch/powerpc/include/asm/kvm_host.h |    1 +
> > arch/powerpc/kvm/booke_emulate.c    |   25 +++++++++++++++++++++++++
> > arch/powerpc/kvm/timing.c           |    1 +
> > 3 files changed, 27 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/kvm_host.h =20
> b/arch/powerpc/include/asm/kvm_host.h
> > index af326cd..0466789 100644
> > --- a/arch/powerpc/include/asm/kvm_host.h
> > +++ b/arch/powerpc/include/asm/kvm_host.h
> > @@ -148,6 +148,7 @@ enum kvm_exit_types {
> > 	EMULATED_TLBWE_EXITS,
> > 	EMULATED_RFI_EXITS,
> > 	EMULATED_RFCI_EXITS,
> > +	EMULATED_RFMCI_EXITS,
>=20
> I would quite frankly prefer to see us abandon the whole exit timing =20
> framework in the kernel and instead use trace points. Then we don't =20
> have to maintain all of this randomly exercised code.

Would this map well to tracepoints?  We're not trying to track discrete =20
events, so much as accumulated time spent in different areas.

-Scott=

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Alexander Graf]

On 07/09/2013 07:16 PM, Scott Wood wrote:
> On 07/08/2013 01:45:58 PM, Alexander Graf wrote:
>>
>> On 03.07.2013, at 15:30, Mihai Caraman wrote:
>>
>> > Some guests are making use of return from machine check instruction
>> > to do crazy things even though the 64-bit kernel doesn't handle yet
>> > this interrupt. Emulate MCSRR0/1 SPR and rfmci instruction 
>> accordingly.
>> >
>> > Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
>> > ---
>> > arch/powerpc/include/asm/kvm_host.h |    1 +
>> > arch/powerpc/kvm/booke_emulate.c    |   25 +++++++++++++++++++++++++
>> > arch/powerpc/kvm/timing.c           |    1 +
>> > 3 files changed, 27 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/arch/powerpc/include/asm/kvm_host.h 
>> b/arch/powerpc/include/asm/kvm_host.h
>> > index af326cd..0466789 100644
>> > --- a/arch/powerpc/include/asm/kvm_host.h
>> > +++ b/arch/powerpc/include/asm/kvm_host.h
>> > @@ -148,6 +148,7 @@ enum kvm_exit_types {
>> >     EMULATED_TLBWE_EXITS,
>> >     EMULATED_RFI_EXITS,
>> >     EMULATED_RFCI_EXITS,
>> > +    EMULATED_RFMCI_EXITS,
>>
>> I would quite frankly prefer to see us abandon the whole exit timing 
>> framework in the kernel and instead use trace points. Then we don't 
>> have to maintain all of this randomly exercised code.
>
> Would this map well to tracepoints?  We're not trying to track 
> discrete events, so much as accumulated time spent in different areas.

I think so. We'd just have to emit tracepoints as soon as we enter 
handle_exit and in prepare_to_enter. Then a user space program should 
have everything it needs to create statistics out of that. It would 
certainly simplify the entry/exit path.


Alex

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Alexander Graf]

On 07/09/2013 07:16 PM, Scott Wood wrote:
> On 07/08/2013 01:45:58 PM, Alexander Graf wrote:
>>
>> On 03.07.2013, at 15:30, Mihai Caraman wrote:
>>
>> > Some guests are making use of return from machine check instruction
>> > to do crazy things even though the 64-bit kernel doesn't handle yet
>> > this interrupt. Emulate MCSRR0/1 SPR and rfmci instruction 
>> accordingly.
>> >
>> > Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
>> > ---
>> > arch/powerpc/include/asm/kvm_host.h |    1 +
>> > arch/powerpc/kvm/booke_emulate.c    |   25 +++++++++++++++++++++++++
>> > arch/powerpc/kvm/timing.c           |    1 +
>> > 3 files changed, 27 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/arch/powerpc/include/asm/kvm_host.h 
>> b/arch/powerpc/include/asm/kvm_host.h
>> > index af326cd..0466789 100644
>> > --- a/arch/powerpc/include/asm/kvm_host.h
>> > +++ b/arch/powerpc/include/asm/kvm_host.h
>> > @@ -148,6 +148,7 @@ enum kvm_exit_types {
>> >     EMULATED_TLBWE_EXITS,
>> >     EMULATED_RFI_EXITS,
>> >     EMULATED_RFCI_EXITS,
>> > +    EMULATED_RFMCI_EXITS,
>>
>> I would quite frankly prefer to see us abandon the whole exit timing 
>> framework in the kernel and instead use trace points. Then we don't 
>> have to maintain all of this randomly exercised code.
>
> Would this map well to tracepoints?  We're not trying to track 
> discrete events, so much as accumulated time spent in different areas.

I think so. We'd just have to emit tracepoints as soon as we enter 
handle_exit and in prepare_to_enter. Then a user space program should 
have everything it needs to create statistics out of that. It would 
certainly simplify the entry/exit path.


Alex

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Alexander Graf]

On 07/09/2013 07:16 PM, Scott Wood wrote:
> On 07/08/2013 01:45:58 PM, Alexander Graf wrote:
>>
>> On 03.07.2013, at 15:30, Mihai Caraman wrote:
>>
>> > Some guests are making use of return from machine check instruction
>> > to do crazy things even though the 64-bit kernel doesn't handle yet
>> > this interrupt. Emulate MCSRR0/1 SPR and rfmci instruction 
>> accordingly.
>> >
>> > Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
>> > ---
>> > arch/powerpc/include/asm/kvm_host.h |    1 +
>> > arch/powerpc/kvm/booke_emulate.c    |   25 +++++++++++++++++++++++++
>> > arch/powerpc/kvm/timing.c           |    1 +
>> > 3 files changed, 27 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/arch/powerpc/include/asm/kvm_host.h 
>> b/arch/powerpc/include/asm/kvm_host.h
>> > index af326cd..0466789 100644
>> > --- a/arch/powerpc/include/asm/kvm_host.h
>> > +++ b/arch/powerpc/include/asm/kvm_host.h
>> > @@ -148,6 +148,7 @@ enum kvm_exit_types {
>> >     EMULATED_TLBWE_EXITS,
>> >     EMULATED_RFI_EXITS,
>> >     EMULATED_RFCI_EXITS,
>> > +    EMULATED_RFMCI_EXITS,
>>
>> I would quite frankly prefer to see us abandon the whole exit timing 
>> framework in the kernel and instead use trace points. Then we don't 
>> have to maintain all of this randomly exercised code.
>
> Would this map well to tracepoints?  We're not trying to track 
> discrete events, so much as accumulated time spent in different areas.

I think so. We'd just have to emit tracepoints as soon as we enter 
handle_exit and in prepare_to_enter. Then a user space program should 
have everything it needs to create statistics out of that. It would 
certainly simplify the entry/exit path.


Alex

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Scott Wood]

On 07/09/2013 12:46:32 PM, Alexander Graf wrote:
> On 07/09/2013 07:16 PM, Scott Wood wrote:
>> On 07/08/2013 01:45:58 PM, Alexander Graf wrote:
>>> 
>>> On 03.07.2013, at 15:30, Mihai Caraman wrote:
>>> 
>>> > Some guests are making use of return from machine check  
>>> instruction
>>> > to do crazy things even though the 64-bit kernel doesn't handle  
>>> yet
>>> > this interrupt. Emulate MCSRR0/1 SPR and rfmci instruction  
>>> accordingly.
>>> >
>>> > Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
>>> > ---
>>> > arch/powerpc/include/asm/kvm_host.h |    1 +
>>> > arch/powerpc/kvm/booke_emulate.c    |   25  
>>> +++++++++++++++++++++++++
>>> > arch/powerpc/kvm/timing.c           |    1 +
>>> > 3 files changed, 27 insertions(+), 0 deletions(-)
>>> >
>>> > diff --git a/arch/powerpc/include/asm/kvm_host.h  
>>> b/arch/powerpc/include/asm/kvm_host.h
>>> > index af326cd..0466789 100644
>>> > --- a/arch/powerpc/include/asm/kvm_host.h
>>> > +++ b/arch/powerpc/include/asm/kvm_host.h
>>> > @@ -148,6 +148,7 @@ enum kvm_exit_types {
>>> >     EMULATED_TLBWE_EXITS,
>>> >     EMULATED_RFI_EXITS,
>>> >     EMULATED_RFCI_EXITS,
>>> > +    EMULATED_RFMCI_EXITS,
>>> 
>>> I would quite frankly prefer to see us abandon the whole exit  
>>> timing framework in the kernel and instead use trace points. Then  
>>> we don't have to maintain all of this randomly exercised code.
>> 
>> Would this map well to tracepoints?  We're not trying to track  
>> discrete events, so much as accumulated time spent in different  
>> areas.
> 
> I think so. We'd just have to emit tracepoints as soon as we enter  
> handle_exit and in prepare_to_enter. Then a user space program should  
> have everything it needs to create statistics out of that. It would  
> certainly simplify the entry/exit path.

I was hoping that wasn't going to be your answer. :-)

Such a change would introduce a new dependency, more complexity, and  
the possibility for bad totals to result from a ring buffer filling  
faster than userspace can drain it.

I also don't see how it would simplify entry/exit, since we'd still  
need to take timestamps in the same places, in order to record a final  
event that says how long a particular event took.

-Scott

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Scott Wood]

On 07/09/2013 12:46:32 PM, Alexander Graf wrote:
> On 07/09/2013 07:16 PM, Scott Wood wrote:
>> On 07/08/2013 01:45:58 PM, Alexander Graf wrote:
>>> 
>>> On 03.07.2013, at 15:30, Mihai Caraman wrote:
>>> 
>>> > Some guests are making use of return from machine check  
>>> instruction
>>> > to do crazy things even though the 64-bit kernel doesn't handle  
>>> yet
>>> > this interrupt. Emulate MCSRR0/1 SPR and rfmci instruction  
>>> accordingly.
>>> >
>>> > Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
>>> > ---
>>> > arch/powerpc/include/asm/kvm_host.h |    1 +
>>> > arch/powerpc/kvm/booke_emulate.c    |   25  
>>> +++++++++++++++++++++++++
>>> > arch/powerpc/kvm/timing.c           |    1 +
>>> > 3 files changed, 27 insertions(+), 0 deletions(-)
>>> >
>>> > diff --git a/arch/powerpc/include/asm/kvm_host.h  
>>> b/arch/powerpc/include/asm/kvm_host.h
>>> > index af326cd..0466789 100644
>>> > --- a/arch/powerpc/include/asm/kvm_host.h
>>> > +++ b/arch/powerpc/include/asm/kvm_host.h
>>> > @@ -148,6 +148,7 @@ enum kvm_exit_types {
>>> >     EMULATED_TLBWE_EXITS,
>>> >     EMULATED_RFI_EXITS,
>>> >     EMULATED_RFCI_EXITS,
>>> > +    EMULATED_RFMCI_EXITS,
>>> 
>>> I would quite frankly prefer to see us abandon the whole exit  
>>> timing framework in the kernel and instead use trace points. Then  
>>> we don't have to maintain all of this randomly exercised code.
>> 
>> Would this map well to tracepoints?  We're not trying to track  
>> discrete events, so much as accumulated time spent in different  
>> areas.
> 
> I think so. We'd just have to emit tracepoints as soon as we enter  
> handle_exit and in prepare_to_enter. Then a user space program should  
> have everything it needs to create statistics out of that. It would  
> certainly simplify the entry/exit path.

I was hoping that wasn't going to be your answer. :-)

Such a change would introduce a new dependency, more complexity, and  
the possibility for bad totals to result from a ring buffer filling  
faster than userspace can drain it.

I also don't see how it would simplify entry/exit, since we'd still  
need to take timestamps in the same places, in order to record a final  
event that says how long a particular event took.

-Scott

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Scott Wood]

On 07/09/2013 12:46:32 PM, Alexander Graf wrote:
> On 07/09/2013 07:16 PM, Scott Wood wrote:
>> On 07/08/2013 01:45:58 PM, Alexander Graf wrote:
>>>=20
>>> On 03.07.2013, at 15:30, Mihai Caraman wrote:
>>>=20
>>> > Some guests are making use of return from machine check =20
>>> instruction
>>> > to do crazy things even though the 64-bit kernel doesn't handle =20
>>> yet
>>> > this interrupt. Emulate MCSRR0/1 SPR and rfmci instruction =20
>>> accordingly.
>>> >
>>> > Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
>>> > ---
>>> > arch/powerpc/include/asm/kvm_host.h |    1 +
>>> > arch/powerpc/kvm/booke_emulate.c    |   25 =20
>>> +++++++++++++++++++++++++
>>> > arch/powerpc/kvm/timing.c           |    1 +
>>> > 3 files changed, 27 insertions(+), 0 deletions(-)
>>> >
>>> > diff --git a/arch/powerpc/include/asm/kvm_host.h =20
>>> b/arch/powerpc/include/asm/kvm_host.h
>>> > index af326cd..0466789 100644
>>> > --- a/arch/powerpc/include/asm/kvm_host.h
>>> > +++ b/arch/powerpc/include/asm/kvm_host.h
>>> > @@ -148,6 +148,7 @@ enum kvm_exit_types {
>>> >     EMULATED_TLBWE_EXITS,
>>> >     EMULATED_RFI_EXITS,
>>> >     EMULATED_RFCI_EXITS,
>>> > +    EMULATED_RFMCI_EXITS,
>>>=20
>>> I would quite frankly prefer to see us abandon the whole exit =20
>>> timing framework in the kernel and instead use trace points. Then =20
>>> we don't have to maintain all of this randomly exercised code.
>>=20
>> Would this map well to tracepoints?  We're not trying to track =20
>> discrete events, so much as accumulated time spent in different =20
>> areas.
>=20
> I think so. We'd just have to emit tracepoints as soon as we enter =20
> handle_exit and in prepare_to_enter. Then a user space program should =20
> have everything it needs to create statistics out of that. It would =20
> certainly simplify the entry/exit path.

I was hoping that wasn't going to be your answer. :-)

Such a change would introduce a new dependency, more complexity, and =20
the possibility for bad totals to result from a ring buffer filling =20
faster than userspace can drain it.

I also don't see how it would simplify entry/exit, since we'd still =20
need to take timestamps in the same places, in order to record a final =20
event that says how long a particular event took.

-Scott=

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Alexander Graf]

On 09.07.2013, at 20:29, Scott Wood wrote:

> On 07/09/2013 12:46:32 PM, Alexander Graf wrote:
>> On 07/09/2013 07:16 PM, Scott Wood wrote:
>>> On 07/08/2013 01:45:58 PM, Alexander Graf wrote:
>>>> On 03.07.2013, at 15:30, Mihai Caraman wrote:
>>>> > Some guests are making use of return from machine check instruction
>>>> > to do crazy things even though the 64-bit kernel doesn't handle yet
>>>> > this interrupt. Emulate MCSRR0/1 SPR and rfmci instruction accordingly.
>>>> >
>>>> > Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
>>>> > ---
>>>> > arch/powerpc/include/asm/kvm_host.h |    1 +
>>>> > arch/powerpc/kvm/booke_emulate.c    |   25 +++++++++++++++++++++++++
>>>> > arch/powerpc/kvm/timing.c           |    1 +
>>>> > 3 files changed, 27 insertions(+), 0 deletions(-)
>>>> >
>>>> > diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
>>>> > index af326cd..0466789 100644
>>>> > --- a/arch/powerpc/include/asm/kvm_host.h
>>>> > +++ b/arch/powerpc/include/asm/kvm_host.h
>>>> > @@ -148,6 +148,7 @@ enum kvm_exit_types {
>>>> >     EMULATED_TLBWE_EXITS,
>>>> >     EMULATED_RFI_EXITS,
>>>> >     EMULATED_RFCI_EXITS,
>>>> > +    EMULATED_RFMCI_EXITS,
>>>> I would quite frankly prefer to see us abandon the whole exit timing framework in the kernel and instead use trace points. Then we don't have to maintain all of this randomly exercised code.
>>> Would this map well to tracepoints?  We're not trying to track discrete events, so much as accumulated time spent in different areas.
>> I think so. We'd just have to emit tracepoints as soon as we enter handle_exit and in prepare_to_enter. Then a user space program should have everything it needs to create statistics out of that. It would certainly simplify the entry/exit path.
> 
> I was hoping that wasn't going to be your answer. :-)
> 
> Such a change would introduce a new dependency, more complexity, and the possibility for bad totals to result from a ring buffer filling faster than userspace can drain it.

Well, at least it would allow for optional tracing :). Today you have to change a compile flag to enable / disable timing stats.

> 
> I also don't see how it would simplify entry/exit, since we'd still need to take timestamps in the same places, in order to record a final event that says how long a particular event took.

Not sure I understand. What the timing stats do is that they measure the time between [exit ... entry], right? We'd do the same thing, just all in C code. That means we would become slightly less accurate, but gain dynamic enabling of the traces and get rid of all the timing stat asm code.


Alex

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Alexander Graf]

On 09.07.2013, at 20:29, Scott Wood wrote:

> On 07/09/2013 12:46:32 PM, Alexander Graf wrote:
>> On 07/09/2013 07:16 PM, Scott Wood wrote:
>>> On 07/08/2013 01:45:58 PM, Alexander Graf wrote:
>>>> On 03.07.2013, at 15:30, Mihai Caraman wrote:
>>>> > Some guests are making use of return from machine check instruction
>>>> > to do crazy things even though the 64-bit kernel doesn't handle yet
>>>> > this interrupt. Emulate MCSRR0/1 SPR and rfmci instruction accordingly.
>>>> >
>>>> > Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
>>>> > ---
>>>> > arch/powerpc/include/asm/kvm_host.h |    1 +
>>>> > arch/powerpc/kvm/booke_emulate.c    |   25 +++++++++++++++++++++++++
>>>> > arch/powerpc/kvm/timing.c           |    1 +
>>>> > 3 files changed, 27 insertions(+), 0 deletions(-)
>>>> >
>>>> > diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
>>>> > index af326cd..0466789 100644
>>>> > --- a/arch/powerpc/include/asm/kvm_host.h
>>>> > +++ b/arch/powerpc/include/asm/kvm_host.h
>>>> > @@ -148,6 +148,7 @@ enum kvm_exit_types {
>>>> >     EMULATED_TLBWE_EXITS,
>>>> >     EMULATED_RFI_EXITS,
>>>> >     EMULATED_RFCI_EXITS,
>>>> > +    EMULATED_RFMCI_EXITS,
>>>> I would quite frankly prefer to see us abandon the whole exit timing framework in the kernel and instead use trace points. Then we don't have to maintain all of this randomly exercised code.
>>> Would this map well to tracepoints?  We're not trying to track discrete events, so much as accumulated time spent in different areas.
>> I think so. We'd just have to emit tracepoints as soon as we enter handle_exit and in prepare_to_enter. Then a user space program should have everything it needs to create statistics out of that. It would certainly simplify the entry/exit path.
> 
> I was hoping that wasn't going to be your answer. :-)
> 
> Such a change would introduce a new dependency, more complexity, and the possibility for bad totals to result from a ring buffer filling faster than userspace can drain it.

Well, at least it would allow for optional tracing :). Today you have to change a compile flag to enable / disable timing stats.

> 
> I also don't see how it would simplify entry/exit, since we'd still need to take timestamps in the same places, in order to record a final event that says how long a particular event took.

Not sure I understand. What the timing stats do is that they measure the time between [exit ... entry], right? We'd do the same thing, just all in C code. That means we would become slightly less accurate, but gain dynamic enabling of the traces and get rid of all the timing stat asm code.


Alex

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Alexander Graf]

On 09.07.2013, at 20:29, Scott Wood wrote:

> On 07/09/2013 12:46:32 PM, Alexander Graf wrote:
>> On 07/09/2013 07:16 PM, Scott Wood wrote:
>>> On 07/08/2013 01:45:58 PM, Alexander Graf wrote:
>>>> On 03.07.2013, at 15:30, Mihai Caraman wrote:
>>>> > Some guests are making use of return from machine check =
instruction
>>>> > to do crazy things even though the 64-bit kernel doesn't handle =
yet
>>>> > this interrupt. Emulate MCSRR0/1 SPR and rfmci instruction =
accordingly.
>>>> >
>>>> > Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
>>>> > ---
>>>> > arch/powerpc/include/asm/kvm_host.h |    1 +
>>>> > arch/powerpc/kvm/booke_emulate.c    |   25 =
+++++++++++++++++++++++++
>>>> > arch/powerpc/kvm/timing.c           |    1 +
>>>> > 3 files changed, 27 insertions(+), 0 deletions(-)
>>>> >
>>>> > diff --git a/arch/powerpc/include/asm/kvm_host.h =
b/arch/powerpc/include/asm/kvm_host.h
>>>> > index af326cd..0466789 100644
>>>> > --- a/arch/powerpc/include/asm/kvm_host.h
>>>> > +++ b/arch/powerpc/include/asm/kvm_host.h
>>>> > @@ -148,6 +148,7 @@ enum kvm_exit_types {
>>>> >     EMULATED_TLBWE_EXITS,
>>>> >     EMULATED_RFI_EXITS,
>>>> >     EMULATED_RFCI_EXITS,
>>>> > +    EMULATED_RFMCI_EXITS,
>>>> I would quite frankly prefer to see us abandon the whole exit =
timing framework in the kernel and instead use trace points. Then we =
don't have to maintain all of this randomly exercised code.
>>> Would this map well to tracepoints?  We're not trying to track =
discrete events, so much as accumulated time spent in different areas.
>> I think so. We'd just have to emit tracepoints as soon as we enter =
handle_exit and in prepare_to_enter. Then a user space program should =
have everything it needs to create statistics out of that. It would =
certainly simplify the entry/exit path.
>=20
> I was hoping that wasn't going to be your answer. :-)
>=20
> Such a change would introduce a new dependency, more complexity, and =
the possibility for bad totals to result from a ring buffer filling =
faster than userspace can drain it.

Well, at least it would allow for optional tracing :). Today you have to =
change a compile flag to enable / disable timing stats.

>=20
> I also don't see how it would simplify entry/exit, since we'd still =
need to take timestamps in the same places, in order to record a final =
event that says how long a particular event took.

Not sure I understand. What the timing stats do is that they measure the =
time between [exit ... entry], right? We'd do the same thing, just all =
in C code. That means we would become slightly less accurate, but gain =
dynamic enabling of the traces and get rid of all the timing stat asm =
code.


Alex

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Scott Wood]

On 07/09/2013 04:49:32 PM, Alexander Graf wrote:
> 
> On 09.07.2013, at 20:29, Scott Wood wrote:
> 
> > On 07/09/2013 12:46:32 PM, Alexander Graf wrote:
> >> On 07/09/2013 07:16 PM, Scott Wood wrote:
> >>> On 07/08/2013 01:45:58 PM, Alexander Graf wrote:
> >>>> On 03.07.2013, at 15:30, Mihai Caraman wrote:
> >>>> > Some guests are making use of return from machine check  
> instruction
> >>>> > to do crazy things even though the 64-bit kernel doesn't  
> handle yet
> >>>> > this interrupt. Emulate MCSRR0/1 SPR and rfmci instruction  
> accordingly.
> >>>> >
> >>>> > Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> >>>> > ---
> >>>> > arch/powerpc/include/asm/kvm_host.h |    1 +
> >>>> > arch/powerpc/kvm/booke_emulate.c    |   25  
> +++++++++++++++++++++++++
> >>>> > arch/powerpc/kvm/timing.c           |    1 +
> >>>> > 3 files changed, 27 insertions(+), 0 deletions(-)
> >>>> >
> >>>> > diff --git a/arch/powerpc/include/asm/kvm_host.h  
> b/arch/powerpc/include/asm/kvm_host.h
> >>>> > index af326cd..0466789 100644
> >>>> > --- a/arch/powerpc/include/asm/kvm_host.h
> >>>> > +++ b/arch/powerpc/include/asm/kvm_host.h
> >>>> > @@ -148,6 +148,7 @@ enum kvm_exit_types {
> >>>> >     EMULATED_TLBWE_EXITS,
> >>>> >     EMULATED_RFI_EXITS,
> >>>> >     EMULATED_RFCI_EXITS,
> >>>> > +    EMULATED_RFMCI_EXITS,
> >>>> I would quite frankly prefer to see us abandon the whole exit  
> timing framework in the kernel and instead use trace points. Then we  
> don't have to maintain all of this randomly exercised code.
> >>> Would this map well to tracepoints?  We're not trying to track  
> discrete events, so much as accumulated time spent in different areas.
> >> I think so. We'd just have to emit tracepoints as soon as we enter  
> handle_exit and in prepare_to_enter. Then a user space program should  
> have everything it needs to create statistics out of that. It would  
> certainly simplify the entry/exit path.
> >
> > I was hoping that wasn't going to be your answer. :-)
> >
> > Such a change would introduce a new dependency, more complexity,  
> and the possibility for bad totals to result from a ring buffer  
> filling faster than userspace can drain it.
> 
> Well, at least it would allow for optional tracing :). Today you have  
> to change a compile flag to enable / disable timing stats.
> 
> >
> > I also don't see how it would simplify entry/exit, since we'd still  
> need to take timestamps in the same places, in order to record a  
> final event that says how long a particular event took.
> 
> Not sure I understand. What the timing stats do is that they measure  
> the time between [exit ... entry], right? We'd do the same thing,  
> just all in C code. That means we would become slightly less  
> accurate, but gain dynamic enabling of the traces and get rid of all  
> the timing stat asm code.

Compile-time enabling bothers me less than a loss of accuracy (not just  
a small loss by moving into C code, but a potential for a large loss if  
we overflow the buffer) and a dependency on a userspace tool (both in  
terms of the tool needing to be written, and in the hassle of ensuring  
that it's present in the root filesystem of whatever system I'm  
testing).  And the whole mechanism will be more complicated.

Lots of debug options are enabled at build time; why must this be  
different?

-Scott

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Scott Wood]

On 07/09/2013 04:49:32 PM, Alexander Graf wrote:
> 
> On 09.07.2013, at 20:29, Scott Wood wrote:
> 
> > On 07/09/2013 12:46:32 PM, Alexander Graf wrote:
> >> On 07/09/2013 07:16 PM, Scott Wood wrote:
> >>> On 07/08/2013 01:45:58 PM, Alexander Graf wrote:
> >>>> On 03.07.2013, at 15:30, Mihai Caraman wrote:
> >>>> > Some guests are making use of return from machine check  
> instruction
> >>>> > to do crazy things even though the 64-bit kernel doesn't  
> handle yet
> >>>> > this interrupt. Emulate MCSRR0/1 SPR and rfmci instruction  
> accordingly.
> >>>> >
> >>>> > Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> >>>> > ---
> >>>> > arch/powerpc/include/asm/kvm_host.h |    1 +
> >>>> > arch/powerpc/kvm/booke_emulate.c    |   25  
> +++++++++++++++++++++++++
> >>>> > arch/powerpc/kvm/timing.c           |    1 +
> >>>> > 3 files changed, 27 insertions(+), 0 deletions(-)
> >>>> >
> >>>> > diff --git a/arch/powerpc/include/asm/kvm_host.h  
> b/arch/powerpc/include/asm/kvm_host.h
> >>>> > index af326cd..0466789 100644
> >>>> > --- a/arch/powerpc/include/asm/kvm_host.h
> >>>> > +++ b/arch/powerpc/include/asm/kvm_host.h
> >>>> > @@ -148,6 +148,7 @@ enum kvm_exit_types {
> >>>> >     EMULATED_TLBWE_EXITS,
> >>>> >     EMULATED_RFI_EXITS,
> >>>> >     EMULATED_RFCI_EXITS,
> >>>> > +    EMULATED_RFMCI_EXITS,
> >>>> I would quite frankly prefer to see us abandon the whole exit  
> timing framework in the kernel and instead use trace points. Then we  
> don't have to maintain all of this randomly exercised code.
> >>> Would this map well to tracepoints?  We're not trying to track  
> discrete events, so much as accumulated time spent in different areas.
> >> I think so. We'd just have to emit tracepoints as soon as we enter  
> handle_exit and in prepare_to_enter. Then a user space program should  
> have everything it needs to create statistics out of that. It would  
> certainly simplify the entry/exit path.
> >
> > I was hoping that wasn't going to be your answer. :-)
> >
> > Such a change would introduce a new dependency, more complexity,  
> and the possibility for bad totals to result from a ring buffer  
> filling faster than userspace can drain it.
> 
> Well, at least it would allow for optional tracing :). Today you have  
> to change a compile flag to enable / disable timing stats.
> 
> >
> > I also don't see how it would simplify entry/exit, since we'd still  
> need to take timestamps in the same places, in order to record a  
> final event that says how long a particular event took.
> 
> Not sure I understand. What the timing stats do is that they measure  
> the time between [exit ... entry], right? We'd do the same thing,  
> just all in C code. That means we would become slightly less  
> accurate, but gain dynamic enabling of the traces and get rid of all  
> the timing stat asm code.

Compile-time enabling bothers me less than a loss of accuracy (not just  
a small loss by moving into C code, but a potential for a large loss if  
we overflow the buffer) and a dependency on a userspace tool (both in  
terms of the tool needing to be written, and in the hassle of ensuring  
that it's present in the root filesystem of whatever system I'm  
testing).  And the whole mechanism will be more complicated.

Lots of debug options are enabled at build time; why must this be  
different?

-Scott

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Scott Wood]

On 07/09/2013 04:49:32 PM, Alexander Graf wrote:
>=20
> On 09.07.2013, at 20:29, Scott Wood wrote:
>=20
> > On 07/09/2013 12:46:32 PM, Alexander Graf wrote:
> >> On 07/09/2013 07:16 PM, Scott Wood wrote:
> >>> On 07/08/2013 01:45:58 PM, Alexander Graf wrote:
> >>>> On 03.07.2013, at 15:30, Mihai Caraman wrote:
> >>>> > Some guests are making use of return from machine check =20
> instruction
> >>>> > to do crazy things even though the 64-bit kernel doesn't =20
> handle yet
> >>>> > this interrupt. Emulate MCSRR0/1 SPR and rfmci instruction =20
> accordingly.
> >>>> >
> >>>> > Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> >>>> > ---
> >>>> > arch/powerpc/include/asm/kvm_host.h |    1 +
> >>>> > arch/powerpc/kvm/booke_emulate.c    |   25 =20
> +++++++++++++++++++++++++
> >>>> > arch/powerpc/kvm/timing.c           |    1 +
> >>>> > 3 files changed, 27 insertions(+), 0 deletions(-)
> >>>> >
> >>>> > diff --git a/arch/powerpc/include/asm/kvm_host.h =20
> b/arch/powerpc/include/asm/kvm_host.h
> >>>> > index af326cd..0466789 100644
> >>>> > --- a/arch/powerpc/include/asm/kvm_host.h
> >>>> > +++ b/arch/powerpc/include/asm/kvm_host.h
> >>>> > @@ -148,6 +148,7 @@ enum kvm_exit_types {
> >>>> >     EMULATED_TLBWE_EXITS,
> >>>> >     EMULATED_RFI_EXITS,
> >>>> >     EMULATED_RFCI_EXITS,
> >>>> > +    EMULATED_RFMCI_EXITS,
> >>>> I would quite frankly prefer to see us abandon the whole exit =20
> timing framework in the kernel and instead use trace points. Then we =20
> don't have to maintain all of this randomly exercised code.
> >>> Would this map well to tracepoints?  We're not trying to track =20
> discrete events, so much as accumulated time spent in different areas.
> >> I think so. We'd just have to emit tracepoints as soon as we enter =20
> handle_exit and in prepare_to_enter. Then a user space program should =20
> have everything it needs to create statistics out of that. It would =20
> certainly simplify the entry/exit path.
> >
> > I was hoping that wasn't going to be your answer. :-)
> >
> > Such a change would introduce a new dependency, more complexity, =20
> and the possibility for bad totals to result from a ring buffer =20
> filling faster than userspace can drain it.
>=20
> Well, at least it would allow for optional tracing :). Today you have =20
> to change a compile flag to enable / disable timing stats.
>=20
> >
> > I also don't see how it would simplify entry/exit, since we'd still =20
> need to take timestamps in the same places, in order to record a =20
> final event that says how long a particular event took.
>=20
> Not sure I understand. What the timing stats do is that they measure =20
> the time between [exit ... entry], right? We'd do the same thing, =20
> just all in C code. That means we would become slightly less =20
> accurate, but gain dynamic enabling of the traces and get rid of all =20
> the timing stat asm code.

Compile-time enabling bothers me less than a loss of accuracy (not just =20
a small loss by moving into C code, but a potential for a large loss if =20
we overflow the buffer) and a dependency on a userspace tool (both in =20
terms of the tool needing to be written, and in the hassle of ensuring =20
that it's present in the root filesystem of whatever system I'm =20
testing).  And the whole mechanism will be more complicated.

Lots of debug options are enabled at build time; why must this be =20
different?

-Scott=

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Alexander Graf]

On 09.07.2013, at 23:54, Scott Wood wrote:

> On 07/09/2013 04:49:32 PM, Alexander Graf wrote:
>> On 09.07.2013, at 20:29, Scott Wood wrote:
>> > On 07/09/2013 12:46:32 PM, Alexander Graf wrote:
>> >> On 07/09/2013 07:16 PM, Scott Wood wrote:
>> >>> On 07/08/2013 01:45:58 PM, Alexander Graf wrote:
>> >>>> On 03.07.2013, at 15:30, Mihai Caraman wrote:
>> >>>> > Some guests are making use of return from machine check instruction
>> >>>> > to do crazy things even though the 64-bit kernel doesn't handle yet
>> >>>> > this interrupt. Emulate MCSRR0/1 SPR and rfmci instruction accordingly.
>> >>>> >
>> >>>> > Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
>> >>>> > ---
>> >>>> > arch/powerpc/include/asm/kvm_host.h |    1 +
>> >>>> > arch/powerpc/kvm/booke_emulate.c    |   25 +++++++++++++++++++++++++
>> >>>> > arch/powerpc/kvm/timing.c           |    1 +
>> >>>> > 3 files changed, 27 insertions(+), 0 deletions(-)
>> >>>> >
>> >>>> > diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
>> >>>> > index af326cd..0466789 100644
>> >>>> > --- a/arch/powerpc/include/asm/kvm_host.h
>> >>>> > +++ b/arch/powerpc/include/asm/kvm_host.h
>> >>>> > @@ -148,6 +148,7 @@ enum kvm_exit_types {
>> >>>> >     EMULATED_TLBWE_EXITS,
>> >>>> >     EMULATED_RFI_EXITS,
>> >>>> >     EMULATED_RFCI_EXITS,
>> >>>> > +    EMULATED_RFMCI_EXITS,
>> >>>> I would quite frankly prefer to see us abandon the whole exit timing framework in the kernel and instead use trace points. Then we don't have to maintain all of this randomly exercised code.
>> >>> Would this map well to tracepoints?  We're not trying to track discrete events, so much as accumulated time spent in different areas.
>> >> I think so. We'd just have to emit tracepoints as soon as we enter handle_exit and in prepare_to_enter. Then a user space program should have everything it needs to create statistics out of that. It would certainly simplify the entry/exit path.
>> >
>> > I was hoping that wasn't going to be your answer. :-)
>> >
>> > Such a change would introduce a new dependency, more complexity, and the possibility for bad totals to result from a ring buffer filling faster than userspace can drain it.
>> Well, at least it would allow for optional tracing :). Today you have to change a compile flag to enable / disable timing stats.
>> >
>> > I also don't see how it would simplify entry/exit, since we'd still need to take timestamps in the same places, in order to record a final event that says how long a particular event took.
>> Not sure I understand. What the timing stats do is that they measure the time between [exit ... entry], right? We'd do the same thing, just all in C code. That means we would become slightly less accurate, but gain dynamic enabling of the traces and get rid of all the timing stat asm code.
> 
> Compile-time enabling bothers me less than a loss of accuracy (not just a small loss by moving into C code, but a potential for a large loss if we overflow the buffer)

Then don't overflow the buffer. Make it large enough. IIRC ftrace improved recently to dynamically increase the buffer size too.

Steven, do I remember correctly here?

> and a dependency on a userspace tool

We already have that for kvm_stat. It's a simple python script - and you surely have python on your rootfs, no?

> (both in terms of the tool needing to be written, and in the hassle of ensuring that it's present in the root filesystem of whatever system I'm testing).  And the whole mechanism will be more complicated.

It'll also be more flexible at the same time. You could take the logs and actually check what's going on to debug issues that you're encountering for example.

We could even go as far as sharing the same tool with other architectures, so that we only have to learn how to debug things once.

> Lots of debug options are enabled at build time; why must this be different?

Because I think it's valuable as debug tool for cases where compile time switches are not the best way of debugging things. It's not a high profile thing to tackle for me tbh, but I don't really think working heavily on the timing stat thing is the correct path to walk along.


Alex

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Alexander Graf]

On 09.07.2013, at 23:54, Scott Wood wrote:

> On 07/09/2013 04:49:32 PM, Alexander Graf wrote:
>> On 09.07.2013, at 20:29, Scott Wood wrote:
>> > On 07/09/2013 12:46:32 PM, Alexander Graf wrote:
>> >> On 07/09/2013 07:16 PM, Scott Wood wrote:
>> >>> On 07/08/2013 01:45:58 PM, Alexander Graf wrote:
>> >>>> On 03.07.2013, at 15:30, Mihai Caraman wrote:
>> >>>> > Some guests are making use of return from machine check instruction
>> >>>> > to do crazy things even though the 64-bit kernel doesn't handle yet
>> >>>> > this interrupt. Emulate MCSRR0/1 SPR and rfmci instruction accordingly.
>> >>>> >
>> >>>> > Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
>> >>>> > ---
>> >>>> > arch/powerpc/include/asm/kvm_host.h |    1 +
>> >>>> > arch/powerpc/kvm/booke_emulate.c    |   25 +++++++++++++++++++++++++
>> >>>> > arch/powerpc/kvm/timing.c           |    1 +
>> >>>> > 3 files changed, 27 insertions(+), 0 deletions(-)
>> >>>> >
>> >>>> > diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
>> >>>> > index af326cd..0466789 100644
>> >>>> > --- a/arch/powerpc/include/asm/kvm_host.h
>> >>>> > +++ b/arch/powerpc/include/asm/kvm_host.h
>> >>>> > @@ -148,6 +148,7 @@ enum kvm_exit_types {
>> >>>> >     EMULATED_TLBWE_EXITS,
>> >>>> >     EMULATED_RFI_EXITS,
>> >>>> >     EMULATED_RFCI_EXITS,
>> >>>> > +    EMULATED_RFMCI_EXITS,
>> >>>> I would quite frankly prefer to see us abandon the whole exit timing framework in the kernel and instead use trace points. Then we don't have to maintain all of this randomly exercised code.
>> >>> Would this map well to tracepoints?  We're not trying to track discrete events, so much as accumulated time spent in different areas.
>> >> I think so. We'd just have to emit tracepoints as soon as we enter handle_exit and in prepare_to_enter. Then a user space program should have everything it needs to create statistics out of that. It would certainly simplify the entry/exit path.
>> >
>> > I was hoping that wasn't going to be your answer. :-)
>> >
>> > Such a change would introduce a new dependency, more complexity, and the possibility for bad totals to result from a ring buffer filling faster than userspace can drain it.
>> Well, at least it would allow for optional tracing :). Today you have to change a compile flag to enable / disable timing stats.
>> >
>> > I also don't see how it would simplify entry/exit, since we'd still need to take timestamps in the same places, in order to record a final event that says how long a particular event took.
>> Not sure I understand. What the timing stats do is that they measure the time between [exit ... entry], right? We'd do the same thing, just all in C code. That means we would become slightly less accurate, but gain dynamic enabling of the traces and get rid of all the timing stat asm code.
> 
> Compile-time enabling bothers me less than a loss of accuracy (not just a small loss by moving into C code, but a potential for a large loss if we overflow the buffer)

Then don't overflow the buffer. Make it large enough. IIRC ftrace improved recently to dynamically increase the buffer size too.

Steven, do I remember correctly here?

> and a dependency on a userspace tool

We already have that for kvm_stat. It's a simple python script - and you surely have python on your rootfs, no?

> (both in terms of the tool needing to be written, and in the hassle of ensuring that it's present in the root filesystem of whatever system I'm testing).  And the whole mechanism will be more complicated.

It'll also be more flexible at the same time. You could take the logs and actually check what's going on to debug issues that you're encountering for example.

We could even go as far as sharing the same tool with other architectures, so that we only have to learn how to debug things once.

> Lots of debug options are enabled at build time; why must this be different?

Because I think it's valuable as debug tool for cases where compile time switches are not the best way of debugging things. It's not a high profile thing to tackle for me tbh, but I don't really think working heavily on the timing stat thing is the correct path to walk along.


Alex

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Alexander Graf]

On 09.07.2013, at 23:54, Scott Wood wrote:

> On 07/09/2013 04:49:32 PM, Alexander Graf wrote:
>> On 09.07.2013, at 20:29, Scott Wood wrote:
>> > On 07/09/2013 12:46:32 PM, Alexander Graf wrote:
>> >> On 07/09/2013 07:16 PM, Scott Wood wrote:
>> >>> On 07/08/2013 01:45:58 PM, Alexander Graf wrote:
>> >>>> On 03.07.2013, at 15:30, Mihai Caraman wrote:
>> >>>> > Some guests are making use of return from machine check =
instruction
>> >>>> > to do crazy things even though the 64-bit kernel doesn't =
handle yet
>> >>>> > this interrupt. Emulate MCSRR0/1 SPR and rfmci instruction =
accordingly.
>> >>>> >
>> >>>> > Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
>> >>>> > ---
>> >>>> > arch/powerpc/include/asm/kvm_host.h |    1 +
>> >>>> > arch/powerpc/kvm/booke_emulate.c    |   25 =
+++++++++++++++++++++++++
>> >>>> > arch/powerpc/kvm/timing.c           |    1 +
>> >>>> > 3 files changed, 27 insertions(+), 0 deletions(-)
>> >>>> >
>> >>>> > diff --git a/arch/powerpc/include/asm/kvm_host.h =
b/arch/powerpc/include/asm/kvm_host.h
>> >>>> > index af326cd..0466789 100644
>> >>>> > --- a/arch/powerpc/include/asm/kvm_host.h
>> >>>> > +++ b/arch/powerpc/include/asm/kvm_host.h
>> >>>> > @@ -148,6 +148,7 @@ enum kvm_exit_types {
>> >>>> >     EMULATED_TLBWE_EXITS,
>> >>>> >     EMULATED_RFI_EXITS,
>> >>>> >     EMULATED_RFCI_EXITS,
>> >>>> > +    EMULATED_RFMCI_EXITS,
>> >>>> I would quite frankly prefer to see us abandon the whole exit =
timing framework in the kernel and instead use trace points. Then we =
don't have to maintain all of this randomly exercised code.
>> >>> Would this map well to tracepoints?  We're not trying to track =
discrete events, so much as accumulated time spent in different areas.
>> >> I think so. We'd just have to emit tracepoints as soon as we enter =
handle_exit and in prepare_to_enter. Then a user space program should =
have everything it needs to create statistics out of that. It would =
certainly simplify the entry/exit path.
>> >
>> > I was hoping that wasn't going to be your answer. :-)
>> >
>> > Such a change would introduce a new dependency, more complexity, =
and the possibility for bad totals to result from a ring buffer filling =
faster than userspace can drain it.
>> Well, at least it would allow for optional tracing :). Today you have =
to change a compile flag to enable / disable timing stats.
>> >
>> > I also don't see how it would simplify entry/exit, since we'd still =
need to take timestamps in the same places, in order to record a final =
event that says how long a particular event took.
>> Not sure I understand. What the timing stats do is that they measure =
the time between [exit ... entry], right? We'd do the same thing, just =
all in C code. That means we would become slightly less accurate, but =
gain dynamic enabling of the traces and get rid of all the timing stat =
asm code.
>=20
> Compile-time enabling bothers me less than a loss of accuracy (not =
just a small loss by moving into C code, but a potential for a large =
loss if we overflow the buffer)

Then don't overflow the buffer. Make it large enough. IIRC ftrace =
improved recently to dynamically increase the buffer size too.

Steven, do I remember correctly here?

> and a dependency on a userspace tool

We already have that for kvm_stat. It's a simple python script - and you =
surely have python on your rootfs, no?

> (both in terms of the tool needing to be written, and in the hassle of =
ensuring that it's present in the root filesystem of whatever system I'm =
testing).  And the whole mechanism will be more complicated.

It'll also be more flexible at the same time. You could take the logs =
and actually check what's going on to debug issues that you're =
encountering for example.

We could even go as far as sharing the same tool with other =
architectures, so that we only have to learn how to debug things once.

> Lots of debug options are enabled at build time; why must this be =
different?

Because I think it's valuable as debug tool for cases where compile time =
switches are not the best way of debugging things. It's not a high =
profile thing to tackle for me tbh, but I don't really think working =
heavily on the timing stat thing is the correct path to walk along.


Alex

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Scott Wood]

On 07/09/2013 05:00:26 PM, Alexander Graf wrote:
> 
> On 09.07.2013, at 23:54, Scott Wood wrote:
> 
> > On 07/09/2013 04:49:32 PM, Alexander Graf wrote:
> >> Not sure I understand. What the timing stats do is that they  
> measure the time between [exit ... entry], right? We'd do the same  
> thing, just all in C code. That means we would become slightly less  
> accurate, but gain dynamic enabling of the traces and get rid of all  
> the timing stat asm code.
> >
> > Compile-time enabling bothers me less than a loss of accuracy (not  
> just a small loss by moving into C code, but a potential for a large  
> loss if we overflow the buffer)
> 
> Then don't overflow the buffer. Make it large enough.

How large is that?  Does the tool recognize and report when overflow  
happens?

How much will the overhead of running some python script on the host,  
consuming a large volume of data, affect the results?

> IIRC ftrace improved recently to dynamically increase the buffer size  
> too.
> 
> Steven, do I remember correctly here?

Yay more complexity.

So now we get to worry about possible memory allocations happening when  
we try to log something?  Or if there is a way to do an "atomic" log,  
we're back to the "buffer might be full" situation.

> > and a dependency on a userspace tool
> 
> We already have that for kvm_stat. It's a simple python script - and  
> you surely have python on your rootfs, no?
> 
> > (both in terms of the tool needing to be written, and in the hassle  
> of ensuring that it's present in the root filesystem of whatever  
> system I'm testing).  And the whole mechanism will be more  
> complicated.
> 
> It'll also be more flexible at the same time. You could take the logs  
> and actually check what's going on to debug issues that you're  
> encountering for example.
> 
> We could even go as far as sharing the same tool with other  
> architectures, so that we only have to learn how to debug things once.

Have you encountered an actual need for this flexibility, or is it  
theoretical?

Is there common infrastructure for dealing with measuring intervals and  
tracking statistics thereof, rather than just tracking points and  
letting userspace connect the dots (though it could still do that as an  
option)?  Even if it must be done in userspace, it doesn't seem like  
something that should be KVM-specific.

> > Lots of debug options are enabled at build time; why must this be  
> different?
> 
> Because I think it's valuable as debug tool for cases where compile  
> time switches are not the best way of debugging things. It's not a  
> high profile thing to tackle for me tbh, but I don't really think  
> working heavily on the timing stat thing is the correct path to walk  
> along.

Adding new exit types isn't "working heavily" on it.

-Scott

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Scott Wood]

On 07/09/2013 05:00:26 PM, Alexander Graf wrote:
> 
> On 09.07.2013, at 23:54, Scott Wood wrote:
> 
> > On 07/09/2013 04:49:32 PM, Alexander Graf wrote:
> >> Not sure I understand. What the timing stats do is that they  
> measure the time between [exit ... entry], right? We'd do the same  
> thing, just all in C code. That means we would become slightly less  
> accurate, but gain dynamic enabling of the traces and get rid of all  
> the timing stat asm code.
> >
> > Compile-time enabling bothers me less than a loss of accuracy (not  
> just a small loss by moving into C code, but a potential for a large  
> loss if we overflow the buffer)
> 
> Then don't overflow the buffer. Make it large enough.

How large is that?  Does the tool recognize and report when overflow  
happens?

How much will the overhead of running some python script on the host,  
consuming a large volume of data, affect the results?

> IIRC ftrace improved recently to dynamically increase the buffer size  
> too.
> 
> Steven, do I remember correctly here?

Yay more complexity.

So now we get to worry about possible memory allocations happening when  
we try to log something?  Or if there is a way to do an "atomic" log,  
we're back to the "buffer might be full" situation.

> > and a dependency on a userspace tool
> 
> We already have that for kvm_stat. It's a simple python script - and  
> you surely have python on your rootfs, no?
> 
> > (both in terms of the tool needing to be written, and in the hassle  
> of ensuring that it's present in the root filesystem of whatever  
> system I'm testing).  And the whole mechanism will be more  
> complicated.
> 
> It'll also be more flexible at the same time. You could take the logs  
> and actually check what's going on to debug issues that you're  
> encountering for example.
> 
> We could even go as far as sharing the same tool with other  
> architectures, so that we only have to learn how to debug things once.

Have you encountered an actual need for this flexibility, or is it  
theoretical?

Is there common infrastructure for dealing with measuring intervals and  
tracking statistics thereof, rather than just tracking points and  
letting userspace connect the dots (though it could still do that as an  
option)?  Even if it must be done in userspace, it doesn't seem like  
something that should be KVM-specific.

> > Lots of debug options are enabled at build time; why must this be  
> different?
> 
> Because I think it's valuable as debug tool for cases where compile  
> time switches are not the best way of debugging things. It's not a  
> high profile thing to tackle for me tbh, but I don't really think  
> working heavily on the timing stat thing is the correct path to walk  
> along.

Adding new exit types isn't "working heavily" on it.

-Scott

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Scott Wood]

On 07/09/2013 05:00:26 PM, Alexander Graf wrote:
>=20
> On 09.07.2013, at 23:54, Scott Wood wrote:
>=20
> > On 07/09/2013 04:49:32 PM, Alexander Graf wrote:
> >> Not sure I understand. What the timing stats do is that they =20
> measure the time between [exit ... entry], right? We'd do the same =20
> thing, just all in C code. That means we would become slightly less =20
> accurate, but gain dynamic enabling of the traces and get rid of all =20
> the timing stat asm code.
> >
> > Compile-time enabling bothers me less than a loss of accuracy (not =20
> just a small loss by moving into C code, but a potential for a large =20
> loss if we overflow the buffer)
>=20
> Then don't overflow the buffer. Make it large enough.

How large is that?  Does the tool recognize and report when overflow =20
happens?

How much will the overhead of running some python script on the host, =20
consuming a large volume of data, affect the results?

> IIRC ftrace improved recently to dynamically increase the buffer size =20
> too.
>=20
> Steven, do I remember correctly here?

Yay more complexity.

So now we get to worry about possible memory allocations happening when =20
we try to log something?  Or if there is a way to do an "atomic" log, =20
we're back to the "buffer might be full" situation.

> > and a dependency on a userspace tool
>=20
> We already have that for kvm_stat. It's a simple python script - and =20
> you surely have python on your rootfs, no?
>=20
> > (both in terms of the tool needing to be written, and in the hassle =20
> of ensuring that it's present in the root filesystem of whatever =20
> system I'm testing).  And the whole mechanism will be more =20
> complicated.
>=20
> It'll also be more flexible at the same time. You could take the logs =20
> and actually check what's going on to debug issues that you're =20
> encountering for example.
>=20
> We could even go as far as sharing the same tool with other =20
> architectures, so that we only have to learn how to debug things once.

Have you encountered an actual need for this flexibility, or is it =20
theoretical?

Is there common infrastructure for dealing with measuring intervals and =20
tracking statistics thereof, rather than just tracking points and =20
letting userspace connect the dots (though it could still do that as an =20
option)?  Even if it must be done in userspace, it doesn't seem like =20
something that should be KVM-specific.

> > Lots of debug options are enabled at build time; why must this be =20
> different?
>=20
> Because I think it's valuable as debug tool for cases where compile =20
> time switches are not the best way of debugging things. It's not a =20
> high profile thing to tackle for me tbh, but I don't really think =20
> working heavily on the timing stat thing is the correct path to walk =20
> along.

Adding new exit types isn't "working heavily" on it.

-Scott=

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Steven Rostedt]

On Tue, 2013-07-09 at 17:26 -0500, Scott Wood wrote:
> On 07/09/2013 05:00:26 PM, Alexander Graf wrote:
> > 
> > On 09.07.2013, at 23:54, Scott Wood wrote:
> > 
> > > On 07/09/2013 04:49:32 PM, Alexander Graf wrote:
> > >> Not sure I understand. What the timing stats do is that they  
> > measure the time between [exit ... entry], right? We'd do the same  
> > thing, just all in C code. That means we would become slightly less  
> > accurate, but gain dynamic enabling of the traces and get rid of all  
> > the timing stat asm code.
> > >
> > > Compile-time enabling bothers me less than a loss of accuracy (not  
> > just a small loss by moving into C code, but a potential for a large  
> > loss if we overflow the buffer)
> > 
> > Then don't overflow the buffer. Make it large enough.
> 
> How large is that?  Does the tool recognize and report when overflow  
> happens?

Note, the ftrace buffers allow you to see when overflow does happen.

> 
> How much will the overhead of running some python script on the host,  
> consuming a large volume of data, affect the results?

This doesn't need to be python, and you can read the buffers in binary
as well. Mauro wrote a tool that uses ftrace for MCE errors. You can
probably do something similar. I need to get the code that reads ftrace
binary buffers out as a library.


> 
> > IIRC ftrace improved recently to dynamically increase the buffer size  
> > too.

What did change was that you can create buffers for your own use.

> > 
> > Steven, do I remember correctly here?
> 
> Yay more complexity.

What? Is ftrace complex? ;-)

> 
> So now we get to worry about possible memory allocations happening when  
> we try to log something?  Or if there is a way to do an "atomic" log,  
> we're back to the "buffer might be full" situation.

Nope, ftrace doesn't do dynamic allocation here.

-- Steve

> 
> > > and a dependency on a userspace tool
> > 
> > We already have that for kvm_stat. It's a simple python script - and  
> > you surely have python on your rootfs, no?
> > 
> > > (both in terms of the tool needing to be written, and in the hassle  
> > of ensuring that it's present in the root filesystem of whatever  
> > system I'm testing).  And the whole mechanism will be more  
> > complicated.
> > 
> > It'll also be more flexible at the same time. You could take the logs  
> > and actually check what's going on to debug issues that you're  
> > encountering for example.
> > 
> > We could even go as far as sharing the same tool with other  
> > architectures, so that we only have to learn how to debug things once.
> 
> Have you encountered an actual need for this flexibility, or is it  
> theoretical?
> 
> Is there common infrastructure for dealing with measuring intervals and  
> tracking statistics thereof, rather than just tracking points and  
> letting userspace connect the dots (though it could still do that as an  
> option)?  Even if it must be done in userspace, it doesn't seem like  
> something that should be KVM-specific.
> 
> > > Lots of debug options are enabled at build time; why must this be  
> > different?
> > 
> > Because I think it's valuable as debug tool for cases where compile  
> > time switches are not the best way of debugging things. It's not a  
> > high profile thing to tackle for me tbh, but I don't really think  
> > working heavily on the timing stat thing is the correct path to walk  
> > along.
> 
> Adding new exit types isn't "working heavily" on it.
> 
> -Scott

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Steven Rostedt]

On Tue, 2013-07-09 at 17:26 -0500, Scott Wood wrote:
> On 07/09/2013 05:00:26 PM, Alexander Graf wrote:
> > 
> > On 09.07.2013, at 23:54, Scott Wood wrote:
> > 
> > > On 07/09/2013 04:49:32 PM, Alexander Graf wrote:
> > >> Not sure I understand. What the timing stats do is that they  
> > measure the time between [exit ... entry], right? We'd do the same  
> > thing, just all in C code. That means we would become slightly less  
> > accurate, but gain dynamic enabling of the traces and get rid of all  
> > the timing stat asm code.
> > >
> > > Compile-time enabling bothers me less than a loss of accuracy (not  
> > just a small loss by moving into C code, but a potential for a large  
> > loss if we overflow the buffer)
> > 
> > Then don't overflow the buffer. Make it large enough.
> 
> How large is that?  Does the tool recognize and report when overflow  
> happens?

Note, the ftrace buffers allow you to see when overflow does happen.

> 
> How much will the overhead of running some python script on the host,  
> consuming a large volume of data, affect the results?

This doesn't need to be python, and you can read the buffers in binary
as well. Mauro wrote a tool that uses ftrace for MCE errors. You can
probably do something similar. I need to get the code that reads ftrace
binary buffers out as a library.


> 
> > IIRC ftrace improved recently to dynamically increase the buffer size  
> > too.

What did change was that you can create buffers for your own use.

> > 
> > Steven, do I remember correctly here?
> 
> Yay more complexity.

What? Is ftrace complex? ;-)

> 
> So now we get to worry about possible memory allocations happening when  
> we try to log something?  Or if there is a way to do an "atomic" log,  
> we're back to the "buffer might be full" situation.

Nope, ftrace doesn't do dynamic allocation here.

-- Steve

> 
> > > and a dependency on a userspace tool
> > 
> > We already have that for kvm_stat. It's a simple python script - and  
> > you surely have python on your rootfs, no?
> > 
> > > (both in terms of the tool needing to be written, and in the hassle  
> > of ensuring that it's present in the root filesystem of whatever  
> > system I'm testing).  And the whole mechanism will be more  
> > complicated.
> > 
> > It'll also be more flexible at the same time. You could take the logs  
> > and actually check what's going on to debug issues that you're  
> > encountering for example.
> > 
> > We could even go as far as sharing the same tool with other  
> > architectures, so that we only have to learn how to debug things once.
> 
> Have you encountered an actual need for this flexibility, or is it  
> theoretical?
> 
> Is there common infrastructure for dealing with measuring intervals and  
> tracking statistics thereof, rather than just tracking points and  
> letting userspace connect the dots (though it could still do that as an  
> option)?  Even if it must be done in userspace, it doesn't seem like  
> something that should be KVM-specific.
> 
> > > Lots of debug options are enabled at build time; why must this be  
> > different?
> > 
> > Because I think it's valuable as debug tool for cases where compile  
> > time switches are not the best way of debugging things. It's not a  
> > high profile thing to tackle for me tbh, but I don't really think  
> > working heavily on the timing stat thing is the correct path to walk  
> > along.
> 
> Adding new exit types isn't "working heavily" on it.
> 
> -Scott

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Steven Rostedt]

On Tue, 2013-07-09 at 17:26 -0500, Scott Wood wrote:
> On 07/09/2013 05:00:26 PM, Alexander Graf wrote:
> > 
> > On 09.07.2013, at 23:54, Scott Wood wrote:
> > 
> > > On 07/09/2013 04:49:32 PM, Alexander Graf wrote:
> > >> Not sure I understand. What the timing stats do is that they  
> > measure the time between [exit ... entry], right? We'd do the same  
> > thing, just all in C code. That means we would become slightly less  
> > accurate, but gain dynamic enabling of the traces and get rid of all  
> > the timing stat asm code.
> > >
> > > Compile-time enabling bothers me less than a loss of accuracy (not  
> > just a small loss by moving into C code, but a potential for a large  
> > loss if we overflow the buffer)
> > 
> > Then don't overflow the buffer. Make it large enough.
> 
> How large is that?  Does the tool recognize and report when overflow  
> happens?

Note, the ftrace buffers allow you to see when overflow does happen.

> 
> How much will the overhead of running some python script on the host,  
> consuming a large volume of data, affect the results?

This doesn't need to be python, and you can read the buffers in binary
as well. Mauro wrote a tool that uses ftrace for MCE errors. You can
probably do something similar. I need to get the code that reads ftrace
binary buffers out as a library.


> 
> > IIRC ftrace improved recently to dynamically increase the buffer size  
> > too.

What did change was that you can create buffers for your own use.

> > 
> > Steven, do I remember correctly here?
> 
> Yay more complexity.

What? Is ftrace complex? ;-)

> 
> So now we get to worry about possible memory allocations happening when  
> we try to log something?  Or if there is a way to do an "atomic" log,  
> we're back to the "buffer might be full" situation.

Nope, ftrace doesn't do dynamic allocation here.

-- Steve

> 
> > > and a dependency on a userspace tool
> > 
> > We already have that for kvm_stat. It's a simple python script - and  
> > you surely have python on your rootfs, no?
> > 
> > > (both in terms of the tool needing to be written, and in the hassle  
> > of ensuring that it's present in the root filesystem of whatever  
> > system I'm testing).  And the whole mechanism will be more  
> > complicated.
> > 
> > It'll also be more flexible at the same time. You could take the logs  
> > and actually check what's going on to debug issues that you're  
> > encountering for example.
> > 
> > We could even go as far as sharing the same tool with other  
> > architectures, so that we only have to learn how to debug things once.
> 
> Have you encountered an actual need for this flexibility, or is it  
> theoretical?
> 
> Is there common infrastructure for dealing with measuring intervals and  
> tracking statistics thereof, rather than just tracking points and  
> letting userspace connect the dots (though it could still do that as an  
> option)?  Even if it must be done in userspace, it doesn't seem like  
> something that should be KVM-specific.
> 
> > > Lots of debug options are enabled at build time; why must this be  
> > different?
> > 
> > Because I think it's valuable as debug tool for cases where compile  
> > time switches are not the best way of debugging things. It's not a  
> > high profile thing to tackle for me tbh, but I don't really think  
> > working heavily on the timing stat thing is the correct path to walk  
> > along.
> 
> Adding new exit types isn't "working heavily" on it.
> 
> -Scott

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Alexander Graf]

On 10.07.2013, at 00:26, Scott Wood wrote:

> On 07/09/2013 05:00:26 PM, Alexander Graf wrote:
>> On 09.07.2013, at 23:54, Scott Wood wrote:
>> > On 07/09/2013 04:49:32 PM, Alexander Graf wrote:
>> >> Not sure I understand. What the timing stats do is that they measure the time between [exit ... entry], right? We'd do the same thing, just all in C code. That means we would become slightly less accurate, but gain dynamic enabling of the traces and get rid of all the timing stat asm code.
>> >
>> > Compile-time enabling bothers me less than a loss of accuracy (not just a small loss by moving into C code, but a potential for a large loss if we overflow the buffer)
>> Then don't overflow the buffer. Make it large enough.
> 
> How large is that?  Does the tool recognize and report when overflow happens?
> 
> How much will the overhead of running some python script on the host, consuming a large volume of data, affect the results?
> 
>> IIRC ftrace improved recently to dynamically increase the buffer size too.
>> Steven, do I remember correctly here?
> 
> Yay more complexity.
> 
> So now we get to worry about possible memory allocations happening when we try to log something?  Or if there is a way to do an "atomic" log, we're back to the "buffer might be full" situation.
> 
>> > and a dependency on a userspace tool
>> We already have that for kvm_stat. It's a simple python script - and you surely have python on your rootfs, no?
>> > (both in terms of the tool needing to be written, and in the hassle of ensuring that it's present in the root filesystem of whatever system I'm testing).  And the whole mechanism will be more complicated.
>> It'll also be more flexible at the same time. You could take the logs and actually check what's going on to debug issues that you're encountering for example.
>> We could even go as far as sharing the same tool with other architectures, so that we only have to learn how to debug things once.
> 
> Have you encountered an actual need for this flexibility, or is it theoretical?

Yeah, first thing I did back then to actually debug kvm failures was to add trace points.

> Is there common infrastructure for dealing with measuring intervals and tracking statistics thereof, rather than just tracking points and letting userspace connect the dots (though it could still do that as an option)?  Even if it must be done in userspace, it doesn't seem like something that should be KVM-specific.

Would you like to have different ways of measuring mm subsystem overhead? I don't :). The same goes for KVM really. If we could converge towards a single user space interface to get exit timings, it'd make debugging a lot easier.

We already have this for the debugfs counters btw. And the timing framework does break kvm_stat today already, as it emits textual stats rather than numbers which all of the other debugfs stats do. But at least I can take the x86 kvm_stat tool and run it on ppc just fine to see exit stats.

> 
>> > Lots of debug options are enabled at build time; why must this be different?
>> Because I think it's valuable as debug tool for cases where compile time switches are not the best way of debugging things. It's not a high profile thing to tackle for me tbh, but I don't really think working heavily on the timing stat thing is the correct path to walk along.
> 
> Adding new exit types isn't "working heavily" on it.

No, but the fact that the first patch is a fix to add exit stats for exits that we missed out before doesn't give me a lot of confidence that lots of people use timing stats. And I am always very weary of #ifdef'ed code, as it blows up the test matrix heavily.


Alex

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Alexander Graf]

On 10.07.2013, at 00:26, Scott Wood wrote:

> On 07/09/2013 05:00:26 PM, Alexander Graf wrote:
>> On 09.07.2013, at 23:54, Scott Wood wrote:
>> > On 07/09/2013 04:49:32 PM, Alexander Graf wrote:
>> >> Not sure I understand. What the timing stats do is that they measure the time between [exit ... entry], right? We'd do the same thing, just all in C code. That means we would become slightly less accurate, but gain dynamic enabling of the traces and get rid of all the timing stat asm code.
>> >
>> > Compile-time enabling bothers me less than a loss of accuracy (not just a small loss by moving into C code, but a potential for a large loss if we overflow the buffer)
>> Then don't overflow the buffer. Make it large enough.
> 
> How large is that?  Does the tool recognize and report when overflow happens?
> 
> How much will the overhead of running some python script on the host, consuming a large volume of data, affect the results?
> 
>> IIRC ftrace improved recently to dynamically increase the buffer size too.
>> Steven, do I remember correctly here?
> 
> Yay more complexity.
> 
> So now we get to worry about possible memory allocations happening when we try to log something?  Or if there is a way to do an "atomic" log, we're back to the "buffer might be full" situation.
> 
>> > and a dependency on a userspace tool
>> We already have that for kvm_stat. It's a simple python script - and you surely have python on your rootfs, no?
>> > (both in terms of the tool needing to be written, and in the hassle of ensuring that it's present in the root filesystem of whatever system I'm testing).  And the whole mechanism will be more complicated.
>> It'll also be more flexible at the same time. You could take the logs and actually check what's going on to debug issues that you're encountering for example.
>> We could even go as far as sharing the same tool with other architectures, so that we only have to learn how to debug things once.
> 
> Have you encountered an actual need for this flexibility, or is it theoretical?

Yeah, first thing I did back then to actually debug kvm failures was to add trace points.

> Is there common infrastructure for dealing with measuring intervals and tracking statistics thereof, rather than just tracking points and letting userspace connect the dots (though it could still do that as an option)?  Even if it must be done in userspace, it doesn't seem like something that should be KVM-specific.

Would you like to have different ways of measuring mm subsystem overhead? I don't :). The same goes for KVM really. If we could converge towards a single user space interface to get exit timings, it'd make debugging a lot easier.

We already have this for the debugfs counters btw. And the timing framework does break kvm_stat today already, as it emits textual stats rather than numbers which all of the other debugfs stats do. But at least I can take the x86 kvm_stat tool and run it on ppc just fine to see exit stats.

> 
>> > Lots of debug options are enabled at build time; why must this be different?
>> Because I think it's valuable as debug tool for cases where compile time switches are not the best way of debugging things. It's not a high profile thing to tackle for me tbh, but I don't really think working heavily on the timing stat thing is the correct path to walk along.
> 
> Adding new exit types isn't "working heavily" on it.

No, but the fact that the first patch is a fix to add exit stats for exits that we missed out before doesn't give me a lot of confidence that lots of people use timing stats. And I am always very weary of #ifdef'ed code, as it blows up the test matrix heavily.


Alex

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Alexander Graf]

On 10.07.2013, at 00:26, Scott Wood wrote:

> On 07/09/2013 05:00:26 PM, Alexander Graf wrote:
>> On 09.07.2013, at 23:54, Scott Wood wrote:
>> > On 07/09/2013 04:49:32 PM, Alexander Graf wrote:
>> >> Not sure I understand. What the timing stats do is that they =
measure the time between [exit ... entry], right? We'd do the same =
thing, just all in C code. That means we would become slightly less =
accurate, but gain dynamic enabling of the traces and get rid of all the =
timing stat asm code.
>> >
>> > Compile-time enabling bothers me less than a loss of accuracy (not =
just a small loss by moving into C code, but a potential for a large =
loss if we overflow the buffer)
>> Then don't overflow the buffer. Make it large enough.
>=20
> How large is that?  Does the tool recognize and report when overflow =
happens?
>=20
> How much will the overhead of running some python script on the host, =
consuming a large volume of data, affect the results?
>=20
>> IIRC ftrace improved recently to dynamically increase the buffer size =
too.
>> Steven, do I remember correctly here?
>=20
> Yay more complexity.
>=20
> So now we get to worry about possible memory allocations happening =
when we try to log something?  Or if there is a way to do an "atomic" =
log, we're back to the "buffer might be full" situation.
>=20
>> > and a dependency on a userspace tool
>> We already have that for kvm_stat. It's a simple python script - and =
you surely have python on your rootfs, no?
>> > (both in terms of the tool needing to be written, and in the hassle =
of ensuring that it's present in the root filesystem of whatever system =
I'm testing).  And the whole mechanism will be more complicated.
>> It'll also be more flexible at the same time. You could take the logs =
and actually check what's going on to debug issues that you're =
encountering for example.
>> We could even go as far as sharing the same tool with other =
architectures, so that we only have to learn how to debug things once.
>=20
> Have you encountered an actual need for this flexibility, or is it =
theoretical?

Yeah, first thing I did back then to actually debug kvm failures was to =
add trace points.

> Is there common infrastructure for dealing with measuring intervals =
and tracking statistics thereof, rather than just tracking points and =
letting userspace connect the dots (though it could still do that as an =
option)?  Even if it must be done in userspace, it doesn't seem like =
something that should be KVM-specific.

Would you like to have different ways of measuring mm subsystem =
overhead? I don't :). The same goes for KVM really. If we could converge =
towards a single user space interface to get exit timings, it'd make =
debugging a lot easier.

We already have this for the debugfs counters btw. And the timing =
framework does break kvm_stat today already, as it emits textual stats =
rather than numbers which all of the other debugfs stats do. But at =
least I can take the x86 kvm_stat tool and run it on ppc just fine to =
see exit stats.

>=20
>> > Lots of debug options are enabled at build time; why must this be =
different?
>> Because I think it's valuable as debug tool for cases where compile =
time switches are not the best way of debugging things. It's not a high =
profile thing to tackle for me tbh, but I don't really think working =
heavily on the timing stat thing is the correct path to walk along.
>=20
> Adding new exit types isn't "working heavily" on it.

No, but the fact that the first patch is a fix to add exit stats for =
exits that we missed out before doesn't give me a lot of confidence that =
lots of people use timing stats. And I am always very weary of #ifdef'ed =
code, as it blows up the test matrix heavily.


Alex

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Scott Wood]

On 07/10/2013 05:23:36 AM, Alexander Graf wrote:
> 
> On 10.07.2013, at 00:26, Scott Wood wrote:
> 
> > On 07/09/2013 05:00:26 PM, Alexander Graf wrote:
> >> It'll also be more flexible at the same time. You could take the  
> logs and actually check what's going on to debug issues that you're  
> encountering for example.
> >> We could even go as far as sharing the same tool with other  
> architectures, so that we only have to learn how to debug things once.
> >
> > Have you encountered an actual need for this flexibility, or is it  
> theoretical?
> 
> Yeah, first thing I did back then to actually debug kvm failures was  
> to add trace points.

I meant specifically for handling exit timings this way.

> > Is there common infrastructure for dealing with measuring intervals  
> and tracking statistics thereof, rather than just tracking points and  
> letting userspace connect the dots (though it could still do that as  
> an option)?  Even if it must be done in userspace, it doesn't seem  
> like something that should be KVM-specific.
> 
> Would you like to have different ways of measuring mm subsystem  
> overhead? I don't :). The same goes for KVM really. If we could  
> converge towards a single user space interface to get exit timings,  
> it'd make debugging a lot easier.

I agree -- that's why I said it doesn't seem like something that should  
be KVM-specific.  But that's orthogonal to whether it's done in kernel  
space or user space.  The ability to get begin/end events from  
userspace would be nice when it is specifically requested, but it would  
also be nice if the kernel could track some basic statistics so we  
wouldn't have to ship so much data around to arrive at the same result.

At the very least, I'd like such a tool/infrastructure to exist before  
we start complaining about doing minor maintenance of the current  
mechanism.

> We already have this for the debugfs counters btw. And the timing  
> framework does break kvm_stat today already, as it emits textual  
> stats rather than numbers which all of the other debugfs stats do.  
> But at least I can take the x86 kvm_stat tool and run it on ppc just  
> fine to see exit stats.

We already have what?  The last two sentences seem contradictory -- can  
you or can't you use kvm_stat as is?  I'm not familiar with kvm_stat.

What does x86 KVM expose in debugfs?

> >> > Lots of debug options are enabled at build time; why must this  
> be different?
> >> Because I think it's valuable as debug tool for cases where  
> compile time switches are not the best way of debugging things. It's  
> not a high profile thing to tackle for me tbh, but I don't really  
> think working heavily on the timing stat thing is the correct path to  
> walk along.
> >
> > Adding new exit types isn't "working heavily" on it.
> 
> No, but the fact that the first patch is a fix to add exit stats for  
> exits that we missed out before doesn't give me a lot of confidence  
> that lots of people use timing stats. And I am always very weary of  
> #ifdef'ed code, as it blows up the test matrix heavily.

I used it quite a lot when I was doing KVM performance work.  It's just  
been a while since I last did that.

-Scott

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Scott Wood]

On 07/10/2013 05:23:36 AM, Alexander Graf wrote:
> 
> On 10.07.2013, at 00:26, Scott Wood wrote:
> 
> > On 07/09/2013 05:00:26 PM, Alexander Graf wrote:
> >> It'll also be more flexible at the same time. You could take the  
> logs and actually check what's going on to debug issues that you're  
> encountering for example.
> >> We could even go as far as sharing the same tool with other  
> architectures, so that we only have to learn how to debug things once.
> >
> > Have you encountered an actual need for this flexibility, or is it  
> theoretical?
> 
> Yeah, first thing I did back then to actually debug kvm failures was  
> to add trace points.

I meant specifically for handling exit timings this way.

> > Is there common infrastructure for dealing with measuring intervals  
> and tracking statistics thereof, rather than just tracking points and  
> letting userspace connect the dots (though it could still do that as  
> an option)?  Even if it must be done in userspace, it doesn't seem  
> like something that should be KVM-specific.
> 
> Would you like to have different ways of measuring mm subsystem  
> overhead? I don't :). The same goes for KVM really. If we could  
> converge towards a single user space interface to get exit timings,  
> it'd make debugging a lot easier.

I agree -- that's why I said it doesn't seem like something that should  
be KVM-specific.  But that's orthogonal to whether it's done in kernel  
space or user space.  The ability to get begin/end events from  
userspace would be nice when it is specifically requested, but it would  
also be nice if the kernel could track some basic statistics so we  
wouldn't have to ship so much data around to arrive at the same result.

At the very least, I'd like such a tool/infrastructure to exist before  
we start complaining about doing minor maintenance of the current  
mechanism.

> We already have this for the debugfs counters btw. And the timing  
> framework does break kvm_stat today already, as it emits textual  
> stats rather than numbers which all of the other debugfs stats do.  
> But at least I can take the x86 kvm_stat tool and run it on ppc just  
> fine to see exit stats.

We already have what?  The last two sentences seem contradictory -- can  
you or can't you use kvm_stat as is?  I'm not familiar with kvm_stat.

What does x86 KVM expose in debugfs?

> >> > Lots of debug options are enabled at build time; why must this  
> be different?
> >> Because I think it's valuable as debug tool for cases where  
> compile time switches are not the best way of debugging things. It's  
> not a high profile thing to tackle for me tbh, but I don't really  
> think working heavily on the timing stat thing is the correct path to  
> walk along.
> >
> > Adding new exit types isn't "working heavily" on it.
> 
> No, but the fact that the first patch is a fix to add exit stats for  
> exits that we missed out before doesn't give me a lot of confidence  
> that lots of people use timing stats. And I am always very weary of  
> #ifdef'ed code, as it blows up the test matrix heavily.

I used it quite a lot when I was doing KVM performance work.  It's just  
been a while since I last did that.

-Scott

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Scott Wood]

On 07/10/2013 05:23:36 AM, Alexander Graf wrote:
>=20
> On 10.07.2013, at 00:26, Scott Wood wrote:
>=20
> > On 07/09/2013 05:00:26 PM, Alexander Graf wrote:
> >> It'll also be more flexible at the same time. You could take the =20
> logs and actually check what's going on to debug issues that you're =20
> encountering for example.
> >> We could even go as far as sharing the same tool with other =20
> architectures, so that we only have to learn how to debug things once.
> >
> > Have you encountered an actual need for this flexibility, or is it =20
> theoretical?
>=20
> Yeah, first thing I did back then to actually debug kvm failures was =20
> to add trace points.

I meant specifically for handling exit timings this way.

> > Is there common infrastructure for dealing with measuring intervals =20
> and tracking statistics thereof, rather than just tracking points and =20
> letting userspace connect the dots (though it could still do that as =20
> an option)?  Even if it must be done in userspace, it doesn't seem =20
> like something that should be KVM-specific.
>=20
> Would you like to have different ways of measuring mm subsystem =20
> overhead? I don't :). The same goes for KVM really. If we could =20
> converge towards a single user space interface to get exit timings, =20
> it'd make debugging a lot easier.

I agree -- that's why I said it doesn't seem like something that should =20
be KVM-specific.  But that's orthogonal to whether it's done in kernel =20
space or user space.  The ability to get begin/end events from =20
userspace would be nice when it is specifically requested, but it would =20
also be nice if the kernel could track some basic statistics so we =20
wouldn't have to ship so much data around to arrive at the same result.

At the very least, I'd like such a tool/infrastructure to exist before =20
we start complaining about doing minor maintenance of the current =20
mechanism.

> We already have this for the debugfs counters btw. And the timing =20
> framework does break kvm_stat today already, as it emits textual =20
> stats rather than numbers which all of the other debugfs stats do. =20
> But at least I can take the x86 kvm_stat tool and run it on ppc just =20
> fine to see exit stats.

We already have what?  The last two sentences seem contradictory -- can =20
you or can't you use kvm_stat as is?  I'm not familiar with kvm_stat.

What does x86 KVM expose in debugfs?

> >> > Lots of debug options are enabled at build time; why must this =20
> be different?
> >> Because I think it's valuable as debug tool for cases where =20
> compile time switches are not the best way of debugging things. It's =20
> not a high profile thing to tackle for me tbh, but I don't really =20
> think working heavily on the timing stat thing is the correct path to =20
> walk along.
> >
> > Adding new exit types isn't "working heavily" on it.
>=20
> No, but the fact that the first patch is a fix to add exit stats for =20
> exits that we missed out before doesn't give me a lot of confidence =20
> that lots of people use timing stats. And I am always very weary of =20
> #ifdef'ed code, as it blows up the test matrix heavily.

I used it quite a lot when I was doing KVM performance work.  It's just =20
been a while since I last did that.

-Scott=

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Alexander Graf]

On 10.07.2013, at 20:24, Scott Wood wrote:

> On 07/10/2013 05:23:36 AM, Alexander Graf wrote:
>> On 10.07.2013, at 00:26, Scott Wood wrote:
>> > On 07/09/2013 05:00:26 PM, Alexander Graf wrote:
>> >> It'll also be more flexible at the same time. You could take the logs and actually check what's going on to debug issues that you're encountering for example.
>> >> We could even go as far as sharing the same tool with other architectures, so that we only have to learn how to debug things once.
>> >
>> > Have you encountered an actual need for this flexibility, or is it theoretical?
>> Yeah, first thing I did back then to actually debug kvm failures was to add trace points.
> 
> I meant specifically for handling exit timings this way.

No, but I did encounter the need for debugging exits. And the only thing we would need to get exit timing stats once we already have trace points for exit types would be to have trace points for guest entry and maybe type specific events to indicate what the exit is about as well.

> 
>> > Is there common infrastructure for dealing with measuring intervals and tracking statistics thereof, rather than just tracking points and letting userspace connect the dots (though it could still do that as an option)?  Even if it must be done in userspace, it doesn't seem like something that should be KVM-specific.
>> Would you like to have different ways of measuring mm subsystem overhead? I don't :). The same goes for KVM really. If we could converge towards a single user space interface to get exit timings, it'd make debugging a lot easier.
> 
> I agree -- that's why I said it doesn't seem like something that should be KVM-specific.  But that's orthogonal to whether it's done in kernel space or user space.  The ability to get begin/end events from userspace would be nice when it is specifically requested, but it would also be nice if the kernel could track some basic statistics so we wouldn't have to ship so much data around to arrive at the same result.
> 
> At the very least, I'd like such a tool/infrastructure to exist before we start complaining about doing minor maintenance of the current mechanism.

I admit that I don't fully understand qemu/scripts/kvm/kvm_stat, but it seems to me as if it already does pretty much what we want. It sets up a filter to only get events and their time stamps through.

It does use normal exit trace points on x86 to replace the old debugfs based stat counters. And it seems to work reasonably well for that.

> 
>> We already have this for the debugfs counters btw. And the timing framework does break kvm_stat today already, as it emits textual stats rather than numbers which all of the other debugfs stats do. But at least I can take the x86 kvm_stat tool and run it on ppc just fine to see exit stats.
> 
> We already have what?  The last two sentences seem contradictory -- can you or can't you use kvm_stat as is?  I'm not familiar with kvm_stat.

Kvm_stat back in the day used debugfs to give you an idea on what exit event happens most often. That mechanism got replaced by trace points later which the current kvm_stat uses.

I still have a copy of the old kvm_stat that I always use to get a first feeling for what goes wrong if something goes wrong. The original code couldn't deal with the fact that we have a debugfs file that contains text though. I patched it locally. It also works just fine if you simply disable timing stats, since then you won't have the text file.

> What does x86 KVM expose in debugfs?

The same thing it always exposed - exit stats. I am fairly sure Avi wanted to completely deprecate that interface in favor of the trace point based approach, but I don't think he ever got around to it.


Alex

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Alexander Graf]

On 10.07.2013, at 20:24, Scott Wood wrote:

> On 07/10/2013 05:23:36 AM, Alexander Graf wrote:
>> On 10.07.2013, at 00:26, Scott Wood wrote:
>> > On 07/09/2013 05:00:26 PM, Alexander Graf wrote:
>> >> It'll also be more flexible at the same time. You could take the logs and actually check what's going on to debug issues that you're encountering for example.
>> >> We could even go as far as sharing the same tool with other architectures, so that we only have to learn how to debug things once.
>> >
>> > Have you encountered an actual need for this flexibility, or is it theoretical?
>> Yeah, first thing I did back then to actually debug kvm failures was to add trace points.
> 
> I meant specifically for handling exit timings this way.

No, but I did encounter the need for debugging exits. And the only thing we would need to get exit timing stats once we already have trace points for exit types would be to have trace points for guest entry and maybe type specific events to indicate what the exit is about as well.

> 
>> > Is there common infrastructure for dealing with measuring intervals and tracking statistics thereof, rather than just tracking points and letting userspace connect the dots (though it could still do that as an option)?  Even if it must be done in userspace, it doesn't seem like something that should be KVM-specific.
>> Would you like to have different ways of measuring mm subsystem overhead? I don't :). The same goes for KVM really. If we could converge towards a single user space interface to get exit timings, it'd make debugging a lot easier.
> 
> I agree -- that's why I said it doesn't seem like something that should be KVM-specific.  But that's orthogonal to whether it's done in kernel space or user space.  The ability to get begin/end events from userspace would be nice when it is specifically requested, but it would also be nice if the kernel could track some basic statistics so we wouldn't have to ship so much data around to arrive at the same result.
> 
> At the very least, I'd like such a tool/infrastructure to exist before we start complaining about doing minor maintenance of the current mechanism.

I admit that I don't fully understand qemu/scripts/kvm/kvm_stat, but it seems to me as if it already does pretty much what we want. It sets up a filter to only get events and their time stamps through.

It does use normal exit trace points on x86 to replace the old debugfs based stat counters. And it seems to work reasonably well for that.

> 
>> We already have this for the debugfs counters btw. And the timing framework does break kvm_stat today already, as it emits textual stats rather than numbers which all of the other debugfs stats do. But at least I can take the x86 kvm_stat tool and run it on ppc just fine to see exit stats.
> 
> We already have what?  The last two sentences seem contradictory -- can you or can't you use kvm_stat as is?  I'm not familiar with kvm_stat.

Kvm_stat back in the day used debugfs to give you an idea on what exit event happens most often. That mechanism got replaced by trace points later which the current kvm_stat uses.

I still have a copy of the old kvm_stat that I always use to get a first feeling for what goes wrong if something goes wrong. The original code couldn't deal with the fact that we have a debugfs file that contains text though. I patched it locally. It also works just fine if you simply disable timing stats, since then you won't have the text file.

> What does x86 KVM expose in debugfs?

The same thing it always exposed - exit stats. I am fairly sure Avi wanted to completely deprecate that interface in favor of the trace point based approach, but I don't think he ever got around to it.


Alex

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Alexander Graf]

On 10.07.2013, at 20:24, Scott Wood wrote:

> On 07/10/2013 05:23:36 AM, Alexander Graf wrote:
>> On 10.07.2013, at 00:26, Scott Wood wrote:
>> > On 07/09/2013 05:00:26 PM, Alexander Graf wrote:
>> >> It'll also be more flexible at the same time. You could take the =
logs and actually check what's going on to debug issues that you're =
encountering for example.
>> >> We could even go as far as sharing the same tool with other =
architectures, so that we only have to learn how to debug things once.
>> >
>> > Have you encountered an actual need for this flexibility, or is it =
theoretical?
>> Yeah, first thing I did back then to actually debug kvm failures was =
to add trace points.
>=20
> I meant specifically for handling exit timings this way.

No, but I did encounter the need for debugging exits. And the only thing =
we would need to get exit timing stats once we already have trace points =
for exit types would be to have trace points for guest entry and maybe =
type specific events to indicate what the exit is about as well.

>=20
>> > Is there common infrastructure for dealing with measuring intervals =
and tracking statistics thereof, rather than just tracking points and =
letting userspace connect the dots (though it could still do that as an =
option)?  Even if it must be done in userspace, it doesn't seem like =
something that should be KVM-specific.
>> Would you like to have different ways of measuring mm subsystem =
overhead? I don't :). The same goes for KVM really. If we could converge =
towards a single user space interface to get exit timings, it'd make =
debugging a lot easier.
>=20
> I agree -- that's why I said it doesn't seem like something that =
should be KVM-specific.  But that's orthogonal to whether it's done in =
kernel space or user space.  The ability to get begin/end events from =
userspace would be nice when it is specifically requested, but it would =
also be nice if the kernel could track some basic statistics so we =
wouldn't have to ship so much data around to arrive at the same result.
>=20
> At the very least, I'd like such a tool/infrastructure to exist before =
we start complaining about doing minor maintenance of the current =
mechanism.

I admit that I don't fully understand qemu/scripts/kvm/kvm_stat, but it =
seems to me as if it already does pretty much what we want. It sets up a =
filter to only get events and their time stamps through.

It does use normal exit trace points on x86 to replace the old debugfs =
based stat counters. And it seems to work reasonably well for that.

>=20
>> We already have this for the debugfs counters btw. And the timing =
framework does break kvm_stat today already, as it emits textual stats =
rather than numbers which all of the other debugfs stats do. But at =
least I can take the x86 kvm_stat tool and run it on ppc just fine to =
see exit stats.
>=20
> We already have what?  The last two sentences seem contradictory -- =
can you or can't you use kvm_stat as is?  I'm not familiar with =
kvm_stat.

Kvm_stat back in the day used debugfs to give you an idea on what exit =
event happens most often. That mechanism got replaced by trace points =
later which the current kvm_stat uses.

I still have a copy of the old kvm_stat that I always use to get a first =
feeling for what goes wrong if something goes wrong. The original code =
couldn't deal with the fact that we have a debugfs file that contains =
text though. I patched it locally. It also works just fine if you simply =
disable timing stats, since then you won't have the text file.

> What does x86 KVM expose in debugfs?

The same thing it always exposed - exit stats. I am fairly sure Avi =
wanted to completely deprecate that interface in favor of the trace =
point based approach, but I don't think he ever got around to it.


Alex

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Steven Rostedt]

On Wed, 2013-07-10 at 00:00 +0200, Alexander Graf wrote:

> Then don't overflow the buffer. Make it large enough. IIRC ftrace improved recently to dynamically increase the buffer size too.
> 
> Steven, do I remember correctly here?

Not really. Ftrace only dynamically increases the buffer when the trace
is first used. Other than that, the size is static. I also wouldn't
suggest allocating the buffer when needed as that has the overhead of
allocating memory.

-- Steve

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Steven Rostedt]

On Wed, 2013-07-10 at 00:00 +0200, Alexander Graf wrote:

> Then don't overflow the buffer. Make it large enough. IIRC ftrace improved recently to dynamically increase the buffer size too.
> 
> Steven, do I remember correctly here?

Not really. Ftrace only dynamically increases the buffer when the trace
is first used. Other than that, the size is static. I also wouldn't
suggest allocating the buffer when needed as that has the overhead of
allocating memory.

-- Steve

Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

[Steven Rostedt]

On Wed, 2013-07-10 at 00:00 +0200, Alexander Graf wrote:

> Then don't overflow the buffer. Make it large enough. IIRC ftrace improved recently to dynamically increase the buffer size too.
> 
> Steven, do I remember correctly here?

Not really. Ftrace only dynamically increases the buffer when the trace
is first used. Other than that, the size is static. I also wouldn't
suggest allocating the buffer when needed as that has the overhead of
allocating memory.

-- Steve

Re: [PATCH 1/2] KVM: PPC: Fix kvm_exit_names array

[Alexander Graf]

On 03.07.2013, at 15:30, Mihai Caraman wrote:

> Some exit ids where left out from kvm_exit_names array.
> 
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> ---
> arch/powerpc/kvm/timing.c |    4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/timing.c b/arch/powerpc/kvm/timing.c
> index 07b6110..c392d26 100644
> --- a/arch/powerpc/kvm/timing.c
> +++ b/arch/powerpc/kvm/timing.c
> @@ -135,7 +135,9 @@ static const char *kvm_exit_names[__NUMBER_OF_KVM_EXIT_TYPES] = {
> 	[USR_PR_INST] =             "USR_PR_INST",
> 	[FP_UNAVAIL] =              "FP_UNAVAIL",
> 	[DEBUG_EXITS] =             "DEBUG",
> -	[TIMEINGUEST] =             "TIMEINGUEST"
> +	[TIMEINGUEST] =             "TIMEINGUEST",
> +	[DBELL_EXITS] =             "DBELL",
> +	[GDBELL_EXITS] =            "GDBELL"

Please add a comma at the end here, so that we don't have to uselessly touch the entry next time again.


Alex

Re: [PATCH 1/2] KVM: PPC: Fix kvm_exit_names array

[Alexander Graf]

On 03.07.2013, at 15:30, Mihai Caraman wrote:

> Some exit ids where left out from kvm_exit_names array.
> 
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> ---
> arch/powerpc/kvm/timing.c |    4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/timing.c b/arch/powerpc/kvm/timing.c
> index 07b6110..c392d26 100644
> --- a/arch/powerpc/kvm/timing.c
> +++ b/arch/powerpc/kvm/timing.c
> @@ -135,7 +135,9 @@ static const char *kvm_exit_names[__NUMBER_OF_KVM_EXIT_TYPES] = {
> 	[USR_PR_INST] =             "USR_PR_INST",
> 	[FP_UNAVAIL] =              "FP_UNAVAIL",
> 	[DEBUG_EXITS] =             "DEBUG",
> -	[TIMEINGUEST] =             "TIMEINGUEST"
> +	[TIMEINGUEST] =             "TIMEINGUEST",
> +	[DBELL_EXITS] =             "DBELL",
> +	[GDBELL_EXITS] =            "GDBELL"

Please add a comma at the end here, so that we don't have to uselessly touch the entry next time again.


Alex

Re: [PATCH 1/2] KVM: PPC: Fix kvm_exit_names array

[Alexander Graf]

On 03.07.2013, at 15:30, Mihai Caraman wrote:

> Some exit ids where left out from kvm_exit_names array.
>=20
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> ---
> arch/powerpc/kvm/timing.c |    4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>=20
> diff --git a/arch/powerpc/kvm/timing.c b/arch/powerpc/kvm/timing.c
> index 07b6110..c392d26 100644
> --- a/arch/powerpc/kvm/timing.c
> +++ b/arch/powerpc/kvm/timing.c
> @@ -135,7 +135,9 @@ static const char =
*kvm_exit_names[__NUMBER_OF_KVM_EXIT_TYPES] =3D {
> 	[USR_PR_INST] =3D             "USR_PR_INST",
> 	[FP_UNAVAIL] =3D              "FP_UNAVAIL",
> 	[DEBUG_EXITS] =3D             "DEBUG",
> -	[TIMEINGUEST] =3D             "TIMEINGUEST"
> +	[TIMEINGUEST] =3D             "TIMEINGUEST",
> +	[DBELL_EXITS] =3D             "DBELL",
> +	[GDBELL_EXITS] =3D            "GDBELL"

Please add a comma at the end here, so that we don't have to uselessly =
touch the entry next time again.


Alex