Re: [PATCH 08/10] xen/arm: vgic-v2: Don't check the size when we ignore the write/read a zero

[Ian Campbell <Ian.Campbell@citrix.com>]

On Mon, 2015-01-19 at 16:29 +0000, Julien Grall wrote:
> Some registers, such as GICD_ITARGET0 can be read/write with different
> size.
> 
> When the write is ignored only word-access is checked. This will lead to
> a domain crash if the guest is trying to access via byte-word.
> 
> In general, it's not necessary/important to check the size.

In general it is, unless you have a reference which says otherwise for a
given register.

In the specific case of ITARGET this is true though, but as with the v3
version I disagree with printing in the RAZ/WI cases, we should print
only on bad width accesses.

I think this probably means you want to switch the goto's in the ITARGET
case to something else, probably implementing read_as_zero_* for checked
cases.

>  It's better
> to log it to let know the guest that its access will have no effect.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> 
> ---
>     This patch should be backported to Xen 4.4 and 4.5 as it fixes
>     byte-access to GICD_ITARGET0.
> 
>     Although, this patch won't apply directly to Xen 4.4.
> ---
>  xen/arch/arm/vgic-v2.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
> index a34a0c7..8c6ca72 100644
> --- a/xen/arch/arm/vgic-v2.c
> +++ b/xen/arch/arm/vgic-v2.c
> @@ -227,7 +227,8 @@ bad_width:
>      return 0;
>  
>  read_as_zero:
> -    if ( dabt.size != DABT_WORD ) goto bad_width;
> +    gdprintk(XENLOG_DEBUG, "vGICD: read as zero width, %d r%d offset %#08x\n",
> +             dabt.size, dabt.reg, gicd_reg);
>      *r = 0;
>      return 1;
>  }
> @@ -498,7 +499,8 @@ bad_width:
>      return 0;
>  
>  write_ignore:
> -    if ( dabt.size != DABT_WORD ) goto bad_width;
> +    gdprintk(XENLOG_DEBUG, "vGICD: ignore write width %d r%d offset %#08x\n",
> +             dabt.size, dabt.reg, gicd_reg);
>      return 1;
>  }
>