[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [V10 PATCH 12/23] PVH xen: Support privileged op emulation for PVH

On Thu, 8 Aug 2013 15:18:56 +0100
George Dunlap <George.Dunlap@xxxxxxxxxxxxx> wrote:

> On Thu, Aug 8, 2013 at 2:59 AM, Mukesh Rathor
> <mukesh.rathor@xxxxxxxxxx> wrote:
> > On Wed, 7 Aug 2013 14:49:50 +0100
> > George Dunlap <dunlapg@xxxxxxxxx> wrote:
> >
> >> On Wed, Jul 24, 2013 at 2:59 AM, Mukesh Rathor
> >> <mukesh.rathor@xxxxxxxxxx> wrote:
> The problem I have is that you still pass in *both* the value of
> regs->$SEGMENT_REGISTER, *and* an enum of a segment register, and use
> one in one case, and another in a different case.  That's just a
> really ugly interface.
> What I'd like to see is for read_descriptor_sel() to *just* take
> which_sel (perhaps renamed sreg or something, since it's referring to
> a segment register), and in the PV case, read the appropriate segment
> register, then calling read_descriptor().  Then you don't have this
> crazy thing where you set two variables (sel and which_cs) all over
> the place.

Hmm... lemme make sure I understand precisely, what you mean is
something like:

static int read_descriptor_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)

    uint sel;
    if (!pvh)
        sel = read_pv_segreg(which_sel)
        return read_descriptor(sel, v, regs, base, limit, ar, vm86attr);

where read_pv_segreg() has one long case statment:
   case x86_seg_cs
       return read_segment_register(v, regs, cs);
   case x86_seg_cs
       return read_segment_register(v, regs, ds);

Then emulate_privileged_op() will not be setting data_sel, but 
only which_sel, except for one place:

            if ( lm_ovr == lm_seg_none || data_sel < 4 )
                switch ( lm_ovr )
                case lm_seg_none:

That sounds like a good change to me. Jan, you OK with this?


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.