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