Problem with reading from ata[solved]

[Klakap]

Good day,
I have problem. My method for reading ata doesnt work. It stuck in command "while ( ( (inb(0x1F7) & ATA_SR_DRQ)==1 ) ) {}" . I have detected hard disc on primary master. I use Virtualbox. Please why virtualbox doesnt respond data from hard disc?

Code: Select all

void ata_sector(uint16_t base, uint8_t type, uint64_t addr, uint8_t drive) {
    uint16_t tmpword=0;

    ata_base=base;

    ata_error=UNDETECTED;

    drive=inb(ata_base+ATA_PORT_STATUS);
    if(drive==0xFF) {
        ata_error=DETECTED; 
        tp("ata is undetected");
        return;
    }

    tmpword=inb(ata_base+ATA_PORT_STATUS);
    if(drive==ATA_MASTER) {
    outb(ata_base+ATA_PORT_DRV, 0xE0);
    }
    else { // ATA_SLAVE
    outb(ata_base+ATA_PORT_DRV, 0xF0);
    }

    if ( ( (inb(ATA_PORT_STATUS) & ATA_SR_ERR)==1 ) ) {  //error?
        tp("ata error 2"); 
        return;
    }

    //PIO28
    outb(ata_base+ATA_PORT_FEATURES, 0x00);  
    outb(ata_base+ATA_PORT_SCT_COUNT, 0x01);  
    outb(ata_base+ATA_PORT_SCT_NUMBER, (unsigned char)addr);  
    outb(ata_base+ATA_PORT_CYL_LOW, (unsigned char)(addr >> 8));  
    outb(ata_base+ATA_PORT_CYL_HIGH, (unsigned char)(addr >> 16)); 

    if(type==ATA_READ) {
    outb(ata_base+ATA_PORT_COMMAND, ATA_CMD_READ_PIO);  // Send command
    }
    else {
    outb(ata_base+ATA_PORT_COMMAND, ATA_CMD_WRITE_PIO);
    }

    // waiting for data
    while ( ( (inb(0x1F7) & ATA_SR_DRQ)==1 ) ) {}
    
    for (uint8_t idx = 0; idx < 255; idx++)
    {
        if(type==ATA_READ) {
            buffer[idx] = inw(ata_base + ATA_PORT_DATA);
        }
        else {
            tmpword=(unsigned short)buffer[idx];
            outw(ata_base + ATA_PORT_DATA, tmpword);
        }
    }
}

I am reading data with command "ata_sector(0x1F0, ATA_READ, 1, ATA_MASTER);" Please help me.

PS sorry for my bad english

[zity]

Hi Klakap,

This while loop does not make any sense:

Code: Select all

while ( ( (inb(0x1F7) & ATA_SR_DRQ)==1 ) ) {}

ATA_SR_DRQ is bit 3 (i.e. 0x08) which means that "inb(0x1F7) & ATA_SR_DRQ" either returns 0x00 or 0x08, but never one. For this reason, the while loop will always exit immediately.

In fact, you make this mistake several times in the posted code.

[nullplan]

In addition to what ziti pointed out, it is generally a bad idea to have busy wait loops without any kind of relaxation and timeout. I'm thinking something like

Code: Select all

uint64_t to = clock() + ticks_from_ms(1000);
while ((inb(0x1f7) & ATA_SR_DRQ) && clock() < to) {
  asm("pause");
}

Of course, now you will have to check if DRQ is actually gone after the loop. Or you put the above in a function and formulate it a bit differently

Code: Select all

int wait_for_drq0(int port) {
uint64_t to = clock() + ticks_from_ms(1000);
for (;;) {
  if (!(inb(port) & ATA_SR_DRQ)) return 0;
  if (clock >= to) return -ETIMEDOUT;
  asm ("pause");
}

[Klakap]

Very thank you for help. But I have next problem. I make little change in my code(I am using emulator, not real hardware, then now I dont make timeout code):

Code: Select all

void ata_sector(uint16_t base, uint8_t type, uint64_t addr, uint8_t drive) {
    uint16_t tmpword=0;

    if(drive==ATA_MASTER) {
    outb(base+ATA_PORT_DRV, 0xE0);
    }
    else { // ATA_SLAVE
    outb(base+ATA_PORT_DRV, 0xF0);
    }

    if ( ( (inb(base + ATA_PORT_STATUS) & ATA_SR_ERR)==ATA_SR_ERR ) ) { 
        tp("ata error 2"); 
        return;
    }

    //PIO28
    outb(base+ATA_PORT_FEATURES, 0x00); 
    outb(base+ATA_PORT_SCT_COUNT, 0x01); 
    outb(base+ATA_PORT_SCT_NUMBER, (unsigned char)addr);
    outb(base+ATA_PORT_CYL_LOW, (unsigned char)(addr >> 8));
    outb(base+ATA_PORT_CYL_HIGH, (unsigned char)(addr >> 16));
    //type
    if(type==ATA_READ) {
    outb(base+ATA_PORT_COMMAND, ATA_CMD_READ_PIO);  // Send command
    }
    else {
    outb(base+ATA_PORT_COMMAND, ATA_CMD_WRITE_PIO);
    }

    //wait
    while (!(inb(0x1F7) & 0x08)) {}

    for (uint8_t idx = 0; idx < 255; idx++)
    {
        if(type==ATA_READ) {
            buffer[idx] = inw(base + ATA_PORT_DATA);
        }
        else {
            tmpword=(unsigned short)buffer[idx];
            outw(base + ATA_PORT_DATA, tmpword);
        }
    }

}

But the output is still 256. I try start thic code in Bochs and in terminal is lot of

Code: Select all

00095230497e[DEV  ] write to port 0x01f0 with len 1 ignored

in write and in read:

Code: Select all

00095230869e[DEV  ] read from port 0x01f0 with len 1 returns 0xff

Please, what is wrong? I dont see any error in my code, but emulators dont write or read from hard disc image.

[Octocontrabass]

Your "inw" and "outw" functions are wrong. They should be using words, but they're using bytes.

[Klakap]

Thank you for answer, but Virtualbox now still send 0 (I was try something write and nothing happend) and in Bochs is output 255 and in terminal is the same as I write.

[Octocontrabass]

If Bochs is still displaying those errors, it means your "inw" and "outw" functions are still wrong.

[Klakap]

But I am change "inw" and "outw" on "inb" and "outb".

[Octocontrabass]

I think you've misunderstood something. You should be using "inw" and "outw" to read/write to ATA_PORT_DATA. The problem is your "inw" and "outw" don't work.

[Klakap]

My "inw" and "outw" code:

Code: Select all

static inline uint16_t inw(uint16_t port)
{
    uint16_t ret;
    asm volatile ( "inb %1, %0"
                   : "=a"(ret)
                   : "Nd"(port) );
    return ret;
}

static inline void outw(uint16_t port, uint16_t val)
{
    asm volatile ( "outb %0, %1" : : "a"(val), "Nd"(port) );
}

Please what is wrong?

//EDIT: Bochs send to data port number 511

[Octocontrabass]

Code: Select all

    asm volatile ( "inb %1, %0"
    asm volatile ( "outb %0, %1" : : "a"(val), "Nd"(port) );

Shouldn't those be "inw" and "outw"?

[Klakap]

Very thank you! Now, Bochs dont respond any messages, but I cant write anything. Output is still 0. Too same in qemu. Please whats wrong?

//EDIT: Virtualbox output is strange:

[BenLunt]

Hi,

Code: Select all

    //wait
    while (!(inb(0x1F7) & 0x08)) {}

Here you are waiting for the DRQ bit to become set. What if it never becomes set? You will wait here for a long time...

Code: Select all

    for (uint8_t idx = 0; idx < 255; idx++)
    {
        if(type==ATA_READ) {
            buffer[idx] = inw(base + ATA_PORT_DATA);
        }
        else {
            tmpword=(unsigned short)buffer[idx];
            outw(base + ATA_PORT_DATA, tmpword);
        }
    }

There are a lot of errors with that block of code. Is uint8_t an 8-bit unsigned byte? If so, idx will never get to the last word you want to write (or read). If you check for below 255, the last word is ignored. The controller will always wait for that last word to be read/written unless you issue another command or a reset. Changing to:

Code: Select all

    for (uint8_t idx = 0; idx < 256; idx++)

will cause a lot more problems than you think. Hint: it is okay to use machine-sized integers within 'for' loops, i.e.: the 'int' keyword.

Also, what type (and size) is buffer? If buffer is a 16-bit word sized, then your read is okay, and your write is okay too, but why the tmpword? With the tmpword, this tells me that your buffer is probably a byte sized buffer and if so, you got all kinds of problems. First, on the read, you overwrite the high byte of every previous word written. Second, on the read (and write) you point to the next indexed byte, not word.

What size is your buffer? When I ask size, what size is an element of that buffer? Byte, Word, DWord, etc.?

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

[Klakap]

Very thank you for your advice! I am change code and Bochs now working good without any error. I am very pleased with it. I have only one question. When I start this code in qemu or Virtualbox, first ata read/write command work good. Second command is stuck on while. Please why?

My code:

Code: Select all

uint16_t buffer[255];

void ata_sector(uint16_t base, uint8_t type, uint64_t addr, uint8_t drive) {
    if(drive==ATA_MASTER) {
    outb(base+ATA_PORT_DRV, 0xE0);
    }
    else { // ATA_SLAVE
    outb(base+ATA_PORT_DRV, 0xF0);
    }

    if ( ( (inb(base + ATA_PORT_STATUS) & ATA_SR_ERR)==ATA_SR_ERR ) ) { 
        tp("ata error 2"); 
        return;
    }

    //PIO28
    outb(base+ATA_PORT_FEATURES, 0x00);
    outb(base+ATA_PORT_SCT_COUNT, 0x01);
    outb(base+ATA_PORT_SCT_NUMBER, (unsigned char)addr);
    outb(base+ATA_PORT_CYL_LOW, (unsigned char)(addr >> 8));
    outb(base+ATA_PORT_CYL_HIGH, (unsigned char)(addr >> 16));
    //type
    if(type==ATA_READ) {
    outb(base+ATA_PORT_COMMAND, 0x20);  // Send command
    }
    else {
    outb(base+ATA_PORT_COMMAND, 0x30);
    }

    //wait
    while (!(inb(base+ATA_PORT_STATUS) & 0x08)) {}

    for (int idx = 0; idx < 256; idx++)
    {
        if(type==ATA_READ) {
            buffer[idx] = inw(base + ATA_PORT_DATA);
        }
        else {
            outw(base + ATA_PORT_DATA, buffer[idx]);
        }
    }

}

[BenLunt]

Real quick, I have only a minute, but your buffer is short by a word. If you are reading 256 words, your buffer needs to be 256 words as well.

Also, you don't need to keep selecting the drive. Keep an account of which drive is selected. Then if you call this routine and the desired drive is already selected, skip the selection. This will save time with the read/write process. Of course if you are using 28-bit LBA's you must write to the ADDRESS (DEV_HEAD) register anyway. In this case, keep a byte representation of this register. After calculation of the new value, if it is the same as the old value, skip the write.

As for the stuck on the second read/write, did you acknowledge the interrupt by reading from the status register? The controller will not become ready until you acknowledge the interrupt, even if you have interrupts off.

Hope this helps,
Ben
- http://www.fysnet.net/osdesign_book_series.htm