Re: [PATCH v2 07/41] arm/acpi : Introduce ARM Boot Architecture Flags in FADT

[Parth Dixit <parth.dixit@linaro.org>]

+shannon

On 24 May 2015 at 11:33, Parth Dixit <parth.dixit@linaro.org> wrote:
>
>
> On 18 May 2015 at 19:59, Julien Grall <julien.grall@citrix.com> wrote:
>>
>> Hi Parth,
>>
>> On 17/05/15 21:03, Parth Dixit wrote:
>> > 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 | 10 ++++++++--
>> >  1 file changed, 8 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/xen/include/acpi/actbl.h b/xen/include/acpi/actbl.h
>> > index 856945d..96fd1d8 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_revision;       /* Minor version of this FADT structure */
>>
>> Wrong indentation. The file is using hard tab.
>>
>> >       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 */
>> > @@ -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)
>>
>> Linux is using ACPI_FADT_OFFSET(minor_revision) + 1. Can you use the
>> same here?
>>
>> Also, I've notice that the patch (see 9eb1105) is slightly different.
>> Mostly documenting the ACPI version for the fields.
>>
>> Can you update the patch based on Linux code to use the latest version
>> (i.e the version upstreamed)?
>>
> sure, i'll update it to be in sync with linux.
>>
>> Regards,
>>
>> --
>> Julien Grall
>
>