linux-kernel - Re: [PATCH] irqchip/gicv3-its: Disable the ITS before initializing it.

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] [day] [month] [year] [list]

Date:   Fri, 19 Aug 2016 09:43:13 +0100
From:   Marc Zyngier <marc.zyngier@....com>
To:     David Daney <ddaney.cavm@...il.com>, linux-kernel@...r.kernel.org,
        Thomas Gleixner <tglx@...utronix.de>,
        Jason Cooper <jason@...edaemon.net>
Cc:     Robert Richter <rric@...nel.org>,
        linux-arm-kernel@...ts.infradead.org,
        David Daney <david.daney@...ium.com>
Subject: Re: [PATCH] irqchip/gicv3-its: Disable the ITS before initializing
 it.

On 18/08/16 23:41, David Daney wrote:
> From: David Daney <david.daney@...ium.com>
> 
> When starting a kexec/kdump kernel, the GIC ITS will already have been
> enabled.  According to the ARM Generic Interrupt Controller
> Architecture Specification (GIC architecture Version 3.0 and version
> 4.0), writing to GITS_BASER<n> or GITS_CBASER is "UNPREDICTABLE" when
> the ITS is enabled.  On Cavium Thunder systems, this prevents the ITS
> from being initializing in the kexec/kdump kernel, resulting in
> failure to register/enable interrupts for all devices.
> 
> The fix is to disable the ITS if it is not already in the disabled
> state.  This allows the ITS to be properly initialized and then
> re-enabled in the kexec/kdump kernel.
> 
> Signed-off-by: David Daney <david.daney@...ium.com>
> ---
>  drivers/irqchip/irq-gic-v3-its.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 7ceaba8..36b9c28 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -1545,7 +1545,12 @@ static int its_force_quiescent(void __iomem *base)
>  	u32 val;
>  
>  	val = readl_relaxed(base + GITS_CTLR);
> -	if (val & GITS_CTLR_QUIESCENT)
> +	/*
> +	 * GIC architecture specification requires the ITS to be both
> +	 * disabled and quiescent for writes to GITS_BASER<n> or
> +	 * GITS_CBASER to not have UNPREDICTABLE results.
> +	 */
> +	if ((val & GITS_CTLR_QUIESCENT) && !(val & GITS_CTLR_ENABLE))
>  		return 0;
>  
>  	/* Disable the generation of all interrupts to this ITS */
> 

Acked-by: Marc Zyngier <marc.zyngier@....com>

I'll queue it for the next batch of fixes that I plan to send to tglx on
Monday.

Still, there is the question of how safe it is to do a kexec on a system
that has an ITS that can modify memory behind our back, since we have no
way to tell an irqchip to disable itself altogether (not a device and
all that mess).

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...