* [PATCH v2] irqchip: gic-v3: Extend collection table @ 2023-06-07 9:45 wangwudi 2023-06-08 8:10 ` Marc Zyngier 0 siblings, 1 reply; 4+ messages in thread From: wangwudi @ 2023-06-07 9:45 UTC (permalink / raw) To: linux-kernel; +Cc: liaochang1, wangwudi, Thomas Gleixner, Marc Zyngier 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 <tglx@linutronix.de> Cc: Marc Zyngier <maz@kernel.org> Signed-off-by: wangwudi <wangwudi@hisilicon.com> --- 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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] irqchip: gic-v3: Extend collection table 2023-06-07 9:45 [PATCH v2] irqchip: gic-v3: Extend collection table wangwudi @ 2023-06-08 8:10 ` Marc Zyngier [not found] ` <82ea3d910d104fbb8df9b27585085895@hisilicon.com> 0 siblings, 1 reply; 4+ messages in thread From: Marc Zyngier @ 2023-06-08 8:10 UTC (permalink / raw) To: wangwudi; +Cc: linux-kernel, liaochang1, Thomas Gleixner On Wed, 07 Jun 2023 10:45:13 +0100, wangwudi <wangwudi@hisilicon.com> 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 <tglx@linutronix.de> > Cc: Marc Zyngier <maz@kernel.org> > Signed-off-by: wangwudi <wangwudi@hisilicon.com> > --- > > 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. ^ permalink raw reply [flat|nested] 4+ messages in thread
[parent not found: <82ea3d910d104fbb8df9b27585085895@hisilicon.com>]
* Re: [PATCH v2] irqchip: gic-v3: Extend collection table [not found] ` <82ea3d910d104fbb8df9b27585085895@hisilicon.com> @ 2023-06-09 10:02 ` wangwudi 2023-06-09 13:10 ` Marc Zyngier 0 siblings, 1 reply; 4+ messages in thread From: wangwudi @ 2023-06-09 10:02 UTC (permalink / raw) To: Marc Zyngier; +Cc: linux-kernel, liaochang (A), Thomas Gleixner Hi Marc, 在 2023/6/9 17:24, wangwudi 写道: > > > -----邮件原件----- > 发件人: Marc Zyngier [mailto:maz@kernel.org] > 发送时间: 2023年6月8日 16:10 > 收件人: wangwudi <wangwudi@hisilicon.com> > 抄送: linux-kernel@vger.kernel.org; liaochang (A) <liaochang1@huawei.com>; Thomas Gleixner <tglx@linutronix.de> > 主题: Re: [PATCH v2] irqchip: gic-v3: Extend collection table > > On Wed, 07 Jun 2023 10:45:13 +0100, > wangwudi <wangwudi@hisilicon.com> 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 <tglx@linutronix.de> >> Cc: Marc Zyngier <maz@kernel.org> >> Signed-off-by: wangwudi <wangwudi@hisilicon.com> >> --- >> >> 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. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] irqchip: gic-v3: Extend collection table 2023-06-09 10:02 ` wangwudi @ 2023-06-09 13:10 ` Marc Zyngier 0 siblings, 0 replies; 4+ messages in thread From: Marc Zyngier @ 2023-06-09 13:10 UTC (permalink / raw) To: wangwudi; +Cc: linux-kernel, liaochang (A), Thomas Gleixner On Fri, 09 Jun 2023 11:02:04 +0100, wangwudi <wangwudi@hisilicon.com> wrote: > > Hi Marc, > > 在 2023/6/9 17:24, wangwudi 写道: > > > > > > -----邮件原件----- > > 发件人: Marc Zyngier [mailto:maz@kernel.org] > > 发送时间: 2023年6月8日 16:10 > > 收件人: wangwudi <wangwudi@hisilicon.com> > > 抄送: linux-kernel@vger.kernel.org; liaochang (A) <liaochang1@huawei.com>; Thomas Gleixner <tglx@linutronix.de> > > 主题: Re: [PATCH v2] irqchip: gic-v3: Extend collection table > > > > On Wed, 07 Jun 2023 10:45:13 +0100, > > wangwudi <wangwudi@hisilicon.com> 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 <tglx@linutronix.de> > >> Cc: Marc Zyngier <maz@kernel.org> > >> Signed-off-by: wangwudi <wangwudi@hisilicon.com> > >> --- > >> > >> 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. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-06-09 13:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-07 9:45 [PATCH v2] irqchip: gic-v3: Extend collection table wangwudi
2023-06-08 8:10 ` Marc Zyngier
[not found] ` <82ea3d910d104fbb8df9b27585085895@hisilicon.com>
2023-06-09 10:02 ` wangwudi
2023-06-09 13:10 ` Marc Zyngier
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).