On 06.07.2011, at 00:22, Blue Swirl wrote:
> On Wed, Jul 6, 2011 at 1:13 AM, Alexander Graf <ag...@suse.de> wrote:
>>
>> 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).
>
> Neither seems to be very attractive option. It may be enough to
> optimize just RAM accesses and forget about extra bswap for MMIO for
> now.
How about something like this on top? Plus the q, uw and st version of course.
The inline is there on purpose, moving the be/le specific code (which is rarely
used) out of the way from the often(?) used native ones. Eventually, TCG
generated code comes into these, no?
diff --git a/exec.c b/exec.c
index 5f2f87e..f281ba4 100644
--- a/exec.c
+++ b/exec.c
@@ -4127,7 +4127,8 @@ void cpu_physical_memory_unmap(void *buffer,
target_phys_addr_t len,
}
/* warning: addr must be aligned */
-uint32_t ldl_phys(target_phys_addr_t addr)
+static inline uint32_t ldl_phys_internal(target_phys_addr_t addr,
+ enum device_endian endian)
{
int io_index;
uint8_t *ptr;
@@ -4149,31 +4150,47 @@ uint32_t ldl_phys(target_phys_addr_t addr)
if (p)
addr = (addr & ~TARGET_PAGE_MASK) + p->region_offset;
val = io_mem_read[io_index][2](io_mem_opaque[io_index], addr);
+#if defined(TARGET_WORDS_BIGENDIAN)
+ if (endian == DEVICE_LITTLE_ENDIAN) {
+ val = bswap32(val);
+ }
+#else
+ if (endian == DEVICE_BIG_ENDIAN) {
+ val = bswap32(val);
+ }
+#endif
} else {
/* RAM case */
ptr = qemu_get_ram_ptr(pd & TARGET_PAGE_MASK) +
(addr & ~TARGET_PAGE_MASK);
- val = ldl_p(ptr);
+ switch (endian) {
+ case DEVICE_LITTLE_ENDIAN:
+ val = ldl_le_p(ptr);
+ break;
+ case DEVICE_BIG_ENDIAN:
+ val = ldl_be_p(ptr);
+ break;
+ default:
+ val = ldl_p(ptr);
+ break;
+ }
}
return val;
}
+uint32_t ldl_phys(target_phys_addr_t addr)
+{
+ return ldl_phys_internal(addr, DEVICE_NATIVE_ENDIAN);
+}
+
uint32_t ldl_le_phys(target_phys_addr_t addr)
{
-#if defined(TARGET_WORDS_BIGENDIAN)
- return bswap32(ldl_phys(addr));
-#else
- return ldl_phys(addr);
-#endif
+ return ldl_phys_internal(addr, DEVICE_LITTLE_ENDIAN);
}
uint32_t ldl_be_phys(target_phys_addr_t addr)
{
-#if defined(TARGET_WORDS_BIGENDIAN)
- return ldl_phys(addr);
-#else
- return bswap32(ldl_phys(addr));
-#endif
+ return ldl_phys_internal(addr, DEVICE_BIG_ENDIAN);
}
/* warning: addr must be aligned */
Alex
