ATA driver

[BMW]

Hi all,

I wrote an ATA driver for reading sectors from disk in LBA28 mode.

Here it is:
ata.c:

Code: Select all

#ifndef DISK_H_INCLUDED
#include "ata.h"
#define DISK_H_INCLUDED
#endif

BOOL ata_wait_until_not_busy(uint32_t timeout_ms)
{
	uint8_t status = inportb(0x1F7);
	uint32_t startTick = get_tick_count();
	while(status & 0x88) //BSY and DRQ must be clear
	{
		if(get_tick_count() > startTick + timeout_ms)
			return FALSE;
		status = inportb(0x1F7);
	}
	return TRUE;
}

void ata_send_command(unsigned char command)
{
	outportb(0x1F7, command);
}

void ata_select_drive(uint8_t drive)
{
	uint8_t buf = inportb(0x1F6);
	if(drive)
		buf |= 0x10; //set bit 4
	
	buf = 0;
	
	outportb(0x1F6, buf);
}

void ata_interrupt_enable(BOOL enable)
{
	uint8_t buf = 0;
	if(enable)
		buf |= 2;
	
	outportb(0x3F6, buf);
}

BOOL ata_read_lba28(uint32_t LBA, uint8_t sectorCount, void* dest)
{
	if(!ata_wait_until_not_busy(1000))
		return FALSE;
	
	outportb(0x1F2, sectorCount); //sector count
	outportb(0x1F3, LBA & 0xFF); //LBA low byte
	outportb(0x1F4, (LBA >> 8) & 0xFF); //LBA mid byte
	outportb(0x1F5, (LBA >> 16) & 0xFF); //LBA high byte
	outportb(0x1F6, (LBA >> 24) & 0x0F); //bits 24-28 of LBA
	ata_select_drive(0);
	ata_interrupt_enable(FALSE);
	ata_send_command(0x20); //read sectors with retry
	
	uint8_t status = 0;
	
	for(unsigned int i=0;i<4;i++)
	{
		status = inportb(0x1F7);
		if(!(status & 0x80)) //BSY bit clear
		{
			if(status & 0x8) //DRQ bit set
				goto data_ready;
		}
	}
	
	while(1)
	{
		status = inportb(0x1F7);
		if(status & 0x80) //BSY set
			continue;
		else
			break;
		
		if(status & 0x1) //ERR set
			return FALSE;
		
		if(status & 0x20) //DF set
			return FALSE;
	}
	
	
data_ready: ;
	
	uint16_t* p = (uint16_t*)dest;
	
	for(unsigned int i=0;i<sectorCount;i++)
	{
		for(unsigned int i=0;i<256;i++)
		{
			*p = inportw(0x1F0);
			p++;
		}
		
		inportb(0x1F7);
		inportb(0x1F7);
		inportb(0x1F7);
		inportb(0x1F7);//400ns delay
		
		while(1)
		{
			status = inportb(0x1F7);
			if(status & 0x80) //BSY set
				continue;
			else
				break;
			
			if(status & 0x1) //ERR set
				return FALSE;
			
			if(status & 0x20) //DF set
				return FALSE;
		}
	}
	
	return TRUE;
}

ata.h:

Code: Select all

#ifndef STDINC_H_INCLUDED
#include "stdinc.h"
#define STDINC_H_INCLUDED
#endif

#ifndef TIMER_H_INCLUDED
#include "timer.h"
#define TIMER_H_INCLUDED
#endif

BOOL ata_wait_until_not_busy(uint32_t timeout_ms);
void ata_send_command(unsigned char command);
void ata_select_drive(uint8_t drive);
void ata_interrupt_enable(BOOL enable);
BOOL ata_read_lba28(uint32_t LBA, uint8_t sectorCount, void* dest);

If I call the read function like this

Code: Select all

if(ata_read_lba28(0, 1, (void*)0x00140000))
		print_string("success");
[code]
it prints "success". However if I do a memory dump, the bootsector (what I am reading from the disk) is nowhere to be found.

I am new to making drivers, so the code is probably full of errors....

I will do more debugging, but there might be a blatent error in my driver, so I thought I might as well post it on here.

Also, some people may be offended by that [b]goto[/b], IDGAF.

[Kazinsal]

Also, some people may be offended by that goto, IDGAF.

You should. You're programming in C, not C64 BASIC. In a structured language like C, goto is often unwise and leads to poorly structured code, and sometimes leads to the introduction of nasty, nasty bugs.

Perhaps you should read "GOTO Considered Harmful". Then again, considering your attitude, perhaps you shouldn't bother. You probably won't learn anything from it.

[linguofreak]

Also, some people may be offended by that goto, IDGAF.

While many of us may be offended, that's the least of your worries. The velociraptors are not only offended, but also hungry.

[iansjack]

I will do more debugging

Good choice.

I'm not offended by people using "goto", but I've better things to do than trying to understand badly written code.

[BMW]

Also, some people may be offended by that goto, IDGAF.

You should. You're programming in C, not C64 BASIC. In a structured language like C, goto is often unwise and leads to poorly structured code, and sometimes leads to the introduction of nasty, nasty bugs.

Perhaps you should read "GOTO Considered Harmful". Then again, considering your attitude, perhaps you shouldn't bother. You probably won't learn anything from it.

I am programming in C, exactly. GOTO IS A C STATEMENT!!!!!! (surprise surprise)

And I already told you... IDGAF... lol. I hardly ever use gotos, but in this case I believe it is totally harmless and is an effective solution to a problem.

[BMW]

I will do more debugging

Good choice.

I'm not offended people using "goto", but I've better things to do than trying to understand badly written code.

Is that code badly written because of the goto? Or just in general?

[Combuster]

It is in general. Magic numbers, include guards outside the header, and no respect for maintaining the existing content of bitfields.

[iansjack]

I am programming in C, exactly. GOTO IS A C STATEMENT!!!!!!

Here is some valid C-code: http://www.ioccc.org/2012/blakely/blakely.c . Would you consider it good code? Can you understand what it does?

Valid code is not necessarily well written code.

Is that code badly written because of the goto?

That's enough for me to not want to read any further.

[BMW]

Ok good point iansjack.

But we are getting off topic, as always happens with my posts.

Does anyone have anything helpful to say relating to my original question?

[iansjack]

Does anyone have anything helpful to say relating to my original question?

Yes.

I will do more debugging

Good choice.

You know that is the answer to your problem. It's really a bit lazy to keep posting code and asking others to debug it for you. You can't keep doing that with every step of your OS. Do you really want to write an OS or are you asking others to do it for you?

You acknowledged in your OP that (a) You need to do more debugging, and (b) The use of the "goto" statement is not likely to encourage others to help you. Heed your own advice.

[BMW]

Does anyone have anything helpful to say relating to my original question?

Yes.

I will do more debugging

Good choice.

You know that is the answer to your problem. It's really a bit lazy to keep posting code and asking others to debug it for you. You can't keep doing that with every step of your OS. Do you really want to write an OS or are you asking others to do it for you?

You acknowledged in your OP that (a) You need to do more debugging, and (b) The use of the "goto" statement is not likely to encourage others to help you. Heed your own advice.

Well I have been debugging for the past half hour... its really wierd... I commented out the line that actually writes the data from the port to memory (*p = inport...) THEN IT PAGE FAULTED...???

And I am not asking people to debug it for me, I just posted it in case there were any blatent errors in my code.

[iansjack]

OK. What could be going wrong? (Obviously something is not correct.) Look at the simple basics first:

Are you selecting the correct drive?
Are you selecting LBA mode?
Are you setting the correct values in the registers for the LBA addresses?

Read your code with those in mind and see if it helps. You can check the first two by loking at what value you send to port 0x1F6.

[BMW]

OK. What could be going wrong? (Obviously something is not correct.) Look at the simple basics first:

Are you selecting the correct drive?
Are you selecting LBA mode?
Are you setting the correct values in the registers for the LBA addresses?

Read your code with those in mind and see if it helps. You can check the first two by loking at what value you send to port 0x1F6.

Thank you so much, I forgot about setting LBA mode. I was going off some document that said that bit 6 of port 0x1F6 was "reserved" and said it was "always zero"... http://www.nondot.org/sabre/os/files/Disk/IDE-tech.html

Now I get a GPF, but I think I'll be able to sort that out pretty quickly.

Once again, thank you so much.

[BMW]

Sorted .

Just a random question,

On this page: http://wiki.osdev.org/ATA_PIO_Mode about half way down, under x86 directions for 28bit PIO, it has this for step 2:
2. Send a NULL byte to port 0x1F1, if you like (it is ignored and wastes lots of CPU time): outb(0x1F1, 0x00)

Errm... some sort of joke?

EDIT: ATM I am identity mapping the first 4MB of memory. If I make this memory writable, I get a page fault...?? Can you not execute code on a writable page?

EDIT2: Can the kernel write to read only pages by default (WP bit in CR0)?

[Combuster]

Does anyone have anything helpful to say relating to my original question?

Most flames are best ignored — after you've checked whether they are really flames, not pointers to the ways in which you have screwed up, and not cleverly ciphered answers to your real question (this happens as well).

I did point out a bug there...