From mboxrd@z Thu Jan  1 00:00:00 1970
From: Vijay Kilari <vijay.kilari@gmail.com>
Subject: Re: [PATCH v7 18/28] xen/arm: ITS: Export ITS info to
 Virtual ITS
Date: Thu, 24 Sep 2015 10:56:27 +0530
Message-ID: <CALicx6u18yysgdUC0uzh_ZnfSW2bacm9V3RB7AD=G8sgNELMiQ@mail.gmail.com>
References: <1442581755-2668-1-git-send-email-vijay.kilari@gmail.com>
	<1442581755-2668-19-git-send-email-vijay.kilari@gmail.com>
	<5602634E.4000900@citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <5602634E.4000900@citrix.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Julien Grall <julien.grall@citrix.com>
Cc: Ian Campbell <Ian.Campbell@citrix.com>, Stefano Stabellini <stefano.stabellini@eu.citrix.com>, Prasun Kapoor <Prasun.Kapoor@caviumnetworks.com>, Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>, Tim Deegan <tim@xen.org>, "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>, Stefano Stabellini <stefano.stabellini@citrix.com>, manish.jaggi@caviumnetworks.com
List-Id: xen-devel@lists.xenproject.org

On Wed, Sep 23, 2015 at 2:01 PM, Julien Grall <julien.grall@citrix.com> wrote:
> Hi Vijay,
>
> On 18/09/15 14:09, vijay.kilari@gmail.com wrote:
>> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
>> index 0d5c61c..f3346d3 100644
>> --- a/xen/arch/arm/gic-v3-its.c
>> +++ b/xen/arch/arm/gic-v3-its.c
>> @@ -37,6 +37,7 @@
>>  #include <asm/gic.h>
>>  #include <asm/gic_v3_defs.h>
>>  #include <asm/gic-its.h>
>> +#include <asm/vits.h>
>>  #include <xen/log2.h>
>>
>>  #define its_print(lvl, fmt, ...)                                      \
>> @@ -88,6 +89,11 @@ struct its_node {
>>      struct dt_device_node   *dt_node;
>>  };
>>
>> +static struct {
>> +    uint8_t eventID_bits;
>> +    uint8_t devID_bits;
>> +} its_data;
>> +
>>  #define ITS_ITT_ALIGN    SZ_256
>>
>>  static LIST_HEAD(its_nodes);
>> @@ -914,6 +920,8 @@ int its_assign_device(struct domain *d, u32 vdevid, u32 pdevid)
>>
>>      for ( i = 0; i < pdev->event_map.nr_lpis; i++ )
>>      {
>> +        ASSERT(i < (1 << its_data.eventID_bits));
>
> I'd like to understand why you choose to use an ASSERT here.
>
> ASSERT means that we expect this condition never happen, although the
> nr_lpis is calculated using the number of MSI handled by a device.
>
> Given that we don't control the number of MSI, a proper check in
> its_add_device seem better than this ASSERT.
>
>> +
>>          plpi = its_get_plpi(pdev, i);
>>          /* TODO: Route lpi */
>>      }
>> @@ -1366,6 +1374,8 @@ static int its_probe(struct dt_device_node *node)
>>      its->phys_size = its_size;
>>      typer = readl_relaxed(its_base + GITS_TYPER);
>>      its->ite_size = ((typer >> 4) & 0xf) + 1;
>> +    its_data.eventID_bits = GITS_TYPER_IDBITS(typer);
>> +    its_data.devID_bits = GITS_TYPER_DEVBITS(typer);
>
> The 2 fields will be override every time a new ITS node will be initialized.
>
> What ensure that all the ITS have the same number of Event ID and Device ID?

Nothing of I thing of ensures this at HW level. Either we should
assume that all the ITS
have number of Event and Device ID or we choose lower of all the ITS?.

Regards
Vijay