On 25/10/16 23:25, David Gibson wrote:
> On Tue, Oct 25, 2016 at 06:01:41PM +1100, Alexey Kardashevskiy wrote:
>> On 24/10/16 15:59, David Gibson wrote:
>>> ide-test uses many explicit inb() / outb() operations for its IO, which
>>> means it's not portable to non-x86 platforms. This cleans it up to use
>>> the libqos PCI accessors instead.
>>>
>>> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
> [snip]
>
>>> -static void send_scsi_cdb_read10(uint64_t lba, int nblocks)
>>> +static void send_scsi_cdb_read10(QPCIDevice *dev, void *ide_base,
>>> + uint64_t lba, int nblocks)
>>> {
>>> Read10CDB pkt = { .padding = 0 };
>>> int i;
>>> @@ -670,7 +717,8 @@ static void send_scsi_cdb_read10(uint64_t lba, int
>>> nblocks)
>>>
>>> /* Send Packet */
>>> for (i = 0; i < sizeof(Read10CDB)/2; i++) {
>>> - outw(IDE_BASE + reg_data, cpu_to_le16(((uint16_t *)&pkt)[i]));
>>> + qpci_io_writew(dev, ide_base + reg_data,
>>> + le16_to_cpu(((uint16_t *)&pkt)[i]));
>>
>>
>> cpu_to_le16 -> le16_to_cpu conversion here and below (at the very end) is
>> not obvious. Right above this chunk the @pkt fields are initialized as BE:
>>
>> /* Construct SCSI CDB packet */
>> pkt.opcode = 0x28;
>> pkt.lba = cpu_to_be32(lba);
>> pkt.nblocks = cpu_to_be16(nblocks);
>>
>> outw() seems to be CPU-endian, and qpci_io_writew() as well, or not?
>
> outw() is guest CPU endian (which is stupid, but that's another
> matter). qpci_io_writew() is different - it is always LE, because PCI
> devices are always LE (well, ok, nearly always).
>
> So, yes, this is a bit confusing. Here's what's going on:
> * the SCSI standard uses BE, so that's what we put into the
> packet structure
> * We need to transfer the packet to the device as a bytestream - so
> no endianness conversions
> * But.. we do so in 16-bit chunks
> * .. and qpci_io_writew() is designed to take CPU values and write
> them out as LE - ie, it contains an implicit cpu_to_le16()dev->bus->pio_writew() calls outw() which calls qtest_outw() and qtest_sendf() where @value is a text - where does this implicit cpu_to_le16() happen? Or I am reading the code wrong? The other branch (for MMIO) in qpci_io_writew() calls cpu_to_le16() explicitly. I'd expect a function with a generic name as qpci_io_writew() to always take data in the some known and always the same endianness (LE in this case as it is PCI). In the chunk above we convert host-CPU-endian @lba to BE then treat it as LE when converting to CPU-endian and then expect qpci_io_writew() to do swapping again (or not - depends on BAR type - IO vs. MMIO - or conversion always happens?), this confuses me a lot. However, everybody else is happy so am I :) -- Alexey
signature.asc
Description: OpenPGP digital signature
