Re: [PATCH v2] irqchip/gicv3-its: Add workaround for hip09 ITS erratum 162100801

[Marc Zyngier <maz@kernel.org>]

On Tue, 12 Nov 2024 09:04:26 +0000,
Zhou Wang <wangzhou1@hisilicon.com> wrote:
> 
> When enabling GICv4.1 in hip09, VMAPP will fail to clear some caches
> during unmapping operation, which will cause some vSGIs lost.
> 
> To fix the issue, it needs to send VINVALL command after VMOVP.
> 
> Signed-off-by: Nianyao Tang <tangnianyao@huawei.com>
> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
> ---
>  Documentation/arch/arm64/silicon-errata.rst |  2 ++
>  arch/arm64/Kconfig                          | 10 +++++++
>  drivers/irqchip/irq-gic-v3-its.c            | 29 +++++++++++++++++++++
>  3 files changed, 41 insertions(+)
> 
> diff --git a/Documentation/arch/arm64/silicon-errata.rst b/Documentation/arch/arm64/silicon-errata.rst
> index 65bfab1b1861..77db10e944f0 100644
> --- a/Documentation/arch/arm64/silicon-errata.rst
> +++ b/Documentation/arch/arm64/silicon-errata.rst
> @@ -258,6 +258,8 @@ stable kernels.
>  | Hisilicon      | Hip{08,09,10,10C| #162001900      | N/A                         |
>  |                | ,11} SMMU PMCG  |                 |                             |
>  +----------------+-----------------+-----------------+-----------------------------+
> +| Hisilicon      | Hip09           | #162100801      | HISILICON_ERRATUM_162100801 |
> ++----------------+-----------------+-----------------+-----------------------------+
>  +----------------+-----------------+-----------------+-----------------------------+
>  | Qualcomm Tech. | Kryo/Falkor v1  | E1003           | QCOM_FALKOR_ERRATUM_1003    |
>  +----------------+-----------------+-----------------+-----------------------------+
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 70d7f4f20225..d41cf6bf1458 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1232,6 +1232,16 @@ config HISILICON_ERRATUM_161600802
>  
>  	  If unsure, say Y.
>  
> +config HISILICON_ERRATUM_162100801
> +	bool "Hip09 162100801 erratum support"
> +	default y
> +	help
> +	  When enabling GICv4.1 in hip09, VMAPP will fail to clear some caches
> +	  during unmapping operation, which will cause some vSGIs lost.
> +	  So fix it by sending VINVALL command after VMOVP.
> +
> +	  If unsure, say Y.
> +
>  config QCOM_FALKOR_ERRATUM_1003
>  	bool "Falkor E1003: Incorrect translation due to ASID change"
>  	default y
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 52f625e07658..e98d13d0adf9 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -44,6 +44,7 @@
>  #define ITS_FLAGS_WORKAROUND_CAVIUM_22375	(1ULL << 1)
>  #define ITS_FLAGS_WORKAROUND_CAVIUM_23144	(1ULL << 2)
>  #define ITS_FLAGS_FORCE_NON_SHAREABLE		(1ULL << 3)
> +#define ITS_FLAGS_WORKAROUND_HISILICON_162100801	(1ULL << 4)
>  
>  #define RD_LOCAL_LPI_ENABLED                    BIT(0)
>  #define RD_LOCAL_PENDTABLE_PREALLOCATED         BIT(1)
> @@ -3804,6 +3805,7 @@ static int its_vpe_set_affinity(struct irq_data *d,
>  	struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
>  	unsigned int from, cpu = nr_cpu_ids;
>  	struct cpumask *table_mask;
> +	struct its_node *its;
>  	unsigned long flags;
>  
>  	/*
> @@ -3866,6 +3868,17 @@ static int its_vpe_set_affinity(struct irq_data *d,
>  	vpe->col_idx = cpu;
>  
>  	its_send_vmovp(vpe);
> +
> +	/*
> +	 * Version of ITS is same in one system. As there is no cache in ITS,
> +	 * and only cache in related GICR should be clean, so one VINVALL is
> +	 * enough here.
> +	 */
> +	its = list_first_entry(&its_nodes, struct its_node, entry);
> +	if ((its->flags & ITS_FLAGS_WORKAROUND_HISILICON_162100801) &&
> +	    is_v4_1(its))
> +		its_send_vinvall(its, vpe);

Can this be done using the GICR_INVALLR register instead? I would
expect it to be a bit better performance wise, as it doesn't require
to take a global lock. Something like the hack below (untested).

Thanks,

	M.

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index fdec478ba5e7..d30d6ee727c1 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -62,6 +62,8 @@ static u32 lpi_id_bits;
 
 static u8 __ro_after_init lpi_prop_prio;
 
+static struct its_node *find_4_1_its(void);
+
 /*
  * Collection structure - just an ID, and a redistributor address to
  * ping. We use one per CPU as a bag of interrupts assigned to this
@@ -3797,6 +3799,22 @@ static void its_vpe_db_proxy_move(struct its_vpe *vpe, int from, int to)
 	raw_spin_unlock_irqrestore(&vpe_proxy.lock, flags);
 }
 
+static void its_vpe_4_1_invall_locked(int cpu, struct its_vpe *vpe)
+{
+	void __iomem *rdbase;
+	u64 val;
+
+	val  = GICR_INVALLR_V;
+	val |= FIELD_PREP(GICR_INVALLR_VPEID, vpe->vpe_id);
+
+	raw_spin_lock(&gic_data_rdist_cpu(cpu)->rd_lock);
+	rdbase = per_cpu_ptr(gic_rdists->rdist, cpu)->rd_base;
+	gic_write_lpir(val, rdbase + GICR_INVALLR);
+
+	wait_for_syncr(rdbase);
+	raw_spin_unlock(&gic_data_rdist_cpu(cpu)->rd_lock);
+}
+
 static int its_vpe_set_affinity(struct irq_data *d,
 				const struct cpumask *mask_val,
 				bool force)
@@ -3849,6 +3867,10 @@ static int its_vpe_set_affinity(struct irq_data *d,
 	vpe->col_idx = cpu;
 
 	its_send_vmovp(vpe);
+
+	if (find_4_1_its()->flags & ITS_FLAGS_WORKAROUND_HISILICON_162100801)
+		its_vpe_4_1_invall_locked(cpu, vpe);
+
 	its_vpe_db_proxy_move(vpe, from, cpu);
 
 out:
@@ -4156,22 +4178,12 @@ static void its_vpe_4_1_deschedule(struct its_vpe *vpe,
 
 static void its_vpe_4_1_invall(struct its_vpe *vpe)
 {
-	void __iomem *rdbase;
 	unsigned long flags;
-	u64 val;
 	int cpu;
 
-	val  = GICR_INVALLR_V;
-	val |= FIELD_PREP(GICR_INVALLR_VPEID, vpe->vpe_id);
-
 	/* Target the redistributor this vPE is currently known on */
 	cpu = vpe_to_cpuid_lock(vpe, &flags);
-	raw_spin_lock(&gic_data_rdist_cpu(cpu)->rd_lock);
-	rdbase = per_cpu_ptr(gic_rdists->rdist, cpu)->rd_base;
-	gic_write_lpir(val, rdbase + GICR_INVALLR);
-
-	wait_for_syncr(rdbase);
-	raw_spin_unlock(&gic_data_rdist_cpu(cpu)->rd_lock);
+	its_vpe_4_1_invall_locked(cpu, vpe);
 	vpe_to_cpuid_unlock(vpe, flags);
 }
 

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