On 27/09/2016 09:43, 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. 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()"?
I think the "&& host_big_endian" is a side effect of a wrong
byte-swapping done in pci-spapr.c. As we read the value with the
endianess of the target CPU, this should be not needed.
I'm going to rework this part and define some
target_leXX_to_cpu()/target_cpu_to_leXX() macros.
Laurent
