[RFC PATCH] hw/arm/armv7m: Expose and access System Control Space as little endian

Philippe Mathieu-Daudé posted 1 patch 1 year, 4 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20240924191932.49386-1-philmd@linaro.org
Maintainers: Peter Maydell <peter.maydell@linaro.org>
There is a newer version of this series
hw/arm/armv7m.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
[RFC PATCH] hw/arm/armv7m: Expose and access System Control Space as little endian
Posted by Philippe Mathieu-Daudé 1 year, 4 months ago
Per the Armv7-M Architecture Reference Manual (ARM DDI 0403E):

  The System Control Space (SCS, address range 0xE000E000 to
  0xE000EFFF) is a memory-mapped 4KB address space that provides
  32-bit registers for configuration, status reporting and control.
  All accesses to the SCS are little endian.

Expose the region as a little-endian one and force dispatched
accesses to also be in little endianness.

Fixes: d5d680cacc ("memory: Access MemoryRegion with endianness")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/armv7m.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 7c68525a9e..4a01b970e6 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -140,7 +140,7 @@ static MemTxResult v7m_sysreg_ns_write(void *opaque, hwaddr addr,
         /* S accesses to the alias act like NS accesses to the real region */
         attrs.secure = 0;
         return memory_region_dispatch_write(mr, addr, value,
-                                            size_memop(size) | MO_TE, attrs);
+                                            size_memop(size) | MO_LE, attrs);
     } else {
         /* NS attrs are RAZ/WI for privileged, and BusFault for user */
         if (attrs.user) {
@@ -160,7 +160,7 @@ static MemTxResult v7m_sysreg_ns_read(void *opaque, hwaddr addr,
         /* S accesses to the alias act like NS accesses to the real region */
         attrs.secure = 0;
         return memory_region_dispatch_read(mr, addr, data,
-                                           size_memop(size) | MO_TE, attrs);
+                                           size_memop(size) | MO_LE, attrs);
     } else {
         /* NS attrs are RAZ/WI for privileged, and BusFault for user */
         if (attrs.user) {
@@ -174,7 +174,7 @@ static MemTxResult v7m_sysreg_ns_read(void *opaque, hwaddr addr,
 static const MemoryRegionOps v7m_sysreg_ns_ops = {
     .read_with_attrs = v7m_sysreg_ns_read,
     .write_with_attrs = v7m_sysreg_ns_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
 static MemTxResult v7m_systick_write(void *opaque, hwaddr addr,
@@ -187,7 +187,7 @@ static MemTxResult v7m_systick_write(void *opaque, hwaddr addr,
     /* Direct the access to the correct systick */
     mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->systick[attrs.secure]), 0);
     return memory_region_dispatch_write(mr, addr, value,
-                                        size_memop(size) | MO_TE, attrs);
+                                        size_memop(size) | MO_LE, attrs);
 }
 
 static MemTxResult v7m_systick_read(void *opaque, hwaddr addr,
@@ -199,14 +199,14 @@ static MemTxResult v7m_systick_read(void *opaque, hwaddr addr,
 
     /* Direct the access to the correct systick */
     mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->systick[attrs.secure]), 0);
-    return memory_region_dispatch_read(mr, addr, data, size_memop(size) | MO_TE,
-                                       attrs);
+    return memory_region_dispatch_read(mr, addr, data,
+                                       size_memop(size) | MO_LE, attrs);
 }
 
 static const MemoryRegionOps v7m_systick_ops = {
     .read_with_attrs = v7m_systick_read,
     .write_with_attrs = v7m_systick_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
 /*
-- 
2.45.2


Re: [RFC PATCH] hw/arm/armv7m: Expose and access System Control Space as little endian
Posted by Peter Maydell 1 year, 4 months ago
On Tue, 24 Sept 2024 at 20:19, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Per the Armv7-M Architecture Reference Manual (ARM DDI 0403E):
>
>   The System Control Space (SCS, address range 0xE000E000 to
>   0xE000EFFF) is a memory-mapped 4KB address space that provides
>   32-bit registers for configuration, status reporting and control.
>   All accesses to the SCS are little endian.
>
> Expose the region as a little-endian one and force dispatched
> accesses to also be in little endianness.
>
> Fixes: d5d680cacc ("memory: Access MemoryRegion with endianness")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

What's the rationale for this change? All Arm system emulator
builds are TARGET_LITTLE_ENDIAN, so MO_TE and MO_LE have
identical behaviour, as do DEVICE_LITTLE_ENDIAN and
DEVICE_TARGET_ENDIAN.

thanks
-- PMM
Re: [RFC PATCH] hw/arm/armv7m: Expose and access System Control Space as little endian
Posted by Philippe Mathieu-Daudé 1 year, 2 months ago
On 25/9/24 13:04, Peter Maydell wrote:
> On Tue, 24 Sept 2024 at 20:19, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> Per the Armv7-M Architecture Reference Manual (ARM DDI 0403E):
>>
>>    The System Control Space (SCS, address range 0xE000E000 to
>>    0xE000EFFF) is a memory-mapped 4KB address space that provides
>>    32-bit registers for configuration, status reporting and control.
>>    All accesses to the SCS are little endian.
>>
>> Expose the region as a little-endian one and force dispatched
>> accesses to also be in little endianness.
>>
>> Fixes: d5d680cacc ("memory: Access MemoryRegion with endianness")
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> What's the rationale for this change? All Arm system emulator
> builds are TARGET_LITTLE_ENDIAN, so MO_TE and MO_LE have
> identical behaviour, as do DEVICE_LITTLE_ENDIAN and
> DEVICE_TARGET_ENDIAN.

When building a single heterogeneous binary, all device
models can be built. We want to remove MO_TE, either
using the proper endianness (like in this case) or use
both, one MemoryRegionOps per endianness, letting the
machine picking up the proper one.

I'll reword the description to be clearer.

Regards,

Phil.