On Mon, Dec 24, 2012 at 02:35:35AM +0000, Zhang, Yang Z wrote: > Zhang, Yang Z wrote on 2012-12-24: > > Gleb Natapov wrote on 2012-12-20: > >> On Mon, Dec 17, 2012 at 01:30:50PM +0800, Yang Zhang wrote: > >>> basically to benefit from apicv, we need clear MSR bitmap for > >>> corresponding x2apic MSRs: > >>> 0x800 - 0x8ff: no read intercept for apicv register virtualization > >>> TPR,EOI,SELF-IPI: no write intercept for virtual interrupt delivery > >> We do not set "Virtualize x2APIC mode" bit in secondary execution > >> control. If I read the spec correctly without that those MSR read/writes > >> will go straight to physical local APIC. > > Right. Now it cannot get benefit, but we may enable it in future and then > > we can > > benefit from it. Without enabling it you cannot disable MSR intercept for x2apic MSRs.
> how about to add the following check: > if (apicv_enabled && virtual_x2apic_enabled) > clear_msr(); > I do not understand what do you mean here. > > >>> Signed-off-by: Yang Zhang <[email protected]> > >>> Signed-off-by: Kevin Tian <[email protected]> > >>> --- > >>> arch/x86/kvm/vmx.c | 62 > >>> ++++++++++++++++++++++++++++++++++++++++++++++------ 1 files changed, > >>> 55 insertions(+), 7 deletions(-) > >>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > >>> index be66c3e..9b5e7a2 100644 > >>> --- a/arch/x86/kvm/vmx.c > >>> +++ b/arch/x86/kvm/vmx.c > >>> @@ -3773,7 +3773,10 @@ static void free_vpid(struct vcpu_vmx *vmx) > >>> spin_unlock(&vmx_vpid_lock); > >>> } > >>> -static void __vmx_disable_intercept_for_msr(unsigned long > >>> *msr_bitmap, u32 msr) +#define MSR_TYPE_R 1 +#define MSR_TYPE_W 2 > >>> +static void __vmx_disable_intercept_for_msr(unsigned long > >>> *msr_bitmap, + u32 msr, int type) > >>> { > >>> int f = sizeof(unsigned long); > >>> @@ -3786,20 +3789,52 @@ static void > >> __vmx_disable_intercept_for_msr(unsigned long *msr_bitmap, u32 msr) > >>> * We can control MSRs 0x00000000-0x00001fff and > >>> 0xc0000000-0xc0001fff. */ if (msr <= 0x1fff) { > >>> - __clear_bit(msr, msr_bitmap + 0x000 / f); /* read-low */ > >>> - __clear_bit(msr, msr_bitmap + 0x800 / f); /* write-low */ > >>> + if (type & MSR_TYPE_R) > >>> + /* read-low */ > >>> + __clear_bit(msr, msr_bitmap + 0x000 / f); > >>> + > >>> + if (type & MSR_TYPE_W) > >>> + /* write-low */ > >>> + __clear_bit(msr, msr_bitmap + 0x800 / f); > >>> + > >>> } else if ((msr >= 0xc0000000) && (msr <= 0xc0001fff)) { > >>> msr &= 0x1fff; > >>> - __clear_bit(msr, msr_bitmap + 0x400 / f); /* read-high */ > >>> - __clear_bit(msr, msr_bitmap + 0xc00 / f); /* write-high */ > >>> + if (type & MSR_TYPE_R) > >>> + /* read-high */ > >>> + __clear_bit(msr, msr_bitmap + 0x400 / f); > >>> + > >>> + if (type & MSR_TYPE_W) > >>> + /* write-high */ > >>> + __clear_bit(msr, msr_bitmap + 0xc00 / f); > >>> + > >>> } > >>> } > >>> > >>> static void vmx_disable_intercept_for_msr(u32 msr, bool longmode_only) > >>> { > >>> if (!longmode_only) > >>> - __vmx_disable_intercept_for_msr(vmx_msr_bitmap_legacy, msr); > >>> - __vmx_disable_intercept_for_msr(vmx_msr_bitmap_longmode, msr); > >>> + __vmx_disable_intercept_for_msr(vmx_msr_bitmap_legacy, + > >>> msr, > >>> MSR_TYPE_R | MSR_TYPE_W); > >>> + __vmx_disable_intercept_for_msr(vmx_msr_bitmap_longmode, + > >>> msr, > >>> MSR_TYPE_R | MSR_TYPE_W); +} + +static void > >>> vmx_disable_intercept_for_msr_read(u32 msr, bool longmode_only) +{ + > >>> if (!longmode_only) > >>> + __vmx_disable_intercept_for_msr(vmx_msr_bitmap_legacy, + > >>> msr, > >>> MSR_TYPE_R); + > >>> __vmx_disable_intercept_for_msr(vmx_msr_bitmap_longmode, + > >>> msr, > >>> MSR_TYPE_R); +} + +static void vmx_disable_intercept_for_msr_write(u32 > >>> msr, bool longmode_only) +{ + if (!longmode_only) > >>> + __vmx_disable_intercept_for_msr(vmx_msr_bitmap_legacy, + > >>> msr, > >>> MSR_TYPE_W); + > >>> __vmx_disable_intercept_for_msr(vmx_msr_bitmap_longmode, + > >>> msr, > >>> MSR_TYPE_W); > >>> } > >>> > >>> /* @@ -7633,6 +7668,19 @@ static int __init vmx_init(void) > >>> vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_ESP, false); > >>> vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_EIP, false); > >>> + if (enable_apicv_reg_vid) { > >>> + int msr; > >>> + for (msr = 0x800; msr <= 0x8ff; msr++) > >>> + vmx_disable_intercept_for_msr_read(msr, false); > >>> + > >>> + /* TPR */ > >>> + vmx_disable_intercept_for_msr_write(0x808, false); > >>> + /* EOI */ > >>> + vmx_disable_intercept_for_msr_write(0x80b, false); > >>> + /* SELF-IPI */ > >>> + vmx_disable_intercept_for_msr_write(0x83f, false); > >>> + } > >>> + > >>> if (enable_ept) { > >>> kvm_mmu_set_mask_ptes(0ull, > >>> (enable_ept_ad_bits) ? VMX_EPT_ACCESS_BIT : 0ull, > >>> -- > >>> 1.7.1 > >> > >> -- > >> Gleb. > > > > > > Best regards, > > Yang > > > > > Best regards, > Yang > -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
