From mboxrd@z Thu Jan  1 00:00:00 1970
From: Julien Grall <julien.grall@linaro.org>
Subject: Re: [PATCH v7 2/5] xen/arm: Add virtual GICv3 support
Date: Mon, 14 Jul 2014 16:59:23 +0100
Message-ID: <53C3FE5B.60304@linaro.org>
References: <1405083092-20216-1-git-send-email-vijay.kilari@gmail.com>
	<1405083092-20216-3-git-send-email-vijay.kilari@gmail.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: <1405083092-20216-3-git-send-email-vijay.kilari@gmail.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: vijay.kilari@gmail.com, Ian.Campbell@citrix.com, stefano.stabellini@eu.citrix.com, stefano.stabellini@citrix.com, tim@xen.org, xen-devel@lists.xen.org
Cc: Prasun.Kapoor@caviumnetworks.com, vijaya.kumar@caviumnetworks.com, manish.jaggi@caviumnetworks.com
List-Id: xen-devel@lists.xenproject.org

Hi Vijay,

On 07/11/2014 01:51 PM, vijay.kilari@gmail.com wrote:
> +static int vgic_v3_rdistr_mmio_read(struct vcpu *v, mmio_info_t *info)
> +{
> +    uint32_t offset;
> +
> +    offset = info->gpa & (v->domain->arch.vgic.rdist_stride - 1);
> +
> +    if ( offset < SZ_64K )
> +        return __vgic_v3_rdistr_rd_mmio_read(v, info, offset);
> +    else  if ( (offset >= SZ_64K) && (offset < 2 * SZ_64K) )
> +        return vgic_v3_rdistr_sgi_mmio_read(v, info, (offset - SZ_64K));
> +    else
> +        gdprintk(XENLOG_WARNING, "vGICv3: vGICR: unknown gpa read address \
> +                  %"PRIpaddr"\n", info->gpa);

I though I already said that on a previous version...

This is wrong, when you use
     "foo\
         base\n"

The compiler will concatenate the 2 strings without remove the tab/space
on the second one. So you will print smth like:

"foo           base\n"

You have to use
	"foo"
  	"base\n"

The remark is the same on various place within this file.

> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index d223db4..3647497 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -69,6 +69,11 @@ int domain_vgic_init(struct domain *d)
>  
>      switch ( gic_hw_version() )
>      {
> +#ifdef CONFIG_ARM_64
> +    case GIC_V3:
> +        vgic_v3_init(d);

You forgot to check the return value of vgic_v3_init.

Regards,

-- 
Julien Grall