Page 1 of 2

ATA PIO driver is broken *again*

Posted: Sat Dec 17, 2022 6:07 am
by 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 :D

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

Re: ATA PIO driver is broken

Posted: Sat Dec 17, 2022 7:48 am
by thewrongchristian
If you have 0x40000 sectors, you can read sectors at LBA 0 to 0x40000-1.

Re: ATA PIO driver is broken

Posted: Sat Dec 17, 2022 7:54 am
by Cyao
thewrongchristian wrote: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

Re: ATA PIO driver is broken

Posted: Sat Dec 17, 2022 1:11 pm
by Octocontrabass
Cyao wrote: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).

Re: ATA PIO driver is broken

Posted: Sat Dec 17, 2022 2:51 pm
by Cyao
Octocontrabass wrote:
Cyao wrote: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.

Re: ATA PIO driver is broken *again*

Posted: Tue Dec 20, 2022 4:37 am
by 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)

Re: ATA PIO driver is broken *again*

Posted: Tue Dec 20, 2022 11:13 am
by Octocontrabass
Cyao wrote: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?

Re: ATA PIO driver is broken *again*

Posted: Tue Dec 20, 2022 11:45 am
by Cyao
Octocontrabass wrote: 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 :(
Octocontrabass wrote:

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
Octocontrabass wrote:

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));
Octocontrabass wrote:

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.

Re: ATA PIO driver is broken *again*

Posted: Tue Dec 20, 2022 12:15 pm
by Octocontrabass
Cyao wrote: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?
Cyao wrote: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.
Cyao wrote:ops that was a pure mistake, changed to

Code: Select all

outb(drive_port + DRIVE_SELECT, 0xA0 | (drive_slave << 4));
Why?
Cyao wrote: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.

Re: ATA PIO driver is broken *again*

Posted: Tue Dec 20, 2022 1:54 pm
by Cyao
Octocontrabass wrote:
Cyao wrote: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.

Re: ATA PIO driver is broken *again*

Posted: Tue Dec 20, 2022 2:00 pm
by 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 :D

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

Re: ATA PIO driver is broken *again*

Posted: Tue Dec 20, 2022 2:11 pm
by 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.

Re: ATA PIO driver is broken *again*

Posted: Tue Dec 20, 2022 2:26 pm
by Cyao
Octocontrabass wrote:You shouldn't be flushing the cache. Get rid of that.
Yes I alr removed it
Octocontrabass wrote: 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

Re: ATA PIO driver is broken *again*

Posted: Tue Dec 20, 2022 2:40 pm
by 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.

Re: ATA PIO driver is broken *again*

Posted: Tue Dec 20, 2022 2:53 pm
by 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).