|
[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 |