[hal] Modification in interrupt handler assignment

[JuanSapriza]

Currently, the interrupt handlers are defined as weak functions in plic.c:

__attribute__((weak, optimize("O0"))) void handler_irq_uart( void ) { } 

Added to an array of function pointer:

handler_funct_t handlers[PLIC_INTR_SRCS_NUM] = {&handler_irq_uart, 
                                                &handler_irq_gpio, 
                                                &handler_irq_i2c, 
                                                &handler_irq_spi,
                                                &handler_irq_ext};

and then, when an interrupt happens, the PLIC checks which pin triggered the interrupt. Then locates that pin inside one of the possible ranges:

...
#define I2C_ID_START    33
#define I2C_ID_END      48
#define SPI_ID          49
#define EXT_IRQ_START   50
#define EXT_IRQ_END     63

Each of which correspond to one of the handler functions. The function is then called through the pointer

void handler_irq_external(void){
  plic_irq_id_t int_id = 0;
  plic_result_t res = plic_irq_claim(&int_id);
  irq_sources_t type = plic_get_irq_src_type(int_id);
  if(type != IRQ_BAD) {
    handlers[type]();               // HERE THE FUNCTION IS CALLED
    plic_intr_flag = 1;
    plic_irq_complete(&int_id);
  }
}

The weak implementations can then be redefined in the application to attend the interrupt.

A problem appears when the application uses an external peripheral. The handler_irq_ext has no way of determining which pin was the source of the interrupt and, therefore, if it corresponds to it.

One possible solution is to pass the id of the pin to the handler:

void handler_irq_external(void){
  plic_irq_id_t int_id = 0;
  plic_result_t res = plic_irq_claim(&int_id);
  irq_sources_t type = plic_get_irq_src_type(int_id);
  if(type != IRQ_BAD) {
    handlers[type]( int_id );       // THE INTERRUPT ID IS PASSED
    plic_intr_flag = 1;

    plic_irq_complete(&int_id);
  }
}

A minor inconvenience of this solution is that it requires every handler to take a parameter (to use the function-pointer-array), despite they will not use it.

__attribute__((weak, optimize("O0"))) void handler_irq_uart(plic_irq_id_t id) { } 

⚠️ It is important to solve this issue at HAL level and not let the particular project (e.g. CGRA-X-HEEP) solve it. If each project modifies the HAL, they will have to merge/resolve conflicts every time they revendorize X-HEEP.

---

A similar problem could appear if in a project a new interrupt is added (i.e. displaces the EXT_IRQ_START forward).This happened once with the DMA in #219. Luckily for us, this is a modification inside the X-HEEP project, so it will be merged into the HAL. If the same modification was done in an outside project, they would have to fight each revendorization.

---

My proposal to this is:

• Define a PLIC interrupt handler format.
• Each application with a PLIC interrupt declares a function to handle the interrupt.
• Calls a PLIC function to "introduce the function", including a pointer to the function and the ID that it will attend.

e.g.

void dma_plic_interrupt_handler( void ){
   printf("This is only printed if the DMA PLIC interrupt is triggered!\n");
}

void main(){
   plic_init();
   plic_new_handler_for_id( &dma_plic_interrupt_handler, DMA_ID );
   . . .
}

void plic_new_handler_for_id( void* ptr, plic_irq_id_t id ){
    handlers_list[ id ] = ptr;
}

⚠️ Before calling this functions it is important to a) check they are not null pointers, and/or b) initialize them to a dummy function. Maybe an intermediate solution would be to store the pointer values as pointer - pointer_to_dummy_function and then always call (pointer + pointer_to_dummy_function)() to make sure that, if they were not initialized, we would still call pointer_to_dummy_function.

This way, the PLIC does not need to know which peripherals its attending.

Furthermore, we could define ranges of interrupts:

plic_new_handler_for_range( &safety_pins_interrupt_handler,  GPIO_INTR_15,  GPIO_INTR_20);

This way the PLIC does not need to know the ranges either. This would require that the function loops from one id to the other filling the array with the desired handler.

---

Further features we noticed are:

• Independence from hardware connections
• Is still compatible with weak implementation of handlers
• There is no need to check which was the triggered source (for the I2C HAL, for instance).

---

@JoseCalero, I could get into this straight away if you agree. This way we waive @StefanoAlbini96 and I can get this change before I re-vendorize both in the DMA branch and in the CGRA-X-HEEP project.

[JuanSapriza]

We have discussed this issue and agree on:

• The need of passing the ID to the interrupt
• Keeping all modules (PLIC and peripherals) self contained

We have concerns on how to achieve this. Options that are being discussed:

• Make each peripheral provide the handler and where the connection is done (as suggested in this issue). The problem this brings is the dependency between the peripheral and the type of interrupt controller being used and how it works.
• Make each interrupt controller know what it has connected. This has the problem of needing to modify the HAL on every change in the connections.
• Have an intermediate layer to specify the connections. This adds an additional translation and complexity level.

[JuanSapriza]

We decided:

• Rename handler functions from PLIC and FIC to be the same
• Have large array in the PLIC where the indexes are the IDs of the interrupt sources
• The non-used ones have a dummy function pointer
• Each entry of the array is initialized by the plic at init time.
• We will pass the ID to the handlers
• We remove the ranges classification
• Juan will start working on this.

[JuanSapriza]

We further discussed this topic today.
The structure would be having something like this for the PLIC HAL:

#include "core.h"
#include "i2c.h"

int32_t handlers[ INTR__size ]; 
int32_t priority[ INTR__size ];

void plic_init(){
    handlers[INTR_ID_I2C_REQ] = &i2c_intr_handler_general
    handlers[INTR_ID_I2C_ABC] = &i2c_intr_handler_general
    . . . 
    
   priority[INTR_ID_I2C_JKL] = 1;
   . . .
}

And in the peripherals HAL simply have the handlers defined.

This way, there is no need to include the PLIC/FIC modules from the peripherals and they are the only file that needs to be modified in case of a change.

The priorities, if they have to be changed on runtime, can be done through a PLIC SDK.

[davideschiavone]

we should also generates those ranges rather than using fixed ones (hjson file)

[JuanSapriza]

We just met with @StefanoAlbini96 and @ruben-roalvarez

We analyzed an issue with the previous idea: When you want to extend X-HEEP, you would need to modify the PLIC. This left us 3 possible alternatives:

1. PLIC makes the assignments (the last proposed option): Make the assignment between IDs and handlers in the PLIC HAL, which would require:
   a) Including all peripherals in the PLIC HAL
   b) Making the PLIC non-portable
   c) Modifying the PLIC HAL to extend X-HEEP and add the change to a vendor patch
2. Peripherals make the assignments (the inverse idea): Make the assignment on a peripheral-to-peripheral basis on their HAL, which would require:
   a) Including the PLIC on every peripheral
   b) Making peripherals non-portable
3. SDK/Application makes the assignments: let the assignment be done in the application, which would require
   a) Including the PLIC and peripheral in the application or SDK
   b) Update example applications to include this change (that should later be done at SDK level for non-extended peripherals)

We considered option 3) would be the most reasonable because:
a) It keeps both PLIC and peripherals portable
b) Does not require any modification of X-HEEP drivers from an application level
c) Allows to extend the logic to the FIC and make peripherals interrupt-source-agnostic (which is not as useful as extended peripherals would not be connected to the FIC anyways, although we could add this possibility in the future).
d) It is how it is done in many commercial SDKs, and for many similar handler-assignment situations.

We will leave this in pause until Jose comes back so we can further discuss these options.

Note: When I say extended peripherals I mean those which do not come with X-HEEP (e.g. the CGRA, vs. those which do are included, e.g. I2C, DMA). They are added in a repository along with X-HEEP and need to be paired both hardware and software-wise in additional files. See below image.

[JuanSapriza]

Today in the firmware meeting we further discussed this issue.

To avoid calling the PLIC from all examples, a good compromise is stick to the previous proposal (making assignments in the vector inside the PLIC init) but leaving weak implementations ONLY FOR THE EXTERNAL INTERRUPTS.

This way, only for the external apps we should make the assignment at SDK/application level.

[JuanSapriza]

Will be closed when #363 is merged