[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [RFC PATCH v2 07/22] ARM: vGIC: introduce priority setter/getter

On 16/08/17 17:48, Andre Przywara wrote:

On 11/08/17 15:10, Julien Grall wrote:
Hi Andre,

On 21/07/17 20:59, Andre Przywara wrote:
Since the GICs MMIO access always covers a number of IRQs at once,
introduce wrapper functions which loop over those IRQs, take their
locks and read or update the priority values.
This will be used in a later patch.

Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
 xen/arch/arm/vgic.c        | 37 +++++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/vgic.h |  5 +++++
 2 files changed, 42 insertions(+)

diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 434b7e2..b2c9632 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -243,6 +243,43 @@ static int vgic_get_virq_priority(struct vcpu *v,
unsigned int virq)
     return ACCESS_ONCE(rank->priority[virq & INTERRUPT_RANK_MASK]);


The name gives the impression that you may have IPRIORITYR with only 1
IRQ. But this is not true. The registers is always 4. However, you are
able to access using byte or word.

+uint32_t vgic_fetch_irq_priority(struct vcpu *v, unsigned int nrirqs,

I am well aware that the vgic code is mixing between virq and irq.
Moving forward, we should use virq to avoid confusion.

+                                 unsigned int first_irq)

Please stay consistent, with the naming. Either nr_irqs/first_irq or
nrirqs/firstirq. But not a mix.

I totally agree, but check this out:
xen/include/asm-arm/irq.h:#define nr_irqs NR_IRQS

So wherever you write nr_irqs in *any* part of ARM IRQ code you end up
with a compile error ...
Not easy to fix, though, hence I moved to the name without the
underscore, even though I don't really like it.

Oh. On a side note, nr_irqs does not cover all the IRQs. It only covers up to SPIs. Which is a little bit odd.

Anyway, maybe you would rename it to nr. I think it is fairly straight forward that you deal with IRQ.


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.