Re: [Xen-devel] [RFC PATCH 04/12] hvmloader: add ACPI enabling for Q35

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Mar 13, 2018 at 04:33:49AM +1000, Alexey Gerasimenko 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@xxxxxxxxx>
> ---
>  tools/firmware/hvmloader/hvmloader.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/firmware/hvmloader/hvmloader.c 
> b/tools/firmware/hvmloader/hvmloader.c
> index f603f68ded..070698440e 100644
> --- a/tools/firmware/hvmloader/hvmloader.c
> +++ b/tools/firmware/hvmloader/hvmloader.c
> @@ -257,9 +257,16 @@ 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
> +
> +    if (get_pc_machine_type() == MACHINE_TYPE_Q35)
> +        acpi_enable_val = ICH9_ACPI_ENABLE;
> +    else
> +        acpi_enable_val = PIIX4_ACPI_ENABLE;

Coding style, but I would rather:

switch ( get_pc_machine_type() )
{
case MACHINE_TYPE_Q35:
...
case MACHINE_TYPE_I440:
...
default:
BUG();
}

I think storing the machine type in a global variable is better than
calling get_pc_machine_type each time.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel