Wincy Van wrote on 2015-01-28: > On Wed, Jan 28, 2015 at 8:33 PM, Zhang, Yang Z <yang.z.zhang@xxxxxxxxx> > wrote: >>>> >>>> You are right, but this is not fit for all the cases, we should >>>> custom the nested_msr_bitmap. >>>> e.g. Currently L0 wants to intercept some of the x2apic msrs' reading: >>>> if (enable_apicv) { >>>> for (msr = 0x800; msr <= 0x8ff; msr++) >>>> vmx_disable_intercept_msr_read_x2apic(msr); /* >>>> According SDM, in x2apic mode, the whole id reg >>>> is >>> used. >>>> * But in KVM, it only use the highest eight bits. >>>> Need > to >>>> * intercept it */ >>>> vmx_enable_intercept_msr_read_x2apic(0x802); /* TMCCT >>>> */ vmx_enable_intercept_msr_read_x2apic(0x839); /* >>>> TPR */ vmx_disable_intercept_msr_write_x2apic(0x808); > /* >>>> EOI >>> */ >>>> vmx_disable_intercept_msr_write_x2apic(0x80b); /* >>>> SELF-IPI */ >>>> vmx_disable_intercept_msr_write_x2apic(0x83f); >>>> } >>>> But L1 may not want this. So I think we are better to deal with >>>> the >>> >>> Actually, from L0's point, it is totally unaware of the L2. The only >>> thing L0 aware is that the CPU should follow L0's configuration when >>> VCPU is running. So if L0 wants to trap a msr, then the read operation >>> to this msr should cause vmexit unconditionally no matter who is >>> running(who means L1, L2, L3.....). >>> >>>> msrs seperately, there is not a common way suit for all the cases. >>>> If other features want to intercept a msr in nested entry, they >>>> can put the custom code in nested_vmx_merge_msr_bitmap. >>> >>> Yes, if other features want to do it in 'nested' entry, they can >>> fill nested_vmx_merge_msr_bitmap. But if in non-nested case, it >>> should be our responsibly to handle it correctly, how about add following check: >>> >>> if (type & MSR_TYPE_R && !test_bit(msr, vmcs01_msr_bitmap) && >>> !test_bit(msr, msr_bitmap_l1 + 0x000 / f)) >>> __clear_bit(msr, msr_bitmap_nested + 0x000 / f); >> >> >> Anyway, this is not necessary for your current patch. We can consider >> it later if there really have other features will use it. >> > > Yep, I know what you mean now, for other msrs which are not forwarded > access by a mechanism like virtual-apic page, we should intercept it > unconditionally. I think we should ensure the msr can be allowed > before call nested_vmx_disable_intercept_for_msr, if L0 want to > intercept it, just do not call nested_vmx_disable_intercept_for_msr. Yes, this is a solution. But I prefer to handle it in nested code path since others may not familiar with nested and may block it by mistake. > > !test_bit(msr, vmcs01_msr_bitmap) will introduce a problem that some > of the msrs will be affcted by vmcs01_msr_bitmap, TMCCT and TPR, for example. > Intercept reading for these msrs is okay, but it is not efficient. TMCCT is always trapped by most VMM. I don't think TPR is trapped in KVM. > > Thanks, > Wincy Best regards, Yang ��.n��������+%������w��{.n�����o�^n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�