On 24/6/2022 7:43 pm, Sebastian Huber wrote: > The GICv3 support is used by AArch32 (indicated by the ARM_MULTILIB_ARCH_V4 > define) and AArch64 targets. Use the existing WRITE_SR() abstraction to > access > the interrupt group 0 and 1 enable registers. This fixes the build for the > AArch32 target.
It needs to be tested on hardware before I am OK with it. It also needs EL3 firmware, ie TF-A, to correctly initialise a system. I would be OK with qemu if it can be shown it honours the security level correctly. I however have no time to determine this as I have Versal hardware that did not like the changes. > --- > bsps/include/dev/irq/arm-gicv3.h | 23 ++++++----------------- > 1 file changed, 6 insertions(+), 17 deletions(-) > > diff --git a/bsps/include/dev/irq/arm-gicv3.h > b/bsps/include/dev/irq/arm-gicv3.h > index a79368ebdf..7db7bad034 100644 > --- a/bsps/include/dev/irq/arm-gicv3.h > +++ b/bsps/include/dev/irq/arm-gicv3.h > @@ -116,9 +116,11 @@ extern "C" { > #else /* ARM_MULTILIB_ARCH_V4 */ > > /* AArch64 GICv3 registers are not named in GCC */ The FreeBSD would suggest this is not entirely true? May be it is for aarch32? > -#define ICC_IGRPEN0 "S3_0_C12_C12_6, %0" > -#define ICC_IGRPEN1 "S3_0_C12_C12_7, %0" > +#define ICC_IGRPEN0_EL1 "S3_0_C12_C12_6, %0" > +#define ICC_IGRPEN1_EL1 "S3_0_C12_C12_7, %0" This looks like it is only a label change and so it is the same opcode. Is that correct? > #define ICC_IGRPEN1_EL3 "S3_6_C12_C12_7, %0" > +#define ICC_IGRPEN0 ICC_IGRPEN0_EL1 > +#define ICC_IGRPEN1 ICC_IGRPEN1_EL1 > #define ICC_PMR "S3_0_C4_C6_0, %0" > #define ICC_EOIR1 "S3_0_C12_C12_1, %0" > #define ICC_SRE "S3_0_C12_C12_5, %0" > @@ -300,20 +302,6 @@ static void gicv3_init_dist(volatile gic_dist *dist) > } > } > > -/* > - * A better way to access these registers than special opcodes > - */ > -#define isb() __asm __volatile("isb" : : : "memory") > - > -#define WRITE_SPECIALREG(reg, _val) \ > - __asm __volatile("msr " __STRING(reg) ", %0" : : "r"((uint64_t)_val)) > - > -#define gic_icc_write(reg, val) \ > -do { \ > - WRITE_SPECIALREG(icc_ ##reg ##_el1, val); \ > - isb(); \ > -} while (0) > - > static void gicv3_init_cpu_interface(uint32_t cpu_index) > { > uint32_t sre_value = 0x7; > @@ -335,7 +323,8 @@ static void gicv3_init_cpu_interface(uint32_t cpu_index) > } > > /* Enable interrupt groups 0 and 1 */ > - gic_icc_write(IGRPEN1, 1);' This has been tested and works on a Versal. > + WRITE_SR(ICC_IGRPEN0, 0x1); This crashed in EL1 on a Versal with 2021.2 TF-A. Why do you need to set this here? > + WRITE_SR(ICC_IGRPEN1, 0x1); This instruction also generated an exception. It has been a while but I am pretty sure I had to comment this one and when I did no interrupts happened. The code I ported from FreeBSD worked. I also think the FreeBSD calls are easier to review and so maintain. I find those ARM type registers difficult to find and check. Chris _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel