ATA PIO driver is broken *again*

[Cyao]

PS. I've got another new problem at the problem that is still about my ATA PIO driver, so I posted it at the bottom of this thread instead of creating a new thread

Hi all!

I've bumped into a weird problem yesterday. I wanted to write some stuff to the last sector of the hard disk using ATA PIO, so I located the max addressable sectors from the output from the IDENTIFY command (got 0x40000), so I tried write to there. But bochs trowed me the error

Code: Select all

00039372670e[HD    ] logical address out of bounds (1031438/262144) - aborting command

Which is weird, using the bochs debugger I can clearly see that I'm writing the right values into the right ports:

Code: Select all

[...]
Next at t=39372658
(0) [0x0000000081de] 0008:00000000000081de (unk. ctxt): out dx, al                ; ee
<bochs:36> calc dx
0x1f3 499
<bochs:37> calc al
0xff 255
[...]
Next at t=39372662
(0) [0x0000000081e7] 0008:00000000000081e7 (unk. ctxt): out dx, al                ; ee
<bochs:42> calc dx
0x1f4 500
<bochs:43> calc al
0xff 255
[...]
(0) [0x0000000081f0] 0008:00000000000081f0 (unk. ctxt): out dx, al                ; ee
<bochs:48> calc dx
0x1f5 501
<bochs:49> calc al
0x3 3
[...]
Next at t=39372670
(0) [0x0000000081fb] 0008:00000000000081fb (unk. ctxt): out dx, al                ; ee
<bochs:54> calc dx
0x1f7 503
<bochs:55> calc al
0x30 48
<bochs:56> s
Next at t=39372671
(0) [0x0000000081fc] 0008:00000000000081fc (unk. ctxt): mov edx, esi              ; 89f2
<bochs:57> 00039372670e[HD    ] logical address out of bounds (1031438/262144) - aborting command

Can someone tell me what I am doing wrong? Thanks

Here is my code:
https://github.com/cheyao/AchieveOS/blo ... ain.c#L239

[thewrongchristian]

If you have 0x40000 sectors, you can read sectors at LBA 0 to 0x40000-1.

[Cyao]

If you have 0x40000 sectors, you can read sectors at LBA 0 to 0x40000-1.

It also isn't working for 0x40000-1 , I already tried it

[Octocontrabass]

Can someone tell me what I am doing wrong?

The drive thinks you're using CHS instead of LBA. Check the value you're writing to the drive/head register (0x1F6).

[Cyao]

Can someone tell me what I am doing wrong?

The drive thinks you're using CHS instead of LBA. Check the value you're writing to the drive/head register (0x1F6).

Ahh thanks a lot! Didn't see there was a LBA bit in the drive select register.

[Cyao]

Hello, I'm back again with my ATA PIO driver.

This time I've upgraded it to 48 bit LBA, and it broke XD

The problem is that after a few calls to the driver, the driver gets stuck in the polling loop (where I've commented stuck here). I thing I did everything according to the specs, but I'm not 100% sure, can someone help me have a look?

Code: Select all

void write_disk(const uint64_t lba, const uint16_t sectors, uint16_t *buffer) {
	outb(drive_port + DRIVE_SELECT, 0x40 | (drive_slave << 4)); // drive select with lba bit set
	ata_io_wait(drive_port);
	outb(drive_port + SECTOR_COUNT, sectors >> 8);
	outb(drive_port + LBA_LOW, (lba & 0x0000ff000000) >> 24);
	outb(drive_port + LBA_MID, (lba & 0x00ff00000000) >> 32);
	outb(drive_port + LBA_HIGH, (lba & 0xff0000000000) >> 40);

	outb(drive_port + SECTOR_COUNT, sectors);
	outb(drive_port + LBA_LOW, (lba & 0x000000ff) >> 0);
	outb(drive_port + LBA_MID, (lba & 0x0000ff00) >> 8);
	outb(drive_port + LBA_HIGH, (lba & 0x00ff0000) >> 16);
	outb(drive_port + COMMAND_REGISTER, 0x34);  // Write disk command

	for (uint32_t i = 0; i < sectors; i++) {
		ata_io_wait(drive_port);

		while (1) {
			// Stuck here
			uint8_t status = inb(drive_port + COMMAND_REGISTER);

			if (!(status & 0x80) && (status & 0x08))
				break;
			else if (status & 0x01)
				return;

			ata_io_wait(drive_port);
		}

		for (int j = 0; j < 256; j++, buffer++)
			outw(drive_port + DATA, *buffer);
	}

	// Flush cache
	ata_io_wait(drive_port);
	outb(drive_port + DRIVE_SELECT, 0xA0);
	ata_io_wait(drive_port);
	outb(drive_port + COMMAND_REGISTER, 0xE7);
	ata_io_wait(drive_port);
}

If you need to see any more code, its Here, (Yes it's in the cdrom folder, but this function is ATA PIO)

[Octocontrabass]

The problem is that after a few calls to the driver, the driver gets stuck in the polling loop (where I've commented stuck here).

So, you're stuck polling the status register? What status does the status register report when you get stuck?

Code: Select all

	outb(drive_port + DRIVE_SELECT, 0x40 | (drive_slave << 4)); // drive select with lba bit set

That should be 0xE0 instead of 0x40. I doubt it makes a difference, but just in case...

Code: Select all

	outb(drive_port + DRIVE_SELECT, 0xA0);

Why?

Code: Select all

	outb(drive_port + COMMAND_REGISTER, 0xE7);

Why?

[Cyao]

So, you're stuck polling the status register? What status does the status register report when you get stuck?

It gives me 0x50 when it gets stuck, It occurs on the 5th or 6th or 7th write, I've been running qemu for like 2 mins and no change

Code: Select all

	outb(drive_port + DRIVE_SELECT, 0x40 | (drive_slave << 4)); // drive select with lba bit set

That should be 0xE0 instead of 0x40. I doubt it makes a difference, but just in case...

On the wiki it said Send 0x40 for the "master" or 0x50 for the "slave" to port 0x1F6: outb(0x1F6, 0x40 | (slavebit << 4)), but after I changed to 0xE0, still no change

Code: Select all

	outb(drive_port + DRIVE_SELECT, 0xA0);

Why?

ops that was a pure mistake, changed to

Code: Select all

outb(drive_port + DRIVE_SELECT, 0xA0 | (drive_slave << 4));

Code: Select all

	outb(drive_port + COMMAND_REGISTER, 0xE7);

Why?

On the wiki it said

Make sure to do a Cache Flush (ATA command 0xE7) after each write command completes.

[Octocontrabass]

It gives me 0x50 when it gets stuck, It occurs on the 5th or 6th or 7th write, I've been running qemu for like 2 mins and no change

It sounds like the drive thinks you're done writing before you're actually done writing. How many sectors are you trying to write? How many sectors did you tell the drive you're going to write?

On the wiki it said Send 0x40 for the "master" or 0x50 for the "slave" to port 0x1F6: outb(0x1F6, 0x40 | (slavebit << 4)), but after I changed to 0xE0, still no change

The wiki's information about ATA is questionable at best. You should set those two bits until the ATA specification says otherwise.

ops that was a pure mistake, changed to

Code: Select all

outb(drive_port + DRIVE_SELECT, 0xA0 | (drive_slave << 4));

Why?

On the wiki it said

Make sure to do a Cache Flush (ATA command 0xE7) after each write command completes.

Nonsense, that completely defeats the purpose of a cache. Don't do that.

[Cyao]

It gives me 0x50 when it gets stuck, It occurs on the 5th or 6th or 7th write, I've been running qemu for like 2 mins and no change

It sounds like the drive thinks you're done writing before you're actually done writing. How many sectors are you trying to write? How many sectors did you tell the drive you're going to write?

Well it depends, i just found out sometimes it can fail on every time I write, the sectors vary from 1 to cca. 285 sectors (size of my kernel), and even sometimes it completely succeeds without getting stuck in the loop. I told the drive that i'm going to write the sectors I'm going to write.

[Cyao]

Well I've some good news, after changing the flush cache command to a 800ns delay (2 ata waits), my write driver now works 100% the time, it is just a bit slow (takes like 10 seconds to burn ~150kb onto the disk), but atleast it's working

Edit: well I think It didn't fix it, it was just me getting lucky

[Octocontrabass]

You shouldn't be flushing the cache. Get rid of that.

Does your ata_io_wait() function not check the status register? The 400ns delay is the minimum required to switch between two devices before reading the status register returns the correct status; you still need to wait for the drive to actually be ready before you can send the next command.

[Cyao]

You shouldn't be flushing the cache. Get rid of that.

Yes I alr removed it

Does your ata_io_wait() function not check the status register? The 400ns delay is the minimum required to switch between two devices before reading the status register returns the correct status; you still need to wait for the drive to actually be ready before you can send the next command.

Well it checks the alternative status register, not the status register, but it should be a 400ns delay

[Octocontrabass]

Okay, but are you actually checking the value of the status register? A delay is not enough; you need to also make sure the status register indicates that the drive is ready.

QEMU has a trace feature that may be helpful; for example, add "-d trace:ide_*" to log every drive-related I/O port access.

[rdos]

ATA PIO has some nasty race conditions if not implemented properly. I don't think fixed delays makes any sense since the timing of real devices will differ. You might get that to work with emulators, but it is sure to break with real devices.

Comparing with my code, I can see that you are not checking drive for being ready at the beginning or for command being accepted after the command has been sent. IOW, before sending a command to the drive you must make sure that bit 7 is clear. After sending the command you must wait for bit 6 to become set. Before sending data you should only check bit 3 for being set not bit 7 for being clear (you must make sure it's clear before you send anything).