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()
* So, we use le16_to_cpu() to cancel out that swap, so that we write
out the bytes in the correct order
If this were a Linux driver here, then we'd use a writew_rep() type
primitive, instead of writew(). The rep / streaming variants don't
byteswap exactly because they're designed for streaming out bytestream
data, not word values.
Really what we want here is a memwrite() type operation, but that
doesn't quite work because this is PIO, not MMIO, so on x86 it turns
into CPU ins and outs which don't have "streaming" variants.
> > }
> > }
> >
> > @@ -683,13 +731,17 @@ static void nsleep(int64_t nsecs)
> >
> > static uint8_t ide_wait_clear(uint8_t flag)
> > {
> > + QPCIDevice *dev;
> > + void *bmdma_base, *ide_base;
> > uint8_t data;
> > time_t st;
> >
> > + dev = get_pci_device(&bmdma_base, &ide_base);
> > +
> > /* Wait with a 5 second timeout */
> > time(&st);
> > while (true) {
> > - data = inb(IDE_BASE + reg_status);
> > + data = qpci_io_readb(dev, ide_base + reg_status);
> > if (!(data & flag)) {
> > return data;
> > }
> > @@ -723,6 +775,8 @@ static void ide_wait_intr(int irq)
> >
> > static void cdrom_pio_impl(int nblocks)
> > {
> > + QPCIDevice *dev;
> > + void *bmdma_base, *ide_base;
> > FILE *fh;
> > int patt_blocks = MAX(16, nblocks);
> > size_t patt_len = ATAPI_BLOCK_SIZE * patt_blocks;
> > @@ -741,13 +795,15 @@ static void cdrom_pio_impl(int nblocks)
> >
> > ide_test_start("-drive
> > if=none,file=%s,media=cdrom,format=raw,id=sr0,index=0 "
> > "-device ide-cd,drive=sr0,bus=ide.0", tmp_path);
> > + dev = get_pci_device(&bmdma_base, &ide_base);
> > qtest_irq_intercept_in(global_qtest, "ioapic");
> >
> > /* PACKET command on device 0 */
> > - outb(IDE_BASE + reg_device, 0);
> > - outb(IDE_BASE + reg_lba_middle, BYTE_COUNT_LIMIT & 0xFF);
> > - outb(IDE_BASE + reg_lba_high, (BYTE_COUNT_LIMIT >> 8 & 0xFF));
> > - outb(IDE_BASE + reg_command, CMD_PACKET);
> > + qpci_io_writeb(dev, ide_base + reg_device, 0);
> > + qpci_io_writeb(dev, ide_base + reg_lba_middle, BYTE_COUNT_LIMIT &
> > 0xFF);
> > + qpci_io_writeb(dev, ide_base + reg_lba_high,
> > + (BYTE_COUNT_LIMIT >> 8 & 0xFF));
> > + qpci_io_writeb(dev, ide_base + reg_command, CMD_PACKET);
> > /* HP0: Check_Status_A State */
> > nsleep(400);
> > data = ide_wait_clear(BSY);
> > @@ -756,7 +812,7 @@ static void cdrom_pio_impl(int nblocks)
> > assert_bit_clear(data, ERR | DF | BSY);
> >
> > /* SCSI CDB (READ10) -- read n*2048 bytes from block 0 */
> > - send_scsi_cdb_read10(0, nblocks);
> > + send_scsi_cdb_read10(dev, ide_base, 0, nblocks);
> >
> > /* Read data back: occurs in bursts of 'BYTE_COUNT_LIMIT' bytes.
> > * If BYTE_COUNT_LIMIT is odd, we transfer BYTE_COUNT_LIMIT - 1 bytes.
> > @@ -780,7 +836,8 @@ static void cdrom_pio_impl(int nblocks)
> >
> > /* HP4: Transfer_Data */
> > for (j = 0; j < MIN((limit / 2), rem); j++) {
> > - rx[offset + j] = le16_to_cpu(inw(IDE_BASE + reg_data));
> > + rx[offset + j] = cpu_to_le16(qpci_io_readw(dev,
> > + ide_base +
> > reg_data));
> > }
> > }
> >
> >
>
>
--
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
