Message ID | 0f1bebffe29c96c5b66c83b62b0c67752114c53a.1665137247.git.mykyta_poturai@epam.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [01/19] xen/arm: Implement PSCI system suspend | expand |
Hi, On 07/10/2022 11:32, Mykyta Poturai wrote: > From: Mirela Simonovic <mirela.simonovic@aggios.com> > > System suspend may lead to a state where GIC would be powered down. > Therefore, Xen should save/restore the context of GIC on suspend/resume. > Note that the context consists of states of registers which are > controlled by the hypervisor. Other GIC registers which are accessible > by guests are saved/restored on context switch. > Tested on Xilinx Ultrascale+ MPSoC with (and without) powering down > the GIC. > > Signed-off-by: Mirela Simonovic <mirela.simonovic@aggios.com> > Signed-off-by: Saeed Nowshadi <saeed.nowshadi@xilinx.com> Your signed-off-by is missing. > --- > xen/arch/arm/gic-v2.c | 138 +++++++++++++++++++++++++++++++++++++- > xen/arch/arm/gic.c | 27 ++++++++ > xen/include/asm-arm/gic.h | 8 +++ > 3 files changed, 172 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c > index b2adc8ec9a..b077b66d92 100644 > --- a/xen/arch/arm/gic-v2.c > +++ b/xen/arch/arm/gic-v2.c > @@ -123,6 +123,23 @@ static DEFINE_PER_CPU(u8, gic_cpu_id); > /* Maximum cpu interface per GIC */ > #define NR_GIC_CPU_IF 8 > > +/* GICv2 registers to be saved/restored on system suspend/resume */ > +struct gicv2_context { > + /* GICC context */ > + uint32_t gicc_ctlr; > + uint32_t gicc_pmr; > + uint32_t gicc_bpr; > + /* GICD context */ > + uint32_t gicd_ctlr; > + uint32_t *gicd_isenabler; > + uint32_t *gicd_isactiver; > + uint32_t *gicd_ipriorityr; > + uint32_t *gicd_itargetsr; > + uint32_t *gicd_icfgr; > +}; > + > +static struct gicv2_context gicv2_context; > + > static inline void writeb_gicd(uint8_t val, unsigned int offset) > { > writeb_relaxed(val, gicv2.map_dbase + offset); > @@ -1251,6 +1268,40 @@ static void __init gicv2_acpi_init(void) > static void __init gicv2_acpi_init(void) { } > #endif > > +static int gicv2_alloc_context(struct gicv2_context *gc) > +{ > + uint32_t n = gicv2_info.nr_lines; > + > + gc->gicd_isenabler = xzalloc_array(uint32_t, DIV_ROUND_UP(n, 32)); > + if ( !gc->gicd_isenabler ) > + goto err_free; > + > + gc->gicd_isactiver = xzalloc_array(uint32_t, DIV_ROUND_UP(n, 32)); > + if ( !gc->gicd_isactiver ) > + goto err_free; > + > + gc->gicd_itargetsr = xzalloc_array(uint32_t, DIV_ROUND_UP(n, 4)); > + if ( !gc->gicd_itargetsr ) > + goto err_free; > + > + gc->gicd_ipriorityr = xzalloc_array(uint32_t, DIV_ROUND_UP(n, 4)); > + if ( !gc->gicd_ipriorityr ) > + goto err_free; > + > + gc->gicd_icfgr = xzalloc_array(uint32_t, DIV_ROUND_UP(n, 16)); > + if ( !gc->gicd_icfgr ) > + goto err_free; > + > + return 0; > +err_free: > + xfree(gc->gicd_icfgr); > + xfree(gc->gicd_ipriorityr); > + xfree(gc->gicd_itargetsr); > + xfree(gc->gicd_isactiver); > + xfree(gc->gicd_isenabler); Please add a newline. > + return -ENOMEM; > +} > + > static int __init gicv2_init(void) > { > uint32_t aliased_offset = 0; > @@ -1318,7 +1369,8 @@ static int __init gicv2_init(void) > > spin_unlock(&gicv2.lock); > > - return 0; > + /* Allocate memory to be used for saving GIC context during the suspend */ > + return gicv2_alloc_context(&gicv2_context); As I pointed out in the previous version, suspend/resume is not going to be widely used. So I don't think we should prevent booting if we can't allocate the memory. In addition to that, I think we should consider to have: * a command line option to avoid wasting memory if the feature is not going to be used * ifdef the suspend/resume code to allow an integrator to compile out any related code. > } > > static void gicv2_do_LPI(unsigned int lpi) > @@ -1327,6 +1379,88 @@ static void gicv2_do_LPI(unsigned int lpi) > BUG(); > } > > +static int gicv2_suspend(void) > +{ > + int i; This should be unsigned int. > + > + ASSERT(gicv2_context.gicd_isenabler); > + ASSERT(gicv2_context.gicd_isactiver); > + ASSERT(gicv2_context.gicd_ipriorityr); > + ASSERT(gicv2_context.gicd_itargetsr); > + ASSERT(gicv2_context.gicd_icfgr); > + > + /* Save GICC configuration */ > + gicv2_context.gicc_ctlr = readl_gicc(GICC_CTLR); > + gicv2_context.gicc_pmr = readl_gicc(GICC_PMR); > + gicv2_context.gicc_bpr = readl_gicc(GICC_BPR); > + > + /* Save GICD configuration */ > + gicv2_context.gicd_ctlr = readl_gicd(GICD_CTLR); > + > + for ( i = 0; i < DIV_ROUND_UP(gicv2_info.nr_lines, 32); i++ ) > + gicv2_context.gicd_isenabler[i] = readl_gicd(GICD_ISENABLER + i * 4); > + > + for ( i = 0; i < DIV_ROUND_UP(gicv2_info.nr_lines, 32); i++ ) > + gicv2_context.gicd_isactiver[i] = readl_gicd(GICD_ISACTIVER + i * 4); > + > + for ( i = 0; i < DIV_ROUND_UP(gicv2_info.nr_lines, 4); i++ ) > + gicv2_context.gicd_ipriorityr[i] = readl_gicd(GICD_IPRIORITYR + i * 4); > + > + for ( i = 0; i < DIV_ROUND_UP(gicv2_info.nr_lines, 4); i++ ) > + gicv2_context.gicd_itargetsr[i] = readl_gicd(GICD_ITARGETSR + i * 4); > + > + for ( i = 0; i < DIV_ROUND_UP(gicv2_info.nr_lines, 16); i++ ) > + gicv2_context.gicd_icfgr[i] = readl_gicd(GICD_ICFGR + i * 4); > + > + return 0; > +} > + > +static void gicv2_resume(void) > +{ > + int i; Ditto. > + > + ASSERT(gicv2_context.gicd_isenabler); > + ASSERT(gicv2_context.gicd_isactiver); > + ASSERT(gicv2_context.gicd_ipriorityr); > + ASSERT(gicv2_context.gicd_itargetsr); > + ASSERT(gicv2_context.gicd_icfgr); > + > + /* Disable CPU interface and distributor */ > + writel_gicc(0, GICC_CTLR); > + writel_gicd(0, GICD_CTLR); We don't need those two lines during boot. So why do you need them during resume? > + > + /* Restore GICD configuration */ > + for ( i = 0; i < DIV_ROUND_UP(gicv2_info.nr_lines, 32); i++ ) { > + writel_gicd(0xffffffff, GICD_ICENABLER + i * 4); > + writel_gicd(gicv2_context.gicd_isenabler[i], GICD_ISENABLER + i * 4); > + } > + > + for ( i = 0; i < DIV_ROUND_UP(gicv2_info.nr_lines, 32); i++ ) { > + writel_gicd(0xffffffff, GICD_ICACTIVER + i * 4); > + writel_gicd(gicv2_context.gicd_isactiver[i], GICD_ISACTIVER + i * 4); > + } > + > + for ( i = 0; i < DIV_ROUND_UP(gicv2_info.nr_lines, 4); i++ ) > + writel_gicd(gicv2_context.gicd_ipriorityr[i], GICD_IPRIORITYR + i * 4); > + > + for ( i = 0; i < DIV_ROUND_UP(gicv2_info.nr_lines, 4); i++ ) > + writel_gicd(gicv2_context.gicd_itargetsr[i], GICD_ITARGETSR + i * 4); > + > + for ( i = 0; i < DIV_ROUND_UP(gicv2_info.nr_lines, 16); i++ ) > + writel_gicd(gicv2_context.gicd_icfgr[i], GICD_ICFGR + i * 4); > + > + /* Make sure all registers are restored and enable distributor */ The first part reads like there is a missing barrier. > + writel_gicd(gicv2_context.gicd_ctlr | GICD_CTL_ENABLE, GICD_CTLR); > + > + /* Restore GIC CPU interface configuration */ > + writel_gicc(gicv2_context.gicc_pmr, GICC_PMR); > + writel_gicc(gicv2_context.gicc_bpr, GICC_BPR); > + > + /* Enable GIC CPU interface */ > + writel_gicc(gicv2_context.gicc_ctlr | GICC_CTL_ENABLE | GICC_CTL_EOI, > + GICC_CTLR); > +} > + > const static struct gic_hw_operations gicv2_ops = { > .info = &gicv2_info, > .init = gicv2_init, > @@ -1361,6 +1495,8 @@ const static struct gic_hw_operations gicv2_ops = { > .map_hwdom_extra_mappings = gicv2_map_hwdown_extra_mappings, > .iomem_deny_access = gicv2_iomem_deny_access, > .do_LPI = gicv2_do_LPI, > + .suspend = gicv2_suspend, > + .resume = gicv2_resume, > }; > > /* Set up the GIC */ > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > index 3b0331b538..e9feb1fd3b 100644 > --- a/xen/arch/arm/gic.c > +++ b/xen/arch/arm/gic.c > @@ -467,6 +467,33 @@ int gic_iomem_deny_access(const struct domain *d) > return gic_hw_ops->iomem_deny_access(d); > } > > +int gic_suspend(void) > +{ > + /* Must be called by boot CPU#0 with interrupts disabled */ > + ASSERT(!local_irq_is_enabled()); > + ASSERT(!smp_processor_id()); > + > + if ( !gic_hw_ops->suspend || !gic_hw_ops->resume ) > + return -ENOSYS; > + > + gic_hw_ops->suspend(); > + > + return 0; > +} > + > +void gic_resume(void) > +{ > + /* > + * Must be called by boot CPU#0 with interrupts disabled after gic_suspend > + * has returned successfully. > + */ > + ASSERT(!local_irq_is_enabled()); > + ASSERT(!smp_processor_id()); > + ASSERT(gic_hw_ops->resume); > + > + gic_hw_ops->resume(); > +} > + > static int cpu_gic_callback(struct notifier_block *nfb, > unsigned long action, > void *hcpu) > diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h > index c7f0c343d1..113e39460d 100644 > --- a/xen/include/asm-arm/gic.h > +++ b/xen/include/asm-arm/gic.h > @@ -275,6 +275,10 @@ extern int gicv_setup(struct domain *d); > extern void gic_save_state(struct vcpu *v); > extern void gic_restore_state(struct vcpu *v); > > +/* Suspend/resume */ > +extern int gic_suspend(void); > +extern void gic_resume(void); > + > /* SGI (AKA IPIs) */ > enum gic_sgi { > GIC_SGI_EVENT_CHECK, > @@ -390,6 +394,10 @@ struct gic_hw_operations { > int (*iomem_deny_access)(const struct domain *d); > /* Handle LPIs, which require special handling */ > void (*do_LPI)(unsigned int lpi); > + /* Save GIC configuration due to the system suspend */ > + int (*suspend)(void); > + /* Restore GIC configuration due to the system resume */ > + void (*resume)(void); > }; > > extern const struct gic_hw_operations *gic_hw_ops; Cheers,
diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c index b2adc8ec9a..b077b66d92 100644 --- a/xen/arch/arm/gic-v2.c +++ b/xen/arch/arm/gic-v2.c @@ -123,6 +123,23 @@ static DEFINE_PER_CPU(u8, gic_cpu_id); /* Maximum cpu interface per GIC */ #define NR_GIC_CPU_IF 8 +/* GICv2 registers to be saved/restored on system suspend/resume */ +struct gicv2_context { + /* GICC context */ + uint32_t gicc_ctlr; + uint32_t gicc_pmr; + uint32_t gicc_bpr; + /* GICD context */ + uint32_t gicd_ctlr; + uint32_t *gicd_isenabler; + uint32_t *gicd_isactiver; + uint32_t *gicd_ipriorityr; + uint32_t *gicd_itargetsr; + uint32_t *gicd_icfgr; +}; + +static struct gicv2_context gicv2_context; + static inline void writeb_gicd(uint8_t val, unsigned int offset) { writeb_relaxed(val, gicv2.map_dbase + offset); @@ -1251,6 +1268,40 @@ static void __init gicv2_acpi_init(void) static void __init gicv2_acpi_init(void) { } #endif +static int gicv2_alloc_context(struct gicv2_context *gc) +{ + uint32_t n = gicv2_info.nr_lines; + + gc->gicd_isenabler = xzalloc_array(uint32_t, DIV_ROUND_UP(n, 32)); + if ( !gc->gicd_isenabler ) + goto err_free; + + gc->gicd_isactiver = xzalloc_array(uint32_t, DIV_ROUND_UP(n, 32)); + if ( !gc->gicd_isactiver ) + goto err_free; + + gc->gicd_itargetsr = xzalloc_array(uint32_t, DIV_ROUND_UP(n, 4)); + if ( !gc->gicd_itargetsr ) + goto err_free; + + gc->gicd_ipriorityr = xzalloc_array(uint32_t, DIV_ROUND_UP(n, 4)); + if ( !gc->gicd_ipriorityr ) + goto err_free; + + gc->gicd_icfgr = xzalloc_array(uint32_t, DIV_ROUND_UP(n, 16)); + if ( !gc->gicd_icfgr ) + goto err_free; + + return 0; +err_free: + xfree(gc->gicd_icfgr); + xfree(gc->gicd_ipriorityr); + xfree(gc->gicd_itargetsr); + xfree(gc->gicd_isactiver); + xfree(gc->gicd_isenabler); + return -ENOMEM; +} + static int __init gicv2_init(void) { uint32_t aliased_offset = 0; @@ -1318,7 +1369,8 @@ static int __init gicv2_init(void) spin_unlock(&gicv2.lock); - return 0; + /* Allocate memory to be used for saving GIC context during the suspend */ + return gicv2_alloc_context(&gicv2_context); } static void gicv2_do_LPI(unsigned int lpi) @@ -1327,6 +1379,88 @@ static void gicv2_do_LPI(unsigned int lpi) BUG(); } +static int gicv2_suspend(void) +{ + int i; + + ASSERT(gicv2_context.gicd_isenabler); + ASSERT(gicv2_context.gicd_isactiver); + ASSERT(gicv2_context.gicd_ipriorityr); + ASSERT(gicv2_context.gicd_itargetsr); + ASSERT(gicv2_context.gicd_icfgr); + + /* Save GICC configuration */ + gicv2_context.gicc_ctlr = readl_gicc(GICC_CTLR); + gicv2_context.gicc_pmr = readl_gicc(GICC_PMR); + gicv2_context.gicc_bpr = readl_gicc(GICC_BPR); + + /* Save GICD configuration */ + gicv2_context.gicd_ctlr = readl_gicd(GICD_CTLR); + + for ( i = 0; i < DIV_ROUND_UP(gicv2_info.nr_lines, 32); i++ ) + gicv2_context.gicd_isenabler[i] = readl_gicd(GICD_ISENABLER + i * 4); + + for ( i = 0; i < DIV_ROUND_UP(gicv2_info.nr_lines, 32); i++ ) + gicv2_context.gicd_isactiver[i] = readl_gicd(GICD_ISACTIVER + i * 4); + + for ( i = 0; i < DIV_ROUND_UP(gicv2_info.nr_lines, 4); i++ ) + gicv2_context.gicd_ipriorityr[i] = readl_gicd(GICD_IPRIORITYR + i * 4); + + for ( i = 0; i < DIV_ROUND_UP(gicv2_info.nr_lines, 4); i++ ) + gicv2_context.gicd_itargetsr[i] = readl_gicd(GICD_ITARGETSR + i * 4); + + for ( i = 0; i < DIV_ROUND_UP(gicv2_info.nr_lines, 16); i++ ) + gicv2_context.gicd_icfgr[i] = readl_gicd(GICD_ICFGR + i * 4); + + return 0; +} + +static void gicv2_resume(void) +{ + int i; + + ASSERT(gicv2_context.gicd_isenabler); + ASSERT(gicv2_context.gicd_isactiver); + ASSERT(gicv2_context.gicd_ipriorityr); + ASSERT(gicv2_context.gicd_itargetsr); + ASSERT(gicv2_context.gicd_icfgr); + + /* Disable CPU interface and distributor */ + writel_gicc(0, GICC_CTLR); + writel_gicd(0, GICD_CTLR); + + /* Restore GICD configuration */ + for ( i = 0; i < DIV_ROUND_UP(gicv2_info.nr_lines, 32); i++ ) { + writel_gicd(0xffffffff, GICD_ICENABLER + i * 4); + writel_gicd(gicv2_context.gicd_isenabler[i], GICD_ISENABLER + i * 4); + } + + for ( i = 0; i < DIV_ROUND_UP(gicv2_info.nr_lines, 32); i++ ) { + writel_gicd(0xffffffff, GICD_ICACTIVER + i * 4); + writel_gicd(gicv2_context.gicd_isactiver[i], GICD_ISACTIVER + i * 4); + } + + for ( i = 0; i < DIV_ROUND_UP(gicv2_info.nr_lines, 4); i++ ) + writel_gicd(gicv2_context.gicd_ipriorityr[i], GICD_IPRIORITYR + i * 4); + + for ( i = 0; i < DIV_ROUND_UP(gicv2_info.nr_lines, 4); i++ ) + writel_gicd(gicv2_context.gicd_itargetsr[i], GICD_ITARGETSR + i * 4); + + for ( i = 0; i < DIV_ROUND_UP(gicv2_info.nr_lines, 16); i++ ) + writel_gicd(gicv2_context.gicd_icfgr[i], GICD_ICFGR + i * 4); + + /* Make sure all registers are restored and enable distributor */ + writel_gicd(gicv2_context.gicd_ctlr | GICD_CTL_ENABLE, GICD_CTLR); + + /* Restore GIC CPU interface configuration */ + writel_gicc(gicv2_context.gicc_pmr, GICC_PMR); + writel_gicc(gicv2_context.gicc_bpr, GICC_BPR); + + /* Enable GIC CPU interface */ + writel_gicc(gicv2_context.gicc_ctlr | GICC_CTL_ENABLE | GICC_CTL_EOI, + GICC_CTLR); +} + const static struct gic_hw_operations gicv2_ops = { .info = &gicv2_info, .init = gicv2_init, @@ -1361,6 +1495,8 @@ const static struct gic_hw_operations gicv2_ops = { .map_hwdom_extra_mappings = gicv2_map_hwdown_extra_mappings, .iomem_deny_access = gicv2_iomem_deny_access, .do_LPI = gicv2_do_LPI, + .suspend = gicv2_suspend, + .resume = gicv2_resume, }; /* Set up the GIC */ diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c index 3b0331b538..e9feb1fd3b 100644 --- a/xen/arch/arm/gic.c +++ b/xen/arch/arm/gic.c @@ -467,6 +467,33 @@ int gic_iomem_deny_access(const struct domain *d) return gic_hw_ops->iomem_deny_access(d); } +int gic_suspend(void) +{ + /* Must be called by boot CPU#0 with interrupts disabled */ + ASSERT(!local_irq_is_enabled()); + ASSERT(!smp_processor_id()); + + if ( !gic_hw_ops->suspend || !gic_hw_ops->resume ) + return -ENOSYS; + + gic_hw_ops->suspend(); + + return 0; +} + +void gic_resume(void) +{ + /* + * Must be called by boot CPU#0 with interrupts disabled after gic_suspend + * has returned successfully. + */ + ASSERT(!local_irq_is_enabled()); + ASSERT(!smp_processor_id()); + ASSERT(gic_hw_ops->resume); + + gic_hw_ops->resume(); +} + static int cpu_gic_callback(struct notifier_block *nfb, unsigned long action, void *hcpu) diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h index c7f0c343d1..113e39460d 100644 --- a/xen/include/asm-arm/gic.h +++ b/xen/include/asm-arm/gic.h @@ -275,6 +275,10 @@ extern int gicv_setup(struct domain *d); extern void gic_save_state(struct vcpu *v); extern void gic_restore_state(struct vcpu *v); +/* Suspend/resume */ +extern int gic_suspend(void); +extern void gic_resume(void); + /* SGI (AKA IPIs) */ enum gic_sgi { GIC_SGI_EVENT_CHECK, @@ -390,6 +394,10 @@ struct gic_hw_operations { int (*iomem_deny_access)(const struct domain *d); /* Handle LPIs, which require special handling */ void (*do_LPI)(unsigned int lpi); + /* Save GIC configuration due to the system suspend */ + int (*suspend)(void); + /* Restore GIC configuration due to the system resume */ + void (*resume)(void); }; extern const struct gic_hw_operations *gic_hw_ops;