On Sat, 15 Jun 2024 at 19:56, Florian Lugou <florian.lu...@provenrun.com> wrote:
>
> CNTHCTL_EL2 based masking of timer interrupts was introduced in
> f6fc36deef6abcee406211f3e2f11ff894b87fa4. This masking was however
> effective no matter whether EL2 was enabled in the current security
> state or not, contrary to arm specification.
>
> Signed-off-by: Florian Lugou <florian.lu...@provenrun.com>
> ---
> target/arm/helper.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index ce31957235..60e2344c68 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -2684,7 +2684,8 @@ static void gt_update_irq(ARMCPU *cpu, int timeridx)
> * If bit CNTHCTL_EL2.CNT[VP]MASK is set, it overrides IMASK.
> * It is RES0 in Secure and NonSecure state.
> */
> - if ((ss == ARMSS_Root || ss == ARMSS_Realm) &&
> + if ((arm_hcr_el2_eff(env) & HCR_E2H) &&
> + (ss == ARMSS_Root || ss == ARMSS_Realm) &&
When the architecture says "is EL2 enabled in the current security state"
it doesn't mean "is HCR_EL2.E2H set?", it means "is this either NonSecure/Realm
or else is SCR_EL2.EEL2 set?". Compare the pseudocode EL2Enabled()
and QEMU's arm_is_el2_enabled() and arm_is_el2_enabled_secstate() functions.
This doesn't mean much in Root state, and for Realm state EL2 is always
enabled (assuming it is implemented).
For this timer check, we're doing I think the same thing as the
pseudocode AArch64.CheckTimerConditions(), which does:
if (IsFeatureImplemented(FEAT_RME) && ss IN {SS_Root, SS_Realm} &&
CNTHCTL_EL2.CNTPMASK == '1') then
imask = '1';
so I'm inclined to say that our current implementation in QEMU is correct.
> ((timeridx == GTIMER_VIRT && (cnthctl & R_CNTHCTL_CNTVMASK_MASK)) ||
> (timeridx == GTIMER_PHYS && (cnthctl & R_CNTHCTL_CNTPMASK_MASK)))) {
> irqstate = 0;
> --
thanks
-- PMM
