On Wed, Jul 08, 2015 at 10:21:57PM -0500, Jonathan A. Kollasch wrote:
On Tue, Jul 07, 2015 at 05:06:03PM -0400, Kevin O'Connor wrote:
Thanks. Are you willing to respin a simpler version of the patch?
Attached.
I don't think the user visible drive descriptions or the debug strings should change. How about the patch below instead?
-Kevin
From 6a668b35e1691de8ebccf9c8b627c58f333cea7a Mon Sep 17 00:00:00 2001
From: Kevin O'Connor kevin@koconnor.net Date: Tue, 14 Jul 2015 15:12:25 -0400 Subject: [PATCH] ata: Make sure "chanid" is relative to PCI device for bootorder file
When specifying drives in the bootorder file, the "drive@x" parameter should be relative to the given PCI device and not relative to the total number of ATA controllers in the machine. This patch separates the tracking of "chanid" (channel number relative to a given PCI devices) from the "ataid" (channel number relative to the total number of ATA channels).
Reported-by: Jonathan A. Kollasch jakllsch@kollasch.net Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/block.c | 6 ++---- src/hw/ata.c | 25 +++++++++++++------------ src/hw/ata.h | 1 + 3 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/src/block.c b/src/block.c index 5e08663..a1054ac 100644 --- a/src/block.c +++ b/src/block.c @@ -74,10 +74,8 @@ get_translation(struct drive_s *drive) u8 type = drive->type; if (CONFIG_QEMU && type == DTYPE_ATA) { // Emulators pass in the translation info via nvram. - u8 ataid = drive->cntl_id; - u8 channel = ataid / 2; - u8 translation = rtc_read(CMOS_BIOS_DISKTRANSFLAG + channel/2); - translation >>= 2 * (ataid % 4); + u8 translation = rtc_read(CMOS_BIOS_DISKTRANSFLAG + drive->cntl_id/4); + translation >>= 2 * (drive->cntl_id % 4); translation &= 0x03; return translation; } diff --git a/src/hw/ata.c b/src/hw/ata.c index 3efc6ab..fbbbbc1 100644 --- a/src/hw/ata.c +++ b/src/hw/ata.c @@ -718,7 +718,7 @@ init_atadrive(struct atadrive_s *dummy, u16 *buffer) memset(adrive, 0, sizeof(*adrive)); adrive->chan_gf = dummy->chan_gf; adrive->slave = dummy->slave; - adrive->drive.cntl_id = adrive->chan_gf->chanid * 2 + dummy->slave; + adrive->drive.cntl_id = adrive->chan_gf->ataid * 2 + dummy->slave; adrive->drive.removable = (buffer[0] & 0x80) ? 1 : 0; return adrive; } @@ -743,7 +743,7 @@ init_drive_atapi(struct atadrive_s *dummy, u16 *buffer) char model[MAXMODEL+1]; char *desc = znprintf(MAXDESCSIZE , "DVD/CD [ata%d-%d: %s ATAPI-%d %s]" - , adrive->chan_gf->chanid, adrive->slave + , adrive->chan_gf->ataid, adrive->slave , ata_extract_model(model, MAXMODEL, buffer) , ata_extract_version(buffer) , (iscd ? "DVD/CD" : "Device")); @@ -795,7 +795,7 @@ init_drive_ata(struct atadrive_s *dummy, u16 *buffer) char model[MAXMODEL+1]; char *desc = znprintf(MAXDESCSIZE , "ata%d-%d: %s ATA-%d Hard-Disk (%u %ciBytes)" - , adrive->chan_gf->chanid, adrive->slave + , adrive->chan_gf->ataid, adrive->slave , ata_extract_model(model, MAXMODEL, buffer) , ata_extract_version(buffer) , (u32)adjsize, adjprefix); @@ -869,7 +869,7 @@ ata_detect(void *data) u8 sc = inb(iobase1+ATA_CB_SC); u8 sn = inb(iobase1+ATA_CB_SN); dprintf(6, "ata_detect ata%d-%d: sc=%x sn=%x dh=%x\n" - , chan_gf->chanid, slave, sc, sn, dh); + , chan_gf->ataid, slave, sc, sn, dh); if (sc != 0x55 || sn != 0xaa || dh != newdh) continue;
@@ -916,16 +916,17 @@ ata_detect(void *data)
// Initialize an ata controller and detect its drives. static void -init_controller(struct pci_device *pci, int irq +init_controller(struct pci_device *pci, int chanid, int irq , u32 port1, u32 port2, u32 master) { - static int chanid = 0; + static int ataid = 0; struct ata_channel_s *chan_gf = malloc_fseg(sizeof(*chan_gf)); if (!chan_gf) { warn_noalloc(); return; } - chan_gf->chanid = chanid++; + chan_gf->ataid = ataid++; + chan_gf->chanid = chanid; chan_gf->irq = irq; chan_gf->pci_bdf = pci ? pci->bdf : -1; chan_gf->pci_tmp = pci; @@ -933,7 +934,7 @@ init_controller(struct pci_device *pci, int irq chan_gf->iobase2 = port2; chan_gf->iomaster = master; dprintf(1, "ATA controller %d at %x/%x/%x (irq %d dev %x)\n" - , chanid, port1, port2, master, irq, chan_gf->pci_bdf); + , ataid, port1, port2, master, irq, chan_gf->pci_bdf); run_thread(ata_detect, chan_gf); }
@@ -969,7 +970,7 @@ init_pciata(struct pci_device *pci, u8 prog_if) port2 = PORT_ATA1_CTRL_BASE; irq = IRQ_ATA1; } - init_controller(pci, irq, port1, port2, master); + init_controller(pci, 0, irq, port1, port2, master);
if (prog_if & 4) { port1 = (pci_config_readl(bdf, PCI_BASE_ADDRESS_2) @@ -982,7 +983,7 @@ init_pciata(struct pci_device *pci, u8 prog_if) port2 = PORT_ATA2_CTRL_BASE; irq = IRQ_ATA2; } - init_controller(pci, irq, port1, port2, master ? master + 8 : 0); + init_controller(pci, 1, irq, port1, port2, master ? master + 8 : 0); }
static void @@ -1014,9 +1015,9 @@ ata_scan(void) if (CONFIG_QEMU && hlist_empty(&PCIDevices)) { // No PCI devices found - probably a QEMU "-M isapc" machine. // Try using ISA ports for ATA controllers. - init_controller(NULL, IRQ_ATA1 + init_controller(NULL, 0, IRQ_ATA1 , PORT_ATA1_CMD_BASE, PORT_ATA1_CTRL_BASE, 0); - init_controller(NULL, IRQ_ATA2 + init_controller(NULL, 1, IRQ_ATA2 , PORT_ATA2_CMD_BASE, PORT_ATA2_CTRL_BASE, 0); return; } diff --git a/src/hw/ata.h b/src/hw/ata.h index 65bfcb7..cd14e59 100644 --- a/src/hw/ata.h +++ b/src/hw/ata.h @@ -11,6 +11,7 @@ struct ata_channel_s { u16 iomaster; u8 irq; u8 chanid; + u8 ataid; int pci_bdf; struct pci_device *pci_tmp; };