[solved]Can't get harddrive reading to work

[Pyrofan1]

i followed this document http://www.nondot.org/sabre/os/files/Disk/IDE-tech.html and came up with this code

Code: Select all

struct HARDDRIVE_SECTOR
{
	unsigned char sector_num;
	unsigned char cylinder_low;
	unsigned char cylinder_high;
	unsigned char drive_head;
	unsigned char sector[512];
	unsigned char error;
};

#define DATA_REGISTER 0x1F0
#define ERROR_REGISTER 0x1F1
#define SECTOR_COUNT_REGISTER 0x1F2
#define SECTOR_NUMBER_REGISTER 0x1F3
#define CYLINDER_LOW_REGISTER 0x1F4
#define CYLINDER_HIGH_REGISTER 0x1F5
#define DRIVE_HEAD_REGISTER 0x1F6
#define STATUS_REGISTER 0x1F7
#define COMMAND_REGISTER 0x1F7
#define DEVICE_CONTROL_REGISTER 0x3F6

struct HARDDRIVE_SECTOR buffer;
unsigned char done;

struct HARDDRIVE_SECTOR read_sector(unsigned char device,unsigned char head,unsigned char cylinder_low,unsigned char cylinder_high,unsigned char sector_num)
{
	unsigned int count;

	buffer.drive_head=0;
	buffer.sector_num=0;
	buffer.cylinder_low=0;
	buffer.cylinder_high=0;
	done=0;
	
	for(count=0;count<512;count++)
	{
		buffer.sector[count]=0;
	}

	while((inportb(STATUS_REGISTER)&0x80)==0x80);
	
	if(device==1) buffer.drive_head=0xA0;
	else buffer.drive_head=0xB0;

	buffer.drive_head=buffer.drive_head|head;
	buffer.sector_num=sector_num;
	buffer.cylinder_low=cylinder_low;
	buffer.cylinder_high=cylinder_high;

	outportb(SECTOR_COUNT_REGISTER,1);
	outportb(SECTOR_NUMBER_REGISTER,sector_num);
	outportb(DRIVE_HEAD_REGISTER,buffer.drive_head);
	outportb(CYLINDER_LOW_REGISTER,cylinder_low);
	outportb(CYLINDER_HIGH_REGISTER,cylinder_high);
	outportb(DEVICE_CONTROL_REGISTER,0x0A);

	while((inportb(STATUS_REGISTER)&0x40)==0x40);
	outportb(COMMAND_REGISTER,0x20);
	while(!done);

	return buffer;
}

void harddrive_handler(void)
{
	unsigned int count;
	Printf("\nharddrive int\n");
	for(count=0;count<512;count++)
	{
		buffer.sector[count]=inportb(DATA_REGISTER);
	}
	done=1;
}

now when i try to read a sector interrupt 14 never fires and i have know idea as to why.

[bewing]

Problem #1 -- while((inportb(STATUS_REGISTER)&0x40)==0x40);
I don't remember what this bit is for (is it the RDY bit?) -- but whatever it is, you shouldn't be waiting for it to clear, at this point. Once the BSY bit is clear, you are ready to go.

Problem #2 -- in your INT handler, you are reading bytes. This is a big no-no. According to the specs, you must read 256 shorts, not 512 bytes. This also means that you probably need to do a little more to ensure that your read buffer is aligned on an even boundary.

Problem #3 -- after the read is complete, you need to read a status register (either regular or alternate). Doing this read resets some disk controller circuitry. You probably want to read the alternate status register, since you are using interrupts.

[Pyrofan1]

i changed my code as you said

Code: Select all

struct HARDDRIVE_SECTOR
{
	unsigned char sector_num;
	unsigned char cylinder_low;
	unsigned char cylinder_high;
	unsigned char drive_head;
	unsigned short sector[256];
	unsigned char error;
}__attribute__((packed));

#define DATA_REGISTER 0x1F0
#define ERROR_REGISTER 0x1F1
#define SECTOR_COUNT_REGISTER 0x1F2
#define SECTOR_NUMBER_REGISTER 0x1F3
#define CYLINDER_LOW_REGISTER 0x1F4
#define CYLINDER_HIGH_REGISTER 0x1F5
#define DRIVE_HEAD_REGISTER 0x1F6
#define STATUS_REGISTER 0x1F7
#define COMMAND_REGISTER 0x1F7
#define DEVICE_CONTROL_REGISTER 0x3F6
#define ALT_STATUS_REGISTER 0x3F6

struct HARDDRIVE_SECTOR buffer;
unsigned char done;

struct HARDDRIVE_SECTOR read_sector(unsigned char device,unsigned char head,unsigned char cylinder_low,unsigned char cylinder_high,unsigned char sector_num)
{
	unsigned int count;
	unsigned char status;

	buffer.drive_head=0;
	buffer.sector_num=0;
	buffer.cylinder_low=0;
	buffer.cylinder_high=0;
	buffer.error=265;
	done=0;
	
	for(count=0;count<512;count++)
	{
		buffer.sector[count]=0;
	}

	while((inportb(STATUS_REGISTER)&0x80)==0x80);
	
	if(device==1) buffer.drive_head=0xA0;
	else buffer.drive_head=0xB0;

	buffer.drive_head=buffer.drive_head|head;
	buffer.sector_num=sector_num;
	buffer.cylinder_low=cylinder_low;
	buffer.cylinder_high=cylinder_high;

	outportb(SECTOR_COUNT_REGISTER,1);
	outportb(SECTOR_NUMBER_REGISTER,sector_num);
	outportb(DRIVE_HEAD_REGISTER,buffer.drive_head);
	outportb(CYLINDER_LOW_REGISTER,cylinder_low);
	outportb(CYLINDER_HIGH_REGISTER,cylinder_high);
	outportb(DEVICE_CONTROL_REGISTER,0x0A);

	while((inportb(STATUS_REGISTER)&0x80)==0x80);
	outportb(COMMAND_REGISTER,0x20);
	while(!done);

	status=inportb(ALT_STATUS_REGISTER);
	if((status&0x01)==0x01);
	{
		buffer.error=inportb(ERROR_REGISTER);
	}
	return buffer;
}

void harddrive_handler(void)
{
	unsigned int count;
	Printf("\nharddrive int\n");
	for(count=0;count<256;count++)
	{
		buffer.sector[count]=inportw(DATA_REGISTER);
	}
	done=1;
}

now the code restarts (which is weird considering i have my idt and error handling all set up and working)

[bewing]

I just noticed that you are sending a 0xA to port 0x3f6 -- why are you doing that? That sets the HOB bit, and one other bit -- if anything, I think you want to set that IO Port to 0, not 0xA. But I don't think you should be sending anything to that port, at all.

And when you are clearing your "sector" array, now, you are still looping 512 times, even though the array is now only 256 long -- so you are overwriting some memory. I doubt that is your problem, though -- I'd really recommend running it in bochs, and singlestepping through this routine -- to see if/where it crashes.

Which device (master/slave) and which CHS are you trying to read? You are certain that the input values are legal? Are you certain your drive supports CHS addressing (is the drive smaller than 8GB)?

[Pyrofan1]

I just noticed that you are sending a 0xA to port 0x3f6 -- why are you doing that?

i mis read the link i posted (that's what you get for reading it at 3 in the morning)

And when you are clearing your "sector" array, now, you are still looping 512 times, even though the array is now only 256 long

that was the source of the crash

Which device (master/slave) and which CHS are you trying to read? You are certain that the input values are legal? Are you certain your drive supports CHS addressing (is the drive smaller than 8GB)?

i'm reading the master, my cylinder is 0, head 0 and sector 1 and unless i'm mistaken that is the address of the partition table. the drive is 100GB.
here is my current code, i changed so that'll turn the 256 shorts into 512 bytes

Code: Select all

struct HARDDRIVE_SECTOR
{
	unsigned char sector_num;
	unsigned char cylinder_low;
	unsigned char cylinder_high;
	unsigned char drive_head;
	unsigned char sector[512];
	unsigned char error;
}__attribute__((packed));

#define DATA_REGISTER 0x1F0
#define ERROR_REGISTER 0x1F1
#define SECTOR_COUNT_REGISTER 0x1F2
#define SECTOR_NUMBER_REGISTER 0x1F3
#define CYLINDER_LOW_REGISTER 0x1F4
#define CYLINDER_HIGH_REGISTER 0x1F5
#define DRIVE_HEAD_REGISTER 0x1F6
#define STATUS_REGISTER 0x1F7
#define COMMAND_REGISTER 0x1F7
#define DEVICE_CONTROL_REGISTER 0x3F6
#define ALT_STATUS_REGISTER 0x3F6

struct HARDDRIVE_SECTOR buffer;
unsigned char done;

struct HARDDRIVE_SECTOR read_sector(unsigned char device,unsigned char head,unsigned char cylinder_low,unsigned char cylinder_high,unsigned char sector_num)
{
	unsigned int count;
	unsigned char status;

	buffer.drive_head=0;
	buffer.sector_num=0;
	buffer.cylinder_low=0;
	buffer.cylinder_high=0;
	buffer.error=255;
	done=0;
	
	for(count=0;count<512;count++)
	{
		buffer.sector[count]=0;
	}

	while((inportb(STATUS_REGISTER)&0x80)==0x80);
	
	if(device==1) buffer.drive_head=0xA0;
	else buffer.drive_head=0xB0;

	buffer.drive_head=buffer.drive_head|head;
	buffer.sector_num=sector_num;
	buffer.cylinder_low=cylinder_low;
	buffer.cylinder_high=cylinder_high;

	outportb(SECTOR_COUNT_REGISTER,1);
	outportb(SECTOR_NUMBER_REGISTER,sector_num);
	outportb(DRIVE_HEAD_REGISTER,buffer.drive_head);
	outportb(CYLINDER_LOW_REGISTER,cylinder_low);
	outportb(CYLINDER_HIGH_REGISTER,cylinder_high);

	while((inportb(STATUS_REGISTER)&0x80)==0x80);
	outportb(COMMAND_REGISTER,0x20);
	while(!done);

	status=inportb(ALT_STATUS_REGISTER);
	if((status&0x01)==0x01);
	{
		buffer.error=inportb(ERROR_REGISTER);
	}
	return buffer;
}

void harddrive_handler(void)
{
	unsigned int count;
	unsigned short b0;
	unsigned char b1;
	unsigned char b2;
	Printf("\nharddrive int\n");
	for(count=0;count<512;count+=2)
	{
		b0=inportw(DATA_REGISTER);
		b1=(unsigned char)(b0&0x0F);
		b2=(unsigned char)((b0&0xF0)>>8);
		buffer.sector[count]=b1;
		buffer.sector[count++]=b2;
		
	}
	done=1;
}

and this is how i'm calling the function

Code: Select all

sector=read_sector(0,0,1,0,0);

[bewing]

Code: Select all

sector=read_sector(0,0,1,0,0);
if(device==1) buffer.drive_head=0xA0; 

Aha! I thought so! You say you are trying to read the master drive, but to do that you need to call the function with device=1! You are calling it with a 0. I'll bet that you don't even have a slave drive attached to the primary bus, so you are just getting garbage now, when you try it?

Code: Select all

struct HARDDRIVE_SECTOR 
{ 
   unsigned char sector_num; 
   unsigned char cylinder_low; 
   unsigned char cylinder_high; 
   unsigned char drive_head; 
   unsigned char sector[512]; 
   unsigned char error; 
}__attribute__((packed)); 

You might want to just declare the sector array as a union of an unsigned char array[512] and a unsigned short array[256] inside your structure. I think it would make your life easier, and it would enforce the proper boundaries. And then you don't have to do the shift operations in your harddrive_handler() function.

And yes, 0,0,1 is the proper CHS for the MBR/Partition table on all harddrives.

[Pyrofan1]

that was the problem. now the interrupt fires.
but now i got a new problem

my structure

Code: Select all

struct HARDDRIVE_SECTOR
{
	unsigned char sector_num;
	unsigned char cylinder_low;
	unsigned char cylinder_high;
	unsigned char drive_head;
	union
	{
		unsigned char bytes[512];
		unsigned short words[256];
	}sector;
	unsigned char error;
}__attribute__((packed));

how i read from the harddrive

Code: Select all

for(count=0;count<256;count++)
{
	buffer.sector.words[count]=inportw(DATA_REGISTER);
		
}

but buffer.sector.bytes only seems to contain 0xFF

[bewing]

Have you dumped out buffer.sector.words to see if it has something other than 0xffff? Have you printed some of the words as they are being read, to see if they are being read in as 0xffff?

Is this the first time you are accessing the IDE bus in your code, or did you do IDENTIFY commands on your drives already?

[Pyrofan1]

Have you dumped out buffer.sector.words to see if it has something other than 0xffff?

it contains 0xFFFF

Have you printed some of the words as they are being read, to see if they are being read in as 0xffff?

they are being read in as 0xFFFF

Is this the first time you are accessing the IDE bus in your code, or did you do IDENTIFY commands on your drives already?

first time

[bewing]

First, I noticed recently that it looks like you are waiting on the 0x80 bit of the status port in two separate while loops. You only need to do it once, so get rid of the second one. That shouldn't change how it's working though -- it was just wasting a little time.

Second, I think I see the problem. I looked again at the parameters that you are passing in. You say you are trying to access CHS = 0,0,1 -- but you are actually calling the function with CHS = 1,0,0 -- and a sector number of 0 IS ALWAYS ILLEGAL. Valid sector numbers are 1 to 63, usually. So, instead of calling with (1, 0, 1, 0, 0) -- call it with (1, 0, 0, 0, 1). You should put a validity check on the input values of sector and head, next, and return some kind of an error if they are bad.

Third, if that doesn't work, let's try changing this to an LBA routine, and see what happens. Just make a copy of read_sector, and change the name to read_LBAsector, or something. Leave all the inputs and everything the same for now. Just change the drive_head values to 0xE0, and 0xF0 (from A0 and B0). Maybe change the call from (1, 0, 0, 0, 1) to read_LBAsector(1, 0, 0, 0, 0), too -- but that's not necessary. The important question is whether you get all 0xff's as input again.

[Pyrofan1]

second, I think I see the problem. I looked again at the parameters that you are passing in. You say you are trying to access CHS = 0,0,1 -- but you are actually calling the function with CHS = 1,0,0 -- and a sector number of 0 IS ALWAYS ILLEGAL. Valid sector numbers are 1 to 63, usually. So, instead of calling with (1, 0, 1, 0, 0) -- call it with (1, 0, 0, 0, 1). You should put a validity check on the input values of sector and head, next, and return some kind of an error if they are bad.

that was the problem. thanks for all your help.