Re: [Xen-devel] [PATCH v5 03/30] ARM: GICv3 ITS: parse and store ITS subnodes from hardware DT

[Andre Przywara <andre.przywara@xxxxxxx>]

Hi,

On 06/04/17 00:26, Stefano Stabellini wrote:
> On Thu, 6 Apr 2017, Andre Przywara wrote:
>> Parse the GIC subnodes in the device tree 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 (as an EXPERT option), use
>> XEN_CONFIG_EXPERT=y on the make command line to see and use the option.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
>> ---
>>  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@xxxxxxx>
>> + * 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);
> 
> This is a problem

So I got rid of this by doing the host_its_list iteration in
vgic-v3-its.c, as Julien suggested. This allows this host list to vanish
from a non-ITS config completely.

Thanks for pointing this out!

Cheers,
Andre.

> 
>> +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.8.2
>>

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