diff mbox series

[1/2] hw/mips: bootloader: Fix write_ulong

Message ID 20211130211729.7116-2-jiaxun.yang@flygoat.com (mailing list archive)
State New, archived
Headers show
Series MIPS misc fixes | expand

Commit Message

Jiaxun Yang Nov. 30, 2021, 9:17 p.m. UTC
bl_gen_write_ulong uses sd for both 32 and 64 bit CPU,
while sd is illegal on 32 bit CPUs.

Replace sd with sw on 32bit CPUs.

Fixes: 3ebbf86 ("hw/mips: Add a bootloader helper")
Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
Should be backported to 6.0 onwards.
---
 hw/mips/bootloader.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Philippe Mathieu-Daudé Nov. 30, 2021, 9:52 p.m. UTC | #1
On 11/30/21 22:17, Jiaxun Yang wrote:
> bl_gen_write_ulong uses sd for both 32 and 64 bit CPU,
> while sd is illegal on 32 bit CPUs.
> 
> Replace sd with sw on 32bit CPUs.
> 
> Fixes: 3ebbf86 ("hw/mips: Add a bootloader helper")
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
> Should be backported to 6.0 onwards.
> ---
>  hw/mips/bootloader.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/mips/bootloader.c b/hw/mips/bootloader.c
> index 6ec8314490..99991f8b2b 100644
> --- a/hw/mips/bootloader.c
> +++ b/hw/mips/bootloader.c
> @@ -182,7 +182,11 @@ void bl_gen_write_ulong(uint32_t **p, target_ulong addr, target_ulong val)
>  {
>      bl_gen_load_ulong(p, BL_REG_K0, val);
>      bl_gen_load_ulong(p, BL_REG_K1, addr);
> -    bl_gen_sd(p, BL_REG_K0, BL_REG_K1, 0x0);
> +    if (bootcpu_supports_isa(ISA_MIPS3)) {
> +        bl_gen_sd(p, BL_REG_K0, BL_REG_K1, 0x0);
> +    } else {
> +        bl_gen_sw(p, BL_REG_K0, BL_REG_K1, 0x0);
> +    }

We have bl_gen_load_ulong(); having bl_gen_store_ulong()
would make the API even. Mind sending a patch? Otherwise:

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>  }
>  
>  void bl_gen_write_u32(uint32_t **p, target_ulong addr, uint32_t val)
>
Jiaxun Yang Dec. 2, 2021, 10:51 a.m. UTC | #2
在2021年11月30日十一月 下午9:52,Philippe Mathieu-Daudé写道:
> On 11/30/21 22:17, Jiaxun Yang wrote:
>> bl_gen_write_ulong uses sd for both 32 and 64 bit CPU,
>> while sd is illegal on 32 bit CPUs.
>> 
>> Replace sd with sw on 32bit CPUs.
>> 
>> Fixes: 3ebbf86 ("hw/mips: Add a bootloader helper")
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> ---
>> Should be backported to 6.0 onwards.
>> ---
>>  hw/mips/bootloader.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>> 
>> diff --git a/hw/mips/bootloader.c b/hw/mips/bootloader.c
>> index 6ec8314490..99991f8b2b 100644
>> --- a/hw/mips/bootloader.c
>> +++ b/hw/mips/bootloader.c
>> @@ -182,7 +182,11 @@ void bl_gen_write_ulong(uint32_t **p, target_ulong addr, target_ulong val)
>>  {
>>      bl_gen_load_ulong(p, BL_REG_K0, val);
>>      bl_gen_load_ulong(p, BL_REG_K1, addr);
>> -    bl_gen_sd(p, BL_REG_K0, BL_REG_K1, 0x0);
>> +    if (bootcpu_supports_isa(ISA_MIPS3)) {
>> +        bl_gen_sd(p, BL_REG_K0, BL_REG_K1, 0x0);
>> +    } else {
>> +        bl_gen_sw(p, BL_REG_K0, BL_REG_K1, 0x0);
>> +    }
>
> We have bl_gen_load_ulong(); having bl_gen_store_ulong()
> would make the API even. Mind sending a patch? Otherwise:

Should I revisit this set or start another patch?

Thanks.

>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
>>  }
>>  
>>  void bl_gen_write_u32(uint32_t **p, target_ulong addr, uint32_t val)
>>
Philippe Mathieu-Daudé Dec. 2, 2021, 6:01 p.m. UTC | #3
On 12/2/21 11:51, Jiaxun Yang wrote:
> 在2021年11月30日十一月 下午9:52,Philippe Mathieu-Daudé写道:
>> On 11/30/21 22:17, Jiaxun Yang wrote:
>>> bl_gen_write_ulong uses sd for both 32 and 64 bit CPU,
>>> while sd is illegal on 32 bit CPUs.
>>>
>>> Replace sd with sw on 32bit CPUs.
>>>
>>> Fixes: 3ebbf86 ("hw/mips: Add a bootloader helper")
>>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>> ---
>>> Should be backported to 6.0 onwards.
>>> ---
>>>  hw/mips/bootloader.c | 6 +++++-
>>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/mips/bootloader.c b/hw/mips/bootloader.c
>>> index 6ec8314490..99991f8b2b 100644
>>> --- a/hw/mips/bootloader.c
>>> +++ b/hw/mips/bootloader.c
>>> @@ -182,7 +182,11 @@ void bl_gen_write_ulong(uint32_t **p, target_ulong addr, target_ulong val)
>>>  {
>>>      bl_gen_load_ulong(p, BL_REG_K0, val);
>>>      bl_gen_load_ulong(p, BL_REG_K1, addr);
>>> -    bl_gen_sd(p, BL_REG_K0, BL_REG_K1, 0x0);
>>> +    if (bootcpu_supports_isa(ISA_MIPS3)) {
>>> +        bl_gen_sd(p, BL_REG_K0, BL_REG_K1, 0x0);
>>> +    } else {
>>> +        bl_gen_sw(p, BL_REG_K0, BL_REG_K1, 0x0);
>>> +    }
>>
>> We have bl_gen_load_ulong(); having bl_gen_store_ulong()
>> would make the API even. Mind sending a patch? Otherwise:
> 
> Should I revisit this set or start another patch?

Another patch :)
Jiaxun Yang Dec. 5, 2021, 3:25 p.m. UTC | #4
在2021年12月2日十二月 下午6:01,Philippe Mathieu-Daudé写道:
> On 12/2/21 11:51, Jiaxun Yang wrote:
>> 在2021年11月30日十一月 下午9:52,Philippe Mathieu-Daudé写道:
>>> On 11/30/21 22:17, Jiaxun Yang wrote:
>>>> bl_gen_write_ulong uses sd for both 32 and 64 bit CPU,
>>>> while sd is illegal on 32 bit CPUs.
>>>>
>>>> Replace sd with sw on 32bit CPUs.
>>>>
>>>> Fixes: 3ebbf86 ("hw/mips: Add a bootloader helper")
>>>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>>> ---
>>>> Should be backported to 6.0 onwards.
>>>> ---
>>>>  hw/mips/bootloader.c | 6 +++++-
>>>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/hw/mips/bootloader.c b/hw/mips/bootloader.c
>>>> index 6ec8314490..99991f8b2b 100644
>>>> --- a/hw/mips/bootloader.c
>>>> +++ b/hw/mips/bootloader.c
>>>> @@ -182,7 +182,11 @@ void bl_gen_write_ulong(uint32_t **p, target_ulong addr, target_ulong val)
>>>>  {
>>>>      bl_gen_load_ulong(p, BL_REG_K0, val);
>>>>      bl_gen_load_ulong(p, BL_REG_K1, addr);
>>>> -    bl_gen_sd(p, BL_REG_K0, BL_REG_K1, 0x0);
>>>> +    if (bootcpu_supports_isa(ISA_MIPS3)) {
>>>> +        bl_gen_sd(p, BL_REG_K0, BL_REG_K1, 0x0);
>>>> +    } else {
>>>> +        bl_gen_sw(p, BL_REG_K0, BL_REG_K1, 0x0);
>>>> +    }
>>>
>>> We have bl_gen_load_ulong(); having bl_gen_store_ulong()
>>> would make the API even. Mind sending a patch? Otherwise:
>> 
>> Should I revisit this set or start another patch?
>
> Another patch :)

Just revisited the code and bl_gen_load_ulong means load a target_ulong
immedaite number, not load from memory so it's not even.

I'd leave it as is.

Thanks.
diff mbox series

Patch

diff --git a/hw/mips/bootloader.c b/hw/mips/bootloader.c
index 6ec8314490..99991f8b2b 100644
--- a/hw/mips/bootloader.c
+++ b/hw/mips/bootloader.c
@@ -182,7 +182,11 @@  void bl_gen_write_ulong(uint32_t **p, target_ulong addr, target_ulong val)
 {
     bl_gen_load_ulong(p, BL_REG_K0, val);
     bl_gen_load_ulong(p, BL_REG_K1, addr);
-    bl_gen_sd(p, BL_REG_K0, BL_REG_K1, 0x0);
+    if (bootcpu_supports_isa(ISA_MIPS3)) {
+        bl_gen_sd(p, BL_REG_K0, BL_REG_K1, 0x0);
+    } else {
+        bl_gen_sw(p, BL_REG_K0, BL_REG_K1, 0x0);
+    }
 }
 
 void bl_gen_write_u32(uint32_t **p, target_ulong addr, uint32_t val)