IRQ_GetMode/IRQ_GetMode

[ua1arn]

IRQ_GetMode function lack Secutity Group information
Can be fixed by adding line
mode |= !! GIC_GetGroup((IRQn_Type)irqn) * IRQ_MODE_DOMAIN_NONSECURE;

IRQ_SetMode not clear previously setted Group information
Remove condition before GIC_SetGroup and use

    // Apply configuration if no mode error
    if (status == 0) {
      GIC_SetConfiguration((IRQn_Type)irqn, cfg);
      GIC_SetTarget       ((IRQn_Type)irqn, cpu);

      GIC_SetGroup ((IRQn_Type)irqn, !! secure);	// Group 0 (Secure) and Group 1 (Non-secure).
    }

Full text:

/// Configure interrupt request mode.
int32_t IRQ_SetMode (IRQn_ID_t irqn, uint32_t mode) {
  uint32_t val;
  uint8_t cfg;
  uint8_t secure;
  uint8_t cpu;
  int32_t status = 0;

  if ((irqn >= 0) && (irqn < (IRQn_ID_t)IRQ_GIC_LINE_COUNT)) {
    // Check triggering mode
    val = (mode & IRQ_MODE_TRIG_Msk);

    if (val == IRQ_MODE_TRIG_LEVEL) {
      cfg = 0x00U;
    } else if (val == IRQ_MODE_TRIG_EDGE) {
      cfg = 0x02U;
    } else {
      cfg = 0x00U;
      status = -1;
    }

    val = (mode & IRQ_MODE_MODEL_Msk);
    if (val == IRQ_MODE_MODEL_1N) {
      cfg |= 1;   // 1-N model
    }

    // Check interrupt type
    val = mode & IRQ_MODE_TYPE_Msk;

    if (val != IRQ_MODE_TYPE_IRQ) {
      status = -1;
    }

    // Check interrupt domain
    val = mode & IRQ_MODE_DOMAIN_Msk;

    if (val == IRQ_MODE_DOMAIN_NONSECURE) {
      secure = 0U;
    } else {
      // Check security extensions support
      val = GIC_DistributorInfo() & (1UL << 10U);

      if (val != 0U) {
        // Security extensions are supported
        secure = 1U;	// Group 0 (Secure) and Group 1 (Non-secure).
      } else {
        secure = 0U;
        status = -1;
      }
    }

    // Check interrupt CPU targets
    val = mode & IRQ_MODE_CPU_Msk;

    if (val == IRQ_MODE_CPU_ALL) {
      cpu = 0xFFU;
    } else {
      cpu = (uint8_t)(val >> IRQ_MODE_CPU_Pos);
    }

    // Apply configuration if no mode error
    if (status == 0) {
      GIC_SetConfiguration((IRQn_Type)irqn, cfg);
      GIC_SetTarget       ((IRQn_Type)irqn, cpu);

      GIC_SetGroup ((IRQn_Type)irqn, !! secure);	// Group 0 (Secure) and Group 1 (Non-secure).
    }
  }

  return (status);
}


/// Get interrupt mode configuration.
uint32_t IRQ_GetMode (IRQn_ID_t irqn) {
  uint32_t mode;
  uint32_t val;

  if ((irqn >= 0) && (irqn < (IRQn_ID_t)IRQ_GIC_LINE_COUNT)) {
    mode = IRQ_MODE_TYPE_IRQ;

    // Get trigger mode
    val = GIC_GetConfiguration((IRQn_Type)irqn);

    if ((val & 2U) != 0U) {
      // Corresponding interrupt is edge triggered
      mode |= IRQ_MODE_TRIG_EDGE;
    } else {
      // Corresponding interrupt is level triggered
      mode |= IRQ_MODE_TRIG_LEVEL;
    }

    if (val & 1U) {
      mode |= IRQ_MODE_MODEL_1N;
    }
    // Get interrupt CPU targets
    mode |= GIC_GetTarget ((IRQn_Type)irqn) << IRQ_MODE_CPU_Pos;

    mode |= !! GIC_GetGroup((IRQn_Type)irqn) * IRQ_MODE_DOMAIN_NONSECURE;

  } else {
    mode = IRQ_MODE_ERROR;
  }

  return (mode);
}

[JonatanAntoni]

Hi @ua1arn, these features are community contributions. Perhaps @Masmiseim36 or others working on Cortex-A area can comment here?

I'd be fine to accept contribution to Cortex-A code. Can this feature somehow be tested in Core or CoreValidation tests?

[JonatanAntoni]

@ua1arn, could you please double check PR #281. The changes look minor and not too bad to me. Does this small change already fix your issues?

[ua1arn]

if (secure != 0U) {
GIC_SetGroup((IRQn_Type)irqn, secure);
}
GIC_SetGroup((IRQn_Type)irqn, **!**secure);

In mode field secure flag at non-zero state meaning group "1". AI make wrong code.

[Masmiseim36]

@ua1arn, could you please double check PR #281. The changes look minor and not too bad to me. Does this small change already fix your issues?

Unfortunately, I am not an expert on security extensions and am unable to evaluate the proposed changes.
Perhaps the original author of the routines, Vladimir Umek, can help here (see commit 496e255).

[ua1arn]

I mean - setting with IRQ_MODE_DOMAIN_Msk should return same value of this mask at query.
GIC register should be assigned to fresh value in any cases.
It is not expert level - it is expected behavior of getter/setter.

check for zero IRQ_MODE_DOMAIN_NONSECURE is my miss...