Re: [PATCH v2 2/4] x86/pvh: Set 32b PVH guest mode in XEN_DOMCTL_set_address_size

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

On 07/07/2015 05:15 AM, Jan Beulich wrote:
>>>> On 29.06.15 at 22:21, <boris.ostrovsky@oracle.com> wrote:
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -377,25 +377,25 @@ int switch_compat(struct domain *d)
>>       struct vcpu *v;
>>       int rc;
>>   
>> -    if ( is_pvh_domain(d) )
>> -    {
>> -        printk(XENLOG_G_INFO
>> -               "Xen currently does not support 32bit PVH guests\n");
>> -        return -EINVAL;
>> -    }
>> -
>>       if ( !may_switch_mode(d) )
>>           return -EACCES;
>>       if ( has_32bit_shinfo(d) )
>>           return 0;
>>   
>> -    d->arch.is_32bit_pv = d->arch.has_32bit_shinfo = 1;
>> +    d->arch.has_32bit_shinfo = 1;
>> +    if ( is_pv_domain(d) )
>> +        d->arch.is_32bit_pv = 1;
>>   
>>       for_each_vcpu( d, v )
>>       {
>>           rc = setup_compat_arg_xlat(v);
>>           if ( !rc )
>> -            rc = setup_compat_l4(v);
>> +        {
>> +            if ( !is_pvh_domain(d) )
>> +                rc = setup_compat_l4(v);
>> +            else
>> +                rc = hvm_set_mode(v, 4);
>> +        }
>>           if ( rc )
>>               goto undo_and_fail;
>>       }
>> @@ -410,7 +410,7 @@ int switch_compat(struct domain *d)
>>       {
>>           free_compat_arg_xlat(v);
>>   
>> -        if ( !pagetable_is_null(v->arch.guest_table) )
>> +        if ( !is_pvh_domain(d) && !pagetable_is_null(v->arch.guest_table) )
>>               release_compat_l4(v);
>>       }
>>   
> And no respective change to switch_native()?



>
>> @@ -6491,6 +6490,20 @@ enum hvm_intblk nhvm_interrupt_blocked(struct vcpu *v)
>>       return hvm_funcs.nhvm_intr_blocked(v);
>>   }
>>   
>> +int hvm_set_mode(struct vcpu *v, int mode)
>> +{
>> +    if ( mode == 4 )
>> +    {
>> +        v->arch.hvm_vcpu.guest_efer &= ~(EFER_LMA | EFER_LME);
>> +        hvm_update_guest_efer(v);
>> +    }
>> +
>> +    if ( hvm_funcs.set_mode )
>> +        return hvm_funcs.set_mode(v, mode);
>> +
>> +    return 0;
>> +}
> -EOPNOTSUPP?

Why do you think this should be an error? I probably will need to update 
this to handle mode==8 for calls from switch_native() as you pointed out 
above but in general it seems to me it should be OK if this procedure 
doesn't do anything. Below too.

-boris

>
>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> @@ -1763,6 +1763,22 @@ static void vmx_enable_msr_exit_interception(struct domain *d)
>>                                            MSR_TYPE_W);
>>   }
>>   
>> +int vmx_set_mode(struct vcpu *v, int mode)
>> +{
>> +
>> +    if ( !is_pvh_vcpu(v) )
>> +        return 0;
>> +
>> +    if ( mode == 4 )
>> +    {
>> +        vmx_vmcs_enter(v);
>> +        __vmwrite(GUEST_CS_AR_BYTES, 0xc09b);
>> +        vmx_vmcs_exit(v);
>> +    }
>> +
>> +    return 0;
>> +}
> Again -EOPNOTSUPP?
>
> Jan
>