From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v2 3/7] xen/arm: Make gic-v2 code handle hip04-d01 platform Date: Tue, 04 Nov 2014 13:34:14 +0000 Message-ID: <5458D5D6.9090903@linaro.org> References: <1415033196-30529-1-git-send-email-frediano.ziglio@huawei.com> <1415033196-30529-4-git-send-email-frediano.ziglio@huawei.com> <5458D54F.10007@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5458D54F.10007@linaro.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Frediano Ziglio , Ian Campbell , Stefano Stabellini , Tim Deegan Cc: zoltan.kiss@huawei.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 11/04/2014 01:31 PM, Julien Grall wrote: > On 11/03/2014 04:46 PM, Frediano Ziglio wrote: >> The GIC in this platform is mainly compatible with the standard >> GICv2 beside: >> - ITARGET is extended to 16 bit to support 16 CPUs; >> - SGI mask is extended to support 16 CPUs; >> - maximum supported interrupt is 510. >> >> Signed-off-by: Frediano Ziglio >> Signed-off-by: Zoltan Kiss >> --- >> xen/arch/arm/gic-v2.c | 89 +++++++++++++++++++++++++++++++++++++++-------- >> xen/arch/arm/gic.c | 3 +- >> xen/include/asm-arm/gic.h | 4 ++- >> 3 files changed, 80 insertions(+), 16 deletions(-) >> >> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c >> index faad1ff..9461fe3 100644 >> --- a/xen/arch/arm/gic-v2.c >> +++ b/xen/arch/arm/gic-v2.c >> @@ -79,16 +79,23 @@ static struct gic_info gicv2_info; >> * logical CPU numbering. Let's use mapping as returned by the GIC >> * itself >> */ >> -static DEFINE_PER_CPU(u8, gic_cpu_id); >> +static DEFINE_PER_CPU(u16, gic_cpu_id); >> >> /* Maximum cpu interface per GIC */ >> -#define NR_GIC_CPU_IF 8 >> +static unsigned int nr_gic_cpu_if = 8; >> +static unsigned int gicd_sgi_target_shift = GICD_SGI_TARGET_SHIFT; >> +static unsigned int gic_cpu_mask = 0xff; >> >> static inline void writeb_gicd(uint8_t val, unsigned int offset) >> { >> writeb_relaxed(val, gicv2.map_dbase + offset); >> } >> >> +static inline void writew_gicd(uint16_t val, unsigned int offset) >> +{ >> + writew_relaxed(val, gicv2.map_dbase + offset); >> +} >> + >> static inline void writel_gicd(uint32_t val, unsigned int offset) >> { >> writel_relaxed(val, gicv2.map_dbase + offset); >> @@ -132,7 +139,7 @@ static unsigned int gicv2_cpu_mask(const cpumask_t *cpumask) >> cpumask_and(&possible_mask, cpumask, &cpu_possible_map); >> for_each_cpu( cpu, &possible_mask ) >> { >> - ASSERT(cpu < NR_GIC_CPU_IF); >> + ASSERT(cpu < nr_gic_cpu_if); >> mask |= per_cpu(gic_cpu_id, cpu); >> } >> >> @@ -203,6 +210,15 @@ static unsigned int gicv2_read_irq(void) >> return (readl_gicc(GICC_IAR) & GICC_IA_IRQ); >> } >> >> +/* Set target CPU mask (RAZ/WI on uniprocessor) */ >> +static void gicv2_set_irq_mask(int irq, unsigned int mask) >> +{ >> + if ( nr_gic_cpu_if == 16 ) > > This check is very confusing, and even more in patch #5. Sorry, I meant #7. > Code executed under this check describes your platform and not a generic > 16-CPU support (actually there is no spec for it). > > I would introduce a new boolean or hide this check in a macro. > >> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c >> index 70d10d6..8050a65 100644 >> --- a/xen/arch/arm/gic.c >> +++ b/xen/arch/arm/gic.c >> @@ -563,12 +563,13 @@ static void do_sgi(struct cpu_user_regs *regs, enum gic_sgi sgi) >> void gic_interrupt(struct cpu_user_regs *regs, int is_fiq) >> { >> unsigned int irq; >> + unsigned int max_irq = gic_hw_ops->info->nr_lines; >> >> do { >> /* Reading IRQ will ACK it */ >> irq = gic_hw_ops->read_irq(); >> >> - if ( likely(irq >= 16 && irq < 1021) ) >> + if ( likely(irq >= 16 && irq < max_irq) ) > > On the previous version I've asked that need to explain in the commit > message why this change is valid. > > Regards, > -- Julien Grall