diff mbox series

[01/35] acpi: add interface to build device specific AML

Message ID 20220516152610.1963435-2-imammedo@redhat.com (mailing list archive)
State New, archived
Headers show
Series pc/q35: refactor ISA and SMBUS AML generation | expand

Commit Message

Igor Mammedov May 16, 2022, 3:25 p.m. UTC 
There is already ISADeviceClass::build_aml() callback which
builds device specific AML blob for some ISA devices.
To extend the same idea to other devices, add TYPE_ACPI_DEV_AML_IF
Interface that will provide a more generic callback which
will be used not only for ISA but other devices. It will
allow get rid of some data-mining and ad-hoc AML building,
by asking device(s) to generate its own AML blob like it's
done for ISA devices.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 include/hw/acpi/acpi_aml_interface.h | 40 ++++++++++++++++++++++++++++
 hw/acpi/acpi_interface.c             |  8 ++++++
 hw/acpi/meson.build                  |  2 +-
 3 files changed, 49 insertions(+), 1 deletion(-)
 create mode 100644 include/hw/acpi/acpi_aml_interface.h

Comments

Ani Sinha May 18, 2022, 10 a.m. UTC | #1 
On Mon, May 16, 2022 at 8:56 PM Igor Mammedov <imammedo@redhat.com> wrote:
>
> There is already ISADeviceClass::build_aml() callback which
> builds device specific AML blob for some ISA devices.
> To extend the same idea to other devices, add TYPE_ACPI_DEV_AML_IF
> Interface that will provide a more generic callback which
> will be used not only for ISA but other devices. It will
> allow get rid of some data-mining and ad-hoc AML building,
> by asking device(s) to generate its own AML blob like it's
> done for ISA devices.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  include/hw/acpi/acpi_aml_interface.h | 40 ++++++++++++++++++++++++++++
>  hw/acpi/acpi_interface.c             |  8 ++++++
>  hw/acpi/meson.build                  |  2 +-
>  3 files changed, 49 insertions(+), 1 deletion(-)
>  create mode 100644 include/hw/acpi/acpi_aml_interface.h
>
> diff --git a/include/hw/acpi/acpi_aml_interface.h b/include/hw/acpi/acpi_aml_interface.h
> new file mode 100644
> index 0000000000..ab76f0e55d
> --- /dev/null
> +++ b/include/hw/acpi/acpi_aml_interface.h
> @@ -0,0 +1,40 @@
> +#ifndef ACPI_AML_INTERFACE_H
> +#define ACPI_AML_INTERFACE_H
> +
> +#include "qom/object.h"
> +#include "hw/acpi/aml-build.h"
> +
> +#define TYPE_ACPI_DEV_AML_IF "acpi-dev-aml-interface"
> +typedef struct AcpiDevAmlIfClass AcpiDevAmlIfClass;
> +DECLARE_CLASS_CHECKERS(AcpiDevAmlIfClass, ACPI_DEV_AML_IF, TYPE_ACPI_DEV_AML_IF)
> +#define ACPI_DEV_AML_IF(obj) \
> +     INTERFACE_CHECK(AcpiDevAmlIf, (obj), TYPE_ACPI_DEV_AML_IF)
> +
> +typedef struct AcpiDevAmlIf AcpiDevAmlIf;

I do not see where struct AcpiDevAmlIf is defined. I guess this is
through the macro magic.

> +typedef void (*dev_aml_fn)(AcpiDevAmlIf *adev, Aml *scope);
> +
> +/**
> + * AcpiDevAmlIfClass:
> + *
> + * build_dev_aml: adds device specific AML blob to provided scope
> + *
> + * Interface is designed for providing generic callback that builds device
> + * specific AML blob.
> + */
> +struct AcpiDevAmlIfClass {
> +    /* <private> */
> +    InterfaceClass parent_class;
> +
> +    /* <public> */
> +    dev_aml_fn build_dev_aml;
> +};
> +
> +static inline void call_dev_aml_func(DeviceState *dev, Aml *scope)
> +{
> +    if (object_dynamic_cast(OBJECT(dev), TYPE_ACPI_DEV_AML_IF)) {
> +        AcpiDevAmlIfClass *klass = ACPI_DEV_AML_IF_GET_CLASS(dev);
> +        klass->build_dev_aml(ACPI_DEV_AML_IF(dev), scope);
> +    }
> +}
> +
> +#endif
> diff --git a/hw/acpi/acpi_interface.c b/hw/acpi/acpi_interface.c
> index 6583917b8e..c668d361f6 100644
> --- a/hw/acpi/acpi_interface.c
> +++ b/hw/acpi/acpi_interface.c
> @@ -1,5 +1,6 @@
>  #include "qemu/osdep.h"
>  #include "hw/acpi/acpi_dev_interface.h"
> +#include "hw/acpi/acpi_aml_interface.h"
>  #include "qemu/module.h"
>
>  void acpi_send_event(DeviceState *dev, AcpiEventStatusBits event)
> @@ -18,8 +19,15 @@ static void register_types(void)
>          .parent        = TYPE_INTERFACE,
>          .class_size = sizeof(AcpiDeviceIfClass),
>      };
> +    static const TypeInfo acpi_dev_aml_if_info = {
> +        .name          = TYPE_ACPI_DEV_AML_IF,
> +        .parent        = TYPE_INTERFACE,
> +        .class_size = sizeof(AcpiDevAmlIfClass),
> +    };
> +
>
>      type_register_static(&acpi_dev_if_info);
> +    type_register_static(&acpi_dev_aml_if_info);
>  }
>
>  type_init(register_types)
> diff --git a/hw/acpi/meson.build b/hw/acpi/meson.build
> index 8bea2e6933..9504f5ce09 100644
> --- a/hw/acpi/meson.build
> +++ b/hw/acpi/meson.build
> @@ -28,7 +28,7 @@ acpi_ss.add(when: 'CONFIG_PC', if_false: files('acpi-x86-stub.c'))
>  if have_tpm
>    acpi_ss.add(files('tpm.c'))
>  endif
> -softmmu_ss.add(when: 'CONFIG_ACPI', if_false: files('acpi-stub.c', 'aml-build-stub.c', 'ghes-stub.c'))
> +softmmu_ss.add(when: 'CONFIG_ACPI', if_false: files('acpi-stub.c', 'aml-build-stub.c', 'ghes-stub.c', 'acpi_interface.c'))

This is wrong. It should be the stub file not the real thing.

>  softmmu_ss.add_all(when: 'CONFIG_ACPI', if_true: acpi_ss)
>  softmmu_ss.add(when: 'CONFIG_ALL', if_true: files('acpi-stub.c', 'aml-build-stub.c',
>                                                    'acpi-x86-stub.c', 'ipmi-stub.c', 'ghes-stub.c',
> --
> 2.31.1
>
Igor Mammedov May 19, 2022, 12:54 p.m. UTC | #2 
On Wed, 18 May 2022 15:30:07 +0530
Ani Sinha <ani@anisinha.ca> wrote:

> On Mon, May 16, 2022 at 8:56 PM Igor Mammedov <imammedo@redhat.com> wrote:
> >
> > There is already ISADeviceClass::build_aml() callback which
> > builds device specific AML blob for some ISA devices.
> > To extend the same idea to other devices, add TYPE_ACPI_DEV_AML_IF
> > Interface that will provide a more generic callback which
> > will be used not only for ISA but other devices. It will
> > allow get rid of some data-mining and ad-hoc AML building,
> > by asking device(s) to generate its own AML blob like it's
> > done for ISA devices.
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  include/hw/acpi/acpi_aml_interface.h | 40 ++++++++++++++++++++++++++++
> >  hw/acpi/acpi_interface.c             |  8 ++++++
> >  hw/acpi/meson.build                  |  2 +-
> >  3 files changed, 49 insertions(+), 1 deletion(-)
> >  create mode 100644 include/hw/acpi/acpi_aml_interface.h
> >
> > diff --git a/include/hw/acpi/acpi_aml_interface.h b/include/hw/acpi/acpi_aml_interface.h
> > new file mode 100644
> > index 0000000000..ab76f0e55d
> > --- /dev/null
> > +++ b/include/hw/acpi/acpi_aml_interface.h
> > @@ -0,0 +1,40 @@
> > +#ifndef ACPI_AML_INTERFACE_H
> > +#define ACPI_AML_INTERFACE_H
> > +
> > +#include "qom/object.h"
> > +#include "hw/acpi/aml-build.h"
> > +
> > +#define TYPE_ACPI_DEV_AML_IF "acpi-dev-aml-interface"
> > +typedef struct AcpiDevAmlIfClass AcpiDevAmlIfClass;
> > +DECLARE_CLASS_CHECKERS(AcpiDevAmlIfClass, ACPI_DEV_AML_IF, TYPE_ACPI_DEV_AML_IF)
> > +#define ACPI_DEV_AML_IF(obj) \
> > +     INTERFACE_CHECK(AcpiDevAmlIf, (obj), TYPE_ACPI_DEV_AML_IF)
> > +
> > +typedef struct AcpiDevAmlIf AcpiDevAmlIf;  
> 
> I do not see where struct AcpiDevAmlIf is defined. I guess this is
> through the macro magic.

it's used for type checking only of opaque pointer
(Interfaces are not supposed to have any state)

> > +typedef void (*dev_aml_fn)(AcpiDevAmlIf *adev, Aml *scope);
> > +
> > +/**
> > + * AcpiDevAmlIfClass:
> > + *
> > + * build_dev_aml: adds device specific AML blob to provided scope
> > + *
> > + * Interface is designed for providing generic callback that builds device
> > + * specific AML blob.
> > + */
> > +struct AcpiDevAmlIfClass {
> > +    /* <private> */
> > +    InterfaceClass parent_class;
> > +
> > +    /* <public> */
> > +    dev_aml_fn build_dev_aml;
> > +};
> > +
> > +static inline void call_dev_aml_func(DeviceState *dev, Aml *scope)
> > +{
> > +    if (object_dynamic_cast(OBJECT(dev), TYPE_ACPI_DEV_AML_IF)) {
> > +        AcpiDevAmlIfClass *klass = ACPI_DEV_AML_IF_GET_CLASS(dev);
> > +        klass->build_dev_aml(ACPI_DEV_AML_IF(dev), scope);
> > +    }
> > +}
> > +
> > +#endif
> > diff --git a/hw/acpi/acpi_interface.c b/hw/acpi/acpi_interface.c
> > index 6583917b8e..c668d361f6 100644
> > --- a/hw/acpi/acpi_interface.c
> > +++ b/hw/acpi/acpi_interface.c
> > @@ -1,5 +1,6 @@
> >  #include "qemu/osdep.h"
> >  #include "hw/acpi/acpi_dev_interface.h"
> > +#include "hw/acpi/acpi_aml_interface.h"
> >  #include "qemu/module.h"
> >
> >  void acpi_send_event(DeviceState *dev, AcpiEventStatusBits event)
> > @@ -18,8 +19,15 @@ static void register_types(void)
> >          .parent        = TYPE_INTERFACE,
> >          .class_size = sizeof(AcpiDeviceIfClass),
> >      };
> > +    static const TypeInfo acpi_dev_aml_if_info = {
> > +        .name          = TYPE_ACPI_DEV_AML_IF,
> > +        .parent        = TYPE_INTERFACE,
> > +        .class_size = sizeof(AcpiDevAmlIfClass),
> > +    };
> > +
> >
> >      type_register_static(&acpi_dev_if_info);
> > +    type_register_static(&acpi_dev_aml_if_info);
> >  }
> >
> >  type_init(register_types)
> > diff --git a/hw/acpi/meson.build b/hw/acpi/meson.build
> > index 8bea2e6933..9504f5ce09 100644
> > --- a/hw/acpi/meson.build
> > +++ b/hw/acpi/meson.build
> > @@ -28,7 +28,7 @@ acpi_ss.add(when: 'CONFIG_PC', if_false: files('acpi-x86-stub.c'))
> >  if have_tpm
> >    acpi_ss.add(files('tpm.c'))
> >  endif
> > -softmmu_ss.add(when: 'CONFIG_ACPI', if_false: files('acpi-stub.c', 'aml-build-stub.c', 'ghes-stub.c'))
> > +softmmu_ss.add(when: 'CONFIG_ACPI', if_false: files('acpi-stub.c', 'aml-build-stub.c', 'ghes-stub.c', 'acpi_interface.c'))  
> 
> This is wrong. It should be the stub file not the real thing.

I can put type definition into acpi-stub.c, but will be exact duplicate
(i.e. you can't stub QOM type definition), hence acpi_interface.c (which is
mostly QOM type definitions) is being included in non acpi build to avoid
duplication.

> 
> >  softmmu_ss.add_all(when: 'CONFIG_ACPI', if_true: acpi_ss)
> >  softmmu_ss.add(when: 'CONFIG_ALL', if_true: files('acpi-stub.c', 'aml-build-stub.c',
> >                                                    'acpi-x86-stub.c', 'ipmi-stub.c', 'ghes-stub.c',
> > --
> > 2.31.1
> >  
>
diff mbox series

Patch

diff --git a/include/hw/acpi/acpi_aml_interface.h b/include/hw/acpi/acpi_aml_interface.h
new file mode 100644
index 0000000000..ab76f0e55d
--- /dev/null
+++ b/include/hw/acpi/acpi_aml_interface.h
@@ -0,0 +1,40 @@ 
+#ifndef ACPI_AML_INTERFACE_H
+#define ACPI_AML_INTERFACE_H
+
+#include "qom/object.h"
+#include "hw/acpi/aml-build.h"
+
+#define TYPE_ACPI_DEV_AML_IF "acpi-dev-aml-interface"
+typedef struct AcpiDevAmlIfClass AcpiDevAmlIfClass;
+DECLARE_CLASS_CHECKERS(AcpiDevAmlIfClass, ACPI_DEV_AML_IF, TYPE_ACPI_DEV_AML_IF)
+#define ACPI_DEV_AML_IF(obj) \
+     INTERFACE_CHECK(AcpiDevAmlIf, (obj), TYPE_ACPI_DEV_AML_IF)
+
+typedef struct AcpiDevAmlIf AcpiDevAmlIf;
+typedef void (*dev_aml_fn)(AcpiDevAmlIf *adev, Aml *scope);
+
+/**
+ * AcpiDevAmlIfClass:
+ *
+ * build_dev_aml: adds device specific AML blob to provided scope
+ *
+ * Interface is designed for providing generic callback that builds device
+ * specific AML blob.
+ */
+struct AcpiDevAmlIfClass {
+    /* <private> */
+    InterfaceClass parent_class;
+
+    /* <public> */
+    dev_aml_fn build_dev_aml;
+};
+
+static inline void call_dev_aml_func(DeviceState *dev, Aml *scope)
+{
+    if (object_dynamic_cast(OBJECT(dev), TYPE_ACPI_DEV_AML_IF)) {
+        AcpiDevAmlIfClass *klass = ACPI_DEV_AML_IF_GET_CLASS(dev);
+        klass->build_dev_aml(ACPI_DEV_AML_IF(dev), scope);
+    }
+}
+
+#endif
diff --git a/hw/acpi/acpi_interface.c b/hw/acpi/acpi_interface.c
index 6583917b8e..c668d361f6 100644
--- a/hw/acpi/acpi_interface.c
+++ b/hw/acpi/acpi_interface.c
@@ -1,5 +1,6 @@ 
 #include "qemu/osdep.h"
 #include "hw/acpi/acpi_dev_interface.h"
+#include "hw/acpi/acpi_aml_interface.h"
 #include "qemu/module.h"
 
 void acpi_send_event(DeviceState *dev, AcpiEventStatusBits event)
@@ -18,8 +19,15 @@  static void register_types(void)
         .parent        = TYPE_INTERFACE,
         .class_size = sizeof(AcpiDeviceIfClass),
     };
+    static const TypeInfo acpi_dev_aml_if_info = {
+        .name          = TYPE_ACPI_DEV_AML_IF,
+        .parent        = TYPE_INTERFACE,
+        .class_size = sizeof(AcpiDevAmlIfClass),
+    };
+
 
     type_register_static(&acpi_dev_if_info);
+    type_register_static(&acpi_dev_aml_if_info);
 }
 
 type_init(register_types)
diff --git a/hw/acpi/meson.build b/hw/acpi/meson.build
index 8bea2e6933..9504f5ce09 100644
--- a/hw/acpi/meson.build
+++ b/hw/acpi/meson.build
@@ -28,7 +28,7 @@  acpi_ss.add(when: 'CONFIG_PC', if_false: files('acpi-x86-stub.c'))
 if have_tpm
   acpi_ss.add(files('tpm.c'))
 endif
-softmmu_ss.add(when: 'CONFIG_ACPI', if_false: files('acpi-stub.c', 'aml-build-stub.c', 'ghes-stub.c'))
+softmmu_ss.add(when: 'CONFIG_ACPI', if_false: files('acpi-stub.c', 'aml-build-stub.c', 'ghes-stub.c', 'acpi_interface.c'))
 softmmu_ss.add_all(when: 'CONFIG_ACPI', if_true: acpi_ss)
 softmmu_ss.add(when: 'CONFIG_ALL', if_true: files('acpi-stub.c', 'aml-build-stub.c',
                                                   'acpi-x86-stub.c', 'ipmi-stub.c', 'ghes-stub.c',