Message ID | 20200618030027.759594-4-arilou@gmail.com |
---|---|
State | New |
Headers | show |
Series | hyperv: vmbus: ACPI various corrections | expand |
On Thu, 18 Jun 2020 06:00:27 +0300 Jon Doron <arilou@gmail.com> wrote: > Signed-off-by: Jon Doron <arilou@gmail.com> > --- > hw/i386/acpi-build.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 6d9df38e31..38be9e5a58 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -1092,7 +1092,6 @@ static Aml *build_vmbus_device_aml(VMBusBridge *vmbus_bridge) > static void build_isa_devices_aml(Aml *table) > { > ISADevice *fdc = pc_find_fdc0(); > - VMBusBridge *vmbus_bridge = vmbus_bridge_find(); > bool ambiguous; > > Aml *scope = aml_scope("_SB.PCI0.ISA"); > @@ -1113,10 +1112,6 @@ static void build_isa_devices_aml(Aml *table) > isa_build_aml(ISA_BUS(obj), scope); > } > > - if (vmbus_bridge) { > - aml_append(scope, build_vmbus_device_aml(vmbus_bridge)); > - } > - > aml_append(table, scope); > } > > @@ -1661,6 +1656,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > PCIBus *bus = NULL; > TPMIf *tpm = tpm_find(); > int i; > + VMBusBridge *vmbus_bridge = vmbus_bridge_find(); > > dsdt = init_aml_allocator(); > > @@ -1675,6 +1671,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > aml_append(dev, aml_name_decl("_ADR", aml_int(0))); > aml_append(dev, aml_name_decl("_UID", aml_int(1))); > aml_append(sb_scope, dev); > + if (vmbus_bridge) { > + aml_append(sb_scope, build_vmbus_device_aml(vmbus_bridge)); > + } > aml_append(dsdt, sb_scope); > > build_hpet_aml(dsdt); > @@ -1692,6 +1691,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > aml_append(dev, aml_name_decl("_UID", aml_int(1))); > aml_append(dev, build_q35_osc_method()); > aml_append(sb_scope, dev); > + if (vmbus_bridge) { > + aml_append(sb_scope, build_vmbus_device_aml(vmbus_bridge)); > + } > aml_append(dsdt, sb_scope); > > build_hpet_aml(dsdt); why are you duplicating instead of putting one if () block after if (misc->is_piix4) { } else { } ?
On 23/06/2020, Igor Mammedov wrote: >On Thu, 18 Jun 2020 06:00:27 +0300 >Jon Doron <arilou@gmail.com> wrote: > >> Signed-off-by: Jon Doron <arilou@gmail.com> >> --- >> hw/i386/acpi-build.c | 12 +++++++----- >> 1 file changed, 7 insertions(+), 5 deletions(-) >> >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c >> index 6d9df38e31..38be9e5a58 100644 >> --- a/hw/i386/acpi-build.c >> +++ b/hw/i386/acpi-build.c >> @@ -1092,7 +1092,6 @@ static Aml *build_vmbus_device_aml(VMBusBridge *vmbus_bridge) >> static void build_isa_devices_aml(Aml *table) >> { >> ISADevice *fdc = pc_find_fdc0(); >> - VMBusBridge *vmbus_bridge = vmbus_bridge_find(); >> bool ambiguous; >> >> Aml *scope = aml_scope("_SB.PCI0.ISA"); >> @@ -1113,10 +1112,6 @@ static void build_isa_devices_aml(Aml *table) >> isa_build_aml(ISA_BUS(obj), scope); >> } >> >> - if (vmbus_bridge) { >> - aml_append(scope, build_vmbus_device_aml(vmbus_bridge)); >> - } >> - >> aml_append(table, scope); >> } >> >> @@ -1661,6 +1656,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, >> PCIBus *bus = NULL; >> TPMIf *tpm = tpm_find(); >> int i; >> + VMBusBridge *vmbus_bridge = vmbus_bridge_find(); >> >> dsdt = init_aml_allocator(); >> >> @@ -1675,6 +1671,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, >> aml_append(dev, aml_name_decl("_ADR", aml_int(0))); >> aml_append(dev, aml_name_decl("_UID", aml_int(1))); >> aml_append(sb_scope, dev); >> + if (vmbus_bridge) { >> + aml_append(sb_scope, build_vmbus_device_aml(vmbus_bridge)); >> + } >> aml_append(dsdt, sb_scope); >> >> build_hpet_aml(dsdt); >> @@ -1692,6 +1691,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, >> aml_append(dev, aml_name_decl("_UID", aml_int(1))); >> aml_append(dev, build_q35_osc_method()); >> aml_append(sb_scope, dev); >> + if (vmbus_bridge) { >> + aml_append(sb_scope, build_vmbus_device_aml(vmbus_bridge)); >> + } >> aml_append(dsdt, sb_scope); >> >> build_hpet_aml(dsdt); >why are you duplicating instead of putting one if () block after > >if (misc->is_piix4) { >} else { >} > >? > Well it seems like Windows is very "picky" about where you declare the VMBS not sure why if i had moved it to the suggested location as such if (misc->is_piix4) { } else { } if (vmbus_bridge) { aml_append(sb_scope, build_vmbus_device_aml(vmbus_bridge)); aml_append(dsdt, sb_scope); } Windows would BSOD right away with ACPI error. Same goes for declaring it before PCI0 device... -- Jon.
On 25/06/2020, Jon Doron wrote: >On 23/06/2020, Igor Mammedov wrote: >>On Thu, 18 Jun 2020 06:00:27 +0300 >>Jon Doron <arilou@gmail.com> wrote: >> >>>Signed-off-by: Jon Doron <arilou@gmail.com> >>>--- >>> hw/i386/acpi-build.c | 12 +++++++----- >>> 1 file changed, 7 insertions(+), 5 deletions(-) >>> >>>diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c >>>index 6d9df38e31..38be9e5a58 100644 >>>--- a/hw/i386/acpi-build.c >>>+++ b/hw/i386/acpi-build.c >>>@@ -1092,7 +1092,6 @@ static Aml *build_vmbus_device_aml(VMBusBridge *vmbus_bridge) >>> static void build_isa_devices_aml(Aml *table) >>> { >>> ISADevice *fdc = pc_find_fdc0(); >>>- VMBusBridge *vmbus_bridge = vmbus_bridge_find(); >>> bool ambiguous; >>> >>> Aml *scope = aml_scope("_SB.PCI0.ISA"); >>>@@ -1113,10 +1112,6 @@ static void build_isa_devices_aml(Aml *table) >>> isa_build_aml(ISA_BUS(obj), scope); >>> } >>> >>>- if (vmbus_bridge) { >>>- aml_append(scope, build_vmbus_device_aml(vmbus_bridge)); >>>- } >>>- >>> aml_append(table, scope); >>> } >>> >>>@@ -1661,6 +1656,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, >>> PCIBus *bus = NULL; >>> TPMIf *tpm = tpm_find(); >>> int i; >>>+ VMBusBridge *vmbus_bridge = vmbus_bridge_find(); >>> >>> dsdt = init_aml_allocator(); >>> >>>@@ -1675,6 +1671,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, >>> aml_append(dev, aml_name_decl("_ADR", aml_int(0))); >>> aml_append(dev, aml_name_decl("_UID", aml_int(1))); >>> aml_append(sb_scope, dev); >>>+ if (vmbus_bridge) { >>>+ aml_append(sb_scope, build_vmbus_device_aml(vmbus_bridge)); >>>+ } >>> aml_append(dsdt, sb_scope); >>> >>> build_hpet_aml(dsdt); >>>@@ -1692,6 +1691,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, >>> aml_append(dev, aml_name_decl("_UID", aml_int(1))); >>> aml_append(dev, build_q35_osc_method()); >>> aml_append(sb_scope, dev); >>>+ if (vmbus_bridge) { >>>+ aml_append(sb_scope, build_vmbus_device_aml(vmbus_bridge)); >>>+ } >>> aml_append(dsdt, sb_scope); >>> >>> build_hpet_aml(dsdt); >>why are you duplicating instead of putting one if () block after >> >>if (misc->is_piix4) { >>} else { >>} >> >>? >> > >Well it seems like Windows is very "picky" about where you declare the >VMBS not sure why if i had moved it to the suggested location as such > >if (misc->is_piix4) { >} else { >} > >if (vmbus_bridge) { > aml_append(sb_scope, build_vmbus_device_aml(vmbus_bridge)); > aml_append(dsdt, sb_scope); >} > >Windows would BSOD right away with ACPI error. > >Same goes for declaring it before PCI0 device... > >-- Jon. Never mind did a silly mistake ill send a new set shortly
On Thu, 25 Jun 2020 05:25:26 +0300 Jon Doron <arilou@gmail.com> wrote: > On 23/06/2020, Igor Mammedov wrote: > >On Thu, 18 Jun 2020 06:00:27 +0300 > >Jon Doron <arilou@gmail.com> wrote: > > > >> Signed-off-by: Jon Doron <arilou@gmail.com> > >> --- > >> hw/i386/acpi-build.c | 12 +++++++----- > >> 1 file changed, 7 insertions(+), 5 deletions(-) > >> > >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > >> index 6d9df38e31..38be9e5a58 100644 > >> --- a/hw/i386/acpi-build.c > >> +++ b/hw/i386/acpi-build.c > >> @@ -1092,7 +1092,6 @@ static Aml *build_vmbus_device_aml(VMBusBridge *vmbus_bridge) > >> static void build_isa_devices_aml(Aml *table) > >> { > >> ISADevice *fdc = pc_find_fdc0(); > >> - VMBusBridge *vmbus_bridge = vmbus_bridge_find(); > >> bool ambiguous; > >> > >> Aml *scope = aml_scope("_SB.PCI0.ISA"); > >> @@ -1113,10 +1112,6 @@ static void build_isa_devices_aml(Aml *table) > >> isa_build_aml(ISA_BUS(obj), scope); > >> } > >> > >> - if (vmbus_bridge) { > >> - aml_append(scope, build_vmbus_device_aml(vmbus_bridge)); > >> - } > >> - > >> aml_append(table, scope); > >> } > >> > >> @@ -1661,6 +1656,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > >> PCIBus *bus = NULL; > >> TPMIf *tpm = tpm_find(); > >> int i; > >> + VMBusBridge *vmbus_bridge = vmbus_bridge_find(); > >> > >> dsdt = init_aml_allocator(); > >> > >> @@ -1675,6 +1671,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > >> aml_append(dev, aml_name_decl("_ADR", aml_int(0))); > >> aml_append(dev, aml_name_decl("_UID", aml_int(1))); > >> aml_append(sb_scope, dev); > >> + if (vmbus_bridge) { > >> + aml_append(sb_scope, build_vmbus_device_aml(vmbus_bridge)); > >> + } > >> aml_append(dsdt, sb_scope); > >> > >> build_hpet_aml(dsdt); > >> @@ -1692,6 +1691,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > >> aml_append(dev, aml_name_decl("_UID", aml_int(1))); > >> aml_append(dev, build_q35_osc_method()); > >> aml_append(sb_scope, dev); > >> + if (vmbus_bridge) { > >> + aml_append(sb_scope, build_vmbus_device_aml(vmbus_bridge)); > >> + } > >> aml_append(dsdt, sb_scope); > >> > >> build_hpet_aml(dsdt); > >why are you duplicating instead of putting one if () block after > > > >if (misc->is_piix4) { > >} else { > >} > > > >? > > > > Well it seems like Windows is very "picky" about where you declare the > VMBS not sure why if i had moved it to the suggested location as such > > if (misc->is_piix4) { > } else { > } > > if (vmbus_bridge) { > aml_append(sb_scope, build_vmbus_device_aml(vmbus_bridge)); > aml_append(dsdt, sb_scope); > } > > Windows would BSOD right away with ACPI error. > > Same goes for declaring it before PCI0 device... just FYI, Windows has ACPI debugger, so if you attach wingdb at early boot you can set acpi breakpoint and after that enable all kind of debug info (see MSDN) that usually say why exactly it doesn't like something in AML. It used to require checked build and it is tricky to enable but once you figured how, it works like charm. > > -- Jon. >
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 6d9df38e31..38be9e5a58 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -1092,7 +1092,6 @@ static Aml *build_vmbus_device_aml(VMBusBridge *vmbus_bridge) static void build_isa_devices_aml(Aml *table) { ISADevice *fdc = pc_find_fdc0(); - VMBusBridge *vmbus_bridge = vmbus_bridge_find(); bool ambiguous; Aml *scope = aml_scope("_SB.PCI0.ISA"); @@ -1113,10 +1112,6 @@ static void build_isa_devices_aml(Aml *table) isa_build_aml(ISA_BUS(obj), scope); } - if (vmbus_bridge) { - aml_append(scope, build_vmbus_device_aml(vmbus_bridge)); - } - aml_append(table, scope); } @@ -1661,6 +1656,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, PCIBus *bus = NULL; TPMIf *tpm = tpm_find(); int i; + VMBusBridge *vmbus_bridge = vmbus_bridge_find(); dsdt = init_aml_allocator(); @@ -1675,6 +1671,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, aml_append(dev, aml_name_decl("_ADR", aml_int(0))); aml_append(dev, aml_name_decl("_UID", aml_int(1))); aml_append(sb_scope, dev); + if (vmbus_bridge) { + aml_append(sb_scope, build_vmbus_device_aml(vmbus_bridge)); + } aml_append(dsdt, sb_scope); build_hpet_aml(dsdt); @@ -1692,6 +1691,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, aml_append(dev, aml_name_decl("_UID", aml_int(1))); aml_append(dev, build_q35_osc_method()); aml_append(sb_scope, dev); + if (vmbus_bridge) { + aml_append(sb_scope, build_vmbus_device_aml(vmbus_bridge)); + } aml_append(dsdt, sb_scope); build_hpet_aml(dsdt);
Signed-off-by: Jon Doron <arilou@gmail.com> --- hw/i386/acpi-build.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)