From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:41589) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T7h8Q-0004JX-LQ for qemu-devel@nongnu.org; Sat, 01 Sep 2012 02:24:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T7h8P-0004ms-Ik for qemu-devel@nongnu.org; Sat, 01 Sep 2012 02:24:46 -0400 Received: from cantor2.suse.de ([195.135.220.15]:52134 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T7h8P-0004mo-89 for qemu-devel@nongnu.org; Sat, 01 Sep 2012 02:24:45 -0400 References: <201208232109.q7NL9P9t016424@int-mx09.intmail.prod.int.phx2.redhat.com> <50408E06.1070009@redhat.com> <20120831142305.GA12212@redhat.com> <128F51EA-270A-41FA-A284-530EB6599C41@suse.de> <20120831191507.GG12212@redhat.com> <20120831205417.GI12212@redhat.com> In-Reply-To: <20120831205417.GI12212@redhat.com> Mime-Version: 1.0 (1.0) Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=us-ascii Message-Id: <8C50A492-161D-439A-984D-AADF9DF9CEF4@suse.de> From: Alexander Graf Date: Fri, 31 Aug 2012 23:25:52 -0700 Subject: Re: [Qemu-devel] [PATCH] ahci: properly reset PxCMD on HBA reset List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Baron Cc: Kevin Wolf , "aliguori@us.ibm.com" , "mst@redhat.com" , "jan.kiszka@siemens.com" , "qemu-devel@nongnu.org" , "lcapitulino@redhat.com" , "yamahata@valinux.co.jp" , "alex.williamson@redhat.com" , "pbonzini@redhat.com" , "afaerber@suse.de" , "armbru@redhat.com" On 31.08.2012, at 13:54, Jason Baron wrote: > On Fri, Aug 31, 2012 at 01:29:46PM -0700, Alexander Graf wrote: >> On 31.08.2012, at 12:15, Jason Baron wrote: >>=20 >>> On Fri, Aug 31, 2012 at 12:04:59PM -0700, Alexander Graf wrote: >>>> On 31.08.2012, at 07:23, Jason Baron wrote: >>>>=20 >>>>> 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 u= ltimate >>>>>>> with sp1 x64), wouldn't install because it can't find the cdrom or d= isk 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 inser= t it now.' >>>>>>> This can also be reproduced on piix by adding an ahci controller, an= d >>>>>>> observing that windows 7 does not see any devices behind it. >>>>>>>=20 >>>>>>> The problem is that when windows issues a HBA reset, qemu does not r= eset the >>>>>>> individual ports' PxCMD register. Windows 7 then reads back the PxCM= D register >>>>>>> and presumably assumes that the ahci controller has already been ini= tialized. >>>>>>> Windows then never sets up the PxIE register to enable interrupts, a= nd thus it >>>>>>> never gets irqs back when it sends ata device inquiry commands. >>>>>>>=20 >>>>>>> I believe this change brings qemu into ahci 1.3 specification compli= ance. >>>>>>>=20 >>>>>>> Section 10.4.3 HBA Reset: >>>>>>>=20 >>>>>>> " >>>>>>> When GHC.HR is set to '1', GHC.AE, GHC.IE, the IS register, and all p= ort >>>>>>> register fields (except PxFB/PxFBU/PxCLB/PxCLBU) that are not HwInit= in the >>>>>>> HBA's register memory space are reset. >>>>>>> " >>>>>>>=20 >>>>>>> I've also re-tested Fedora 16 and 17 to verify that they continue to= work with >>>>>>> this change. >>>>>>>=20 >>>>>>> Signed-off-by: Jason Baron >>>>>>> --- >>>>>>> hw/ide/ahci.c | 1 + >>>>>>> 1 files changed, 1 insertions(+), 0 deletions(-) >>>>>>>=20 >>>>>>> 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 =3D 0; >>>>>>> pr->irq_mask =3D 0; >>>>>>> pr->scr_ctl =3D 0; >>>>>>> + pr->cmd =3D 0; >>>>>>> ahci_reset_port(s, i); >>>>>>> } >>>>>>> } >>>>>>>=20 >>>>>>=20 >>>>>> In ahci_init() we have: >>>>>>=20 >>>>>> ad->port_regs.cmd =3D PORT_CMD_SPIN_UP | PORT_CMD_POWER_ON; >>>>>>=20 >>>>>> Why is resetting to 0 in ahci_reset() correct? I think we can still g= et >>>>>> this part committed for 1.2 after you either explain why 0 is right o= r >>>>>> send a fixed version. >>>>>>=20 >>>>>=20 >>>>> Good catch. I just tried out the following instead in ahci_reset(): >>>>>=20 >>>>> pr->cmd =3D PORT_CMD_SPIN_UP | PORT_CMD_POWER_ON; >>>>>=20 >>>>> 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. >>>>=20 >>>> Since we reset on init, we can just shove it there completely, right? >>>>=20 >>>>=20 >>>=20 >>> 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. >>=20 >> We register the reset handler as generic reset path, no? It should be cal= led on machine init then. Every reset handler gets called on machine boot. >>=20 >=20 > what about hotplug? There we call init, but not reset? Really? It would be broken then already, no? Alex >=20 > -Jason