Sean Christopherson <sean.j.christopherson@xxxxxxxxx> writes:
> On Thu, Mar 05, 2020 at 07:37:25PM +0100, Vitaly Kuznetsov wrote:
>> As stated in alloc_vmxon_regions(), VMXON region needs to be tagged with
>> revision id from MSR_IA32_VMX_BASIC even in case of eVMCS. The logic to
>> do so is not very straightforward: first, we set
>> hdr.revision_id = KVM_EVMCS_VERSION in alloc_vmcs_cpu() just to reset it
>> back to vmcs_config.revision_id in alloc_vmxon_regions(). Simplify this by
>> introducing 'enum vmcs_type' parameter to alloc_vmcs_cpu().
>>
>> No functional change intended.
>>
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
>> ---
>
> ...
>
>> + * However, even though not explicitly documented by TLFS, VMXArea
>> + * passed as VMXON argument should still be marked with revision_id
>> + * reported by physical CPU.
>
> LOL, nice.
>
>
>> + */
>> + if (type != VMXON_REGION && static_branch_unlikely(&enable_evmcs))
>> vmcs->hdr.revision_id = KVM_EVMCS_VERSION;
>> else
>> vmcs->hdr.revision_id = vmcs_config.revision_id;
>>
>> - if (shadow)
>> + if (type == SHADOW_VMCS_REGION)
>> vmcs->hdr.shadow_vmcs = 1;
>> return vmcs;
>> }
>
>> -struct vmcs *alloc_vmcs_cpu(bool shadow, int cpu, gfp_t flags);
>> +enum vmcs_type {
>> + VMXON_REGION,
>> + VMCS_REGION,
>> + SHADOW_VMCS_REGION,
>> +};
>> +
>> +struct vmcs *alloc_vmcs_cpu(enum vmcs_type type, int cpu, gfp_t flags);
>> void free_vmcs(struct vmcs *vmcs);
>> int alloc_loaded_vmcs(struct loaded_vmcs *loaded_vmcs);
>> void free_loaded_vmcs(struct loaded_vmcs *loaded_vmcs);
>> @@ -498,8 +504,8 @@ void loaded_vmcs_clear(struct loaded_vmcs *loaded_vmcs);
>>
>> static inline struct vmcs *alloc_vmcs(bool shadow)
>
> I think it'd be cleaner overall to take "enum vmcs_type" in alloc_vmcs().
> Then the ternary operator goes away and the callers (all two of 'em) are
> self-documenting.
Ya, it didn't seem to be needed with my initial suggestion to rename
alloc_vmcs_cpu() to alloc_vmx_area_cpu() because in case we think of
VMXON region as something different from VMCS we have only two options:
normal VMCS or shadow VMCS and bool flag works perfectly.
v3 is on the way, stay tuned!
--
Vitaly
