On Tue, Jan 31, 2017 at 1:56 PM, John Snow <js...@redhat.com> wrote: > > > On 01/16/2017 02:49 AM, Ladi Prosek wrote: >> On Fri, Jan 13, 2017 at 8:15 PM, John Snow <js...@redhat.com> wrote: >>> >>> >>> On 01/13/2017 02:01 PM, Ladi Prosek wrote: >>>> On Fri, Jan 13, 2017 at 7:31 PM, John Snow <js...@redhat.com> wrote: >>>>> >>>>> >>>>> On 01/13/2017 01:12 PM, Ladi Prosek wrote: >>>>>> On Fri, Jan 13, 2017 at 6:23 PM, John Snow <js...@redhat.com> wrote: >>>>>>> On 01/13/2017 06:02 AM, Ladi Prosek wrote: >>>>>>>> The AHCI emulation code supports 64-bit addressing and should >>>>>>>> advertise this >>>>>>>> fact in the Host Capabilities register. Both Linux and Windows drivers >>>>>>>> test >>>>>>>> this bit to decide if the upper 32 bits of various registers may be >>>>>>>> written >>>>>>>> to, and at least some versions of Windows have a bug where DMA is >>>>>>>> attempted >>>>>>>> with an address above 4GB but, in the absence of HOST_CAP_64, the >>>>>>>> upper 32 >>>>>>>> bits are left unititialized which leads to a memory corruption. >>>>>>>> >>>>>>>> Signed-off-by: Ladi Prosek <lpro...@redhat.com> >>>>>>>> --- >>>>>>>> hw/ide/ahci.c | 2 +- >>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>>>> >>>>>>>> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c >>>>>>>> index 3c19bda..6a17acf 100644 >>>>>>>> --- a/hw/ide/ahci.c >>>>>>>> +++ b/hw/ide/ahci.c >>>>>>>> @@ -488,7 +488,7 @@ static void ahci_reg_init(AHCIState *s) >>>>>>>> s->control_regs.cap = (s->ports - 1) | >>>>>>>> (AHCI_NUM_COMMAND_SLOTS << 8) | >>>>>>>> (AHCI_SUPPORTED_SPEED_GEN1 << >>>>>>>> AHCI_SUPPORTED_SPEED) | >>>>>>>> - HOST_CAP_NCQ | HOST_CAP_AHCI; >>>>>>>> + HOST_CAP_NCQ | HOST_CAP_AHCI | HOST_CAP_64; >>>>>>>> >>>>>>>> s->control_regs.impl = (1 << s->ports) - 1; >>>>>>>> >>>>>>>> >>>>>>> >>>>>>> Was this tested? What was the use case that prompted this patch, and do >>>>>>> you have a public bugzilla to point to? >>>>>> >>>>>> Sorry, I thought you were aware. Here's the BZ with details: >>>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1411105 >>>>>> >>>>> >>>>> It's for the public record :) >>>>> >>>>> I'm going to amend your commit message, OK? >>>>> >>>>> https://github.com/jnsnow/qemu/commit/f037be92fc0c4580b846134e0355a69d0eccd0d9 >>>> >>>> Minor nit: the OS we know for sure is affected is Windows Server 2008, >>>> without the "R2". Thanks! >>>> >>> >>> I misunderstood. It looked like the initial report was for "SP2," though >>> I see you saying that the R2 version actually worked okay, but by >>> coincidence. >>> >>> I didn't think there *was* an "SP2," for Windows Server, so I >>> interpreted that to mean R2. >>> >>> So this only manifests with vanilla 2008? >> >> We know it manifests on 2008 SP2, which is very different from 2008 >> R2. 2008 is the server variant of Vista, 2008 R2 is the server variant >> of Win7 (yay for marketing names!) >> >> I believe that 2008 SP2 32-bit is the only OS where it's been confirmed so >> far. >> > > Thanks for clearing that up for me, I re-edited the commit message > upstream. If it looks OK, I'll likely merge this when I get back home > after FOSDEM.
Looks good, thanks! > --js