qemu-ppc
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-ppc] [Qemu-devel] [PATCHv3] PPC: Fix interrupt MSR value for c


From: Jakub Jermar
Subject: Re: [Qemu-ppc] [Qemu-devel] [PATCHv3] PPC: Fix interrupt MSR value for classic exception models.
Date: Wed, 18 Apr 2012 22:32:01 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20120329 Thunderbird/11.0.1

On 04/18/2012 05:30 PM, Alexander Graf wrote:
> On 04/13/2012 06:53 PM, Mark Cave-Ayland wrote:
>> On 11/04/12 02:08, David Gibson wrote:
>>
>> Hi David,
>>
>>>> Commit 41557447d30eeb944e42069513df13585f5e6c7f introduced a new
>>>> method of
>>>> calculating the MSR for the interrupt context. However this doesn't
>>>> quite
>>>> agree with the PowerISA 2.06B specification (pp. 811-814) since too
>>>> many
>>>> bits were being cleared.
>>>>
>>>> This patch corrects the calculation of the interrupt MSR for classic
>>>> exception
>>>> models whilst including additional comments to clarify which bits
>>>> are being
>>>> changed within both the MSR and the interrupt MSR.
>>>>
>>>> Signed-off-by: Mark Cave-Ayland<address@hidden>
>>>> Signed-off-by: Martin Sucha<address@hidden>
>>>> ---
>>>>   target-ppc/cpu.h    |    2 ++
>>>>   target-ppc/helper.c |   31 ++++++++++++++++++++++++++++---
>>>>   2 files changed, 30 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>>>> index ca6f1cb..9a1c493 100644
>>>> --- a/target-ppc/cpu.h
>>>> +++ b/target-ppc/cpu.h
>>>> @@ -428,6 +428,8 @@ struct ppc_slb_t {
>>>>
>>>>  
>>>> /*****************************************************************************/
>>>>
>>>>   /* Machine state register bits
>>>> definition                                    */
>>>> +#define MSR_BIT(x) ((target_ulong)1<<  MSR_##x)
>>>> +
>>>>   #define MSR_SF   63 /* Sixty-four-bit
>>>> mode                            hflags */
>>>>   #define MSR_TAG  62 /* Tag-active mode (POWERx
>>>> ?)                            */
>>>>   #define MSR_ISF  61 /* Sixty-four-bit interrupt mode on
>>>> 630                  */
>>>> diff --git a/target-ppc/helper.c b/target-ppc/helper.c
>>>> index 63a0dec..99beace 100644
>>>> --- a/target-ppc/helper.c
>>>> +++ b/target-ppc/helper.c
>>>> @@ -2478,11 +2478,36 @@ static inline void powerpc_excp(CPUPPCState
>>>> *env, int excp_model, int excp)
>>>>       qemu_log_mask(CPU_LOG_INT, "Raise exception at " TARGET_FMT_lx
>>>>                     " =>  %08x (%02x)\n", env->nip, excp,
>>>> env->error_code);
>>>>
>>>> -    /* new srr1 value excluding must-be-zero bits */
>>>> +    /* new srr1 value with interrupt-specific bits defaulting to
>>>> zero */
>>>>       msr = env->msr&  ~0x783f0000ULL;
>>>
>>> Sorry, I really should have picked this up in an earlier review round,
>>> but I think the *whole* msr calculation should be made per exception
>>> model, not just the second part.  The common statement above
>>> is.. cryptic, and I'd be mildly surprised if it was correct for all
>>> architected variants.
>>
>> Well the original value above hasn't changed from Alex's original
>> commit (I simply augmented the comment), and it agrees with my reading
>> of the specification pages as directed by Alex. Given that we also
>> agreed to minimise the impact of the patch by making the least amount
>> of changes (and it works for my HelenOS tests while preserving the
>> existing behaviour), do you still think it makes sense to change the
>> whole MSR calculation in this way?
> 
> Does HelenOS break without the patch? It worked fine for me.

Hi Alex,

I've just tested QEMU git (which includes the TLB invalidation fix) and
it seems to work with HelenOS mainline quite nice. Not sure if we can
conclude the other fix is not needed though.

Jakub



reply via email to

[Prev in Thread] Current Thread [Next in Thread]