Re: [PATCH] powerpc: book3s: kvm: Use the saved dsisr and dar values

2013-12-19 Thread Alexander Graf


 Am 19.12.2013 um 08:02 schrieb Aneesh Kumar K.V 
 aneesh.ku...@linux.vnet.ibm.com:
 
 Alexander Graf ag...@suse.de writes:
 
 On 11.11.2013, at 15:02, Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com 
 wrote:
 
 From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
 
 Don't try to compute these values.
 
 Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
 ---
 
 NOTE: I am not sure why we were originally computing dsisr and dar. So may 
 be
 we need a variant of this patch. But with this and the additional patch
 powerpc: book3s: PR: Enable Little Endian PR guest I am able to get a 
 Little Endian
 PR guest to boot.
 
 It's quite easy to find out - git blame tells you all the history and points 
 you to commit ca7f4203b.
 
 commit ca7f4203b9b66e12d0d9968ff7dfe781f3a9695a
 Author: Alexander Graf ag...@suse.de
 Date:   Wed Mar 24 21:48:28 2010 +0100
 
KVM: PPC: Implement alignment interrupt
 
Mac OS X has some applications - namely the Finder - that require 
 alignment
interrupts to work properly. So we need to implement them.
 
But the spec for 970 and 750 also looks different. While 750 requires the
DSISR and DAR fields to reflect some instruction bits (DSISR) and the 
 fault
address (DAR), the 970 declares this as an optional feature. So we need
to reconstruct DSISR and DAR manually.
 
Signed-off-by: Alexander Graf ag...@suse.de
Signed-off-by: Avi Kivity a...@redhat.com
 
 Read this as on 970, alignment interrupts don't give us DSISR and DAR of 
 the faulting instruction as otherwise I wouldn't have implemented it.
 
 So this is clearly a nack on this patch :).
 
 I can possibly do a if (cpu_has_feature(CPU_FTR_ARCH_201)). But do we need
 to do that ? According to Paul we should always find DAR.

Paul only mentioned DAR, not DSISR. Please verify whether 970 gives us a proper 
DAR value - we can then remove that part.

But for DSISR I'm not convinced CPUs above 970 handle this correctly. So we 
would at least need a guest cpu check to find out whether the vcpu expects a 
working dsisr and emulate it then.

I don't really fully understand the problem though. Why does the calculation 
break at all for you?


Alex

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


Re: [PATCH] powerpc: book3s: kvm: Use the saved dsisr and dar values

2013-12-19 Thread Aneesh Kumar K.V
Alexander Graf ag...@suse.de writes:

 Am 19.12.2013 um 08:02 schrieb Aneesh Kumar K.V 
 aneesh.ku...@linux.vnet.ibm.com:
 
 Alexander Graf ag...@suse.de writes:
 
 On 11.11.2013, at 15:02, Aneesh Kumar K.V 
 aneesh.ku...@linux.vnet.ibm.com wrote:
 
 From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
 
 Don't try to compute these values.
 
 Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
 ---
 
 NOTE: I am not sure why we were originally computing dsisr and dar. So may 
 be
 we need a variant of this patch. But with this and the additional patch
 powerpc: book3s: PR: Enable Little Endian PR guest I am able to get a 
 Little Endian
 PR guest to boot.
 
 It's quite easy to find out - git blame tells you all the history and 
 points you to commit ca7f4203b.
 
 commit ca7f4203b9b66e12d0d9968ff7dfe781f3a9695a
 Author: Alexander Graf ag...@suse.de
 Date:   Wed Mar 24 21:48:28 2010 +0100
 
KVM: PPC: Implement alignment interrupt
 
Mac OS X has some applications - namely the Finder - that require 
 alignment
interrupts to work properly. So we need to implement them.
 
But the spec for 970 and 750 also looks different. While 750 requires the
DSISR and DAR fields to reflect some instruction bits (DSISR) and the 
 fault
address (DAR), the 970 declares this as an optional feature. So we need
to reconstruct DSISR and DAR manually.
 
Signed-off-by: Alexander Graf ag...@suse.de
Signed-off-by: Avi Kivity a...@redhat.com
 
 Read this as on 970, alignment interrupts don't give us DSISR and DAR of 
 the faulting instruction as otherwise I wouldn't have implemented it.
 
 So this is clearly a nack on this patch :).
 
 I can possibly do a if (cpu_has_feature(CPU_FTR_ARCH_201)). But do we need
 to do that ? According to Paul we should always find DAR.

 Paul only mentioned DAR, not DSISR. Please verify whether 970 gives us a 
 proper DAR value - we can then remove that part.

 But for DSISR I'm not convinced CPUs above 970 handle this
 correctly. So we would at least need a guest cpu check to find out
 whether the vcpu expects a working dsisr and emulate it then.


 I don't really fully understand the problem though. Why does the
 calculation break at all for you?


IIRC this was to get little endian PR setup to work. This is to avoid
handling new instructions, because in little endian mode we get
alignment interrupt for a larger instructon set

-aneesh

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


Re: [PATCH] powerpc: book3s: kvm: Use the saved dsisr and dar values

2013-12-19 Thread Alexander Graf


 Am 20.12.2013 um 05:37 schrieb Aneesh Kumar K.V 
 aneesh.ku...@linux.vnet.ibm.com:
 
 Alexander Graf ag...@suse.de writes:
 
 Am 19.12.2013 um 08:02 schrieb Aneesh Kumar K.V 
 aneesh.ku...@linux.vnet.ibm.com:
 
 Alexander Graf ag...@suse.de writes:
 
 On 11.11.2013, at 15:02, Aneesh Kumar K.V 
 aneesh.ku...@linux.vnet.ibm.com wrote:
 
 From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
 
 Don't try to compute these values.
 
 Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
 ---
 
 NOTE: I am not sure why we were originally computing dsisr and dar. So 
 may be
 we need a variant of this patch. But with this and the additional patch
 powerpc: book3s: PR: Enable Little Endian PR guest I am able to get a 
 Little Endian
 PR guest to boot.
 
 It's quite easy to find out - git blame tells you all the history and 
 points you to commit ca7f4203b.
 
 commit ca7f4203b9b66e12d0d9968ff7dfe781f3a9695a
 Author: Alexander Graf ag...@suse.de
 Date:   Wed Mar 24 21:48:28 2010 +0100
 
   KVM: PPC: Implement alignment interrupt
 
   Mac OS X has some applications - namely the Finder - that require 
 alignment
   interrupts to work properly. So we need to implement them.
 
   But the spec for 970 and 750 also looks different. While 750 requires the
   DSISR and DAR fields to reflect some instruction bits (DSISR) and the 
 fault
   address (DAR), the 970 declares this as an optional feature. So we need
   to reconstruct DSISR and DAR manually.
 
   Signed-off-by: Alexander Graf ag...@suse.de
   Signed-off-by: Avi Kivity a...@redhat.com
 
 Read this as on 970, alignment interrupts don't give us DSISR and DAR of 
 the faulting instruction as otherwise I wouldn't have implemented it.
 
 So this is clearly a nack on this patch :).
 
 I can possibly do a if (cpu_has_feature(CPU_FTR_ARCH_201)). But do we need
 to do that ? According to Paul we should always find DAR.
 
 Paul only mentioned DAR, not DSISR. Please verify whether 970 gives us a 
 proper DAR value - we can then remove that part.
 
 But for DSISR I'm not convinced CPUs above 970 handle this
 correctly. So we would at least need a guest cpu check to find out
 whether the vcpu expects a working dsisr and emulate it then.
 
 
 I don't really fully understand the problem though. Why does the
 calculation break at all for you?
 
 
 IIRC this was to get little endian PR setup to work. This is to avoid
 handling new instructions, because in little endian mode we get
 alignment interrupt for a larger instructon set

Ok, please limit dar/dsisr calculation to book3s_32 vcpus then :).

Alex

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


Re: [PATCH] powerpc: book3s: kvm: Use the saved dsisr and dar values

2013-12-18 Thread Alexander Graf

On 11.11.2013, at 15:02, Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com 
wrote:

 From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
 
 Don't try to compute these values.
 
 Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
 ---
 
 NOTE: I am not sure why we were originally computing dsisr and dar. So may be
 we need a variant of this patch. But with this and the additional patch
 powerpc: book3s: PR: Enable Little Endian PR guest I am able to get a 
 Little Endian
 PR guest to boot.

It's quite easy to find out - git blame tells you all the history and points 
you to commit ca7f4203b.

commit ca7f4203b9b66e12d0d9968ff7dfe781f3a9695a
Author: Alexander Graf ag...@suse.de
Date:   Wed Mar 24 21:48:28 2010 +0100

KVM: PPC: Implement alignment interrupt

Mac OS X has some applications - namely the Finder - that require alignment
interrupts to work properly. So we need to implement them.

But the spec for 970 and 750 also looks different. While 750 requires the
DSISR and DAR fields to reflect some instruction bits (DSISR) and the fault
address (DAR), the 970 declares this as an optional feature. So we need
to reconstruct DSISR and DAR manually.

Signed-off-by: Alexander Graf ag...@suse.de
Signed-off-by: Avi Kivity a...@redhat.com

Read this as on 970, alignment interrupts don't give us DSISR and DAR of the 
faulting instruction as otherwise I wouldn't have implemented it.

So this is clearly a nack on this patch :).


Alex

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


Re: [PATCH] powerpc: book3s: kvm: Use the saved dsisr and dar values

2013-12-18 Thread Paul Mackerras
On Wed, Dec 18, 2013 at 10:44:08PM +0100, Alexander Graf wrote:
 
 On 11.11.2013, at 15:02, Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com 
 wrote:
 
  From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
  
  Don't try to compute these values.
  
  Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
  ---
  
  NOTE: I am not sure why we were originally computing dsisr and dar. So may 
  be
  we need a variant of this patch. But with this and the additional patch
  powerpc: book3s: PR: Enable Little Endian PR guest I am able to get a 
  Little Endian
  PR guest to boot.
 
 It's quite easy to find out - git blame tells you all the history and points 
 you to commit ca7f4203b.
 
 commit ca7f4203b9b66e12d0d9968ff7dfe781f3a9695a
 Author: Alexander Graf ag...@suse.de
 Date:   Wed Mar 24 21:48:28 2010 +0100
 
 KVM: PPC: Implement alignment interrupt
 
 Mac OS X has some applications - namely the Finder - that require 
 alignment
 interrupts to work properly. So we need to implement them.
 
 But the spec for 970 and 750 also looks different. While 750 requires the
 DSISR and DAR fields to reflect some instruction bits (DSISR) and the 
 fault
 address (DAR), the 970 declares this as an optional feature. So we need
 to reconstruct DSISR and DAR manually.
 
 Signed-off-by: Alexander Graf ag...@suse.de
 Signed-off-by: Avi Kivity a...@redhat.com
 
 Read this as on 970, alignment interrupts don't give us DSISR and DAR of the 
 faulting instruction as otherwise I wouldn't have implemented it.

Although it's optional, all IBM POWER cpus, and as far as I know all
PowerPC cpus, set DAR on an alignment interrupt to the effective
address being accessed.  You have a valid point regarding DSISR, but
it would be nice to skip the computations where either the host CPU
provides the bits, or the virtual CPU doesn't.

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


Re: [PATCH] powerpc: book3s: kvm: Use the saved dsisr and dar values

2013-12-18 Thread Aneesh Kumar K.V
Alexander Graf ag...@suse.de writes:

 On 11.11.2013, at 15:02, Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com 
 wrote:

 From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
 
 Don't try to compute these values.
 
 Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
 ---
 
 NOTE: I am not sure why we were originally computing dsisr and dar. So may be
 we need a variant of this patch. But with this and the additional patch
 powerpc: book3s: PR: Enable Little Endian PR guest I am able to get a 
 Little Endian
 PR guest to boot.

 It's quite easy to find out - git blame tells you all the history and points 
 you to commit ca7f4203b.

 commit ca7f4203b9b66e12d0d9968ff7dfe781f3a9695a
 Author: Alexander Graf ag...@suse.de
 Date:   Wed Mar 24 21:48:28 2010 +0100

 KVM: PPC: Implement alignment interrupt

 Mac OS X has some applications - namely the Finder - that require 
 alignment
 interrupts to work properly. So we need to implement them.

 But the spec for 970 and 750 also looks different. While 750 requires the
 DSISR and DAR fields to reflect some instruction bits (DSISR) and the 
 fault
 address (DAR), the 970 declares this as an optional feature. So we need
 to reconstruct DSISR and DAR manually.

 Signed-off-by: Alexander Graf ag...@suse.de
 Signed-off-by: Avi Kivity a...@redhat.com

 Read this as on 970, alignment interrupts don't give us DSISR and DAR of the 
 faulting instruction as otherwise I wouldn't have implemented it.

 So this is clearly a nack on this patch :).

I can possibly do a if (cpu_has_feature(CPU_FTR_ARCH_201)). But do we need
to do that ? According to Paul we should always find DAR.

-aneesh

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


Re: [PATCH] powerpc: book3s: kvm: Use the saved dsisr and dar values

2013-11-27 Thread Paul Mackerras
On Mon, Nov 11, 2013 at 07:32:57PM +0530, Aneesh Kumar K.V wrote:
 From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
 
 Don't try to compute these values.
 
 Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com

Acked-by: Paul Mackerras pau...@samba.org
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


[PATCH] powerpc: book3s: kvm: Use the saved dsisr and dar values

2013-11-11 Thread Aneesh Kumar K.V
From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com

Don't try to compute these values.

Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
---

NOTE: I am not sure why we were originally computing dsisr and dar. So may be
we need a variant of this patch. But with this and the additional patch
powerpc: book3s: PR: Enable Little Endian PR guest I am able to get a Little 
Endian
PR guest to boot.

 arch/powerpc/kvm/book3s_emulate.c | 64 ++-
 1 file changed, 2 insertions(+), 62 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_emulate.c 
b/arch/powerpc/kvm/book3s_emulate.c
index 99d40f8..62768f9 100644
--- a/arch/powerpc/kvm/book3s_emulate.c
+++ b/arch/powerpc/kvm/book3s_emulate.c
@@ -569,70 +569,10 @@ 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:26Set to bits 6:10 (RT/RS/FRT/FRS)
-* 27:31Set 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 vcpu-arch.fault_dsisr;
 }
 
 ulong kvmppc_alignment_dar(struct kvm_vcpu *vcpu, unsigned int inst)
 {
-   ulong dar = 0;
-   ulong ra = get_ra(inst);
-   ulong rb = get_rb(inst);
-
-   switch (get_op(inst)) {
-   case OP_LFS:
-   case OP_LFD:
-   case OP_STFD:
-   case OP_STFS:
-   if (ra)
-   dar = kvmppc_get_gpr(vcpu, ra);
-   dar += (s32)((s16)inst);
-   break;
-   case 31:
-   if (ra)
-   dar = kvmppc_get_gpr(vcpu, ra);
-   dar += kvmppc_get_gpr(vcpu, rb);
-   break;
-   default:
-   printk(KERN_INFO KVM: Unaligned instruction 0x%x\n, inst);
-   break;
-   }
-
-   return dar;
+   return vcpu-arch.fault_dar;
 }
-- 
1.8.3.2

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