[v3] acpi: i386 tweaks

[PATCH v3 15/15] acpi: simplify build_isa_devices_aml()

Posted by Gerd Hoffmann 5 years, 9 months ago

x86 machines can have a single ISA bus only.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/i386/acpi-build.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index a8b790021974..98b3fd1cdb14 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1055,19 +1055,14 @@ static void build_hpet_aml(Aml *table)
 static void build_isa_devices_aml(Aml *table)
 {
     bool ambiguous;
-
-    Aml *scope = aml_scope("_SB.PCI0.ISA");
     Object *obj = object_resolve_path_type("", TYPE_ISA_BUS, &ambiguous);
+    Aml *scope;
 
-    if (ambiguous) {
-        error_report("Multiple ISA busses, unable to define IPMI ACPI data");
-    } else if (!obj) {
-        error_report("No ISA bus, unable to define IPMI ACPI data");
-    } else {
-        build_acpi_ipmi_devices(scope, BUS(obj), "\\_SB.PCI0.ISA");
-        isa_build_aml(ISA_BUS(obj), scope);
-    }
+    assert(obj && !ambiguous);
 
+    scope = aml_scope("_SB.PCI0.ISA");
+    build_acpi_ipmi_devices(scope, BUS(obj), "\\_SB.PCI0.ISA");
+    isa_build_aml(ISA_BUS(obj), scope);
     aml_append(table, scope);
 }
 
-- 
2.18.2

Re: [PATCH v3 15/15] acpi: simplify build_isa_devices_aml()

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

Cc'ing IPMI maintainer.

On 4/29/20 4:00 PM, Gerd Hoffmann wrote:
> x86 machines can have a single ISA bus only.

I disagree with the comment.
Machines can have multiple ISA bus.
However the PCI IO space can have a single ISA bus.
This patch is about PCI0.

> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>   hw/i386/acpi-build.c | 15 +++++----------
>   1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index a8b790021974..98b3fd1cdb14 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1055,19 +1055,14 @@ static void build_hpet_aml(Aml *table)
>   static void build_isa_devices_aml(Aml *table)
>   {
>       bool ambiguous;
> -
> -    Aml *scope = aml_scope("_SB.PCI0.ISA");
>       Object *obj = object_resolve_path_type("", TYPE_ISA_BUS, &ambiguous);
> +    Aml *scope;
>   
> -    if (ambiguous) {
> -        error_report("Multiple ISA busses, unable to define IPMI ACPI data");
> -    } else if (!obj) {
> -        error_report("No ISA bus, unable to define IPMI ACPI data");
> -    } else {
> -        build_acpi_ipmi_devices(scope, BUS(obj), "\\_SB.PCI0.ISA");
> -        isa_build_aml(ISA_BUS(obj), scope);
> -    }
> +    assert(obj && !ambiguous);
>   
> +    scope = aml_scope("_SB.PCI0.ISA");
> +    build_acpi_ipmi_devices(scope, BUS(obj), "\\_SB.PCI0.ISA");
> +    isa_build_aml(ISA_BUS(obj), scope);
>       aml_append(table, scope);
>   }
>   
>

Re: [PATCH v3 15/15] acpi: simplify build_isa_devices_aml()

Posted by Gerd Hoffmann 5 years, 9 months ago

On Thu, Apr 30, 2020 at 08:48:31AM +0200, Philippe Mathieu-Daudé wrote:
> Cc'ing IPMI maintainer.
> 
> On 4/29/20 4:00 PM, Gerd Hoffmann wrote:
> > x86 machines can have a single ISA bus only.
> 
> I disagree with the comment.
> Machines can have multiple ISA bus.

Note *x86* machines.  Given x86 has a global io address space I still
think this is true.

On some !x86 archs where a mmio window on the bridge is used for the io
address space multiple isa busses are possible.  I'm not sure whenever
this is purely theoretical or whenever such machines exist in practice
and in case of the latter whenever qemu can emulate one.

> >   hw/i386/acpi-build.c | 15 +++++----------
         ^^^^
But in any case !x86 doesn't matter here ;)

take care,
  Gerd

Re: [PATCH v3 15/15] acpi: simplify build_isa_devices_aml()

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


On 5/4/20 3:46 PM, Gerd Hoffmann wrote:
> On Thu, Apr 30, 2020 at 08:48:31AM +0200, Philippe Mathieu-Daudé wrote:
>> Cc'ing IPMI maintainer.
>>
>> On 4/29/20 4:00 PM, Gerd Hoffmann wrote:
>>> x86 machines can have a single ISA bus only.
>>
>> I disagree with the comment.
>> Machines can have multiple ISA bus.
> 
> Note *x86* machines.  Given x86 has a global io address space I still
> think this is true.
> 
> On some !x86 archs where a mmio window on the bridge is used for the io
> address space multiple isa busses are possible.  I'm not sure whenever
> this is purely theoretical or whenever such machines exist in practice
> and in case of the latter whenever qemu can emulate one.

It currently can't.

> 
>>>    hw/i386/acpi-build.c | 15 +++++----------
>           ^^^^
> But in any case !x86 doesn't matter here ;)

Oops I didn't notice...

> 
> take care,
>    Gerd
>