|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH RFC 4/6] xen/arm: platforms: Add Tegra platform to support basic IRQ routing.
On Mon, Sep 05, 2016 at 06:13:59AM -0400, Kyle Temkin wrote:
> From: "Kyle J. Temkin" <temkink@xxxxxxxxxxxx>
>
> 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 this devices attached to this controller as
> connected to the primary GIC, as it will be handling their interrupts.
Is there a link to the specification? Could that be included in the file
or in the commit description?
>
> 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.
>
> Signed-off-by: Kyle Temkin <temkink@xxxxxxxxxxxx>
> ---
> xen/arch/arm/platforms/Makefile | 2 +
> xen/arch/arm/platforms/tegra.c | 339
> ++++++++++++++++++++++++++++++++++
> xen/include/asm-arm/platforms/tegra.h | 50 +++++
> 3 files changed, 391 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..0c3a7f4 100644
> --- a/xen/arch/arm/platforms/Makefile
> +++ b/xen/arch/arm/platforms/Makefile
> @@ -4,7 +4,9 @@ obj-$(CONFIG_ARM_32) += exynos5.o
> obj-$(CONFIG_ARM_32) += midway.o
> obj-$(CONFIG_ARM_32) += omap5.o
> obj-$(CONFIG_ARM_32) += rcar2.o
> +obj-$(CONFIG_ARM_32) += tegra.o
> obj-$(CONFIG_ARM_64) += seattle.o
> obj-$(CONFIG_ARM_32) += sunxi.o
> obj-$(CONFIG_ARM_64) += xgene-storm.o
> obj-$(CONFIG_ARM_64) += xilinx-zynqmp.o
> +obj-$(CONFIG_ARM_64) += tegra.o
> diff --git a/xen/arch/arm/platforms/tegra.c b/xen/arch/arm/platforms/tegra.c
> new file mode 100644
> index 0000000..e75242c
> --- /dev/null
> +++ b/xen/arch/arm/platforms/tegra.c
> @@ -0,0 +1,339 @@
> +/*
> + * 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/config.h>
No config.h pls.
> +#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 = NULL;
No need for NULL.
> +
> +/*
> + * List of legacy interrupt controllers that can be used to route
> + * Tegra interrupts.
> + */
> +static const char * const tegra_interrupt_compat[] __initconst =
> +{
> + "nvidia,tegra120-ictlr", /* Tegra K1 controllers */
> + "nvidia,tegra210-ictlr" /* Tegra X1 controllers */
> +};
> +
> +
> +/**
> + * Returns true if the given IRQ belongs to a supported tegra interrupt
> + * controller.
> + *
> + * @param rirq The raw IRQ to be identified.
> + * @return True iff the given IRQ belongs to a Tegra ictlr.
> + */
> +static bool_t tegra_irq_belongs_to_ictlr(struct dt_raw_irq * rirq) {
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(tegra_interrupt_compat); i++)
Something is off with your style ({, and the lack of spaces around ()).
> + {
> + 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.
> + *
> + * @param rirq The raw IRQ in question.
> + * @return True iff the given IRQ routes to a platform GIC.
> + */
> +static bool_t tegra_irq_is_routable(struct dt_raw_irq * rirq)
And as I found out just recently, use 'bool' instead of bool_t.
> +{
> + /* 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;
> +}
> +
> +/**
> + * Returns the IRQ number for a given device. Tegra IRQs transalate using the
> + * same algorithm as normal GIC IRQs, but aren't parented by the system GIC.
> + *
> + * As a result, translation fails an assertion in the normal translation
> path.
> + * The normal version is essentially dt_irq_xlate wrapped with an assert, so
> + * we'll just call dt_irq_xlate directly.
> + *
> + * @param device The DT node describing the device.
> + * @param index The index of the interrupt within the device node.
> + * @return The translated number of the IRQ, or a negative error code.
> + */
> +static int tegra_irq_for_device(const struct dt_device_node *device, int
> index)
> +{
> + struct dt_raw_irq raw;
> + struct dt_irq dt_irq;
> + int res;
> +
> + res = dt_device_get_raw_irq(device, index, &raw);
> + if ( res )
> + return -ENODEV;
> +
> + /*
> + * The translation function for the Tegra ictlr happens to match the
> + * translation function for the normal GIC, so we'll use that in either
> + * case.
> + */
> + res = dt_irq_xlate(raw.specifier, raw.size, &dt_irq.irq, &dt_irq.type);
> + if ( res )
> + return -ENODEV;
> +
> + if ( irq_set_type(dt_irq.irq, dt_irq.type) )
> + return -ENODEV;
> +
> + return dt_irq.irq;
> +}
> +
> +/**
> + * 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);
No need to re-read the register?
> +
> + iounmap(addr);
> +}
> +
> +/**
> + * Applies an interrupt enable to a given interrupt via the legacy
> + * interrupt controller, and marks that interrupt as a normal interrupt,
> + * rather than a fast IRQ.
> + *
> + * @param irq The hardware IRQ number for the given interrupt.
> + */
> +static void tegra_ictlr_set_interrupt_enable(unsigned int irq, bool enabled)
> +{
> + uint32_t previous_iep_class;
> +
> + /* If we're enabling a given bit, use the SET register; otherwise CLR. */
> + unsigned int register_number =
> + enabled ? TEGRA_ICTLR_CPU_IER_SET : TEGRA_ICTLR_CPU_IER_CLR;
> +
> + /*
> + * Determine the IRQ number in the ictlr domain, and figure out the
> indexA
> + * of the individual controller we're working with. */
Put the */ on its own line please.
> + unsigned int ictlr_irq = irq - NR_LOCAL_IRQS;
> + unsigned int ictlr_number = ictlr_irq / TEGRA_IRQS_PER_ICTLR;
> +
> + /* Get a pointer to the target ictlr. */
> + void __iomem * target_ictlr = tegra_ictlr_base + TEGRA_ICTLR_SIZE *
> ictlr_number;
> +
> + /* Determine the mask we'll be working with. */
> + uint32_t mask = BIT(ictlr_irq % TEGRA_IRQS_PER_ICTLR);
> +
> + /* Sanity check our memory access. */
> + ASSERT(tegra_ictlr_base);
> + ASSERT(ictlr_number < TEGRA_ICTLR_COUNT);
> + ASSERT(irq >= NR_LOCAL_IRQS);
> +
> + /* Enable the given IRQ. */
> + writel(mask, target_ictlr + register_number);
> +
> + /* Mark the interrupt as a normal interrupt-- not a fast IRQ. */
> + previous_iep_class = readl(target_ictlr + TEGRA_ICTLR_CPU_IEP_CLASS);
> + writel(previous_iep_class & ~mask, target_ictlr +
> TEGRA_ICTLR_CPU_IEP_CLASS);
> +}
> +
> +
> +/**
> + * Routes an IRQ to a guest, applying sane values to the ictlr masks.
> + *
> + * @param domain The domain to which the IRQ will be routed.
> + * @param virq The virtual IRQ number.
> + * @param desc The IRQ to be routed.
> + * @param priority The IRQ priority.
> + * @return 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)
Uh oh.
> + {
> + 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.
> + */
> + tegra_ictlr_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.
> + *
> + * @param desc The IRQ to be routed.
> + * @param priority The IRQ priority.
> + */
> +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_ictlr_set_interrupt_enable(irq, true);
> +
> +}
> +
> +/**
> + * 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_initialize_legacy_interrupt_controller(void)
> +{
> + int i;
> +
> + /* Map in the tegra ictlr. */
> + tegra_ictlr_base = ioremap_nocache(TEGRA_ICTLR_BASE,
> + TEGRA_ICTLR_SIZE * TEGRA_ICTLR_COUNT);
Odd, style?
> +
> + if ( !tegra_ictlr_base )
> + panic("Failed to map in the Tegra legacy interrupt controller!\n");
> +
> + /* Initialize each of the legacy interrupt controllers. */
> + for (i = 0; i < TEGRA_ICTLR_COUNT; i++)
Style.
> + {
> + void __iomem *ictlr_n = tegra_ictlr_base + TEGRA_ICTLR_SIZE * i;
> +
> + /* Clear the interrupt enables for every interrupt. */
> + writel(~0, ictlr_n + TEGRA_ICTLR_CPU_IER_CLR);
> +
> + /*
> + * Mark all of our interrupts as normal ARM interrupts (as opposed
> + * to Fast Interrupts.)
> + */
> + writel(0, ictlr_n + TEGRA_ICTLR_CPU_IEP_CLASS);
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * Startup code for the Tegra.
> + */
> +static int tegra_init(void)
__init ?
> +{
> + int rc;
> +
> + rc = tegra_initialize_legacy_interrupt_controller();
How about you jsut make this:
return tegra_initializa...()
> +
> + /*
> + * TODO: Virtualize the MMIO regions for the ictlr, ensuring the hwdom
> + * (and possibly passhtrough domains) can only access ictlr registers for
> + * interrupts they actually own.
> + */
> +
> + return rc;
> +}
> +
> +
> +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,
> + .irq_for_device = tegra_irq_for_device,
> + .route_irq_to_xen = tegra_route_irq_to_xen,
> + .route_irq_to_guest = tegra_route_irq_to_guest,
> +PLATFORM_END
You are missing this:
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 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..821add5
> --- /dev/null
> +++ b/xen/include/asm-arm/platforms/tegra.h
> @@ -0,0 +1,50 @@
> +/*
> + * 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
Add please two underscores at the far end.
> +#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_RESET_BASE 0x7000e400
> +#define TEGRA_RESET_SIZE 4
> +#define TEGRA_RESET_MASK 0x10
> +
> +#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
> +
> +
> +#endif /* __ASM_ARM_PLATFORMS_TEGRA_H */
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> --
> 2.9.2
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> https://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |