Re: [PATCH v3 3/3] acpi: i386: Move VMBus DSDT entry to SB
On Thu, 25 Jun 2020 05:25:26 +0300 Jon Doron wrote: > On 23/06/2020, Igor Mammedov wrote: > >On Thu, 18 Jun 2020 06:00:27 +0300 > >Jon Doron wrote: > > > >> Signed-off-by: Jon Doron > >> --- > >> 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. >
Re: [PATCH v3 3/3] acpi: i386: Move VMBus DSDT entry to SB
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 wrote: Signed-off-by: Jon Doron --- 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
Re: [PATCH v3 3/3] acpi: i386: Move VMBus DSDT entry to SB
On 23/06/2020, Igor Mammedov wrote: On Thu, 18 Jun 2020 06:00:27 +0300 Jon Doron wrote: Signed-off-by: Jon Doron --- 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.
Re: [PATCH v3 3/3] acpi: i386: Move VMBus DSDT entry to SB
On Thu, 18 Jun 2020 06:00:27 +0300 Jon Doron wrote: > Signed-off-by: Jon Doron > --- > 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 { } ?
[PATCH v3 3/3] acpi: i386: Move VMBus DSDT entry to SB
Signed-off-by: Jon Doron --- 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); -- 2.24.1