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). Also WRT to what Zoltan is saying, I think it's safe to just add an extra swap, as long as it's of the correct size. The swap changes the combined action of ops->read and adjust_endianness() to have 1 or 2 swaps instead of 0 and 1, and that is the same as inverting the result of devend_memop(). The loops in access_with_adjusted_size() could be a problem but they do not matter here, because split accesses are never needed (impl.*_access_size are the same as valid.*_access_size). Thanks, Paolo Paolo
