Reading hard disk

[cmp]

I'm implementing a basic hard disk driver, reading it using lba28. There are a couple places where I may have gone wrong with this: I attach the virtual disk file (a .raw file made using qemu-img and formatted/edited using a loopback device and kpartx/fdisk, in fat16) using the -hda option so all of my arguments to qemu are as follows:

Code: Select all

-hda disk.raw -boot d -cdrom os.iso

The disk contains the following structure:

Code: Select all

/hello // is a directory
     /hello/test.txt // contains "this is test data"

I am fairly sure I did this part correctly.

Now, the code for doing a simple read test from the disk is really simple, but for some reason I am getting a bunch of 0s and a couple seemingly random bytes in my return buffer. The disk exists and reading the port back) as well as the controller. I check if the disk exists like this:

Code: Select all

unsigned char poll_drive(unsigned short drive, unsigned char arg) // drive could be 0x1F7 or 0x170 and args 0xA0 or 0xB0
{
	outportb(drive, arg);
	timer_wait(1);
	return inportb(drive) & (1 << 6);
}

And this works fine.
My simple read function looks like this:

Code: Select all

void read_hdd_lba28(char buffer[], unsigned char addr, uint8_t sector_count, unsigned short drive, char is_slave)
{
	outportb(drive + 1, 0x00);
	outportb(drive + 2, sector_count);
	outportb(drive + 3, (unsigned char) ((addr) & 0xFF));
	outportb(drive + 4, (unsigned char)((addr >> 8) & 0xFF));
	outportb(drive + 5, (unsigned char)((addr >> 16) & 0xFF));
	outportb(drive + 6,  0xE0 | (is_slave << 4) | ((addr >> 24) & 0x0F));
	outportb(drive + 7, ATA_CMD_READ_PIO);

	while(!(inportb(drive + 7) & 0x08));

	for (int i = 0; i < 256; i++)
	{
		unsigned short tmpword = inportw(drive);
		buffer[i * 2] = (unsigned char)tmpword;
		buffer[i * 2 + 1] = (unsigned char)(tmpword >> 8);
	}
}

and call it like so:

Code: Select all

	uint8_t* buf;
	char addrbuf[4];
	char secbuf[4];
	uint32_t addr = 0;
	uint32_t sec_ct = 0;

	printf("Start addr: ");
	getstr(addrbuf);
	printf("Sector count: ");
	getstr(secbuf);

	addr = atoi(addrbuf);
	sec_ct = atoi(secbuf);
	
	read_hdd_lba28(buf, addr, sec_ct, PRIMARY_BASE, 0); // primbase is 0x1F0

	for (int i = 0; i < 256 * sec_ct; i++)
    	printf("0x%s ", itoa(buf[i], 0, 16));
    printf("\n");

Now, from what I've read this should work perfectly fine. It's simple code, and I think I understand it all. Is there any reason this shouldn't work?
I've read through the wiki pages for PCI IDE, and had some trouble understanding it all, so I discovered most of my information from other sites. I'd greatly appreciate any help.

Thanks!

[SpyderTL]

That code looks correct to me. The only thing that I see is that you aren't checking the BSY flag (or ERR flag) on the status register, so you may be reading data too early.

The PCI IDE page is a bit of a mess. I would use the ATA PIO Mode page instead.

According to that page, you also need to add some sleep time before checking the status register to make sure the drive has enough time to update it first.

[cmp]

Thanks. I've now changed my poll_drive code to the IDENTIFY command stuff, and it seems to get stuck while polling the status port..
Here's what I've written:

Code: Select all

unsigned char poll_drive(unsigned short drive, unsigned char arg)
{
	outportb(drive + 6, arg);
	outportb(drive + 2, 0x00);
	outportb(drive + 3, 0x00);
	outportb(drive + 4, 0x00);
	outportb(drive + 5, 0x00);
	outportb(drive + 7, 0xEC);
	//timer_wait(1);
	unsigned char iden = inportb(drive + 7);

	if(iden == 0) return 0;

	while(!(inportb(drive + 7) & 0x80));

	uint8_t stat = inportb(drive + 4) + inportb(drive + 5);
	if(stat != 0) return 0;

	while(1)
	{
		unsigned char s = inportb(drive + 7);
		if((s & 0x8) || (s & 1)) break;
	}

	return 1;
}

Is there any reason

Code: Select all

 while(!(inportb(drive + 7) & 0x80)); 

would hang?

[SpyderTL]

There are probably hundreds of reasons. Try checking for the error flag in that loop, and you may find a clue.

[cmp]

The error flag isn't set. I'm kinda stumped here. It seems like pretty simple code. How would I debug something like this?

[SpyderTL]

What about the Busy flag? It should be set after you send the IDENTIFY command, and it should clear after you read the last word of data. If it's never set, the problem is with the command, or the device (qemu setup).

EDIT: This is backwards... the BSY flag should set when you send the IDENTIFY command, and it should clear when data is ready to be read.

[SpyderTL]

I posted this, then deleted it, cause I thought that I had it backward, but now I'm posting it again...

Shouldn't it be

Code: Select all

while((inportb(drive + 7) & 0x80));

?

See if that fixes your problem.

According to the wiki, you need to poll the BSY flag (0x80) and wait till it clears, then you need to poll the DRQ flag (0x08) and the ERR flag (0x01) until one of them sets.

[cmp]

...oh. Damn. That's embarrassing.
Well.. It works now, partially. Initially, it would only read every other byte, however using the function

Code: Select all

static inline void insw(unsigned short port, void *addr, size_t cnt)
{
   __asm volatile("rep; insw"
       : "+D" (addr), "+c" (cnt)
       : "d" (port)
       : "memory");
}

It works fine. But I'd rather not rely on this, as I can't use rep outsw for writing and I like conformity.
So why would

Code: Select all

for (int i = 0; i < 256; i++)
{
	uint16_t tmpword = inportw(drive);
	buffer[i] = tmpword;
}

// inportw function in my libc:
unsigned short inportw(unsigned short port)
{
    unsigned short ret_val;
    __asm __volatile("inw %w1, %0" : "=a" (ret_val) : "d" (port));
    return ret_val;
}

Only read every other byte? I appreciate the help a lot, it was really awesome when it finally read some correct bytes.

This is my full read function:

Code: Select all

unsigned char wait_for_drive(unsigned short drive)
{
	while(1)
	{
		unsigned char stat = inportb(drive + 7);
		if(stat & 0x08 && !(stat & 0x80))
			break;
		else if(stat & 0x01 || !(stat & 0x20))
			return 0;
	}

	return 1;
}

void read_hdd_lba28(unsigned short buffer[], uint32_t addr, unsigned char sector_count, unsigned short drive, char is_slave)
{
	outportb(drive + 6,  0xE0 | (is_slave << 4) | ((addr >> 24) & 0x0F));
	outportb(drive + 1, 0x00);
	outportb(drive + 2, (unsigned char) sector_count);
	outportb(drive + 3, (unsigned char) addr);
	outportb(drive + 4, (unsigned char)(addr >> 8));
	outportb(drive + 5, (unsigned char)(addr >> 16));
	outportb(drive + 7, ATA_CMD_READ_PIO);

	if(!wait_for_drive(drive))
		printf("Error reading from drive at 0x%s", itoa(drive, 0, 16));

	//for (int i = 0; i < 256; i++)
	//{
	//	uint16_t tmpword = inportw(drive);
	//	buffer[i] = tmpword;
	//}

	//for(int i = 0; i < sector_count; i++) // multiple sectors not implemented correctly yet 
		insw(drive, buffer, 256);
}

[SpyderTL]

I'm pretty good at C, and I'm be becoming pretty good at ASM, but I have no idea how to inline ASM in C, so I'm afraid I'm not much help there. But I'm sure someone here can spot the problem...

[gerryg400]

How is 'buffer' defined ?

[cmp]

Code: Select all

unsigned short buf[256 * sec_ct];

Here's all the code that calls it though:

Code: Select all

	char addrbuf[4];
	char secbuf[4];
	uint32_t addr = 0;
	uint32_t sec_ct = 0;

	printf("Start addr: ");
	getstr(addrbuf);
	printf("Sector count: ");
	getstr(secbuf);

	addr = atoi(addrbuf);
	sec_ct = atoi(secbuf);
	
	unsigned short buf[256 * sec_ct];

	if(!read_hdd_lba28(buf, addr, sec_ct, PRIMARY_BASE, 0))
	{
		printf("Failed to read drive..\n");
		return;
	}

	for (int i = 0; i < 256 * sec_ct; i++)
	{
    	printf("%s ", itoa((unsigned short) (buf[i]), 0, 16));
	}
    printf("\n");

[Kazinsal]

Code: Select all

uint32_t sec_ct = 0;

Code: Select all

unsigned short buf[256 * sec_ct];

Really, now. I'm surprised this isn't throwing a compiler error.

[gerryg400]

Code: Select all

char buffer[],

Code: Select all

for (int i = 0; i < 256; i++)
{
   uint16_t tmpword = inportw(drive);
   buffer[i] = tmpword;
}

This doesn't work. You will only end up with every second byte.

[cmp]

Code: Select all

char buffer[],

Code: Select all

for (int i = 0; i < 256; i++)
{
   uint16_t tmpword = inportw(drive);
   buffer[i] = tmpword;
}

This doesn't work. You will only end up with every second byte.

I've changed this, in my most recent post I've changed it to a short array.

Code: Select all

uint32_t sec_ct = 0;

Code: Select all

unsigned short buf[256 * sec_ct];

Really, now. I'm surprised this isn't throwing a compiler error.

It's just 0 initially. I'm setting it to a user given input later, before I pass it to the array.

I've decided to just go along with the rep insw.. Oh well. My only issue now is that it's not reading multiple sectors correctly. Sometimes it works, but when it does the next read just returns a bunch of 0s. I'm guessing this has something to do with it reading another few sectors after the last read. This is all the relevant up to date code:

Code: Select all

unsigned char io_hdd_lba28(unsigned char io_cmd, unsigned short buffer[], uint32_t addr, unsigned char sector_count, unsigned short drive, unsigned char is_slave)
{
	outportb(drive + 6,  0xE0 | (is_slave << 4) | ((addr >> 24) & 0x0F));
	outportb(drive + 1, 0x00);
	outportb(drive + 2, (unsigned char) sector_count);
	outportb(drive + 3, (unsigned char) addr);
	outportb(drive + 4, (unsigned char)(addr >> 8));
	outportb(drive + 5, (unsigned char)(addr >> 16));
	outportb(drive + 7, io_cmd);

	timer_wait(1);

	unsigned char stat = wait_for_drive(drive, ATA_TIMEOUT);

	if(stat != 0)
	{
		char* str;
		if(stat & 0x01 && !(stat & 0x20))
			str = "ERR";
		else if(stat & 0x20 && !(stat & 0x01))
			str = "DF";
		else if(stat & 0x20 && stat & 0x01)
			str = "ERR and DF";
		
		printf("Error reading from drive at 0x%s", itoa(drive, 0, 16));
		printf(" with %s flag(s) set.\n", str);
		return 0;
	}

	//for (int i = 0; i < 256; i++)
	//{
	//	uint16_t tmpword = inportw(drive);
	//	buffer[i] = tmpword;
	//}
	
	if(io_cmd == ATA_CMD_READ_PIO)
	{
		for(int i = 0; i < sector_count; i++)
		{
			insw(drive, buffer, 256);
			buffer += 256;
		}
	}
	else if(io_cmd == ATA_CMD_WRITE_PIO)
	{
		for(int i = 0; i < sector_count * 256; i++)
		{
			outportw(drive, (unsigned short) buffer[i]);
		}
	}

	return 1;
}

These are the two relevant shell commands. They're just to test functionality, so they just write some random stuff. Currently in the write function there is no cache flushing code, but that'll be easy to implement. I plan on doing it soon.

Code: Select all

void write_command()
{	
	unsigned short buf[256];
	for(int i = 0; i < 256; i++)
	{
		buf[i] = 0;
	}
	buf[0] = 0xDE;
	buf[1] = 0xAD;

	if(!io_hdd_lba28(ATA_CMD_WRITE_PIO, buf, 0, 1, PRIMARY_BASE, 0))
	{
		printf("Failed to write drive..\n");
		return;
	}
}

void read_command()
{
	char addrbuf[4];
	char secbuf[4];
	uint32_t addr = 0;
	uint32_t sec_ct = 0;

	printf("Start addr: ");
	getstr(addrbuf);
	printf("Sector count: ");
	getstr(secbuf);

	addr = atoi(addrbuf);
	sec_ct = atoi(secbuf);
	
	unsigned short buf[256 * sec_ct];

	if(!io_hdd_lba28(ATA_CMD_READ_PIO, buf, addr, sec_ct, PRIMARY_BASE, 0))
	{
		printf("Failed to read drive..\n");
		return;
	}

	for (int i = 0; i < 256 * sec_ct; i++)
	{
    	printf("%s ", itoa((unsigned short) (buf[i]), 0, 16));
	}
    printf("\n");
}

I've read through the wiki article very thoroughly. I'm completely lost as to what my issue is caused by. I appreciate the help.

[Roman]

Don't know, if it's what you're looking for, but here is some of my old ATA PIO (as far as I remember LBA48) code and here is the header.