Re: [Xen-devel] [RFC v3 2/6] xen/arm: Add save/restore support for ARM GIC V2

[Tim Deegan <tim@xxxxxxx>]

At 13:05 +0100 on 14 May (1400069153), Julien Grall wrote:
> On 05/14/2014 12:07 PM, Ian Campbell wrote:
> > On Thu, 2014-05-08 at 16:18 -0500, Wei Huang wrote:
> >> diff --git a/xen/include/public/arch-arm/hvm/save.h 
> >> b/xen/include/public/arch-arm/hvm/save.h
> >> index 8312e7b..421a6f6 100644
> >> --- a/xen/include/public/arch-arm/hvm/save.h
> >> +++ b/xen/include/public/arch-arm/hvm/save.h
> >> @@ -40,10 +40,42 @@ struct hvm_save_header
> >>  };
> >>  DECLARE_HVM_SAVE_TYPE(HEADER, 1, struct hvm_save_header);
> >>  
> >> +/* Guest's view of GIC distributor (per-vcpu)
> >> + *   - Based on GICv2 (see "struct vgic_irq_rank")
> >> + *   - Store guest's view of GIC distributor
> >> + *   - Only support SGI and PPI for DomU (DomU doesn't handle SPI)
> >> + */
> >> +struct hvm_arm_vgicd_v2
> >> +{
> >> +    uint32_t ienable;
> >> +    uint32_t iactive;
> >> +    uint32_t ipend;
> >> +    uint32_t pendsgi;
> >> +    uint32_t icfg[2];
> >> +    uint32_t ipriority[8];
> >> +    uint32_t itargets[8];
> >> +};
> >> +DECLARE_HVM_SAVE_TYPE(VGICD_V2, 2, struct hvm_arm_vgicd_v2);
> > 
> > This is the state of 32 interrupts. How do you propose to handle more
> > interrupts than that?
> > 
> > I think it would be sensible to split the domain global state, the
> > distributor and cpu interface base addresses and sizes and the states of
> > any SPIs in here and have a separate per-vpcu set of state for the
> > per-cpu GICD state (SPIs and PPIs mainly).
> > 
> > For the SPI I think you either want to put the above set of state into
> > an array of size NR_GUEST_INTERRUPTS/32 or better make each of the above
> > an array based on NR_GUEST_INTERRUPTS.
> > 
> >> +
> >> +/* Info for hypervisor to manage guests (per-vcpu)
> >> + *   - Based on GICv2
> >> + *   - Mainly store registers of GICH_*
> >> + */
> >> +struct hvm_arm_gich_v2
> >> +{
> >> +    uint32_t gic_hcr;
> >> +    uint32_t gic_vmcr;
> >> +    uint32_t gic_apr;
> >> +    uint32_t gic_lr[64];
> >> +    uint64_t event_mask;
> >> +    uint64_t lr_mask;
> > 
> > I don't think you should be saving any GICH state at all. What should be
> > saved is the corresponding GICC state, i.e. "architectural state" that
> > is observed by the guest. This might mean pickling stuff from the GICH
> > state into a GICC form. (I said this wrt the LRs in a previous round of
> > review)
> 
> What are the advantage to save the GICC state rather than GICH?
> 
> IIRC, the GICH state gives you a representation of the important bits of
> the GICC. Most of GICC can't be restore without any translation and
> writing in GICH (see gic_vmcr that is a collection of multiple GICC
> registers). It seems easier to use GICH state during migration.

The GICC state is the architectural state of the virtual machine;
the GICH state is an implementation detail of how that's achieved by Xen.
We prefer always to put clean architectural state into the save record.
That way if we for any reason change how the VMM is implemented, the
save record format won't be affected by that.

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel