Hi Zenghui, On 2/7/20 4:20 AM, Zenghui Yu wrote: > Hi Eric, > > On 2020/1/28 18:34, Eric Auger wrote: >> its_enable_defaults() is the top init function that allocates the >> command queue and all the requested tables (device, collection, >> lpi config and pending tables), enable LPIs at distributor level >> and ITS level. >> >> gicv3_enable_defaults must be called before. >> >> Signed-off-by: Eric Auger <eric.au...@redhat.com> >> >> --- >> >> v2 -> v3: >> - introduce its_setup_baser in this patch >> - squash "arm/arm64: ITS: Init the command queue" in this patch. >> --- >> lib/arm/asm/gic-v3-its.h | 8 ++++ >> lib/arm/gic-v3-its.c | 89 ++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 97 insertions(+) >> >> diff --git a/lib/arm/asm/gic-v3-its.h b/lib/arm/asm/gic-v3-its.h >> index 815c515..fe73c04 100644 >> --- a/lib/arm/asm/gic-v3-its.h >> +++ b/lib/arm/asm/gic-v3-its.h >> @@ -36,6 +36,8 @@ struct its_data { >> void *base; >> struct its_typer typer; >> struct its_baser baser[GITS_BASER_NR_REGS]; >> + struct its_cmd_block *cmd_base; >> + struct its_cmd_block *cmd_write; >> }; >> extern struct its_data its_data; >> @@ -88,10 +90,16 @@ extern struct its_data its_data; >> #define GITS_BASER_TYPE_DEVICE 1 >> #define GITS_BASER_TYPE_COLLECTION 4 >> + >> +struct its_cmd_block { >> + u64 raw_cmd[4]; >> +}; >> + >> extern void its_parse_typer(void); >> extern void its_init(void); >> extern int its_parse_baser(int i, struct its_baser *baser); >> extern struct its_baser *its_lookup_baser(int type); >> +extern void its_enable_defaults(void); >> #else /* __arm__ */ >> diff --git a/lib/arm/gic-v3-its.c b/lib/arm/gic-v3-its.c >> index 2c0ce13..d1e7e52 100644 >> --- a/lib/arm/gic-v3-its.c >> +++ b/lib/arm/gic-v3-its.c >> @@ -86,3 +86,92 @@ void its_init(void) >> its_parse_baser(i, &its_data.baser[i]); >> } >> +static void its_setup_baser(int i, struct its_baser *baser) >> +{ >> + unsigned long n = (baser->nr_pages * baser->psz) >> PAGE_SHIFT; >> + unsigned long order = is_power_of_2(n) ? fls(n) : fls(n) + 1; >> + u64 val; >> + >> + baser->table_addr = (u64)virt_to_phys(alloc_pages(order)); >> + >> + val = ((u64)baser->table_addr | >> + ((u64)baser->type << GITS_BASER_TYPE_SHIFT) | >> + ((u64)(baser->esz - 1) << GITS_BASER_ENTRY_SIZE_SHIFT) | >> + ((baser->nr_pages - 1) << GITS_BASER_PAGES_SHIFT) | >> + (u64)baser->indirect << 62 | > > I haven't seen the 'nr_pages' and 'indirect' are programmed anywhere > except in its_parse_baser(). It looks like they're treated as RO (but > they shouldn't) and I now don't think it makes sense to parse them in > its_parse_baser(), in patch#5.
First of all please forgive me for the delay. I agree with you on nr_pages. However indirect also indicates the BASER capability to support or not 2 level tables. So I think it makes sense to read it on init. > >> + (u64)baser->valid << 63); >> + >> + switch (baser->psz) { >> + case SZ_4K: >> + val |= GITS_BASER_PAGE_SIZE_4K; >> + break; >> + case SZ_16K: >> + val |= GITS_BASER_PAGE_SIZE_16K; >> + break; >> + case SZ_64K: >> + val |= GITS_BASER_PAGE_SIZE_64K; >> + break; >> + } >> + >> + writeq(val, gicv3_its_base() + GITS_BASER + i * 8); >> +} >> + >> +/** >> + * init_cmd_queue: Allocate the command queue and initialize >> + * CBASER, CREADR, CWRITER > > no 'CREADR'. OK Thanks Eric > > > Thanks, > Zenghui > _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm