* [Qemu-devel] [RFC PATCH] memory: fix access_with_adjusted_size() on big-endian
@ 2017-08-11 14:06 Philippe Mathieu-Daudé
2017-08-11 14:25 ` Paolo Bonzini
0 siblings, 1 reply; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-08-11 14:06 UTC (permalink / raw)
To: Peter Maydell, KONRAD Frederic, Peter Crosthwaite, Paolo Bonzini
Cc: Philippe Mathieu-Daudé, qemu-devel
remove unnecessary memory_region_big_endian()
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
This is part of a branch with many cross-endianness tests for 2.11
Frederic does it helps your issues on armeb?
memory.c | 28 ++++++++++------------------
1 file changed, 10 insertions(+), 18 deletions(-)
diff --git a/memory.c b/memory.c
index c0adc35410..600f5d5b1a 100644
--- a/memory.c
+++ b/memory.c
@@ -338,15 +338,6 @@ static void flatview_simplify(FlatView *view)
}
}
-static bool memory_region_big_endian(MemoryRegion *mr)
-{
-#ifdef TARGET_WORDS_BIGENDIAN
- return mr->ops->endianness != DEVICE_LITTLE_ENDIAN;
-#else
- return mr->ops->endianness == DEVICE_BIG_ENDIAN;
-#endif
-}
-
static bool memory_region_wrong_endianness(MemoryRegion *mr)
{
#ifdef TARGET_WORDS_BIGENDIAN
@@ -572,7 +563,8 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
{
uint64_t access_mask;
unsigned access_size;
- unsigned i;
+ hwaddr access_addr;
+ unsigned offset;
MemTxResult r = MEMTX_OK;
if (!access_size_min) {
@@ -585,17 +577,17 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
/* FIXME: support unaligned access? */
access_size = MAX(MIN(size, access_size_max), access_size_min);
access_mask = -1ULL >> (64 - access_size * 8);
- if (memory_region_big_endian(mr)) {
- for (i = 0; i < size; i += access_size) {
- r |= access(mr, addr + i, value, access_size,
- (size - access_size - i) * 8, access_mask, attrs);
+ access_addr = addr & ~(access_size - 1);
+ if (access_size < size) {
+ for (offset = 0; offset < size; offset += access_size) {
+ r |= access(mr, access_addr + offset, value, access_size,
+ offset * 8, access_mask, attrs);
}
} else {
- for (i = 0; i < size; i += access_size) {
- r |= access(mr, addr + i, value, access_size, i * 8,
- access_mask, attrs);
- }
+ r = access(mr, access_addr, value, access_size,
+ 0, access_mask, attrs);
}
+
return r;
}
--
2.13.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [RFC PATCH] memory: fix access_with_adjusted_size() on big-endian
2017-08-11 14:06 [Qemu-devel] [RFC PATCH] memory: fix access_with_adjusted_size() on big-endian Philippe Mathieu-Daudé
@ 2017-08-11 14:25 ` Paolo Bonzini
2017-08-11 14:49 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 4+ messages in thread
From: Paolo Bonzini @ 2017-08-11 14:25 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Peter Maydell, KONRAD Frederic,
Peter Crosthwaite
Cc: qemu-devel
On 11/08/2017 16:06, Philippe Mathieu-Daudé wrote:
> remove unnecessary memory_region_big_endian()
Can you explain why it's unnecessary?...
Paolo
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> This is part of a branch with many cross-endianness tests for 2.11
>
> Frederic does it helps your issues on armeb?
>
> memory.c | 28 ++++++++++------------------
> 1 file changed, 10 insertions(+), 18 deletions(-)
>
> diff --git a/memory.c b/memory.c
> index c0adc35410..600f5d5b1a 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -338,15 +338,6 @@ static void flatview_simplify(FlatView *view)
> }
> }
>
> -static bool memory_region_big_endian(MemoryRegion *mr)
> -{
> -#ifdef TARGET_WORDS_BIGENDIAN
> - return mr->ops->endianness != DEVICE_LITTLE_ENDIAN;
> -#else
> - return mr->ops->endianness == DEVICE_BIG_ENDIAN;
> -#endif
> -}
> -
> static bool memory_region_wrong_endianness(MemoryRegion *mr)
> {
> #ifdef TARGET_WORDS_BIGENDIAN
> @@ -572,7 +563,8 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
> {
> uint64_t access_mask;
> unsigned access_size;
> - unsigned i;
> + hwaddr access_addr;
> + unsigned offset;
> MemTxResult r = MEMTX_OK;
>
> if (!access_size_min) {
> @@ -585,17 +577,17 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
> /* FIXME: support unaligned access? */
> access_size = MAX(MIN(size, access_size_max), access_size_min);
> access_mask = -1ULL >> (64 - access_size * 8);
> - if (memory_region_big_endian(mr)) {
> - for (i = 0; i < size; i += access_size) {
> - r |= access(mr, addr + i, value, access_size,
> - (size - access_size - i) * 8, access_mask, attrs);
> + access_addr = addr & ~(access_size - 1);
> + if (access_size < size) {
> + for (offset = 0; offset < size; offset += access_size) {
> + r |= access(mr, access_addr + offset, value, access_size,
> + offset * 8, access_mask, attrs);
> }
> } else {
> - for (i = 0; i < size; i += access_size) {
> - r |= access(mr, addr + i, value, access_size, i * 8,
> - access_mask, attrs);
> - }
> + r = access(mr, access_addr, value, access_size,
> + 0, access_mask, attrs);
> }
> +
> return r;
> }
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [RFC PATCH] memory: fix access_with_adjusted_size() on big-endian
2017-08-11 14:25 ` Paolo Bonzini
@ 2017-08-11 14:49 ` Philippe Mathieu-Daudé
2017-08-11 17:13 ` KONRAD Frederic
0 siblings, 1 reply; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-08-11 14:49 UTC (permalink / raw)
To: Paolo Bonzini, Peter Maydell, KONRAD Frederic, Peter Crosthwaite
Cc: qemu-devel
On 08/11/2017 11:25 AM, Paolo Bonzini wrote:
> On 11/08/2017 16:06, Philippe Mathieu-Daudé wrote:
>> remove unnecessary memory_region_big_endian()
>
> Can you explain why it's unnecessary?...
I might have missed a lot here, I'm focusing on something else and
thought this might help Frederic so I only cherry-picked this patch from
a bigger series, since I memory_region_big_endian() is static/unused I
just dropped it, which is surely wrong.
I see in this series I then call "adjust_endianness(mr, value, size)"
before returning. And then lot of unit-tests to understand failures I'm
having accessing PCI bus on armeb.
This branch is few months old and I'm out of context, I shouldn't have
rushed to send this patch, I'll try to get some time during the week-end
to remember.
Cheers,
Phil.
>
> Paolo
>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> This is part of a branch with many cross-endianness tests for 2.11
>>
>> Frederic does it helps your issues on armeb?
>>
>> memory.c | 28 ++++++++++------------------
>> 1 file changed, 10 insertions(+), 18 deletions(-)
>>
>> diff --git a/memory.c b/memory.c
>> index c0adc35410..600f5d5b1a 100644
>> --- a/memory.c
>> +++ b/memory.c
>> @@ -338,15 +338,6 @@ static void flatview_simplify(FlatView *view)
>> }
>> }
>>
>> -static bool memory_region_big_endian(MemoryRegion *mr)
>> -{
>> -#ifdef TARGET_WORDS_BIGENDIAN
>> - return mr->ops->endianness != DEVICE_LITTLE_ENDIAN;
>> -#else
>> - return mr->ops->endianness == DEVICE_BIG_ENDIAN;
>> -#endif
>> -}
>> -
>> static bool memory_region_wrong_endianness(MemoryRegion *mr)
>> {
>> #ifdef TARGET_WORDS_BIGENDIAN
>> @@ -572,7 +563,8 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
>> {
>> uint64_t access_mask;
>> unsigned access_size;
>> - unsigned i;
>> + hwaddr access_addr;
>> + unsigned offset;
>> MemTxResult r = MEMTX_OK;
>>
>> if (!access_size_min) {
>> @@ -585,17 +577,17 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
>> /* FIXME: support unaligned access? */
>> access_size = MAX(MIN(size, access_size_max), access_size_min);
>> access_mask = -1ULL >> (64 - access_size * 8);
>> - if (memory_region_big_endian(mr)) {
>> - for (i = 0; i < size; i += access_size) {
>> - r |= access(mr, addr + i, value, access_size,
>> - (size - access_size - i) * 8, access_mask, attrs);
>> + access_addr = addr & ~(access_size - 1);
>> + if (access_size < size) {
>> + for (offset = 0; offset < size; offset += access_size) {
>> + r |= access(mr, access_addr + offset, value, access_size,
>> + offset * 8, access_mask, attrs);
>> }
>> } else {
>> - for (i = 0; i < size; i += access_size) {
>> - r |= access(mr, addr + i, value, access_size, i * 8,
>> - access_mask, attrs);
>> - }
>> + r = access(mr, access_addr, value, access_size,
>> + 0, access_mask, attrs);
>> }
>> +
>> return r;
>> }
>>
>>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [RFC PATCH] memory: fix access_with_adjusted_size() on big-endian
2017-08-11 14:49 ` Philippe Mathieu-Daudé
@ 2017-08-11 17:13 ` KONRAD Frederic
0 siblings, 0 replies; 4+ messages in thread
From: KONRAD Frederic @ 2017-08-11 17:13 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Paolo Bonzini, Peter Maydell, Peter Crosthwaite, qemu-devel
Hi Philippe,
Thanks for that, I'll try this out when I'm back. The 15th is
blank holiday here.
Thanks,
Fred
On 08/11/2017 04:49 PM, Philippe Mathieu-Daudé wrote:
> On 08/11/2017 11:25 AM, Paolo Bonzini wrote:
>> On 11/08/2017 16:06, Philippe Mathieu-Daudé wrote:
>>> remove unnecessary memory_region_big_endian()
>>
>> Can you explain why it's unnecessary?...
>
> I might have missed a lot here, I'm focusing on something else
> and thought this might help Frederic so I only cherry-picked this
> patch from a bigger series, since I memory_region_big_endian() is
> static/unused I just dropped it, which is surely wrong.
>
> I see in this series I then call "adjust_endianness(mr, value,
> size)" before returning. And then lot of unit-tests to understand
> failures I'm having accessing PCI bus on armeb.
>
> This branch is few months old and I'm out of context, I shouldn't
> have rushed to send this patch, I'll try to get some time during
> the week-end to remember.
>
> Cheers,
>
> Phil.
>
>>
>> Paolo
>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>> This is part of a branch with many cross-endianness tests for
>>> 2.11
>>>
>>> Frederic does it helps your issues on armeb?
>>>
>>> memory.c | 28 ++++++++++------------------
>>> 1 file changed, 10 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/memory.c b/memory.c
>>> index c0adc35410..600f5d5b1a 100644
>>> --- a/memory.c
>>> +++ b/memory.c
>>> @@ -338,15 +338,6 @@ static void flatview_simplify(FlatView
>>> *view)
>>> }
>>> }
>>> -static bool memory_region_big_endian(MemoryRegion *mr)
>>> -{
>>> -#ifdef TARGET_WORDS_BIGENDIAN
>>> - return mr->ops->endianness != DEVICE_LITTLE_ENDIAN;
>>> -#else
>>> - return mr->ops->endianness == DEVICE_BIG_ENDIAN;
>>> -#endif
>>> -}
>>> -
>>> static bool memory_region_wrong_endianness(MemoryRegion *mr)
>>> {
>>> #ifdef TARGET_WORDS_BIGENDIAN
>>> @@ -572,7 +563,8 @@ static MemTxResult
>>> access_with_adjusted_size(hwaddr addr,
>>> {
>>> uint64_t access_mask;
>>> unsigned access_size;
>>> - unsigned i;
>>> + hwaddr access_addr;
>>> + unsigned offset;
>>> MemTxResult r = MEMTX_OK;
>>> if (!access_size_min) {
>>> @@ -585,17 +577,17 @@ static MemTxResult
>>> access_with_adjusted_size(hwaddr addr,
>>> /* FIXME: support unaligned access? */
>>> access_size = MAX(MIN(size, access_size_max),
>>> access_size_min);
>>> access_mask = -1ULL >> (64 - access_size * 8);
>>> - if (memory_region_big_endian(mr)) {
>>> - for (i = 0; i < size; i += access_size) {
>>> - r |= access(mr, addr + i, value, access_size,
>>> - (size - access_size - i) * 8,
>>> access_mask, attrs);
>>> + access_addr = addr & ~(access_size - 1);
>>> + if (access_size < size) {
>>> + for (offset = 0; offset < size; offset += access_size) {
>>> + r |= access(mr, access_addr + offset, value,
>>> access_size,
>>> + offset * 8, access_mask, attrs);
>>> }
>>> } else {
>>> - for (i = 0; i < size; i += access_size) {
>>> - r |= access(mr, addr + i, value, access_size, i * 8,
>>> - access_mask, attrs);
>>> - }
>>> + r = access(mr, access_addr, value, access_size,
>>> + 0, access_mask, attrs);
>>> }
>>> +
>>> return r;
>>> }
>>>
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-08-11 17:14 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-11 14:06 [Qemu-devel] [RFC PATCH] memory: fix access_with_adjusted_size() on big-endian Philippe Mathieu-Daudé
2017-08-11 14:25 ` Paolo Bonzini
2017-08-11 14:49 ` Philippe Mathieu-Daudé
2017-08-11 17:13 ` KONRAD Frederic
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).