Re: [PATCH v3 3/3] acpi: i386: Move VMBus DSDT entry to SB

[Igor Mammedov <imammedo@redhat.com>]

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.
>