Re: [Xen-devel] [RFC PATCH v1 04/10] arm/xen: move gic save and restore registers to gic driver

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Wed, 19 Mar 2014, vijay.kilari@xxxxxxxxx wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxxxxxxxxxx>
> 
> gic saved registers are moved to gic driver.
> This required structure is allocated at runtime
> and is saved & restored.
> 
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxxxxxxxxxx>
> ---
>  xen/arch/arm/domain.c        |    3 +++
>  xen/arch/arm/gic.c           |   36 +++++++++++++++++++++++++++++-------
>  xen/include/asm-arm/domain.h |    3 +--
>  xen/include/asm-arm/gic.h    |    1 +
>  4 files changed, 34 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 82a1e79..292716a 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -466,6 +466,9 @@ int vcpu_initialise(struct vcpu *v)
>      v->arch.saved_context.sp = (register_t)v->arch.cpu_info;
>      v->arch.saved_context.pc = (register_t)continue_new_vcpu;
>  
> +    if ( (rc = vcpu_gic_init(v)) != 0 )
> +        return rc;
> +
>      /* Idle VCPUs don't need the rest of this setup */
>      if ( is_idle_vcpu(v) )
>          return rc;
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 4be0897..973fcf9 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -41,6 +41,13 @@
>  #define GICH ((volatile uint32_t *) FIXMAP_ADDR(FIXMAP_GICH))
>  static void gic_restore_pending_irqs(struct vcpu *v);
>  
> +struct gic_state_data {
> +    uint32_t gic_hcr;
> +    uint32_t gic_vmcr;
> +    uint32_t gic_apr;
> +    uint32_t gic_lr[64];
> +};
> +
>  /* Global state */
>  static struct {
>      paddr_t dbase;       /* Address of distributor registers */
> @@ -98,6 +105,9 @@ irq_desc_t *__irq_to_desc(int irq)
>  void gic_save_state(struct vcpu *v)
>  {
>      int i;
> +    struct gic_state_data *d;
> +    d = (struct gic_state_data *)v->arch.gic_state;
> +
>      ASSERT(!local_irq_is_enabled());
>  
>      /* No need for spinlocks here because interrupts are disabled around
> @@ -105,10 +115,10 @@ void gic_save_state(struct vcpu *v)
>       * accessed simultaneously by another pCPU.
>       */
>      for ( i=0; i<nr_lrs; i++)
> -        v->arch.gic_lr[i] = GICH[GICH_LR + i];
> +        d->gic_lr[i] = GICH[GICH_LR + i];
>      v->arch.lr_mask = this_cpu(lr_mask);
> -    v->arch.gic_apr = GICH[GICH_APR];
> -    v->arch.gic_vmcr = GICH[GICH_VMCR];
> +    d->gic_apr = GICH[GICH_APR];
> +    d->gic_vmcr = GICH[GICH_VMCR];
>      /* Disable until next VCPU scheduled */
>      GICH[GICH_HCR] = 0;
>      isb();
> @@ -117,15 +127,17 @@ void gic_save_state(struct vcpu *v)
>  void gic_restore_state(struct vcpu *v)
>  {
>      int i;
> +    struct gic_state_data *d;
> +    d = (struct gic_state_data *)v->arch.gic_state;
>  
>      if ( is_idle_vcpu(v) )
>          return;
>  
>      this_cpu(lr_mask) = v->arch.lr_mask;
>      for ( i=0; i<nr_lrs; i++)
> -        GICH[GICH_LR + i] = v->arch.gic_lr[i];
> -    GICH[GICH_APR] = v->arch.gic_apr;
> -    GICH[GICH_VMCR] = v->arch.gic_vmcr;
> +        GICH[GICH_LR + i] = d->gic_lr[i];
> +    GICH[GICH_APR] = d->gic_apr;
> +    GICH[GICH_VMCR] = d->gic_vmcr;
>      GICH[GICH_HCR] = GICH_HCR_EN;
>      isb();
>  
> @@ -877,6 +889,14 @@ void gic_interrupt(struct cpu_user_regs *regs, int 
> is_fiq)
>      } while (1);
>  }
>  
> +int vcpu_gic_init(struct vcpu *v)
> +{
> +     v->arch.gic_state = xzalloc(struct gic_state_data);
> +     if(!v->arch.gic_state)
> +        return -ENOMEM;
> +     return 0;
> +}
> +
>  int gicv_setup(struct domain *d)
>  {
>      int ret;
> @@ -1001,6 +1021,8 @@ void gic_dump_info(struct vcpu *v)
>  {
>      int i;
>      struct pending_irq *p;
> +    struct gic_state_data *d;
> +    d = (struct gic_state_data *)v->arch.gic_state;
>  
>      printk("GICH_LRs (vcpu %d) mask=%"PRIx64"\n", v->vcpu_id, 
> v->arch.lr_mask);
>      if ( v == current )
> @@ -1009,7 +1031,7 @@ void gic_dump_info(struct vcpu *v)
>              printk("   HW_LR[%d]=%x\n", i, GICH[GICH_LR + i]);
>      } else {
>          for ( i = 0; i < nr_lrs; i++ )
> -            printk("   VCPU_LR[%d]=%x\n", i, v->arch.gic_lr[i]);
> +            printk("   VCPU_LR[%d]=%x\n", i, d->gic_lr[i]);
>      }
>  
>      list_for_each_entry ( p, &v->arch.vgic.inflight_irqs, inflight )
> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
> index d49ab68..38df789 100644
> --- a/xen/include/asm-arm/domain.h
> +++ b/xen/include/asm-arm/domain.h
> @@ -243,8 +243,7 @@ struct arch_vcpu
>      uint32_t csselr;
>      register_t vmpidr;
>  
> -    uint32_t gic_hcr, gic_vmcr, gic_apr;
> -    uint32_t gic_lr[64];
> +    void *gic_state;
>      uint64_t event_mask;
>      uint64_t lr_mask;
>  
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index 340ef73..debfab8 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -161,6 +161,7 @@ extern int domain_vgic_init(struct domain *d);
>  extern void domain_vgic_free(struct domain *d);
>  
>  extern int vcpu_vgic_init(struct vcpu *v);
> +extern int vcpu_gic_init(struct vcpu *v);
>  
>  extern void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq,int 
> virtual);
>  extern void vgic_clear_pending_irqs(struct vcpu *v);

Same here about avoiding void*.
We could move the definition of struct gic_state_data to gic.h and use
it in domain.h.  We can turn it into a union in the later patches.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel