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

[Vijay Kilari <vijay.kilari@xxxxxxxxx>]

On Thu, Mar 20, 2014 at 10:53 PM, Stefano Stabellini
<stefano.stabellini@xxxxxxxxxxxxx> wrote:
> 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.
Since I made all the changes together, I have directly moved this
to v2 file. Yes, I will try to make it union

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