| Message ID | 1478293494-20640-2-git-send-email-sstabellini@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Stefano, On 04/11/16 21:04, Stefano Stabellini wrote: > Commit 21550029f709072aacf3b90edd574e7d3021b400 removed the > PLATFORM_QUIRK_GIC_64K_STRIDE quirk and introduced a way to > automatically detect that the two GICC pages have a 64K stride. > > However the heuristic requires that the device tree for the platform > reports a GICC size == 128K, which is not the case for some versions of > XGene. > > Fix the issue by partially reverting > 21550029f709072aacf3b90edd574e7d3021b400: > > - reintroduce PLATFORM_QUIRK_GIC_64K_STRIDE for XGene > - force csize and vsize to SZ_128K if csize is initially 4K and if > PLATFORM_QUIRK_GIC_64K_STRIDE > > Also add a warning in case GICC is SZ_128K but not aliased. > > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org> > > --- > > Changes in v2: > - only set csize to SZ_128K if it is initially SZ_4K > - set vsize to match > - add warning if gicc is SZ_128K and not aliased > --- > xen/arch/arm/gic-v2.c | 10 ++++++++-- > xen/arch/arm/platforms/xgene-storm.c | 6 ++++++ > xen/include/asm-arm/platform.h | 6 ++++++ > 3 files changed, 20 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c > index 9bd9d0b..fd2f3b4 100644 > --- a/xen/arch/arm/gic-v2.c > +++ b/xen/arch/arm/gic-v2.c > @@ -965,7 +965,10 @@ static void __init gicv2_dt_init(void) > printk(XENLOG_WARNING "GICv2: WARNING: " > "The GICC size is too small: %#"PRIx64" expected %#x\n", > csize, SZ_8K); > - csize = SZ_8K; > + if ( platform_has_quirk(PLATFORM_QUIRK_GIC_64K_STRIDE) ) > + vsize = csize = SZ_128K; The function does not care of the vsize and does not update it when the size is wrong (see the else you add). So I would stay consistent and drop it. Also, I would add a warning letting the user know that the quirk has been used. > + else > + csize = SZ_8K; > } > > /* > @@ -1189,7 +1192,10 @@ static int __init gicv2_init(void) > printk(XENLOG_WARNING > "GICv2: Adjusting CPU interface base to %#"PRIx64"\n", > cbase + aliased_offset); > - } > + } else if ( csize == SZ_128K ) > + printk(XENLOG_WARNING > + "GICv2: GICC size=%lu but not aliased\n", > + csize); > > gicv2.map_hbase = ioremap_nocache(hbase, PAGE_SIZE); > if ( !gicv2.map_hbase ) > diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c > index 686b19b..c795a95 100644 > --- a/xen/arch/arm/platforms/xgene-storm.c > +++ b/xen/arch/arm/platforms/xgene-storm.c > @@ -67,6 +67,11 @@ static void __init xgene_check_pirq_eoi(void) > "Please upgrade your firmware to the latest version"); > } > > +static uint32_t xgene_storm_quirks(void) > +{ > + return PLATFORM_QUIRK_GIC_64K_STRIDE; > +} > + > static void xgene_storm_reset(void) > { > void __iomem *addr; > @@ -116,6 +121,7 @@ PLATFORM_START(xgene_storm, "APM X-GENE STORM") > .compatible = xgene_storm_dt_compat, > .init = xgene_storm_init, > .reset = xgene_storm_reset, > + .quirks = xgene_storm_quirks, > PLATFORM_END > > /* > diff --git a/xen/include/asm-arm/platform.h b/xen/include/asm-arm/platform.h > index c6e5010..2ea9d61 100644 > --- a/xen/include/asm-arm/platform.h > +++ b/xen/include/asm-arm/platform.h > @@ -39,6 +39,12 @@ struct platform_desc { > const struct dt_device_match *blacklist_dev; > }; > > +/* > + * Quirk for platforms where the 4K GIC register ranges are placed at > + * 64K stride. I believe this comment needs to be updated as the quirk is only used when the device-tree reports 4K. > + */ > +#define PLATFORM_QUIRK_GIC_64K_STRIDE (1 << 0) > + > void __init platform_init(void); > int __init platform_init_time(void); > int __init platform_specific_mapping(struct domain *d); > Regards,
On Mon, 7 Nov 2016, Julien Grall wrote: > Hi Stefano, > > On 04/11/16 21:04, Stefano Stabellini wrote: > > Commit 21550029f709072aacf3b90edd574e7d3021b400 removed the > > PLATFORM_QUIRK_GIC_64K_STRIDE quirk and introduced a way to > > automatically detect that the two GICC pages have a 64K stride. > > > > However the heuristic requires that the device tree for the platform > > reports a GICC size == 128K, which is not the case for some versions of > > XGene. > > > > Fix the issue by partially reverting > > 21550029f709072aacf3b90edd574e7d3021b400: > > > > - reintroduce PLATFORM_QUIRK_GIC_64K_STRIDE for XGene > > - force csize and vsize to SZ_128K if csize is initially 4K and if > > PLATFORM_QUIRK_GIC_64K_STRIDE > > > > Also add a warning in case GICC is SZ_128K but not aliased. > > > > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org> > > > > --- > > > > Changes in v2: > > - only set csize to SZ_128K if it is initially SZ_4K > > - set vsize to match > > - add warning if gicc is SZ_128K and not aliased > > --- > > xen/arch/arm/gic-v2.c | 10 ++++++++-- > > xen/arch/arm/platforms/xgene-storm.c | 6 ++++++ > > xen/include/asm-arm/platform.h | 6 ++++++ > > 3 files changed, 20 insertions(+), 2 deletions(-) > > > > diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c > > index 9bd9d0b..fd2f3b4 100644 > > --- a/xen/arch/arm/gic-v2.c > > +++ b/xen/arch/arm/gic-v2.c > > @@ -965,7 +965,10 @@ static void __init gicv2_dt_init(void) > > printk(XENLOG_WARNING "GICv2: WARNING: " > > "The GICC size is too small: %#"PRIx64" expected %#x\n", > > csize, SZ_8K); > > - csize = SZ_8K; > > + if ( platform_has_quirk(PLATFORM_QUIRK_GIC_64K_STRIDE) ) > > + vsize = csize = SZ_128K; > > The function does not care of the vsize and does not update it when the size > is wrong (see the else you add). So I would stay consistent and drop it. Actually a couple of lines below in the code: if ( csize != vsize ) panic("GICv2: Sizes of GICC (%#"PRIpaddr") and GICV (%#"PRIpaddr") don't match\n", csize, vsize); I think we need to keep it. > Also, I would add a warning letting the user know that the quirk has been > used. Make sense. > > + else > > + csize = SZ_8K; > > } > > > > /* > > @@ -1189,7 +1192,10 @@ static int __init gicv2_init(void) > > printk(XENLOG_WARNING > > "GICv2: Adjusting CPU interface base to %#"PRIx64"\n", > > cbase + aliased_offset); > > - } > > + } else if ( csize == SZ_128K ) > > + printk(XENLOG_WARNING > > + "GICv2: GICC size=%lu but not aliased\n", > > + csize); > > > > gicv2.map_hbase = ioremap_nocache(hbase, PAGE_SIZE); > > if ( !gicv2.map_hbase ) > > diff --git a/xen/arch/arm/platforms/xgene-storm.c > > b/xen/arch/arm/platforms/xgene-storm.c > > index 686b19b..c795a95 100644 > > --- a/xen/arch/arm/platforms/xgene-storm.c > > +++ b/xen/arch/arm/platforms/xgene-storm.c > > @@ -67,6 +67,11 @@ static void __init xgene_check_pirq_eoi(void) > > "Please upgrade your firmware to the latest version"); > > } > > > > +static uint32_t xgene_storm_quirks(void) > > +{ > > + return PLATFORM_QUIRK_GIC_64K_STRIDE; > > +} > > + > > static void xgene_storm_reset(void) > > { > > void __iomem *addr; > > @@ -116,6 +121,7 @@ PLATFORM_START(xgene_storm, "APM X-GENE STORM") > > .compatible = xgene_storm_dt_compat, > > .init = xgene_storm_init, > > .reset = xgene_storm_reset, > > + .quirks = xgene_storm_quirks, > > PLATFORM_END > > > > /* > > diff --git a/xen/include/asm-arm/platform.h b/xen/include/asm-arm/platform.h > > index c6e5010..2ea9d61 100644 > > --- a/xen/include/asm-arm/platform.h > > +++ b/xen/include/asm-arm/platform.h > > @@ -39,6 +39,12 @@ struct platform_desc { > > const struct dt_device_match *blacklist_dev; > > }; > > > > +/* > > + * Quirk for platforms where the 4K GIC register ranges are placed at > > + * 64K stride. > > I believe this comment needs to be updated as the quirk is only used when the > device-tree reports 4K. > > > + */ > > +#define PLATFORM_QUIRK_GIC_64K_STRIDE (1 << 0) > > + > > void __init platform_init(void); > > int __init platform_init_time(void); > > int __init platform_specific_mapping(struct domain *d); > > > > Regards, > > -- > Julien Grall >
diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c index 9bd9d0b..fd2f3b4 100644 --- a/xen/arch/arm/gic-v2.c +++ b/xen/arch/arm/gic-v2.c @@ -965,7 +965,10 @@ static void __init gicv2_dt_init(void) printk(XENLOG_WARNING "GICv2: WARNING: " "The GICC size is too small: %#"PRIx64" expected %#x\n", csize, SZ_8K); - csize = SZ_8K; + if ( platform_has_quirk(PLATFORM_QUIRK_GIC_64K_STRIDE) ) + vsize = csize = SZ_128K; + else + csize = SZ_8K; } /* @@ -1189,7 +1192,10 @@ static int __init gicv2_init(void) printk(XENLOG_WARNING "GICv2: Adjusting CPU interface base to %#"PRIx64"\n", cbase + aliased_offset); - } + } else if ( csize == SZ_128K ) + printk(XENLOG_WARNING + "GICv2: GICC size=%lu but not aliased\n", + csize); gicv2.map_hbase = ioremap_nocache(hbase, PAGE_SIZE); if ( !gicv2.map_hbase ) diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c index 686b19b..c795a95 100644 --- a/xen/arch/arm/platforms/xgene-storm.c +++ b/xen/arch/arm/platforms/xgene-storm.c @@ -67,6 +67,11 @@ static void __init xgene_check_pirq_eoi(void) "Please upgrade your firmware to the latest version"); } +static uint32_t xgene_storm_quirks(void) +{ + return PLATFORM_QUIRK_GIC_64K_STRIDE; +} + static void xgene_storm_reset(void) { void __iomem *addr; @@ -116,6 +121,7 @@ PLATFORM_START(xgene_storm, "APM X-GENE STORM") .compatible = xgene_storm_dt_compat, .init = xgene_storm_init, .reset = xgene_storm_reset, + .quirks = xgene_storm_quirks, PLATFORM_END /* diff --git a/xen/include/asm-arm/platform.h b/xen/include/asm-arm/platform.h index c6e5010..2ea9d61 100644 --- a/xen/include/asm-arm/platform.h +++ b/xen/include/asm-arm/platform.h @@ -39,6 +39,12 @@ struct platform_desc { const struct dt_device_match *blacklist_dev; }; +/* + * Quirk for platforms where the 4K GIC register ranges are placed at + * 64K stride. + */ +#define PLATFORM_QUIRK_GIC_64K_STRIDE (1 << 0) + void __init platform_init(void); int __init platform_init_time(void); int __init platform_specific_mapping(struct domain *d);
Commit 21550029f709072aacf3b90edd574e7d3021b400 removed the PLATFORM_QUIRK_GIC_64K_STRIDE quirk and introduced a way to automatically detect that the two GICC pages have a 64K stride. However the heuristic requires that the device tree for the platform reports a GICC size == 128K, which is not the case for some versions of XGene. Fix the issue by partially reverting 21550029f709072aacf3b90edd574e7d3021b400: - reintroduce PLATFORM_QUIRK_GIC_64K_STRIDE for XGene - force csize and vsize to SZ_128K if csize is initially 4K and if PLATFORM_QUIRK_GIC_64K_STRIDE Also add a warning in case GICC is SZ_128K but not aliased. Signed-off-by: Stefano Stabellini <sstabellini@kernel.org> --- Changes in v2: - only set csize to SZ_128K if it is initially SZ_4K - set vsize to match - add warning if gicc is SZ_128K and not aliased --- xen/arch/arm/gic-v2.c | 10 ++++++++-- xen/arch/arm/platforms/xgene-storm.c | 6 ++++++ xen/include/asm-arm/platform.h | 6 ++++++ 3 files changed, 20 insertions(+), 2 deletions(-)