| Message ID | 1428310441-5412-1-git-send-email-pranavkumar@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi Pranav, On Mon, Apr 06, 2015 at 02:24:01PM +0530, Pranavkumar Sawargaonkar wrote: > In old X-Gene Storm firmware and DT, secure mode addresses have been > mentioned in GICv2 node. In this case maintenance interrupt is used > instead of EOI HW method. > > This patch checks the GIC Distributor Base Address to enable EOI quirk > for old firmware. > > Ref: > http://lists.xen.org/archives/html/xen-devel/2014-07/msg01263.html > > Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> I have tested this on the m400 cluster and can confirm that it prevents the issue with ab trashing the machine: Tested-by: Christoffer Dall <christoffer.dall@linaro.org>
On 8 April 2015 at 13:58, Christoffer Dall <christoffer.dall@linaro.org> wrote: > Hi Pranav, > > On Mon, Apr 06, 2015 at 02:24:01PM +0530, Pranavkumar Sawargaonkar wrote: >> In old X-Gene Storm firmware and DT, secure mode addresses have been >> mentioned in GICv2 node. In this case maintenance interrupt is used >> instead of EOI HW method. >> >> This patch checks the GIC Distributor Base Address to enable EOI quirk >> for old firmware. >> >> Ref: >> http://lists.xen.org/archives/html/xen-devel/2014-07/msg01263.html >> >> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> > > I have tested this on the m400 cluster and can confirm that it prevents > the issue with ab trashing the machine: > > Tested-by: Christoffer Dall <christoffer.dall@linaro.org> Thanks Christoffer for cross checking. will sent out a new revision of the patch with comments addressed. Thanks, Pranav
Hi Ian, On 15 April 2015 at 21:46, Ian Campbell <ian.campbell@citrix.com> wrote: > On Tue, 2015-04-07 at 11:29 +0100, Julien Grall wrote: >> >> This would avoid to have this loop and rely on there is always only one >> >> interrupt controller in the DT. >> > >> > That is true, however we do know that on this SoC there is only one GIC, >> > so it might be acceptable to rely on this knowledge: this is already >> > very platform specific code. >> >> If we keep the loop, I would add a comment on the above the loop >> explaining that we rely on there is always only GIC. >> >> Although, I was wondering if we could simply rely on the node path to >> get the DT node? > > If we know that all $OLD firmwares used the same path (or small set of > paths) then that could work. > > Or the loop could be replaced with dt_find_interrupt_controller, passing > the known compatible value for the systems in question. Yeah that is good option as there is no change in the path between firmware revisions. Thanks, Pranav > > Ian. >
diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c index eee650e..dd7cbfc 100644 --- a/xen/arch/arm/platforms/xgene-storm.c +++ b/xen/arch/arm/platforms/xgene-storm.c @@ -22,6 +22,7 @@ #include <asm/platform.h> #include <xen/stdbool.h> #include <xen/vmap.h> +#include <xen/device_tree.h> #include <asm/io.h> #include <asm/gic.h> @@ -35,9 +36,41 @@ static u64 reset_addr, reset_size; static u32 reset_mask; static bool reset_vals_valid = false; +#define XGENE_SEC_GICV2_DIST_ADDR 0x78010000 +static u32 quirk_guest_pirq_need_eoi; + +static void xgene_check_pirq_eoi(void) +{ + struct dt_device_node *node; + int res; + paddr_t dbase; + + dt_for_each_device_node( dt_host, node ) + { + if ( !dt_get_property(node, "interrupt-controller", NULL) ) + continue; + + res = dt_device_get_address(node, 0, &dbase, NULL); + if ( !dbase ) + panic("%s: Cannot find a valid address for the " + "distributor", __func__); + + /* + * In old X-Gene Storm firmware and DT, secure mode addresses have + * been mentioned in GICv2 node. We have to use maintenance interrupt + * instead of EOI HW in this case. We check the GIC Distributor Base + * Address to maintain compatibility with older firmware. + */ + if (dbase == XGENE_SEC_GICV2_DIST_ADDR) + quirk_guest_pirq_need_eoi = PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI; + else + quirk_guest_pirq_need_eoi = 0; + } +} + static uint32_t xgene_storm_quirks(void) { - return PLATFORM_QUIRK_GIC_64K_STRIDE|PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI; + return PLATFORM_QUIRK_GIC_64K_STRIDE| quirk_guest_pirq_need_eoi; } static int map_one_mmio(struct domain *d, const char *what, @@ -216,6 +249,8 @@ static int xgene_storm_init(void) reset_mask = XGENE_RESET_MASK; reset_vals_valid = true; + xgene_check_pirq_eoi(); + return 0; }
In old X-Gene Storm firmware and DT, secure mode addresses have been mentioned in GICv2 node. In this case maintenance interrupt is used instead of EOI HW method. This patch checks the GIC Distributor Base Address to enable EOI quirk for old firmware. Ref: http://lists.xen.org/archives/html/xen-devel/2014-07/msg01263.html Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> --- xen/arch/arm/platforms/xgene-storm.c | 37 +++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-)