diff mbox series

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

Message ID 20200618030027.759594-4-arilou@gmail.com
State New
Headers show
Series hyperv: vmbus: ACPI various corrections | expand

Commit Message

Jon Doron June 18, 2020, 3 a.m. UTC 
Signed-off-by: Jon Doron <arilou@gmail.com>
---
 hw/i386/acpi-build.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Comments

Igor Mammedov June 23, 2020, 3:09 p.m. UTC | #1 
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 {
}

?
Jon Doron June 25, 2020, 2:25 a.m. UTC | #2 
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.
Jon Doron June 25, 2020, 4:46 a.m. UTC | #3 
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
Igor Mammedov June 25, 2020, 6:39 p.m. UTC | #4 
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 mbox series

Patch

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