Re: [PATCH 04/18] PVH xen: add params to read_segment_register

[Mukesh Rathor <mukesh.rathor@oracle.com>]

On Thu, 06 Jun 2013 07:48:49 +0100
"Jan Beulich" <JBeulich@suse.com> wrote:

> >>> On 06.06.13 at 03:25, Mukesh Rathor <mukesh.rathor@oracle.com>
> >>> wrote:
> > On Fri, 31 May 2013 11:00:12 +0100
> > "Jan Beulich" <JBeulich@suse.com> wrote:
> > 
> > save_segments() calls.
> > 
> > BTW, I can't use current in the macro because of call from
> > save_segments(). 
> > 
> >> at all (i.e. things ought to work much like the if() portion of
> >> show_registers() which you _do not_ modify).
> > 
> > Yeah, it was on hold because I've been investigating guest_cr[]
> > sanity, and found that I was missing:
> > 
> >     v->arch.hvm_vcpu.guest_cr[4] = value;
> > 
> > So, my next version will add that and update show_registers() for
> > PVH. I can scratch off another fixme from my list.
> 
> But you don't answer the underlying question that I raised: Is
> accessing the struct cpu_user_regs selector register fields valid
> at all? Again, the #VMEXIT handling code only stores garbage
> into them (in debug builds, in non-debug builds it simply leaves
> the fields unaltered).

Are you looking at the right version? The patch doesn't have much DEBUG
code anymore. The selectors are updated every VMExit whether DEBUG or not:

static void read_vmcs_selectors(struct cpu_user_regs *regs)
{
    regs->cs = __vmread(GUEST_CS_SELECTOR);
    regs->ss = __vmread(GUEST_SS_SELECTOR);
    regs->ds = __vmread(GUEST_DS_SELECTOR);
    regs->es = __vmread(GUEST_ES_SELECTOR);
    regs->gs = __vmread(GUEST_GS_SELECTOR);
    regs->fs = __vmread(GUEST_FS_SELECTOR);
}

void vmx_pvh_vmexit_handler(struct cpu_user_regs *regs)
{
    unsigned long exit_qualification;
    unsigned int exit_reason = __vmread(VM_EXIT_REASON);
    int rc=0, ccpu = smp_processor_id();
    struct vcpu *v = current;

    dbgp1("PVH:[%d]left VMCS exitreas:%d RIP:%lx RSP:%lx EFLAGS:%lx CR0:%lx\n",
          ccpu, exit_reason, regs->rip, regs->rsp, regs->rflags,
          __vmread(GUEST_CR0));

    /* guest_kernel_mode() needs cs. read_segment_register needs others */
    read_vmcs_selectors(regs);
.....


You are very thorough, so I'm sure it's not this obvious and I am just
missing something, so please bear with me if that's the case. I'll
continue staring at it... 

thanks
Mukesh