[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v1 4/6] vvmx: add hvm_max_vmx_msr_policy
>>> On 26.06.17 at 12:44, <sergey.dyasli@xxxxxxxxxx> wrote: > --- a/xen/arch/x86/hvm/vmx/vmcs.c > +++ b/xen/arch/x86/hvm/vmx/vmcs.c > @@ -244,6 +244,8 @@ static u32 adjust_vmx_controls( > return ctl; > } > > +void calculate_hvm_max_policy(void); As said for a prior patch, this once again needs to move to a header which is also being included by the producer. > --- a/xen/arch/x86/hvm/vmx/vvmx.c > +++ b/xen/arch/x86/hvm/vmx/vvmx.c > @@ -1941,6 +1941,8 @@ int nvmx_handle_invvpid(struct cpu_user_regs *regs) > return X86EMUL_OKAY; > } > > +struct vmx_msr_policy __read_mostly hvm_max_vmx_msr_policy; Wouldn't vvmx_max_msr_policy be unambiguous enough a name, but shorter to type? > @@ -1948,6 +1950,134 @@ int nvmx_handle_invvpid(struct cpu_user_regs *regs) > (((__emul_value(enable1, default1) & host_value) & (~0ul << 32)) | \ > ((uint32_t)(__emul_value(enable1, default1) | host_value))) > > +void __init calculate_hvm_max_policy(void) This is not a suitable name for a VMX specific function. I should have noticed and said this in patch 2 already, so please consider it applicable there too. > +{ > + struct vmx_msr_policy *p = &hvm_max_vmx_msr_policy; > + uint64_t data, *msr; > + u32 default1_bits; > + > + *p = raw_vmx_msr_policy; > + > + /* XXX: vmcs_revision_id for nested virt */ There was no such comment (presumably indicating something that yet needs doing) in the old code - what's this about? Can't this be implemented instead of such a comment be added? > + /* MSR_IA32_VMX_VMCS_ENUM */ > + /* The max index of VVMCS encoding is 0x1f. */ > + data = 0x1f << 1; > + msr = &p->msr[MSR_IA32_VMX_VMCS_ENUM - MSR_IA32_VMX_BASIC]; > + *msr = data; > + > + /* MSR_IA32_VMX_CR0_FIXED0 */ > + /* PG, PE bits must be 1 in VMX operation */ > + data = X86_CR0_PE | X86_CR0_PG; > + msr = &p->msr[MSR_IA32_VMX_CR0_FIXED0 - MSR_IA32_VMX_BASIC]; > + *msr = data; > + > + /* MSR_IA32_VMX_CR0_FIXED1 */ > + /* allow 0-settings for all bits */ > + data = 0xffffffff; > + msr = &p->msr[MSR_IA32_VMX_CR0_FIXED1 - MSR_IA32_VMX_BASIC]; > + *msr = data; > + > + /* MSR_IA32_VMX_CR4_FIXED0 */ > + /* VMXE bit must be 1 in VMX operation */ > + data = X86_CR4_VMXE; > + msr = &p->msr[MSR_IA32_VMX_CR4_FIXED0 - MSR_IA32_VMX_BASIC]; > + *msr = data; I don't see a need for using "data" as an intermediate variable in any of the three cases above. > + /* MSR_IA32_VMX_CR4_FIXED1 */ > + /* Treated dynamically */ > + > + /* MSR_IA32_VMX_MISC */ > + /* Do not support CR3-target feature now */ > + msr = &p->msr[MSR_IA32_VMX_MISC - MSR_IA32_VMX_BASIC]; > + *msr = *msr & ~VMX_MISC_CR3_TARGET; &= > + /* MSR_IA32_VMX_EPT_VPID_CAP */ > + data = nept_get_ept_vpid_cap(); > + msr = &p->msr[MSR_IA32_VMX_EPT_VPID_CAP - MSR_IA32_VMX_BASIC]; > + *msr = data; No need to use "data" again. > + /* MSR_IA32_VMX_VMFUNC is N/A */ > + p->available &= ~0x20000; Please use an expression again here. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |