From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v3 10/16] xen/arm: move vgic rank data to gic header file Date: Tue, 15 Apr 2014 20:10:15 +0100 Message-ID: <534D8417.6010108@linaro.org> References: <1397560675-29861-1-git-send-email-vijay.kilari@gmail.com> <1397560675-29861-11-git-send-email-vijay.kilari@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1397560675-29861-11-git-send-email-vijay.kilari@gmail.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: vijay.kilari@gmail.com Cc: Ian.Campbell@citrix.com, stefano.stabellini@eu.citrix.com, Prasun.Kapoor@caviumnetworks.com, vijaya.kumar@caviumnetworks.com, xen-devel@lists.xen.org, stefano.stabellini@citrix.com List-Id: xen-devel@lists.xenproject.org Hello Vijaya, Thank you for the patch. On 04/15/2014 12:17 PM, vijay.kilari@gmail.com wrote: > diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c > index 4830b5d..f73247d 100644 > --- a/xen/arch/arm/vgic.c > +++ b/xen/arch/arm/vgic.c > @@ -30,6 +30,7 @@ > #include > #include > #include > +#include > > #define REG(n) (n/4) > > @@ -69,7 +70,7 @@ static struct vgic_irq_rank *vgic_irq_rank(struct vcpu *v, int b, int n) > int rank = REG_RANK_NR(b, n); > > if ( rank == 0 ) > - return &v->arch.vgic.private_irqs; > + return v->arch.vgic.private_irqs; > else if ( rank <= DOMAIN_NR_RANKS(v->domain) ) > return &v->domain->arch.vgic.shared_irqs[rank - 1]; > else > @@ -117,9 +118,14 @@ void domain_vgic_free(struct domain *d) > int vcpu_vgic_init(struct vcpu *v) > { > int i; > - memset(&v->arch.vgic.private_irqs, 0, sizeof(v->arch.vgic.private_irqs)); > > - spin_lock_init(&v->arch.vgic.private_irqs.lock); > + v->arch.vgic.private_irqs = xzalloc(struct vgic_irq_rank); > + if ( v->arch.vgic.private_irqs == NULL ) > + return -ENOMEM; > + > + memset(v->arch.vgic.private_irqs, 0, sizeof(v->arch.vgic.private_irqs)); > + xzalloc will zeroed the structure for you. You don't need to zeroed again. BTW the sizeof was wrong here :). > + spin_lock_init(&v->arch.vgic.private_irqs->lock); > > memset(&v->arch.vgic.pending_irqs, 0, sizeof(v->arch.vgic.pending_irqs)); > for (i = 0; i < 32; i++) > @@ -130,7 +136,7 @@ int vcpu_vgic_init(struct vcpu *v) > > /* For SGI and PPI the target is always this CPU */ > for ( i = 0 ; i < 8 ; i++ ) > - v->arch.vgic.private_irqs.itargets[i] = > + v->arch.vgic.private_irqs->itargets[i] = > (1<<(v->vcpu_id+0)) > | (1<<(v->vcpu_id+8)) > | (1<<(v->vcpu_id+16)) > @@ -142,6 +148,12 @@ int vcpu_vgic_init(struct vcpu *v) > return 0; > } > > +int vcpu_vgic_free(struct vcpu *v) > +{ > + xfree(v->arch.vgic.private_irqs); Can you add a newline here for clarity? > + return 0; > +} > + > #define vgic_lock(v) spin_lock_irq(&(v)->domain->arch.vgic.lock) > #define vgic_unlock(v) spin_unlock_irq(&(v)->domain->arch.vgic.lock) > [..] > diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h > new file mode 100644 > index 0000000..104a87d > --- /dev/null > +++ b/xen/include/asm-arm/vgic.h > @@ -0,0 +1,40 @@ > +/* > + * ARM Virtual Generic Interrupt Controller support > + * > + * Ian Campbell > + * Copyright (c) 2011 Citrix Systems. > + * Hmmm... where does the copyright come from? > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#ifndef __ASM_ARM_VGIC_H__ > +#define __ASM_ARM_VGIC_H__ > + > +/* Represents state corresponding to a block of 32 interrupts */ > +struct vgic_irq_rank { > + spinlock_t lock; /* Covers access to all other members of this struct */ > + uint32_t ienable, iactive, ipend, pendsgi; > + uint32_t icfg[2]; > + uint32_t ipriority[8]; > + uint32_t itargets[8]; > +}; > + > +extern int vcpu_vgic_free(struct vcpu *v); > +#endif #endif /* __ASM_ARM_VGIC_H__ */ Regards, -- Julien Grall