Re: [Qemu-devel] [PATCH] hw/acpi: fix Q35 support for legacy Windows OS

[Marcel Apfelbaum <marcel@redhat.com>]

On 03/07/2016 05:16 PM, Igor Mammedov wrote:
> On Mon, 7 Mar 2016 15:21:44 +0200
> Marcel Apfelbaum <marcel@redhat.com> wrote:
>
>> On 03/07/2016 02:36 PM, Michael S. Tsirkin wrote:
>>> On Mon, Mar 07, 2016 at 01:06:43PM +0100, Igor Mammedov wrote:
>>>> On Sun,  6 Mar 2016 14:17:56 +0200
>>>> Marcel Apfelbaum <marcel@redhat.com> wrote:
>>>>
>>>>> Legacy Windows operating systems like Windows XP and Windows 2003
>>>>> require _DIS method to be present for all interrupt links.
>>>>>
>>>>> PC machines already have a no-op implemented for GSI links, add
>>>>> it also in Q35.
>>>> Maybe I don't see but only LINKS has no-op _DIS, the rest of
>>>> links have _DIS that does something. According to spec
>>>> _STA must return disabled status after _DIS has been called.
>>>
>>> A no-op _DIS is OK as long as the interrupt can not be disabled.
>>> Marcel, how about a comment explaining this?
>>
>> Sure, I'll add a comment and re-post.
>>
>>>
>>> BTW, going over it again:
>>> - we really need to set separate UIDs for them, might
>>>     be a good idea to fix that, might it not?
>>
>> Absolutely (in my opinion), Igor is OK if I'll submit a patch for it?
>> Since we're getting rid of old Q35 machines we don't care about compatibility here.
> Sure, provided Windows doesn't complain/crash.
>

I'll be sure to check enough Windows guests,
Thanks,
Marcel

>>
>>> -
>>>       method = aml_method("_SRS", 1, AML_NOTSERIALIZED);
>>>       aml_append(method, aml_create_dword_field(aml_arg(0), aml_int(5), "PRRI"));
>>>       aml_append(method, aml_store(aml_name("PRRI"), reg));
>>>       aml_append(dev, method);
>>>
>>> should this be serialized as it creates a local name (PRRI)?
> I'd make it serialize for other reason,
> it shouldn't happen but what if method were executed in parallel
>
>>>
>>
>> I'll let Igor answer this.
>>
>> Thanks,
>> Marcel
>>
>>>>
>>>>>
>>>>> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
>>>>> ---
>>>>>
>>>>> Hi,
>>>>>
>>>>> I tested this patch with WinXP and Win 2003, but also with Win 10, and Fedora.
>>>>> This solves a BSOD early in the setup process.
>>>>>
>>>>> WinXP/2003 can be tested using:
>>>>>        -device piix3-ide,id=legacyide \
>>>>>        -drive file=winxp-q35.qcow2,if=none,id=disk -device ide-hd,drive=disk,bus=legacyide.0 \
>>>>>        -drive file=xp_sp3.iso,if=none,id=cdrom -device ide-cd,drive=cdrom,bus=legacyide.1
>>>>>
>>>>> Please note that you have to use the piix3-ide because Win2003 (and probably XP)
>>>>> does not have Q35 AHCI drivers inbox.
>>>>>
>>>>> Thanks,
>>>>> Marcel
>>>>>
>>>>>    hw/i386/acpi-build.c | 3 +++
>>>>>    1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>>>>> index 52c9470..4c66568 100644
>>>>> --- a/hw/i386/acpi-build.c
>>>>> +++ b/hw/i386/acpi-build.c
>>>>> @@ -1509,6 +1509,9 @@ static Aml *build_gsi_link_dev(const char *name, uint8_t uid, uint8_t gsi)
>>>>>
>>>>>        aml_append(dev, aml_name_decl("_CRS", crs));
>>>>>
>>>>> +    method = aml_method("_DIS", 0, AML_NOTSERIALIZED);
>>>>> +    aml_append(dev, method);
>>>>> +
>>>>>        method = aml_method("_SRS", 1, AML_NOTSERIALIZED);
>>>>>        aml_append(dev, method);
>>>>>
>>
>