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

[Julien Grall <julien.grall@arm.com>]

Hi Andre,

On 03/16/2017 11:20 AM, Andre Przywara wrote:
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 2e023d1..bf64c61 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -45,6 +45,10 @@ config ACPI
>  config HAS_GICV3
>  	bool
>
> +config HAS_ITS
> +        bool "GICv3 ITS MSI controller support"
> +        depends on HAS_GICV3

IIRC, we discussed that GICv3 ITS will be a technical preview for Xen 
4.9. I think case I think it should depends on EXPERT mode to avoid been 
shipped by default.

> +
>  endmenu
>
>  menu "ARM errata workaround via the alternative framework"

[...]

> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> new file mode 100644
> index 0000000..4056e5b
> --- /dev/null
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -0,0 +1,73 @@
> +/*
> + * 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>
> +
> +LIST_HEAD(host_its_list);

On the previous version we agreed that host_its_list should be confined 
to gic-v3-its.c. So why is it still exported?

[...]

> 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 @@


[...]

> +#else
> +
> +static LIST_HEAD(host_its_list);

This is quite ugly and could really be avoided if the host_its_list was 
confined as we agreed in the previous version.

Cheers,

-- 
Julien Grall

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