ATA driver problem

Question about which tools to use, bugs, the best way to implement a function, etc should go here. Don't forget to see if your question is answered in the wiki first! When in doubt post here.
WinExperements
Member
Member
Posts: 97
Joined: Thu Jul 14, 2022 9:45 am
Contact:

Re: ATA driver problem

Post by WinExperements »

Octocontrabass wrote:You have to send a command that involves a data transfer before the drive will set DRQ.
Okay I send the IDENTIFY command and what command I should send or that enough?
Octocontrabass
Member
Member
Posts: 5560
Joined: Mon Mar 25, 2013 7:01 pm

Re: ATA driver problem

Post by Octocontrabass »

That should be enough.

Check the other registers for the ATAPI signature. You might be dealing with an ATAPI drive that doesn't follow ATA correctly.
WinExperements
Member
Member
Posts: 97
Joined: Thu Jul 14, 2022 9:45 am
Contact:

Re: ATA driver problem

Post by WinExperements »

So my problem is that my driver trying to send command to drive that still reseting after soft reset command, so my question is how I can correctly wait for the reset using only the disk IO ports or I must use the system sleep functions?
Octocontrabass
Member
Member
Posts: 5560
Joined: Mon Mar 25, 2013 7:01 pm

Re: ATA driver problem

Post by Octocontrabass »

The status register will tell you when the drive is done resetting and ready to accept commands.
WinExperements
Member
Member
Posts: 97
Joined: Thu Jul 14, 2022 9:45 am
Contact:

Re: ATA driver problem

Post by WinExperements »

How I can identify hard drive in SATA IDE mode? My drive with my driver returns me device fault and error code is 0xff
Octocontrabass
Member
Member
Posts: 5560
Joined: Mon Mar 25, 2013 7:01 pm

Re: ATA driver problem

Post by Octocontrabass »

Identifying a SATA drive on an IDE controller works the same as any other ATA drive on an IDE controller.

What is the value of the status register? The device fault bit only applies when the busy bit is clear.
WinExperements
Member
Member
Posts: 97
Joined: Thu Jul 14, 2022 9:45 am
Contact:

Re: ATA driver problem

Post by WinExperements »

Octocontrabass wrote:What is the value of the status register?

0x7f
Octocontrabass
Member
Member
Posts: 5560
Joined: Mon Mar 25, 2013 7:01 pm

Re: ATA driver problem

Post by Octocontrabass »

That combination of status and error register values means there's no drive present.
WinExperements
Member
Member
Posts: 97
Joined: Thu Jul 14, 2022 9:45 am
Contact:

Re: ATA driver problem

Post by WinExperements »

So, hello again! I have some problems with reading the sectors, my driver is getting the base register address and ctrl register address from PCI as descripted in the wiki, when i run this code on bochs, virtualbox or qemu the code successfully read the sectors, but when i runed it on the real hardware i got strange behaivor, the register reports successfully sector reading, but the drive actually doesn't do that(the disk active led doesn't power on and only 0xff transfered from the port). On bochs also i got continiosly interrupt 0x27. Here is my read code:

Code: Select all

static void ata_vdev_readBlock(vfs_node_t *node,int blockNo,int how, void *buf) {
        ata_device_t *dev = node->device;
        if (dev->type == IDE_ATAPI) {
                kprintf("Currently doesn't supported!\n");
                return;
        }
    	//outb(dev->base+ATA_REG_CONTROL,2);
	   kprintf("ata: read: dev ctrl: 0x%x\n",dev->base);
        void *dd = buf;
        int doCopy = false;
        uint64_t lba = blockNo;
        uint16_t sectors = how/512;
        if (sectors == 0) {
                sectors = 1;
                buf = kmalloc(512);
                doCopy = 1;
        }
        kprintf("Read %d sectors at %d\r\n",sectors,lba);
        if (sectors == 0) sectors = 1;
        outb(dev->base+ATA_REG_HDDEVSEL,0x40);
        ata_io_wait(dev);
        outportb(dev->base + ATA_REG_SECCOUNT0, 0);
	    outportb(dev->base + ATA_REG_LBA0, (lba & 0xff000000) >> 24);
	    outportb(dev->base + ATA_REG_LBA1, (lba & 0xff00000000) >> 32);
	    outportb(dev->base + ATA_REG_LBA2, (lba & 0xff0000000000) >> 40);

	    outportb(dev->base + ATA_REG_SECCOUNT0, sectors);
	    outportb(dev->base + ATA_REG_LBA0, (lba & 0x000000ff) >>  0);
	    outportb(dev->base + ATA_REG_LBA1, (lba & 0x0000ff00) >>  8);
	    outportb(dev->base + ATA_REG_LBA2, (lba & 0x00ff0000) >> 16);
        outb(dev->base+ATA_REG_COMMAND,ATA_CMD_READ_PIO_EXT); // ATA_CMD_READ_PIO_EXT
        ata_io_wait(dev);
	//kprintf("ata: command setted\n");
	//kprintf("ata: interrupts disabled\n");
        uint8_t i;
        // convert buffer
        uint8_t *bu = (uint8_t *)buf;
        //arch_sti();
	//kprintf("going to read loop\n");
        for (i = 0; i < sectors; i++) {
                while(1) {
                        uint8_t status = inb(dev->base+ATA_REG_STATUS);
			//kprintf("Waiting untill drive remove the ATA_SR_DRQ\n");
                        if (status & ATA_SR_DRQ) {
                                // Disk is ready to transfer data
                                break;
                        } else if (status & ATA_SR_ERR) {
                                kprintf("ATA: Disk error. Cancel\n");
                                return;
                        }
                }
		//kprintf("ata: sector readed\n");
                insw(dev->base,bu,256);
                bu+=256;
        }
	//kprintf("ata: sectors readed\n");
        if (doCopy) {
                memcpy(dd,buf,how);
                kfree(buf);
        }
	//kprintf("ata: goodbay\n");
}
Octocontrabass
Member
Member
Posts: 5560
Joined: Mon Mar 25, 2013 7:01 pm

Re: ATA driver problem

Post by Octocontrabass »

WinExperements wrote:On bochs also i got continiosly interrupt 0x27.
Which devices are connected to that interrupt?
WinExperements wrote:

Code: Select all

        outb(dev->base+ATA_REG_COMMAND,ATA_CMD_READ_PIO_EXT); // ATA_CMD_READ_PIO_EXT
Drives smaller than about 128GB often don't support 48-bit LBA.
WinExperements wrote:

Code: Select all

                        uint8_t status = inb(dev->base+ATA_REG_STATUS);
			//kprintf("Waiting untill drive remove the ATA_SR_DRQ\n");
                        if (status & ATA_SR_DRQ) {
If BSY is set, none of the other bits in the status register are valid. You must wait until BSY is clear before trying to interpret the other bits. You should use interrupts instead of polling the status register.
WinExperements
Member
Member
Posts: 97
Joined: Thu Jul 14, 2022 9:45 am
Contact:

Re: ATA driver problem

Post by WinExperements »

Hello again! So i have problem sending ATA Identify command to drive.
When i send the command the drive, the drive is raise the IRQ, i read the status register to indicate for the drive that we handle the interrupt, but drive won't stop send there IRQs. I receive infinite count of interrupts from ATA when send the Identify command even if i readed the data from data port.
Also i noticed that the bug occurs if i boot system in Legacy Mode or when i boot it on Legacy BIOS machine.
On virtual machines such as VirtualBox,QEMU all works fine. On Bochs i also got infinite amount of interrupts from ATA, but debug messages are empty.
What i did wrong?
Here is my drive detect code:

Code: Select all

bool ata_device_init(ata_device_t *dev,bool second) {
    arch_sti();
    ata_soft_reset(dev);
    ata_io_wait(dev);
    int bit = (second ? 0xB0 : 0xA0);
    outb(dev->base+ATA_REG_HDDEVSEL, bit | dev->slave << 4);
    ata_io_wait(dev);
    outb(dev->base+ATA_REG_SECCOUNT0,0);
    outb(dev->base+ATA_REG_LBA0,0);
    outb(dev->base+ATA_REG_LBA1,0);
    outb(dev->base+ATA_REG_LBA2,0);
    // Send drive identify command
    inter_ata = dev;
    dev->ident = 1;
    kprintf("ATA: sending command\r\n");
    outb(dev->base+ATA_REG_COMMAND,ATA_CMD_IDENTIFY);
    if (!inb(dev->base + ATA_REG_STATUS)) {
	    kprintf("ATA: no drive\r\n");
	    return false;
	}
    uint8_t lba1 = inb(dev->base+ATA_REG_LBA1);
    uint8_t lba2 = inb(dev->base+ATA_REG_LBA2);
    if (lba1 != 0 || lba2 != 0) {
	    kprintf("Not ATA device\r\n");
	    return false;
	}
    kprintf("Waiting until DRQ or ERR to be setted\r\n");
    uint8_t drq,err = 0;
    while(!drq && !err) {
		if (dev->ident == 0) break;
		//kprintf("DRQ: %d ERR: %d. STATUS: %d\r\n",drq,err,inb(dev->base+ATA_REG_STATUS));
	    drq = inb(dev->base + ATA_REG_STATUS) & ATA_SR_DRQ;
	    err = inb(dev->base + ATA_REG_STATUS) & ATA_SR_ERR;
	}
    if (err) {
	    kprintf("ATA: Drive error!\r\n");
	    return false;
	}
    kprintf("ATA: Reading using PIO mode\r\n");
    if (dev->ident) {
	    kprintf("ATA: No IRQ rised to read\r\n");

  //  kprintf("The device is ready and doesn't reported any errors, reading the identify structure...");
		uint16_t *buf = (uint16_t *)&dev->identify;
		for (int i = 0; i < 256; i++) {
			buf[i] = inw(dev->base);
		}
	}
  	kprintf("readed\n");
	uint8_t *ptr = (uint8_t *)&dev->identify.model;
	for (int i = 0; i < 39; i+=2) {
		uint8_t tmp = ptr[i+1];
		ptr[i+1] = ptr[i];
		ptr[i] = tmp;
	}
	return true;
}
And the IRQ handler code:

Code: Select all

void *ata_irq_handler(void *stack) {
	// ATA specifications
	kprintf("ATA IRQ!\r\n");
	inb(inter_ata->base+ATA_REG_STATUS);
	if (inter_ata->ident) {
		uint16_t *data = (uint16_t *)&inter_ata->identify;
		for (int i = 0; i < 256; i++) {
			data[i] = inw(inter_ata->base);
		}
		kprintf("Device identify readed!\r\n");
	inter_ata->ident = 0;
	}
    	//inb(inter_ata->base+ATA_REG_STATUS);
	if (inter_ata->busy) {
		inter_ata->busy = 0;
	}
    outb(inter_ata->bar4+4,0x00);
    /*kprintf("ATA IRQ handler!\n");
    kprintf("Status of drive(after sending there commands): 0x%x\n",inb(inter_ata->base+ATA_REG_STATUS));
    kprintf("Error register: 0x%x\n",inb(inter_ata->base+ATA_REG_ERROR));*/
    return stack;
}
Octocontrabass
Member
Member
Posts: 5560
Joined: Mon Mar 25, 2013 7:01 pm

Re: ATA driver problem

Post by Octocontrabass »

WinExperements wrote:

Code: Select all

    int bit = (second ? 0xB0 : 0xA0);
    outb(dev->base+ATA_REG_HDDEVSEL, bit | dev->slave << 4);
Why do you have two different methods to set bit 4?
WinExperements wrote:

Code: Select all

    outb(dev->base+ATA_REG_COMMAND,ATA_CMD_IDENTIFY);
    if (!inb(dev->base + ATA_REG_STATUS)) {
This is a race condition. If you need to check the drive's status while you're waiting for an interrupt, use the alternate status register.
WinExperements wrote:

Code: Select all

    uint8_t lba1 = inb(dev->base+ATA_REG_LBA1);
    uint8_t lba2 = inb(dev->base+ATA_REG_LBA2);
You can't read these registers while the drive is busy, and the drive is probably going to be busy if you just sent a command.
WinExperements wrote:

Code: Select all

	    drq = inb(dev->base + ATA_REG_STATUS) & ATA_SR_DRQ;
	    err = inb(dev->base + ATA_REG_STATUS) & ATA_SR_ERR;
Again, you need to use the alternate status register to prevent a race condition. You must check the status of the BSY bit before trying to interpret other bits in the status register. You don't need to read the status register twice to read two bits.
WinExperements wrote:

Code: Select all

		uint16_t *buf = (uint16_t *)&dev->identify;
		for (int i = 0; i < 256; i++) {
			buf[i] = inw(dev->base);
You have code to read the buffer twice.
WinExperements wrote:

Code: Select all

	inb(inter_ata->base+ATA_REG_STATUS);
You shouldn't discard the value of the status register. It's probably fine if you're just testing, though.
WinExperements wrote:

Code: Select all

		uint16_t *data = (uint16_t *)&inter_ata->identify;
		for (int i = 0; i < 256; i++) {
			data[i] = inw(inter_ata->base);
You have code to read the buffer twice.
WinExperements wrote:

Code: Select all

    outb(inter_ata->bar4+4,0x00);
Which register is this supposed to be? You don't need to touch any DMA registers when you're doing a PIO transfer.
WinExperements
Member
Member
Posts: 97
Joined: Thu Jul 14, 2022 9:45 am
Contact:

Re: ATA driver problem

Post by WinExperements »

So, i rewrited the device init code using the Wiki code, but as soon as i enable the device interrupts using the ATA_REG_CONTROL and issue the ATA DMA read command i start to receiving endless count of interrupts from IDE controller.
Here is my new version of device init code:

Code: Select all

bool ata_device_init(ata_device_t *dev,int id) { 
	unsigned char err = 0,status;
	ide_write(dev,ATA_REG_CONTROL,2);
	ide_write(dev,ATA_REG_HDDEVSEL,0xA0 | (id << 4));
	ata_io_wait(dev);
	ide_write(dev,ATA_REG_COMMAND,ATA_CMD_IDENTIFY);
	ata_io_wait(dev);
	if (ide_read(dev,ATA_REG_STATUS) == 0) return false;
	kprintf("Waiting device to clear BSY state...");
	while(1) {
		status = ide_read(dev,ATA_REG_STATUS);
		if ((status & ATA_SR_ERR)) {
			// Looks like, we send wrong command?
			uint8_t t1 = ide_read(dev,ATA_REG_LBA1);
			uint8_t t2 = ide_read(dev,ATA_REG_LBA2);
			if (t1 == 0x14 && t2 == 0xEB) {
				// fault caused by WRONG command.
				kprintf("ATAPI device fault(normal,ignore)\r\n");
				dev->type = IDE_ATAPI;
				break;
			} else {
				kprintf("First device fault received 0x%x 0x%x\r\n",t1,t2);
				return false;
			}
		}
		if (!(status & ATA_SR_BSY) && (status & ATA_SR_DRQ)) break;
	}
	kprintf("cleared\r\n");
	if (dev->type == IDE_ATAPI) {
		// Selected by first fault handler.
		goto atapiDetect;
	}
	uint8_t ch = ide_read(dev,ATA_REG_LBA1);
	uint8_t cl = ide_read(dev,ATA_REG_LBA2);
	if (cl == 0x14 && ch == 0xEB) {
atapiDetect:
		kprintf("ATAPI device detected!\r\n");
		dev->type = IDE_ATAPI;
		ide_write(dev,ATA_REG_COMMAND,ATA_CMD_IDENTIFY_PACKET);
		ata_io_wait(dev);
		// Repeat process
		while(1) {
			status = ide_read(dev,ATA_REG_STATUS);
			if ((status & ATA_SR_ERR)) {
				kprintf("Device fault fallback!\r\n");
				return false;
			}
			if (!(status & ATA_SR_BSY) && (status & ATA_SR_DRQ)) break;
		}
	} else if (cl == 0x69 && ch == 0x96) {
		kprintf("ATA device!\r\n");
	} else if (cl == 0x0 && ch == 0) {
		kprintf("We skiped something?!\r\n");
	} else {
		kprintf("Unknown type\r\n");
		return false;
	}
	uint16_t *identf = (uint16_t *)&dev->identify;
	for (int i = 0; i < 256; i++) {
		identf[i] = inw(dev->base);
	}
	kprintf("readed\n");
	uint8_t *ptr = (uint8_t *)&dev->identify.model;
	for (int i = 0; i < 39; i+=2) {
		uint8_t tmp = ptr[i+1];
		ptr[i+1] = ptr[i];
		ptr[i] = tmp;
	}
	kprintf("ATA: Detected device name: %s, type: %s\r\n",dev->identify.model,dev->type == IDE_ATAPI ? "ATAPI" : "IDE");
	ata_create_device(dev->type == IDE_ATA,dev);
	dev->type = IDE_ATA;
	return true;
}
The DMA reading code itself:

Code: Select all

static void ata_vdev_readBlock(vfs_node_t *node,int blockNo,int how, void *buf) {
        ata_device_t *dev = node->device;
        if (dev->type == IDE_ATAPI) {
                kprintf("Currently doesn't supported!\n");
                return;
        }
        void *dd = buf;
        int doCopy = false;
        uint64_t lba = blockNo;
        uint16_t sectors = how/512;
        //kprintf("Read %d sectors at %d\r\n",sectors,lba);
        if (sectors == 0) sectors = 1;
	if (sectors > 255) {
		kprintf("ATA: Maximum count of sectors that able to read per one command is 255!\n");
		sectors = 254;
	}
	if (blockNo == 0) {
		goto readActual;
	}
	kprintf("%s: called\r\n",__func__);
	// Before issusing real READ DMA command, check if we don't readed the sectors previously
readActual:
	ide_write(dev,ATA_REG_CONTROL,0);
	kprintf("%s: actual reading begin\r\n",__func__);
	// DMA support!
	outb(dev->bar4,0x00);
	outl(dev->bar4 + 0x04, dev->dma_prdt_phys);
	outb(dev->bar4 + 0x2, inb(dev->bar4 + 0x02) | 0x04 | 0x02);
	outb(dev->bar4, 0x08);
	while (1) {
		uint8_t status = inb(dev->base + ATA_REG_STATUS);
		if (!(status & ATA_SR_BSY)) break;
	}
	int cache_sectors = sectors;
        outb(dev->base+ATA_REG_HDDEVSEL,0x40);
        ata_io_wait(dev);
        outb(dev->base + ATA_REG_FEATURES, 0x01);
        outb(dev->base+ATA_REG_SECCOUNT0,(cache_sectors >> 8) & 0xFF);
	outb(dev->base+ATA_REG_LBA0, (lba & 0xff000000) >> 24);
	outb(dev->base+ATA_REG_LBA1, (lba & 0xff000000) >> 32);
	outb(dev->base+ATA_REG_LBA2, (lba & 0xff000000) >> 48);
	outb(dev->base+ATA_REG_SECCOUNT0,cache_sectors & 0xFF);
	outb(dev->base+ATA_REG_LBA0,lba & 0xFF);
	outb(dev->base+ATA_REG_LBA1,(lba >> 8) & 0xFF);
	outb(dev->base+ATA_REG_LBA2,(lba >> 16) & 0xFF);
        uint8_t i;
        // convert buffer
        uint8_t *bu = (uint8_t *)buf;
        while (1) {
		uint8_t status = inb(dev->base + ATA_REG_STATUS);
		if (!(status & ATA_SR_BSY) && (status & ATA_SR_DRDY)) break;
	}
	outb(dev->base + ATA_REG_COMMAND, ATA_CMD_READ_DMA_EXT);
	ata_io_wait(dev);

	outb(dev->bar4, 0x08 | 0x01);
	inter_ata->busy = 1;
	arch_sti();
	/*while (1) {
		int status = inb(dev->bar4 + 0x02);
		int dstatus = inb(dev->base + ATA_REG_STATUS);
		if (!(status & 0x04)) {
			continue;
		}
		if (!(dstatus & ATA_SR_BSY)) {
			break;
		}
	}
*/
	while(inter_ata->busy);
	/* Inform device we are done. */
	outb(dev->bar4 + 0x2, inb(dev->bar4 + 0x02) | 0x04 | 0x02);
        if (doCopy) {
                memcpy(dd,buf,how);
                kfree(buf);
        }
	if (sectors > 255) {
		kprintf("ATA: Sectors count are bigger that 0xFF, repeat process\n");
		sectors-=255;
		ata_vdev_readBlock(node,blockNo+255,sectors*512,bu);
	}
	// Dump the PDRT
	if (how > 8192) {
		kprintf("WARRNING: DMA support only 8 sectors :). So we copy only 8 sectors\r\n");
	}
	if (buf == NULL) {
		//kprintf("Zero buffer!!!\r\n");
		dev->latestSector = blockNo;
		return;
	}
	memcpy(bu,dev->dma_start,(how > 8192) ? 8192 : how);
	kprintf("%s: ended\r\n",__func__);
}
Current version of device block reading code is for testing, so for example we don't read the real count of cache sectors.
Also i notice that the bug/problem shows only if system boots in Legacy Mode using the EFI legacy boot or by real legacy BIOS.
EDIT: Rewrite some parts of code like in the wiki, nothing changed.
Octocontrabass
Member
Member
Posts: 5560
Joined: Mon Mar 25, 2013 7:01 pm

Re: ATA driver problem

Post by Octocontrabass »

WinExperements wrote:

Code: Select all

		if ((status & ATA_SR_ERR)) {
You must wait for the BSY bit to clear before you try to interpret any other status bits.
WinExperements wrote:

Code: Select all

	} else if (cl == 0x69 && ch == 0x96) {
SATA devices only return SATA-specific signatures after reset, not after an IDENTIFY DEVICE command. This signature indicates a SATA port multiplier.
WinExperements wrote:

Code: Select all

	} else if (cl == 0x0 && ch == 0) {
This signature indicates an IDE disk.
WinExperements wrote:

Code: Select all

		kprintf("ATA: Maximum count of sectors that able to read per one command is 255!\n");
No, the maximum is 65536 with READ DMA EXT (and 256 with READ DMA).
WinExperements wrote:

Code: Select all

	while (1) {
		uint8_t status = inb(dev->base + ATA_REG_STATUS);
		if (!(status & ATA_SR_BSY)) break;
	}
Why are you polling the status register? You should already know whether you're waiting for the drive to finish the last command you sent.
WinExperements wrote:

Code: Select all

        outb(dev->base+ATA_REG_HDDEVSEL,0x40);
I'm sure it makes no difference for READ DMA EXT, but it's a good idea to use 0xE0 instead of 0x40.
WinExperements wrote:

Code: Select all

        outb(dev->base + ATA_REG_FEATURES, 0x01);
READ DMA EXT doesn't use the features register.
WinExperements wrote:

Code: Select all

	outb(dev->base+ATA_REG_LBA1, (lba & 0xff000000) >> 32);
	outb(dev->base+ATA_REG_LBA2, (lba & 0xff000000) >> 48);
These values will always be zero instead of the correct LBA bits.
WinExperements wrote:

Code: Select all

        while (1) {
		uint8_t status = inb(dev->base + ATA_REG_STATUS);
		if (!(status & ATA_SR_BSY) && (status & ATA_SR_DRDY)) break;
	}
Why are you polling the status register again?
WinExperements wrote:

Code: Select all

		kprintf("WARRNING: DMA support only 8 sectors :). So we copy only 8 sectors\r\n");
DMA is not limited to 8 sectors, and 8192 bytes is either 16 sectors or 2 sectors depending on the sector size.

How does your interrupt handler acknowledge the interrupt? I could see you receiving an interrupt storm if that isn't done correctly.
WinExperements
Member
Member
Posts: 97
Joined: Thu Jul 14, 2022 9:45 am
Contact:

Re: ATA driver problem

Post by WinExperements »

Octocontrabass wrote: How does your interrupt handler acknowledge the interrupt? I could see you receiving an interrupt storm if that isn't done correctly.
This is my IDE IRQ handler code:

Code: Select all

void *ata_irq_handler(void *stack) {
	// ATA specifications
	kprintf("ATA IRQ!\r\n");
	inb(inter_ata->base+ATA_REG_STATUS);
    	//inb(inter_ata->base+ATA_REG_STATUS);
	if (inter_ata->busy) {
		inter_ata->busy = 0;
	}
    outb(inter_ata->bar4+4,0x00);
    /*kprintf("ATA IRQ handler!\n");
    kprintf("Status of drive(after sending there commands): 0x%x\n",inb(inter_ata->base+ATA_REG_STATUS));
    kprintf("Error register: 0x%x\n",inb(inter_ata->base+ATA_REG_ERROR));*/
    return stack;
}
Currently inter_ata is setted by the ata_vdev_readBlock before writing the command to the command register.
Post Reply