Re: [PATCH v4 01/27] ARM: GICv3 ITS: parse and store ITS subnodes from hardware DT

[Stefano Stabellini <sstabellini@kernel.org>]

On Mon, 3 Apr 2017, Andre Przywara wrote:
> Parse the DT GIC subnodes to find every ITS MSI controller the hardware
> offers. Store that information in a list to both propagate all of them
> later to Dom0, but also to be able to iterate over all ITSes.
> This introduces an ITS Kconfig option.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>
> ---
>  xen/arch/arm/Kconfig             |  5 +++
>  xen/arch/arm/Makefile            |  1 +
>  xen/arch/arm/gic-v3-its.c        | 77 ++++++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/gic-v3.c            | 10 +++---
>  xen/include/asm-arm/gic_v3_its.h | 67 ++++++++++++++++++++++++++++++++++
>  5 files changed, 156 insertions(+), 4 deletions(-)
>  create mode 100644 xen/arch/arm/gic-v3-its.c
>  create mode 100644 xen/include/asm-arm/gic_v3_its.h
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 43123e6..d46b98c 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -45,6 +45,11 @@ config ACPI
>  config HAS_GICV3
>  	bool
>  
> +config HAS_ITS
> +        bool
> +        prompt "GICv3 ITS MSI controller support" if EXPERT = "y"
> +        depends on HAS_GICV3
> +
>  endmenu
>  
>  menu "ARM errata workaround via the alternative framework"
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 0ce94a8..39c0a03 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -18,6 +18,7 @@ obj-$(EARLY_PRINTK) += early_printk.o
>  obj-y += gic.o
>  obj-y += gic-v2.o
>  obj-$(CONFIG_HAS_GICV3) += gic-v3.o
> +obj-$(CONFIG_HAS_ITS) += gic-v3-its.o
>  obj-y += guestcopy.o
>  obj-y += hvm.o
>  obj-y += io.o
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> new file mode 100644
> index 0000000..6b02349
> --- /dev/null
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -0,0 +1,77 @@
> +/*
> + * xen/arch/arm/gic-v3-its.c
> + *
> + * ARM GICv3 Interrupt Translation Service (ITS) support
> + *
> + * Copyright (C) 2016,2017 - ARM Ltd
> + *
> + * 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; under version 2 of the License.
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <xen/lib.h>
> +#include <asm/gic_v3_defs.h>
> +#include <asm/gic_v3_its.h>
> +
> +/*
> + * No lock here, as this list gets only populated upon boot while scanning
> + * firmware tables for all host ITSes, and only gets iterated afterwards.
> + */
> +LIST_HEAD(host_its_list);
> +
> +bool gicv3_its_host_has_its(void)
> +{
> +    return !list_empty(&host_its_list);
> +}
> +
> +/* Scan the DT for any ITS nodes and create a list of host ITSes out of it. */
> +void gicv3_its_dt_init(const struct dt_device_node *node)
> +{
> +    const struct dt_device_node *its = NULL;
> +    struct host_its *its_data;
> +
> +    /*
> +     * Check for ITS MSI subnodes. If any, add the ITS register
> +     * frames to the ITS list.
> +     */
> +    dt_for_each_child_node(node, its)
> +    {
> +        uint64_t addr, size;
> +
> +        if ( !dt_device_is_compatible(its, "arm,gic-v3-its") )
> +            continue;
> +
> +        if ( dt_device_get_address(its, 0, &addr, &size) )
> +            panic("GICv3: Cannot find a valid ITS frame address");
> +
> +        its_data = xzalloc(struct host_its);
> +        if ( !its_data )
> +            panic("GICv3: Cannot allocate memory for ITS frame");
> +
> +        its_data->addr = addr;
> +        its_data->size = size;
> +        its_data->dt_node = its;
> +
> +        printk("GICv3: Found ITS @0x%lx\n", addr);
> +
> +        list_add_tail(&its_data->entry, &host_its_list);
> +    }
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index 695f01f..b626298 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -42,6 +42,7 @@
>  #include <asm/device.h>
>  #include <asm/gic.h>
>  #include <asm/gic_v3_defs.h>
> +#include <asm/gic_v3_its.h>
>  #include <asm/cpufeature.h>
>  #include <asm/acpi.h>
>  
> @@ -1227,11 +1228,12 @@ static void __init gicv3_dt_init(void)
>       */
>      res = dt_device_get_address(node, 1 + gicv3.rdist_count,
>                                  &cbase, &csize);
> -    if ( res )
> -        return;
> +    if ( !res )
> +        dt_device_get_address(node, 1 + gicv3.rdist_count + 2,
> +                              &vbase, &vsize);
>  
> -    dt_device_get_address(node, 1 + gicv3.rdist_count + 2,
> -                          &vbase, &vsize);
> +    /* Check for ITS child nodes and build the host ITS list accordingly. */
> +    gicv3_its_dt_init(node);
>  }
>  
>  static int gicv3_iomem_deny_access(const struct domain *d)
> diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h
> new file mode 100644
> index 0000000..765a655
> --- /dev/null
> +++ b/xen/include/asm-arm/gic_v3_its.h
> @@ -0,0 +1,67 @@
> +/*
> + * ARM GICv3 ITS support
> + *
> + * Andre Przywara <andre.przywara@arm.com>
> + * Copyright (c) 2016,2017 ARM Ltd.
> + *
> + * 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; under version 2 of the License.
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef __ASM_ARM_ITS_H__
> +#define __ASM_ARM_ITS_H__
> +
> +#include <xen/device_tree.h>
> +
> +/* data structure for each hardware ITS */
> +struct host_its {
> +    struct list_head entry;
> +    const struct dt_device_node *dt_node;
> +    paddr_t addr;
> +    paddr_t size;
> +};
> +
> +
> +#ifdef CONFIG_HAS_ITS
> +
> +extern struct list_head host_its_list;
> +
> +/* Parse the host DT and pick up all host ITSes. */
> +void gicv3_its_dt_init(const struct dt_device_node *node);
> +
> +bool gicv3_its_host_has_its(void);
> +
> +#else
> +
> +static LIST_HEAD(host_its_list);

Sorry for not spotting this earlier, but shouldn't this be extern
instead of static? If we define a static variable in an header file, I
think it will end up being created in every .c file that includes this
header?


> +static inline void gicv3_its_dt_init(const struct dt_device_node *node)
> +{
> +}
> +
> +static inline bool gicv3_its_host_has_its(void)
> +{
> +    return false;
> +}
> +
> +#endif /* CONFIG_HAS_ITS */
> +
> +#endif
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> -- 
> 2.9.0
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel