OSDev.org

I recently got my SATA/AHCI driver to properly detect and initialize drives, and now I'm on to updating my filesystem logic to use SATA instead of IDE.

When I initialize my drive, I first perform a read. That seems to work correctly, or at the least it doesn't crash. However, when I call the same method again from my filesystem module, it crashes.
Through debugging, I've determined that the crash occurs in my read method, right when it tries to memset the cmd header:
In the above console output, you can see the outputs "C.2" and "D" near the top. This is from the initialization of the drive, and then that's followed by "Init success: /dev/sd0", so all's good there.

However, when I try to read from it again (after the OS boots) to read files from the filesystem, we only get as far as "C.2" before the OS crashes.

I'm positive this is because I'm doing something wrong in the read method, but I'm not sure what. The full source for my AHCI driver is hereif you're interested.

Edit:

What's interesting to me, and why I output it in the console, is that both cmdheader->ctbau and cmdheader->ctba and different the second time I invoke it, and oddly the same as one another on the second invocation. I suspect that is ultimately the root cause of the problem, but I'm not sure what that implies.

jwhitehorn wrote:What's interesting to me, and why I output it in the console, is that both cmdheader->ctbau and cmdheader->ctba and different the second time I invoke it, and oddly the same as one another on the second invocation. I suspect that is ultimately the root cause of the problem, but I'm not sure what that implies.

It means something is overwriting the memory that contains your command list with a bunch of 0x01 bytes. There are several issues with the code, though, so it's hard to say exactly what's wrong.

Shouldn't you call the kernel allocator instead of hoping there's unused memory at a fixed address?

This is setting the base address to the contents of the buffer instead of the address of the buffer. Also, you need to write the physical address of the buffer here, but this is probably a virtual address. (Did you add the * operator to try to fix the physical/virtual mismatch like you did with the & operator from earlier?)

You're using paging, so the physical memory used by your buffer may not be contiguous. Since pages are (usually) 4kiB, you probably want to use 4kiB blocks in your scatter/gather list. You'll need smarter logic if the buffer address isn't always aligned to a 4kiB boundary. (I have no idea why the wiki code uses 8kiB blocks; it looks like it was written assuming paging is disabled, so it has no reason to split the transfer into blocks in the first place.)

This is unrelated, but you need to separate the PCI code from the AHCI code if you want to have drivers for more than one PCI device at a time.

Thank you, this helps a lot.

Octocontrabass wrote: It means something is overwriting the memory that contains your command list with a bunch of 0x01 bytes. There are several issues with the code, though, so it's hard to say exactly what's wrong.

Shouldn't you call the kernel allocator instead of hoping there's unused memory at a fixed address?

This is setting the base address to the contents of the buffer instead of the address of the buffer. Also, you need to write the physical address of the buffer here, but this is probably a virtual address. (Did you add the * operator to try to fix the physical/virtual mismatch like you did with the & operator from earlier?)

You're using paging, so the physical memory used by your buffer may not be contiguous. Since pages are (usually) 4kiB, you probably want to use 4kiB blocks in your scatter/gather list. You'll need smarter logic if the buffer address isn't always aligned to a 4kiB boundary. (I have no idea why the wiki code uses 8kiB blocks; it looks like it was written assuming paging is disabled, so it has no reason to split the transfer into blocks in the first place.)

This is unrelated, but you need to separate the PCI code from the AHCI code if you want to have drivers for more than one PCI device at a time.

I think, to sum it up, the sample code in the wiki has a lot of flaws if you're using paging, and I didn't fully appreciate that until just recently. I think I mentioned in another post, but paging isn't I necessarily sought out - it just came with the territory of 64bit mode on x86. I've since been reading up more on paging and learning, since clearly that has ramification with device drivers - something I also didn't fully realize until recently.

I'm going to take some time and go over each of your suggestions, and also review for other scenarios where I'm making faulty memory assumptions. Thanks!

jwhitehorn wrote:I think, to sum it up, the sample code in the wiki has a lot of flaws if you're using paging, and I didn't fully appreciate that until just recently. I think I mentioned in another post, but paging isn't I necessarily sought out - it just came with the territory of 64bit mode on x86. I've since been reading up more on paging and learning, since clearly that has ramification with device drivers - something I also didn't fully realize until recently.

I'm going to take some time and go over each of your suggestions, and also review for other scenarios where I'm making faulty memory assumptions. Thanks!

To be clear, just having paging enabled is not the issue, since you can set up identity mapping between virtual and physical addresses even in that case. Even though paging is mandatory with 64-bit mode, it doesn't have to have any effect

davmac314 wrote: To be clear, just having paging enabled is not the issue, since you can set up identity mapping between virtual and physical addresses even in that case. Even though paging is mandatory with 64-bit mode, it doesn't have to have any effect

Perhaps I didn't elaborate enough.

I do fully understand that paging doesn't have to be a problem IF you map between virtual and physical memory, but paging IS a problem IF you follow the AHCI guide in the wiki as-is and are obvious to this as I was.

I hope I made more sense of that.

Octocontrabass wrote: Shouldn't you call the kernel allocator instead of hoping there's unused memory at a fixed address?

I suspect a recent problem I'm running into is related to this bit of feedback.

I attempted to allocate memory, but I'm mostly certain I'm doing this wrong. I generally understand how memory allocations work in user space, but in kernel space I guess I'm turned around. Also, I'm at a loss for how much memory should be allocated for an AHCI port.

In regards to "hoping there was unused memory...", yeah that too was something the wiki's sample code illustrated.

On a plus side, I can mostly read from a SATA drive now. I'm just hitting a couple of small snags (which I suspect are related). The kernel reboots occasionally when booting in SATA mode (see the previously linked thread), and there is this odd case where one sector (out of about a dozen I was successfully able to read) isn't coming back with the expected data. Before even posting my thread about the reboots I suspected the latter was the result of the former - e.g., I'm writing stuff to a bad memory location, and something is being overwritten.

jwhitehorn wrote:Also, I'm at a loss for how much memory should be allocated for an AHCI port.

The AHCI specification explains how much memory you need to reserve for each structure in memory. Some of them don't have a fixed size, so it's up to you how big they'll be.

The code on the wiki just divides up a big block of contiguous memory, but you don't have to do it that way.

Octocontrabass wrote: The AHCI specification explains how much memory you need to reserve for each structure in memory. Some of them don't have a fixed size, so it's up to you how big they'll be.

Cool, thanks!

I certainly have seen where it does specify fixed sizes for some, but (as you said) others don't seem to be specified. I was assuming I missed something, but it doesn't sound like it.

I really appreciate it!