Only single level table is supported to the collection table, and only
one page is allocated.
Extend collection table to support more CPUs:
1. Recalculate the page number of collection table based on the number of
CPUs.
2. Add 2 level tables to collection table.
3. Add GITS_TYPER_CIDBITS macros.
It is noticed in an internal simulation research:
- the page_size of collection table is 4 KB
- the entry_size of collection table is 16 Byte
- with 512 CPUs
Cc: Thomas Gleixner <[email protected]>
Cc: Marc Zyngier <[email protected]>
Signed-off-by: wangwudi <[email protected]>
---
ChangeLog:
v1-->v2:
1. Support 2 level table
2. Rewrite the commit log
drivers/irqchip/irq-gic-v3-its.c | 62 ++++++++++++++++++++++++++++++--------
include/linux/irqchip/arm-gic-v3.h | 3 ++
2 files changed, 53 insertions(+), 12 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 0ec2b1e1df75..573ef26ad449 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -126,6 +126,7 @@ struct its_node {
#define is_v4(its) (!!((its)->typer & GITS_TYPER_VLPIS))
#define is_v4_1(its) (!!((its)->typer & GITS_TYPER_VMAPP))
#define device_ids(its) (FIELD_GET(GITS_TYPER_DEVBITS, (its)->typer) + 1)
+#define collection_ids(its) (FIELD_GET(GITS_TYPER_CIDBITS, (its)->typer) + 1)
#define ITS_ITT_ALIGN SZ_256
@@ -2626,6 +2627,10 @@ static int its_alloc_tables(struct its_node *its)
indirect = its_parse_indirect_baser(its, baser, &order,
ITS_MAX_VPEID_BITS);
break;
+ case GITS_BASER_TYPE_COLLECTION:
+ indirect = its_parse_indirect_baser(its, baser, &order,
+ order_base_2(num_possible_cpus()));
+ break;
}
err = its_setup_baser(its, baser, cache, shr, order, indirect);
@@ -3230,18 +3235,6 @@ static void its_cpu_init_collection(struct its_node *its)
its_send_invall(its, &its->collections[cpu]);
}
-static void its_cpu_init_collections(void)
-{
- struct its_node *its;
-
- raw_spin_lock(&its_lock);
-
- list_for_each_entry(its, &its_nodes, entry)
- its_cpu_init_collection(its);
-
- raw_spin_unlock(&its_lock);
-}
-
static struct its_device *its_find_device(struct its_node *its, u32 dev_id)
{
struct its_device *its_dev = NULL, *tmp;
@@ -3316,6 +3309,51 @@ static bool its_alloc_table_entry(struct its_node *its,
return true;
}
+static bool its_alloc_collection_table(struct its_node *its, struct its_baser *baser)
+{
+ int cpu = smp_processor_id();
+ int cpu_ids = 16;
+
+ if (its->typer & GITS_TYPER_CIL)
+ cpu_ids = collection_ids(its);
+
+ if (!(ilog2(cpu) < cpu_ids)) {
+ pr_warn("ITS: CPU%d out of Collection ID range for %dbits", cpu, cpu_ids);
+ return false;
+ }
+
+ if (!its_alloc_table_entry(its, baser, cpu)) {
+ pr_warn("ITS: CPU%d failed to allocate collection l2 table", cpu);
+ return false;
+ }
+
+ return true;
+}
+
+static bool its_cpu_init_collections(void)
+{
+ struct its_node *its;
+ struct its_baser *baser;
+
+ raw_spin_lock(&its_lock);
+
+ list_for_each_entry(its, &its_nodes, entry) {
+ baser = its_get_baser(its, GITS_BASER_TYPE_COLLECTION);
+ if (!baser) {
+ raw_spin_unlock(&its_lock);
+ return false;
+ }
+
+ if (!its_alloc_collection_table(its, baser)) {
+ raw_spin_unlock(&its_lock);
+ return false;
+ }
+ its_cpu_init_collection(its);
+ }
+ raw_spin_unlock(&its_lock);
+ return true;
+}
+
static bool its_alloc_device_table(struct its_node *its, u32 dev_id)
{
struct its_baser *baser;
diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
index 728691365464..35e83da8961f 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -400,6 +400,9 @@
#define GITS_TYPER_PTA (1UL << 19)
#define GITS_TYPER_HCC_SHIFT 24
#define GITS_TYPER_HCC(r) (((r) >> GITS_TYPER_HCC_SHIFT) & 0xff)
+#define GITS_TYPER_CIDBITS_SHIFT 32
+#define GITS_TYPER_CIDBITS GENMASK_ULL(35, 32)
+#define GITS_TYPER_CIL (1ULL << 36)
#define GITS_TYPER_VMOVP (1ULL << 37)
#define GITS_TYPER_VMAPP (1ULL << 40)
#define GITS_TYPER_SVPET GENMASK_ULL(42, 41)
--
2.7.4
On Wed, 07 Jun 2023 10:45:13 +0100,
wangwudi <[email protected]> wrote:
>
> Only single level table is supported to the collection table, and only
> one page is allocated.
>
> Extend collection table to support more CPUs:
> 1. Recalculate the page number of collection table based on the number of
> CPUs.
> 2. Add 2 level tables to collection table.
> 3. Add GITS_TYPER_CIDBITS macros.
>
> It is noticed in an internal simulation research:
> - the page_size of collection table is 4 KB
> - the entry_size of collection table is 16 Byte
> - with 512 CPUs
>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Signed-off-by: wangwudi <[email protected]>
> ---
>
> ChangeLog:
> v1-->v2:
> 1. Support 2 level table
> 2. Rewrite the commit log
>
> drivers/irqchip/irq-gic-v3-its.c | 62 ++++++++++++++++++++++++++++++--------
> include/linux/irqchip/arm-gic-v3.h | 3 ++
> 2 files changed, 53 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 0ec2b1e1df75..573ef26ad449 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -126,6 +126,7 @@ struct its_node {
> #define is_v4(its) (!!((its)->typer & GITS_TYPER_VLPIS))
> #define is_v4_1(its) (!!((its)->typer & GITS_TYPER_VMAPP))
> #define device_ids(its) (FIELD_GET(GITS_TYPER_DEVBITS, (its)->typer) + 1)
> +#define collection_ids(its) (FIELD_GET(GITS_TYPER_CIDBITS, (its)->typer) + 1)
>
> #define ITS_ITT_ALIGN SZ_256
>
> @@ -2626,6 +2627,10 @@ static int its_alloc_tables(struct its_node *its)
> indirect = its_parse_indirect_baser(its, baser, &order,
> ITS_MAX_VPEID_BITS);
> break;
> + case GITS_BASER_TYPE_COLLECTION:
> + indirect = its_parse_indirect_baser(its, baser, &order,
> + order_base_2(num_possible_cpus()));
> + break;
Nice try, but no. See below.
> }
>
> err = its_setup_baser(its, baser, cache, shr, order, indirect);
> @@ -3230,18 +3235,6 @@ static void its_cpu_init_collection(struct its_node *its)
> its_send_invall(its, &its->collections[cpu]);
> }
>
> -static void its_cpu_init_collections(void)
> -{
> - struct its_node *its;
> -
> - raw_spin_lock(&its_lock);
> -
> - list_for_each_entry(its, &its_nodes, entry)
> - its_cpu_init_collection(its);
> -
> - raw_spin_unlock(&its_lock);
> -}
> -
> static struct its_device *its_find_device(struct its_node *its, u32 dev_id)
> {
> struct its_device *its_dev = NULL, *tmp;
> @@ -3316,6 +3309,51 @@ static bool its_alloc_table_entry(struct its_node *its,
> return true;
> }
>
> +static bool its_alloc_collection_table(struct its_node *its, struct its_baser *baser)
> +{
> + int cpu = smp_processor_id();
> + int cpu_ids = 16;
> +
> + if (its->typer & GITS_TYPER_CIL)
> + cpu_ids = collection_ids(its);
> +
> + if (!(ilog2(cpu) < cpu_ids)) {
> + pr_warn("ITS: CPU%d out of Collection ID range for %dbits", cpu, cpu_ids);
> + return false;
> + }
> +
> + if (!its_alloc_table_entry(its, baser, cpu)) {
> + pr_warn("ITS: CPU%d failed to allocate collection l2 table", cpu);
> + return false;
> + }
> +
> + return true;
> +}
> +
> +static bool its_cpu_init_collections(void)
> +{
> + struct its_node *its;
> + struct its_baser *baser;
> +
> + raw_spin_lock(&its_lock);
> +
> + list_for_each_entry(its, &its_nodes, entry) {
> + baser = its_get_baser(its, GITS_BASER_TYPE_COLLECTION);
> + if (!baser) {
> + raw_spin_unlock(&its_lock);
> + return false;
> + }
This looks wrong. ITSs that have a non-zero HCC field may not need
memory to back their collections at all, such as GIC500. There may not
even be a BASERn register holding the memory.
So this patch more or less *guarantees* to break most implementation
that are more than 5 year old.
M.
--
Without deviation from the norm, progress is not possible.
Hi Marc,
在 2023/6/9 17:24, wangwudi 写道:
>
>
> -----邮件原件-----
> 发件人: Marc Zyngier [mailto:[email protected]]
> 发送时间: 2023年6月8日 16:10
> 收件人: wangwudi <[email protected]>
> 抄送: [email protected]; liaochang (A) <[email protected]>; Thomas Gleixner <[email protected]>
> 主题: Re: [PATCH v2] irqchip: gic-v3: Extend collection table
>
> On Wed, 07 Jun 2023 10:45:13 +0100,
> wangwudi <[email protected]> wrote:
>>
>> Only single level table is supported to the collection table, and only
>> one page is allocated.
>>
>> Extend collection table to support more CPUs:
>> 1. Recalculate the page number of collection table based on the number
>> of CPUs.
>> 2. Add 2 level tables to collection table.
>> 3. Add GITS_TYPER_CIDBITS macros.
>>
>> It is noticed in an internal simulation research:
>> - the page_size of collection table is 4 KB
>> - the entry_size of collection table is 16 Byte
>> - with 512 CPUs
>>
>> Cc: Thomas Gleixner <[email protected]>
>> Cc: Marc Zyngier <[email protected]>
>> Signed-off-by: wangwudi <[email protected]>
>> ---
>>
>> ChangeLog:
>> v1-->v2:
>> 1. Support 2 level table
>> 2. Rewrite the commit log
>>
>> drivers/irqchip/irq-gic-v3-its.c | 62 ++++++++++++++++++++++++++++++--------
>> include/linux/irqchip/arm-gic-v3.h | 3 ++
>> 2 files changed, 53 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-gic-v3-its.c
>> b/drivers/irqchip/irq-gic-v3-its.c
>> index 0ec2b1e1df75..573ef26ad449 100644
>> --- a/drivers/irqchip/irq-gic-v3-its.c
>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>> @@ -126,6 +126,7 @@ struct its_node {
>> #define is_v4(its) (!!((its)->typer & GITS_TYPER_VLPIS))
>> #define is_v4_1(its) (!!((its)->typer & GITS_TYPER_VMAPP))
>> #define device_ids(its) (FIELD_GET(GITS_TYPER_DEVBITS, (its)->typer) + 1)
>> +#define collection_ids(its) (FIELD_GET(GITS_TYPER_CIDBITS, (its)->typer) + 1)
>>
>> #define ITS_ITT_ALIGN SZ_256
>>
>> @@ -2626,6 +2627,10 @@ static int its_alloc_tables(struct its_node *its)
>> indirect = its_parse_indirect_baser(its, baser, &order,
>> ITS_MAX_VPEID_BITS);
>> break;
>> + case GITS_BASER_TYPE_COLLECTION:
>> + indirect = its_parse_indirect_baser(its, baser, &order,
>> + order_base_2(num_possible_cpus()));
>> + break;
>
> Nice try, but no. See below.
>
>> }
>>
>> err = its_setup_baser(its, baser, cache, shr, order, indirect); @@
>> -3230,18 +3235,6 @@ static void its_cpu_init_collection(struct its_node *its)
>> its_send_invall(its, &its->collections[cpu]); }
>>
>> -static void its_cpu_init_collections(void) -{
>> - struct its_node *its;
>> -
>> - raw_spin_lock(&its_lock);
>> -
>> - list_for_each_entry(its, &its_nodes, entry)
>> - its_cpu_init_collection(its);
>> -
>> - raw_spin_unlock(&its_lock);
>> -}
>> -
>> static struct its_device *its_find_device(struct its_node *its, u32
>> dev_id) {
>> struct its_device *its_dev = NULL, *tmp; @@ -3316,6 +3309,51 @@
>> static bool its_alloc_table_entry(struct its_node *its,
>> return true;
>> }
>>
>> +static bool its_alloc_collection_table(struct its_node *its, struct
>> +its_baser *baser) {
>> + int cpu = smp_processor_id();
>> + int cpu_ids = 16;
>> +
>> + if (its->typer & GITS_TYPER_CIL)
>> + cpu_ids = collection_ids(its);
>> +
>> + if (!(ilog2(cpu) < cpu_ids)) {
>> + pr_warn("ITS: CPU%d out of Collection ID range for %dbits", cpu, cpu_ids);
>> + return false;
>> + }
>> +
>> + if (!its_alloc_table_entry(its, baser, cpu)) {
>> + pr_warn("ITS: CPU%d failed to allocate collection l2 table", cpu);
>> + return false;
>> + }
>> +
>> + return true;
>> +}
>> +
>> +static bool its_cpu_init_collections(void) {
>> + struct its_node *its;
>> + struct its_baser *baser;
>> +
>> + raw_spin_lock(&its_lock);
>> +
>> + list_for_each_entry(its, &its_nodes, entry) {
>> + baser = its_get_baser(its, GITS_BASER_TYPE_COLLECTION);
>> + if (!baser) {
>> + raw_spin_unlock(&its_lock);
>> + return false;
>> + }
>
> This looks wrong. ITSs that have a non-zero HCC field may not need memory to back their collections at all, such as GIC500. There may not even be a BASERn register holding the memory.
>
> So this patch more or less *guarantees* to break most implementation that are more than 5 year old.
>
For the collection table, if the HCC field is not zero, neither l1-table nor
l2-table table is allocated. How do you think?
> M.
>
> --
> Without deviation from the norm, progress is not possible.
On Fri, 09 Jun 2023 11:02:04 +0100,
wangwudi <[email protected]> wrote:
>
> Hi Marc,
>
> 在 2023/6/9 17:24, wangwudi 写道:
> >
> >
> > -----邮件原件-----
> > 发件人: Marc Zyngier [mailto:[email protected]]
> > 发送时间: 2023年6月8日 16:10
> > 收件人: wangwudi <[email protected]>
> > 抄送: [email protected]; liaochang (A) <[email protected]>; Thomas Gleixner <[email protected]>
> > 主题: Re: [PATCH v2] irqchip: gic-v3: Extend collection table
> >
> > On Wed, 07 Jun 2023 10:45:13 +0100,
> > wangwudi <[email protected]> wrote:
> >>
> >> Only single level table is supported to the collection table, and only
> >> one page is allocated.
> >>
> >> Extend collection table to support more CPUs:
> >> 1. Recalculate the page number of collection table based on the number
> >> of CPUs.
> >> 2. Add 2 level tables to collection table.
> >> 3. Add GITS_TYPER_CIDBITS macros.
> >>
> >> It is noticed in an internal simulation research:
> >> - the page_size of collection table is 4 KB
> >> - the entry_size of collection table is 16 Byte
> >> - with 512 CPUs
> >>
> >> Cc: Thomas Gleixner <[email protected]>
> >> Cc: Marc Zyngier <[email protected]>
> >> Signed-off-by: wangwudi <[email protected]>
> >> ---
> >>
> >> ChangeLog:
> >> v1-->v2:
> >> 1. Support 2 level table
> >> 2. Rewrite the commit log
> >>
> >> drivers/irqchip/irq-gic-v3-its.c | 62 ++++++++++++++++++++++++++++++--------
> >> include/linux/irqchip/arm-gic-v3.h | 3 ++
> >> 2 files changed, 53 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/drivers/irqchip/irq-gic-v3-its.c
> >> b/drivers/irqchip/irq-gic-v3-its.c
> >> index 0ec2b1e1df75..573ef26ad449 100644
> >> --- a/drivers/irqchip/irq-gic-v3-its.c
> >> +++ b/drivers/irqchip/irq-gic-v3-its.c
> >> @@ -126,6 +126,7 @@ struct its_node {
> >> #define is_v4(its) (!!((its)->typer & GITS_TYPER_VLPIS))
> >> #define is_v4_1(its) (!!((its)->typer & GITS_TYPER_VMAPP))
> >> #define device_ids(its) (FIELD_GET(GITS_TYPER_DEVBITS, (its)->typer) + 1)
> >> +#define collection_ids(its) (FIELD_GET(GITS_TYPER_CIDBITS, (its)->typer) + 1)
> >>
> >> #define ITS_ITT_ALIGN SZ_256
> >>
> >> @@ -2626,6 +2627,10 @@ static int its_alloc_tables(struct its_node *its)
> >> indirect = its_parse_indirect_baser(its, baser, &order,
> >> ITS_MAX_VPEID_BITS);
> >> break;
> >> + case GITS_BASER_TYPE_COLLECTION:
> >> + indirect = its_parse_indirect_baser(its, baser, &order,
> >> + order_base_2(num_possible_cpus()));
> >> + break;
> >
> > Nice try, but no. See below.
> >
> >> }
> >>
> >> err = its_setup_baser(its, baser, cache, shr, order, indirect); @@
> >> -3230,18 +3235,6 @@ static void its_cpu_init_collection(struct its_node *its)
> >> its_send_invall(its, &its->collections[cpu]); }
> >>
> >> -static void its_cpu_init_collections(void) -{
> >> - struct its_node *its;
> >> -
> >> - raw_spin_lock(&its_lock);
> >> -
> >> - list_for_each_entry(its, &its_nodes, entry)
> >> - its_cpu_init_collection(its);
> >> -
> >> - raw_spin_unlock(&its_lock);
> >> -}
> >> -
> >> static struct its_device *its_find_device(struct its_node *its, u32
> >> dev_id) {
> >> struct its_device *its_dev = NULL, *tmp; @@ -3316,6 +3309,51 @@
> >> static bool its_alloc_table_entry(struct its_node *its,
> >> return true;
> >> }
> >>
> >> +static bool its_alloc_collection_table(struct its_node *its, struct
> >> +its_baser *baser) {
> >> + int cpu = smp_processor_id();
> >> + int cpu_ids = 16;
> >> +
> >> + if (its->typer & GITS_TYPER_CIL)
> >> + cpu_ids = collection_ids(its);
> >> +
> >> + if (!(ilog2(cpu) < cpu_ids)) {
> >> + pr_warn("ITS: CPU%d out of Collection ID range for %dbits", cpu, cpu_ids);
> >> + return false;
> >> + }
> >> +
> >> + if (!its_alloc_table_entry(its, baser, cpu)) {
> >> + pr_warn("ITS: CPU%d failed to allocate collection l2 table", cpu);
> >> + return false;
> >> + }
> >> +
> >> + return true;
> >> +}
> >> +
> >> +static bool its_cpu_init_collections(void) {
> >> + struct its_node *its;
> >> + struct its_baser *baser;
> >> +
> >> + raw_spin_lock(&its_lock);
> >> +
> >> + list_for_each_entry(its, &its_nodes, entry) {
> >> + baser = its_get_baser(its, GITS_BASER_TYPE_COLLECTION);
> >> + if (!baser) {
> >> + raw_spin_unlock(&its_lock);
> >> + return false;
> >> + }
> >
> > This looks wrong. ITSs that have a non-zero HCC field may not need
> > memory to back their collections at all, such as GIC500. There may
> > not even be a BASERn register holding the memory.
> >
> > So this patch more or less *guarantees* to break most
> > implementation that are more than 5 year old.
> >
>
> For the collection table, if the HCC field is not zero, neither
> l1-table nor l2-table table is allocated. How do you think?
What do I think? I've already said what I think, right under the 4
lines of code that break anything with a GIC500.
M.
--
Without deviation from the norm, progress is not possible.