On Wed, Jan 28, 2015 at 7:25 PM, Zhang, Yang Z <yang.z.zhang@xxxxxxxxx> wrote: > Wincy Van wrote on 2015-01-28: >> On Wed, Jan 28, 2015 at 4:05 PM, Zhang, Yang Z <yang.z.zhang@xxxxxxxxx> >> wrote: >>>> @@ -8344,7 +8394,68 @@ static int >>>> nested_vmx_check_msr_bitmap_controls(struct kvm_vcpu *vcpu, static >>>> inline bool nested_vmx_merge_msr_bitmap(struct kvm_vcpu *vcpu, >>>> struct vmcs12 >>>> *vmcs12) { - return false; + struct page *page; + >>>> unsigned long *msr_bitmap; + + if >>>> (!nested_cpu_has_virt_x2apic_mode(vmcs12)) + return >>>> false; + + page = nested_get_page(vcpu, vmcs12->msr_bitmap); + >>>> if (!page) { + WARN_ON(1); + return >>>> false; + } + msr_bitmap = (unsigned long *)kmap(page); + >>>> if (!msr_bitmap) { + >>>> nested_release_page_clean(page); + WARN_ON(1); + >>>> return false; + } + + memset(vmx_msr_bitmap_nested, >>>> 0xff, PAGE_SIZE); + + if >>>> (nested_cpu_has_virt_x2apic_mode(vmcs12)) + /* TPR is >>>> allowed */ + >>>> nested_vmx_disable_intercept_for_msr(msr_bitmap, + >>>> vmx_msr_bitmap_nested, + >>>> APIC_BASE_MSR + (APIC_TASKPRI >> 4), + >>>> MSR_TYPE_R | MSR_TYPE_W); >>> >>> I didn't understand what this function does? Per my understanding, >>> you only >> need to set the (vmx_msr_bitmap_nested = vmcs01->msr_bitmap | >> vmcs12->msr_bitmap) and inject the nested vmexit to L1 if the bit in >> vmcs12->msr_bitmap is setting. Am I missing some patches? >> >> In the beginning, I want to do "vmcs01->msr_bitmap | >> vmcs12->msr_bitmap", but I remember that there isn't a instruction to >> do a bit or operation in two pages effectively, so I do the bit or >> operation in nested_vmx_disable_intercept_for_msr. If the hardware do >> not support this, I think it is faster if we deal with the bits on demand. >> nested_vmx_merge_msr_bitmap is used to merge L0's and L1's bitmaps, >> any features can put their logic here. > > You construct the nested_msr_bitmap based on vmcs12->msr_bitmap, what happens if vmcs01->msr_bitmap want to trap this msr? > If L0 wants to intercept a msr, we should set vmx_msr_bitmap_legacy(_x2apic) and vmx_msr_bitmap_longmode(_x2apic), and that bitmaps should only be loaded in non-nested entry. Currently we only clear the corresponding bits if L1 enables virtualize x2apic mode, all the other bits are all set. On nested entry, we load nested_msr_bitmap, on nested vmexit, we will restore L0's bitmap. In the nested entry, L0 only care about L2's msr access, not L1's. I think that in nested entry, nested_msr_bitmap is "vmcs01->msr_bitmap" as your mentioned above. Thanks, Wincy -- 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