| Message ID | 1275073245-20352-1-git-send-email-thomas_ml@monjalon.net |
|---|---|
| State | New |
| Headers | show |
Am 28.05.2010 um 21:00 schrieb Thomas Monjalon: > From: Thomas Monjalon <thomas@monjalon.net> > > This line was a bit clear. > The next lines set or reset this bit (LE) depending of another bit > (ILE). > So the first line is useless. > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net> > --- > target-ppc/helper.c | 1 - > 1 files changed, 0 insertions(+), 1 deletions(-) > > diff --git a/target-ppc/helper.c b/target-ppc/helper.c > index 3d843b5..dabf1fd 100644 > --- a/target-ppc/helper.c > +++ b/target-ppc/helper.c > @@ -2591,7 +2591,6 @@ static inline void powerpc_excp(CPUState *env, > int excp_model, int excp) > #if 0 /* Fix this: not on all targets */ > new_msr &= ~((target_ulong)1 << MSR_PMM); > #endif > - new_msr &= ~((target_ulong)1 << MSR_LE); > if (msr_ile) > new_msr |= (target_ulong)1 << MSR_LE; > else Following line is new_msr &= ~((target_ulong)1 << MSR_LE); so this should be fine. Reviewed-by: Andreas Faerber <andreas.faerber@web.de> Andreas
On 29.05.2010, at 15:31, Andreas Färber wrote: > Am 28.05.2010 um 21:00 schrieb Thomas Monjalon: > >> From: Thomas Monjalon <thomas@monjalon.net> >> >> This line was a bit clear. >> The next lines set or reset this bit (LE) depending of another bit (ILE). >> So the first line is useless. >> >> Signed-off-by: Thomas Monjalon <thomas@monjalon.net> >> --- >> target-ppc/helper.c | 1 - >> 1 files changed, 0 insertions(+), 1 deletions(-) >> >> diff --git a/target-ppc/helper.c b/target-ppc/helper.c >> index 3d843b5..dabf1fd 100644 >> --- a/target-ppc/helper.c >> +++ b/target-ppc/helper.c >> @@ -2591,7 +2591,6 @@ static inline void powerpc_excp(CPUState *env, int excp_model, int excp) >> #if 0 /* Fix this: not on all targets */ >> new_msr &= ~((target_ulong)1 << MSR_PMM); >> #endif >> - new_msr &= ~((target_ulong)1 << MSR_LE); >> if (msr_ile) >> new_msr |= (target_ulong)1 << MSR_LE; >> else > > Following line is > new_msr &= ~((target_ulong)1 << MSR_LE); > so this should be fine. > > Reviewed-by: Andreas Faerber <andreas.faerber@web.de> Thomas, have you verified that it still boots a ppc64 guest? Alex
Alexander Graf wrote:
> Thomas, have you verified that it still boots a ppc64 guest?
No, but I don't see any specific link between ppc64 and this change.
This is the current (pseudo)code:
LE = 0
if ILE == 1
LE = 1
else
LE = 0
In all cases, LE will be overwritten after the first line.
I cannot see any justification to keep the first line.
Thomas Monjalon wrote: > Alexander Graf wrote: > >> Thomas, have you verified that it still boots a ppc64 guest? >> > > No, but I don't see any specific link between ppc64 and this change. > > This is the current (pseudo)code: > LE = 0 > if ILE == 1 > LE = 1 > else > LE = 0 > In all cases, LE will be overwritten after the first line. > > I cannot see any justification to keep the first line. > I agree. Alex
On Fri, May 28, 2010 at 09:00:45PM +0200, Thomas Monjalon wrote: > From: Thomas Monjalon <thomas@monjalon.net> > > This line was a bit clear. > The next lines set or reset this bit (LE) depending of another bit (ILE). > So the first line is useless. Thanks, applied. > Signed-off-by: Thomas Monjalon <thomas@monjalon.net> > --- > target-ppc/helper.c | 1 - > 1 files changed, 0 insertions(+), 1 deletions(-) > > diff --git a/target-ppc/helper.c b/target-ppc/helper.c > index 3d843b5..dabf1fd 100644 > --- a/target-ppc/helper.c > +++ b/target-ppc/helper.c > @@ -2591,7 +2591,6 @@ static inline void powerpc_excp(CPUState *env, int excp_model, int excp) > #if 0 /* Fix this: not on all targets */ > new_msr &= ~((target_ulong)1 << MSR_PMM); > #endif > - new_msr &= ~((target_ulong)1 << MSR_LE); > if (msr_ile) > new_msr |= (target_ulong)1 << MSR_LE; > else > -- > 1.7.1 > > > > >
diff --git a/target-ppc/helper.c b/target-ppc/helper.c index 3d843b5..dabf1fd 100644 --- a/target-ppc/helper.c +++ b/target-ppc/helper.c @@ -2591,7 +2591,6 @@ static inline void powerpc_excp(CPUState *env, int excp_model, int excp) #if 0 /* Fix this: not on all targets */ new_msr &= ~((target_ulong)1 << MSR_PMM); #endif - new_msr &= ~((target_ulong)1 << MSR_LE); if (msr_ile) new_msr |= (target_ulong)1 << MSR_LE; else