Do you not need to save-restore for non-secure el2 ?
Regards,
Oza.
From: Manish Pandey2 <Manish.Pandey2@arm.com>
Sent: Wednesday, December 3, 2025 3:01 AM
To: Pawandeep Oza (QUIC) <quic_poza@quicinc.com>; 'tf-a@lists.trustedfirmware.org' <tf-a@lists.trustedfirmware.org>; Boyan Karatotev <Boyan.Karatotev@arm.com>
Subject: Re: [TF-A] Re: [RFE] lib/el3_runtime: set NS bit if secure el2 is not implemented
Before replying to your email, I want to highlight a concern: I did not receive the original email from Pawandeep (it seems there may be an issue with the mailing list).
Regarding the technical point when the system does not include an S-EL2 component, there is no need to save or restore the
icc_sre_el2_val register.
There is a patch currently under review [1] that moves the GIC save/restore logic to be invoked from the SPM dispatcher (when S-EL2 is present). As a result, configurations running at S-EL1 will
not trigger this path.
From: Boyan Karatotev via TF-A <tf-a@lists.trustedfirmware.org>
Sent: 03 December 2025 09:05
To: Pawandeep Oza (QUIC) <quic_poza@quicinc.com>; 'tf-a@lists.trustedfirmware.org' <tf-a@lists.trustedfirmware.org>
Subject: [TF-A] Re: [RFE] lib/el3_runtime: set NS bit if secure el2 is not implemented
Hi Oza,
On 02/12/2025 23:07, Pawandeep Oza (QUIC) via TF-A wrote:
>
> Please have a look at following patch, where on our platform we try to maintain single image of TFA (for custom CPU and Cortex A55)
> Cortex A55 does not have Secure EL2 implemented, while on the other hand our custom CPU has secure EL2 (and we run Hafnium there)
>
> On Cortex A55 ARM AEM model:
> write_icc_sre_el2(read_el2_ctx_common(ctx, icc_sre_el2));
>
> the context was set before as
> u_register_t icc_sre_el2_val = ICC_SRE_DIB_BIT | ICC_SRE_DFB_BIT |
> ICC_SRE_EN_BIT | ICC_SRE_SRE_BIT;
>
> and setting ICC_SRE_DIB_BIT | ICC_SRE_DFB_BIT causes crash since secure EL2 is not implemented.
Just to clarify, is it the bits that are being set or the entire register? What's the exact crash you're seeing?
> resulting into following patch which resolves the issue. seeking feedback/discussion if I can post it to upstream TFA,
You can always post to upstream for review and is usually the best place to consider changes.
> let me know if I am missing something here.
>
>
> lib/el3_runtime: set NS bit if secure el2 is not implemented
>
> before setting icc_sre_el2 set NS bit for non-secure context so that
> the ICC_SRE_DIB_BIT and ICC_SRE_DFB_BIT are preserved
>
> Signed-off-by: Oza Pawandeep <quic_poza@quicinc.com>
>
> diff --git a/lib/el3_runtime/aarch64/context_mgmt.c b/lib/el3_runtime/aarch64/context_mgmt.c
> index e31255868..5100f2f00 100644
> --- a/lib/el3_runtime/aarch64/context_mgmt.c
> +++ b/lib/el3_runtime/aarch64/context_mgmt.c
> @@ -1411,7 +1411,18 @@ static void el2_sysregs_context_restore_gic(el2_sysregs_t *ctx, uint32_t securit
> u_register_t scr_el3 = read_scr_el3();
>
> #if defined(SPD_spmd) && SPMD_SPM_AT_SEL2
> - write_icc_sre_el2(read_el2_ctx_common(ctx, icc_sre_el2));
> + if (is_feat_sel2_supported()) {
> + write_icc_sre_el2(read_el2_ctx_common(ctx, icc_sre_el2));
> + } else {
> + write_scr_el3(scr_el3 | SCR_NS_BIT);
> + isb();
> +
I suspect a more correct approach is to ensure that the NS bit has been set for EL3 execution. The problem with this is that the SPMD_SPM_AT_SEL2 flag means that there will be an SPM and it will be SEL2. Would a simple `if (is_feat_sel2_supported()) write_icc_sre_el2()
else nop` work? The gicv3 driver should have already set gic registers and since they won't change that would be safe.
> + write_icc_sre_el2(read_el2_ctx_common(ctx, icc_sre_el2));
> +
> + write_scr_el3(scr_el3);
> + isb();
> + }
> +
> #else
> write_scr_el3(scr_el3 | SCR_NS_BIT);
> isb();
>
>
> Regards,
> Oza.
Thanks,
Boyan
--
TF-A mailing list -- tf-a@lists.trustedfirmware.org
To unsubscribe send an email to
tf-a-leave@lists.trustedfirmware.org