Hi Leonid,
On 27/08/2025 19:24, Leonid Komarianskyi wrote:
Introduced two new helper functions: gic_is_valid_line and
gic_is_spi. The first function helps determine whether an IRQ
number is less than the number of lines supported by hardware. The
second function additionally checks if the IRQ number falls within the
SPI range. Also, updated the appropriate checks to use these new helper
functions.
The current checks for the real GIC are very similar to those for the
vGIC but serve a different purpose. For GIC-related code, the interrupt
numbers should be validated based on whether the hardware can operate
with such interrupts. On the other hand, for the vGIC, the indexes must
also be verified to ensure they are available for a specific domain. The
first reason for introducing these helper functions is to avoid
potential confusion with vGIC-related checks. The second reason is to
consolidate similar code into separate functions, which can be more
easily extended by additional conditions, e.g., when implementing
extended SPI interrupts.
The changes, which replace open-coded checks with the use of the new
helper functions, do not introduce any functional changes, as the helper
functions follow the current IRQ index verification logic.
Signed-off-by: Leonid Komarianskyi <leonid_komarians...@epam.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babc...@epam.com>
With one remark below:
Acked-by: Julien Grall <jgr...@amazon.com>
---
Changes in V4:
- removed redundant parentheses
- added reviewed-by from Volodymyr Babchuk
Changes in V3:
- renamed gic_is_valid_irq to gic_is_valid_line and gic_is_shared_irq to
gic_is_spi
- updated commit message
Changes in V2:
- introduced this patch
Changes for V4:
Changes in V4:
- removed redundant parentheses
- added reviewed-by from Volodymyr Babchuk
---
xen/arch/arm/gic.c | 2 +-
xen/arch/arm/include/asm/gic.h | 9 +++++++++
xen/arch/arm/irq.c | 2 +-
3 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index e80fe0ca24..9220eef6ea 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -111,7 +111,7 @@ static void gic_set_irq_priority(struct irq_desc *desc,
unsigned int priority)
void gic_route_irq_to_xen(struct irq_desc *desc, unsigned int priority)
{
ASSERT(priority <= 0xff); /* Only 8 bits of priority */
- ASSERT(desc->irq < gic_number_lines());/* Can't route interrupts that
don't exist */
+ ASSERT(gic_is_valid_line(desc->irq));/* Can't route interrupts that don't
exist */
As you are touching the line. It is over 80 characters. Can you move the
command in a separate line?
ASSERT(test_bit(_IRQ_DISABLED, &desc->status));
ASSERT(spin_is_locked(&desc->lock));
Cheers,
--
Julien Grall