On 31.08.2012, at 12:15, Jason Baron <jba...@redhat.com> wrote:
> On Fri, Aug 31, 2012 at 12:04:59PM -0700, Alexander Graf wrote: >> On 31.08.2012, at 07:23, Jason Baron wrote: >> >>> On Fri, Aug 31, 2012 at 12:12:22PM +0200, Kevin Wolf wrote: >>>> Am 23.08.2012 23:09, schrieb Jason Baron: >>>>> While testing q35, I found that windows 7 (specifically, windows 7 >>>>> ultimate >>>>> with sp1 x64), wouldn't install because it can't find the cdrom or disk >>>>> drive. >>>>> The failure message is: 'A required cd/dvd device driver is missing. If >>>>> you >>>>> have a driver floppy disk, CD, DVD, or USB flash drive, please insert it >>>>> now.' >>>>> This can also be reproduced on piix by adding an ahci controller, and >>>>> observing that windows 7 does not see any devices behind it. >>>>> >>>>> The problem is that when windows issues a HBA reset, qemu does not reset >>>>> the >>>>> individual ports' PxCMD register. Windows 7 then reads back the PxCMD >>>>> register >>>>> and presumably assumes that the ahci controller has already been >>>>> initialized. >>>>> Windows then never sets up the PxIE register to enable interrupts, and >>>>> thus it >>>>> never gets irqs back when it sends ata device inquiry commands. >>>>> >>>>> I believe this change brings qemu into ahci 1.3 specification compliance. >>>>> >>>>> Section 10.4.3 HBA Reset: >>>>> >>>>> " >>>>> When GHC.HR is set to '1', GHC.AE, GHC.IE, the IS register, and all port >>>>> register fields (except PxFB/PxFBU/PxCLB/PxCLBU) that are not HwInit in >>>>> the >>>>> HBA's register memory space are reset. >>>>> " >>>>> >>>>> I've also re-tested Fedora 16 and 17 to verify that they continue to work >>>>> with >>>>> this change. >>>>> >>>>> Signed-off-by: Jason Baron <jba...@redhat.com> >>>>> --- >>>>> hw/ide/ahci.c | 1 + >>>>> 1 files changed, 1 insertions(+), 0 deletions(-) >>>>> >>>>> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c >>>>> index 9fe89a5..06c236f 100644 >>>>> --- a/hw/ide/ahci.c >>>>> +++ b/hw/ide/ahci.c >>>>> @@ -1199,6 +1199,7 @@ void ahci_reset(AHCIState *s) >>>>> pr->irq_stat = 0; >>>>> pr->irq_mask = 0; >>>>> pr->scr_ctl = 0; >>>>> + pr->cmd = 0; >>>>> ahci_reset_port(s, i); >>>>> } >>>>> } >>>>> >>>> >>>> In ahci_init() we have: >>>> >>>> ad->port_regs.cmd = PORT_CMD_SPIN_UP | PORT_CMD_POWER_ON; >>>> >>>> Why is resetting to 0 in ahci_reset() correct? I think we can still get >>>> this part committed for 1.2 after you either explain why 0 is right or >>>> send a fixed version. >>>> >>> >>> Good catch. I just tried out the following instead in ahci_reset(): >>> >>> pr->cmd = PORT_CMD_SPIN_UP | PORT_CMD_POWER_ON; >>> >>> And it continues to make Windows7 find the ahci disks. I'd like to do a >>> little more testing, but I will send an updated path. >> >> Since we reset on init, we can just shove it there completely, right? >> >> > > Actually, we don't call reset on init. I'm thinking for 1.2, we can just > add the fix to the reset path, and as Kevin suggested in 1.3, we will > look to add a call to ahci_reset() in the init path. We register the reset handler as generic reset path, no? It should be called on machine init then. Every reset handler gets called on machine boot. Alex > > Thanks, > > -Jason
