Re: [PATCH v3 2/2] arch/arm: time: Add support for parsing interrupts by names

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

HI Andrei,

> On 13 Mar 2023, at 14:08, Andrei Cherechesu (OSS) 
> <andrei.cherechesu@xxxxxxxxxxx> wrote:
> 
> From: Andrei Cherechesu <andrei.cherechesu@xxxxxxx>
> 
> 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 treated 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,
> 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@xxxxxxx>

Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>

Cheers
Bertrand

> ---
> xen/arch/arm/include/asm/time.h |  3 ++-
> xen/arch/arm/time.c             | 26 ++++++++++++++++++++++----
> 2 files changed, 24 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..0b482d7db3 100644
> --- a/xen/arch/arm/time.c
> +++ b/xen/arch/arm/time.c
> @@ -149,15 +149,33 @@ static void __init init_dt_xen_time(void)
> {
>     int res;
>     unsigned int i;
> +    bool has_names;
> +    static const char * const timer_irq_names[MAX_TIMER_PPI] __initconst = {
> +        [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",
> +    };
> +
> +    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;
>     }
> }
> 
> -- 
> 2.35.1
>