Message ID | 55F15353.2000800@arm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 10/09/15 10:54, Marc Zyngier wrote: > Hi Julian, Hi Marc, > On 09/09/15 20:23, Julien Grall wrote: >> Hi, >> >> I've been trying the latest linus/master (a794b4f), which include this >> patch, as baremetal kernel on X-gene. This is failing on early boot >> without much log. >> >> After bisecting the tree, I found the error coming from this patch. >> While this patch is valid, it made me remembered that X-Gene (at least >> the first version) as an odd GICv2. >> >> The GICC is divided in 2 area of 4K, each one aligned at a 64KB address. >> This means that, the address of GICC_DIR won't be 0x1000 but 0x10000. > > Not really. I already mentioned that one a while ago: > > http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/332249.html Sorry I haven't seen this thread on the ML. > The first page of GIC is aliased over the first 64kB, and the second > page aliased over the second 64kB. So you get a consistent mapping if > you use (base + 0xF000) to address GICC. Also, the DT that's in > mainline is showing a 4kB CPU interface, which doesn't enable > EOImode==1. > You must be using a firmware that's newer than mine, since > I'm perfectly able to boot my Mustang with these patches. My U-boot firmware is: U-Boot 2013.04-mustang_sw_1.15.12 (May 20 2015 - 10:03:33) The interrupt controller node looks like: interrupt-controller@78090000 { reg = <0x0 0x78090000 0x0 0x10000 0x0 0x780a0000 0x0 0x20000 0x0 0x780c0000 0x0 0x10000 0x0 0x780e0000 0x0 0x20000>; interrupts = <0x1 0x9 0xf04>; compatible = "arm,cortex-a15-gic"; #interrupt-cells = <0x3>; phandle = <0x1>; interrupt-controller; linux,phandle = <0x1>; }; Note that we have a recent firmware which correct the GICD region to use the non-secure one rather than the secure. See [1] for more details. > >> We had the same issue on Xen when we did the first port of X-gene [1]. >> Although, we choose to add a quirk in Xen for this platform in order to >> map contiguously in the virtual memory the 2 part of GICC. >> >> Note that, back then, Ian suggested to extend the bindings to support a >> such platform [2]. AFAICT, there was no follow-up on it. > > The main problem here is not to update the binding, but the fact that > you *cannot* update the DT on x-gene (the firmware will replace your > GIC node with what it thinks it is), and the APM guys can't be bothered > to fix their stuff. > > In the meantime, can you give the following patch a shot? My Mustang is > wired to a 4kB CPU interface, so I'll need your help to test it. I applied the two patches on top of linus/master and I'm able to boot correctly on X-gene. Thank you! Regards, [1] http://lists.xen.org/archives/html/xen-devel/2015-04/msg02816.html
On 10/09/15 17:23, Julien Grall wrote: > On 10/09/15 10:54, Marc Zyngier wrote: [...] >> In the meantime, can you give the following patch a shot? My Mustang is >> wired to a 4kB CPU interface, so I'll need your help to test it. > > I applied the two patches on top of linus/master and I'm able to boot > correctly on X-gene. Thank you! Thanks for testing. Can I put your Tested-by tag on the patch when I send it to Thomas? Thanks, M.
On 10/09/15 17:30, Marc Zyngier wrote: > On 10/09/15 17:23, Julien Grall wrote: >> On 10/09/15 10:54, Marc Zyngier wrote: > > [...] > >>> In the meantime, can you give the following patch a shot? My Mustang is >>> wired to a 4kB CPU interface, so I'll need your help to test it. >> >> I applied the two patches on top of linus/master and I'm able to boot >> correctly on X-gene. Thank you! > > Thanks for testing. Can I put your Tested-by tag on the patch when I > send it to Thomas? Sure: Tested-by: Julien Grall <julien.grall@citrix.com> Regards,
On Thu, 2015-09-10 at 17:23 +0100, Julien Grall wrote: > I applied the two patches on top of linus/master and I'm able to boot > correctly on X-gene. Thank you! Perhaps we should replicate this approach in Xen and get rid of PLATFORM_QUIRK_GIC_64K_STRIDE? Ian. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/09/2015 11:54, Ian Campbell wrote: > On Thu, 2015-09-10 at 17:23 +0100, Julien Grall wrote: >> I applied the two patches on top of linus/master and I'm able to boot >> correctly on X-gene. Thank you! > > Perhaps we should replicate this approach in Xen and get rid of > PLATFORM_QUIRK_GIC_64K_STRIDE? I was thinking to do it. But, I wasn't sure if it was worth to get a such "ugly" patch compare to the quirk. Regards,
On 11/09/15 11:59, Julien Grall wrote: > > > On 11/09/2015 11:54, Ian Campbell wrote: >> On Thu, 2015-09-10 at 17:23 +0100, Julien Grall wrote: >>> I applied the two patches on top of linus/master and I'm able to boot >>> correctly on X-gene. Thank you! >> >> Perhaps we should replicate this approach in Xen and get rid of >> PLATFORM_QUIRK_GIC_64K_STRIDE? > > I was thinking to do it. But, I wasn't sure if it was worth to get a > such "ugly" patch compare to the quirk. It is not a quirk. It is actually recommended in the SBSA spec. The patch is ugly because we can't do the right thing on the one platform that actually implemented ARM's own recommendation (we can't tell the bloody firmware to stop overriding our DT). I would otherwise have added a "arm,use-sbsa-aliasing" property (or something similar) instead of trying to guess things... M.
On 11/09/15 12:09, Marc Zyngier wrote: > On 11/09/15 11:59, Julien Grall wrote: >> >> >> On 11/09/2015 11:54, Ian Campbell wrote: >>> On Thu, 2015-09-10 at 17:23 +0100, Julien Grall wrote: >>>> I applied the two patches on top of linus/master and I'm able to boot >>>> correctly on X-gene. Thank you! >>> >>> Perhaps we should replicate this approach in Xen and get rid of >>> PLATFORM_QUIRK_GIC_64K_STRIDE? >> >> I was thinking to do it. But, I wasn't sure if it was worth to get a >> such "ugly" patch compare to the quirk. > > It is not a quirk. It is actually recommended in the SBSA spec. The > patch is ugly because we can't do the right thing on the one platform > that actually implemented ARM's own recommendation (we can't tell the > bloody firmware to stop overriding our DT). > > I would otherwise have added a "arm,use-sbsa-aliasing" property (or > something similar) instead of trying to guess things... I will give a look to port this patch on Xen. Regards,
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index e6b7ed5..b62f2b2 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -1119,12 +1119,50 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, #ifdef CONFIG_OF static int gic_cnt __initdata; +static bool gic_check_eoimode(struct device_node *node, void __iomem **base) +{ + struct resource cpuif_res; + + of_address_to_resource(node, 1, &cpuif_res); + + if (!is_hyp_mode_available()) + return false; + if (resource_size(&cpuif_res) < SZ_8K) + return false; + if (resource_size(&cpuif_res) == SZ_128K) { + u32 val; + + /* + * Verify that we have a GIC400 aliased over the first + * 64kB by checking the GICC_IIDR register. + */ + val = readl_relaxed(*base + GIC_CPU_IDENT); + if (val != 0x0202043B) + return false; + + val = readl_relaxed(*base + GIC_CPU_IDENT + 0xF000); + if (val != 0x0202043B) + return false; + + /* + * Move the base up by 60kB, so that we have a 8kB + * contiguous region, which allows us to use GICC_DIR + * at its normal offset. + */ + *base += 0xF000; + cpuif_res.start += 0xF000; + pr_warn("GIC: Adjusting CPU interface base to %pa", + &cpuif_res.start); + } + + return true; +} + static int __init gic_of_init(struct device_node *node, struct device_node *parent) { void __iomem *cpu_base; void __iomem *dist_base; - struct resource cpu_res; u32 percpu_offset; int irq; @@ -1137,14 +1175,11 @@ gic_of_init(struct device_node *node, struct device_node *parent) cpu_base = of_iomap(node, 1); WARN(!cpu_base, "unable to map gic cpu registers\n"); - of_address_to_resource(node, 1, &cpu_res); - /* * Disable split EOI/Deactivate if either HYP is not available * or the CPU interface is too small. */ - if (gic_cnt == 0 && (!is_hyp_mode_available() || - resource_size(&cpu_res) < SZ_8K)) + if (gic_cnt == 0 && !gic_check_eoimode(node, &cpu_base)) static_key_slow_dec(&supports_deactivate); if (of_property_read_u32(node, "cpu-offset", &percpu_offset))