AHCI - Stopping Port

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
jwhitehorn
Posts: 18
Joined: Thu Jun 25, 2020 8:29 pm

AHCI - Stopping Port

Post by jwhitehorn »

Hello.

First, let me say how happy I am that this website exists. I've poured over a good deal of helpful information here in the past several months, and am truly grateful for all the assistance it has provided.

Unfortunantely I have been hitting a snag that I cannot seem to get past. When attempting to implement AHCI I am getting hung up with one problem or another. In it's current state my AHCI driver hangs when attempting to stop an AHCI port. This code is mostly straight from the wiki:

Code: Select all

uint16 ahci_stop_port(HBA_PORT *port) {
       // Clear ST (bit0)
       port->cmd &= ~HBA_PxCMD_ST;
       	// Clear FRE (bit4)
       	port->cmd &= ~HBA_PxCMD_FRE;

       	// Wait until FR (bit14), CR (bit15) are cleared
       	while(1)
       	{
       		if (port->cmd & HBA_PxCMD_FR)
       			continue;
       		if (port->cmd & HBA_PxCMD_CR)
       			continue;
       		break;
       	}

       // Clear FRE (bit4)
       return 1;
}
Specifically PxCMD.FR never gets cleared.

I am running this in QEMU, with:

Code: Select all

qemu-system-x86_64 -device ahci,id=ahci -smp 3 -m 512 -net none -hda ./bin/boot.vdi -drive id=disk,file=./bin/fs.vdi,if=none -device ide-drive,drive=disk,bus=ahci.0
During init, I discover the AHCI device:
probing PCI...
PCI: 0:0.0: 8086:1237: class: 6.0 (Bridge device) irq: 0
PCI: 0:1.0: 8086:7000: class: 6.1 (Bridge device) irq: 0
PCI: 0:1.1: 8086:7010: class: 1.1 (Storage controller) irq: 0
PCI: 0:1.3: 8086:7113: class: 6.80 (Bridge device) irq: 9
PCI: 0:2.0: 1234:1111: class: 3.0 (Display controller) irq: 0
PCI: 0:3.0: 8086:2922: class: 1.6 (Storage controller) irq: 11 bar5: febf1000
Intel ICH9 controller found (bus=0, slot=3, func=0, abar=0xfebf1000)
HBA in legacy mode
SATA device detected:
port[19].sig = c990ffff
ipm=5, spd=c, det=3
rebasing port[19]...
And I have (I believe correctly) tested device type status:

Code: Select all

void ahci_try_setup_known_device(char *dev_name, uint64 ahci_base_mem, uint16 bus, uint16 slot, uint16 func) {
    cprintf("%s controller found (bus=%d, slot=%d, func=%d, abar=0x%x)\n", dev_name, bus, slot, func, ahci_base_mem);

    HBA_MEM *ptr = (HBA_MEM *)&ahci_base_mem;
    cprintf(" HBA in ");
    if(ptr->ghc == 0x0){
        cprintf("legacy mode\n");
    }else{
        cprintf("AHCI-only mode\n");
    }

    uint64 pi = ptr->pi;
    for(int i = 0; (i != 32); i++){
        uint64 port_mask = 1 << i;
        if((pi & port_mask) == 0x0){
            continue;
        }

        HBA_PORT *hba_port = (HBA_PORT *) &ptr->ports[i];

        if(hba_port->sig != SATA_SIG_ATAPI && hba_port->sig != SATA_SIG_SEMB && hba_port->sig != SATA_SIG_PM){
            //we may have found a SATA device, but what is the status of this device?
            uint32 ssts = hba_port->ssts;

            uint8 ipm = (ssts >> 8) & 0x0F;
            uint8 spd = (ssts >> 4) & 0x0F;
            uint8 det = ssts & 0x7; //the Device Detection (DET) flags are the bottom 3 bits

            if (det != HBA_PORT_DET_PRESENT && ipm != HBA_PORT_IPM_ACTIVE){
                //nope
		 	}else if(hba_port->sig==SATA_SIG_ATAPI){
                //ATAPI device
			}else if(hba_port->sig==SATA_SIG_SEMB){

			}else if(hba_port->sig==SATA_SIG_PM){
                //port multiplier detected
			}else{
                cprintf("SATA device detected:\n");
                cprintf("   port[%d].sig = %x\n", i, hba_port->sig);
                cprintf("   ipm=%x, spd=%x, det=%x\n", ipm, spd, det);
                ahci_sata_init(hba_port, i);
			}
        }
    }
}
So, I am puzzled why this is not stopping. I am really hoping someone here can help me learn what I'm doing incorrectly.
Last edited by jwhitehorn on Fri Jun 26, 2020 3:35 pm, edited 1 time in total.
thewrongchristian
Member
Member
Posts: 426
Joined: Tue Apr 03, 2018 2:44 am

Re: AHCI - Stopping Port

Post by thewrongchristian »

jwhitehorn wrote:Hello.

First, let me say how happy I am that this website exists. I've poored over a good deal of helpful information here in the past several months, and am truly grateful for all the assistance it has provided.

Unfortunantely I have been hitting a snag that I cannot seem to get past. When attempting to implement AHCI I am getting hung up with one problem or another. In it's current state my AHCI driver hangs when attempting to stop an AHCI port. This code is mostly straight from the wiki:

Code: Select all

uint16 ahci_stop_port(HBA_PORT *port) {
       // Clear ST (bit0)
       port->cmd &= ~HBA_PxCMD_ST;
       	// Clear FRE (bit4)
       	port->cmd &= ~HBA_PxCMD_FRE;

       	// Wait until FR (bit14), CR (bit15) are cleared
       	while(1)
       	{
       		if (port->cmd & HBA_PxCMD_FR)
       			continue;
       		if (port->cmd & HBA_PxCMD_CR)
       			continue;
       		break;
       	}

       // Clear FRE (bit4)
       return 1;
}
So, I am puzzled why this is not stopping. I am really hoping someone here can help me learn what I'm doing incorrectly.
Check you're using MMIO property. You need to make sure the MMIO regions are not cached, and that you're accessing via volatile pointers, so the kernel knows not to cache the values locally in a register.
jwhitehorn
Posts: 18
Joined: Thu Jun 25, 2020 8:29 pm

Re: AHCI - Stopping Port

Post by jwhitehorn »

Thank you very much for your reply!

So, in an effort to fully understand you:

Are you simply saying that I should use the volatile keyword when I define my structs? If so, I've already done that:

Code: Select all

typedef volatile struct tagHBA_PORT {
	uint32 clb;		   // 0x00, command list base address, 1K-byte aligned
	uint32 clbu;	   // 0x04, command list base address upper 32 bits
	uint32 fb;         // 0x08, FIS base address, 256-byte aligned
	uint32 fbu;        // 0x0C, FIS base address upper 32 bits
	uint32 is;         // 0x10, interrupt status
	uint32 ie;         // 0x14, interrupt enable
	uint32 cmd;        // 0x18, command and status
	uint32 rsv0;       // 0x1C, Reserved
	uint32 tfd;        // 0x20, task file data
	uint32 sig;        // 0x24, signature
	uint32 ssts;       // 0x28, SATA status (SCR0:SStatus)
	uint32 sctl;       // 0x2C, SATA control (SCR2:SControl)
	uint32 serr;       // 0x30, SATA error (SCR1:SError)
	uint32 sact;       // 0x34, SATA active (SCR3:SActive)
	uint32 ci;         // 0x38, command issue
	uint32 sntf;       // 0x3C, SATA notification (SCR4:SNotification)
	uint32 fbs;        // 0x40, FIS-based switch control
	uint32 rsv1[11];   // 0x44 ~ 0x6F, Reserved
	uint32 vendor[4];  // 0x70 ~ 0x7F, vendor specific
} HBA_PORT;
I do suspect you're correct though, I'm just not sure what else I need to do to prevent it from being cached.
thewrongchristian
Member
Member
Posts: 426
Joined: Tue Apr 03, 2018 2:44 am

Re: AHCI - Stopping Port

Post by thewrongchristian »

jwhitehorn wrote:Thank you very much for your reply!

So, in an effort to fully understand you:

Are you simply saying that I should use the volatile keyword when I define my structs? If so, I've already done that:

Code: Select all

typedef volatile struct tagHBA_PORT {
	uint32 clb;		   // 0x00, command list base address, 1K-byte aligned
	uint32 clbu;	   // 0x04, command list base address upper 32 bits
	uint32 fb;         // 0x08, FIS base address, 256-byte aligned
	uint32 fbu;        // 0x0C, FIS base address upper 32 bits
	uint32 is;         // 0x10, interrupt status
	uint32 ie;         // 0x14, interrupt enable
	uint32 cmd;        // 0x18, command and status
	uint32 rsv0;       // 0x1C, Reserved
	uint32 tfd;        // 0x20, task file data
	uint32 sig;        // 0x24, signature
	uint32 ssts;       // 0x28, SATA status (SCR0:SStatus)
	uint32 sctl;       // 0x2C, SATA control (SCR2:SControl)
	uint32 serr;       // 0x30, SATA error (SCR1:SError)
	uint32 sact;       // 0x34, SATA active (SCR3:SActive)
	uint32 ci;         // 0x38, command issue
	uint32 sntf;       // 0x3C, SATA notification (SCR4:SNotification)
	uint32 fbs;        // 0x40, FIS-based switch control
	uint32 rsv1[11];   // 0x44 ~ 0x6F, Reserved
	uint32 vendor[4];  // 0x70 ~ 0x7F, vendor specific
} HBA_PORT;
I do suspect you're correct though, I'm just not sure what else I need to do to prevent it from being cached.
You can turn off caching at the PTE level. On x86, bit 4 of the PTE, if set, will disable caching for that page, so reads will always come from the underlying device. Else, reads will simply come from the CPU cache which you're reading furiously in a loop (and getting a great cache hit rate!)
Octocontrabass
Member
Member
Posts: 5575
Joined: Mon Mar 25, 2013 7:01 pm

Re: AHCI - Stopping Port

Post by Octocontrabass »

thewrongchristian wrote:Else, reads will simply come from the CPU cache which you're reading furiously in a loop (and getting a great cache hit rate!)
Shouldn't firmware have already configured the MTRRs so that MMIO is not cacheable?
jwhitehorn
Posts: 18
Joined: Thu Jun 25, 2020 8:29 pm

Re: AHCI - Stopping Port

Post by jwhitehorn »

I am reading up on all of that (started with this https://wiki.osdev.org/Paging). I see what you're saying, but this is new for me.

I'm essentially working with a modified version of the xv6 OS (from MIT) and, among other initial work, was hoping to get SATA support going. I have yet looked much at the paging code, as I figured that wasn't necessary yet. Sounds like I was wrong.

I see in the wiki on AHCI it says (emphasis mine):
This memory area should be configured as uncacheable as they are memory mapped hardware registers, not normal prefetchable RAM. For the same reason, the data structures are declared as "volatile" to prevent the compiler from over optimizing the code.
At first I assumed the second sentence was ellaborating on the first, but I can see how those are two different things that must be done.
thewrongchristian
Member
Member
Posts: 426
Joined: Tue Apr 03, 2018 2:44 am

Re: AHCI - Stopping Port

Post by thewrongchristian »

jwhitehorn wrote:I am reading up on all of that (started with this https://wiki.osdev.org/Paging). I see what you're saying, but this is new for me.

I'm essentially working with a modified version of the xv6 OS (from MIT) and, among other initial work, was hoping to get SATA support going. I have yet looked much at the paging code, as I figured that wasn't necessary yet. Sounds like I was wrong.

I see in the wiki on AHCI it says (emphasis mine):
This memory area should be configured as uncacheable as they are memory mapped hardware registers, not normal prefetchable RAM. For the same reason, the data structures are declared as "volatile" to prevent the compiler from over optimizing the code.
At first I assumed the second sentence was ellaborating on the first, but I can see how those are two different things that must be done.
Might be worth just having a look at the disassembly of the generated code. If you're fetching the memory inside the loop, then code wise you probably have the data correctly declared as volatile. If the memory fetch is outside the loop, then you need to sort out the volatile declarations.
Octocontrabass wrote:
thewrongchristian wrote:Else, reads will simply come from the CPU cache which you're reading furiously in a loop (and getting a great cache hit rate!)
Shouldn't firmware have already configured the MTRRs so that MMIO is not cacheable?
Ah yes, probably. I'm still mostly in QEMU world with my development, so I have limited practical experience with MMIO.

In fact, the original poster is also running in QEMU, so it's probably not the issue anyway.
Post Reply