1. Patchew
  2. QEMU
  3. View series

[RFC PATCH] hw/intc: avoid byte swap fiddling in gicv3 its path

Alex Bennée posted 1 patch 2 weeks, 4 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260119120030.2593993-1-alex.bennee@linaro.org
Maintainers: Peter Maydell <peter.maydell@linaro.org>
hw/intc/arm_gicv3_its_common.c | 4 ++--
hw/intc/arm_gicv3_its_kvm.c    | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
[RFC PATCH] hw/intc: avoid byte swap fiddling in gicv3 its path
Posted by Alex Bennée 2 weeks, 4 days ago 
The GIC should always be a little-endian device as big-endian
behaviour is a function of the current CPU configuration not the
system as a whole. This allows us to keep the MSI data in plain host
order rather then potentially truncating with multiple byte swaps of
different sizes.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 hw/intc/arm_gicv3_its_common.c | 4 ++--
 hw/intc/arm_gicv3_its_kvm.c    | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/intc/arm_gicv3_its_common.c b/hw/intc/arm_gicv3_its_common.c
index e946e3fb87b..60a5abd8d3e 100644
--- a/hw/intc/arm_gicv3_its_common.c
+++ b/hw/intc/arm_gicv3_its_common.c
@@ -81,7 +81,7 @@ static MemTxResult gicv3_its_trans_write(void *opaque, hwaddr offset,
     if (offset == 0x0040 && ((size == 2) || (size == 4))) {
         GICv3ITSState *s = ARM_GICV3_ITS_COMMON(opaque);
         GICv3ITSCommonClass *c = ARM_GICV3_ITS_COMMON_GET_CLASS(s);
-        int ret = c->send_msi(s, le64_to_cpu(value), attrs.requester_id);
+        int ret = c->send_msi(s, value, attrs.requester_id);
 
         if (ret <= 0) {
             qemu_log_mask(LOG_GUEST_ERROR,
@@ -97,7 +97,7 @@ static MemTxResult gicv3_its_trans_write(void *opaque, hwaddr offset,
 static const MemoryRegionOps gicv3_its_trans_ops = {
     .read_with_attrs = gicv3_its_trans_read,
     .write_with_attrs = gicv3_its_trans_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
 void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps *ops,
diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
index ae12d41eee1..a8d6d4fb540 100644
--- a/hw/intc/arm_gicv3_its_kvm.c
+++ b/hw/intc/arm_gicv3_its_kvm.c
@@ -58,7 +58,7 @@ static int kvm_its_send_msi(GICv3ITSState *s, uint32_t value, uint16_t devid)
 
     msi.address_lo = extract64(s->gits_translater_gpa, 0, 32);
     msi.address_hi = extract64(s->gits_translater_gpa, 32, 32);
-    msi.data = le32_to_cpu(value);
+    msi.data = value;
     msi.flags = KVM_MSI_VALID_DEVID;
     msi.devid = devid;
     memset(msi.pad, 0, sizeof(msi.pad));
-- 
2.47.3
Re: [RFC PATCH] hw/intc: avoid byte swap fiddling in gicv3 its path
Posted by Peter Maydell 2 weeks, 4 days ago 
On Mon, 19 Jan 2026 at 12:00, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> The GIC should always be a little-endian device as big-endian
> behaviour is a function of the current CPU configuration not the
> system as a whole. This allows us to keep the MSI data in plain host
> order rather then potentially truncating with multiple byte swaps of
> different sizes.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  hw/intc/arm_gicv3_its_common.c | 4 ++--
>  hw/intc/arm_gicv3_its_kvm.c    | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/intc/arm_gicv3_its_common.c b/hw/intc/arm_gicv3_its_common.c
> index e946e3fb87b..60a5abd8d3e 100644
> --- a/hw/intc/arm_gicv3_its_common.c
> +++ b/hw/intc/arm_gicv3_its_common.c
> @@ -81,7 +81,7 @@ static MemTxResult gicv3_its_trans_write(void *opaque, hwaddr offset,
>      if (offset == 0x0040 && ((size == 2) || (size == 4))) {
>          GICv3ITSState *s = ARM_GICV3_ITS_COMMON(opaque);
>          GICv3ITSCommonClass *c = ARM_GICV3_ITS_COMMON_GET_CLASS(s);
> -        int ret = c->send_msi(s, le64_to_cpu(value), attrs.requester_id);
> +        int ret = c->send_msi(s, value, attrs.requester_id);
>
>          if (ret <= 0) {
>              qemu_log_mask(LOG_GUEST_ERROR,

> diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
> index ae12d41eee1..a8d6d4fb540 100644
> --- a/hw/intc/arm_gicv3_its_kvm.c
> +++ b/hw/intc/arm_gicv3_its_kvm.c
> @@ -58,7 +58,7 @@ static int kvm_its_send_msi(GICv3ITSState *s, uint32_t value, uint16_t devid)
>
>      msi.address_lo = extract64(s->gits_translater_gpa, 0, 32);
>      msi.address_hi = extract64(s->gits_translater_gpa, 32, 32);
> -    msi.data = le32_to_cpu(value);
> +    msi.data = value;
>      msi.flags = KVM_MSI_VALID_DEVID;
>      msi.devid = devid;
>      memset(msi.pad, 0, sizeof(msi.pad));

Having looked at this code a bit more carefully and figured out
that the send_msi method isn't calling core QEMU code but just
one bit of our ITS implementation talking to another, I agree
that these changes are correct:

 * the kernel KVM_SIGNAL_MSI ioctl expects the data field to
   be in host byte order
 * the send_msi() method is entirely up to us how to
   specify, and "host byte order" is the least surprising
   one for a function call
 * write methods of a MemoryRegionOps always get their
   data in host byte order

So we should write this up in the commit message. This is
technically a bug fix (albeit one which only affects anybody
adventurous enough to run KVM on big-endian AArch64 :-))
so we could probably cc: stable too.

I also noticed while looking through the code that we
only seem to implement the send_msi method on the KVM
ITS. This works because TCG passes in its own MemoryRegionOps
for the "translation" MemoryRegion. But that means all
this base class code that calls a send_msi method on
the subclass is used only by KVM. We could tidy this up by:
 * moving gicv3_its_trans_{read,write} from the base class
   to the KVM subclass
 * having gicv3_its_init_mmio() not have fallback code
   for tops == NULL
 * directly call kvm_its_send_msi() instead of indirecting
   through a send_msi method
 * remove the send_msi method pointer entirely

Second side note: kvm_irqchip_send_msi() in accel/kvm/kvm-all.c
also has a rather dodgy looking le32_to_cpu() call in it,
which is probably the inspiration for this one in the Arm code.
Luckily, we never use that function on Arm, and its only
callers are in always-LE-host targets. But all those callsites
seem to assume the msi.data field should be in host order
so this is probably something that somebody who cares about
x86 hosts might like to tidy up.

thanks
-- PMM
Re: [RFC PATCH] hw/intc: avoid byte swap fiddling in gicv3 its path
Posted by Peter Maydell 2 weeks, 4 days ago 
On Mon, 19 Jan 2026 at 12:00, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> The GIC should always be a little-endian device as big-endian
> behaviour is a function of the current CPU configuration not the
> system as a whole. This allows us to keep the MSI data in plain host
> order rather then potentially truncating with multiple byte swaps of
> different sizes.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  hw/intc/arm_gicv3_its_common.c | 4 ++--
>  hw/intc/arm_gicv3_its_kvm.c    | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/intc/arm_gicv3_its_common.c b/hw/intc/arm_gicv3_its_common.c
> index e946e3fb87b..60a5abd8d3e 100644
> --- a/hw/intc/arm_gicv3_its_common.c
> +++ b/hw/intc/arm_gicv3_its_common.c
> @@ -81,7 +81,7 @@ static MemTxResult gicv3_its_trans_write(void *opaque, hwaddr offset,
>      if (offset == 0x0040 && ((size == 2) || (size == 4))) {
>          GICv3ITSState *s = ARM_GICV3_ITS_COMMON(opaque);
>          GICv3ITSCommonClass *c = ARM_GICV3_ITS_COMMON_GET_CLASS(s);
> -        int ret = c->send_msi(s, le64_to_cpu(value), attrs.requester_id);
> +        int ret = c->send_msi(s, value, attrs.requester_id);

This change is either fixing a bug on big-endian hosts,
or breaking big-endian hosts. Which is it ?

>          if (ret <= 0) {
>              qemu_log_mask(LOG_GUEST_ERROR,
> @@ -97,7 +97,7 @@ static MemTxResult gicv3_its_trans_write(void *opaque, hwaddr offset,
>  static const MemoryRegionOps gicv3_its_trans_ops = {
>      .read_with_attrs = gicv3_its_trans_read,
>      .write_with_attrs = gicv3_its_trans_write,
> -    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .endianness = DEVICE_LITTLE_ENDIAN,

This change is fine, as all arm system binaries are LE, so
it is not a behavioural change.

>  };
>
>  void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps *ops,
> diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
> index ae12d41eee1..a8d6d4fb540 100644
> --- a/hw/intc/arm_gicv3_its_kvm.c
> +++ b/hw/intc/arm_gicv3_its_kvm.c
> @@ -58,7 +58,7 @@ static int kvm_its_send_msi(GICv3ITSState *s, uint32_t value, uint16_t devid)
>
>      msi.address_lo = extract64(s->gits_translater_gpa, 0, 32);
>      msi.address_hi = extract64(s->gits_translater_gpa, 32, 32);
> -    msi.data = le32_to_cpu(value);
> +    msi.data = value;
>      msi.flags = KVM_MSI_VALID_DEVID;
>      msi.devid = devid;
>      memset(msi.pad, 0, sizeof(msi.pad));

This is nominally in the same category as the first change,
with the exception that we know that any host with KVM is
going to be little-endian, so the bug being fixed or
introduced is only conceptual.

thanks
-- PMM
Re: [RFC PATCH] hw/intc: avoid byte swap fiddling in gicv3 its path
Posted by Philippe Mathieu-Daudé 2 weeks, 4 days ago 
On 19/1/26 13:00, Alex Bennée wrote:
> The GIC should always be a little-endian device as big-endian
> behaviour is a function of the current CPU configuration not the
> system as a whole. This allows us to keep the MSI data in plain host
> order rather then potentially truncating with multiple byte swaps of
> different sizes.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   hw/intc/arm_gicv3_its_common.c | 4 ++--
>   hw/intc/arm_gicv3_its_kvm.c    | 2 +-
>   2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/intc/arm_gicv3_its_common.c b/hw/intc/arm_gicv3_its_common.c
> index e946e3fb87b..60a5abd8d3e 100644
> --- a/hw/intc/arm_gicv3_its_common.c
> +++ b/hw/intc/arm_gicv3_its_common.c
> @@ -81,7 +81,7 @@ static MemTxResult gicv3_its_trans_write(void *opaque, hwaddr offset,
>       if (offset == 0x0040 && ((size == 2) || (size == 4))) {
>           GICv3ITSState *s = ARM_GICV3_ITS_COMMON(opaque);
>           GICv3ITSCommonClass *c = ARM_GICV3_ITS_COMMON_GET_CLASS(s);
> -        int ret = c->send_msi(s, le64_to_cpu(value), attrs.requester_id);
> +        int ret = c->send_msi(s, value, attrs.requester_id);
>   
>           if (ret <= 0) {
>               qemu_log_mask(LOG_GUEST_ERROR,
> @@ -97,7 +97,7 @@ static MemTxResult gicv3_its_trans_write(void *opaque, hwaddr offset,
>   static const MemoryRegionOps gicv3_its_trans_ops = {
>       .read_with_attrs = gicv3_its_trans_read,
>       .write_with_attrs = gicv3_its_trans_write,
> -    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
>   };
>   
>   void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps *ops,
> diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
> index ae12d41eee1..a8d6d4fb540 100644
> --- a/hw/intc/arm_gicv3_its_kvm.c
> +++ b/hw/intc/arm_gicv3_its_kvm.c
> @@ -58,7 +58,7 @@ static int kvm_its_send_msi(GICv3ITSState *s, uint32_t value, uint16_t devid)
>   
>       msi.address_lo = extract64(s->gits_translater_gpa, 0, 32);
>       msi.address_hi = extract64(s->gits_translater_gpa, 32, 32);
> -    msi.data = le32_to_cpu(value);
> +    msi.data = value;
>       msi.flags = KVM_MSI_VALID_DEVID;
>       msi.devid = devid;
>       memset(msi.pad, 0, sizeof(msi.pad));

Could we also clean the other GIC uses in the same patch?

$ git grep DEVICE_NATIVE_ENDIAN hw/intc/arm_gic*
hw/intc/arm_gic.c:2065:        .endianness = DEVICE_NATIVE_ENDIAN,
hw/intc/arm_gic.c:2070:        .endianness = DEVICE_NATIVE_ENDIAN,
hw/intc/arm_gic.c:2077:    .endianness = DEVICE_NATIVE_ENDIAN,
hw/intc/arm_gic.c:2084:        .endianness = DEVICE_NATIVE_ENDIAN,
hw/intc/arm_gic.c:2089:        .endianness = DEVICE_NATIVE_ENDIAN,
hw/intc/arm_gic.c:2096:    .endianness = DEVICE_NATIVE_ENDIAN,
hw/intc/arm_gicv3.c:420:        .endianness = DEVICE_NATIVE_ENDIAN,
hw/intc/arm_gicv3.c:429:        .endianness = DEVICE_NATIVE_ENDIAN,
hw/intc/arm_gicv3_its.c:1909:    .endianness = DEVICE_NATIVE_ENDIAN,
hw/intc/arm_gicv3_its.c:1919:    .endianness = DEVICE_NATIVE_ENDIAN,
hw/intc/arm_gicv3_its_common.c:100:    .endianness = DEVICE_NATIVE_ENDIAN,
Re: [RFC PATCH] hw/intc: avoid byte swap fiddling in gicv3 its path
Posted by Alex Bennée 2 weeks, 4 days ago 
Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> On 19/1/26 13:00, Alex Bennée wrote:
>> The GIC should always be a little-endian device as big-endian
>> behaviour is a function of the current CPU configuration not the
>> system as a whole. This allows us to keep the MSI data in plain host
>> order rather then potentially truncating with multiple byte swaps of
>> different sizes.
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>   hw/intc/arm_gicv3_its_common.c | 4 ++--
>>   hw/intc/arm_gicv3_its_kvm.c    | 2 +-
>>   2 files changed, 3 insertions(+), 3 deletions(-)
>> diff --git a/hw/intc/arm_gicv3_its_common.c
>> b/hw/intc/arm_gicv3_its_common.c
>> index e946e3fb87b..60a5abd8d3e 100644
>> --- a/hw/intc/arm_gicv3_its_common.c
>> +++ b/hw/intc/arm_gicv3_its_common.c
>> @@ -81,7 +81,7 @@ static MemTxResult gicv3_its_trans_write(void *opaque, hwaddr offset,
>>       if (offset == 0x0040 && ((size == 2) || (size == 4))) {
>>           GICv3ITSState *s = ARM_GICV3_ITS_COMMON(opaque);
>>           GICv3ITSCommonClass *c = ARM_GICV3_ITS_COMMON_GET_CLASS(s);
>> -        int ret = c->send_msi(s, le64_to_cpu(value), attrs.requester_id);
>> +        int ret = c->send_msi(s, value, attrs.requester_id);
>>             if (ret <= 0) {
>>               qemu_log_mask(LOG_GUEST_ERROR,
>> @@ -97,7 +97,7 @@ static MemTxResult gicv3_its_trans_write(void *opaque, hwaddr offset,
>>   static const MemoryRegionOps gicv3_its_trans_ops = {
>>       .read_with_attrs = gicv3_its_trans_read,
>>       .write_with_attrs = gicv3_its_trans_write,
>> -    .endianness = DEVICE_NATIVE_ENDIAN,
>> +    .endianness = DEVICE_LITTLE_ENDIAN,
>>   };
>>     void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps
>> *ops,
>> diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
>> index ae12d41eee1..a8d6d4fb540 100644
>> --- a/hw/intc/arm_gicv3_its_kvm.c
>> +++ b/hw/intc/arm_gicv3_its_kvm.c
>> @@ -58,7 +58,7 @@ static int kvm_its_send_msi(GICv3ITSState *s, uint32_t value, uint16_t devid)
>>         msi.address_lo = extract64(s->gits_translater_gpa, 0, 32);
>>       msi.address_hi = extract64(s->gits_translater_gpa, 32, 32);
>> -    msi.data = le32_to_cpu(value);
>> +    msi.data = value;
>>       msi.flags = KVM_MSI_VALID_DEVID;
>>       msi.devid = devid;
>>       memset(msi.pad, 0, sizeof(msi.pad));
>
> Could we also clean the other GIC uses in the same patch?
>
> $ git grep DEVICE_NATIVE_ENDIAN hw/intc/arm_gic*
> hw/intc/arm_gic.c:2065:        .endianness = DEVICE_NATIVE_ENDIAN,
> hw/intc/arm_gic.c:2070:        .endianness = DEVICE_NATIVE_ENDIAN,
> hw/intc/arm_gic.c:2077:    .endianness = DEVICE_NATIVE_ENDIAN,
> hw/intc/arm_gic.c:2084:        .endianness = DEVICE_NATIVE_ENDIAN,
> hw/intc/arm_gic.c:2089:        .endianness = DEVICE_NATIVE_ENDIAN,
> hw/intc/arm_gic.c:2096:    .endianness = DEVICE_NATIVE_ENDIAN,
> hw/intc/arm_gicv3.c:420:        .endianness = DEVICE_NATIVE_ENDIAN,
> hw/intc/arm_gicv3.c:429:        .endianness = DEVICE_NATIVE_ENDIAN,
> hw/intc/arm_gicv3_its.c:1909:    .endianness = DEVICE_NATIVE_ENDIAN,
> hw/intc/arm_gicv3_its.c:1919:    .endianness = DEVICE_NATIVE_ENDIAN,
> hw/intc/arm_gicv3_its_common.c:100:    .endianness = DEVICE_NATIVE_ENDIAN,

I did look to see if we where doing any byte swaps but I forgot to check
the .endianness fields. I can re-spin.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [RFC PATCH] hw/intc: avoid byte swap fiddling in gicv3 its path
Posted by Peter Maydell 2 weeks, 4 days ago 
On Mon, 19 Jan 2026 at 13:34, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>
> > On 19/1/26 13:00, Alex Bennée wrote:
> >> The GIC should always be a little-endian device as big-endian
> >> behaviour is a function of the current CPU configuration not the
> >> system as a whole. This allows us to keep the MSI data in plain host
> >> order rather then potentially truncating with multiple byte swaps of
> >> different sizes.
> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> >> ---
> >>   hw/intc/arm_gicv3_its_common.c | 4 ++--
> >>   hw/intc/arm_gicv3_its_kvm.c    | 2 +-
> >>   2 files changed, 3 insertions(+), 3 deletions(-)
> >> diff --git a/hw/intc/arm_gicv3_its_common.c
> >> b/hw/intc/arm_gicv3_its_common.c
> >> index e946e3fb87b..60a5abd8d3e 100644
> >> --- a/hw/intc/arm_gicv3_its_common.c
> >> +++ b/hw/intc/arm_gicv3_its_common.c
> >> @@ -81,7 +81,7 @@ static MemTxResult gicv3_its_trans_write(void *opaque, hwaddr offset,
> >>       if (offset == 0x0040 && ((size == 2) || (size == 4))) {
> >>           GICv3ITSState *s = ARM_GICV3_ITS_COMMON(opaque);
> >>           GICv3ITSCommonClass *c = ARM_GICV3_ITS_COMMON_GET_CLASS(s);
> >> -        int ret = c->send_msi(s, le64_to_cpu(value), attrs.requester_id);
> >> +        int ret = c->send_msi(s, value, attrs.requester_id);
> >>             if (ret <= 0) {
> >>               qemu_log_mask(LOG_GUEST_ERROR,
> >> @@ -97,7 +97,7 @@ static MemTxResult gicv3_its_trans_write(void *opaque, hwaddr offset,
> >>   static const MemoryRegionOps gicv3_its_trans_ops = {
> >>       .read_with_attrs = gicv3_its_trans_read,
> >>       .write_with_attrs = gicv3_its_trans_write,
> >> -    .endianness = DEVICE_NATIVE_ENDIAN,
> >> +    .endianness = DEVICE_LITTLE_ENDIAN,
> >>   };
> >>     void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps
> >> *ops,
> >> diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
> >> index ae12d41eee1..a8d6d4fb540 100644
> >> --- a/hw/intc/arm_gicv3_its_kvm.c
> >> +++ b/hw/intc/arm_gicv3_its_kvm.c
> >> @@ -58,7 +58,7 @@ static int kvm_its_send_msi(GICv3ITSState *s, uint32_t value, uint16_t devid)
> >>         msi.address_lo = extract64(s->gits_translater_gpa, 0, 32);
> >>       msi.address_hi = extract64(s->gits_translater_gpa, 32, 32);
> >> -    msi.data = le32_to_cpu(value);
> >> +    msi.data = value;
> >>       msi.flags = KVM_MSI_VALID_DEVID;
> >>       msi.devid = devid;
> >>       memset(msi.pad, 0, sizeof(msi.pad));
> >
> > Could we also clean the other GIC uses in the same patch?
> >
> > $ git grep DEVICE_NATIVE_ENDIAN hw/intc/arm_gic*
> > hw/intc/arm_gic.c:2065:        .endianness = DEVICE_NATIVE_ENDIAN,
> > hw/intc/arm_gic.c:2070:        .endianness = DEVICE_NATIVE_ENDIAN,
> > hw/intc/arm_gic.c:2077:    .endianness = DEVICE_NATIVE_ENDIAN,
> > hw/intc/arm_gic.c:2084:        .endianness = DEVICE_NATIVE_ENDIAN,
> > hw/intc/arm_gic.c:2089:        .endianness = DEVICE_NATIVE_ENDIAN,
> > hw/intc/arm_gic.c:2096:    .endianness = DEVICE_NATIVE_ENDIAN,
> > hw/intc/arm_gicv3.c:420:        .endianness = DEVICE_NATIVE_ENDIAN,
> > hw/intc/arm_gicv3.c:429:        .endianness = DEVICE_NATIVE_ENDIAN,
> > hw/intc/arm_gicv3_its.c:1909:    .endianness = DEVICE_NATIVE_ENDIAN,
> > hw/intc/arm_gicv3_its.c:1919:    .endianness = DEVICE_NATIVE_ENDIAN,
> > hw/intc/arm_gicv3_its_common.c:100:    .endianness = DEVICE_NATIVE_ENDIAN,
>
> I did look to see if we where doing any byte swaps but I forgot to check
> the .endianness fields. I can re-spin.

I think we should separate fixing up MemoryRegionOps
endianness fields (clearly safe no-behaviour-change commit)
from the le32_to_cpu/le64_to_cpu change (probably a bug fix
to be cc'd to stable, needs explanation of what it's doing).

thanks
-- PMM
Re: [RFC PATCH] hw/intc: avoid byte swap fiddling in gicv3 its path
Posted by Philippe Mathieu-Daudé 2 weeks, 4 days ago 
On 19/1/26 13:00, Alex Bennée wrote:
> The GIC should always be a little-endian device as big-endian
> behaviour is a function of the current CPU configuration not the
> system as a whole. This allows us to keep the MSI data in plain host
> order rather then potentially truncating with multiple byte swaps of
> different sizes.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   hw/intc/arm_gicv3_its_common.c | 4 ++--
>   hw/intc/arm_gicv3_its_kvm.c    | 2 +-
>   2 files changed, 3 insertions(+), 3 deletions(-)


>   static const MemoryRegionOps gicv3_its_trans_ops = {
>       .read_with_attrs = gicv3_its_trans_read,
>       .write_with_attrs = gicv3_its_trans_write,
> -    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .endianness = DEVICE_LITTLE_ENDIAN,

\o/

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

>   };

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.