[PATCH] powerpc: book3s: kvm: Use the saved dsisr and dar values
From: "Aneesh Kumar K.V" Don't try to compute these values. Signed-off-by: Aneesh Kumar K.V --- 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
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" > > Don't try to compute these values. > > Signed-off-by: Aneesh Kumar K.V Acked-by: Paul Mackerras ___ 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 wrote: > From: "Aneesh Kumar K.V" > > Don't try to compute these values. > > Signed-off-by: Aneesh Kumar K.V > --- > > 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 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 Signed-off-by: Avi Kivity 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 > wrote: > > > From: "Aneesh Kumar K.V" > > > > Don't try to compute these values. > > > > Signed-off-by: Aneesh Kumar K.V > > --- > > > > 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 > 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 > Signed-off-by: Avi Kivity > > 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 writes: > On 11.11.2013, at 15:02, Aneesh Kumar K.V > wrote: > >> From: "Aneesh Kumar K.V" >> >> Don't try to compute these values. >> >> Signed-off-by: Aneesh Kumar K.V >> --- >> >> 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 > 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 > Signed-off-by: Avi Kivity > > 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
> Am 19.12.2013 um 08:02 schrieb "Aneesh Kumar K.V" > : > > Alexander Graf writes: > >>> On 11.11.2013, at 15:02, Aneesh Kumar K.V >>> wrote: >>> >>> From: "Aneesh Kumar K.V" >>> >>> Don't try to compute these values. >>> >>> Signed-off-by: Aneesh Kumar K.V >>> --- >>> >>> 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 >> 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 >>Signed-off-by: Avi Kivity >> >> 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 writes: >> Am 19.12.2013 um 08:02 schrieb "Aneesh Kumar K.V" >> : >> >> Alexander Graf writes: >> On 11.11.2013, at 15:02, Aneesh Kumar K.V wrote: From: "Aneesh Kumar K.V" Don't try to compute these values. Signed-off-by: Aneesh Kumar K.V --- 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 >>> 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 >>>Signed-off-by: Avi Kivity >>> >>> 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" > : > > Alexander Graf writes: > >>> Am 19.12.2013 um 08:02 schrieb "Aneesh Kumar K.V" >>> : >>> >>> Alexander Graf writes: >>> > On 11.11.2013, at 15:02, Aneesh Kumar K.V > wrote: > > From: "Aneesh Kumar K.V" > > Don't try to compute these values. > > Signed-off-by: Aneesh Kumar K.V > --- > > 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 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 Signed-off-by: Avi Kivity 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