ATA read/write sectors' mis-behavior

[mvt]

Hi, I'm a beginner in OSdev. I followed the barebone tutorial and used the grub bootloader to create a simple OS in an iso file
I've set up GDT, IDT, and PIC, which work fine

Now I want to have storage in my OS, so I used

Code: Select all

qemu-img create -f raw storage.img 4G

to create a disk image file, and used

Code: Select all

qemu-system-i386 -cdrom os.iso -drive file=storage.img,format=raw -boot d

to test my OS in qemu

I also followed this page
https://wiki.osdev.org/ATA_read/write_sectors#See_also
to write the read and write functions in c

However, the read function above doesn't read the disk properly
I used a binary editor to write the first 512 bytes(a sector) of my storage.img to 0x01
The output of the read function doesn't match the contents in the disk
Expected:
11111...
Outputed:
00001111...
Sometimes the 1s at the beginning would become 0es

The write function doesn't work in 99% of the time, as the wait_for_disk() function will reach the timeout, but once it doesn't reach the timeout it will work properly

Thanks for everyone who can help

Code: Select all

#ifndef FILE_SYSTEM_H
#define FILE_SYSTEM_H

#include <stdint.h>
#include "../../libraries/util/io.h"
#include "../../libraries/terminal/terminalio.h"

// ATA Ports
#define ATA_DATA_PORT 0x1F0
#define ATA_ERROR_PORT 0x1F1
#define ATA_SECTOR_COUNT_PORT 0x1F2
#define ATA_LBA_LOW_PORT 0x1F3
#define ATA_LBA_MID_PORT 0x1F4
#define ATA_LBA_HIGH_PORT 0x1F5
#define ATA_DRIVE_PORT 0x1F6
#define ATA_COMMAND_PORT 0x1F7
#define ATA_STATUS_PORT 0x1F7

// ATA Commands
#define ATA_CMD_READ_SECTORS 0x24
#define ATA_CMD_WRITE_SECTORS 0x34

// Buffer to hold the read sector data
// this will store the read and written info
static uint8_t data_buffer[512] = {0};

int wait_for_disk()
{
    uint32_t time = 0;
    // Poll for completion (you might want to implement a timeout mechanism)
    while ((inw(ATA_STATUS_PORT) & 0x80) != 0x80)
    {
        ++time;
        if(time > 1000000)//timeout
        {
            printf("failed\n");
            return 0;
        }
    }
    return 1;
}

void read_sector(uint32_t sector_number)
{
    outb(ATA_ERROR_PORT, 0);
    // Select drive (Assuming drive 0, replace with appropriate value if needed)
    // set 111 to enter LBA mode
    outb(ATA_DRIVE_PORT, 0xE0 | ((sector_number >> 24) & 0x0F));
    
    // Set sector count to 1
    outb(ATA_SECTOR_COUNT_PORT, 1);

    // Set LBA address
    outb(ATA_LBA_LOW_PORT, sector_number & 0xFF);
    outb(ATA_LBA_MID_PORT, (sector_number >> 8) & 0xFF);
    outb(ATA_LBA_HIGH_PORT, (sector_number >> 16) & 0xFF);

    // Issue the read command
    outb(ATA_COMMAND_PORT, ATA_CMD_READ_SECTORS);
    if(!wait_for_disk())
    {
        return;
    }

    // Read data from the data port
    for(int i = 0; i < 512; i += 2)
    {
        uint16_t temp = inw(ATA_DATA_PORT);
        data_buffer[i] = (uint8_t)temp;
        data_buffer[i + 1] = (uint8_t)(temp >> 8);
    }
}

void write_sector(uint32_t sector_number)
{
    // Select drive (Assuming drive 0, replace with appropriate value if needed)
    // set 111 to enter LBA mode
    outb(ATA_DRIVE_PORT, 0xE0 | ((sector_number >> 24) & 0x0F));

    // Set sector count to 1
    outb(ATA_SECTOR_COUNT_PORT, 1);

    // Set LBA address
    outb(ATA_LBA_LOW_PORT, sector_number & 0xFF);
    outb(ATA_LBA_MID_PORT, (sector_number >> 8) & 0xFF);
    outb(ATA_LBA_HIGH_PORT, (sector_number >> 16) & 0xFF);

    // Issue the read command
    outb(ATA_COMMAND_PORT, ATA_CMD_WRITE_SECTORS);
    if(!wait_for_disk())
    {
        return;
    }
    printf("?");

    //write the data from the data buffer
    for(int i = 0; i < 512; i += 2)
    {
        outw(ATA_DATA_PORT, (data_buffer[i] << 8) | (data_buffer[i + 1]));
    }
}

#endif

this is my inb, outb, inw, and outw implementation

Code: Select all

void outb(uint16_t port, uint8_t value) //implemented in asm
{
    __asm__ volatile(
        "out %0, %1"
        :
        : "a" (value), "Nd" (port)
    );
}

uint8_t inb(uint8_t port) //implemented in asm
{
    uint8_t result;
    __asm__ volatile(
        "in %w1, %0"
        : "=a" (result)
        : "Nd" (port)
    );
    return result;
}

uint16_t inw(uint16_t port) //implemented in asm
{
    uint16_t result;
    __asm__ volatile(
        "in %w1, %0"
        : "=a" (result)
        : "Nd" (port)
    );
    return result;
}

void outw(uint16_t port, uint16_t value) //implemented in asm
{
    __asm__ volatile(
        "out %0, %1"
        :
        : "a" (value), "Nd" (port)
    );
}

void io_wait()
{
    outb(0x80, 0);
}

[Octocontrabass]

Code: Select all

#define ATA_CMD_READ_SECTORS 0x24
#define ATA_CMD_WRITE_SECTORS 0x34

READ SECTORS is 0x20, not 0x24. WRITE SECTORS is 0x30, not 0x34. Fixing those might help a little bit.