Re: [PATCH] powerpc: book3s: kvm: Use the saved dsisr and dar values
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
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
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
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
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
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
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
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