Why GIC Group1 Secure interrupt enabling is different between TF-A and non-TF-A builds ?

[himz-os]

Hi All,

I noticed something interesting while looking at the GICv3 initialization code in gic driver. When TF-A (CFG_WITH_ARM_TRUSTED_FW) is defined we don't enable Group 1 Secure interrupts below code.

optee_os/core/drivers/gic.c

Line 629 in fd6196d

io_setbits32(gd->gicd_base + GICD_CTLR, GICD_CTLR_ENABLEGRP1S);

While I understand that TF-A handles the GIC initialization (as the code comments mention), I'm curious why this specific Group 1 Secure interrupt configuration is treated differently ?

Would appreciate to know some insight into the reasoning behind this implementation.

Thanks

[himz-os]

Hi Team,

Any update on above query ? Thanks

[jenswi-linaro]

TF-A is supposed to set GICD_CTLR_ENABLEGRP1S, unless I'm missing something.

[himz-os]

Hi @jenswi-linaro ,

As per below code (TF-A) only enables the CTLR_ENABLE_G1S_BIT control bit when there are interrupts specifically configured as INTR_GROUP1S (Group 1 Secure). If no interrupts are registered with the INTR_GROUP1S configuration in TF-A, the CTLR_ENABLE_G1S_BIT will remain disabled.
https://github.com/ARM-software/arm-trusted-firmware/blob/master/drivers/arm/gic/v3/gicv3_helpers.c#L254

Let me know if I am missing something.

[jenswi-linaro]

We are expecting TF-A to configure the secure SGIs, so GICD_CTLR_ENABLEGRP1S should be set. But it's a bit obscure. It might be worth comparing with how Hafnium (SPMC at S-EL2) initializes the GIC.

[himz-os]

Hi @jenswi-linaro,

We are expecting TF-A to configure the secure SGIs, so GICD_CTLR_ENABLEGRP1S should be set. But it's a bit obscure.

I agree, that we shouldn't assume all platforms will enable secure SGIs in TF-A - that feels like a pretty risky assumption to make. Different platforms might need to handle this differently.

I think optee-os is supposed to handle G1S interrupts, it should probably be the one in charge of enabling them too, right? The way it's set up now, with TF-A enabling them first and then optee-os coming in later... well, OPTEE-OS might accidentally clear pending interrupts while GICv3 controller initialization. That just doesn't feel like right to me.

It might be worth comparing with how Hafnium (SPMC at S-EL2) initializes the GIC.

I don't really know much about SPMC running at S-EL2 honestly - haven't played around with it myself yet.

[jenswi-linaro]

I checked, and Hafnium enables it also, in a recent patch: commit 5084bdce2045 ("fix(gicv3): enable G1S interrupts if not enabled by TF-A"). Can you create a PR to fix it in OP-TEE?

[himz-os]

Hi @jenswi-linaro ,

Thanks for the replay. Below commit make sense.
commit 5084bdce2045 ("fix(gicv3): enable G1S interrupts if not enabled by TF-A").

Can you create a PR to fix it in OP-TEE?
I Need to check internally and comeback.

[github-actions]

This issue has been marked as a stale issue because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment, otherwise this issue will automatically be closed in 5 days. Note, that you can always re-open a closed issue at any time.

[github-actions]

This issue has been marked as a stale issue because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment, otherwise this issue will automatically be closed in 5 days. Note, that you can always re-open a closed issue at any time.

[github-actions]

This issue has been marked as a stale issue because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment, otherwise this issue will automatically be closed in 5 days. Note, that you can always re-open a closed issue at any time.