GitLab

This is based on the rpi implementation from
https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/2746

.
Signed-off-by: Andrew Walbran <qwandor@google.com>
Change-Id: I5fe324fcd9d5e232091e01267ea12147c46bc9c1

Same SoC has different personality by creating different number of:
- cores
- clusters.

As a result, the platform specific power domain tree will be created
after identify the personality of the SoC.
Hence, platform specific power domain tree may not be same for all the
personality of the soc.

Thus, psci library code will deduce the 'plat_core_count', while
populating the power domain tree topology and return the number of
cores.

PLATFORM_CORE_COUNT will still be valid for a SoC, such that
psci_plat_core_count <= PLATFORM_CORE_COUNT.

PLATFORM_CORE_COUNT will continued to be defined by platform to create
the data structures.
Signed-off-by: Pankaj Gupta <pankaj.gupta@nxp.com>
Change-Id: I1f5c47647631cae2dcdad540d64cf09757db7185

cpu_idx is used as mix of `unsigned int` and `signed int` in code
with typecasting at some places. This change is to unify the
cpu_idx as `unsigned int` as underlying API;s `plat_my_core_pos`
returns `unsigned int`

It was discovered via coverity issue CID 354715
Signed-off-by: Deepika Bhavnani <deepika.bhavnani@arm.com>
Change-Id: I4f0adb0c596ff1177210c5fe803bff853f2e54ce

Signed-off-by: Deepika Bhavnani <deepika.bhavnani@arm.com>
Change-Id: I4a496d5a8e7a9a127cd6224c968539eb74932fca

GCC diagnostics were added to ignore array boundaries, instead
of ignoring GCC warning current code will check for array boundaries
and perform and array update only for valid elements.

Resolves: `CID 246574` `CID 246710` `CID 246651`
Signed-off-by: Deepika Bhavnani <deepika.bhavnani@arm.com>
Change-Id: I7530ecf7a1707351c6ee87e90cc3d33574088f57

NOTE: AARCH32/AARCH64 macros are now deprecated in favor of __aarch64__.

All common C compilers pre-define the same macros to signal which
architecture the code is being compiled for: __arm__ for AArch32 (or
earlier versions) and __aarch64__ for AArch64. There's no need for TF-A
to define its own custom macros for this. In order to unify code with
the export headers (which use __aarch64__ to avoid another dependency),
let's deprecate the AARCH32 and AARCH64 macros and switch the code base
over to the pre-defined standard macro. (Since it is somewhat
unintuitive that __arm__ only means AArch32, let's standardize on only
using __aarch64__.)

Change-Id: Ic77de4b052297d77f38fc95f95f65a8ee70cf200
Signed-off-by: Julius Werner <jwerner@chromium.org>

When acquiring or releasing the power domain locks for a given CPU the
parent nodes are looked up by walking the up the PD tree list on both the
acquire and release path, only one set of lookups is needed. Fetch the
parent nodes first and pass this list into both the acquire and release
functions to avoid the double lookup.

This also allows us to not have to do this lookup after coherency has
been exited during the core power down sequence. The shared struct
psci_cpu_pd_nodes is not placed in coherent memory like is done
for psci_non_cpu_pd_nodes and doing so would negatively affect
performance. With this patch we remove the need to have it in coherent
memory by moving the access out of psci_release_pwr_domain_locks().
Signed-off-by: Andrew F. Davis <afd@ti.com>
Change-Id: I7b9cfa9d31148dea0f5e21091c8b45ef7fe4c4ab

Enforce full include path for includes. Deprecate old paths.

The following folders inside include/lib have been left unchanged:

- include/lib/cpus/${ARCH}
- include/lib/el3_runtime/${ARCH}

The reason for this change is that having a global namespace for
includes isn't a good idea. It defeats one of the advantages of having
folders and it introduces problems that are sometimes subtle (because
you may not know the header you are actually including if there are two
of them).

For example, this patch had to be created because two headers were
called the same way: e0ea0928 ("Fix gpio includes of mt8173 platform
to avoid collision."). More recently, this patch has had similar
problems: 46f9b2c3 ("drivers: add tzc380 support").

This problem was introduced in commit 4ecca339

 ("Move include and
source files to logical locations"). At that time, there weren't too
many headers so it wasn't a real issue. However, time has shown that
this creates problems.

Platforms that want to preserve the way they include headers may add the
removed paths to PLAT_INCLUDES, but this is discouraged.

Change-Id: I39dc53ed98f9e297a5966e723d1936d6ccf2fc8f
Signed-off-by: Antonio Nino Diaz <antonio.ninodiaz@arm.com>

When a platform enables its caches before it accesses the
psci_non_cpu_pd_nodes structure then explicit cache maintenance
is not needed.
Signed-off-by: Andrew F. Davis <afd@ti.com>

The MMU is not disabled in this path, update the comment to
reflect this. Also clarify that both paths call prepare_cpu_pwr_dwn(),
but the second path does stack cache maintenance.
Signed-off-by: Andrew F. Davis <afd@ti.com>

Mark the initialization functions in BL31, such as context management,
EHF, RAS and PSCI as __init so that they can be reclaimed by the
platform when no longer needed

Change-Id: I7446aeee3dde8950b0f410cb766b7a2312c20130
Signed-off-by: Daniel Boulby <daniel.boulby@arm.com>

Change-Id: I40d040aa05bcbf11536a96ce59827711456b93a8
Co-authored-by: Antonio Nino Diaz <antonio.ninodiaz@arm.com>
Signed-off-by: Antonio Nino Diaz <antonio.ninodiaz@arm.com>

If either USE_COHERENT_MEM or HW_ASSISTED_COHERENCY being true
should cause us to not enter the ifdef block, then the logic
is not correct here. Posibly bad use of De Morgan's law?
Fix this.
Signed-off-by: Andrew F. Davis <afd@ti.com>

MISRA C-2012 Rules 10.1, 10.3, 17.8 and 20.7.

Change-Id: I3980bd2a1d845559af4bbe2887a0250d0506a064
Signed-off-by: Antonio Nino Diaz <antonio.ninodiaz@arm.com>

Fix MISRA C-2012 Directive 4.9 and Rule 21.1 defects.

Change-Id: I96c216317d38741ee632d2640cd7b36e6723d5c2
Signed-off-by: Antonio Nino Diaz <antonio.ninodiaz@arm.com>

This patch includes various fixes for PSCI STAT functionality
relating to timestamp collection:

1. The PSCI stat accounting for retention states for higher level
power domains were done outside the locks which could lead to
spurious values in some race conditions. This is moved inside
the locks. Also, the call to start the stat accounting was redundant
which is now removed.

2. The timestamp wrap-around case when calculating residency did
not cater for AArch32. This is now fixed.

3. In the warm boot path, `plat_psci_stat_accounting_stop()` was
getting invoked prior to population of target power states. This
is now corrected.

Change-Id: I851526455304fb74ff0a724f4d5318cd89e19589
Signed-off-by: Soby Mathew <soby.mathew@arm.com>

Previously commit 555ebb34db8f3424c1b394df2f10ecf9c1f70901 attmpted to fix this
GCC issue:

services/std_svc/psci/psci_common.c: In function 'psci_do_state_coordination':
services/std_svc/psci/psci_common.c:220:27: error: array subscript is above
array bounds [-Werror=array-bounds]
  psci_req_local_pwr_states[pwrlvl - 1][cpu_idx] = req_pwr_state;

This fix doesn't work as asserts aren't built in non-debug build flows.

Let's use GCCs #pragma option (documented here:
https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html

) to avoid
this false positive instead.
Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>

This patch enables the 'sign-compare' flag, to enable warning/errors
for comparisons between signed/unsigned variables. The warning has
been enabled for all the Tegra platforms, to start with.
Signed-off-by: Varun Wadekar <vwadekar@nvidia.com>

In AArch64, privileged exception levels control the execution state
(a.k.a. register width) of the immediate lower Exception Level; i.e.
whether the lower exception level executes in AArch64 or AArch32 state.
For an exception level to have its execution state changed at run time,
it must request the change by raising a synchronous exception to the
higher exception level.

This patch implements and adds such a provision to the ARM SiP service,
by which an immediate lower exception level can request to switch its
execution state. The execution state is switched if the request is:

  - raised from non-secure world;

  - raised on the primary CPU, before any secondaries are brought online
    with CPU_ON PSCI call;

  - raised from an exception level immediately below EL3: EL2, if
    implemented; otherwise NS EL1.

If successful, the SMC doesn't return to the caller, but to the entry
point supplied with the call. Otherwise, the caller will observe the SMC
returning with STATE_SW_E_DENIED code. If ARM Trusted Firmware is built
for AArch32, the feature is not supported, and the call will always
fail.

For the ARM SiP service:

  - Add SMC function IDs for both AArch32 and AArch64;
  - Increment the SiP service minor version to 2;
  - Adjust the number of supported SiP service calls.

Add documentation for ARM SiP service.

Fixes ARM-software/tf-issues#436

Change-Id: I4347f2d6232e69fbfbe333b340fcd0caed0a4cea
Signed-off-by: Jeenu Viswambharan <jeenu.viswambharan@arm.com>

To make software license auditing simpler, use SPDX[0] license
identifiers instead of duplicating the license text in every file.

NOTE: Files that have been imported by FreeBSD have not been modified.

[0]: https://spdx.org/



Change-Id: I80a00e1f641b8cc075ca5a95b10607ed9ed8761a
Signed-off-by: dp-arm <dimitris.papastamos@arm.com>

The current PSCI implementation can apply certain optimizations upon the
assumption that all PSCI participants are cache-coherent.

  - Skip performing cache maintenance during power-up.

  - Skip performing cache maintenance during power-down:

    At present, on the power-down path, CPU driver disables caches and
    MMU, and performs cache maintenance in preparation for powering down
    the CPU. This means that PSCI must perform additional cache
    maintenance on the extant stack for correct functioning.

    If all participating CPUs are cache-coherent, CPU driver would
    neither disable MMU nor perform cache maintenance. The CPU being
    powered down, therefore, remain cache-coherent throughout all PSCI
    call paths. This in turn means that PSCI cache maintenance
    operations are not required during power down.

  - Choose spin locks instead of bakery locks:

    The current PSCI implementation must synchronize both cache-coherent
    and non-cache-coherent participants. Mutual exclusion primitives are
    not guaranteed to function on non-coherent memory. For this reason,
    the current PSCI implementation had to resort to bakery locks.

    If all participants are cache-coherent, the implementation can
    enable MMU and data caches early, and substitute bakery locks for
    spin locks. Spin locks make use of architectural mutual exclusion
    primitives, and are lighter and faster.

The optimizations are applied when HW_ASSISTED_COHERENCY build option is
enabled, as it's expected that all PSCI participants are cache-coherent
in those systems.

Change-Id: Iac51c3ed318ea7e2120f6b6a46fd2db2eae46ede
Signed-off-by: Jeenu Viswambharan <jeenu.viswambharan@arm.com>

The PSCI implementation performs cache maintenance operations on its
data structures to ensure their visibility to both cache-coherent and
non-cache-coherent participants. These cache maintenance operations
can be skipped if all PSCI participants are cache-coherent. When
HW_ASSISTED_COHERENCY build option is enabled, we assume PSCI
participants are cache-coherent.

For usage abstraction, this patch introduces wrappers for PSCI cache
maintenance and barrier operations used for state coordination: they are
effectively NOPs when HW_ASSISTED_COHERENCY is enabled, but are
applied otherwise.

Also refactor local state usage and associated cache operations to make
it clearer.

Change-Id: I77f17a90cba41085b7188c1345fe5731c99fad87
Signed-off-by: Jeenu Viswambharan <jeenu.viswambharan@arm.com>

This patch introduces the following three platform interfaces:

* void plat_psci_stat_accounting_start(const psci_power_state_t *state_info)

  This is an optional hook that platforms can implement in order
  to perform accounting before entering a low power state.  This
  typically involves capturing a timestamp.

* void plat_psci_stat_accounting_stop(const psci_power_state_t *state_info)

  This is an optional hook that platforms can implement in order
  to perform accounting after exiting from a low power state.  This
  typically involves capturing a timestamp.

* u_register_t plat_psci_stat_get_residency(unsigned int lvl,
	const psci_power_state_t *state_info,
	unsigned int last_cpu_index)

  This is an optional hook that platforms can implement in order
  to calculate the PSCI stat residency.

If any of these interfaces are overridden by the platform, it is
recommended that all of them are.

By default `ENABLE_PSCI_STAT` is disabled.  If `ENABLE_PSCI_STAT`
is set but `ENABLE_PMF` is not set then an alternative PSCI stat
collection backend must be provided.  If both are set, then default
weak definitions of these functions are provided, using PMF to
calculate the residency.

NOTE: Previously, platforms did not have to explicitly set
`ENABLE_PMF` since this was automatically done by the top-level
Makefile.

Change-Id: I17b47804dea68c77bc284df15ee1ccd66bc4b79b
Signed-off-by: dp-arm <dimitris.papastamos@arm.com>

Replace all use of memset by zeromem when zeroing moderately-sized
structure by applying the following transformation:
memset(x, 0, sizeof(x)) => zeromem(x, sizeof(x))

As the Trusted Firmware is compiled with -ffreestanding, it forbids the
compiler from using __builtin_memset and forces it to generate calls to
the slow memset implementation. Zeromem is a near drop in replacement
for this use case, with a more efficient implementation on both AArch32
and AArch64.

Change-Id: Ia7f3a90e888b96d056881be09f0b4d65b41aa79e
Signed-off-by: Douglas Raillard <douglas.raillard@arm.com>

This patch adds AArch32 support to PSCI library, as follows :

* The `psci_helpers.S` is implemented for AArch32.

* AArch32 version of internal helper function `psci_get_ns_ep_info()` is
  defined.

* The PSCI Library is responsible for the Non Secure context initialization.
  Hence a library interface `psci_prepare_next_non_secure_ctx()` is introduced
  to enable EL3 runtime firmware to initialize the non secure context without
  invoking context management library APIs.

Change-Id: I25595b0cc2dbfdf39dbf7c589b875cba33317b9d

A PSCI CPU_SUSPEND request to place a CPU in retention states at power levels
higher than the CPU power level is subject to the same state coordination as a
power down state. A CPU could implement multiple retention states at a
particular power level. When exiting WFI, the non-CPU power levels may be in a
different retention state to what was initially requested, therefore each CPU
should refresh its view of the states of all power levels.

Previously, a CPU re-used the state of the power levels when it entered the
retention state. This patch fixes this issue by ensuring that a CPU upon exit
from retention reads the state of each power level afresh.

Change-Id: I93b5f5065c63400c6fd2598dbaafac385748f989

This patch introduces the PSCI Library interface. The major changes
introduced are as follows:

* Earlier BL31 was responsible for Architectural initialization during cold
boot via bl31_arch_setup() whereas PSCI was responsible for the same during
warm boot. This functionality is now consolidated by the PSCI library
and it does Architectural initialization via psci_arch_setup() during both
cold and warm boots.

* Earlier the warm boot entry point was always `psci_entrypoint()`. This was
not flexible enough as a library interface. Now PSCI expects the runtime
firmware to provide the entry point via `psci_setup()`. A new function
`bl31_warm_entrypoint` is introduced in BL31 and the previous
`psci_entrypoint()` is deprecated.

* The `smc_helpers.h` is reorganized to separate the SMC Calling Convention
defines from the Trusted Firmware SMC helpers. The former is now in a new
header file `smcc.h` and the SMC helpers are moved to Architecture specific
header.

* The CPU context is used by PSCI for context initialization and
restoration after power down (PSCI Context). It is also used by BL31 for SMC
handling and context management during Normal-Secure world switch (SMC
Context). The `psci_smc_handler()` interface is redefined to not use SMC
helper macros thus enabling to decouple the PSCI context from EL3 runtime
firmware SMC context. This enables PSCI to be integrated with other runtime
firmware using a different SMC context.

NOTE: With this patch the architectural setup done in `bl31_arch_setup()`
is done as part of `psci_setup()` and hence `bl31_platform_setup()` will be
invoked prior to architectural setup. It is highly unlikely that the platform
setup will depend on architectural setup and cause any failure. Please be
be aware of this change in sequence.

Change-Id: I7f497a08d33be234bbb822c28146250cb20dab73

This patch moves the PSCI services and BL31 frameworks like context
management and per-cpu data into new library components `PSCI` and
`el3_runtime` respectively. This enables PSCI to be built independently from
BL31. A new `psci_lib.mk` makefile is introduced which adds the relevant
PSCI library sources and gets included by `bl31.mk`. Other changes which
are done as part of this patch are:

* The runtime services framework is now moved to the `common/` folder to
  enable reuse.
* The `asm_macros.S` and `assert_macros.S` helpers are moved to architecture
  specific folder.
* The `plat_psci_common.c` is moved from the `plat/common/aarch64/` folder
  to `plat/common` folder. The original file location now has a stub which
  just includes the file from new location to maintain platform compatibility.

Most of the changes wouldn't affect platform builds as they just involve
changes to the generic bl1.mk and bl31.mk makefiles.

NOTE: THE `plat_psci_common.c` FILE HAS MOVED LOCATION AND THE STUB FILE AT
THE ORIGINAL LOCATION IS NOW DEPRECATED. PLATFORMS SHOULD MODIFY THEIR
MAKEFILES TO INCLUDE THE FILE FROM THE NEW LOCATION.

Change-Id: I6bd87d5b59424995c6a65ef8076d4fda91ad5e86

This patch fixes some coding guideline warnings reported by the checkpatch
script. Only files related to upcoming feature development have been fixed.

Change-Id: I26fbce75c02ed62f00493ed6c106fe7c863ddbc5

This patch reworks type usage in generic code, drivers and ARM platform files
to make it more portable. The major changes done with respect to
type usage are as listed below:

* Use uintptr_t for storing address instead of uint64_t or unsigned long.
* Review usage of unsigned long as it can no longer be assumed to be 64 bit.
* Use u_register_t for register values whose width varies depending on
  whether AArch64 or AArch32.
* Use generic C types where-ever possible.

In addition to the above changes, this patch also modifies format specifiers
in print invocations so that they are AArch64/AArch32 agnostic. Only files
related to upcoming feature development have been reworked.

Change-Id: I9f8c78347c5a52ba7027ff389791f1dad63ee5f8

This patch adds following optional PSCI STAT functions:

- PSCI_STAT_RESIDENCY: This call returns the amount of time spent
  in power_state in microseconds, by the node represented by the
  `target_cpu` and the highest level of `power_state`.

- PSCI_STAT_COUNT: This call returns the number of times a
  `power_state` has been used by the node represented by the
  `target_cpu` and the highest power level of `power_state`.

These APIs provides residency statistics for power states that has
been used by the platform. They are implemented according to v1.0
of the PSCI specification.

By default this optional feature is disabled in the PSCI
implementation. To enable it, set the boolean flag
`ENABLE_PSCI_STAT` to 1. This also sets `ENABLE_PMF` to 1.

Change-Id: Ie62e9d37d6d416ccb1813acd7f616d1ddd3e8aff

When BL31 is compiled at `-O3` optimization level using Linaro GCC 4.9
AArch64 toolchain, it reports the following error:

```
services/std_svc/psci/psci_common.c: In function 'psci_do_state_coordination':
services/std_svc/psci/psci_common.c:220:27: error: array subscript is above
array bounds [-Werror=array-bounds]
  psci_req_local_pwr_states[pwrlvl - 1][cpu_idx] = req_pwr_state;
                           ^
```

This error is a false positive and this patch resolves the error by asserting
the array bounds in `psci_do_state_coordination()`.

Fixes ARM-software/tf-issues#347

Change-Id: I3584ed7b2e28faf455b082cb3281d6e1d11d6495

Migrate all direct usage of __attribute__ to usage of their
corresponding macros from cdefs.h.
e.g.:
 - __attribute__((unused)) -> __unused
Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>

This patch unifies the bakery lock api's across coherent and normal
memory implementation of locks by using same data type `bakery_lock_t`
and similar arguments to functions.

A separate section `bakery_lock` has been created and used to allocate
memory for bakery locks using `DEFINE_BAKERY_LOCK`. When locks are
allocated in normal memory, each lock for a core has to spread
across multiple cache lines. By using the total size allocated in a
separate cache line for a single core at compile time, the memory for
other core locks is allocated at link time by multiplying the single
core locks size with (PLATFORM_CORE_COUNT - 1). The normal memory lock
algorithm now uses lock address instead of the `id` in the per_cpu_data.
For locks allocated in coherent memory, it moves locks from
tzfw_coherent_memory to bakery_lock section.

The bakery locks are allocated as part of bss or in coherent memory
depending on usage of coherent memory. Both these regions are
initialised to zero as part of run_time_init before locks are used.
Hence, bakery_lock_init() is made an empty function as the lock memory
is already initialised to zero.

The above design lead to the removal of psci bakery locks from
non_cpu_power_pd_node to psci_locks.

NOTE: THE BAKERY LOCK API WHEN USE_COHERENT_MEM IS NOT SET HAS CHANGED.
THIS IS A BREAKING CHANGE FOR ALL PLATFORM PORTS THAT ALLOCATE BAKERY
LOCKS IN NORMAL MEMORY.

Change-Id: Ic3751c0066b8032dcbf9d88f1d4dc73d15f61d8b

This patch reworks the PSCI generic implementation to conform to ARM
Trusted Firmware coding guidelines as described here:
https://github.com/ARM-software/arm-trusted-firmware/wiki

This patch also reviews the use of signed data types within PSCI
Generic code and replaces them with their unsigned counterparts wherever
they are not appropriate. The PSCI_INVALID_DATA macro which was defined
to -1 is now replaced with PSCI_INVALID_PWR_LVL macro which is defined
to PLAT_MAX_PWR_LVL + 1.

Change-Id: Iaea422d0e46fc314e0b173c2b4c16e0d56b2515a

As per PSCI1.0 specification, the error code to be returned when an invalid
non secure entrypoint address is specified by the PSCI client for CPU_SUSPEND,
CPU_ON or SYSTEM_SUSPEND must be PSCI_E_INVALID_ADDRESS. The current PSCI
implementation returned PSCI_E_INVAL_PARAMS. This patch rectifies this error
and also implements a common helper function to validate the entrypoint
information to be used across these PSCI API implementations.

Change-Id: I52d697d236c8bf0cd3297da4008c8e8c2399b170

This commit does the switch to the new PSCI framework implementation replacing
the existing files in PSCI folder with the ones in PSCI1.0 folder. The
corresponding makefiles are modified as required for the new implementation.
The platform.h header file is also is switched to the new one
as required by the new frameworks. The build flag ENABLE_PLAT_COMPAT defaults
to 1 to enable compatibility layer which let the existing platform ports to
continue to build and run with minimal changes.

The default weak implementation of platform_get_core_pos() is now removed from
platform_helpers.S and is provided by the compatibility layer.

Note: The Secure Payloads and their dispatchers still use the old platform
and framework APIs and hence it is expected that the ENABLE_PLAT_COMPAT build
flag will remain enabled in subsequent patch. The compatibility for SPDs using
the older APIs on platforms migrated to the new APIs will be added in the
following patch.

Change-Id: I18c51b3a085b564aa05fdd98d11c9f3335712719

This patch adds support for SYSTEM_SUSPEND API as mentioned in the PSCI 1.0
specification. This API, on being invoked on the last running core on a
supported platform, will put the system into a low power mode with memory
retention.

The psci_afflvl_suspend() internal API has been reused as most of the actions
to suspend a system are the same as invoking the PSCI CPU_SUSPEND API with the
target affinity level as 'system'. This API needs the 'power state' parameter
for the target low power state. This parameter is not passed by the caller of
the SYSTEM_SUSPEND API. Hence, the platform needs to implement the
get_sys_suspend_power_state() platform function to provide this information.
Also, the platform also needs to add support for suspending the system to the
existing 'plat_pm_ops' functions: affinst_suspend() and
affinst_suspend_finish().

Change-Id: Ib6bf10809cb4e9b92f463755608889aedd83cef5

mpidr_set_aff_inst() is left shifting an int constant and an
unsigned char value to construct an MPIDR. For affinity level 3 a
shift of 32 would result in shifting out of the 32-bit type and
have no effect on the MPIDR.

These values need to be extended to unsigned long before shifting
to ensure correct results for affinity level 3.

Change-Id: I1ef40afea535f14cfd820c347a065a228e8f4536

All coding style violations have been fixed in a previous patch and
since then, each individual patch has been checked in this regard.
However, the latest version of the checkpatch.pl script from the Linux
kernel is more advanced and it is able to flag new errors in the
Trusted Firmware codebase. This patch fixes them.

Change-Id: I1f332f2440984be85d36b231bb83260368987077