On 05/26/11 20:09, Scott Duplichan wrote:
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.
Well, it breaks on qemu, and it does because of bugs in the code.
Have you ever written code for real hardware or do work strictly with software models?
I've actually done linux driver development (video4linux) before I've started working in the virtualization field.
]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?
That is one possible reason, yes. Thats why I want understand which changes are needed to make ahci work on real hardware and especially *why* they are needed. Quite possible that qemu needs a fix, that certainly wouldn't be the first emulation bug ...
The way you check irqbits is a clear bug in your patch though. It is pure luck that you don't hit it on real hardware, probably due to the timing being different.
] 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.
The device sends status register updates using the device to host register fis, right? Why do you check for updates of the other fis types? I don't see the point in doing so. Also you are reading error + status from the dma setup fis where those fields don't exist.
Why you are ignoring the status register when figuring whenever a operation was successful or not?
]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.
Ok, so switching the status polling alone doesn't cut it. Your patch also removes the loop waiting for the device becoming ready in ahci_port_probe(). Why?
It is possible that real hardware is simply alot slower than the qemu emulated hardware. Have you tried to increate the delay and/or the number of allowed retries to give the devices more time to become ready instead of just removing the loop?
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?"
I want it work everythere. That includes qemu of course.
cheers, Gerd