2016-04-07 03:20-0500, Suravee Suthikulpanit: > This patch introduces AVIC-related data structure, and AVIC > initialization code. > > There are three main data structures for AVIC: > * Virtual APIC (vAPIC) backing page (per-VCPU) > * Physical APIC ID table (per-VM) > * Logical APIC ID table (per-VM) > > Currently, AVIC is disabled by default. Users can manually > enable AVIC via kernel boot option kvm-amd.avic=1 or during > kvm-amd module loading with parameter avic=1. > > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx> > --- > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > @@ -14,6 +14,9 @@ > * the COPYING file in the top-level directory. > * > */ > + > +#define pr_fmt(fmt) "SVM: " fmt > + > #include <linux/kvm_host.h> > > #include "irq.h" > @@ -78,6 +81,11 @@ MODULE_DEVICE_TABLE(x86cpu, svm_cpu_id); > #define TSC_RATIO_MIN 0x0000000000000001ULL > #define TSC_RATIO_MAX 0x000000ffffffffffULL > > +#define AVIC_HPA_MASK ~((0xFFFULL << 52) || 0xFFF) > + > +/* NOTE: Current max index allowed for physical APIC ID table is 255 */ > +#define AVIC_PHYSICAL_ID_MAX 0xFF 0xff is broadcast, so shouldn't the actual last one be 0xfe? > @@ -234,6 +257,18 @@ enum { > /* TPR and CR2 are always written before VMRUN */ > #define VMCB_ALWAYS_DIRTY_MASK ((1U << VMCB_INTR) | (1U << VMCB_CR2)) > > +#define VMCB_AVIC_APIC_BAR_MASK 0xFFFFFFFFFF000ULL > + > +static inline bool svm_vcpu_avic_enabled(struct vcpu_svm *svm) > +{ > + return (avic && (svm->vmcb->control.int_ctl & AVIC_ENABLE_MASK)); > +} I think it'd be better to use kvm_vcpu_apicv_active() instead. In any case, have just one function to tell whether avic is enabled. > @@ -1000,6 +1042,24 @@ static void svm_adjust_tsc_offset_guest(struct kvm_vcpu *vcpu, s64 adjustment) > mark_dirty(svm->vmcb, VMCB_INTERCEPTS); > } > > +static void avic_init_vmcb(struct vcpu_svm *svm) > +{ > + struct vmcb *vmcb = svm->vmcb; > + struct kvm_arch *vm_data = &svm->vcpu.kvm->arch; > + phys_addr_t bpa = page_to_phys(svm->avic_backing_page); > + phys_addr_t lpa = page_to_phys(vm_data->avic_logical_id_table_page); > + phys_addr_t ppa = page_to_phys(vm_data->avic_physical_id_table_page); > + > + if (!vmcb) > + return; We can call this function only from init_vmcb and therefore drop this return. > @@ -1113,6 +1173,142 @@ static void init_vmcb(struct vcpu_svm *svm) > +static u64 *avic_get_physical_id_entry(struct kvm_vcpu *vcpu, int index) > +{ > + u64 *avic_physical_id_table; > + struct kvm_arch *vm_data = &vcpu->kvm->arch; > + > + /* Note: APIC ID = 0xff is used for broadcast. > + * APIC ID > 0xff is reserved. > + */ > + if (index >= 0xff) Isn't this AVIC_PHYSICAL_ID_MAX? > +static int avic_init_backing_page(struct kvm_vcpu *vcpu) > +{ > + int ret; > + u64 *entry, new_entry; > + int id = vcpu->vcpu_id; > + struct vcpu_svm *svm = to_svm(vcpu); > + > + ret = avic_init_access_page(vcpu); > + if (ret) > + return ret; > + > + if (id >= AVIC_PHYSICAL_ID_MAX) > + return -EINVAL; > + > + if (!svm->vcpu.arch.apic->regs) > + return -EINVAL; > + > + svm->avic_backing_page = virt_to_page(svm->vcpu.arch.apic->regs); > + > + avic_init_vmcb(svm); avic_init_vmcb() should be dropped from here. It does nothing. > +static void avic_vcpu_uninit(struct kvm_vcpu *vcpu) > +{ > + struct vcpu_svm *svm = to_svm(vcpu); > + > + if (!avic) > + return; > + > + if (svm->avic_physical_id_cache) > + svm->avic_physical_id_cache = NULL; > +} I think it's safe to drop avic_vcpu_uninit(). This function is only called when we are freeing the vcpu, so the value is not used anymore. > static void svm_refresh_apicv_exec_ctrl(struct kvm_vcpu *vcpu) > { > + struct vcpu_svm *svm = to_svm(vcpu); > + struct vmcb *vmcb = svm->vmcb; > + > + if (!avic) > + return; > + > + vmcb->control.int_ctl &= ~AVIC_ENABLE_MASK; (This function doesn't really "refresh" avic, it just disables. VMX isn't much better, so it's bearable to do so ...) -- 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