Re: [PATCH v2 7/9] i386: acpi: Don't build HPET ACPI entry if HPET is disabled

[Igor Mammedov]

On Mon,  8 Feb 2021 13:57:26 -0800
isaku.yamahata@gmail.com wrote:

> From: Sean Christopherson <sean.j.christopherson@intel.com>
> 
> Omit HPET AML if the HPET is disabled, QEMU is not emulating it and the
> guest may get confused by seeing HPET in the ACPI tables without a
> "physical" device present.
> 
> The change of DSDT when -no-hpet is as follows.
> 
> @@ -141,47 +141,6 @@ DefinitionBlock ("", "DSDT", 1, "BOCHS "
>          }
>      }
> 
> -    Scope (_SB)
> -    {
> -        Device (HPET)
> -        {
> -            Name (_HID, EisaId ("PNP0103") /* HPET System Timer */)  // 
> _HID: Hardware ID
> -            Name (_UID, Zero)  // _UID: Unique ID
> -            OperationRegion (HPTM, SystemMemory, 0xFED00000, 0x0400)
> -            Field (HPTM, DWordAcc, Lock, Preserve)
> -            {
> -                VEND,   32,
> -                PRD,    32
> -            }
> -
> -            Method (_STA, 0, NotSerialized)  // _STA: Status
> -            {
> -                Local0 = VEND /* \_SB_.HPET.VEND */
> -                Local1 = PRD /* \_SB_.HPET.PRD_ */
> -                Local0 >>= 0x10
> -                If (((Local0 == Zero) || (Local0 == 0xFFFF)))
> -                {
> -                    Return (Zero)
> -                }
> -
> -                If (((Local1 == Zero) || (Local1 > 0x05F5E100)))
> -                {
> -                    Return (Zero)
> -                }
> -
> -                Return (0x0F)
> -            }
> -
> -            Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource 
> Settings
> -            {
> -                Memory32Fixed (ReadOnly,
> -                    0xFED00000,         // Address Base
> -                    0x00000400,         // Address Length
> -                    )
> -            })
> -        }
> -    }
> -
>      Scope (_SB.PCI0)
>      {
>          Device (ISA)
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>

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

> ---
>  hw/i386/acpi-build.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index bcb1f65c1d..73ec0b6d32 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1405,7 +1405,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>          aml_append(sb_scope, dev);
>          aml_append(dsdt, sb_scope);
>  
> -        build_hpet_aml(dsdt);
> +        if (misc->has_hpet) {
> +            build_hpet_aml(dsdt);
> +        }
>          build_piix4_isa_bridge(dsdt);
>          build_isa_devices_aml(dsdt);
>          if (pm->pcihp_bridge_en || pm->pcihp_root_en) {
> @@ -1450,7 +1452,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>  
>          aml_append(dsdt, sb_scope);
>  
> -        build_hpet_aml(dsdt);
> +        if (misc->has_hpet) {
> +            build_hpet_aml(dsdt);
> +        }
>          build_q35_isa_bridge(dsdt);
>          build_isa_devices_aml(dsdt);
>          build_q35_pci0_int(dsdt);