On 04/05/17 17:39, Julien Grall wrote:
Hi Andre,
On 04/05/17 16:31, Andre Przywara wrote:
Currently we store the priority for newly triggered IRQs in the rank
structure. To get eventually rid of this structure, move this value
into the struct pending_irq. This one already contains a priority value,
but we have to keep the two apart, as the priority for guest visible
IRQs must not change while they are in a VCPU.
This patch introduces a framework to get some per-IRQ information for a
number of interrupts and collate them into one register. Similarily
s/Similarily/Similarly/
there is the opposite function to spread values from one register into
multiple pending_irq's.
Also, the last paragraph is a call to split the patch in two:
1) Introducing the framework
2) Move priority from irq_rank to struct pending_irq
Signed-off-by: Andre Przywara <andre.przyw...@arm.com>
---
xen/arch/arm/vgic-v2.c | 33 +++++++++--------------------
xen/arch/arm/vgic-v3.c | 33 ++++++++++-------------------
xen/arch/arm/vgic.c | 53
++++++++++++++++++++++++++++++++++------------
xen/include/asm-arm/vgic.h | 17 ++++++---------
4 files changed, 67 insertions(+), 69 deletions(-)
diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index dc9f95b..5c59fb4 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -171,6 +171,7 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v,
mmio_info_t *info,
struct vgic_irq_rank *rank;
int gicd_reg = (int)(info->gpa - v->domain->arch.vgic.dbase);
unsigned long flags;
+ unsigned int irq;
s/irq/virq/
perfc_incr(vgicd_reads);
@@ -250,22 +251,10 @@ static int vgic_v2_distr_mmio_read(struct vcpu
*v, mmio_info_t *info,
goto read_as_zero;
case VRANGE32(GICD_IPRIORITYR, GICD_IPRIORITYRN):
- {
- uint32_t ipriorityr;
-
if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto
bad_width;
- rank = vgic_rank_offset(v, 8, gicd_reg - GICD_IPRIORITYR,
DABT_WORD);
- if ( rank == NULL ) goto read_as_zero;
-
- vgic_lock_rank(v, rank, flags);
- ipriorityr = rank->ipriorityr[REG_RANK_INDEX(8,
- gicd_reg -
GICD_IPRIORITYR,
- DABT_WORD)];
- vgic_unlock_rank(v, rank, flags);
- *r = vgic_reg32_extract(ipriorityr, info);
-
+ irq = gicd_reg - GICD_IPRIORITYR;
Actually there are an issue with this code. You don't handle correctly
byte access and vgic_reg32_extract expects the interrupt
IHMO, this kind of problem could be handled directly in
gather_irq_info_priority by passing the offset. See how we deal in
vgic_fetch_itarger.
The behavior of those functions should really be explained the commit
message.
Also I don't see any check to prevent reading interrupts outside of the
one supported by the vGIC for this domain.
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
