Re: [Xen-devel] [PATCH v5 1/6] xen/arm: vgic-v2: Implement correctly ITARGETSR0 - ITARGETSR7 read-only

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Mon, 9 Nov 2015, Julien Grall wrote:
> Each ITARGETSR register are 4-byte wide and the offset is in byte.
> 
> The current implementation is computing the end of the range wrongly
> resulting to emulate only ITARGETSR{0,1} read-only. The rest will be
> treated as read-write.
> 
> As 8 registers should be read-only, the end of the range should be
> ITARGETSR + (4 * 8) - 1.
> 
> For convenience introduce ITARGETSR7 and ITARGETSR8.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>

Reviewed-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>


>     This would be a good candidate to backport. Without it a guest could
>     modify ITARGETSR{0-7} and redirect the interrupt to the wrong vCPU.
> 
>     Spotted while testing to boot FreeBSD guest with this series.
>     FreeBSD is writing in ITARGETSR{0 - 7} and will therefore crash xen
>     due to the valid ASSERT in vgic_store_itargetsr.
> 
>     Note that the emulation is not properly emulated the last register
>     of each range. I'm planning to fix it in a follow-up series.
> 
>     Changes in v5:
>         - Patch added
> ---
>  xen/arch/arm/vgic-v2.c    | 4 ++--
>  xen/include/asm-arm/gic.h | 2 ++
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
> index f7d784b..041291c 100644
> --- a/xen/arch/arm/vgic-v2.c
> +++ b/xen/arch/arm/vgic-v2.c
> @@ -338,11 +338,11 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, 
> mmio_info_t *info,
>                 v, r, gicd_reg - GICD_ICACTIVER);
>          return 0;
>  
> -    case GICD_ITARGETSR ... GICD_ITARGETSR + 7:
> +    case GICD_ITARGETSR ... GICD_ITARGETSR7:
>          /* SGI/PPI target is read only */
>          goto write_ignore_32;
>  
> -    case GICD_ITARGETSR + 8 ... GICD_ITARGETSRN:
> +    case GICD_ITARGETSR8 ... GICD_ITARGETSRN:
>      {
>          /* unsigned long needed for find_next_bit */
>          unsigned long target;
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index 0116481..3064d1c 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -42,6 +42,8 @@
>  #define GICD_IPRIORITYR (0x400)
>  #define GICD_IPRIORITYRN (0x7F8)
>  #define GICD_ITARGETSR  (0x800)
> +#define GICD_ITARGETSR7 (0x81C)
> +#define GICD_ITARGETSR8 (0x820)
>  #define GICD_ITARGETSRN (0xBF8)
>  #define GICD_ICFGR      (0xC00)
>  #define GICD_ICFGRN     (0xCFC)
> -- 
> 2.1.4
> 

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