| Message ID | 1429872417-20506-1-git-send-email-pranavkumar@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi Julien, On 25 April 2015 at 21:48, Julien Grall <julien.grall@citrix.com> wrote: > Hi Pranav, > > On 24/04/2015 15:46, 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 >> >> ChangeLog: >> >> V2: >> - Fine tune interrupt controller node search as per comments on V1 patch >> - Incorporating other misc comments on V1. >> V1: >> - Initial patch. > > > The changelog should be separated from the commit message by a "---" follow > by a new line. So git automatically will stripped the changelog when Ian > will apply the patch to Xen git. Thanks I will fix this along with other comments to post v3. - Pranav > > >> Tested-by: Christoffer Dall <christoffer.dall@linaro.org> >> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> >> --- >> xen/arch/arm/platforms/xgene-storm.c | 42 >> +++++++++++++++++++++++++++++++++- >> 1 file changed, 41 insertions(+), 1 deletion(-) >> >> diff --git a/xen/arch/arm/platforms/xgene-storm.c >> b/xen/arch/arm/platforms/xgene-storm.c >> index 1812e5b..c9a6dfc 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,46 @@ 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 __read_mostly xgene_quirks = PLATFORM_QUIRK_GIC_64K_STRIDE; >> + >> +static void __init xgene_check_pirq_eoi(void) >> +{ >> + struct dt_device_node *node; > > > I think this can be const. > >> + int res; >> + paddr_t dbase; >> + static const struct dt_device_match xgene_dt_int_ctrl_match[] = > > > The static is not necessary here. > >> + { >> + DT_MATCH_COMPATIBLE("arm,cortex-a15-gic"), >> + { /*sentinel*/ }, >> + }; >> + >> + node = dt_find_interrupt_controller(xgene_dt_int_ctrl_match); >> + if ( !node ) >> + panic("%s: Can not find interrupt controller node\n", __func__); > > > s/Can not/Cannot/ ? > > Panic will add a newline. So it's not necessary here. > > Although I'm not sure if we should use panic here. All the platform code is > returning an error code which will be handled to an upper function in the > stack (currently it's platform_init). > > I don't have a strong opinion on it. I will let the ARM maintainers decide. > >> + >> + res = dt_device_get_address(node, 0, &dbase, NULL); >> + if ( !dbase ) >> + panic("%s: Cannot find a valid address for the " >> + "distributor", __func__); > > > The indentation looks wrong here. Also, I'm not sure why you split the > message. The line won't be longer than 80 columns. > >> + >> + /* >> + * 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 ) >> + { >> + xgene_quirks |= PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI; >> + printk("Xen: warning: Using OLD X-Gene Firmware," >> + "disabling PIRQ EOI mode ...\n"); > > > Indentation > > Regards, > > -- > Julien Grall
diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c index 1812e5b..c9a6dfc 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,46 @@ 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 __read_mostly xgene_quirks = PLATFORM_QUIRK_GIC_64K_STRIDE; + +static void __init xgene_check_pirq_eoi(void) +{ + struct dt_device_node *node; + int res; + paddr_t dbase; + static const struct dt_device_match xgene_dt_int_ctrl_match[] = + { + DT_MATCH_COMPATIBLE("arm,cortex-a15-gic"), + { /*sentinel*/ }, + }; + + node = dt_find_interrupt_controller(xgene_dt_int_ctrl_match); + if ( !node ) + panic("%s: Can not find interrupt controller node\n", __func__); + + 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 ) + { + xgene_quirks |= PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI; + printk("Xen: warning: Using OLD X-Gene Firmware," + "disabling PIRQ EOI mode ...\n"); + } +} + static uint32_t xgene_storm_quirks(void) { - return PLATFORM_QUIRK_GIC_64K_STRIDE|PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI; + return xgene_quirks; } static int map_one_mmio(struct domain *d, const char *what, @@ -216,6 +254,8 @@ static int xgene_storm_init(void) reset_mask = XGENE_RESET_MASK; reset_vals_valid = true; + xgene_check_pirq_eoi(); + return 0; }