[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [RFC PATCH 06/10] ARM: vGIC: move config from irq_rank to struct pending_irq
Hi Andre,
On 04/05/17 16:31, Andre Przywara wrote:
Currently we store the interrupt type configuration (level or edge
triggered) in the rank structure. Move this value into struct pending_irq.
We just need one bit (edge or level), so use one of the status bits.
Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
---
xen/arch/arm/vgic-v2.c | 29 ++++++++++-------------------
xen/arch/arm/vgic-v3.c | 31 ++++++++++++-------------------
xen/arch/arm/vgic.c | 39 ++++++++++++++++++++-------------------
xen/include/asm-arm/vgic.h | 7 ++++++-
4 files changed, 48 insertions(+), 58 deletions(-)
diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index 5c59fb4..795173c 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -278,20 +278,10 @@ 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 = vgic_reg32_extract(icfgr, info);
-
+ irq = (gicd_reg - GICD_ICFGR) * 4;
This would be nicer to have that handle directly in
gather_irq_info_config rather than open-coding everywhere. Also it
avoids this confusion of what irq actually meaning in this context.
I was also expecting some check on whether the interrupts is valid for
the vGIC.
+ *r = vgic_reg32_extract(gather_irq_info_config(v, irq), info);
return 1;
- }
case VRANGE32(0xD00, 0xDFC):
goto read_impl_defined;
@@ -534,15 +524,16 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v,
mmio_info_t *info,
goto write_ignore_32;
case VRANGE32(GICD_ICFGR2, GICD_ICFGRN): /* SPIs */
+ {
+ 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 write_ignore;
- vgic_lock_rank(v, rank, flags);
- vgic_reg32_update(&rank->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;
Ditto for the irq and the check.
+ icfgr = gather_irq_info_config(v, irq);
+ vgic_reg32_update(&icfgr, r, info);
+ scatter_irq_info_config(v, irq, icfgr);
return 1;
+ }
case VRANGE32(0xD00, 0xDFC):
goto write_impl_defined;
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 10db939..7989989 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -521,20 +521,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 = vgic_reg32_extract(icfgr, info);
-
+ irq = (reg - GICD_IPRIORITYR) * 4;
+ if ( irq >= v->domain->arch.vgic.nr_spis + 32 ) goto read_as_zero;
This should really be an helper.
+ *r = vgic_reg32_extract(gather_irq_info_config(v, irq), info);
return 1;
- }
default:
printk(XENLOG_G_ERR
@@ -636,17 +627,19 @@ 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 */
+ {
+ uint32_t icfgr;
+
/* 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);
- vgic_reg32_update(&rank->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;
+ icfgr = gather_irq_info_config(v, irq);
+ vgic_reg32_update(&icfgr, r, info);
+ scatter_irq_info_config(v, irq, icfgr);
return 1;
+ }
default:
printk(XENLOG_G_ERR
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 68eef47..02c1d12 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -235,6 +235,19 @@ static void set_priority(struct pending_irq *p, uint8_t
prio)
p->new_priority = prio;
}
+unsigned int extract_config(struct pending_irq *p)
Why this is exported? Also naming.
+{
+ return test_bit(GIC_IRQ_GUEST_EDGE, &p->status) ? 2 : 0;
Please document 0, 2. Likely with a some define.
+}
+
+static void set_config(struct pending_irq *p, unsigned int config)
Naming.
+{
+ if ( config < 2 )
Ditto.
+ clear_bit(GIC_IRQ_GUEST_EDGE, &p->status);
+ else
+ set_bit(GIC_IRQ_GUEST_EDGE, &p->status);
+}
+
#define DEFINE_GATHER_IRQ_INFO(name, get_val, shift) \
uint32_t gather_irq_info_##name(struct vcpu *v, unsigned int irq) \
@@ -267,6 +280,8 @@ void scatter_irq_info_##name(struct vcpu *v, unsigned int
irq, \
/* grep fodder: gather_irq_info_priority, scatter_irq_info_priority below */
DEFINE_GATHER_IRQ_INFO(priority, extract_priority, 8)
DEFINE_SCATTER_IRQ_INFO(priority, set_priority, 8)
+DEFINE_GATHER_IRQ_INFO(config, extract_config, 2)
+DEFINE_SCATTER_IRQ_INFO(config, set_config, 2)
bool vgic_migrate_irq(struct vcpu *old, struct vcpu *new, unsigned int irq)
{
@@ -357,27 +372,11 @@ void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n)
}
}
-#define VGIC_ICFG_MASK(intr) (1 << ((2 * ((intr) % 16)) + 1))
-
-/* The function should be called with the rank lock taken */
-static inline unsigned int vgic_get_virq_type(struct vcpu *v, int n, int index)
-{
- struct vgic_irq_rank *r = vgic_get_rank(v, n);
- uint32_t tr = r->icfg[index >> 4];
-
- ASSERT(spin_is_locked(&r->lock));
-
- if ( tr & VGIC_ICFG_MASK(index) )
- return IRQ_TYPE_EDGE_RISING;
- else
- return IRQ_TYPE_LEVEL_HIGH;
-}
-
void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
{
const unsigned long mask = r;
struct pending_irq *p;
- unsigned int irq;
+ unsigned int irq, int_type;
unsigned long flags;
int i = 0;
struct vcpu *v_target;
@@ -392,6 +391,8 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
spin_lock(&p->lock);
set_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
+ int_type = test_bit(GIC_IRQ_GUEST_EDGE, &p->status) ?
+ IRQ_TYPE_EDGE_RISING : IRQ_TYPE_LEVEL_HIGH;
if ( !list_empty(&p->inflight) && !test_bit(GIC_IRQ_GUEST_VISIBLE,
&p->status) )
gic_raise_guest_irq(v_target, p);
@@ -399,15 +400,15 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
spin_unlock_irqrestore(&v_target->arch.vgic.lock, flags);
if ( p->desc != NULL )
{
- irq_set_affinity(p->desc, cpumask_of(v_target->processor));
spin_lock_irqsave(&p->desc->lock, flags);
+ irq_set_affinity(p->desc, cpumask_of(v_target->processor));
/*
* The irq cannot be a PPI, we only support delivery of SPIs
* to guests.
*/
ASSERT(irq >= 32);
if ( irq_type_set_by_domain(d) )
- gic_set_irq_type(p->desc, vgic_get_virq_type(v, n, i));
+ gic_set_irq_type(p->desc, int_type);
p->desc->handler->enable(p->desc);
spin_unlock_irqrestore(&p->desc->lock, flags);
}
diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
index 38a5e76..931a672 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -60,12 +60,15 @@ struct pending_irq
* vcpu while it is still inflight and on an GICH_LR register on the
* old vcpu.
*
+ * GIC_IRQ_GUEST_EDGE: the IRQ is an edge triggered interrupt.
Also, explain that by default the interrupt will be level.
+ *
*/
#define GIC_IRQ_GUEST_QUEUED 0
#define GIC_IRQ_GUEST_ACTIVE 1
#define GIC_IRQ_GUEST_VISIBLE 2
#define GIC_IRQ_GUEST_ENABLED 3
#define GIC_IRQ_GUEST_MIGRATING 4
+#define GIC_IRQ_GUEST_EDGE 5
unsigned long status;
struct irq_desc *desc; /* only set it the irq corresponds to a physical
irq */
unsigned int irq;
@@ -102,7 +105,6 @@ struct vgic_irq_rank {
uint8_t index;
uint32_t ienable;
- uint32_t icfg[2];
/*
* It's more convenient to store a target VCPU per vIRQ
@@ -173,6 +175,9 @@ static inline int REG_RANK_NR(int b, uint32_t n)
uint32_t gather_irq_info_priority(struct vcpu *v, unsigned int irq);
void scatter_irq_info_priority(struct vcpu *v, unsigned int irq,
unsigned int value);
+uint32_t gather_irq_info_config(struct vcpu *v, unsigned int irq);
+void scatter_irq_info_config(struct vcpu *v, unsigned int irq,
+ unsigned int value);
#define VGIC_REG_MASK(size) ((~0UL) >> (BITS_PER_LONG - ((1 << (size)) * 8)))
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
