[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] ahci: properly reset PxCMD on HBA reset
From: |
Alexander Graf |
Subject: |
Re: [Qemu-devel] [PATCH] ahci: properly reset PxCMD on HBA reset |
Date: |
Fri, 31 Aug 2012 12:04:59 -0700 |
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 <address@hidden>
>>> ---
>>> 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?
Alex