[PATCH 19/41] bsps/irq: Implement new directives for GICv2/3

Kinsey Moore kinsey.moore at oarcorp.com
Tue Jul 13 12:55:56 UTC 2021


On 7/13/2021 00:16, Sebastian Huber wrote:
> On 13/07/2021 04:46, Kinsey Moore wrote:
>>> index a1ba5e9112..6f5d4015e4 100644
>>> --- a/bsps/shared/dev/irq/arm-gicv2.c
>>> +++ b/bsps/shared/dev/irq/arm-gicv2.c
>>> @@ -1,5 +1,5 @@
>>>   /*
>>> - * Copyright (c) 2013, 2019 embedded brains GmbH.  All rights 
>>> reserved.
>>> + * Copyright (c) 2013, 2021 embedded brains GmbH.  All rights 
>>> reserved.
>>>    *
>>>    *  embedded brains GmbH
>>>    *  Dornierstr. 4
>>> @@ -69,6 +69,28 @@ rtems_status_code bsp_interrupt_get_attributes(
>>>     rtems_interrupt_attributes *attributes
>>>   )
>>>   {
>>> +  attributes->is_maskable = true;
>>> +  attributes->maybe_enable = true;
>>> +
>>> +  if ( vector <= ARM_GIC_IRQ_SGI_LAST ) {
>>> +    attributes->always_enabled = true;
>>
>> As far as I'm aware, SGIs can be enabled or disabled using 
>> GICD_ISENABLER0 just like
>>
>> PPI or SPI interrupts for both GICv2 and GICv3. Section 3.1.2 of the 
>> GICv2 architecture
>>
>> spec (IHI0048B) references this, though I have seen implementations 
>> where certain SGI
>>
>> and PPI interrupts are hard-wired enabled or disabled and that state 
>> can't be changed
>>
>> (which is also covered in this section).
>
> Ok, on Qemu and the i.MX7D the SGI are always enabled. I would keep 
> the attributes like this until we have a system which is different. I 
> will remove the check in bsp_interrupt_vector_enable/disable(). So, in 
> the worst case, the attributes are wrong.
I only mention it because I've seen it on ZynqMP hardware. Interrupt 
enable bits for interrupts 0-24 are locked with 0-7 permanently enabled 
and 8-24 permanently disabled. I think the QEMU GICv3 driver allows all 
SGIs to be enabled or disabled. I tried to get more information about 
whether those bits can be unlocked, but nothing has been forthcoming: 
https://forums.xilinx.com/t5/Processor-System-Design-and-AXI/Zynq-Ultrascale-MPSoC-SGI-and-PPI-enable/td-p/1212370
>
>>
>>> +    attributes->can_enable = true;
>>> +    attributes->can_cause = true;
>>> +    attributes->can_cause_on = true;
>>> +    attributes->cleared_by_acknowledge = true;
>>> +  } else {
>>> +    attributes->can_disable = true;
>>> +    attributes->can_cause = true;
>>> +    attributes->can_clear = true;
>>> +
>>> +    if ( vector > ARM_GIC_IRQ_PPI_LAST ) {
>>> +      /* SPI */
>>> +      attributes->can_enable = true;
>>> +      attributes->can_get_affinity = true;
>>> +      attributes->can_set_affinity = true;
>>> +    }
>>> +  }
>>> +
>>>     return RTEMS_SUCCESSFUL;
>>>   }
>>> @@ -77,16 +99,25 @@ rtems_status_code bsp_interrupt_is_pending(
>>>     bool               *pending
>>>   )
>>>   {
>>> - bsp_interrupt_assert(bsp_interrupt_is_valid_vector(vector));
>>> -  bsp_interrupt_assert(pending != NULL);
>>> -  *pending = false;
>>> -  return RTEMS_UNSATISFIED;
>>> +  volatile gic_dist *dist = ARM_GIC_DIST;
>>> +
>>> +  *pending = gic_id_is_pending(dist, vector);
>>> +  return RTEMS_SUCCESSFUL;
>>>   }
>>>   rtems_status_code bsp_interrupt_cause(rtems_vector_number vector)
>>>   {
>>> bsp_interrupt_assert(bsp_interrupt_is_valid_vector(vector));
>>> -  return RTEMS_UNSATISFIED;
>>> +
>>> +  if (vector <= ARM_GIC_IRQ_SGI_LAST) {
>>> +    arm_gic_trigger_sgi(vector, 1U << _SMP_Get_current_processor());
>>> +  } else {
>>> +    volatile gic_dist *dist = ARM_GIC_DIST;
>>> +
>>> +    gic_id_set_pending(dist, vector);
>>> +  }
>>> +
>>> +  return RTEMS_SUCCESSFUL;
>>>   }
>>>   #if defined(RTEMS_SMP)
>>> @@ -95,15 +126,27 @@ rtems_status_code bsp_interrupt_cause_on(
>>>     uint32_t            cpu_index
>>>   )
>>>   {
>>> - bsp_interrupt_assert(bsp_interrupt_is_valid_vector(vector));
>>> -  return RTEMS_UNSATISFIED;
>>> +  if (vector >= 16) {
>>> +    return RTEMS_UNSATISFIED;
>>> +  }
>>> +
>>> +  arm_gic_trigger_sgi(vector, 1U << cpu_index);
>>> +  return RTEMS_SUCCESSFUL;
>>>   }
>>>   #endif
>>>   rtems_status_code bsp_interrupt_clear(rtems_vector_number vector)
>>>   {
>>> +  volatile gic_dist *dist = ARM_GIC_DIST;
>>> +
>>> bsp_interrupt_assert(bsp_interrupt_is_valid_vector(vector));
>>> -  return RTEMS_UNSATISFIED;
>>> +
>>> +  if (vector <= ARM_GIC_IRQ_SGI_LAST) {
>>> +    return RTEMS_UNSATISFIED;
>>> +  }
>>> +
>>> +  gic_id_clear_pending(dist, vector);
>>> +  return RTEMS_SUCCESSFUL;
>>>   }
>>>   rtems_status_code bsp_interrupt_vector_is_enabled(
>>> @@ -113,8 +156,16 @@ rtems_status_code bsp_interrupt_vector_is_enabled(
>>>   {
>>> bsp_interrupt_assert(bsp_interrupt_is_valid_vector(vector));
>>>     bsp_interrupt_assert(enabled != NULL);
>>> -  *enabled = false;
>>> -  return RTEMS_UNSATISFIED;
>>> +
>>> +  if (vector <= ARM_GIC_IRQ_SGI_LAST) {
>>> +    *enabled = true;
>>> +  } else {
>>> +    volatile gic_dist *dist = ARM_GIC_DIST;
>>> +
>>> +    *enabled = gic_id_is_enabled(dist, vector);
>>> +  }
>>> +
>>> +  return RTEMS_SUCCESSFUL;
>>>   }
>>>   rtems_status_code bsp_interrupt_vector_enable(rtems_vector_number 
>>> vector)
>>> @@ -133,6 +184,11 @@ rtems_status_code 
>>> bsp_interrupt_vector_disable(rtems_vector_number vector)
>>> bsp_interrupt_assert(bsp_interrupt_is_valid_vector(vector));
>>> +  if (vector <= ARM_GIC_IRQ_SGI_LAST) {
>>> +    /* SGI cannot be disabled */
>>> +    return RTEMS_UNSATISFIED;
>>> +  }
>>> +
>
> I will remove this check here.
>
>>>     gic_id_disable(dist, vector);
>>>     return RTEMS_SUCCESSFUL;
>>>   }
>>> @@ -207,8 +263,8 @@ BSP_START_TEXT_SECTION void 
>>> arm_gic_irq_initialize_secondary_cpu(void)
>>>     dist->icdigr[0] = 0xffffffff;
>>>   #endif
>>> -  /* Initialize Peripheral Private Interrupts (PPIs) */
>>> -  for (id = 0; id < 32; ++id) {
>>> +  /* Initialize priority of SGIs and PPIs */
>>> +  for (id = 0; id <= ARM_GIC_IRQ_PPI_LAST; ++id) {
>>>       gic_id_set_priority(dist, id, PRIORITY_DEFAULT);
>>>     }
>>> @@ -300,6 +356,10 @@ rtems_status_code bsp_interrupt_set_affinity(
>>>     volatile gic_dist *dist = ARM_GIC_DIST;
>>>     uint8_t targets = (uint8_t) 
>>> _Processor_mask_To_uint32_t(affinity, 0);
>>> +  if ( vector <= ARM_GIC_IRQ_PPI_LAST ) {
>>> +    return RTEMS_UNSATISFIED;
>>> +  }
>>> +
>>>     gic_id_set_targets(dist, vector, targets);
>>>     return RTEMS_SUCCESSFUL;
>>>   }
>>> @@ -310,8 +370,13 @@ rtems_status_code bsp_interrupt_get_affinity(
>>>   )
>>>   {
>>>     volatile gic_dist *dist = ARM_GIC_DIST;
>>> -  uint8_t targets = gic_id_get_targets(dist, vector);
>>> +  uint8_t targets;
>>> +
>>> +  if ( vector <= ARM_GIC_IRQ_PPI_LAST ) {
>>> +    return RTEMS_UNSATISFIED;
>>> +  }
>>> +  targets = gic_id_get_targets(dist, vector);
>>>     _Processor_mask_From_uint32_t(affinity, targets, 0);
>>>     return RTEMS_SUCCESSFUL;
>>>   }
>>> diff --git a/bsps/shared/dev/irq/arm-gicv3.c 
>>> b/bsps/shared/dev/irq/arm-gicv3.c
>>> index 8db3053ffd..211f4d35c4 100644
>>> --- a/bsps/shared/dev/irq/arm-gicv3.c
>>> +++ b/bsps/shared/dev/irq/arm-gicv3.c
>>> @@ -169,6 +169,25 @@ rtems_status_code bsp_interrupt_get_attributes(
>>>     rtems_interrupt_attributes *attributes
>>>   )
>>>   {
>>> +  attributes->is_maskable = true;
>>> +  attributes->can_enable = true;
>>> +
>>> +  if ( vector <= ARM_GIC_IRQ_SGI_LAST ) {
>>> +    attributes->can_cause = true;
>>> +    attributes->can_cause_on = true;
>>> +    attributes->cleared_by_acknowledge = true;
>>> +  } else {
>>> +    attributes->can_disable = true;
>>> +    attributes->can_cause = true;
>>> +    attributes->can_clear = true;
>>> +
>>> +    if ( vector > ARM_GIC_IRQ_PPI_LAST ) {
>>> +      /* SPI */
>>
>> The actual number of SPI interrupts supported by GICv3 can vary 
>> depending on the configuration
>>
>> of the IP. This should check that the provided vector is within that 
>> range.
>
> Yes, I will change this to
>
> attributes->maybe_enable = true;
>
> I would not do the probing here, since this would require to do it 
> also in bsp_interrupt_vector_is_valid(). The probing could be done 
> using bsp_interrupt_vector_enable() and 
> bsp_interrupt_vector_is_enabled().

That sounds fine.


Thanks,

Kinsey



More information about the devel mailing list