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: Fri, 09 May 2014 10:17:39 +0100 Message-ID: <536C9D33.5080607@linaro.org> References: <1399583908-21755-1-git-send-email-w1.huang@samsung.com> <1399583908-21755-3-git-send-email-w1.huang@samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1399583908-21755-3-git-send-email-w1.huang@samsung.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: Wei Huang , xen-devel@lists.xen.org Cc: keir@xen.org, ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com, tim@xen.org, jaeyong.yoo@samsung.com, jbeulich@suse.com, ian.jackson@eu.citrix.com, yjhyun.yoo@samsung.com List-Id: xen-devel@lists.xenproject.org Hi Wei, (Adding Vijay in CC). Vijay is working on GICv3 support in Xen. His patch series and this patch will clash sooner. I think you should work together to avoid big reworking later. On 08/05/14 22:18, Wei Huang wrote: > This patch implements a save/restore support for > ARM guest GIC. Two types of GIC V2 states are saved seperately: separately > 1) VGICD_* contains the GIC distributor state from > guest VM's view; 2) GICH_* is the GIC virtual control I would add a newline before "2)", we don't care about long commit message :). > state from hypervisor's persepctive. perspective > > Signed-off-by: Evgeny Fedotov > Signed-off-by: Wei Huang > --- > xen/arch/arm/vgic.c | 171 ++++++++++++++++++++++++++++++++ > xen/include/public/arch-arm/hvm/save.h | 34 ++++++- > 2 files changed, 204 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c > index 4cf6470..505e944 100644 > --- a/xen/arch/arm/vgic.c > +++ b/xen/arch/arm/vgic.c > @@ -24,6 +24,7 @@ > #include > #include > #include > +#include > > #include > > @@ -73,6 +74,110 @@ static struct vgic_irq_rank *vgic_irq_rank(struct vcpu *v, int b, int n) > return NULL; > } > > +/* Save guest VM's distributor info into a context to support domains > + * save/restore. Such info represents guest VM's view of its GIC > + * distributor (GICD_*). > + */ > +static int hvm_vgicd_save(struct domain *d, hvm_domain_context_t *h) > +{ > + struct hvm_arm_vgicd_v2 ctxt; > + struct vcpu *v; > + struct vgic_irq_rank *rank; > + int rc = 0; > + > + /* Save the state for each VCPU */ > + for_each_vcpu( d, v ) > + { > + rank = &v->arch.vgic.private_irqs; > + > + /* IENABLE, IACTIVE, IPEND, PENDSGI */ > + ctxt.ienable = rank->ienable; > + ctxt.iactive = rank->iactive; > + ctxt.ipend = rank->ipend; > + ctxt.pendsgi = rank->pendsgi; > + > + /* ICFG */ > + ctxt.icfg[0] = rank->icfg[0]; > + ctxt.icfg[1] = rank->icfg[1]; I would use the same pattern as IPRIOTITY and ITARGETS. [..] > +/* Load guest VM's distributor info from a context to support domain > + * save/restore. The info is loaded into vgic_irq_rank. > + */ > +static int hvm_vgicd_load(struct domain *d, hvm_domain_context_t *h) > +{ [..] > + /* ICFG */ > + rank->icfg[0] = ctxt.icfg[0]; > + rank->icfg[1] = ctxt.icfg[1]; Same remark here. [..] > +/* Save GIC virtual control state into a context to support save/restore. > + * The info reprsents most of GICH_* registers. */ represents Regards, -- Julien Grall