[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v4 12/17] xen/arm: ITS: Initialize LPI irq descriptors and route
Hi Vijay,
On 10/07/2015 09:42, vijay.kilari@xxxxxxxxx wrote:
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index e6004d2..53554e6 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -895,7 +895,7 @@ static void gicv3_update_lr(int lr, const struct
pending_irq *p,
val |= ((uint64_t)p->priority & 0xff) << GICH_LR_PRIORITY_SHIFT;
val |= ((uint64_t)p->irq & GICH_LR_VIRTUAL_MASK) << GICH_LR_VIRTUAL_SHIFT;
- if ( p->desc != NULL )
+ if ( p->desc != NULL && !(is_lpi(p->irq)) )
It seems that you replaced all the p->desc != NULL by "p->desc != NULL
&& !is_lpi(p->irq).
Why don't you avoid to set p->desc in this case?
You may also want to put some explanation in the commit message to
explain why you don't have to set the GICH_LR.HW bit for LPIs.
val |= GICH_LR_HW | (((uint64_t)p->desc->irq & GICH_LR_PHYSICAL_MASK)
<< GICH_LR_PHYSICAL_SHIFT);
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 3ebadcf..92d2be9 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -68,11 +68,18 @@ enum gic_version gic_hw_version(void)
return gic_hw_ops->info->hw_version;
}
+/* Only validates PPIs/SGIs/SPIs supported */
This comment seems wrong. The function doesn't validate the IRQ but
return the number of Lines (i.e PPIs/SGIs/SPIs).
unsigned int gic_number_lines(void)
{
return gic_hw_ops->info->nr_lines;
}
[...]
int gic_route_irq_to_guest(struct domain *d, unsigned int virq,
struct irq_desc *desc, unsigned int priority)
{
@@ -454,7 +472,7 @@ static void gic_update_one_lr(struct vcpu *v, int i)
if ( test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) &&
test_and_clear_bit(GIC_IRQ_GUEST_QUEUED, &p->status) )
{
- if ( p->desc == NULL )
+ if ( p->desc == NULL || is_lpi(irq) )
{
lr_val.state |= GICH_LR_PENDING;
gic_hw_ops->write_lr(i, &lr_val);
@@ -677,7 +695,7 @@ void gic_interrupt(struct cpu_user_regs *regs, int is_fiq)
/* Reading IRQ will ACK it */
irq = gic_hw_ops->read_irq();
- if ( likely(irq >= 16 && irq < 1020) )
+ if ( (likely(irq >= 16 && irq < 1020)) || is_lpi(irq) )
Please move the is_lpi(irq) in likely.
{
local_irq_enable();
do_IRQ(regs, irq, is_fiq);
[...]
@@ -208,7 +226,7 @@ int request_irq(unsigned int irq, unsigned int irqflags,
* which interrupt is which (messes up the interrupt freeing
* logic etc).
*/
- if ( irq >= nr_irqs )
+ if ( irq >= nr_irqs && !is_lpi(irq) )
Technically nr_irqs should contain the total number of IRQ and not only
the number of SPI/PPI/SGI.
Either modify nr_irqs or use gic_is_valid_irq which would do the same here.
return -EINVAL;
if ( !handler )
return -EINVAL;
@@ -267,9 +285,14 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq,
int is_fiq)
set_bit(_IRQ_INPROGRESS, &desc->status);
desc->arch.eoi_cpu = smp_processor_id();
+#ifdef CONFIG_ARM_64
+ if ( is_lpi(irq) )
+ vgic_vcpu_inject_lpi(info->d, irq);
+ else
+#endif
/* the irq cannot be a PPI, we only support delivery of SPIs to
* guests */
- vgic_vcpu_inject_spi(info->d, info->virq);
+ vgic_vcpu_inject_spi(info->d, info->virq);
goto out_no_end;
}
@@ -436,7 +459,8 @@ err:
bool_t is_assignable_irq(unsigned int irq)
{
/* For now, we can only route SPIs to the guest */
- return ((irq >= NR_LOCAL_IRQS) && (irq < gic_number_lines()));
+ return (((irq >= NR_LOCAL_IRQS) && (irq < gic_number_lines())) ||
+ is_lpi(irq));
If you modify the function, please also modify the comment which become
invalid now.
[...]
diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c
index bbcc7bb..4649b07 100644
--- a/xen/arch/arm/vgic-v3-its.c
+++ b/xen/arch/arm/vgic-v3-its.c
@@ -625,6 +625,15 @@ err:
return 0;
}
+uint8_t vgic_its_get_priority(struct vcpu *v, uint32_t pid)
+{
+ uint8_t priority;
+
+ priority = readb_relaxed(v->domain->arch.vits->prop_page + pid);
Why do you use readb_relaxed here? This should only be used for Device MMIO.
Although, you need to ensure that the value will be correctly
synchronize if another CPU is writing in prop_page which is protected by
prop_lock.
+ priority &= LPI_PRIORITY_MASK;
+
+ return priority;
+}
static int vgic_v3_gits_lpi_mmio_read(struct vcpu *v, mmio_info_t *info)
{
uint32_t offset;
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 69bf1ff..537ed3d 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -226,6 +226,9 @@ extern void gic_route_irq_to_xen(struct irq_desc *desc,
const cpumask_t *cpu_mas
extern int gic_route_irq_to_guest(struct domain *, unsigned int virq,
struct irq_desc *desc,
unsigned int priority);
+extern int gic_route_lpi_to_guest(struct domain *d, unsigned int virq,
+ struct irq_desc *desc,
+ unsigned int priority);
/* Remove an IRQ passthrough to a guest */
int gic_remove_irq_from_guest(struct domain *d, unsigned int virq,
@@ -282,8 +285,10 @@ extern void send_SGI_allbutself(enum gic_sgi sgi);
/* print useful debug info */
extern void gic_dump_info(struct vcpu *v);
-/* Number of interrupt lines */
+/* Number of interrupt lines (SPIs)*/
This is not really true. The interrupts lines is equals to PPIs + SGIs +
SPIs.
extern unsigned int gic_number_lines(void);
+/* Check if irq is valid SPI or LPI */
This comment is not true. This function is used to check that an IRQ is
valid in general, not only for SPI or LPI.
+bool_t gic_is_valid_irq(unsigned int irq);
/* Number of interrupt id bits supported */
extern unsigned int gic_nr_id_bits(void);
/* LPI support info */
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
