diff mbox series

[v7,2/4] hw/acpi: add indication for i8042 in IA-PC boot flags of the FADT table

Message ID 20220304154032.2071585-3-ani@anisinha.ca
State New
Headers show
Series hw/acpi: add indication for i8042 in IA-PC boot flags of the FADT table | expand

Commit Message

Ani Sinha March 4, 2022, 3:40 p.m. UTC
From: Liav Albani <liavalb@gmail.com>

This can allow the guest OS to determine more easily if i8042 controller
is present in the system or not, so it doesn't need to do probing of the
controller, but just initialize it immediately, before enumerating the
ACPI AML namespace.

The 8042 bit in IAPC_BOOT_ARCH was introduced from ACPI spec v2 (FADT
revision 2 and above). Therefore, in this change, we only enable this bit for
x86/q35 machine types since x86/i440fx machines use FADT ACPI table with
revision 1.

Signed-off-by: Liav Albani <liavalb@gmail.com>
Signed-off-by: Ani Sinha <ani@anisinha.ca>
---
 hw/acpi/aml-build.c         |  8 +++++++-
 hw/i386/acpi-build.c        |  8 ++++++++
 include/hw/acpi/acpi-defs.h |  1 +
 include/hw/input/i8042.h    | 15 +++++++++++++++
 4 files changed, 31 insertions(+), 1 deletion(-)

Comments

Michael S. Tsirkin March 6, 2022, 10:36 a.m. UTC | #1
On Fri, Mar 04, 2022 at 09:10:30PM +0530, Ani Sinha wrote:
> From: Liav Albani <liavalb@gmail.com>
> 
> This can allow the guest OS to determine more easily if i8042 controller
> is present in the system or not, so it doesn't need to do probing of the
> controller, but just initialize it immediately, before enumerating the
> ACPI AML namespace.
> 
> The 8042 bit in IAPC_BOOT_ARCH was introduced from ACPI spec v2 (FADT
> revision 2 and above). Therefore, in this change, we only enable this bit for
> x86/q35 machine types since x86/i440fx machines use FADT ACPI table with
> revision 1.
> 
> Signed-off-by: Liav Albani <liavalb@gmail.com>
> Signed-off-by: Ani Sinha <ani@anisinha.ca>
> ---
>  hw/acpi/aml-build.c         |  8 +++++++-
>  hw/i386/acpi-build.c        |  8 ++++++++
>  include/hw/acpi/acpi-defs.h |  1 +
>  include/hw/input/i8042.h    | 15 +++++++++++++++
>  4 files changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 8966e16320..1773cf55f1 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -2152,7 +2152,13 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
>      build_append_int_noprefix(tbl, 0, 1); /* DAY_ALRM */
>      build_append_int_noprefix(tbl, 0, 1); /* MON_ALRM */
>      build_append_int_noprefix(tbl, f->rtc_century, 1); /* CENTURY */
> -    build_append_int_noprefix(tbl, 0, 2); /* IAPC_BOOT_ARCH */
> +    /* IAPC_BOOT_ARCH */
> +    if (f->rev == 1) {
> +        build_append_int_noprefix(tbl, 0, 2);
> +    } else {
> +        /* since ACPI v2.0 */
> +        build_append_int_noprefix(tbl, f->iapc_boot_arch, 2);
> +    }
>      build_append_int_noprefix(tbl, 0, 1); /* Reserved */
>      build_append_int_noprefix(tbl, f->flags, 4); /* Flags */
>  
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index ebd47aa26f..4ad4d7286c 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -38,6 +38,7 @@
>  #include "hw/nvram/fw_cfg.h"
>  #include "hw/acpi/bios-linker-loader.h"
>  #include "hw/isa/isa.h"
> +#include "hw/input/i8042.h"
>  #include "hw/block/fdc.h"
>  #include "hw/acpi/memory_hotplug.h"
>  #include "sysemu/tpm.h"
> @@ -192,6 +193,13 @@ static void init_common_fadt_data(MachineState *ms, Object *o,
>              .address = object_property_get_uint(o, ACPI_PM_PROP_GPE0_BLK, NULL)
>          },
>      };
> +
> +    /*
> +     * ACPI v2, Table 5-10 - Fixed ACPI Description Table Boot Architecture
> +     * Flags, bit offset 1 - 8042.
> +     */

I think we should drop this comment (and one for microvm).
If you like, add

/*
Table 5-8 Fixed ACPI Description Table (FADT) Format
IAPC_BOOT_ARCH
*/

instead.


Let's do that with a patch on top, we have a soft freeze to consider.

> +    fadt.iapc_boot_arch = iapc_boot_arch_8042();
> +
>      *data = fadt;
>  }
>  
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index c97e8633ad..2b42e4192b 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -77,6 +77,7 @@ typedef struct AcpiFadtData {
>      uint16_t plvl2_lat;        /* P_LVL2_LAT */
>      uint16_t plvl3_lat;        /* P_LVL3_LAT */
>      uint16_t arm_boot_arch;    /* ARM_BOOT_ARCH */
> +    uint16_t iapc_boot_arch;   /* IAPC_BOOT_ARCH */
>      uint8_t minor_ver;         /* FADT Minor Version */
>  
>      /*
> diff --git a/include/hw/input/i8042.h b/include/hw/input/i8042.h
> index 1d90432dae..e070f546e4 100644
> --- a/include/hw/input/i8042.h
> +++ b/include/hw/input/i8042.h
> @@ -23,4 +23,19 @@ void i8042_mm_init(qemu_irq kbd_irq, qemu_irq mouse_irq,
>  void i8042_isa_mouse_fake_event(ISAKBDState *isa);
>  void i8042_setup_a20_line(ISADevice *dev, qemu_irq a20_out);
>  
> +static inline bool i8042_present(void)
> +{
> +    bool amb = false;
> +    return object_resolve_path_type("", TYPE_I8042, &amb) || amb;
> +}
> +
> +/*
> + * ACPI v2, Table 5-10 - Fixed ACPI Description Table Boot Architecture
> + * Flags, bit offset 1 - 8042.
> + */
> +static inline uint16_t iapc_boot_arch_8042(void)
> +{
> +    return i8042_present() ? 0x1 << 1 : 0x0 ;
> +}
> +
>  #endif /* HW_INPUT_I8042_H */
> -- 
> 2.25.1
Ani Sinha March 6, 2022, 2:26 p.m. UTC | #2
On Sun, Mar 6, 2022 at 4:06 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Fri, Mar 04, 2022 at 09:10:30PM +0530, Ani Sinha wrote:
> > From: Liav Albani <liavalb@gmail.com>
> >
> > This can allow the guest OS to determine more easily if i8042 controller
> > is present in the system or not, so it doesn't need to do probing of the
> > controller, but just initialize it immediately, before enumerating the
> > ACPI AML namespace.
> >
> > The 8042 bit in IAPC_BOOT_ARCH was introduced from ACPI spec v2 (FADT
> > revision 2 and above). Therefore, in this change, we only enable this bit for
> > x86/q35 machine types since x86/i440fx machines use FADT ACPI table with
> > revision 1.
> >
> > Signed-off-by: Liav Albani <liavalb@gmail.com>
> > Signed-off-by: Ani Sinha <ani@anisinha.ca>
> > ---
> >  hw/acpi/aml-build.c         |  8 +++++++-
> >  hw/i386/acpi-build.c        |  8 ++++++++
> >  include/hw/acpi/acpi-defs.h |  1 +
> >  include/hw/input/i8042.h    | 15 +++++++++++++++
> >  4 files changed, 31 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> > index 8966e16320..1773cf55f1 100644
> > --- a/hw/acpi/aml-build.c
> > +++ b/hw/acpi/aml-build.c
> > @@ -2152,7 +2152,13 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
> >      build_append_int_noprefix(tbl, 0, 1); /* DAY_ALRM */
> >      build_append_int_noprefix(tbl, 0, 1); /* MON_ALRM */
> >      build_append_int_noprefix(tbl, f->rtc_century, 1); /* CENTURY */
> > -    build_append_int_noprefix(tbl, 0, 2); /* IAPC_BOOT_ARCH */
> > +    /* IAPC_BOOT_ARCH */
> > +    if (f->rev == 1) {
> > +        build_append_int_noprefix(tbl, 0, 2);
> > +    } else {
> > +        /* since ACPI v2.0 */
> > +        build_append_int_noprefix(tbl, f->iapc_boot_arch, 2);
> > +    }
> >      build_append_int_noprefix(tbl, 0, 1); /* Reserved */
> >      build_append_int_noprefix(tbl, f->flags, 4); /* Flags */
> >
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index ebd47aa26f..4ad4d7286c 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -38,6 +38,7 @@
> >  #include "hw/nvram/fw_cfg.h"
> >  #include "hw/acpi/bios-linker-loader.h"
> >  #include "hw/isa/isa.h"
> > +#include "hw/input/i8042.h"
> >  #include "hw/block/fdc.h"
> >  #include "hw/acpi/memory_hotplug.h"
> >  #include "sysemu/tpm.h"
> > @@ -192,6 +193,13 @@ static void init_common_fadt_data(MachineState *ms, Object *o,
> >              .address = object_property_get_uint(o, ACPI_PM_PROP_GPE0_BLK, NULL)
> >          },
> >      };
> > +
> > +    /*
> > +     * ACPI v2, Table 5-10 - Fixed ACPI Description Table Boot Architecture
> > +     * Flags, bit offset 1 - 8042.
> > +     */
>
> I think we should drop this comment (and one for microvm).
> If you like, add
>
> /*
> Table 5-8 Fixed ACPI Description Table (FADT) Format
> IAPC_BOOT_ARCH
> */
>
> instead.
>
>
> Let's do that with a patch on top, we have a soft freeze to consider.

Sure.

>
> > +    fadt.iapc_boot_arch = iapc_boot_arch_8042();
> > +
> >      *data = fadt;
> >  }
> >
> > diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> > index c97e8633ad..2b42e4192b 100644
> > --- a/include/hw/acpi/acpi-defs.h
> > +++ b/include/hw/acpi/acpi-defs.h
> > @@ -77,6 +77,7 @@ typedef struct AcpiFadtData {
> >      uint16_t plvl2_lat;        /* P_LVL2_LAT */
> >      uint16_t plvl3_lat;        /* P_LVL3_LAT */
> >      uint16_t arm_boot_arch;    /* ARM_BOOT_ARCH */
> > +    uint16_t iapc_boot_arch;   /* IAPC_BOOT_ARCH */
> >      uint8_t minor_ver;         /* FADT Minor Version */
> >
> >      /*
> > diff --git a/include/hw/input/i8042.h b/include/hw/input/i8042.h
> > index 1d90432dae..e070f546e4 100644
> > --- a/include/hw/input/i8042.h
> > +++ b/include/hw/input/i8042.h
> > @@ -23,4 +23,19 @@ void i8042_mm_init(qemu_irq kbd_irq, qemu_irq mouse_irq,
> >  void i8042_isa_mouse_fake_event(ISAKBDState *isa);
> >  void i8042_setup_a20_line(ISADevice *dev, qemu_irq a20_out);
> >
> > +static inline bool i8042_present(void)
> > +{
> > +    bool amb = false;
> > +    return object_resolve_path_type("", TYPE_I8042, &amb) || amb;
> > +}
> > +
> > +/*
> > + * ACPI v2, Table 5-10 - Fixed ACPI Description Table Boot Architecture
> > + * Flags, bit offset 1 - 8042.
> > + */
> > +static inline uint16_t iapc_boot_arch_8042(void)
> > +{
> > +    return i8042_present() ? 0x1 << 1 : 0x0 ;
> > +}
> > +
> >  #endif /* HW_INPUT_I8042_H */
> > --
> > 2.25.1
>
Igor Mammedov March 7, 2022, 11:12 a.m. UTC | #3
On Sun, 6 Mar 2022 19:56:04 +0530
Ani Sinha <ani@anisinha.ca> wrote:

> On Sun, Mar 6, 2022 at 4:06 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Fri, Mar 04, 2022 at 09:10:30PM +0530, Ani Sinha wrote:  
> > > From: Liav Albani <liavalb@gmail.com>
> > >
> > > This can allow the guest OS to determine more easily if i8042 controller
> > > is present in the system or not, so it doesn't need to do probing of the
> > > controller, but just initialize it immediately, before enumerating the
> > > ACPI AML namespace.
> > >
> > > The 8042 bit in IAPC_BOOT_ARCH was introduced from ACPI spec v2 (FADT
> > > revision 2 and above). Therefore, in this change, we only enable this bit for
> > > x86/q35 machine types since x86/i440fx machines use FADT ACPI table with
> > > revision 1.
> > >
> > > Signed-off-by: Liav Albani <liavalb@gmail.com>
> > > Signed-off-by: Ani Sinha <ani@anisinha.ca>
> > > ---
> > >  hw/acpi/aml-build.c         |  8 +++++++-
> > >  hw/i386/acpi-build.c        |  8 ++++++++
> > >  include/hw/acpi/acpi-defs.h |  1 +
> > >  include/hw/input/i8042.h    | 15 +++++++++++++++
> > >  4 files changed, 31 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> > > index 8966e16320..1773cf55f1 100644
> > > --- a/hw/acpi/aml-build.c
> > > +++ b/hw/acpi/aml-build.c
> > > @@ -2152,7 +2152,13 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
> > >      build_append_int_noprefix(tbl, 0, 1); /* DAY_ALRM */
> > >      build_append_int_noprefix(tbl, 0, 1); /* MON_ALRM */
> > >      build_append_int_noprefix(tbl, f->rtc_century, 1); /* CENTURY */
> > > -    build_append_int_noprefix(tbl, 0, 2); /* IAPC_BOOT_ARCH */
> > > +    /* IAPC_BOOT_ARCH */
> > > +    if (f->rev == 1) {
> > > +        build_append_int_noprefix(tbl, 0, 2);
> > > +    } else {
> > > +        /* since ACPI v2.0 */
> > > +        build_append_int_noprefix(tbl, f->iapc_boot_arch, 2);
> > > +    }
> > >      build_append_int_noprefix(tbl, 0, 1); /* Reserved */
> > >      build_append_int_noprefix(tbl, f->flags, 4); /* Flags */
> > >
> > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > index ebd47aa26f..4ad4d7286c 100644
> > > --- a/hw/i386/acpi-build.c
> > > +++ b/hw/i386/acpi-build.c
> > > @@ -38,6 +38,7 @@
> > >  #include "hw/nvram/fw_cfg.h"
> > >  #include "hw/acpi/bios-linker-loader.h"
> > >  #include "hw/isa/isa.h"
> > > +#include "hw/input/i8042.h"
> > >  #include "hw/block/fdc.h"
> > >  #include "hw/acpi/memory_hotplug.h"
> > >  #include "sysemu/tpm.h"
> > > @@ -192,6 +193,13 @@ static void init_common_fadt_data(MachineState *ms, Object *o,
> > >              .address = object_property_get_uint(o, ACPI_PM_PROP_GPE0_BLK, NULL)
> > >          },
> > >      };
> > > +
> > > +    /*
> > > +     * ACPI v2, Table 5-10 - Fixed ACPI Description Table Boot Architecture
> > > +     * Flags, bit offset 1 - 8042.
> > > +     */  
> >
> > I think we should drop this comment (and one for microvm).
> > If you like, add
> >
> > /*
> > Table 5-8 Fixed ACPI Description Table (FADT) Format
> > IAPC_BOOT_ARCH
> > */
> >
> > instead.

I'd keep spec version though,
 all chapter/table numbering is version depended. 

> >
> >
> > Let's do that with a patch on top, we have a soft freeze to consider.  
> 
> Sure.
> 
> >  
> > > +    fadt.iapc_boot_arch = iapc_boot_arch_8042();
> > > +
> > >      *data = fadt;
> > >  }
> > >
> > > diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> > > index c97e8633ad..2b42e4192b 100644
> > > --- a/include/hw/acpi/acpi-defs.h
> > > +++ b/include/hw/acpi/acpi-defs.h
> > > @@ -77,6 +77,7 @@ typedef struct AcpiFadtData {
> > >      uint16_t plvl2_lat;        /* P_LVL2_LAT */
> > >      uint16_t plvl3_lat;        /* P_LVL3_LAT */
> > >      uint16_t arm_boot_arch;    /* ARM_BOOT_ARCH */
> > > +    uint16_t iapc_boot_arch;   /* IAPC_BOOT_ARCH */
> > >      uint8_t minor_ver;         /* FADT Minor Version */
> > >
> > >      /*
> > > diff --git a/include/hw/input/i8042.h b/include/hw/input/i8042.h
> > > index 1d90432dae..e070f546e4 100644
> > > --- a/include/hw/input/i8042.h
> > > +++ b/include/hw/input/i8042.h
> > > @@ -23,4 +23,19 @@ void i8042_mm_init(qemu_irq kbd_irq, qemu_irq mouse_irq,
> > >  void i8042_isa_mouse_fake_event(ISAKBDState *isa);
> > >  void i8042_setup_a20_line(ISADevice *dev, qemu_irq a20_out);
> > >
> > > +static inline bool i8042_present(void)
> > > +{
> > > +    bool amb = false;
> > > +    return object_resolve_path_type("", TYPE_I8042, &amb) || amb;
> > > +}
> > > +
> > > +/*
> > > + * ACPI v2, Table 5-10 - Fixed ACPI Description Table Boot Architecture
> > > + * Flags, bit offset 1 - 8042.
> > > + */
> > > +static inline uint16_t iapc_boot_arch_8042(void)
> > > +{
> > > +    return i8042_present() ? 0x1 << 1 : 0x0 ;
> > > +}
> > > +
> > >  #endif /* HW_INPUT_I8042_H */
> > > --
> > > 2.25.1  
> >  
>
diff mbox series

Patch

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 8966e16320..1773cf55f1 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -2152,7 +2152,13 @@  void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
     build_append_int_noprefix(tbl, 0, 1); /* DAY_ALRM */
     build_append_int_noprefix(tbl, 0, 1); /* MON_ALRM */
     build_append_int_noprefix(tbl, f->rtc_century, 1); /* CENTURY */
-    build_append_int_noprefix(tbl, 0, 2); /* IAPC_BOOT_ARCH */
+    /* IAPC_BOOT_ARCH */
+    if (f->rev == 1) {
+        build_append_int_noprefix(tbl, 0, 2);
+    } else {
+        /* since ACPI v2.0 */
+        build_append_int_noprefix(tbl, f->iapc_boot_arch, 2);
+    }
     build_append_int_noprefix(tbl, 0, 1); /* Reserved */
     build_append_int_noprefix(tbl, f->flags, 4); /* Flags */
 
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index ebd47aa26f..4ad4d7286c 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -38,6 +38,7 @@ 
 #include "hw/nvram/fw_cfg.h"
 #include "hw/acpi/bios-linker-loader.h"
 #include "hw/isa/isa.h"
+#include "hw/input/i8042.h"
 #include "hw/block/fdc.h"
 #include "hw/acpi/memory_hotplug.h"
 #include "sysemu/tpm.h"
@@ -192,6 +193,13 @@  static void init_common_fadt_data(MachineState *ms, Object *o,
             .address = object_property_get_uint(o, ACPI_PM_PROP_GPE0_BLK, NULL)
         },
     };
+
+    /*
+     * ACPI v2, Table 5-10 - Fixed ACPI Description Table Boot Architecture
+     * Flags, bit offset 1 - 8042.
+     */
+    fadt.iapc_boot_arch = iapc_boot_arch_8042();
+
     *data = fadt;
 }
 
diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
index c97e8633ad..2b42e4192b 100644
--- a/include/hw/acpi/acpi-defs.h
+++ b/include/hw/acpi/acpi-defs.h
@@ -77,6 +77,7 @@  typedef struct AcpiFadtData {
     uint16_t plvl2_lat;        /* P_LVL2_LAT */
     uint16_t plvl3_lat;        /* P_LVL3_LAT */
     uint16_t arm_boot_arch;    /* ARM_BOOT_ARCH */
+    uint16_t iapc_boot_arch;   /* IAPC_BOOT_ARCH */
     uint8_t minor_ver;         /* FADT Minor Version */
 
     /*
diff --git a/include/hw/input/i8042.h b/include/hw/input/i8042.h
index 1d90432dae..e070f546e4 100644
--- a/include/hw/input/i8042.h
+++ b/include/hw/input/i8042.h
@@ -23,4 +23,19 @@  void i8042_mm_init(qemu_irq kbd_irq, qemu_irq mouse_irq,
 void i8042_isa_mouse_fake_event(ISAKBDState *isa);
 void i8042_setup_a20_line(ISADevice *dev, qemu_irq a20_out);
 
+static inline bool i8042_present(void)
+{
+    bool amb = false;
+    return object_resolve_path_type("", TYPE_I8042, &amb) || amb;
+}
+
+/*
+ * ACPI v2, Table 5-10 - Fixed ACPI Description Table Boot Architecture
+ * Flags, bit offset 1 - 8042.
+ */
+static inline uint16_t iapc_boot_arch_8042(void)
+{
+    return i8042_present() ? 0x1 << 1 : 0x0 ;
+}
+
 #endif /* HW_INPUT_I8042_H */