| Message ID | 1457266676-31535-1-git-send-email-marcel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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); >
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); > >
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); >>>
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); > >>>
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); > >>> >
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); >>>>> >> >
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);
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(+)