Re: [Qemu-devel] [PATCH] target-sparc: fix register corruption in ldstub if there is no write permission

2016-06-24 Thread Mark Cave-Ayland

On 24/06/16 17:01, Artyom Tarasenko wrote:


On Fri, Jun 24, 2016 at 5:51 PM, Mark Cave-Ayland
 wrote:

On 24/06/16 13:34, Artyom Tarasenko wrote:


Signed-off-by: Artyom Tarasenko 
---
 target-sparc/translate.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 5111cf0..065326c 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -5187,12 +5187,15 @@ printf("ops, illegal rdhpr\n");
 case 0xd:   /* ldstub -- XXX: should be atomically */
 {
 TCGv r_const;
+TCGv tmp = tcg_temp_new();

 gen_address_mask(dc, cpu_addr);
-tcg_gen_qemu_ld8u(cpu_val, cpu_addr,
dc->mem_idx);
+tcg_gen_qemu_ld8u(tmp, cpu_addr, dc->mem_idx);
 r_const = tcg_const_tl(0xff);
 tcg_gen_qemu_st8(r_const, cpu_addr, dc->mem_idx);
+tcg_gen_mov_tl(cpu_val, tmp);
 tcg_temp_free(r_const);
+tcg_temp_free(tmp);
 }
 break;
 case 0x0f:



Looks like you beat me to it - I can confirm that this fixes the issue here
for me. Whilst testing I noticed another regression under qemu-system-sparc,
however bisection reveals that this isn't caused by a SPARC-specific patch
(and can be followed up separately) so:

Tested-by: Mark Cave-Ayland 



Good. Then we can route it via your tree. (With Richard's Reviewed-by)
I'm still worried why it didn't hit us before.


Oops, looks like our mails overlapped. In that case I'll send a pull 
request ASAP.



ATB,

Mark.




Re: [Qemu-devel] [PATCH] target-sparc: fix register corruption in ldstub if there is no write permission

2016-06-24 Thread Mark Cave-Ayland

On 24/06/16 16:58, Richard Henderson wrote:


On 06/24/2016 05:34 AM, Artyom Tarasenko wrote:

Signed-off-by: Artyom Tarasenko 
---
 target-sparc/translate.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 5111cf0..065326c 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -5187,12 +5187,15 @@ printf("ops, illegal rdhpr\n");
 case 0xd:   /* ldstub -- XXX: should be
atomically */
 {
 TCGv r_const;
+TCGv tmp = tcg_temp_new();

 gen_address_mask(dc, cpu_addr);
-tcg_gen_qemu_ld8u(cpu_val, cpu_addr,
dc->mem_idx);
+tcg_gen_qemu_ld8u(tmp, cpu_addr, dc->mem_idx);
 r_const = tcg_const_tl(0xff);
 tcg_gen_qemu_st8(r_const, cpu_addr,
dc->mem_idx);
+tcg_gen_mov_tl(cpu_val, tmp);
 tcg_temp_free(r_const);
+tcg_temp_free(tmp);


ldstub_asi has the same problem on mainline.

It looks like I fixed that one on my sparc branch though.


r~


In that case would it make sense to prepend this patch to a v4 respin of 
your latest SPARC patchset (as tested by Artyom)?



ATB,

Mark.




Re: [Qemu-devel] [PATCH] target-sparc: fix register corruption in ldstub if there is no write permission

2016-06-24 Thread Artyom Tarasenko
On Fri, Jun 24, 2016 at 5:51 PM, Mark Cave-Ayland
 wrote:
> On 24/06/16 13:34, Artyom Tarasenko wrote:
>
>> Signed-off-by: Artyom Tarasenko 
>> ---
>>  target-sparc/translate.c | 5 -
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/target-sparc/translate.c b/target-sparc/translate.c
>> index 5111cf0..065326c 100644
>> --- a/target-sparc/translate.c
>> +++ b/target-sparc/translate.c
>> @@ -5187,12 +5187,15 @@ printf("ops, illegal rdhpr\n");
>>  case 0xd:   /* ldstub -- XXX: should be atomically */
>>  {
>>  TCGv r_const;
>> +TCGv tmp = tcg_temp_new();
>>
>>  gen_address_mask(dc, cpu_addr);
>> -tcg_gen_qemu_ld8u(cpu_val, cpu_addr,
>> dc->mem_idx);
>> +tcg_gen_qemu_ld8u(tmp, cpu_addr, dc->mem_idx);
>>  r_const = tcg_const_tl(0xff);
>>  tcg_gen_qemu_st8(r_const, cpu_addr, dc->mem_idx);
>> +tcg_gen_mov_tl(cpu_val, tmp);
>>  tcg_temp_free(r_const);
>> +tcg_temp_free(tmp);
>>  }
>>  break;
>>  case 0x0f:
>>
>
> Looks like you beat me to it - I can confirm that this fixes the issue here
> for me. Whilst testing I noticed another regression under qemu-system-sparc,
> however bisection reveals that this isn't caused by a SPARC-specific patch
> (and can be followed up separately) so:
>
> Tested-by: Mark Cave-Ayland 
>

Good. Then we can route it via your tree. (With Richard's Reviewed-by)
I'm still worried why it didn't hit us before.

Artyom

-- 
Regards,
Artyom Tarasenko

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



Re: [Qemu-devel] [PATCH] target-sparc: fix register corruption in ldstub if there is no write permission

2016-06-24 Thread Richard Henderson

On 06/24/2016 05:34 AM, Artyom Tarasenko wrote:

Signed-off-by: Artyom Tarasenko 
---
 target-sparc/translate.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 5111cf0..065326c 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -5187,12 +5187,15 @@ printf("ops, illegal rdhpr\n");
 case 0xd:   /* ldstub -- XXX: should be atomically */
 {
 TCGv r_const;
+TCGv tmp = tcg_temp_new();

 gen_address_mask(dc, cpu_addr);
-tcg_gen_qemu_ld8u(cpu_val, cpu_addr, dc->mem_idx);
+tcg_gen_qemu_ld8u(tmp, cpu_addr, dc->mem_idx);
 r_const = tcg_const_tl(0xff);
 tcg_gen_qemu_st8(r_const, cpu_addr, dc->mem_idx);
+tcg_gen_mov_tl(cpu_val, tmp);
 tcg_temp_free(r_const);
+tcg_temp_free(tmp);


ldstub_asi has the same problem on mainline.

It looks like I fixed that one on my sparc branch though.


r~




Re: [Qemu-devel] [PATCH] target-sparc: fix register corruption in ldstub if there is no write permission

2016-06-24 Thread Mark Cave-Ayland

On 24/06/16 13:34, Artyom Tarasenko wrote:


Signed-off-by: Artyom Tarasenko 
---
 target-sparc/translate.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 5111cf0..065326c 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -5187,12 +5187,15 @@ printf("ops, illegal rdhpr\n");
 case 0xd:   /* ldstub -- XXX: should be atomically */
 {
 TCGv r_const;
+TCGv tmp = tcg_temp_new();

 gen_address_mask(dc, cpu_addr);
-tcg_gen_qemu_ld8u(cpu_val, cpu_addr, dc->mem_idx);
+tcg_gen_qemu_ld8u(tmp, cpu_addr, dc->mem_idx);
 r_const = tcg_const_tl(0xff);
 tcg_gen_qemu_st8(r_const, cpu_addr, dc->mem_idx);
+tcg_gen_mov_tl(cpu_val, tmp);
 tcg_temp_free(r_const);
+tcg_temp_free(tmp);
 }
 break;
 case 0x0f:



Looks like you beat me to it - I can confirm that this fixes the issue 
here for me. Whilst testing I noticed another regression under 
qemu-system-sparc, however bisection reveals that this isn't caused by a 
SPARC-specific patch (and can be followed up separately) so:


Tested-by: Mark Cave-Ayland 


ATB,

Mark.



Re: [Qemu-devel] [PATCH] target-sparc: fix register corruption in ldstub if there is no write permission

2016-06-24 Thread Richard Henderson

On 06/24/2016 05:34 AM, Artyom Tarasenko wrote:

Signed-off-by: Artyom Tarasenko 
---
 target-sparc/translate.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)


Reviewed-by: Richard Henderson 


r~