Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update i.MX ELE support #7256

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

saschahauer
Copy link
Contributor

Implement plat_rng_init() for i.MX SoCs with ELE unit and enable ELE support for SoCs which have it.

@sahilnxp
Copy link
Contributor

sahilnxp commented Feb 3, 2025

Hi @saschahauer

Did you test this pull request on all platforms (i.MX8ULP, i.MX91, i.MX93) impacted by this?

@@ -462,6 +464,7 @@ static TEE_Result imx_ele_rng_get_random(void *buffer, size_t size)
reg_pair_from_64((uint64_t)pa, &cmd.addr_msb, &cmd.addr_lsb);

cmd.size = (uint32_t)size;
update_crc(&msg);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hunk shouldn't be here. Will remove

@saschahauer
Copy link
Contributor Author

Hi @saschahauer

Did you test this pull request on all platforms (i.MX8ULP, i.MX91, i.MX93) impacted by this?

No, only on i.MX93. I don't have access to the other platforms

TEE_Result res = TEE_ERROR_GENERIC;
uint64_t timeout = timeout_init_us(10 * 1000);
TEE_Result res;
paddr_t pa;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TEE_Result = TEE_<something>;
paddr_t pa = 0;


cmd.size = (uint32_t)size;

memcpy(msg.data.u8, &cmd, sizeof(cmd));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update_crc(&msg) really not needed?
If that is the case, could you explicitly mention that in the commit message.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This patch is based on [1] which removes this as well, so indeed it doesn't seem to be needed.
However, it also doesn't hurt, so I am considering just to keep it in.

[1] nxp-imx/imx-optee-os@e7dadb8

@@ -504,6 +506,26 @@ unsigned long plat_get_aslr_seed(void)
return aslr;
}

void plat_rng_init(void)
{
TEE_Result res;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

= TEE_<something>

@@ -504,6 +506,26 @@ unsigned long plat_get_aslr_seed(void)
return aslr;
}

void plat_rng_init(void)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should consider whether or not CFG_WITH_SOFTWARE_PRNG is enabled?

plat_get_aslr_seed() open codes functionality for retrieving random data
from the ELE. Factor out his code to a function for later reuse. Add
support for being called with MMU enabled along the way.

Signed-off-by: Sascha Hauer <[email protected]>
Implement plat_rng_init() for ELE equipped SoCs to make sure we have a
non deterministic random seed.

Signed-off-by: Sascha Hauer <[email protected]>
@sahilnxp
Copy link
Contributor

sahilnxp commented Feb 4, 2025

@saschahauer Regarding enabling the ELE by default on every platform, Sorry, we cannot do that as of now because we have faced issues in past in which both Linux and OP-TEE tries to access the ELE and create issues which is mentioned in the commit message too.

There is only one MU to communicate with ELE, which cannot be
dedicated on OP-TEE side all the time. There may be ELE services
running on Linux side, which can cause conflict with OP-TEE.

ELE APIs are changed a lot in this time (just like CRC thing is not needed anymore, session is not needed while getting tee_otp_get_die_id() and more) also we are using TRUST MU dedicated for OP-TEE side which will remove the issue of MU sharing between Linux and OP-TEE.

We are in process of taking care of these changes and will upstream in coming weeks.

@saschahauer
Copy link
Contributor Author

@saschahauer Regarding enabling the ELE by default on every platform, Sorry, we cannot do that as of now because we have faced issues in past in which both Linux and OP-TEE tries to access the ELE and create issues which is mentioned in the commit message too.

There is only one MU to communicate with ELE, which cannot be
dedicated on OP-TEE side all the time. There may be ELE services
running on Linux side, which can cause conflict with OP-TEE.

I am aware of this and mentioned it in the commit message:

However, the ELE is currently only used for implementation of
plat_rng_init() and tee_otp_get_hw_unique_key(). The former is used only
during initialisation and the latter might be used during runtime, but
the ELE is actually only accessed once and the HUK is then cached for
later calls.

To put it short: The ELE is currently used during OP-TEE initialisation when no Linux is running.
Linux can have the ELE all for itself when running.

The ELE provides the HUK on the SoCs which have it, so having support
for it on these SoCs is really prudent.

The ELE has been disabled by default in [1] and [2] because of:

| There is only one MU to communicate with ELE, which cannot be
| dedicated on OP-TEE side all the time. There may be ELE services
| running on Linux side, which can cause conflict with OP-TEE.

However, the ELE is currently only used for implementation of
plat_rng_init() and tee_otp_get_hw_unique_key(). The former is used only
during initialisation and the latter might be used during runtime, but
the ELE is actually only accessed once and the HUK is then cached for
later calls.

Enable the ELE by default again to make the config for i.MX8ulp and i.MX9
more useful.

[1] commit: 29b4cb6 ("core: imx: disable ELE support on imx8ulp, imx93 by default")
[2] commit: 136cc65 ("core: imx: disable ELE support on i.MX91 by default")

Signed-off-by: Sascha Hauer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants