Re: [PATCH 04/17] hvmloader: add ACPI enabling for Q35

["Roger Pau Monné" <roger.pau@citrix.com>]

On Fri, Mar 13, 2026 at 04:35:05PM +0000, Thierry Escande wrote:
> In order to turn on ACPI for OS, we need to write a chipset-specific value
> to SMI_CMD register (sort of imitation of the APM->ACPI switch on real
> systems). Modify acpi_enable_sci() function to support both i440 and Q35
> emulation.
> 
> Signed-off-by: Alexey Gerasimenko <x1917x@gmail.com>
> Signed-off-by: Thierry Escande <thierry.escande@vates.tech>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

It's not great to add more stuff into hvmloader when we want to move
out of it, but it's also not helpful to tie the Q35 addition to the
removal of hvmloader.

> ---
>  tools/firmware/hvmloader/hvmloader.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/firmware/hvmloader/hvmloader.c b/tools/firmware/hvmloader/hvmloader.c
> index 626cc53649..f6cc3fa4b9 100644
> --- a/tools/firmware/hvmloader/hvmloader.c
> +++ b/tools/firmware/hvmloader/hvmloader.c
> @@ -258,9 +258,24 @@ static const struct bios_config *detect_bios(void)
>  static void acpi_enable_sci(void)
>  {
>      uint8_t pm1a_cnt_val;
> +    uint8_t acpi_enable_val;
>  
> -#define PIIX4_SMI_CMD_IOPORT 0xb2
> +#define SMI_CMD_IOPORT       0xb2
>  #define PIIX4_ACPI_ENABLE    0xf1
> +#define ICH9_ACPI_ENABLE     0x02
> +
> +    switch ( machine_type )
> +    {
> +    case MACHINE_TYPE_Q35:
> +        acpi_enable_val = ICH9_ACPI_ENABLE;
> +        break;
> +    case MACHINE_TYPE_I440:
> +        acpi_enable_val = PIIX4_ACPI_ENABLE;
> +        break;

We might want to add a newline after the break statements.

Thanks, Roger.