[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 5/8] xen: arm: add interfaces to save/restore the state of a PPI.
Hi Ian, On 10/11/15 16:21, Ian Campbell wrote: > Make use of the GICD I[SC]ACTIVER registers to save and > restore the active state of the interrupt. > > For edge triggered interrupts we also need to context switch the > pending bit via I[SC]PENDR. Note that for level triggered interrupts > SPENDR sets a latch which is only cleared by ICPENDR (and not by h/w > state changes), therefore we do not want to context switch the pending > state for level PPIs -- instead we rely on the context switch of the > peripheral to restore the correct level. > > Tested on GIC v2 only. I will give a try on the foundation model with GICv3. > Unused as yet, will be used by the vtimer code shortly. > > Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx> > --- > xen/arch/arm/gic-v2.c | 67 > ++++++++++++++++++++++++++++++++++++++++++++ > xen/arch/arm/gic-v3.c | 67 > ++++++++++++++++++++++++++++++++++++++++++++ > xen/arch/arm/gic.c | 54 +++++++++++++++++++++++++++++++++++ > xen/arch/arm/irq.c | 7 +++++ > xen/include/asm-arm/domain.h | 11 ++++++++ > xen/include/asm-arm/gic.h | 16 +++++++++++ > xen/include/asm-arm/irq.h | 3 ++ > 7 files changed, 225 insertions(+) > > diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c > index 01e36b5..5308c35 100644 > --- a/xen/arch/arm/gic-v2.c > +++ b/xen/arch/arm/gic-v2.c > @@ -81,6 +81,9 @@ static DEFINE_PER_CPU(u8, gic_cpu_id); > /* Maximum cpu interface per GIC */ > #define NR_GIC_CPU_IF 8 > > +static void gicv2_irq_enable(struct irq_desc *desc); > +static void gicv2_irq_disable(struct irq_desc *desc); > + > static inline void writeb_gicd(uint8_t val, unsigned int offset) > { > writeb_relaxed(val, gicv2.map_dbase + offset); > @@ -149,6 +152,37 @@ static void gicv2_save_state(struct vcpu *v) > writel_gich(0, GICH_HCR); > } > > +static void gicv2_save_and_mask_hwppi(struct irq_desc *desc, > + struct hwppi_state *s) > +{ > + const unsigned int mask = (1u << desc->irq); I would add a comment to explain that PPI are always below 31. Otherwise this construction would be invalid. > + const unsigned int pendingr = readl_gicd(GICD_ISPENDR); > + const unsigned int activer = readl_gicd(GICD_ISACTIVER); > + const unsigned int enabler = readl_gicd(GICD_ISENABLER); pendingr, activer, enabler are 32-bit register. Please use uint32_t here to make it clear. > + const bool is_edge = !!(desc->arch.type & DT_IRQ_TYPE_EDGE_BOTH); > + > + s->active = !!(activer & mask); > + s->enabled = !!(enabler & mask); > + s->pending = !!(pendingr & mask); > + > + /* Write a 1 to IC...R to clear the corresponding bit of state */ > + if ( s->active ) > + writel_gicd(mask, GICD_ICACTIVER); NIT: Newline here for clarity. > + /* > + * For an edge interrupt clear the pending state, for a level interrupt > + * this clears the latch there is no need since saving the peripheral > state > + * (and/or restoring the next VCPU) will cause the correct action. > + */ > + if ( is_edge && s->pending ) > + writel_gicd(mask, GICD_ICPENDR); > + > + if ( s->enabled ) > + gicv2_irq_disable(desc); Don't we want to disable the IRQ first and then saving the state? > + > + ASSERT(!(readl_gicd(GICD_ISACTIVER) & mask)); > + ASSERT(!(readl_gicd(GICD_ISENABLER) & mask)); > +} > + > static void gicv2_restore_state(const struct vcpu *v) > { > int i; > @@ -161,6 +195,37 @@ static void gicv2_restore_state(const struct vcpu *v) > writel_gich(GICH_HCR_EN, GICH_HCR); > } > > +static void gicv2_restore_hwppi(struct irq_desc *desc, > + const struct hwppi_state *s) > +{ > + const unsigned int mask = (1u << desc->irq); > + const bool is_edge = !!(desc->arch.type & DT_IRQ_TYPE_EDGE_BOTH); > + > + /* > + * The IRQ must always have been set inactive and masked etc by > + * the saving of the previous state via save_and_mask_hwppi. > + */ > + ASSERT(!(readl_gicd(GICD_ISACTIVER) & mask)); > + ASSERT(!(readl_gicd(GICD_ISENABLER) & mask)); > + > + if ( s->active ) > + writel_gicd(mask, GICD_ICACTIVER); > + > + /* > + * Restore pending state for edge triggered interrupts only. For > + * level triggered interrupts the level will be restored as > + * necessary by restoring the state of the relevant peripheral. > + * > + * For a level triggered interrupt ISPENDR acts as a *latch* which > + * is only cleared by ICPENDR (i.e. the input level is no longer > + * relevant). We certainly do not want that here. > + */ > + if ( is_edge && s->pending ) > + writel_gicd(mask, GICD_ISPENDR); NIT: New line for clarity. > + if ( s->enabled ) > + gicv2_irq_enable(desc); > +} > + > static void gicv2_dump_state(const struct vcpu *v) > { > int i; > @@ -744,7 +809,9 @@ const static struct gic_hw_operations gicv2_ops = { > .init = gicv2_init, > .secondary_init = gicv2_secondary_cpu_init, > .save_state = gicv2_save_state, > + .save_and_mask_hwppi = gicv2_save_and_mask_hwppi, > .restore_state = gicv2_restore_state, > + .restore_hwppi = gicv2_restore_hwppi, > .dump_state = gicv2_dump_state, > .gic_host_irq_type = &gicv2_host_irq_type, > .gic_guest_irq_type = &gicv2_guest_irq_type, > diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c > index 4fe0c37..cfe705a 100644 > --- a/xen/arch/arm/gic-v3.c > +++ b/xen/arch/arm/gic-v3.c > @@ -61,6 +61,9 @@ static DEFINE_PER_CPU(void __iomem*, rbase); > #define GICD_RDIST_BASE (this_cpu(rbase)) > #define GICD_RDIST_SGI_BASE (GICD_RDIST_BASE + SZ_64K) > > +void gicv3_irq_enable(struct irq_desc *desc); > +void gicv3_irq_disable(struct irq_desc *desc); > + > /* > * Saves all 16(Max) LR registers. Though number of LRs implemented > * is implementation specific. > @@ -373,6 +376,37 @@ static void gicv3_save_state(struct vcpu *v) > v->arch.gic.v3.sre_el1 = READ_SYSREG32(ICC_SRE_EL1); > } > > +static void gicv3_save_and_mask_hwppi(struct irq_desc *desc, > + struct hwppi_state *s) > +{ > + const unsigned int mask = (1u << desc->irq); > + const unsigned int pendingr = readl_relaxed(GICD_RDIST_SGI_BASE + > GICR_ISPENDR); > + const unsigned int activer = readl_gicd(GICD_RDIST_SGI_BASE + > GICR_ISACTIVER); > + const unsigned int enabler = readl_gicd(GICD_RDIST_SGI_BASE + > GICR_ISENABLER); Those registers don't exists. Did you try to build the GICv3 code? [...] > void gic_restore_state(struct vcpu *v) > { > ASSERT(!local_irq_is_enabled()); > @@ -94,6 +124,30 @@ void gic_restore_state(struct vcpu *v) > gic_restore_pending_irqs(v); > } > > +void gic_restore_hwppi(struct vcpu *v, > + const unsigned virq, > + const struct hwppi_state *s) > +{ > + struct pending_irq *p = irq_to_pending(v, virq); > + struct irq_desc *desc = irq_to_desc(s->irq); > + > + spin_lock(&desc->lock); > + > + ASSERT(virq >= 16 && virq < 32); > + ASSERT(!is_idle_vcpu(v)); > + > + p->desc = desc; /* Migrate to new physical processor */ Is it safe? For GICv3, the re-distributor of a VCPU A could be access by VCPU B which means that all the operations (disable/enable...) could be done while we restore the PPI. > + > + irq_set_virq(desc, virq); > + > + gic_hw_ops->restore_hwppi(desc, s); > + > + if ( s->inprogress ) > + set_bit(_IRQ_INPROGRESS, &desc->status); > + > + spin_unlock(&desc->lock); > +} > + > /* > * needs to be called with a valid cpu_mask, ie each cpu in the mask has > * already called gic_cpu_init > diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c > index 5b073d1..95be10e 100644 > --- a/xen/arch/arm/irq.c > +++ b/xen/arch/arm/irq.c > @@ -138,6 +138,13 @@ static inline struct irq_guest > *irq_get_guest_info(struct irq_desc *desc) > return desc->action->dev_id; > } > > +void irq_set_virq(struct irq_desc *desc, unsigned virq) > +{ > + struct irq_guest *info = irq_get_guest_info(desc); > + ASSERT(test_bit(_IRQ_PER_CPU, &desc->status)); > + info->virq = virq; > +} > + > static inline struct domain *irq_get_domain(struct irq_desc *desc) > { > return irq_get_guest_info(desc)->d; > diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h > index c56f06e..550e28b 100644 > --- a/xen/include/asm-arm/domain.h > +++ b/xen/include/asm-arm/domain.h > @@ -34,6 +34,17 @@ enum domain_type { > extern int dom0_11_mapping; > #define is_domain_direct_mapped(d) ((d) == hardware_domain && > dom0_11_mapping) > > +struct hwppi_state { > + /* h/w state */ > + unsigned irq; > + unsigned long enabled:1; > + unsigned long pending:1; > + unsigned long active:1; > + > + /* Xen s/w state */ > + unsigned long inprogress:1; > +}; > + > struct vtimer { > struct vcpu *v; > int irq; > diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h > index 0116481..fe9af3e 100644 > --- a/xen/include/asm-arm/gic.h > +++ b/xen/include/asm-arm/gic.h > @@ -258,6 +258,20 @@ extern int gicv_setup(struct domain *d); > extern void gic_save_state(struct vcpu *v); > extern void gic_restore_state(struct vcpu *v); > > +/* > + * Save/restore the state of a single PPI which must be routed to > + * <current-vcpu> (that is, is defined to be injected to the current > + * vcpu). I would add a comment here to explain that we expect the device which use this PPI to be quiet while we save/restore. For instance we want to disable the timer before saving the state. Otherwise we will mess up the state. > + */ > +struct hwppi_state; > +extern void gic_hwppi_state_init(struct hwppi_state *s, unsigned irq); > +extern void gic_hwppi_set_pending(struct hwppi_state *s); > +extern void gic_save_and_mask_hwppi(struct vcpu *v, unsigned irq, > + struct hwppi_state *s); > +extern void gic_restore_hwppi(struct vcpu *v, > + const unsigned virq, > + const struct hwppi_state *s); > + > /* SGI (AKA IPIs) */ > enum gic_sgi { > GIC_SGI_EVENT_CHECK = 0, > @@ -308,8 +322,10 @@ struct gic_hw_operations { > int (*init)(void); > /* Save GIC registers */ > void (*save_state)(struct vcpu *); > + void (*save_and_mask_hwppi)(struct irq_desc *desc, struct hwppi_state > *s); > /* Restore GIC registers */ > void (*restore_state)(const struct vcpu *); > + void (*restore_hwppi)(struct irq_desc *desc, const struct hwppi_state > *s); > /* Dump GIC LR register information */ > void (*dump_state)(const struct vcpu *); > > diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h > index f33c331..d354e3b 100644 > --- a/xen/include/asm-arm/irq.h > +++ b/xen/include/asm-arm/irq.h > @@ -43,6 +43,7 @@ void init_secondary_IRQ(void); > > int route_irq_to_guest(struct domain *d, unsigned int virq, > unsigned int irq, const char *devname); > +int route_irq_to_current_vcpu(unsigned int irq, const char *devname); The prototype belongs to patch #6. 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 |