AHCI, writing does not work (next operation hangs)

[savver]

Hi, colleagues. I write AHCI driver for x86 and have a ploblem with writing (on real hardware, chipset ICH8).
After write command (ATA_CMD_WRITE_DMA_EX = 0x35), any next command (read or write) does not complete (PxCI not reset to '0', I have
infinite expectation in cycle "while(PxCI != 0)").But write command completes "success" before this (PxCI = 0, PxSERR = 0, PxSACT = 0, PxTFD = 0x50 (bit 7, BSY, = 0)).
But writing works in QEMU, we can read data after writing, no endless expectations on reading, and we can see correct read data.
So code does not work on hardware... I think, write operation does not complete in fact...
Reading works correct (ATA_CMD_READ_DMA_EX = 0xC8), we can call "ahci_read" function (without writing) many times without errors, hangs.
Read data is always correct.

The sequence of operations during writing:
1) find free slot
2) form Command_Header in Command_List
3) form PDRT in Command_Table, save source data adress in it
4) form Command_FIS in Command_Table
5) PxCI = 1 << slot;
6) while(PxCI != 0);
Code is similar to this https://github.com/sahilpparmar/SBUnix/ ... /fs/ahci.c (function "write")

...last debug showed that after writing completion FIS_D2H is empty, but PxCI=0 (he was =1 after writing start, but it became =0 a little bit later).
When I read data, FIS_D2H != 0 after operation completion. DMA does not work correct?

Has anyone dealt with such a problem?

[BenLunt]

Without looking at your code, I can only speculate.

You are using the keyword: FIS_D2H. D2H indicates that this structure is data coming from the Device. D2H = Device to Host...

You need to be sending a H2D structure for the command. Host2Device, yes? Or have I missed something?

Also, do you set bit 3 in the 'Control' member of the FIS_H2D structure? From my notes:

Code: Select all

  cmd_fis->control = 0x08; // setting bit 3 ensures that this FIS gets sent since the Device Control
                           //  register will have bit 3 cleared.  A FIS will only be sent if the
                           //  Command register and command field are different, or the Device Control
                           //  register and the control field are different.

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

[savver]

Without looking at your code, I can only speculate.

You are using the keyword: FIS_D2H. D2H indicates that this structure is data coming from the Device. D2H = Device to Host...

You need to be sending a H2D structure for the command. Host2Device, yes? Or have I missed something?

Also, do you set bit 3 in the 'Control' member of the FIS_H2D structure? From my notes:

Code: Select all

  cmd_fis->control = 0x08; // setting bit 3 ensures that this FIS gets sent since the Device Control
                           //  register will have bit 3 cleared.  A FIS will only be sent if the
                           //  Command register and command field are different, or the Device Control
                           //  register and the control field are different.

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

Yes, I send H2D structure for commands,
'control' member equals null... ok, try with writing "8", thanks.

[savver]

Without looking at your code, I can only speculate.

You are right, this is my code below (last version)
Code in "main()"

Code: Select all

// 1. HBA reset & rebase
ahci_probe( devs, dev_cnt, (ahci_dev_t*)&ahci_devs, AHCI_PROBE_DEVS, &ports_map);
ahci_hba_reset(&ahci_devs[0]);
ahci_port_rebase_mix(&ahci_devs[0], ahci_devs[0].port, 0, (uint32_t)0xffdf800);

// 2. ATA Identify - work ok
ahci_identify(ahci_devs[0].port, dbuf_stat);

// 3. Test Reading - work ok
memset(dbuf_stat, 0, sizeof(dbuf_stat));
ahci_res = ahci_read(ahci_devs[0].port, WR_SECTOR_ADDR, 0, DATA_SEC_CNT, dbuf_stat);
assert (ahci_res == 0, "ahci_read_res = %d", ahci_res);

// 4. Cycle of writing
ahci_res = 0;
for(int j = 0; j < 5; j++)
{
	ahci_res |= ahci_write( ahci_devs[0].port, WR_SECTOR_ADDR + j, 0, DATA_SEC_CNT, dbufw_stat );
	
		//__ahci_flush_cache(ahci_devs[0].port); - not shure, that it works correct
		ahci_fis_dump( ahci_devs[0].port );
		ahci_port_info(ahci_devs[0].port);
		printf("ahci_write_res = %d", ahci_res);
		
		delay_ms(3*1000);
		
		dbufw_stat[0] *= 2;
}

Function of writing:

Code: Select all

int ahci_write(HBA_PORT *port, uint32_t startl, uint32_t starth, uint32_t count, uint16_t * buf)
{
    printf("\n=== write (%08x) ===\n", startl);

    uintptr_t p;
    port->is = 0xffff;        // Clear pending interrupt bits
    int spin = 0;             // Spin lock timeout counter
    int slot = find_cmdslot(port);
    printf("W0:slot=%d,clf=%d ", slot, sizeof(FIS_REG_H2D)/sizeof(uint32_t));
    if (slot == -1)
        return EINVAL;

    p  = port->clb | (port->clbu << 32);
    HBA_CMD_HEADER *   cmdheader = (HBA_CMD_HEADER *)(p);

    cmdheader += slot;
    cmdheader->cfl = sizeof(FIS_REG_H2D)/sizeof(uint32_t);   // Command FIS size
    cmdheader->w = 1;               //
    cmdheader->c = 1;               /** mom_1, must be == 1 (!!!) **/
    cmdheader->p = 1;               /** mom_2, if==0 hang in 'while(ci != 0)s' (!!!) **/
    // 8K bytes (16 sectors) per PRDT
    cmdheader->prdtl = (uint16_t)((count-1)>>4) + 1;    // PRDT entries count

    p = cmdheader->ctba | (cmdheader->ctbau << 32);
    HBA_CMD_TBL *   cmdtbl = (HBA_CMD_TBL *)(p);

    memset(cmdtbl, 0, sizeof(HBA_CMD_TBL) + (cmdheader->prdtl-1)*sizeof(HBA_PRDT_ENTRY));
    int i = 0;
    // 8K bytes (16 sectors) per PRDT
    for (i=0; i<cmdheader->prdtl-1; i++)
    {
        cmdtbl->prdt_entry[i].dba  = (uint32_t)((uint64_t)buf & 0xFFFFFFFF);
        cmdtbl->prdt_entry[i].dbau = (uint32_t)(((uint64_t)buf >> 32) & 0xFFFFFFFF);
        cmdtbl->prdt_entry[i].dbc = 8*1024-1;     // 8K bytes
        cmdtbl->prdt_entry[i].i = 0;
        buf += 4*1024;  // 4K words
        count -= 16;    // 16 sectors
   }
    /**If the final Data FIS transfer in a command is for an odd number of 16-bit words, the transmitter�s
Transport layer is responsible for padding the final Dword of a FIS with zeros. If the HBA receives one
more word than is indicated in the PRD table due to this padding requirement, the HBA shall not signal
this as an overflow condition. In addition, if the HBA inserts padding as required in a FIS it is transmitting,
an overflow error shall not be indicated. The PRD Byte Count field shall be updated based on the
number of words specified in the PRD table, ignoring any additional padding.**/

    // Last entry
    cmdtbl->prdt_entry[i].dba   = (uint32_t)((uint64_t)buf & 0xFFFFFFFF);
    cmdtbl->prdt_entry[i].dbau  = (uint32_t)(((uint64_t)buf >> 32) & 0xFFFFFFFF);
    cmdtbl->prdt_entry[i].dbc   = count << 9 ;   // 512 bytes per sector
    cmdtbl->prdt_entry[i].i     = 1; // interrupt gen

    // Setup command
    FIS_REG_H2D *   cmdfis      = (FIS_REG_H2D *)(&cmdtbl->cfis);
    memset(cmdfis, 0, sizeof(FIS_REG_H2D));

    cmdfis->fis_type    = FIS_TYPE_REG_H2D;
    cmdfis->c           = 1;  // Command
    cmdfis->command     = ATA_CMD_WRITE_DMA_EX;

    cmdfis->lba0        = (uint8_t)startl;
    cmdfis->lba1        = (uint8_t)(startl >> 8);
    cmdfis->lba2        = (uint8_t)(startl >> 16);
    cmdfis->device      =  1 << 6 ;  // LBA mode //0xE0;

    cmdfis->lba3        = (uint8_t)(startl >> 24);
    cmdfis->lba4        = (uint8_t)starth;
    cmdfis->lba5        = (uint8_t)(starth >> 8);

    cmdfis->countl      = count & 0xff;
    cmdfis->counth      = count >> 8;

    cmdfis->control      = 8;  /** mom_3. also try write 0x08, 0x00 (!!!) **/

    // The below loop waits until the port is no longer busy before issuing
    // a new command
    while((port->tfd & (ATA_DEV_BUSY | ATA_DEV_DRQ)) && spin < 10000000)
    {
        spin++;
    }
    if(spin == 10000000)
    {
        printf("Port is hung %d\n", EDEADLK);
        return EDEADLK;
    }

    port->ci = 1 << slot;       // Issue command
    _ahci_engine_start(port);   /** mom_4 **/
    printf("W1:port->cmd=%08x,ci=%08x,tfd=%08x,sact=%08x ", port->cmd,port->ci, port->tfd, port->sact);
    delay_ms(1);	//debug only

    // Wait for completion
    uint32_t    cntr = 0;
    while (1)
    {
        // In some longer duration reads, it may be helpful to spin on the DPS bit
        // in the PxIS port field as well (1 << 5)
        if ((port->ci & (1<<slot)) == 0)
                break;
        if (port->is & HBA_PxIS_TFES)   // Task file error
        {
            printf("Read disk error\n");
            return EIO;
        }
        if(cntr++ > 1000*1000)
        {
            printf("WE:port->ci=%08x,is=%08x,tfd=%08x,sact=%08x ", port->ci, port->is, port->tfd, port->sact);
            _ahci_engine_stop(port);
            while(1);
        }
    }
    printf("W2:port->ci=%08x,tfd=%08x,sact=%08x ", port->ci, port->tfd, port->sact);
    // Check again
    if (port->is & HBA_PxIS_TFES)
    {
        printf("Read disk error\n");
        _ahci_engine_stop(port);
        return EIO;
    }
    while(port->ci != 0)
    {
   //     print("[%d]", k++);
    }
    printf("W:ok (ci=%d,sig=%08x)\n", port->ci,port->sig);

    _ahci_engine_stop(port);  /** mom_4 **/
    return 0;
}

Code: Select all

void  _ahci_engine_stop(HBA_PORT* port)
{
    port->cmd &= ~(1 << 0);
    port->cmd &= ~(1 << 4); //FRE
    while(1)
    {
        if(port->cmd & AHCI_PORT_CMD_FIS_ON)  //FR
            continue;
        if(port->cmd & AHCI_PORT_CMD_LIST_ON) //CR
            continue;

        break;
    }
}
//---------------------------------------------
void   _ahci_engine_start(HBA_PORT* port)
{
    port->cmd |= (1 << 4);
    port->cmd |= (1 << 0);
    port->is = 0;
}

Some remarks about code (note them as /** mom.x **/ in code)...
mom.1, mom.2
(_ahci_start_engine/stop_engine are used, delay pause between writings = 3 sec)
1) If "cmdheader->c = 0, cmdheader->p->0", function hangs in cycle "while(port->ci != 0)" (port->ci = 1, port->is = 0x01000020, port->tfd = 0xD0)
2) If "cmdheader->c = 1, cmdheader->p->0", 1st call of function completes success (port->ci = 0, port->tfd = 0x50, port->sact = 0), but 2nd call is hang in cycle "while(port->ci != 0)" (port->ci = 1, port->is = 0, port->tfd = 0xD0).
3) If "cmdheader->c = 1, cmdheader->p->1" all calls complete success!!! Next reading show that data was written correct, but... if we decrease delay between writings less than 3 sec (for example, 2 sec), 2nd call completes with errors (port->is=0x41000001, port->tfd = 0x8451; fis_d2h: sts=0x51,err=0x84)... cache? I saw in ATA commands FLUSH_CACHE, but I didn't see its use in small OSes.
4) mom.3, "cmdfis->control = 0" or "= 8" don't influence
5) mom4, why I call "_ahci_start_engine"/"_ahci_stop_engine"... I saw it in one baremetal project, I got a version that write data in cycle (write correct) with such "method" and big delays between function calls. But I did not see the engine so often turn_on/off in other implementations.
if comment these calls, 1st call complete success, 2nd - with error (port->ci = 0, port->is = 0x41000021, port->tfd = 0x8451).

[BenLunt]

Sorry, I don't have time to look over your code in detail, but I did see a few things. The C and P flags should be zero (mom1). Also, I don't see where you set the PRDBC field to zero. If my notes are correct, this field must be zero before the command is sent.

I think that possibly you have a count "off by one" somewhere and the controller is still waiting for you to read/write one more "burst" of data. Clear the PRDBC field to zero, the C and P flags as well, and then double check that you have your count correct.

I may have a little more time this afternoon to look over your code in more detail.

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