[Qemu-devel] [PATCH 1/2 for-2.6] target-sparc: fix Nucleus quad LDD 128 bit access for windowed registers

[Qemu-devel] [PATCH 1/2 for-2.6] target-sparc: fix Nucleus quad LDD 128 bit access for windowed registers

[Artyom Tarasenko]

Fix register offset calculation when regwptr is used.

Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
---
 target-sparc/ldst_helper.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/target-sparc/ldst_helper.c b/target-sparc/ldst_helper.c
index 2b0221c..a383074 100644
--- a/target-sparc/ldst_helper.c
+++ b/target-sparc/ldst_helper.c
@@ -2059,11 +2059,11 @@ void helper_ldda_asi(CPUSPARCState *env, target_ulong addr, int asi, int rd)
                 bswap64s(&env->gregs[rd + 1]);
             }
         } else {
-            env->regwptr[rd] = cpu_ldq_nucleus(env, addr);
-            env->regwptr[rd + 1] = cpu_ldq_nucleus(env, addr + 8);
+            env->regwptr[rd - 8] = cpu_ldq_nucleus(env, addr);
+            env->regwptr[rd + 1 - 8] = cpu_ldq_nucleus(env, addr + 8);
             if (asi == 0x2c) {
-                bswap64s(&env->regwptr[rd]);
-                bswap64s(&env->regwptr[rd + 1]);
+                bswap64s(&env->regwptr[rd - 8]);
+                bswap64s(&env->regwptr[rd + 1 - 8]);
             }
         }
         break;
@@ -2076,8 +2076,8 @@ void helper_ldda_asi(CPUSPARCState *env, target_ulong addr, int asi, int rd)
             env->gregs[rd] = helper_ld_asi(env, addr, asi, 4, 0);
             env->gregs[rd + 1] = helper_ld_asi(env, addr + 4, asi, 4, 0);
         } else {
-            env->regwptr[rd] = helper_ld_asi(env, addr, asi, 4, 0);
-            env->regwptr[rd + 1] = helper_ld_asi(env, addr + 4, asi, 4, 0);
+            env->regwptr[rd - 8] = helper_ld_asi(env, addr, asi, 4, 0);
+            env->regwptr[rd + 1 - 8] = helper_ld_asi(env, addr + 4, asi, 4, 0);
         }
         break;
     }
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/2 for-2.6] target-sparc: fix Trap Based Address Register behavior for sparc64

[Artyom Tarasenko]

Accoding the chapter 7.6 Trap Processing of the SPARC Architecture Manual v9,
the Trap Based Address Register is not modified as a trap is taken.

This fix allows booting FreeBSD-10.3-RELEASE-sparc64.

Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
---
 target-sparc/int64_helper.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/target-sparc/int64_helper.c b/target-sparc/int64_helper.c
index aa876cd..29360fa 100644
--- a/target-sparc/int64_helper.c
+++ b/target-sparc/int64_helper.c
@@ -158,9 +158,8 @@ void sparc_cpu_do_interrupt(CPUState *cs)
     } else if ((intno & 0x1c0) == TT_FILL) {
         cpu_set_cwp(env, cpu_cwp_inc(env, env->cwp + 1));
     }
-    env->tbr &= ~0x7fffULL;
-    env->tbr |= ((env->tl > 1) ? 1 << 14 : 0) | (intno << 5);
-    env->pc = env->tbr;
+    env->pc = env->tbr  & ~0x7fffULL;
+    env->pc |= ((env->tl > 1) ? 1 << 14 : 0) | (intno << 5);
     env->npc = env->pc + 4;
     cs->exception_index = -1;
 }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 2/2 for-2.6] target-sparc: fix Trap Based Address Register behavior for sparc64

[Mark Cave-Ayland]

On 14/04/16 10:29, Artyom Tarasenko wrote:

> Accoding the chapter 7.6 Trap Processing of the SPARC Architecture Manual v9,
> the Trap Based Address Register is not modified as a trap is taken.
> 
> This fix allows booting FreeBSD-10.3-RELEASE-sparc64.
> 
> Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
> ---
>  target-sparc/int64_helper.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/target-sparc/int64_helper.c b/target-sparc/int64_helper.c
> index aa876cd..29360fa 100644
> --- a/target-sparc/int64_helper.c
> +++ b/target-sparc/int64_helper.c
> @@ -158,9 +158,8 @@ void sparc_cpu_do_interrupt(CPUState *cs)
>      } else if ((intno & 0x1c0) == TT_FILL) {
>          cpu_set_cwp(env, cpu_cwp_inc(env, env->cwp + 1));
>      }
> -    env->tbr &= ~0x7fffULL;
> -    env->tbr |= ((env->tl > 1) ? 1 << 14 : 0) | (intno << 5);
> -    env->pc = env->tbr;
> +    env->pc = env->tbr  & ~0x7fffULL;
> +    env->pc |= ((env->tl > 1) ? 1 << 14 : 0) | (intno << 5);
>      env->npc = env->pc + 4;
>      cs->exception_index = -1;
>  }
> 

Gosh yes, this is entirely obvious when you consider that taking a trap
shouldn't actually alter the register containing the address of the trap
table in memory :(

Again the patch applies with fuzz and checkpatch.pl complains about
line-endings but if you can rebase/resend then you can add my
Reviewed-by. Given that such a trivial fix unlocks FreeBSD then I'm okay
with this in 2.6 too.


ATB,

Mark.

Re: [Qemu-devel] [PATCH 2/2 for-2.6] target-sparc: fix Trap Based Address Register behavior for sparc64

[Artyom Tarasenko]

On Thu, Apr 14, 2016 at 3:43 PM, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> On 14/04/16 10:29, Artyom Tarasenko wrote:
>
>> Accoding the chapter 7.6 Trap Processing of the SPARC Architecture Manual v9,
>> the Trap Based Address Register is not modified as a trap is taken.
>>
>> This fix allows booting FreeBSD-10.3-RELEASE-sparc64.
>>
>> Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
>> ---
>>  target-sparc/int64_helper.c | 5 ++---
>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/target-sparc/int64_helper.c b/target-sparc/int64_helper.c
>> index aa876cd..29360fa 100644
>> --- a/target-sparc/int64_helper.c
>> +++ b/target-sparc/int64_helper.c
>> @@ -158,9 +158,8 @@ void sparc_cpu_do_interrupt(CPUState *cs)
>>      } else if ((intno & 0x1c0) == TT_FILL) {
>>          cpu_set_cwp(env, cpu_cwp_inc(env, env->cwp + 1));
>>      }
>> -    env->tbr &= ~0x7fffULL;
>> -    env->tbr |= ((env->tl > 1) ? 1 << 14 : 0) | (intno << 5);
>> -    env->pc = env->tbr;
>> +    env->pc = env->tbr  & ~0x7fffULL;
>> +    env->pc |= ((env->tl > 1) ? 1 << 14 : 0) | (intno << 5);
>>      env->npc = env->pc + 4;
>>      cs->exception_index = -1;
>>  }
>>
>
> Gosh yes, this is entirely obvious when you consider that taking a trap
> shouldn't actually alter the register containing the address of the trap
> table in memory :(

Well, it does alter it in SPARCv8, I guess that's where the bug is coming from.

>
> Again the patch applies with fuzz and checkpatch.pl complains about
> line-endings but if you can rebase/resend then you can add my

This is strange. I've rebased it today on
8b4aaba736e55c8ab6d71350f850a6642f0165b9,
and

$  scripts/checkpatch.pl
0002-target-sparc-fix-Trap-Based-Address-Register-behavio.patch
total: 0 errors, 0 warnings, 11 lines checked
0002-target-sparc-fix-Trap-Based-Address-Register-behavio.patch has no
obvious style problems and is ready for Submission.

Artyom

> Reviewed-by. Given that such a trivial fix unlocks FreeBSD then I'm okay
> with this in 2.6 too.
>
>
> ATB,
>
> Mark.
>



-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PATCH 1/2 for-2.6] target-sparc: fix Nucleus quad LDD 128 bit access for windowed registers

[Mark Cave-Ayland]

On 14/04/16 10:29, Artyom Tarasenko wrote:

> Fix register offset calculation when regwptr is used.
> 
> Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
> ---
>  target-sparc/ldst_helper.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/target-sparc/ldst_helper.c b/target-sparc/ldst_helper.c
> index 2b0221c..a383074 100644
> --- a/target-sparc/ldst_helper.c
> +++ b/target-sparc/ldst_helper.c
> @@ -2059,11 +2059,11 @@ void helper_ldda_asi(CPUSPARCState *env, target_ulong addr, int asi, int rd)
>                  bswap64s(&env->gregs[rd + 1]);
>              }
>          } else {
> -            env->regwptr[rd] = cpu_ldq_nucleus(env, addr);
> -            env->regwptr[rd + 1] = cpu_ldq_nucleus(env, addr + 8);
> +            env->regwptr[rd - 8] = cpu_ldq_nucleus(env, addr);
> +            env->regwptr[rd + 1 - 8] = cpu_ldq_nucleus(env, addr + 8);
>              if (asi == 0x2c) {
> -                bswap64s(&env->regwptr[rd]);
> -                bswap64s(&env->regwptr[rd + 1]);
> +                bswap64s(&env->regwptr[rd - 8]);
> +                bswap64s(&env->regwptr[rd + 1 - 8]);
>              }
>          }
>          break;
> @@ -2076,8 +2076,8 @@ void helper_ldda_asi(CPUSPARCState *env, target_ulong addr, int asi, int rd)
>              env->gregs[rd] = helper_ld_asi(env, addr, asi, 4, 0);
>              env->gregs[rd + 1] = helper_ld_asi(env, addr + 4, asi, 4, 0);
>          } else {
> -            env->regwptr[rd] = helper_ld_asi(env, addr, asi, 4, 0);
> -            env->regwptr[rd + 1] = helper_ld_asi(env, addr + 4, asi, 4, 0);
> +            env->regwptr[rd - 8] = helper_ld_asi(env, addr, asi, 4, 0);
> +            env->regwptr[rd + 1 - 8] = helper_ld_asi(env, addr + 4, asi, 4, 0);
>          }
>          break;
>      }
> 

So it seems that without this patch any ldda on a non-global register is
placing the result into the wrong register which is fairly nasty.

The patch does apply, but only with fuzz, and checkpatch.pl reports
several warnings with spacing/line endings. If you can rebase to master
and resend then you can add my Reviewed-by and I'm okay with this for
2.6 - register corruption is not good.


ATB,

Mark.

Re: [Qemu-devel] [PATCH 1/2 for-2.6] target-sparc: fix Nucleus quad LDD 128 bit access for windowed registers

[Artyom Tarasenko]

On Thu, Apr 14, 2016 at 3:42 PM, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> On 14/04/16 10:29, Artyom Tarasenko wrote:
>
>> Fix register offset calculation when regwptr is used.
>>
>> Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
>> ---
>>  target-sparc/ldst_helper.c | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/target-sparc/ldst_helper.c b/target-sparc/ldst_helper.c
>> index 2b0221c..a383074 100644
>> --- a/target-sparc/ldst_helper.c
>> +++ b/target-sparc/ldst_helper.c
>> @@ -2059,11 +2059,11 @@ void helper_ldda_asi(CPUSPARCState *env, target_ulong addr, int asi, int rd)
>>                  bswap64s(&env->gregs[rd + 1]);
>>              }
>>          } else {
>> -            env->regwptr[rd] = cpu_ldq_nucleus(env, addr);
>> -            env->regwptr[rd + 1] = cpu_ldq_nucleus(env, addr + 8);
>> +            env->regwptr[rd - 8] = cpu_ldq_nucleus(env, addr);
>> +            env->regwptr[rd + 1 - 8] = cpu_ldq_nucleus(env, addr + 8);
>>              if (asi == 0x2c) {
>> -                bswap64s(&env->regwptr[rd]);
>> -                bswap64s(&env->regwptr[rd + 1]);
>> +                bswap64s(&env->regwptr[rd - 8]);
>> +                bswap64s(&env->regwptr[rd + 1 - 8]);
>>              }
>>          }
>>          break;
>> @@ -2076,8 +2076,8 @@ void helper_ldda_asi(CPUSPARCState *env, target_ulong addr, int asi, int rd)
>>              env->gregs[rd] = helper_ld_asi(env, addr, asi, 4, 0);
>>              env->gregs[rd + 1] = helper_ld_asi(env, addr + 4, asi, 4, 0);
>>          } else {
>> -            env->regwptr[rd] = helper_ld_asi(env, addr, asi, 4, 0);
>> -            env->regwptr[rd + 1] = helper_ld_asi(env, addr + 4, asi, 4, 0);
>> +            env->regwptr[rd - 8] = helper_ld_asi(env, addr, asi, 4, 0);
>> +            env->regwptr[rd + 1 - 8] = helper_ld_asi(env, addr + 4, asi, 4, 0);
>>          }
>>          break;
>>      }
>>
>
> So it seems that without this patch any ldda on a non-global register is
> placing the result into the wrong register which is fairly nasty.
>
> The patch does apply, but only with fuzz, and checkpatch.pl reports
> several warnings with spacing/line endings. If you can rebase to master
> and resend then you can add my Reviewed-by and I'm okay with this for
> 2.6 - register corruption is not good.

Can you please show  the output of your checkpatch.pl report? I get:

 $  scripts/checkpatch.pl
0001-target-sparc-fix-Nucleus-quad-LDD-128-bit-access-for.patch
total: 0 errors, 0 warnings, 25 lines checked
0001-target-sparc-fix-Nucleus-quad-LDD-128-bit-access-for.patch has no
obvious style problems and is ready for Submission.

-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PATCH 1/2 for-2.6] target-sparc: fix Nucleus quad LDD 128 bit access for windowed registers

[Mark Cave-Ayland]

On 14/04/16 15:04, Artyom Tarasenko wrote:

> Can you please show  the output of your checkpatch.pl report? I get:
> 
>  $  scripts/checkpatch.pl
> 0001-target-sparc-fix-Nucleus-quad-LDD-128-bit-access-for.patch
> total: 0 errors, 0 warnings, 25 lines checked
> 0001-target-sparc-fix-Nucleus-quad-LDD-128-bit-access-for.patch has no
> obvious style problems and is ready for Submission.

Okay I've just done an update pull and complete rebuild which fixes the
fuzz, but I'm still seeing line ending errors in checkpatch.pl:


$ ./scripts/checkpatch.pl /tmp/artyom/\[PATCH\ 1_2\ for-2.6\]\
target-sparc\:\ fix\ Nucleus\ quad\ LDD\ 128\ bit\ access\ for\
windowed\ registers.eml
ERROR: DOS line endings
#75: FILE: target-sparc/ldst_helper.c:2062:
+            env->regwptr[rd - 8] = cpu_ldq_nucleus(env, addr);^M$

ERROR: DOS line endings
#76: FILE: target-sparc/ldst_helper.c:2063:
+            env->regwptr[rd + 1 - 8] = cpu_ldq_nucleus(env, addr + 8);^M$

ERROR: DOS line endings
#80: FILE: target-sparc/ldst_helper.c:2065:
+                bswap64s(&env->regwptr[rd - 8]);^M$

ERROR: DOS line endings
#81: FILE: target-sparc/ldst_helper.c:2066:
+                bswap64s(&env->regwptr[rd + 1 - 8]);^M$

ERROR: DOS line endings
#91: FILE: target-sparc/ldst_helper.c:2079:
+            env->regwptr[rd - 8] = helper_ld_asi(env, addr, asi, 4, 0);^M$

ERROR: DOS line endings
#92: FILE: target-sparc/ldst_helper.c:2080:
+            env->regwptr[rd + 1 - 8] = helper_ld_asi(env, addr + 4,
asi, 4, 0);^M$

total: 6 errors, 0 warnings, 25 lines checked

/tmp/artyom/[PATCH 1_2 for-2.6] target-sparc: fix Nucleus quad LDD 128
bit access for windowed registers.eml has style problems, please review.
 If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

and:

$ ./scripts/checkpatch.pl /tmp/artyom/\[PATCH\ 2_2\ for-2.6\]\
target-sparc\:\ fix\ Trap\ Based\ Address\ Register\ behavior\ for\
sparc64.eml
ERROR: DOS line endings
#85: FILE: target-sparc/int64_helper.c:161:
+    env->pc = env->tbr  & ~0x7fffULL;^M$

ERROR: DOS line endings
#86: FILE: target-sparc/int64_helper.c:162:
+    env->pc |= ((env->tl > 1) ? 1 << 14 : 0) | (intno << 5);^M$

total: 2 errors, 0 warnings, 11 lines checked

/tmp/artyom/[PATCH 2_2 for-2.6] target-sparc: fix Trap Based Address
Register behavior for sparc64.eml has style problems, please review.  If
any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.


Possibly the DOS line endings are being added by a mail program?


ATB,

Mark.

Re: [Qemu-devel] [PATCH 1/2 for-2.6] target-sparc: fix Nucleus quad LDD 128 bit access for windowed registers

[Artyom Tarasenko]

On Thu, Apr 14, 2016 at 4:20 PM, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> On 14/04/16 15:04, Artyom Tarasenko wrote:
>
>> Can you please show  the output of your checkpatch.pl report? I get:
>>
>>  $  scripts/checkpatch.pl
>> 0001-target-sparc-fix-Nucleus-quad-LDD-128-bit-access-for.patch
>> total: 0 errors, 0 warnings, 25 lines checked
>> 0001-target-sparc-fix-Nucleus-quad-LDD-128-bit-access-for.patch has no
>> obvious style problems and is ready for Submission.
>
> Okay I've just done an update pull and complete rebuild which fixes the
> fuzz, but I'm still seeing line ending errors in checkpatch.pl:
>
>
> $ ./scripts/checkpatch.pl /tmp/artyom/\[PATCH\ 1_2\ for-2.6\]\
> target-sparc\:\ fix\ Nucleus\ quad\ LDD\ 128\ bit\ access\ for\
> windowed\ registers.eml
> ERROR: DOS line endings
> #75: FILE: target-sparc/ldst_helper.c:2062:
> +            env->regwptr[rd - 8] = cpu_ldq_nucleus(env, addr);^M$
>
> ERROR: DOS line endings
> #76: FILE: target-sparc/ldst_helper.c:2063:
> +            env->regwptr[rd + 1 - 8] = cpu_ldq_nucleus(env, addr + 8);^M$
>
> ERROR: DOS line endings
> #80: FILE: target-sparc/ldst_helper.c:2065:
> +                bswap64s(&env->regwptr[rd - 8]);^M$
>
> ERROR: DOS line endings
> #81: FILE: target-sparc/ldst_helper.c:2066:
> +                bswap64s(&env->regwptr[rd + 1 - 8]);^M$
>
> ERROR: DOS line endings
> #91: FILE: target-sparc/ldst_helper.c:2079:
> +            env->regwptr[rd - 8] = helper_ld_asi(env, addr, asi, 4, 0);^M$
>
> ERROR: DOS line endings
> #92: FILE: target-sparc/ldst_helper.c:2080:
> +            env->regwptr[rd + 1 - 8] = helper_ld_asi(env, addr + 4,
> asi, 4, 0);^M$
>
> total: 6 errors, 0 warnings, 25 lines checked
>
> /tmp/artyom/[PATCH 1_2 for-2.6] target-sparc: fix Nucleus quad LDD 128
> bit access for windowed registers.eml has style problems, please review.
>  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
>
> and:
>
> $ ./scripts/checkpatch.pl /tmp/artyom/\[PATCH\ 2_2\ for-2.6\]\
> target-sparc\:\ fix\ Trap\ Based\ Address\ Register\ behavior\ for\
> sparc64.eml
> ERROR: DOS line endings
> #85: FILE: target-sparc/int64_helper.c:161:
> +    env->pc = env->tbr  & ~0x7fffULL;^M$
>
> ERROR: DOS line endings
> #86: FILE: target-sparc/int64_helper.c:162:
> +    env->pc |= ((env->tl > 1) ? 1 << 14 : 0) | (intno << 5);^M$
>
> total: 2 errors, 0 warnings, 11 lines checked
>
> /tmp/artyom/[PATCH 2_2 for-2.6] target-sparc: fix Trap Based Address
> Register behavior for sparc64.eml has style problems, please review.  If
> any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
>
>
> Possibly the DOS line endings are being added by a mail program?

I use git send-email (which uses msmtp). Used to work fine.

Clicking on "download mbox" on
https://patchwork.ozlabs.org/patch/610402/ and running checkpatch.pl
reveals no errors.

Are you sure the problem is not on your side?

Kind regards,
Artyom


-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PATCH 1/2 for-2.6] target-sparc: fix Nucleus quad LDD 128 bit access for windowed registers

[Mark Cave-Ayland]

On 14/04/16 15:49, Artyom Tarasenko wrote:

>> Possibly the DOS line endings are being added by a mail program?
> 
> I use git send-email (which uses msmtp). Used to work fine.
> 
> Clicking on "download mbox" on
> https://patchwork.ozlabs.org/patch/610402/ and running checkpatch.pl
> reveals no errors.
> 
> Are you sure the problem is not on your side?

In that case please accept my apologies - it seems something in that
patch is triggering my mail client to output CR+LF when exporting
messages. If I switch over to mutt then then indeed the patches appear
as they should.

I'm not sure if Richard is around at the moment, but they look trivial
enough and fix some nasty register corruption bugs so I think they are
worth adding to 2.6. If you can re-send with my R-B then I'll send a
pull request.


ATB,

Mark.