From mboxrd@z Thu Jan 1 00:00:00 1970 From: Zoltan Kiss Subject: Re: [PATCH] xen/arm: gic-hip04: Resync the driver with the GICv2 Date: Thu, 07 May 2015 09:52:41 +0100 Message-ID: <554B27D9.6020104@linaro.org> References: <1430938350-23985-1-git-send-email-julien.grall@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YqHXx-0002ri-FT for xen-devel@lists.xenproject.org; Thu, 07 May 2015 08:52:45 +0000 Received: by wief7 with SMTP id f7so8463403wie.0 for ; Thu, 07 May 2015 01:52:43 -0700 (PDT) In-Reply-To: <1430938350-23985-1-git-send-email-julien.grall@citrix.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: Julien Grall , xen-devel@lists.xenproject.org Cc: tim@xen.org, stefano.stabellini@citrix.com, ian.campbell@citrix.com, Zoltan Kiss List-Id: xen-devel@lists.xenproject.org Looks good at first glance, let me try it on a board. On 06/05/15 19:52, Julien Grall wrote: > The GIC hip04 driver was differring from GICv2. I suspect that some of > the changes in the common GIC code make boot fail on hip04. Although, I > don't have a platform to check so it has been only build tested. > > List of GICv2 commit ported to the HIP04: > commit ce12e6dba4b2d120e35dffd95a745452224e7144 > Author: Edgar E. Iglesias > Date: Fri Apr 10 16:21:10 2015 +1000 > > xen/arm: Don't write to GICH_MISR > > GICH_MISR is read-only in GICv2. > > Signed-off-by: Edgar E. Iglesias > Reviewed-by: Julien Grall > Acked-by: Ian Campbell > > commit 2eb4f996547dc632aa94b2b7b4f783bec8ffe457 > Author: Julien Grall > Date: Wed Apr 1 17:21:47 2015 +0100 > > xen/arm: gic: GICv2 & GICv3 only supports 1020 physical interrupts > > GICD_TYPER.ITLinesNumber can encode up to 1024 interrupts. Although, > IRQ 1020-1023 are reserved for special purpose. > > The result is used by the callers of gic_number_lines in order to check > the validity of an IRQ. > > Signed-off-by: Julien Grall > Acked-by: Ian Campbell > Cc: Frediano Ziglio > Cc: Zoltan Kiss > > commit e2d486b385ce58b6db7561417de28ba837dcd4ac > Author: Julien Grall > Date: Wed Apr 1 17:21:34 2015 +0100 > > xen/arm: Divide GIC initialization in 2 parts > > Currently the function to translate IRQ from the device tree is set > unconditionally to be able to be able to retrieve serial/timer IRQ > before the GIC has been initialized. > > It assumes that the xlate function won't ever changed. We may also need > to have the primary interrupt controller very early. > > Rework the gic initialization in 2 parts: > - gic_preinit: Get the interrupt controller device tree node and > set up GIC and xlate callbacks > - gic_init: Initialize the interrupt controller and the boot CPU > interrupts. > > The former function will be called just after the IRQ subsystem as been > initialized. > > Signed-off-by: Julien Grall > Acked-by: Stefano Stabellini > Acked-by: Ian Campbell > Cc: Frediano Ziglio > Cc: Zoltan Kiss > > Signed-off-by: Julien Grall > Cc: Zoltan Kiss > --- > > So far I've got no news about previous changes in the GIC [1] applied in > April... Since then the driver is likely broken and would prevent Xen to > boot on a supported platform. This patch is trying to fix it even though > I don't have a board to test. > > It's a prerequisite for an upcoming series which update/rework the > vGICv2 drivers and will impact the GICv2 drivers (~10 patches). > > I'm concerned to see a newly driver (pushed last march) already orphan. > Does Huawei still plan to maintain this driver? > > [1] http://lists.xenproject.org/archives/html/xen-devel/2015-03/msg02465.html > --- > xen/arch/arm/gic-hip04.c | 89 ++++++++++++++++++++++++++---------------------- > 1 file changed, 48 insertions(+), 41 deletions(-) > > diff --git a/xen/arch/arm/gic-hip04.c b/xen/arch/arm/gic-hip04.c > index 073ad33..efdccdb 100644 > --- a/xen/arch/arm/gic-hip04.c > +++ b/xen/arch/arm/gic-hip04.c > @@ -267,6 +267,7 @@ static void __init hip04gic_dist_init(void) > uint32_t type; > uint32_t cpumask; > uint32_t gic_cpus; > + unsigned int nr_lines; > int i; > > cpumask = readl_gicd(GICD_ITARGETSR) & 0xffff; > @@ -276,31 +277,34 @@ static void __init hip04gic_dist_init(void) > writel_gicd(0, GICD_CTLR); > > type = readl_gicd(GICD_TYPER); > - gicv2_info.nr_lines = 32 * ((type & GICD_TYPE_LINES) + 1); > + nr_lines = 32 * ((type & GICD_TYPE_LINES) + 1); > gic_cpus = 16; > printk("GIC-HIP04: %d lines, %d cpu%s%s (IID %8.8x).\n", > - gicv2_info.nr_lines, gic_cpus, (gic_cpus == 1) ? "" : "s", > + nr_lines, gic_cpus, (gic_cpus == 1) ? "" : "s", > (type & GICD_TYPE_SEC) ? ", secure" : "", > readl_gicd(GICD_IIDR)); > > /* Default all global IRQs to level, active low */ > - for ( i = 32; i < gicv2_info.nr_lines; i += 16 ) > + for ( i = 32; i < nr_lines; i += 16 ) > writel_gicd(0x0, GICD_ICFGR + (i / 16) * 4); > > /* Route all global IRQs to this CPU */ > - for ( i = 32; i < gicv2_info.nr_lines; i += 2 ) > + for ( i = 32; i < nr_lines; i += 2 ) > writel_gicd(cpumask, GICD_ITARGETSR + (i / 2) * 4); > > /* Default priority for global interrupts */ > - for ( i = 32; i < gicv2_info.nr_lines; i += 4 ) > + for ( i = 32; i < nr_lines; i += 4 ) > writel_gicd(GIC_PRI_IRQ << 24 | GIC_PRI_IRQ << 16 | > GIC_PRI_IRQ << 8 | GIC_PRI_IRQ, > GICD_IPRIORITYR + (i / 4) * 4); > > /* Disable all global interrupts */ > - for ( i = 32; i < gicv2_info.nr_lines; i += 32 ) > + for ( i = 32; i < nr_lines; i += 32 ) > writel_gicd(~0x0, GICD_ICENABLER + (i / 32) * 4); > > + /* Only 1020 interrupts are supported */ > + gicv2_info.nr_lines = min(1020U, nr_lines); > + > /* Turn on the distributor */ > writel_gicd(GICD_CTL_ENABLE, GICD_CTLR); > } > @@ -351,8 +355,6 @@ static void __cpuinit hip04gic_hyp_init(void) > vtr = readl_gich(GICH_VTR); > nr_lrs = (vtr & GICH_V2_VTR_NRLRGS) + 1; > gicv2_info.nr_lrs = nr_lrs; > - > - writel_gich(GICH_MISR_EOI, GICH_MISR); > } > > static void __cpuinit hip04gic_hyp_disable(void) > @@ -688,37 +690,10 @@ static hw_irq_controller hip04gic_guest_irq_type = { > .set_affinity = hip04gic_irq_set_affinity, > }; > > -const static struct gic_hw_operations hip04gic_ops = { > - .info = &gicv2_info, > - .secondary_init = hip04gic_secondary_cpu_init, > - .save_state = hip04gic_save_state, > - .restore_state = hip04gic_restore_state, > - .dump_state = hip04gic_dump_state, > - .gicv_setup = hip04gicv_setup, > - .gic_host_irq_type = &hip04gic_host_irq_type, > - .gic_guest_irq_type = &hip04gic_guest_irq_type, > - .eoi_irq = hip04gic_eoi_irq, > - .deactivate_irq = hip04gic_dir_irq, > - .read_irq = hip04gic_read_irq, > - .set_irq_properties = hip04gic_set_irq_properties, > - .send_SGI = hip04gic_send_SGI, > - .disable_interface = hip04gic_disable_interface, > - .update_lr = hip04gic_update_lr, > - .update_hcr_status = hip04gic_hcr_status, > - .clear_lr = hip04gic_clear_lr, > - .read_lr = hip04gic_read_lr, > - .write_lr = hip04gic_write_lr, > - .read_vmcr_priority = hip04gic_read_vmcr_priority, > - .read_apr = hip04gic_read_apr, > - .make_dt_node = hip04gic_make_dt_node, > -}; > - > -/* Set up the GIC */ > -static int __init hip04gic_init(struct dt_device_node *node, const void *data) > +static int __init hip04gic_init(void) > { > int res; > - > - dt_device_set_used_by(node, DOMID_XEN); > + const struct dt_device_node *node = gicv2_info.node; > > res = dt_device_get_address(node, 0, &gicv2.dbase, NULL); > if ( res || !gicv2.dbase || (gicv2.dbase & ~PAGE_MASK) ) > @@ -741,9 +716,6 @@ static int __init hip04gic_init(struct dt_device_node *node, const void *data) > panic("GIC-HIP04: Cannot find the maintenance IRQ"); > gicv2_info.maintenance_irq = res; > > - /* Set the GIC as the primary interrupt controller */ > - dt_interrupt_controller = node; > - > /* TODO: Add check on distributor, cpu size */ > > printk("GIC-HIP04 initialization:\n" > @@ -788,8 +760,43 @@ static int __init hip04gic_init(struct dt_device_node *node, const void *data) > > spin_unlock(&gicv2.lock); > > + return 0; > +} > + > +const static struct gic_hw_operations hip04gic_ops = { > + .info = &gicv2_info, > + .init = hip04gic_init, > + .secondary_init = hip04gic_secondary_cpu_init, > + .save_state = hip04gic_save_state, > + .restore_state = hip04gic_restore_state, > + .dump_state = hip04gic_dump_state, > + .gicv_setup = hip04gicv_setup, > + .gic_host_irq_type = &hip04gic_host_irq_type, > + .gic_guest_irq_type = &hip04gic_guest_irq_type, > + .eoi_irq = hip04gic_eoi_irq, > + .deactivate_irq = hip04gic_dir_irq, > + .read_irq = hip04gic_read_irq, > + .set_irq_properties = hip04gic_set_irq_properties, > + .send_SGI = hip04gic_send_SGI, > + .disable_interface = hip04gic_disable_interface, > + .update_lr = hip04gic_update_lr, > + .update_hcr_status = hip04gic_hcr_status, > + .clear_lr = hip04gic_clear_lr, > + .read_lr = hip04gic_read_lr, > + .write_lr = hip04gic_write_lr, > + .read_vmcr_priority = hip04gic_read_vmcr_priority, > + .read_apr = hip04gic_read_apr, > + .make_dt_node = hip04gic_make_dt_node, > +}; > + > +/* Set up the GIC */ > +static int __init hip04gic_preinit(struct dt_device_node *node, > + const void *data) > +{ > gicv2_info.hw_version = GIC_V2; > + gicv2_info.node = node; > register_gic_ops(&hip04gic_ops); > + dt_irq_xlate = gic_irq_xlate; > > return 0; > } > @@ -802,7 +809,7 @@ static const struct dt_device_match hip04gic_dt_match[] __initconst = > > DT_DEVICE_START(hip04gic, "GIC-HIP04", DEVICE_GIC) > .dt_match = hip04gic_dt_match, > - .init = hip04gic_init, > + .init = hip04gic_preinit, > DT_DEVICE_END > > /* >