Wincy Van wrote on 2015-01-28:
> 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
>>> vmcs12->in 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
>>> vmcs12->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.
Mmm... I think if the bit is setting in vmcs01->msr_bitmap means whenever the VCPU(no matter in nested guest mode or not) accesses this msr should cause vmexit, not only L1. That's why need to construct the vmcs02->msr_bitmap based on (vmcs01->msr_bitmap ORed vmcs12->msr_bitmap).
>
> Thanks,
> Wincy
Best regards,
Yang
��.n��������+%������w��{.n�����o�^n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�
