Re: [PATCH v7 2/5] xen/arm: Add virtual GICv3 support

[Julien Grall <julien.grall@linaro.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