Re: [PATCH v5 10/12] xen/arm: vgic-v3: add emulation of GICv3.1 eSPI registers

[Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>]

Hi Leonid,

Leonid Komarianskyi <Leonid_Komarianskyi@epam.com> writes:

> Implemented support for GICv3.1 extended SPI registers for vGICv3,
> allowing the emulation of eSPI-specific behavior for guest domains.
> The implementation includes read and write emulation for eSPI-related
> registers (e.g., GICD_ISENABLERnE, GICD_IROUTERnE, and others),
> following a similar approach to the handling of regular SPIs.
>
> The eSPI registers, previously located in reserved address ranges,
> are now adjusted to support MMIO read and write operations correctly
> when CONFIG_GICV3_ESPI is enabled.
>
> The availability of eSPIs and the number of emulated extended SPIs
> for guest domains is reported by setting the appropriate bits in the
> GICD_TYPER register, based on the number of eSPIs requested by the
> domain and supported by the hardware. In cases where the configuration
> option is disabled, the hardware does not support eSPIs, or the domain
> does not request such interrupts, the functionality remains unchanged.
>
> Signed-off-by: Leonid Komarianskyi <leonid_komarianskyi@epam.com>

I have a couple of comments about coding style, but apart from that it
looks really good. With these issues fixed:

Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

>
> ---
> Changes in V5:
> - since eSPI-specific defines and macros are now available even when the
>   appropriate config is disabled, this allows us to remove many
>   #ifdef guards and reuse the existing code for regular SPIs for eSPIs as
>   well, as eSPIs are processed similarly. This improves code readability
>   and consolidates the register ranges for SPIs and eSPIs in a single
>   place, simplifying future changes or fixes for SPIs and their
>   counterparts from the extended range
> - moved vgic_ext_rank_offset() from
>   [08/12] xen/arm: vgic: add resource management for extended SPIs
>   as the function was unused before this patch
> - added stub implementation of vgic_ext_rank_offset() when CONFIG_GICV3_ESPI=n
> - removed unnecessary defines for reserved ranges, which were introduced in
>   V4 to reduce the number of #ifdefs. The issue is now resolved by
>   allowing the use of SPI-specific offsets and reworking the logic
>
> Changes in V4:
> - added missing RAZ and write ignore eSPI-specific registers ranges:
>   GICD_NSACRnE and GICD_IGRPMODRnE
> - changed previously reserved range to cover GICD_NSACRnE and
>   GICD_IGRPMODRnE
> - introduced GICD_RESERVED_RANGE<n>_START/END defines to remove
>   hardcoded values and reduce the number of ifdefs
> - fixed reserved ranges with eSPI option enabled: added missing range
>   0x0F30-0x0F7C
> - updated the logic for domains that do not support eSPI, but Xen is
>   compiled with the eSPI option. Now, prior to other MMIO checks, we
>   verify whether eSPI is available for the domain or not. If not, it
>   behaves as it does currently - RAZ and WI
> - fixed print for GICD_ICACTIVERnE
> - fixed new lines formatting for switch-case
>
> Changes in V3:
> - changed vgic_store_irouter parameters - instead of offset virq is
>   used, to remove the additional bool espi parameter and simplify
>   checks. Also, adjusted parameters for regular SPI. Since the offset
>   parameter was used only for calculating virq number and then reused for
>   finding rank offset, it will not affect functionality.
> - fixed formatting for goto lables - added newlines after condition
> - fixed logs for GICD_ISACTIVERnE and GICD_ICACTIVERnE handlers
> - removed #ifdefs in 2 places where they were adjacent and could be merged
>
> Changes in V2:
> - add missing rank index conversion for pending and inflight irqs
> ---
>  xen/arch/arm/include/asm/vgic.h |   4 +
>  xen/arch/arm/vgic-v3.c          | 198 ++++++++++++++++++++++++++------
>  xen/arch/arm/vgic.c             |  23 ++++
>  3 files changed, 192 insertions(+), 33 deletions(-)
>
> diff --git a/xen/arch/arm/include/asm/vgic.h b/xen/arch/arm/include/asm/vgic.h
> index 3aa22114ba..103bc3c74b 100644
> --- a/xen/arch/arm/include/asm/vgic.h
> +++ b/xen/arch/arm/include/asm/vgic.h
> @@ -314,6 +314,10 @@ extern struct vgic_irq_rank *vgic_rank_offset(struct vcpu *v,
>                                                unsigned int b,
>                                                unsigned int n,
>                                                unsigned int s);
> +extern struct vgic_irq_rank *vgic_ext_rank_offset(struct vcpu *v,
> +                                                  unsigned int b,
> +                                                  unsigned int n,
> +                                                  unsigned int s);
>  extern struct vgic_irq_rank *vgic_rank_irq(struct vcpu *v, unsigned int irq);
>  extern void vgic_disable_irqs(struct vcpu *v, uint32_t r, unsigned int n);
>  extern void vgic_enable_irqs(struct vcpu *v, uint32_t r, unsigned int n);
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index 4369c55177..b5d766c98f 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -111,13 +111,10 @@ static uint64_t vgic_fetch_irouter(struct vgic_irq_rank *rank,
>   * Note the offset will be aligned to the appropriate boundary.
>   */
>  static void vgic_store_irouter(struct domain *d, struct vgic_irq_rank *rank,
> -                               unsigned int offset, uint64_t irouter)
> +                               unsigned int virq, uint64_t irouter)
>  {
>      struct vcpu *new_vcpu, *old_vcpu;
> -    unsigned int virq;
> -
> -    /* There is 1 vIRQ per IROUTER */
> -    virq = offset / NR_BYTES_PER_IROUTER;
> +    unsigned int offset;
>  
>      /*
>       * The IROUTER0-31, used for SGIs/PPIs, are reserved and should
> @@ -685,13 +682,20 @@ static int __vgic_v3_distr_common_mmio_read(const char *name, struct vcpu *v,
>      {
>      case VRANGE32(GICD_IGROUPR, GICD_IGROUPRN):
>      case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
> +    case VRANGE32(GICD_IGROUPRnE, GICD_IGROUPRnEN):
> +    case VRANGE32(GICD_IGRPMODRnE, GICD_IGRPMODRnEN):
>          /* We do not implement security extensions for guests, read zero */
>          if ( dabt.size != DABT_WORD ) goto bad_width;
>          goto read_as_zero;
>  
>      case VRANGE32(GICD_ISENABLER, GICD_ISENABLERN):
> +    case VRANGE32(GICD_ISENABLERnE, GICD_ISENABLERnEN):
>          if ( dabt.size != DABT_WORD ) goto bad_width;
> -        rank = vgic_rank_offset(v, 1, reg - GICD_ISENABLER, DABT_WORD);
> +        if ( reg >= GICD_ISENABLERnE )
> +            rank = vgic_ext_rank_offset(v, 1, reg - GICD_ISENABLERnE,
> +                                        DABT_WORD);
> +        else
> +            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 = vreg_reg32_extract(rank->ienable, info);
> @@ -699,8 +703,13 @@ static int __vgic_v3_distr_common_mmio_read(const char *name, struct vcpu *v,
>          return 1;
>  
>      case VRANGE32(GICD_ICENABLER, GICD_ICENABLERN):
> +    case VRANGE32(GICD_ICENABLERnE, GICD_ICENABLERnEN):
>          if ( dabt.size != DABT_WORD ) goto bad_width;
> -        rank = vgic_rank_offset(v, 1, reg - GICD_ICENABLER, DABT_WORD);
> +        if ( reg >= GICD_ICENABLERnE )
> +            rank = vgic_ext_rank_offset(v, 1, reg - GICD_ICENABLERnE,
> +                                        DABT_WORD);
> +        else
> +            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 = vreg_reg32_extract(rank->ienable, info);
> @@ -710,20 +719,29 @@ static int __vgic_v3_distr_common_mmio_read(const char *name, struct vcpu *v,
>      /* Read the pending status of an IRQ via GICD/GICR is not supported */
>      case VRANGE32(GICD_ISPENDR, GICD_ISPENDRN):
>      case VRANGE32(GICD_ICPENDR, GICD_ICPENDRN):
> +    case VRANGE32(GICD_ISPENDRnE, GICD_ISPENDRnEN):
> +    case VRANGE32(GICD_ICPENDRnE, GICD_ICPENDRnEN):
>          goto read_as_zero;
>  
>      /* Read the active status of an IRQ via GICD/GICR is not supported */
>      case VRANGE32(GICD_ISACTIVER, GICD_ISACTIVERN):
>      case VRANGE32(GICD_ICACTIVER, GICD_ICACTIVERN):
> +    case VRANGE32(GICD_ISACTIVERnE, GICD_ISACTIVERnEN):
> +    case VRANGE32(GICD_ICACTIVERnE, GICD_ICACTIVERnEN):
>          goto read_as_zero;
>  
>      case VRANGE32(GICD_IPRIORITYR, GICD_IPRIORITYRN):
> +    case VRANGE32(GICD_IPRIORITYRnE, GICD_IPRIORITYRnEN):
>      {
>          uint32_t ipriorityr;
>          uint8_t rank_index;
>  
>          if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
> -        rank = vgic_rank_offset(v, 8, reg - GICD_IPRIORITYR, DABT_WORD);
> +        if ( reg >= GICD_IPRIORITYRnE )
> +            rank = vgic_ext_rank_offset(v, 8, reg - GICD_IPRIORITYRnE,
> +                                        DABT_WORD);
> +        else
> +            rank = vgic_rank_offset(v, 8, reg - GICD_IPRIORITYR, DABT_WORD);
>          if ( rank == NULL ) goto read_as_zero;
>          rank_index = REG_RANK_INDEX(8, reg - GICD_IPRIORITYR, DABT_WORD);
>  
> @@ -737,11 +755,15 @@ static int __vgic_v3_distr_common_mmio_read(const char *name, struct vcpu *v,
>      }
>  
>      case VRANGE32(GICD_ICFGR, GICD_ICFGRN):
> +    case VRANGE32(GICD_ICFGRnE, GICD_ICFGRnEN):
>      {
>          uint32_t icfgr;
>  
>          if ( dabt.size != DABT_WORD ) goto bad_width;
> -        rank = vgic_rank_offset(v, 2, reg - GICD_ICFGR, DABT_WORD);
> +        if ( reg >= GICD_ICFGRnE )
> +            rank = vgic_ext_rank_offset(v, 2, reg - GICD_ICFGRnE, DABT_WORD);
> +        else
> +            rank = vgic_rank_offset(v, 2, reg - GICD_ICFGR, DABT_WORD);
>          if ( rank == NULL ) goto read_as_zero;
>          vgic_lock_rank(v, rank, flags);
>          icfgr = rank->icfg[REG_RANK_INDEX(2, reg - GICD_ICFGR, DABT_WORD)];
> @@ -782,46 +804,81 @@ static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
>      {
>      case VRANGE32(GICD_IGROUPR, GICD_IGROUPRN):
>      case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
> +    case VRANGE32(GICD_IGROUPRnE, GICD_IGROUPRnEN):
> +    case VRANGE32(GICD_IGRPMODRnE, GICD_IGRPMODRnEN):
>          /* We do not implement security extensions for guests, write ignore */
>          goto write_ignore_32;
>  
>      case VRANGE32(GICD_ISENABLER, GICD_ISENABLERN):
> +    case VRANGE32(GICD_ISENABLERnE, GICD_ISENABLERnEN):
>          if ( dabt.size != DABT_WORD ) goto bad_width;
> -        rank = vgic_rank_offset(v, 1, reg - GICD_ISENABLER, DABT_WORD);
> +        if ( reg >= GICD_ISENABLERnE )
> +            rank = vgic_ext_rank_offset(v, 1, reg - GICD_ISENABLERnE,
> +                                        DABT_WORD);
> +        else
> +            rank = vgic_rank_offset(v, 1, reg - GICD_ISENABLER, DABT_WORD);
>          if ( rank == NULL ) goto write_ignore;
>          vgic_lock_rank(v, rank, flags);
>          tr = rank->ienable;
>          vreg_reg32_setbits(&rank->ienable, r, info);
> -        vgic_enable_irqs(v, (rank->ienable) & (~tr), rank->index);
> +        if ( reg >= GICD_ISENABLERnE )
> +            vgic_enable_irqs(v, (rank->ienable) & (~tr),
> +                             EXT_RANK_IDX2NUM(rank->index));
> +        else
> +            vgic_enable_irqs(v, (rank->ienable) & (~tr), rank->index);
>          vgic_unlock_rank(v, rank, flags);
>          return 1;
>  
>      case VRANGE32(GICD_ICENABLER, GICD_ICENABLERN):
> +    case VRANGE32(GICD_ICENABLERnE, GICD_ICENABLERnEN):
>          if ( dabt.size != DABT_WORD ) goto bad_width;
> -        rank = vgic_rank_offset(v, 1, reg - GICD_ICENABLER, DABT_WORD);
> +        if ( reg >= GICD_ICENABLERnE )
> +            rank = vgic_ext_rank_offset(v, 1, reg - GICD_ICENABLERnE,
> +                                        DABT_WORD);
> +        else
> +            rank = vgic_rank_offset(v, 1, reg - GICD_ICENABLER, DABT_WORD);
>          if ( rank == NULL ) goto write_ignore;
>          vgic_lock_rank(v, rank, flags);
>          tr = rank->ienable;
>          vreg_reg32_clearbits(&rank->ienable, r, info);
> -        vgic_disable_irqs(v, (~rank->ienable) & tr, rank->index);
> +        if ( reg >= GICD_ICENABLERnE )
> +            vgic_disable_irqs(v, (~rank->ienable) & tr,
> +                              EXT_RANK_IDX2NUM(rank->index));
> +        else
> +            vgic_disable_irqs(v, (~rank->ienable) & tr, rank->index);
>          vgic_unlock_rank(v, rank, flags);
>          return 1;
>  
>      case VRANGE32(GICD_ISPENDR, GICD_ISPENDRN):
> +    case VRANGE32(GICD_ISPENDRnE, GICD_ISPENDRnEN):
>          if ( dabt.size != DABT_WORD ) goto bad_width;
> -        rank = vgic_rank_offset(v, 1, reg - GICD_ISPENDR, DABT_WORD);
> +        if ( reg >= GICD_ISPENDRnE )
> +            rank = vgic_ext_rank_offset(v, 1, reg - GICD_ISPENDRnE, DABT_WORD);
> +        else
> +            rank = vgic_rank_offset(v, 1, reg - GICD_ISPENDR, DABT_WORD);
>          if ( rank == NULL ) goto write_ignore;
>  
> -        vgic_set_irqs_pending(v, r, rank->index);
> +        if ( reg >= GICD_ISPENDRnE )
> +            vgic_set_irqs_pending(v, r, EXT_RANK_IDX2NUM(rank->index));
> +        else
> +            vgic_set_irqs_pending(v, r, rank->index);
>  
>          return 1;
>  
>      case VRANGE32(GICD_ICPENDR, GICD_ICPENDRN):
> +    case VRANGE32(GICD_ICPENDRnE, GICD_ICPENDRnEN):
>          if ( dabt.size != DABT_WORD ) goto bad_width;
> -        rank = vgic_rank_offset(v, 1, reg - GICD_ICPENDR, DABT_WORD);
> +        if ( reg >= GICD_ICPENDRnE )
> +            rank = vgic_ext_rank_offset(v, 1, reg - GICD_ICPENDRnE, DABT_WORD);
> +        else
> +            rank = vgic_rank_offset(v, 1, reg - GICD_ICPENDR, DABT_WORD);
>          if ( rank == NULL ) goto write_ignore;
>  
> -        vgic_check_inflight_irqs_pending(v, rank->index, r);
> +        if ( reg >= GICD_ICPENDRnE )
> +            vgic_check_inflight_irqs_pending(v,
> +                                             EXT_RANK_IDX2NUM(rank->index), r);
> +        else
> +            vgic_check_inflight_irqs_pending(v, rank->index, r);
>  
>          goto write_ignore;
>  
> @@ -838,16 +895,38 @@ static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
>                 v, name, r, reg - GICD_ICACTIVER);
>          goto write_ignore_32;
>  
> +    case VRANGE32(GICD_ISACTIVERnE, GICD_ISACTIVERnEN):
> +        if ( dabt.size != DABT_WORD )
> +            goto bad_width;
> +        printk(XENLOG_G_ERR
> +               "%pv: %s: unhandled word write %#"PRIregister" to ISACTIVER%dE\n",
> +               v, name, r, reg - GICD_ISACTIVERnE);
> +        return 0;
> +
> +    case VRANGE32(GICD_ICACTIVERnE, GICD_ICACTIVERnEN):
> +        printk(XENLOG_G_ERR
> +               "%pv: %s: unhandled word write %#"PRIregister" to ICACTIVER%dE\n",
> +               v, name, r, reg - GICD_ICACTIVERnE);
> +        goto write_ignore_32;
> +
>      case VRANGE32(GICD_IPRIORITYR, GICD_IPRIORITYRN):
> +    case VRANGE32(GICD_IPRIORITYRnE, GICD_IPRIORITYRnEN):
>      {
> -        uint32_t *ipriorityr, priority;
> +        uint32_t *ipriorityr, priority, offset;
>  
>          if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width;
> -        rank = vgic_rank_offset(v, 8, reg - GICD_IPRIORITYR, DABT_WORD);
> +        if ( reg >= GICD_IPRIORITYRnE ) {

Brace should go on new line

> +            offset = reg - GICD_IPRIORITYRnE;
> +            rank = vgic_ext_rank_offset(v, 8, offset, DABT_WORD);
> +        }
> +        else
> +        {
> +            offset = reg - GICD_IPRIORITYR;
> +            rank = vgic_rank_offset(v, 8, offset, DABT_WORD);
> +        }
>          if ( rank == NULL ) goto write_ignore;
>          vgic_lock_rank(v, rank, flags);
> -        ipriorityr = &rank->ipriorityr[REG_RANK_INDEX(8, reg - GICD_IPRIORITYR,
> -                                                      DABT_WORD)];
> +        ipriorityr = &rank->ipriorityr[REG_RANK_INDEX(8, offset, DABT_WORD)];
>          priority = ACCESS_ONCE(*ipriorityr);
>          vreg_reg32_update(&priority, r, info);
>          ACCESS_ONCE(*ipriorityr) = priority;
> @@ -859,10 +938,14 @@ static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
>          goto write_ignore_32;
>  
>      case VRANGE32(GICD_ICFGR + 4, GICD_ICFGRN): /* PPI + SPIs */
> +    case VRANGE32(GICD_ICFGRnE, GICD_ICFGRnEN):
>          /* ICFGR1 for PPI's, which is implementation defined
>             if ICFGR1 is programmable or not. We chose to program */
>          if ( dabt.size != DABT_WORD ) goto bad_width;
> -        rank = vgic_rank_offset(v, 2, reg - GICD_ICFGR, DABT_WORD);
> +        if ( reg >= GICD_ICFGRnE )
> +            rank = vgic_ext_rank_offset(v, 2, reg - GICD_ICFGRnE, DABT_WORD);
> +        else
> +            rank = vgic_rank_offset(v, 2, reg - GICD_ICFGR, DABT_WORD);
>          if ( rank == NULL ) goto write_ignore;
>          vgic_lock_rank(v, rank, flags);
>          vreg_reg32_update(&rank->icfg[REG_RANK_INDEX(2, reg - GICD_ICFGR,
> @@ -1129,6 +1212,16 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
>              typer |= GICD_TYPE_LPIS;
>  
>          typer |= (v->domain->arch.vgic.intid_bits - 1) << GICD_TYPE_ID_BITS_SHIFT;
> +#ifdef CONFIG_GICV3_ESPI
> +        if ( v->domain->arch.vgic.nr_espis > 0 )
> +        {
> +            /* Set eSPI support bit for the domain */
> +            typer |= GICD_TYPER_ESPI;
> +            /* Set ESPI range bits */
> +            typer |= (DIV_ROUND_UP(v->domain->arch.vgic.nr_espis, 32) - 1)
> +                       << GICD_TYPER_ESPI_RANGE_SHIFT;
> +        }
> +#endif
>  
>          *r = vreg_reg32_extract(typer, info);
>  
> @@ -1194,6 +1287,16 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
>      case VRANGE32(GICD_IPRIORITYR, GICD_IPRIORITYRN):
>      case VRANGE32(GICD_ICFGR, GICD_ICFGRN):
>      case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
> +    case VRANGE32(GICD_IGROUPRnE, GICD_IGROUPRnEN):
> +    case VRANGE32(GICD_ISENABLERnE, GICD_ISENABLERnEN):
> +    case VRANGE32(GICD_ICENABLERnE, GICD_ICENABLERnEN):
> +    case VRANGE32(GICD_ISPENDRnE, GICD_ISPENDRnEN):
> +    case VRANGE32(GICD_ICPENDRnE, GICD_ICPENDRnEN):
> +    case VRANGE32(GICD_ISACTIVERnE, GICD_ISACTIVERnEN):
> +    case VRANGE32(GICD_ICACTIVERnE, GICD_ICACTIVERnEN):
> +    case VRANGE32(GICD_IPRIORITYRnE, GICD_IPRIORITYRnEN):
> +    case VRANGE32(GICD_ICFGRnE, GICD_ICFGRnEN):
> +    case VRANGE32(GICD_IGRPMODRnE, GICD_IGRPMODRnEN):
>          /*
>           * Above all register are common with GICR and GICD
>           * Manage in common
> @@ -1201,6 +1304,7 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
>          return __vgic_v3_distr_common_mmio_read("vGICD", v, info, gicd_reg, r);
>  
>      case VRANGE32(GICD_NSACR, GICD_NSACRN):
> +    case VRANGE32(GICD_NSACRnE, GICD_NSACRnEN):
>          /* We do not implement security extensions for guests, read zero */
>          goto read_as_zero_32;
>  
> @@ -1216,16 +1320,21 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
>          /* Replaced with GICR_ISPENDR0. So ignore write */
>          goto read_as_zero_32;
>  
> -    case VRANGE32(0x0F30, 0x60FC):
> +    case VRANGE32(0x0F30, 0x0FFC):
>          goto read_reserved;
>  
>      case VRANGE64(GICD_IROUTER32, GICD_IROUTER1019):
> +    case VRANGE64(GICD_IROUTERnE, GICD_IROUTERnEN):
>      {
>          uint64_t irouter;
>  
>          if ( !vgic_reg64_check_access(dabt) ) goto bad_width;
> -        rank = vgic_rank_offset(v, 64, gicd_reg - GICD_IROUTER,
> -                                DABT_DOUBLE_WORD);
> +        if ( gicd_reg >= GICD_IROUTERnE )
> +            rank = vgic_ext_rank_offset(v, 64, gicd_reg - GICD_IROUTERnE,
> +                                        DABT_DOUBLE_WORD);
> +        else
> +            rank = vgic_rank_offset(v, 64, gicd_reg - GICD_IROUTER,
> +                                    DABT_DOUBLE_WORD);
>          if ( rank == NULL ) goto read_as_zero;
>          vgic_lock_rank(v, rank, flags);
>          irouter = vgic_fetch_irouter(rank, gicd_reg - GICD_IROUTER);
> @@ -1235,8 +1344,8 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
>  
>          return 1;
>      }
> -
> -    case VRANGE32(0x7FE0, 0xBFFC):
> +    case VRANGE32(0x3700, 0x60FC):
> +    case VRANGE32(0xA004, 0xBFFC):
>          goto read_reserved;
>  
>      case VRANGE32(0xC000, 0xFFCC):
> @@ -1382,12 +1491,23 @@ static int vgic_v3_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
>      case VRANGE32(GICD_IPRIORITYR, GICD_IPRIORITYRN):
>      case VRANGE32(GICD_ICFGR, GICD_ICFGRN):
>      case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
> +    case VRANGE32(GICD_IGROUPRnE, GICD_IGROUPRnEN):
> +    case VRANGE32(GICD_ISENABLERnE, GICD_ISENABLERnEN):
> +    case VRANGE32(GICD_ICENABLERnE, GICD_ICENABLERnEN):
> +    case VRANGE32(GICD_ISPENDRnE, GICD_ISPENDRnEN):
> +    case VRANGE32(GICD_ICPENDRnE, GICD_ICPENDRnEN):
> +    case VRANGE32(GICD_ISACTIVERnE, GICD_ISACTIVERnEN):
> +    case VRANGE32(GICD_ICACTIVERnE, GICD_ICACTIVERnEN):
> +    case VRANGE32(GICD_IPRIORITYRnE, GICD_IPRIORITYRnEN):
> +    case VRANGE32(GICD_ICFGRnE, GICD_ICFGRnEN):
> +    case VRANGE32(GICD_IGRPMODRnE, GICD_IGRPMODRnEN):
>          /* Above registers are common with GICR and GICD
>           * Manage in common */
>          return __vgic_v3_distr_common_mmio_write("vGICD", v, info,
>                                                   gicd_reg, r);
>  
>      case VRANGE32(GICD_NSACR, GICD_NSACRN):
> +    case VRANGE32(GICD_NSACRnE, GICD_NSACRnEN):
>          /* We do not implement security extensions for guests, write ignore */
>          goto write_ignore_32;
>  
> @@ -1405,26 +1525,38 @@ static int vgic_v3_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
>          if ( dabt.size != DABT_WORD ) goto bad_width;
>          return 0;
>  
> -    case VRANGE32(0x0F30, 0x60FC):
> +    case VRANGE32(0x0F30, 0x0FFC):
>          goto write_reserved;
>  
>      case VRANGE64(GICD_IROUTER32, GICD_IROUTER1019):
> +    case VRANGE64(GICD_IROUTERnE, GICD_IROUTERnEN):
>      {
>          uint64_t irouter;
> +        unsigned int offset, virq;
>  
>          if ( !vgic_reg64_check_access(dabt) ) goto bad_width;
> -        rank = vgic_rank_offset(v, 64, gicd_reg - GICD_IROUTER,
> -                                DABT_DOUBLE_WORD);
> +        if ( gicd_reg >= GICD_IROUTERnE ) {

Braces should go into separate line

> +            offset = gicd_reg - GICD_IROUTERnE;
> +            rank = vgic_ext_rank_offset(v, 64, offset, DABT_DOUBLE_WORD);
> +        } else {

... so "else" also should be on a separate line

> +            offset = gicd_reg - GICD_IROUTER;
> +            rank = vgic_rank_offset(v, 64, offset, DABT_DOUBLE_WORD);
> +        }
>          if ( rank == NULL ) goto write_ignore;
>          vgic_lock_rank(v, rank, flags);
> -        irouter = vgic_fetch_irouter(rank, gicd_reg - GICD_IROUTER);
> +        irouter = vgic_fetch_irouter(rank, offset);
>          vreg_reg64_update(&irouter, r, info);
> -        vgic_store_irouter(v->domain, rank, gicd_reg - GICD_IROUTER, irouter);
> +        if ( gicd_reg >= GICD_IROUTERnE )
> +            virq = ESPI_IDX2INTID(offset / NR_BYTES_PER_IROUTER);
> +        else
> +            virq = offset / NR_BYTES_PER_IROUTER;
> +        vgic_store_irouter(v->domain, rank, virq, irouter);
>          vgic_unlock_rank(v, rank, flags);
>          return 1;
>      }
>  
> -    case VRANGE32(0x7FE0, 0xBFFC):
> +    case VRANGE32(0x3700, 0x60FC):
> +    case VRANGE32(0xA004, 0xBFFC):
>          goto write_reserved;
>  
>      case VRANGE32(0xC000, 0xFFCC):
> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index c9b9528c66..27ffdf316c 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -193,6 +193,18 @@ int domain_vgic_register(struct domain *d, unsigned int *mmio_count)
>  }
>  
>  #ifdef CONFIG_GICV3_ESPI
> +/*
> + * The function behavior is the same as for regular SPIs (vgic_rank_offset),
> + * but it operates with extended SPI ranks.
> + */
> +struct vgic_irq_rank *vgic_ext_rank_offset(struct vcpu *v, unsigned int b,
> +                                           unsigned int n, unsigned int s)
> +{
> +    unsigned int rank = REG_RANK_NR(b, (n >> s));
> +
> +    return vgic_get_rank(v, rank + EXT_RANK_MIN);
> +}
> +
>  static unsigned int vgic_num_spi_lines(struct domain *d)
>  {
>      return d->arch.vgic.nr_spis + d->arch.vgic.nr_espis;
> @@ -241,6 +253,17 @@ struct pending_irq *espi_to_pending(struct domain *d, unsigned int irq)
>  {
>      return NULL;
>  }
> +
> +/*
> + * It is expected that, in the case of CONFIG_GICV3_ESPI=n, the function will
> + * return NULL. In this scenario, mmio_read/mmio_write will be treated as
> + * RAZ/WI, as expected.
> + */
> +struct vgic_irq_rank *vgic_ext_rank_offset(struct vcpu *v, unsigned int b,
> +                                           unsigned int n, unsigned int s)
> +{
> +    return NULL;
> +}
>  #endif
>  
>  static unsigned int vgic_num_alloc_irqs(struct domain *d)

-- 
WBR, Volodymyr