Re: [Xen-devel] [PATCH RFC v13 06/20] pvh: vmx-specific changes

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.11.13 at 15:14, George Dunlap <george.dunlap@xxxxxxxxxxxxx> wrote:
> On 26/09/13 16:29, Jan Beulich wrote:
>>>>> On 23.09.13 at 18:49, George Dunlap <george.dunlap@xxxxxxxxxxxxx> wrote:
>>> +    required = X86_CR4_PAE | X86_CR4_VMXE | X86_CR4_OSFXSR;
>>> +    if ( (real_cr4_to_pv_guest_cr4(mmu_cr4_features) & required) != 
>>> required )
>>> +    {
>>> +        printk(XENLOG_G_INFO "PVH: required CR4 features not 
>>> available:%lx\n",
>>> +               required);
>>> +        return -EOPNOTSUPP;
>>> +    }
>>> +
>>> +    /* Check for required configuration options */
>>> +    if ( !paging_mode_hap(v->domain) )
>>> +    {
>>> +        printk(XENLOG_G_INFO "HAP is required for PVH guest.\n");
>>> +        return -EINVAL;
>>> +    }
>>> +    /*
>>> +     * If rdtsc exiting is turned on and it goes thru 
>>> emulate_privileged_op,
>>> +     * then pv_vcpu.ctrlreg must be added to the pvh struct.
>>> +     */
>>> +    if ( v->domain->arch.vtsc )
>>> +    {
>>> +        printk(XENLOG_G_INFO
>>> +               "At present PVH only supports the default timer mode\n");
>>> +        return -EINVAL;
>>> +    }
>> ... but all of these are pretty generic (apart from the X86_CR4_VMXE
>> in the CR4 mask checked above, but I wonder whether that
>> shouldn't be checked much earlier - for HVM guests no such check
>> exists here afaik).
> 
> The hap check can be removed and checked in hvm_domain_initialise(), and 
> the vtsc one moved to tsc_set_info().
> 
> Is it really necessary to check these bits in cr4?  Or is this more or 
> less an ASSERT() to make sure people haven't accidentally disabled these 
> in the real->guest cr4 conversion?

That's a question to Mukesh really; I don't know the motivation
for them being there.

Apart from that the check is also guest independent, and hence
could be done - if needed at all - once (when setting the proposed
pvh_enabled).

So
- PAE is always there and doesn't require checking,
- VMXE would better be replaced by hvm_enabled being a prereq
  for pvh_enabled,
- FXSR is also always there (now that we haven't been supporting
  32-bit hosts for quite a while), and it escapes me what relation
  this has to PVH anyway.

Jan


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