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

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

[Marcel Apfelbaum]

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.

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);
 
-- 
2.4.3

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

[Igor Mammedov]

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.


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

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

[Michael S. Tsirkin]

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?

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

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

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

[Marcel Apfelbaum]

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.

> -
>      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'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);
>>>

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

[Michael S. Tsirkin]

On Mon, Mar 07, 2016 at 03:21:44PM +0200, Marcel Apfelbaum 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.

ACPI tables are guest code, so we need not worry about compatibility
anyway.

> >-
> >     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'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);
> >>>

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

[Igor Mammedov]

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.

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

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

[Marcel Apfelbaum]

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