| Message ID | 20170313180432.7067-2-krzk@kernel.org |
|---|---|
| State | New |
| Headers | show |
On 03/13/2017 03:04 PM, Krzysztof Kozlowski wrote: > Recent Linux kernel (tested next-20170224) was complaining about missing > GIC mask and was unable to bring up secondary CPU: > > [ 0.000000] NR_IRQS:16 nr_irqs:16 16 > [ 0.000000] GIC CPU mask not found - kernel will fail to boot. > ... > [ 0.400492] smp: Bringing up secondary CPUs ... > [ 1.413184] CPU1: failed to boot: -110 > [ 1.423981] smp: Brought up 1 node, 1 CPU > > In its instance_init() call, the Exynos GIC driver was setting GIC > memory mappings for each CPU, from 1 up to "num-cpu" property. The > Exynos4210 machine init call on the other hand, first created Exynos GIC > device and then set the "num-cpu" property which was too late. The init > already happened with default "num-cpu" value of 1 thus GIC mappings > were created only for the first CPU. > > Split the Exynos GIC init code into realize function so the code will > see updated "num-cpu" property. This fixes the warning and brings > second CPU: > [ 0.435780] CPU1: thread -1, cpu 1, socket 9, mpidr 80000901 > [ 0.451838] smp: Brought up 1 node, 2 CPUs > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > hw/intc/exynos4210_gic.c | 21 ++++++++++++++------- > 1 file changed, 14 insertions(+), 7 deletions(-) > > diff --git a/hw/intc/exynos4210_gic.c b/hw/intc/exynos4210_gic.c > index 2a55817b7660..222cfd6c6387 100644 > --- a/hw/intc/exynos4210_gic.c > +++ b/hw/intc/exynos4210_gic.c > @@ -283,9 +283,20 @@ static void exynos4210_gic_set_irq(void *opaque, int irq, int level) > > static void exynos4210_gic_init(Object *obj) > { > - DeviceState *dev = DEVICE(obj); > Exynos4210GicState *s = EXYNOS4210_GIC(obj); > - SysBusDevice *sbd = SYS_BUS_DEVICE(obj); > + > + memory_region_init(&s->cpu_container, obj, "exynos4210-cpu-container", > + EXYNOS4210_EXT_GIC_CPU_REGION_SIZE); > + memory_region_init(&s->dist_container, obj, "exynos4210-dist-container", > + EXYNOS4210_EXT_GIC_DIST_REGION_SIZE); > + > +} > + > +static void exynos4210_gic_realize(DeviceState *dev, Error **errp) > +{ > + Exynos4210GicState *s = EXYNOS4210_GIC(dev); > + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); > + Object *obj = OBJECT(dev); > uint32_t i; > const char cpu_prefix[] = "exynos4210-gic-alias_cpu"; > const char dist_prefix[] = "exynos4210-gic-alias_dist"; > @@ -306,11 +317,6 @@ static void exynos4210_gic_init(Object *obj) > qdev_init_gpio_in(dev, exynos4210_gic_set_irq, > EXYNOS4210_GIC_NIRQ - 32); > > - memory_region_init(&s->cpu_container, obj, "exynos4210-cpu-container", > - EXYNOS4210_EXT_GIC_CPU_REGION_SIZE); > - memory_region_init(&s->dist_container, obj, "exynos4210-dist-container", > - EXYNOS4210_EXT_GIC_DIST_REGION_SIZE); > - > for (i = 0; i < s->num_cpu; i++) { > /* Map CPU interface per SMP Core */ > sprintf(cpu_alias_name, "%s%x", cpu_prefix, i); > @@ -346,6 +352,7 @@ static void exynos4210_gic_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > > + dc->realize = exynos4210_gic_realize; > dc->props = exynos4210_gic_properties; > } > >
On Mon, Mar 13, 2017 at 04:32:40PM -0300, Philippe Mathieu-Daudé wrote: > On 03/13/2017 03:04 PM, Krzysztof Kozlowski wrote: > > Recent Linux kernel (tested next-20170224) was complaining about missing > > GIC mask and was unable to bring up secondary CPU: > > > > [ 0.000000] NR_IRQS:16 nr_irqs:16 16 > > [ 0.000000] GIC CPU mask not found - kernel will fail to boot. > > ... > > [ 0.400492] smp: Bringing up secondary CPUs ... > > [ 1.413184] CPU1: failed to boot: -110 > > [ 1.423981] smp: Brought up 1 node, 1 CPU > > > > In its instance_init() call, the Exynos GIC driver was setting GIC > > memory mappings for each CPU, from 1 up to "num-cpu" property. The > > Exynos4210 machine init call on the other hand, first created Exynos GIC > > device and then set the "num-cpu" property which was too late. The init > > already happened with default "num-cpu" value of 1 thus GIC mappings > > were created only for the first CPU. > > > > Split the Exynos GIC init code into realize function so the code will > > see updated "num-cpu" property. This fixes the warning and brings > > second CPU: > > [ 0.435780] CPU1: thread -1, cpu 1, socket 9, mpidr 80000901 > > [ 0.451838] smp: Brought up 1 node, 2 CPUs > > > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Beside bringing secondary CPU, this patch fixes also Software Generated Interrupts. Without it, none of the SGIs are coming (except CPU wakeup): IPI0: 6281 CPU wakeup interrupts IPI1: 0 Timer broadcast interrupts IPI2: 0 Rescheduling interrupts IPI3: 0 Function call interrupts IPI4: 0 CPU stop interrupts IPI5: 0 IRQ work interrupts IPI6: 0 completion interrupts This is pretty annoying because lack of SGIs means lack of IPIs thus for example IRQ work cannot be executed. Without IRQ work, the kernel hangs on power down on cpufreq shutdown because in cpufreq_dbs_governor_stop() there is irq_work_sync() but none of irq_work interrupts were handled. I still did not solve the issue with cpuidle (AFTR). I am trying to implement CPU power off (needed for AFTR) but it is one-way so far (no wakeup). Anyway the workaround is to just disable cpuidle. Overall, what is the status of this patch? Should I resend? I can also extend the commit description with paragraph about SGI/IPI. Best regards, Krzysztof
diff --git a/hw/intc/exynos4210_gic.c b/hw/intc/exynos4210_gic.c index 2a55817b7660..222cfd6c6387 100644 --- a/hw/intc/exynos4210_gic.c +++ b/hw/intc/exynos4210_gic.c @@ -283,9 +283,20 @@ static void exynos4210_gic_set_irq(void *opaque, int irq, int level) static void exynos4210_gic_init(Object *obj) { - DeviceState *dev = DEVICE(obj); Exynos4210GicState *s = EXYNOS4210_GIC(obj); - SysBusDevice *sbd = SYS_BUS_DEVICE(obj); + + memory_region_init(&s->cpu_container, obj, "exynos4210-cpu-container", + EXYNOS4210_EXT_GIC_CPU_REGION_SIZE); + memory_region_init(&s->dist_container, obj, "exynos4210-dist-container", + EXYNOS4210_EXT_GIC_DIST_REGION_SIZE); + +} + +static void exynos4210_gic_realize(DeviceState *dev, Error **errp) +{ + Exynos4210GicState *s = EXYNOS4210_GIC(dev); + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); + Object *obj = OBJECT(dev); uint32_t i; const char cpu_prefix[] = "exynos4210-gic-alias_cpu"; const char dist_prefix[] = "exynos4210-gic-alias_dist"; @@ -306,11 +317,6 @@ static void exynos4210_gic_init(Object *obj) qdev_init_gpio_in(dev, exynos4210_gic_set_irq, EXYNOS4210_GIC_NIRQ - 32); - memory_region_init(&s->cpu_container, obj, "exynos4210-cpu-container", - EXYNOS4210_EXT_GIC_CPU_REGION_SIZE); - memory_region_init(&s->dist_container, obj, "exynos4210-dist-container", - EXYNOS4210_EXT_GIC_DIST_REGION_SIZE); - for (i = 0; i < s->num_cpu; i++) { /* Map CPU interface per SMP Core */ sprintf(cpu_alias_name, "%s%x", cpu_prefix, i); @@ -346,6 +352,7 @@ static void exynos4210_gic_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); + dc->realize = exynos4210_gic_realize; dc->props = exynos4210_gic_properties; }
Recent Linux kernel (tested next-20170224) was complaining about missing GIC mask and was unable to bring up secondary CPU: [ 0.000000] NR_IRQS:16 nr_irqs:16 16 [ 0.000000] GIC CPU mask not found - kernel will fail to boot. ... [ 0.400492] smp: Bringing up secondary CPUs ... [ 1.413184] CPU1: failed to boot: -110 [ 1.423981] smp: Brought up 1 node, 1 CPU In its instance_init() call, the Exynos GIC driver was setting GIC memory mappings for each CPU, from 1 up to "num-cpu" property. The Exynos4210 machine init call on the other hand, first created Exynos GIC device and then set the "num-cpu" property which was too late. The init already happened with default "num-cpu" value of 1 thus GIC mappings were created only for the first CPU. Split the Exynos GIC init code into realize function so the code will see updated "num-cpu" property. This fixes the warning and brings second CPU: [ 0.435780] CPU1: thread -1, cpu 1, socket 9, mpidr 80000901 [ 0.451838] smp: Brought up 1 node, 2 CPUs Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/exynos4210_gic.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)