On Wed, Jan 28, 2015 at 5:37 AM, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote: > > > On 24/01/2015 11:21, Wincy Van wrote: >> + memset(vmx_msr_bitmap_nested, 0xff, PAGE_SIZE); > > Most bytes are always 0xff. It's better to initialize it to 0xff once, > and set the bit here if !nested_cpu_has_virt_x2apic_mode(vmcs12). Indeed, will do. > >> + if (nested_cpu_has_virt_x2apic_mode(vmcs12)) > > Please add braces here, because of the /* */ command below. Will do. > >> + /* 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); >> >> +static inline int nested_vmx_check_virt_x2apic(struct kvm_vcpu *vcpu, >> + struct vmcs12 *vmcs12) >> +{ >> + if (nested_cpu_has2(vmcs12, SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES)) >> + return -EINVAL; > > No need for this function and nested_cpu_has_virt_x2apic_mode. Just > inline them in their caller(s). Same for other cases throughout the series. > Do you mean that we should also inline the same functions in the other patches of this patch set? I think these functions will keep the code tidy, just like the functions as nested_cpu_has_preemption_timer, nested_cpu_has_ept, etc. -- 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