[PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Aneesh Kumar K.V]

Although it's optional IBM POWER cpus always had DAR value set on
alignment interrupt. So don't try to compute these values.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
Changes from V3:
* Use make_dsisr instead of checking feature flag to decide whether to use
  saved dsisr or not

 arch/powerpc/include/asm/disassemble.h | 34 +++++++++++++++++++++++++++
 arch/powerpc/kernel/align.c            | 34 +--------------------------
 arch/powerpc/kvm/book3s_emulate.c      | 43 ++++------------------------------
 3 files changed, 40 insertions(+), 71 deletions(-)

diff --git a/arch/powerpc/include/asm/disassemble.h b/arch/powerpc/include/asm/disassemble.h
index 856f8deb557a..6330a61b875a 100644
--- a/arch/powerpc/include/asm/disassemble.h
+++ b/arch/powerpc/include/asm/disassemble.h
@@ -81,4 +81,38 @@ static inline unsigned int get_oc(u32 inst)
 {
 	return (inst >> 11) & 0x7fff;
 }
+
+#define IS_XFORM(inst)	(get_op(inst)  == 31)
+#define IS_DSFORM(inst)	(get_op(inst) >= 56)
+
+/*
+ * Create a DSISR value from the instruction
+ */
+static inline unsigned make_dsisr(unsigned instr)
+{
+	unsigned dsisr;
+
+
+	/* bits  6:15 --> 22:31 */
+	dsisr = (instr & 0x03ff0000) >> 16;
+
+	if (IS_XFORM(instr)) {
+		/* bits 29:30 --> 15:16 */
+		dsisr |= (instr & 0x00000006) << 14;
+		/* bit     25 -->    17 */
+		dsisr |= (instr & 0x00000040) << 8;
+		/* bits 21:24 --> 18:21 */
+		dsisr |= (instr & 0x00000780) << 3;
+	} else {
+		/* bit      5 -->    17 */
+		dsisr |= (instr & 0x04000000) >> 12;
+		/* bits  1: 4 --> 18:21 */
+		dsisr |= (instr & 0x78000000) >> 17;
+		/* bits 30:31 --> 12:13 */
+		if (IS_DSFORM(instr))
+			dsisr |= (instr & 0x00000003) << 18;
+	}
+
+	return dsisr;
+}
 #endif /* __ASM_PPC_DISASSEMBLE_H__ */
diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
index 94908af308d8..34f55524d456 100644
--- a/arch/powerpc/kernel/align.c
+++ b/arch/powerpc/kernel/align.c
@@ -25,14 +25,13 @@
 #include <asm/cputable.h>
 #include <asm/emulated_ops.h>
 #include <asm/switch_to.h>
+#include <asm/disassemble.h>
 
 struct aligninfo {
 	unsigned char len;
 	unsigned char flags;
 };
 
-#define IS_XFORM(inst)	(((inst) >> 26) == 31)
-#define IS_DSFORM(inst)	(((inst) >> 26) >= 56)
 
 #define INVALID	{ 0, 0 }
 
@@ -192,37 +191,6 @@ static struct aligninfo aligninfo[128] = {
 };
 
 /*
- * Create a DSISR value from the instruction
- */
-static inline unsigned make_dsisr(unsigned instr)
-{
-	unsigned dsisr;
-
-
-	/* bits  6:15 --> 22:31 */
-	dsisr = (instr & 0x03ff0000) >> 16;
-
-	if (IS_XFORM(instr)) {
-		/* bits 29:30 --> 15:16 */
-		dsisr |= (instr & 0x00000006) << 14;
-		/* bit     25 -->    17 */
-		dsisr |= (instr & 0x00000040) << 8;
-		/* bits 21:24 --> 18:21 */
-		dsisr |= (instr & 0x00000780) << 3;
-	} else {
-		/* bit      5 -->    17 */
-		dsisr |= (instr & 0x04000000) >> 12;
-		/* bits  1: 4 --> 18:21 */
-		dsisr |= (instr & 0x78000000) >> 17;
-		/* bits 30:31 --> 12:13 */
-		if (IS_DSFORM(instr))
-			dsisr |= (instr & 0x00000003) << 18;
-	}
-
-	return dsisr;
-}
-
-/*
  * The dcbz (data cache block zero) instruction
  * gives an alignment fault if used on non-cacheable
  * memory.  We handle the fault mainly for the
diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c
index 99d40f8977e8..04c38f049dfd 100644
--- a/arch/powerpc/kvm/book3s_emulate.c
+++ b/arch/powerpc/kvm/book3s_emulate.c
@@ -569,48 +569,14 @@ unprivileged:
 
 u32 kvmppc_alignment_dsisr(struct kvm_vcpu *vcpu, unsigned int inst)
 {
-	u32 dsisr = 0;
-
-	/*
-	 * This is what the spec says about DSISR bits (not mentioned = 0):
-	 *
-	 * 12:13		[DS]	Set to bits 30:31
-	 * 15:16		[X]	Set to bits 29:30
-	 * 17			[X]	Set to bit 25
-	 *			[D/DS]	Set to bit 5
-	 * 18:21		[X]	Set to bits 21:24
-	 *			[D/DS]	Set to bits 1:4
-	 * 22:26			Set to bits 6:10 (RT/RS/FRT/FRS)
-	 * 27:31			Set to bits 11:15 (RA)
-	 */
-
-	switch (get_op(inst)) {
-	/* D-form */
-	case OP_LFS:
-	case OP_LFD:
-	case OP_STFD:
-	case OP_STFS:
-		dsisr |= (inst >> 12) & 0x4000;	/* bit 17 */
-		dsisr |= (inst >> 17) & 0x3c00; /* bits 18:21 */
-		break;
-	/* X-form */
-	case 31:
-		dsisr |= (inst << 14) & 0x18000; /* bits 15:16 */
-		dsisr |= (inst << 8)  & 0x04000; /* bit 17 */
-		dsisr |= (inst << 3)  & 0x03c00; /* bits 18:21 */
-		break;
-	default:
-		printk(KERN_INFO "KVM: Unaligned instruction 0x%x\n", inst);
-		break;
-	}
-
-	dsisr |= (inst >> 16) & 0x03ff; /* bits 22:31 */
-
-	return dsisr;
+	return make_dsisr(inst);
 }
 
 ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
 {
+#ifdef CONFIG_PPC_BOOK3S_64
+	return vcpu->arch.fault_dar;
+#else
 	ulong dar = 0;
 	ulong ra = get_ra(inst);
 	ulong rb = get_rb(inst);
@@ -635,4 +601,5 @@ ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
 	}
 
 	return dar;
+#endif
 }
-- 
1.9.1

[PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Aneesh Kumar K.V]

Although it's optional IBM POWER cpus always had DAR value set on
alignment interrupt. So don't try to compute these values.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
Changes from V3:
* Use make_dsisr instead of checking feature flag to decide whether to use
  saved dsisr or not

 arch/powerpc/include/asm/disassemble.h | 34 +++++++++++++++++++++++++++
 arch/powerpc/kernel/align.c            | 34 +--------------------------
 arch/powerpc/kvm/book3s_emulate.c      | 43 ++++------------------------------
 3 files changed, 40 insertions(+), 71 deletions(-)

diff --git a/arch/powerpc/include/asm/disassemble.h b/arch/powerpc/include/asm/disassemble.h
index 856f8deb557a..6330a61b875a 100644
--- a/arch/powerpc/include/asm/disassemble.h
+++ b/arch/powerpc/include/asm/disassemble.h
@@ -81,4 +81,38 @@ static inline unsigned int get_oc(u32 inst)
 {
 	return (inst >> 11) & 0x7fff;
 }
+
+#define IS_XFORM(inst)	(get_op(inst)  = 31)
+#define IS_DSFORM(inst)	(get_op(inst) >= 56)
+
+/*
+ * Create a DSISR value from the instruction
+ */
+static inline unsigned make_dsisr(unsigned instr)
+{
+	unsigned dsisr;
+
+
+	/* bits  6:15 --> 22:31 */
+	dsisr = (instr & 0x03ff0000) >> 16;
+
+	if (IS_XFORM(instr)) {
+		/* bits 29:30 --> 15:16 */
+		dsisr |= (instr & 0x00000006) << 14;
+		/* bit     25 -->    17 */
+		dsisr |= (instr & 0x00000040) << 8;
+		/* bits 21:24 --> 18:21 */
+		dsisr |= (instr & 0x00000780) << 3;
+	} else {
+		/* bit      5 -->    17 */
+		dsisr |= (instr & 0x04000000) >> 12;
+		/* bits  1: 4 --> 18:21 */
+		dsisr |= (instr & 0x78000000) >> 17;
+		/* bits 30:31 --> 12:13 */
+		if (IS_DSFORM(instr))
+			dsisr |= (instr & 0x00000003) << 18;
+	}
+
+	return dsisr;
+}
 #endif /* __ASM_PPC_DISASSEMBLE_H__ */
diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
index 94908af308d8..34f55524d456 100644
--- a/arch/powerpc/kernel/align.c
+++ b/arch/powerpc/kernel/align.c
@@ -25,14 +25,13 @@
 #include <asm/cputable.h>
 #include <asm/emulated_ops.h>
 #include <asm/switch_to.h>
+#include <asm/disassemble.h>
 
 struct aligninfo {
 	unsigned char len;
 	unsigned char flags;
 };
 
-#define IS_XFORM(inst)	(((inst) >> 26) = 31)
-#define IS_DSFORM(inst)	(((inst) >> 26) >= 56)
 
 #define INVALID	{ 0, 0 }
 
@@ -192,37 +191,6 @@ static struct aligninfo aligninfo[128] = {
 };
 
 /*
- * Create a DSISR value from the instruction
- */
-static inline unsigned make_dsisr(unsigned instr)
-{
-	unsigned dsisr;
-
-
-	/* bits  6:15 --> 22:31 */
-	dsisr = (instr & 0x03ff0000) >> 16;
-
-	if (IS_XFORM(instr)) {
-		/* bits 29:30 --> 15:16 */
-		dsisr |= (instr & 0x00000006) << 14;
-		/* bit     25 -->    17 */
-		dsisr |= (instr & 0x00000040) << 8;
-		/* bits 21:24 --> 18:21 */
-		dsisr |= (instr & 0x00000780) << 3;
-	} else {
-		/* bit      5 -->    17 */
-		dsisr |= (instr & 0x04000000) >> 12;
-		/* bits  1: 4 --> 18:21 */
-		dsisr |= (instr & 0x78000000) >> 17;
-		/* bits 30:31 --> 12:13 */
-		if (IS_DSFORM(instr))
-			dsisr |= (instr & 0x00000003) << 18;
-	}
-
-	return dsisr;
-}
-
-/*
  * The dcbz (data cache block zero) instruction
  * gives an alignment fault if used on non-cacheable
  * memory.  We handle the fault mainly for the
diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c
index 99d40f8977e8..04c38f049dfd 100644
--- a/arch/powerpc/kvm/book3s_emulate.c
+++ b/arch/powerpc/kvm/book3s_emulate.c
@@ -569,48 +569,14 @@ unprivileged:
 
 u32 kvmppc_alignment_dsisr(struct kvm_vcpu *vcpu, unsigned int inst)
 {
-	u32 dsisr = 0;
-
-	/*
-	 * This is what the spec says about DSISR bits (not mentioned = 0):
-	 *
-	 * 12:13		[DS]	Set to bits 30:31
-	 * 15:16		[X]	Set to bits 29:30
-	 * 17			[X]	Set to bit 25
-	 *			[D/DS]	Set to bit 5
-	 * 18:21		[X]	Set to bits 21:24
-	 *			[D/DS]	Set to bits 1:4
-	 * 22:26			Set to bits 6:10 (RT/RS/FRT/FRS)
-	 * 27:31			Set to bits 11:15 (RA)
-	 */
-
-	switch (get_op(inst)) {
-	/* D-form */
-	case OP_LFS:
-	case OP_LFD:
-	case OP_STFD:
-	case OP_STFS:
-		dsisr |= (inst >> 12) & 0x4000;	/* bit 17 */
-		dsisr |= (inst >> 17) & 0x3c00; /* bits 18:21 */
-		break;
-	/* X-form */
-	case 31:
-		dsisr |= (inst << 14) & 0x18000; /* bits 15:16 */
-		dsisr |= (inst << 8)  & 0x04000; /* bit 17 */
-		dsisr |= (inst << 3)  & 0x03c00; /* bits 18:21 */
-		break;
-	default:
-		printk(KERN_INFO "KVM: Unaligned instruction 0x%x\n", inst);
-		break;
-	}
-
-	dsisr |= (inst >> 16) & 0x03ff; /* bits 22:31 */
-
-	return dsisr;
+	return make_dsisr(inst);
 }
 
 ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
 {
+#ifdef CONFIG_PPC_BOOK3S_64
+	return vcpu->arch.fault_dar;
+#else
 	ulong dar = 0;
 	ulong ra = get_ra(inst);
 	ulong rb = get_rb(inst);
@@ -635,4 +601,5 @@ ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
 	}
 
 	return dar;
+#endif
 }
-- 
1.9.1

[PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Aneesh Kumar K.V]

Although it's optional IBM POWER cpus always had DAR value set on
alignment interrupt. So don't try to compute these values.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
Changes from V3:
* Use make_dsisr instead of checking feature flag to decide whether to use
  saved dsisr or not

 arch/powerpc/include/asm/disassemble.h | 34 +++++++++++++++++++++++++++
 arch/powerpc/kernel/align.c            | 34 +--------------------------
 arch/powerpc/kvm/book3s_emulate.c      | 43 ++++------------------------------
 3 files changed, 40 insertions(+), 71 deletions(-)

diff --git a/arch/powerpc/include/asm/disassemble.h b/arch/powerpc/include/asm/disassemble.h
index 856f8deb557a..6330a61b875a 100644
--- a/arch/powerpc/include/asm/disassemble.h
+++ b/arch/powerpc/include/asm/disassemble.h
@@ -81,4 +81,38 @@ static inline unsigned int get_oc(u32 inst)
 {
 	return (inst >> 11) & 0x7fff;
 }
+
+#define IS_XFORM(inst)	(get_op(inst)  == 31)
+#define IS_DSFORM(inst)	(get_op(inst) >= 56)
+
+/*
+ * Create a DSISR value from the instruction
+ */
+static inline unsigned make_dsisr(unsigned instr)
+{
+	unsigned dsisr;
+
+
+	/* bits  6:15 --> 22:31 */
+	dsisr = (instr & 0x03ff0000) >> 16;
+
+	if (IS_XFORM(instr)) {
+		/* bits 29:30 --> 15:16 */
+		dsisr |= (instr & 0x00000006) << 14;
+		/* bit     25 -->    17 */
+		dsisr |= (instr & 0x00000040) << 8;
+		/* bits 21:24 --> 18:21 */
+		dsisr |= (instr & 0x00000780) << 3;
+	} else {
+		/* bit      5 -->    17 */
+		dsisr |= (instr & 0x04000000) >> 12;
+		/* bits  1: 4 --> 18:21 */
+		dsisr |= (instr & 0x78000000) >> 17;
+		/* bits 30:31 --> 12:13 */
+		if (IS_DSFORM(instr))
+			dsisr |= (instr & 0x00000003) << 18;
+	}
+
+	return dsisr;
+}
 #endif /* __ASM_PPC_DISASSEMBLE_H__ */
diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
index 94908af308d8..34f55524d456 100644
--- a/arch/powerpc/kernel/align.c
+++ b/arch/powerpc/kernel/align.c
@@ -25,14 +25,13 @@
 #include <asm/cputable.h>
 #include <asm/emulated_ops.h>
 #include <asm/switch_to.h>
+#include <asm/disassemble.h>
 
 struct aligninfo {
 	unsigned char len;
 	unsigned char flags;
 };
 
-#define IS_XFORM(inst)	(((inst) >> 26) == 31)
-#define IS_DSFORM(inst)	(((inst) >> 26) >= 56)
 
 #define INVALID	{ 0, 0 }
 
@@ -192,37 +191,6 @@ static struct aligninfo aligninfo[128] = {
 };
 
 /*
- * Create a DSISR value from the instruction
- */
-static inline unsigned make_dsisr(unsigned instr)
-{
-	unsigned dsisr;
-
-
-	/* bits  6:15 --> 22:31 */
-	dsisr = (instr & 0x03ff0000) >> 16;
-
-	if (IS_XFORM(instr)) {
-		/* bits 29:30 --> 15:16 */
-		dsisr |= (instr & 0x00000006) << 14;
-		/* bit     25 -->    17 */
-		dsisr |= (instr & 0x00000040) << 8;
-		/* bits 21:24 --> 18:21 */
-		dsisr |= (instr & 0x00000780) << 3;
-	} else {
-		/* bit      5 -->    17 */
-		dsisr |= (instr & 0x04000000) >> 12;
-		/* bits  1: 4 --> 18:21 */
-		dsisr |= (instr & 0x78000000) >> 17;
-		/* bits 30:31 --> 12:13 */
-		if (IS_DSFORM(instr))
-			dsisr |= (instr & 0x00000003) << 18;
-	}
-
-	return dsisr;
-}
-
-/*
  * The dcbz (data cache block zero) instruction
  * gives an alignment fault if used on non-cacheable
  * memory.  We handle the fault mainly for the
diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c
index 99d40f8977e8..04c38f049dfd 100644
--- a/arch/powerpc/kvm/book3s_emulate.c
+++ b/arch/powerpc/kvm/book3s_emulate.c
@@ -569,48 +569,14 @@ unprivileged:
 
 u32 kvmppc_alignment_dsisr(struct kvm_vcpu *vcpu, unsigned int inst)
 {
-	u32 dsisr = 0;
-
-	/*
-	 * This is what the spec says about DSISR bits (not mentioned = 0):
-	 *
-	 * 12:13		[DS]	Set to bits 30:31
-	 * 15:16		[X]	Set to bits 29:30
-	 * 17			[X]	Set to bit 25
-	 *			[D/DS]	Set to bit 5
-	 * 18:21		[X]	Set to bits 21:24
-	 *			[D/DS]	Set to bits 1:4
-	 * 22:26			Set to bits 6:10 (RT/RS/FRT/FRS)
-	 * 27:31			Set to bits 11:15 (RA)
-	 */
-
-	switch (get_op(inst)) {
-	/* D-form */
-	case OP_LFS:
-	case OP_LFD:
-	case OP_STFD:
-	case OP_STFS:
-		dsisr |= (inst >> 12) & 0x4000;	/* bit 17 */
-		dsisr |= (inst >> 17) & 0x3c00; /* bits 18:21 */
-		break;
-	/* X-form */
-	case 31:
-		dsisr |= (inst << 14) & 0x18000; /* bits 15:16 */
-		dsisr |= (inst << 8)  & 0x04000; /* bit 17 */
-		dsisr |= (inst << 3)  & 0x03c00; /* bits 18:21 */
-		break;
-	default:
-		printk(KERN_INFO "KVM: Unaligned instruction 0x%x\n", inst);
-		break;
-	}
-
-	dsisr |= (inst >> 16) & 0x03ff; /* bits 22:31 */
-
-	return dsisr;
+	return make_dsisr(inst);
 }
 
 ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
 {
+#ifdef CONFIG_PPC_BOOK3S_64
+	return vcpu->arch.fault_dar;
+#else
 	ulong dar = 0;
 	ulong ra = get_ra(inst);
 	ulong rb = get_rb(inst);
@@ -635,4 +601,5 @@ ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
 	}
 
 	return dar;
+#endif
 }
-- 
1.9.1

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Alexander Graf]

On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
> Although it's optional IBM POWER cpus always had DAR value set on
> alignment interrupt. So don't try to compute these values.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
> Changes from V3:
> * Use make_dsisr instead of checking feature flag to decide whether to use
>    saved dsisr or not
>
>   arch/powerpc/include/asm/disassemble.h | 34 +++++++++++++++++++++++++++
>   arch/powerpc/kernel/align.c            | 34 +--------------------------
>   arch/powerpc/kvm/book3s_emulate.c      | 43 ++++------------------------------
>   3 files changed, 40 insertions(+), 71 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/disassemble.h b/arch/powerpc/include/asm/disassemble.h
> index 856f8deb557a..6330a61b875a 100644
> --- a/arch/powerpc/include/asm/disassemble.h
> +++ b/arch/powerpc/include/asm/disassemble.h
> @@ -81,4 +81,38 @@ static inline unsigned int get_oc(u32 inst)
>   {
>   	return (inst >> 11) & 0x7fff;
>   }
> +
> +#define IS_XFORM(inst)	(get_op(inst)  = 31)
> +#define IS_DSFORM(inst)	(get_op(inst) >= 56)
> +
> +/*
> + * Create a DSISR value from the instruction
> + */
> +static inline unsigned make_dsisr(unsigned instr)
> +{
> +	unsigned dsisr;
> +
> +
> +	/* bits  6:15 --> 22:31 */
> +	dsisr = (instr & 0x03ff0000) >> 16;
> +
> +	if (IS_XFORM(instr)) {
> +		/* bits 29:30 --> 15:16 */
> +		dsisr |= (instr & 0x00000006) << 14;
> +		/* bit     25 -->    17 */
> +		dsisr |= (instr & 0x00000040) << 8;
> +		/* bits 21:24 --> 18:21 */
> +		dsisr |= (instr & 0x00000780) << 3;
> +	} else {
> +		/* bit      5 -->    17 */
> +		dsisr |= (instr & 0x04000000) >> 12;
> +		/* bits  1: 4 --> 18:21 */
> +		dsisr |= (instr & 0x78000000) >> 17;
> +		/* bits 30:31 --> 12:13 */
> +		if (IS_DSFORM(instr))
> +			dsisr |= (instr & 0x00000003) << 18;
> +	}
> +
> +	return dsisr;
> +}
>   #endif /* __ASM_PPC_DISASSEMBLE_H__ */
> diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
> index 94908af308d8..34f55524d456 100644
> --- a/arch/powerpc/kernel/align.c
> +++ b/arch/powerpc/kernel/align.c
> @@ -25,14 +25,13 @@
>   #include <asm/cputable.h>
>   #include <asm/emulated_ops.h>
>   #include <asm/switch_to.h>
> +#include <asm/disassemble.h>
>   
>   struct aligninfo {
>   	unsigned char len;
>   	unsigned char flags;
>   };
>   
> -#define IS_XFORM(inst)	(((inst) >> 26) = 31)
> -#define IS_DSFORM(inst)	(((inst) >> 26) >= 56)
>   
>   #define INVALID	{ 0, 0 }
>   
> @@ -192,37 +191,6 @@ static struct aligninfo aligninfo[128] = {
>   };
>   
>   /*
> - * Create a DSISR value from the instruction
> - */
> -static inline unsigned make_dsisr(unsigned instr)
> -{
> -	unsigned dsisr;
> -
> -
> -	/* bits  6:15 --> 22:31 */
> -	dsisr = (instr & 0x03ff0000) >> 16;
> -
> -	if (IS_XFORM(instr)) {
> -		/* bits 29:30 --> 15:16 */
> -		dsisr |= (instr & 0x00000006) << 14;
> -		/* bit     25 -->    17 */
> -		dsisr |= (instr & 0x00000040) << 8;
> -		/* bits 21:24 --> 18:21 */
> -		dsisr |= (instr & 0x00000780) << 3;
> -	} else {
> -		/* bit      5 -->    17 */
> -		dsisr |= (instr & 0x04000000) >> 12;
> -		/* bits  1: 4 --> 18:21 */
> -		dsisr |= (instr & 0x78000000) >> 17;
> -		/* bits 30:31 --> 12:13 */
> -		if (IS_DSFORM(instr))
> -			dsisr |= (instr & 0x00000003) << 18;
> -	}
> -
> -	return dsisr;
> -}
> -
> -/*
>    * The dcbz (data cache block zero) instruction
>    * gives an alignment fault if used on non-cacheable
>    * memory.  We handle the fault mainly for the
> diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c
> index 99d40f8977e8..04c38f049dfd 100644
> --- a/arch/powerpc/kvm/book3s_emulate.c
> +++ b/arch/powerpc/kvm/book3s_emulate.c
> @@ -569,48 +569,14 @@ unprivileged:
>   
>   u32 kvmppc_alignment_dsisr(struct kvm_vcpu *vcpu, unsigned int inst)
>   {
> -	u32 dsisr = 0;
> -
> -	/*
> -	 * This is what the spec says about DSISR bits (not mentioned = 0):
> -	 *
> -	 * 12:13		[DS]	Set to bits 30:31
> -	 * 15:16		[X]	Set to bits 29:30
> -	 * 17			[X]	Set to bit 25
> -	 *			[D/DS]	Set to bit 5
> -	 * 18:21		[X]	Set to bits 21:24
> -	 *			[D/DS]	Set to bits 1:4
> -	 * 22:26			Set to bits 6:10 (RT/RS/FRT/FRS)
> -	 * 27:31			Set to bits 11:15 (RA)
> -	 */
> -
> -	switch (get_op(inst)) {
> -	/* D-form */
> -	case OP_LFS:
> -	case OP_LFD:
> -	case OP_STFD:
> -	case OP_STFS:
> -		dsisr |= (inst >> 12) & 0x4000;	/* bit 17 */
> -		dsisr |= (inst >> 17) & 0x3c00; /* bits 18:21 */
> -		break;
> -	/* X-form */
> -	case 31:
> -		dsisr |= (inst << 14) & 0x18000; /* bits 15:16 */
> -		dsisr |= (inst << 8)  & 0x04000; /* bit 17 */
> -		dsisr |= (inst << 3)  & 0x03c00; /* bits 18:21 */
> -		break;
> -	default:
> -		printk(KERN_INFO "KVM: Unaligned instruction 0x%x\n", inst);
> -		break;
> -	}
> -
> -	dsisr |= (inst >> 16) & 0x03ff; /* bits 22:31 */
> -
> -	return dsisr;
> +	return make_dsisr(inst);
>   }
>   
>   ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>   {
> +#ifdef CONFIG_PPC_BOOK3S_64
> +	return vcpu->arch.fault_dar;

How about PA6T and G5s?


Alex

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Alexander Graf]

On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
> Although it's optional IBM POWER cpus always had DAR value set on
> alignment interrupt. So don't try to compute these values.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
> Changes from V3:
> * Use make_dsisr instead of checking feature flag to decide whether to use
>    saved dsisr or not
>
>   arch/powerpc/include/asm/disassemble.h | 34 +++++++++++++++++++++++++++
>   arch/powerpc/kernel/align.c            | 34 +--------------------------
>   arch/powerpc/kvm/book3s_emulate.c      | 43 ++++------------------------------
>   3 files changed, 40 insertions(+), 71 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/disassemble.h b/arch/powerpc/include/asm/disassemble.h
> index 856f8deb557a..6330a61b875a 100644
> --- a/arch/powerpc/include/asm/disassemble.h
> +++ b/arch/powerpc/include/asm/disassemble.h
> @@ -81,4 +81,38 @@ static inline unsigned int get_oc(u32 inst)
>   {
>   	return (inst >> 11) & 0x7fff;
>   }
> +
> +#define IS_XFORM(inst)	(get_op(inst)  == 31)
> +#define IS_DSFORM(inst)	(get_op(inst) >= 56)
> +
> +/*
> + * Create a DSISR value from the instruction
> + */
> +static inline unsigned make_dsisr(unsigned instr)
> +{
> +	unsigned dsisr;
> +
> +
> +	/* bits  6:15 --> 22:31 */
> +	dsisr = (instr & 0x03ff0000) >> 16;
> +
> +	if (IS_XFORM(instr)) {
> +		/* bits 29:30 --> 15:16 */
> +		dsisr |= (instr & 0x00000006) << 14;
> +		/* bit     25 -->    17 */
> +		dsisr |= (instr & 0x00000040) << 8;
> +		/* bits 21:24 --> 18:21 */
> +		dsisr |= (instr & 0x00000780) << 3;
> +	} else {
> +		/* bit      5 -->    17 */
> +		dsisr |= (instr & 0x04000000) >> 12;
> +		/* bits  1: 4 --> 18:21 */
> +		dsisr |= (instr & 0x78000000) >> 17;
> +		/* bits 30:31 --> 12:13 */
> +		if (IS_DSFORM(instr))
> +			dsisr |= (instr & 0x00000003) << 18;
> +	}
> +
> +	return dsisr;
> +}
>   #endif /* __ASM_PPC_DISASSEMBLE_H__ */
> diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
> index 94908af308d8..34f55524d456 100644
> --- a/arch/powerpc/kernel/align.c
> +++ b/arch/powerpc/kernel/align.c
> @@ -25,14 +25,13 @@
>   #include <asm/cputable.h>
>   #include <asm/emulated_ops.h>
>   #include <asm/switch_to.h>
> +#include <asm/disassemble.h>
>   
>   struct aligninfo {
>   	unsigned char len;
>   	unsigned char flags;
>   };
>   
> -#define IS_XFORM(inst)	(((inst) >> 26) == 31)
> -#define IS_DSFORM(inst)	(((inst) >> 26) >= 56)
>   
>   #define INVALID	{ 0, 0 }
>   
> @@ -192,37 +191,6 @@ static struct aligninfo aligninfo[128] = {
>   };
>   
>   /*
> - * Create a DSISR value from the instruction
> - */
> -static inline unsigned make_dsisr(unsigned instr)
> -{
> -	unsigned dsisr;
> -
> -
> -	/* bits  6:15 --> 22:31 */
> -	dsisr = (instr & 0x03ff0000) >> 16;
> -
> -	if (IS_XFORM(instr)) {
> -		/* bits 29:30 --> 15:16 */
> -		dsisr |= (instr & 0x00000006) << 14;
> -		/* bit     25 -->    17 */
> -		dsisr |= (instr & 0x00000040) << 8;
> -		/* bits 21:24 --> 18:21 */
> -		dsisr |= (instr & 0x00000780) << 3;
> -	} else {
> -		/* bit      5 -->    17 */
> -		dsisr |= (instr & 0x04000000) >> 12;
> -		/* bits  1: 4 --> 18:21 */
> -		dsisr |= (instr & 0x78000000) >> 17;
> -		/* bits 30:31 --> 12:13 */
> -		if (IS_DSFORM(instr))
> -			dsisr |= (instr & 0x00000003) << 18;
> -	}
> -
> -	return dsisr;
> -}
> -
> -/*
>    * The dcbz (data cache block zero) instruction
>    * gives an alignment fault if used on non-cacheable
>    * memory.  We handle the fault mainly for the
> diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c
> index 99d40f8977e8..04c38f049dfd 100644
> --- a/arch/powerpc/kvm/book3s_emulate.c
> +++ b/arch/powerpc/kvm/book3s_emulate.c
> @@ -569,48 +569,14 @@ unprivileged:
>   
>   u32 kvmppc_alignment_dsisr(struct kvm_vcpu *vcpu, unsigned int inst)
>   {
> -	u32 dsisr = 0;
> -
> -	/*
> -	 * This is what the spec says about DSISR bits (not mentioned = 0):
> -	 *
> -	 * 12:13		[DS]	Set to bits 30:31
> -	 * 15:16		[X]	Set to bits 29:30
> -	 * 17			[X]	Set to bit 25
> -	 *			[D/DS]	Set to bit 5
> -	 * 18:21		[X]	Set to bits 21:24
> -	 *			[D/DS]	Set to bits 1:4
> -	 * 22:26			Set to bits 6:10 (RT/RS/FRT/FRS)
> -	 * 27:31			Set to bits 11:15 (RA)
> -	 */
> -
> -	switch (get_op(inst)) {
> -	/* D-form */
> -	case OP_LFS:
> -	case OP_LFD:
> -	case OP_STFD:
> -	case OP_STFS:
> -		dsisr |= (inst >> 12) & 0x4000;	/* bit 17 */
> -		dsisr |= (inst >> 17) & 0x3c00; /* bits 18:21 */
> -		break;
> -	/* X-form */
> -	case 31:
> -		dsisr |= (inst << 14) & 0x18000; /* bits 15:16 */
> -		dsisr |= (inst << 8)  & 0x04000; /* bit 17 */
> -		dsisr |= (inst << 3)  & 0x03c00; /* bits 18:21 */
> -		break;
> -	default:
> -		printk(KERN_INFO "KVM: Unaligned instruction 0x%x\n", inst);
> -		break;
> -	}
> -
> -	dsisr |= (inst >> 16) & 0x03ff; /* bits 22:31 */
> -
> -	return dsisr;
> +	return make_dsisr(inst);
>   }
>   
>   ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>   {
> +#ifdef CONFIG_PPC_BOOK3S_64
> +	return vcpu->arch.fault_dar;

How about PA6T and G5s?


Alex

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Alexander Graf]

On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
> Although it's optional IBM POWER cpus always had DAR value set on
> alignment interrupt. So don't try to compute these values.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
> Changes from V3:
> * Use make_dsisr instead of checking feature flag to decide whether to use
>    saved dsisr or not
>
>   arch/powerpc/include/asm/disassemble.h | 34 +++++++++++++++++++++++++++
>   arch/powerpc/kernel/align.c            | 34 +--------------------------
>   arch/powerpc/kvm/book3s_emulate.c      | 43 ++++------------------------------
>   3 files changed, 40 insertions(+), 71 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/disassemble.h b/arch/powerpc/include/asm/disassemble.h
> index 856f8deb557a..6330a61b875a 100644
> --- a/arch/powerpc/include/asm/disassemble.h
> +++ b/arch/powerpc/include/asm/disassemble.h
> @@ -81,4 +81,38 @@ static inline unsigned int get_oc(u32 inst)
>   {
>   	return (inst >> 11) & 0x7fff;
>   }
> +
> +#define IS_XFORM(inst)	(get_op(inst)  == 31)
> +#define IS_DSFORM(inst)	(get_op(inst) >= 56)
> +
> +/*
> + * Create a DSISR value from the instruction
> + */
> +static inline unsigned make_dsisr(unsigned instr)
> +{
> +	unsigned dsisr;
> +
> +
> +	/* bits  6:15 --> 22:31 */
> +	dsisr = (instr & 0x03ff0000) >> 16;
> +
> +	if (IS_XFORM(instr)) {
> +		/* bits 29:30 --> 15:16 */
> +		dsisr |= (instr & 0x00000006) << 14;
> +		/* bit     25 -->    17 */
> +		dsisr |= (instr & 0x00000040) << 8;
> +		/* bits 21:24 --> 18:21 */
> +		dsisr |= (instr & 0x00000780) << 3;
> +	} else {
> +		/* bit      5 -->    17 */
> +		dsisr |= (instr & 0x04000000) >> 12;
> +		/* bits  1: 4 --> 18:21 */
> +		dsisr |= (instr & 0x78000000) >> 17;
> +		/* bits 30:31 --> 12:13 */
> +		if (IS_DSFORM(instr))
> +			dsisr |= (instr & 0x00000003) << 18;
> +	}
> +
> +	return dsisr;
> +}
>   #endif /* __ASM_PPC_DISASSEMBLE_H__ */
> diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
> index 94908af308d8..34f55524d456 100644
> --- a/arch/powerpc/kernel/align.c
> +++ b/arch/powerpc/kernel/align.c
> @@ -25,14 +25,13 @@
>   #include <asm/cputable.h>
>   #include <asm/emulated_ops.h>
>   #include <asm/switch_to.h>
> +#include <asm/disassemble.h>
>   
>   struct aligninfo {
>   	unsigned char len;
>   	unsigned char flags;
>   };
>   
> -#define IS_XFORM(inst)	(((inst) >> 26) == 31)
> -#define IS_DSFORM(inst)	(((inst) >> 26) >= 56)
>   
>   #define INVALID	{ 0, 0 }
>   
> @@ -192,37 +191,6 @@ static struct aligninfo aligninfo[128] = {
>   };
>   
>   /*
> - * Create a DSISR value from the instruction
> - */
> -static inline unsigned make_dsisr(unsigned instr)
> -{
> -	unsigned dsisr;
> -
> -
> -	/* bits  6:15 --> 22:31 */
> -	dsisr = (instr & 0x03ff0000) >> 16;
> -
> -	if (IS_XFORM(instr)) {
> -		/* bits 29:30 --> 15:16 */
> -		dsisr |= (instr & 0x00000006) << 14;
> -		/* bit     25 -->    17 */
> -		dsisr |= (instr & 0x00000040) << 8;
> -		/* bits 21:24 --> 18:21 */
> -		dsisr |= (instr & 0x00000780) << 3;
> -	} else {
> -		/* bit      5 -->    17 */
> -		dsisr |= (instr & 0x04000000) >> 12;
> -		/* bits  1: 4 --> 18:21 */
> -		dsisr |= (instr & 0x78000000) >> 17;
> -		/* bits 30:31 --> 12:13 */
> -		if (IS_DSFORM(instr))
> -			dsisr |= (instr & 0x00000003) << 18;
> -	}
> -
> -	return dsisr;
> -}
> -
> -/*
>    * The dcbz (data cache block zero) instruction
>    * gives an alignment fault if used on non-cacheable
>    * memory.  We handle the fault mainly for the
> diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c
> index 99d40f8977e8..04c38f049dfd 100644
> --- a/arch/powerpc/kvm/book3s_emulate.c
> +++ b/arch/powerpc/kvm/book3s_emulate.c
> @@ -569,48 +569,14 @@ unprivileged:
>   
>   u32 kvmppc_alignment_dsisr(struct kvm_vcpu *vcpu, unsigned int inst)
>   {
> -	u32 dsisr = 0;
> -
> -	/*
> -	 * This is what the spec says about DSISR bits (not mentioned = 0):
> -	 *
> -	 * 12:13		[DS]	Set to bits 30:31
> -	 * 15:16		[X]	Set to bits 29:30
> -	 * 17			[X]	Set to bit 25
> -	 *			[D/DS]	Set to bit 5
> -	 * 18:21		[X]	Set to bits 21:24
> -	 *			[D/DS]	Set to bits 1:4
> -	 * 22:26			Set to bits 6:10 (RT/RS/FRT/FRS)
> -	 * 27:31			Set to bits 11:15 (RA)
> -	 */
> -
> -	switch (get_op(inst)) {
> -	/* D-form */
> -	case OP_LFS:
> -	case OP_LFD:
> -	case OP_STFD:
> -	case OP_STFS:
> -		dsisr |= (inst >> 12) & 0x4000;	/* bit 17 */
> -		dsisr |= (inst >> 17) & 0x3c00; /* bits 18:21 */
> -		break;
> -	/* X-form */
> -	case 31:
> -		dsisr |= (inst << 14) & 0x18000; /* bits 15:16 */
> -		dsisr |= (inst << 8)  & 0x04000; /* bit 17 */
> -		dsisr |= (inst << 3)  & 0x03c00; /* bits 18:21 */
> -		break;
> -	default:
> -		printk(KERN_INFO "KVM: Unaligned instruction 0x%x\n", inst);
> -		break;
> -	}
> -
> -	dsisr |= (inst >> 16) & 0x03ff; /* bits 22:31 */
> -
> -	return dsisr;
> +	return make_dsisr(inst);
>   }
>   
>   ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>   {
> +#ifdef CONFIG_PPC_BOOK3S_64
> +	return vcpu->arch.fault_dar;

How about PA6T and G5s?


Alex

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Aneesh Kumar K.V]

Alexander Graf <agraf@suse.de> writes:

> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>> Although it's optional IBM POWER cpus always had DAR value set on
>> alignment interrupt. So don't try to compute these values.
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>> ---
>> Changes from V3:
>> * Use make_dsisr instead of checking feature flag to decide whether to use
>>    saved dsisr or not
>>

....

>>   ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>>   {
>> +#ifdef CONFIG_PPC_BOOK3S_64
>> +	return vcpu->arch.fault_dar;
>
> How about PA6T and G5s?
>
>

Paul mentioned that BOOK3S always had DAR value set on alignment
interrupt. And the patch is to enable/collect correct DAR value when
running with Little Endian PR guest. Now to limit the impact and to
enable Little Endian PR guest, I ended up doing the conditional code
only for book3s 64 for which we know for sure that we set DAR value.

-aneesh

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Aneesh Kumar K.V]

Alexander Graf <agraf@suse.de> writes:

> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>> Although it's optional IBM POWER cpus always had DAR value set on
>> alignment interrupt. So don't try to compute these values.
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>> ---
>> Changes from V3:
>> * Use make_dsisr instead of checking feature flag to decide whether to use
>>    saved dsisr or not
>>

....

>>   ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>>   {
>> +#ifdef CONFIG_PPC_BOOK3S_64
>> +	return vcpu->arch.fault_dar;
>
> How about PA6T and G5s?
>
>

Paul mentioned that BOOK3S always had DAR value set on alignment
interrupt. And the patch is to enable/collect correct DAR value when
running with Little Endian PR guest. Now to limit the impact and to
enable Little Endian PR guest, I ended up doing the conditional code
only for book3s 64 for which we know for sure that we set DAR value.

-aneesh

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Aneesh Kumar K.V]

Alexander Graf <agraf@suse.de> writes:

> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>> Although it's optional IBM POWER cpus always had DAR value set on
>> alignment interrupt. So don't try to compute these values.
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>> ---
>> Changes from V3:
>> * Use make_dsisr instead of checking feature flag to decide whether to use
>>    saved dsisr or not
>>

....

>>   ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>>   {
>> +#ifdef CONFIG_PPC_BOOK3S_64
>> +	return vcpu->arch.fault_dar;
>
> How about PA6T and G5s?
>
>

Paul mentioned that BOOK3S always had DAR value set on alignment
interrupt. And the patch is to enable/collect correct DAR value when
running with Little Endian PR guest. Now to limit the impact and to
enable Little Endian PR guest, I ended up doing the conditional code
only for book3s 64 for which we know for sure that we set DAR value.

-aneesh

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Alexander Graf]

On 05/05/2014 04:26 PM, Aneesh Kumar K.V wrote:
> Alexander Graf <agraf@suse.de> writes:
>
>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>> Although it's optional IBM POWER cpus always had DAR value set on
>>> alignment interrupt. So don't try to compute these values.
>>>
>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>> ---
>>> Changes from V3:
>>> * Use make_dsisr instead of checking feature flag to decide whether to use
>>>     saved dsisr or not
>>>
> ....
>
>>>    ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>>>    {
>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>> +	return vcpu->arch.fault_dar;
>> How about PA6T and G5s?
>>
>>
> Paul mentioned that BOOK3S always had DAR value set on alignment
> interrupt. And the patch is to enable/collect correct DAR value when
> running with Little Endian PR guest. Now to limit the impact and to
> enable Little Endian PR guest, I ended up doing the conditional code
> only for book3s 64 for which we know for sure that we set DAR value.

Yes, and I'm asking whether we know that this statement holds true for 
PA6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is 
at least developed by IBM, I'd assume its semantics here are similar to 
POWER4, but for PA6T I wouldn't be so sure.


Alex

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Alexander Graf]

On 05/05/2014 04:26 PM, Aneesh Kumar K.V wrote:
> Alexander Graf <agraf@suse.de> writes:
>
>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>> Although it's optional IBM POWER cpus always had DAR value set on
>>> alignment interrupt. So don't try to compute these values.
>>>
>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>> ---
>>> Changes from V3:
>>> * Use make_dsisr instead of checking feature flag to decide whether to use
>>>     saved dsisr or not
>>>
> ....
>
>>>    ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>>>    {
>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>> +	return vcpu->arch.fault_dar;
>> How about PA6T and G5s?
>>
>>
> Paul mentioned that BOOK3S always had DAR value set on alignment
> interrupt. And the patch is to enable/collect correct DAR value when
> running with Little Endian PR guest. Now to limit the impact and to
> enable Little Endian PR guest, I ended up doing the conditional code
> only for book3s 64 for which we know for sure that we set DAR value.

Yes, and I'm asking whether we know that this statement holds true for 
PA6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is 
at least developed by IBM, I'd assume its semantics here are similar to 
POWER4, but for PA6T I wouldn't be so sure.


Alex

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Alexander Graf]

On 05/05/2014 04:26 PM, Aneesh Kumar K.V wrote:
> Alexander Graf <agraf@suse.de> writes:
>
>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>> Although it's optional IBM POWER cpus always had DAR value set on
>>> alignment interrupt. So don't try to compute these values.
>>>
>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>> ---
>>> Changes from V3:
>>> * Use make_dsisr instead of checking feature flag to decide whether to use
>>>     saved dsisr or not
>>>
> ....
>
>>>    ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>>>    {
>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>> +	return vcpu->arch.fault_dar;
>> How about PA6T and G5s?
>>
>>
> Paul mentioned that BOOK3S always had DAR value set on alignment
> interrupt. And the patch is to enable/collect correct DAR value when
> running with Little Endian PR guest. Now to limit the impact and to
> enable Little Endian PR guest, I ended up doing the conditional code
> only for book3s 64 for which we know for sure that we set DAR value.

Yes, and I'm asking whether we know that this statement holds true for 
PA6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is 
at least developed by IBM, I'd assume its semantics here are similar to 
POWER4, but for PA6T I wouldn't be so sure.


Alex

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Aneesh Kumar K.V]

Alexander Graf <agraf@suse.de> writes:

> On 05/05/2014 04:26 PM, Aneesh Kumar K.V wrote:
>> Alexander Graf <agraf@suse.de> writes:
>>
>>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>> Although it's optional IBM POWER cpus always had DAR value set on
>>>> alignment interrupt. So don't try to compute these values.
>>>>
>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>> ---
>>>> Changes from V3:
>>>> * Use make_dsisr instead of checking feature flag to decide whether to use
>>>>     saved dsisr or not
>>>>
>> ....
>>
>>>>    ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>>>>    {
>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>> +	return vcpu->arch.fault_dar;
>>> How about PA6T and G5s?
>>>
>>>
>> Paul mentioned that BOOK3S always had DAR value set on alignment
>> interrupt. And the patch is to enable/collect correct DAR value when
>> running with Little Endian PR guest. Now to limit the impact and to
>> enable Little Endian PR guest, I ended up doing the conditional code
>> only for book3s 64 for which we know for sure that we set DAR value.
>
> Yes, and I'm asking whether we know that this statement holds true for 
> PA6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is 
> at least developed by IBM, I'd assume its semantics here are similar to 
> POWER4, but for PA6T I wouldn't be so sure.

I will have to defer to Paul on that question. But that should not
prevent this patch from going upstream right ?

-aneesh

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Aneesh Kumar K.V]

Alexander Graf <agraf@suse.de> writes:

> On 05/05/2014 04:26 PM, Aneesh Kumar K.V wrote:
>> Alexander Graf <agraf@suse.de> writes:
>>
>>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>> Although it's optional IBM POWER cpus always had DAR value set on
>>>> alignment interrupt. So don't try to compute these values.
>>>>
>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>> ---
>>>> Changes from V3:
>>>> * Use make_dsisr instead of checking feature flag to decide whether to use
>>>>     saved dsisr or not
>>>>
>> ....
>>
>>>>    ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>>>>    {
>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>> +	return vcpu->arch.fault_dar;
>>> How about PA6T and G5s?
>>>
>>>
>> Paul mentioned that BOOK3S always had DAR value set on alignment
>> interrupt. And the patch is to enable/collect correct DAR value when
>> running with Little Endian PR guest. Now to limit the impact and to
>> enable Little Endian PR guest, I ended up doing the conditional code
>> only for book3s 64 for which we know for sure that we set DAR value.
>
> Yes, and I'm asking whether we know that this statement holds true for 
> PA6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is 
> at least developed by IBM, I'd assume its semantics here are similar to 
> POWER4, but for PA6T I wouldn't be so sure.

I will have to defer to Paul on that question. But that should not
prevent this patch from going upstream right ?

-aneesh

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Aneesh Kumar K.V]

Alexander Graf <agraf@suse.de> writes:

> On 05/05/2014 04:26 PM, Aneesh Kumar K.V wrote:
>> Alexander Graf <agraf@suse.de> writes:
>>
>>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>> Although it's optional IBM POWER cpus always had DAR value set on
>>>> alignment interrupt. So don't try to compute these values.
>>>>
>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>> ---
>>>> Changes from V3:
>>>> * Use make_dsisr instead of checking feature flag to decide whether to use
>>>>     saved dsisr or not
>>>>
>> ....
>>
>>>>    ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>>>>    {
>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>> +	return vcpu->arch.fault_dar;
>>> How about PA6T and G5s?
>>>
>>>
>> Paul mentioned that BOOK3S always had DAR value set on alignment
>> interrupt. And the patch is to enable/collect correct DAR value when
>> running with Little Endian PR guest. Now to limit the impact and to
>> enable Little Endian PR guest, I ended up doing the conditional code
>> only for book3s 64 for which we know for sure that we set DAR value.
>
> Yes, and I'm asking whether we know that this statement holds true for 
> PA6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is 
> at least developed by IBM, I'd assume its semantics here are similar to 
> POWER4, but for PA6T I wouldn't be so sure.

I will have to defer to Paul on that question. But that should not
prevent this patch from going upstream right ?

-aneesh

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Alexander Graf]

> Am 05.05.2014 um 16:50 schrieb "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>:
> 
> Alexander Graf <agraf@suse.de> writes:
> 
>>> On 05/05/2014 04:26 PM, Aneesh Kumar K.V wrote:
>>> Alexander Graf <agraf@suse.de> writes:
>>> 
>>>>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>>> Although it's optional IBM POWER cpus always had DAR value set on
>>>>> alignment interrupt. So don't try to compute these values.
>>>>> 
>>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>>> ---
>>>>> Changes from V3:
>>>>> * Use make_dsisr instead of checking feature flag to decide whether to use
>>>>>    saved dsisr or not
>>> ....
>>> 
>>>>>   ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>>>>>   {
>>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>>> +    return vcpu->arch.fault_dar;
>>>> How about PA6T and G5s?
>>> Paul mentioned that BOOK3S always had DAR value set on alignment
>>> interrupt. And the patch is to enable/collect correct DAR value when
>>> running with Little Endian PR guest. Now to limit the impact and to
>>> enable Little Endian PR guest, I ended up doing the conditional code
>>> only for book3s 64 for which we know for sure that we set DAR value.
>> 
>> Yes, and I'm asking whether we know that this statement holds true for 
>> PA6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is 
>> at least developed by IBM, I'd assume its semantics here are similar to 
>> POWER4, but for PA6T I wouldn't be so sure.
> 
> I will have to defer to Paul on that question. But that should not
> prevent this patch from going upstream right ?

Regressions are big no-gos.

Alex

> 
> -aneesh
> 

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Alexander Graf]

> Am 05.05.2014 um 16:50 schrieb "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>:
> 
> Alexander Graf <agraf@suse.de> writes:
> 
>>> On 05/05/2014 04:26 PM, Aneesh Kumar K.V wrote:
>>> Alexander Graf <agraf@suse.de> writes:
>>> 
>>>>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>>> Although it's optional IBM POWER cpus always had DAR value set on
>>>>> alignment interrupt. So don't try to compute these values.
>>>>> 
>>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>>> ---
>>>>> Changes from V3:
>>>>> * Use make_dsisr instead of checking feature flag to decide whether to use
>>>>>    saved dsisr or not
>>> ....
>>> 
>>>>>   ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>>>>>   {
>>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>>> +    return vcpu->arch.fault_dar;
>>>> How about PA6T and G5s?
>>> Paul mentioned that BOOK3S always had DAR value set on alignment
>>> interrupt. And the patch is to enable/collect correct DAR value when
>>> running with Little Endian PR guest. Now to limit the impact and to
>>> enable Little Endian PR guest, I ended up doing the conditional code
>>> only for book3s 64 for which we know for sure that we set DAR value.
>> 
>> Yes, and I'm asking whether we know that this statement holds true for 
>> PA6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is 
>> at least developed by IBM, I'd assume its semantics here are similar to 
>> POWER4, but for PA6T I wouldn't be so sure.
> 
> I will have to defer to Paul on that question. But that should not
> prevent this patch from going upstream right ?

Regressions are big no-gos.

Alex

> 
> -aneesh
> 

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Alexander Graf]

> Am 05.05.2014 um 16:50 schrieb "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet=
.ibm.com>:
>=20
> Alexander Graf <agraf@suse.de> writes:
>=20
>>> On 05/05/2014 04:26 PM, Aneesh Kumar K.V wrote:
>>> Alexander Graf <agraf@suse.de> writes:
>>>=20
>>>>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>>> Although it's optional IBM POWER cpus always had DAR value set on
>>>>> alignment interrupt. So don't try to compute these values.
>>>>>=20
>>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>>> ---
>>>>> Changes from V3:
>>>>> * Use make_dsisr instead of checking feature flag to decide whether to=
 use
>>>>>    saved dsisr or not
>>> ....
>>>=20
>>>>>   ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)=

>>>>>   {
>>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>>> +    return vcpu->arch.fault_dar;
>>>> How about PA6T and G5s?
>>> Paul mentioned that BOOK3S always had DAR value set on alignment
>>> interrupt. And the patch is to enable/collect correct DAR value when
>>> running with Little Endian PR guest. Now to limit the impact and to
>>> enable Little Endian PR guest, I ended up doing the conditional code
>>> only for book3s 64 for which we know for sure that we set DAR value.
>>=20
>> Yes, and I'm asking whether we know that this statement holds true for=20=

>> PA6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is=20=

>> at least developed by IBM, I'd assume its semantics here are similar to=20=

>> POWER4, but for PA6T I wouldn't be so sure.
>=20
> I will have to defer to Paul on that question. But that should not
> prevent this patch from going upstream right ?

Regressions are big no-gos.

Alex

>=20
> -aneesh
>=20

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Olof Johansson]

[-- Attachment #1: Type: text/plain, Size: 1866 bytes --]

2014-05-05 7:43 GMT-07:00 Alexander Graf <agraf@suse.de>:

> On 05/05/2014 04:26 PM, Aneesh Kumar K.V wrote:
>
>> Alexander Graf <agraf@suse.de> writes:
>>
>>  On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>
>>>> Although it's optional IBM POWER cpus always had DAR value set on
>>>> alignment interrupt. So don't try to compute these values.
>>>>
>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>> ---
>>>> Changes from V3:
>>>> * Use make_dsisr instead of checking feature flag to decide whether to
>>>> use
>>>>     saved dsisr or not
>>>>
>>>>  ....
>>
>>     ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>>>>    {
>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>> +       return vcpu->arch.fault_dar;
>>>>
>>> How about PA6T and G5s?
>>>
>>>
>>>  Paul mentioned that BOOK3S always had DAR value set on alignment
>> interrupt. And the patch is to enable/collect correct DAR value when
>> running with Little Endian PR guest. Now to limit the impact and to
>> enable Little Endian PR guest, I ended up doing the conditional code
>> only for book3s 64 for which we know for sure that we set DAR value.
>>
>
> Yes, and I'm asking whether we know that this statement holds true for
> PA6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is at
> least developed by IBM, I'd assume its semantics here are similar to
> POWER4, but for PA6T I wouldn't be so sure.
>
>
Thanks for looking out for us, obviously IBM doesn't (based on the reply a
minute ago).

In the end, since there's been no work to enable KVM on PA6T, I'm not too
worried. I guess it's one more thing to sort out (and check for) whenever
someone does that.

I definitely don't have cycles to deal with that myself at this time. I can
help find hardware for someone who wants to, but even then I'm guessing the
interest is pretty limited.


-Olof

[-- Attachment #2: Type: text/html, Size: 3115 bytes --]

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Olof Johansson]

[-- Attachment #1.1: Type: text/plain, Size: 1866 bytes --]

2014-05-05 7:43 GMT-07:00 Alexander Graf <agraf@suse.de>:

> On 05/05/2014 04:26 PM, Aneesh Kumar K.V wrote:
>
>> Alexander Graf <agraf@suse.de> writes:
>>
>>  On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>
>>>> Although it's optional IBM POWER cpus always had DAR value set on
>>>> alignment interrupt. So don't try to compute these values.
>>>>
>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>> ---
>>>> Changes from V3:
>>>> * Use make_dsisr instead of checking feature flag to decide whether to
>>>> use
>>>>     saved dsisr or not
>>>>
>>>>  ....
>>
>>     ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>>>>    {
>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>> +       return vcpu->arch.fault_dar;
>>>>
>>> How about PA6T and G5s?
>>>
>>>
>>>  Paul mentioned that BOOK3S always had DAR value set on alignment
>> interrupt. And the patch is to enable/collect correct DAR value when
>> running with Little Endian PR guest. Now to limit the impact and to
>> enable Little Endian PR guest, I ended up doing the conditional code
>> only for book3s 64 for which we know for sure that we set DAR value.
>>
>
> Yes, and I'm asking whether we know that this statement holds true for
> PA6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is at
> least developed by IBM, I'd assume its semantics here are similar to
> POWER4, but for PA6T I wouldn't be so sure.
>
>
Thanks for looking out for us, obviously IBM doesn't (based on the reply a
minute ago).

In the end, since there's been no work to enable KVM on PA6T, I'm not too
worried. I guess it's one more thing to sort out (and check for) whenever
someone does that.

I definitely don't have cycles to deal with that myself at this time. I can
help find hardware for someone who wants to, but even then I'm guessing the
interest is pretty limited.


-Olof

[-- Attachment #1.2: Type: text/html, Size: 3115 bytes --]

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Aneesh Kumar K.V]

Olof Johansson <olofj@google.com> writes:

> 2014-05-05 7:43 GMT-07:00 Alexander Graf <agraf@suse.de>:
>
>> On 05/05/2014 04:26 PM, Aneesh Kumar K.V wrote:
>>
>>> Alexander Graf <agraf@suse.de> writes:
>>>
>>>  On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>>
>>>>> Although it's optional IBM POWER cpus always had DAR value set on
>>>>> alignment interrupt. So don't try to compute these values.
>>>>>
>>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>>> ---
>>>>> Changes from V3:
>>>>> * Use make_dsisr instead of checking feature flag to decide whether to
>>>>> use
>>>>>     saved dsisr or not
>>>>>
>>>>>  ....
>>>
>>>     ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>>>>>    {
>>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>>> +       return vcpu->arch.fault_dar;
>>>>>
>>>> How about PA6T and G5s?
>>>>
>>>>
>>>>  Paul mentioned that BOOK3S always had DAR value set on alignment
>>> interrupt. And the patch is to enable/collect correct DAR value when
>>> running with Little Endian PR guest. Now to limit the impact and to
>>> enable Little Endian PR guest, I ended up doing the conditional code
>>> only for book3s 64 for which we know for sure that we set DAR value.
>>>
>>
>> Yes, and I'm asking whether we know that this statement holds true for
>> PA6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is at
>> least developed by IBM, I'd assume its semantics here are similar to
>> POWER4, but for PA6T I wouldn't be so sure.
>>
>>
> Thanks for looking out for us, obviously IBM doesn't (based on the reply a
> minute ago).

The reason I deferred the question to Paul is really because I don't
know enough about PA6T and G5 to comment. I intentionally restricted the
changes to BOOK3S_64 because I wanted to make sure I don't break
anything else. It is in no way to hint that others don't care.

-aneesh

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Aneesh Kumar K.V]

Olof Johansson <olofj@google.com> writes:

> 2014-05-05 7:43 GMT-07:00 Alexander Graf <agraf@suse.de>:
>
>> On 05/05/2014 04:26 PM, Aneesh Kumar K.V wrote:
>>
>>> Alexander Graf <agraf@suse.de> writes:
>>>
>>>  On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>>
>>>>> Although it's optional IBM POWER cpus always had DAR value set on
>>>>> alignment interrupt. So don't try to compute these values.
>>>>>
>>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>>> ---
>>>>> Changes from V3:
>>>>> * Use make_dsisr instead of checking feature flag to decide whether to
>>>>> use
>>>>>     saved dsisr or not
>>>>>
>>>>>  ....
>>>
>>>     ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>>>>>    {
>>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>>> +       return vcpu->arch.fault_dar;
>>>>>
>>>> How about PA6T and G5s?
>>>>
>>>>
>>>>  Paul mentioned that BOOK3S always had DAR value set on alignment
>>> interrupt. And the patch is to enable/collect correct DAR value when
>>> running with Little Endian PR guest. Now to limit the impact and to
>>> enable Little Endian PR guest, I ended up doing the conditional code
>>> only for book3s 64 for which we know for sure that we set DAR value.
>>>
>>
>> Yes, and I'm asking whether we know that this statement holds true for
>> PA6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is at
>> least developed by IBM, I'd assume its semantics here are similar to
>> POWER4, but for PA6T I wouldn't be so sure.
>>
>>
> Thanks for looking out for us, obviously IBM doesn't (based on the reply a
> minute ago).

The reason I deferred the question to Paul is really because I don't
know enough about PA6T and G5 to comment. I intentionally restricted the
changes to BOOK3S_64 because I wanted to make sure I don't break
anything else. It is in no way to hint that others don't care.

-aneesh

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Aneesh Kumar K.V]

Olof Johansson <olofj@google.com> writes:

> 2014-05-05 7:43 GMT-07:00 Alexander Graf <agraf@suse.de>:
>
>> On 05/05/2014 04:26 PM, Aneesh Kumar K.V wrote:
>>
>>> Alexander Graf <agraf@suse.de> writes:
>>>
>>>  On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>>
>>>>> Although it's optional IBM POWER cpus always had DAR value set on
>>>>> alignment interrupt. So don't try to compute these values.
>>>>>
>>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>>> ---
>>>>> Changes from V3:
>>>>> * Use make_dsisr instead of checking feature flag to decide whether to
>>>>> use
>>>>>     saved dsisr or not
>>>>>
>>>>>  ....
>>>
>>>     ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>>>>>    {
>>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>>> +       return vcpu->arch.fault_dar;
>>>>>
>>>> How about PA6T and G5s?
>>>>
>>>>
>>>>  Paul mentioned that BOOK3S always had DAR value set on alignment
>>> interrupt. And the patch is to enable/collect correct DAR value when
>>> running with Little Endian PR guest. Now to limit the impact and to
>>> enable Little Endian PR guest, I ended up doing the conditional code
>>> only for book3s 64 for which we know for sure that we set DAR value.
>>>
>>
>> Yes, and I'm asking whether we know that this statement holds true for
>> PA6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is at
>> least developed by IBM, I'd assume its semantics here are similar to
>> POWER4, but for PA6T I wouldn't be so sure.
>>
>>
> Thanks for looking out for us, obviously IBM doesn't (based on the reply a
> minute ago).

The reason I deferred the question to Paul is really because I don't
know enough about PA6T and G5 to comment. I intentionally restricted the
changes to BOOK3S_64 because I wanted to make sure I don't break
anything else. It is in no way to hint that others don't care.

-aneesh

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Olof Johansson]

2014-05-05 8:03 GMT-07:00 Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>:
> Olof Johansson <olofj@google.com> writes:
>
>> 2014-05-05 7:43 GMT-07:00 Alexander Graf <agraf@suse.de>:
>>
>>> On 05/05/2014 04:26 PM, Aneesh Kumar K.V wrote:
>>>
>>>> Alexander Graf <agraf@suse.de> writes:
>>>>
>>>>  On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>>>
>>>>>> Although it's optional IBM POWER cpus always had DAR value set on
>>>>>> alignment interrupt. So don't try to compute these values.
>>>>>>
>>>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>>>> ---
>>>>>> Changes from V3:
>>>>>> * Use make_dsisr instead of checking feature flag to decide whether to
>>>>>> use
>>>>>>     saved dsisr or not
>>>>>>
>>>>>>  ....
>>>>
>>>>     ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>>>>>>    {
>>>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>>>> +       return vcpu->arch.fault_dar;
>>>>>>
>>>>> How about PA6T and G5s?
>>>>>
>>>>>
>>>>>  Paul mentioned that BOOK3S always had DAR value set on alignment
>>>> interrupt. And the patch is to enable/collect correct DAR value when
>>>> running with Little Endian PR guest. Now to limit the impact and to
>>>> enable Little Endian PR guest, I ended up doing the conditional code
>>>> only for book3s 64 for which we know for sure that we set DAR value.
>>>>
>>>
>>> Yes, and I'm asking whether we know that this statement holds true for
>>> PA6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is at
>>> least developed by IBM, I'd assume its semantics here are similar to
>>> POWER4, but for PA6T I wouldn't be so sure.
>>>
>>>
>> Thanks for looking out for us, obviously IBM doesn't (based on the reply a
>> minute ago).
>
> The reason I deferred the question to Paul is really because I don't
> know enough about PA6T and G5 to comment. I intentionally restricted the
> changes to BOOK3S_64 because I wanted to make sure I don't break
> anything else. It is in no way to hint that others don't care.

Ah, I see -- the disconnect is that you don't think PA6T and 970 are
64-bit book3s CPUs. They are.


-Olof

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Olof Johansson]

2014-05-05 8:03 GMT-07:00 Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>:
> Olof Johansson <olofj@google.com> writes:
>
>> 2014-05-05 7:43 GMT-07:00 Alexander Graf <agraf@suse.de>:
>>
>>> On 05/05/2014 04:26 PM, Aneesh Kumar K.V wrote:
>>>
>>>> Alexander Graf <agraf@suse.de> writes:
>>>>
>>>>  On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>>>
>>>>>> Although it's optional IBM POWER cpus always had DAR value set on
>>>>>> alignment interrupt. So don't try to compute these values.
>>>>>>
>>>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>>>> ---
>>>>>> Changes from V3:
>>>>>> * Use make_dsisr instead of checking feature flag to decide whether to
>>>>>> use
>>>>>>     saved dsisr or not
>>>>>>
>>>>>>  ....
>>>>
>>>>     ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>>>>>>    {
>>>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>>>> +       return vcpu->arch.fault_dar;
>>>>>>
>>>>> How about PA6T and G5s?
>>>>>
>>>>>
>>>>>  Paul mentioned that BOOK3S always had DAR value set on alignment
>>>> interrupt. And the patch is to enable/collect correct DAR value when
>>>> running with Little Endian PR guest. Now to limit the impact and to
>>>> enable Little Endian PR guest, I ended up doing the conditional code
>>>> only for book3s 64 for which we know for sure that we set DAR value.
>>>>
>>>
>>> Yes, and I'm asking whether we know that this statement holds true for
>>> PA6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is at
>>> least developed by IBM, I'd assume its semantics here are similar to
>>> POWER4, but for PA6T I wouldn't be so sure.
>>>
>>>
>> Thanks for looking out for us, obviously IBM doesn't (based on the reply a
>> minute ago).
>
> The reason I deferred the question to Paul is really because I don't
> know enough about PA6T and G5 to comment. I intentionally restricted the
> changes to BOOK3S_64 because I wanted to make sure I don't break
> anything else. It is in no way to hint that others don't care.

Ah, I see -- the disconnect is that you don't think PA6T and 970 are
64-bit book3s CPUs. They are.


-Olof
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Olof Johansson]

2014-05-05 8:03 GMT-07:00 Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>:
> Olof Johansson <olofj@google.com> writes:
>
>> 2014-05-05 7:43 GMT-07:00 Alexander Graf <agraf@suse.de>:
>>
>>> On 05/05/2014 04:26 PM, Aneesh Kumar K.V wrote:
>>>
>>>> Alexander Graf <agraf@suse.de> writes:
>>>>
>>>>  On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>>>
>>>>>> Although it's optional IBM POWER cpus always had DAR value set on
>>>>>> alignment interrupt. So don't try to compute these values.
>>>>>>
>>>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>>>> ---
>>>>>> Changes from V3:
>>>>>> * Use make_dsisr instead of checking feature flag to decide whether to
>>>>>> use
>>>>>>     saved dsisr or not
>>>>>>
>>>>>>  ....
>>>>
>>>>     ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>>>>>>    {
>>>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>>>> +       return vcpu->arch.fault_dar;
>>>>>>
>>>>> How about PA6T and G5s?
>>>>>
>>>>>
>>>>>  Paul mentioned that BOOK3S always had DAR value set on alignment
>>>> interrupt. And the patch is to enable/collect correct DAR value when
>>>> running with Little Endian PR guest. Now to limit the impact and to
>>>> enable Little Endian PR guest, I ended up doing the conditional code
>>>> only for book3s 64 for which we know for sure that we set DAR value.
>>>>
>>>
>>> Yes, and I'm asking whether we know that this statement holds true for
>>> PA6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is at
>>> least developed by IBM, I'd assume its semantics here are similar to
>>> POWER4, but for PA6T I wouldn't be so sure.
>>>
>>>
>> Thanks for looking out for us, obviously IBM doesn't (based on the reply a
>> minute ago).
>
> The reason I deferred the question to Paul is really because I don't
> know enough about PA6T and G5 to comment. I intentionally restricted the
> changes to BOOK3S_64 because I wanted to make sure I don't break
> anything else. It is in no way to hint that others don't care.

Ah, I see -- the disconnect is that you don't think PA6T and 970 are
64-bit book3s CPUs. They are.


-Olof

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Olof Johansson]

[Now without HTML email -- it's what you get for cc:ing me at work
instead of my upstream email :)]

2014-05-05 7:43 GMT-07:00 Alexander Graf <agraf@suse.de>:
>
> On 05/05/2014 04:26 PM, Aneesh Kumar K.V wrote:
>>
>> Alexander Graf <agraf@suse.de> writes:
>>
>>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>>
>>>> Although it's optional IBM POWER cpus always had DAR value set on
>>>> alignment interrupt. So don't try to compute these values.
>>>>
>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>> ---
>>>> Changes from V3:
>>>> * Use make_dsisr instead of checking feature flag to decide whether to use
>>>>     saved dsisr or not
>>>>
>> ....
>>
>>>>    ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>>>>    {
>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>> +       return vcpu->arch.fault_dar;
>>>
>>> How about PA6T and G5s?
>>>
>>>
>> Paul mentioned that BOOK3S always had DAR value set on alignment
>> interrupt. And the patch is to enable/collect correct DAR value when
>> running with Little Endian PR guest. Now to limit the impact and to
>> enable Little Endian PR guest, I ended up doing the conditional code
>> only for book3s 64 for which we know for sure that we set DAR value.
>
>
> Yes, and I'm asking whether we know that this statement holds true for PA6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is at least developed by IBM, I'd assume its semantics here are similar to POWER4, but for PA6T I wouldn't be so sure.
>

Thanks for looking out for us, obviously IBM doesn't (based on the
reply a minute ago).

In the end, since there's been no work to enable KVM on PA6T, I'm not
too worried. I guess it's one more thing to sort out (and check for)
whenever someone does that.

I definitely don't have cycles to deal with that myself at this time.
I can help find hardware for someone who wants to, but even then I'm
guessing the interest is pretty limited.


-Olof

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Olof Johansson]

[Now without HTML email -- it's what you get for cc:ing me at work
instead of my upstream email :)]

2014-05-05 7:43 GMT-07:00 Alexander Graf <agraf@suse.de>:
>
> On 05/05/2014 04:26 PM, Aneesh Kumar K.V wrote:
>>
>> Alexander Graf <agraf@suse.de> writes:
>>
>>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>>
>>>> Although it's optional IBM POWER cpus always had DAR value set on
>>>> alignment interrupt. So don't try to compute these values.
>>>>
>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>> ---
>>>> Changes from V3:
>>>> * Use make_dsisr instead of checking feature flag to decide whether to use
>>>>     saved dsisr or not
>>>>
>> ....
>>
>>>>    ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>>>>    {
>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>> +       return vcpu->arch.fault_dar;
>>>
>>> How about PA6T and G5s?
>>>
>>>
>> Paul mentioned that BOOK3S always had DAR value set on alignment
>> interrupt. And the patch is to enable/collect correct DAR value when
>> running with Little Endian PR guest. Now to limit the impact and to
>> enable Little Endian PR guest, I ended up doing the conditional code
>> only for book3s 64 for which we know for sure that we set DAR value.
>
>
> Yes, and I'm asking whether we know that this statement holds true for PA6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is at least developed by IBM, I'd assume its semantics here are similar to POWER4, but for PA6T I wouldn't be so sure.
>

Thanks for looking out for us, obviously IBM doesn't (based on the
reply a minute ago).

In the end, since there's been no work to enable KVM on PA6T, I'm not
too worried. I guess it's one more thing to sort out (and check for)
whenever someone does that.

I definitely don't have cycles to deal with that myself at this time.
I can help find hardware for someone who wants to, but even then I'm
guessing the interest is pretty limited.


-Olof

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Olof Johansson]

[Now without HTML email -- it's what you get for cc:ing me at work
instead of my upstream email :)]

2014-05-05 7:43 GMT-07:00 Alexander Graf <agraf@suse.de>:
>
> On 05/05/2014 04:26 PM, Aneesh Kumar K.V wrote:
>>
>> Alexander Graf <agraf@suse.de> writes:
>>
>>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>>
>>>> Although it's optional IBM POWER cpus always had DAR value set on
>>>> alignment interrupt. So don't try to compute these values.
>>>>
>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>> ---
>>>> Changes from V3:
>>>> * Use make_dsisr instead of checking feature flag to decide whether to=
 use
>>>>     saved dsisr or not
>>>>
>> ....
>>
>>>>    ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst=
)
>>>>    {
>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>> +       return vcpu->arch.fault_dar;
>>>
>>> How about PA6T and G5s?
>>>
>>>
>> Paul mentioned that BOOK3S always had DAR value set on alignment
>> interrupt. And the patch is to enable/collect correct DAR value when
>> running with Little Endian PR guest. Now to limit the impact and to
>> enable Little Endian PR guest, I ended up doing the conditional code
>> only for book3s 64 for which we know for sure that we set DAR value.
>
>
> Yes, and I'm asking whether we know that this statement holds true for PA=
6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is at lea=
st developed by IBM, I'd assume its semantics here are similar to POWER4, b=
ut for PA6T I wouldn't be so sure.
>

Thanks for looking out for us, obviously IBM doesn't (based on the
reply a minute ago).

In the end, since there's been no work to enable KVM on PA6T, I'm not
too worried. I guess it's one more thing to sort out (and check for)
whenever someone does that.

I definitely don't have cycles to deal with that myself at this time.
I can help find hardware for someone who wants to, but even then I'm
guessing the interest is pretty limited.


-Olof

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Alexander Graf]

> Am 05.05.2014 um 16:57 schrieb Olof Johansson <olofj@google.com>:
> 
> [Now without HTML email -- it's what you get for cc:ing me at work
> instead of my upstream email :)]
> 
> 2014-05-05 7:43 GMT-07:00 Alexander Graf <agraf@suse.de>:
>> 
>>> On 05/05/2014 04:26 PM, Aneesh Kumar K.V wrote:
>>> 
>>> Alexander Graf <agraf@suse.de> writes:
>>> 
>>>>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>>> 
>>>>> Although it's optional IBM POWER cpus always had DAR value set on
>>>>> alignment interrupt. So don't try to compute these values.
>>>>> 
>>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>>> ---
>>>>> Changes from V3:
>>>>> * Use make_dsisr instead of checking feature flag to decide whether to use
>>>>>    saved dsisr or not
>>> ....
>>> 
>>>>>   ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>>>>>   {
>>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>>> +       return vcpu->arch.fault_dar;
>>>> 
>>>> How about PA6T and G5s?
>>> Paul mentioned that BOOK3S always had DAR value set on alignment
>>> interrupt. And the patch is to enable/collect correct DAR value when
>>> running with Little Endian PR guest. Now to limit the impact and to
>>> enable Little Endian PR guest, I ended up doing the conditional code
>>> only for book3s 64 for which we know for sure that we set DAR value.
>> 
>> 
>> Yes, and I'm asking whether we know that this statement holds true for PA6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is at least developed by IBM, I'd assume its semantics here are similar to POWER4, but for PA6T I wouldn't be so sure.
> 
> Thanks for looking out for us, obviously IBM doesn't (based on the
> reply a minute ago).
> 
> In the end, since there's been no work to enable KVM on PA6T, I'm not
> too worried. I guess it's one more thing to sort out (and check for)
> whenever someone does that.
> 
> I definitely don't have cycles to deal with that myself at this time.
> I can help find hardware for someone who wants to, but even then I'm
> guessing the interest is pretty limited.

I know of at least 1 person who successfully runs PR KVM on a PA6T, so it's neither neglected nor non-working.

If you can get me access to a pa6t system I can easily check whether alignment interrupts generate dar and dsisr properly :).


Alex

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Alexander Graf]

> Am 05.05.2014 um 16:57 schrieb Olof Johansson <olofj@google.com>:
> 
> [Now without HTML email -- it's what you get for cc:ing me at work
> instead of my upstream email :)]
> 
> 2014-05-05 7:43 GMT-07:00 Alexander Graf <agraf@suse.de>:
>> 
>>> On 05/05/2014 04:26 PM, Aneesh Kumar K.V wrote:
>>> 
>>> Alexander Graf <agraf@suse.de> writes:
>>> 
>>>>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>>> 
>>>>> Although it's optional IBM POWER cpus always had DAR value set on
>>>>> alignment interrupt. So don't try to compute these values.
>>>>> 
>>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>>> ---
>>>>> Changes from V3:
>>>>> * Use make_dsisr instead of checking feature flag to decide whether to use
>>>>>    saved dsisr or not
>>> ....
>>> 
>>>>>   ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>>>>>   {
>>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>>> +       return vcpu->arch.fault_dar;
>>>> 
>>>> How about PA6T and G5s?
>>> Paul mentioned that BOOK3S always had DAR value set on alignment
>>> interrupt. And the patch is to enable/collect correct DAR value when
>>> running with Little Endian PR guest. Now to limit the impact and to
>>> enable Little Endian PR guest, I ended up doing the conditional code
>>> only for book3s 64 for which we know for sure that we set DAR value.
>> 
>> 
>> Yes, and I'm asking whether we know that this statement holds true for PA6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is at least developed by IBM, I'd assume its semantics here are similar to POWER4, but for PA6T I wouldn't be so sure.
> 
> Thanks for looking out for us, obviously IBM doesn't (based on the
> reply a minute ago).
> 
> In the end, since there's been no work to enable KVM on PA6T, I'm not
> too worried. I guess it's one more thing to sort out (and check for)
> whenever someone does that.
> 
> I definitely don't have cycles to deal with that myself at this time.
> I can help find hardware for someone who wants to, but even then I'm
> guessing the interest is pretty limited.

I know of at least 1 person who successfully runs PR KVM on a PA6T, so it's neither neglected nor non-working.

If you can get me access to a pa6t system I can easily check whether alignment interrupts generate dar and dsisr properly :).


Alex

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Alexander Graf]

> Am 05.05.2014 um 16:57 schrieb Olof Johansson <olofj@google.com>:
>=20
> [Now without HTML email -- it's what you get for cc:ing me at work
> instead of my upstream email :)]
>=20
> 2014-05-05 7:43 GMT-07:00 Alexander Graf <agraf@suse.de>:
>>=20
>>> On 05/05/2014 04:26 PM, Aneesh Kumar K.V wrote:
>>>=20
>>> Alexander Graf <agraf@suse.de> writes:
>>>=20
>>>>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>>>=20
>>>>> Although it's optional IBM POWER cpus always had DAR value set on
>>>>> alignment interrupt. So don't try to compute these values.
>>>>>=20
>>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>>> ---
>>>>> Changes from V3:
>>>>> * Use make_dsisr instead of checking feature flag to decide whether to=
 use
>>>>>    saved dsisr or not
>>> ....
>>>=20
>>>>>   ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)=

>>>>>   {
>>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>>> +       return vcpu->arch.fault_dar;
>>>>=20
>>>> How about PA6T and G5s?
>>> Paul mentioned that BOOK3S always had DAR value set on alignment
>>> interrupt. And the patch is to enable/collect correct DAR value when
>>> running with Little Endian PR guest. Now to limit the impact and to
>>> enable Little Endian PR guest, I ended up doing the conditional code
>>> only for book3s 64 for which we know for sure that we set DAR value.
>>=20
>>=20
>> Yes, and I'm asking whether we know that this statement holds true for PA=
6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is at leas=
t developed by IBM, I'd assume its semantics here are similar to POWER4, but=
 for PA6T I wouldn't be so sure.
>=20
> Thanks for looking out for us, obviously IBM doesn't (based on the
> reply a minute ago).
>=20
> In the end, since there's been no work to enable KVM on PA6T, I'm not
> too worried. I guess it's one more thing to sort out (and check for)
> whenever someone does that.
>=20
> I definitely don't have cycles to deal with that myself at this time.
> I can help find hardware for someone who wants to, but even then I'm
> guessing the interest is pretty limited.

I know of at least 1 person who successfully runs PR KVM on a PA6T, so it's n=
either neglected nor non-working.

If you can get me access to a pa6t system I can easily check whether alignme=
nt interrupts generate dar and dsisr properly :).


Alex

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Christian Zigotzky]

Am 05.05.14 16:57, schrieb Olof Johansson:
> [Now without HTML email -- it's what you get for cc:ing me at work
> instead of my upstream email :)]
>
> 2014-05-05 7:43 GMT-07:00 Alexander Graf <agraf@suse.de>:
>> On 05/05/2014 04:26 PM, Aneesh Kumar K.V wrote:
>>> Alexander Graf <agraf@suse.de> writes:
>>>
>>>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>>> Although it's optional IBM POWER cpus always had DAR value set on
>>>>> alignment interrupt. So don't try to compute these values.
>>>>>
>>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>>> ---
>>>>> Changes from V3:
>>>>> * Use make_dsisr instead of checking feature flag to decide whether to use
>>>>>      saved dsisr or not
>>>>>
>>> ....
>>>
>>>>>     ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>>>>>     {
>>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>>> +       return vcpu->arch.fault_dar;
>>>> How about PA6T and G5s?
>>>>
>>>>
>>> Paul mentioned that BOOK3S always had DAR value set on alignment
>>> interrupt. And the patch is to enable/collect correct DAR value when
>>> running with Little Endian PR guest. Now to limit the impact and to
>>> enable Little Endian PR guest, I ended up doing the conditional code
>>> only for book3s 64 for which we know for sure that we set DAR value.
>>
>> Yes, and I'm asking whether we know that this statement holds true for PA6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is at least developed by IBM, I'd assume its semantics here are similar to POWER4, but for PA6T I wouldn't be so sure.
>>
> Thanks for looking out for us, obviously IBM doesn't (based on the
> reply a minute ago).
>
> In the end, since there's been no work to enable KVM on PA6T, I'm not
> too worried. I guess it's one more thing to sort out (and check for)
> whenever someone does that.
>
> I definitely don't have cycles to deal with that myself at this time.
> I can help find hardware for someone who wants to, but even then I'm
> guessing the interest is pretty limited.
>
>
> -Olof
> --
> 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
>
Just for info: "PR" KVM works great on my PA6T machine. I booted the 
Lubuntu 14.04 PowerPC live DVD on a QEMU virtual machine with "PR" KVM 
successfully. But Mac OS X Jaguar, Panther, and Tiger don't boot with 
KVM on Mac-on-Linux and QEMU. See 
http://forum.hyperion-entertainment.biz/viewtopic.php?f5&t\x1747.

-- Christian

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Christian Zigotzky]

Am 05.05.14 16:57, schrieb Olof Johansson:
> [Now without HTML email -- it's what you get for cc:ing me at work
> instead of my upstream email :)]
>
> 2014-05-05 7:43 GMT-07:00 Alexander Graf <agraf@suse.de>:
>> On 05/05/2014 04:26 PM, Aneesh Kumar K.V wrote:
>>> Alexander Graf <agraf@suse.de> writes:
>>>
>>>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>>> Although it's optional IBM POWER cpus always had DAR value set on
>>>>> alignment interrupt. So don't try to compute these values.
>>>>>
>>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>>> ---
>>>>> Changes from V3:
>>>>> * Use make_dsisr instead of checking feature flag to decide whether to use
>>>>>      saved dsisr or not
>>>>>
>>> ....
>>>
>>>>>     ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>>>>>     {
>>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>>> +       return vcpu->arch.fault_dar;
>>>> How about PA6T and G5s?
>>>>
>>>>
>>> Paul mentioned that BOOK3S always had DAR value set on alignment
>>> interrupt. And the patch is to enable/collect correct DAR value when
>>> running with Little Endian PR guest. Now to limit the impact and to
>>> enable Little Endian PR guest, I ended up doing the conditional code
>>> only for book3s 64 for which we know for sure that we set DAR value.
>>
>> Yes, and I'm asking whether we know that this statement holds true for PA6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is at least developed by IBM, I'd assume its semantics here are similar to POWER4, but for PA6T I wouldn't be so sure.
>>
> Thanks for looking out for us, obviously IBM doesn't (based on the
> reply a minute ago).
>
> In the end, since there's been no work to enable KVM on PA6T, I'm not
> too worried. I guess it's one more thing to sort out (and check for)
> whenever someone does that.
>
> I definitely don't have cycles to deal with that myself at this time.
> I can help find hardware for someone who wants to, but even then I'm
> guessing the interest is pretty limited.
>
>
> -Olof
> --
> 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
>
Just for info: "PR" KVM works great on my PA6T machine. I booted the 
Lubuntu 14.04 PowerPC live DVD on a QEMU virtual machine with "PR" KVM 
successfully. But Mac OS X Jaguar, Panther, and Tiger don't boot with 
KVM on Mac-on-Linux and QEMU. See 
http://forum.hyperion-entertainment.biz/viewtopic.php?f=35&t=1747.

-- Christian
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Christian Zigotzky]

Am 05.05.14 16:57, schrieb Olof Johansson:
> [Now without HTML email -- it's what you get for cc:ing me at work
> instead of my upstream email :)]
>
> 2014-05-05 7:43 GMT-07:00 Alexander Graf <agraf@suse.de>:
>> On 05/05/2014 04:26 PM, Aneesh Kumar K.V wrote:
>>> Alexander Graf <agraf@suse.de> writes:
>>>
>>>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>>> Although it's optional IBM POWER cpus always had DAR value set on
>>>>> alignment interrupt. So don't try to compute these values.
>>>>>
>>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>>> ---
>>>>> Changes from V3:
>>>>> * Use make_dsisr instead of checking feature flag to decide whether to use
>>>>>      saved dsisr or not
>>>>>
>>> ....
>>>
>>>>>     ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
>>>>>     {
>>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>>> +       return vcpu->arch.fault_dar;
>>>> How about PA6T and G5s?
>>>>
>>>>
>>> Paul mentioned that BOOK3S always had DAR value set on alignment
>>> interrupt. And the patch is to enable/collect correct DAR value when
>>> running with Little Endian PR guest. Now to limit the impact and to
>>> enable Little Endian PR guest, I ended up doing the conditional code
>>> only for book3s 64 for which we know for sure that we set DAR value.
>>
>> Yes, and I'm asking whether we know that this statement holds true for PA6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is at least developed by IBM, I'd assume its semantics here are similar to POWER4, but for PA6T I wouldn't be so sure.
>>
> Thanks for looking out for us, obviously IBM doesn't (based on the
> reply a minute ago).
>
> In the end, since there's been no work to enable KVM on PA6T, I'm not
> too worried. I guess it's one more thing to sort out (and check for)
> whenever someone does that.
>
> I definitely don't have cycles to deal with that myself at this time.
> I can help find hardware for someone who wants to, but even then I'm
> guessing the interest is pretty limited.
>
>
> -Olof
> --
> 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
>
Just for info: "PR" KVM works great on my PA6T machine. I booted the 
Lubuntu 14.04 PowerPC live DVD on a QEMU virtual machine with "PR" KVM 
successfully. But Mac OS X Jaguar, Panther, and Tiger don't boot with 
KVM on Mac-on-Linux and QEMU. See 
http://forum.hyperion-entertainment.biz/viewtopic.php?f=35&t=1747.

-- Christian

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Benjamin Herrenschmidt]

On Mon, 2014-05-05 at 16:43 +0200, Alexander Graf wrote:
> > Paul mentioned that BOOK3S always had DAR value set on alignment
> > interrupt. And the patch is to enable/collect correct DAR value when
> > running with Little Endian PR guest. Now to limit the impact and to
> > enable Little Endian PR guest, I ended up doing the conditional code
> > only for book3s 64 for which we know for sure that we set DAR value.
> 
> Yes, and I'm asking whether we know that this statement holds true for 
> PA6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is 
> at least developed by IBM, I'd assume its semantics here are similar to 
> POWER4, but for PA6T I wouldn't be so sure.

I am not aware of any PowerPC processor that does not set DAR on
alignment interrupts. Paul, are you ?

Cheers,
Ben.

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Benjamin Herrenschmidt]

On Mon, 2014-05-05 at 16:43 +0200, Alexander Graf wrote:
> > Paul mentioned that BOOK3S always had DAR value set on alignment
> > interrupt. And the patch is to enable/collect correct DAR value when
> > running with Little Endian PR guest. Now to limit the impact and to
> > enable Little Endian PR guest, I ended up doing the conditional code
> > only for book3s 64 for which we know for sure that we set DAR value.
> 
> Yes, and I'm asking whether we know that this statement holds true for 
> PA6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is 
> at least developed by IBM, I'd assume its semantics here are similar to 
> POWER4, but for PA6T I wouldn't be so sure.

I am not aware of any PowerPC processor that does not set DAR on
alignment interrupts. Paul, are you ?

Cheers,
Ben.

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Benjamin Herrenschmidt]

On Mon, 2014-05-05 at 16:43 +0200, Alexander Graf wrote:
> > Paul mentioned that BOOK3S always had DAR value set on alignment
> > interrupt. And the patch is to enable/collect correct DAR value when
> > running with Little Endian PR guest. Now to limit the impact and to
> > enable Little Endian PR guest, I ended up doing the conditional code
> > only for book3s 64 for which we know for sure that we set DAR value.
> 
> Yes, and I'm asking whether we know that this statement holds true for 
> PA6T and G5 chips which I wouldn't consider IBM POWER. Since the G5 is 
> at least developed by IBM, I'd assume its semantics here are similar to 
> POWER4, but for PA6T I wouldn't be so sure.

I am not aware of any PowerPC processor that does not set DAR on
alignment interrupts. Paul, are you ?

Cheers,
Ben.

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Benjamin Herrenschmidt]

On Mon, 2014-05-05 at 19:56 +0530, Aneesh Kumar K.V wrote:
> 
> Paul mentioned that BOOK3S always had DAR value set on alignment
> interrupt. And the patch is to enable/collect correct DAR value when
> running with Little Endian PR guest. Now to limit the impact and to
> enable Little Endian PR guest, I ended up doing the conditional code
> only for book3s 64 for which we know for sure that we set DAR value.

Only BookS ? Afaik, the kernel align.c unconditionally uses DAR on
every processor type. It's DSISR that may or may not be populated
but afaik DAR always is.

Cheers,
Ben.

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Benjamin Herrenschmidt]

On Mon, 2014-05-05 at 19:56 +0530, Aneesh Kumar K.V wrote:
> 
> Paul mentioned that BOOK3S always had DAR value set on alignment
> interrupt. And the patch is to enable/collect correct DAR value when
> running with Little Endian PR guest. Now to limit the impact and to
> enable Little Endian PR guest, I ended up doing the conditional code
> only for book3s 64 for which we know for sure that we set DAR value.

Only BookS ? Afaik, the kernel align.c unconditionally uses DAR on
every processor type. It's DSISR that may or may not be populated
but afaik DAR always is.

Cheers,
Ben.

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Benjamin Herrenschmidt]

On Mon, 2014-05-05 at 19:56 +0530, Aneesh Kumar K.V wrote:
> 
> Paul mentioned that BOOK3S always had DAR value set on alignment
> interrupt. And the patch is to enable/collect correct DAR value when
> running with Little Endian PR guest. Now to limit the impact and to
> enable Little Endian PR guest, I ended up doing the conditional code
> only for book3s 64 for which we know for sure that we set DAR value.

Only BookS ? Afaik, the kernel align.c unconditionally uses DAR on
every processor type. It's DSISR that may or may not be populated
but afaik DAR always is.

Cheers,
Ben.

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Paul Mackerras]

On Mon, May 05, 2014 at 01:19:30PM +0200, Alexander Graf wrote:
> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
> >+#ifdef CONFIG_PPC_BOOK3S_64
> >+	return vcpu->arch.fault_dar;
> 
> How about PA6T and G5s?

G5 sets DAR on an alignment interrupt.

As for PA6T, I don't know for sure, but if it doesn't, ordinary
alignment interrupts wouldn't be handled properly, since the code in
arch/powerpc/kernel/align.c assumes DAR contains the address being
accessed on all PowerPC CPUs.

Did PA Semi ever publish a user manual for the PA6T, I wonder?

Paul.

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Paul Mackerras]

On Mon, May 05, 2014 at 01:19:30PM +0200, Alexander Graf wrote:
> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
> >+#ifdef CONFIG_PPC_BOOK3S_64
> >+	return vcpu->arch.fault_dar;
> 
> How about PA6T and G5s?

G5 sets DAR on an alignment interrupt.

As for PA6T, I don't know for sure, but if it doesn't, ordinary
alignment interrupts wouldn't be handled properly, since the code in
arch/powerpc/kernel/align.c assumes DAR contains the address being
accessed on all PowerPC CPUs.

Did PA Semi ever publish a user manual for the PA6T, I wonder?

Paul.

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Paul Mackerras]

On Mon, May 05, 2014 at 01:19:30PM +0200, Alexander Graf wrote:
> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
> >+#ifdef CONFIG_PPC_BOOK3S_64
> >+	return vcpu->arch.fault_dar;
> 
> How about PA6T and G5s?

G5 sets DAR on an alignment interrupt.

As for PA6T, I don't know for sure, but if it doesn't, ordinary
alignment interrupts wouldn't be handled properly, since the code in
arch/powerpc/kernel/align.c assumes DAR contains the address being
accessed on all PowerPC CPUs.

Did PA Semi ever publish a user manual for the PA6T, I wonder?

Paul.

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Alexander Graf]

On 06.05.14 02:41, Paul Mackerras wrote:
> On Mon, May 05, 2014 at 01:19:30PM +0200, Alexander Graf wrote:
>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>> +	return vcpu->arch.fault_dar;
>> How about PA6T and G5s?
> G5 sets DAR on an alignment interrupt.
>
> As for PA6T, I don't know for sure, but if it doesn't, ordinary
> alignment interrupts wouldn't be handled properly, since the code in
> arch/powerpc/kernel/align.c assumes DAR contains the address being
> accessed on all PowerPC CPUs.

Now that's a good point. If we simply behave like Linux, I'm fine. This 
definitely deserves a comment on the #ifdef in the code.


Alex

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Alexander Graf]

On 06.05.14 02:41, Paul Mackerras wrote:
> On Mon, May 05, 2014 at 01:19:30PM +0200, Alexander Graf wrote:
>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>> +	return vcpu->arch.fault_dar;
>> How about PA6T and G5s?
> G5 sets DAR on an alignment interrupt.
>
> As for PA6T, I don't know for sure, but if it doesn't, ordinary
> alignment interrupts wouldn't be handled properly, since the code in
> arch/powerpc/kernel/align.c assumes DAR contains the address being
> accessed on all PowerPC CPUs.

Now that's a good point. If we simply behave like Linux, I'm fine. This 
definitely deserves a comment on the #ifdef in the code.


Alex

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Alexander Graf]

On 06.05.14 02:41, Paul Mackerras wrote:
> On Mon, May 05, 2014 at 01:19:30PM +0200, Alexander Graf wrote:
>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>> +	return vcpu->arch.fault_dar;
>> How about PA6T and G5s?
> G5 sets DAR on an alignment interrupt.
>
> As for PA6T, I don't know for sure, but if it doesn't, ordinary
> alignment interrupts wouldn't be handled properly, since the code in
> arch/powerpc/kernel/align.c assumes DAR contains the address being
> accessed on all PowerPC CPUs.

Now that's a good point. If we simply behave like Linux, I'm fine. This 
definitely deserves a comment on the #ifdef in the code.


Alex

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Aneesh Kumar K.V]

Alexander Graf <agraf@suse.de> writes:

> On 06.05.14 02:41, Paul Mackerras wrote:
>> On Mon, May 05, 2014 at 01:19:30PM +0200, Alexander Graf wrote:
>>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>> +	return vcpu->arch.fault_dar;
>>> How about PA6T and G5s?
>> G5 sets DAR on an alignment interrupt.
>>
>> As for PA6T, I don't know for sure, but if it doesn't, ordinary
>> alignment interrupts wouldn't be handled properly, since the code in
>> arch/powerpc/kernel/align.c assumes DAR contains the address being
>> accessed on all PowerPC CPUs.
>
> Now that's a good point. If we simply behave like Linux, I'm fine. This 
> definitely deserves a comment on the #ifdef in the code.
>

Will update and send V5

-aneesh

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Aneesh Kumar K.V]

Alexander Graf <agraf@suse.de> writes:

> On 06.05.14 02:41, Paul Mackerras wrote:
>> On Mon, May 05, 2014 at 01:19:30PM +0200, Alexander Graf wrote:
>>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>> +	return vcpu->arch.fault_dar;
>>> How about PA6T and G5s?
>> G5 sets DAR on an alignment interrupt.
>>
>> As for PA6T, I don't know for sure, but if it doesn't, ordinary
>> alignment interrupts wouldn't be handled properly, since the code in
>> arch/powerpc/kernel/align.c assumes DAR contains the address being
>> accessed on all PowerPC CPUs.
>
> Now that's a good point. If we simply behave like Linux, I'm fine. This 
> definitely deserves a comment on the #ifdef in the code.
>

Will update and send V5

-aneesh

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Aneesh Kumar K.V]

Alexander Graf <agraf@suse.de> writes:

> On 06.05.14 02:41, Paul Mackerras wrote:
>> On Mon, May 05, 2014 at 01:19:30PM +0200, Alexander Graf wrote:
>>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>> +	return vcpu->arch.fault_dar;
>>> How about PA6T and G5s?
>> G5 sets DAR on an alignment interrupt.
>>
>> As for PA6T, I don't know for sure, but if it doesn't, ordinary
>> alignment interrupts wouldn't be handled properly, since the code in
>> arch/powerpc/kernel/align.c assumes DAR contains the address being
>> accessed on all PowerPC CPUs.
>
> Now that's a good point. If we simply behave like Linux, I'm fine. This 
> definitely deserves a comment on the #ifdef in the code.
>

Will update and send V5

-aneesh

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Aneesh Kumar K.V]

Alexander Graf <agraf@suse.de> writes:

> On 06.05.14 02:41, Paul Mackerras wrote:
>> On Mon, May 05, 2014 at 01:19:30PM +0200, Alexander Graf wrote:
>>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>> +	return vcpu->arch.fault_dar;
>>> How about PA6T and G5s?
>> G5 sets DAR on an alignment interrupt.
>>
>> As for PA6T, I don't know for sure, but if it doesn't, ordinary
>> alignment interrupts wouldn't be handled properly, since the code in
>> arch/powerpc/kernel/align.c assumes DAR contains the address being
>> accessed on all PowerPC CPUs.
>
> Now that's a good point. If we simply behave like Linux, I'm fine. This 
> definitely deserves a comment on the #ifdef in the code.


How about ?

#ifdef CONFIG_PPC_BOOK3S_64
	/*
	 * Linux always expect a valid  dar as per alignment
	 * interrupt handling code (fix_alignment()). Don't compute the dar
	 * value here, instead used the saved dar value. Right now we restrict
	 * this only for BOOK3S-64.
	 */
	return vcpu->arch.fault_dar;
#else


-aneesh

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Aneesh Kumar K.V]

Alexander Graf <agraf@suse.de> writes:

> On 06.05.14 02:41, Paul Mackerras wrote:
>> On Mon, May 05, 2014 at 01:19:30PM +0200, Alexander Graf wrote:
>>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>> +	return vcpu->arch.fault_dar;
>>> How about PA6T and G5s?
>> G5 sets DAR on an alignment interrupt.
>>
>> As for PA6T, I don't know for sure, but if it doesn't, ordinary
>> alignment interrupts wouldn't be handled properly, since the code in
>> arch/powerpc/kernel/align.c assumes DAR contains the address being
>> accessed on all PowerPC CPUs.
>
> Now that's a good point. If we simply behave like Linux, I'm fine. This 
> definitely deserves a comment on the #ifdef in the code.


How about ?

#ifdef CONFIG_PPC_BOOK3S_64
	/*
	 * Linux always expect a valid  dar as per alignment
	 * interrupt handling code (fix_alignment()). Don't compute the dar
	 * value here, instead used the saved dar value. Right now we restrict
	 * this only for BOOK3S-64.
	 */
	return vcpu->arch.fault_dar;
#else


-aneesh

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Aneesh Kumar K.V]

Alexander Graf <agraf@suse.de> writes:

> On 06.05.14 02:41, Paul Mackerras wrote:
>> On Mon, May 05, 2014 at 01:19:30PM +0200, Alexander Graf wrote:
>>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>> +	return vcpu->arch.fault_dar;
>>> How about PA6T and G5s?
>> G5 sets DAR on an alignment interrupt.
>>
>> As for PA6T, I don't know for sure, but if it doesn't, ordinary
>> alignment interrupts wouldn't be handled properly, since the code in
>> arch/powerpc/kernel/align.c assumes DAR contains the address being
>> accessed on all PowerPC CPUs.
>
> Now that's a good point. If we simply behave like Linux, I'm fine. This 
> definitely deserves a comment on the #ifdef in the code.


How about ?

#ifdef CONFIG_PPC_BOOK3S_64
	/*
	 * Linux always expect a valid  dar as per alignment
	 * interrupt handling code (fix_alignment()). Don't compute the dar
	 * value here, instead used the saved dar value. Right now we restrict
	 * this only for BOOK3S-64.
	 */
	return vcpu->arch.fault_dar;
#else


-aneesh

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Alexander Graf]

On 05/06/2014 04:12 PM, Aneesh Kumar K.V wrote:
> Alexander Graf <agraf@suse.de> writes:
>
>> On 06.05.14 02:41, Paul Mackerras wrote:
>>> On Mon, May 05, 2014 at 01:19:30PM +0200, Alexander Graf wrote:
>>>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>>> +	return vcpu->arch.fault_dar;
>>>> How about PA6T and G5s?
>>> G5 sets DAR on an alignment interrupt.
>>>
>>> As for PA6T, I don't know for sure, but if it doesn't, ordinary
>>> alignment interrupts wouldn't be handled properly, since the code in
>>> arch/powerpc/kernel/align.c assumes DAR contains the address being
>>> accessed on all PowerPC CPUs.
>> Now that's a good point. If we simply behave like Linux, I'm fine. This
>> definitely deserves a comment on the #ifdef in the code.
>
> How about ?
>
> #ifdef CONFIG_PPC_BOOK3S_64
> 	/*
> 	 * Linux always expect a valid  dar as per alignment
> 	 * interrupt handling code (fix_alignment()). Don't compute the dar
> 	 * value here, instead used the saved dar value. Right now we restrict
> 	 * this only for BOOK3S-64.
> 	 */

/* Linux's fix_alignment() assumes that DAR is valid, so can we */


Alex

> 	return vcpu->arch.fault_dar;
> #else
>
>
> -aneesh
>

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Alexander Graf]

On 05/06/2014 04:12 PM, Aneesh Kumar K.V wrote:
> Alexander Graf <agraf@suse.de> writes:
>
>> On 06.05.14 02:41, Paul Mackerras wrote:
>>> On Mon, May 05, 2014 at 01:19:30PM +0200, Alexander Graf wrote:
>>>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>>> +	return vcpu->arch.fault_dar;
>>>> How about PA6T and G5s?
>>> G5 sets DAR on an alignment interrupt.
>>>
>>> As for PA6T, I don't know for sure, but if it doesn't, ordinary
>>> alignment interrupts wouldn't be handled properly, since the code in
>>> arch/powerpc/kernel/align.c assumes DAR contains the address being
>>> accessed on all PowerPC CPUs.
>> Now that's a good point. If we simply behave like Linux, I'm fine. This
>> definitely deserves a comment on the #ifdef in the code.
>
> How about ?
>
> #ifdef CONFIG_PPC_BOOK3S_64
> 	/*
> 	 * Linux always expect a valid  dar as per alignment
> 	 * interrupt handling code (fix_alignment()). Don't compute the dar
> 	 * value here, instead used the saved dar value. Right now we restrict
> 	 * this only for BOOK3S-64.
> 	 */

/* Linux's fix_alignment() assumes that DAR is valid, so can we */


Alex

> 	return vcpu->arch.fault_dar;
> #else
>
>
> -aneesh
>

Re: [PATCH V4] POWERPC: BOOK3S: KVM: Use the saved dar value and generic make_dsisr

[Alexander Graf]

On 05/06/2014 04:12 PM, Aneesh Kumar K.V wrote:
> Alexander Graf <agraf@suse.de> writes:
>
>> On 06.05.14 02:41, Paul Mackerras wrote:
>>> On Mon, May 05, 2014 at 01:19:30PM +0200, Alexander Graf wrote:
>>>> On 05/04/2014 07:21 PM, Aneesh Kumar K.V wrote:
>>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>>> +	return vcpu->arch.fault_dar;
>>>> How about PA6T and G5s?
>>> G5 sets DAR on an alignment interrupt.
>>>
>>> As for PA6T, I don't know for sure, but if it doesn't, ordinary
>>> alignment interrupts wouldn't be handled properly, since the code in
>>> arch/powerpc/kernel/align.c assumes DAR contains the address being
>>> accessed on all PowerPC CPUs.
>> Now that's a good point. If we simply behave like Linux, I'm fine. This
>> definitely deserves a comment on the #ifdef in the code.
>
> How about ?
>
> #ifdef CONFIG_PPC_BOOK3S_64
> 	/*
> 	 * Linux always expect a valid  dar as per alignment
> 	 * interrupt handling code (fix_alignment()). Don't compute the dar
> 	 * value here, instead used the saved dar value. Right now we restrict
> 	 * this only for BOOK3S-64.
> 	 */

/* Linux's fix_alignment() assumes that DAR is valid, so can we */


Alex

> 	return vcpu->arch.fault_dar;
> #else
>
>
> -aneesh
>