From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wei Huang Subject: Re: [RFC v3 4/6] xen/arm: Add save/restore support for guest core registers Date: Fri, 09 May 2014 11:35:18 -0500 Message-ID: <536D03C6.4080800@samsung.com> References: <1399583908-21755-1-git-send-email-w1.huang@samsung.com> <1399583908-21755-5-git-send-email-w1.huang@samsung.com> <536C0EF9.3080303@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-reply-to: <536C0EF9.3080303@citrix.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: Andrew Cooper , xen-devel@lists.xen.org Cc: keir@xen.org, ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com, ian.jackson@eu.citrix.com, julien.grall@linaro.org, tim@xen.org, jaeyong.yoo@samsung.com, jbeulich@suse.com, yjhyun.yoo@samsung.com List-Id: xen-devel@lists.xenproject.org On 05/08/2014 06:10 PM, Andrew Cooper wrote: > On 08/05/2014 22:18, Wei Huang wrote: >> +#ifdef CONFIG_ARM_32 >> + v->arch.ifar = ctxt.ifar; >> + v->arch.dfar = ctxt.dfar; >> + v->arch.dfsr = ctxt.dfsr; >> +#else >> + v->arch.far = ctxt.far; >> + v->arch.esr = ctxt.esr; >> +#endif > > Where you have code like this, please use a union in the structure to > reduce its size. I thought about it from your comments. We can only combine ifar and dfar into the far register. But it became a bit confusing. Will try again. > >> + /* ======= Guest Core Registers ======= >> + * - Each reg is multiplexed for AArch64 and AArch32 guests, if possible >> + * - Each comments, /AArch64_reg, AArch32_reg/, describes its >> + * corresponding 64- and 32-bit register name. "NA" means >> + * "Not Applicable". >> + * - Check "struct vcpu_guest_core_regs" for details. >> + */ >> + uint64_t x0; /* x0, r0_usr */ >> + uint64_t x1; /* x1, r1_usr */ >> + uint64_t x2; /* x2, r2_usr */ >> + uint64_t x3; /* x3, r3_usr */ >> + uint64_t x4; /* x4, r4_usr */ >> + uint64_t x5; /* x5, r5_usr */ >> + uint64_t x6; /* x6, r6_usr */ >> + uint64_t x7; /* x7, r7_usr */ >> + uint64_t x8; /* x8, r8_usr */ >> + uint64_t x9; /* x9, r9_usr */ >> + uint64_t x10; /* x10, r10_usr */ >> + uint64_t x11; /* x11, r11_usr */ >> + uint64_t x12; /* x12, r12_usr */ >> + uint64_t x13; /* x13, sp_usr */ >> + uint64_t x14; /* x14, lr_usr; */ >> + uint64_t x15; /* x15, __unused_sp_hyp */ >> + uint64_t x16; /* x16, lr_irq */ >> + uint64_t x17; /* x17, sp_irq */ >> + uint64_t x18; /* x18, lr_svc */ >> + uint64_t x19; /* x19, sp_svc */ >> + uint64_t x20; /* x20, lr_abt */ >> + uint64_t x21; /* x21, sp_abt */ >> + uint64_t x22; /* x22, lr_und */ >> + uint64_t x23; /* x23, sp_und */ >> + uint64_t x24; /* x24, r8_fiq */ >> + uint64_t x25; /* x25, r9_fiq */ >> + uint64_t x26; /* x26, r10_fiq */ >> + uint64_t x27; /* x27, r11_fiq */ >> + uint64_t x28; /* x28, r12_fiq */ >> + uint64_t x29; /* fp, sp_fiq */ >> + uint64_t x30; /* lr, lr_fiq */ > > Please use "uint64_t x[31];" and some loops. > Register multiplexing is very confusing when 32-bit VM is running on a 64-bit Xen. Even though your request is reasonable, I still consider expanding the register names here helpful. At least it gives me space to add comments to show the register mapping layout. I believe others will find it useful as well.