On Tue, 13 Apr 2021 07:40:10 +0100,
414777...@qq.com wrote:
>
> From: Mengguang Peng <pengmenggu...@phytium.com.cn>
>
> - After ITS suspend, in the ATF(arm-trusted-firmware),
> gicv3_rdistif_init_restore() just restore GICR_CTLR.Enable_LPIs bit
> of boot cpu.
>
> - In its_cpu_init_lpis() of kernel, gic_data_rdist()->lpi_enable
> will block setting GICR_CTLR_ENABLE_LPIS bit of the other CPUs
> when ITS resume after suspend.
>
> Signed-off-by: Mengguang Peng <pengmenggu...@phytium.com.cn>
> ---
> drivers/irqchip/irq-gic-v3-its.c | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c
> b/drivers/irqchip/irq-gic-v3-its.c
> index ed46e60..8167397 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -4777,7 +4777,7 @@ static int its_save_disable(void)
> static void its_restore_enable(void)
> {
> struct its_node *its;
> - int ret;
> + int ret, cpu;
>
> raw_spin_lock(&its_lock);
> list_for_each_entry(its, &its_nodes, entry) {
> @@ -4831,6 +4831,22 @@ static void its_restore_enable(void)
> GITS_TYPER_HCC(gic_read_typer(base + GITS_TYPER)))
> its_cpu_init_collection(its);
> }
> +
> + /*
> + * Enable LPIs: firmware just restore GICR_CTLR.Enable_LPIs
> + * of boot cpu, the other CPUs also should be restore.
> + */
> + for_each_online_cpu(cpu) {
> + void __iomem *rbase = gic_data_rdist_cpu(cpu)->rd_base;
> + u32 val;
> +
> + /* Enable LPIs */
> + val = readl_relaxed(rbase + GICR_CTLR);
> + if (val)
> + continue;
> + val |= GICR_CTLR_ENABLE_LPIS;
> + writel_relaxed(val, rbase + GICR_CTLR);
> + }
> raw_spin_unlock(&its_lock);
> }
This looks completely flawed. If GICR_CTLR.Enable_LPIs is clear on
resume, how can you trust the rest of the bits in the register? How
can you trust *any* register in the redistributor?
And what makes you think it is valid or safe to blindly enable LPIs
without even checking whether they were enabled the first place?
I will reiterate my take on this: if the firmware messes with the RD
on suspend, please address the problem in the firmware so that the RDs
are correctly restored on each CPU.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
