| Message ID | 1491508074-31647-5-git-send-email-cjp256@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 6 Apr 2017, Chris Patterson wrote: > From: "Chris Patterson" <pattersonc@ainfosec.com> > > Tegra devices have a legacy interrupt controller (lic, or ictlr) that > must be programmed in parallel with their primary GIC. For all intents > and purposes, we treat these devices attached to this controller as > connected to the primary GIC, as it will be handling their interrupts. > > This commit adds support for exposing the ictlr to the hardware domain; > but a future commit will extend this to support exposing a virtualized > version of the ictlr to the hardware domain, and to ensure that > interrupts are unmasked properly when routed to a Xen, or to a domain > other than the hardware domain. > > Authored-by: Kyle Temkin <temkink@ainfosec.com> > Signed-off-by: Kyle Temkin <temkink@ainfosec.com> > Signed-off-by: Chris Patterson <pattersonc@ainfosec.com> > --- > > changes since rfc: > - use bool instead of bool_t > - formatting & code style cleanup > - fix dt compat label (nvidia,tegra120 -> nvidia,tegra124) for K1 > - separate mediated legacy interrupt controller into its own module > - split tegra_ictlr_set_interrupt_enable() into > tegra_lic_set_interrupt_type_normal() and > tegra_lic_set_interrupt_enable() > - added a couple helper functions to reduce duplicated logic > - added wrapper tegra_lic_readl and writel functions for external use (mlic) > - re-order defines in tegra.h > - cleanup tegra_init() that was previously in patch 6 > > --- > > xen/arch/arm/platforms/Makefile | 2 + > xen/arch/arm/platforms/tegra.c | 313 ++++++++++++++++++++++++++++++++++ > xen/include/asm-arm/platforms/tegra.h | 54 ++++++ > 3 files changed, 369 insertions(+) > create mode 100644 xen/arch/arm/platforms/tegra.c > create mode 100644 xen/include/asm-arm/platforms/tegra.h > > diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile > index 49fa683..d7033d2 100644 > --- a/xen/arch/arm/platforms/Makefile > +++ b/xen/arch/arm/platforms/Makefile > @@ -6,5 +6,7 @@ obj-$(CONFIG_ARM_32) += omap5.o > obj-$(CONFIG_ARM_32) += rcar2.o > obj-$(CONFIG_ARM_64) += seattle.o > obj-$(CONFIG_ARM_32) += sunxi.o > +obj-$(CONFIG_ARM_32) += tegra.o > +obj-$(CONFIG_ARM_64) += tegra.o > obj-$(CONFIG_ARM_64) += xgene-storm.o > obj-$(CONFIG_ARM_64) += xilinx-zynqmp.o > diff --git a/xen/arch/arm/platforms/tegra.c b/xen/arch/arm/platforms/tegra.c > new file mode 100644 > index 0000000..bdd9966 > --- /dev/null > +++ b/xen/arch/arm/platforms/tegra.c > @@ -0,0 +1,312 @@ > +/* > + * NVIDIA Tegra specific settings > + * > + * Ian Campbell; Copyright (c) 2014 Citrix Systems > + * Kyle Temkin; Copyright (c) 2016 Assured Information Security, Inc. > + * Chris Patterson; Copyright (c) 2016 Assured Information Security, Inc. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include <xen/lib.h> > +#include <xen/stdbool.h> > +#include <xen/sched.h> > +#include <xen/vmap.h> > + > +#include <asm/io.h> > +#include <asm/gic.h> > +#include <asm/platform.h> > +#include <asm/platforms/tegra.h> > + > +/* Permanent mapping to the Tegra legacy interrupt controller. */ > +static void __iomem *tegra_ictlr_base; > + > +/* > + * List of legacy interrupt controllers that can be used to route > + * Tegra interrupts. > + */ > +static const char * const tegra_interrupt_compat[] __initconst = > +{ > + "nvidia,tegra124-ictlr", /* Tegra K1 controllers */ > + "nvidia,tegra210-ictlr" /* Tegra X1 controllers */ > +}; > + > +/* > + * Returns true iff the given IRQ belongs to a supported tegra interrupt > + * controller. > + */ > +static bool tegra_irq_belongs_to_ictlr(const struct dt_raw_irq * rirq) { > + int i; > + > + for ( i = 0; i < ARRAY_SIZE(tegra_interrupt_compat); i++ ) { > + if ( dt_device_is_compatible(rirq->controller, tegra_interrupt_compat[i]) ) > + return true; > + } > + > + return false; > +} > + > +/* > + * Returns true iff the given IRQ is routable -- that is, if it is descended > + * from the platform's primary GIC. > + */ > +static bool tegra_irq_is_routable(const struct dt_raw_irq * rirq) > +{ > + /* If the IRQ connects directly to our GIC, it's trivially routable. */ > + if ( rirq->controller == dt_interrupt_controller ) > + return true; > + > + /* > + * If the IRQ belongs to a legacy interrupt controller, then it's > + * effectively owned by the GIC, and is routable. > + */ > + if ( tegra_irq_belongs_to_ictlr(rirq) ) > + return true; > + > + return false; > +} > + > +/* > + * Platform-specific reset code for the Tegra devices. > + * Should not return. > + */ > +static void tegra_reset(void) > +{ > + void __iomem *addr; > + u32 val; > + > + addr = ioremap_nocache(TEGRA_RESET_BASE, TEGRA_RESET_SIZE); > + if ( !addr ) > + { > + printk(XENLOG_ERR "Tegra: Unable to map tegra reset address. Reset failed!\n"); > + return; > + } > + > + /* Write into the reset device. */ > + val = readl(addr) | TEGRA_RESET_MASK; > + writel(val, addr); > + > + /* Should not get here */ > + iounmap(addr); > +} > + > +/* > + * Convert irq line to index of servicing legacy interrupt controller. > + */ > +static unsigned int tegra_lic_irq_to_ictlr_index(unsigned int irq) > +{ > + return (irq - NR_LOCAL_IRQS) / TEGRA_IRQS_PER_ICTLR; > +} > + > +/* > + * Convert irq line to index of irq within servicing interrupt controller. > + */ > +static unsigned int tegra_lic_irq_to_ictlr_irq_index(unsigned int irq) > +{ > + return (irq - NR_LOCAL_IRQS) % TEGRA_IRQS_PER_ICTLR; > +} > + > +/* > + * Mark interrupt as normal rather than a fast IRQ. > + */ > +static void tegra_lic_set_interrupt_type_normal(unsigned int irq) > +{ > + uint32_t previous_iep_class; > + unsigned int ictlr_index = tegra_lic_irq_to_ictlr_index(irq); > + unsigned int ictlr_irq_index = tegra_lic_irq_to_ictlr_irq_index(irq); > + uint32_t mask = BIT(ictlr_irq_index); > + > + /* Mark the interrupt as a normal interrupt-- not a fast IRQ. */ > + previous_iep_class = tegra_lic_readl(ictlr_index, TEGRA_ICTLR_CPU_IEP_CLASS); > + tegra_lic_writel(ictlr_index, TEGRA_ICTLR_CPU_IEP_CLASS, previous_iep_class & ~mask); > +} > + > +/* > + * Enable/disable interrupt line for specified irq. > + */ > +static void tegra_lic_set_interrupt_enable(unsigned int irq, bool enabled) > +{ > + unsigned int ictlr_index = tegra_lic_irq_to_ictlr_index(irq); > + unsigned int ictlr_irq_index = tegra_lic_irq_to_ictlr_irq_index(irq); > + uint32_t mask = BIT(ictlr_irq_index); > + > + if ( enabled ) > + tegra_lic_writel(ictlr_index, TEGRA_ICTLR_CPU_IER_SET, mask); > + else > + tegra_lic_writel(ictlr_index, TEGRA_ICTLR_CPU_IER_CLR, mask); > +} > + > +/* > + * Routes an IRQ to a guest, applying sane values to the ictlr masks. > + * Returns 0 on success, or an error code on failure. > + */ > +static int tegra_route_irq_to_guest(struct domain *d, unsigned int virq, > + struct irq_desc *desc, unsigned int priority) > +{ > + /* Program the core GIC to deliver the interrupt to the guest. */ > + int rc = gic_route_irq_to_guest(d, virq, desc, priority); > + > + /* If we couldn't route the IRQ via the GIC, bail out. */ > + if ( rc ) > + { > + printk(XENLOG_ERR "Tegra LIC: Couldn't program GIC to route vIRQ %d (%d).\n", > + desc->irq, rc); > + return rc; > + } Remove the warning. > + /* > + * If this is a local IRQ, it's not masked by the ictlr, so we > + * don't need to perform any ictlr manipulation. > + */ > + if ( desc->irq < NR_LOCAL_IRQS ) > + return rc; > + > + /* > + * If this is the hardware domain, it will have real access to the ictlr, > + * and will program the ictlr itself, so it should start with the ictlr > + * disabled. If we're not the hwdom, the domain won't interact with the > + * ictlr, and the interrupt shouldn't be masked. Either way, first > + * set the interrupt type to normal (if previously set to fast IRQ). > + */ > + tegra_lic_set_interrupt_type_normal(desc->irq); > + tegra_lic_set_interrupt_enable(desc->irq, !is_hardware_domain(d)); > + return rc; > +} > + > + > +/* > + * Routes an IRQ to Xen. This method both performs the core IRQ routing, and > + * sets up any ictlr routing necessary. > + */ > +static void tegra_route_irq_to_xen(struct irq_desc *desc, unsigned int priority) > +{ > + unsigned int irq = desc->irq; > + > + /* Program the core GIC to deliver the interrupt to Xen. */ > + gic_route_irq_to_xen(desc, priority); > + > + /* > + * If this is a local IRQ, it's not masked by the ictlr, so we > + * don't need to perform any ictlr manipulation. > + */ > + if ( irq < NR_LOCAL_IRQS ) > + return; > + > + /* > + * Enable the interrupt in the ictlr. Xen only uses the GIC to > + * perform masking, so we'll enable the interrupt to prevent ictlr > + * gating of the interrupt. > + */ > + tegra_lic_set_interrupt_type_normal(desc->irq); > + tegra_lic_set_interrupt_enable(desc->irq, true); > +} > + > +/* > + * Read register from specified legacy interrupt interrupt controller. > + */ > +uint32_t tegra_lic_readl(unsigned int ictlr_index, unsigned int register_offset) > +{ > + ASSERT(tegra_ictlr_base); > + ASSERT(ictlr_index < TEGRA_ICTLR_COUNT); > + ASSERT(register_offset < TEGRA_ICTLR_SIZE); > + return readl(tegra_ictlr_base + ictlr_index * TEGRA_ICTLR_SIZE + > + register_offset); > +} > + > +/* > + * Write register for specified legacy interrupt interrupt controller. > + */ > +void tegra_lic_writel(unsigned int ictlr_index, unsigned int register_offset, uint32_t value) > +{ > + ASSERT(tegra_ictlr_base); > + ASSERT(ictlr_index < TEGRA_ICTLR_COUNT); > + ASSERT(register_offset < TEGRA_ICTLR_SIZE); > + writel(value, tegra_ictlr_base + ictlr_index * TEGRA_ICTLR_SIZE + > + register_offset); > +} > + > +/* > + * Initialize the Tegra legacy interrupt controller, placing each interrupt > + * into a default state. These defaults ensure that stray interrupts don't > + * affect Xen. > + */ > +static int tegra_lic_init(void) > +{ > + int i; > + > + /* Map in the tegra ictlr. */ > + tegra_ictlr_base = ioremap_nocache(TEGRA_ICTLR_BASE, > + TEGRA_ICTLR_SIZE * TEGRA_ICTLR_COUNT); > + > + if ( !tegra_ictlr_base ) > + panic("Failed to map in the Tegra legacy interrupt controller"); > + > + /* Initialize each of the legacy interrupt controllers. */ > + for ( i = 0; i < TEGRA_ICTLR_COUNT; i++ ) { > + > + /* Clear the interrupt enables for every interrupt. */ > + tegra_lic_writel(i, TEGRA_ICTLR_CPU_IER_CLR, ~0); > + > + /* > + * Mark all of our interrupts as normal ARM interrupts (as opposed > + * to Fast Interrupts.) > + */ > + tegra_lic_writel(i, TEGRA_ICTLR_CPU_IEP_CLASS, 0); > + } > + > + return 0; > +} > + > +/** > + * Startup code for the Tegra. > + */ > +static int tegra_init(void) __init > +{ > + return tegra_lic_init(); > +} > + > + > +static const char * const tegra_dt_compat[] __initconst = > +{ > + "nvidia,tegra120", /* Tegra K1 */ This is still tegra120 (not tegra124), is that intended? If so, it is still missing from arch/arm*/boot/dts. Do you have a pointer? Also, do we need both tegra_dt_compat and tegra_interrupt_compat? Can we keep only one? > + "nvidia,tegra210", /* Tegra X1 */ > + NULL > +}; > + > +static const struct dt_device_match tegra_blacklist_dev[] __initconst = > +{ > + /* > + * The UARTs share a page which runs the risk of mapping the Xen console > + * UART to dom0, so don't map any of them. > + */ > + DT_MATCH_COMPATIBLE("nvidia,tegra20-uart"), > + { /* sentinel */ }, > +}; > + > +PLATFORM_START(tegra, "Tegra") > + .blacklist_dev = tegra_blacklist_dev, > + .compatible = tegra_dt_compat, > + .init = tegra_init, > + .reset = tegra_reset, > + .irq_is_routable = tegra_irq_is_routable, > + .route_irq_to_xen = tegra_route_irq_to_xen, > + .route_irq_to_guest = tegra_route_irq_to_guest, > +PLATFORM_END > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * tab-width: 4 > + * indent-tabs-mode: nil > + * End: > + */ > diff --git a/xen/include/asm-arm/platforms/tegra.h b/xen/include/asm-arm/platforms/tegra.h > new file mode 100644 > index 0000000..e9cd792 > --- /dev/null > +++ b/xen/include/asm-arm/platforms/tegra.h > @@ -0,0 +1,54 @@ > +/* > + * NVIDIA Tegra platform definitions > + * > + * Kyle Temkin; Copyright (c) 2016 Assured Information Security, Inc. > + * Chris Patterson; Copyright (c) 2016 Assured Information Security, Inc. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > + > +#ifndef __ASM_ARM_PLATFORMS_TEGRA_H > +#define __ASM_ARM_PLATFORMS_TEGRA_H > + > +#define TEGRA_ICTLR_BASE 0x60004000 > +#define TEGRA_ICTLR_SIZE 0x00000100 > +#define TEGRA_ICTLR_COUNT 6 > +#define TEGRA_IRQS_PER_ICTLR 32 > + > +#define TEGRA_ICTLR_CPU_IER 0x20 > +#define TEGRA_ICTLR_CPU_IER_SET 0x24 > +#define TEGRA_ICTLR_CPU_IER_CLR 0x28 > +#define TEGRA_ICTLR_CPU_IEP_CLASS 0x2C > + > +#define TEGRA_ICTLR_COP_IER 0x30 > +#define TEGRA_ICTLR_COP_IER_SET 0x34 > +#define TEGRA_ICTLR_COP_IER_CLR 0x38 > +#define TEGRA_ICTLR_COP_IEP_CLASS 0x3c > + > +#define TEGRA_RESET_BASE 0x7000e400 > +#define TEGRA_RESET_SIZE 4 > +#define TEGRA_RESET_MASK 0x10 > + > +uint32_t tegra_lic_readl(unsigned int ictlr_index, unsigned int register_offset); > +void tegra_lic_writel(unsigned int ictlr_index, unsigned int register_offset, uint32_t value); > + > +#endif /* __ASM_ARM_PLATFORMS_TEGRA_H */ > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * tab-width: 4 > + * indent-tabs-mode: nil > + * End: > + */ > -- > 2.1.4 >
>> +static const char * const tegra_dt_compat[] __initconst = >> +{ >> + "nvidia,tegra120", /* Tegra K1 */ > > This is still tegra120 (not tegra124), is that intended? If so, it is > still missing from arch/arm*/boot/dts. Do you have a pointer? It was not intended; thank you for catching it. I must have lost that fixup somewhere along the way... > Also, do we need both tegra_dt_compat and tegra_interrupt_compat? Can we > keep only one? The purpose of tegra_interrupt_compat is to maintain a tegra-specific whitelist of interrupt controllers we know how to route. Presumably, there may be custom boards out there that may have additional interrupt routing capabilities that this patch set would not support as-is. I'm not sure of an appropriate way to maintain that logic and merge them. However, I am certainly open to suggestion, if you have any ideas. Thanks for the review! -Chris
Hello Chris, On 17/04/2017 16:03, Chris Patterson wrote: >>> +static const char * const tegra_dt_compat[] __initconst = >>> +{ >>> + "nvidia,tegra120", /* Tegra K1 */ >> >> This is still tegra120 (not tegra124), is that intended? If so, it is >> still missing from arch/arm*/boot/dts. Do you have a pointer? > > It was not intended; thank you for catching it. I must have lost that > fixup somewhere along the way... > >> Also, do we need both tegra_dt_compat and tegra_interrupt_compat? Can we >> keep only one? > > The purpose of tegra_interrupt_compat is to maintain a tegra-specific > whitelist of interrupt controllers we know how to route. Presumably, > there may be custom boards out there that may have additional > interrupt routing capabilities that this patch set would not support > as-is. I'm not sure of an appropriate way to maintain that logic and > merge them. However, I am certainly open to suggestion, if you have > any ideas. Those custom boards would likely have a different machine compatible string because the SOC would be different. So I think the tegra_interrupt_compat is not necessary. Cheers,
Hello, On 06/04/2017 20:47, Chris Patterson wrote: > From: "Chris Patterson" <pattersonc@ainfosec.com> > > Tegra devices have a legacy interrupt controller (lic, or ictlr) that > must be programmed in parallel with their primary GIC. For all intents > and purposes, we treat these devices attached to this controller as > connected to the primary GIC, as it will be handling their interrupts. > > This commit adds support for exposing the ictlr to the hardware domain; > but a future commit will extend this to support exposing a virtualized > version of the ictlr to the hardware domain, and to ensure that > interrupts are unmasked properly when routed to a Xen, or to a domain > other than the hardware domain. Can you please provide a link to the public documentation (if there is any) about the SOC? > > Authored-by: Kyle Temkin <temkink@ainfosec.com> See my question on patch #2. > Signed-off-by: Kyle Temkin <temkink@ainfosec.com> > Signed-off-by: Chris Patterson <pattersonc@ainfosec.com> > --- > > changes since rfc: > - use bool instead of bool_t > - formatting & code style cleanup > - fix dt compat label (nvidia,tegra120 -> nvidia,tegra124) for K1 > - separate mediated legacy interrupt controller into its own module > - split tegra_ictlr_set_interrupt_enable() into > tegra_lic_set_interrupt_type_normal() and > tegra_lic_set_interrupt_enable() > - added a couple helper functions to reduce duplicated logic > - added wrapper tegra_lic_readl and writel functions for external use (mlic) > - re-order defines in tegra.h > - cleanup tegra_init() that was previously in patch 6 > > --- > > xen/arch/arm/platforms/Makefile | 2 + > xen/arch/arm/platforms/tegra.c | 313 ++++++++++++++++++++++++++++++++++ > xen/include/asm-arm/platforms/tegra.h | 54 ++++++ > 3 files changed, 369 insertions(+) > create mode 100644 xen/arch/arm/platforms/tegra.c > create mode 100644 xen/include/asm-arm/platforms/tegra.h > > diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile > index 49fa683..d7033d2 100644 > --- a/xen/arch/arm/platforms/Makefile > +++ b/xen/arch/arm/platforms/Makefile > @@ -6,5 +6,7 @@ obj-$(CONFIG_ARM_32) += omap5.o > obj-$(CONFIG_ARM_32) += rcar2.o > obj-$(CONFIG_ARM_64) += seattle.o > obj-$(CONFIG_ARM_32) += sunxi.o > +obj-$(CONFIG_ARM_32) += tegra.o > +obj-$(CONFIG_ARM_64) += tegra.o Please use obj-y rather duplicating the code. > obj-$(CONFIG_ARM_64) += xgene-storm.o > obj-$(CONFIG_ARM_64) += xilinx-zynqmp.o > diff --git a/xen/arch/arm/platforms/tegra.c b/xen/arch/arm/platforms/tegra.c > new file mode 100644 > index 0000000..bdd9966 > --- /dev/null > +++ b/xen/arch/arm/platforms/tegra.c > @@ -0,0 +1,312 @@ > +/* > + * NVIDIA Tegra specific settings > + * > + * Ian Campbell; Copyright (c) 2014 Citrix Systems Ian Campbell is not mentioned in the commit message. Is he the original author of the code? How much did you modify? > + * Kyle Temkin; Copyright (c) 2016 Assured Information Security, Inc. > + * Chris Patterson; Copyright (c) 2016 Assured Information Security, Inc. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include <xen/lib.h> > +#include <xen/stdbool.h> > +#include <xen/sched.h> > +#include <xen/vmap.h> > + > +#include <asm/io.h> > +#include <asm/gic.h> > +#include <asm/platform.h> > +#include <asm/platforms/tegra.h> > + > +/* Permanent mapping to the Tegra legacy interrupt controller. */ > +static void __iomem *tegra_ictlr_base; > + > +/* > + * List of legacy interrupt controllers that can be used to route > + * Tegra interrupts. > + */ > +static const char * const tegra_interrupt_compat[] __initconst = > +{ > + "nvidia,tegra124-ictlr", /* Tegra K1 controllers */ > + "nvidia,tegra210-ictlr" /* Tegra X1 controllers */ > +}; > + > +/* > + * Returns true iff the given IRQ belongs to a supported tegra interrupt > + * controller. > + */ > +static bool tegra_irq_belongs_to_ictlr(const struct dt_raw_irq * rirq) { Coding style. > + int i; > + > + for ( i = 0; i < ARRAY_SIZE(tegra_interrupt_compat); i++ ) { Coding style. > + if ( dt_device_is_compatible(rirq->controller, tegra_interrupt_compat[i]) ) > + return true; > + } > + > + return false; > +} > + > +/* > + * Returns true iff the given IRQ is routable -- that is, if it is descended > + * from the platform's primary GIC. > + */ > +static bool tegra_irq_is_routable(const struct dt_raw_irq * rirq) > +{ > + /* If the IRQ connects directly to our GIC, it's trivially routable. */ > + if ( rirq->controller == dt_interrupt_controller ) > + return true; > + > + /* > + * If the IRQ belongs to a legacy interrupt controller, then it's > + * effectively owned by the GIC, and is routable. > + */ > + if ( tegra_irq_belongs_to_ictlr(rirq) ) > + return true; > + > + return false; > +} > + > +/* > + * Platform-specific reset code for the Tegra devices. > + * Should not return. > + */ > +static void tegra_reset(void) > +{ > + void __iomem *addr; > + u32 val; > + > + addr = ioremap_nocache(TEGRA_RESET_BASE, TEGRA_RESET_SIZE); > + if ( !addr ) > + { > + printk(XENLOG_ERR "Tegra: Unable to map tegra reset address. Reset failed!\n"); > + return; > + } > + > + /* Write into the reset device. */ > + val = readl(addr) | TEGRA_RESET_MASK; > + writel(val, addr); > + > + /* Should not get here */ > + iounmap(addr); > +} > + > +/* > + * Convert irq line to index of servicing legacy interrupt controller. > + */ > +static unsigned int tegra_lic_irq_to_ictlr_index(unsigned int irq) > +{ > + return (irq - NR_LOCAL_IRQS) / TEGRA_IRQS_PER_ICTLR; > +} > + > +/* > + * Convert irq line to index of irq within servicing interrupt controller. > + */ > +static unsigned int tegra_lic_irq_to_ictlr_irq_index(unsigned int irq) > +{ > + return (irq - NR_LOCAL_IRQS) % TEGRA_IRQS_PER_ICTLR; > +} > + > +/* > + * Mark interrupt as normal rather than a fast IRQ. > + */ > +static void tegra_lic_set_interrupt_type_normal(unsigned int irq) > +{ > + uint32_t previous_iep_class; > + unsigned int ictlr_index = tegra_lic_irq_to_ictlr_index(irq); > + unsigned int ictlr_irq_index = tegra_lic_irq_to_ictlr_irq_index(irq); > + uint32_t mask = BIT(ictlr_irq_index); > + > + /* Mark the interrupt as a normal interrupt-- not a fast IRQ. */ > + previous_iep_class = tegra_lic_readl(ictlr_index, TEGRA_ICTLR_CPU_IEP_CLASS); > + tegra_lic_writel(ictlr_index, TEGRA_ICTLR_CPU_IEP_CLASS, previous_iep_class & ~mask); > +} > + > +/* > + * Enable/disable interrupt line for specified irq. > + */ > +static void tegra_lic_set_interrupt_enable(unsigned int irq, bool enabled) > +{ > + unsigned int ictlr_index = tegra_lic_irq_to_ictlr_index(irq); > + unsigned int ictlr_irq_index = tegra_lic_irq_to_ictlr_irq_index(irq); > + uint32_t mask = BIT(ictlr_irq_index); > + > + if ( enabled ) > + tegra_lic_writel(ictlr_index, TEGRA_ICTLR_CPU_IER_SET, mask); > + else > + tegra_lic_writel(ictlr_index, TEGRA_ICTLR_CPU_IER_CLR, mask); > +} > + > +/* > + * Routes an IRQ to a guest, applying sane values to the ictlr masks. > + * Returns 0 on success, or an error code on failure. > + */ > +static int tegra_route_irq_to_guest(struct domain *d, unsigned int virq, > + struct irq_desc *desc, unsigned int priority) The indentation looks wrong. > +{ > + /* Program the core GIC to deliver the interrupt to the guest. */ > + int rc = gic_route_irq_to_guest(d, virq, desc, priority); > + > + /* If we couldn't route the IRQ via the GIC, bail out. */ > + if ( rc ) > + { > + printk(XENLOG_ERR "Tegra LIC: Couldn't program GIC to route vIRQ %d (%d).\n", s/%d/%u/ for both. > + desc->irq, rc); > + return rc; > + } > + > + /* > + * If this is a local IRQ, it's not masked by the ictlr, so we > + * don't need to perform any ictlr manipulation. > + */ > + if ( desc->irq < NR_LOCAL_IRQS ) > + return rc; > + > + /* > + * If this is the hardware domain, it will have real access to the ictlr, > + * and will program the ictlr itself, so it should start with the ictlr > + * disabled. If we're not the hwdom, the domain won't interact with the > + * ictlr, and the interrupt shouldn't be masked. Either way, first > + * set the interrupt type to normal (if previously set to fast IRQ). So why do you want the hardware domain to interact with the ictlr? Could not you hide it completely? > + */ > + tegra_lic_set_interrupt_type_normal(desc->irq); > + tegra_lic_set_interrupt_enable(desc->irq, !is_hardware_domain(d)); What would happen if you enable the interrupt here for the guest? Should not you do it when the guest is requesting to enable (see vgic_enable_irqs). Also, how about EOI an interrupt? Lastly, newline here please. > + return rc; > +} > + > + > +/* > + * Routes an IRQ to Xen. This method both performs the core IRQ routing, and > + * sets up any ictlr routing necessary. > + */ > +static void tegra_route_irq_to_xen(struct irq_desc *desc, unsigned int priority) > +{ > + unsigned int irq = desc->irq; > + > + /* Program the core GIC to deliver the interrupt to Xen. */ > + gic_route_irq_to_xen(desc, priority); > + > + /* > + * If this is a local IRQ, it's not masked by the ictlr, so we > + * don't need to perform any ictlr manipulation. > + */ > + if ( irq < NR_LOCAL_IRQS ) > + return; > + > + /* > + * Enable the interrupt in the ictlr. Xen only uses the GIC to > + * perform masking, so we'll enable the interrupt to prevent ictlr > + * gating of the interrupt. > + */ > + tegra_lic_set_interrupt_type_normal(desc->irq); > + tegra_lic_set_interrupt_enable(desc->irq, true); > +} > + > +/* > + * Read register from specified legacy interrupt interrupt controller. > + */ > +uint32_t tegra_lic_readl(unsigned int ictlr_index, unsigned int register_offset) > +{ > + ASSERT(tegra_ictlr_base); > + ASSERT(ictlr_index < TEGRA_ICTLR_COUNT); > + ASSERT(register_offset < TEGRA_ICTLR_SIZE); > + return readl(tegra_ictlr_base + ictlr_index * TEGRA_ICTLR_SIZE + > + register_offset); > +} > + > +/* > + * Write register for specified legacy interrupt interrupt controller. > + */ > +void tegra_lic_writel(unsigned int ictlr_index, unsigned int register_offset, uint32_t value) > +{ > + ASSERT(tegra_ictlr_base); > + ASSERT(ictlr_index < TEGRA_ICTLR_COUNT); > + ASSERT(register_offset < TEGRA_ICTLR_SIZE); > + writel(value, tegra_ictlr_base + ictlr_index * TEGRA_ICTLR_SIZE + > + register_offset); > +} > + > +/* > + * Initialize the Tegra legacy interrupt controller, placing each interrupt > + * into a default state. These defaults ensure that stray interrupts don't > + * affect Xen. > + */ > +static int tegra_lic_init(void) > +{ > + int i; > + > + /* Map in the tegra ictlr. */ > + tegra_ictlr_base = ioremap_nocache(TEGRA_ICTLR_BASE, > + TEGRA_ICTLR_SIZE * TEGRA_ICTLR_COUNT); Can we avoid hardcoding MMIO if they can be found through the DT? > + > + if ( !tegra_ictlr_base ) > + panic("Failed to map in the Tegra legacy interrupt controller"); > + > + /* Initialize each of the legacy interrupt controllers. */ > + for ( i = 0; i < TEGRA_ICTLR_COUNT; i++ ) { Coding style. > + > + /* Clear the interrupt enables for every interrupt. */ > + tegra_lic_writel(i, TEGRA_ICTLR_CPU_IER_CLR, ~0); > + > + /* > + * Mark all of our interrupts as normal ARM interrupts (as opposed > + * to Fast Interrupts.) > + */ > + tegra_lic_writel(i, TEGRA_ICTLR_CPU_IEP_CLASS, 0); > + } > + > + return 0; > +} > + > +/** > + * Startup code for the Tegra. > + */ > +static int tegra_init(void) > +{ > + return tegra_lic_init(); > +} > + > + No need for 2 newlines. One is enough. > +static const char * const tegra_dt_compat[] __initconst = > +{ > + "nvidia,tegra120", /* Tegra K1 */ > + "nvidia,tegra210", /* Tegra X1 */ > + NULL > +}; > + > +static const struct dt_device_match tegra_blacklist_dev[] __initconst = > +{ > + /* > + * The UARTs share a page which runs the risk of mapping the Xen console > + * UART to dom0, so don't map any of them. > + */ > + DT_MATCH_COMPATIBLE("nvidia,tegra20-uart"), > + { /* sentinel */ }, > +}; > + > +PLATFORM_START(tegra, "Tegra") > + .blacklist_dev = tegra_blacklist_dev, > + .compatible = tegra_dt_compat, > + .init = tegra_init, > + .reset = tegra_reset, > + .irq_is_routable = tegra_irq_is_routable, > + .route_irq_to_xen = tegra_route_irq_to_xen, > + .route_irq_to_guest = tegra_route_irq_to_guest, > +PLATFORM_END > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * tab-width: 4 > + * indent-tabs-mode: nil > + * End: > + */ > diff --git a/xen/include/asm-arm/platforms/tegra.h b/xen/include/asm-arm/platforms/tegra.h > new file mode 100644 > index 0000000..e9cd792 > --- /dev/null > +++ b/xen/include/asm-arm/platforms/tegra.h > @@ -0,0 +1,54 @@ > +/* > + * NVIDIA Tegra platform definitions > + * > + * Kyle Temkin; Copyright (c) 2016 Assured Information Security, Inc. > + * Chris Patterson; Copyright (c) 2016 Assured Information Security, Inc. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > + > +#ifndef __ASM_ARM_PLATFORMS_TEGRA_H > +#define __ASM_ARM_PLATFORMS_TEGRA_H > + > +#define TEGRA_ICTLR_BASE 0x60004000 > +#define TEGRA_ICTLR_SIZE 0x00000100 > +#define TEGRA_ICTLR_COUNT 6 > +#define TEGRA_IRQS_PER_ICTLR 32 > + > +#define TEGRA_ICTLR_CPU_IER 0x20 > +#define TEGRA_ICTLR_CPU_IER_SET 0x24 > +#define TEGRA_ICTLR_CPU_IER_CLR 0x28 > +#define TEGRA_ICTLR_CPU_IEP_CLASS 0x2C > + > +#define TEGRA_ICTLR_COP_IER 0x30 > +#define TEGRA_ICTLR_COP_IER_SET 0x34 > +#define TEGRA_ICTLR_COP_IER_CLR 0x38 > +#define TEGRA_ICTLR_COP_IEP_CLASS 0x3c > + > +#define TEGRA_RESET_BASE 0x7000e400 > +#define TEGRA_RESET_SIZE 4 > +#define TEGRA_RESET_MASK 0x10 > + > +uint32_t tegra_lic_readl(unsigned int ictlr_index, unsigned int register_offset); > +void tegra_lic_writel(unsigned int ictlr_index, unsigned int register_offset, uint32_t value); > + > +#endif /* __ASM_ARM_PLATFORMS_TEGRA_H */ > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * tab-width: 4 > + * indent-tabs-mode: nil > + * End: > + */ > Cheers,
>> The purpose of tegra_interrupt_compat is to maintain a tegra-specific >> whitelist of interrupt controllers we know how to route. Presumably, >> there may be custom boards out there that may have additional >> interrupt routing capabilities that this patch set would not support >> as-is. I'm not sure of an appropriate way to maintain that logic and >> merge them. However, I am certainly open to suggestion, if you have >> any ideas. > > > Those custom boards would likely have a different machine compatible string > because the SOC would be different. So I think the tegra_interrupt_compat is > not necessary. > Sorry about the delayed response... This check effectively filters out GPIO controllers. Any GPIO-sourced interrupts route through the GPIO controller's interrupt, and need not apply for map_irq_to_domain(). Devices requiring GPIO-based interrupts would need to be passed through with the associated controller (or maybe virtualize the GPIO routing?).
> > So why do you want the hardware domain to interact with the ictlr? Could not > you hide it completely? > snip > What would happen if you enable the interrupt here for the guest? Should not > you do it when the guest is requesting to enable (see vgic_enable_irqs). > > > Also, how about EOI an interrupt? We could possibly hide the legacy controller, but that has its own challenges. Notably, the LIC allows configuration for forwarding FIQ vs IRQ, and setting wake-up sources. If we accept limitations to those configurations, we could possibly hide it entirely (or just for non-Dom0 guests with device passthrough?), and then I think we would need platform hooks for accomplishing the masking/unmasking/eoi alongside the vgic. If we don't want to limit the configuration options - we would need to surface the mediated LIC in the device trees for guests with device pass-through. > Will incorporate rest of review into next version, thanks!
Hi Chris, On 06/07/17 23:00, Chris Patterson wrote: >>> The purpose of tegra_interrupt_compat is to maintain a tegra-specific >>> whitelist of interrupt controllers we know how to route. Presumably, >>> there may be custom boards out there that may have additional >>> interrupt routing capabilities that this patch set would not support >>> as-is. I'm not sure of an appropriate way to maintain that logic and >>> merge them. However, I am certainly open to suggestion, if you have >>> any ideas. >> >> >> Those custom boards would likely have a different machine compatible string >> because the SOC would be different. So I think the tegra_interrupt_compat is >> not necessary. >> > > Sorry about the delayed response... > > This check effectively filters out GPIO controllers. Any GPIO-sourced > interrupts route through the GPIO controller's interrupt, and need not > apply for map_irq_to_domain(). Devices requiring GPIO-based > interrupts would need to be passed through with the associated > controller (or maybe virtualize the GPIO routing?). I am a bit confused. Could you give a concrete example based on tegra platform? Cheers,
Hi Chris, On 07/07/17 00:12, Chris Patterson wrote: >> >> So why do you want the hardware domain to interact with the ictlr? Could not >> you hide it completely? >> > > snip > >> What would happen if you enable the interrupt here for the guest? Should not >> you do it when the guest is requesting to enable (see vgic_enable_irqs). >> >> >> Also, how about EOI an interrupt? > > We could possibly hide the legacy controller, but that has its own > challenges. Notably, the LIC allows configuration for forwarding FIQ > vs IRQ, and setting wake-up sources. FIQ are not supported for domain. So I am not sure why you would want a guest to configure that. Furthermore, could you explain what is wake-up sources and why a guest would need it? > > If we accept limitations to those configurations, we could possibly > hide it entirely (or just for non-Dom0 guests with device > passthrough?), and then I think we would need platform hooks for > accomplishing the masking/unmasking/eoi alongside the vgic. If we > don't want to limit the configuration options - we would need to > surface the mediated LIC in the device trees for guests with device > pass-through. > >> > > Will incorporate rest of review into next version, thanks! > Cheers,
On Fri, Jul 7, 2017 at 12:25 PM, Julien Grall <julien.grall@arm.com> wrote: > Hi Chris, > > > On 06/07/17 23:00, Chris Patterson wrote: >>>> >>>> The purpose of tegra_interrupt_compat is to maintain a tegra-specific >>>> whitelist of interrupt controllers we know how to route. Presumably, >>>> there may be custom boards out there that may have additional >>>> interrupt routing capabilities that this patch set would not support >>>> as-is. I'm not sure of an appropriate way to maintain that logic and >>>> merge them. However, I am certainly open to suggestion, if you have >>>> any ideas. >>> >>> >>> >>> Those custom boards would likely have a different machine compatible >>> string >>> because the SOC would be different. So I think the tegra_interrupt_compat >>> is >>> not necessary. >>> >> >> Sorry about the delayed response... >> >> This check effectively filters out GPIO controllers. Any GPIO-sourced >> interrupts route through the GPIO controller's interrupt, and need not >> apply for map_irq_to_domain(). Devices requiring GPIO-based >> interrupts would need to be passed through with the associated >> controller (or maybe virtualize the GPIO routing?). > > > I am a bit confused. Could you give a concrete example based on tegra > platform? > I'm thinking of devices where interrupt-parent is &gpio (as opposed to &lic or &gic). Here is an example on tegra-smaug: wifi: wifi_bcm4354 { compatible = "bcm,bcm4354"; interrupt-parent = <&gpio>; interrupts = <TEGRA_GPIO(H, 2) IRQ_TYPE_LEVEL_HIGH>; wl_reg_on = <&gpio TEGRA_GPIO(H, 1) GPIO_ACTIVE_HIGH>; wl_host_wake = <&gpio TEGRA_GPIO(H, 2) IRQ_TYPE_LEVEL_HIGH>; nvidia,pmc-wakeup = <&pmc PMC_WAKE_TYPE_EVENT 8 PMC_TRIGGER_TYPE_HIGH>; };
On Fri, Jul 7, 2017 at 12:30 PM, Julien Grall <julien.grall@arm.com> wrote: > Hi Chris, > > On 07/07/17 00:12, Chris Patterson wrote: >>> >>> >>> So why do you want the hardware domain to interact with the ictlr? Could >>> not >>> you hide it completely? >>> >> >> snip >> >>> What would happen if you enable the interrupt here for the guest? Should >>> not >>> you do it when the guest is requesting to enable (see vgic_enable_irqs). >>> >>> >>> Also, how about EOI an interrupt? >> >> >> We could possibly hide the legacy controller, but that has its own >> challenges. Notably, the LIC allows configuration for forwarding FIQ >> vs IRQ, and setting wake-up sources. > > > FIQ are not supported for domain. So I am not sure why you would want a > guest to configure that. > Fair point, I did not know that and didn't want to assume there was not a case otherwise... > Furthermore, could you explain what is wake-up sources and why a guest would > need it? > I would expect any driver using irq_set_irq_wake()? A little more background from the reference manual (http://developer.nvidia.com/embedded/dlc/tegra-x1-technical-reference-manual): " The Legacy Interrupt Controller (LIC) is primarily used for BPMP (ARM7). But it is also used for generating interrupts as wake events for CPUs. This is an important use case when the core is in retention. All of the device hardware interrupt signals are sent to the LIC first, which routes them to the ARM7 BPMP-Lite as well as forwards them to the GIC. The LIC also provides a software set/clear mechanism for all of the interrupts." ... 3.3.1.5 Interrupt Blocking to Support Retention The Tegra X1 device implements blocking of interrupts routed to the GIC, which supports the CPU retention state. The block implementing this feature is represented in Figure 4 as Blocking. To support retention, the LIC contains a one shot disable for all the interrupts. When the system goes into retention, BPMP software sets this bit to disable the interrupts. The Flow Controller watches all the interrupts triggered and triggers the BPMP to bring the core out of retention when any interrupt bit is asserted. Once the system is out of retention, the BLOCK _CCPLEX_GIC_INTR bit is cleared, and the interrupt is serviced by the GIC. " Anything using GPIO to wake (e.g. wifi?) is routed through the LIC. I did not find anything obvious with a quick scan, but perhaps other devices directly wired to the LIC may require it as well. Thoughts?
On Fri, Jul 7, 2017 at 2:53 PM, Chris Patterson <cjp256@gmail.com> wrote: > On Fri, Jul 7, 2017 at 12:30 PM, Julien Grall <julien.grall@arm.com> wrote: >> Hi Chris, >> >> On 07/07/17 00:12, Chris Patterson wrote: >>>> >>>> >>>> So why do you want the hardware domain to interact with the ictlr? Could >>>> not >>>> you hide it completely? >>>> >>> >>> snip >>> >>>> What would happen if you enable the interrupt here for the guest? Should >>>> not >>>> you do it when the guest is requesting to enable (see vgic_enable_irqs). >>>> >>>> >>>> Also, how about EOI an interrupt? >>> >>> >>> We could possibly hide the legacy controller, but that has its own >>> challenges. Notably, the LIC allows configuration for forwarding FIQ >>> vs IRQ, and setting wake-up sources. >> >> >> FIQ are not supported for domain. So I am not sure why you would want a >> guest to configure that. >> > > Fair point, I did not know that and didn't want to assume there was > not a case otherwise... > >> Furthermore, could you explain what is wake-up sources and why a guest would >> need it? >> > > I would expect any driver using irq_set_irq_wake()? A little more > background from the reference manual > (http://developer.nvidia.com/embedded/dlc/tegra-x1-technical-reference-manual): > > " > The Legacy Interrupt Controller (LIC) is primarily used for BPMP > (ARM7). But it is also used for generating interrupts as wake > events for CPUs. This is an important use case when the core is in > retention. All of the device hardware interrupt signals are > sent to the LIC first, which routes them to the ARM7 BPMP-Lite as well > as forwards them to the GIC. The LIC also provides a > software set/clear mechanism for all of the interrupts." > > ... > > 3.3.1.5 Interrupt Blocking to Support Retention > > The Tegra X1 device implements blocking of interrupts routed to the > GIC, which supports the CPU retention state. The block > implementing this feature is represented in Figure 4 as Blocking. > > To support retention, the LIC contains a one shot disable for all the > interrupts. When the system goes into retention, BPMP > software sets this bit to disable the interrupts. > > The Flow Controller watches all the interrupts triggered and triggers > the BPMP to bring the core out of retention when any > interrupt bit is asserted. Once the system is out of retention, the > BLOCK _CCPLEX_GIC_INTR bit is cleared, and the interrupt is > serviced by the GIC. > " > > Anything using GPIO to wake (e.g. wifi?) is routed through the LIC. I > did not find anything obvious with a quick scan, but perhaps other > devices directly wired to the LIC may require it as well. > > Thoughts? Hey Julien. Just a quick ping. I was going to submit an updated patch series, just wanted to see if you had any further thoughts or desired changes with regards to the above. Thanks!
Hi Chris, Sorry for the late reply. On 24/07/17 20:38, Chris Patterson wrote: > On Fri, Jul 7, 2017 at 2:53 PM, Chris Patterson <cjp256@gmail.com> wrote: >> On Fri, Jul 7, 2017 at 12:30 PM, Julien Grall <julien.grall@arm.com> wrote: >>> Hi Chris, >>> >>> On 07/07/17 00:12, Chris Patterson wrote: >>>>> >>>>> >>>>> So why do you want the hardware domain to interact with the ictlr? Could >>>>> not >>>>> you hide it completely? >>>>> >>>> >>>> snip >>>> >>>>> What would happen if you enable the interrupt here for the guest? Should >>>>> not >>>>> you do it when the guest is requesting to enable (see vgic_enable_irqs). >>>>> >>>>> >>>>> Also, how about EOI an interrupt? >>>> >>>> >>>> We could possibly hide the legacy controller, but that has its own >>>> challenges. Notably, the LIC allows configuration for forwarding FIQ >>>> vs IRQ, and setting wake-up sources. >>> >>> >>> FIQ are not supported for domain. So I am not sure why you would want a >>> guest to configure that. >>> >> >> Fair point, I did not know that and didn't want to assume there was >> not a case otherwise... >> >>> Furthermore, could you explain what is wake-up sources and why a guest would >>> need it? >>> >> >> I would expect any driver using irq_set_irq_wake()? A little more >> background from the reference manual >> (http://developer.nvidia.com/embedded/dlc/tegra-x1-technical-reference-manual): >> >> " >> The Legacy Interrupt Controller (LIC) is primarily used for BPMP >> (ARM7). But it is also used for generating interrupts as wake >> events for CPUs. This is an important use case when the core is in >> retention. All of the device hardware interrupt signals are >> sent to the LIC first, which routes them to the ARM7 BPMP-Lite as well >> as forwards them to the GIC. The LIC also provides a >> software set/clear mechanism for all of the interrupts." >> >> ... >> >> 3.3.1.5 Interrupt Blocking to Support Retention >> >> The Tegra X1 device implements blocking of interrupts routed to the >> GIC, which supports the CPU retention state. The block >> implementing this feature is represented in Figure 4 as Blocking. >> >> To support retention, the LIC contains a one shot disable for all the >> interrupts. When the system goes into retention, BPMP >> software sets this bit to disable the interrupts. >> >> The Flow Controller watches all the interrupts triggered and triggers >> the BPMP to bring the core out of retention when any >> interrupt bit is asserted. Once the system is out of retention, the >> BLOCK _CCPLEX_GIC_INTR bit is cleared, and the interrupt is >> serviced by the GIC. >> " >> >> Anything using GPIO to wake (e.g. wifi?) is routed through the LIC. I >> did not find anything obvious with a quick scan, but perhaps other >> devices directly wired to the LIC may require it as well. >> >> Thoughts? I have the feeling it is a lot emulation for supporting only wake-up sources in the guest (or even Dom0). Have you thought about para-virtualizing it? Cheers,
Hi Chris, On 07/07/17 19:08, Chris Patterson wrote: > On Fri, Jul 7, 2017 at 12:25 PM, Julien Grall <julien.grall@arm.com> wrote: >> Hi Chris, >> >> >> On 06/07/17 23:00, Chris Patterson wrote: >>>>> >>>>> The purpose of tegra_interrupt_compat is to maintain a tegra-specific >>>>> whitelist of interrupt controllers we know how to route. Presumably, >>>>> there may be custom boards out there that may have additional >>>>> interrupt routing capabilities that this patch set would not support >>>>> as-is. I'm not sure of an appropriate way to maintain that logic and >>>>> merge them. However, I am certainly open to suggestion, if you have >>>>> any ideas. >>>> >>>> >>>> >>>> Those custom boards would likely have a different machine compatible >>>> string >>>> because the SOC would be different. So I think the tegra_interrupt_compat >>>> is >>>> not necessary. >>>> >>> >>> Sorry about the delayed response... >>> >>> This check effectively filters out GPIO controllers. Any GPIO-sourced >>> interrupts route through the GPIO controller's interrupt, and need not >>> apply for map_irq_to_domain(). Devices requiring GPIO-based >>> interrupts would need to be passed through with the associated >>> controller (or maybe virtualize the GPIO routing?). >> >> >> I am a bit confused. Could you give a concrete example based on tegra >> platform? >> > > I'm thinking of devices where interrupt-parent is &gpio (as opposed to > &lic or &gic). Here is an example on tegra-smaug: > > wifi: wifi_bcm4354 { > compatible = "bcm,bcm4354"; > interrupt-parent = <&gpio>; > interrupts = <TEGRA_GPIO(H, 2) IRQ_TYPE_LEVEL_HIGH>; > wl_reg_on = <&gpio TEGRA_GPIO(H, 1) GPIO_ACTIVE_HIGH>; > wl_host_wake = <&gpio TEGRA_GPIO(H, 2) IRQ_TYPE_LEVEL_HIGH>; > nvidia,pmc-wakeup = <&pmc > PMC_WAKE_TYPE_EVENT 8 PMC_TRIGGER_TYPE_HIGH>; > }; > Hmmm, you are right here. What confuse me is the comment on the top of tegra_irq_is_routable: /* * Returns true iff the given IRQ is routable -- that is, if it i descended * from the platform's primary GIC. */ In both GPIO and LIC, the final interrupt will be coming from the GIC. However, the later seems to have a direct translation with the GIC, hence why they are routable. Right? If so I would update the comment to make it clear. Cheers,
diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile index 49fa683..d7033d2 100644 --- a/xen/arch/arm/platforms/Makefile +++ b/xen/arch/arm/platforms/Makefile @@ -6,5 +6,7 @@ obj-$(CONFIG_ARM_32) += omap5.o obj-$(CONFIG_ARM_32) += rcar2.o obj-$(CONFIG_ARM_64) += seattle.o obj-$(CONFIG_ARM_32) += sunxi.o +obj-$(CONFIG_ARM_32) += tegra.o +obj-$(CONFIG_ARM_64) += tegra.o obj-$(CONFIG_ARM_64) += xgene-storm.o obj-$(CONFIG_ARM_64) += xilinx-zynqmp.o diff --git a/xen/arch/arm/platforms/tegra.c b/xen/arch/arm/platforms/tegra.c new file mode 100644 index 0000000..bdd9966 --- /dev/null +++ b/xen/arch/arm/platforms/tegra.c @@ -0,0 +1,312 @@ +/* + * NVIDIA Tegra specific settings + * + * Ian Campbell; Copyright (c) 2014 Citrix Systems + * Kyle Temkin; Copyright (c) 2016 Assured Information Security, Inc. + * Chris Patterson; Copyright (c) 2016 Assured Information Security, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <xen/lib.h> +#include <xen/stdbool.h> +#include <xen/sched.h> +#include <xen/vmap.h> + +#include <asm/io.h> +#include <asm/gic.h> +#include <asm/platform.h> +#include <asm/platforms/tegra.h> + +/* Permanent mapping to the Tegra legacy interrupt controller. */ +static void __iomem *tegra_ictlr_base; + +/* + * List of legacy interrupt controllers that can be used to route + * Tegra interrupts. + */ +static const char * const tegra_interrupt_compat[] __initconst = +{ + "nvidia,tegra124-ictlr", /* Tegra K1 controllers */ + "nvidia,tegra210-ictlr" /* Tegra X1 controllers */ +}; + +/* + * Returns true iff the given IRQ belongs to a supported tegra interrupt + * controller. + */ +static bool tegra_irq_belongs_to_ictlr(const struct dt_raw_irq * rirq) { + int i; + + for ( i = 0; i < ARRAY_SIZE(tegra_interrupt_compat); i++ ) { + if ( dt_device_is_compatible(rirq->controller, tegra_interrupt_compat[i]) ) + return true; + } + + return false; +} + +/* + * Returns true iff the given IRQ is routable -- that is, if it is descended + * from the platform's primary GIC. + */ +static bool tegra_irq_is_routable(const struct dt_raw_irq * rirq) +{ + /* If the IRQ connects directly to our GIC, it's trivially routable. */ + if ( rirq->controller == dt_interrupt_controller ) + return true; + + /* + * If the IRQ belongs to a legacy interrupt controller, then it's + * effectively owned by the GIC, and is routable. + */ + if ( tegra_irq_belongs_to_ictlr(rirq) ) + return true; + + return false; +} + +/* + * Platform-specific reset code for the Tegra devices. + * Should not return. + */ +static void tegra_reset(void) +{ + void __iomem *addr; + u32 val; + + addr = ioremap_nocache(TEGRA_RESET_BASE, TEGRA_RESET_SIZE); + if ( !addr ) + { + printk(XENLOG_ERR "Tegra: Unable to map tegra reset address. Reset failed!\n"); + return; + } + + /* Write into the reset device. */ + val = readl(addr) | TEGRA_RESET_MASK; + writel(val, addr); + + /* Should not get here */ + iounmap(addr); +} + +/* + * Convert irq line to index of servicing legacy interrupt controller. + */ +static unsigned int tegra_lic_irq_to_ictlr_index(unsigned int irq) +{ + return (irq - NR_LOCAL_IRQS) / TEGRA_IRQS_PER_ICTLR; +} + +/* + * Convert irq line to index of irq within servicing interrupt controller. + */ +static unsigned int tegra_lic_irq_to_ictlr_irq_index(unsigned int irq) +{ + return (irq - NR_LOCAL_IRQS) % TEGRA_IRQS_PER_ICTLR; +} + +/* + * Mark interrupt as normal rather than a fast IRQ. + */ +static void tegra_lic_set_interrupt_type_normal(unsigned int irq) +{ + uint32_t previous_iep_class; + unsigned int ictlr_index = tegra_lic_irq_to_ictlr_index(irq); + unsigned int ictlr_irq_index = tegra_lic_irq_to_ictlr_irq_index(irq); + uint32_t mask = BIT(ictlr_irq_index); + + /* Mark the interrupt as a normal interrupt-- not a fast IRQ. */ + previous_iep_class = tegra_lic_readl(ictlr_index, TEGRA_ICTLR_CPU_IEP_CLASS); + tegra_lic_writel(ictlr_index, TEGRA_ICTLR_CPU_IEP_CLASS, previous_iep_class & ~mask); +} + +/* + * Enable/disable interrupt line for specified irq. + */ +static void tegra_lic_set_interrupt_enable(unsigned int irq, bool enabled) +{ + unsigned int ictlr_index = tegra_lic_irq_to_ictlr_index(irq); + unsigned int ictlr_irq_index = tegra_lic_irq_to_ictlr_irq_index(irq); + uint32_t mask = BIT(ictlr_irq_index); + + if ( enabled ) + tegra_lic_writel(ictlr_index, TEGRA_ICTLR_CPU_IER_SET, mask); + else + tegra_lic_writel(ictlr_index, TEGRA_ICTLR_CPU_IER_CLR, mask); +} + +/* + * Routes an IRQ to a guest, applying sane values to the ictlr masks. + * Returns 0 on success, or an error code on failure. + */ +static int tegra_route_irq_to_guest(struct domain *d, unsigned int virq, + struct irq_desc *desc, unsigned int priority) +{ + /* Program the core GIC to deliver the interrupt to the guest. */ + int rc = gic_route_irq_to_guest(d, virq, desc, priority); + + /* If we couldn't route the IRQ via the GIC, bail out. */ + if ( rc ) + { + printk(XENLOG_ERR "Tegra LIC: Couldn't program GIC to route vIRQ %d (%d).\n", + desc->irq, rc); + return rc; + } + + /* + * If this is a local IRQ, it's not masked by the ictlr, so we + * don't need to perform any ictlr manipulation. + */ + if ( desc->irq < NR_LOCAL_IRQS ) + return rc; + + /* + * If this is the hardware domain, it will have real access to the ictlr, + * and will program the ictlr itself, so it should start with the ictlr + * disabled. If we're not the hwdom, the domain won't interact with the + * ictlr, and the interrupt shouldn't be masked. Either way, first + * set the interrupt type to normal (if previously set to fast IRQ). + */ + tegra_lic_set_interrupt_type_normal(desc->irq); + tegra_lic_set_interrupt_enable(desc->irq, !is_hardware_domain(d)); + return rc; +} + + +/* + * Routes an IRQ to Xen. This method both performs the core IRQ routing, and + * sets up any ictlr routing necessary. + */ +static void tegra_route_irq_to_xen(struct irq_desc *desc, unsigned int priority) +{ + unsigned int irq = desc->irq; + + /* Program the core GIC to deliver the interrupt to Xen. */ + gic_route_irq_to_xen(desc, priority); + + /* + * If this is a local IRQ, it's not masked by the ictlr, so we + * don't need to perform any ictlr manipulation. + */ + if ( irq < NR_LOCAL_IRQS ) + return; + + /* + * Enable the interrupt in the ictlr. Xen only uses the GIC to + * perform masking, so we'll enable the interrupt to prevent ictlr + * gating of the interrupt. + */ + tegra_lic_set_interrupt_type_normal(desc->irq); + tegra_lic_set_interrupt_enable(desc->irq, true); +} + +/* + * Read register from specified legacy interrupt interrupt controller. + */ +uint32_t tegra_lic_readl(unsigned int ictlr_index, unsigned int register_offset) +{ + ASSERT(tegra_ictlr_base); + ASSERT(ictlr_index < TEGRA_ICTLR_COUNT); + ASSERT(register_offset < TEGRA_ICTLR_SIZE); + return readl(tegra_ictlr_base + ictlr_index * TEGRA_ICTLR_SIZE + + register_offset); +} + +/* + * Write register for specified legacy interrupt interrupt controller. + */ +void tegra_lic_writel(unsigned int ictlr_index, unsigned int register_offset, uint32_t value) +{ + ASSERT(tegra_ictlr_base); + ASSERT(ictlr_index < TEGRA_ICTLR_COUNT); + ASSERT(register_offset < TEGRA_ICTLR_SIZE); + writel(value, tegra_ictlr_base + ictlr_index * TEGRA_ICTLR_SIZE + + register_offset); +} + +/* + * Initialize the Tegra legacy interrupt controller, placing each interrupt + * into a default state. These defaults ensure that stray interrupts don't + * affect Xen. + */ +static int tegra_lic_init(void) +{ + int i; + + /* Map in the tegra ictlr. */ + tegra_ictlr_base = ioremap_nocache(TEGRA_ICTLR_BASE, + TEGRA_ICTLR_SIZE * TEGRA_ICTLR_COUNT); + + if ( !tegra_ictlr_base ) + panic("Failed to map in the Tegra legacy interrupt controller"); + + /* Initialize each of the legacy interrupt controllers. */ + for ( i = 0; i < TEGRA_ICTLR_COUNT; i++ ) { + + /* Clear the interrupt enables for every interrupt. */ + tegra_lic_writel(i, TEGRA_ICTLR_CPU_IER_CLR, ~0); + + /* + * Mark all of our interrupts as normal ARM interrupts (as opposed + * to Fast Interrupts.) + */ + tegra_lic_writel(i, TEGRA_ICTLR_CPU_IEP_CLASS, 0); + } + + return 0; +} + +/** + * Startup code for the Tegra. + */ +static int tegra_init(void) +{ + return tegra_lic_init(); +} + + +static const char * const tegra_dt_compat[] __initconst = +{ + "nvidia,tegra120", /* Tegra K1 */ + "nvidia,tegra210", /* Tegra X1 */ + NULL +}; + +static const struct dt_device_match tegra_blacklist_dev[] __initconst = +{ + /* + * The UARTs share a page which runs the risk of mapping the Xen console + * UART to dom0, so don't map any of them. + */ + DT_MATCH_COMPATIBLE("nvidia,tegra20-uart"), + { /* sentinel */ }, +}; + +PLATFORM_START(tegra, "Tegra") + .blacklist_dev = tegra_blacklist_dev, + .compatible = tegra_dt_compat, + .init = tegra_init, + .reset = tegra_reset, + .irq_is_routable = tegra_irq_is_routable, + .route_irq_to_xen = tegra_route_irq_to_xen, + .route_irq_to_guest = tegra_route_irq_to_guest, +PLATFORM_END + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/include/asm-arm/platforms/tegra.h b/xen/include/asm-arm/platforms/tegra.h new file mode 100644 index 0000000..e9cd792 --- /dev/null +++ b/xen/include/asm-arm/platforms/tegra.h @@ -0,0 +1,54 @@ +/* + * NVIDIA Tegra platform definitions + * + * Kyle Temkin; Copyright (c) 2016 Assured Information Security, Inc. + * Chris Patterson; Copyright (c) 2016 Assured Information Security, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + + +#ifndef __ASM_ARM_PLATFORMS_TEGRA_H +#define __ASM_ARM_PLATFORMS_TEGRA_H + +#define TEGRA_ICTLR_BASE 0x60004000 +#define TEGRA_ICTLR_SIZE 0x00000100 +#define TEGRA_ICTLR_COUNT 6 +#define TEGRA_IRQS_PER_ICTLR 32 + +#define TEGRA_ICTLR_CPU_IER 0x20 +#define TEGRA_ICTLR_CPU_IER_SET 0x24 +#define TEGRA_ICTLR_CPU_IER_CLR 0x28 +#define TEGRA_ICTLR_CPU_IEP_CLASS 0x2C + +#define TEGRA_ICTLR_COP_IER 0x30 +#define TEGRA_ICTLR_COP_IER_SET 0x34 +#define TEGRA_ICTLR_COP_IER_CLR 0x38 +#define TEGRA_ICTLR_COP_IEP_CLASS 0x3c + +#define TEGRA_RESET_BASE 0x7000e400 +#define TEGRA_RESET_SIZE 4 +#define TEGRA_RESET_MASK 0x10 + +uint32_t tegra_lic_readl(unsigned int ictlr_index, unsigned int register_offset); +void tegra_lic_writel(unsigned int ictlr_index, unsigned int register_offset, uint32_t value); + +#endif /* __ASM_ARM_PLATFORMS_TEGRA_H */ + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */