Re: [Xen-devel] [RFC PATCH v2 14/22] ARM: vGIC: move virtual IRQ configuration from rank to pending_irq

2017-08-16 Thread Julien Grall 

Hi Andre,

On 21/07/17 21:00, Andre Przywara wrote:

The IRQ configuration (level or edge triggered) for a group of IRQs
are still stored in the irq_rank structure.
Introduce a new bit called GIC_IRQ_GUEST_LEVEL in the "status" field,
which holds that information.
Remove the storage from the irq_rank and use the existing wrappers to
store and retrieve the configuration bit for multiple IRQs.

Signed-off-by: Andre Przywara 
---
 xen/arch/arm/vgic-v2.c | 21 +++-
 xen/arch/arm/vgic-v3.c | 25 --
 xen/arch/arm/vgic.c| 81 +-
 xen/include/asm-arm/vgic.h |  5 ++-
 4 files changed, 73 insertions(+), 59 deletions(-)

diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index a3fd500..0c8a598 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -278,20 +278,12 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, 
mmio_info_t *info,
 goto read_reserved;

 case VRANGE32(GICD_ICFGR, GICD_ICFGRN):
-{
-uint32_t icfgr;
-
 if ( dabt.size != DABT_WORD ) goto bad_width;
-rank = vgic_rank_offset(v, 2, gicd_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, gicd_reg - GICD_ICFGR, 
DABT_WORD)];
-vgic_unlock_rank(v, rank, flags);

-*r = vreg_reg32_extract(icfgr, info);
+irq = (gicd_reg - GICD_ICFGR) * 4;
+*r = vgic_fetch_irq_config(v, irq);

 return 1;
-}

 case VRANGE32(0xD00, 0xDFC):
 goto read_impl_defined;
@@ -529,13 +521,8 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, 
mmio_info_t *info,

 case VRANGE32(GICD_ICFGR2, GICD_ICFGRN): /* SPIs */
 if ( dabt.size != DABT_WORD ) goto bad_width;
-rank = vgic_rank_offset(v, 2, gicd_reg - GICD_ICFGR, DABT_WORD);
-if ( rank == NULL) goto write_ignore;
-vgic_lock_rank(v, rank, flags);
-vreg_reg32_update(>icfg[REG_RANK_INDEX(2, gicd_reg - GICD_ICFGR,
- DABT_WORD)],
-  r, info);
-vgic_unlock_rank(v, rank, flags);
+irq = (gicd_reg - GICD_ICFGR) * 4; /* 2 bit per IRQ */


s/bit/bits/


+vgic_store_irq_config(v, irq, r);
 return 1;

 case VRANGE32(0xD00, 0xDFC):
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index d3356ae..e9e36eb 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -722,20 +722,11 @@ static int __vgic_v3_distr_common_mmio_read(const char 
*name, struct vcpu *v,
 return 1;

 case VRANGE32(GICD_ICFGR, GICD_ICFGRN):
-{
-uint32_t icfgr;
-
 if ( dabt.size != DABT_WORD ) goto bad_width;
-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)];
-vgic_unlock_rank(v, rank, flags);
-
-*r = vreg_reg32_extract(icfgr, info);
-
+irq = (reg - GICD_ICFGR) * 4;
+if ( irq >= v->domain->arch.vgic.nr_spis + 32 ) goto read_as_zero;
+*r = vgic_fetch_irq_config(v, irq);
 return 1;
-}

 default:
 printk(XENLOG_G_ERR
@@ -834,13 +825,9 @@ static int __vgic_v3_distr_common_mmio_write(const char 
*name, struct vcpu *v,
 /* 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 ( rank == NULL ) goto write_ignore;
-vgic_lock_rank(v, rank, flags);
-vreg_reg32_update(>icfg[REG_RANK_INDEX(2, reg - GICD_ICFGR,
- DABT_WORD)],
-  r, info);
-vgic_unlock_rank(v, rank, flags);
+irq = (reg - GICD_ICFGR) * 4;
+if ( irq >= v->domain->arch.vgic.nr_spis + 32 ) goto write_ignore;
+vgic_store_irq_config(v, irq, r);
 return 1;

 default:
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index ddcd99b..e5a4765 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -268,6 +268,55 @@ void vgic_store_irq_priority(struct vcpu *v, unsigned int 
nrirqs,
 local_irq_restore(flags);
 }

+#define IRQS_PER_CFGR   16
+/**
+ * vgic_fetch_irq_config: assemble the configuration bits for a group of 16 
IRQs
+ * @v: the VCPU for private IRQs, any VCPU of a domain for SPIs
+ * @first_irq: the first IRQ to be queried, must be aligned to 16
+ */
+uint32_t vgic_fetch_irq_config(struct vcpu *v, unsigned int first_irq)
+{
+struct pending_irq *pirqs[IRQS_PER_CFGR];
+unsigned long flags;
+uint32_t ret = 0, i;
+
+local_irq_save(flags);
+vgic_lock_irqs(v, IRQS_PER_CFGR, first_irq,

[Xen-devel] [RFC PATCH v2 14/22] ARM: vGIC: move virtual IRQ configuration from rank to pending_irq

2017-07-21 Thread Andre Przywara 
The IRQ configuration (level or edge triggered) for a group of IRQs
are still stored in the irq_rank structure.
Introduce a new bit called GIC_IRQ_GUEST_LEVEL in the "status" field,
which holds that information.
Remove the storage from the irq_rank and use the existing wrappers to
store and retrieve the configuration bit for multiple IRQs.

Signed-off-by: Andre Przywara 
---
 xen/arch/arm/vgic-v2.c | 21 +++-
 xen/arch/arm/vgic-v3.c | 25 --
 xen/arch/arm/vgic.c| 81 +-
 xen/include/asm-arm/vgic.h |  5 ++-
 4 files changed, 73 insertions(+), 59 deletions(-)

diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index a3fd500..0c8a598 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -278,20 +278,12 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, 
mmio_info_t *info,
 goto read_reserved;
 
 case VRANGE32(GICD_ICFGR, GICD_ICFGRN):
-{
-uint32_t icfgr;
-
 if ( dabt.size != DABT_WORD ) goto bad_width;
-rank = vgic_rank_offset(v, 2, gicd_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, gicd_reg - GICD_ICFGR, 
DABT_WORD)];
-vgic_unlock_rank(v, rank, flags);
 
-*r = vreg_reg32_extract(icfgr, info);
+irq = (gicd_reg - GICD_ICFGR) * 4;
+*r = vgic_fetch_irq_config(v, irq);
 
 return 1;
-}
 
 case VRANGE32(0xD00, 0xDFC):
 goto read_impl_defined;
@@ -529,13 +521,8 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, 
mmio_info_t *info,
 
 case VRANGE32(GICD_ICFGR2, GICD_ICFGRN): /* SPIs */
 if ( dabt.size != DABT_WORD ) goto bad_width;
-rank = vgic_rank_offset(v, 2, gicd_reg - GICD_ICFGR, DABT_WORD);
-if ( rank == NULL) goto write_ignore;
-vgic_lock_rank(v, rank, flags);
-vreg_reg32_update(>icfg[REG_RANK_INDEX(2, gicd_reg - GICD_ICFGR,
- DABT_WORD)],
-  r, info);
-vgic_unlock_rank(v, rank, flags);
+irq = (gicd_reg - GICD_ICFGR) * 4; /* 2 bit per IRQ */
+vgic_store_irq_config(v, irq, r);
 return 1;
 
 case VRANGE32(0xD00, 0xDFC):
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index d3356ae..e9e36eb 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -722,20 +722,11 @@ static int __vgic_v3_distr_common_mmio_read(const char 
*name, struct vcpu *v,
 return 1;
 
 case VRANGE32(GICD_ICFGR, GICD_ICFGRN):
-{
-uint32_t icfgr;
-
 if ( dabt.size != DABT_WORD ) goto bad_width;
-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)];
-vgic_unlock_rank(v, rank, flags);
-
-*r = vreg_reg32_extract(icfgr, info);
-
+irq = (reg - GICD_ICFGR) * 4;
+if ( irq >= v->domain->arch.vgic.nr_spis + 32 ) goto read_as_zero;
+*r = vgic_fetch_irq_config(v, irq);
 return 1;
-}
 
 default:
 printk(XENLOG_G_ERR
@@ -834,13 +825,9 @@ static int __vgic_v3_distr_common_mmio_write(const char 
*name, struct vcpu *v,
 /* 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 ( rank == NULL ) goto write_ignore;
-vgic_lock_rank(v, rank, flags);
-vreg_reg32_update(>icfg[REG_RANK_INDEX(2, reg - GICD_ICFGR,
- DABT_WORD)],
-  r, info);
-vgic_unlock_rank(v, rank, flags);
+irq = (reg - GICD_ICFGR) * 4;
+if ( irq >= v->domain->arch.vgic.nr_spis + 32 ) goto write_ignore;
+vgic_store_irq_config(v, irq, r);
 return 1;
 
 default:
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index ddcd99b..e5a4765 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -268,6 +268,55 @@ void vgic_store_irq_priority(struct vcpu *v, unsigned int 
nrirqs,
 local_irq_restore(flags);
 }
 
+#define IRQS_PER_CFGR   16
+/**
+ * vgic_fetch_irq_config: assemble the configuration bits for a group of 16 
IRQs
+ * @v: the VCPU for private IRQs, any VCPU of a domain for SPIs
+ * @first_irq: the first IRQ to be queried, must be aligned to 16
+ */
+uint32_t vgic_fetch_irq_config(struct vcpu *v, unsigned int first_irq)
+{
+struct pending_irq *pirqs[IRQS_PER_CFGR];
+unsigned long flags;
+uint32_t ret = 0, i;
+
+local_irq_save(flags);
+vgic_lock_irqs(v, IRQS_PER_CFGR, first_irq, pirqs);
+
+for ( i = 0; i < IRQS_PER_CFGR; i++ )
+if