linux-kernel - Re: [PATCH v2 06/25] arm64: arch_timer: implement support for interrupt-names

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87tuqdpije.wl-maz@kernel.org>
Date:   Mon, 15 Feb 2021 13:28:05 +0000
From:   Marc Zyngier <maz@...nel.org>
To:     Hector Martin <marcan@...can.st>
Cc:     linux-arm-kernel@...ts.infradead.org,
        Rob Herring <robh@...nel.org>, Arnd Bergmann <arnd@...nel.org>,
        Olof Johansson <olof@...om.net>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        Mark Kettenis <mark.kettenis@...all.nl>,
        Tony Lindgren <tony@...mide.com>,
        Mohamed Mediouni <mohamed.mediouni@...amail.com>,
        Stan Skowronek <stan@...ellium.com>,
        Alexander Graf <graf@...zon.com>,
        Will Deacon <will@...nel.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Mark Rutland <mark.rutland@....com>,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 06/25] arm64: arch_timer: implement support for interrupt-names

On Mon, 15 Feb 2021 12:16:54 +0000,
Hector Martin <marcan@...can.st> wrote:
> 
> This allows the devicetree to correctly represent the available set of
> timers, which varies from device to device, without the need for fake
> dummy interrupts for unavailable slots.
> 
> Also add the hyp-virt timer/PPI, which is not currently used, but worth
> representing.
> 
> Signed-off-by: Hector Martin <marcan@...can.st>
> ---
>  drivers/clocksource/arm_arch_timer.c | 25 +++++++++++++++++++++----
>  include/clocksource/arm_arch_timer.h |  1 +
>  2 files changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index d0177824c518..0e87b6d1ce97 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -63,6 +63,14 @@ struct arch_timer {
>  static u32 arch_timer_rate;
>  static int arch_timer_ppi[ARCH_TIMER_MAX_TIMER_PPI];
>  
> +static const char *arch_timer_ppi_names[ARCH_TIMER_MAX_TIMER_PPI] = {
> +	"phys-secure",
> +	"phys",
> +	"virt",
> +	"hyp-phys",
> +	"hyp-virt",

nit: I'd prefer it if the array was described as:

	[ARCH_TIMER_PHYS_SECURE_PPI] = "phys-secure",
	[...]

just to avoid that the two get out of sync.

> +};
> +
>  static struct clock_event_device __percpu *arch_timer_evt;
>  
>  static enum arch_timer_ppi_nr arch_timer_uses_ppi = ARCH_TIMER_VIRT_PPI;
> @@ -1280,17 +1288,26 @@ static void __init arch_timer_populate_kvm_info(void)
>  
>  static int __init arch_timer_of_init(struct device_node *np)
>  {
> -	int i, ret;
> +	int i, irq, ret;
>  	u32 rate;
> +	bool has_names;
>  
>  	if (arch_timers_present & ARCH_TIMER_TYPE_CP15) {
>  		pr_warn("multiple nodes in dt, skipping\n");
>  		return 0;
>  	}
>  
> -	arch_timers_present |= ARCH_TIMER_TYPE_CP15;

You probably didn't want to drop this line, did you?

> -	for (i = ARCH_TIMER_PHYS_SECURE_PPI; i < ARCH_TIMER_MAX_TIMER_PPI; i++)
> -		arch_timer_ppi[i] = irq_of_parse_and_map(np, i);
> +
> +	has_names = of_property_read_bool(np, "interrupt-names");
> +
> +	for (i = ARCH_TIMER_PHYS_SECURE_PPI; i < ARCH_TIMER_MAX_TIMER_PPI; i++) {
> +		if (has_names)
> +			irq = of_irq_get_byname(np, arch_timer_ppi_names[i]);
> +		else
> +			irq = of_irq_get(np, i);
> +		if (irq > 0)
> +			arch_timer_ppi[i] = irq;
> +	}
>  
>  	arch_timer_populate_kvm_info();
>  
> diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h
> index 1d68d5613dae..73c7139c866f 100644
> --- a/include/clocksource/arm_arch_timer.h
> +++ b/include/clocksource/arm_arch_timer.h
> @@ -32,6 +32,7 @@ enum arch_timer_ppi_nr {
>  	ARCH_TIMER_PHYS_NONSECURE_PPI,
>  	ARCH_TIMER_VIRT_PPI,
>  	ARCH_TIMER_HYP_PPI,
> +	ARCH_TIMER_HYP_VIRT_PPI,
>  	ARCH_TIMER_MAX_TIMER_PPI
>  };

Otherwise looks OK to me.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.