[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [RFC v2 1/6] xen/arm: Save and restore support with hvm context hypercalls
Hello Wei, Thank you for the patch. You are modifying common code in this patch. I've add Jan and Keir. Also, I would create a separate patch for this code movement. On 15/04/14 22:05, Wei Huang wrote: + +HVM_REGISTER_SAVE_RESTORE(GIC, hvm_gic_save_ctxt, hvm_gic_load_ctxt, 1, + HVMSR_PER_VCPU); With new support for different GIC, I would differentiate VGIC and GIC save/restore. Also can you append V2 in the name? GICv3 support will be added soon. [..] + /* * Local variables: * mode: C diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c index 3d6a721..7c47eac 100644 --- a/xen/arch/arm/vtimer.c +++ b/xen/arch/arm/vtimer.c @@ -21,6 +21,7 @@ #include <xen/lib.h> #include <xen/timer.h> #include <xen/sched.h> +#include <xen/hvm/save.h> #include <asm/irq.h> #include <asm/time.h> #include <asm/gic.h> @@ -284,6 +285,76 @@ int vtimer_emulate(struct cpu_user_regs *regs, union hsr hsr) } } +static int hvm_vtimer_save_ctxt(struct domain *d, hvm_domain_context_t *h) +{ + struct hvm_hw_timer ctxt; + struct vcpu *v; + struct vtimer *t; + int i, ret = 0; + + /* Save the state of vtimer and ptimer */ + for_each_vcpu( d, v ) + { + t = &v->arch.virt_timer; + for ( i = 0; i < 2; i++ ) + { Looping here is very confusing, what does mean 0? 1?I would create an helper with the content of this loop and call twice this helper with the correct value in parameter. Smth like: hvm_save_vtimer(TIMER_TYPE_PHYS, &v->arch.phys_timer); hvm_save_vtimer(TIMER_TYPE_VIRT, &v->arch.virt_timer); + ctxt.cval = t->cval; + ctxt.ctl = t->ctl; + ctxt.vtb_offset = i ? d->arch.phys_timer_base.offset : + d->arch.virt_timer_base.offset; + ctxt.type = i ? TIMER_TYPE_PHYS : TIMER_TYPE_VIRT; + + if ( (ret = hvm_save_entry(TIMER, v->vcpu_id, h, &ctxt)) != 0 ) + return ret; + + t = &v->arch.phys_timer; It will avoid this hackish line. + } + } + + return ret; +} + +static int hvm_vtimer_load_ctxt(struct domain *d, hvm_domain_context_t *h) +{ + int vcpuid; + struct hvm_hw_timer ctxt; + struct vcpu *v; + struct vtimer *t = NULL; + + /* 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(TIMER, h, &ctxt) != 0 ) + return -EINVAL; + + if ( ctxt.type == TIMER_TYPE_VIRT ) + { + t = &v->arch.virt_timer; + d->arch.virt_timer_base.offset = ctxt.vtb_offset; + } + else else if ( ctx.type == TIMER_TYPE_PHYS ) ? Then fail if the ctx.type is wrong. Even better I would use a switch case. + { + t = &v->arch.phys_timer; + d->arch.phys_timer_base.offset = ctxt.vtb_offset; Saving {virt,phys}_timer_base.offset which are per-domain seems a waste of space and confusing. + } + + t->cval = ctxt.cval; + t->ctl = ctxt.ctl; + t->v = v; + + return 0; +} + [..] diff --git a/xen/include/asm-arm/hvm/support.h b/xen/include/asm-arm/hvm/support.h new file mode 100644 index 0000000..09f7cb8 --- /dev/null +++ b/xen/include/asm-arm/hvm/support.h @@ -0,0 +1,29 @@ +/* + * asm-arm/hvm/support.h: HVM support routines used by ARM. + * + * Copyright (c) 2014, Samsung Electronics. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program; if not, write to the Free Software Foundation, Inc., 59 Temple + * Place - Suite 330, Boston, MA 02111-1307 USA. + */ + +#ifndef __ASM_ARM_HVM_SUPPORT_H__ +#define __ASM_ARM_HVM_SUPPORT_H__ + +#include <xen/types.h> +#include <public/hvm/ioreq.h> +#include <xen/sched.h> +#include <xen/hvm/save.h> +#include <asm/processor.h> + +#endif /* __ASM_ARM_HVM_SUPPORT_H__ */ diff --git a/xen/include/public/arch-arm/hvm/save.h b/xen/include/public/arch-arm/hvm/save.h index 75b8e65..f6ad258 100644 --- a/xen/include/public/arch-arm/hvm/save.h +++ b/xen/include/public/arch-arm/hvm/save.h @@ -26,6 +26,136 @@ #ifndef __XEN_PUBLIC_HVM_SAVE_ARM_H__ #define __XEN_PUBLIC_HVM_SAVE_ARM_H__ +#define HVM_FILE_MAGIC 0x92385520 +#define HVM_FILE_VERSION 0x00000001 + +struct hvm_save_header +{ + uint32_t magic; /* Must be HVM_FILE_MAGIC */ + uint32_t version; /* File format version */ + uint64_t changeset; /* Version of Xen that saved this file */ + uint32_t cpuid; /* MIDR_EL1 on the saving machine */ +}; +DECLARE_HVM_SAVE_TYPE(HEADER, 1, struct hvm_save_header); + +struct vgic_rank I would rename into vgic_v2_rank. +{ + uint32_t ienable, iactive, ipend, pendsgi; + uint32_t icfg[2]; + uint32_t ipriority[8]; + uint32_t itargets[8]; +}; + +struct hvm_hw_gic I would rename into hvm_hw_vgic. +{ + 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; + struct vgic_rank ppi_state; As said previously, I would separate GIC from VGIC save/restore. -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |