[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v4 05/17] xen/arm: ITS: implement hw_irq_controller for LPIs
Hi Vijay,
On 15/07/2015 09:16, Vijay Kilari wrote:
On Tue, Jul 14, 2015 at 2:48 AM, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
Hi,
On 10/07/2015 09:42, vijay.kilari@xxxxxxxxx wrote:
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index c41e82e..4f3801b 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
+static inline hw_irq_controller *get_host_hw_irq_controller(unsigned int
irq)
+{
+ if ( is_lpi(irq) )
+ return its_hw_ops->lpi_host_irq_type;
+ else
+ return gic_hw_ops->gic_host_irq_type;
+}
This is not what I asked on v3 [1]. The ITS hardware controller shouldn't be
exposed to the common GIC. We have to keep a clean and comprehensible
interface.
What I asked is to replace the gic_host_irq_type variable by a new callback
which will return the correct hw_irq_controller.
For GICv2, it will return the same hw_irq_controller as today. For GICv3, it
will check is the IRQ is an LPI and return the correct controller.
FWIW, it was "ack" by Ian [2].
If we don't want to expose any ITS interfaces to common gic code, then we
have to register callbacks to GICv3 driver.
Why? In fine, the ITS is an integral part of the GICv3, so you could
directly call the ITS code within the GICv3 without any callback.
Actually, you already do that in some place. So I don't see why you
can't do it there...
@@ -149,7 +173,7 @@ int gic_route_irq_to_guest(struct domain *d, unsigned
int virq,
test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) )
goto out;
- desc->handler = gic_hw_ops->gic_guest_irq_type;
+ desc->handler = get_guest_hw_irq_controller(desc->irq);
set_bit(_IRQ_GUEST, &desc->status);
gic_set_irq_properties(desc, cpumask_of(v_target->processor),
priority);
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 2dd43ee..ba8528a 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -35,7 +35,13 @@ static DEFINE_SPINLOCK(local_irqs_type_lock);
struct irq_guest
{
struct domain *d;
- unsigned int virq;
+ union
+ {
+ /* virq refer to virtual irq in case of spi */
+ unsigned int virq;
+ /* virq refer to event ID in case of lpi */
+ unsigned int vid;
Why can't we store the event ID in the irq_guest? As said on v3, this is not
Are you referring to irq_desc in above statement?
Yes sorry.
domain specific [3]. Furthermore, you add support to route LPI in Xen (see
gic_route_irq_to_xen) where you will clearly need the event ID.
void irq_set_affinity(struct irq_desc *desc, const cpumask_t *cpu_mask)
{
if ( desc != NULL )
diff --git a/xen/include/asm-arm/gic-its.h b/xen/include/asm-arm/gic-its.h
index b5e09bd..e8d244f 100644
--- a/xen/include/asm-arm/gic-its.h
+++ b/xen/include/asm-arm/gic-its.h
@@ -161,6 +161,10 @@ typedef union {
* The ITS view of a device.
*/
struct its_device {
+ /* Physical ITS */
+ struct its_node *its;
+ /* Number of Physical LPIs assigned */
+ int nr_lpis;
Why didn't you add this field directly in the patch #4? It would be more
logical.
/*
* ITS registers, offsets from ITS_base
diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h
index 34b492b..55e219f 100644
--- a/xen/include/asm-arm/irq.h
+++ b/xen/include/asm-arm/irq.h
@@ -17,6 +17,8 @@ struct arch_pirq
struct arch_irq_desc {
int eoi_cpu;
unsigned int type;
+ struct its_device *dev;
+ u16 col_id;
It has been suggested by Ian to move col_id in the its_device in the
previous version [4]. Any reason to not doing it?
In round robin fashion each plpi is attached to col_id. So storing
in its_device is not possible. In linux latest col_id is stored in its_device
structure for which set_affinity is called.
You could do round robin on its_device... It would be exactly the same
and save 2 byte if not more with the alignment per irq_desc.
Don't forget that 1 byte in the irq_desc means 1KB added in Xen binary.
These bytes saved could be used to store the event ID.
That remind me, these 2 new fields should only be defined when GICv3 is
used (#ifdef HAS_GICV3).
I'm would be fine if you skip the former for 4.6, but the latter is
mandatory. ITS code shouldn't be compiled on arm32.
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
