[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
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 <mirela.simonovic@xxxxxxxxxx>
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@xxxxxxxxxx>
Signed-off-by: Saeed Nowshadi <saeed.nowshadi@xxxxxxxxxx>
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,
--
Julien Grall
|
