Re: [SeaBIOS] non-emulated AHCI hardware
Hi,
Attached is a two part version of the previous patch. Stage1 is enough
to get through most OS installs. Stage2 adds unaligned buffer support
needed for MS-DOS.
Hmm, patch #1 is still a collection of multiple changes, looks a bit
like trying this and that until it somehow worked, without figuring
which changes where needed to make the box boot. Also the changelog
should describe what was changed and why, not only the effect of the change.
Polling the IRQ status looks like a sensible thing to do. Note that
there might be multiple bits set in the IRQ status register, so you
can't use irqbits == 0x01 to check the status. qemu fails to boot
because of that bug. Also it isn't obvious why you are looking for
others than the Device to Host Register FIS interrupt. Care to explain?
I've attached a patch which switches ahci over to irq status polling,
based on your patch. Works nicely in qemu. Can you give it a spin on
real hardware?
cheers,
Gerd
From 4be00033adb966eb40c1ca2700cc770db0ff682a Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann kra...@redhat.com
Date: Thu, 26 May 2011 13:39:26 +0200
Subject: [PATCH] ahci: use interrupt status register
Poll interrupt status register to figure when the device has updated the
status and possibly finished the request, continue polling until BSY is
clear as we might see multiple status updates per request.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
src/ahci.c | 23 ---
1 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/src/ahci.c b/src/ahci.c
index b820e28..ac3b92a 100644
--- a/src/ahci.c
+++ b/src/ahci.c
@@ -105,7 +105,7 @@ static void ahci_port_writel(struct ahci_ctrl_s *ctrl, u32
pnr, u32 reg, u32 val
static int ahci_command(struct ahci_port_s *port, int iswrite, int isatapi,
void *buffer, u32 bsize)
{
-u32 val, status, success, flags;
+u32 val, status, success, flags, intbits;
struct ahci_ctrl_s *ctrl = GET_GLOBAL(port-ctrl);
struct ahci_cmd_s *cmd = GET_GLOBAL(port-cmd);
struct ahci_fis_s *fis = GET_GLOBAL(port-fis);
@@ -136,14 +136,23 @@ static int ahci_command(struct ahci_port_s *port, int
iswrite, int isatapi,
dprintf(2, AHCI/%d: send cmd ...\n, pnr);
SET_FLATPTR(fis-rfis[2], 0);
+intbits = ahci_port_readl(ctrl, pnr, PORT_IRQ_STAT);
+ahci_port_writel(ctrl, pnr, PORT_IRQ_STAT, intbits);
ahci_port_writel(ctrl, pnr, PORT_SCR_ACT, 1);
ahci_port_writel(ctrl, pnr, PORT_CMD_ISSUE, 1);
-while (ahci_port_readl(ctrl, pnr, PORT_CMD_ISSUE)) {
-yield();
-}
-while ((status = GET_FLATPTR(fis-rfis[2])) == 0) {
-yield();
-}
+
+do {
+for (;;) {
+intbits = ahci_port_readl(ctrl, pnr, PORT_IRQ_STAT);
+ahci_port_writel(ctrl, pnr, PORT_IRQ_STAT, intbits);
+if (intbits 0x01)
+break;
+yield();
+}
+status = GET_FLATPTR(fis-rfis[2]);
+dprintf(2, AHCI/%d: ... intbits 0x%x, status 0x%x ...\n,
+pnr, intbits, status);
+} while (status ATA_CB_STAT_BSY);
success = (0x00 == (status (ATA_CB_STAT_BSY | ATA_CB_STAT_DF |
ATA_CB_STAT_DRQ | ATA_CB_STAT_ERR))
--
1.7.1
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] non-emulated AHCI hardware
Gerd Hoffmann wrote: ] Hi, ] ] Attached is a two part version of the previous patch. Stage1 is enough ] to get through most OS installs. Stage2 adds unaligned buffer support ] needed for MS-DOS. ] ]Hmm, patch #1 is still a collection of multiple changes, looks a bit ]like trying this and that until it somehow worked, without figuring ]which changes where needed to make the box boot. Thank you for the feedback. This is a pretty harsh assessment of work that transforms non-functioning code into something usable on real hardware. Have you ever written code for real hardware or do work strictly with software models? ] Also the changelog ]should describe what was changed and why, not only the effect of the ]change. These documents will help you understand the changes: http://download.intel.com/technology/serialata/pdf/rev1_3.pdf http://www.lttconn.com/res/lttconn/pdres/201005/20100521170123066.pdf ]Polling the IRQ status looks like a sensible thing to do. Note that ]there might be multiple bits set in the IRQ status register, so you ]can't use irqbits == 0x01 to check the status. qemu fails to boot ]because of that bug. Have you considered the possibility that the reason this works on real hardware and not qemu is that the qemu AHCI model has an erratum? ] Also it isn't obvious why you are looking for ]others than the Device to Host Register FIS interrupt. Care to explain? I don't understand this question. ]I've attached a patch which switches ahci over to irq status polling, ]based on your patch. Works nicely in qemu. Can you give it a spin on ]real hardware? It fails on real (AMD) hardware. I am surprised by qemu bias of your questions. The type of question I expected is great to hear it works on AMD hardware, but what about Intel hardware? My answer would be that I cannot easily test the code on Intel hardware. Thanks, Scott ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] non-emulated AHCI hardware
Thanks Scott, I'd like to commit these fixes. Is there any chance you could break them up into separate patches - one per fix? That would be great for review too. thanks, Gerd ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] non-emulated AHCI hardware
On Tue, Apr 26, 2011 at 09:15:04PM -0500, Scott Duplichan wrote: Kevin O'Connor wrote: ]Out of curiosity, can you see what happens if you return ]DISK_RET_EBOUNDARY in the unaligned case? DOS tries the same request 10 times then ignores the error and continues. Thanks for testing. ]It's not currently possible to dynamically allocate memory during ]runtime. However, it is possible to allocate a buffer at init and ]keep it around for later. How big of a buffer would you need? I think a single aligned 512 byte buffer would do. The code could read a sector at a time and memcpy from the seabios aligned buffer into the caller's unaligned buffer. Yeah. One can call malloc_low() during the init phase to allocate this buffer. Ideally, though, there would be some way to share buffer space with the other users (eg, cdemu, ide dma, usb). -Kevin ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] non-emulated AHCI hardware
Scott Duplichan wrote: ]1) When booting a DOS drive, a disk read error occurs at some point ]after autoexec executes. The revised patch (attached) overcomes this problem. It turns out in the latter stage of booting, DOS makes a couple of INT13 read requests with a buffer that is not word aligned. AHCI only supports word aligned buffers. This was causing the data to be shifted by one byte for these reads. The patch adds unaligned support for reads only, which is good enough for the DOS boot problem. The method of this patch actually writes one byte beyond the end of the caller's buffer. But the only OS seen to do this is DOS, so it may be good enough this way. ]2) DOS booting is slower by 100-200 ms. While that isn't much in ]absolute terms, it is a big percentage increase from the 642 ms I ]can get using the IDE interface. Now AHCI DOS boot is slower than IDE DOS boot by only 40 ms, so I am satisfied and have switched my project over to AHCI mode. Thanks, Scott ahci-002.patch Description: Binary data ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] non-emulated AHCI hardware
Scott Duplichan wrote: ]1) When booting a DOS drive, a disk read error occurs at some point ]after autoexec executes. The revised patch (attached) overcomes this problem. It turns out in the latter stage of booting, DOS makes a couple of INT13 read requests with a buffer that is not word aligned. AHCI only supports word aligned buffers. This was causing the data to be shifted by one byte for these reads. The patch adds unaligned support for reads only, which is good enough for the DOS boot problem. The method of this patch actually writes one byte beyond the end of the caller's buffer. But the only OS seen to do this is DOS, so it may be good enough this way. Still it's not nice to write outside the callers buffer. Another OS might call same function and SeaBIOS would end up corrupting some variable. Ungood. I guess memmove() is the only choice? //Peter ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] non-emulated AHCI hardware
Peter Stuge wrote: ]Still it's not nice to write outside the callers buffer. Another OS ]might call same function and SeaBIOS would end up corrupting some ]variable. Ungood. I guess memmove() is the only choice? ]//Peter I had a couple of ideas for a more sound solution. Debugging them is a challenge for me because I am new to seabios development and its method for writing code that works correctly in both 16-bit and 32-bit mode. Anyway, the first method I tried was to allocate a temporary I/O buffer that has the proper alignment. While that creates overhead, it would only be invoked in the rare case of unaligned request. But then there is the possibility that the allocation function will not be able to satisfy the request. Another possibility is splitting the request. The caller's buffer could handle the bigger part, and a stack buffer could be used for the remaining part. Yet another possibility is to backup the byte that gets overwritten by the current patch method and restore it afterwards. While that is an unreliable method for paged code, it might work here. The only danger I can think of is if the caller's buffer is at the end of a physical DRAM range. Thanks, Scott ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] non-emulated AHCI hardware
Scott Duplichan wrote: Another possibility is splitting the request. The caller's buffer could handle the bigger part, and a stack buffer could be used for the remaining part. I think this idea is by far the best! //Peter ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] non-emulated AHCI hardware
On Sat, Feb 12, 2011 at 11:43:24PM +, Jonathan A. Kollasch wrote: Hi, I've been trying SeaBIOS's AHCI driver on AMD SB710 hardware under coreboot. Both AMD's AHCI BIOS and NetBSD's ahcisata(4) drivers are happy enough with the hardware's state after coreboot/SeaBIOS. However, SeaBIOS's AHCI driver just falls into a dead loop waiting for commands to complete. This is due to at least two problems. The immediate problem is that we tell the HBA that a Host To Device FIS is 4 dwords. The correct value is 5 dwords if I read the specs right. But, when the H2D FIS length we give to the HBA is 5, I have a more perplexing problem. The machine hardlocks when HBA registers are read after the command has been issued. When I examine the Command List and Receive FIS Area, etc. before and after issuing the command, they have not changed. Actually it wasn't a hardlock, it was a dead loop. :-) I have it (mostly) working now, just have to get it polished up a bit. Jonathan Kollasch ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
