| Message ID | 20180112212422.148625-5-dbasehore@chromium.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
[I remember asking you to copy Sudeep Hola on this. Please do so the next time around] On Fri, 12 Jan 2018 21:24:18 +0000, Derek Basehore wrote: > > Some platforms power off GIC logic in S3, so we need to save/restore S3 is a not a GIC concept, and is only vaguely mentioned in terms of the rk3399 silicon, if grep serves me right. Please expand on what state this is exactly. > state. This adds a DT-binding to save/restore the GICD/GICR/GITS > states using the new CPU_PM_SYSTEM_ENTER/EXIT CPU PM states. DT binding? I can't see any in this patch. > > Change-Id: I1fb2117296373fa67397fdd4a8960077b241462e It's been mentioned somewhere else in the thread: these tags have no purpose in the kernel. Please sanitise your patches before posting them. > Signed-off-by: Derek Basehore <dbasehore@chromium.org> > Signed-off-by: Brian Norris <briannorris@chromium.org> Who is the author of this patch? If that's a joined authorship, please use the Co-Developed-by: tag. > --- > drivers/irqchip/irq-gic-v3-its.c | 51 ++++++ > drivers/irqchip/irq-gic-v3.c | 333 +++++++++++++++++++++++++++++++++++-- > include/linux/irqchip/arm-gic-v3.h | 17 ++ > 3 files changed, 391 insertions(+), 10 deletions(-) > > diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c > index 06f025fd5726..5cb808e3d0bf 100644 > --- a/drivers/irqchip/irq-gic-v3-its.c > +++ b/drivers/irqchip/irq-gic-v3-its.c > @@ -85,6 +85,16 @@ struct its_baser { > > struct its_device; > > +/* > + * Saved ITS state - this is where saved state for the ITS is stored > + * when it's disabled during system suspend. > + */ > +struct its_ctx { > + u64 cbaser; > + u64 cwriter; Why do you need to save cwriter? Do you expect to perform a save/restore in the middle of a set of commands? I would really expect the system to be in a quiescent state, and the command queue to be reset to an empty state on resume. WHy isn't it so? > + u32 ctlr; > +}; > + > /* > * The ITS structure - contains most of the infrastructure, with the > * top-level MSI domain, the command queue, the collections, and the > @@ -101,6 +111,7 @@ struct its_node { > struct its_collection *collections; > struct fwnode_handle *fwnode_handle; > u64 (*get_msi_base)(struct its_device *its_dev); > + struct its_ctx its_ctx; > struct list_head its_device_list; > u64 flags; > unsigned long list_nr; > @@ -3042,6 +3053,46 @@ static void its_enable_quirks(struct its_node *its) > gic_enable_quirks(iidr, its_quirks, its); > } > > +void its_save_disable(void) > +{ > + struct its_node *its; > + > + spin_lock(&its_lock); > + list_for_each_entry(its, &its_nodes, entry) { > + struct its_ctx *ctx = &its->its_ctx; > + void __iomem *base = its->base; > + int i; > + > + ctx->ctlr = readl_relaxed(base + GITS_CTLR); > + its_force_quiescent(base); What if the ITS fails to become quiescent? > + ctx->cbaser = gits_read_cbaser(base + GITS_CBASER); > + ctx->cwriter = readq_relaxed(base + GITS_CWRITER); How about those systems that do not have a readq (32bit)? Please make sure this builds on 32bit too. > + for (i = 0; i < ARRAY_SIZE(its->tables); i++) > + its->tables[i].val = its_read_baser(its, &its->tables[i]); > + } > + spin_unlock(&its_lock); > +} > + > +void its_restore_enable(void) > +{ > + struct its_node *its; > + > + spin_lock(&its_lock); > + list_for_each_entry(its, &its_nodes, entry) { > + struct its_ctx *ctx = &its->its_ctx; > + void __iomem *base = its->base; > + int i; > + > + gits_write_cbaser(ctx->cbaser, base + GITS_CBASER); > + gits_write_cwriter(ctx->cwriter, base + GITS_CWRITER); > + for (i = 0; i < ARRAY_SIZE(its->tables); i++) > + its_write_baser(its, &its->tables[i], > + its->tables[i].val); > + writel_relaxed(ctx->ctlr, base + GITS_CTLR); > + } > + spin_unlock(&its_lock); > +} > + > static int its_init_domain(struct fwnode_handle *handle, struct its_node *its) > { > struct irq_domain *inner_domain; > diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c > index 9a7a15049903..95d37fb6f458 100644 > --- a/drivers/irqchip/irq-gic-v3.c > +++ b/drivers/irqchip/irq-gic-v3.c > @@ -47,6 +47,36 @@ struct redist_region { > bool single_redist; > }; > > +struct gic_dist_ctx { > + u64 *irouter; > + u32 *igroupr; > + u32 *isenabler; > + u32 *ispendr; > + u32 *isactiver; > + u32 *ipriorityr; > + u32 *icfgr; > + u32 *igrpmodr; > + u32 *nsacr; > + u32 ctlr; > +}; > + > +struct gic_redist_ctx { > + u64 propbaser; > + u64 pendbaser; > + u32 ipriorityr[8]; > + u32 ctlr; > + u32 igroupr0; > + u32 isenabler0; > + u32 ispendr0; > + u32 isactiver0; > + u32 icfgr0; > + u32 icfgr1; > + u32 igrpmodr0; > + u32 nsacr; > +}; > + > +#define GICD_NR_REGS(reg, nr_irq) ((nr_irq) >> GICD_## reg ##_SHIFT) > + > struct gic_chip_data { > struct fwnode_handle *fwnode; > void __iomem *dist_base; > @@ -58,6 +88,8 @@ struct gic_chip_data { > bool has_rss; > unsigned int irq_nr; > struct partition_desc *ppi_descs[16]; > + struct gic_dist_ctx *gicd_ctx; > + struct gic_redist_ctx *gicr_ctx; > }; > > static struct gic_chip_data gic_data __read_mostly; > @@ -133,13 +165,13 @@ static u64 __maybe_unused gic_read_iar(void) > } > #endif > > -static void gic_enable_redist(bool enable) > +static void gic_enable_redist_base(int cpu, bool enable) _base? What does _base mean here? Shouldn't it be something like gic_enable_redist_on_cpu(), or something along these lines? > { > void __iomem *rbase; > u32 count = 1000000; /* 1s! */ > u32 val; > > - rbase = gic_data_rdist_rd_base(); > + rbase = per_cpu_ptr(gic_data.rdists.rdist, cpu)->rd_base; > > val = readl_relaxed(rbase + GICR_WAKER); > if (enable) > @@ -167,6 +199,11 @@ static void gic_enable_redist(bool enable) > enable ? "wakeup" : "sleep"); > } > > +static void gic_enable_redist(bool enable) > +{ > + gic_enable_redist_base(smp_processor_id(), enable); > +} > + > /* > * Routines to disable, enable, EOI and route interrupts > */ > @@ -757,6 +794,155 @@ static int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val, > #define gic_smp_init() do { } while(0) > #endif > > +static void gic_redist_save(int cpu) > +{ > + struct gic_redist_ctx *ctx = &gic_data.gicr_ctx[cpu]; > + void __iomem *base = per_cpu_ptr(gic_data.rdists.rdist, cpu)->rd_base; > + > + gic_do_wait_for_rwp(base); > + ctx->ctlr = readl_relaxed(base + GICR_CTLR); > + ctx->propbaser = readq_relaxed(base + GICR_PROPBASER); Why is this a per-redistributor data? > + ctx->pendbaser = readq_relaxed(base + GICR_PENDBASER); Use the relevant accessors that will ensure proper compilation on 32bit. > + > + base += GICR_SGI_BASE_OFFSET; > + ctx->igroupr0 = readl_relaxed(base + GICR_IGROUPR0); > + ctx->isenabler0 = readl_relaxed(base + GICR_ISENABLER0); > + ctx->ispendr0 = readl_relaxed(base + GICR_ISPENDR0); > + ctx->isactiver0 = readl_relaxed(base + GICR_ISACTIVER0); > + __ioread32_copy(ctx->ipriorityr, base + GICR_IPRIORITYR0, 8); WHy d we need to save the priorities? Aren't they known to be fixed? > + ctx->icfgr0 = readl_relaxed(base + GICR_ICFGR0); > + ctx->icfgr1 = readl_relaxed(base + GICR_ICFGR0 + sizeof(u32)); > + ctx->igrpmodr0 = readl_relaxed(base + GICR_IGRPMODR0); What's the purpose of saving GICR_IGRPMODR0? > + ctx->nsacr = readl_relaxed(base + GICR_NSACR); What is the purpose of saving NSACR? > +} > + > +static void gic_dist_save(void) > +{ > + struct gic_dist_ctx *ctx = gic_data.gicd_ctx; > + void __iomem *base = gic_data.dist_base; > + int irq_nr = gic_data.irq_nr; > + > + __ioread64_copy(ctx->irouter, base + GICD_IROUTER, > + GICD_NR_REGS(IROUTER, irq_nr)); > + __ioread32_copy(ctx->igroupr, base + GICD_IGROUPR, > + GICD_NR_REGS(IGROUPR, irq_nr)); > + __ioread32_copy(ctx->isenabler, base + GICD_ISENABLER, > + GICD_NR_REGS(ISENABLER, irq_nr)); > + __ioread32_copy(ctx->ispendr, base + GICD_ISPENDR, > + GICD_NR_REGS(ISPENDR, irq_nr)); > + __ioread32_copy(ctx->isactiver, base + GICD_ISACTIVER, > + GICD_NR_REGS(ISACTIVER, irq_nr)); > + __ioread32_copy(ctx->icfgr, base + GICD_ICFGR, > + GICD_NR_REGS(ICFGR, irq_nr)); > + __ioread32_copy(ctx->igrpmodr, base + GICD_IGRPMODR, > + GICD_NR_REGS(IGRPMODR, irq_nr)); > + __ioread32_copy(ctx->nsacr, base + GICD_NSACR, > + GICD_NR_REGS(NSACR, irq_nr)); > + ctx->ctlr = readl_relaxed(base + GICD_CTLR); The GICv1/v2 driver already has code to that effect. Do we really need to reinvent the wheel? You just need to special case the few GICv3 specific registers. Also, this driver doesn't use the __ioread/__iowrite stuff. They hide important information (relaxed/non-relaxed), and have very imprecise semantics on 32bit systems when used with 64bit accessors. Please get rid of them and be aware of the 32bit limitations. > +} > + > +static int gic_suspend(void) > +{ > + void __iomem *rbase; > + int cpu; > + > + if (!gic_data.gicd_ctx || !gic_data.gicr_ctx) > + return 0; > + > + for_each_possible_cpu(cpu) { > + /* > + * The current processor will have the redistributor disabled in > + * the next step. > + */ > + if (cpu == smp_processor_id()) > + continue; > + > + /* Each redistributor must be disabled to save state. */ > + rbase = per_cpu_ptr(gic_data.rdists.rdist, cpu)->rd_base; > + if (!(readl_relaxed(rbase + GICR_WAKER) & > + GICR_WAKER_ProcessorSleep)) { > + pr_err("Redistributor for CPU: %d enabled \n", cpu); > + return -EPERM; > + } How can this happen? Is this just to be on the safe side? > + } > + > + /* Disable the redist in case it wasn't in CPU_PM_ENTER. */ > + gic_enable_redist(false); > + for_each_possible_cpu(cpu) > + gic_redist_save(cpu); > + > + its_save_disable(); Why do we need to tie up the core GIC and the ITS? Can't they have independent save/restore entry points? > + gic_dist_save(); > + > + return 0; > +} > + > +static void gic_rdist_restore(int cpu) > +{ > + struct gic_redist_ctx *ctx = &gic_data.gicr_ctx[cpu]; > + void __iomem *base = per_cpu_ptr(gic_data.rdists.rdist, cpu)->rd_base; > + void __iomem *sgi_base = base + GICR_SGI_BASE_OFFSET; > + > + writel_relaxed(ctx->ctlr & ~(GICR_CTLR_ENABLE_LPIS), base + GICR_CTLR); > + gic_do_wait_for_rwp(base); > + writeq_relaxed(ctx->propbaser, base + GICR_PROPBASER); > + writeq_relaxed(ctx->pendbaser, base + GICR_PENDBASER); Without checking for GICR_CTLR.EnableLPIs first? You're heading straight into UNPRED territory. > + > + writel_relaxed(ctx->igroupr0, sgi_base + GICR_IGROUPR0); > + writel_relaxed(ctx->isenabler0, sgi_base + GICR_ISENABLER0); > + writel_relaxed(ctx->ispendr0, sgi_base + GICR_ISPENDR0); > + writel_relaxed(ctx->isactiver0, sgi_base + GICR_ISACTIVER0); > + __iowrite32_copy(sgi_base + GICR_IPRIORITYR0, ctx->ipriorityr, 8); > + writel_relaxed(ctx->icfgr0, sgi_base + GICR_ICFGR0); > + writel_relaxed(ctx->icfgr1, sgi_base + GICR_ICFGR0 + sizeof(u32)); > + writel_relaxed(ctx->igrpmodr0, sgi_base + GICR_IGRPMODR0); > + writel_relaxed(ctx->nsacr, sgi_base + GICR_NSACR); > + writel_relaxed(ctx->ctlr, sgi_base + GICR_CTLR); > + gic_do_wait_for_rwp(base); Most of the remarks I had for the save side apply here too. > +} > + > +static void gic_dist_restore(void) > +{ > + struct gic_dist_ctx *ctx = gic_data.gicd_ctx; > + void __iomem *base = gic_data.dist_base; > + int irq_nr = gic_data.irq_nr; > + > + gic_dist_wait_for_rwp(); > + __iowrite64_copy(base + GICD_IROUTER, ctx->irouter, > + GICD_NR_REGS(IROUTER, irq_nr)); > + __iowrite32_copy(base + GICD_IGROUPR, ctx->igroupr, > + GICD_NR_REGS(IGROUPR, irq_nr)); > + __iowrite32_copy(base + GICD_ISENABLER, ctx->isenabler, > + GICD_NR_REGS(ISENABLER, irq_nr)); > + __iowrite32_copy(base + GICD_ISPENDR, ctx->ispendr, > + GICD_NR_REGS(ISPENDR, irq_nr)); > + __iowrite32_copy(base + GICD_ISACTIVER, ctx->isactiver, > + GICD_NR_REGS(ISACTIVER, irq_nr)); > + __iowrite32_copy(base + GICD_ICFGR, ctx->icfgr, > + GICD_NR_REGS(ICFGR, irq_nr)); > + __iowrite32_copy(base + GICD_IGRPMODR, ctx->igrpmodr, > + GICD_NR_REGS(IGRPMODR, irq_nr)); > + __iowrite32_copy(base + GICD_NSACR, ctx->nsacr, > + GICD_NR_REGS(NSACR, irq_nr)); > + writel_relaxed(ctx->ctlr, base + GICD_CTLR); > + gic_dist_wait_for_rwp(); Same thing. > +} > + > +static void gic_resume(void) > +{ > + int cpu; > + > + if (!gic_data.gicd_ctx || !gic_data.gicr_ctx) > + return; > + > + gic_dist_restore(); > + its_restore_enable(); > + for_each_possible_cpu(cpu) > + gic_rdist_restore(cpu); > + > + gic_enable_redist(true); > +} > + > #ifdef CONFIG_CPU_PM > /* Check whether it's single security state view */ > static bool gic_dist_security_disabled(void) > @@ -767,13 +953,24 @@ static bool gic_dist_security_disabled(void) > static int gic_cpu_pm_notifier(struct notifier_block *self, > unsigned long cmd, void *v) > { > - if (cmd == CPU_PM_EXIT) { > + switch (cmd) { > + case CPU_PM_EXIT: > if (gic_dist_security_disabled()) > gic_enable_redist(true); > gic_cpu_sys_reg_init(); > - } else if (cmd == CPU_PM_ENTER && gic_dist_security_disabled()) { > - gic_write_grpen1(0); > - gic_enable_redist(false); > + break; > + case CPU_PM_ENTER: > + if (gic_dist_security_disabled()) { > + gic_write_grpen1(0); > + gic_enable_redist(false); > + } > + break; > + case CPU_SYSTEM_PM_EXIT: > + gic_resume(); > + break; > + case CPU_SYSTEM_PM_ENTER: > + gic_suspend(); > + break; > } > return NOTIFY_OK; > } > @@ -991,11 +1188,99 @@ static const struct irq_domain_ops partition_domain_ops = { > .select = gic_irq_domain_select, > }; > > +static int gic_populate_ctx(struct gic_dist_ctx *ctx, int irqs) This isn't the GIC context. This is the save area. Please name the function accordingly. > +{ > + int err; > + > + ctx->irouter = kcalloc(GICD_NR_REGS(IROUTER, irqs), > + sizeof(*ctx->irouter), GFP_KERNEL); > + if (IS_ERR(ctx->irouter)) > + return PTR_ERR(ctx->irouter); > + > + ctx->igroupr = kcalloc(GICD_NR_REGS(IGROUPR, irqs), > + sizeof(*ctx->igroupr), GFP_KERNEL); > + if (IS_ERR(ctx->igroupr)) { > + err = PTR_ERR(ctx->igroupr); > + goto out_irouter; > + } > + > + ctx->isenabler = kcalloc(GICD_NR_REGS(ISENABLER, irqs), > + sizeof(*ctx->isenabler), GFP_KERNEL); > + if (IS_ERR(ctx->isenabler)) { > + err = PTR_ERR(ctx->isenabler); > + goto out_igroupr; > + } > + > + ctx->ispendr = kcalloc(GICD_NR_REGS(ISPENDR, irqs), > + sizeof(*ctx->ispendr), GFP_KERNEL); > + if (IS_ERR(ctx->ispendr)) { > + err = PTR_ERR(ctx->ispendr); > + goto out_isenabler; > + } > + > + ctx->isactiver = kcalloc(GICD_NR_REGS(ISACTIVER, irqs), > + sizeof(*ctx->isactiver), GFP_KERNEL); > + if (IS_ERR(ctx->isactiver)) { > + err = PTR_ERR(ctx->isactiver); > + goto out_ispendr; > + } > + > + ctx->ipriorityr = kcalloc(GICD_NR_REGS(IPRIORITYR, irqs), > + sizeof(*ctx->ipriorityr), GFP_KERNEL); > + if (IS_ERR(ctx->ipriorityr)) { > + err = PTR_ERR(ctx->ipriorityr); > + goto out_isactiver; > + } > + > + ctx->icfgr = kcalloc(GICD_NR_REGS(ICFGR, irqs), sizeof(*ctx->icfgr), > + GFP_KERNEL); > + if (IS_ERR(ctx->icfgr)) { > + err = PTR_ERR(ctx->icfgr); > + goto out_ipriorityr; > + } > + > + ctx->igrpmodr = kcalloc(GICD_NR_REGS(IGRPMODR, irqs), > + sizeof(*ctx->igrpmodr), GFP_KERNEL); > + if (IS_ERR(ctx->igrpmodr)) { > + err = PTR_ERR(ctx->igrpmodr); > + goto out_icfgr; > + } > + > + ctx->nsacr = kcalloc(GICD_NR_REGS(NSACR, irqs), sizeof(*ctx->nsacr), > + GFP_KERNEL); > + if (IS_ERR(ctx->nsacr)) { > + err = PTR_ERR(ctx->nsacr); > + goto out_igrpmodr; > + } > + > + return 0; > + > +out_irouter: > + kfree(ctx->irouter); > +out_igroupr: > + kfree(ctx->igroupr); > +out_isenabler: > + kfree(ctx->isenabler); > +out_ispendr: > + kfree(ctx->ispendr); > +out_isactiver: > + kfree(ctx->isactiver); > +out_ipriorityr: > + kfree(ctx->ipriorityr); > +out_icfgr: > + kfree(ctx->icfgr); > +out_igrpmodr: > + kfree(ctx->igrpmodr); WHy do you need all of these labels? Can't you just branch to an error one and free them all in one go? In the end, what is the point of keeping almost all of them if the last allocation fails? > + return err; > +} > + > static int __init gic_init_bases(void __iomem *dist_base, > struct redist_region *rdist_regs, > u32 nr_redist_regions, > u64 redist_stride, > - struct fwnode_handle *handle) > + struct fwnode_handle *handle, > + struct gic_dist_ctx *gicd_ctx, > + struct gic_redist_ctx *gicr_ctx) > { > u32 typer; > int gic_irqs; > @@ -1012,6 +1297,8 @@ static int __init gic_init_bases(void __iomem *dist_base, > gic_data.redist_regions = rdist_regs; > gic_data.nr_redist_regions = nr_redist_regions; > gic_data.redist_stride = redist_stride; > + gic_data.gicd_ctx = gicd_ctx; > + gic_data.gicr_ctx = gicr_ctx; > > /* > * Find out how many interrupts are supported. > @@ -1035,6 +1322,12 @@ static int __init gic_init_bases(void __iomem *dist_base, > goto out_free; > } > > + if (gicd_ctx) { > + err = gic_populate_ctx(gicd_ctx, gic_irqs); > + if (err) > + goto out_free; > + } > + > gic_data.has_rss = !!(typer & GICD_TYPER_RSS); > pr_info("Distributor has %sRange Selector support\n", > gic_data.has_rss ? "" : "no "); > @@ -1190,6 +1483,8 @@ static int __init gic_of_init(struct device_node *node, struct device_node *pare > { > void __iomem *dist_base; > struct redist_region *rdist_regs; > + struct gic_dist_ctx *gicd_ctx = NULL; > + struct gic_redist_ctx *gicr_ctx = NULL; > u64 redist_stride; > u32 nr_redist_regions; > int err, i; > @@ -1232,17 +1527,35 @@ static int __init gic_of_init(struct device_node *node, struct device_node *pare > if (of_property_read_u64(node, "redistributor-stride", &redist_stride)) > redist_stride = 0; > > + if (of_property_read_bool(node, "save-suspend-state")) { > + gicd_ctx = kzalloc(sizeof(*gicd_ctx), GFP_KERNEL); > + if (IS_ERR(gicd_ctx)) { > + err = PTR_ERR(gicd_ctx); > + goto out_unmap_rdist; > + } > + > + gicr_ctx = kcalloc(num_possible_cpus(), sizeof(*gicr_ctx), > + GFP_KERNEL); Since this is a per-cpu allocation, why isn't this a per-cpu allocation? In other words, why isn't the GICR save area allocated as CPUs get matched against their redistributors instead? > + if (IS_ERR(gicr_ctx)) { > + err = PTR_ERR(gicr_ctx); > + goto out_free_gicd_ctx; > + } > + } You really want to kill the box because something went wrong in your save area allocation? It doesn't feel quite right. > + > err = gic_init_bases(dist_base, rdist_regs, nr_redist_regions, > - redist_stride, &node->fwnode); > + redist_stride, &node->fwnode, gicd_ctx, gicr_ctx); > if (err) > - goto out_unmap_rdist; > + goto out_free_gicr_ctx; > > gic_populate_ppi_partitions(node); > > if (static_key_true(&supports_deactivate)) > gic_of_setup_kvm_info(node); > return 0; > - > +out_free_gicr_ctx: > + kfree(gicr_ctx); > +out_free_gicd_ctx: > + kfree(gicd_ctx); > out_unmap_rdist: > for (i = 0; i < nr_redist_regions; i++) > if (rdist_regs[i].redist_base) > diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h > index c00c4c33e432..f086987e3cb4 100644 > --- a/include/linux/irqchip/arm-gic-v3.h > +++ b/include/linux/irqchip/arm-gic-v3.h > @@ -46,6 +46,19 @@ > #define GICD_IDREGS 0xFFD0 > #define GICD_PIDR2 0xFFE8 > > +#define GICD_IGROUPR_SHIFT 5 > +#define GICD_ISENABLER_SHIFT 5 > +#define GICD_ICENABLER_SHIFT GICD_ISENABLER_SHIFT > +#define GICD_ISPENDR_SHIFT 5 > +#define GICD_ICPENDR_SHIFT GICD_ISPENDR_SHIFT > +#define GICD_ISACTIVER_SHIFT 5 > +#define GICD_ICACTIVER_SHIFT GICD_ISACTIVER_SHIFT > +#define GICD_IPRIORITYR_SHIFT 2 > +#define GICD_ICFGR_SHIFT 4 > +#define GICD_IGRPMODR_SHIFT 5 > +#define GICD_NSACR_SHIFT 4 > +#define GICD_IROUTER_SHIFT 0 No, please. We use the SHIFT/MASK suffixes to indicate bit fields. Do not overload this term with other meanings in the context of this driver. > + > /* > * Those registers are actually from GICv2, but the spec demands that they > * are implemented as RES0 if ARE is 1 (which we do in KVM's emulated GICv3). > @@ -188,6 +201,8 @@ > > #define GICR_PENDBASER_PTZ BIT_ULL(62) > > +#define GICR_SGI_BASE_OFFSET (1U << 16) > + > /* > * Re-Distributor registers, offsets from SGI_base > */ > @@ -581,6 +596,8 @@ struct rdists { > > struct irq_domain; > struct fwnode_handle; > +void its_save_disable(void); > +void its_restore_enable(void); > int its_cpu_init(void); > int its_init(struct fwnode_handle *handle, struct rdists *rdists, > struct irq_domain *domain); > -- > 2.16.0.rc1.238.g530d649a79-goog > Overall, this patch needs quite a bit of rework. You should take into account *how* the GIC is used in Linux, and not blindly copy the whole of the register state. You should be more careful with 32bit (at least make sure it still compiles and works in a guest). Also, there is a lot of scope to reuse existing code on the GICv1/2 side, so please investigate this. Finally, please split this patch so that the ITS and the core GICv3 code are patches separately. Thanks, M.
Hi, On Sat, Jan 13, 2018 at 06:10:52PM +0000, Marc Zyngier wrote: > On Fri, 12 Jan 2018 21:24:18 +0000, > Derek Basehore wrote: > > > > Some platforms power off GIC logic in S3, so we need to save/restore > > S3 is a not a GIC concept, and is only vaguely mentioned in terms of > the rk3399 silicon, if grep serves me right. Please expand on what > state this is exactly. > > > state. This adds a DT-binding to save/restore the GICD/GICR/GITS > > states using the new CPU_PM_SYSTEM_ENTER/EXIT CPU PM states. > > DT binding? I can't see any in this patch. > > > > > Change-Id: I1fb2117296373fa67397fdd4a8960077b241462e > > It's been mentioned somewhere else in the thread: these tags have no > purpose in the kernel. Please sanitise your patches before posting them. > > > Signed-off-by: Derek Basehore <dbasehore@chromium.org> > > Signed-off-by: Brian Norris <briannorris@chromium.org> > > Who is the author of this patch? If that's a joined authorship, please > use the Co-Developed-by: tag. I only did some minimal code shuffling when rebasing and working with this code in our downstream tree. I probably didn't actually need to apply my Signed-off-by at the time, but Derek carried it along anyway. Derek is the author, and I'd be perfectly fine dropping my S-o-b from these patches. > > diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c > > index 9a7a15049903..95d37fb6f458 100644 > > --- a/drivers/irqchip/irq-gic-v3.c > > +++ b/drivers/irqchip/irq-gic-v3.c ... > > @@ -991,11 +1188,99 @@ static const struct irq_domain_ops partition_domain_ops = { > > .select = gic_irq_domain_select, > > }; > > > > +static int gic_populate_ctx(struct gic_dist_ctx *ctx, int irqs) > > This isn't the GIC context. This is the save area. Please name the > function accordingly. > > > +{ > > + int err; > > + > > + ctx->irouter = kcalloc(GICD_NR_REGS(IROUTER, irqs), > > + sizeof(*ctx->irouter), GFP_KERNEL); > > + if (IS_ERR(ctx->irouter)) > > + return PTR_ERR(ctx->irouter); > > + > > + ctx->igroupr = kcalloc(GICD_NR_REGS(IGROUPR, irqs), > > + sizeof(*ctx->igroupr), GFP_KERNEL); > > + if (IS_ERR(ctx->igroupr)) { > > + err = PTR_ERR(ctx->igroupr); > > + goto out_irouter; > > + } > > + > > + ctx->isenabler = kcalloc(GICD_NR_REGS(ISENABLER, irqs), > > + sizeof(*ctx->isenabler), GFP_KERNEL); > > + if (IS_ERR(ctx->isenabler)) { > > + err = PTR_ERR(ctx->isenabler); > > + goto out_igroupr; > > + } > > + > > + ctx->ispendr = kcalloc(GICD_NR_REGS(ISPENDR, irqs), > > + sizeof(*ctx->ispendr), GFP_KERNEL); > > + if (IS_ERR(ctx->ispendr)) { > > + err = PTR_ERR(ctx->ispendr); > > + goto out_isenabler; > > + } > > + > > + ctx->isactiver = kcalloc(GICD_NR_REGS(ISACTIVER, irqs), > > + sizeof(*ctx->isactiver), GFP_KERNEL); > > + if (IS_ERR(ctx->isactiver)) { > > + err = PTR_ERR(ctx->isactiver); > > + goto out_ispendr; > > + } > > + > > + ctx->ipriorityr = kcalloc(GICD_NR_REGS(IPRIORITYR, irqs), > > + sizeof(*ctx->ipriorityr), GFP_KERNEL); > > + if (IS_ERR(ctx->ipriorityr)) { > > + err = PTR_ERR(ctx->ipriorityr); > > + goto out_isactiver; > > + } > > + > > + ctx->icfgr = kcalloc(GICD_NR_REGS(ICFGR, irqs), sizeof(*ctx->icfgr), > > + GFP_KERNEL); > > + if (IS_ERR(ctx->icfgr)) { > > + err = PTR_ERR(ctx->icfgr); > > + goto out_ipriorityr; > > + } > > + > > + ctx->igrpmodr = kcalloc(GICD_NR_REGS(IGRPMODR, irqs), > > + sizeof(*ctx->igrpmodr), GFP_KERNEL); > > + if (IS_ERR(ctx->igrpmodr)) { > > + err = PTR_ERR(ctx->igrpmodr); > > + goto out_icfgr; > > + } > > + > > + ctx->nsacr = kcalloc(GICD_NR_REGS(NSACR, irqs), sizeof(*ctx->nsacr), > > + GFP_KERNEL); > > + if (IS_ERR(ctx->nsacr)) { > > + err = PTR_ERR(ctx->nsacr); > > + goto out_igrpmodr; > > + } > > + > > + return 0; > > + > > +out_irouter: > > + kfree(ctx->irouter); > > +out_igroupr: > > + kfree(ctx->igroupr); > > +out_isenabler: > > + kfree(ctx->isenabler); > > +out_ispendr: > > + kfree(ctx->ispendr); > > +out_isactiver: > > + kfree(ctx->isactiver); > > +out_ipriorityr: > > + kfree(ctx->ipriorityr); > > +out_icfgr: > > + kfree(ctx->icfgr); > > +out_igrpmodr: > > + kfree(ctx->igrpmodr); > > WHy do you need all of these labels? Can't you just branch to an error > one and free them all in one go? If you assume that the memory was initially all zero (which it is -- kzalloc()), then you can get away with a single error label (kfree(0) is a no-op). But otherwise, this is the right way to do error handling... > In the end, what is the point of > keeping almost all of them if the last allocation fails? I didn't understand this question until I realized that the error handling is all accidentally done in reverse. If we fail the last allocation, we should be freeing all but the last one. But this code actually only frees 1 bit of memory in that case (i.e., a memory leak). > > + return err; > > +} > > + > > static int __init gic_init_bases(void __iomem *dist_base, > > struct redist_region *rdist_regs, > > u32 nr_redist_regions, > > u64 redist_stride, > > - struct fwnode_handle *handle) > > + struct fwnode_handle *handle, > > + struct gic_dist_ctx *gicd_ctx, > > + struct gic_redist_ctx *gicr_ctx) > > { > > u32 typer; > > int gic_irqs; ... > > @@ -1232,17 +1527,35 @@ static int __init gic_of_init(struct device_node *node, struct device_node *pare > > if (of_property_read_u64(node, "redistributor-stride", &redist_stride)) > > redist_stride = 0; > > > > + if (of_property_read_bool(node, "save-suspend-state")) { > > + gicd_ctx = kzalloc(sizeof(*gicd_ctx), GFP_KERNEL); > > + if (IS_ERR(gicd_ctx)) { > > + err = PTR_ERR(gicd_ctx); > > + goto out_unmap_rdist; > > + } > > + > > + gicr_ctx = kcalloc(num_possible_cpus(), sizeof(*gicr_ctx), > > + GFP_KERNEL); > > Since this is a per-cpu allocation, why isn't this a per-cpu > allocation? In other words, why isn't the GICR save area allocated as > CPUs get matched against their redistributors instead? > > > + if (IS_ERR(gicr_ctx)) { > > + err = PTR_ERR(gicr_ctx); > > + goto out_free_gicd_ctx; > > + } > > + } > > You really want to kill the box because something went wrong in your > save area allocation? It doesn't feel quite right. Isn't that what all drivers (including irqchip drivers) do on failed allocations? What else would we do? Pretend that we can limp along and just b0rk the system when it suspends? Brian
On 18/01/18 23:33, Brian Norris wrote: > Hi, > > On Sat, Jan 13, 2018 at 06:10:52PM +0000, Marc Zyngier wrote: >> On Fri, 12 Jan 2018 21:24:18 +0000, >> Derek Basehore wrote: >>> >>> Some platforms power off GIC logic in S3, so we need to save/restore >> >> S3 is a not a GIC concept, and is only vaguely mentioned in terms of >> the rk3399 silicon, if grep serves me right. Please expand on what >> state this is exactly. >> >>> state. This adds a DT-binding to save/restore the GICD/GICR/GITS >>> states using the new CPU_PM_SYSTEM_ENTER/EXIT CPU PM states. >> >> DT binding? I can't see any in this patch. >> >>> >>> Change-Id: I1fb2117296373fa67397fdd4a8960077b241462e >> >> It's been mentioned somewhere else in the thread: these tags have no >> purpose in the kernel. Please sanitise your patches before posting them. >> >>> Signed-off-by: Derek Basehore <dbasehore@chromium.org> >>> Signed-off-by: Brian Norris <briannorris@chromium.org> >> >> Who is the author of this patch? If that's a joined authorship, please >> use the Co-Developed-by: tag. > > I only did some minimal code shuffling when rebasing and working with > this code in our downstream tree. I probably didn't actually need to > apply my Signed-off-by at the time, but Derek carried it along anyway. > > Derek is the author, and I'd be perfectly fine dropping my S-o-b from > these patches. > >>> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c >>> index 9a7a15049903..95d37fb6f458 100644 >>> --- a/drivers/irqchip/irq-gic-v3.c >>> +++ b/drivers/irqchip/irq-gic-v3.c [...] >>> + if (IS_ERR(gicr_ctx)) { >>> + err = PTR_ERR(gicr_ctx); >>> + goto out_free_gicd_ctx; >>> + } >>> + } >> >> You really want to kill the box because something went wrong in your >> save area allocation? It doesn't feel quite right. > > Isn't that what all drivers (including irqchip drivers) do on failed > allocations? What else would we do? Pretend that we can limp along and > just b0rk the system when it suspends? It would certainly give the user a chance to diagnostic the problem (which is otherwise pretty hard if the system doesn't boot). We kill the system if we cannot continue. In this case, we can. So why not try it? Thanks, M.
On Fri, Jan 19, 2018 at 1:22 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: > On 18/01/18 23:33, Brian Norris wrote: >> Hi, >> >> On Sat, Jan 13, 2018 at 06:10:52PM +0000, Marc Zyngier wrote: >>> On Fri, 12 Jan 2018 21:24:18 +0000, >>> Derek Basehore wrote: >>>> >>>> Some platforms power off GIC logic in S3, so we need to save/restore >>> >>> S3 is a not a GIC concept, and is only vaguely mentioned in terms of >>> the rk3399 silicon, if grep serves me right. Please expand on what >>> state this is exactly. >>> >>>> state. This adds a DT-binding to save/restore the GICD/GICR/GITS >>>> states using the new CPU_PM_SYSTEM_ENTER/EXIT CPU PM states. >>> >>> DT binding? I can't see any in this patch. >>> >>>> >>>> Change-Id: I1fb2117296373fa67397fdd4a8960077b241462e >>> >>> It's been mentioned somewhere else in the thread: these tags have no >>> purpose in the kernel. Please sanitise your patches before posting them. >>> >>>> Signed-off-by: Derek Basehore <dbasehore@chromium.org> >>>> Signed-off-by: Brian Norris <briannorris@chromium.org> >>> >>> Who is the author of this patch? If that's a joined authorship, please >>> use the Co-Developed-by: tag. >> >> I only did some minimal code shuffling when rebasing and working with >> this code in our downstream tree. I probably didn't actually need to >> apply my Signed-off-by at the time, but Derek carried it along anyway. >> >> Derek is the author, and I'd be perfectly fine dropping my S-o-b from >> these patches. >> >>>> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c >>>> index 9a7a15049903..95d37fb6f458 100644 >>>> --- a/drivers/irqchip/irq-gic-v3.c >>>> +++ b/drivers/irqchip/irq-gic-v3.c > > [...] > >>>> + if (IS_ERR(gicr_ctx)) { >>>> + err = PTR_ERR(gicr_ctx); >>>> + goto out_free_gicd_ctx; >>>> + } >>>> + } >>> >>> You really want to kill the box because something went wrong in your >>> save area allocation? It doesn't feel quite right. >> >> Isn't that what all drivers (including irqchip drivers) do on failed >> allocations? What else would we do? Pretend that we can limp along and >> just b0rk the system when it suspends? > It would certainly give the user a chance to diagnostic the problem > (which is otherwise pretty hard if the system doesn't boot). We kill the > system if we cannot continue. In this case, we can. So why not try it? I'm in the middle of a lot of refactoring that will make this irrelevant, so I guess we can leave it at that. I'll disable the feature and print an error in the case of allocation failures (in the parts that remain) in the next version. Still debugging the broken ATF code which is now going to be used, so no ETA on that. > > Thanks, > > M. > -- > Jazz is not dead. It just smells funny...
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c index 06f025fd5726..5cb808e3d0bf 100644 --- a/drivers/irqchip/irq-gic-v3-its.c +++ b/drivers/irqchip/irq-gic-v3-its.c @@ -85,6 +85,16 @@ struct its_baser { struct its_device; +/* + * Saved ITS state - this is where saved state for the ITS is stored + * when it's disabled during system suspend. + */ +struct its_ctx { + u64 cbaser; + u64 cwriter; + u32 ctlr; +}; + /* * The ITS structure - contains most of the infrastructure, with the * top-level MSI domain, the command queue, the collections, and the @@ -101,6 +111,7 @@ struct its_node { struct its_collection *collections; struct fwnode_handle *fwnode_handle; u64 (*get_msi_base)(struct its_device *its_dev); + struct its_ctx its_ctx; struct list_head its_device_list; u64 flags; unsigned long list_nr; @@ -3042,6 +3053,46 @@ static void its_enable_quirks(struct its_node *its) gic_enable_quirks(iidr, its_quirks, its); } +void its_save_disable(void) +{ + struct its_node *its; + + spin_lock(&its_lock); + list_for_each_entry(its, &its_nodes, entry) { + struct its_ctx *ctx = &its->its_ctx; + void __iomem *base = its->base; + int i; + + ctx->ctlr = readl_relaxed(base + GITS_CTLR); + its_force_quiescent(base); + ctx->cbaser = gits_read_cbaser(base + GITS_CBASER); + ctx->cwriter = readq_relaxed(base + GITS_CWRITER); + for (i = 0; i < ARRAY_SIZE(its->tables); i++) + its->tables[i].val = its_read_baser(its, &its->tables[i]); + } + spin_unlock(&its_lock); +} + +void its_restore_enable(void) +{ + struct its_node *its; + + spin_lock(&its_lock); + list_for_each_entry(its, &its_nodes, entry) { + struct its_ctx *ctx = &its->its_ctx; + void __iomem *base = its->base; + int i; + + gits_write_cbaser(ctx->cbaser, base + GITS_CBASER); + gits_write_cwriter(ctx->cwriter, base + GITS_CWRITER); + for (i = 0; i < ARRAY_SIZE(its->tables); i++) + its_write_baser(its, &its->tables[i], + its->tables[i].val); + writel_relaxed(ctx->ctlr, base + GITS_CTLR); + } + spin_unlock(&its_lock); +} + static int its_init_domain(struct fwnode_handle *handle, struct its_node *its) { struct irq_domain *inner_domain; diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c index 9a7a15049903..95d37fb6f458 100644 --- a/drivers/irqchip/irq-gic-v3.c +++ b/drivers/irqchip/irq-gic-v3.c @@ -47,6 +47,36 @@ struct redist_region { bool single_redist; }; +struct gic_dist_ctx { + u64 *irouter; + u32 *igroupr; + u32 *isenabler; + u32 *ispendr; + u32 *isactiver; + u32 *ipriorityr; + u32 *icfgr; + u32 *igrpmodr; + u32 *nsacr; + u32 ctlr; +}; + +struct gic_redist_ctx { + u64 propbaser; + u64 pendbaser; + u32 ipriorityr[8]; + u32 ctlr; + u32 igroupr0; + u32 isenabler0; + u32 ispendr0; + u32 isactiver0; + u32 icfgr0; + u32 icfgr1; + u32 igrpmodr0; + u32 nsacr; +}; + +#define GICD_NR_REGS(reg, nr_irq) ((nr_irq) >> GICD_## reg ##_SHIFT) + struct gic_chip_data { struct fwnode_handle *fwnode; void __iomem *dist_base; @@ -58,6 +88,8 @@ struct gic_chip_data { bool has_rss; unsigned int irq_nr; struct partition_desc *ppi_descs[16]; + struct gic_dist_ctx *gicd_ctx; + struct gic_redist_ctx *gicr_ctx; }; static struct gic_chip_data gic_data __read_mostly; @@ -133,13 +165,13 @@ static u64 __maybe_unused gic_read_iar(void) } #endif -static void gic_enable_redist(bool enable) +static void gic_enable_redist_base(int cpu, bool enable) { void __iomem *rbase; u32 count = 1000000; /* 1s! */ u32 val; - rbase = gic_data_rdist_rd_base(); + rbase = per_cpu_ptr(gic_data.rdists.rdist, cpu)->rd_base; val = readl_relaxed(rbase + GICR_WAKER); if (enable) @@ -167,6 +199,11 @@ static void gic_enable_redist(bool enable) enable ? "wakeup" : "sleep"); } +static void gic_enable_redist(bool enable) +{ + gic_enable_redist_base(smp_processor_id(), enable); +} + /* * Routines to disable, enable, EOI and route interrupts */ @@ -757,6 +794,155 @@ static int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val, #define gic_smp_init() do { } while(0) #endif +static void gic_redist_save(int cpu) +{ + struct gic_redist_ctx *ctx = &gic_data.gicr_ctx[cpu]; + void __iomem *base = per_cpu_ptr(gic_data.rdists.rdist, cpu)->rd_base; + + gic_do_wait_for_rwp(base); + ctx->ctlr = readl_relaxed(base + GICR_CTLR); + ctx->propbaser = readq_relaxed(base + GICR_PROPBASER); + ctx->pendbaser = readq_relaxed(base + GICR_PENDBASER); + + base += GICR_SGI_BASE_OFFSET; + ctx->igroupr0 = readl_relaxed(base + GICR_IGROUPR0); + ctx->isenabler0 = readl_relaxed(base + GICR_ISENABLER0); + ctx->ispendr0 = readl_relaxed(base + GICR_ISPENDR0); + ctx->isactiver0 = readl_relaxed(base + GICR_ISACTIVER0); + __ioread32_copy(ctx->ipriorityr, base + GICR_IPRIORITYR0, 8); + ctx->icfgr0 = readl_relaxed(base + GICR_ICFGR0); + ctx->icfgr1 = readl_relaxed(base + GICR_ICFGR0 + sizeof(u32)); + ctx->igrpmodr0 = readl_relaxed(base + GICR_IGRPMODR0); + ctx->nsacr = readl_relaxed(base + GICR_NSACR); +} + +static void gic_dist_save(void) +{ + struct gic_dist_ctx *ctx = gic_data.gicd_ctx; + void __iomem *base = gic_data.dist_base; + int irq_nr = gic_data.irq_nr; + + __ioread64_copy(ctx->irouter, base + GICD_IROUTER, + GICD_NR_REGS(IROUTER, irq_nr)); + __ioread32_copy(ctx->igroupr, base + GICD_IGROUPR, + GICD_NR_REGS(IGROUPR, irq_nr)); + __ioread32_copy(ctx->isenabler, base + GICD_ISENABLER, + GICD_NR_REGS(ISENABLER, irq_nr)); + __ioread32_copy(ctx->ispendr, base + GICD_ISPENDR, + GICD_NR_REGS(ISPENDR, irq_nr)); + __ioread32_copy(ctx->isactiver, base + GICD_ISACTIVER, + GICD_NR_REGS(ISACTIVER, irq_nr)); + __ioread32_copy(ctx->icfgr, base + GICD_ICFGR, + GICD_NR_REGS(ICFGR, irq_nr)); + __ioread32_copy(ctx->igrpmodr, base + GICD_IGRPMODR, + GICD_NR_REGS(IGRPMODR, irq_nr)); + __ioread32_copy(ctx->nsacr, base + GICD_NSACR, + GICD_NR_REGS(NSACR, irq_nr)); + ctx->ctlr = readl_relaxed(base + GICD_CTLR); +} + +static int gic_suspend(void) +{ + void __iomem *rbase; + int cpu; + + if (!gic_data.gicd_ctx || !gic_data.gicr_ctx) + return 0; + + for_each_possible_cpu(cpu) { + /* + * The current processor will have the redistributor disabled in + * the next step. + */ + if (cpu == smp_processor_id()) + continue; + + /* Each redistributor must be disabled to save state. */ + rbase = per_cpu_ptr(gic_data.rdists.rdist, cpu)->rd_base; + if (!(readl_relaxed(rbase + GICR_WAKER) & + GICR_WAKER_ProcessorSleep)) { + pr_err("Redistributor for CPU: %d enabled \n", cpu); + return -EPERM; + } + } + + /* Disable the redist in case it wasn't in CPU_PM_ENTER. */ + gic_enable_redist(false); + for_each_possible_cpu(cpu) + gic_redist_save(cpu); + + its_save_disable(); + gic_dist_save(); + + return 0; +} + +static void gic_rdist_restore(int cpu) +{ + struct gic_redist_ctx *ctx = &gic_data.gicr_ctx[cpu]; + void __iomem *base = per_cpu_ptr(gic_data.rdists.rdist, cpu)->rd_base; + void __iomem *sgi_base = base + GICR_SGI_BASE_OFFSET; + + writel_relaxed(ctx->ctlr & ~(GICR_CTLR_ENABLE_LPIS), base + GICR_CTLR); + gic_do_wait_for_rwp(base); + writeq_relaxed(ctx->propbaser, base + GICR_PROPBASER); + writeq_relaxed(ctx->pendbaser, base + GICR_PENDBASER); + + writel_relaxed(ctx->igroupr0, sgi_base + GICR_IGROUPR0); + writel_relaxed(ctx->isenabler0, sgi_base + GICR_ISENABLER0); + writel_relaxed(ctx->ispendr0, sgi_base + GICR_ISPENDR0); + writel_relaxed(ctx->isactiver0, sgi_base + GICR_ISACTIVER0); + __iowrite32_copy(sgi_base + GICR_IPRIORITYR0, ctx->ipriorityr, 8); + writel_relaxed(ctx->icfgr0, sgi_base + GICR_ICFGR0); + writel_relaxed(ctx->icfgr1, sgi_base + GICR_ICFGR0 + sizeof(u32)); + writel_relaxed(ctx->igrpmodr0, sgi_base + GICR_IGRPMODR0); + writel_relaxed(ctx->nsacr, sgi_base + GICR_NSACR); + writel_relaxed(ctx->ctlr, sgi_base + GICR_CTLR); + gic_do_wait_for_rwp(base); +} + +static void gic_dist_restore(void) +{ + struct gic_dist_ctx *ctx = gic_data.gicd_ctx; + void __iomem *base = gic_data.dist_base; + int irq_nr = gic_data.irq_nr; + + gic_dist_wait_for_rwp(); + __iowrite64_copy(base + GICD_IROUTER, ctx->irouter, + GICD_NR_REGS(IROUTER, irq_nr)); + __iowrite32_copy(base + GICD_IGROUPR, ctx->igroupr, + GICD_NR_REGS(IGROUPR, irq_nr)); + __iowrite32_copy(base + GICD_ISENABLER, ctx->isenabler, + GICD_NR_REGS(ISENABLER, irq_nr)); + __iowrite32_copy(base + GICD_ISPENDR, ctx->ispendr, + GICD_NR_REGS(ISPENDR, irq_nr)); + __iowrite32_copy(base + GICD_ISACTIVER, ctx->isactiver, + GICD_NR_REGS(ISACTIVER, irq_nr)); + __iowrite32_copy(base + GICD_ICFGR, ctx->icfgr, + GICD_NR_REGS(ICFGR, irq_nr)); + __iowrite32_copy(base + GICD_IGRPMODR, ctx->igrpmodr, + GICD_NR_REGS(IGRPMODR, irq_nr)); + __iowrite32_copy(base + GICD_NSACR, ctx->nsacr, + GICD_NR_REGS(NSACR, irq_nr)); + writel_relaxed(ctx->ctlr, base + GICD_CTLR); + gic_dist_wait_for_rwp(); +} + +static void gic_resume(void) +{ + int cpu; + + if (!gic_data.gicd_ctx || !gic_data.gicr_ctx) + return; + + gic_dist_restore(); + its_restore_enable(); + for_each_possible_cpu(cpu) + gic_rdist_restore(cpu); + + gic_enable_redist(true); +} + #ifdef CONFIG_CPU_PM /* Check whether it's single security state view */ static bool gic_dist_security_disabled(void) @@ -767,13 +953,24 @@ static bool gic_dist_security_disabled(void) static int gic_cpu_pm_notifier(struct notifier_block *self, unsigned long cmd, void *v) { - if (cmd == CPU_PM_EXIT) { + switch (cmd) { + case CPU_PM_EXIT: if (gic_dist_security_disabled()) gic_enable_redist(true); gic_cpu_sys_reg_init(); - } else if (cmd == CPU_PM_ENTER && gic_dist_security_disabled()) { - gic_write_grpen1(0); - gic_enable_redist(false); + break; + case CPU_PM_ENTER: + if (gic_dist_security_disabled()) { + gic_write_grpen1(0); + gic_enable_redist(false); + } + break; + case CPU_SYSTEM_PM_EXIT: + gic_resume(); + break; + case CPU_SYSTEM_PM_ENTER: + gic_suspend(); + break; } return NOTIFY_OK; } @@ -991,11 +1188,99 @@ static const struct irq_domain_ops partition_domain_ops = { .select = gic_irq_domain_select, }; +static int gic_populate_ctx(struct gic_dist_ctx *ctx, int irqs) +{ + int err; + + ctx->irouter = kcalloc(GICD_NR_REGS(IROUTER, irqs), + sizeof(*ctx->irouter), GFP_KERNEL); + if (IS_ERR(ctx->irouter)) + return PTR_ERR(ctx->irouter); + + ctx->igroupr = kcalloc(GICD_NR_REGS(IGROUPR, irqs), + sizeof(*ctx->igroupr), GFP_KERNEL); + if (IS_ERR(ctx->igroupr)) { + err = PTR_ERR(ctx->igroupr); + goto out_irouter; + } + + ctx->isenabler = kcalloc(GICD_NR_REGS(ISENABLER, irqs), + sizeof(*ctx->isenabler), GFP_KERNEL); + if (IS_ERR(ctx->isenabler)) { + err = PTR_ERR(ctx->isenabler); + goto out_igroupr; + } + + ctx->ispendr = kcalloc(GICD_NR_REGS(ISPENDR, irqs), + sizeof(*ctx->ispendr), GFP_KERNEL); + if (IS_ERR(ctx->ispendr)) { + err = PTR_ERR(ctx->ispendr); + goto out_isenabler; + } + + ctx->isactiver = kcalloc(GICD_NR_REGS(ISACTIVER, irqs), + sizeof(*ctx->isactiver), GFP_KERNEL); + if (IS_ERR(ctx->isactiver)) { + err = PTR_ERR(ctx->isactiver); + goto out_ispendr; + } + + ctx->ipriorityr = kcalloc(GICD_NR_REGS(IPRIORITYR, irqs), + sizeof(*ctx->ipriorityr), GFP_KERNEL); + if (IS_ERR(ctx->ipriorityr)) { + err = PTR_ERR(ctx->ipriorityr); + goto out_isactiver; + } + + ctx->icfgr = kcalloc(GICD_NR_REGS(ICFGR, irqs), sizeof(*ctx->icfgr), + GFP_KERNEL); + if (IS_ERR(ctx->icfgr)) { + err = PTR_ERR(ctx->icfgr); + goto out_ipriorityr; + } + + ctx->igrpmodr = kcalloc(GICD_NR_REGS(IGRPMODR, irqs), + sizeof(*ctx->igrpmodr), GFP_KERNEL); + if (IS_ERR(ctx->igrpmodr)) { + err = PTR_ERR(ctx->igrpmodr); + goto out_icfgr; + } + + ctx->nsacr = kcalloc(GICD_NR_REGS(NSACR, irqs), sizeof(*ctx->nsacr), + GFP_KERNEL); + if (IS_ERR(ctx->nsacr)) { + err = PTR_ERR(ctx->nsacr); + goto out_igrpmodr; + } + + return 0; + +out_irouter: + kfree(ctx->irouter); +out_igroupr: + kfree(ctx->igroupr); +out_isenabler: + kfree(ctx->isenabler); +out_ispendr: + kfree(ctx->ispendr); +out_isactiver: + kfree(ctx->isactiver); +out_ipriorityr: + kfree(ctx->ipriorityr); +out_icfgr: + kfree(ctx->icfgr); +out_igrpmodr: + kfree(ctx->igrpmodr); + return err; +} + static int __init gic_init_bases(void __iomem *dist_base, struct redist_region *rdist_regs, u32 nr_redist_regions, u64 redist_stride, - struct fwnode_handle *handle) + struct fwnode_handle *handle, + struct gic_dist_ctx *gicd_ctx, + struct gic_redist_ctx *gicr_ctx) { u32 typer; int gic_irqs; @@ -1012,6 +1297,8 @@ static int __init gic_init_bases(void __iomem *dist_base, gic_data.redist_regions = rdist_regs; gic_data.nr_redist_regions = nr_redist_regions; gic_data.redist_stride = redist_stride; + gic_data.gicd_ctx = gicd_ctx; + gic_data.gicr_ctx = gicr_ctx; /* * Find out how many interrupts are supported. @@ -1035,6 +1322,12 @@ static int __init gic_init_bases(void __iomem *dist_base, goto out_free; } + if (gicd_ctx) { + err = gic_populate_ctx(gicd_ctx, gic_irqs); + if (err) + goto out_free; + } + gic_data.has_rss = !!(typer & GICD_TYPER_RSS); pr_info("Distributor has %sRange Selector support\n", gic_data.has_rss ? "" : "no "); @@ -1190,6 +1483,8 @@ static int __init gic_of_init(struct device_node *node, struct device_node *pare { void __iomem *dist_base; struct redist_region *rdist_regs; + struct gic_dist_ctx *gicd_ctx = NULL; + struct gic_redist_ctx *gicr_ctx = NULL; u64 redist_stride; u32 nr_redist_regions; int err, i; @@ -1232,17 +1527,35 @@ static int __init gic_of_init(struct device_node *node, struct device_node *pare if (of_property_read_u64(node, "redistributor-stride", &redist_stride)) redist_stride = 0; + if (of_property_read_bool(node, "save-suspend-state")) { + gicd_ctx = kzalloc(sizeof(*gicd_ctx), GFP_KERNEL); + if (IS_ERR(gicd_ctx)) { + err = PTR_ERR(gicd_ctx); + goto out_unmap_rdist; + } + + gicr_ctx = kcalloc(num_possible_cpus(), sizeof(*gicr_ctx), + GFP_KERNEL); + if (IS_ERR(gicr_ctx)) { + err = PTR_ERR(gicr_ctx); + goto out_free_gicd_ctx; + } + } + err = gic_init_bases(dist_base, rdist_regs, nr_redist_regions, - redist_stride, &node->fwnode); + redist_stride, &node->fwnode, gicd_ctx, gicr_ctx); if (err) - goto out_unmap_rdist; + goto out_free_gicr_ctx; gic_populate_ppi_partitions(node); if (static_key_true(&supports_deactivate)) gic_of_setup_kvm_info(node); return 0; - +out_free_gicr_ctx: + kfree(gicr_ctx); +out_free_gicd_ctx: + kfree(gicd_ctx); out_unmap_rdist: for (i = 0; i < nr_redist_regions; i++) if (rdist_regs[i].redist_base) diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h index c00c4c33e432..f086987e3cb4 100644 --- a/include/linux/irqchip/arm-gic-v3.h +++ b/include/linux/irqchip/arm-gic-v3.h @@ -46,6 +46,19 @@ #define GICD_IDREGS 0xFFD0 #define GICD_PIDR2 0xFFE8 +#define GICD_IGROUPR_SHIFT 5 +#define GICD_ISENABLER_SHIFT 5 +#define GICD_ICENABLER_SHIFT GICD_ISENABLER_SHIFT +#define GICD_ISPENDR_SHIFT 5 +#define GICD_ICPENDR_SHIFT GICD_ISPENDR_SHIFT +#define GICD_ISACTIVER_SHIFT 5 +#define GICD_ICACTIVER_SHIFT GICD_ISACTIVER_SHIFT +#define GICD_IPRIORITYR_SHIFT 2 +#define GICD_ICFGR_SHIFT 4 +#define GICD_IGRPMODR_SHIFT 5 +#define GICD_NSACR_SHIFT 4 +#define GICD_IROUTER_SHIFT 0 + /* * Those registers are actually from GICv2, but the spec demands that they * are implemented as RES0 if ARE is 1 (which we do in KVM's emulated GICv3). @@ -188,6 +201,8 @@ #define GICR_PENDBASER_PTZ BIT_ULL(62) +#define GICR_SGI_BASE_OFFSET (1U << 16) + /* * Re-Distributor registers, offsets from SGI_base */ @@ -581,6 +596,8 @@ struct rdists { struct irq_domain; struct fwnode_handle; +void its_save_disable(void); +void its_restore_enable(void); int its_cpu_init(void); int its_init(struct fwnode_handle *handle, struct rdists *rdists, struct irq_domain *domain);