On 28/3/25 18:34, Paolo Bonzini wrote:
On Fri, Mar 28, 2025 at 3:16 PM BALATON Zoltan <bala...@eik.bme.hu> wrote:
It should be fine.  You should take into account:

- the endianness produced by pci_data_read/pci_data_write (always little
endian)

- the endianness expected by the guest (big endian under the conditions in
the patch)

- the endianness expected by memory.c (always little endian, as specified in
gt64120_pci_data_ops)

Because there is either zero or one mismatch, bswap32 is fine.

This may worth a comment but I'm still not convinced this works on big
endian host because I think pci_data_read returns val in host endianness
and if you want big endian then you only need to bswap on LE host not on
BE host. Was this tested on BE host and confirmed it works?

Looking again at the code, there is definitely one problem: since you have

+        .min_access_size = 1,
+        .max_access_size = 4,

the bswap can also be bswap16 if size == 2 (and bswap32 only if size == 4).

Per 'PCI LOCAL BUS SPECIFICATION, REV. 3.0':

'''
3.2.2.3.2. Software Generation of Configuration Transactions

Anytime a host bridge sees a full DWORD I/O write from the host
to CONFIG_ADDRESS, the bridge must latch the data into its
CONFIG_ADDRESS register. On full DWORD I/O reads to CONFIG_ADDRESS,
the bridge must return the data in CONFIG_ADDRESS. Any other types
of accesses to this address (non-DWORD) have no effect on CONFIG_ADDRESS and are executed as normal I/O transactions on the PCI bus. Therefore,
the only I/O Space consumed by this register is a DWORD at the given
address. I/O devices that share the same address but use BYTE or WORD
registers are not affected because their transactions will pass through
the host bridge unchanged.
'''

IIUC we don't need the bswap16.


Reply via email to