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.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Reply via email to