On 7/10/20 5:17 PM, Philippe Mathieu-Daudé wrote:
> 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.
I'll squash Peter suggested fix:
-- >8 --
--- a/target/avr/helper.c
+++ b/target/avr/helper.c
@@ -232,7 +232,9 @@ target_ulong helper_inb(CPUAVRState *env, uint32_t port)
break;
default:
/* not a special register, pass to normal memory access */
- cpu_physical_memory_read(OFFSET_IO_REGISTERS + port, &data, 1);
+ data = address_space_ldub(&address_space_memory,
+ OFFSET_IO_REGISTERS + port,
+ MEMTXATTRS_UNSPECIFIED, NULL);
}
return data;
@@ -289,7 +291,8 @@ void helper_outb(CPUAVRState *env, uint32_t port,
uint32_t data)
break;
default:
/* not a special register, pass to normal memory access */
- cpu_physical_memory_write(OFFSET_IO_REGISTERS + port, &data, 1);
+ address_space_stb(&address_space_memory, OFFSET_IO_REGISTERS +
port,
+ data, MEMTXATTRS_UNSPECIFIED, NULL);
}
}
@@ -305,13 +308,14 @@ target_ulong helper_fullrd(CPUAVRState *env,
uint32_t addr)
if (addr < NUMBER_OF_CPU_REGISTERS) {
/* CPU registers */
- data = env->r[addr];
+ data = cpu_to_le32(env->r[addr]);
} else if (addr < NUMBER_OF_CPU_REGISTERS + NUMBER_OF_IO_REGISTERS) {
/* IO registers */
data = helper_inb(env, addr - NUMBER_OF_CPU_REGISTERS);
} else {
/* memory */
- cpu_physical_memory_read(OFFSET_DATA + addr, &data, 1);
+ data = address_space_ldub(&address_space_memory, OFFSET_DATA +
addr,
+ MEMTXATTRS_UNSPECIFIED, NULL);
}
return data;
}
@@ -337,6 +341,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);
+ address_space_stb(&address_space_memory, OFFSET_DATA + addr, data,
+ MEMTXATTRS_UNSPECIFIED, NULL);
}
}
---