Patchwork target-ppc: remove useless line

login
register
mail settings
Submitter Thomas Monjalon
Date May 28, 2010, 7 p.m.
Message ID <1275073245-20352-1-git-send-email-thomas_ml@monjalon.net>
Download mbox | patch
Permalink /patch/53955/
State New
Headers show

Comments

Thomas Monjalon - May 28, 2010, 7 p.m.
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(-)
Andreas Färber - May 29, 2010, 1:31 p.m.
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
Alexander Graf - May 31, 2010, 9:49 a.m.
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
Thomas Monjalon - May 31, 2010, 1:11 p.m.
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.
Alexander Graf - May 31, 2010, 2:59 p.m.
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
Aurelien Jarno - May 31, 2010, 6:45 p.m.
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
> 
> 
> 
> 
>

Patch

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