* [Qemu-devel] [Bug 608107] [NEW] ppc fails to clear MSR_POW when incurring exception
@ 2010-07-21 8:53 till
2010-07-21 8:53 ` [Qemu-devel] [Bug 608107] " till
` (4 more replies)
0 siblings, 5 replies; 17+ messages in thread
From: till @ 2010-07-21 8:53 UTC (permalink / raw)
To: qemu-devel
Public bug reported:
QEMU VERSION: 0.12.4
According to FreeScale's 'Programming Environments Manual for 32-bit Implementations of the PowerPC Architecture' [MPCFPE32B, Rev.3, 9/2005], section 6.5, table 6-7, an interrupt resets MSR_POW to zero but qemu-0.12.4 fails to do so.
Resetting the bit is necessary in order to bring the processor out of power-management since otherwise it goes to sleep right away in the exception handler, i.e., it is impossible to leave PM-mode.
** Affects: qemu
Importance: Undecided
Status: New
--
ppc fails to clear MSR_POW when incurring exception
https://bugs.launchpad.net/bugs/608107
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
Status in QEMU: New
Bug description:
QEMU VERSION: 0.12.4
According to FreeScale's 'Programming Environments Manual for 32-bit Implementations of the PowerPC Architecture' [MPCFPE32B, Rev.3, 9/2005], section 6.5, table 6-7, an interrupt resets MSR_POW to zero but qemu-0.12.4 fails to do so.
Resetting the bit is necessary in order to bring the processor out of power-management since otherwise it goes to sleep right away in the exception handler, i.e., it is impossible to leave PM-mode.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [Bug 608107] Re: ppc fails to clear MSR_POW when incurring exception
2010-07-21 8:53 [Qemu-devel] [Bug 608107] [NEW] ppc fails to clear MSR_POW when incurring exception till
@ 2010-07-21 8:53 ` till
2010-09-10 12:32 ` [Qemu-devel] [PATCH] target-ppc: clear MSR_POW on interrupt Thomas Monjalon
2010-09-10 9:22 ` [Qemu-devel] [Bug 608107] Re: ppc fails to clear MSR_POW when incurring exception Thomas Monjalon
` (3 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: till @ 2010-07-21 8:53 UTC (permalink / raw)
To: qemu-devel
** Patch added: "Suggested fix"
http://launchpadlibrarian.net/52250654/ppc-msr_pow-clear.diff
--
ppc fails to clear MSR_POW when incurring exception
https://bugs.launchpad.net/bugs/608107
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
Status in QEMU: New
Bug description:
QEMU VERSION: 0.12.4
According to FreeScale's 'Programming Environments Manual for 32-bit Implementations of the PowerPC Architecture' [MPCFPE32B, Rev.3, 9/2005], section 6.5, table 6-7, an interrupt resets MSR_POW to zero but qemu-0.12.4 fails to do so.
Resetting the bit is necessary in order to bring the processor out of power-management since otherwise it goes to sleep right away in the exception handler, i.e., it is impossible to leave PM-mode.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [Bug 608107] Re: ppc fails to clear MSR_POW when incurring exception
2010-07-21 8:53 [Qemu-devel] [Bug 608107] [NEW] ppc fails to clear MSR_POW when incurring exception till
2010-07-21 8:53 ` [Qemu-devel] [Bug 608107] " till
@ 2010-09-10 9:22 ` Thomas Monjalon
2010-09-13 13:02 ` till
` (2 subsequent siblings)
4 siblings, 0 replies; 17+ messages in thread
From: Thomas Monjalon @ 2010-09-10 9:22 UTC (permalink / raw)
To: qemu-devel
** Changed in: qemu
Status: New => Confirmed
--
ppc fails to clear MSR_POW when incurring exception
https://bugs.launchpad.net/bugs/608107
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
Status in QEMU: Confirmed
Bug description:
QEMU VERSION: 0.12.4
According to FreeScale's 'Programming Environments Manual for 32-bit Implementations of the PowerPC Architecture' [MPCFPE32B, Rev.3, 9/2005], section 6.5, table 6-7, an interrupt resets MSR_POW to zero but qemu-0.12.4 fails to do so.
Resetting the bit is necessary in order to bring the processor out of power-management since otherwise it goes to sleep right away in the exception handler, i.e., it is impossible to leave PM-mode.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH] target-ppc: clear MSR_POW on interrupt
2010-07-21 8:53 ` [Qemu-devel] [Bug 608107] " till
@ 2010-09-10 12:32 ` Thomas Monjalon
2010-09-10 13:02 ` Alexander Graf
0 siblings, 1 reply; 17+ messages in thread
From: Thomas Monjalon @ 2010-09-10 12:32 UTC (permalink / raw)
To: qemu-devel; +Cc: Bug 608107
From: till <608107@bugs.launchpad.net>
According to FreeScale's 'Programming Environments Manual for 32-bit
Implementations of the PowerPC Architecture' [MPCFPE32B, Rev.3, 9/2005],
section 6.5, table 6-7, an interrupt resets MSR_POW to zero but qemu-0.12.4
fails to do so.
Resetting the bit is necessary in order to bring the processor out of power
management since otherwise it goes to sleep right away in the exception
handler, i.e., it is impossible to leave PM-mode.
https://bugs.launchpad.net/qemu/+bug/608107
Signed-off-by: till <608107@bugs.launchpad.net>
Acked-by: Thomas Monjalon <thomas@monjalon.net>
---
target-ppc/helper.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/target-ppc/helper.c b/target-ppc/helper.c
index f66fb30..0bb353e 100644
--- a/target-ppc/helper.c
+++ b/target-ppc/helper.c
@@ -2578,6 +2578,7 @@ static inline void powerpc_excp(CPUState *env, int excp_model, int excp)
if (new_msr & ((1 << MSR_IR) | (1 << MSR_DR)))
tlb_flush(env, 1);
/* reload MSR with correct bits */
+ new_msr &= ~((target_ulong)1 << MSR_POW);
new_msr &= ~((target_ulong)1 << MSR_EE);
new_msr &= ~((target_ulong)1 << MSR_PR);
new_msr &= ~((target_ulong)1 << MSR_FP);
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH] target-ppc: clear MSR_POW on interrupt
2010-09-10 12:32 ` [Qemu-devel] [PATCH] target-ppc: clear MSR_POW on interrupt Thomas Monjalon
@ 2010-09-10 13:02 ` Alexander Graf
2010-09-10 15:03 ` Thomas Monjalon
0 siblings, 1 reply; 17+ messages in thread
From: Alexander Graf @ 2010-09-10 13:02 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: Bug 608107, qemu-devel
Thomas Monjalon wrote:
> From: till <608107@bugs.launchpad.net>
>
> According to FreeScale's 'Programming Environments Manual for 32-bit
> Implementations of the PowerPC Architecture' [MPCFPE32B, Rev.3, 9/2005],
> section 6.5, table 6-7, an interrupt resets MSR_POW to zero but qemu-0.12.4
> fails to do so.
> Resetting the bit is necessary in order to bring the processor out of power
> management since otherwise it goes to sleep right away in the exception
> handler, i.e., it is impossible to leave PM-mode.
>
This doesn't look right. POW shouldn't even get stored in SRR1. Could
you please redo the patch and make sure that mtmsr masks out MSR_POW?
Alex
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH] target-ppc: clear MSR_POW on interrupt
2010-09-10 13:02 ` Alexander Graf
@ 2010-09-10 15:03 ` Thomas Monjalon
2010-09-10 15:26 ` Alexander Graf
0 siblings, 1 reply; 17+ messages in thread
From: Thomas Monjalon @ 2010-09-10 15:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexander Graf
Alexander Graf wrote:
> Thomas Monjalon wrote:
> > From: till <608107@bugs.launchpad.net>
> >
> > According to FreeScale's 'Programming Environments Manual for 32-bit
> > Implementations of the PowerPC Architecture' [MPCFPE32B, Rev.3, 9/2005],
> > section 6.5, table 6-7, an interrupt resets MSR_POW to zero but
> > qemu-0.12.4 fails to do so.
> > Resetting the bit is necessary in order to bring the processor out of
> > power management since otherwise it goes to sleep right away in the
> > exception handler, i.e., it is impossible to leave PM-mode.
>
> This doesn't look right. POW shouldn't even get stored in SRR1. Could
> you please redo the patch and make sure that mtmsr masks out MSR_POW?
Could you point sections of the specification for these requirements ?
I think SRR1 can save any bits. Non significant bits can be overriden and are
masked out when RFI occurs.
--
Thomas
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH] target-ppc: clear MSR_POW on interrupt
2010-09-10 15:03 ` Thomas Monjalon
@ 2010-09-10 15:26 ` Alexander Graf
2010-09-10 15:46 ` Thomas Monjalon
0 siblings, 1 reply; 17+ messages in thread
From: Alexander Graf @ 2010-09-10 15:26 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: qemu-devel
Thomas Monjalon wrote:
> Alexander Graf wrote:
>
>> Thomas Monjalon wrote:
>>
>>> From: till <608107@bugs.launchpad.net>
>>>
>>> According to FreeScale's 'Programming Environments Manual for 32-bit
>>> Implementations of the PowerPC Architecture' [MPCFPE32B, Rev.3, 9/2005],
>>> section 6.5, table 6-7, an interrupt resets MSR_POW to zero but
>>> qemu-0.12.4 fails to do so.
>>> Resetting the bit is necessary in order to bring the processor out of
>>> power management since otherwise it goes to sleep right away in the
>>> exception handler, i.e., it is impossible to leave PM-mode.
>>>
>> This doesn't look right. POW shouldn't even get stored in SRR1. Could
>> you please redo the patch and make sure that mtmsr masks out MSR_POW?
>>
>
> Could you point sections of the specification for these requirements ?
>
> I think SRR1 can save any bits. Non significant bits can be overriden and are
> masked out when RFI occurs.
>
I'm not saying I'm 100% sure on this, but take a look at the e300
reference manual
(http://cache.freescale.com/files/32bit/doc/ref_manual/e300coreRM.pdf)
section 5.2.3. POW is bit 13 in this notion. I don't see it getting
saved to SRR1.
Alex
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH] target-ppc: clear MSR_POW on interrupt
2010-09-10 15:26 ` Alexander Graf
@ 2010-09-10 15:46 ` Thomas Monjalon
2010-09-10 15:50 ` Alexander Graf
0 siblings, 1 reply; 17+ messages in thread
From: Thomas Monjalon @ 2010-09-10 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexander Graf
Alexander Graf wrote:
> Thomas Monjalon wrote:
> > Alexander Graf wrote:
> >> Thomas Monjalon wrote:
> >>> From: till <608107@bugs.launchpad.net>
> >>>
> >>> According to FreeScale's 'Programming Environments Manual for 32-bit
> >>> Implementations of the PowerPC Architecture' [MPCFPE32B, Rev.3,
> >>> 9/2005], section 6.5, table 6-7, an interrupt resets MSR_POW to zero
> >>> but qemu-0.12.4 fails to do so.
> >>> Resetting the bit is necessary in order to bring the processor out of
> >>> power management since otherwise it goes to sleep right away in the
> >>> exception handler, i.e., it is impossible to leave PM-mode.
> >>
> >> This doesn't look right. POW shouldn't even get stored in SRR1. Could
> >> you please redo the patch and make sure that mtmsr masks out MSR_POW?
> >
> > Could you point sections of the specification for these requirements ?
> >
> > I think SRR1 can save any bits. Non significant bits can be overriden and
> > are masked out when RFI occurs.
>
> I'm not saying I'm 100% sure on this, but take a look at the e300
> reference manual
> (http://cache.freescale.com/files/32bit/doc/ref_manual/e300coreRM.pdf)
> section 5.2.3. POW is bit 13 in this notion. I don't see it getting
> saved to SRR1.
The current implementation (commit c3d420e) save all the bits and restore only
the needed ones (excluding POW). So POW is cleared after an interrupt.
But the subject of this patch wasn't on saved bits. It is to clear POW in the
interrupt context. For an unknown reason, it's not done and it's clearly a bug
to fix.
--
Thomas
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH] target-ppc: clear MSR_POW on interrupt
2010-09-10 15:46 ` Thomas Monjalon
@ 2010-09-10 15:50 ` Alexander Graf
2010-09-10 16:08 ` Edgar E. Iglesias
0 siblings, 1 reply; 17+ messages in thread
From: Alexander Graf @ 2010-09-10 15:50 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: qemu-devel
Thomas Monjalon wrote:
> Alexander Graf wrote:
>
>> Thomas Monjalon wrote:
>>
>>> Alexander Graf wrote:
>>>
>>>> Thomas Monjalon wrote:
>>>>
>>>>> From: till <608107@bugs.launchpad.net>
>>>>>
>>>>> According to FreeScale's 'Programming Environments Manual for 32-bit
>>>>> Implementations of the PowerPC Architecture' [MPCFPE32B, Rev.3,
>>>>> 9/2005], section 6.5, table 6-7, an interrupt resets MSR_POW to zero
>>>>> but qemu-0.12.4 fails to do so.
>>>>> Resetting the bit is necessary in order to bring the processor out of
>>>>> power management since otherwise it goes to sleep right away in the
>>>>> exception handler, i.e., it is impossible to leave PM-mode.
>>>>>
>>>> This doesn't look right. POW shouldn't even get stored in SRR1. Could
>>>> you please redo the patch and make sure that mtmsr masks out MSR_POW?
>>>>
>>> Could you point sections of the specification for these requirements ?
>>>
>>> I think SRR1 can save any bits. Non significant bits can be overriden and
>>> are masked out when RFI occurs.
>>>
>> I'm not saying I'm 100% sure on this, but take a look at the e300
>> reference manual
>> (http://cache.freescale.com/files/32bit/doc/ref_manual/e300coreRM.pdf)
>> section 5.2.3. POW is bit 13 in this notion. I don't see it getting
>> saved to SRR1.
>>
>
> The current implementation (commit c3d420e) save all the bits and restore only
> the needed ones (excluding POW). So POW is cleared after an interrupt.
>
> But the subject of this patch wasn't on saved bits. It is to clear POW in the
> interrupt context. For an unknown reason, it's not done and it's clearly a bug
> to fix.
>
I completely agree. In fact, the mere fact that a bit can slip through
this loop indicates that something goes wrong.
As far as SRR1 goes, PowerISA 2.06 draws an even more complicated
picture: Page 816 says that SRR1 bits 42:44 indicate the power save exit
reason while 46:47 indicate how much state was transferred. Bit 45 would
be MSR_POW which is defined as "set to 0".
Because that means that we'll have to set MSR_POW in to 0 in SRR1 and
the interrupt switching function, we can just set it to 0 on mtmsr. That
was my point :).
Alex
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH] target-ppc: clear MSR_POW on interrupt
2010-09-10 15:50 ` Alexander Graf
@ 2010-09-10 16:08 ` Edgar E. Iglesias
2010-09-10 16:35 ` Alexander Graf
0 siblings, 1 reply; 17+ messages in thread
From: Edgar E. Iglesias @ 2010-09-10 16:08 UTC (permalink / raw)
To: Alexander Graf; +Cc: Thomas Monjalon, qemu-devel
On Fri, Sep 10, 2010 at 05:50:27PM +0200, Alexander Graf wrote:
> Thomas Monjalon wrote:
> > Alexander Graf wrote:
> >
> >> Thomas Monjalon wrote:
> >>
> >>> Alexander Graf wrote:
> >>>
> >>>> Thomas Monjalon wrote:
> >>>>
> >>>>> From: till <608107@bugs.launchpad.net>
> >>>>>
> >>>>> According to FreeScale's 'Programming Environments Manual for 32-bit
> >>>>> Implementations of the PowerPC Architecture' [MPCFPE32B, Rev.3,
> >>>>> 9/2005], section 6.5, table 6-7, an interrupt resets MSR_POW to zero
> >>>>> but qemu-0.12.4 fails to do so.
> >>>>> Resetting the bit is necessary in order to bring the processor out of
> >>>>> power management since otherwise it goes to sleep right away in the
> >>>>> exception handler, i.e., it is impossible to leave PM-mode.
> >>>>>
> >>>> This doesn't look right. POW shouldn't even get stored in SRR1. Could
> >>>> you please redo the patch and make sure that mtmsr masks out MSR_POW?
> >>>>
> >>> Could you point sections of the specification for these requirements ?
> >>>
> >>> I think SRR1 can save any bits. Non significant bits can be overriden and
> >>> are masked out when RFI occurs.
> >>>
> >> I'm not saying I'm 100% sure on this, but take a look at the e300
> >> reference manual
> >> (http://cache.freescale.com/files/32bit/doc/ref_manual/e300coreRM.pdf)
> >> section 5.2.3. POW is bit 13 in this notion. I don't see it getting
> >> saved to SRR1.
> >>
> >
> > The current implementation (commit c3d420e) save all the bits and restore only
> > the needed ones (excluding POW). So POW is cleared after an interrupt.
> >
> > But the subject of this patch wasn't on saved bits. It is to clear POW in the
> > interrupt context. For an unknown reason, it's not done and it's clearly a bug
> > to fix.
> >
>
> I completely agree. In fact, the mere fact that a bit can slip through
> this loop indicates that something goes wrong.
>
> As far as SRR1 goes, PowerISA 2.06 draws an even more complicated
> picture: Page 816 says that SRR1 bits 42:44 indicate the power save exit
> reason while 46:47 indicate how much state was transferred. Bit 45 would
> be MSR_POW which is defined as "set to 0".
>
> Because that means that we'll have to set MSR_POW in to 0 in SRR1 and
> the interrupt switching function, we can just set it to 0 on mtmsr. That
> was my point :).
Hi,
I'm not very familiar with PPC but the way I interpret it, we should clear
the POW bit in both MSR and SRR1 when the interrupt waking the CPU is
taken. While the CPU is sleeping, the POW bit should remain set in MSR
though.
As Alex comments, the submited patch fails to make sure the SRR1 bit
13 is cleared when taking the interrupt, but I don't think clearing
MSR[POW] in mtmsr is the right thing. In practice it probably doesn't
matter much, possibly a bit annoying if you inspect the sleeping state
from a debugger or similar.
I think mtmsr should set MSR[POW] and powerpc_excp() should clear
MSR[POW] prior to copying MSR into SRR1.
Not sure, but thats the way I interpret it...
Cheers
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH] target-ppc: clear MSR_POW on interrupt
2010-09-10 16:08 ` Edgar E. Iglesias
@ 2010-09-10 16:35 ` Alexander Graf
2010-09-10 16:58 ` Thomas Monjalon
0 siblings, 1 reply; 17+ messages in thread
From: Alexander Graf @ 2010-09-10 16:35 UTC (permalink / raw)
To: Edgar E. Iglesias; +Cc: Thomas Monjalon, qemu-devel
Am 10.09.2010 um 18:08 schrieb "Edgar E. Iglesias" <edgar.iglesias@gmail.com>:
> On Fri, Sep 10, 2010 at 05:50:27PM +0200, Alexander Graf wrote:
>> Thomas Monjalon wrote:
>>> Alexander Graf wrote:
>>>
>>>> Thomas Monjalon wrote:
>>>>
>>>>> Alexander Graf wrote:
>>>>>
>>>>>> Thomas Monjalon wrote:
>>>>>>
>>>>>>> From: till <608107@bugs.launchpad.net>
>>>>>>>
>>>>>>> According to FreeScale's 'Programming Environments Manual for 32-bit
>>>>>>> Implementations of the PowerPC Architecture' [MPCFPE32B, Rev.3,
>>>>>>> 9/2005], section 6.5, table 6-7, an interrupt resets MSR_POW to zero
>>>>>>> but qemu-0.12.4 fails to do so.
>>>>>>> Resetting the bit is necessary in order to bring the processor out of
>>>>>>> power management since otherwise it goes to sleep right away in the
>>>>>>> exception handler, i.e., it is impossible to leave PM-mode.
>>>>>>>
>>>>>> This doesn't look right. POW shouldn't even get stored in SRR1. Could
>>>>>> you please redo the patch and make sure that mtmsr masks out MSR_POW?
>>>>>>
>>>>> Could you point sections of the specification for these requirements ?
>>>>>
>>>>> I think SRR1 can save any bits. Non significant bits can be overriden and
>>>>> are masked out when RFI occurs.
>>>>>
>>>> I'm not saying I'm 100% sure on this, but take a look at the e300
>>>> reference manual
>>>> (http://cache.freescale.com/files/32bit/doc/ref_manual/e300coreRM.pdf)
>>>> section 5.2.3. POW is bit 13 in this notion. I don't see it getting
>>>> saved to SRR1.
>>>>
>>>
>>> The current implementation (commit c3d420e) save all the bits and restore only
>>> the needed ones (excluding POW). So POW is cleared after an interrupt.
>>>
>>> But the subject of this patch wasn't on saved bits. It is to clear POW in the
>>> interrupt context. For an unknown reason, it's not done and it's clearly a bug
>>> to fix.
>>>
>>
>> I completely agree. In fact, the mere fact that a bit can slip through
>> this loop indicates that something goes wrong.
>>
>> As far as SRR1 goes, PowerISA 2.06 draws an even more complicated
>> picture: Page 816 says that SRR1 bits 42:44 indicate the power save exit
>> reason while 46:47 indicate how much state was transferred. Bit 45 would
>> be MSR_POW which is defined as "set to 0".
>>
>> Because that means that we'll have to set MSR_POW in to 0 in SRR1 and
>> the interrupt switching function, we can just set it to 0 on mtmsr. That
>> was my point :).
>
> Hi,
>
> I'm not very familiar with PPC but the way I interpret it, we should clear
> the POW bit in both MSR and SRR1 when the interrupt waking the CPU is
> taken. While the CPU is sleeping, the POW bit should remain set in MSR
> though.
>
> As Alex comments, the submited patch fails to make sure the SRR1 bit
> 13 is cleared when taking the interrupt, but I don't think clearing
> MSR[POW] in mtmsr is the right thing. In practice it probably doesn't
> matter much, possibly a bit annoying if you inspect the sleeping state
> from a debugger or similar.
>
> I think mtmsr should set MSR[POW] and powerpc_excp() should clear
> MSR[POW] prior to copying MSR into SRR1.
>
> Not sure, but thats the way I interpret it...
Agreed.
Alex
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH] target-ppc: clear MSR_POW on interrupt
2010-09-10 16:35 ` Alexander Graf
@ 2010-09-10 16:58 ` Thomas Monjalon
2010-09-10 19:57 ` Edgar E. Iglesias
2010-09-10 23:10 ` Alexander Graf
0 siblings, 2 replies; 17+ messages in thread
From: Thomas Monjalon @ 2010-09-10 16:58 UTC (permalink / raw)
To: qemu-devel; +Cc: Edgar E. Iglesias, Alexander Graf
Alexander Graf wrote:
> Am 10.09.2010 um 18:08 schrieb "Edgar E. Iglesias"
<edgar.iglesias@gmail.com>:
> > On Fri, Sep 10, 2010 at 05:50:27PM +0200, Alexander Graf wrote:
> >> Thomas Monjalon wrote:
> >>> Alexander Graf wrote:
> >>>> Thomas Monjalon wrote:
> >>>>> Alexander Graf wrote:
> >>>>>> Thomas Monjalon wrote:
> >>>>>>> From: till <608107@bugs.launchpad.net>
> >>>>>>>
> >>>>>>> According to FreeScale's 'Programming Environments Manual for
> >>>>>>> 32-bit Implementations of the PowerPC Architecture' [MPCFPE32B,
> >>>>>>> Rev.3, 9/2005], section 6.5, table 6-7, an interrupt resets
> >>>>>>> MSR_POW to zero but qemu-0.12.4 fails to do so.
> >>>>>>> Resetting the bit is necessary in order to bring the processor out
> >>>>>>> of power management since otherwise it goes to sleep right away in
> >>>>>>> the exception handler, i.e., it is impossible to leave PM-mode.
> >>>>>>
> >>>>>> This doesn't look right. POW shouldn't even get stored in SRR1.
> >>>>>> Could you please redo the patch and make sure that mtmsr masks out
> >>>>>> MSR_POW?
> >>>>>
> >>>>> Could you point sections of the specification for these requirements
> >>>>> ?
> >>>>>
> >>>>> I think SRR1 can save any bits. Non significant bits can be overriden
> >>>>> and are masked out when RFI occurs.
> >>>>
> >>>> I'm not saying I'm 100% sure on this, but take a look at the e300
> >>>> reference manual
> >>>> (http://cache.freescale.com/files/32bit/doc/ref_manual/e300coreRM.pdf)
> >>>> section 5.2.3. POW is bit 13 in this notion. I don't see it getting
> >>>> saved to SRR1.
> >>>
> >>> The current implementation (commit c3d420e) save all the bits and
> >>> restore only the needed ones (excluding POW). So POW is cleared after
> >>> an interrupt.
> >>>
> >>> But the subject of this patch wasn't on saved bits. It is to clear POW
> >>> in the interrupt context. For an unknown reason, it's not done and
> >>> it's clearly a bug to fix.
> >>
> >> I completely agree. In fact, the mere fact that a bit can slip through
> >> this loop indicates that something goes wrong.
> >>
> >> As far as SRR1 goes, PowerISA 2.06 draws an even more complicated
> >> picture: Page 816 says that SRR1 bits 42:44 indicate the power save exit
> >> reason while 46:47 indicate how much state was transferred. Bit 45 would
> >> be MSR_POW which is defined as "set to 0".
> >>
> >> Because that means that we'll have to set MSR_POW in to 0 in SRR1 and
> >> the interrupt switching function, we can just set it to 0 on mtmsr. That
> >> was my point :).
> >
> > Hi,
> >
> > I'm not very familiar with PPC but the way I interpret it, we should
> > clear the POW bit in both MSR and SRR1 when the interrupt waking the CPU
> > is taken. While the CPU is sleeping, the POW bit should remain set in
> > MSR though.
> >
> > As Alex comments, the submited patch fails to make sure the SRR1 bit
> > 13 is cleared when taking the interrupt, but I don't think clearing
> > MSR[POW] in mtmsr is the right thing. In practice it probably doesn't
> > matter much, possibly a bit annoying if you inspect the sleeping state
> > from a debugger or similar.
> >
> > I think mtmsr should set MSR[POW] and powerpc_excp() should clear
> > MSR[POW] prior to copying MSR into SRR1.
> >
> > Not sure, but thats the way I interpret it...
>
> Agreed.
>
> Alex
I think we should first ack and commit this patch as it is sure it is needed.
Regarding the SRR1 question, it can be another patch because it is more
general than the POW bit.
In the commit c3d420e which was about MSR restoring in RFI, I have removed the
clearing of bits in SRR1. But I am OK to redo it after check that some
exceptions doesn't rely on the presence of these bits. I mean clearing bits in
SRR1 can be architecture-specific and exception-specific. Each exception has its
own scheme for SRR1 clearing.
--
Thomas
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH] target-ppc: clear MSR_POW on interrupt
2010-09-10 16:58 ` Thomas Monjalon
@ 2010-09-10 19:57 ` Edgar E. Iglesias
2010-09-10 23:10 ` Alexander Graf
1 sibling, 0 replies; 17+ messages in thread
From: Edgar E. Iglesias @ 2010-09-10 19:57 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: qemu-devel, Alexander Graf
On Fri, Sep 10, 2010 at 06:58:10PM +0200, Thomas Monjalon wrote:
> Alexander Graf wrote:
> > Am 10.09.2010 um 18:08 schrieb "Edgar E. Iglesias"
> <edgar.iglesias@gmail.com>:
> > > On Fri, Sep 10, 2010 at 05:50:27PM +0200, Alexander Graf wrote:
> > >> Thomas Monjalon wrote:
> > >>> Alexander Graf wrote:
> > >>>> Thomas Monjalon wrote:
> > >>>>> Alexander Graf wrote:
> > >>>>>> Thomas Monjalon wrote:
> > >>>>>>> From: till <608107@bugs.launchpad.net>
> > >>>>>>>
> > >>>>>>> According to FreeScale's 'Programming Environments Manual for
> > >>>>>>> 32-bit Implementations of the PowerPC Architecture' [MPCFPE32B,
> > >>>>>>> Rev.3, 9/2005], section 6.5, table 6-7, an interrupt resets
> > >>>>>>> MSR_POW to zero but qemu-0.12.4 fails to do so.
> > >>>>>>> Resetting the bit is necessary in order to bring the processor out
> > >>>>>>> of power management since otherwise it goes to sleep right away in
> > >>>>>>> the exception handler, i.e., it is impossible to leave PM-mode.
> > >>>>>>
> > >>>>>> This doesn't look right. POW shouldn't even get stored in SRR1.
> > >>>>>> Could you please redo the patch and make sure that mtmsr masks out
> > >>>>>> MSR_POW?
> > >>>>>
> > >>>>> Could you point sections of the specification for these requirements
> > >>>>> ?
> > >>>>>
> > >>>>> I think SRR1 can save any bits. Non significant bits can be overriden
> > >>>>> and are masked out when RFI occurs.
> > >>>>
> > >>>> I'm not saying I'm 100% sure on this, but take a look at the e300
> > >>>> reference manual
> > >>>> (http://cache.freescale.com/files/32bit/doc/ref_manual/e300coreRM.pdf)
> > >>>> section 5.2.3. POW is bit 13 in this notion. I don't see it getting
> > >>>> saved to SRR1.
> > >>>
> > >>> The current implementation (commit c3d420e) save all the bits and
> > >>> restore only the needed ones (excluding POW). So POW is cleared after
> > >>> an interrupt.
> > >>>
> > >>> But the subject of this patch wasn't on saved bits. It is to clear POW
> > >>> in the interrupt context. For an unknown reason, it's not done and
> > >>> it's clearly a bug to fix.
> > >>
> > >> I completely agree. In fact, the mere fact that a bit can slip through
> > >> this loop indicates that something goes wrong.
> > >>
> > >> As far as SRR1 goes, PowerISA 2.06 draws an even more complicated
> > >> picture: Page 816 says that SRR1 bits 42:44 indicate the power save exit
> > >> reason while 46:47 indicate how much state was transferred. Bit 45 would
> > >> be MSR_POW which is defined as "set to 0".
> > >>
> > >> Because that means that we'll have to set MSR_POW in to 0 in SRR1 and
> > >> the interrupt switching function, we can just set it to 0 on mtmsr. That
> > >> was my point :).
> > >
> > > Hi,
> > >
> > > I'm not very familiar with PPC but the way I interpret it, we should
> > > clear the POW bit in both MSR and SRR1 when the interrupt waking the CPU
> > > is taken. While the CPU is sleeping, the POW bit should remain set in
> > > MSR though.
> > >
> > > As Alex comments, the submited patch fails to make sure the SRR1 bit
> > > 13 is cleared when taking the interrupt, but I don't think clearing
> > > MSR[POW] in mtmsr is the right thing. In practice it probably doesn't
> > > matter much, possibly a bit annoying if you inspect the sleeping state
> > > from a debugger or similar.
> > >
> > > I think mtmsr should set MSR[POW] and powerpc_excp() should clear
> > > MSR[POW] prior to copying MSR into SRR1.
> > >
> > > Not sure, but thats the way I interpret it...
> >
> > Agreed.
> >
> > Alex
>
> I think we should first ack and commit this patch as it is sure it is needed.
>
> Regarding the SRR1 question, it can be another patch because it is more
> general than the POW bit.
>
> In the commit c3d420e which was about MSR restoring in RFI, I have removed the
> clearing of bits in SRR1. But I am OK to redo it after check that some
> exceptions doesn't rely on the presence of these bits. I mean clearing bits in
> SRR1 can be architecture-specific and exception-specific. Each exception has its
> own scheme for SRR1 clearing.
But the MSR[POW] should be cleared prior to beeing copied to
SRR1. I dont see the point in submitting a followup patch
that moves the clearing a couple of lines up the c-file.
What about the following?
commit 7022e89d74a4299dbb431c89a684a428119242ab
Author: Edgar E. Iglesias <edgar.iglesias@gmail.com>
Date: Fri Sep 10 21:36:09 2010 +0200
powerpc: Clear MSR[POW] when taking exceptions
When taking exceptions, clear the POW or WE bit to inditcate
that the CPU is awake. Do it prior to saving the MSR into SRR1.
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
diff --git a/target-ppc/helper.c b/target-ppc/helper.c
index d342b09..c25c610 100644
--- a/target-ppc/helper.c
+++ b/target-ppc/helper.c
@@ -2075,6 +2075,8 @@ static inline void powerpc_excp(CPUState *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);
msr = env->msr;
+ /* Taking an exception wakes up the core, clear the POW/WE bit. */
+ msr &= ~((target_ulong)1 << MSR_POW);
new_msr = msr;
srr0 = SPR_SRR0;
srr1 = SPR_SRR1;
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH] target-ppc: clear MSR_POW on interrupt
2010-09-10 16:58 ` Thomas Monjalon
2010-09-10 19:57 ` Edgar E. Iglesias
@ 2010-09-10 23:10 ` Alexander Graf
1 sibling, 0 replies; 17+ messages in thread
From: Alexander Graf @ 2010-09-10 23:10 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: Edgar E. Iglesias, qemu-devel
On 10.09.2010, at 18:58, Thomas Monjalon wrote:
> Alexander Graf wrote:
>> Am 10.09.2010 um 18:08 schrieb "Edgar E. Iglesias"
> <edgar.iglesias@gmail.com>:
>>> On Fri, Sep 10, 2010 at 05:50:27PM +0200, Alexander Graf wrote:
>>>> Thomas Monjalon wrote:
>>>>> Alexander Graf wrote:
>>>>>> Thomas Monjalon wrote:
>>>>>>> Alexander Graf wrote:
>>>>>>>> Thomas Monjalon wrote:
>>>>>>>>> From: till <608107@bugs.launchpad.net>
>>>>>>>>>
>>>>>>>>> According to FreeScale's 'Programming Environments Manual for
>>>>>>>>> 32-bit Implementations of the PowerPC Architecture' [MPCFPE32B,
>>>>>>>>> Rev.3, 9/2005], section 6.5, table 6-7, an interrupt resets
>>>>>>>>> MSR_POW to zero but qemu-0.12.4 fails to do so.
>>>>>>>>> Resetting the bit is necessary in order to bring the processor out
>>>>>>>>> of power management since otherwise it goes to sleep right away in
>>>>>>>>> the exception handler, i.e., it is impossible to leave PM-mode.
>>>>>>>>
>>>>>>>> This doesn't look right. POW shouldn't even get stored in SRR1.
>>>>>>>> Could you please redo the patch and make sure that mtmsr masks out
>>>>>>>> MSR_POW?
>>>>>>>
>>>>>>> Could you point sections of the specification for these requirements
>>>>>>> ?
>>>>>>>
>>>>>>> I think SRR1 can save any bits. Non significant bits can be overriden
>>>>>>> and are masked out when RFI occurs.
>>>>>>
>>>>>> I'm not saying I'm 100% sure on this, but take a look at the e300
>>>>>> reference manual
>>>>>> (http://cache.freescale.com/files/32bit/doc/ref_manual/e300coreRM.pdf)
>>>>>> section 5.2.3. POW is bit 13 in this notion. I don't see it getting
>>>>>> saved to SRR1.
>>>>>
>>>>> The current implementation (commit c3d420e) save all the bits and
>>>>> restore only the needed ones (excluding POW). So POW is cleared after
>>>>> an interrupt.
>>>>>
>>>>> But the subject of this patch wasn't on saved bits. It is to clear POW
>>>>> in the interrupt context. For an unknown reason, it's not done and
>>>>> it's clearly a bug to fix.
>>>>
>>>> I completely agree. In fact, the mere fact that a bit can slip through
>>>> this loop indicates that something goes wrong.
>>>>
>>>> As far as SRR1 goes, PowerISA 2.06 draws an even more complicated
>>>> picture: Page 816 says that SRR1 bits 42:44 indicate the power save exit
>>>> reason while 46:47 indicate how much state was transferred. Bit 45 would
>>>> be MSR_POW which is defined as "set to 0".
>>>>
>>>> Because that means that we'll have to set MSR_POW in to 0 in SRR1 and
>>>> the interrupt switching function, we can just set it to 0 on mtmsr. That
>>>> was my point :).
>>>
>>> Hi,
>>>
>>> I'm not very familiar with PPC but the way I interpret it, we should
>>> clear the POW bit in both MSR and SRR1 when the interrupt waking the CPU
>>> is taken. While the CPU is sleeping, the POW bit should remain set in
>>> MSR though.
>>>
>>> As Alex comments, the submited patch fails to make sure the SRR1 bit
>>> 13 is cleared when taking the interrupt, but I don't think clearing
>>> MSR[POW] in mtmsr is the right thing. In practice it probably doesn't
>>> matter much, possibly a bit annoying if you inspect the sleeping state
>>> from a debugger or similar.
>>>
>>> I think mtmsr should set MSR[POW] and powerpc_excp() should clear
>>> MSR[POW] prior to copying MSR into SRR1.
>>>
>>> Not sure, but thats the way I interpret it...
>>
>> Agreed.
>>
>> Alex
>
> I think we should first ack and commit this patch as it is sure it is needed.
>
> Regarding the SRR1 question, it can be another patch because it is more
> general than the POW bit.
> In the commit c3d420e which was about MSR restoring in RFI, I have removed the
> clearing of bits in SRR1. But I am OK to redo it after check that some
> exceptions doesn't rely on the presence of these bits. I mean clearing bits in
> SRR1 can be architecture-specific and exception-specific. Each exception has its
> own scheme for SRR1 clearing.
I'm really torn on this one. While you're right, I'd really like to see both issues fixed. To be honest, I don't think the whole function as is makes too much sense. Why would the MSR of an exception handler be based off the MSR we had before?
Alex
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [Bug 608107] Re: ppc fails to clear MSR_POW when incurring exception
2010-07-21 8:53 [Qemu-devel] [Bug 608107] [NEW] ppc fails to clear MSR_POW when incurring exception till
2010-07-21 8:53 ` [Qemu-devel] [Bug 608107] " till
2010-09-10 9:22 ` [Qemu-devel] [Bug 608107] Re: ppc fails to clear MSR_POW when incurring exception Thomas Monjalon
@ 2010-09-13 13:02 ` till
2012-08-03 2:56 ` Samuel Bronson
2016-10-31 9:11 ` Thomas Huth
4 siblings, 0 replies; 17+ messages in thread
From: till @ 2010-09-13 13:02 UTC (permalink / raw)
To: qemu-devel
I'm afraid I don't understand. My the problem and fix doesn't address mtmsr at all.
It just makes sure MSR_POW is cleared in MSR when an exception occurs.
Do you mean MSR_POW should masked from MSR before saving it to SRR1?
That's already taken care of (target-ppc/helper.c:2074 [qemu-0.12.4]).
--
ppc fails to clear MSR_POW when incurring exception
https://bugs.launchpad.net/bugs/608107
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
Status in QEMU: Confirmed
Bug description:
QEMU VERSION: 0.12.4
According to FreeScale's 'Programming Environments Manual for 32-bit Implementations of the PowerPC Architecture' [MPCFPE32B, Rev.3, 9/2005], section 6.5, table 6-7, an interrupt resets MSR_POW to zero but qemu-0.12.4 fails to do so.
Resetting the bit is necessary in order to bring the processor out of power-management since otherwise it goes to sleep right away in the exception handler, i.e., it is impossible to leave PM-mode.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [Bug 608107] Re: ppc fails to clear MSR_POW when incurring exception
2010-07-21 8:53 [Qemu-devel] [Bug 608107] [NEW] ppc fails to clear MSR_POW when incurring exception till
` (2 preceding siblings ...)
2010-09-13 13:02 ` till
@ 2012-08-03 2:56 ` Samuel Bronson
2016-10-31 9:11 ` Thomas Huth
4 siblings, 0 replies; 17+ messages in thread
From: Samuel Bronson @ 2012-08-03 2:56 UTC (permalink / raw)
To: qemu-devel
** Tags added: ppc
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/608107
Title:
ppc fails to clear MSR_POW when incurring exception
Status in QEMU:
Confirmed
Bug description:
QEMU VERSION: 0.12.4
According to FreeScale's 'Programming Environments Manual for 32-bit Implementations of the PowerPC Architecture' [MPCFPE32B, Rev.3, 9/2005], section 6.5, table 6-7, an interrupt resets MSR_POW to zero but qemu-0.12.4 fails to do so.
Resetting the bit is necessary in order to bring the processor out of power-management since otherwise it goes to sleep right away in the exception handler, i.e., it is impossible to leave PM-mode.
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/608107/+subscriptions
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [Bug 608107] Re: ppc fails to clear MSR_POW when incurring exception
2010-07-21 8:53 [Qemu-devel] [Bug 608107] [NEW] ppc fails to clear MSR_POW when incurring exception till
` (3 preceding siblings ...)
2012-08-03 2:56 ` Samuel Bronson
@ 2016-10-31 9:11 ` Thomas Huth
4 siblings, 0 replies; 17+ messages in thread
From: Thomas Huth @ 2016-10-31 9:11 UTC (permalink / raw)
To: qemu-devel
As far as I can see, this problem has been fixed by this commit here:
http://git.qemu.org/?p=qemu.git;a=commitdiff;h=41557447d30eeb944e4
... so I'm setting the status to "Fix released" now.
** Changed in: qemu
Status: Confirmed => Fix Released
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/608107
Title:
ppc fails to clear MSR_POW when incurring exception
Status in QEMU:
Fix Released
Bug description:
QEMU VERSION: 0.12.4
According to FreeScale's 'Programming Environments Manual for 32-bit Implementations of the PowerPC Architecture' [MPCFPE32B, Rev.3, 9/2005], section 6.5, table 6-7, an interrupt resets MSR_POW to zero but qemu-0.12.4 fails to do so.
Resetting the bit is necessary in order to bring the processor out of power-management since otherwise it goes to sleep right away in the exception handler, i.e., it is impossible to leave PM-mode.
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/608107/+subscriptions
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2016-10-31 9:29 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-07-21 8:53 [Qemu-devel] [Bug 608107] [NEW] ppc fails to clear MSR_POW when incurring exception till
2010-07-21 8:53 ` [Qemu-devel] [Bug 608107] " till
2010-09-10 12:32 ` [Qemu-devel] [PATCH] target-ppc: clear MSR_POW on interrupt Thomas Monjalon
2010-09-10 13:02 ` Alexander Graf
2010-09-10 15:03 ` Thomas Monjalon
2010-09-10 15:26 ` Alexander Graf
2010-09-10 15:46 ` Thomas Monjalon
2010-09-10 15:50 ` Alexander Graf
2010-09-10 16:08 ` Edgar E. Iglesias
2010-09-10 16:35 ` Alexander Graf
2010-09-10 16:58 ` Thomas Monjalon
2010-09-10 19:57 ` Edgar E. Iglesias
2010-09-10 23:10 ` Alexander Graf
2010-09-10 9:22 ` [Qemu-devel] [Bug 608107] Re: ppc fails to clear MSR_POW when incurring exception Thomas Monjalon
2010-09-13 13:02 ` till
2012-08-03 2:56 ` Samuel Bronson
2016-10-31 9:11 ` Thomas Huth
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.