[v2,06/12] acpi: add ISADeviceClass->build_aml()
diff mbox series

Message ID 20200403080502.8154-7-kraxel@redhat.com
State New
Headers show
Series
  • acpi: i386 tweaks
Related show

Commit Message

Gerd Hoffmann April 3, 2020, 8:04 a.m. UTC
Also add isa_aml_build() function which walks all isa devices.
This allows to move aml builder code to isa devices.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/hw/isa/isa.h |  2 ++
 hw/i386/acpi-build.c |  1 +
 hw/isa/isa-bus.c     | 15 +++++++++++++++
 3 files changed, 18 insertions(+)

Comments

Igor Mammedov April 3, 2020, 10:05 a.m. UTC | #1
On Fri,  3 Apr 2020 10:04:56 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> Also add isa_aml_build() function which walks all isa devices.
> This allows to move aml builder code to isa devices.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  include/hw/isa/isa.h |  2 ++
>  hw/i386/acpi-build.c |  1 +
>  hw/isa/isa-bus.c     | 15 +++++++++++++++
>  3 files changed, 18 insertions(+)
> 
> diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
> index e9ac1f1205a4..1534f8826453 100644
> --- a/include/hw/isa/isa.h
> +++ b/include/hw/isa/isa.h
> @@ -70,6 +70,7 @@ typedef struct IsaDmaClass {
>  
>  typedef struct ISADeviceClass {
>      DeviceClass parent_class;
> +    void (*build_aml)(ISADevice *dev, Aml *scope);
>  } ISADeviceClass;
>  
>  struct ISABus {
> @@ -108,6 +109,7 @@ ISADevice *isa_try_create(ISABus *bus, const char *name);
>  ISADevice *isa_create_simple(ISABus *bus, const char *name);
>  
>  ISADevice *isa_vga_init(ISABus *bus);
> +void isa_build_aml(ISABus *bus, Aml *scope);
>  
>  /**
>   * isa_register_ioport: Install an I/O port region on the ISA bus.
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 5d2b9b099684..77fc9df74735 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1259,6 +1259,7 @@ static void build_isa_devices_aml(Aml *table)
>          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);
>      }
>  
>      aml_append(table, scope);
> diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
> index 798dd9194e8f..1f2189f4d5db 100644
> --- a/hw/isa/isa-bus.c
> +++ b/hw/isa/isa-bus.c
> @@ -207,6 +207,21 @@ ISADevice *isa_vga_init(ISABus *bus)
>      }
>  }
>  
> +void isa_build_aml(ISABus *bus, Aml *scope)
> +{
> +    BusChild *kid;
> +    ISADevice *dev;
> +    ISADeviceClass *dc;
> +
> +    QTAILQ_FOREACH(kid, &bus->parent_obj.children, sibling) {
> +        dev = ISA_DEVICE(kid->child);
> +        dc = ISA_DEVICE_GET_CLASS(dev);
> +        if (dc->build_aml) {
> +            dc->build_aml(dev, scope);
> +        }
> +    }
> +}
> +
>  static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent)
>  {
>      ISADevice *d = ISA_DEVICE(dev);
Igor Mammedov April 6, 2020, 10:22 a.m. UTC | #2
On Fri,  3 Apr 2020 10:04:56 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> Also add isa_aml_build() function which walks all isa devices.
> This allows to move aml builder code to isa devices.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/hw/isa/isa.h |  2 ++
>  hw/i386/acpi-build.c |  1 +
>  hw/isa/isa-bus.c     | 15 +++++++++++++++
>  3 files changed, 18 insertions(+)
> 
> diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
> index e9ac1f1205a4..1534f8826453 100644
> --- a/include/hw/isa/isa.h
> +++ b/include/hw/isa/isa.h
> @@ -70,6 +70,7 @@ typedef struct IsaDmaClass {
>  
>  typedef struct ISADeviceClass {
>      DeviceClass parent_class;
> +    void (*build_aml)(ISADevice *dev, Aml *scope);
>  } ISADeviceClass;
>  
>  struct ISABus {
> @@ -108,6 +109,7 @@ ISADevice *isa_try_create(ISABus *bus, const char *name);
>  ISADevice *isa_create_simple(ISABus *bus, const char *name);
>  
>  ISADevice *isa_vga_init(ISABus *bus);
> +void isa_build_aml(ISABus *bus, Aml *scope);
>  
>  /**
>   * isa_register_ioport: Install an I/O port region on the ISA bus.
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 5d2b9b099684..77fc9df74735 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1259,6 +1259,7 @@ static void build_isa_devices_aml(Aml *table)
>          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);

is it possible to have more than 1 ISA bus on pc/q35 machine?

I'm asking because there is following clause
  if (ambiguous) {
just above this context while devices you are moving here all defined
unconditionally before that

>      }
>  
>      aml_append(table, scope);
> diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
> index 798dd9194e8f..1f2189f4d5db 100644
> --- a/hw/isa/isa-bus.c
> +++ b/hw/isa/isa-bus.c
> @@ -207,6 +207,21 @@ ISADevice *isa_vga_init(ISABus *bus)
>      }
>  }
>  
> +void isa_build_aml(ISABus *bus, Aml *scope)
> +{
> +    BusChild *kid;
> +    ISADevice *dev;
> +    ISADeviceClass *dc;
> +
> +    QTAILQ_FOREACH(kid, &bus->parent_obj.children, sibling) {
> +        dev = ISA_DEVICE(kid->child);
> +        dc = ISA_DEVICE_GET_CLASS(dev);
> +        if (dc->build_aml) {
> +            dc->build_aml(dev, scope);
> +        }
> +    }
> +}
> +
>  static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent)
>  {
>      ISADevice *d = ISA_DEVICE(dev);
Gerd Hoffmann April 7, 2020, 10:18 a.m. UTC | #3
On Mon, Apr 06, 2020 at 12:22:31PM +0200, Igor Mammedov wrote:
> On Fri,  3 Apr 2020 10:04:56 +0200
> Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
> > Also add isa_aml_build() function which walks all isa devices.
> > This allows to move aml builder code to isa devices.
> > 
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> >  include/hw/isa/isa.h |  2 ++
> >  hw/i386/acpi-build.c |  1 +
> >  hw/isa/isa-bus.c     | 15 +++++++++++++++
> >  3 files changed, 18 insertions(+)
> > 
> > diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
> > index e9ac1f1205a4..1534f8826453 100644
> > --- a/include/hw/isa/isa.h
> > +++ b/include/hw/isa/isa.h
> > @@ -70,6 +70,7 @@ typedef struct IsaDmaClass {
> >  
> >  typedef struct ISADeviceClass {
> >      DeviceClass parent_class;
> > +    void (*build_aml)(ISADevice *dev, Aml *scope);
> >  } ISADeviceClass;
> >  
> >  struct ISABus {
> > @@ -108,6 +109,7 @@ ISADevice *isa_try_create(ISABus *bus, const char *name);
> >  ISADevice *isa_create_simple(ISABus *bus, const char *name);
> >  
> >  ISADevice *isa_vga_init(ISABus *bus);
> > +void isa_build_aml(ISABus *bus, Aml *scope);
> >  
> >  /**
> >   * isa_register_ioport: Install an I/O port region on the ISA bus.
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 5d2b9b099684..77fc9df74735 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -1259,6 +1259,7 @@ static void build_isa_devices_aml(Aml *table)
> >          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);
> 
> is it possible to have more than 1 ISA bus on pc/q35 machine?

I don't think this is possible on the x86 architecture due to the way
io ports are addressed.  I think you can have multiple isa busses on
architectures where the isa io address space is just a mmio window on
the isa bridge.  Dunno whenever such machines actually exist in
practice.

I think we can safely compress the whole thing into "assert(obj &&
!ambiguous)".

take care,
  Gerd

Patch
diff mbox series

diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index e9ac1f1205a4..1534f8826453 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -70,6 +70,7 @@  typedef struct IsaDmaClass {
 
 typedef struct ISADeviceClass {
     DeviceClass parent_class;
+    void (*build_aml)(ISADevice *dev, Aml *scope);
 } ISADeviceClass;
 
 struct ISABus {
@@ -108,6 +109,7 @@  ISADevice *isa_try_create(ISABus *bus, const char *name);
 ISADevice *isa_create_simple(ISABus *bus, const char *name);
 
 ISADevice *isa_vga_init(ISABus *bus);
+void isa_build_aml(ISABus *bus, Aml *scope);
 
 /**
  * isa_register_ioport: Install an I/O port region on the ISA bus.
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 5d2b9b099684..77fc9df74735 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1259,6 +1259,7 @@  static void build_isa_devices_aml(Aml *table)
         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);
     }
 
     aml_append(table, scope);
diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index 798dd9194e8f..1f2189f4d5db 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -207,6 +207,21 @@  ISADevice *isa_vga_init(ISABus *bus)
     }
 }
 
+void isa_build_aml(ISABus *bus, Aml *scope)
+{
+    BusChild *kid;
+    ISADevice *dev;
+    ISADeviceClass *dc;
+
+    QTAILQ_FOREACH(kid, &bus->parent_obj.children, sibling) {
+        dev = ISA_DEVICE(kid->child);
+        dc = ISA_DEVICE_GET_CLASS(dev);
+        if (dc->build_aml) {
+            dc->build_aml(dev, scope);
+        }
+    }
+}
+
 static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent)
 {
     ISADevice *d = ISA_DEVICE(dev);