diff mbox

[Xen-devel,RFC,04/35] ACPI / ACPICA: Introduce ARM Boot Architecture Flags in FADT

Message ID 1423058539-26403-5-git-send-email-parth.dixit@linaro.org
State New
Headers show

Commit Message

Parth Dixit Feb. 4, 2015, 2:01 p.m. UTC
From: Naresh Bhat <naresh.bhat@linaro.org>

    The Power State Coordination Interface (PSCI) defines an API that
    can be used to coordinate power control amongst the various supervisory
    systems concurrently running on a device. ACPI support for this
    technology would require the addition of two flags: PSCI_COMPLIANT and
    PSCI_USE_HVC. When set, the former signals to the OS that the hardware
    is PSCI compliant. The latter selects the appropriate conduit for PSCI
    calls by toggling between Hypervisor Calls (HVC) and Secure Monitor
    Calls (SMC).

    An ARM Boot Architecture Flags structure to support new ARM hardware
    was introduced in FADT in ACPI 5.1, add the code accordingly to
    implement that in ACPICA core.

    Since ACPI 5.1 doesn't support self defined PSCI function IDs,
    which means that only PSCI 0.2+ is supported in ACPI.

    Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
    Signed-off-by: Naresh Bhat <naresh.bhat@linaro.org>
---
 xen/include/acpi/actbl.h | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Comments

Julien Grall Feb. 4, 2015, 9:03 p.m. UTC | #1
Hi Parth,

On 04/02/2015 14:01, parth.dixit@linaro.org wrote:
> From: Naresh Bhat <naresh.bhat@linaro.org>
>
>      The Power State Coordination Interface (PSCI) defines an API that
>      can be used to coordinate power control amongst the various supervisory
>      systems concurrently running on a device. ACPI support for this
>      technology would require the addition of two flags: PSCI_COMPLIANT and
>      PSCI_USE_HVC. When set, the former signals to the OS that the hardware
>      is PSCI compliant. The latter selects the appropriate conduit for PSCI
>      calls by toggling between Hypervisor Calls (HVC) and Secure Monitor
>      Calls (SMC).
>
>      An ARM Boot Architecture Flags structure to support new ARM hardware
>      was introduced in FADT in ACPI 5.1, add the code accordingly to
>      implement that in ACPICA core.
>
>      Since ACPI 5.1 doesn't support self defined PSCI function IDs,
>      which means that only PSCI 0.2+ is supported in ACPI.
>
>      Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
>      Signed-off-by: Naresh Bhat <naresh.bhat@linaro.org>
> ---
>   xen/include/acpi/actbl.h | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/xen/include/acpi/actbl.h b/xen/include/acpi/actbl.h
> index 856945d..34e8673 100644
> --- a/xen/include/acpi/actbl.h
> +++ b/xen/include/acpi/actbl.h
> @@ -244,7 +244,8 @@ struct acpi_table_fadt {
>   	u32 flags;		/* Miscellaneous flag bits (see below for individual flags) */
>   	struct acpi_generic_address reset_register;	/* 64-bit address of the Reset register */
>   	u8 reset_value;		/* Value to write to the reset_register port to reset the system */
> -	u8 reserved4[3];	/* Reserved, must be zero */
> +        u16 arm_boot_flags;     /* ARM Boot Architecture Flags (see below for individual flags) */
> +        u8 minor_version;       /* Minor version of this FADT structure */

The indentation is wrong. Beware that Xen is using space rather than tab 
for the indentation.

>   	u64 Xfacs;		/* 64-bit physical address of FACS */
>   	u64 Xdsdt;		/* 64-bit physical address of DSDT */
>   	struct acpi_generic_address xpm1a_event_block;	/* 64-bit Extended Power Mgt 1a Event Reg Blk address */
> @@ -259,7 +260,7 @@ struct acpi_table_fadt {
>   	struct acpi_generic_address sleep_status;	/* 64-bit Sleep Status register */
>   };
>
> -/* Masks for FADT Boot Architecture Flags (boot_flags) */
> +/* Masks for FADT IA-PC Boot Architecture Flags (boot_flags) [Vx]=Introduced in this FADT revision */

What does "this FADT revision" means? Please be more specific by giving 
the version.

>
>   #define ACPI_FADT_LEGACY_DEVICES    (1)  	/* 00: [V2] System has LPC or ISA bus devices */
>   #define ACPI_FADT_8042              (1<<1)	/* 01: [V3] System has an 8042 controller on port 60/64 */
> @@ -270,6 +271,11 @@ struct acpi_table_fadt {
>
>   #define FADT2_REVISION_ID               3
>
> +/* Masks for FADT ARM Boot Architecture Flags (arm_boot_flags) */
> +
> +#define ACPI_FADT_PSCI_COMPLIANT    (1)        /* 00: PSCI 0.2+ is implemented */
> +#define ACPI_FADT_PSCI_USE_HVC      (1<<1)     /* 01: HVC must be used instead of SMC as the PSCI conduit */
> +
>   /* Masks for FADT flags */
>
>   #define ACPI_FADT_WBINVD            (1)	/* 00: [V1] The wbinvd instruction works properly */
> @@ -345,7 +351,7 @@ enum acpi_prefered_pm_profiles {
>    *     FADT V5  size: 0x10C
>    */
>   #define ACPI_FADT_V1_SIZE       (u32) (ACPI_FADT_OFFSET (flags) + 4)
> -#define ACPI_FADT_V2_SIZE       (u32) (ACPI_FADT_OFFSET (reserved4[0]) + 3)
> +#define ACPI_FADT_V2_SIZE       (u32) (ACPI_FADT_OFFSET (arm_boot_flags) + 3)
>   #define ACPI_FADT_V3_SIZE       (u32) (ACPI_FADT_OFFSET (sleep_control))
>   #define ACPI_FADT_V5_SIZE       (u32) (sizeof (struct acpi_table_fadt))

Regards,
Julien Grall Feb. 5, 2015, 2:09 p.m. UTC | #2
Hi Ian,

On 05/02/2015 19:06, Ian Campbell wrote:
> On Wed, 2015-02-04 at 21:03 +0000, Julien Grall wrote:
>>> -/* Masks for FADT Boot Architecture Flags (boot_flags) */
>>> +/* Masks for FADT IA-PC Boot Architecture Flags (boot_flags) [Vx]=Introduced in this FADT revision */
>>
>> What does "this FADT revision" means? Please be more specific by giving
>> the version.
>
> Isn't it a reference to the comments on each of the following entries
> which start [V2], [V3], etc?

Ah right, I was confused because this patch doesn't add any [Vx], but 
only update the comment. The commit message should be updated to reflex 
this thing.

BTW, shouldn't we precise the version of the PSCI flags?

Regards,
Julien Grall Feb. 5, 2015, 2:10 p.m. UTC | #3
Hi Ian,

On 05/02/2015 19:06, Ian Campbell wrote:
> On Wed, 2015-02-04 at 21:03 +0000, Julien Grall wrote:
>>> -/* Masks for FADT Boot Architecture Flags (boot_flags) */
>>> +/* Masks for FADT IA-PC Boot Architecture Flags (boot_flags) [Vx]=Introduced in this FADT revision */
>>
>> What does "this FADT revision" means? Please be more specific by giving
>> the version.
>
> Isn't it a reference to the comments on each of the following entries
> which start [V2], [V3], etc?

Ah right, I was confused because this patch doesn't add any [Vx], but 
only update the comment. The commit message should be updated to reflex 
this thing.

BTW, shouldn't we precise the version of the PSCI flags?

Regards,
diff mbox

Patch

diff --git a/xen/include/acpi/actbl.h b/xen/include/acpi/actbl.h
index 856945d..34e8673 100644
--- a/xen/include/acpi/actbl.h
+++ b/xen/include/acpi/actbl.h
@@ -244,7 +244,8 @@  struct acpi_table_fadt {
 	u32 flags;		/* Miscellaneous flag bits (see below for individual flags) */
 	struct acpi_generic_address reset_register;	/* 64-bit address of the Reset register */
 	u8 reset_value;		/* Value to write to the reset_register port to reset the system */
-	u8 reserved4[3];	/* Reserved, must be zero */
+        u16 arm_boot_flags;     /* ARM Boot Architecture Flags (see below for individual flags) */
+        u8 minor_version;       /* Minor version of this FADT structure */
 	u64 Xfacs;		/* 64-bit physical address of FACS */
 	u64 Xdsdt;		/* 64-bit physical address of DSDT */
 	struct acpi_generic_address xpm1a_event_block;	/* 64-bit Extended Power Mgt 1a Event Reg Blk address */
@@ -259,7 +260,7 @@  struct acpi_table_fadt {
 	struct acpi_generic_address sleep_status;	/* 64-bit Sleep Status register */
 };
 
-/* Masks for FADT Boot Architecture Flags (boot_flags) */
+/* Masks for FADT IA-PC Boot Architecture Flags (boot_flags) [Vx]=Introduced in this FADT revision */
 
 #define ACPI_FADT_LEGACY_DEVICES    (1)  	/* 00: [V2] System has LPC or ISA bus devices */
 #define ACPI_FADT_8042              (1<<1)	/* 01: [V3] System has an 8042 controller on port 60/64 */
@@ -270,6 +271,11 @@  struct acpi_table_fadt {
 
 #define FADT2_REVISION_ID               3
 
+/* Masks for FADT ARM Boot Architecture Flags (arm_boot_flags) */
+
+#define ACPI_FADT_PSCI_COMPLIANT    (1)        /* 00: PSCI 0.2+ is implemented */
+#define ACPI_FADT_PSCI_USE_HVC      (1<<1)     /* 01: HVC must be used instead of SMC as the PSCI conduit */
+
 /* Masks for FADT flags */
 
 #define ACPI_FADT_WBINVD            (1)	/* 00: [V1] The wbinvd instruction works properly */
@@ -345,7 +351,7 @@  enum acpi_prefered_pm_profiles {
  *     FADT V5  size: 0x10C
  */
 #define ACPI_FADT_V1_SIZE       (u32) (ACPI_FADT_OFFSET (flags) + 4)
-#define ACPI_FADT_V2_SIZE       (u32) (ACPI_FADT_OFFSET (reserved4[0]) + 3)
+#define ACPI_FADT_V2_SIZE       (u32) (ACPI_FADT_OFFSET (arm_boot_flags) + 3)
 #define ACPI_FADT_V3_SIZE       (u32) (ACPI_FADT_OFFSET (sleep_control))
 #define ACPI_FADT_V5_SIZE       (u32) (sizeof (struct acpi_table_fadt))