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

[Roger Pau Monné]

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 
> ---
>  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_ENABLE0xf1
> +#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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

[Wei Liu]

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 
> ---
>  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_ENABLE0xf1
> +#define ICH9_ACPI_ENABLE 0x02
> +
> +if (get_pc_machine_type() == MACHINE_TYPE_Q35)

Coding style.

And the previous patch can be folded into this one.

Wei.

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

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

[Alexey Gerasimenko]

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 
---
 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_ENABLE0xf1
+#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;
 
 /*
  * PIIX4 emulation in QEMU has SCI_EN=0 by default. We have no legacy
@@ -267,7 +274,7 @@ static void acpi_enable_sci(void)
  */
 pm1a_cnt_val = inb(ACPI_PM1A_CNT_BLK_ADDRESS_V1);
 if ( !(pm1a_cnt_val & ACPI_PM1C_SCI_EN) )
-outb(PIIX4_SMI_CMD_IOPORT, PIIX4_ACPI_ENABLE);
+outb(SMI_CMD_IOPORT, acpi_enable_val);
 
 pm1a_cnt_val = inb(ACPI_PM1A_CNT_BLK_ADDRESS_V1);
 BUG_ON(!(pm1a_cnt_val & ACPI_PM1C_SCI_EN));
-- 
2.11.0


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