diff mbox series

[Xen-devel,02/14,v4] xen/arm: vpl011: Define generic vreg_reg* access functions in vreg.h

Message ID 1496769929-23355-3-git-send-email-bhupinder.thakur@linaro.org
State New
Headers show
Series PL011 emulation support in Xen | expand

Commit Message

Bhupinder Thakur June 6, 2017, 5:25 p.m. UTC
This patch redefines the vgic_reg* access functions to vreg_reg* functions.
These are generic functions, which will be used by the vgic emulation code
to access the vgic registers.

PL011 emulation code will also use vreg_reg* access functions.

Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
---
CC: ss
CC: jg

Changes since v3:
- Renamed DEFINE_VREG_REG_HELPERS to VREG_REG_HELPERS.

 xen/arch/arm/vgic-v2.c     |  28 +++++------
 xen/arch/arm/vgic-v3.c     |  40 ++++++++--------
 xen/include/asm-arm/vreg.h | 115 ++++++++++++++++++++++-----------------------
 3 files changed, 91 insertions(+), 92 deletions(-)

Comments

Julien Grall June 9, 2017, 12:54 p.m. UTC | #1
Hi Bhupinder,

On 06/06/17 18:25, Bhupinder Thakur wrote:
> -/* N-bit register helpers */
> -#define VGIC_REG_HELPERS(sz, offmask)                                   \
> -static inline register_t vgic_reg##sz##_extract(uint##sz##_t reg,       \
> -                                                const mmio_info_t *info)\

[...]

> +#define VREG_REG_HELPERS(sz, offmask)                                           \
> +/* N-bit register helpers */                                                    \
> +static inline register_t vreg_reg##sz##_extract(uint##sz##_t reg,               \
> +                                                  const mmio_info_t *info)      \

Please don't modify the indentation. There is no reason to do that and 
make the review more difficult to do. This patch should *only* replace 
vgic by vreg.

> @@ -211,10 +211,9 @@ static inline void vgic_reg##sz##_clearbits(uint##sz##_t *reg,          \
>   * unsigned long rather than uint64_t
>   */
>  #if BITS_PER_LONG == 64
> -VGIC_REG_HELPERS(64, 0x7);
> +VREG_REG_HELPERS(64, 0x7);
>  #endif
> -VGIC_REG_HELPERS(32, 0x3);
> -

Why did you drop this line?

> -#undef VGIC_REG_HELPERS
> +VREG_REG_HELPERS(32, 0x3);
> +#undef VREG_REG_HELPERS
>
>  #endif /* __ASM_ARM_VREG__ */
>

Cheers,
Andre Przywara June 19, 2017, 9:33 a.m. UTC | #2
Hi Bhupinder,

I think the commit message is a bit misleading.
Actually you *rename* functions and their call sites, and also this
touches the VGIC code, so shouldn't it mention both in the first line of
the commit message? After all this patch really has not much to do with
vpl011.

On 06/06/17 18:25, Bhupinder Thakur wrote:
> This patch redefines the vgic_reg* access functions to vreg_reg* functions.
> These are generic functions, which will be used by the vgic emulation code
> to access the vgic registers.
> 
> PL011 emulation code will also use vreg_reg* access functions.

Also I am sorry to be the bearer of bad news (and also for being the
origin of this), but I am afraid you have to rework this when you rebase
it against origin/staging, since the ITS emulation has been merged.
So while actual conflicts seem to be trivial, there are now many new
users of vgic_reg??_* which you have to change as well.
Should be rather mechanical, though.

Cheers,
Andre.

> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
> ---
> CC: ss
> CC: jg
> 
> Changes since v3:
> - Renamed DEFINE_VREG_REG_HELPERS to VREG_REG_HELPERS.
> 
>  xen/arch/arm/vgic-v2.c     |  28 +++++------
>  xen/arch/arm/vgic-v3.c     |  40 ++++++++--------
>  xen/include/asm-arm/vreg.h | 115 ++++++++++++++++++++++-----------------------
>  3 files changed, 91 insertions(+), 92 deletions(-)
> 
> diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
> index dc9f95b..3e35a90 100644
> --- a/xen/arch/arm/vgic-v2.c
> +++ b/xen/arch/arm/vgic-v2.c
> @@ -179,7 +179,7 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
>      case VREG32(GICD_CTLR):
>          if ( dabt.size != DABT_WORD ) goto bad_width;
>          vgic_lock(v);
> -        *r = vgic_reg32_extract(v->domain->arch.vgic.ctlr, info);
> +        *r = vreg_reg32_extract(v->domain->arch.vgic.ctlr, info);
>          vgic_unlock(v);
>          return 1;
>  
> @@ -194,7 +194,7 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
>              | DIV_ROUND_UP(v->domain->arch.vgic.nr_spis, 32);
>          vgic_unlock(v);
>  
> -        *r = vgic_reg32_extract(typer, info);
> +        *r = vreg_reg32_extract(typer, info);
>  
>          return 1;
>      }
> @@ -205,7 +205,7 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
>           * XXX Do we need a JEP106 manufacturer ID?
>           * Just use the physical h/w value for now
>           */
> -        *r = vgic_reg32_extract(0x0000043b, info);
> +        *r = vreg_reg32_extract(0x0000043b, info);
>          return 1;
>  
>      case VRANGE32(0x00C, 0x01C):
> @@ -226,7 +226,7 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
>          rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISENABLER, DABT_WORD);
>          if ( rank == NULL) goto read_as_zero;
>          vgic_lock_rank(v, rank, flags);
> -        *r = vgic_reg32_extract(rank->ienable, info);
> +        *r = vreg_reg32_extract(rank->ienable, info);
>          vgic_unlock_rank(v, rank, flags);
>          return 1;
>  
> @@ -235,7 +235,7 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
>          rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICENABLER, DABT_WORD);
>          if ( rank == NULL) goto read_as_zero;
>          vgic_lock_rank(v, rank, flags);
> -        *r = vgic_reg32_extract(rank->ienable, info);
> +        *r = vreg_reg32_extract(rank->ienable, info);
>          vgic_unlock_rank(v, rank, flags);
>          return 1;
>  
> @@ -262,7 +262,7 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
>                                                       gicd_reg - GICD_IPRIORITYR,
>                                                       DABT_WORD)];
>          vgic_unlock_rank(v, rank, flags);
> -        *r = vgic_reg32_extract(ipriorityr, info);
> +        *r = vreg_reg32_extract(ipriorityr, info);
>  
>          return 1;
>      }
> @@ -280,7 +280,7 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
>          vgic_lock_rank(v, rank, flags);
>          itargetsr = vgic_fetch_itargetsr(rank, gicd_reg - GICD_ITARGETSR);
>          vgic_unlock_rank(v, rank, flags);
> -        *r = vgic_reg32_extract(itargetsr, info);
> +        *r = vreg_reg32_extract(itargetsr, info);
>  
>          return 1;
>      }
> @@ -299,7 +299,7 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
>          icfgr = rank->icfg[REG_RANK_INDEX(2, gicd_reg - GICD_ICFGR, DABT_WORD)];
>          vgic_unlock_rank(v, rank, flags);
>  
> -        *r = vgic_reg32_extract(icfgr, info);
> +        *r = vreg_reg32_extract(icfgr, info);
>  
>          return 1;
>      }
> @@ -424,7 +424,7 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
>          if ( dabt.size != DABT_WORD ) goto bad_width;
>          /* Ignore all but the enable bit */
>          vgic_lock(v);
> -        vgic_reg32_update(&v->domain->arch.vgic.ctlr, r, info);
> +        vreg_reg32_update(&v->domain->arch.vgic.ctlr, r, info);
>          v->domain->arch.vgic.ctlr &= GICD_CTL_ENABLE;
>          vgic_unlock(v);
>  
> @@ -454,7 +454,7 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
>          if ( rank == NULL) goto write_ignore;
>          vgic_lock_rank(v, rank, flags);
>          tr = rank->ienable;
> -        vgic_reg32_setbits(&rank->ienable, r, info);
> +        vreg_reg32_setbits(&rank->ienable, r, info);
>          vgic_enable_irqs(v, (rank->ienable) & (~tr), rank->index);
>          vgic_unlock_rank(v, rank, flags);
>          return 1;
> @@ -465,7 +465,7 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
>          if ( rank == NULL) goto write_ignore;
>          vgic_lock_rank(v, rank, flags);
>          tr = rank->ienable;
> -        vgic_reg32_clearbits(&rank->ienable, r, info);
> +        vreg_reg32_clearbits(&rank->ienable, r, info);
>          vgic_disable_irqs(v, (~rank->ienable) & tr, rank->index);
>          vgic_unlock_rank(v, rank, flags);
>          return 1;
> @@ -508,7 +508,7 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
>          ipriorityr = &rank->ipriorityr[REG_RANK_INDEX(8,
>                                                        gicd_reg - GICD_IPRIORITYR,
>                                                        DABT_WORD)];
> -        vgic_reg32_update(ipriorityr, r, info);
> +        vreg_reg32_update(ipriorityr, r, info);
>          vgic_unlock_rank(v, rank, flags);
>          return 1;
>      }
> @@ -529,7 +529,7 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
>          if ( rank == NULL) goto write_ignore;
>          vgic_lock_rank(v, rank, flags);
>          itargetsr = vgic_fetch_itargetsr(rank, gicd_reg - GICD_ITARGETSR);
> -        vgic_reg32_update(&itargetsr, r, info);
> +        vreg_reg32_update(&itargetsr, r, info);
>          vgic_store_itargetsr(v->domain, rank, gicd_reg - GICD_ITARGETSR,
>                               itargetsr);
>          vgic_unlock_rank(v, rank, flags);
> @@ -551,7 +551,7 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
>          rank = vgic_rank_offset(v, 2, gicd_reg - GICD_ICFGR, DABT_WORD);
>          if ( rank == NULL) goto write_ignore;
>          vgic_lock_rank(v, rank, flags);
> -        vgic_reg32_update(&rank->icfg[REG_RANK_INDEX(2, gicd_reg - GICD_ICFGR,
> +        vreg_reg32_update(&rank->icfg[REG_RANK_INDEX(2, gicd_reg - GICD_ICFGR,
>                                                       DABT_WORD)],
>                            r, info);
>          vgic_unlock_rank(v, rank, flags);
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index d10757a..e1213d9 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -181,7 +181,7 @@ static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info,
>  
>      case VREG32(GICR_IIDR):
>          if ( dabt.size != DABT_WORD ) goto bad_width;
> -        *r = vgic_reg32_extract(GICV3_GICR_IIDR_VAL, info);
> +        *r = vreg_reg32_extract(GICV3_GICR_IIDR_VAL, info);
>          return 1;
>  
>      case VREG64(GICR_TYPER):
> @@ -199,7 +199,7 @@ static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info,
>          if ( v->arch.vgic.flags & VGIC_V3_RDIST_LAST )
>              typer |= GICR_TYPER_LAST;
>  
> -        *r = vgic_reg64_extract(typer, info);
> +        *r = vreg_reg64_extract(typer, info);
>  
>          return 1;
>      }
> @@ -257,7 +257,7 @@ static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info,
>      case VREG32(GICR_SYNCR):
>          if ( dabt.size != DABT_WORD ) goto bad_width;
>          /* RO . But when read it always returns busy bito bit[0] */
> -        *r = vgic_reg32_extract(GICR_SYNCR_NOT_BUSY, info);
> +        *r = vreg_reg32_extract(GICR_SYNCR_NOT_BUSY, info);
>          return 1;
>  
>      case 0x00C8:
> @@ -284,7 +284,7 @@ static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info,
>  
>      case VREG32(GICR_PIDR2):
>          if ( dabt.size != DABT_WORD ) goto bad_width;
> -        *r = vgic_reg32_extract(GICV3_GICR_PIDR2, info);
> +        *r = vreg_reg32_extract(GICV3_GICR_PIDR2, info);
>           return 1;
>  
>      case 0xFFEC ... 0xFFFC:
> @@ -328,7 +328,7 @@ read_reserved:
>      return 1;
>  
>  read_unknown:
> -    *r = vgic_reg64_extract(0xdeadbeafdeadbeaf, info);
> +    *r = vreg_reg64_extract(0xdeadbeafdeadbeaf, info);
>      return 1;
>  }
>  
> @@ -489,7 +489,7 @@ static int __vgic_v3_distr_common_mmio_read(const char *name, struct vcpu *v,
>          rank = vgic_rank_offset(v, 1, reg - GICD_ISENABLER, DABT_WORD);
>          if ( rank == NULL ) goto read_as_zero;
>          vgic_lock_rank(v, rank, flags);
> -        *r = vgic_reg32_extract(rank->ienable, info);
> +        *r = vreg_reg32_extract(rank->ienable, info);
>          vgic_unlock_rank(v, rank, flags);
>          return 1;
>  
> @@ -498,7 +498,7 @@ static int __vgic_v3_distr_common_mmio_read(const char *name, struct vcpu *v,
>          rank = vgic_rank_offset(v, 1, reg - GICD_ICENABLER, DABT_WORD);
>          if ( rank == NULL ) goto read_as_zero;
>          vgic_lock_rank(v, rank, flags);
> -        *r = vgic_reg32_extract(rank->ienable, info);
> +        *r = vreg_reg32_extract(rank->ienable, info);
>          vgic_unlock_rank(v, rank, flags);
>          return 1;
>  
> @@ -525,7 +525,7 @@ static int __vgic_v3_distr_common_mmio_read(const char *name, struct vcpu *v,
>                                                       DABT_WORD)];
>          vgic_unlock_rank(v, rank, flags);
>  
> -        *r = vgic_reg32_extract(ipriorityr, info);
> +        *r = vreg_reg32_extract(ipriorityr, info);
>  
>          return 1;
>      }
> @@ -541,7 +541,7 @@ static int __vgic_v3_distr_common_mmio_read(const char *name, struct vcpu *v,
>          icfgr = rank->icfg[REG_RANK_INDEX(2, reg - GICD_ICFGR, DABT_WORD)];
>          vgic_unlock_rank(v, rank, flags);
>  
> -        *r = vgic_reg32_extract(icfgr, info);
> +        *r = vreg_reg32_extract(icfgr, info);
>  
>          return 1;
>      }
> @@ -585,7 +585,7 @@ static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
>          if ( rank == NULL ) goto write_ignore;
>          vgic_lock_rank(v, rank, flags);
>          tr = rank->ienable;
> -        vgic_reg32_setbits(&rank->ienable, r, info);
> +        vreg_reg32_setbits(&rank->ienable, r, info);
>          vgic_enable_irqs(v, (rank->ienable) & (~tr), rank->index);
>          vgic_unlock_rank(v, rank, flags);
>          return 1;
> @@ -596,7 +596,7 @@ static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
>          if ( rank == NULL ) goto write_ignore;
>          vgic_lock_rank(v, rank, flags);
>          tr = rank->ienable;
> -        vgic_reg32_clearbits(&rank->ienable, r, info);
> +        vreg_reg32_clearbits(&rank->ienable, r, info);
>          vgic_disable_irqs(v, (~rank->ienable) & tr, rank->index);
>          vgic_unlock_rank(v, rank, flags);
>          return 1;
> @@ -638,7 +638,7 @@ static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
>          vgic_lock_rank(v, rank, flags);
>          ipriorityr = &rank->ipriorityr[REG_RANK_INDEX(8, reg - GICD_IPRIORITYR,
>                                                        DABT_WORD)];
> -        vgic_reg32_update(ipriorityr, r, info);
> +        vreg_reg32_update(ipriorityr, r, info);
>          vgic_unlock_rank(v, rank, flags);
>          return 1;
>      }
> @@ -653,7 +653,7 @@ static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
>          rank = vgic_rank_offset(v, 2, reg - GICD_ICFGR, DABT_WORD);
>          if ( rank == NULL ) goto write_ignore;
>          vgic_lock_rank(v, rank, flags);
> -        vgic_reg32_update(&rank->icfg[REG_RANK_INDEX(2, reg - GICD_ICFGR,
> +        vreg_reg32_update(&rank->icfg[REG_RANK_INDEX(2, reg - GICD_ICFGR,
>                                                       DABT_WORD)],
>                            r, info);
>          vgic_unlock_rank(v, rank, flags);
> @@ -901,7 +901,7 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
>      case VREG32(GICD_CTLR):
>          if ( dabt.size != DABT_WORD ) goto bad_width;
>          vgic_lock(v);
> -        *r = vgic_reg32_extract(v->domain->arch.vgic.ctlr, info);
> +        *r = vreg_reg32_extract(v->domain->arch.vgic.ctlr, info);
>          vgic_unlock(v);
>          return 1;
>  
> @@ -926,14 +926,14 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
>  
>          typer |= (irq_bits - 1) << GICD_TYPE_ID_BITS_SHIFT;
>  
> -        *r = vgic_reg32_extract(typer, info);
> +        *r = vreg_reg32_extract(typer, info);
>  
>          return 1;
>      }
>  
>      case VREG32(GICD_IIDR):
>          if ( dabt.size != DABT_WORD ) goto bad_width;
> -        *r = vgic_reg32_extract(GICV3_GICD_IIDR_VAL, info);
> +        *r = vreg_reg32_extract(GICV3_GICD_IIDR_VAL, info);
>          return 1;
>  
>      case VREG32(0x000C):
> @@ -1026,7 +1026,7 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
>          irouter = vgic_fetch_irouter(rank, gicd_reg - GICD_IROUTER);
>          vgic_unlock_rank(v, rank, flags);
>  
> -        *r = vgic_reg64_extract(irouter, info);
> +        *r = vreg_reg64_extract(irouter, info);
>  
>          return 1;
>      }
> @@ -1044,7 +1044,7 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
>      case VREG32(GICD_PIDR2):
>          /* GICv3 identification value */
>          if ( dabt.size != DABT_WORD ) goto bad_width;
> -        *r = vgic_reg32_extract(GICV3_GICD_PIDR2, info);
> +        *r = vreg_reg32_extract(GICV3_GICD_PIDR2, info);
>          return 1;
>  
>      case VRANGE32(0xFFEC, 0xFFFC):
> @@ -1107,7 +1107,7 @@ static int vgic_v3_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
>  
>          vgic_lock(v);
>  
> -        vgic_reg32_update(&ctlr, r, info);
> +        vreg_reg32_update(&ctlr, r, info);
>  
>          /* Only EnableGrp1A can be changed */
>          if ( ctlr & GICD_CTLR_ENABLE_G1A )
> @@ -1213,7 +1213,7 @@ static int vgic_v3_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
>          if ( rank == NULL ) goto write_ignore;
>          vgic_lock_rank(v, rank, flags);
>          irouter = vgic_fetch_irouter(rank, gicd_reg - GICD_IROUTER);
> -        vgic_reg64_update(&irouter, r, info);
> +        vreg_reg64_update(&irouter, r, info);
>          vgic_store_irouter(v->domain, rank, gicd_reg - GICD_IROUTER, irouter);
>          vgic_unlock_rank(v, rank, flags);
>          return 1;
> diff --git a/xen/include/asm-arm/vreg.h b/xen/include/asm-arm/vreg.h
> index 348584f..16207ce 100644
> --- a/xen/include/asm-arm/vreg.h
> +++ b/xen/include/asm-arm/vreg.h
> @@ -107,102 +107,102 @@ static inline bool vreg_emulate_sysreg64(struct cpu_user_regs *regs, union hsr h
>  
>  #endif
>  
> -#define VGIC_REG_MASK(size) ((~0UL) >> (BITS_PER_LONG - ((1 << (size)) * 8)))
> +#define VREG_REG_MASK(size) ((~0UL) >> (BITS_PER_LONG - ((1 << (size)) * 8)))
>  
>  /*
>   * The check on the size supported by the register has to be done by
> - * the caller of vgic_regN_*.
> + * the caller of vreg_regN_*.
>   *
> - * vgic_reg_* should never be called directly. Instead use the vgic_regN_*
> + * vreg_reg_* should never be called directly. Instead use the vreg_regN_*
>   * according to size of the emulated register
>   *
>   * Note that the alignment fault will always be taken in the guest
>   * (see B3.12.7 DDI0406.b).
>   */
> -static inline register_t vgic_reg_extract(unsigned long reg,
> +static inline register_t vreg_reg_extract(unsigned long reg,
>                                            unsigned int offset,
>                                            enum dabt_size size)
>  {
>      reg >>= 8 * offset;
> -    reg &= VGIC_REG_MASK(size);
> +    reg &= VREG_REG_MASK(size);
>  
>      return reg;
>  }
>  
> -static inline void vgic_reg_update(unsigned long *reg, register_t val,
> +static inline void vreg_reg_update(unsigned long *reg, register_t val,
>                                     unsigned int offset,
>                                     enum dabt_size size)
>  {
> -    unsigned long mask = VGIC_REG_MASK(size);
> +    unsigned long mask = VREG_REG_MASK(size);
>      int shift = offset * 8;
>  
>      *reg &= ~(mask << shift);
>      *reg |= ((unsigned long)val & mask) << shift;
>  }
>  
> -static inline void vgic_reg_setbits(unsigned long *reg, register_t bits,
> +static inline void vreg_reg_setbits(unsigned long *reg, register_t bits,
>                                      unsigned int offset,
>                                      enum dabt_size size)
>  {
> -    unsigned long mask = VGIC_REG_MASK(size);
> +    unsigned long mask = VREG_REG_MASK(size);
>      int shift = offset * 8;
>  
>      *reg |= ((unsigned long)bits & mask) << shift;
>  }
>  
> -static inline void vgic_reg_clearbits(unsigned long *reg, register_t bits,
> +static inline void vreg_reg_clearbits(unsigned long *reg, register_t bits,
>                                        unsigned int offset,
>                                        enum dabt_size size)
>  {
> -    unsigned long mask = VGIC_REG_MASK(size);
> +    unsigned long mask = VREG_REG_MASK(size);
>      int shift = offset * 8;
>  
>      *reg &= ~(((unsigned long)bits & mask) << shift);
>  }
>  
> -/* N-bit register helpers */
> -#define VGIC_REG_HELPERS(sz, offmask)                                   \
> -static inline register_t vgic_reg##sz##_extract(uint##sz##_t reg,       \
> -                                                const mmio_info_t *info)\
> -{                                                                       \
> -    return vgic_reg_extract(reg, info->gpa & offmask,                   \
> -                            info->dabt.size);                           \
> -}                                                                       \
> -                                                                        \
> -static inline void vgic_reg##sz##_update(uint##sz##_t *reg,             \
> -                                         register_t val,                \
> -                                         const mmio_info_t *info)       \
> -{                                                                       \
> -    unsigned long tmp = *reg;                                           \
> -                                                                        \
> -    vgic_reg_update(&tmp, val, info->gpa & offmask,                     \
> -                    info->dabt.size);                                   \
> -                                                                        \
> -    *reg = tmp;                                                         \
> -}                                                                       \
> -                                                                        \
> -static inline void vgic_reg##sz##_setbits(uint##sz##_t *reg,            \
> -                                          register_t bits,              \
> -                                          const mmio_info_t *info)      \
> -{                                                                       \
> -    unsigned long tmp = *reg;                                           \
> -                                                                        \
> -    vgic_reg_setbits(&tmp, bits, info->gpa & offmask,                   \
> -                     info->dabt.size);                                  \
> -                                                                        \
> -    *reg = tmp;                                                         \
> -}                                                                       \
> -                                                                        \
> -static inline void vgic_reg##sz##_clearbits(uint##sz##_t *reg,          \
> -                                            register_t bits,            \
> -                                            const mmio_info_t *info)    \
> -{                                                                       \
> -    unsigned long tmp = *reg;                                           \
> -                                                                        \
> -    vgic_reg_clearbits(&tmp, bits, info->gpa & offmask,                 \
> -                       info->dabt.size);                                \
> -                                                                        \
> -    *reg = tmp;                                                         \
> +#define VREG_REG_HELPERS(sz, offmask)                                           \
> +/* N-bit register helpers */                                                    \
> +static inline register_t vreg_reg##sz##_extract(uint##sz##_t reg,               \
> +                                                  const mmio_info_t *info)      \
> +{                                                                               \
> +    return vreg_reg_extract(reg, info->gpa & offmask,                           \
> +                            info->dabt.size);                                   \
> +}                                                                               \
> +                                                                                \
> +static inline void vreg_reg##sz##_update(uint##sz##_t *reg,                     \
> +                                           register_t val,                      \
> +                                           const mmio_info_t *info)             \
> +{                                                                               \
> +    unsigned long tmp = *reg;                                                   \
> +                                                                                \
> +    vreg_reg_update(&tmp, val, info->gpa & offmask,                             \
> +                    info->dabt.size);                                           \
> +                                                                                \
> +    *reg = tmp;                                                                 \
> +}                                                                               \
> +                                                                                \
> +static inline void vreg_reg##sz##_setbits(uint##sz##_t *reg,                    \
> +                                            register_t bits,                    \
> +                                            const mmio_info_t *info)            \
> +{                                                                               \
> +    unsigned long tmp = *reg;                                                   \
> +                                                                                \
> +    vreg_reg_setbits(&tmp, bits, info->gpa & offmask,                           \
> +                     info->dabt.size);                                          \
> +                                                                                \
> +    *reg = tmp;                                                                 \
> +}                                                                               \
> +                                                                                \
> +static inline void vreg_reg##sz##_clearbits(uint##sz##_t *reg,                  \
> +                                              register_t bits,                  \
> +                                              const mmio_info_t *info)          \
> +{                                                                               \
> +    unsigned long tmp = *reg;                                                   \
> +                                                                                \
> +    vreg_reg_clearbits(&tmp, bits, info->gpa & offmask,                         \
> +                       info->dabt.size);                                        \
> +                                                                                \
> +    *reg = tmp;                                                                 \
>  }
>  
>  /*
> @@ -211,10 +211,9 @@ static inline void vgic_reg##sz##_clearbits(uint##sz##_t *reg,          \
>   * unsigned long rather than uint64_t
>   */
>  #if BITS_PER_LONG == 64
> -VGIC_REG_HELPERS(64, 0x7);
> +VREG_REG_HELPERS(64, 0x7);
>  #endif
> -VGIC_REG_HELPERS(32, 0x3);
> -
> -#undef VGIC_REG_HELPERS
> +VREG_REG_HELPERS(32, 0x3);
> +#undef VREG_REG_HELPERS
>  
>  #endif /* __ASM_ARM_VREG__ */
>
Bhupinder Thakur June 19, 2017, 4:53 p.m. UTC | #3
Hi Andre,

On 19 June 2017 at 15:03, Andre Przywara <andre.przywara@arm.com> wrote:
> Hi Bhupinder,
>
> I think the commit message is a bit misleading.
> Actually you *rename* functions and their call sites, and also this
> touches the VGIC code, so shouldn't it mention both in the first line of
> the commit message? After all this patch really has not much to do with
> vpl011.
>
I will modify the commit message to indicate this commit renames
vgic_reg* to vreg_reg*
and modifies all the places where this call is made.

> On 06/06/17 18:25, Bhupinder Thakur wrote:
>> This patch redefines the vgic_reg* access functions to vreg_reg* functions.
>> These are generic functions, which will be used by the vgic emulation code
>> to access the vgic registers.
>>
>> PL011 emulation code will also use vreg_reg* access functions.
>
> Also I am sorry to be the bearer of bad news (and also for being the
> origin of this), but I am afraid you have to rework this when you rebase
> it against origin/staging, since the ITS emulation has been merged.
> So while actual conflicts seem to be trivial, there are now many new
> users of vgic_reg??_* which you have to change as well.
> Should be rather mechanical, though.
>
I will rebase it on origin/staging and merge the changes. How do I
enable ITS code compilation to verify that it is compiling fine with
new
vreg_reg* functions? Is ITS code not compiled in by default?

Regards,
Bhupinder
Andre Przywara June 19, 2017, 4:59 p.m. UTC | #4
Hi,

On 19/06/17 17:53, Bhupinder Thakur wrote:
> Hi Andre,
> 
> On 19 June 2017 at 15:03, Andre Przywara <andre.przywara@arm.com> wrote:
>> Hi Bhupinder,
>>
>> I think the commit message is a bit misleading.
>> Actually you *rename* functions and their call sites, and also this
>> touches the VGIC code, so shouldn't it mention both in the first line of
>> the commit message? After all this patch really has not much to do with
>> vpl011.
>>
> I will modify the commit message to indicate this commit renames
> vgic_reg* to vreg_reg*
> and modifies all the places where this call is made.

Thanks!

>> On 06/06/17 18:25, Bhupinder Thakur wrote:
>>> This patch redefines the vgic_reg* access functions to vreg_reg* functions.
>>> These are generic functions, which will be used by the vgic emulation code
>>> to access the vgic registers.
>>>
>>> PL011 emulation code will also use vreg_reg* access functions.
>>
>> Also I am sorry to be the bearer of bad news (and also for being the
>> origin of this), but I am afraid you have to rework this when you rebase
>> it against origin/staging, since the ITS emulation has been merged.
>> So while actual conflicts seem to be trivial, there are now many new
>> users of vgic_reg??_* which you have to change as well.
>> Should be rather mechanical, though.
>>
> I will rebase it on origin/staging and merge the changes.

Thanks - and sorry for the mess ;-)

> How do I
> enable ITS code compilation to verify that it is compiling fine with
> new vreg_reg* functions? Is ITS code not compiled in by default?

You need to add "XEN_CONFIG_EXPERT=y" to every make invocation, so both
for any configuration and for the actual build.
If it doesn't prompt you for the ITS, please type:
$ make -C xen menuconfig XEN_CONFIG_EXPERT=y
then select "GICv3 ITS MSI controller support" under "Architecture
Features" (should only be needed once).
You should check xen/.config for having a line with "CONFIG_HAS_ITS=y".

Cheers,
Andre.
diff mbox series

Patch

diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index dc9f95b..3e35a90 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -179,7 +179,7 @@  static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
     case VREG32(GICD_CTLR):
         if ( dabt.size != DABT_WORD ) goto bad_width;
         vgic_lock(v);
-        *r = vgic_reg32_extract(v->domain->arch.vgic.ctlr, info);
+        *r = vreg_reg32_extract(v->domain->arch.vgic.ctlr, info);
         vgic_unlock(v);
         return 1;
 
@@ -194,7 +194,7 @@  static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
             | DIV_ROUND_UP(v->domain->arch.vgic.nr_spis, 32);
         vgic_unlock(v);
 
-        *r = vgic_reg32_extract(typer, info);
+        *r = vreg_reg32_extract(typer, info);
 
         return 1;
     }
@@ -205,7 +205,7 @@  static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
          * XXX Do we need a JEP106 manufacturer ID?
          * Just use the physical h/w value for now
          */
-        *r = vgic_reg32_extract(0x0000043b, info);
+        *r = vreg_reg32_extract(0x0000043b, info);
         return 1;
 
     case VRANGE32(0x00C, 0x01C):
@@ -226,7 +226,7 @@  static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
         rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISENABLER, DABT_WORD);
         if ( rank == NULL) goto read_as_zero;
         vgic_lock_rank(v, rank, flags);
-        *r = vgic_reg32_extract(rank->ienable, info);
+        *r = vreg_reg32_extract(rank->ienable, info);
         vgic_unlock_rank(v, rank, flags);
         return 1;
 
@@ -235,7 +235,7 @@  static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
         rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICENABLER, DABT_WORD);
         if ( rank == NULL) goto read_as_zero;
         vgic_lock_rank(v, rank, flags);
-        *r = vgic_reg32_extract(rank->ienable, info);
+        *r = vreg_reg32_extract(rank->ienable, info);
         vgic_unlock_rank(v, rank, flags);
         return 1;
 
@@ -262,7 +262,7 @@  static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
                                                      gicd_reg - GICD_IPRIORITYR,
                                                      DABT_WORD)];
         vgic_unlock_rank(v, rank, flags);
-        *r = vgic_reg32_extract(ipriorityr, info);
+        *r = vreg_reg32_extract(ipriorityr, info);
 
         return 1;
     }
@@ -280,7 +280,7 @@  static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
         vgic_lock_rank(v, rank, flags);
         itargetsr = vgic_fetch_itargetsr(rank, gicd_reg - GICD_ITARGETSR);
         vgic_unlock_rank(v, rank, flags);
-        *r = vgic_reg32_extract(itargetsr, info);
+        *r = vreg_reg32_extract(itargetsr, info);
 
         return 1;
     }
@@ -299,7 +299,7 @@  static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
         icfgr = rank->icfg[REG_RANK_INDEX(2, gicd_reg - GICD_ICFGR, DABT_WORD)];
         vgic_unlock_rank(v, rank, flags);
 
-        *r = vgic_reg32_extract(icfgr, info);
+        *r = vreg_reg32_extract(icfgr, info);
 
         return 1;
     }
@@ -424,7 +424,7 @@  static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
         if ( dabt.size != DABT_WORD ) goto bad_width;
         /* Ignore all but the enable bit */
         vgic_lock(v);
-        vgic_reg32_update(&v->domain->arch.vgic.ctlr, r, info);
+        vreg_reg32_update(&v->domain->arch.vgic.ctlr, r, info);
         v->domain->arch.vgic.ctlr &= GICD_CTL_ENABLE;
         vgic_unlock(v);
 
@@ -454,7 +454,7 @@  static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
         if ( rank == NULL) goto write_ignore;
         vgic_lock_rank(v, rank, flags);
         tr = rank->ienable;
-        vgic_reg32_setbits(&rank->ienable, r, info);
+        vreg_reg32_setbits(&rank->ienable, r, info);
         vgic_enable_irqs(v, (rank->ienable) & (~tr), rank->index);
         vgic_unlock_rank(v, rank, flags);
         return 1;
@@ -465,7 +465,7 @@  static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
         if ( rank == NULL) goto write_ignore;
         vgic_lock_rank(v, rank, flags);
         tr = rank->ienable;
-        vgic_reg32_clearbits(&rank->ienable, r, info);
+        vreg_reg32_clearbits(&rank->ienable, r, info);
         vgic_disable_irqs(v, (~rank->ienable) & tr, rank->index);
         vgic_unlock_rank(v, rank, flags);
         return 1;
@@ -508,7 +508,7 @@  static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
         ipriorityr = &rank->ipriorityr[REG_RANK_INDEX(8,
                                                       gicd_reg - GICD_IPRIORITYR,
                                                       DABT_WORD)];
-        vgic_reg32_update(ipriorityr, r, info);
+        vreg_reg32_update(ipriorityr, r, info);
         vgic_unlock_rank(v, rank, flags);
         return 1;
     }
@@ -529,7 +529,7 @@  static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
         if ( rank == NULL) goto write_ignore;
         vgic_lock_rank(v, rank, flags);
         itargetsr = vgic_fetch_itargetsr(rank, gicd_reg - GICD_ITARGETSR);
-        vgic_reg32_update(&itargetsr, r, info);
+        vreg_reg32_update(&itargetsr, r, info);
         vgic_store_itargetsr(v->domain, rank, gicd_reg - GICD_ITARGETSR,
                              itargetsr);
         vgic_unlock_rank(v, rank, flags);
@@ -551,7 +551,7 @@  static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
         rank = vgic_rank_offset(v, 2, gicd_reg - GICD_ICFGR, DABT_WORD);
         if ( rank == NULL) goto write_ignore;
         vgic_lock_rank(v, rank, flags);
-        vgic_reg32_update(&rank->icfg[REG_RANK_INDEX(2, gicd_reg - GICD_ICFGR,
+        vreg_reg32_update(&rank->icfg[REG_RANK_INDEX(2, gicd_reg - GICD_ICFGR,
                                                      DABT_WORD)],
                           r, info);
         vgic_unlock_rank(v, rank, flags);
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index d10757a..e1213d9 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -181,7 +181,7 @@  static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info,
 
     case VREG32(GICR_IIDR):
         if ( dabt.size != DABT_WORD ) goto bad_width;
-        *r = vgic_reg32_extract(GICV3_GICR_IIDR_VAL, info);
+        *r = vreg_reg32_extract(GICV3_GICR_IIDR_VAL, info);
         return 1;
 
     case VREG64(GICR_TYPER):
@@ -199,7 +199,7 @@  static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info,
         if ( v->arch.vgic.flags & VGIC_V3_RDIST_LAST )
             typer |= GICR_TYPER_LAST;
 
-        *r = vgic_reg64_extract(typer, info);
+        *r = vreg_reg64_extract(typer, info);
 
         return 1;
     }
@@ -257,7 +257,7 @@  static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info,
     case VREG32(GICR_SYNCR):
         if ( dabt.size != DABT_WORD ) goto bad_width;
         /* RO . But when read it always returns busy bito bit[0] */
-        *r = vgic_reg32_extract(GICR_SYNCR_NOT_BUSY, info);
+        *r = vreg_reg32_extract(GICR_SYNCR_NOT_BUSY, info);
         return 1;
 
     case 0x00C8:
@@ -284,7 +284,7 @@  static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info,
 
     case VREG32(GICR_PIDR2):
         if ( dabt.size != DABT_WORD ) goto bad_width;
-        *r = vgic_reg32_extract(GICV3_GICR_PIDR2, info);
+        *r = vreg_reg32_extract(GICV3_GICR_PIDR2, info);
          return 1;
 
     case 0xFFEC ... 0xFFFC:
@@ -328,7 +328,7 @@  read_reserved:
     return 1;
 
 read_unknown:
-    *r = vgic_reg64_extract(0xdeadbeafdeadbeaf, info);
+    *r = vreg_reg64_extract(0xdeadbeafdeadbeaf, info);
     return 1;
 }
 
@@ -489,7 +489,7 @@  static int __vgic_v3_distr_common_mmio_read(const char *name, struct vcpu *v,
         rank = vgic_rank_offset(v, 1, reg - GICD_ISENABLER, DABT_WORD);
         if ( rank == NULL ) goto read_as_zero;
         vgic_lock_rank(v, rank, flags);
-        *r = vgic_reg32_extract(rank->ienable, info);
+        *r = vreg_reg32_extract(rank->ienable, info);
         vgic_unlock_rank(v, rank, flags);
         return 1;
 
@@ -498,7 +498,7 @@  static int __vgic_v3_distr_common_mmio_read(const char *name, struct vcpu *v,
         rank = vgic_rank_offset(v, 1, reg - GICD_ICENABLER, DABT_WORD);
         if ( rank == NULL ) goto read_as_zero;
         vgic_lock_rank(v, rank, flags);
-        *r = vgic_reg32_extract(rank->ienable, info);
+        *r = vreg_reg32_extract(rank->ienable, info);
         vgic_unlock_rank(v, rank, flags);
         return 1;
 
@@ -525,7 +525,7 @@  static int __vgic_v3_distr_common_mmio_read(const char *name, struct vcpu *v,
                                                      DABT_WORD)];
         vgic_unlock_rank(v, rank, flags);
 
-        *r = vgic_reg32_extract(ipriorityr, info);
+        *r = vreg_reg32_extract(ipriorityr, info);
 
         return 1;
     }
@@ -541,7 +541,7 @@  static int __vgic_v3_distr_common_mmio_read(const char *name, struct vcpu *v,
         icfgr = rank->icfg[REG_RANK_INDEX(2, reg - GICD_ICFGR, DABT_WORD)];
         vgic_unlock_rank(v, rank, flags);
 
-        *r = vgic_reg32_extract(icfgr, info);
+        *r = vreg_reg32_extract(icfgr, info);
 
         return 1;
     }
@@ -585,7 +585,7 @@  static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
         if ( rank == NULL ) goto write_ignore;
         vgic_lock_rank(v, rank, flags);
         tr = rank->ienable;
-        vgic_reg32_setbits(&rank->ienable, r, info);
+        vreg_reg32_setbits(&rank->ienable, r, info);
         vgic_enable_irqs(v, (rank->ienable) & (~tr), rank->index);
         vgic_unlock_rank(v, rank, flags);
         return 1;
@@ -596,7 +596,7 @@  static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
         if ( rank == NULL ) goto write_ignore;
         vgic_lock_rank(v, rank, flags);
         tr = rank->ienable;
-        vgic_reg32_clearbits(&rank->ienable, r, info);
+        vreg_reg32_clearbits(&rank->ienable, r, info);
         vgic_disable_irqs(v, (~rank->ienable) & tr, rank->index);
         vgic_unlock_rank(v, rank, flags);
         return 1;
@@ -638,7 +638,7 @@  static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
         vgic_lock_rank(v, rank, flags);
         ipriorityr = &rank->ipriorityr[REG_RANK_INDEX(8, reg - GICD_IPRIORITYR,
                                                       DABT_WORD)];
-        vgic_reg32_update(ipriorityr, r, info);
+        vreg_reg32_update(ipriorityr, r, info);
         vgic_unlock_rank(v, rank, flags);
         return 1;
     }
@@ -653,7 +653,7 @@  static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
         rank = vgic_rank_offset(v, 2, reg - GICD_ICFGR, DABT_WORD);
         if ( rank == NULL ) goto write_ignore;
         vgic_lock_rank(v, rank, flags);
-        vgic_reg32_update(&rank->icfg[REG_RANK_INDEX(2, reg - GICD_ICFGR,
+        vreg_reg32_update(&rank->icfg[REG_RANK_INDEX(2, reg - GICD_ICFGR,
                                                      DABT_WORD)],
                           r, info);
         vgic_unlock_rank(v, rank, flags);
@@ -901,7 +901,7 @@  static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
     case VREG32(GICD_CTLR):
         if ( dabt.size != DABT_WORD ) goto bad_width;
         vgic_lock(v);
-        *r = vgic_reg32_extract(v->domain->arch.vgic.ctlr, info);
+        *r = vreg_reg32_extract(v->domain->arch.vgic.ctlr, info);
         vgic_unlock(v);
         return 1;
 
@@ -926,14 +926,14 @@  static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
 
         typer |= (irq_bits - 1) << GICD_TYPE_ID_BITS_SHIFT;
 
-        *r = vgic_reg32_extract(typer, info);
+        *r = vreg_reg32_extract(typer, info);
 
         return 1;
     }
 
     case VREG32(GICD_IIDR):
         if ( dabt.size != DABT_WORD ) goto bad_width;
-        *r = vgic_reg32_extract(GICV3_GICD_IIDR_VAL, info);
+        *r = vreg_reg32_extract(GICV3_GICD_IIDR_VAL, info);
         return 1;
 
     case VREG32(0x000C):
@@ -1026,7 +1026,7 @@  static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
         irouter = vgic_fetch_irouter(rank, gicd_reg - GICD_IROUTER);
         vgic_unlock_rank(v, rank, flags);
 
-        *r = vgic_reg64_extract(irouter, info);
+        *r = vreg_reg64_extract(irouter, info);
 
         return 1;
     }
@@ -1044,7 +1044,7 @@  static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
     case VREG32(GICD_PIDR2):
         /* GICv3 identification value */
         if ( dabt.size != DABT_WORD ) goto bad_width;
-        *r = vgic_reg32_extract(GICV3_GICD_PIDR2, info);
+        *r = vreg_reg32_extract(GICV3_GICD_PIDR2, info);
         return 1;
 
     case VRANGE32(0xFFEC, 0xFFFC):
@@ -1107,7 +1107,7 @@  static int vgic_v3_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
 
         vgic_lock(v);
 
-        vgic_reg32_update(&ctlr, r, info);
+        vreg_reg32_update(&ctlr, r, info);
 
         /* Only EnableGrp1A can be changed */
         if ( ctlr & GICD_CTLR_ENABLE_G1A )
@@ -1213,7 +1213,7 @@  static int vgic_v3_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
         if ( rank == NULL ) goto write_ignore;
         vgic_lock_rank(v, rank, flags);
         irouter = vgic_fetch_irouter(rank, gicd_reg - GICD_IROUTER);
-        vgic_reg64_update(&irouter, r, info);
+        vreg_reg64_update(&irouter, r, info);
         vgic_store_irouter(v->domain, rank, gicd_reg - GICD_IROUTER, irouter);
         vgic_unlock_rank(v, rank, flags);
         return 1;
diff --git a/xen/include/asm-arm/vreg.h b/xen/include/asm-arm/vreg.h
index 348584f..16207ce 100644
--- a/xen/include/asm-arm/vreg.h
+++ b/xen/include/asm-arm/vreg.h
@@ -107,102 +107,102 @@  static inline bool vreg_emulate_sysreg64(struct cpu_user_regs *regs, union hsr h
 
 #endif
 
-#define VGIC_REG_MASK(size) ((~0UL) >> (BITS_PER_LONG - ((1 << (size)) * 8)))
+#define VREG_REG_MASK(size) ((~0UL) >> (BITS_PER_LONG - ((1 << (size)) * 8)))
 
 /*
  * The check on the size supported by the register has to be done by
- * the caller of vgic_regN_*.
+ * the caller of vreg_regN_*.
  *
- * vgic_reg_* should never be called directly. Instead use the vgic_regN_*
+ * vreg_reg_* should never be called directly. Instead use the vreg_regN_*
  * according to size of the emulated register
  *
  * Note that the alignment fault will always be taken in the guest
  * (see B3.12.7 DDI0406.b).
  */
-static inline register_t vgic_reg_extract(unsigned long reg,
+static inline register_t vreg_reg_extract(unsigned long reg,
                                           unsigned int offset,
                                           enum dabt_size size)
 {
     reg >>= 8 * offset;
-    reg &= VGIC_REG_MASK(size);
+    reg &= VREG_REG_MASK(size);
 
     return reg;
 }
 
-static inline void vgic_reg_update(unsigned long *reg, register_t val,
+static inline void vreg_reg_update(unsigned long *reg, register_t val,
                                    unsigned int offset,
                                    enum dabt_size size)
 {
-    unsigned long mask = VGIC_REG_MASK(size);
+    unsigned long mask = VREG_REG_MASK(size);
     int shift = offset * 8;
 
     *reg &= ~(mask << shift);
     *reg |= ((unsigned long)val & mask) << shift;
 }
 
-static inline void vgic_reg_setbits(unsigned long *reg, register_t bits,
+static inline void vreg_reg_setbits(unsigned long *reg, register_t bits,
                                     unsigned int offset,
                                     enum dabt_size size)
 {
-    unsigned long mask = VGIC_REG_MASK(size);
+    unsigned long mask = VREG_REG_MASK(size);
     int shift = offset * 8;
 
     *reg |= ((unsigned long)bits & mask) << shift;
 }
 
-static inline void vgic_reg_clearbits(unsigned long *reg, register_t bits,
+static inline void vreg_reg_clearbits(unsigned long *reg, register_t bits,
                                       unsigned int offset,
                                       enum dabt_size size)
 {
-    unsigned long mask = VGIC_REG_MASK(size);
+    unsigned long mask = VREG_REG_MASK(size);
     int shift = offset * 8;
 
     *reg &= ~(((unsigned long)bits & mask) << shift);
 }
 
-/* N-bit register helpers */
-#define VGIC_REG_HELPERS(sz, offmask)                                   \
-static inline register_t vgic_reg##sz##_extract(uint##sz##_t reg,       \
-                                                const mmio_info_t *info)\
-{                                                                       \
-    return vgic_reg_extract(reg, info->gpa & offmask,                   \
-                            info->dabt.size);                           \
-}                                                                       \
-                                                                        \
-static inline void vgic_reg##sz##_update(uint##sz##_t *reg,             \
-                                         register_t val,                \
-                                         const mmio_info_t *info)       \
-{                                                                       \
-    unsigned long tmp = *reg;                                           \
-                                                                        \
-    vgic_reg_update(&tmp, val, info->gpa & offmask,                     \
-                    info->dabt.size);                                   \
-                                                                        \
-    *reg = tmp;                                                         \
-}                                                                       \
-                                                                        \
-static inline void vgic_reg##sz##_setbits(uint##sz##_t *reg,            \
-                                          register_t bits,              \
-                                          const mmio_info_t *info)      \
-{                                                                       \
-    unsigned long tmp = *reg;                                           \
-                                                                        \
-    vgic_reg_setbits(&tmp, bits, info->gpa & offmask,                   \
-                     info->dabt.size);                                  \
-                                                                        \
-    *reg = tmp;                                                         \
-}                                                                       \
-                                                                        \
-static inline void vgic_reg##sz##_clearbits(uint##sz##_t *reg,          \
-                                            register_t bits,            \
-                                            const mmio_info_t *info)    \
-{                                                                       \
-    unsigned long tmp = *reg;                                           \
-                                                                        \
-    vgic_reg_clearbits(&tmp, bits, info->gpa & offmask,                 \
-                       info->dabt.size);                                \
-                                                                        \
-    *reg = tmp;                                                         \
+#define VREG_REG_HELPERS(sz, offmask)                                           \
+/* N-bit register helpers */                                                    \
+static inline register_t vreg_reg##sz##_extract(uint##sz##_t reg,               \
+                                                  const mmio_info_t *info)      \
+{                                                                               \
+    return vreg_reg_extract(reg, info->gpa & offmask,                           \
+                            info->dabt.size);                                   \
+}                                                                               \
+                                                                                \
+static inline void vreg_reg##sz##_update(uint##sz##_t *reg,                     \
+                                           register_t val,                      \
+                                           const mmio_info_t *info)             \
+{                                                                               \
+    unsigned long tmp = *reg;                                                   \
+                                                                                \
+    vreg_reg_update(&tmp, val, info->gpa & offmask,                             \
+                    info->dabt.size);                                           \
+                                                                                \
+    *reg = tmp;                                                                 \
+}                                                                               \
+                                                                                \
+static inline void vreg_reg##sz##_setbits(uint##sz##_t *reg,                    \
+                                            register_t bits,                    \
+                                            const mmio_info_t *info)            \
+{                                                                               \
+    unsigned long tmp = *reg;                                                   \
+                                                                                \
+    vreg_reg_setbits(&tmp, bits, info->gpa & offmask,                           \
+                     info->dabt.size);                                          \
+                                                                                \
+    *reg = tmp;                                                                 \
+}                                                                               \
+                                                                                \
+static inline void vreg_reg##sz##_clearbits(uint##sz##_t *reg,                  \
+                                              register_t bits,                  \
+                                              const mmio_info_t *info)          \
+{                                                                               \
+    unsigned long tmp = *reg;                                                   \
+                                                                                \
+    vreg_reg_clearbits(&tmp, bits, info->gpa & offmask,                         \
+                       info->dabt.size);                                        \
+                                                                                \
+    *reg = tmp;                                                                 \
 }
 
 /*
@@ -211,10 +211,9 @@  static inline void vgic_reg##sz##_clearbits(uint##sz##_t *reg,          \
  * unsigned long rather than uint64_t
  */
 #if BITS_PER_LONG == 64
-VGIC_REG_HELPERS(64, 0x7);
+VREG_REG_HELPERS(64, 0x7);
 #endif
-VGIC_REG_HELPERS(32, 0x3);
-
-#undef VGIC_REG_HELPERS
+VREG_REG_HELPERS(32, 0x3);
+#undef VREG_REG_HELPERS
 
 #endif /* __ASM_ARM_VREG__ */