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

[Julien Grall <julien.grall@linaro.org>]

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.

Regards,

-- 
Julien Grall