* [PATCH] powerpc: book3s: kvm: Use the saved dsisr and dar values
@ 2013-11-11 14:02 Aneesh Kumar K.V
2013-11-28 4:06 ` Paul Mackerras
2013-12-18 21:44 ` Alexander Graf
0 siblings, 2 replies; 8+ messages in thread
From: Aneesh Kumar K.V @ 2013-11-11 14:02 UTC (permalink / raw)
To: agraf, benh, paulus; +Cc: linuxppc-dev, kvm, kvm-ppc, Aneesh Kumar K.V
From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Don't try to compute these values.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@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: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 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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] powerpc: book3s: kvm: Use the saved dsisr and dar values
2013-11-11 14:02 [PATCH] powerpc: book3s: kvm: Use the saved dsisr and dar values Aneesh Kumar K.V
@ 2013-11-28 4:06 ` Paul Mackerras
2013-12-18 21:44 ` Alexander Graf
1 sibling, 0 replies; 8+ messages in thread
From: Paul Mackerras @ 2013-11-28 4:06 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: linuxppc-dev, agraf, kvm-ppc, kvm
On Mon, Nov 11, 2013 at 07:32:57PM +0530, Aneesh Kumar K.V wrote:
> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>
> Don't try to compute these values.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Acked-by: Paul Mackerras <paulus@samba.org>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] powerpc: book3s: kvm: Use the saved dsisr and dar values
2013-11-11 14:02 [PATCH] powerpc: book3s: kvm: Use the saved dsisr and dar values Aneesh Kumar K.V
2013-11-28 4:06 ` Paul Mackerras
@ 2013-12-18 21:44 ` Alexander Graf
2013-12-19 2:26 ` Paul Mackerras
2013-12-19 7:02 ` Aneesh Kumar K.V
1 sibling, 2 replies; 8+ messages in thread
From: Alexander Graf @ 2013-12-18 21:44 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Paul Mackerras, linuxppc-dev, kvm-ppc,
kvm@vger.kernel.org mailing list
On 11.11.2013, at 15:02, Aneesh Kumar K.V =
<aneesh.kumar@linux.vnet.ibm.com> wrote:
> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>=20
> Don't try to compute these values.
>=20
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
>=20
> 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 <agraf@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 <agraf@suse.de>
Signed-off-by: Avi Kivity <avi@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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] powerpc: book3s: kvm: Use the saved dsisr and dar values
2013-12-18 21:44 ` Alexander Graf
@ 2013-12-19 2:26 ` Paul Mackerras
2013-12-19 7:02 ` Aneesh Kumar K.V
1 sibling, 0 replies; 8+ messages in thread
From: Paul Mackerras @ 2013-12-19 2:26 UTC (permalink / raw)
To: Alexander Graf
Cc: linuxppc-dev, Aneesh Kumar K.V, kvm-ppc,
kvm@vger.kernel.org mailing list
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.kumar@linux.vnet.ibm.com> wrote:
>
> > From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
> >
> > Don't try to compute these values.
> >
> > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@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 <agraf@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 <agraf@suse.de>
> Signed-off-by: Avi Kivity <avi@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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] powerpc: book3s: kvm: Use the saved dsisr and dar values
2013-12-18 21:44 ` Alexander Graf
2013-12-19 2:26 ` Paul Mackerras
@ 2013-12-19 7:02 ` Aneesh Kumar K.V
2013-12-19 12:35 ` Alexander Graf
1 sibling, 1 reply; 8+ messages in thread
From: Aneesh Kumar K.V @ 2013-12-19 7:02 UTC (permalink / raw)
To: Alexander Graf
Cc: Paul Mackerras, linuxppc-dev, kvm-ppc,
kvm@vger.kernel.org mailing list
Alexander Graf <agraf@suse.de> writes:
> On 11.11.2013, at 15:02, Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> wrote:
>
>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>>
>> Don't try to compute these values.
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@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 <agraf@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 <agraf@suse.de>
> Signed-off-by: Avi Kivity <avi@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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] powerpc: book3s: kvm: Use the saved dsisr and dar values
2013-12-19 7:02 ` Aneesh Kumar K.V
@ 2013-12-19 12:35 ` Alexander Graf
2013-12-20 4:37 ` Aneesh Kumar K.V
0 siblings, 1 reply; 8+ messages in thread
From: Alexander Graf @ 2013-12-19 12:35 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Paul Mackerras, linuxppc-dev@lists.ozlabs.org,
kvm-ppc@vger.kernel.org, kvm@vger.kernel.org mailing list
> Am 19.12.2013 um 08:02 schrieb "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet=
.ibm.com>:
>=20
> Alexander Graf <agraf@suse.de> writes:
>=20
>>> On 11.11.2013, at 15:02, Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.c=
om> wrote:
>>>=20
>>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>>>=20
>>> Don't try to compute these values.
>>>=20
>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>> ---
>>>=20
>>> NOTE: I am not sure why we were originally computing dsisr and dar. So m=
ay 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 L=
ittle Endian
>>> PR guest to boot.
>>=20
>> It's quite easy to find out - git blame tells you all the history and poi=
nts you to commit ca7f4203b.
>>=20
>> commit ca7f4203b9b66e12d0d9968ff7dfe781f3a9695a
>> Author: Alexander Graf <agraf@suse.de>
>> Date: Wed Mar 24 21:48:28 2010 +0100
>>=20
>> KVM: PPC: Implement alignment interrupt
>>=20
>> Mac OS X has some applications - namely the Finder - that require alig=
nment
>> interrupts to work properly. So we need to implement them.
>>=20
>> But the spec for 970 and 750 also looks different. While 750 requires t=
he
>> DSISR and DAR fields to reflect some instruction bits (DSISR) and the f=
ault
>> address (DAR), the 970 declares this as an optional feature. So we nee=
d
>> to reconstruct DSISR and DAR manually.
>>=20
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> Signed-off-by: Avi Kivity <avi@redhat.com>
>>=20
>> 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.
>>=20
>> So this is clearly a nack on this patch :).
>=20
> 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 pro=
per DAR value - we can then remove that part.
But for DSISR I'm not convinced CPUs above 970 handle this correctly. So we w=
ould at least need a guest cpu check to find out whether the vcpu expects a w=
orking dsisr and emulate it then.
I don't really fully understand the problem though. Why does the calculation=
break at all for you?
Alex
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] powerpc: book3s: kvm: Use the saved dsisr and dar values
2013-12-19 12:35 ` Alexander Graf
@ 2013-12-20 4:37 ` Aneesh Kumar K.V
2013-12-20 6:38 ` Alexander Graf
0 siblings, 1 reply; 8+ messages in thread
From: Aneesh Kumar K.V @ 2013-12-20 4:37 UTC (permalink / raw)
To: Alexander Graf
Cc: Paul Mackerras, linuxppc-dev@lists.ozlabs.org,
kvm-ppc@vger.kernel.org, kvm@vger.kernel.org mailing list
Alexander Graf <agraf@suse.de> writes:
>> Am 19.12.2013 um 08:02 schrieb "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>:
>>
>> Alexander Graf <agraf@suse.de> writes:
>>
>>>> On 11.11.2013, at 15:02, Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> wrote:
>>>>
>>>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>>>>
>>>> Don't try to compute these values.
>>>>
>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@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 <agraf@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 <agraf@suse.de>
>>> Signed-off-by: Avi Kivity <avi@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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] powerpc: book3s: kvm: Use the saved dsisr and dar values
2013-12-20 4:37 ` Aneesh Kumar K.V
@ 2013-12-20 6:38 ` Alexander Graf
0 siblings, 0 replies; 8+ messages in thread
From: Alexander Graf @ 2013-12-20 6:38 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Paul Mackerras, linuxppc-dev@lists.ozlabs.org,
kvm-ppc@vger.kernel.org, kvm@vger.kernel.org mailing list
> Am 20.12.2013 um 05:37 schrieb "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet=
.ibm.com>:
>=20
> Alexander Graf <agraf@suse.de> writes:
>=20
>>> Am 19.12.2013 um 08:02 schrieb "Aneesh Kumar K.V" <aneesh.kumar@linux.vn=
et.ibm.com>:
>>>=20
>>> Alexander Graf <agraf@suse.de> writes:
>>>=20
>>>>> On 11.11.2013, at 15:02, Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm=
.com> wrote:
>>>>>=20
>>>>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>>>>>=20
>>>>> Don't try to compute these values.
>>>>>=20
>>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>>> ---
>>>>>=20
>>>>> 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 patc=
h
>>>>> "powerpc: book3s: PR: Enable Little Endian PR guest" I am able to get a=
Little Endian
>>>>> PR guest to boot.
>>>>=20
>>>> It's quite easy to find out - git blame tells you all the history and p=
oints you to commit ca7f4203b.
>>>>=20
>>>> commit ca7f4203b9b66e12d0d9968ff7dfe781f3a9695a
>>>> Author: Alexander Graf <agraf@suse.de>
>>>> Date: Wed Mar 24 21:48:28 2010 +0100
>>>>=20
>>>> KVM: PPC: Implement alignment interrupt
>>>>=20
>>>> Mac OS X has some applications - namely the Finder - that require ali=
gnment
>>>> interrupts to work properly. So we need to implement them.
>>>>=20
>>>> 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 ne=
ed
>>>> to reconstruct DSISR and DAR manually.
>>>>=20
>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>> Signed-off-by: Avi Kivity <avi@redhat.com>
>>>>=20
>>>> Read this as "on 970, alignment interrupts don't give us DSISR and DAR o=
f the faulting instruction" as otherwise I wouldn't have implemented it.
>>>>=20
>>>> So this is clearly a nack on this patch :).
>>>=20
>>> I can possibly do a if (cpu_has_feature(CPU_FTR_ARCH_201)). But do we ne=
ed
>>> to do that ? According to Paul we should always find DAR.
>>=20
>> Paul only mentioned DAR, not DSISR. Please verify whether 970 gives us a p=
roper DAR value - we can then remove that part.
>>=20
>> 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.
>=20
>>=20
>> I don't really fully understand the problem though. Why does the
>> calculation break at all for you?
>=20
>=20
> 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
>=20
> -aneesh
>=20
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-12-20 6:38 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-11 14:02 [PATCH] powerpc: book3s: kvm: Use the saved dsisr and dar values Aneesh Kumar K.V
2013-11-28 4:06 ` Paul Mackerras
2013-12-18 21:44 ` Alexander Graf
2013-12-19 2:26 ` Paul Mackerras
2013-12-19 7:02 ` Aneesh Kumar K.V
2013-12-19 12:35 ` Alexander Graf
2013-12-20 4:37 ` Aneesh Kumar K.V
2013-12-20 6:38 ` Alexander Graf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).