[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 6/8] xen: arm: supporting routing a PPI to the current vcpu.
Hi Ian, On 10/11/15 16:21, Ian Campbell wrote: > That is whichever vcpu is resident when the interrupt fires. An > interrupt is in this state when both IRQ_GUEST and IRQ_PER_CPU are set > in the descriptor status. Only PPIs can be in this mode. > > This requires some peripheral specific code to make use of the > previously introduced functionality to save and restore the PPI state. > The vtimer driver will do so shortly. > > Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx> > --- > xen/arch/arm/gic.c | 24 ++++++++++++++ > xen/arch/arm/irq.c | 84 > +++++++++++++++++++++++++++++++++++++++++------ > xen/include/asm-arm/gic.h | 2 ++ > xen/include/asm-arm/irq.h | 2 +- > 4 files changed, 101 insertions(+), 11 deletions(-) > > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > index e294e89..aea913b 100644 > --- a/xen/arch/arm/gic.c > +++ b/xen/arch/arm/gic.c > @@ -177,6 +177,30 @@ void gic_route_irq_to_xen(struct irq_desc *desc, const > cpumask_t *cpu_mask, > gic_set_irq_properties(desc, cpu_mask, priority); > } > > +/* Program the GIC to route an interrupt to the current guest /* * Program ... And missing full stop. > + * > + * That is the IRQ is delivered to whichever VCPU happens to be > + * resident on the PCPU when the interrupt arrives. s/That is// ? > + * > + * Currently the interrupt *must* be a PPI and the code responsible > + * for the related hardware must save and restore the PPI with > + * gic_save_and_mask_hwppi/gic_restore_hwppi. > + */ > +int gic_route_irq_to_current_guest(struct irq_desc *desc, > + unsigned int priority) > +{ > + ASSERT(spin_is_locked(&desc->lock)); > + ASSERT(desc->irq >= 16 && desc->irq < 32); > + > + desc->handler = gic_hw_ops->gic_guest_irq_type; > + set_bit(_IRQ_GUEST, &desc->status); > + set_bit(_IRQ_PER_CPU, &desc->status); > + > + gic_set_irq_properties(desc, cpumask_of(smp_processor_id()), > GIC_PRI_IRQ); > + > + return 0; > +} > + > /* Program the GIC to route an interrupt to a guest > * - desc.lock must be held > */ > diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c > index 95be10e..ca18403 100644 > --- a/xen/arch/arm/irq.c > +++ b/xen/arch/arm/irq.c > @@ -229,12 +229,15 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int > irq, int is_fiq) > set_bit(_IRQ_INPROGRESS, &desc->status); > > /* > - * The irq cannot be a PPI, we only support delivery of SPIs to > - * guests. > + * A PPI exposed to a guest must always be in IRQ_GUEST|IRQ_PER_CPU > + * mode ("route to active VCPU"), so we use current. > + * > + * For SPI we look up the required target as normal. I would mention that the SPI are delivered to a specific guest. > */ > - v = vgic_get_target_vcpu(info->d->vcpu[0], info->virq); > - vgic_vcpu_inject_irq(v, info->virq); > + v = test_bit(_IRQ_PER_CPU, &desc->status) ? current : > + vgic_get_target_vcpu(info->d->vcpu[0], info->virq); > > + vgic_vcpu_inject_irq(v, info->virq); Nit: Newline > goto out_no_end; > } > > @@ -363,11 +366,15 @@ int setup_irq(unsigned int irq, unsigned int irqflags, > struct irqaction *new) > > if ( test_bit(_IRQ_GUEST, &desc->status) ) > { > - struct domain *d = irq_get_domain(desc); > + struct irq_guest *info = irq_get_guest_info(desc); > > spin_unlock_irqrestore(&desc->lock, flags); > - printk(XENLOG_ERR "ERROR: IRQ %u is already in use by the domain > %u\n", > - irq, d->domain_id); > + if ( !test_bit(_IRQ_PER_CPU, &desc->status) ) > + printk(XENLOG_ERR "ERROR: IRQ %u is already in use by domain > %u\n", > + irq, info->d->domain_id); > + else > + printk(XENLOG_ERR > + "ERROR: IRQ %u is already in use by <current-vcpu>\n", > irq); AFAICT this function will be called one time per physical CPU. Could we print something to differentiate error on CPU0 to CPU1? > return -EBUSY; > } > > @@ -410,7 +417,7 @@ static int setup_guest_irq(struct irq_desc *desc, > unsigned int virq, > { > const unsigned irq = desc->irq; > struct irqaction *action; > - int retval = 0; > + int retval; > > ASSERT(spin_is_locked(&desc->lock)); > > @@ -451,12 +458,21 @@ static int setup_guest_irq(struct irq_desc *desc, > unsigned int virq, > d->domain_id, irq, irq_get_guest_info(desc)->virq); > retval = -EBUSY; > } > + else > + retval = 0; I don't see why this change is necessary here. Shouldn't it belong to patch #3? > goto out; > } > > if ( test_bit(_IRQ_GUEST, &desc->status) ) > - printk(XENLOG_G_ERR "IRQ %u is already used by domain %u\n", > - irq, ad->domain_id); > + { > + if ( !test_bit(_IRQ_PER_CPU, &desc->status) ) > + printk(XENLOG_ERR You switch the printk level from XENLOG_G_ERR to XENLOG_ERR. Is it wanted? > + "ERROR: IRQ %u is already used by domain %u\n", > + irq, ad->domain_id); > + else > + printk(XENLOG_ERR > + "ERROR: IRQ %u is already used by <current-vcpu>\n", > irq); Same remark as setup_irq for the error message. > + } > else > printk(XENLOG_G_ERR "IRQ %u is already used by Xen\n", irq); > retval = -EBUSY; > @@ -542,6 +558,54 @@ free_info: > return retval; > } > > +/* > + * Route a PPI such that it is always delivered to the current vcpu on > + * the pcpu. The driver for the peripheral must use > + * gic_{save_and_mask,restore}_hwppi as part of the context switch. > + */ > +int route_hwppi_to_current_vcpu(unsigned int irq, const char *devname) > +{ > + struct irq_guest *info; > + struct irq_desc *desc; > + unsigned long flags; > + int retval = 0; > + > + /* Can only route PPIs to current VCPU */ > + if ( irq < 16 || irq >= 32 ) > + return -EINVAL; > + > + desc = irq_to_desc(irq); > + > + info = xmalloc(struct irq_guest); > + if ( !info ) > + return -ENOMEM; > + > + info->d = NULL; /* Routed to current vcpu, so no specific domain */ > + /* info->virq is set by gic_restore_hwppi. */ > + > + spin_lock_irqsave(&desc->lock, flags); > + > + retval = setup_guest_irq(desc, irq, flags, info, devname); Why do you set the parameter virq to irq? > + if ( retval ) > + { > + xfree(info); > + return retval; > + } > + > + retval = gic_route_irq_to_current_guest(desc, GIC_PRI_IRQ); > + > + spin_unlock_irqrestore(&desc->lock, flags); > + > + if ( retval ) > + { > + release_irq(desc->irq, info); > + xfree(info); > + return retval; > + } > + > + return 0; > +} > + Regards, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |