Messages in this thread | | | Date | Mon, 18 Jul 2022 15:35:47 +0800 | Subject | Re: [PATCH] drivers: irqchip: Allocate alignment addr by ITS_BASER.Page_size | From | wangwudi <> |
| |
Hi Marc,
在 2022/7/16 17:30, Marc Zyngier 写道: > On Sat, 16 Jul 2022 08:05:36 +0100, > wangwudi <wangwudi@hisilicon.com> wrote: >> >> The description of the ITS_BASER.Physical_Address field in the ARM GIC spec is as >> follows: >> "The address must be aligned to the size specified in the Page Size field." >> The Page_Size field in ITS_BASER might be RO. >> >> Currently, the address is aligned based on the system page_size, not the HW >> Page_Size field. In some case, this is in contradiction with the spec. >> >> For example: >> ITS_BASER.Page_Size indicate 16K, and kernel page size is 4K. >> If HW need 4K-size memory, the driver may alloc a 4K aligned address. >> This has been proven in hardware. > > Ah, interesting bug. Thanks for bringing this up. Can you describe how > this occurs? I suspect you are using indirect tables. >
Sure. In the system, kernel page size is 4K, and ITS_BASER.Page_Size is 16K.
As you suspected, HW used indirect VPE table and indication supports a small number of vpeids, like 2-bits vpeid. So that HW requires only less than 4K- size memory, and 16K aligned base address. But driver alloctes 4K aligend base address.
>> >> Cc: Thomas Gleixner <tglx@linutronix.de> >> Cc: Marc Zyngier <maz@kernel.org> >> Signed-off-by: wangwudi <wangwudi@hisilicon.com> >> --- >> drivers/irqchip/irq-gic-v3-its.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c >> index 5ff09de6c48f..0e25e887d45c 100644 >> --- a/drivers/irqchip/irq-gic-v3-its.c >> +++ b/drivers/irqchip/irq-gic-v3-its.c >> @@ -2310,6 +2310,9 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser, >> order = get_order(GITS_BASER_PAGES_MAX * psz); >> } >> >> + if ((psz > PAGE_SIZE) && (PAGE_ORDER_TO_SIZE(order) < psz)) { >> + order = get_order(psz); >> + } >> page = alloc_pages_node(its->numa_node, GFP_KERNEL | __GFP_ZERO, order); >> if (!page) >> return -ENOMEM; > > However, I don't see how you end-up with the incorrect value here. > > * No indirect table: > alloc_its_tables(): > order = get_order(baser->psz); > > * Indirect tables: > alloc_its_tables(): > order = get_order(baser->psz); > its_parse_indirect_baser(): > new_order = *order; > new_order = max_t(u32, get_order(esz << ids), new_order); > > So in both cases, we should end-up with order >= get_order(psz). Yes, totally agree.
> > Clearly, I'm missing a path, but your commit message doesn't make it > obvious. Can you please enlighten me? > My commit is based on the premise: "alloc_pages_node gives a size-aligend address". For example, if HW apply for 4K-size memory, then allocated address is 4K aligned.
> Thanks, > > M. >
| |