Detecting ATA drives

[Matt1223]

Hello,

I'm currently checking out this page https://wiki.osdev.org/PCI_IDE_Controller. I copied the ide_initialize funtion to my system and when I run it I'm not getting what I'm supposed to get. The problem occures when I run my OS on a real machine. The machine has a primary master hdd and primary slave cd-rom attached. I'm getting the right data for the hdd however every other data is not right. I shows that all 4 devices exist and size and model data is basically trash. Did anybody deal with this page and knows what's wrong with the function?

[Octocontrabass]

There are too many pages on the wiki that try to explain ATA and that is one of the worst ones.

It ignores the PCI class code when deciding whether to use the BARs or the legacy I/O addresses. Only the PCI class code can tell you which you should use.

I spotted at least one infinite loop that it could get stuck in when no drive is connected. I spotted at least one instance where it doesn't wait for the drive to be ready before attempting to access the drive.

It relies on extremely broken inline assembly. It assumes both that you're using GCC and that you're using segmentation in a way that's incompatible with GCC.

It attempts to use 32-bit access on a 16-bit port without first ensuring that the underlying hardware is capable of 32-bit access.

[Matt1223]

There are too many pages on the wiki that try to explain ATA and that is one of the worst ones.

Do you know any page that is worh looking at?

[Octocontrabass]

This page seems to be more accurate. You still shouldn't copy the example code, but there are fewer mistakes.

You should also read the PCI IDE controller specification. It's written assuming you're already familiar with legacy ISA IDE adapters, so it's very short and only explains the important PCI bits.

[Matt1223]

This page seems to be more accurate. You still shouldn't copy the example code, but there are fewer mistakes.

So it says in the article that I should set LBA0, LBA1 and LBA2 to 0 before calling the identify command. How do I know whether I should use outb or outw to do so? The size of the register depends on whether or not the drive supports LBA48.

[Octocontrabass]

You edited your post while I was writing a reply, so I guess you figured out the other problems already.

So it says in the article that I should set LBA0, LBA1 and LBA2 to 0 before calling the identify command. How do I know whether I should use outb or outw to do so? The size of the register depends on whether or not the drive supports LBA48.

You should use outb(). Even if those registers are 16-bit, they only accept 8-bit writes.

[Matt1223]

I'm not sure how to implement soft reset. I wrote something like that but it doesn't look right at all. I don't know how can I set or unset certain bit on a register that I can't even read (it returns status register instead).

Code: Select all

static void soft_reset(uint8_t channel){
   outb(channels[channel].ctrl + ATA_REG_CONTROL, inb(channels[channel].ctrl + ATA_REG_CONTROL) | 0x4);
   long_wait();
   outb(channels[channel].ctrl + ATA_REG_CONTROL, inb(channels[channel].ctrl + ATA_REG_CONTROL) & 0xFB);
}

[Matt1223]

So I wrote a new ATA_init() function where I fixed some of the issues. Unfortuantely, I'm still getting some weird behavior. On the machine I'm running my OS there is nothing attached to secondary channel but the status register doesn't turn to 0 after sending Identify command. You can see what's happenning on the picture I attached. I don't think it's the problem with secondary channel on the machine because when I connect the devices to secondary channel I'm having the same problem. It recognizes attached devices correctly.

Here is the code:

Code: Select all

void ATA_init(){
   terminal_print(debugTerminal, "Initializing ATA...\n");
   channels[0].base = 0x1F0;
   channels[0].ctrl = 0x3F6;
   channels[1].base = 0x170;
   channels[1].ctrl = 0x376;
   int id = 0;
   for(int ch=0; ch<2; ch++){
      for(int dr=0; dr<2; dr++){
         ATADevices[id].exists = false;
         ATADevices[id].channel = ch;
         ATADevices[id].drive = dr;
         id++;
      }
   }
   for(int id=0; id<4; id++){
      int ch = ATADevices[id].channel;
      int dr = ATADevices[id].drive;
      // https://wiki.osdev.org/ATA_PIO_Mode#IDENTIFY_command
      terminal_print(debugTerminal, "Status reg before command: %x\n", inb(channels[ch].base + ATA_REG_STATUS));
      outb(channels[ch].base + ATA_REG_HDDEVSEL, (uint8_t []){0xA0, 0xB0}[dr]);
      io_wait();
      outb(channels[ch].base + ATA_REG_LBAlo, 0);
      io_wait();
      outb(channels[ch].base + ATA_REG_LBAmid, 0);
      io_wait();
      outb(channels[ch].base + ATA_REG_LBAhi, 0);
      io_wait();
      outb(channels[ch].base + ATA_REG_COMMAND, (uint8_t)(ATA_CMD_IDENTIFY));
      long_wait();
      terminal_print(debugTerminal, "Status reg after command: %x\n", inb(channels[ch].base + ATA_REG_STATUS));
      if(inb(channels[ch].base + ATA_REG_STATUS) == 0){
         terminal_print(debugTerminal, "Drive %d doesn't exist\n", id);
         continue;
      }
      terminal_print(debugTerminal, "Drive %d seems to exist\n", id);
      uint8_t type = IDE_ATA;

      // Polling
      uint8_t error = 0, status = 0;
      while((status & ATA_SR_BSY) || !(status & ATA_SR_DRQ)) {
         status = inb(channels[ch].base + ATA_REG_STATUS);
         if(status & ATA_SR_ERR){ // If Err, Device is not ATA.
            error = inb(channels[ch].base + ATA_REG_ERROR);
            break;
         }
      }
      if(error){
         print_error(error);
         uint8_t LBAmid = inb(channels[ch].base + ATA_REG_LBAmid);
         uint8_t LBAhi = inb(channels[ch].base + ATA_REG_LBAhi);
         if (LBAmid == 0x14 && LBAhi == 0xEB)
            type = IDE_ATAPI;
         else if (LBAmid == 0x69 && LBAhi == 0x96)
            type = IDE_ATAPI;
         else{
            terminal_print(debugTerminal, "\t-> unknown type of a device. Running soft reset on the channel and skipping device\n", id);
            soft_reset(ch);
            continue; // Unknown Type (may not be a device).
         }
         terminal_print(debugTerminal, "\t-> it's ATAPI device\n", id);
         outb(channels[ch].base + ATA_REG_COMMAND, (uint8_t)(ATA_CMD_IDENTIFY_PACKET));
         long_wait();
         error = 0, status = 0;
         while((status & ATA_SR_BSY) || !(status & ATA_SR_DRQ)){
            status = inb(channels[ch].base + ATA_REG_STATUS);
            // ATAPI devices seem to throw ATA_ER_ABRT error even when everything is right for some reason so I commented it out
            /*if(status & ATA_SR_ERR){  
               error = inb(channels[ch].base + ATA_REG_ERROR);
               if(error != ATA_ER_ABRT)
                  break;
            }*/
         }
         if(error){
            print_error(error);
            terminal_print(debugTerminal, "\t-> unexpected error. Running soft reset on the channel and skipping device\n", id);
            soft_reset(ch);
            continue;
         }
      }
      
      // Reading data
      char data_buffer[512];
      insw(channels[ch].base + ATA_REG_DATA, (uint16_t *)data_buffer, 256);
      ATADevices[id].exists = true;
      ATADevices[id].type = type;
      ATADevices[id].signature = *((uint16_t *)(data_buffer + ATA_IDENT_DEVICETYPE));
      ATADevices[id].capabilities = *((uint16_t *)(data_buffer + ATA_IDENT_CAPABILITIES));
      ATADevices[id].commandSets = *((uint32_t *)(data_buffer + ATA_IDENT_COMMANDSETS));

      if(ATADevices[id].commandSets & (1 << 26)) // Device uses 48-Bit Addressing
         ATADevices[id].size = *((int *)(data_buffer + ATA_IDENT_MAX_LBA_EXT));
      else // Device uses CHS or 28-bit Addressing
         ATADevices[id].size = *((int *)(data_buffer + ATA_IDENT_MAX_LBA));

      for(int k=0; k<40; k+=2){
         ATADevices[id].model[k] = data_buffer[ATA_IDENT_MODEL + k + 1];
         ATADevices[id].model[k + 1] = data_buffer[ATA_IDENT_MODEL + k];
         ATADevices[id].model[40] = 0; // Terminate String.
      }

      terminal_print(debugTerminal, "\t-> size: %uGB, type: %s, model: %s\n",
         ATADevices[id].size / 1024 / 1024 / 2,
         (char *[]){"ATA", "ATAPI"}[ATADevices[id].type],
         ATADevices[id].model);
   }
   terminal_print(debugTerminal, "[X] ATA ready!\n");
}

[Octocontrabass]

I'm not sure how to implement soft reset. I wrote something like that but it doesn't look right at all. I don't know how can I set or unset certain bit on a register that I can't even read (it returns status register instead).

You don't need to read the device control register, six of the eight bits are fixed values and you can decide what to write for the other two. If you did need to read it, though, I'd suspect the problem is related to the broken wiki code you're still relying on. (What is ATA_REG_CONTROL?)

On the machine I'm running my OS there is nothing attached to secondary channel but the status register doesn't turn to 0 after sending Identify command.

And it never will, because there's nothing attached to make it be zero. Specifically, seven of the eight status register bits are completely disconnected; the BSY bit is attached to a resistor to force it to be 0. IDE controllers often have this resistor so drivers don't have to wait for BSY to clear before determining that no drive is attached. This is why the status register returns 0x7F before you've written anything, and 0x6C after you've written 0xEC. (On a controller without the resistor, you would instead see 0xFF before writing anything and 0xEC after writing 0xEC.)

Incidentally, your screenshot shows the status register has bit 7 (BSY) set after you've written commands to the attached drives. Wouldn't it be more useful to wait for BSY to clear and then print the status register so you can see the drive's response?

Code: Select all

            // ATAPI devices seem to throw ATA_ER_ABRT error even when everything is right for some reason so I commented it out

You're reading the error register before BSY has cleared, so you're probably seeing the abort from the prior IDENTIFY command. You need to wait until BSY is clear.

Code: Select all

      ATADevices[id].signature = *((uint16_t *)(data_buffer + ATA_IDENT_DEVICETYPE));
      ATADevices[id].capabilities = *((uint16_t *)(data_buffer + ATA_IDENT_CAPABILITIES));
      ATADevices[id].commandSets = *((uint32_t *)(data_buffer + ATA_IDENT_COMMANDSETS));

      if(ATADevices[id].commandSets & (1 << 26)) // Device uses 48-Bit Addressing
         ATADevices[id].size = *((int *)(data_buffer + ATA_IDENT_MAX_LBA_EXT));
      else // Device uses CHS or 28-bit Addressing
         ATADevices[id].size = *((int *)(data_buffer + ATA_IDENT_MAX_LBA));

Type punning by pointer casts is undefined behavior.

[Matt1223]

(What is ATA_REG_CONTROL?)

It's an offset from control base to Device Control Register so just 0.

And it never will, because there's nothing attached to make it be zero. Specifically, seven of the eight status register bits are completely disconnected; the BSY bit is attached to a resistor to force it to be 0. IDE controllers often have this resistor so drivers don't have to wait for BSY to clear before determining that no drive is attached. This is why the status register returns 0x7F before you've written anything, and 0x6C after you've written 0xEC. (On a controller without the resistor, you would instead see 0xFF before writing anything and 0xEC after writing 0xEC.)

So it makes more sense to check whether the BSY bit is set instead of comparing status register to 0. It works well on the real machine but it unfortunately doesn't work on the emulators because the emulated devices are so fast that the BSY bit is practically never set. What should I do now. Do I have a separate code for emulators and the real machine? It doesn't sound like a pretty solution.

Code: Select all

uint8_t status = inb(channels[ch].base + ATA_REG_STATUS);
if(status == 0 || status == 0x6C || status == 0xEC){
         terminal_print(debugTerminal, "Drive %d doesn't exist\n", id);
         continue;
      }

Maybe something like that would be sufficient?

Incidentally, your screenshot shows the status register has bit 7 (BSY) set after you've written commands to the attached drives. Wouldn't it be more useful to wait for BSY to clear and then print the status register so you can see the drive's response?

I'm going to delete this part anyway. It was just a temporary debug code.

You're reading the error register before BSY has cleared, so you're probably seeing the abort from the prior IDENTIFY command. You need to wait until BSY is clear.

You're right. Thank you!

Type punning by pointer casts is undefined behavior.

So how can I extract data from the buffer then? Taking it out word by word or is there a nicer solution?

[Octocontrabass]

So it makes more sense to check whether the BSY bit is set instead of comparing status register to 0. It works well on the real machine but it unfortunately doesn't work on the emulators because the emulated devices are so fast that the BSY bit is practically never set. What should I do now. Do I have a separate code for emulators and the real machine? It doesn't sound like a pretty solution.

You need to check the status register before sending the command too. If the BSY bit is set, wait for it to clear with a timeout; if it doesn't clear within a reasonable time, there's no drive. If it contains nonsense such as 0x7F, there's no drive. Don't send any commands if you're already sure there's no drive.

After you send the command, check the status register again. You're expecting to see the drive either request a data transfer (to return the identify data) or abort the command (because ATAPI drives always abort the command). If the drive stays busy for too long or reports some other status, there's no drive.

You shouldn't rely on status register bits that are obsolete or otherwise dependent on the version of the ATA standard.

Maybe something like that would be sufficient?

If you're really looking at open bus, there's no guarantee which value you'll see in the status register. You'll have to check whether the drive reports a status you expect, and take any other nonsense to mean no drive is present.

So how can I extract data from the buffer then? Taking it out word by word or is there a nicer solution?

You can use a union or you can use memcpy(). (Or __builtin_memcpy() for some nice compiler optimizations.)