| Message ID | 20230309161933.1336367-3-andrei.cherechesu@oss.nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix ARM Generic Timer interrupt parsing | expand |
Hi Andrei, On 09/03/2023 17:19, Andrei Cherechesu (OSS) wrote: > > > From: Andrei Cherechesu <andrei.cherechesu@nxp.com> > > Added support for parsing the ARM generic timer interrupts DT > node by the "interrupt-names" property, if it is available. > > If not available, the usual parsing based on the expected > IRQ order is performed. > > Also changed to treating returning 0 as an error case for the > platform_get_irq() calls, since it is not a valid PPI ID and > treating it as a valid case would only cause Xen to BUG() later, > during vgic_reserve_virq(). vgic_reserve_virq() itself does not call BUG(), so to be more precise, instead of "during vgic_reserve_virq()", how about: "when trying to reserve vIRQ being SGI." > > Added the "hyp-virt" PPI to the timer PPI list, even > though it's currently not in use. If the "hyp-virt" PPI is > not found, the hypervisor won't panic. > > Signed-off-by: Andrei Cherechesu <andrei.cherechesu@nxp.com> > --- > xen/arch/arm/include/asm/time.h | 3 ++- > xen/arch/arm/time.c | 27 +++++++++++++++++++++++---- > 2 files changed, 25 insertions(+), 5 deletions(-) > > diff --git a/xen/arch/arm/include/asm/time.h b/xen/arch/arm/include/asm/time.h > index 4b401c1110..49ad8c1a6d 100644 > --- a/xen/arch/arm/include/asm/time.h > +++ b/xen/arch/arm/include/asm/time.h > @@ -82,7 +82,8 @@ enum timer_ppi > TIMER_PHYS_NONSECURE_PPI = 1, > TIMER_VIRT_PPI = 2, > TIMER_HYP_PPI = 3, > - MAX_TIMER_PPI = 4, > + TIMER_HYP_VIRT_PPI = 4, > + MAX_TIMER_PPI = 5, > }; > > /* > diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c > index 433d7be909..868e03ecf6 100644 > --- a/xen/arch/arm/time.c > +++ b/xen/arch/arm/time.c > @@ -38,6 +38,14 @@ uint32_t __read_mostly timer_dt_clock_frequency; > > static unsigned int timer_irq[MAX_TIMER_PPI]; > > +static const char *timer_irq_names[MAX_TIMER_PPI] = { This wants to be marked as __initconst as it is const and only used in init code. So it would be: static const char *const timer_irq_names[... Furthermore, as the only user of this array is init_dt_xen_time(), you could move the definition to the function itself. ~Michal
diff --git a/xen/arch/arm/include/asm/time.h b/xen/arch/arm/include/asm/time.h index 4b401c1110..49ad8c1a6d 100644 --- a/xen/arch/arm/include/asm/time.h +++ b/xen/arch/arm/include/asm/time.h @@ -82,7 +82,8 @@ enum timer_ppi TIMER_PHYS_NONSECURE_PPI = 1, TIMER_VIRT_PPI = 2, TIMER_HYP_PPI = 3, - MAX_TIMER_PPI = 4, + TIMER_HYP_VIRT_PPI = 4, + MAX_TIMER_PPI = 5, }; /* diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c index 433d7be909..868e03ecf6 100644 --- a/xen/arch/arm/time.c +++ b/xen/arch/arm/time.c @@ -38,6 +38,14 @@ uint32_t __read_mostly timer_dt_clock_frequency; static unsigned int timer_irq[MAX_TIMER_PPI]; +static const char *timer_irq_names[MAX_TIMER_PPI] = { + [TIMER_PHYS_SECURE_PPI] = "sec-phys", + [TIMER_PHYS_NONSECURE_PPI] = "phys", + [TIMER_VIRT_PPI] = "virt", + [TIMER_HYP_PPI] = "hyp-phys", + [TIMER_HYP_VIRT_PPI] = "hyp-virt", +}; + unsigned int timer_get_irq(enum timer_ppi ppi) { ASSERT(ppi >= TIMER_PHYS_SECURE_PPI && ppi < MAX_TIMER_PPI); @@ -149,15 +157,26 @@ static void __init init_dt_xen_time(void) { int res; unsigned int i; + bool has_names; + + has_names = dt_property_read_bool(timer, "interrupt-names"); /* Retrieve all IRQs for the timer */ for ( i = TIMER_PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++ ) { - res = platform_get_irq(timer, i); - - if ( res < 0 ) + if ( has_names ) + res = platform_get_irq_byname(timer, timer_irq_names[i]); + else + res = platform_get_irq(timer, i); + + if ( res > 0 ) + timer_irq[i] = res; + /* + * Do not panic if "hyp-virt" PPI is not found, since it's not + * currently used. + */ + else if ( i != TIMER_HYP_VIRT_PPI ) panic("Timer: Unable to retrieve IRQ %u from the device tree\n", i); - timer_irq[i] = res; } }