qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] ahci: advertise HOST_CAP_64


From: John Snow
Subject: Re: [Qemu-devel] [PATCH] ahci: advertise HOST_CAP_64
Date: Fri, 13 Jan 2017 14:15:53 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0


On 01/13/2017 02:01 PM, Ladi Prosek wrote:
> On Fri, Jan 13, 2017 at 7:31 PM, John Snow <address@hidden> wrote:
>>
>>
>> On 01/13/2017 01:12 PM, Ladi Prosek wrote:
>>> On Fri, Jan 13, 2017 at 6:23 PM, John Snow <address@hidden> 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 <address@hidden>
>>>>> ---
>>>>>  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 <address@hidden>
>>
>> Thanks, applied to my IDE tree:
>>
>> https://github.com/jnsnow/qemu/commits/ide
>> https://github.com/jnsnow/qemu.git
>>
>> --js



reply via email to

[Prev in Thread] Current Thread [Next in Thread]