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>
---
Changes in V2:
- add missing rank index conversion for pending and inflight irqs
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
---
xen/arch/arm/vgic-v3.c | 275 +++++++++++++++++++++++++++++++++++++++--
1 file changed, 266 insertions(+), 9 deletions(-)
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 4369c55177..56c539bb1b 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,6 +682,9 @@ 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):
+#ifdef CONFIG_GICV3_ESPI
+ case VRANGE32(GICD_IGROUPRnE, GICD_IGROUPRnEN):
+#endif
/* We do not implement security extensions for guests, read zero */
if ( dabt.size != DABT_WORD ) goto bad_width;
goto read_as_zero;
@@ -710,11 +710,19 @@ 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):
+#ifdef CONFIG_GICV3_ESPI
+ case VRANGE32(GICD_ISPENDRnE, GICD_ISPENDRnEN):
+ case VRANGE32(GICD_ICPENDRnE, GICD_ICPENDRnEN):
+#endif
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):
+#ifdef CONFIG_GICV3_ESPI
+ case VRANGE32(GICD_ISACTIVERnE, GICD_ISACTIVERnEN):
+ case VRANGE32(GICD_ICACTIVERnE, GICD_ICACTIVERnEN):
+#endif
goto read_as_zero;
case VRANGE32(GICD_IPRIORITYR, GICD_IPRIORITYRN):
@@ -752,6 +760,69 @@ static int __vgic_v3_distr_common_mmio_read(const char *name, struct vcpu *v,
return 1;
}
+#ifdef CONFIG_GICV3_ESPI
+ case VRANGE32(GICD_ISENABLERnE, GICD_ISENABLERnEN):
+ if ( dabt.size != DABT_WORD )
+ goto bad_width;
+ rank = vgic_ext_rank_offset(v, 1, reg - GICD_ISENABLERnE, DABT_WORD);
+ if ( rank == NULL )
+ goto read_as_zero;
+ vgic_lock_rank(v, rank, flags);
+ *r = vreg_reg32_extract(rank->ienable, info);
+ vgic_unlock_rank(v, rank, flags);
+ return 1;
+
+ case VRANGE32(GICD_ICENABLERnE, GICD_ICENABLERnEN):
+ if ( dabt.size != DABT_WORD )
+ goto bad_width;
+ rank = vgic_ext_rank_offset(v, 1, reg - GICD_ICENABLERnE, DABT_WORD);
+ if ( rank == NULL )
+ goto read_as_zero;
+ vgic_lock_rank(v, rank, flags);
+ *r = vreg_reg32_extract(rank->ienable, info);
+ vgic_unlock_rank(v, rank, flags);
+ return 1;
+
+ 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_ext_rank_offset(v, 8, reg - GICD_IPRIORITYRnE, DABT_WORD);
+ if ( rank == NULL )
+ goto read_as_zero;
+ rank_index = REG_RANK_INDEX(8, reg - GICD_IPRIORITYRnE, DABT_WORD);
+
+ vgic_lock_rank(v, rank, flags);
+ ipriorityr = ACCESS_ONCE(rank->ipriorityr[rank_index]);
+ vgic_unlock_rank(v, rank, flags);
+
+ *r = vreg_reg32_extract(ipriorityr, info);
+
+ return 1;
+ }
+
+ case VRANGE32(GICD_ICFGRnE, GICD_ICFGRnEN):
+ {
+ uint32_t icfgr;
+
+ if ( dabt.size != DABT_WORD )
+ goto bad_width;
+ rank = vgic_ext_rank_offset(v, 2, reg - GICD_ICFGRnE, DABT_WORD);
+ if ( rank == NULL )
+ goto read_as_zero;
+ vgic_lock_rank(v, rank, flags);
+ icfgr = rank->icfg[REG_RANK_INDEX(2, reg - GICD_ICFGRnE, DABT_WORD)];
+ vgic_unlock_rank(v, rank, flags);
+
+ *r = vreg_reg32_extract(icfgr, info);
+
+ return 1;
+ }
+#endif
+
default:
printk(XENLOG_G_ERR
"%pv: %s: unhandled read r%d offset %#08x\n",
@@ -782,6 +853,9 @@ 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):
+#ifdef CONFIG_GICV3_ESPI
+ case VRANGE32(GICD_IGROUPRnE, GICD_IGROUPRnEN):
+#endif
/* We do not implement security extensions for guests, write ignore */
goto write_ignore_32;
@@ -871,6 +945,99 @@ static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
vgic_unlock_rank(v, rank, flags);
return 1;
+#ifdef CONFIG_GICV3_ESPI
+ case VRANGE32(GICD_ISENABLERnE, GICD_ISENABLERnEN):
+ if ( dabt.size != DABT_WORD )
+ goto bad_width;
+ rank = vgic_ext_rank_offset(v, 1, reg - GICD_ISENABLERnE, 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), EXT_RANK_IDX2NUM(rank->index));
+ vgic_unlock_rank(v, rank, flags);
+ return 1;
+
+ case VRANGE32(GICD_ICENABLERnE, GICD_ICENABLERnEN):
+ if ( dabt.size != DABT_WORD )
+ goto bad_width;
+ rank = vgic_ext_rank_offset(v, 1, reg - GICD_ICENABLERnE, 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, EXT_RANK_IDX2NUM(rank->index));
+ vgic_unlock_rank(v, rank, flags);
+ return 1;
+
+ case VRANGE32(GICD_ISPENDRnE, GICD_ISPENDRnEN):
+ if ( dabt.size != DABT_WORD )
+ goto bad_width;
+ rank = vgic_ext_rank_offset(v, 1, reg - GICD_ISPENDRnE, DABT_WORD);
+ if ( rank == NULL )
+ goto write_ignore;
+
+ vgic_set_irqs_pending(v, r, EXT_RANK_IDX2NUM(rank->index));
+
+ return 1;
+
+ case VRANGE32(GICD_ICPENDRnE, GICD_ICPENDRnEN):
+ if ( dabt.size != DABT_WORD )
+ goto bad_width;
+ rank = vgic_ext_rank_offset(v, 1, reg - GICD_ICPENDRnE, DABT_WORD);
+ if ( rank == NULL )
+ goto write_ignore;
+
+ vgic_check_inflight_irqs_pending(v, EXT_RANK_IDX2NUM(rank->index), r);
+
+ goto write_ignore;
+ 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_ICACTIVER);
+ goto write_ignore_32;
+
+ case VRANGE32(GICD_IPRIORITYRnE, GICD_IPRIORITYRnEN):
+ {
+ uint32_t *ipriorityr, priority;
+
+ if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD )
+ goto bad_width;
+ rank = vgic_ext_rank_offset(v, 8, reg - GICD_IPRIORITYRnE, DABT_WORD);
+ if ( rank == NULL ) goto write_ignore;
+ vgic_lock_rank(v, rank, flags);
+ ipriorityr = &rank->ipriorityr[REG_RANK_INDEX(8, reg - GICD_IPRIORITYRnE,
+ DABT_WORD)];
+ priority = ACCESS_ONCE(*ipriorityr);
+ vreg_reg32_update(&priority, r, info);
+ ACCESS_ONCE(*ipriorityr) = priority;
+ vgic_unlock_rank(v, rank, flags);
+ return 1;
+ }
+ case VRANGE32(GICD_ICFGRnE, GICD_ICFGRnEN):
+ if ( dabt.size != DABT_WORD )
+ goto bad_width;
+ rank = vgic_ext_rank_offset(v, 2, reg - GICD_ICFGRnE, 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_ICFGRnE,
+ DABT_WORD)],
+ r, info);
+ vgic_unlock_rank(v, rank, flags);
+ return 1;
+#endif
+
default:
printk(XENLOG_G_ERR
"%pv: %s: unhandled write r%d=%"PRIregister" offset %#08x\n",
@@ -1129,6 +1296,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 +1371,18 @@ 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):
+
+#ifdef CONFIG_GICV3_ESPI
+ 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):
+#endif
/*
* Above all register are common with GICR and GICD
* Manage in common
@@ -1216,7 +1405,11 @@ 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;
+#ifdef CONFIG_GICV3_ESPI
+ case VRANGE32(0x3100, 0x60FC):
+#else
case VRANGE32(0x0F30, 0x60FC):
+#endif
goto read_reserved;
case VRANGE64(GICD_IROUTER32, GICD_IROUTER1019):
@@ -1235,8 +1428,30 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
return 1;
}
+#ifdef CONFIG_GICV3_ESPI
+ case VRANGE64(GICD_IROUTERnE, GICD_IROUTERnEN):
+ {
+ uint64_t irouter;
+
+ if ( !vgic_reg64_check_access(dabt) )
+ goto bad_width;
+ rank = vgic_ext_rank_offset(v, 64, gicd_reg - GICD_IROUTERnE,
+ DABT_DOUBLE_WORD);
+ if ( rank == NULL )
+ goto read_as_zero;
+ vgic_lock_rank(v, rank, flags);
+ irouter = vgic_fetch_irouter(rank, gicd_reg - GICD_IROUTERnE);
+ vgic_unlock_rank(v, rank, flags);
+ *r = vreg_reg64_extract(irouter, info);
+
+ return 1;
+ }
+
+ case VRANGE32(0xA004, 0xBFFC):
+#else
case VRANGE32(0x7FE0, 0xBFFC):
+#endif
goto read_reserved;
case VRANGE32(0xC000, 0xFFCC):
@@ -1382,6 +1597,18 @@ 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):
+
+#ifdef CONFIG_GICV3_ESPI
+ 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):
+#endif
/* Above registers are common with GICR and GICD
* Manage in common */
return __vgic_v3_distr_common_mmio_write("vGICD", v, info,
@@ -1405,26 +1632,56 @@ static int vgic_v3_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
if ( dabt.size != DABT_WORD ) goto bad_width;
return 0;
+#ifdef CONFIG_GICV3_ESPI
+ case VRANGE32(0x3100, 0x60FC):
+#else
case VRANGE32(0x0F30, 0x60FC):
+#endif
goto write_reserved;
case VRANGE64(GICD_IROUTER32, GICD_IROUTER1019):
{
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);
+ 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);
+ virq = offset / NR_BYTES_PER_IROUTER;
+ vgic_store_irouter(v->domain, rank, virq, irouter);
+ vgic_unlock_rank(v, rank, flags);
+ return 1;
+ }
+
+#ifdef CONFIG_GICV3_ESPI
+ case VRANGE64(GICD_IROUTERnE, GICD_IROUTERnEN):
+ {
+ uint64_t irouter;
+ unsigned int offset, virq;
+
+ if ( !vgic_reg64_check_access(dabt) )
+ goto bad_width;
+ offset = gicd_reg - GICD_IROUTERnE;
+ rank = vgic_ext_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, offset);
vreg_reg64_update(&irouter, r, info);
- vgic_store_irouter(v->domain, rank, gicd_reg - GICD_IROUTER, irouter);
+ virq = ESPI_IDX2INTID(offset / NR_BYTES_PER_IROUTER);
+ vgic_store_irouter(v->domain, rank, virq, irouter);
vgic_unlock_rank(v, rank, flags);
return 1;
}
+ case VRANGE32(0xA004, 0xBFFC):
+#else
case VRANGE32(0x7FE0, 0xBFFC):
+#endif
goto write_reserved;
case VRANGE32(0xC000, 0xFFCC):
--
2.34.1
On 26.08.25 17:05, Leonid Komarianskyi wrote:
Hello Leonid
> 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>
>
> ---
> Changes in V2:
> - add missing rank index conversion for pending and inflight irqs
>
> 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
> ---
> xen/arch/arm/vgic-v3.c | 275 +++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 266 insertions(+), 9 deletions(-)
>
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index 4369c55177..56c539bb1b 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,6 +682,9 @@ 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):
> +#ifdef CONFIG_GICV3_ESPI
> + case VRANGE32(GICD_IGROUPRnE, GICD_IGROUPRnEN):
> +#endif
> /* We do not implement security extensions for guests, read zero */
> if ( dabt.size != DABT_WORD ) goto bad_width;
> goto read_as_zero;
> @@ -710,11 +710,19 @@ 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):
> +#ifdef CONFIG_GICV3_ESPI
> + case VRANGE32(GICD_ISPENDRnE, GICD_ISPENDRnEN):
> + case VRANGE32(GICD_ICPENDRnE, GICD_ICPENDRnEN):
> +#endif
> 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):
> +#ifdef CONFIG_GICV3_ESPI
> + case VRANGE32(GICD_ISACTIVERnE, GICD_ISACTIVERnEN):
> + case VRANGE32(GICD_ICACTIVERnE, GICD_ICACTIVERnEN):
> +#endif
> goto read_as_zero;
>
> case VRANGE32(GICD_IPRIORITYR, GICD_IPRIORITYRN):
> @@ -752,6 +760,69 @@ static int __vgic_v3_distr_common_mmio_read(const char *name, struct vcpu *v,
> return 1;
> }
>
> +#ifdef CONFIG_GICV3_ESPI
> + case VRANGE32(GICD_ISENABLERnE, GICD_ISENABLERnEN):
> + if ( dabt.size != DABT_WORD )
> + goto bad_width;
> + rank = vgic_ext_rank_offset(v, 1, reg - GICD_ISENABLERnE, DABT_WORD);
> + if ( rank == NULL )
> + goto read_as_zero;
> + vgic_lock_rank(v, rank, flags);
> + *r = vreg_reg32_extract(rank->ienable, info);
> + vgic_unlock_rank(v, rank, flags);
> + return 1;
> +
> + case VRANGE32(GICD_ICENABLERnE, GICD_ICENABLERnEN):
> + if ( dabt.size != DABT_WORD )
> + goto bad_width;
> + rank = vgic_ext_rank_offset(v, 1, reg - GICD_ICENABLERnE, DABT_WORD);
> + if ( rank == NULL )
> + goto read_as_zero;
> + vgic_lock_rank(v, rank, flags);
> + *r = vreg_reg32_extract(rank->ienable, info);
> + vgic_unlock_rank(v, rank, flags);
> + return 1;
> +
> + 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_ext_rank_offset(v, 8, reg - GICD_IPRIORITYRnE, DABT_WORD);
> + if ( rank == NULL )
> + goto read_as_zero;
> + rank_index = REG_RANK_INDEX(8, reg - GICD_IPRIORITYRnE, DABT_WORD);
> +
> + vgic_lock_rank(v, rank, flags);
> + ipriorityr = ACCESS_ONCE(rank->ipriorityr[rank_index]);
> + vgic_unlock_rank(v, rank, flags);
> +
> + *r = vreg_reg32_extract(ipriorityr, info);
> +
> + return 1;
> + }
> +
> + case VRANGE32(GICD_ICFGRnE, GICD_ICFGRnEN):
> + {
> + uint32_t icfgr;
> +
> + if ( dabt.size != DABT_WORD )
> + goto bad_width;
> + rank = vgic_ext_rank_offset(v, 2, reg - GICD_ICFGRnE, DABT_WORD);
> + if ( rank == NULL )
> + goto read_as_zero;
> + vgic_lock_rank(v, rank, flags);
> + icfgr = rank->icfg[REG_RANK_INDEX(2, reg - GICD_ICFGRnE, DABT_WORD)];
> + vgic_unlock_rank(v, rank, flags);
> +
> + *r = vreg_reg32_extract(icfgr, info);
> +
> + return 1;
> + }
> +#endif
> +
> default:
> printk(XENLOG_G_ERR
> "%pv: %s: unhandled read r%d offset %#08x\n",
> @@ -782,6 +853,9 @@ 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):
> +#ifdef CONFIG_GICV3_ESPI
> + case VRANGE32(GICD_IGROUPRnE, GICD_IGROUPRnEN):
> +#endif
> /* We do not implement security extensions for guests, write ignore */
> goto write_ignore_32;
>
> @@ -871,6 +945,99 @@ static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
> vgic_unlock_rank(v, rank, flags);
> return 1;
>
> +#ifdef CONFIG_GICV3_ESPI
> + case VRANGE32(GICD_ISENABLERnE, GICD_ISENABLERnEN):
> + if ( dabt.size != DABT_WORD )
> + goto bad_width;
> + rank = vgic_ext_rank_offset(v, 1, reg - GICD_ISENABLERnE, 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), EXT_RANK_IDX2NUM(rank->index));
> + vgic_unlock_rank(v, rank, flags);
> + return 1;
> +
> + case VRANGE32(GICD_ICENABLERnE, GICD_ICENABLERnEN):
> + if ( dabt.size != DABT_WORD )
> + goto bad_width;
> + rank = vgic_ext_rank_offset(v, 1, reg - GICD_ICENABLERnE, 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, EXT_RANK_IDX2NUM(rank->index));
> + vgic_unlock_rank(v, rank, flags);
> + return 1;
> +
> + case VRANGE32(GICD_ISPENDRnE, GICD_ISPENDRnEN):
> + if ( dabt.size != DABT_WORD )
> + goto bad_width;
> + rank = vgic_ext_rank_offset(v, 1, reg - GICD_ISPENDRnE, DABT_WORD);
> + if ( rank == NULL )
> + goto write_ignore;
> +
> + vgic_set_irqs_pending(v, r, EXT_RANK_IDX2NUM(rank->index));
> +
> + return 1;
> +
> + case VRANGE32(GICD_ICPENDRnE, GICD_ICPENDRnEN):
> + if ( dabt.size != DABT_WORD )
> + goto bad_width;
> + rank = vgic_ext_rank_offset(v, 1, reg - GICD_ICPENDRnE, DABT_WORD);
> + if ( rank == NULL )
> + goto write_ignore;
> +
> + vgic_check_inflight_irqs_pending(v, EXT_RANK_IDX2NUM(rank->index), r);
> +
> + goto write_ignore;
> + 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;
Guest write access to GICD_ISACTIVER<n>E will lead to abort. But, I know
you just repeated the logic for regular GICD_ISACTIVER<n>.
> +
> + 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_ICACTIVER);
s/GICD_ICACTIVER/GICD_ICACTIVERnE
> + goto write_ignore_32;
> +
> + case VRANGE32(GICD_IPRIORITYRnE, GICD_IPRIORITYRnEN):
> + {
> + uint32_t *ipriorityr, priority;
> +
> + if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD )
> + goto bad_width;
> + rank = vgic_ext_rank_offset(v, 8, reg - GICD_IPRIORITYRnE, DABT_WORD);
> + if ( rank == NULL ) goto write_ignore;
> + vgic_lock_rank(v, rank, flags);
> + ipriorityr = &rank->ipriorityr[REG_RANK_INDEX(8, reg - GICD_IPRIORITYRnE,
> + DABT_WORD)];
> + priority = ACCESS_ONCE(*ipriorityr);
> + vreg_reg32_update(&priority, r, info);
> + ACCESS_ONCE(*ipriorityr) = priority;
> + vgic_unlock_rank(v, rank, flags);
> + return 1;
> + }
NIT: emply line please (and in similar places)
> + case VRANGE32(GICD_ICFGRnE, GICD_ICFGRnEN):
> + if ( dabt.size != DABT_WORD )
> + goto bad_width;
> + rank = vgic_ext_rank_offset(v, 2, reg - GICD_ICFGRnE, 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_ICFGRnE,
> + DABT_WORD)],
> + r, info);
> + vgic_unlock_rank(v, rank, flags);
> + return 1;
> +#endif
> +
> default:
> printk(XENLOG_G_ERR
> "%pv: %s: unhandled write r%d=%"PRIregister" offset %#08x\n",
> @@ -1129,6 +1296,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 +1371,18 @@ 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):
> +
> +#ifdef CONFIG_GICV3_ESPI
> + 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):
> +#endif
GICD_IGRPMODR<n>E is missed? I guess, it should be RAZ as regular
GICD_IGRPMODR<n>.
Also GICD_NSACR<n>E is missed, although the case for regular
GICD_NSACR<n> is present (not visible in patch context).
> /*
> * Above all register are common with GICR and GICD
> * Manage in common
> @@ -1216,7 +1405,11 @@ 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;
>
> +#ifdef CONFIG_GICV3_ESPI
> + case VRANGE32(0x3100, 0x60FC):
> +#else
> case VRANGE32(0x0F30, 0x60FC):
> +#endif
> goto read_reserved;
>
> case VRANGE64(GICD_IROUTER32, GICD_IROUTER1019):
> @@ -1235,8 +1428,30 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
>
> return 1;
> }
> +#ifdef CONFIG_GICV3_ESPI
> + case VRANGE64(GICD_IROUTERnE, GICD_IROUTERnEN):
> + {
> + uint64_t irouter;
> +
> + if ( !vgic_reg64_check_access(dabt) )
> + goto bad_width;
> + rank = vgic_ext_rank_offset(v, 64, gicd_reg - GICD_IROUTERnE,
> + DABT_DOUBLE_WORD);
> + if ( rank == NULL )
> + goto read_as_zero;
> + vgic_lock_rank(v, rank, flags);
> + irouter = vgic_fetch_irouter(rank, gicd_reg - GICD_IROUTERnE);
> + vgic_unlock_rank(v, rank, flags);
>
> + *r = vreg_reg64_extract(irouter, info);
> +
> + return 1;
> + }
> +
> + case VRANGE32(0xA004, 0xBFFC):
> +#else
> case VRANGE32(0x7FE0, 0xBFFC):
> +#endif
> goto read_reserved;
>
> case VRANGE32(0xC000, 0xFFCC):
> @@ -1382,6 +1597,18 @@ 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):
> +
> +#ifdef CONFIG_GICV3_ESPI
> + 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):
> +#endif
GICD_IGRPMODR<n>E is missed? I guess, it should be WI as regular
GICD_IGRPMODR<n>.
Also GICD_NSACR<n>E is missed, although the case for regular
GICD_NSACR<n> is present (not visible in patch context).
> /* Above registers are common with GICR and GICD
> * Manage in common */
> return __vgic_v3_distr_common_mmio_write("vGICD", v, info,
> @@ -1405,26 +1632,56 @@ static int vgic_v3_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
> if ( dabt.size != DABT_WORD ) goto bad_width;
> return 0;
>
> +#ifdef CONFIG_GICV3_ESPI
> + case VRANGE32(0x3100, 0x60FC):
> +#else
> case VRANGE32(0x0F30, 0x60FC):
> +#endif
I wonder, can we have #defines for these magics (at least for the start
of the reserved range)?
> goto write_reserved;
>
> case VRANGE64(GICD_IROUTER32, GICD_IROUTER1019):
> {
> 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);
> + 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);
> + virq = offset / NR_BYTES_PER_IROUTER;
> + vgic_store_irouter(v->domain, rank, virq, irouter);
> + vgic_unlock_rank(v, rank, flags);
> + return 1;
> + }
> +
> +#ifdef CONFIG_GICV3_ESPI
> + case VRANGE64(GICD_IROUTERnE, GICD_IROUTERnEN):
> + {
> + uint64_t irouter;
> + unsigned int offset, virq;
> +
> + if ( !vgic_reg64_check_access(dabt) )
> + goto bad_width;
> + offset = gicd_reg - GICD_IROUTERnE;
> + rank = vgic_ext_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, offset);
> vreg_reg64_update(&irouter, r, info);
> - vgic_store_irouter(v->domain, rank, gicd_reg - GICD_IROUTER, irouter);
> + virq = ESPI_IDX2INTID(offset / NR_BYTES_PER_IROUTER);
> + vgic_store_irouter(v->domain, rank, virq, irouter);
> vgic_unlock_rank(v, rank, flags);
> return 1;
> }
>
> + case VRANGE32(0xA004, 0xBFFC):
> +#else
> case VRANGE32(0x7FE0, 0xBFFC):
> +#endif
> goto write_reserved;
>
> case VRANGE32(0xC000, 0xFFCC):
Hello Oleksandr,
Thank you for your close review.
On 26.08.25 22:57, Oleksandr Tyshchenko wrote:
>
>
> On 26.08.25 17:05, Leonid Komarianskyi wrote:
>
> Hello Leonid
>
>
>> 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>
>>
>> ---
>> Changes in V2:
>> - add missing rank index conversion for pending and inflight irqs
>>
>> 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
>> ---
>> xen/arch/arm/vgic-v3.c | 275 +++++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 266 insertions(+), 9 deletions(-)
>>
>> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
>> index 4369c55177..56c539bb1b 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,6 +682,9 @@ 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):
>> +#ifdef CONFIG_GICV3_ESPI
>> + case VRANGE32(GICD_IGROUPRnE, GICD_IGROUPRnEN):
>> +#endif
>> /* We do not implement security extensions for guests, read
>> zero */
>> if ( dabt.size != DABT_WORD ) goto bad_width;
>> goto read_as_zero;
>> @@ -710,11 +710,19 @@ 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):
>> +#ifdef CONFIG_GICV3_ESPI
>> + case VRANGE32(GICD_ISPENDRnE, GICD_ISPENDRnEN):
>> + case VRANGE32(GICD_ICPENDRnE, GICD_ICPENDRnEN):
>> +#endif
>> 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):
>> +#ifdef CONFIG_GICV3_ESPI
>> + case VRANGE32(GICD_ISACTIVERnE, GICD_ISACTIVERnEN):
>> + case VRANGE32(GICD_ICACTIVERnE, GICD_ICACTIVERnEN):
>> +#endif
>> goto read_as_zero;
>> case VRANGE32(GICD_IPRIORITYR, GICD_IPRIORITYRN):
>> @@ -752,6 +760,69 @@ static int __vgic_v3_distr_common_mmio_read(const
>> char *name, struct vcpu *v,
>> return 1;
>> }
>> +#ifdef CONFIG_GICV3_ESPI
>> + case VRANGE32(GICD_ISENABLERnE, GICD_ISENABLERnEN):
>> + if ( dabt.size != DABT_WORD )
>> + goto bad_width;
>> + rank = vgic_ext_rank_offset(v, 1, reg - GICD_ISENABLERnE,
>> DABT_WORD);
>> + if ( rank == NULL )
>> + goto read_as_zero;
>> + vgic_lock_rank(v, rank, flags);
>> + *r = vreg_reg32_extract(rank->ienable, info);
>> + vgic_unlock_rank(v, rank, flags);
>> + return 1;
>> +
>> + case VRANGE32(GICD_ICENABLERnE, GICD_ICENABLERnEN):
>> + if ( dabt.size != DABT_WORD )
>> + goto bad_width;
>> + rank = vgic_ext_rank_offset(v, 1, reg - GICD_ICENABLERnE,
>> DABT_WORD);
>> + if ( rank == NULL )
>> + goto read_as_zero;
>> + vgic_lock_rank(v, rank, flags);
>> + *r = vreg_reg32_extract(rank->ienable, info);
>> + vgic_unlock_rank(v, rank, flags);
>> + return 1;
>> +
>> + 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_ext_rank_offset(v, 8, reg - GICD_IPRIORITYRnE,
>> DABT_WORD);
>> + if ( rank == NULL )
>> + goto read_as_zero;
>> + rank_index = REG_RANK_INDEX(8, reg - GICD_IPRIORITYRnE,
>> DABT_WORD);
>> +
>> + vgic_lock_rank(v, rank, flags);
>> + ipriorityr = ACCESS_ONCE(rank->ipriorityr[rank_index]);
>> + vgic_unlock_rank(v, rank, flags);
>> +
>> + *r = vreg_reg32_extract(ipriorityr, info);
>> +
>> + return 1;
>> + }
>> +
>> + case VRANGE32(GICD_ICFGRnE, GICD_ICFGRnEN):
>> + {
>> + uint32_t icfgr;
>> +
>> + if ( dabt.size != DABT_WORD )
>> + goto bad_width;
>> + rank = vgic_ext_rank_offset(v, 2, reg - GICD_ICFGRnE,
>> DABT_WORD);
>> + if ( rank == NULL )
>> + goto read_as_zero;
>> + vgic_lock_rank(v, rank, flags);
>> + icfgr = rank->icfg[REG_RANK_INDEX(2, reg - GICD_ICFGRnE,
>> DABT_WORD)];
>> + vgic_unlock_rank(v, rank, flags);
>> +
>> + *r = vreg_reg32_extract(icfgr, info);
>> +
>> + return 1;
>> + }
>> +#endif
>> +
>> default:
>> printk(XENLOG_G_ERR
>> "%pv: %s: unhandled read r%d offset %#08x\n",
>> @@ -782,6 +853,9 @@ 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):
>> +#ifdef CONFIG_GICV3_ESPI
>> + case VRANGE32(GICD_IGROUPRnE, GICD_IGROUPRnEN):
>> +#endif
>> /* We do not implement security extensions for guests, write
>> ignore */
>> goto write_ignore_32;
>> @@ -871,6 +945,99 @@ static int
>> __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
>> vgic_unlock_rank(v, rank, flags);
>> return 1;
>> +#ifdef CONFIG_GICV3_ESPI
>> + case VRANGE32(GICD_ISENABLERnE, GICD_ISENABLERnEN):
>> + if ( dabt.size != DABT_WORD )
>> + goto bad_width;
>> + rank = vgic_ext_rank_offset(v, 1, reg - GICD_ISENABLERnE,
>> 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),
>> EXT_RANK_IDX2NUM(rank->index));
>> + vgic_unlock_rank(v, rank, flags);
>> + return 1;
>> +
>> + case VRANGE32(GICD_ICENABLERnE, GICD_ICENABLERnEN):
>> + if ( dabt.size != DABT_WORD )
>> + goto bad_width;
>> + rank = vgic_ext_rank_offset(v, 1, reg - GICD_ICENABLERnE,
>> 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,
>> EXT_RANK_IDX2NUM(rank->index));
>> + vgic_unlock_rank(v, rank, flags);
>> + return 1;
>> +
>> + case VRANGE32(GICD_ISPENDRnE, GICD_ISPENDRnEN):
>> + if ( dabt.size != DABT_WORD )
>> + goto bad_width;
>> + rank = vgic_ext_rank_offset(v, 1, reg - GICD_ISPENDRnE,
>> DABT_WORD);
>> + if ( rank == NULL )
>> + goto write_ignore;
>> +
>> + vgic_set_irqs_pending(v, r, EXT_RANK_IDX2NUM(rank->index));
>> +
>> + return 1;
>> +
>> + case VRANGE32(GICD_ICPENDRnE, GICD_ICPENDRnEN):
>> + if ( dabt.size != DABT_WORD )
>> + goto bad_width;
>> + rank = vgic_ext_rank_offset(v, 1, reg - GICD_ICPENDRnE,
>> DABT_WORD);
>> + if ( rank == NULL )
>> + goto write_ignore;
>> +
>> + vgic_check_inflight_irqs_pending(v, EXT_RANK_IDX2NUM(rank-
>> >index), r);
>> +
>> + goto write_ignore;
>> + 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;
>
> Guest write access to GICD_ISACTIVER<n>E will lead to abort. But, I know
> you just repeated the logic for regular GICD_ISACTIVER<n>.
>
>
Could you please clarify the scenario in which it leads to an abort?
Since it is actually a stub case, it should just print an error message
and that's it.. Do you mean that, in this case, we need to goto
write_ignore_32 label instead of returning 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_ICACTIVER);
>
> s/GICD_ICACTIVER/GICD_ICACTIVERnE
>
>
I will fix that in V4.
>> + goto write_ignore_32;
>> +
>> + case VRANGE32(GICD_IPRIORITYRnE, GICD_IPRIORITYRnEN):
>> + {
>> + uint32_t *ipriorityr, priority;
>> +
>> + if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD )
>> + goto bad_width;
>> + rank = vgic_ext_rank_offset(v, 8, reg - GICD_IPRIORITYRnE,
>> DABT_WORD);
>> + if ( rank == NULL ) goto write_ignore;
>> + vgic_lock_rank(v, rank, flags);
>> + ipriorityr = &rank->ipriorityr[REG_RANK_INDEX(8, reg -
>> GICD_IPRIORITYRnE,
>> + DABT_WORD)];
>> + priority = ACCESS_ONCE(*ipriorityr);
>> + vreg_reg32_update(&priority, r, info);
>> + ACCESS_ONCE(*ipriorityr) = priority;
>> + vgic_unlock_rank(v, rank, flags);
>> + return 1;
>> + }
>
> NIT: emply line please (and in similar places)
>
I will recheck formatting and fix it in V4.
>> + case VRANGE32(GICD_ICFGRnE, GICD_ICFGRnEN):
>> + if ( dabt.size != DABT_WORD )
>> + goto bad_width;
>> + rank = vgic_ext_rank_offset(v, 2, reg - GICD_ICFGRnE,
>> 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_ICFGRnE,
>> + DABT_WORD)],
>> + r, info);
>> + vgic_unlock_rank(v, rank, flags);
>> + return 1;
>> +#endif
>> +
>> default:
>> printk(XENLOG_G_ERR
>> "%pv: %s: unhandled write r%d=%"PRIregister" offset
>> %#08x\n",
>> @@ -1129,6 +1296,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 +1371,18 @@ 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):
>> +
>> +#ifdef CONFIG_GICV3_ESPI
>> + 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):
>> +#endif
>
> GICD_IGRPMODR<n>E is missed? I guess, it should be RAZ as regular
> GICD_IGRPMODR<n>.
>
> Also GICD_NSACR<n>E is missed, although the case for regular
> GICD_NSACR<n> is present (not visible in patch context).
>
Yes, I missed them, since they are not required for real GIC hardware..
I will update the patch that adds eSPI-specific defines and make the
appropriate changes in this patch.
>> /*
>> * Above all register are common with GICR and GICD
>> * Manage in common
>> @@ -1216,7 +1405,11 @@ 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;
>> +#ifdef CONFIG_GICV3_ESPI
>> + case VRANGE32(0x3100, 0x60FC):
>> +#else
>> case VRANGE32(0x0F30, 0x60FC):
>> +#endif
>> goto read_reserved;
>> case VRANGE64(GICD_IROUTER32, GICD_IROUTER1019):
>> @@ -1235,8 +1428,30 @@ static int vgic_v3_distr_mmio_read(struct vcpu
>> *v, mmio_info_t *info,
>> return 1;
>> }
>> +#ifdef CONFIG_GICV3_ESPI
>> + case VRANGE64(GICD_IROUTERnE, GICD_IROUTERnEN):
>> + {
>> + uint64_t irouter;
>> +
>> + if ( !vgic_reg64_check_access(dabt) )
>> + goto bad_width;
>> + rank = vgic_ext_rank_offset(v, 64, gicd_reg - GICD_IROUTERnE,
>> + DABT_DOUBLE_WORD);
>> + if ( rank == NULL )
>> + goto read_as_zero;
>> + vgic_lock_rank(v, rank, flags);
>> + irouter = vgic_fetch_irouter(rank, gicd_reg - GICD_IROUTERnE);
>> + vgic_unlock_rank(v, rank, flags);
>> + *r = vreg_reg64_extract(irouter, info);
>> +
>> + return 1;
>> + }
>> +
>> + case VRANGE32(0xA004, 0xBFFC):
>> +#else
>> case VRANGE32(0x7FE0, 0xBFFC):
>> +#endif
>> goto read_reserved;
>> case VRANGE32(0xC000, 0xFFCC):
>> @@ -1382,6 +1597,18 @@ 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):
>> +
>> +#ifdef CONFIG_GICV3_ESPI
>> + 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):
>> +#endif
>
> GICD_IGRPMODR<n>E is missed? I guess, it should be WI as regular
> GICD_IGRPMODR<n>.
>
>
> Also GICD_NSACR<n>E is missed, although the case for regular
> GICD_NSACR<n> is present (not visible in patch context).
I will update the patch that adds eSPI-specific defines and make the
appropriate changes in this patch.
>
>> /* Above registers are common with GICR and GICD
>> * Manage in common */
>> return __vgic_v3_distr_common_mmio_write("vGICD", v, info,
>> @@ -1405,26 +1632,56 @@ static int vgic_v3_distr_mmio_write(struct
>> vcpu *v, mmio_info_t *info,
>> if ( dabt.size != DABT_WORD ) goto bad_width;
>> return 0;
>> +#ifdef CONFIG_GICV3_ESPI
>> + case VRANGE32(0x3100, 0x60FC):
>> +#else
>> case VRANGE32(0x0F30, 0x60FC):
>> +#endif
>
> I wonder, can we have #defines for these magics (at least for the start
> of the reserved range)?
>
>> goto write_reserved;
>> case VRANGE64(GICD_IROUTER32, GICD_IROUTER1019):
>> {
>> 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);
>> + 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);
>> + virq = offset / NR_BYTES_PER_IROUTER;
>> + vgic_store_irouter(v->domain, rank, virq, irouter);
>> + vgic_unlock_rank(v, rank, flags);
>> + return 1;
>> + }
>> +
>> +#ifdef CONFIG_GICV3_ESPI
>> + case VRANGE64(GICD_IROUTERnE, GICD_IROUTERnEN):
>> + {
>> + uint64_t irouter;
>> + unsigned int offset, virq;
>> +
>> + if ( !vgic_reg64_check_access(dabt) )
>> + goto bad_width;
>> + offset = gicd_reg - GICD_IROUTERnE;
>> + rank = vgic_ext_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, offset);
>> vreg_reg64_update(&irouter, r, info);
>> - vgic_store_irouter(v->domain, rank, gicd_reg - GICD_IROUTER,
>> irouter);
>> + virq = ESPI_IDX2INTID(offset / NR_BYTES_PER_IROUTER);
>> + vgic_store_irouter(v->domain, rank, virq, irouter);
>> vgic_unlock_rank(v, rank, flags);
>> return 1;
>> }
>> + case VRANGE32(0xA004, 0xBFFC):
>> +#else
>> case VRANGE32(0x7FE0, 0xBFFC):
>> +#endif
>> goto write_reserved;
>> case VRANGE32(0xC000, 0xFFCC):
>
Best regards,
Leonid.
On 27.08.25 14:13, Leonid Komarianskyi wrote:
> Hello Oleksandr,
Hello Leonid
>
> Thank you for your close review.
>
> On 26.08.25 22:57, Oleksandr Tyshchenko wrote:
>>
>>
>> On 26.08.25 17:05, Leonid Komarianskyi wrote:
>>
>> Hello Leonid
>>
>>
>>> 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>
>>>
>>> ---
>>> Changes in V2:
>>> - add missing rank index conversion for pending and inflight irqs
>>>
>>> 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
>>> ---
>>> xen/arch/arm/vgic-v3.c | 275 +++++++++++++++++++++++++++++++++++++++--
>>> 1 file changed, 266 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
>>> index 4369c55177..56c539bb1b 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,6 +682,9 @@ 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):
>>> +#ifdef CONFIG_GICV3_ESPI
>>> + case VRANGE32(GICD_IGROUPRnE, GICD_IGROUPRnEN):
>>> +#endif
>>> /* We do not implement security extensions for guests, read
>>> zero */
>>> if ( dabt.size != DABT_WORD ) goto bad_width;
>>> goto read_as_zero;
>>> @@ -710,11 +710,19 @@ 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):
>>> +#ifdef CONFIG_GICV3_ESPI
>>> + case VRANGE32(GICD_ISPENDRnE, GICD_ISPENDRnEN):
>>> + case VRANGE32(GICD_ICPENDRnE, GICD_ICPENDRnEN):
>>> +#endif
>>> 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):
>>> +#ifdef CONFIG_GICV3_ESPI
>>> + case VRANGE32(GICD_ISACTIVERnE, GICD_ISACTIVERnEN):
>>> + case VRANGE32(GICD_ICACTIVERnE, GICD_ICACTIVERnEN):
>>> +#endif
>>> goto read_as_zero;
>>> case VRANGE32(GICD_IPRIORITYR, GICD_IPRIORITYRN):
>>> @@ -752,6 +760,69 @@ static int __vgic_v3_distr_common_mmio_read(const
>>> char *name, struct vcpu *v,
>>> return 1;
>>> }
>>> +#ifdef CONFIG_GICV3_ESPI
>>> + case VRANGE32(GICD_ISENABLERnE, GICD_ISENABLERnEN):
>>> + if ( dabt.size != DABT_WORD )
>>> + goto bad_width;
>>> + rank = vgic_ext_rank_offset(v, 1, reg - GICD_ISENABLERnE,
>>> DABT_WORD);
>>> + if ( rank == NULL )
>>> + goto read_as_zero;
>>> + vgic_lock_rank(v, rank, flags);
>>> + *r = vreg_reg32_extract(rank->ienable, info);
>>> + vgic_unlock_rank(v, rank, flags);
>>> + return 1;
>>> +
>>> + case VRANGE32(GICD_ICENABLERnE, GICD_ICENABLERnEN):
>>> + if ( dabt.size != DABT_WORD )
>>> + goto bad_width;
>>> + rank = vgic_ext_rank_offset(v, 1, reg - GICD_ICENABLERnE,
>>> DABT_WORD);
>>> + if ( rank == NULL )
>>> + goto read_as_zero;
>>> + vgic_lock_rank(v, rank, flags);
>>> + *r = vreg_reg32_extract(rank->ienable, info);
>>> + vgic_unlock_rank(v, rank, flags);
>>> + return 1;
>>> +
>>> + 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_ext_rank_offset(v, 8, reg - GICD_IPRIORITYRnE,
>>> DABT_WORD);
>>> + if ( rank == NULL )
>>> + goto read_as_zero;
>>> + rank_index = REG_RANK_INDEX(8, reg - GICD_IPRIORITYRnE,
>>> DABT_WORD);
>>> +
>>> + vgic_lock_rank(v, rank, flags);
>>> + ipriorityr = ACCESS_ONCE(rank->ipriorityr[rank_index]);
>>> + vgic_unlock_rank(v, rank, flags);
>>> +
>>> + *r = vreg_reg32_extract(ipriorityr, info);
>>> +
>>> + return 1;
>>> + }
>>> +
>>> + case VRANGE32(GICD_ICFGRnE, GICD_ICFGRnEN):
>>> + {
>>> + uint32_t icfgr;
>>> +
>>> + if ( dabt.size != DABT_WORD )
>>> + goto bad_width;
>>> + rank = vgic_ext_rank_offset(v, 2, reg - GICD_ICFGRnE,
>>> DABT_WORD);
>>> + if ( rank == NULL )
>>> + goto read_as_zero;
>>> + vgic_lock_rank(v, rank, flags);
>>> + icfgr = rank->icfg[REG_RANK_INDEX(2, reg - GICD_ICFGRnE,
>>> DABT_WORD)];
>>> + vgic_unlock_rank(v, rank, flags);
>>> +
>>> + *r = vreg_reg32_extract(icfgr, info);
>>> +
>>> + return 1;
>>> + }
>>> +#endif
>>> +
>>> default:
>>> printk(XENLOG_G_ERR
>>> "%pv: %s: unhandled read r%d offset %#08x\n",
>>> @@ -782,6 +853,9 @@ 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):
>>> +#ifdef CONFIG_GICV3_ESPI
>>> + case VRANGE32(GICD_IGROUPRnE, GICD_IGROUPRnEN):
>>> +#endif
>>> /* We do not implement security extensions for guests, write
>>> ignore */
>>> goto write_ignore_32;
>>> @@ -871,6 +945,99 @@ static int
>>> __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
>>> vgic_unlock_rank(v, rank, flags);
>>> return 1;
>>> +#ifdef CONFIG_GICV3_ESPI
>>> + case VRANGE32(GICD_ISENABLERnE, GICD_ISENABLERnEN):
>>> + if ( dabt.size != DABT_WORD )
>>> + goto bad_width;
>>> + rank = vgic_ext_rank_offset(v, 1, reg - GICD_ISENABLERnE,
>>> 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),
>>> EXT_RANK_IDX2NUM(rank->index));
>>> + vgic_unlock_rank(v, rank, flags);
>>> + return 1;
>>> +
>>> + case VRANGE32(GICD_ICENABLERnE, GICD_ICENABLERnEN):
>>> + if ( dabt.size != DABT_WORD )
>>> + goto bad_width;
>>> + rank = vgic_ext_rank_offset(v, 1, reg - GICD_ICENABLERnE,
>>> 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,
>>> EXT_RANK_IDX2NUM(rank->index));
>>> + vgic_unlock_rank(v, rank, flags);
>>> + return 1;
>>> +
>>> + case VRANGE32(GICD_ISPENDRnE, GICD_ISPENDRnEN):
>>> + if ( dabt.size != DABT_WORD )
>>> + goto bad_width;
>>> + rank = vgic_ext_rank_offset(v, 1, reg - GICD_ISPENDRnE,
>>> DABT_WORD);
>>> + if ( rank == NULL )
>>> + goto write_ignore;
>>> +
>>> + vgic_set_irqs_pending(v, r, EXT_RANK_IDX2NUM(rank->index));
>>> +
>>> + return 1;
>>> +
>>> + case VRANGE32(GICD_ICPENDRnE, GICD_ICPENDRnEN):
>>> + if ( dabt.size != DABT_WORD )
>>> + goto bad_width;
>>> + rank = vgic_ext_rank_offset(v, 1, reg - GICD_ICPENDRnE,
>>> DABT_WORD);
>>> + if ( rank == NULL )
>>> + goto write_ignore;
>>> +
>>> + vgic_check_inflight_irqs_pending(v, EXT_RANK_IDX2NUM(rank-
>>>> index), r);
>>> +
>>> + goto write_ignore;
>>> + 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;
>>
>> Guest write access to GICD_ISACTIVER<n>E will lead to abort. But, I know
>> you just repeated the logic for regular GICD_ISACTIVER<n>.
>>
>>
>
> Could you please clarify the scenario in which it leads to an abort?
> Since it is actually a stub case, it should just print an error message
> and that's it..
"return 0;" will lead to injecting a fault into the guest.
Do you mean that, in this case, we need to goto
> write_ignore_32 label instead of returning 0?
My comment was not a direct request to change anything, but rather
thinking out loud.
Unfortunally, I cannot answer why regular GICD_ISACTIVER<n> is emulated
in that way, but perhaps the injecting fault into the guest is the
lesser evil than emulating it incorrectly...
Interestingly, for GICv2 we have a slighly relaxed version; it looks
like writing 0 will not cause an abort and will be WI.
25f9e80201f3688e0c4d5c4e43e4b6143b441c52
xen/arm: Ignore write to GICD_ISACTIVERn registers (vgic-v2)
Now, with the introduction of extended GICD_ISACTIVER<n>E you retained
the logic. One thing that needs mentioning is that before your series,
guest write access to extended GICD_ISACTIVER<n>E would be WI, but
with your series and CONFIG_GICV3_ESPI=y it will be an abort even
if running on GIC3 HW where eSPI is not supported.
At least, I would mention that in the patch description.
>
>>> +
>>> + 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_ICACTIVER);
>>
>> s/GICD_ICACTIVER/GICD_ICACTIVERnE
>>
>>
>
> I will fix that in V4.
>
>>> + goto write_ignore_32;
[snip]
Hello Oleksandr,
Thank you for your explanation.
On 27.08.25 16:44, Oleksandr Tyshchenko wrote:
>
>
> On 27.08.25 14:13, Leonid Komarianskyi wrote:
>> Hello Oleksandr,
>
> Hello Leonid
>
>>
>> Thank you for your close review.
>>
>> On 26.08.25 22:57, Oleksandr Tyshchenko wrote:
>>>
>>>
>>> On 26.08.25 17:05, Leonid Komarianskyi wrote:
>>>
>>> Hello Leonid
>>>
>>>
>>>> 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>
>>>>
>>>> ---
>>>> Changes in V2:
>>>> - add missing rank index conversion for pending and inflight irqs
>>>>
>>>> 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
>>>> ---
>>>> xen/arch/arm/vgic-v3.c | 275 ++++++++++++++++++++++++++++++++++++
>>>> +++--
>>>> 1 file changed, 266 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
>>>> index 4369c55177..56c539bb1b 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,6 +682,9 @@ 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):
>>>> +#ifdef CONFIG_GICV3_ESPI
>>>> + case VRANGE32(GICD_IGROUPRnE, GICD_IGROUPRnEN):
>>>> +#endif
>>>> /* We do not implement security extensions for guests, read
>>>> zero */
>>>> if ( dabt.size != DABT_WORD ) goto bad_width;
>>>> goto read_as_zero;
>>>> @@ -710,11 +710,19 @@ 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):
>>>> +#ifdef CONFIG_GICV3_ESPI
>>>> + case VRANGE32(GICD_ISPENDRnE, GICD_ISPENDRnEN):
>>>> + case VRANGE32(GICD_ICPENDRnE, GICD_ICPENDRnEN):
>>>> +#endif
>>>> 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):
>>>> +#ifdef CONFIG_GICV3_ESPI
>>>> + case VRANGE32(GICD_ISACTIVERnE, GICD_ISACTIVERnEN):
>>>> + case VRANGE32(GICD_ICACTIVERnE, GICD_ICACTIVERnEN):
>>>> +#endif
>>>> goto read_as_zero;
>>>> case VRANGE32(GICD_IPRIORITYR, GICD_IPRIORITYRN):
>>>> @@ -752,6 +760,69 @@ static int __vgic_v3_distr_common_mmio_read(const
>>>> char *name, struct vcpu *v,
>>>> return 1;
>>>> }
>>>> +#ifdef CONFIG_GICV3_ESPI
>>>> + case VRANGE32(GICD_ISENABLERnE, GICD_ISENABLERnEN):
>>>> + if ( dabt.size != DABT_WORD )
>>>> + goto bad_width;
>>>> + rank = vgic_ext_rank_offset(v, 1, reg - GICD_ISENABLERnE,
>>>> DABT_WORD);
>>>> + if ( rank == NULL )
>>>> + goto read_as_zero;
>>>> + vgic_lock_rank(v, rank, flags);
>>>> + *r = vreg_reg32_extract(rank->ienable, info);
>>>> + vgic_unlock_rank(v, rank, flags);
>>>> + return 1;
>>>> +
>>>> + case VRANGE32(GICD_ICENABLERnE, GICD_ICENABLERnEN):
>>>> + if ( dabt.size != DABT_WORD )
>>>> + goto bad_width;
>>>> + rank = vgic_ext_rank_offset(v, 1, reg - GICD_ICENABLERnE,
>>>> DABT_WORD);
>>>> + if ( rank == NULL )
>>>> + goto read_as_zero;
>>>> + vgic_lock_rank(v, rank, flags);
>>>> + *r = vreg_reg32_extract(rank->ienable, info);
>>>> + vgic_unlock_rank(v, rank, flags);
>>>> + return 1;
>>>> +
>>>> + 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_ext_rank_offset(v, 8, reg - GICD_IPRIORITYRnE,
>>>> DABT_WORD);
>>>> + if ( rank == NULL )
>>>> + goto read_as_zero;
>>>> + rank_index = REG_RANK_INDEX(8, reg - GICD_IPRIORITYRnE,
>>>> DABT_WORD);
>>>> +
>>>> + vgic_lock_rank(v, rank, flags);
>>>> + ipriorityr = ACCESS_ONCE(rank->ipriorityr[rank_index]);
>>>> + vgic_unlock_rank(v, rank, flags);
>>>> +
>>>> + *r = vreg_reg32_extract(ipriorityr, info);
>>>> +
>>>> + return 1;
>>>> + }
>>>> +
>>>> + case VRANGE32(GICD_ICFGRnE, GICD_ICFGRnEN):
>>>> + {
>>>> + uint32_t icfgr;
>>>> +
>>>> + if ( dabt.size != DABT_WORD )
>>>> + goto bad_width;
>>>> + rank = vgic_ext_rank_offset(v, 2, reg - GICD_ICFGRnE,
>>>> DABT_WORD);
>>>> + if ( rank == NULL )
>>>> + goto read_as_zero;
>>>> + vgic_lock_rank(v, rank, flags);
>>>> + icfgr = rank->icfg[REG_RANK_INDEX(2, reg - GICD_ICFGRnE,
>>>> DABT_WORD)];
>>>> + vgic_unlock_rank(v, rank, flags);
>>>> +
>>>> + *r = vreg_reg32_extract(icfgr, info);
>>>> +
>>>> + return 1;
>>>> + }
>>>> +#endif
>>>> +
>>>> default:
>>>> printk(XENLOG_G_ERR
>>>> "%pv: %s: unhandled read r%d offset %#08x\n",
>>>> @@ -782,6 +853,9 @@ 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):
>>>> +#ifdef CONFIG_GICV3_ESPI
>>>> + case VRANGE32(GICD_IGROUPRnE, GICD_IGROUPRnEN):
>>>> +#endif
>>>> /* We do not implement security extensions for guests, write
>>>> ignore */
>>>> goto write_ignore_32;
>>>> @@ -871,6 +945,99 @@ static int
>>>> __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
>>>> vgic_unlock_rank(v, rank, flags);
>>>> return 1;
>>>> +#ifdef CONFIG_GICV3_ESPI
>>>> + case VRANGE32(GICD_ISENABLERnE, GICD_ISENABLERnEN):
>>>> + if ( dabt.size != DABT_WORD )
>>>> + goto bad_width;
>>>> + rank = vgic_ext_rank_offset(v, 1, reg - GICD_ISENABLERnE,
>>>> 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),
>>>> EXT_RANK_IDX2NUM(rank->index));
>>>> + vgic_unlock_rank(v, rank, flags);
>>>> + return 1;
>>>> +
>>>> + case VRANGE32(GICD_ICENABLERnE, GICD_ICENABLERnEN):
>>>> + if ( dabt.size != DABT_WORD )
>>>> + goto bad_width;
>>>> + rank = vgic_ext_rank_offset(v, 1, reg - GICD_ICENABLERnE,
>>>> 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,
>>>> EXT_RANK_IDX2NUM(rank->index));
>>>> + vgic_unlock_rank(v, rank, flags);
>>>> + return 1;
>>>> +
>>>> + case VRANGE32(GICD_ISPENDRnE, GICD_ISPENDRnEN):
>>>> + if ( dabt.size != DABT_WORD )
>>>> + goto bad_width;
>>>> + rank = vgic_ext_rank_offset(v, 1, reg - GICD_ISPENDRnE,
>>>> DABT_WORD);
>>>> + if ( rank == NULL )
>>>> + goto write_ignore;
>>>> +
>>>> + vgic_set_irqs_pending(v, r, EXT_RANK_IDX2NUM(rank->index));
>>>> +
>>>> + return 1;
>>>> +
>>>> + case VRANGE32(GICD_ICPENDRnE, GICD_ICPENDRnEN):
>>>> + if ( dabt.size != DABT_WORD )
>>>> + goto bad_width;
>>>> + rank = vgic_ext_rank_offset(v, 1, reg - GICD_ICPENDRnE,
>>>> DABT_WORD);
>>>> + if ( rank == NULL )
>>>> + goto write_ignore;
>>>> +
>>>> + vgic_check_inflight_irqs_pending(v, EXT_RANK_IDX2NUM(rank-
>>>>> index), r);
>>>> +
>>>> + goto write_ignore;
>>>> + 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;
>>>
>>> Guest write access to GICD_ISACTIVER<n>E will lead to abort. But, I know
>>> you just repeated the logic for regular GICD_ISACTIVER<n>.
>>>
>>>
>>
>> Could you please clarify the scenario in which it leads to an abort?
>> Since it is actually a stub case, it should just print an error message
>> and that's it..
>
> "return 0;" will lead to injecting a fault into the guest.
>
> Do you mean that, in this case, we need to goto
>> write_ignore_32 label instead of returning 0?
>
> My comment was not a direct request to change anything, but rather
> thinking out loud.
>
> Unfortunally, I cannot answer why regular GICD_ISACTIVER<n> is emulated
> in that way, but perhaps the injecting fault into the guest is the
> lesser evil than emulating it incorrectly...
>
> Interestingly, for GICv2 we have a slighly relaxed version; it looks
> like writing 0 will not cause an abort and will be WI.
> 25f9e80201f3688e0c4d5c4e43e4b6143b441c52
> xen/arm: Ignore write to GICD_ISACTIVERn registers (vgic-v2)
>
> Now, with the introduction of extended GICD_ISACTIVER<n>E you retained
> the logic. One thing that needs mentioning is that before your series,
> guest write access to extended GICD_ISACTIVER<n>E would be WI, but
> with your series and CONFIG_GICV3_ESPI=y it will be an abort even
> if running on GIC3 HW where eSPI is not supported.
>
> At least, I would mention that in the patch description.
>
>
This is really interesting..
I think it might be worth introducing one more eSPI-specific field for
vgic, such as a bool has_epsi (or by checking if nr_espis is non-zero).
In cases where the hardware does not support eSPIs (or eSPIs are not
assigned to the guest domain), but the eSPI config is enabled, we could
go to write_reserved for all eSPI-specific registers. This would
definitely work the same way it does now. Additionally, we could return
RAZ in the same case while reading eSPI-specific registers.
This can be done like this (for mmio_write):
case VRANGE32(GICD_ISENABLERnE, GICD_ISENABLERnEN):
if (v->domain->arch.vgic.nr_espi == 0)
goto write_ignore;
if ( dabt.size != DABT_WORD )
goto bad_width;
And for mmio_read:
case VRANGE32(GICD_ISENABLERnE, GICD_ISENABLERnEN):
if (v->domain->arch.vgic.nr_espi == 0)
goto read_as_zero;
if ( dabt.size != DABT_WORD )
goto bad_width;
(and for other eSPI registers, including GICD_ISACTIVER<n>E)
Also with has_epsi, it would be much easier:
if (!v->domain->arch.vgic.has_epsi)
....
What do you think about this?
>>
>>>> +
>>>> + 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_ICACTIVER);
>>>
>>> s/GICD_ICACTIVER/GICD_ICACTIVERnE
>>>
>>>
>>
>> I will fix that in V4.
>>
>>>> + goto write_ignore_32;
>
>
> [snip]
Best regards,
Leonid
© 2016 - 2026 Red Hat, Inc.