[SOLVED] ATAPI Doesn't fire IRQ

[ishidex2]

I've been struggling with this issue for a while, for some reason ATAPI simply doesn't fire IRQ when packet is sent, when I try to read status register I get these flags: 01011000, which (probably?) means that its still accepting data, even though I sent everything. I used this article as a core for implementation: https://wiki.osdev.org/PCI_IDE_Controller


header:

Code: Select all

#define S2_ATA_BIT_BSY     0x80    // Busy
#define S2_ATA_BIT_DRDY    0x40    // Drive ready
#define S2_ATA_BIT_DEVF    0x20    // Drive write fault
#define S2_ATA_BIT_DSC     0x10    // Drive seek complete
#define S2_ATA_BIT_DRQ     0x08    // Data request ready
#define S2_ATA_BIT_CORR    0x04    // Corrected data
#define S2_ATA_BIT_IDX     0x02    // Index
#define S2_ATA_BIT_ERR     0x01    // Error

#define S2_ATA_REG_DATA       0x00
#define S2_ATA_REG_ERROR      0x01
#define S2_ATA_REG_FEATURES   0x01
#define S2_ATA_REG_SECCOUNT0  0x02
#define S2_ATA_REG_LBA0       0x03
#define S2_ATA_REG_LBA1       0x04
#define S2_ATA_REG_LBA2       0x05
#define S2_ATA_REG_HDDEVSEL   0x06
#define S2_ATA_REG_COMMAND    0x07
#define S2_ATA_REG_STATUS    0x07
#define S2_ATA_REG_CONTROL    0x206
#define S2_ATA_REG_ALTSTATUS    0x206

#define S2_ATAERR_NOERR 0
#define S2_ATAERR_ERR 0
#define S2_ATAERR_DEVF 1
#define S2_ATAERR_DRQ 2

#define S2_ATA_IERR_BBK      0x80    // Bad block
#define S2_ATA_IERR_UNC      0x40    // Uncorrectable data
#define S2_ATA_IERR_MC       0x20    // Media changed
#define S2_ATA_IERR_IDNF     0x10    // ID mark not found
#define S2_ATA_IERR_MCR      0x08    // Media change request
#define S2_ATA_IERR_ABRT     0x04    // Command aborted
#define S2_ATA_IERR_TK0NF    0x02    // Track 0 not found
#define S2_ATA_IERR_AMNF     0x01    // No address mark

#define S2_ATA_TYPE_ATAPI 0
#define S2_ATA_TYPE_ATA 1

#define S2_ATAPI_SECTOR_SIZE 2048

typedef struct
{
    bool isPrimary;
    s2_UInt16 base;
    s2_UInt16 busMaster;
    s2_UInt8 nIEN;
} s2_IDEChannel;

typedef struct
{
    s2_UInt8 type;
    s2_IDEChannel *channel;
    bool exists;
    bool isMaster;
    s2_UInt16 signature;
    s2_UInt16 features;
    s2_UInt32 commands;
    s2_UInt32 capacity;
    s2_Byte model[41];
} s2_IDEDevice;

c file (without identify function (should I post it here?)):

Code: Select all

s2_Byte s2_atapiPacket[12];

s2_UInt8 s2_IDERead(s2_IDEDevice *dev, s2_UInt8 offset)
{   
    return s2_InB(dev->channel->base+offset);
}

void s2_IDEWrite(s2_IDEDevice *dev, s2_UInt16 offset, s2_UInt16 data)
{

    // Data
    if (offset == 0)
        s2_OutW(dev->channel->base, data);
    else
        s2_OutB(dev->channel->base+offset, data);
    
}

s2_UInt8 s2_IDEPoll(s2_IDEDevice *dev)
{
    for (int i = 0; i < 4; i++)
        s2_IDERead(dev->channel->base, S2_ATA_REG_ALTSTATUS);

    while (s2_IDERead(dev->channel->base, S2_ATA_REG_STATUS) & S2_ATA_BIT_BSY) {};

    s2_UInt8 state = s2_IDERead(dev->channel->base, S2_ATA_REG_STATUS);
    
    if (state & S2_ATA_BIT_ERR)
        return S2_ATAERR_ERR;

    if (state & S2_ATA_BIT_DRQ)
        return S2_ATAERR_ERR;

    if (state & S2_ATA_BIT_DEVF)
        return S2_ATAERR_DEVF;

    return S2_ATAERR_NOERR;
}
s2_UInt8 s2_IDEATAPIRead(s2_IDEDevice *dev, s2_UInt32 lba, s2_Byte *buffer)
{
    s2_UInt8 error;
    s2_IDEWrite(dev, S2_ATA_REG_CONTROL, dev->channel->nIEN = ataLock = 0);

    s2_atapiPacket[ 0] = 0xA8; // Read packet command
    s2_atapiPacket[ 1] = 0x0;
    s2_atapiPacket[ 2] = (lba >> 24) & 0xFF;
    s2_atapiPacket[ 3] = (lba >> 16) & 0xFF;
    s2_atapiPacket[ 4] = (lba >> 8) & 0xFF;
    s2_atapiPacket[ 5] = (lba >> 0) & 0xFF;
    s2_atapiPacket[ 6] = 0x0;
    s2_atapiPacket[ 7] = 0x0;
    s2_atapiPacket[ 8] = 0x0;
    s2_atapiPacket[ 9] = 1; // Number of sectors
    s2_atapiPacket[10] = 0x0;
    s2_atapiPacket[11] = 0x0;

    s2_IDEWrite(dev, S2_ATA_REG_HDDEVSEL, dev->isMaster ? (0xA0) : (0xB0));

    // Delay
    for(int i = 0; i < 4; i++)
       s2_IDERead(dev, S2_ATA_REG_ALTSTATUS);

    // PIO mode
    s2_IDEWrite(dev, S2_ATA_REG_FEATURES, 0);

    // Size of buffer
    s2_IDEWrite(dev, S2_ATA_REG_LBA1, S2_ATAPI_SECTOR_SIZE & 0xFF);
    s2_IDEWrite(dev, S2_ATA_REG_LBA2, S2_ATAPI_SECTOR_SIZE >> 8);

    // Send packet
    s2_IDEWrite(dev, S2_ATA_REG_COMMAND, 0xA0); // Packet command

    if (error = s2_IDEPoll(dev)) return error;

    s2_TVMPrintA("Status register value:");
    s2_TVMPrintA(s2_ToHex(s2_IDERead(dev, S2_ATA_REG_STATUS)));

    asm volatile("rep   outsw" : : "c"(6), "d"(dev->channel->base), "S"(s2_atapiPacket));   // Send Packet Data

    // Wait for irq
    while (!ataLock) {}
    ataLock = false;

    s2_TVMPrintA("got here");
    if (error = s2_IDEPoll(dev)) return error;

}

My irq handler looks like this

Code: Select all

void irq14_handler(void) 
{
    ataLock = true;
    s2_TVMPrint("sfdasd", 0x70, 0); // Debug
    s2_OutB(0xA0, 0x20);
    s2_OutB(0x20, 0x20); //EOI
}

[Klakap]

Are you sure that your ATAPI fires irq14? On most cases, it fire irq15.

[iansjack]

Irq15 is the standard for the secondary bus. 14 is correct for the primary.

[ishidex2]

Irq15 is the standard for the secondary bus. 14 is correct for the primary.

Yes i copied the exact same code for irq 15 and it still doesn't work, one thing I figured out is that irq14 actually fires, but irq 15 doesn't (i.e. when i try to call function on primary bus irq14 works, but when i put secondary irq15 doesn't work)

[iansjack]

Silly question but you have actually unmasked the interrupt when setting up the PIC?

[ishidex2]

Silly question but you have actually unmasked the interrupt when setting up the PIC?

EOI?

[ishidex2]

Silly question but you have actually unmasked the interrupt when setting up the PIC?

EOI?

I added unmasking code but it still doesn't work

[Octocontrabass]

Code: Select all

    asm volatile("rep   outsw" : : "c"(6), "d"(dev->channel->base), "S"(s2_atapiPacket));   // Send Packet Data

This code is wrong. You must specify corresponding output operands for the input registers that get modified (ECX and ESI), and you must add the "volatile" keyword to prevent GCC from throwing the whole thing away after you fix the missing output operands. I believe you also need a memory clobber to tell GCC that you're accessing the contents of the array.

[ishidex2]

Code: Select all

    asm volatile("rep   outsw" : : "c"(6), "d"(dev->channel->base), "S"(s2_atapiPacket));   // Send Packet Data

This code is wrong. You must specify corresponding output operands for the input registers that get modified (ECX and ESI), and you must add the "volatile" keyword to prevent GCC from throwing the whole thing away after you fix the missing output operands. I believe you also need a memory clobber to tell GCC that you're accessing the contents of the array.

I don't quite understand that, I thought and esi are already set by "S" and "d"? And rep just repeats the instruction?

[ishidex2]

I replaced that with simple chain of OutW and it doesn't help

Code: Select all

   OutW(bus, ATAPI_CMD_READ<<8);
   OutW(bus, 0);
   OutW(bus, 0);
   OutW(bus, 0);
   OutW(bus, 0);
   OutW(bus, 0);
   OutW(bus, 0);
   OutW(bus, 0);
   OutW(bus, 0);
   OutW(bus, 1);
   OutW(bus, 0);
   OutW(bus, 0);

[BenLunt]

When the issue is interrupts and the IDE, this is usually and most likely an issue with reading (or not reading) the STATUS register.

With your code:

Code: Select all

for (int i = 0; i < 4; i++)
        s2_IDERead(dev->channel->base, S2_ATA_REG_ALTSTATUS);

    while (s2_IDERead(dev->channel->base, S2_ATA_REG_STATUS) & S2_ATA_BIT_BSY) {};

You are reading the Alternate STATUS register with the first four reads and then the actual status register in the Poll loop.

You should *always* read the Alternate Status register and *never* read the Actual Status register, only reading the Actual Status register to clear the Interrupt State of the controller. Period.

Also, depending on your compiler options and the capabilities of the optimizer, it might optimize all of that code above out anyway. About the only way to ensure that it does not, is to use the Volatile keyword, or make the s2_IDERead() function an external function.

Anyway, never read the Actual Status register until after you have had the interrupt. In other words, only read it in your IRQ. Everywhere else, read the Alternate Status register.

Ben
- http://www.fysnet.net/media_storage_devices.htm

[ishidex2]

No way! It works! After 6 days of struggle. Though I removed reading status from identify, how do I poll it for busy? I have PITSleep function and it works, but curious if there's another way.

[Octocontrabass]

I don't quite understand that, I thought and esi are already set by "S" and "d"? And rep just repeats the instruction?

Yes, but your "c" and "d" and "S" are input operands, and GCC assumes input operands do not change. REP OUTSW modifies ECX and ESI, so you need to add output operands so GCC knows that those registers will be changed.

[ishidex2]

One thing I found: The irq only fires on Identify function, I've replaced all STATUS refs to ALTSTATUS and it indeed fires on start (if you don't set nIEN), but on read it doesn't fire anything, here's my defines

Code: Select all

#define S2_ATA_REG_CONTROL    0x206
#define S2_ATA_REG_ALTSTATUS  0x206

[Octocontrabass]

I've replaced all STATUS refs to ALTSTATUS

You need to read STATUS once after each IRQ you receive. A good place to do it is in your IRQ handler.