Re: [PATCH 09/19] xen/arm: Implement GIC suspend/resume functions (gicv2 only)
Hi, On 07/10/2022 11:32, Mykyta Poturai wrote: From: Mirela Simonovic 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 Signed-off-by: Saeed Nowshadi 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(); -return 0; +/* Allocate memory to be used for saving GIC context during the suspend */ +return gicv2_alloc_context(_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. + +
[PATCH 09/19] xen/arm: Implement GIC suspend/resume functions (gicv2 only)
From: Mirela Simonovic 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 Signed-off-by: Saeed Nowshadi --- 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); +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(); -return 0; +/* Allocate memory to be used for saving GIC context during the suspend */ +return gicv2_alloc_context(_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(0x, GICD_ICENABLER + i * 4); +writel_gicd(gicv2_context.gicd_isenabler[i], GICD_ISENABLER + i * 4); +} + +for ( i = 0; i <