> -----Original Message----- > From: Avi Kivity [mailto:avi@xxxxxxxxxx] > Sent: Friday, June 29, 2012 10:52 PM > To: Mao, Junjie > Cc: 'kvm@xxxxxxxxxxxxxxx' > Subject: Re: [PATCH v4] KVM: x86: Implement PCID/INVPCID for guests with > EPT > > On 06/29/2012 05:37 AM, Mao, Junjie wrote: > > > > > > > > > > > static void vmx_set_supported_cpuid(u32 func, struct > > > > kvm_cpuid_entry2 > > > > *entry) @@ -6610,6 +6641,9 @@ static void prepare_vmcs02(struct > > > kvm_vcpu *vcpu, struct vmcs12 *vmcs12) > > > > page_to_phys(vmx->nested.apic_access_page)); > > > > } > > > > > > > > + /* Explicitly disable INVPCID until PCID for L2 guest is supported > */ > > > > + exec_control &= ~SECONDARY_EXEC_ENABLE_INVPCID; > > > > + > > > > > > We can't just disable it without the guest knowing. If we don't > > > expose INCPCID through the MSR, then we should fail VMKLAUNCH or > > > VMRESUME is this bit is set. > > > > I think this means I can replace the code here with a check in > nested_vmx_run. Do I understand correctly? > > Correct, but the check already exists: > if (!vmx_control_verify(vmcs12->cpu_based_vm_exec_control, > nested_vmx_procbased_ctls_low, > nested_vmx_procbased_ctls_high) || > !vmx_control_verify(vmcs12->secondary_vm_exec_control, > nested_vmx_secondary_ctls_low, > nested_vmx_secondary_ctls_high) || > !vmx_control_verify(vmcs12->pin_based_vm_exec_control, > nested_vmx_pinbased_ctls_low, nested_vmx_pinbased_ctls_high) > || > !vmx_control_verify(vmcs12->vm_exit_controls, > nested_vmx_exit_ctls_low, nested_vmx_exit_ctls_high) || > !vmx_control_verify(vmcs12->vm_entry_controls, > nested_vmx_entry_ctls_low, nested_vmx_entry_ctls_high)) > { > nested_vmx_failValid(vcpu, > VMXERR_ENTRY_INVALID_CONTROL_FIELD); > return 1; > } > > So all that is needed is to initializr nested_vmx_entry_ctls_high properly. nested_vmx_entry_ctls_high only contains SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES at present, which means it should be safe to simply remove the code. > > > > > > > > vmcs_write32(SECONDARY_VM_EXEC_CONTROL, > exec_control); > > > > } > > > > > > > > @@ -528,6 +528,10 @@ int kvm_set_cr0(struct kvm_vcpu *vcpu, > > > > unsigned > > > long cr0) > > > > return 1; > > > > } > > > > > > > > + if ((old_cr0 & X86_CR0_PG) && !(cr0 & X86_CR0_PG) && > > > > + kvm_read_cr4_bits(vcpu, X86_CR4_PCIDE)) > > > > + return 1; > > > > > > Why check old_cr0? > > > > MOV to CR0 causes a general-protection exception if it would clear CR0.PG to > 0 while CR4.PCIDE = 1. This check reflects the restriction. > > It should not be possible to have cr0.pg=0 and cr4.pcide=1 in the first place. > We are guaranteed that old_cr0.pg=1. > I see. Thanks for the suggestion. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
