Date:   Thu, 16 Dec 2021 19:03:15 +0000
From:   Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
To:     Marc Zyngier <maz@kernel.org>
Cc:     linux-kernel@vger.kernel.org, Thomas Gleixner <tglx@linutronix.de>,
        kernel-team@android.com, Jay Chen <jkchen@linux.alibaba.com>
Subject: Re: [PATCH] irqchip/gic-v4: Disable redistributors' view of the VPE
 table at boot time
Message-ID: <20211216190315.GA14220@lpieralisi>
References: <20211216144804.1578566-1-maz@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20211216144804.1578566-1-maz@kernel.org>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

On Thu, Dec 16, 2021 at 02:48:04PM +0000, Marc Zyngier wrote:
> Jay Chen reported that using a kdump kernel on a GICv4.1 system
> results in a RAS error being delivered when the secondary kernel
> configures the ITS's view of the new VPE table.
> 
> As it turns out, that's because each RD still has a pointer to
> the previous instance of the VPE table, and that particular
> implementation is very upset by seeing two bits of the HW that
> should point to the same table with different values.
> 
> To solve this, let's invalidate any reference that any RD has to
> the VPE table when discovering the RDs. The ITS can then be
> programmed as expected.

It makes sense. I believe there is an additional question though,
related to ITSes sharing the VPE table (SVPET) with RDs.

IIUC, all ITSes within a given affinity (that therefore are sharing the
VPE table) need to be quiesced before allocating a new VPE table.

Again, I am off the radar for a while and this patch makes sense on its
own, just raising the question since I was trying to understand whether
that can be an additional issue to solve on kexec; I will follow up
on this query.

It would be nice to know Alibaba's GIC HW topology if possible.

Thanks for putting together the fix and merging it.

Lorenzo

> Reported-by: Jay Chen <jkchen@linux.alibaba.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Link: https://lore.kernel.org/r/20211214064716.21407-1-jkchen@linux.alibaba.com
> ---
>  drivers/irqchip/irq-gic-v3.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index daec3309b014..86397522e786 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -920,6 +920,22 @@ static int __gic_update_rdist_properties(struct redist_region *region,
>  {
>  	u64 typer = gic_read_typer(ptr + GICR_TYPER);
>  
> +	/* Boot-time cleanip */
> +	if ((typer & GICR_TYPER_VLPIS) && (typer & GICR_TYPER_RVPEID)) {
> +		u64 val;
> +
> +		/* Deactivate any present vPE */
> +		val = gicr_read_vpendbaser(ptr + SZ_128K + GICR_VPENDBASER);
> +		if (val & GICR_VPENDBASER_Valid)
> +			gicr_write_vpendbaser(GICR_VPENDBASER_PendingLast,
> +					      ptr + SZ_128K + GICR_VPENDBASER);
> +
> +		/* Mark the VPE table as invalid */
> +		val = gicr_read_vpropbaser(ptr + SZ_128K + GICR_VPROPBASER);
> +		val &= ~GICR_VPROPBASER_4_1_VALID;
> +		gicr_write_vpropbaser(val, ptr + SZ_128K + GICR_VPROPBASER);
> +	}
> +
>  	gic_data.rdists.has_vlpis &= !!(typer & GICR_TYPER_VLPIS);
>  
>  	/* RVPEID implies some form of DirectLPI, no matter what the doc says... :-/ */
> -- 
> 2.30.2
>