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! >> 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