Re: [Xen-devel] [PATCH 09/17] [V3]PVH xen: create PVH vmcs, and initialization

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.04.13 at 03:02, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> wrote:
> +static int hvm_pvh_vcpu_initialise(struct vcpu *v)
> +{
> +    int rc;
> +
> +    if ( (rc = hvm_funcs.vcpu_initialise(v)) != 0 )
> +        return rc;
> +
> +    softirq_tasklet_init(&v->arch.hvm_vcpu.assert_evtchn_irq_tasklet,
> +                         (void(*)(unsigned long))hvm_assert_evtchn_irq,
> +                         (unsigned long)v);
> +
> +    v->arch.hvm_vcpu.hcall_64bit = 1;

One more case of not being 32-bit ready, but also not having an
easy way to spot the place later?

> +    v->arch.hvm_vcpu.hvm_pvh.vcpu_info_mfn = INVALID_MFN;
> +    v->arch.user_regs.eflags = 2;
> +    v->arch.hvm_vcpu.inject_trap.vector = -1;
> +
> +    if ( (rc = hvm_vcpu_cacheattr_init(v)) != 0 )
> +    {
> +        hvm_funcs.vcpu_destroy(v);
> +        return rc;
> +    }
> +
> +    /* 
> +     * During domain shutdown: pvh_vmx_vmexit_handler->emulate_privileged_op
> +     * -> guest_io_read -> pv_pit_handler -> handle_speaker_io -> _spin_lock
> +     *  so we call pit_init to initialize the spin lock.
> +     */
> +    if ( v->vcpu_id == 0 )
> +        pit_init(v, cpu_khz);

I'm sorry, but - what? This is either a generic (i.e. not shutdown
specific) problem (and then the comment is misleading), or you
have a problem with the shutdown path that should be fixed
there, not with some hacky workaround.

> @@ -4514,6 +4591,8 @@ static int hvm_memory_event_traps(long p, uint32_t 
> reason,
>  
>  void hvm_memory_event_cr0(unsigned long value, unsigned long old) 
>  {
> +    if ( is_pvh_vcpu(current) )
> +        return;

This is temporary only, isn't it? If so, it should be clearly marked
as such.

> +    vmentry_ctl &= ~VM_ENTRY_LOAD_DEBUG_CTLS;

So you're running the guest with host settings? How's that going
to work?

> +    vmentry_ctl &= ~VM_ENTRY_LOAD_GUEST_EFER;
> +    vmentry_ctl &= ~VM_ENTRY_SMM;
> +    vmentry_ctl &= ~VM_ENTRY_DEACT_DUAL_MONITOR;
> +    vmentry_ctl |= VM_ENTRY_IA32E_MODE;       /* GUEST_EFER.LME/LMA ignored 
> */

Again not 32-bit ready without mark?

Jan


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