ATA driver problem

[WinExperements]

Hello, i have some problems with ATA drive detection, my detected drives has empty names and it's have incorrect count of sectors. Why?
It's my code for detection drives:

Code: Select all

int ata_device_detect(ata_device_t *dev) {
	ata_soft_reset(dev);
	ata_io_wait(dev);
	io_writePort(dev->base+ATA_REG_HDDEVSEL,0xA0 | dev->slave << 4);
	ata_io_wait(dev);
	ata_status_wait(dev,10000);
	unsigned char cl = io_readPort(dev->base + ATA_REG_LBA1);
	unsigned char ch = io_readPort(dev->base + ATA_REG_LBA2);
	if (ch == 0xff && ch == 0xff) {
		return 0;
	}
	if ((cl == 0x00 && ch == 0x00) || (cl == 0x3c && ch == 0xc3)) {
		ata_device_init(dev);
		int sectors = ata_max_offset(dev);
		if (sectors == 0) {
			// ATA must have at less 1 sector(512B)
			printf("ATA: device sectors are zero!\n");
			return 0; 
		}
		printf("ATA: detected drive with %x size! CL: %x, CH: %x\n",sectors,cl,ch);
	}
	return 0;
}
void ata_device_init(ata_device_t *dev) {
	io_writePort(dev->base+1,1);
	io_writePort(dev->ctrl,0);
	io_writePort(dev->base+ATA_REG_HDDEVSEL, 0xA0 | dev->slave << 4);
	ata_io_wait(dev);
	io_writePort(dev->base+ATA_REG_COMMAND,ATA_CMD_IDENTIFY);
	ata_io_wait(dev);
	io_readPort(dev->base+ATA_REG_COMMAND);
	ata_wait(dev,0);
	uint16_t *buf = (uint16_t *)&dev->identify;
	for (int i = 0; i < 256; i++) {
		buf[i] = io_readPortW(dev->base);
	}
	uint8_t *ptr = (uint8_t *)&dev->identify.model;
	for (int i = 0; i < 39; i+=2) {
		uint8_t tmp = ptr[i+1];
		ptr[i+1] = ptr[i];
		ptr[i] = tmp;
	}
}

[Octocontrabass]

Why?

Good question. What kind of debugging have you done so far?

Code: Select all

	if (ch == 0xff && ch == 0xff) {

Typo?

Code: Select all

	uint16_t *buf = (uint16_t *)&dev->identify;

Type punning through pointers is undefined behavior. (Type punning through unions is also undefined behavior, but GCC and Clang allow it as an extension.)

[WinExperements]

After some debugging, i can't understand why cl and ch always zero, but ATA devices didn't connected.
I upload the full source code here maybe you find any error, because i not fully understand where my error.

Code: Select all

#include <terminal.h>
#include<io.h>
#include<atapi/atapi.h>
#include<serial.h>
#include<dev.h>
#include<mm/pmm.h>
#include<interrupts.h>
// === Internal functions here ===
typedef struct {
	uint16_t flags;
	uint16_t unused1[9];
	char     serial[20];
	uint16_t unused2[3];
	char     firmware[8];
	char     model[40];
	uint16_t sectors_per_int;
	uint16_t unused3;
	uint16_t capabilities[2];
	uint16_t unused4[2];
	uint16_t valid_ext_data;
	uint16_t unused5[5];
	uint16_t size_of_rw_mult;
	uint32_t sectors_28;
	uint16_t unused6[38];
	uint64_t sectors_48;
	uint16_t unused7[152];
} __attribute__((packed)) ata_identify_t;
typedef struct {
	uint8_t reserved;
	uint8_t channel;
	uint8_t drive;
	uint8_t type;
	uint16_t signature;
	uint16_t sup;
	uint32_t cmd_sets;
	uint32_t size;
	char model[41];
	uint16_t base;
	uint16_t ctrl;
	uint16_t nein;
	int slave;
	ata_identify_t identify;
} ata_device_t;
#define ATA_SR_BSY     0x80
#define ATA_SR_DRDY    0x40
#define ATA_SR_DF      0x20
#define ATA_SR_DSC     0x10
#define ATA_SR_DRQ     0x08
#define ATA_SR_CORR    0x04
#define ATA_SR_IDX     0x02
#define ATA_SR_ERR     0x01

#define ATA_ER_BBK      0x80
#define ATA_ER_UNC      0x40
#define ATA_ER_MC       0x20
#define ATA_ER_IDNF     0x10
#define ATA_ER_MCR      0x08
#define ATA_ER_ABRT     0x04
#define ATA_ER_TK0NF    0x02
#define ATA_ER_AMNF     0x01

#define ATA_CMD_READ_PIO          0x20
#define ATA_CMD_READ_PIO_EXT      0x24
#define ATA_CMD_READ_DMA          0xC8
#define ATA_CMD_READ_DMA_EXT      0x25
#define ATA_CMD_WRITE_PIO         0x30
#define ATA_CMD_WRITE_PIO_EXT     0x34
#define ATA_CMD_WRITE_DMA         0xCA
#define ATA_CMD_WRITE_DMA_EXT     0x35
#define ATA_CMD_CACHE_FLUSH       0xE7
#define ATA_CMD_CACHE_FLUSH_EXT   0xEA
#define ATA_CMD_PACKET            0xA0
#define ATA_CMD_IDENTIFY_PACKET   0xA1
#define ATA_CMD_IDENTIFY          0xEC

#define ATAPI_CMD_READ       0xA8
#define ATAPI_CMD_EJECT      0x1B

#define ATA_IDENT_DEVICETYPE   0
#define ATA_IDENT_CYLINDERS    2
#define ATA_IDENT_HEADS        6
#define ATA_IDENT_SECTORS      12
#define ATA_IDENT_SERIAL       20
#define ATA_IDENT_MODEL        54
#define ATA_IDENT_CAPABILITIES 98
#define ATA_IDENT_FIELDVALID   106
#define ATA_IDENT_MAX_LBA      120
#define ATA_IDENT_COMMANDSETS  164
#define ATA_IDENT_MAX_LBA_EXT  200

#define IDE_ATA        0x00
#define IDE_ATAPI      0x01
 
#define ATA_MASTER     0x00
#define ATA_SLAVE      0x01

#define ATA_REG_DATA       0x00
#define ATA_REG_ERROR      0x01
#define ATA_REG_FEATURES   0x01
#define ATA_REG_SECCOUNT0  0x02
#define ATA_REG_LBA0       0x03
#define ATA_REG_LBA1       0x04
#define ATA_REG_LBA2       0x05
#define ATA_REG_HDDEVSEL   0x06
#define ATA_REG_COMMAND    0x07
#define ATA_REG_STATUS     0x07
#define ATA_REG_SECCOUNT1  0x08
#define ATA_REG_LBA3       0x09
#define ATA_REG_LBA4       0x0A
#define ATA_REG_LBA5       0x0B
#define ATA_REG_CONTROL    0x0C
#define ATA_REG_ALTSTATUS  0x0C
#define ATA_REG_DEVADDRESS 0x0D
ata_device_t ata_primary_master   = {.base = 0x1F0, .ctrl = 0x3F6, .slave = 0};
ata_device_t ata_primary_slave    = {.base = 0x1F0, .ctrl = 0x3F6, .slave = 1};
ata_device_t ata_secondary_master = {.base = 0x170, .ctrl = 0x376, .slave = 0};
ata_device_t ata_secondary_slave  = {.base = 0x170, .ctrl = 0x376, .slave = 1};
void ata_io_wait(ata_device_t *dev) {
	io_readPort(dev->base+ATA_REG_ALTSTATUS);
	io_readPort(dev->base+ATA_REG_ALTSTATUS);
	io_readPort(dev->base+ATA_REG_ALTSTATUS);
	io_readPort(dev->base+ATA_REG_ALTSTATUS);
}
int ata_status_wait(ata_device_t *dev,int timeout) {
	int status;
	if (timeout > 0) {
		int i =0;
		while((status = io_readPort(dev->base+ATA_REG_STATUS) & ATA_SR_BSY && (i < timeout))) i++;
	} else {
		while((status = io_readPort(dev->base+ATA_REG_STATUS)) & ATA_SR_BSY);
	}
	return status;	
}
int ata_wait(ata_device_t *dev,int advanced) {
	uint8_t status;
	ata_io_wait(dev);
	status = ata_status_wait(dev,-1); // wait forever!!
	if (advanced) {
		status = io_readPort(dev->base+ATA_REG_STATUS);
		if (status & ATA_SR_ERR) return true;
		if (status & ATA_SR_DF) return true;
		if (!(status & ATA_SR_DRQ)) return true;
	}
	return false;
}
void ata_soft_reset(ata_device_t *dev) {
	io_writePort(dev->ctrl,0x04);
	ata_io_wait(dev);
	io_writePort(dev->ctrl,0x00);
}
int ata_max_offset(ata_device_t *dev) {
	uint64_t sectors = dev->identify.sectors_48;
	if (!sectors) {
		sectors = dev->identify.sectors_28;
	}
	return sectors * 512;
}
// Initialization and detection code
void ata_device_init(ata_device_t *dev) {
	io_writePort(dev->base+1,1);
	io_writePort(dev->ctrl,0);
	io_writePort(dev->base+ATA_REG_HDDEVSEL, 0xA0 | dev->slave << 4);
	ata_io_wait(dev);
	io_writePort(dev->base+ATA_REG_COMMAND,ATA_CMD_IDENTIFY);
	ata_io_wait(dev);
	io_readPort(dev->base+ATA_REG_COMMAND);
	ata_wait(dev,0);
	uint16_t *buf = (uint16_t *)&dev->identify;
	for (int i = 0; i < 256; i++) {
		buf[i] = io_readPortW(dev->base);
	}
	uint8_t *ptr = (uint8_t *)&dev->identify.model;
	for (int i = 0; i < 39; i+=2) {
		uint8_t tmp = ptr[i+1];
		ptr[i+1] = ptr[i];
		ptr[i] = tmp;
	}
}
int ata_device_detect(ata_device_t *dev) {
	ata_soft_reset(dev);
	ata_io_wait(dev);
	io_writePort(dev->base+ATA_REG_HDDEVSEL,0xA0 | dev->slave << 4);
	ata_io_wait(dev);
	ata_status_wait(dev,10000);
	unsigned char cl = io_readPort(dev->base + ATA_REG_LBA1);
	unsigned char ch = io_readPort(dev->base + ATA_REG_LBA2);
	if (ch == 0xff && cl == 0xff) {
		return 0;
	}
	if ((cl == 0x00 && ch == 0x00) || (cl == 0x3c && ch == 0xc3)) {
		ata_device_init(dev);
		int sectors = ata_max_offset(dev);
		if (sectors == 0) {
			// ATA must have at less 1 sector(512B)
			printf("ATA: device sectors are zero!\n");
			return 0; 
		}
		printf("ATA: detected drive with %x size! CL: %x, CH: %x\n",sectors,cl,ch);
	}
	return 0;
}
// === Public functions here ===
void atapi_init() {
	printf("ATA device driver\n");
	printf("ATA: detection drives\n");
	ata_device_detect(&ata_primary_master);
	ata_device_detect(&ata_primary_slave);
	ata_device_detect(&ata_secondary_master);
	ata_device_detect(&ata_secondary_slave);
	printf("ATA: detection finished\n");
}

[Octocontrabass]

After some debugging,

Again, what kind of debugging? What did you check? What was correct, and what wasn't?

Code: Select all

#define ATA_REG_SECCOUNT1  0x08
#define ATA_REG_LBA3       0x09
#define ATA_REG_LBA4       0x0A
#define ATA_REG_LBA5       0x0B
#define ATA_REG_CONTROL    0x0C
#define ATA_REG_ALTSTATUS  0x0C
#define ATA_REG_DEVADDRESS 0x0D

It's a bad idea to copy someone else's code if you don't understand how it works.

[nullplan]

Type punning through pointers is undefined behavior. (Type punning through unions is also undefined behavior, but GCC and Clang allow it as an extension.)

The first statement is mostly correct, since it breaks the strict aliasing rule, unless you type pun to a character type (since pointers to character type are allowed to alias all objects).

The second is wrong in C. In C99, footnote 82 defines the behavior:

82) If the member used to access the contents of a union object is not the same as the member last used to
store a value in the object, the appropriate part of the object representation of the value is reinterpreted
as an object representation in the new type as described in 6.2.6 (a process sometimes called "type
punning"). This might be a trap representation.

An earlier draft still contained language saying this was implementation-defined, but that did not make it into the final C99 standard. And even that would explicitly contradict your assertion that it is undefined.

C11 retains the same footnote, although it is now footnote 95. C18 also retains the footnote, it is now footnote 97.

C++ appears to be more restrictive about that, although in the draft standard I looked at, they never straight up come out and say it. They say that at most one data member of a union can be active at any time, but never what that means. There is no sentence like "reads from a non-active data member are ill-formed" or anything. But C++ does not have the above footnote or anything like it.

[WinExperements]

Hello, after trying to run it in boсhs it gives the messages:

Code: Select all

read from port 0x0170 with len 1 returns 0xff

It's shows me it when the driver trying to read the identify structure here:

Code: Select all

for (int i = 0; i < 256; i++) {
                buf[i] = io_readPortW(dev->base);
        }

What i am doing wrong with it?

[Octocontrabass]

What i am doing wrong with it?

Your io_readPortW() function is reading a byte instead of a word.

[WinExperements]

Hello again, yeah my io_readPortW read byte instead of word, so after the fix, bochs shows me this error:

Code: Select all

IO read(0x0170) with drq == 0: last command was 00h

Does it mean that my driver reads from non inserted/non existing hard drive device or something else?
EDIT: How i can correctly check if drive not inserted because my checking not work and always returns true?

[Octocontrabass]

Does it mean that my driver reads from non inserted/non existing hard drive device or something else?

Enable logging for debug events and the log will tell you whether you've selected a nonexistent device.

How i can correctly check if drive not inserted because my checking not work and always returns true?

Send an IDENTIFY DEVICE command and use either the arrival of an IRQ or the contents of the (alternate) status register to determine whether the device responded. Hard drives will indicate they're ready to transfer data, optical drives will indicate the command aborted. If a device is not present, you won't receive a response.

[WinExperements]

Hello! So i trying to run this driver as module at boot, and i have this error in bochs:

Code: Select all

01846199354e[DMA   ] read: unsupported address=000c
01846199377e[DMA   ] read: unsupported address=000c

And this happends when the driver trying to initialize the drive, so what is this error and how to fix it?

[Octocontrabass]

That error means you're reading the I/O port at address 0xC.

You fix it by identifying the code that reads I/O port 0xC and determining why it's doing that.

[WinExperements]

Hello again! So I am trying to test the driver on real hardware, and when the driver trying to do pulling the drive always returns as status 0x50, also in the error register value are 0x4. What is this error and how to fix it? Yeah i read about ATA in wiki, but don't understand how to fix it.

[Octocontrabass]

The status register says the drive is ready.

The error register doesn't say anything because the status register says there was no error.

What did your driver do right before the status changed to this unexpected value?

[WinExperements]

What did your driver do right before the status changed to this unexpected value?

My driver do nothing, just waiting for status to be changed to DRQ like the tutorial on wiki, so maybe I incorrectly checks the status or something?

[Octocontrabass]

If your driver did nothing, that's the problem. You have to send a command that involves a data transfer before the drive will set DRQ.