[PATCH v2] irq/arm-gicv3.h: Customize ICC_IGRPEN0/1 init

[Chris Johns]

> On 8 Jul 2022, at 6:15 pm, Sebastian Huber <sebastian.huber at embedded-brains.de> wrote:
> 
> On 08.07.22 09:55, Chris Johns wrote:
>> I have finally managed to test this patch and it does not work on a
>> secure/non-secure Versal. I did a `distclean configure` and rebuild so it would
>> pick up the option changes.
>> The `BSP_ARM_GIC_ICC_IGRPEN0 write is still in the build and it is a secure access:
>>   /* Initialize the group 0 and 1 interrupt enable */
>> #ifdef BSP_ARM_GIC_ICC_IGRPEN0
>>   WRITE_SR(ICC_IGRPEN0, BSP_ARM_GIC_ICC_IGRPEN0);
>>     103e7e5c:   52800020        mov     w0, #0x1                        // #1
>>     103e7e60:   d518ccc0        msr     icc_igrpen0_el1, x0
>> #endif
>> 
> 
> Sorry, the variants pattern was wrong. It should be:
> 
> diff --git a/spec/build/bsps/optarmgic-icc-igrpen0.yml b/spec/build/bsps/optarmgic-icc-igrpen0.yml
> index 5acd37617d..29a4d7ad51 100644
> --- a/spec/build/bsps/optarmgic-icc-igrpen0.yml
> +++ b/spec/build/bsps/optarmgic-icc-igrpen0.yml
> @@ -9,7 +9,7 @@ default: 1
> default-by-variant:
> - value: null
>   variants:
> -  - aarch64
> +  - aarch64/.*
> description: |
>   Optionally, defines the initial value of the ICC_IGRPEN0 register of the ARM
>   GIC CPU Interface.
> 
> I will fix this in v3.

Great. I have the hardware now so I can test. 

>> I tested this piece of code and it is OK:
>> #ifdef BSP_ARM_GIC_ICC_IGRPEN1
>>   WRITE_SR(ICC_IGRPEN1, BSP_ARM_GIC_ICC_IGRPEN1);
>>     103e7e64:   d518cce0        msr     icc_igrpen1_el1, x0
>> #endif
>> Why is this register conditional because I think it is always needed?
> 
> I don't know.

I think there is no need to be conditional. Interrupts need this. 

>> I think the default for BSP_ARM_GIC_ICC_IGRPEN0 should be disabled.
> 
> Yes, for aarch64.

Sound good and thanks

Have a good weekend
Chris