On Tue, Sep 27, 2016 at 09:43:27AM +0200, Laurent Vivier wrote: > > > On 27/09/2016 05:53, David Gibson wrote: > > On Mon, Sep 26, 2016 at 04:10:49PM +0200, Laurent Vivier wrote: > > >> void qusb_pci_init_one(QPCIBus *pcibus, struct qhc *hc, uint32_t devfn, > >> int bar) > >> { > >> hc->dev = qpci_device_find(pcibus, devfn); > >> @@ -31,6 +38,13 @@ void uhci_port_test(struct qhc *hc, int port, uint16_t > >> expect) > >> uint16_t value = qpci_io_readw(hc->dev, addr); > >> uint16_t mask = ~(UHCI_PORT_WRITE_CLEAR | UHCI_PORT_RSVD1); > >> > >> + if (qtest_big_endian() && host_big_endian) { > >> + /* little endian device on big endian guest > >> + * must be swapped on big endian host > >> + */ > >> + value = bswap16(value); > >> + } > >> + > > > > Hm.. should the qpci_io_*() helpers handle the endian conversion? > > I'm really wondering how to manage correctly this case (I've the same > kind of issue with virtio). > > The protocol between guest and test program reads/writes data using the > guest CPU endianess, so it works in the overall case.
Oh.. That seems bogus; maybe we should change that. IIUC the protocol is transferring whole word values as strings. In which case we should be transferring whole word values - endianness should only come into play if we're associating bytes in the value with addresses. So using (poorly defined) guest endianness is a design error here - we should use "host endianness"; really "no endianness". > But in this case, > hcd-uhci is a little-endian device (.endianness = DEVICE_LITTLE_ENDIAN) > on a big endian machine, so I think in the linux driver we should have a > "le16_to_cpu()". But in our case we can't use "le16_to_cpu()" because > endianess of the host cpu is not the same has the one of the guest CPU. > Perhaps I should add a "target_le16_to_cpu()"? > > Thanks, > Laurent > -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature