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

From: "Aneesh Kumar K.V" <[email-masked]>
Date: Fri, 20 Dec 2013 10:07:07 +0530

Alexander Graf <[email-masked]> writes:

>> Am 19.12.2013 um 08:02 schrieb "Aneesh Kumar K.V" <[email-masked]>:
>> 
>> Alexander Graf <[email-masked]> writes:
>> 
>>>> On 11.11.2013, at 15:02, Aneesh Kumar K.V <[email-masked]> wrote:
>>>> 
>>>> From: "Aneesh Kumar K.V" <[email-masked]>
>>>> 
>>>> Don't try to compute these values.
>>>> 
>>>> Signed-off-by: Aneesh Kumar K.V <[email-masked]>
>>>> ---
>>>> 
>>>> 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 <[email-masked]>
>>> 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 <[email-masked]>
>>>    Signed-off-by: Avi Kivity <[email-masked]>
>>> 
>>> 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