[v1] Consolidate PIIX south bridges

[PATCH 42/42] hw/i386/acpi-build: Resolve PIIX ISA bridge rather than ACPI controller

Posted by Bernhard Beschow 3 years, 5 months ago

Resolving the PIIX ISA bridge rather than the PIIX ACPI controller mirrors
the ICH9 code one line below.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i386/acpi-build.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 8af75b1e22..d7bb1ccb26 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -288,7 +288,7 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm)
 
 static void acpi_get_misc_info(AcpiMiscInfo *info)
 {
-    Object *piix = object_resolve_type_unambiguous(TYPE_PIIX4_PM);
+    Object *piix = object_resolve_type_unambiguous(TYPE_PIIX_PCI_DEVICE);
     Object *lpc = object_resolve_type_unambiguous(TYPE_ICH9_LPC_DEVICE);
     assert(!!piix != !!lpc);
 
-- 
2.37.3

Re: [PATCH 42/42] hw/i386/acpi-build: Resolve PIIX ISA bridge rather than ACPI controller

Posted by Philippe Mathieu-Daudé via 3 years, 5 months ago

On 1/9/22 18:26, Bernhard Beschow wrote:
> Resolving the PIIX ISA bridge rather than the PIIX ACPI controller mirrors
> the ICH9 code one line below.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/i386/acpi-build.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 8af75b1e22..d7bb1ccb26 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -288,7 +288,7 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm)
>   
>   static void acpi_get_misc_info(AcpiMiscInfo *info)
>   {
> -    Object *piix = object_resolve_type_unambiguous(TYPE_PIIX4_PM);
> +    Object *piix = object_resolve_type_unambiguous(TYPE_PIIX_PCI_DEVICE);
>       Object *lpc = object_resolve_type_unambiguous(TYPE_ICH9_LPC_DEVICE);
>       assert(!!piix != !!lpc);
>   

This looks correct to  me w.r.t the hardware, but my understanding is
some x86 machines allow abusing the PIIX ACPI PCI function, by plugging
it alone, without the rest of the south bridge... Then this patch would
regress such Frankenstein use :/

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH 42/42] hw/i386/acpi-build: Resolve PIIX ISA bridge rather than ACPI controller

Posted by Bernhard Beschow 3 years, 5 months ago

Am 1. September 2022 21:05:03 UTC schrieb "Philippe Mathieu-Daudé" <f4bug@amsat.org>:
>On 1/9/22 18:26, Bernhard Beschow wrote:
>> Resolving the PIIX ISA bridge rather than the PIIX ACPI controller mirrors
>> the ICH9 code one line below.
>> 
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>>   hw/i386/acpi-build.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index 8af75b1e22..d7bb1ccb26 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -288,7 +288,7 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm)
>>     static void acpi_get_misc_info(AcpiMiscInfo *info)
>>   {
>> -    Object *piix = object_resolve_type_unambiguous(TYPE_PIIX4_PM);
>> +    Object *piix = object_resolve_type_unambiguous(TYPE_PIIX_PCI_DEVICE);
>>       Object *lpc = object_resolve_type_unambiguous(TYPE_ICH9_LPC_DEVICE);
>>       assert(!!piix != !!lpc);
>>   
>
>This looks correct to  me w.r.t the hardware, but my understanding is
>some x86 machines allow abusing the PIIX ACPI PCI function, by plugging
>it alone, without the rest of the south bridge... Then this patch would
>regress such Frankenstein use :/

TYPE_PIIX4_PM is not user-creatable and is only instantiated in pc_piix.c if the southbridge is as well, so those should be equivalent. Since acpi_get_misc_info() is more about the board and south bridge and not about PM, I think checking for TYPE_PIIX_PCI_DEVICE is therefore more appropriate here.

>
>Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>