Re: RFC: PVH set vcpu info context in vmcs....

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

On Tue, 13 Aug 2013 11:56:36 +0100
"Jan Beulich" <JBeulich@suse.com> wrote:

> >>> On 13.08.13 at 03:45, Mukesh Rathor <mukesh.rathor@oracle.com>
> >>> wrote:
> > int vmx_pvh_set_vcpu_info(struct vcpu *v, struct vcpu_guest_context
> > *ctxtp) {
> >     int rc;
> > 
> >     if ( v->vcpu_id == 0 )
> >         return 0;
> 
> Bogus/pointless.

No, we don't have anything for the boot vcpu. It's totally coming up
on the flat address space. For non boot, the vcpu is coming up on the
kernel GDT. Recall it's a PV guest (coming up in an HVM container).

> >     if ( !(ctxtp->flags & VGCF_in_kernel) )
> >         return -EINVAL;
> > 
> >     if ( ctxtp->ldt_base || ctxtp->ldt_ents ||
> >          (ctxtp->user_regs.cs & 4) || ctxtp->user_regs.ss || 
> >          ctxtp->user_regs.es || ctxtp->user_regs.ds )
> >         return -EINVAL;
> 
> How about FS/GS? If you don't enforce these selectors to be zero
> too, then loading only base and selector values below isn't
> sufficient (and again potentially inconsistent).
> 
> > 
> >     if ( ctxtp->user_regs.cs == 0 )
> >         return -EINVAL;
> 
> Perhaps also check RPL == 0?

OK. I still think we should just remove the check for VGCF_in_kernel,
why do we care for PVH? For 64bit PV, we needed that, but a PVH guest
can come up any RPL it chooses to, right?

Below is updated version.

thanks
Mukesh


/*
 * Set vmcs fields in support of vcpu_op -> VCPUOP_initialise hcall used
 * to initialise a secondary vcpu prior to boot. Called from
 * arch_set_info_guest() which sets the (PVH relevant) non-vmcs fields.
 *
 * In case of linux:
 *     The call comes from cpu_initialize_context().  (boot vcpu 0 context is
 *     set by the tools via do_domctl -> vcpu_initialise).
 *
 * PVH 32bitfixme: this function needs to be modified for 32bit guest.
 */
int vmx_pvh_set_vcpu_info(struct vcpu *v, struct vcpu_guest_context *ctxtp)
{
    int rc = 0;

    if ( v->vcpu_id == 0 )
        return 0;

    if ( !(ctxtp->flags & VGCF_in_kernel) )
        return -EINVAL;

    if ( ctxtp->ldt_base || ctxtp->ldt_ents ||
         (ctxtp->user_regs.cs & 4) || ctxtp->user_regs.ss ||
         ctxtp->user_regs.es || ctxtp->user_regs.ds ||
         ctxtp->user_regs.fs || ctxtp->user_regs.gs )
         
        return -EINVAL;

    if ( ctxtp->user_regs.cs == 0 || (ctxtp->user_regs.cs & 3) == 3 )
        return -EINVAL;

    vmx_vmcs_enter(v);
    __vmwrite(GUEST_GDTR_BASE, ctxtp->gdt.pvh.addr);
    __vmwrite(GUEST_GDTR_LIMIT, ctxtp->gdt.pvh.limit);

    __vmwrite(GUEST_FS_BASE, ctxtp->fs_base);
    __vmwrite(GUEST_GS_BASE, ctxtp->gs_base_kernel);

    /* IA-32e: ss/es/ds are ignored, we load cs only. */
    __vmwrite(GUEST_CS_SELECTOR, ctxtp->user_regs.cs);
    if ( (rc = hvm_load_segment_selector(v, x86_seg_cs, ctxtp->user_regs.cs)) )
        goto out;

    if ( (rc = vmx_add_guest_msr(MSR_SHADOW_GS_BASE)) )
        goto out;
    vmx_write_guest_msr(MSR_SHADOW_GS_BASE, ctxtp->gs_base_user);

 out:
    vmx_vmcs_exit(v);
    return 0;
}