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. --js