Re: [PULL 00/32] AVR port

[Philippe Mathieu-Daudé]

On 7/10/20 5:12 PM, Peter Maydell wrote:
> On Fri, 10 Jul 2020 at 16:03, Thomas Huth <thuth@redhat.com> wrote:
>> Endianess bug ... this should fix it:
>>
>> diff --git a/target/avr/helper.c b/target/avr/helper.c
>> --- a/target/avr/helper.c
>> +++ b/target/avr/helper.c
>> @@ -337,6 +337,7 @@ void helper_fullwr(CPUAVRState *env, uint32_t data,
>> uint32_t addr)
>>          helper_outb(env, addr - NUMBER_OF_CPU_REGISTERS, data);
>>      } else {
>>          /* memory */
>> -        cpu_physical_memory_write(OFFSET_DATA + addr, &data, 1);
>> +        uint8_t data8 = data;
>> +        cpu_physical_memory_write(OFFSET_DATA + addr, &data8, 1);
>>      }
> 
> Or equivalently
>   address_space_stb(&address_space_memory, data, MEMTXATTRS_UNSPECIFIED, 
> NULL);
> 
> (better choices of address space may be available, but this is
> the exact-same-behaviour one).

Ah, this is my stashed fix:

-- >8 --
@@ -320,8 +320,10 @@ target_ulong helper_fullrd(CPUAVRState *env,
uint32_t addr)
  *  this function implements ST instruction when there is a posibility
to write
  *  into a CPU register
  */
-void helper_fullwr(CPUAVRState *env, uint32_t data, uint32_t addr)
+void helper_fullwr(CPUAVRState *env, uint32_t data32, uint32_t addr)
 {
+    uint8_t data = data32;
+    assert(data == data32);
+
     env->fullacc = false;

---

3 ways to do the same :) The assert is probably superfluous.

I don't like the fact that env->r[addr] (which is u8) is silently casted
from u32.