| Message ID | 20250310045842.2650784-5-pierrick.bouvier@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | make system memory API available for common code | expand |
On 3/9/25 21:58, Pierrick Bouvier wrote: > Will allow to make system/memory.c common later. > > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > --- > include/exec/memory.h | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) I guess this was split from patch 3 without updating descriptions, and this is the TARGET_BIG_ENDIAN reference removed. Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On 3/10/25 08:25, Richard Henderson wrote: > On 3/9/25 21:58, Pierrick Bouvier wrote: >> Will allow to make system/memory.c common later. >> >> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >> --- >> include/exec/memory.h | 12 +++++------- >> 1 file changed, 5 insertions(+), 7 deletions(-) > > I guess this was split from patch 3 without updating descriptions, > and this is the TARGET_BIG_ENDIAN reference removed. > This was intentionally splitted, and the subject mentions the goal: make devend_memop target agnostic. As added in the description, it's needed to allow system/memory.c to be compiled as common code. > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > > > r~
On 3/9/25 21:58, Pierrick Bouvier wrote: > Will allow to make system/memory.c common later. > > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > --- > include/exec/memory.h | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/include/exec/memory.h b/include/exec/memory.h > index 7c20f36a312..698179b26d2 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -3164,25 +3164,23 @@ address_space_write_cached(MemoryRegionCache *cache, hwaddr addr, > MemTxResult address_space_set(AddressSpace *as, hwaddr addr, > uint8_t c, hwaddr len, MemTxAttrs attrs); > > -#ifdef COMPILING_PER_TARGET > /* enum device_endian to MemOp. */ > static inline MemOp devend_memop(enum device_endian end) > { > QEMU_BUILD_BUG_ON(DEVICE_HOST_ENDIAN != DEVICE_LITTLE_ENDIAN && > DEVICE_HOST_ENDIAN != DEVICE_BIG_ENDIAN); > > -#if HOST_BIG_ENDIAN != TARGET_BIG_ENDIAN > - /* Swap if non-host endianness or native (target) endianness */ > - return (end == DEVICE_HOST_ENDIAN) ? 0 : MO_BSWAP; > -#else > + if (HOST_BIG_ENDIAN != target_words_bigendian()) { > + /* Swap if non-host endianness or native (target) endianness */ > + return (end == DEVICE_HOST_ENDIAN) ? 0 : MO_BSWAP; > + } > + > const int non_host_endianness = > DEVICE_LITTLE_ENDIAN ^ DEVICE_BIG_ENDIAN ^ DEVICE_HOST_ENDIAN; > > /* In this case, native (target) endianness needs no swap. */ > return (end == non_host_endianness) ? MO_BSWAP : 0; > -#endif > } > -#endif /* COMPILING_PER_TARGET */ Someone (me?) was trying to be overly clever here. We can simplify this function and conditionally avoid the function call: bool big_endian = (end == DEVICE_NATIVE_ENDIAN ? target_words_bigendian() : end == DEVICE_BIG_ENDIAN); return big_endian ? MO_BE : MO_LE; r~
On 3/10/25 09:30, Richard Henderson wrote: > On 3/9/25 21:58, Pierrick Bouvier wrote: >> Will allow to make system/memory.c common later. >> >> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >> --- >> include/exec/memory.h | 12 +++++------- >> 1 file changed, 5 insertions(+), 7 deletions(-) >> >> diff --git a/include/exec/memory.h b/include/exec/memory.h >> index 7c20f36a312..698179b26d2 100644 >> --- a/include/exec/memory.h >> +++ b/include/exec/memory.h >> @@ -3164,25 +3164,23 @@ address_space_write_cached(MemoryRegionCache *cache, hwaddr addr, >> MemTxResult address_space_set(AddressSpace *as, hwaddr addr, >> uint8_t c, hwaddr len, MemTxAttrs attrs); >> >> -#ifdef COMPILING_PER_TARGET >> /* enum device_endian to MemOp. */ >> static inline MemOp devend_memop(enum device_endian end) >> { >> QEMU_BUILD_BUG_ON(DEVICE_HOST_ENDIAN != DEVICE_LITTLE_ENDIAN && >> DEVICE_HOST_ENDIAN != DEVICE_BIG_ENDIAN); >> >> -#if HOST_BIG_ENDIAN != TARGET_BIG_ENDIAN >> - /* Swap if non-host endianness or native (target) endianness */ >> - return (end == DEVICE_HOST_ENDIAN) ? 0 : MO_BSWAP; >> -#else >> + if (HOST_BIG_ENDIAN != target_words_bigendian()) { >> + /* Swap if non-host endianness or native (target) endianness */ >> + return (end == DEVICE_HOST_ENDIAN) ? 0 : MO_BSWAP; >> + } >> + >> const int non_host_endianness = >> DEVICE_LITTLE_ENDIAN ^ DEVICE_BIG_ENDIAN ^ DEVICE_HOST_ENDIAN; >> >> /* In this case, native (target) endianness needs no swap. */ >> return (end == non_host_endianness) ? MO_BSWAP : 0; >> -#endif >> } >> -#endif /* COMPILING_PER_TARGET */ > > Someone (me?) was trying to be overly clever here. Tends to happen through the QEMU codebase :). > We can simplify this function and conditionally avoid the function call: > > bool big_endian = (end == DEVICE_NATIVE_ENDIAN > ? target_words_bigendian() > : end == DEVICE_BIG_ENDIAN); > return big_endian ? MO_BE : MO_LE; > That's fine for me (still requires a bit of cleverness to read it at first). > > r~
diff --git a/include/exec/memory.h b/include/exec/memory.h index 7c20f36a312..698179b26d2 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -3164,25 +3164,23 @@ address_space_write_cached(MemoryRegionCache *cache, hwaddr addr, MemTxResult address_space_set(AddressSpace *as, hwaddr addr, uint8_t c, hwaddr len, MemTxAttrs attrs); -#ifdef COMPILING_PER_TARGET /* enum device_endian to MemOp. */ static inline MemOp devend_memop(enum device_endian end) { QEMU_BUILD_BUG_ON(DEVICE_HOST_ENDIAN != DEVICE_LITTLE_ENDIAN && DEVICE_HOST_ENDIAN != DEVICE_BIG_ENDIAN); -#if HOST_BIG_ENDIAN != TARGET_BIG_ENDIAN - /* Swap if non-host endianness or native (target) endianness */ - return (end == DEVICE_HOST_ENDIAN) ? 0 : MO_BSWAP; -#else + if (HOST_BIG_ENDIAN != target_words_bigendian()) { + /* Swap if non-host endianness or native (target) endianness */ + return (end == DEVICE_HOST_ENDIAN) ? 0 : MO_BSWAP; + } + const int non_host_endianness = DEVICE_LITTLE_ENDIAN ^ DEVICE_BIG_ENDIAN ^ DEVICE_HOST_ENDIAN; /* In this case, native (target) endianness needs no swap. */ return (end == non_host_endianness) ? MO_BSWAP : 0; -#endif } -#endif /* COMPILING_PER_TARGET */ /* * Inhibit technologies that require discarding of pages in RAM blocks, e.g.,
Will allow to make system/memory.c common later. Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> --- include/exec/memory.h | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)