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? >>> In short, Windows guests run into issues in certain virtual HW >>> configurations if the bit is not set. I have tested the patch and >>> verified that it fixes the failing cases. >>> >> >> Great. I'm CCing qemu-stable as this should probably be included in >> 2.7.2 / 2.8.1 / etc. >> >>>> It looks correct via the spec, thank you for finding this oversight. It >>>> looks like everywhere this bit would make a difference we already do the >>>> correct thing for having this bit set. >>>> >>>> From what I can gather from the spec, it looks as if even 32bit guests >>>> must ensure that the upper 32bit registers are cleared, so I think we're >>>> all set here. >>>> >>>> Reviewed-by: John Snow <js...@redhat.com> >> >> Thanks, applied to my IDE tree: >> >> https://github.com/jnsnow/qemu/commits/ide >> https://github.com/jnsnow/qemu.git >> >> --js