From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [RFC v3 2/6] xen/arm: Add save/restore support for ARM GIC V2 Date: Wed, 14 May 2014 13:05:53 +0100 Message-ID: <53735C21.2080104@linaro.org> References: <1399583908-21755-1-git-send-email-w1.huang@samsung.com> <1399583908-21755-3-git-send-email-w1.huang@samsung.com> <1400065670.29366.27.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1400065670.29366.27.camel@kazak.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell , Wei Huang Cc: keir@xen.org, stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com, tim@xen.org, jaeyong.yoo@samsung.com, xen-devel@lists.xen.org, jbeulich@suse.com, ian.jackson@eu.citrix.com, yjhyun.yoo@samsung.com List-Id: xen-devel@lists.xenproject.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