On Mon, 19 Jan 2026 at 13:34, Alex Bennée <[email protected]> wrote: > > Philippe Mathieu-Daudé <[email protected]> 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 <[email protected]> > >> --- > >> 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
