Re: [PATCH v6 02/13] xen/arm: gic-v2: Implement GIC suspend/resume functions

[Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>]

Mykola Kvach <xakep.amatop@xxxxxxxxx> writes:

> 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.
>
> Signed-off-by: Mirela Simonovic <mirela.simonovic@xxxxxxxxxx>
> Signed-off-by: Saeed Nowshadi <saeed.nowshadi@xxxxxxxxxx>
> Signed-off-by: Mykyta Poturai <mykyta_poturai@xxxxxxxx>
> Signed-off-by: Mykola Kvach <mykola_kvach@xxxxxxxx>

Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>

> ---
> Changes in v6:
> - drop extra func/line printing from dprintk
> - drop checking context allocation from resume handler
> - merge some loops where it is possible
>
> Changes in v4:
>   - Add error logging for allocation failures
>
> Changes in v3:
>   - Drop asserts and return error codes instead.
>   - Wrap code with CONFIG_SYSTEM_SUSPEND.
>
> Changes in v2:
>   - Minor fixes after review.
> ---
>  xen/arch/arm/gic-v2.c          | 143 +++++++++++++++++++++++++++++++++
>  xen/arch/arm/gic.c             |  29 +++++++
>  xen/arch/arm/include/asm/gic.h |  12 +++
>  3 files changed, 184 insertions(+)
>
> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
> index b23e72a3d0..6373599e69 100644
> --- a/xen/arch/arm/gic-v2.c
> +++ b/xen/arch/arm/gic-v2.c
> @@ -1098,6 +1098,140 @@ static int gicv2_iomem_deny_access(struct domain *d)
>      return iomem_deny_access(d, mfn, mfn + nr);
>  }
>  
> +#ifdef CONFIG_SYSTEM_SUSPEND
> +
> +/* 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 int gicv2_suspend(void)
> +{
> +    unsigned int i;
> +
> +    if ( !gicv2_context.gicd_isenabler )
> +    {
> +        dprintk(XENLOG_WARNING, "GICv2 suspend context not allocated!\n");
> +        return -ENOMEM;
> +    }
> +
> +    /* 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);
> +        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);
> +        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)
> +{
> +    unsigned int i;
> +
> +    gicv2_cpu_disable();
> +    /* Disable distributor */
> +    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);
> +
> +        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);
> +        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);
> +}
> +
> +static void 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;
> +
> + err_free:
> +    printk(XENLOG_ERR "Failed to allocate memory for GICv2 suspend 
> context\n");
> +
> +    xfree(gc->gicd_icfgr);
> +    xfree(gc->gicd_ipriorityr);
> +    xfree(gc->gicd_itargetsr);
> +    xfree(gc->gicd_isactiver);
> +    xfree(gc->gicd_isenabler);
> +
> +    memset(gc, 0, sizeof(*gc));
> +}
> +
> +#endif /* CONFIG_SYSTEM_SUSPEND */
> +
>  #ifdef CONFIG_ACPI
>  static unsigned long gicv2_get_hwdom_extra_madt_size(const struct domain *d)
>  {
> @@ -1302,6 +1436,11 @@ static int __init gicv2_init(void)
>  
>      spin_unlock(&gicv2.lock);
>  
> +#ifdef CONFIG_SYSTEM_SUSPEND
> +    /* Allocate memory to be used for saving GIC context during the suspend 
> */
> +    gicv2_alloc_context(&gicv2_context);
> +#endif /* CONFIG_SYSTEM_SUSPEND */
> +
>      return 0;
>  }
>  
> @@ -1345,6 +1484,10 @@ static const struct gic_hw_operations gicv2_ops = {
>      .map_hwdom_extra_mappings = gicv2_map_hwdom_extra_mappings,
>      .iomem_deny_access   = gicv2_iomem_deny_access,
>      .do_LPI              = gicv2_do_LPI,
> +#ifdef CONFIG_SYSTEM_SUSPEND
> +    .suspend             = gicv2_suspend,
> +    .resume              = gicv2_resume,
> +#endif /* CONFIG_SYSTEM_SUSPEND */
>  };
>  
>  /* Set up the GIC */
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index e80fe0ca24..a018bd7715 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -425,6 +425,35 @@ int gic_iomem_deny_access(struct domain *d)
>      return gic_hw_ops->iomem_deny_access(d);
>  }
>  
> +#ifdef CONFIG_SYSTEM_SUSPEND
> +
> +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;
> +
> +    return gic_hw_ops->suspend();
> +}
> +
> +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();
> +}
> +
> +#endif /* CONFIG_SYSTEM_SUSPEND */
> +
>  static int cpu_gic_callback(struct notifier_block *nfb,
>                              unsigned long action,
>                              void *hcpu)
> diff --git a/xen/arch/arm/include/asm/gic.h b/xen/arch/arm/include/asm/gic.h
> index 541f0eeb80..a706303008 100644
> --- a/xen/arch/arm/include/asm/gic.h
> +++ b/xen/arch/arm/include/asm/gic.h
> @@ -280,6 +280,12 @@ extern int gicv_setup(struct domain *d);
>  extern void gic_save_state(struct vcpu *v);
>  extern void gic_restore_state(struct vcpu *v);
>  
> +#ifdef CONFIG_SYSTEM_SUSPEND
> +/* Suspend/resume */
> +extern int gic_suspend(void);
> +extern void gic_resume(void);
> +#endif /* CONFIG_SYSTEM_SUSPEND */
> +
>  /* SGI (AKA IPIs) */
>  enum gic_sgi {
>      GIC_SGI_EVENT_CHECK,
> @@ -395,6 +401,12 @@ struct gic_hw_operations {
>      int (*iomem_deny_access)(struct domain *d);
>      /* Handle LPIs, which require special handling */
>      void (*do_LPI)(unsigned int lpi);
> +#ifdef CONFIG_SYSTEM_SUSPEND
> +    /* Save GIC configuration due to the system suspend */
> +    int (*suspend)(void);
> +    /* Restore GIC configuration due to the system resume */
> +    void (*resume)(void);
> +#endif /* CONFIG_SYSTEM_SUSPEND */
>  };
>  
>  extern const struct gic_hw_operations *gic_hw_ops;

-- 
WBR, Volodymyr