| Message ID | 201208232109.q7NL9P9t016424@int-mx09.intmail.prod.int.phx2.redhat.com |
|---|---|
| State | New |
| Headers | show |
On 23.08.2012, at 23:09, Jason Baron wrote: > 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. What a nasty little bug. If it makes it work for you, the change is all fine from my POV (and should go into 1.2). Thanks a lot for pulling off the effort to debug through all this. It sure must've been terribly hard. Alex
On Thu, 23 Aug 2012 17:09:25 -0400 Jason Baron <jbaron@redhat.com> wrote: > 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. I've tested this but unfortunately it doesn't fix the issues I'm getting with OpenBSD 5.1 and NetBSD 5.1 as reported here (section 'master with ahci'): http://lists.gnu.org/archive/html/qemu-devel/2012-08/msg03329.html PS: Those could be kernel issues, of course. > > 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 <jbaron@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); > } > }
Am 24.08.2012 um 07:18 schrieb Luiz Capitulino <lcapitulino@redhat.com>: > On Thu, 23 Aug 2012 17:09:25 -0400 > Jason Baron <jbaron@redhat.com> wrote: > >> 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. > > I've tested this but unfortunately it doesn't fix the issues I'm getting with > OpenBSD 5.1 and NetBSD 5.1 as reported here (section 'master with ahci'): > > http://lists.gnu.org/archive/html/qemu-devel/2012-08/msg03329.html > > PS: Those could be kernel issues, of course. The BSDs already fail in detecting the cotroller as a whole, right? Not just disks on it. Alex > >> >> 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 <jbaron@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); >> } >> } >
On Fri, 24 Aug 2012 07:34:51 -0700 Alexander Graf <agraf@suse.de> wrote: > > > Am 24.08.2012 um 07:18 schrieb Luiz Capitulino <lcapitulino@redhat.com>: > > > On Thu, 23 Aug 2012 17:09:25 -0400 > > Jason Baron <jbaron@redhat.com> wrote: > > > >> 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. > > > > I've tested this but unfortunately it doesn't fix the issues I'm getting with > > OpenBSD 5.1 and NetBSD 5.1 as reported here (section 'master with ahci'): > > > > http://lists.gnu.org/archive/html/qemu-devel/2012-08/msg03329.html > > > > PS: Those could be kernel issues, of course. > > The BSDs already fail in detecting the cotroller as a whole, right? Not just disks on it. Thanks for the clarification. Didn't try to understand the patch, automatically thought it could be related to my last testing report.
On Fri, Aug 24, 2012 at 06:39:02AM +0200, Alexander Graf wrote: > > 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. > > What a nasty little bug. If it makes it work for you, the change is all fine from my POV (and should go into 1.2). > Ok, I don't see it 1.2.0-rc2. If others are ok with this for 1.2, whose tree should this go through? Thanks, -Jason
On 30.08.2012, at 12:59, Jason Baron <jbaron@redhat.com> wrote: > On Fri, Aug 24, 2012 at 06:39:02AM +0200, Alexander Graf wrote: >>> 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. >> >> What a nasty little bug. If it makes it work for you, the change is all fine from my POV (and should go into 1.2). >> > > Ok, I don't see it 1.2.0-rc2. If others are ok with this for 1.2, whose > tree should this go through? Kevin :) Alex > > Thanks, > > -Jason
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 <jbaron@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. I think for 1.3 there's still a bit more left. We're missing some more fields in a reset: tfdata isn't set at all, should be set to 0x7f in ahci_reset_port(), and cmd_issue should be set to 0. And shouldn't ahci_init() actually call ahci_reset() in order to ensure consistent results? Kevin
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 <jbaron@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. > I think for 1.3 there's still a bit more left. We're missing some more > fields in a reset: tfdata isn't set at all, should be set to 0x7f in > ahci_reset_port(), and cmd_issue should be set to 0. And shouldn't > ahci_init() actually call ahci_reset() in order to ensure consistent > results? > Agreed, those all look like good additional cleanups for 1.3. I will add them to my q35 treee for testing. Thanks, -Jason
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 <jbaron@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? Alex
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 <jbaron@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. Thanks, -Jason
On 31.08.2012, at 12:15, Jason Baron <jbaron@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 <jbaron@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
On Fri, Aug 31, 2012 at 01:29:46PM -0700, Alexander Graf wrote: > On 31.08.2012, at 12:15, Jason Baron <jbaron@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 <jbaron@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. > Right, I guess I was concerned that there might be a path where init is called but not reset. I also was being cautious as I wanted this for 1.2. That said, it does seem safe to remove it from init. Thanks, -Jason
On Fri, Aug 31, 2012 at 01:29:46PM -0700, Alexander Graf wrote: > On 31.08.2012, at 12:15, Jason Baron <jbaron@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 <jbaron@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. > what about hotplug? There we call init, but not reset? -Jason
On 31.08.2012, at 13:54, Jason Baron <jbaron@redhat.com> wrote: > On Fri, Aug 31, 2012 at 01:29:46PM -0700, Alexander Graf wrote: >> On 31.08.2012, at 12:15, Jason Baron <jbaron@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 <jbaron@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. >> > > what about hotplug? There we call init, but not reset? Really? It would be broken then already, no? Alex > > -Jason
On Fri, Aug 31, 2012 at 11:25:52PM -0700, Alexander Graf wrote: > On 31.08.2012, at 13:54, Jason Baron <jbaron@redhat.com> wrote: > > > On Fri, Aug 31, 2012 at 01:29:46PM -0700, Alexander Graf wrote: > >> On 31.08.2012, at 12:15, Jason Baron <jbaron@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 <jbaron@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. > >> > > > > what about hotplug? There we call init, but not reset? > > Really? It would be broken then already, no? > Yes, you are right - hotplug calls reset. I've thus dropped the clear from the init path. Please see my v2 posting. Thanks, -Jason
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); } }
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 <jbaron@redhat.com> --- hw/ide/ahci.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)