Questions about the implementation of ata read/write directive

Question about which tools to use, bugs, the best way to implement a function, etc should go here. Don't forget to see if your question is answered in the wiki first! When in doubt post here.
Post Reply
protegee6155
Posts: 24
Joined: Tue Dec 16, 2025 4:17 am

Questions about the implementation of ata read/write directive

Post by protegee6155 »

Hi, In most implementations of the ATA 28-bit read/write commands that I’ve come across, multiple requests are issued for reading one sector at a time, rather than sending a single command with a sector count of X.

Is there a particular reason for this approach?

I tried implementing the latter method (using a multi-sector request), but I’m running into an issue where the second IRQ (indicating the drive is ready for the next transfer) never arrives - although this seems to happen only in my read implementation.

Would anyone be able to spot a possible issue in my implementation or explain why single-sector requests are more commonly used?

Code: Select all

/* =========================================================
                        MACROS
   ========================================================= */

/**
 * Macro: ATA_SELECT_DELAY
 * -------------------------------------
 * Performs 4 dummy reads from the ATA status port to allow
 * the drive time to switch between Master/Slave selection.
 * Required by ATA hardware timing rules.
 */
#define ATA_SELECT_DELAY()                    \
    do {                                      \
        inb(ATA_PRIMARY_IO + ATA_REG_STATUS);  \
        inb(ATA_PRIMARY_IO + ATA_REG_STATUS);  \
        inb(ATA_PRIMARY_IO + ATA_REG_STATUS);  \
        inb(ATA_PRIMARY_IO + ATA_REG_STATUS);  \
    } while (0)

/**
 * Macro: WAIT_FOR_ATA_RESPONSE_TO_BE_DONE
 * -------------------------------------
 * Halts the CPU with `hlt` until the IRQ handler signals
 * that the current ATA request has finished processing.
 */
#define WAIT_FOR_ATA_RESPONSE_TO_BE_DONE()     \
    while (!ata_responce.done) {               \
        asm volatile("hlt");                   \
    }

/* =========================================================
                    REQUEST / RESPONSE STRUCTS
   ========================================================= */

/**
 * Struct: ata_request_struct
 * -------------------------------------
 * Holds a pending ATA command request.
 * Supports 28-bit LBA Read and Write operations.
 */
typedef struct ata_request_struct {
    volatile device_id_t device_id; // Target ATA device (primary/secondary, master/slave)
    volatile uint8_t pending;       // 1 = request active, 0 = idle
    volatile uint8_t request_type;  // Command type (identify, read, write)

    union {
        volatile struct {
            uint32_t sector_address; // LBA28 address to read from
            uint32_t sector_count;   // Number of sectors to read
        } read;

        volatile struct {
            uint32_t sector_address; // LBA28 address to write to
            size_t   sector_count;   // Number of sectors to write
            void    *data;           // Pointer to data (must be sector_count * 512 bytes)
        } write;
    } specific;
} ata_request_t;


/**
 * Struct: ata_responce_struct
 * -------------------------------------
 * Holds the ATA device response after IRQ handling.
 */
typedef struct ata_responce_struct {
    volatile device_id_t device_id; // Source device of the last handled response
    volatile uint8_t done:4;        // 1 = request handled and finished
    volatile uint8_t error:4;       // 1 = request resulted in error

    union {
        volatile struct {
            uint32_t sector_index;  // Which sector was just read in this IRQ cycle
            uint32_t sectors_read;  // The amount of sectors already read
            uint8_t *buffer;        // Memory destination for disk → RAM data
        } read_resp;

        volatile struct {
            uint8_t sector_write_done; // Set by IRQ after one sector was written
            uint32_t  sectors_written; // The amount of sectores that were written
        } write_resp;
    } specific;
} ata_responce_t;

/* Global static functions */
static uint32_t ata_response_handler(cpu_status_t *regs);

/* Global driver request/response state */
ata_request_t  ata_request;
ata_responce_t ata_responce;

/* =========================================================
                        WAIT HELPERS
   ========================================================= */

void ata_wait_bsy(ata_drive_t *drv) {
    while (inb(drv->device_id.ctrl_base) & ATA_SR_BSY)
        asm volatile("hlt");
}

void ata_wait_drq(ata_drive_t *drv) {
    while (!(inb(drv->device_id.ctrl_base) & ATA_SR_DRQ))
        asm volatile("hlt");
}

Code: Select all

/**
 * Function: ata_handle_write_single_sector_responce
 * -------------------------------------
 * Marks that a sector write operation completed.
 */
static uint8_t ata_handle_write_single_sector_responce() {
    ata_responce.specific.write_resp.sector_write_done = 1;
    return 0;
}

uint8_t ata_write28_request(ata_drive_t *drive, uint32_t sector, uint8_t count, uint8_t *buffer) {
    ata_request.device_id  = drive->device_id;
    ata_request.pending    = 0;
    ata_request.request_type = ATA_CMD_WRITE_PIO;
    ata_request.specific.write.sector_address = sector;
    ata_request.specific.write.sector_count   = count;
    ata_request.specific.write.data           = buffer;

    ata_responce.device_id  = drive->device_id;
    ata_responce.done       = 0;
    ata_responce.error      = 0;
    ata_responce.specific.write_resp.sector_write_done = 0;
    ata_responce.specific.write_resp.sectors_written   = 0;

    // Select device + LBA bits
    outb(drive->device_id.io_base + ATA_REG_HDDEVSEL, 0xE0 | (drive->device_id.master<<4) | ((sector>>24)&0x0F));
    outb(drive->device_id.io_base + ATA_REG_FEATURES, 0);  // send Null (0) to the feature register (don't know why)
    outb(drive->device_id.io_base + ATA_REG_SECCOUNT, count);
    outb(drive->device_id.io_base + ATA_REG_LBA_LOW,  sector & 0xFF);
    outb(drive->device_id.io_base + ATA_REG_LBA_MID, (sector >> 8) & 0xFF);
    outb(drive->device_id.io_base + ATA_REG_LBA_HIGH,(sector >>16) & 0xFF);

    // Send WRITE command
    ata_request.pending = 1;
    ATA_SELECT_DELAY();
    outb(drive->device_id.io_base + ATA_REG_COMMAND, ATA_CMD_WRITE_PIO);

    // Write sector data (256 words per sector)
    for (uint32_t s = 0; s < count; s++) {
        ata_responce.specific.write_resp.sector_write_done = 0;

        /* wait for drive to be ready to accept data for this sector */
        ata_wait_bsy(drive);
        ata_wait_drq(drive);

        uint16_t *src = (uint16_t*)buffer + (ATA_SECTOR_SIZE / 2)*s;
        for (uint32_t i = 0; i < (ATA_SECTOR_SIZE / 2); i++) {
            outw(drive->device_id.io_base + ATA_REG_DATA, src[i]);
            asm volatile("jmp .+2"); // short mandatory delay
        }

        // Wait for IRQ to mark sector done
        while (!ata_responce.specific.write_resp.sector_write_done)
             asm volatile("hlt");
    }

    return ata_responce.error;
}

/* =========================================================
                   MAIN IRQ RESPONSE HANDLER
   ========================================================= */

/**
 * Function: ata_response_handler
 * -------------------------------------
 * Unified interrupt handler for all ATA requests.
 *   - Verifies a request is pending
 *   - Calls the correct sub-handler
 *   - Signals completion to waiting caller
 *   - Sends EOI to the :contentReference[oaicite:0]{index=0}
 */
static uint32_t ata_response_handler(cpu_status_t *regs) {
    uint8_t st;
    uint32_t err = -ENO;

    if (!ata_request.pending) {
        printf("ATA: IRQ triggered but no request was pending!\n");
        err = -EIRQ;
        goto irq_end;
    }

    switch (ata_request.request_type) {
        case ATA_CMD_IDENTIFY:
            ata_request.pending = 0;
            ata_responce.error = ata_handle_identify_responce();
            ata_responce.done = 1;
            break;

        case ATA_CMD_READ_PIO:
            ata_responce.error = ata_handle_read_single_sector_responce();
            if (ata_responce.error) {
                ata_request.pending = 0;
                ata_responce.done = 1;
                break;
            }

            ata_responce.specific.read_resp.sectors_read++;

            if (ata_responce.specific.read_resp.sectors_read >=
                ata_request.specific.read.sector_count) {
                ata_request.pending = 0;
                ata_responce.done = 1;
            } else {
                ata_request.pending = 1;
                ata_responce.done = 0;
            }
            break;

        case ATA_CMD_WRITE_PIO:
            ata_handle_write_single_sector_responce();

            ata_responce.specific.write_resp.sectors_written++;  /* need to add this field */

            if (ata_responce.specific.write_resp.sectors_written >=
                ata_request.specific.write.sector_count) {
                ata_request.pending = 0;
                ata_responce.done   = 1;
            } else {
                ata_request.pending = 1;
                ata_responce.done   = 0;
            }
            break;

        case ATA_CMD_FLUSH:
            ata_request.pending = 0;
            st = inb(ata_request.device_id.io_base + ATA_REG_STATUS);
            if (st & ATA_SR_ERR)
                ata_responce.error = 1;
            ata_responce.done = 1;
            break;

        default:
            ata_request.pending = 0;
            printf("ATA Drive Error: Unknown command\n");
            err = -EIRQ;
            ata_responce.done = 1;
            break;
    }

irq_end:
    /* Review: we should probably not send the pic eoi here */
    outb(PIC2_COMMAND, PIC_EOI);
    outb(PIC1_COMMAND, PIC_EOI);
    return err;
}
Octocontrabass
Member
Member
Posts: 6248
Joined: Mon Mar 25, 2013 7:01 pm

Re: Questions about the implementation of ata read/write directive

Post by Octocontrabass »

protegee6155 wrote: Sun Apr 26, 2026 8:14 amIs there a particular reason for this approach?
It's the easiest to implement. Reading or writing more than one sector at a time is an optimization.
protegee6155 wrote: Sun Apr 26, 2026 8:14 amWould anyone be able to spot a possible issue in my implementation or explain why single-sector requests are more commonly used?
I don't see where you read the drive's status register to acknowledge the IRQ.

Also, your drive selection delay needs to happen immediately after the register write that selects the drive. (...If your hardware needs that delay at all. It might only be necessary on really old PCs.)
protegee6155
Posts: 24
Joined: Tue Dec 16, 2025 4:17 am

Re: Questions about the implementation of ata read/write directive

Post by protegee6155 »

Code: Select all

uint8_t ata_write28_request(ata_drive_t *drive, uint32_t sector, uint8_t count, uint8_t *buffer) {
    ata_request.device_id  = drive->device_id;
    ata_request.pending    = 0;
    ata_request.request_type = ATA_CMD_WRITE_PIO;
    ata_request.specific.write.sector_address = sector;
    ata_request.specific.write.sector_count   = count;
    ata_request.specific.write.data           = buffer;

    ata_responce.device_id  = drive->device_id;
    ata_responce.done       = 0;
    ata_responce.error      = 0;
    ata_responce.specific.write_resp.sector_write_done = 0;
    ata_responce.specific.write_resp.sectors_written   = 0;

    // Select device + LBA bits
    outb(drive->device_id.io_base + ATA_REG_HDDEVSEL, 0xE0 | (drive->device_id.master<<4) | ((sector>>24)&0x0F));
    outb(drive->device_id.io_base + ATA_REG_FEATURES, 0);  // send Null (0) to the feature register (don't know why)
    outb(drive->device_id.io_base + ATA_REG_SECCOUNT, count);
    outb(drive->device_id.io_base + ATA_REG_LBA_LOW,  sector & 0xFF);
    outb(drive->device_id.io_base + ATA_REG_LBA_MID, (sector >> 8) & 0xFF);
    outb(drive->device_id.io_base + ATA_REG_LBA_HIGH,(sector >>16) & 0xFF);

    // Send WRITE command
    ata_request.pending = 1;
    ATA_SELECT_DELAY();
    outb(drive->device_id.io_base + ATA_REG_COMMAND, ATA_CMD_WRITE_PIO);

    // Write sector data (256 words per sector)
    for (uint32_t s = 0; s < count; s++) {
        ata_responce.specific.write_resp.sector_write_done = 0;

        /* wait for drive to be ready to accept data for this sector */
        ata_wait_bsy(drive);
        ata_wait_drq(drive);

        uint16_t *src = (uint16_t*)buffer + (ATA_SECTOR_SIZE / 2)*s;
        for (uint32_t i = 0; i < (ATA_SECTOR_SIZE / 2); i++) {
            outw(drive->device_id.io_base + ATA_REG_DATA, src[i]);
            asm volatile("jmp .+2"); // short mandatory delay
        }
        printf("%d\n", s);
        // Wait for IRQ to mark sector done
        while (!ata_responce.specific.write_resp.sector_write_done)
             asm volatile("hlt");
    }

    return ata_responce.error;
}
do you mean something like this? that still don't work, i would also say that testing it again (with the added row of printf("%d\n", s), i can verify that i indeed get stuck on s=1 in the while after that row).
Octocontrabass
Member
Member
Posts: 6248
Joined: Mon Mar 25, 2013 7:01 pm

Re: Questions about the implementation of ata read/write directive

Post by Octocontrabass »

protegee6155 wrote: Mon Apr 27, 2026 12:44 amdo you mean something like this?
You didn't change anything.
Post Reply