|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 6/6] xen/arm: platforms/tegra: Ensure the hwdom can only affect its own interrupts
On Thu, 6 Apr 2017, Chris Patterson wrote:
> From: Chris Patterson <pattersonc@xxxxxxxxxxxx>
>
> Several Tegra hardware devices, and the Tegra device tree, expect
> the presence of a Tegra Legacy Interrupt Controller (LIC) in the hardware
> domain. Accordingly, we'll need to expose (most of) the LIC's registers
> to the hardware domain.
>
> As the Tegra LIC provides the ability to modify interrupt delivery (e.g.
> by masking interrupts, forcing asserting/clearing them, or adjusting
> their prority), it's important that the hardware domain's access be
> mediated. This commit adds read/write handlers that prohibit
> modification of register sections corresponding to interrupts not owned
> by the hardware domain.
>
> Note that this is written to be domain agnostic; this allows the
> potential to e.g. map the ictlr into multiple domains if this is desired
> for passthrough in the future.
>
> Authored-by: Kyle Temkin <temkink@xxxxxxxxxxxx>
> Signed-off-by: Kyle Temkin <temkink@xxxxxxxxxxxx>
> Signed-off-by: Chris Patterson <pattersonc@xxxxxxxxxxxx>
> ---
>
> changes since rfc:
> - documentation, formatting & code style cleanup
> - drop tegra_init changes (folded into patch 4)
>
> ---
> xen/arch/arm/platforms/Makefile | 2 +
> xen/arch/arm/platforms/tegra-mlic.c | 261
> +++++++++++++++++++++++++++++
> xen/arch/arm/platforms/tegra.c | 13 ++
> xen/include/asm-arm/platforms/tegra-mlic.h | 34 ++++
> 4 files changed, 310 insertions(+)
> create mode 100644 xen/arch/arm/platforms/tegra-mlic.c
> create mode 100644 xen/include/asm-arm/platforms/tegra-mlic.h
>
> diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile
> index d7033d2..5701e62 100644
> --- a/xen/arch/arm/platforms/Makefile
> +++ b/xen/arch/arm/platforms/Makefile
> @@ -7,6 +7,8 @@ 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_32) += tegra-mlic.o
> obj-$(CONFIG_ARM_64) += tegra.o
> +obj-$(CONFIG_ARM_64) += tegra-mlic.o
> obj-$(CONFIG_ARM_64) += xgene-storm.o
> obj-$(CONFIG_ARM_64) += xilinx-zynqmp.o
> diff --git a/xen/arch/arm/platforms/tegra-mlic.c
> b/xen/arch/arm/platforms/tegra-mlic.c
> new file mode 100644
> index 0000000..ad77766
> --- /dev/null
> +++ b/xen/arch/arm/platforms/tegra-mlic.c
> @@ -0,0 +1,260 @@
> +/*
> + * xen/arch/arm/tegra_mlic.c
> + *
> + * Mediator for Tegra Legacy Interrupt Controller
> + *
> + * This module allow the hardware domain to have access to the sections of
> + * the legacy interrupt controller that correspond to its devices,
> + * but disallow access to the sections controlled by other domains
> + * or by Xen.
> + *
> + * 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 <xen/iocap.h>
> +
> +#include <asm/io.h>
> +#include <asm/gic.h>
> +#include <asm/platform.h>
> +#include <asm/platforms/tegra.h>
> +#include <asm/platforms/tegra-mlic.h>
> +#include <asm/mmio.h>
> +#include <xen/perfc.h>
> +
> +static int tegra_mlic_mmio_read(struct vcpu *v, mmio_info_t *info,
> + register_t *r, void *priv);
> +static int tegra_mlic_mmio_write(struct vcpu *v, mmio_info_t *info,
> + register_t r, void *priv);
> +
> +static const struct mmio_handler_ops tegra_mlic_mmio_handler = {
> + .read = tegra_mlic_mmio_read,
> + .write = tegra_mlic_mmio_write,
> +};
> +
> +/*
> + * Parses a LIC MMIO read or write, and extracts the information needed to
> + * complete the request.
> + *
> + * info: Information describing the MMIO read/write being performed
> + * ictlr_index: The interrupt controller number in the ictlr (e.g. 0-5)
> + * register_offset: The register offset into the specified interrupt
> controller
> + * (e.g. TEGRA_ICTLR_CPU_IER_SET)
> + * irq_base: The number of the first IRQ represented by the given ictlr.
> + */
> +static void tegra_mlic_parse_mmio_request(mmio_info_t *info,
> + uint32_t *ictlr_index, uint32_t *register_offset, uint32_t *irq_base)
style: parameters alignment
> +{
> + /* Determine the offset of the access into the ICTLR region. */
> + uint32_t offset = info->gpa - TEGRA_ICTLR_BASE;
> + uint32_t ictlr = offset / TEGRA_ICTLR_SIZE;
> + uint32_t reg = offset % TEGRA_ICTLR_SIZE;
> +
> + if ( ictlr_index )
> + *ictlr_index = ictlr;
> +
> + if ( register_offset )
> + *register_offset = reg;
> +
> + if ( irq_base )
> + *irq_base = (ictlr * TEGRA_IRQS_PER_ICTLR) + NR_LOCAL_IRQS;
> +
> + /* Ensure that we've only been handed a valid offset within our region.
> */
> + BUG_ON(ictlr >= TEGRA_ICTLR_COUNT);
> + BUG_ON(offset >= (TEGRA_ICTLR_COUNT * TEGRA_ICTLR_SIZE));
> + BUG_ON((ictlr * TEGRA_ICTLR_SIZE + reg) != offset);
> +}
> +
> +/*
> + * Returns true iff the given IRQ is currently routed to the given domain.
> + */
> +static bool irq_owned_by_domain(int irq, struct domain *d)
> +{
> + struct irq_desc *desc = irq_to_desc(irq);
> + domid_t domid;
> + unsigned long flags;
> +
> + BUG_ON(!desc);
> +
> + spin_lock_irqsave(&desc->lock, flags);
> + domid = irq_get_domain_id(desc);
> + spin_unlock_irqrestore(&desc->lock, flags);
> +
> + return (d->domain_id == domid);
> +}
> +
> +/*
> + * Mediates an MMIO-read to the Tegra legacy interrupt controller.
> + * Ensures that each domain only is passed interrupt state for its
> + * own interupts.
> + */
> +static int tegra_mlic_mmio_read(struct vcpu *v, mmio_info_t *info,
> + register_t *target_register, void *priv)
> +{
> + register_t raw_value;
> + unsigned int ictlr_index;
> + unsigned int register_offset;
> + unsigned int irq_base;
> + int i;
> +
> + perfc_incr(tegra_mlic_reads);
> +
> + tegra_mlic_parse_mmio_request(info, &ictlr_index, ®ister_offset,
> + &irq_base);
> +
> + /* Sanity check the read. */
> + if ( register_offset & 0x3 )
> + {
> + printk(XENLOG_G_ERR "d%u: Tegra LIC: Attempt to read unaligned ictlr
> addr"
> + "(%" PRIpaddr ")\n", current->domain->domain_id,
> info->gpa);
> + domain_crash_synchronous();
> + }
> +
> + if ( info->dabt.size != DABT_WORD )
> + {
> + printk(XENLOG_G_ERR "d%u: Tegra LIC: Non-word read from ictlr addr"
> + "%" PRIpaddr "\n", current->domain->domain_id,
> info->gpa);
> + domain_crash_synchronous();
> + }
> +
> +
> + /* Perform the core ictlr read. */
> + raw_value = tegra_lic_readl(ictlr_index, register_offset);
> +
> + /*
> + * We don't want to leak information about interrupts not controlled
> + * by the active domain. Thus, we'll zero out any ictlr slots for
> + * IRQs not owned by the given domain.
> + */
> + for ( i = 0; i < TEGRA_IRQS_PER_ICTLR; ++i ) {
style: { on newline
> + int irq = irq_base + i;
> +
> + if ( !irq_owned_by_domain(irq, current->domain) )
> + raw_value &= ~( 1 << i );
> + }
> +
> + /* Finally, set the target register to our read value */
> + *target_register = raw_value;
> + return 1;
> +}
> +
> +/*
> + * Mediates an MMIO-read to the Tegra legacy interrupt controller.
> + * Ensures that each domain only can only control is own interrupts.
> + */
> +static int tegra_mlic_mmio_write(struct vcpu *v, mmio_info_t *info,
> + register_t new_value, void *priv)
> +{
> + register_t write_mask = 0;
> + register_t raw_value;
> + unsigned int ictlr_index;
> + unsigned int register_offset;
> + unsigned int irq_base;
> + int i;
> +
> + perfc_incr(tegra_mlic_writes);
> +
> + tegra_mlic_parse_mmio_request(info, &ictlr_index, ®ister_offset,
> + &irq_base);
> +
> + /* Sanity check the read. */
> + if ( register_offset & 0x3 ) {
style: { on newline
> + printk(XENLOG_G_ERR "d%u: Tegra LIC: Attempt to write unaligned
> ictlr addr"
> + "(%" PRIpaddr ")\n", current->domain->domain_id,
> info->gpa);
> + domain_crash_synchronous();
> + return 0;
> + }
> +
> + if ( info->dabt.size != DABT_WORD ) {
style: { on newline
> + printk(XENLOG_G_ERR "d%u: Tegra LIC: Non-word write to ictlr addr"
> + "%" PRIpaddr "\n", current->domain->domain_id,
> info->gpa);
> + domain_crash_synchronous();
> + return 0;
> + }
> +
> + /*
> + * We only want to write to bits that correspond to interrupts that the
> + * current domain controls. Accordingly, we'll create a mask that has a
> + * single bit set for each writable bit.
> + */
> + for ( i = 0; i < TEGRA_IRQS_PER_ICTLR; ++i ) {
style: { on newline
> + int irq = irq_base + i;
> +
> + if ( irq_owned_by_domain(irq, current->domain) )
> + write_mask |= ( 1 << i );
> + }
> +
> + /*
> + * Read in the original value. We'll use this to ensure that we maintain
> + * the bit values for any bits not actively controlled by this domain.
> Note
> + * that we can perform this read without side effects, so this shouldn't
> + * change the actual operation being performed.
> + */
> + raw_value = tegra_lic_readl(ictlr_index, register_offset);
> +
> + /* Remove bits that the guest is not allowed to write. */
> + raw_value &= ~write_mask;
> + raw_value |= (write_mask & new_value);
> +
> + /* Finally perform the write. */
> + tegra_lic_writel(ictlr_index, register_offset, raw_value);
> + return 1;
> +}
> +
> +/*
> + * Set up the hardware domain for the Tegra, giving it mediated access to the
> + * platform's legacy interrupt controller.
> + */
> +int domain_tegra_mlic_init(struct domain *d)
> +{
> + int rc;
> + unsigned long pfn_start, pfn_end;
> +
> + ASSERT( is_hardware_domain(d) );
> +
> + pfn_start = paddr_to_pfn(TEGRA_ICTLR_BASE);
> + pfn_end = DIV_ROUND_UP(TEGRA_ICTLR_BASE + (TEGRA_ICTLR_SIZE *
> TEGRA_ICTLR_COUNT), PAGE_SIZE);
> +
> + /* Force all access to the ictlr to go through our mediator. */
> + rc = iomem_deny_access(d, pfn_start, pfn_end);
> +
> + if ( rc )
> + panic("Failed to deny access to the Tegra LIC iomem");
> +
> + rc = unmap_mmio_regions(d, _gfn(pfn_start),
> + pfn_end - pfn_start + 1,
> + _mfn(pfn_start));
Please avoid mapping the region at boot time, if possible. Maybe you can
do that using blacklist_dev?
> + if ( rc )
> + panic("Failed to deny access to the Tegra LIC");
> +
> + register_mmio_handler(d, &tegra_mlic_mmio_handler,
> + TEGRA_ICTLR_BASE,
> + TEGRA_ICTLR_SIZE * TEGRA_ICTLR_COUNT,
> + NULL);
> +
> + return 0;
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/platforms/tegra.c b/xen/arch/arm/platforms/tegra.c
> index bdd9966..ee62708 100644
> --- a/xen/arch/arm/platforms/tegra.c
> +++ b/xen/arch/arm/platforms/tegra.c
> @@ -20,11 +20,14 @@
> #include <xen/stdbool.h>
> #include <xen/sched.h>
> #include <xen/vmap.h>
> +#include <xen/iocap.h>
>
> #include <asm/io.h>
> #include <asm/gic.h>
> #include <asm/platform.h>
> #include <asm/platforms/tegra.h>
> +#include <asm/platforms/tegra-mlic.h>
> +#include <asm/mmio.h>
>
> /* Permanent mapping to the Tegra legacy interrupt controller. */
> static void __iomem *tegra_ictlr_base;
> @@ -211,6 +214,15 @@ static void tegra_route_irq_to_xen(struct irq_desc
> *desc, unsigned int priority)
> }
>
> /*
> + * Use platform specific mapping for hook to initialize mediated legacy
> + * interrupt controller for hardware domain.
> + */
> +static int tegra_specific_mapping(struct domain *d)
> +{
> + return domain_tegra_mlic_init(d);
domain_tegra_mlic_init doesn't map anything. In fact, it unmaps a memory
region. Please call domain_tegra_mlic_init from another function, maybe
tegra_init.
> +}
> +
> +/*
> * Read register from specified legacy interrupt interrupt controller.
> */
> uint32_t tegra_lic_readl(unsigned int ictlr_index, unsigned int
> register_offset)
> @@ -300,6 +312,7 @@ PLATFORM_START(tegra, "Tegra")
> .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,
> + .specific_mapping = tegra_specific_mapping,
> PLATFORM_END
>
> /*
> diff --git a/xen/include/asm-arm/platforms/tegra-mlic.h
> b/xen/include/asm-arm/platforms/tegra-mlic.h
> new file mode 100644
> index 0000000..fee10c4
> --- /dev/null
> +++ b/xen/include/asm-arm/platforms/tegra-mlic.h
> @@ -0,0 +1,34 @@
> +/*
> + * xen/arch/arm/vuart.h
> + *
> + * Mediated Tegra Legacy Interrupt Controller
> + *
> + * 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_MLIC_H
> +#define __ASM_ARM_PLATFORMS_TEGRA_MLIC_H
> +
> +int domain_tegra_mlic_init(struct domain *d);
> +
> +#endif /* __ASM_ARM_PLATFORMS_TEGRA_MLIC_H */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> --
> 2.1.4
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |