Re: [Xen-devel] [PATCH RFC v12 14/21] pvh: read_descriptor for PVH guests.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.09.13 at 18:25, George Dunlap <george.dunlap@xxxxxxxxxxxxx> wrote:
> This is in preparation for enabling emulated privops for PVH guests.
> 
> This one unfortunately has a bit more of an impedance mismatch:
> 
> * For PV, the selector is hard-coded in by passing #name in the
>   #define.  For PVH, we have to do a switch statement.
> 
> * For PV, given the desrciptor, it can read the resulting base, limit,
>   &c directly; for PVH, we have to read the values currently loaded in the 
> vmcs.
> 
> This leads to a rather awkward construct where we *both* read the
> descriptor, *and* specify a selector, and introduce an intermediate 
> function,
> read_descriptor_sel.
> 
> Unfortunately, without introducing a rather pointless switch()
> statement to the PV path similar to the one in the PVH path, there's
> no way to make this less awkward.
> 
> Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> Signed-off-by: Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

with one really minor comment:

> +static int read_descriptor_sel(unsigned int sel,
> +                               enum x86_segment which_sel,
> +                               struct vcpu *v,
> +                               const struct cpu_user_regs *regs,
> +                               unsigned long *base,
> +                               unsigned long *limit,
> +                               unsigned int *ar,
> +                               unsigned int vm86attr)
> +{
> +    struct segment_register seg;
> +    bool_t long_mode;
> +
> +    if ( !is_pvh_vcpu(v) )
> +        return read_descriptor(sel, v, regs, base, limit, ar, vm86attr);
> +
> +    hvm_get_segment_register(v, x86_seg_cs, &seg);
> +    long_mode = seg.attr.fields.l;
> +
> +    if ( which_sel != x86_seg_cs )
> +        hvm_get_segment_register(v, which_sel, &seg);
> +
> +    /* "ar" is returned packed as in segment_attributes_t. Fix it up. */
> +    *ar = seg.attr.bytes;
> +    *ar = (*ar & 0xff ) | ((*ar & 0xf00) << 4);

Stray blank before the first closing parenthesis.

Jan


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