[PATCH 0/5] irqchip/gic-v4.1: Cleanup and fixes for GICv4.1

[PATCH 0/5] irqchip/gic-v4.1: Cleanup and fixes for GICv4.1

[Zenghui Yu]

Hi,

This series contains some cleanups, VPROPBASER field programming fix
and level2 vPE table allocation enhancement, collected while looking
through the GICv4.1 driver one more time.

Hope they will help, thanks!

Zenghui Yu (5):
  irqchip/gic-v4.1: Fix programming of GICR_VPROPBASER_4_1_SIZE
  irqchip/gic-v4.1: Set vpe_l1_base for all redistributors
  irqchip/gic-v4.1: Ensure L2 vPE table is allocated at RD level
  irqchip/gic-v4.1: Drop 'tmp' in inherit_vpe_l1_table_from_rd()
  irqchip/gic-v3-its: Remove superfluous WARN_ON

 drivers/irqchip/irq-gic-v3-its.c   | 80 +++++++++++++++++++++++++++---
 include/linux/irqchip/arm-gic-v3.h |  2 +-
 2 files changed, 75 insertions(+), 7 deletions(-)

-- 
2.19.1


_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH 1/5] irqchip/gic-v4.1: Fix programming of GICR_VPROPBASER_4_1_SIZE

[Zenghui Yu]

The Size field of GICv4.1 VPROPBASER register indicates number of
pages minus one and together Page_Size and Size control the vPEID
width. Let's respect this requirement of the architecture.

Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index e5a25d97f8db..992bc72cab6f 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -2531,7 +2531,7 @@ static int allocate_vpe_l1_table(void)
 		npg = 1;
 	}
 
-	val |= FIELD_PREP(GICR_VPROPBASER_4_1_SIZE, npg);
+	val |= FIELD_PREP(GICR_VPROPBASER_4_1_SIZE, npg - 1);
 
 	/* Right, that's the number of CPU pages we need for L1 */
 	np = DIV_ROUND_UP(npg * psz, PAGE_SIZE);
-- 
2.19.1


_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 1/5] irqchip/gic-v4.1: Fix programming of GICR_VPROPBASER_4_1_SIZE

[Marc Zyngier]

On 2020-02-04 09:09, Zenghui Yu wrote:
> The Size field of GICv4.1 VPROPBASER register indicates number of
> pages minus one and together Page_Size and Size control the vPEID
> width. Let's respect this requirement of the architecture.
> 
> Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
> ---
>  drivers/irqchip/irq-gic-v3-its.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its.c 
> b/drivers/irqchip/irq-gic-v3-its.c
> index e5a25d97f8db..992bc72cab6f 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -2531,7 +2531,7 @@ static int allocate_vpe_l1_table(void)
>  		npg = 1;
>  	}
> 
> -	val |= FIELD_PREP(GICR_VPROPBASER_4_1_SIZE, npg);
> +	val |= FIELD_PREP(GICR_VPROPBASER_4_1_SIZE, npg - 1);
> 
>  	/* Right, that's the number of CPU pages we need for L1 */
>  	np = DIV_ROUND_UP(npg * psz, PAGE_SIZE);

Indeed, nice catch.

         M.
-- 
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH 2/5] irqchip/gic-v4.1: Set vpe_l1_base for all redistributors

[Zenghui Yu]

Currently, we will not set vpe_l1_page for the current RD if we can
inherit the vPE configuration table from another RD (or ITS), which
results in an inconsistency between RDs within the same CommonLPIAff
group.

Let's rename it to vpe_l1_base to indicate the base address of the
vPE configuration table of this RD, and set it properly for *all*
v4.1 redistributors.

Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
---
 drivers/irqchip/irq-gic-v3-its.c   | 5 ++++-
 include/linux/irqchip/arm-gic-v3.h | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 992bc72cab6f..0f1fe56ce0af 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -2376,6 +2376,8 @@ static u64 inherit_vpe_l1_table_from_its(void)
 			continue;
 
 		/* We have a winner! */
+		gic_data_rdist()->vpe_l1_base = its->tables[2].base;
+
 		val  = GICR_VPROPBASER_4_1_VALID;
 		if (baser & GITS_BASER_INDIRECT)
 			val |= GICR_VPROPBASER_4_1_INDIRECT;
@@ -2432,6 +2434,7 @@ static u64 inherit_vpe_l1_table_from_rd(cpumask_t **mask)
 		val = gits_read_vpropbaser(base + SZ_128K + GICR_VPROPBASER);
 		val &= ~GICR_VPROPBASER_4_1_Z;
 
+		gic_data_rdist()->vpe_l1_base = gic_data_rdist_cpu(cpu)->vpe_l1_base;
 		*mask = gic_data_rdist_cpu(cpu)->vpe_table_mask;
 
 		return val;
@@ -2542,7 +2545,7 @@ static int allocate_vpe_l1_table(void)
 	if (!page)
 		return -ENOMEM;
 
-	gic_data_rdist()->vpe_l1_page = page;
+	gic_data_rdist()->vpe_l1_base = page_address(page);
 	pa = virt_to_phys(page_address(page));
 	WARN_ON(!IS_ALIGNED(pa, psz));
 
diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
index f0b8ca766e7d..83439bfb6c5b 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -652,10 +652,10 @@ struct rdists {
 	struct {
 		void __iomem	*rd_base;
 		struct page	*pend_page;
-		struct page	*vpe_l1_page;
 		phys_addr_t	phys_base;
 		bool		lpi_enabled;
 		cpumask_t	*vpe_table_mask;
+		void		*vpe_l1_base;
 	} __percpu		*rdist;
 	phys_addr_t		prop_table_pa;
 	void			*prop_table_va;
-- 
2.19.1


_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH RFC 3/5] irqchip/gic-v4.1: Ensure L2 vPE table is allocated at RD level

[Zenghui Yu]

In GICv4, we will ensure that level2 vPE table memory is allocated
for the specified vpe_id on all v4 ITS, in its_alloc_vpe_table().
This still works well for the typical GICv4.1 implementation, where
the new vPE table is shared between the ITSs and the RDs.

To make things explicit, introduce allocate_vpe_l1_table_entry() to
make sure that the L2 tables are allocated on all v4.1 RDs. We're
likely not need to allocate memory in it because the vPE table is
shared and (L2 table is) already allocated at ITS level, except
for the case where the ITS doesn't share anything (say SVPET == 0,
practically unlikely but architecturally allowed).

The implementation of allocate_vpe_l1_table_entry() is mostly
copied from its_alloc_table_entry().

Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 68 ++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 0f1fe56ce0af..c1d01454eec8 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -2443,6 +2443,64 @@ static u64 inherit_vpe_l1_table_from_rd(cpumask_t **mask)
 	return 0;
 }
 
+static int allocate_vpe_l1_table_entry(int cpu, u32 id)
+{
+	void __iomem *base = gic_data_rdist_cpu(cpu)->rd_base;
+	u64 val, gpsz, npg;
+	unsigned int psz, esz, idx;
+	struct page *page;
+	__le64 *table;
+
+	if (!gic_rdists->has_rvpeid)
+		return true;
+
+	val  = gits_read_vpropbaser(base + SZ_128K + GICR_VPROPBASER);
+
+	esz  = FIELD_GET(GICR_VPROPBASER_4_1_ENTRY_SIZE, val) + 1;
+	gpsz = FIELD_GET(GICR_VPROPBASER_4_1_PAGE_SIZE, val);
+	npg  = FIELD_GET(GICR_VPROPBASER_4_1_SIZE, val) + 1;
+
+	switch (gpsz) {
+	default:
+		WARN_ON(1);
+		/* fall through */
+	case GIC_PAGE_SIZE_4K:
+		psz = SZ_4K;
+		break;
+	case GIC_PAGE_SIZE_16K:
+		psz = SZ_16K;
+		break;
+	case GIC_PAGE_SIZE_64K:
+		psz = SZ_64K;
+		break;
+	}
+
+	/* Don't allow vpe_id that exceeds single, flat table limit */
+	if (!(val & GICR_VPROPBASER_4_1_INDIRECT))
+		return (id < (npg * psz / (esz * SZ_8)));
+
+	/* Compute 1st level table index & check if that exceeds table limit */
+	idx = id >> ilog2(psz / (esz * SZ_8));
+	if (idx >= (npg * psz / GITS_LVL1_ENTRY_SIZE))
+		return false;
+
+	table = gic_data_rdist_cpu(cpu)->vpe_l1_base;
+
+	/* Allocate memory for 2nd level table */
+	if (!table[idx]) {
+		page = alloc_pages(GFP_KERNEL | __GFP_ZERO, get_order(psz));
+		if (!page)
+			return false;
+
+		table[idx] = cpu_to_le64(page_to_phys(page) | GITS_BASER_VALID);
+
+		/* Ensure updated table contents are visible to RD hardware */
+		dsb(sy);
+	}
+
+	return true;
+}
+
 static int allocate_vpe_l1_table(void)
 {
 	void __iomem *vlpi_base = gic_data_rdist_vlpi_base();
@@ -2957,6 +3015,7 @@ static bool its_alloc_device_table(struct its_node *its, u32 dev_id)
 static bool its_alloc_vpe_table(u32 vpe_id)
 {
 	struct its_node *its;
+	int cpu;
 
 	/*
 	 * Make sure the L2 tables are allocated on *all* v4 ITSs. We
@@ -2979,6 +3038,15 @@ static bool its_alloc_vpe_table(u32 vpe_id)
 			return false;
 	}
 
+	/*
+	 * Make sure the L2 tables are allocated for all copies of
+	 * the L1 table on *all* v4.1 RDs.
+	 */
+	for_each_possible_cpu(cpu) {
+		if (!allocate_vpe_l1_table_entry(cpu, vpe_id))
+			return false;
+	}
+
 	return true;
 }
 
-- 
2.19.1


_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH RFC 3/5] irqchip/gic-v4.1: Ensure L2 vPE table is allocated at RD level

[Marc Zyngier]

Hi Zenghui,

Thanks for this. A few comments below.

On 2020-02-04 09:09, Zenghui Yu wrote:
> In GICv4, we will ensure that level2 vPE table memory is allocated
> for the specified vpe_id on all v4 ITS, in its_alloc_vpe_table().
> This still works well for the typical GICv4.1 implementation, where
> the new vPE table is shared between the ITSs and the RDs.
> 
> To make things explicit, introduce allocate_vpe_l1_table_entry() to
> make sure that the L2 tables are allocated on all v4.1 RDs. We're
> likely not need to allocate memory in it because the vPE table is
> shared and (L2 table is) already allocated at ITS level, except
> for the case where the ITS doesn't share anything (say SVPET == 0,
> practically unlikely but architecturally allowed).

Huh... Interesting. I definitely don't anticipate the case to pop up,
but you are right, this is architecturally allowed.

> The implementation of allocate_vpe_l1_table_entry() is mostly
> copied from its_alloc_table_entry().
> 
> Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
> ---
>  drivers/irqchip/irq-gic-v3-its.c | 68 ++++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its.c 
> b/drivers/irqchip/irq-gic-v3-its.c
> index 0f1fe56ce0af..c1d01454eec8 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -2443,6 +2443,64 @@ static u64 
> inherit_vpe_l1_table_from_rd(cpumask_t **mask)
>  	return 0;
>  }
> 
> +static int allocate_vpe_l1_table_entry(int cpu, u32 id)

Given that this actually allocates the L2 table, I'd rather have it 
called
something like allocate_vpe_l2_table() as the pendant of 
allocate_vpe_l1_table().

This should also properly return a bool rather then an int.

> +{
> +	void __iomem *base = gic_data_rdist_cpu(cpu)->rd_base;
> +	u64 val, gpsz, npg;
> +	unsigned int psz, esz, idx;
> +	struct page *page;
> +	__le64 *table;
> +
> +	if (!gic_rdists->has_rvpeid)
> +		return true;
> +
> +	val  = gits_read_vpropbaser(base + SZ_128K + GICR_VPROPBASER);
> +
> +	esz  = FIELD_GET(GICR_VPROPBASER_4_1_ENTRY_SIZE, val) + 1;
> +	gpsz = FIELD_GET(GICR_VPROPBASER_4_1_PAGE_SIZE, val);
> +	npg  = FIELD_GET(GICR_VPROPBASER_4_1_SIZE, val) + 1;
> +
> +	switch (gpsz) {
> +	default:
> +		WARN_ON(1);
> +		/* fall through */
> +	case GIC_PAGE_SIZE_4K:
> +		psz = SZ_4K;
> +		break;
> +	case GIC_PAGE_SIZE_16K:
> +		psz = SZ_16K;
> +		break;
> +	case GIC_PAGE_SIZE_64K:
> +		psz = SZ_64K;
> +		break;
> +	}
> +
> +	/* Don't allow vpe_id that exceeds single, flat table limit */
> +	if (!(val & GICR_VPROPBASER_4_1_INDIRECT))
> +		return (id < (npg * psz / (esz * SZ_8)));
> +
> +	/* Compute 1st level table index & check if that exceeds table limit 
> */
> +	idx = id >> ilog2(psz / (esz * SZ_8));
> +	if (idx >= (npg * psz / GITS_LVL1_ENTRY_SIZE))
> +		return false;
> +
> +	table = gic_data_rdist_cpu(cpu)->vpe_l1_base;
> +
> +	/* Allocate memory for 2nd level table */
> +	if (!table[idx]) {
> +		page = alloc_pages(GFP_KERNEL | __GFP_ZERO, get_order(psz));
> +		if (!page)
> +			return false;
> +
> +		table[idx] = cpu_to_le64(page_to_phys(page) | GITS_BASER_VALID);

I think we may need a cache flushing here in some circumstances. We 
certainly
have it in its_alloc_table_entry.

> +
> +		/* Ensure updated table contents are visible to RD hardware */
> +		dsb(sy);
> +	}
> +
> +	return true;
> +}
> +
>  static int allocate_vpe_l1_table(void)
>  {
>  	void __iomem *vlpi_base = gic_data_rdist_vlpi_base();
> @@ -2957,6 +3015,7 @@ static bool its_alloc_device_table(struct
> its_node *its, u32 dev_id)
>  static bool its_alloc_vpe_table(u32 vpe_id)
>  {
>  	struct its_node *its;
> +	int cpu;
> 
>  	/*
>  	 * Make sure the L2 tables are allocated on *all* v4 ITSs. We
> @@ -2979,6 +3038,15 @@ static bool its_alloc_vpe_table(u32 vpe_id)
>  			return false;
>  	}
> 
> +	/*
> +	 * Make sure the L2 tables are allocated for all copies of
> +	 * the L1 table on *all* v4.1 RDs.
> +	 */
> +	for_each_possible_cpu(cpu) {

not: You could predicate this on gic_rdists->has_rvpeid so that you 
don't
iterate pointlessly on non v4.1 HW.

> +		if (!allocate_vpe_l1_table_entry(cpu, vpe_id))
> +			return false;
> +	}
> +
>  	return true;
>  }

Otherwise, looks good to me.

         M.
-- 
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH 4/5] irqchip/gic-v4.1: Drop 'tmp' in inherit_vpe_l1_table_from_rd()

[Zenghui Yu]

The variable 'tmp' in inherit_vpe_l1_table_from_rd() is actually
not needed, drop it.

Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index c1d01454eec8..fc799ad7cd19 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -2415,14 +2415,12 @@ static u64 inherit_vpe_l1_table_from_rd(cpumask_t **mask)
 
 	for_each_possible_cpu(cpu) {
 		void __iomem *base = gic_data_rdist_cpu(cpu)->rd_base;
-		u32 tmp;
 
 		if (!base || cpu == smp_processor_id())
 			continue;
 
 		val = gic_read_typer(base + GICR_TYPER);
-		tmp = compute_common_aff(val);
-		if (tmp != aff)
+		if (aff != compute_common_aff(val))
 			continue;
 
 		/*
-- 
2.19.1


_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH 5/5] irqchip/gic-v3-its: Remove superfluous WARN_ON

[Zenghui Yu]

"ITS virtual pending table not cleaning" is already complained inside
its_clear_vpend_valid(), there's no need to trigger a WARN_ON again.

Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index fc799ad7cd19..718946fbd382 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -2849,7 +2849,6 @@ static void its_cpu_init_lpis(void)
 		 * corrupting memory.
 		 */
 		val = its_clear_vpend_valid(vlpi_base, 0, 0);
-		WARN_ON(val & GICR_VPENDBASER_Dirty);
 	}
 
 	if (allocate_vpe_l1_table()) {
-- 
2.19.1


_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 0/5] irqchip/gic-v4.1: Cleanup and fixes for GICv4.1

[Marc Zyngier]

Hi Zenghui,

On 2020-02-04 09:09, Zenghui Yu wrote:
> Hi,
> 
> This series contains some cleanups, VPROPBASER field programming fix
> and level2 vPE table allocation enhancement, collected while looking
> through the GICv4.1 driver one more time.
> 
> Hope they will help, thanks!
> 
> Zenghui Yu (5):
>   irqchip/gic-v4.1: Fix programming of GICR_VPROPBASER_4_1_SIZE
>   irqchip/gic-v4.1: Set vpe_l1_base for all redistributors
>   irqchip/gic-v4.1: Ensure L2 vPE table is allocated at RD level
>   irqchip/gic-v4.1: Drop 'tmp' in inherit_vpe_l1_table_from_rd()
>   irqchip/gic-v3-its: Remove superfluous WARN_ON
> 
>  drivers/irqchip/irq-gic-v3-its.c   | 80 +++++++++++++++++++++++++++---
>  include/linux/irqchip/arm-gic-v3.h |  2 +-
>  2 files changed, 75 insertions(+), 7 deletions(-)

Thanks a lot for this, much appreciated, I'm quite happy with the 
overall
state of the series. If you can just address the couple of nits I have 
on
patch #3, I'll then queue the series and send off to Thomas together 
with
the rest of the queued fixes.

Thanks,

          M.
-- 
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 0/5] irqchip/gic-v4.1: Cleanup and fixes for GICv4.1

[Zenghui Yu]

Hi Marc,

On 2020/2/5 20:46, Marc Zyngier wrote:
> Hi Zenghui,
> 
> On 2020-02-04 09:09, Zenghui Yu wrote:
>> Hi,
>>
>> This series contains some cleanups, VPROPBASER field programming fix
>> and level2 vPE table allocation enhancement, collected while looking
>> through the GICv4.1 driver one more time.
>>
>> Hope they will help, thanks!
>>
>> Zenghui Yu (5):
>>   irqchip/gic-v4.1: Fix programming of GICR_VPROPBASER_4_1_SIZE
>>   irqchip/gic-v4.1: Set vpe_l1_base for all redistributors
>>   irqchip/gic-v4.1: Ensure L2 vPE table is allocated at RD level
>>   irqchip/gic-v4.1: Drop 'tmp' in inherit_vpe_l1_table_from_rd()
>>   irqchip/gic-v3-its: Remove superfluous WARN_ON
>>
>>  drivers/irqchip/irq-gic-v3-its.c   | 80 +++++++++++++++++++++++++++---
>>  include/linux/irqchip/arm-gic-v3.h |  2 +-
>>  2 files changed, 75 insertions(+), 7 deletions(-)
> 
> Thanks a lot for this, much appreciated, I'm quite happy with the overall
> state of the series. If you can just address the couple of nits I have on
> patch #3, I'll then queue the series and send off to Thomas together with
> the rest of the queued fixes.

I will respin patch#3 with your suggestion and send v2 today.
Thanks for your review!


Zenghui

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm