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

From: Julien Grall <julien.grall@arm.com>
To: Stefano Stabellini <sstabellini@kernel.org>,
	Andre Przywara <andre.przywara@arm.com>
Cc: xen-devel@lists.xenproject.org,
	Shanker Donthineni <shankerd@codeaurora.org>,
	Vijay Kilari <vijay.kilari@gmail.com>
Subject: Re: [PATCH v5 03/30] ARM: GICv3 ITS: parse and store ITS subnodes from hardware DT
Date: Thu, 6 Apr 2017 13:39:23 +0100	[thread overview]
Message-ID: <632f38ce-4775-914d-7dac-afe5d3f6ab5a@arm.com> (raw)
In-Reply-To: <alpine.DEB.2.10.1704051626270.3188@sstabellini-ThinkPad-X260>

Hi Andre,

On 06/04/17 00:26, Stefano Stabellini wrote:
> On Thu, 6 Apr 2017, Andre Przywara wrote:
>> 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);
>
> This is a problem

As far as understand this is here because host_its_list is used 
unconditionally in vgic-v3.c. But this should not be necessary if you 
followed what I asked at least 5 times now and kept ignoring. I.e moving 
the creation of the multiple ITS in a separate function in vgic-v3-its.c.

We are one day before the code freeze, if you want this to get merged. 
Then please address *all* the comments.

Cheers,

-- 
Julien Grall

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