Re: [UNIKRAFT PATCH 0/2] plat/{kvm, linuxu}: statically allocate IRQ handlers

[Costin Lupu <costin.lupu@xxxxxxxxx>]

Hi Hugo,

On 6/23/20 12:16 PM, Hugo Lefeuvre wrote:
> IRQ handler entries are currently allocated dynamically. This has two
> undesired consequences: (1) this requires the dynamic memory allocator to
> be initialized when ukplat_irq_init() is called, which happens quite early,
> and (2) this makes the registering of IRQ handlers quite expensive.

Although these changes are quite useful, I do have the following comments:

1) It's not uncommon to allocate memory for IRQ handling. Linux does it.
But this is a more generic problem for which we don't have a solution
upstream: providing memory allocation means for any kernel subsystem,
meaning that some memory allocator should be available for the IRQ
subsystem as well.

2) As I said, Linux does it. But it's fine if we want to have the IRQ
handling data preallocated. What's not fine is replacing one
functionality with another. If we will find that we also need the
dynamic registering for some research idea we will have to revert to the
old changes. What I'm trying to say is that it would be better to have
an abstraction that would provide either dynamically or statically
allocated IRQ handling data.

3) Speaking of abstractions... What happened with the changes for Xen?
Isn't Xen a target platform anymore? Besides this, your changes add
duplicated code. The IRQ handling could have been changed/refactored
even more so that we could use the same code for all platforms. If we
want to touch these areas now then we should do it properly. Otherwise
the code will divert *again* towards opposite directions for each platform.

4) I see you're replacing __MAX_IRQ with specialized values for each
platform. What's the reason behind that? Btw, __MAX_IRQ was introduced
for the ARM changes. Did you tested this for ARM as well?

Cheers,
Costin