|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH RFC XEN v1 09/14] xen: arm: Save and restore GIC state.
On Wed, 9 Dec 2015, Ian Campbell wrote:
> Currently only GICv2 support is implemented.
>
> Given the differing architectural state between the GICv2 and v3 I
> ended up with separate save records. I'm not sure if this is the best
> plan. I have also split the state into GICD (per domain) and GICC (per
> vcpu, although also including banked GICD state).
I think it makes sense to have a per-vcpu and a per-domain save/restore
function pair.
> There are some restrictions on the guest behaviour (since PV suspend
> is cooperative this is OK). These are added to arch-arm.h.
>
> The primary requirement is that there be no active interrupts, which
> can be achieved on the guest side with some sort of CPU rendezvous
> with IRQs disabled (as is done in Linux's stop_machine framework).
>
> It is already a feature of the PV suspend protocol that the event
> channel state is not saved and the guest is expected to rebind any
> event channels, therefore losing a pending evtchn upcall is harmless.
>
> Right now there is no support for SPIs at all, but this is OK since
> such things are only exposed to guests via passthrough, which is
> incompatible with migration.
>
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> ---
> xen/arch/arm/vgic-v2.c | 135
> +++++++++++++++++++++++++++++++++
> xen/arch/arm/vgic-v3.c | 35 +++++++++
> xen/arch/arm/vgic.c | 123 ++++++++++++++++++++++++++++++
> xen/include/asm-arm/vgic.h | 5 ++
> xen/include/public/arch-arm.h | 13 ++++
> xen/include/public/arch-arm/hvm/save.h | 43 ++++++++++-
> 6 files changed, 353 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
> index 2c73133..e02edcf 100644
> --- a/xen/arch/arm/vgic-v2.c
> +++ b/xen/arch/arm/vgic-v2.c
> @@ -25,6 +25,7 @@
> #include <xen/irq.h>
> #include <xen/sched.h>
> #include <xen/sizes.h>
> +#include <xen/hvm/save.h>
>
> #include <asm/current.h>
>
> @@ -721,6 +722,140 @@ int vgic_v2_init(struct domain *d)
> return 0;
> }
>
> +
> +static int gic_v2_gicd_save(struct domain *d, hvm_domain_context_t *h)
> +{
> + struct hvm_hw_gic_v2_gicd ctxt;
> +
> + if ( d->arch.vgic.version != GIC_V2 )
> + return 0;
> +
> + if ( d->arch.vgic.nr_spis )
> + return -ENOSYS;
> +
> + ctxt.ctlr = d->arch.vgic.ctlr;
Do we need to save/restore vgic.dbase?
> + if ( hvm_save_entry(GICV2_GICD, 0, h, &ctxt) != 0 )
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> +static int gic_v2_gicd_load(struct domain *d, hvm_domain_context_t *h)
> +{
> + struct hvm_hw_gic_v2_gicd ctxt;
> +
> + if ( d->arch.vgic.version != GIC_V2 )
> + {
> + dprintk(XENLOG_G_ERR,
> + "HVM%d restore: Cannot restore GIC v2 state into domain with
> %s\n",
> + d->domain_id, gic_hw_desc(d->arch.vgic.version));
> + return -EINVAL;
> + }
> +
> + if ( hvm_load_entry(GICV2_GICD, h, &ctxt) != 0 )
> + return -EINVAL;
> +
> + d->arch.vgic.ctlr = ctxt.ctlr;
> +
> + return 0;
> +}
> +
> +HVM_REGISTER_SAVE_RESTORE(GICV2_GICD,
> + gic_v2_gicd_save, gic_v2_gicd_load,
> + 1, HVMSR_PER_DOM);
> +
> +static int gic_v2_gicc_save(struct domain *d, hvm_domain_context_t *h)
> +{
> + struct hvm_hw_gic_v2_gicc ctxt;
> + struct vcpu *v;
> + int rc, i;
> +
> + if ( d->arch.vgic.version != GIC_V2 )
> + return 0;
> +
> + /* Save the state of GICs */
> + for_each_vcpu( d, v )
> + {
> + ctxt.vmcr = v->arch.gic.v2.vmcr;
> + ctxt.apr = v->arch.gic.v2.apr;
> +
> + /* Save PPI and SGI states (per-CPU) */
> + spin_lock(&v->arch.vgic.lock);
vgic_lock
Is the domain already paused at this point? If so, maybe we could get
away without locking?
> + for ( i = 0; i < NR_GIC_LOCAL_IRQS ; i++ )
> + {
> + struct hvm_hw_irq *s = &ctxt.local_irqs[i];
> +
> + if ( (rc = vgic_irq_save_core(v, i, s)) < 0 )
> + {
> + spin_unlock(&v->arch.vgic.lock);
> + goto err;
> + }
> + }
> + spin_unlock(&v->arch.vgic.lock);
> +
> + if ( (rc = hvm_save_entry(GICV2_GICC, v->vcpu_id, h, &ctxt)) != 0 )
> + goto err;
> + }
> +
> + rc = 0;
> +err:
> + return rc;
> +}
> +
> +static int gic_v2_gicc_load(struct domain *d, hvm_domain_context_t *h)
> +{
> + struct hvm_hw_gic_v2_gicc ctxt;
> + int vcpuid;
> + struct vcpu *v;
> + int i, rc;
> +
> + if ( d->arch.vgic.version != GIC_V2 )
> + {
> + dprintk(XENLOG_G_ERR,
> + "HVM%d restore: Cannot restore GIC v2 state into domain
> using gic %d\n",
> + d->domain_id, d->arch.vgic.version);
> + return -EINVAL;
> + }
> +
> + /* Which vcpu is this? */
> + vcpuid = hvm_load_instance(h);
> + if ( vcpuid >= d->max_vcpus || (v = d->vcpu[vcpuid]) == NULL )
> + {
> + dprintk(XENLOG_G_ERR, "HVM restore: dom%u has no vcpu%u\n",
> + d->domain_id, vcpuid);
> + return -EINVAL;
> + }
> +
> + if ( hvm_load_entry(GICV2_GICC, h, &ctxt) != 0 )
> + return -EINVAL;
> +
> + v->arch.gic.v2.vmcr = ctxt.vmcr;
> + v->arch.gic.v2.apr = ctxt.apr;
> +
> + /* Restore PPI and SGI states (per-CPU) */
> + spin_lock(&v->arch.vgic.lock);
same here
> + for ( i = 0; i < NR_GIC_LOCAL_IRQS ; i++ )
> + {
> + struct hvm_hw_irq *s = &ctxt.local_irqs[i];
> +
> + ASSERT(s->irq == i);
> +
> + if ( (rc = vgic_irq_restore_core(v, s)) < 0 )
> + goto err;
> + }
> +
> + gic_dump_info(v);
> +
> +err:
> + spin_unlock(&v->arch.vgic.lock);
> + return rc;
> +}
> +
> +HVM_REGISTER_SAVE_RESTORE(GICV2_GICC, gic_v2_gicc_save, gic_v2_gicc_load,
> + 1, HVMSR_PER_VCPU);
> +
> /*
> * Local variables:
> * mode: C
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index 985e866..af418ae 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -26,6 +26,7 @@
> #include <xen/irq.h>
> #include <xen/sched.h>
> #include <xen/sizes.h>
> +#include <xen/hvm/save.h>
> #include <asm/current.h>
> #include <asm/mmio.h>
> #include <asm/gic_v3_defs.h>
> @@ -1495,6 +1496,40 @@ int vgic_v3_init(struct domain *d)
> return 0;
> }
>
> +
> +static int gic_v3_save(struct domain *d, hvm_domain_context_t *h)
> +{
> + if ( d->arch.vgic.version != GIC_V3 )
> + return 0;
> +
> + if ( d->arch.vgic.nr_spis )
> + return -ENOSYS;
> +
> + dprintk(XENLOG_G_ERR, "HVM%d save: Cannot save GIC v3\n",
> + d->domain_id);
> +
> + return -ENOSYS;
> +
> +}
> +
> +static int gic_v3_load(struct domain *d, hvm_domain_context_t *h)
> +{
> + if ( d->arch.vgic.version != GIC_V3 )
> + {
> + dprintk(XENLOG_G_ERR,
> + "HVM%d restore: Cannot restore GIC v3 state into domain with
> %s\n",
> + d->domain_id, gic_hw_desc(d->arch.vgic.version));
> + return -EINVAL;
> + }
> +
> + dprintk(XENLOG_G_ERR, "HVM%d restore: Cannot restore GIC v3\n",
> + d->domain_id);
> +
> + return -ENOSYS;
> +}
> +
> +HVM_REGISTER_SAVE_RESTORE(GICV3, gic_v3_save, gic_v3_load, 1,
> HVMSR_PER_VCPU);
> +
> /*
> * Local variables:
> * mode: C
> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index 531ce5d..b3cebac 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -25,6 +25,7 @@
> #include <xen/irq.h>
> #include <xen/sched.h>
> #include <xen/perfc.h>
> +#include <xen/hvm/save.h>
>
> #include <asm/current.h>
>
> @@ -570,6 +571,128 @@ void vgic_free_virq(struct domain *d, unsigned int virq)
> clear_bit(virq, d->arch.vgic.allocated_irqs);
> }
>
> +int vgic_irq_save_core(struct vcpu *v, unsigned irq, struct hvm_hw_irq *s)
> +{
> + struct pending_irq *p = irq_to_pending(v, irq);
> + struct vgic_irq_rank *rank = vgic_rank_irq(v, p->irq);
> +
> + const bool enabled = test_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
> + const bool queued = test_bit(GIC_IRQ_GUEST_QUEUED, &p->status);
> + const bool visible = test_bit(GIC_IRQ_GUEST_VISIBLE, &p->status);
> +
> + spin_lock(&rank->lock);
vgic_rank_lock. Maybe we need to take the vgic lock because there might
be incoming hardware interrupts for the domain but I don't think we need
the rank lock, do we?
> + ASSERT(p->irq < 32); /* SPI not supported yet */
> +
> + s->irq = p->irq;
> + s->state = 0;
> + s->priority = rank->priority[p->irq % 32];
> +
> + if ( enabled )
> + s->state |= HVM_HW_IRQ_STATE_ENABLED;
> +
> + /*
> + * If it is queued then it is pending as far as the guest is
> + * concerned, even if we've not managed to find an LR to expose
> + * this.
> + */
> + if ( queued )
> + s->state |= HVM_HW_IRQ_STATE_PENDING;
> +
> + /*
> + * If the IRQ is visible in an LR then there may be state which
> + * p->status is not aware of.
> + */
> + if ( visible )
> + {
> + struct gic_lr lr;
> +
> + gic_vcpu_saved_lr(v, p->lr, &lr);
> +
> + if ( lr.state & GICH_LR_PENDING )
> + s->state |= HVM_HW_IRQ_STATE_PENDING;
This is unnecessary: given that the LRs are updated on hypervisor entry,
you can be sure at this point they are up to date. If an irq is
GIC_IRQ_GUEST_VISIBLE, then it means that is PENDING or ACTIVE (or
PENDING and ACTIVE). Given that we have mandated that there cannot be
any ACTIVE interrupts, then we could skip all this and just
if ( visible )
s->state |= HVM_HW_IRQ_STATE_PENDING;
> + /*
> + * HVM_HW_IRQ_STATE_ACTIVE: We currently do not handle save/restore
> + * with active interrupts.
> + */
> + if ( lr.state & GICH_LR_ACTIVE )
> + {
> + dprintk(XENLOG_G_ERR,
> + "HVM%d save: Cannot save active local IRQ%u\n",
> + v->domain->domain_id, irq);
> + return -EINVAL;
> + }
> + }
> +
> + /*
> + * HVM_HW_IRQ_STATE_EDGE: we implement ICFG[0] and [1] as RAZ/WI.
> + */
> +
> + spin_unlock(&rank->lock);
> +
> + return 0;
> +}
We need to save/restore the target vcpu for each virq.
> +int vgic_irq_restore_core(struct vcpu *v, struct hvm_hw_irq *s)
> +{
> + struct pending_irq *p = irq_to_pending(v, s->irq);
> + struct vgic_irq_rank *rank = vgic_rank_irq(v, s->irq);
> + bool reraise = 0;
> +
> + /*
> + * Level/Edge, we implement ICFG for SGI/PPI as RAZ/WI, so this bit
> cannot
> + * be set.
> + */
> + if ( s->state & HVM_HW_IRQ_STATE_EDGE )
> + {
> + dprintk(XENLOG_G_ERR,
> + "HVM%d restore: Cannot restore an edge triggered local
> IRQ%u\n",
> + v->domain->domain_id, s->irq);
> + return -EINVAL;
> + }
> +
> + if ( s->state & HVM_HW_IRQ_STATE_ACTIVE )
> + {
> + dprintk(XENLOG_G_ERR,
> + "HVM%d restore: Cannot restore active local IRQ%u\n",
> + v->domain->domain_id, s->irq);
> + return -EINVAL;
> + }
> +
> + spin_lock(&rank->lock);
same comment on the rank lock
> + ASSERT(s->irq < 32); /* SPI not supported yet */
> +
> + rank->priority[s->irq % 32] = s->priority;
we need to restore the target vcpu
> + if ( s->state & HVM_HW_IRQ_STATE_ENABLED )
> + set_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
> +
> + /* If the IRQ was either PENDING or ACTIVE, then QUEUE it to be
> reinjected. */
> + if ( s->state & (HVM_HW_IRQ_STATE_PENDING|HVM_HW_IRQ_STATE_ACTIVE) )
> + {
> + reraise = 1;
> + set_bit(GIC_IRQ_GUEST_QUEUED, &p->status);
> + }
> +
> + /* This will check for enabled etc */
> + if ( reraise ) /* v != current, so will add to lr_pending */
You don't need reraise: just add the call to gic_raise_guest_irq under
the previous if statement.
> + {
> + /*
> + * This uses the current IPRIORITYR, which may differ from
> + * when the IRQ was actually made pending. h/w spec probably
> + * allows this, XXXX check
> + */
From this VM point of view the IPRIORITYR is s->priority. I would remove
the comment.
> + gic_raise_guest_irq(v, s->irq, s->priority);
> + }
> +
> + spin_unlock(&rank->lock);
> +
> + return 0;
> +}
> +
> +
> /*
> * Local variables:
> * mode: C
> diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
> index 005f822..bfa9c60 100644
> --- a/xen/include/asm-arm/vgic.h
> +++ b/xen/include/asm-arm/vgic.h
> @@ -333,6 +333,11 @@ static inline int vgic_allocate_spi(struct domain *d)
>
> extern void vgic_free_virq(struct domain *d, unsigned int virq);
>
> +struct hvm_hw_irq;
> +extern int vgic_irq_save_core(struct vcpu *v, unsigned irq,
> + struct hvm_hw_irq *s);
> +extern int vgic_irq_restore_core(struct vcpu *v, struct hvm_hw_irq *s);
> +
> void vgic_v2_setup_hw(paddr_t dbase, paddr_t cbase, paddr_t csize,
> paddr_t vbase, uint32_t aliased_offset);
>
> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> index 6322548..8df80ca 100644
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -163,6 +163,19 @@
> * at Documentation/devicetree/bindings/arm/xen.txt.
> */
>
> +/*
> + * Requirements for Save/Restore/Migrate.
> + *
> + * When we are suspending we require the VM to have quiesced itself
> + * before calling HYPERVISOR_suspend(). This means:
> + *
> + * - There must be no active interrupt (SGI, PPI or SPI).
> + * - All VCPUs must have interrupts masked.
> + *
> + * Upon restore any event channel upcall (via the event channel PPI)
> + * which was pending upon save will be lost.
> + */
> +
> #define XEN_HYPERCALL_TAG 0XEA1
>
> #define int64_aligned_t int64_t __attribute__((aligned(8)))
> diff --git a/xen/include/public/arch-arm/hvm/save.h
> b/xen/include/public/arch-arm/hvm/save.h
> index 7b92c9c..db916b1 100644
> --- a/xen/include/public/arch-arm/hvm/save.h
> +++ b/xen/include/public/arch-arm/hvm/save.h
> @@ -93,10 +93,51 @@ struct hvm_hw_timer
>
> DECLARE_HVM_SAVE_TYPE(A15_TIMER, 3, struct hvm_hw_timer);
>
> +
> +/* Domain global GICD state */
> +struct hvm_hw_gic_v2_gicd
> +{
> + uint32_t ctlr;
> +};
> +DECLARE_HVM_SAVE_TYPE(GICV2_GICD, 4, struct hvm_hw_gic_v2_gicd);
> +
> +struct hvm_hw_irq
> +{
> + uint32_t irq;
> +#define HVM_HW_IRQ_STATE_ENABLED (1UL << 0)
> +#define HVM_HW_IRQ_STATE_PENDING (1UL << 1)
> +#define HVM_HW_IRQ_STATE_ACTIVE (1UL << 2)
> +#define HVM_HW_IRQ_STATE_EDGE (1UL << 3)
> + uint8_t state;
> + uint8_t priority;
> +};
> +
> +/* Per-vcpu GICC state (and per-VCPU GICD, e.g. SGI+PPI) */
> +struct hvm_hw_gic_v2_gicc
> +{
> + /* GICC state */
> + uint32_t vmcr; /* GICC_{PMR,BPR,ABPR,CTLR} state */
> + uint32_t apr; /* Active priorities */
> +
> + /*
> + * SGI + PPI state.
> + */
> + struct hvm_hw_irq local_irqs[32];
> +};
> +
> +DECLARE_HVM_SAVE_TYPE(GICV2_GICC, 5, struct hvm_hw_gic_v2_gicc);
> +
> +struct hvm_hw_gic_v3
> +{
> + /* TODO */
> +};
> +
> +DECLARE_HVM_SAVE_TYPE(GICV3, 6, struct hvm_hw_gic_v3);
> +
> /*
> * Largest type-code in use
> */
> -#define HVM_SAVE_CODE_MAX 3
> +#define HVM_SAVE_CODE_MAX 6
>
> #endif
>
> --
> 2.6.1
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |