Re: [Xen-devel] [PATCH 09/18] PVH xen: Support privileged op emulation for PVH

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.07.13 at 04:00, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> wrote:
> On Wed, 03 Jul 2013 11:21:20 +0100 "Jan Beulich" <JBeulich@xxxxxxxx> wrote:
>> Even if it really is (which I doubt), you still would make PVH
>> different from both PV and HVM, which both don't populate the
>> selector fields of the frame (PV obviously has ->cs and ->ss
>> populated [by the CPU], but HVM avoids even that).
> 
> And what's wrong with PVH being little different?

There's nothing wrong with this as long as it's for a useful purpose,
and without introducing hidden dependencies (the latter is what is
happening here). Being different just for the purpose of being
different is not desirable (and likely not even acceptable, as in any
case this makes code more difficult to understand).

>> We'll have to see - at the first glance I don't follow...
> 
> Here's what I am talking about:
> 
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1241,10 +1241,10 @@ static void save_segments(struct vcpu *v)
>      struct cpu_user_regs *regs = &v->arch.user_regs;
>      unsigned int dirty_segment_mask = 0;
>  
> -    regs->ds = read_segment_register(v, regs, ds);
> -    regs->es = read_segment_register(v, regs, es);
> -    regs->fs = read_segment_register(v, regs, fs);
> -    regs->gs = read_segment_register(v, regs, gs);
> +    regs->ds = read_segment_register(v, regs, x86_seg_ds);
> +    regs->es = read_segment_register(v, regs, x86_seg_es);
> +    regs->fs = read_segment_register(v, regs, x86_seg_fs);
> +    regs->gs = read_segment_register(v, regs, x86_seg_gs);

This I think is completely pointless a change if you keep the thing
being a macro (using token concatenation):

#define read_segment_register(vcpu, regs, name)               \
({  u16 sel_;                                                 \
    const struct vcpu *vcpu_ = (vcpu);                        \
    const struct cpu_user_regs *regs_ = (regs);               \
                                                              \
    if ( is_pvh_vcpu(vcpu_) && guest_mode(regs_) )            \
        sel_ = pvh_get_selector(vcpu_, x86_seg_##name);       \
    else                                                      \
        asm volatile ( "movw %%" #name ",%0" : "=r" (sel_) ); \
    sel_;                                                     \
})

Jan


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