From mboxrd@z Thu Jan 1 00:00:00 1970 From: Parth Dixit Subject: Re: [PATCH v2 07/41] arm/acpi : Introduce ARM Boot Architecture Flags in FADT Date: Sun, 24 May 2015 11:33:42 +0530 Message-ID: References: <1431893048-5214-1-git-send-email-parth.dixit@linaro.org> <1431893048-5214-8-git-send-email-parth.dixit@linaro.org> <5559F72F.6070802@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============3533730770151226169==" Return-path: In-Reply-To: <5559F72F.6070802@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Julien Grall Cc: keir@xen.org, Ian Campbell , andrew.cooper3@citrix.com, tim@xen.org, xen-devel , Stefano Stabellini , Jan Beulich , Christoffer Dall List-Id: xen-devel@lists.xenproject.org --===============3533730770151226169== Content-Type: multipart/alternative; boundary=001a1136fe96c928f20516cda779 --001a1136fe96c928f20516cda779 Content-Type: text/plain; charset=UTF-8 On 18 May 2015 at 19:59, Julien Grall 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 > > Signed-off-by: Naresh Bhat > > --- > > 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 > --001a1136fe96c928f20516cda779 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable


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 supervisor= y
> 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>
> ---
>=C2=A0 xen/include/acpi/actbl.h | 10 ++++++++--
>=C2=A0 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 {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0u32 flags;=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 /* Miscellaneous flag bits (see below for individual flags) = */
>=C2=A0 =C2=A0 =C2=A0 =C2=A0struct acpi_generic_address reset_register;= =C2=A0 =C2=A0 =C2=A0/* 64-bit address of the Reset register */
>=C2=A0 =C2=A0 =C2=A0 =C2=A0u8 reset_value;=C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0/* Value to write to the reset_register port to reset the system */ > -=C2=A0 =C2=A0 =C2=A0u8 reserved4[3];=C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Re= served, must be zero */
> +=C2=A0 =C2=A0 u16 arm_boot_flags;=C2=A0 =C2=A0 =C2=A0/* ARM Boot Arch= itecture Flags (see below for individual flags) */
> +=C2=A0 =C2=A0 u8 minor_revision;=C2=A0 =C2=A0 =C2=A0 =C2=A0/* Minor v= ersion of this FADT structure */

Wrong indentation. The file is using hard tab.

>=C2=A0 =C2=A0 =C2=A0 =C2=A0u64 Xfacs;=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 /* 64-bit physical address of FACS */
>=C2=A0 =C2=A0 =C2=A0 =C2=A0u64 Xdsdt;=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 /* 64-bit physical address of DSDT */
>=C2=A0 =C2=A0 =C2=A0 =C2=A0struct acpi_generic_address xpm1a_event_bloc= k;=C2=A0 /* 64-bit Extended Power Mgt 1a Event Reg Blk address */
> @@ -270,6 +271,11 @@ struct acpi_table_fadt {
>
>=C2=A0 #define FADT2_REVISION_ID=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A03
>
> +/* Masks for FADT ARM Boot Architecture Flags (arm_boot_flags) */
> +
> +#define ACPI_FADT_PSCI_COMPLIANT=C2=A0 =C2=A0 (1)=C2=A0 =C2=A0 =C2=A0= =C2=A0 /* 00: PSCI 0.2+ is implemented */
> +#define ACPI_FADT_PSCI_USE_HVC=C2=A0 =C2=A0 =C2=A0 (1<<1)=C2=A0= =C2=A0 =C2=A0/* 01: HVC must be used instead of SMC as the PSCI conduit */=
> +
>=C2=A0 /* Masks for FADT flags */
>
>=C2=A0 #define ACPI_FADT_WBINVD=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 (1)=C2=A0 =C2=A0 =C2=A0 /* 00: [V1] The wbinvd instruction works proper= ly */
> @@ -345,7 +351,7 @@ enum acpi_prefered_pm_profiles {
>=C2=A0 =C2=A0*=C2=A0 =C2=A0 =C2=A0FADT V5=C2=A0 size: 0x10C
>=C2=A0 =C2=A0*/
>=C2=A0 #define ACPI_FADT_V1_SIZE=C2=A0 =C2=A0 =C2=A0 =C2=A0(u32) (ACPI_= FADT_OFFSET (flags) + 4)
> -#define ACPI_FADT_V2_SIZE=C2=A0 =C2=A0 =C2=A0 =C2=A0(u32) (ACPI_FADT_= OFFSET (reserved4[0]) + 3)
> +#define ACPI_FADT_V2_SIZE=C2=A0 =C2=A0 =C2=A0 =C2=A0(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

--001a1136fe96c928f20516cda779-- --===============3533730770151226169== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============3533730770151226169==--