Re: [Xen-devel] [RFC v3 4/6] xen/arm: Add save/restore support for guest core registers

[Wei Huang <w1.huang@xxxxxxxxxxx>]

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.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel