Sorry for not explain clearly. This patch replaces vmcs_write32_fixedbits() with adjust_vmx_controls(), and add some relevant fields to vmcs_config for the global using.
Put vmcs situation in global variable enables us using it to check current vmcs condition and deal with different types of CPU, for we may add some feature which is not supported by all types of CPU. And adjust_vmx_controls() also offers a optional(all filled by 0 now, but will be extended after) feature test . In the future, we can decide how VMCS would be filled by detecting CPU's capability in setup_vmcs_config(). I attached modified patch. If you need more information, please let me know. Thanks Yang, Sheng -----Original Message----- From: Avi Kivity [mailto:[EMAIL PROTECTED] Sent: 2007年7月26日 13:20 To: Yang, Sheng Cc: [email protected] Subject: Re: [kvm-devel] [PATCH]Abstract vmcs feature detect part Yang, Sheng wrote: > This patch changes a method to check cpu capability, so we can set vmcs > more conveniently. > > Please explain the motivation for the change. What is more convenient? > > -static __init void setup_vmcs_descriptor(void) > +static __init u32 adjust_vmx_controls(u32 ctl_min, u32 ctl_opt, u32 > msr) > +{ > + u32 vmx_msr_low, vmx_msr_high, ctl = ctl_min | ctl_opt; > Tabs not spaces for indentation. If you initialize a variable during definition, put it on its own line. > + > + rdmsr(msr, vmx_msr_low, vmx_msr_high); > + > + ctl &= vmx_msr_high; /* bit == 0 in high word ==> must be zero */ > + ctl |= vmx_msr_low; /* bit == 1 in low word ==> must be one */ > + > + /* Ensure minimum (required) set of control bits are supported. */ > + if (ctl_min & ~ctl) return 0; > No single-line ifs, please. > + > + return ctl; > +} > + > This replaces vmcs_write32_fixedbits(). Why? And if it is necessary, it should be in a separate patch. > +static __init int setup_vmcs_config(void) > { > u32 vmx_msr_low, vmx_msr_high; > + u32 min, opt; > + u32 _pin_based_exec_control = 0; > + u32 _cpu_based_exec_control = 0; > + u32 _vmexit_control = 0; > + u32 _vmentry_control = 0; > + > + min = (PIN_BASED_EXT_INTR_MASK | > + PIN_BASED_NMI_EXITING); > While we're limited to 80 columns, there's no reason to stop at 40. > + opt = 0; > + _pin_based_exec_control = adjust_vmx_controls( > + min, opt, MSR_IA32_VMX_PINBASED_CTLS); > + if (_pin_based_exec_control == 0) { > + return -1; > + } > Single statement if () bodies should be without braces. > + > + min = (CPU_BASED_HLT_EXITING > + | CPU_BASED_CR8_LOAD_EXITING /* 20.6.2 */ > + | CPU_BASED_CR8_STORE_EXITING /* 20.6.2 */ > + | CPU_BASED_USE_IO_BITMAPS /* 20.6.2 */ > + | CPU_BASED_MOV_DR_EXITING > + | CPU_BASED_USE_TSC_OFFSETING /* 21.3 */ > + ); > + opt = 0; > + _cpu_based_exec_control = adjust_vmx_controls( > + min, opt, MSR_IA32_VMX_PROCBASED_CTLS); > + if (_cpu_based_exec_control == 0) { > + return -1; > + } > + > + min = VM_EXIT_HOST_ADDR_SPACE_SIZE; > + opt = 0; > + _vmexit_control = adjust_vmx_controls( > + min, opt, MSR_IA32_VMX_EXIT_CTLS); > + if (_vmexit_control == 0) { > + return -1; > + } > + > + min = opt = 0; > + _vmentry_control = adjust_vmx_controls( > + min, opt, MSR_IA32_VMX_ENTRY_CTLS); > + if (_vmentry_control == 0) { > + return -1; > + } > > rdmsr(MSR_IA32_VMX_BASIC, vmx_msr_low, vmx_msr_high); > - vmcs_descriptor.size = vmx_msr_high & 0x1fff; > - vmcs_descriptor.order = get_order(vmcs_descriptor.size); > - vmcs_descriptor.revision_id = vmx_msr_low; > + > + vmcs_config.size = vmx_msr_high & 0x1fff; > + vmcs_config.order = get_order(vmcs_config.size); > + vmcs_config.revision_id = vmx_msr_low; > + > + vmcs_config.pin_based_exec_ctrl = _pin_based_exec_control; > + vmcs_config.cpu_based_exec_ctrl = _cpu_based_exec_control; > + vmcs_config.vmexit_ctrl = _vmexit_control; > + vmcs_config.vmentry_ctrl = _vmentry_control; > + > + /* IA-32 SDM Vol 3B: VMCS size is never greater than 4kB. */ > + if ((vmx_msr_high & 0x1fff) > PAGE_SIZE) { > + return -1; > + } > + > +#ifdef __x86_64__ > + /* IA-32 SDM Vol 3B: 64-bit CPUs always have > VMX_BASIC_MSR[48]==0. */ > + if (vmx_msr_high & (1u<<16)) { > + return -1; > + } > +#endif > + > + /* Require Write-Back (WB) memory type for VMCS accesses. */ > + if (((vmx_msr_high >> 18) & 15) != 6) { > + return -1; > + } > + return 0; > } > This looks like a rewrite. Please make patches incremental. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic.
vmcs_config.patch
Description: vmcs_config.patch
------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________ kvm-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/kvm-devel
