Palacios Public Git Repository

To checkout Palacios execute

  git clone http://v3vee.org/palacios/palacios.web/palacios.git
This will give you the master branch. You probably want the devel branch or one of the release branches. To switch to the devel branch, simply execute
  cd palacios
  git checkout --track -b devel origin/devel
The other branches are similar.


Cleanup and sanity-checking of OOB accesses and pointer-to-local issues (Coverity...
[palacios.git] / palacios / src / devices / ide.c
index e0da650..e9a90a8 100644 (file)
@@ -119,7 +119,7 @@ struct ide_hd_state {
      * for multiple sector ops this equals mult_sector_num
      * for standard ops this equals 1
      */
-    uint32_t cur_sector_num;
+    uint64_t cur_sector_num;
 };
 
 struct ide_drive {
@@ -137,11 +137,11 @@ struct ide_drive {
     char model[41];
 
     // Where we are in the data transfer
-    uint32_t transfer_index;
+    uint64_t transfer_index;
 
     // the length of a transfer
     // calculated for easy access
-    uint32_t transfer_length;
+    uint64_t transfer_length;
 
     uint64_t current_lba;
 
@@ -153,13 +153,25 @@ struct ide_drive {
     uint32_t num_heads;
     uint32_t num_sectors;
 
+
+    struct lba48_state {
+       // all start at zero
+       uint64_t lba;                  
+       uint16_t sector_count;            // for LBA48
+       uint8_t  sector_count_state;      // two step write to 1f2/172 (high first)
+       uint8_t  lba41_state;             // two step write to 1f3
+        uint8_t  lba52_state;             // two step write to 1f4
+        uint8_t  lba63_state;             // two step write to 15
+    } lba48;
+
     void * private_data;
     
     union {
-       uint8_t sector_count;             // 0x1f2,0x172
-       struct atapi_irq_flags irq_flags;
+       uint8_t sector_count;             // 0x1f2,0x172  (ATA)
+       struct atapi_irq_flags irq_flags; // (ATAPI ONLY)
     } __attribute__((packed));
 
+
     union {
        uint8_t sector_num;               // 0x1f3,0x173
        uint8_t lba0;
@@ -259,6 +271,18 @@ static inline uint32_t le_to_be_32(const uint32_t val) {
 }
 
 
+static inline int is_lba28(struct ide_channel * channel) {
+    return channel->drive_head.lba_mode && channel->drive_head.rsvd1 && channel->drive_head.rsvd2;
+}
+
+static inline int is_lba48(struct ide_channel * channel) {
+    return channel->drive_head.lba_mode && !channel->drive_head.rsvd1 && !channel->drive_head.rsvd2;
+}
+
+static inline int is_chs(struct ide_channel * channel) {
+    return !channel->drive_head.lba_mode;
+}
+
 static inline int get_channel_index(ushort_t port) {
     if (((port & 0xfff8) == 0x1f0) ||
        ((port & 0xfffe) == 0x3f6) || 
@@ -275,7 +299,12 @@ static inline int get_channel_index(ushort_t port) {
 
 static inline struct ide_channel * get_selected_channel(struct ide_internal * ide, ushort_t port) {
     int channel_idx = get_channel_index(port);    
-    return &(ide->channels[channel_idx]);
+    if (channel_idx >= 0) { 
+       return &(ide->channels[channel_idx]);
+    } else {
+       PrintError(VM_NONE,VCORE_NONE,"ide: Cannot Determine Selected Channel\n");
+       return 0;
+    }
 }
 
 static inline struct ide_drive * get_selected_drive(struct ide_channel * channel) {
@@ -283,19 +312,18 @@ static inline struct ide_drive * get_selected_drive(struct ide_channel * channel
 }
 
 
-static inline int is_lba_enabled(struct ide_channel * channel) {
-    return channel->drive_head.lba_mode;
-}
 
 
 /* Drive Commands */
 static void ide_raise_irq(struct ide_internal * ide, struct ide_channel * channel) {
     if (channel->ctrl_reg.irq_disable == 0) {
 
-       //PrintError(info->vm_info, info, "Raising IDE Interrupt %d\n", channel->irq);
+       PrintDebug(ide->vm,VCORE_NONE, "Raising IDE Interrupt %d\n", channel->irq);
 
         channel->dma_status.int_gen = 1;
         v3_raise_irq(ide->vm, channel->irq);
+    } else {
+       PrintDebug(ide->vm,VCORE_NONE, "IDE Interrupt %d cannot be raised as irq is disabled on channel\n",channel->irq);
     }
 }
 
@@ -331,7 +359,7 @@ static void channel_reset(struct ide_channel * channel) {
     channel->error_reg.val = 0x01;
 
     // clear commands
-    channel->cmd_reg = 0x00;
+    channel->cmd_reg = 0;  // NOP
 
     channel->ctrl_reg.irq_disable = 0;
 }
@@ -348,6 +376,9 @@ static void channel_reset_complete(struct ide_channel * channel) {
 
 
 static void ide_abort_command(struct ide_internal * ide, struct ide_channel * channel) {
+
+    PrintDebug(VM_NONE,VCORE_NONE,"Aborting IDE Command\n");
+
     channel->status.val = 0x41; // Error + ready
     channel->error_reg.val = 0x04; // No idea...
 
@@ -375,7 +406,7 @@ static void print_prd_table(struct ide_internal * ide, struct ide_channel * chan
 
     while (1) {
        uint32_t prd_entry_addr = channel->dma_prd_addr + (sizeof(struct ide_dma_prd) * index);
-       int ret;
+       int ret = 0;
 
        ret = v3_read_gpa_memory(&(ide->vm->cores[0]), prd_entry_addr, sizeof(struct ide_dma_prd), (void *)&prd_entry);
        
@@ -481,7 +512,11 @@ static int dma_read(struct guest_info * core, struct ide_internal * ide, struct
                    cmd_ret = v3_write_gpa_memory(core, prd_entry.base_addr + prd_offset, 
                                                  bytes_to_write, drive->data_buf); 
 
-                   // check cmd_ret
+                   if (cmd_ret!=bytes_to_write) { 
+                       PrintError(core->vm_info, core, "Failed to write data to memory\n");
+                       return -1;
+                   }
+
 
 
                    bytes_to_write = 0;
@@ -534,7 +569,7 @@ static int dma_read(struct guest_info * core, struct ide_internal * ide, struct
            if (atapi_cmd_is_data_op(drive->cd_state.atapi_cmd)) {
                if (drive->transfer_index % ATAPI_BLOCK_SIZE) {
                    PrintError(core->vm_info, core, "We currently don't handle ATAPI BLOCKS that span PRD descriptors\n");
-                   PrintError(core->vm_info, core, "transfer_index=%d, transfer_length=%d\n", 
+                   PrintError(core->vm_info, core, "transfer_index=%llu, transfer_length=%llu\n", 
                               drive->transfer_index, drive->transfer_length);
                    return -1;
                }
@@ -651,7 +686,7 @@ static int dma_write(struct guest_info * core, struct ide_internal * ide, struct
 
        if ((prd_entry.end_of_table == 1) && (bytes_left > 0)) {
            PrintError(core->vm_info, core, "DMA table not large enough for data transfer...\n");
-           PrintError(core->vm_info, core, "\t(bytes_left=%u) (transfer_length=%u)...\n", 
+           PrintError(core->vm_info, core, "\t(bytes_left=%u) (transfer_length=%llu)...\n", 
                       bytes_left, drive->transfer_length);
            PrintError(core->vm_info, core, "PRD Addr: %x, PRD Len: %d, EOT: %d\n", 
                       prd_entry.base_addr, prd_entry.size, prd_entry.end_of_table);
@@ -688,6 +723,14 @@ static int dma_write(struct guest_info * core, struct ide_internal * ide, struct
 
 #define DMA_CHANNEL_FLAG  0x08
 
+/*
+  Note that DMA model is as follows:
+
+    1. Write the PRD pointer to the busmaster (DMA engine)
+    2. Start the transfer on the device
+    3. Tell the busmaster to start shoveling data (active DMA)
+*/
+
 static int write_dma_port(struct guest_info * core, ushort_t port, void * src, uint_t length, void * private_data) {
     struct ide_internal * ide = (struct ide_internal *)private_data;
     uint16_t port_offset = port & (DMA_CHANNEL_FLAG - 1);
@@ -700,40 +743,48 @@ static int write_dma_port(struct guest_info * core, ushort_t port, void * src, u
     switch (port_offset) {
        case DMA_CMD_PORT:
            channel->dma_cmd.val = *(uint8_t *)src;
+           
+           PrintDebug(core->vm_info, core, "IDE: dma command write:  0x%x\n", channel->dma_cmd.val);
 
            if (channel->dma_cmd.start == 0) {
                channel->dma_tbl_index = 0;
            } else {
+               // Launch DMA operation, interrupt at end
+
                channel->dma_status.active = 1;
 
                if (channel->dma_cmd.read == 1) {
-                   // DMA Read
+                   // DMA Read the whole thing - dma_read will raise irq
                    if (dma_read(core, ide, channel) == -1) {
                        PrintError(core->vm_info, core, "Failed DMA Read\n");
                        return -1;
                    }
                } else {
-                   // DMA write
+                   // DMA write the whole thing - dma_write will raiase irw
                    if (dma_write(core, ide, channel) == -1) {
                        PrintError(core->vm_info, core, "Failed DMA Write\n");
                        return -1;
                    }
                }
-
-               channel->dma_cmd.val &= 0x09;
+               
+               // DMA complete
+               // Note that guest cannot abort a DMA transfer
+               channel->dma_cmd.start = 0;
            }
 
            break;
            
        case DMA_STATUS_PORT: {
+           // This is intended to clear status
+
            uint8_t val = *(uint8_t *)src;
 
            if (length != 1) {
-               PrintError(core->vm_info, core, "Invalid read length for DMA status port\n");
+               PrintError(core->vm_info, core, "Invalid write length for DMA status port\n");
                return -1;
            }
 
-           // weirdness
+           // but preserve certain bits
            channel->dma_status.val = ((val & 0x60) | 
                                       (channel->dma_status.val & 0x01) |
                                       (channel->dma_status.val & ~val & 0x06));
@@ -808,7 +859,7 @@ static int write_cmd_port(struct guest_info * core, ushort_t port, void * src, u
     
     switch (channel->cmd_reg) {
 
-       case 0xa1: // ATAPI Identify Device Packet
+       case ATA_PIDENTIFY: // ATAPI Identify Device Packet (CDROM)
            if (drive->drive_type != BLOCK_CDROM) {
                drive_reset(drive);
 
@@ -824,7 +875,8 @@ static int write_cmd_port(struct guest_info * core, ushort_t port, void * src, u
                ide_raise_irq(ide, channel);
            }
            break;
-       case 0xec: // Identify Device
+
+       case ATA_IDENTIFY: // Identify Device
            if (drive->drive_type != BLOCK_DISK) {
                drive_reset(drive);
 
@@ -840,7 +892,7 @@ static int write_cmd_port(struct guest_info * core, ushort_t port, void * src, u
            }
            break;
 
-       case 0xa0: // ATAPI Command Packet
+       case ATA_PACKETCMD: // ATAPI Command Packet (CDROM)
            if (drive->drive_type != BLOCK_CDROM) {
                ide_abort_command(ide, channel);
            }
@@ -858,87 +910,99 @@ static int write_cmd_port(struct guest_info * core, ushort_t port, void * src, u
 
            break;
 
-       case 0x20: // Read Sectors with Retry
-       case 0x21: // Read Sectors without Retry
-           drive->hd_state.cur_sector_num = 1;
+       case ATA_READ:      // Read Sectors with Retry
+       case ATA_READ_ONCE: // Read Sectors without Retry
+       case ATA_MULTREAD:  // Read multiple sectors per ire
+       case ATA_READ_EXT:  // Read Sectors Extended (LBA48)
+
+           if (channel->cmd_reg==ATA_MULTREAD) { 
+               drive->hd_state.cur_sector_num = drive->hd_state.mult_sector_num;
+           } else {
+               drive->hd_state.cur_sector_num = 1;
+           }
 
            if (ata_read_sectors(ide, channel) == -1) {
                PrintError(core->vm_info, core, "Error reading sectors\n");
-               return -1;
+               ide_abort_command(ide,channel);
            }
            break;
 
-       case 0x24: // Read Sectors Extended
-           drive->hd_state.cur_sector_num = 1;
+       case ATA_WRITE:            // Write Sector with retry
+       case ATA_WRITE_ONCE:       // Write Sector without retry
+       case ATA_MULTWRITE:        // Write multiple sectors per irq
+       case ATA_WRITE_EXT:        // Write Sectors Extended (LBA48)
 
-           if (ata_read_sectors_ext(ide, channel) == -1) {
-               PrintError(core->vm_info, core, "Error reading extended sectors\n");
-               return -1;
+           if (channel->cmd_reg==ATA_MULTWRITE) { 
+               drive->hd_state.cur_sector_num = drive->hd_state.mult_sector_num;
+           } else {
+               drive->hd_state.cur_sector_num = 1;
+           }
+
+           if (ata_write_sectors(ide, channel) == -1) {
+               PrintError(core->vm_info, core, "Error writing sectors\n");
+               ide_abort_command(ide,channel);
            }
            break;
 
-       case 0xc8: // Read DMA with retry
-       case 0xc9: { // Read DMA
-           uint32_t sect_cnt = (drive->sector_count == 0) ? 256 : drive->sector_count;
+       case ATA_READDMA:            // Read DMA with retry
+       case ATA_READDMA_ONCE:       // Read DMA without retry
+       case ATA_READDMA_EXT:      { // Read DMA (LBA48)
+           uint64_t sect_cnt;
 
-           if (ata_get_lba(ide, channel, &(drive->current_lba)) == -1) {
+           if (ata_get_lba_and_size(ide, channel, &(drive->current_lba), &sect_cnt) == -1) {
+                PrintError(core->vm_info, core, "Error getting LBA for DMA READ\n");
                ide_abort_command(ide, channel);
-               return 0;
+               return length;
            }
            
-           drive->hd_state.cur_sector_num = 1;
+           drive->hd_state.cur_sector_num = 1;  // Not used for DMA
            
            drive->transfer_length = sect_cnt * HD_SECTOR_SIZE;
            drive->transfer_index = 0;
 
-           if (channel->dma_status.active == 1) {
-               // DMA Read
-               if (dma_read(core, ide, channel) == -1) {
-                   PrintError(core->vm_info, core, "Failed DMA Read\n");
-                   return -1;
-               }
-           }
+           // Now we wait for the transfer to be intiated by flipping the 
+           // bus-master start bit
            break;
        }
 
-       case 0xca: { // Write DMA
-           uint32_t sect_cnt = (drive->sector_count == 0) ? 256 : drive->sector_count;
+       case ATA_WRITEDMA:        // Write DMA with retry
+       case ATA_WRITEDMA_ONCE:   // Write DMA without retry
+       case ATA_WRITEDMA_EXT:  { // Write DMA (LBA48)
+
+           uint64_t sect_cnt;
 
-           if (ata_get_lba(ide, channel, &(drive->current_lba)) == -1) {
+           if (ata_get_lba_and_size(ide, channel, &(drive->current_lba),&sect_cnt) == -1) {
+               PrintError(core->vm_info,core,"Cannot get lba\n");
                ide_abort_command(ide, channel);
-               return 0;
+               return length;
            }
 
-           drive->hd_state.cur_sector_num = 1;
+           drive->hd_state.cur_sector_num = 1;  // Not used for DMA
 
            drive->transfer_length = sect_cnt * HD_SECTOR_SIZE;
            drive->transfer_index = 0;
 
-           if (channel->dma_status.active == 1) {
-               // DMA Write
-               if (dma_write(core, ide, channel) == -1) {
-                   PrintError(core->vm_info, core, "Failed DMA Write\n");
-                   return -1;
-               }
-           }
+           // Now we wait for the transfer to be intiated by flipping the 
+           // bus-master start bit
            break;
        }
-       case 0xe0: // Standby Now 1
-       case 0xe1: // Set Idle Immediate
-       case 0xe2: // Standby
-       case 0xe3: // Set Idle 1
-       case 0xe6: // Sleep Now 1
-       case 0x94: // Standby Now 2
-       case 0x95: // Idle Immediate (CFA)
-       case 0x96: // Standby 2
-       case 0x97: // Set idle 2
-       case 0x99: // Sleep Now 2
+
+       case ATA_STANDBYNOW1: // Standby Now 1
+       case ATA_IDLEIMMEDIATE: // Set Idle Immediate
+       case ATA_STANDBY: // Standby
+       case ATA_SETIDLE1: // Set Idle 1
+       case ATA_SLEEPNOW1: // Sleep Now 1
+       case ATA_STANDBYNOW2: // Standby Now 2
+       case ATA_IDLEIMMEDIATE2: // Idle Immediate (CFA)
+       case ATA_STANDBY2: // Standby 2
+       case ATA_SETIDLE2: // Set idle 2
+       case ATA_SLEEPNOW2: // Sleep Now 2
            channel->status.val = 0;
            channel->status.ready = 1;
            ide_raise_irq(ide, channel);
            break;
 
-       case 0xef: // Set Features
+       case ATA_SETFEATURES: // Set Features
            // Prior to this the features register has been written to. 
            // This command tells the drive to check if the new value is supported (the value is drive specific)
            // Common is that bit0=DMA enable
@@ -955,24 +1019,22 @@ static int write_cmd_port(struct guest_info * core, ushort_t port, void * src, u
            ide_raise_irq(ide, channel);
            break;
 
-       case 0x91:  // Initialize Drive Parameters
-       case 0x10:  // recalibrate?
+       case ATA_SPECIFY:  // Initialize Drive Parameters
+       case ATA_RECAL:  // recalibrate?
            channel->status.error = 0;
            channel->status.ready = 1;
            channel->status.seek_complete = 1;
            ide_raise_irq(ide, channel);
            break;
-       case 0xc6: { // Set multiple mode (IDE Block mode) 
-           // This makes the drive transfer multiple sectors before generating an interrupt
-           uint32_t tmp_sect_num = drive->sector_num; // GCC SUCKS
 
-           if (tmp_sect_num > MAX_MULT_SECTORS) {
-               ide_abort_command(ide, channel);
-               break;
-           }
+       case ATA_SETMULT: { // Set multiple mode (IDE Block mode) 
+           // This makes the drive transfer multiple sectors before generating an interrupt
 
            if (drive->sector_count == 0) {
+               PrintError(core->vm_info,core,"Attempt to set multiple to zero\n");
                drive->hd_state.mult_sector_num= 1;
+               ide_abort_command(ide,channel);
+               break;
            } else {
                drive->hd_state.mult_sector_num = drive->sector_count;
            }
@@ -985,7 +1047,7 @@ static int write_cmd_port(struct guest_info * core, ushort_t port, void * src, u
            break;
        }
 
-       case 0x08: // Reset Device
+       case ATA_DEVICE_RESET: // Reset Device
            drive_reset(drive);
            channel->error_reg.val = 0x01;
            channel->status.busy = 0;
@@ -995,7 +1057,7 @@ static int write_cmd_port(struct guest_info * core, ushort_t port, void * src, u
            channel->status.error = 0;
            break;
 
-       case 0xe5: // Check power mode
+       case ATA_CHECKPOWERMODE1: // Check power mode
            drive->sector_count = 0xff; /* 0x00=standby, 0x80=idle, 0xff=active or idle */
            channel->status.busy = 0;
            channel->status.ready = 1;
@@ -1004,66 +1066,43 @@ static int write_cmd_port(struct guest_info * core, ushort_t port, void * src, u
            channel->status.error = 0;
            break;
 
-       case 0xc4:  // read multiple sectors
-           drive->hd_state.cur_sector_num = drive->hd_state.mult_sector_num;
        default:
            PrintError(core->vm_info, core, "Unimplemented IDE command (%x)\n", channel->cmd_reg);
-           return -1;
+           ide_abort_command(ide, channel);
+           break;
     }
 
     return length;
 }
 
 
-static int write_data_port(struct guest_info * core, ushort_t port, void * src, uint_t length, void * priv_data) {
-    struct ide_internal * ide = priv_data;
-    struct ide_channel * channel = get_selected_channel(ide, port);
-    struct ide_drive * drive = get_selected_drive(channel);
-
-    //    PrintDebug(core->vm_info, core, "IDE: Writing Data Port %x (val=%x, len=%d)\n", 
-    //        port, *(uint32_t *)src, length);
-    
-    memcpy(drive->data_buf + drive->transfer_index, src, length);    
-    drive->transfer_index += length;
-
-    // Transfer is complete, dispatch the command
-    if (drive->transfer_index >= drive->transfer_length) {
-       switch (channel->cmd_reg) {
-           case 0x30: // Write Sectors
-               PrintError(core->vm_info, core, "Writing Data not yet implemented\n");
-               return -1;
-               
-           case 0xa0: // ATAPI packet command
-               if (atapi_handle_packet(core, ide, channel) == -1) {
-                   PrintError(core->vm_info, core, "Error handling ATAPI packet\n");
-                   return -1;
-               }
-               break;
-           default:
-               PrintError(core->vm_info, core, "Unhandld IDE Command %x\n", channel->cmd_reg);
-               return -1;
-       }
-    }
-
-    return length;
-}
 
 
-static int read_hd_data(uint8_t * dst, uint_t length, struct ide_internal * ide, struct ide_channel * channel) {
+static int read_hd_data(uint8_t * dst, uint64_t length, struct ide_internal * ide, struct ide_channel * channel) {
     struct ide_drive * drive = get_selected_drive(channel);
-    int data_offset = drive->transfer_index % HD_SECTOR_SIZE;
+    uint64_t data_offset = drive->transfer_index % HD_SECTOR_SIZE;
 
 
+    PrintDebug(VM_NONE,VCORE_NONE, "Read HD data:  transfer_index %llu transfer length %llu current sector numer %llu\n",
+              drive->transfer_index, drive->transfer_length, 
+              drive->hd_state.cur_sector_num);
 
-    if (drive->transfer_index >= drive->transfer_length) {
-       PrintError(VM_NONE, VCORE_NONE, "Buffer overrun... (xfer_len=%d) (cur_idx=%x) (post_idx=%d)\n",
+    if (drive->transfer_index >= drive->transfer_length && drive->transfer_index>=DATA_BUFFER_SIZE) {
+       PrintError(VM_NONE, VCORE_NONE, "Buffer overrun... (xfer_len=%llu) (cur_idx=%llu) (post_idx=%llu)\n",
                   drive->transfer_length, drive->transfer_index,
                   drive->transfer_index + length);
        return -1;
     }
 
-    
+
+    if (data_offset + length > HD_SECTOR_SIZE) { 
+       PrintError(VM_NONE,VCORE_NONE,"Read spans sectors (data_offset=%llu length=%llu)!\n",data_offset,length);
+    }
+   
+    // For index==0, the read has been done in ata_read_sectors
     if ((data_offset == 0) && (drive->transfer_index > 0)) {
+       // advance to next sector and read it
+       
        drive->current_lba++;
 
        if (ata_read(ide, channel, drive->data_buf, 1) == -1) {
@@ -1092,22 +1131,15 @@ static int read_hd_data(uint8_t * dst, uint_t length, struct ide_internal * ide,
        (drive->transfer_index == drive->transfer_length)) {
        if (drive->transfer_index < drive->transfer_length) {
            // An increment is complete, but there is still more data to be transferred...
-           PrintDebug(VM_NONE, VCORE_NONE, "Integral Complete, still transferring more sectors\n");
+           PrintDebug(VM_NONE, VCORE_NONE, "Increment Complete, still transferring more sectors\n");
            channel->status.data_req = 1;
-
-           drive->irq_flags.c_d = 0;
        } else {
            PrintDebug(VM_NONE, VCORE_NONE, "Final Sector Transferred\n");
            // This was the final read of the request
            channel->status.data_req = 0;
-
-           
-           drive->irq_flags.c_d = 1;
-           drive->irq_flags.rel = 0;
        }
 
        channel->status.ready = 1;
-       drive->irq_flags.io_dir = 1;
        channel->status.busy = 0;
 
        ide_raise_irq(ide, channel);
@@ -1117,22 +1149,88 @@ static int read_hd_data(uint8_t * dst, uint_t length, struct ide_internal * ide,
     return length;
 }
 
+static int write_hd_data(uint8_t * src, uint64_t length, struct ide_internal * ide, struct ide_channel * channel) {
+    struct ide_drive * drive = get_selected_drive(channel);
+    uint64_t data_offset = drive->transfer_index % HD_SECTOR_SIZE;
+
+
+    PrintDebug(VM_NONE,VCORE_NONE, "Write HD data:  transfer_index %llu transfer length %llu current sector numer %llu\n",
+              drive->transfer_index, drive->transfer_length, 
+              drive->hd_state.cur_sector_num);
+
+    if (drive->transfer_index >= drive->transfer_length) {
+       PrintError(VM_NONE, VCORE_NONE, "Buffer overrun... (xfer_len=%llu) (cur_idx=%llu) (post_idx=%llu)\n",
+                  drive->transfer_length, drive->transfer_index,
+                  drive->transfer_index + length);
+       return -1;
+    }
+
+    if (data_offset + length > HD_SECTOR_SIZE) { 
+       PrintError(VM_NONE,VCORE_NONE,"Write spans sectors (data_offset=%llu length=%llu)!\n",data_offset,length);
+    }
+
+    // Copy data into our buffer - there will be room due to
+    // (a) the ata_write test below is flushing sectors
+    // (b) if we somehow get a sector-stradling write (an error), this will
+    //     be OK since the buffer itself is >1 sector in memory
+    memcpy(drive->data_buf + data_offset, src, length);
+
+    drive->transfer_index += length;
+
+    if ((data_offset+length) >= HD_SECTOR_SIZE) {
+       // Write out the sector we just finished
+       if (ata_write(ide, channel, drive->data_buf, 1) == -1) {
+           PrintError(VM_NONE, VCORE_NONE, "Could not write next disk sector\n");
+           return -1;
+       }
+
+       // go onto next sector
+       drive->current_lba++;
+    }
+
+    /* This is the trigger for interrupt injection.
+     * For write single sector commands we interrupt after every sector
+     * For multi sector reads we interrupt only at end of the cluster size (mult_sector_num)
+     * cur_sector_num is configured depending on the operation we are currently running
+     * We also trigger an interrupt if this is the last byte to transfer, regardless of sector count
+     */
+    if (((drive->transfer_index % (HD_SECTOR_SIZE * drive->hd_state.cur_sector_num)) == 0) || 
+       (drive->transfer_index == drive->transfer_length)) {
+       if (drive->transfer_index < drive->transfer_length) {
+           // An increment is complete, but there is still more data to be transferred...
+           PrintDebug(VM_NONE, VCORE_NONE, "Increment Complete, still transferring more sectors\n");
+           channel->status.data_req = 1;
+       } else {
+           PrintDebug(VM_NONE, VCORE_NONE, "Final Sector Transferred\n");
+           // This was the final read of the request
+           channel->status.data_req = 0;
+       }
+
+       channel->status.ready = 1;
+       channel->status.busy = 0;
+
+       ide_raise_irq(ide, channel);
+    }
+
+    return length;
+}
+
 
 
-static int read_cd_data(uint8_t * dst, uint_t length, struct ide_internal * ide, struct ide_channel * channel) {
+static int read_cd_data(uint8_t * dst, uint64_t length, struct ide_internal * ide, struct ide_channel * channel) {
     struct ide_drive * drive = get_selected_drive(channel);
-    int data_offset = drive->transfer_index % ATAPI_BLOCK_SIZE;
+    uint64_t data_offset = drive->transfer_index % ATAPI_BLOCK_SIZE;
     //  int req_offset = drive->transfer_index % drive->req_len;
     
     if (drive->cd_state.atapi_cmd != 0x28) {
-        PrintDebug(VM_NONE, VCORE_NONE, "IDE: Reading CD Data (len=%d) (req_len=%d)\n", length, drive->req_len);
-       PrintDebug(VM_NONE, VCORE_NONE, "IDE: transfer len=%d, transfer idx=%d\n", drive->transfer_length, drive->transfer_index);
+        PrintDebug(VM_NONE, VCORE_NONE, "IDE: Reading CD Data (len=%llu) (req_len=%u)\n", length, drive->req_len);
+       PrintDebug(VM_NONE, VCORE_NONE, "IDE: transfer len=%llu, transfer idx=%llu\n", drive->transfer_length, drive->transfer_index);
     }
 
     
 
-    if (drive->transfer_index >= drive->transfer_length) {
-       PrintError(VM_NONE, VCORE_NONE, "Buffer Overrun... (xfer_len=%d) (cur_idx=%d) (post_idx=%d)\n", 
+    if (drive->transfer_index >= drive->transfer_length && drive->transfer_index>=DATA_BUFFER_SIZE) {
+       PrintError(VM_NONE, VCORE_NONE, "Buffer Overrun... (xfer_len=%llu) (cur_idx=%llu) (post_idx=%llu)\n", 
                   drive->transfer_length, drive->transfer_index, 
                   drive->transfer_index + length);
        return -1;
@@ -1208,15 +1306,16 @@ static int read_drive_id( uint8_t * dst, uint_t length, struct ide_internal * id
 }
 
 
-static int ide_read_data_port(struct guest_info * core, ushort_t port, void * dst, uint_t length, void * priv_data) {
+
+static int read_data_port(struct guest_info * core, ushort_t port, void * dst, uint_t length, void * priv_data) {
     struct ide_internal * ide = priv_data;
     struct ide_channel * channel = get_selected_channel(ide, port);
     struct ide_drive * drive = get_selected_drive(channel);
 
-    //       PrintDebug(core->vm_info, core, "IDE: Reading Data Port %x (len=%d)\n", port, length);
+    //PrintDebug(core->vm_info, core, "IDE: Reading Data Port %x (len=%d)\n", port, length);
 
-    if ((channel->cmd_reg == 0xec) ||
-       (channel->cmd_reg == 0xa1)) {
+    if ((channel->cmd_reg == ATA_IDENTIFY) ||
+       (channel->cmd_reg == ATA_PIDENTIFY)) {
        return read_drive_id((uint8_t *)dst, length, ide, channel);
     }
 
@@ -1237,6 +1336,44 @@ static int ide_read_data_port(struct guest_info * core, ushort_t port, void * ds
     return length;
 }
 
+// For the write side, we care both about
+// direct PIO writes to a drive as well as 
+// writes that pass a packet through to an CD
+static int write_data_port(struct guest_info * core, ushort_t port, void * src, uint_t length, void * priv_data) {
+    struct ide_internal * ide = priv_data;
+    struct ide_channel * channel = get_selected_channel(ide, port);
+    struct ide_drive * drive = get_selected_drive(channel);
+
+    PrintDebug(core->vm_info, core, "IDE: Writing Data Port %x (val=%x, len=%d)\n", 
+            port, *(uint32_t *)src, length);
+
+    if (drive->drive_type == BLOCK_CDROM) {
+       if (channel->cmd_reg == ATA_PACKETCMD) { 
+           // short command packet - no check for space... 
+           memcpy(drive->data_buf + drive->transfer_index, src, length);
+           drive->transfer_index += length;
+           if (drive->transfer_index >= drive->transfer_length) {
+               if (atapi_handle_packet(core, ide, channel) == -1) {
+                   PrintError(core->vm_info, core, "Error handling ATAPI packet\n");
+                   return -1;
+               }
+           }
+       } else {
+           PrintError(core->vm_info,core,"Unknown command %x on CD ROM\n",channel->cmd_reg);
+           return -1;
+       }
+    } else if (drive->drive_type == BLOCK_DISK) {
+       if (write_hd_data((uint8_t *)src, length, ide, channel) == -1) {
+           PrintError(core->vm_info, core, "IDE: Could not write HD Data\n");
+           return -1;
+       }
+    } else {
+       // nothing ... do not support writable cd
+    }
+
+    return length;
+}
+
 static int write_port_std(struct guest_info * core, ushort_t port, void * src, uint_t length, void * priv_data) {
     struct ide_internal * ide = priv_data;
     struct ide_channel * channel = get_selected_channel(ide, port);
@@ -1272,34 +1409,123 @@ static int write_port_std(struct guest_info * core, ushort_t port, void * src, u
 
        case PRI_SECT_CNT_PORT:
        case SEC_SECT_CNT_PORT:
+           // update CHS and LBA28 state
            channel->drives[0].sector_count = *(uint8_t *)src;
            channel->drives[1].sector_count = *(uint8_t *)src;
+
+           // update LBA48 state
+           if (is_lba48(channel)) {
+               uint16_t val = *(uint8_t*)src; // top bits zero;
+               if (!channel->drives[0].lba48.sector_count_state) { 
+                   channel->drives[0].lba48.sector_count = val<<8;
+               } else {
+                   channel->drives[0].lba48.sector_count |= val;
+               }
+               channel->drives[0].lba48.sector_count_state ^= 1;
+               if (!channel->drives[1].lba48.sector_count_state) { 
+                   channel->drives[1].lba48.sector_count = val<<8;
+               } else {
+                   channel->drives[1].lba48.sector_count |= val;
+               }
+               channel->drives[0].lba48.sector_count_state ^= 1;
+           }
+           
            break;
 
        case PRI_SECT_NUM_PORT:
        case SEC_SECT_NUM_PORT:
+           // update CHS and LBA28 state
            channel->drives[0].sector_num = *(uint8_t *)src;
            channel->drives[1].sector_num = *(uint8_t *)src;
+
+           // update LBA48 state
+           if (is_lba48(channel)) {
+               uint64_t val = *(uint8_t *)src; // lob off top 7 bytes;
+               if (!channel->drives[0].lba48.lba41_state) { 
+                   channel->drives[0].lba48.lba |= val<<24; 
+               } else {
+                   channel->drives[0].lba48.lba |= val;
+               }
+               channel->drives[0].lba48.lba41_state ^= 1;
+               if (!channel->drives[1].lba48.lba41_state) { 
+                   channel->drives[1].lba48.lba |= val<<24; 
+               } else {
+                   channel->drives[1].lba48.lba |= val;
+               }
+               channel->drives[1].lba48.lba41_state ^= 1;
+           }
+
            break;
        case PRI_CYL_LOW_PORT:
        case SEC_CYL_LOW_PORT:
+           // update CHS and LBA28 state
            channel->drives[0].cylinder_low = *(uint8_t *)src;
            channel->drives[1].cylinder_low = *(uint8_t *)src;
+
+           // update LBA48 state
+           if (is_lba48(channel)) {
+               uint64_t val = *(uint8_t *)src; // lob off top 7 bytes;
+               if (!channel->drives[0].lba48.lba52_state) { 
+                   channel->drives[0].lba48.lba |= val<<32; 
+               } else {
+                   channel->drives[0].lba48.lba |= val<<8;
+               }
+               channel->drives[0].lba48.lba52_state ^= 1;
+               if (!channel->drives[1].lba48.lba52_state) { 
+                   channel->drives[1].lba48.lba |= val<<32; 
+               } else {
+                   channel->drives[1].lba48.lba |= val<<8;
+               }
+               channel->drives[1].lba48.lba52_state ^= 1;
+           }
+
            break;
 
        case PRI_CYL_HIGH_PORT:
        case SEC_CYL_HIGH_PORT:
+           // update CHS and LBA28 state
            channel->drives[0].cylinder_high = *(uint8_t *)src;
            channel->drives[1].cylinder_high = *(uint8_t *)src;
+
+           // update LBA48 state
+           if (is_lba48(channel)) {
+               uint64_t val = *(uint8_t *)src; // lob off top 7 bytes;
+               if (!channel->drives[0].lba48.lba63_state) { 
+                   channel->drives[0].lba48.lba |= val<<40; 
+               } else {
+                   channel->drives[0].lba48.lba |= val<<16;
+               }
+               channel->drives[0].lba48.lba63_state ^= 1;
+               if (!channel->drives[1].lba48.lba63_state) { 
+                   channel->drives[1].lba48.lba |= val<<40; 
+               } else {
+                   channel->drives[1].lba48.lba |= val<<16;
+               }
+               channel->drives[1].lba48.lba63_state ^= 1;
+           }
+
            break;
 
        case PRI_DRV_SEL_PORT:
        case SEC_DRV_SEL_PORT: {
-           channel->drive_head.val = *(uint8_t *)src;
+           struct ide_drive_head_reg nh, oh;
+
+           oh.val = channel->drive_head.val;
+           channel->drive_head.val = nh.val = *(uint8_t *)src;
+
+           // has LBA flipped?
+           if ((oh.val & 0xe0) != (nh.val & 0xe0)) {
+               // reset LBA48 state
+               channel->drives[0].lba48.sector_count_state=0;
+               channel->drives[0].lba48.lba41_state=0;
+               channel->drives[0].lba48.lba52_state=0;
+               channel->drives[0].lba48.lba63_state=0;
+               channel->drives[1].lba48.sector_count_state=0;
+               channel->drives[1].lba48.lba41_state=0;
+               channel->drives[1].lba48.lba52_state=0;
+               channel->drives[1].lba48.lba63_state=0;
+           }
            
-           // make sure the reserved bits are ok..
-           // JRL TODO: check with new ramdisk to make sure this is right...
-           channel->drive_head.val |= 0xa0;
 
            drive = get_selected_drive(channel);
 
@@ -1442,6 +1668,9 @@ static void init_channel(struct ide_channel * channel) {
     int i = 0;
 
     channel->error_reg.val = 0x01;
+
+    //** channel->features = 0x0;
+
     channel->drive_head.val = 0x00;
     channel->status.val = 0x00;
     channel->cmd_reg = 0x00;
@@ -1471,18 +1700,16 @@ static int pci_config_update(struct pci_device * pci_dev, uint32_t reg_num, void
 }
 
 static int init_ide_state(struct ide_internal * ide) {
-    int i;
 
     /* 
      * Check if the PIIX 3 actually represents both IDE channels in a single PCI entry 
      */
 
-    for (i = 0; i < 1; i++) {
-       init_channel(&(ide->channels[i]));
+    init_channel(&(ide->channels[0]));
+    ide->channels[0].irq = PRI_DEFAULT_IRQ ;
 
-       // JRL: this is a terrible hack...
-       ide->channels[i].irq = PRI_DEFAULT_IRQ + i;
-    }
+    init_channel(&(ide->channels[1]));
+    ide->channels[1].irq = SEC_DEFAULT_IRQ ;
 
 
     return 0;
@@ -1547,6 +1774,8 @@ static int ide_save_extended(struct v3_chkpt *chkpt, char *id, void * private_da
        V3_CHKPT_SAVE(ctx, "PRD_ADDR", ch->dma_prd_addr, savefailout);
        V3_CHKPT_SAVE(ctx, "DMA_TBL_IDX", ch->dma_tbl_index, savefailout);
 
+
+
        v3_chkpt_close_ctx(ctx); ctx=0;
 
        for (drive_num = 0; drive_num < 2; drive_num++) {
@@ -1589,6 +1818,13 @@ static int ide_save_extended(struct v3_chkpt *chkpt, char *id, void * private_da
              PrintError(VM_NONE, VCORE_NONE, "Invalid drive type %d\n",drive->drive_type);
              goto savefailout;
            }
+
+           V3_CHKPT_SAVE(ctx, "LBA48_LBA", drive->lba48.lba, savefailout);
+           V3_CHKPT_SAVE(ctx, "LBA48_SECTOR_COUNT", drive->lba48.sector_count, savefailout);
+           V3_CHKPT_SAVE(ctx, "LBA48_SECTOR_COUNT_STATE", drive->lba48.sector_count_state, savefailout);
+           V3_CHKPT_SAVE(ctx, "LBA48_LBA41_STATE", drive->lba48.lba41_state, savefailout);
+           V3_CHKPT_SAVE(ctx, "LBA48_LBA52_STATE", drive->lba48.lba52_state, savefailout);
+           V3_CHKPT_SAVE(ctx, "LBA48_LBA63_STATE", drive->lba48.lba63_state, savefailout);
            
            v3_chkpt_close_ctx(ctx); ctx=0;
        }
@@ -1689,6 +1925,14 @@ static int ide_load_extended(struct v3_chkpt *chkpt, char *id, void * private_da
              PrintError(VM_NONE, VCORE_NONE, "Invalid drive type %d\n",drive->drive_type);
              goto loadfailout;
            }
+
+           V3_CHKPT_LOAD(ctx, "LBA48_LBA", drive->lba48.lba, loadfailout);
+           V3_CHKPT_LOAD(ctx, "LBA48_SECTOR_COUNT", drive->lba48.sector_count, loadfailout);
+           V3_CHKPT_LOAD(ctx, "LBA48_SECTOR_COUNT_STATE", drive->lba48.sector_count_state, loadfailout);
+           V3_CHKPT_LOAD(ctx, "LBA48_LBA41_STATE", drive->lba48.lba41_state, loadfailout);
+           V3_CHKPT_LOAD(ctx, "LBA48_LBA52_STATE", drive->lba48.lba52_state, loadfailout);
+           V3_CHKPT_LOAD(ctx, "LBA48_LBA63_STATE", drive->lba48.lba63_state, loadfailout);
+           
        }
     }
 // goodout:
@@ -1751,7 +1995,8 @@ static int connect_fn(struct v3_vm_info * vm,
     }
 
     if (model_str != NULL) {
-       strncpy(drive->model, model_str, sizeof(drive->model) - 1);
+       strncpy(drive->model, model_str, sizeof(drive->model));
+       drive->model[sizeof(drive->model)-1] = 0;
     }
 
     if (strcasecmp(type_str, "cdrom") == 0) {
@@ -1819,6 +2064,8 @@ static int ide_init(struct v3_vm_info * vm, v3_cfg_tree_t * cfg) {
        }
 
        ide->southbridge = (struct v3_southbridge *)(southbridge->private_data);
+    } else {
+       PrintError(vm,VCORE_NONE,"Strange - you don't have a PCI bus\n");
     }
 
     PrintDebug(vm, VCORE_NONE, "IDE: Creating IDE bus x 2\n");
@@ -1840,7 +2087,7 @@ static int ide_init(struct v3_vm_info * vm, v3_cfg_tree_t * cfg) {
     PrintDebug(vm, VCORE_NONE, "Connecting to IDE IO ports\n");
 
     ret |= v3_dev_hook_io(dev, PRI_DATA_PORT, 
-                         &ide_read_data_port, &write_data_port);
+                         &read_data_port, &write_data_port);
     ret |= v3_dev_hook_io(dev, PRI_FEATURES_PORT, 
                          &read_port_std, &write_port_std);
     ret |= v3_dev_hook_io(dev, PRI_SECT_CNT_PORT, 
@@ -1857,7 +2104,7 @@ static int ide_init(struct v3_vm_info * vm, v3_cfg_tree_t * cfg) {
                          &read_port_std, &write_cmd_port);
 
     ret |= v3_dev_hook_io(dev, SEC_DATA_PORT, 
-                         &ide_read_data_port, &write_data_port);
+                         &read_data_port, &write_data_port);
     ret |= v3_dev_hook_io(dev, SEC_FEATURES_PORT, 
                          &read_port_std, &write_port_std);
     ret |= v3_dev_hook_io(dev, SEC_SECT_CNT_PORT, 
@@ -1902,7 +2149,7 @@ static int ide_init(struct v3_vm_info * vm, v3_cfg_tree_t * cfg) {
        struct pci_device * pci_dev = NULL;
        int i;
 
-       PrintDebug(vm, VCORE_NONE, "Connecting IDE to PCI bus\n");
+       V3_Print(vm, VCORE_NONE, "Connecting IDE to PCI bus\n");
 
        for (i = 0; i < 6; i++) {
            bars[i].type = PCI_BAR_NONE;