Re: [Qemu-devel] [Qemu-ppc] [PATCH 4/4] tests: enable ohci/uhci/xhci tests on PPC64

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2139 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]