On 06.07.2011, at 00:05, Blue Swirl wrote: > On Wed, Jul 6, 2011 at 12:55 AM, Alexander Graf <ag...@suse.de> wrote: >> >> On 05.07.2011, at 23:48, Blue Swirl wrote: >> >>> On Tue, Jul 5, 2011 at 7:28 PM, Alexander Graf <ag...@suse.de> wrote: >>>> Device code some times needs to access physical memory and does that >>>> through the ld./st._phys functions. However, these are the exact same >>>> functions that the CPU uses to access memory, which means they will >>>> be endianness swapped depending on the target CPU. >>>> >>>> However, devices don't know about the CPU's endianness, but instead >>>> access memory directly using their own interface to the memory bus, >>>> so they need some way to read data with their native endianness. >>>> >>>> This patch adds _le and _be functions to ld./st._phys. >>>> >>>> Signed-off-by: Alexander Graf <ag...@suse.de> >>>> --- >>>> cpu-common.h | 12 +++++++ >>>> exec.c | 102 >>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>> 2 files changed, 114 insertions(+), 0 deletions(-) >>>> >>>> diff --git a/cpu-common.h b/cpu-common.h >>>> index b027e43..c6a2b5f 100644 >>>> --- a/cpu-common.h >>>> +++ b/cpu-common.h >>>> @@ -135,14 +135,26 @@ void qemu_flush_coalesced_mmio_buffer(void); >>>> >>>> uint32_t ldub_phys(target_phys_addr_t addr); >>>> uint32_t lduw_phys(target_phys_addr_t addr); >>>> +uint32_t lduw_le_phys(target_phys_addr_t addr); >>>> +uint32_t lduw_be_phys(target_phys_addr_t addr); >>>> uint32_t ldl_phys(target_phys_addr_t addr); >>>> +uint32_t ldl_le_phys(target_phys_addr_t addr); >>>> +uint32_t ldl_be_phys(target_phys_addr_t addr); >>>> uint64_t ldq_phys(target_phys_addr_t addr); >>>> +uint64_t ldq_le_phys(target_phys_addr_t addr); >>>> +uint64_t ldq_be_phys(target_phys_addr_t addr); >>>> void stl_phys_notdirty(target_phys_addr_t addr, uint32_t val); >>>> void stq_phys_notdirty(target_phys_addr_t addr, uint64_t val); >>>> void stb_phys(target_phys_addr_t addr, uint32_t val); >>>> void stw_phys(target_phys_addr_t addr, uint32_t val); >>>> +void stw_le_phys(target_phys_addr_t addr, uint32_t val); >>>> +void stw_be_phys(target_phys_addr_t addr, uint32_t val); >>>> void stl_phys(target_phys_addr_t addr, uint32_t val); >>>> +void stl_le_phys(target_phys_addr_t addr, uint32_t val); >>>> +void stl_be_phys(target_phys_addr_t addr, uint32_t val); >>>> void stq_phys(target_phys_addr_t addr, uint64_t val); >>>> +void stq_le_phys(target_phys_addr_t addr, uint64_t val); >>>> +void stq_be_phys(target_phys_addr_t addr, uint64_t val); >>>> >>>> void cpu_physical_memory_write_rom(target_phys_addr_t addr, >>>> const uint8_t *buf, int len); >>>> diff --git a/exec.c b/exec.c >>>> index 4c45299..5f2f87e 100644 >>>> --- a/exec.c >>>> +++ b/exec.c >>>> @@ -4158,6 +4158,24 @@ uint32_t ldl_phys(target_phys_addr_t addr) >>>> return val; >>>> } >>>> >>>> +uint32_t ldl_le_phys(target_phys_addr_t addr) >>>> +{ >>>> +#if defined(TARGET_WORDS_BIGENDIAN) >>>> + return bswap32(ldl_phys(addr)); >>> >>> This would introduce a second bswap in some cases. Please make instead >>> two versions of ldl_phys which use ldl_le/be_p instead of ldl_p. Then >>> ldl_phys could be #defined to the suitable function. >> >> Yeah, I was thinking to do that one at first and then realized how MMIO gets >> tricky, since we also need to potentially swap it again, as by now it >> returns values in target CPU endianness. But if you prefer, I can dig into >> that. > > Maybe the swapendian thing could be adjusted so that it's possible to > access the device in either LE or BE way directly? For example, there > could be two io_mem_read/write tables, the current one for CPU and > another one for device MMIO with known endianness. Or even three > tables: CPU, BE, LE.
If you take a look at the patches following this one, you can easily see how few devices actually use these functions. I don't think the additional memory usage would count up for a couple of byte swaps here really. We could however try to be clever and directly check if the device mmio is a swapendian function and just bypass it. I'm just not sure it's worth the additional overhead for the non-swapped case (which should be the normal one). Alex