Re: [Xen-devel] [RFC PATCH 1/15]: PVH xen: turn gdb_frames/gdt_ents into union

[Tim Deegan <tim@xxxxxxx>]

At 15:37 +0000 on 24 Jan (1359041870), Jan Beulich wrote:
> >>> On 24.01.13 at 15:29, Tim Deegan <tim@xxxxxxx> wrote:
> > At 17:25 -0800 on 11 Jan (1357925122), Mukesh Rathor wrote:
> >> diff -r bf249cd5f2c1 -r 278d7a933d88 xen/include/public/arch-x86/xen.h
> >> --- a/xen/include/public/arch-x86/xen.h    Tue Oct 30 18:12:11 2012 +0000
> >> +++ b/xen/include/public/arch-x86/xen.h    Fri Jan 11 16:19:40 2013 -0800
> >> @@ -157,7 +157,16 @@ struct vcpu_guest_context {
> >>      struct cpu_user_regs user_regs;         /* User-level CPU registers   
> >>   
> > */
> >>      struct trap_info trap_ctxt[256];        /* Virtual IDT                
> >>   
> > */
> >>      unsigned long ldt_base, ldt_ents;       /* LDT (linear address, # 
> >> ents) 
> > */
> >> -    unsigned long gdt_frames[16], gdt_ents; /* GDT (machine frames, # 
> >> ents) 
> > */
> >> +    union {
> >> +        struct {
> >> +            /* GDT (machine frames, # ents) */
> >> +            unsigned long gdt_frames[16], gdt_ents;
> >> +        } pv;
> >> +        struct {
> >> +            /* PVH: GDTR addr and size */   
> >> +            unsigned long gdtaddr, gdtsz;
> > 
> > The GDT size is always 16 bits; I'd be inclined to make the addr
> > explicitly 64 bits too, to help out 32-bit toolstacks. 
> > 
> >> +        } pvh;
> >> +    } u;
> > 
> > Also, would you consider renaming this as:
> > 
> >     union {
> >         struct {
> >             /* GDT (machine frames, # ents) */
> >             unsigned long frames[16], ents;
> >         } pv;
> >         struct {
> >             /* PVH: GDTR addr and size */   
> >             unsigned long addr, sz;
> >         } pvh;
> >     } gdt;
> > 
> > ?  Then the calling code looks a little nicer. 
> 
> Did you overlook that it is a public header that gets modified
> here?

No, but you already pointed that out so I didn't feel the need to.  I'm
just suggesting that if we're going to change the naming here, we can
change it to something nicer.

Tim.

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