Re: [Xen-devel] [PATCH 8/18 V2]: PVH xen: domain creation code changes

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.03.13 at 02:29, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> wrote:
> On Mon, 18 Mar 2013 11:57:43 +0000
> "Jan Beulich" <JBeulich@xxxxxxxx> wrote:
> 
>> >>> On 16.03.13 at 01:36, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>
>> >>> wrote:
>> > --- a/xen/include/asm-x86/system.h
>> > +++ b/xen/include/asm-x86/system.h
>> > @@ -4,9 +4,15 @@
>> >  #include <xen/lib.h>
>> >  #include <asm/bitops.h>
>> >  
>> > +/* We need vcpu because during context switch, going from pure PV
>> > to PVH,
>> > + * in save_segments(), current has been updated to next, and no
>> > longer pointing
>> > + * to the pure PV. Note: for PVH, we update regs->selectors on
>> > each vmexit */ #define read_segment_register(vcpu, regs,
>> > name)                 \ ({  u16
>> > __sel;                                                  \
>> > -    asm volatile ( "movw %%" STR(name) ",%0" : "=r" (__sel) );  \
>> > +    if (is_pvh_vcpu(vcpu))
>> > \
>> > +        __sel = regs->name;                                     \
>> > +    else                                                         \
>> > +        asm volatile ( "movw %%" STR(name) ",%0" :
>> > "=r" (__sel) );  \
>> > __sel;                                                      \ })
>> 
>> In a generic macro like this, please make sure you evaluate each
>> argument exactly once, and you properly parenthesize all uses of
>> macro arguments.
> 
> Hmm... at a loss. The only think I'm able to come up with here is
> paranthesis around regs, and spaces in the if statement. Both vcpu and
> regs are used only once.

Not really - vcpu is, but regs has one path where it gets evaluated,
and one path where it doesn't get used.

Btw, no matter whether there are other precedents, I do think that
the use of STR() here is misguided too - #name seems like the way
to go to me. STR() really is needed when you want the argument to
be further macro expanded before getting converted to a string,
but here you want the exact opposite - the guarantee that no
macro expansion happens (or else the "regs->name" use would
break).

Jan


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