Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1332762 > unrolled thread

[PART1 RFC 0/9] KVM: x86: Introduce SVM AVIC support

Back to article view | Back to linux.kernel

Contents

  [PART1 RFC 0/9] KVM: x86: Introduce SVM AVIC support Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> - 2016-02-12 15:10 +0100
    [PART1 RFC 4/9] KVM: x86: Detect and Initialize AVIC support Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> - 2016-02-12 15:10 +0100
      Re: [PART1 RFC 4/9] KVM: x86: Detect and Initialize AVIC support Borislav Petkov <bp@alien8.de> - 2016-02-12 15:20 +0100
        Re: [PART1 RFC 4/9] KVM: x86: Detect and Initialize AVIC support Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-02-12 16:50 +0100
    [PART1 RFC 9/9] svm: Manage vcpu load/unload when enable AVIC Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> - 2016-02-12 15:10 +0100
      Re: [PART1 RFC 9/9] svm: Manage vcpu load/unload when enable AVIC Paolo Bonzini <pbonzini@redhat.com> - 2016-02-12 16:50 +0100
    [PART1 RFC 6/9] svm: Add interrupt injection via AVIC Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> - 2016-02-12 15:10 +0100
      Re: [PART1 RFC 6/9] svm: Add interrupt injection via AVIC Borislav Petkov <bp@alien8.de> - 2016-02-12 15:20 +0100
        Re: [PART1 RFC 6/9] svm: Add interrupt injection via AVIC Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-02-12 17:00 +0100
          Re: [PART1 RFC 6/9] svm: Add interrupt injection via AVIC Borislav Petkov <bp@alien8.de> - 2016-02-12 18:20 +0100
            Re: [PART1 RFC 6/9] svm: Add interrupt injection via AVIC Paolo Bonzini <pbonzini@redhat.com> - 2016-02-12 19:30 +0100
              Re: [PART1 RFC 6/9] svm: Add interrupt injection via AVIC Borislav Petkov <bp@alien8.de> - 2016-02-12 19:40 +0100
                Re: [PART1 RFC 6/9] svm: Add interrupt injection via AVIC Paolo Bonzini <pbonzini@redhat.com> - 2016-02-12 20:00 +0100
                  Re: [PART1 RFC 6/9] svm: Add interrupt injection via AVIC Borislav Petkov <bp@alien8.de> - 2016-02-12 20:40 +0100
                    Re: [PART1 RFC 6/9] svm: Add interrupt injection via AVIC Ingo Molnar <mingo@kernel.org> - 2016-02-16 09:00 +0100
                      [PATCH] x86/msr: Document msr-index.h rule for addition Borislav Petkov <bp@alien8.de> - 2016-02-16 09:40 +0100
      Re: [PART1 RFC 6/9] svm: Add interrupt injection via AVIC Paolo Bonzini <pbonzini@redhat.com> - 2016-02-12 17:00 +0100
        Re: [PART1 RFC 6/9] svm: Add interrupt injection via AVIC Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-02-12 17:30 +0100
          Re: [PART1 RFC 6/9] svm: Add interrupt injection via AVIC Paolo Bonzini <pbonzini@redhat.com> - 2016-02-12 19:20 +0100
            Re: [PART1 RFC 6/9] svm: Add interrupt injection via AVIC Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-02-12 20:40 +0100
            Re: [PART1 RFC 6/9] svm: Add interrupt injection via AVIC Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-02-19 13:00 +0100
    [PART1 RFC 5/9] svm: Add VMEXIT handlers for AVIC Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> - 2016-02-12 15:10 +0100
      Re: [PART1 RFC 5/9] svm: Add VMEXIT handlers for AVIC Paolo Bonzini <pbonzini@redhat.com> - 2016-02-12 16:40 +0100
        Re: [PART1 RFC 5/9] svm: Add VMEXIT handlers for AVIC Radim Krčmář <rkrcmar@redhat.com> - 2016-02-15 20:30 +0100
        Re: [PART1 RFC 5/9] svm: Add VMEXIT handlers for AVIC Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-02-16 07:40 +0100
          Re: [PART1 RFC 5/9] svm: Add VMEXIT handlers for AVIC Paolo Bonzini <pbonzini@redhat.com> - 2016-02-16 13:20 +0100
            Re: [PART1 RFC 5/9] svm: Add VMEXIT handlers for AVIC Radim Krčmář <rkrcmar@redhat.com> - 2016-02-16 15:20 +0100
              Re: [PART1 RFC 5/9] svm: Add VMEXIT handlers for AVIC Paolo Bonzini <pbonzini@redhat.com> - 2016-02-16 18:00 +0100
                Re: [PART1 RFC 5/9] svm: Add VMEXIT handlers for AVIC Radim Krčmář <rkrcmar@redhat.com> - 2016-02-16 19:10 +0100
                  Re: [PART1 RFC 5/9] svm: Add VMEXIT handlers for AVIC Radim Krčmář <rkrcmar@redhat.com> - 2016-02-18 15:20 +0100
                    Re: [PART1 RFC 5/9] svm: Add VMEXIT handlers for AVIC Paolo Bonzini <pbonzini@redhat.com> - 2016-02-18 16:00 +0100
                      Re: [PART1 RFC 5/9] svm: Add VMEXIT handlers for AVIC Radim Krčmář <rkrcmar@redhat.com> - 2016-02-18 16:50 +0100
                        Re: [PART1 RFC 5/9] svm: Add VMEXIT handlers for AVIC Paolo Bonzini <pbonzini@redhat.com> - 2016-02-18 17:00 +0100
                          Re: [PART1 RFC 5/9] svm: Add VMEXIT handlers for AVIC Radim Krčmář <rkrcmar@redhat.com> - 2016-02-18 17:30 +0100
                            Re: [PART1 RFC 5/9] svm: Add VMEXIT handlers for AVIC Paolo Bonzini <pbonzini@redhat.com> - 2016-02-18 18:20 +0100
                              Re: [PART1 RFC 5/9] svm: Add VMEXIT handlers for AVIC Paolo Bonzini <pbonzini@redhat.com> - 2016-02-19 12:50 +0100
                    Re: [PART1 RFC 5/9] svm: Add VMEXIT handlers for AVIC Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-02-19 12:40 +0100
    Re: [PART1 RFC 0/9] KVM: x86: Introduce SVM AVIC support Paolo Bonzini <pbonzini@redhat.com> - 2016-02-12 19:20 +0100
      Re: [PART1 RFC 0/9] KVM: x86: Introduce SVM AVIC support Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-02-12 21:00 +0100
        Re: [PART1 RFC 0/9] KVM: x86: Introduce SVM AVIC support Paolo Bonzini <pbonzini@redhat.com> - 2016-02-12 21:10 +0100

Page 1 of 2  [1] 2  Next page →

#1332762 — [PART1 RFC 0/9] KVM: x86: Introduce SVM AVIC support

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

OVERVIEW
========
This patch set is the first of the two-part patch series to introduce 
the new AMD Advance Virtual Interrupt Controller (AVIC) support.

Basically, SVM AVIC hardware virtualizes local APIC registers of each
vCPU via the virtual APIC (vAPIC) backing page. This allows guest access
to certain APIC registers without the need to emulate the hardware behavior
in the hypervisor. More information about AVIC can be found in the
AMD64 Architecture Programmer’s Manual Volume 2 - System Programming.

  http://support.amd.com/TechDocs/24593.pdf

For SVM AVIC, we extend the existing kvm_amd driver to:
  * Check CPUID to detect AVIC support in the processor
  * Program new fields in VMCB to enable AVIC
  * Introduce new AVIC data structures and add code to manage them
  * Handle two new AVIC #VMEXITs
  * Add new interrupt intjection code using vAPIC backing page
    instead of the existing V_IRQ, V_INTR_PRIO, V_INTR_VECTOR,
    and V_IGN_TPR fileds

Currently, this patch series does not enable AVIC by default.
Users can enable SVM AVIC by specifying avic=1 during insmod kvm-amd.

Later, in part 2, we will introduce the IOMMU AVIC support, which
provides speed up for PCI device passthrough use case by allowing
the IOMMU hardware to inject interrupt directly into the guest via
the vAPIC backing page.

PERFORMANCE RESULTS
===================
Currently, AVIC is supported in the AMD family 15h models 6Xh
(Carrizo) processors. Therefore, it is used to collect the 
perforamance data shown below.

Generaly, SVM AVIC alone (w/o IOMMU AVIC) should provide speedup for
IPI interrupt since hypervisor does not require VMEXIT to inject
these interrupts. Also, it should speed up the case when hypervisor
wants to inject an interrupt into a running guest by setting the
corresponded IRR bit in the vAPIC backing page and trigger
AVIC_DOORBELL MSR.

IPI PERFORMANCE
===============
For IPI, I have collected some performance number on 2 and 4 CPU running
hackbech with the following detail:

  hackbench -p -l 100000
  Running in process mode with 10 groups using 40 file descriptors each (== 400 tasks)
  Each sender will pass 100000 messages of 100 bytes

                |    2 vcpus    |    4 vcpus 
 ------------------------------------------------
         Vanila |  273.76       |  190.21	
  AVIC disabled |  260.51 (~5%) |  184.40 (~5%)
          AVIC  |  239.03 (~10%)|  166.37 (~10%)

OVERALL PERFORMANCE
===================
Enabling AVIC should helps speeding up workloads, which generate
large amount of interrupts. However, it requires additional logics to:
  * Maintain AVIC-specific data structures during vCPU load/unload
    due to schedule in/out.
  * Track and manange interrupt pending in vAPIC backing page.

The goal is to minimize the overhead of AVIC in most cases, so that
we can achieve equivalent or improvement in overall performance when
enabling AVIC.

This is an on-going investigation and to be discussed.

CURRENT UNSUPPORT USE-CASES
===========================
    - Nested VM
    - VM Migration

GITHUB
======
Latest git tree can be found at:
    http://github.com/ssuthiku/linux.git    avic_part1_rfc

Any feedback and comments are very much appreciated.

Thank you,
Suravee

Suravee Suthikulpanit (9):
  KVM: x86: Misc LAPIC changes to exposes helper functions
  svm: Introduce new AVIC VMCB registers
  svm: clean up V_TPR, V_IRQ, V_INTR_PRIO, and V_INTR_MASKING
  KVM: x86: Detect and Initialize AVIC support
  svm: Add VMEXIT handlers for AVIC
  svm: Add interrupt injection via AVIC
  svm: Do not expose x2APIC when enable AVIC
  svm: Do not intercept CR8 when enable AVIC
  svm: Manage vcpu load/unload when enable AVIC

 arch/x86/include/asm/cpufeature.h |   1 +
 arch/x86/include/asm/kvm_host.h   |   4 +
 arch/x86/include/asm/msr-index.h  |   1 +
 arch/x86/include/asm/svm.h        |  38 +-
 arch/x86/include/uapi/asm/svm.h   |   9 +-
 arch/x86/kernel/cpu/scattered.c   |   1 +
 arch/x86/kvm/lapic.c              |  53 +--
 arch/x86/kvm/lapic.h              |   5 +
 arch/x86/kvm/svm.c                | 882 +++++++++++++++++++++++++++++++++++++-
 arch/x86/kvm/x86.c                |   4 +-
 10 files changed, 940 insertions(+), 58 deletions(-)

-- 
1.9.1

[toc] | [next] | [standalone]

#1332766 — [PART1 RFC 4/9] KVM: x86: Detect and Initialize AVIC support

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

This patch introduces AVIC-related data structure, and AVIC
intitialization 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)

In order to accommodate the new per-VM tables, we introduce
a new per-VM arch-specific void pointer, struct kvm_arch.arch_data.
This will point to the newly introduced struct svm_vm_data.

This patch also introduces code to detect the new new SVM feature CPUID
Fn8000_000A_EDX[13], which identifies support for AMD Advance Virtual
Interrupt Controller (AVIC).

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@amd.com>
---
 arch/x86/include/asm/cpufeature.h |   1 +
 arch/x86/include/asm/kvm_host.h   |   2 +
 arch/x86/kernel/cpu/scattered.c   |   1 +
 arch/x86/kvm/svm.c                | 404 +++++++++++++++++++++++++++++++++++++-
 4 files changed, 407 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 7ad8c94..ee85900 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -203,6 +203,7 @@
 
 #define X86_FEATURE_VMMCALL     ( 8*32+15) /* Prefer vmmcall to vmcall */
 #define X86_FEATURE_XENPV       ( 8*32+16) /* "" Xen paravirtual guest */
+#define X86_FEATURE_AVIC        ( 8*32+17) /* AMD Virtual Interrupt Controller support */
 
 
 /* Intel-defined CPU features, CPUID level 0x00000007:0 (ebx), word 9 */
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 44adbb8..7b78328 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -754,6 +754,8 @@ struct kvm_arch {
 
 	bool irqchip_split;
 	u8 nr_reserved_ioapic_pins;
+
+	void *arch_data;
 };
 
 struct kvm_vm_stat {
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index 8cb57df..88cfbe7 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -37,6 +37,7 @@ void init_scattered_cpuid_features(struct cpuinfo_x86 *c)
 		{ X86_FEATURE_HW_PSTATE,	CR_EDX, 7, 0x80000007, 0 },
 		{ X86_FEATURE_CPB,		CR_EDX, 9, 0x80000007, 0 },
 		{ X86_FEATURE_PROC_FEEDBACK,	CR_EDX,11, 0x80000007, 0 },
+		{ X86_FEATURE_AVIC,		CR_EDX,13, 0x8000000a, 0 },
 		{ 0, 0, 0, 0, 0 }
 	};
 
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index ca185fb..9440b48 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -78,6 +78,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_PHY_APIC_ID_MAX	0xFF
+
 static bool erratum_383_found __read_mostly;
 
 static const u32 host_save_user_msrs[] = {
@@ -162,6 +167,36 @@ struct vcpu_svm {
 
 	/* cached guest cpuid flags for faster access */
 	bool nrips_enabled	: 1;
+
+	struct page *avic_bk_page;
+};
+
+struct __attribute__ ((__packed__))
+svm_avic_log_ait_entry {
+	u32 guest_phy_apic_id	: 8,
+	    res			: 23,
+	    valid		: 1;
+};
+
+struct __attribute__ ((__packed__))
+svm_avic_phy_ait_entry {
+	u64 host_phy_apic_id	: 8,
+	    res1		: 4,
+	    bk_pg_ptr		: 40,
+	    res2		: 10,
+	    is_running		: 1,
+	    valid		: 1;
+};
+
+/* Note: This structure is per VM */
+struct svm_vm_data {
+	atomic_t count;
+	u32 ldr_mode;
+	u32 avic_max_vcpu_id;
+	u32 avic_tag;
+
+	struct page *avic_log_ait_page;
+	struct page *avic_phy_ait_page;
 };
 
 static DEFINE_PER_CPU(u64, current_tsc_ratio);
@@ -205,6 +240,10 @@ module_param(npt, int, S_IRUGO);
 static int nested = true;
 module_param(nested, int, S_IRUGO);
 
+/* enable / disable AVIC */
+static int avic = false;
+module_param(avic, int, S_IRUGO);
+
 static void svm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0);
 static void svm_flush_tlb(struct kvm_vcpu *vcpu);
 static void svm_complete_interrupts(struct vcpu_svm *svm);
@@ -234,6 +273,13 @@ 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 void avic_update_vapic_bar(struct vcpu_svm *svm, u64 data)
+{
+	svm->vmcb->control.avic_vapic_bar = data & VMCB_AVIC_APIC_BAR_MASK;
+}
+
 static inline void mark_all_dirty(struct vmcb *vmcb)
 {
 	vmcb->control.clean = 0;
@@ -923,6 +969,13 @@ static __init int svm_hardware_setup(void)
 	} else
 		kvm_disable_tdp();
 
+	if (avic && (!npt_enabled || !boot_cpu_has(X86_FEATURE_AVIC)))
+		avic = false;
+
+	if (avic) {
+		printk(KERN_INFO "kvm: AVIC enabled\n");
+	}
+
 	return 0;
 
 err:
@@ -1000,6 +1053,27 @@ 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 svm_vm_data *vm_data = svm->vcpu.kvm->arch.arch_data;
+	phys_addr_t bpa = PFN_PHYS(page_to_pfn(svm->avic_bk_page));
+	phys_addr_t lpa = PFN_PHYS(page_to_pfn(vm_data->avic_log_ait_page));
+	phys_addr_t ppa = PFN_PHYS(page_to_pfn(vm_data->avic_phy_ait_page));
+
+	if (!vmcb)
+		return;
+
+	pr_debug("SVM: %s: bpa=%#llx, lpa=%#llx, ppa=%#llx\n",
+		 __func__, bpa, lpa, ppa);
+
+	vmcb->control.avic_enable = 1;
+	vmcb->control.avic_bk_page = bpa & AVIC_HPA_MASK;
+	vmcb->control.avic_log_apic_id = lpa & AVIC_HPA_MASK;
+	vmcb->control.avic_phy_apic_id = ppa & AVIC_HPA_MASK;
+	vmcb->control.avic_phy_apic_id |= AVIC_PHY_APIC_ID_MAX;
+}
+
 static void init_vmcb(struct vcpu_svm *svm)
 {
 	struct vmcb_control_area *control = &svm->vmcb->control;
@@ -1113,6 +1187,309 @@ static void init_vmcb(struct vcpu_svm *svm)
 	mark_all_dirty(svm->vmcb);
 
 	enable_gif(svm);
+
+	if (avic)
+		avic_init_vmcb(svm);
+}
+
+static struct svm_avic_phy_ait_entry *
+avic_get_phy_ait_entry(struct kvm_vcpu *vcpu, int index)
+{
+	struct svm_avic_phy_ait_entry *avic_phy_ait;
+	struct svm_vm_data *vm_data = vcpu->kvm->arch.arch_data;
+
+	if (!vm_data)
+		return NULL;
+
+	/* Note: APIC ID = 0xff is used for broadcast.
+	 *       APIC ID > 0xff is reserved.
+	 */
+	if (index >= 0xff)
+		return NULL;
+
+	avic_phy_ait = page_address(vm_data->avic_phy_ait_page);
+
+	return &avic_phy_ait[index];
+}
+
+struct svm_avic_log_ait_entry *
+avic_get_log_ait_entry(struct kvm_vcpu *vcpu, u8 mda, bool is_flat)
+{
+	struct svm_vm_data *vm_data = vcpu->kvm->arch.arch_data;
+	int index;
+	struct svm_avic_log_ait_entry *avic_log_ait;
+
+	if (!vm_data)
+		return NULL;
+
+	if (is_flat) { /* flat */
+		if (mda > 7)
+			return NULL;
+		index = mda;
+	} else { /* cluster */
+		int apic_id = mda & 0xf;
+		int cluster_id = (mda & 0xf0) >> 8;
+
+		if (apic_id > 4 || cluster_id >= 0xf)
+			return NULL;
+		index = (cluster_id << 2) + apic_id;
+	}
+	avic_log_ait = (struct svm_avic_log_ait_entry *)
+				page_address(vm_data->avic_log_ait_page);
+
+	return &avic_log_ait[index];
+}
+
+static inline void avic_set_bk_page_entry(struct vcpu_svm *svm, int reg_off, u32 val)
+{
+	void *avic_bk = page_address(svm->avic_bk_page);
+
+	*((u32 *) (avic_bk + reg_off)) = val;
+}
+
+static inline u32 *avic_get_bk_page_entry(struct vcpu_svm *svm, u32 offset)
+{
+	char *tmp = (char*)page_address(svm->avic_bk_page);
+
+	return (u32*)(tmp+offset);
+}
+
+static int avic_init_log_apic_entry(struct kvm_vcpu *vcpu, u8 g_phy_apic_id,
+				    u8 log_apic_id)
+{
+	u32 mod;
+	struct svm_avic_log_ait_entry *entry;
+	struct vcpu_svm *svm = to_svm(vcpu);
+
+	if (!svm)
+		return -EINVAL;
+
+	mod = (*avic_get_bk_page_entry(svm, APIC_DFR) >> 28) & 0xf;
+	entry = avic_get_log_ait_entry(vcpu, log_apic_id, (mod == 0xf));
+	if (!entry)
+		return -EINVAL;
+	entry->guest_phy_apic_id = g_phy_apic_id;
+	entry->valid = 1;
+
+	return 0;
+}
+
+static int avic_init_bk_page(struct kvm_vcpu *vcpu)
+{
+	int i;
+	u64 addr;
+	struct page *page;
+	int id = vcpu->vcpu_id;
+	struct kvm *kvm = vcpu->kvm;
+	struct kvm_lapic *apic = vcpu->arch.apic;
+	struct vcpu_svm *svm = to_svm(vcpu);
+
+	addr = APIC_DEFAULT_PHYS_BASE + (id * PAGE_SIZE);
+	page = gfn_to_page(kvm, addr >> PAGE_SHIFT);
+	if (is_error_page(page))
+		return -EFAULT;
+
+	/*
+	 * Do not pin the page in memory, so that memory hot-unplug
+	 * is able to migrate it.
+	 */
+	put_page(page);
+
+	/* Setting up AVIC Backing Page */
+	svm->avic_bk_page = page;
+	clear_page(kmap(page));
+	pr_debug("SVM: %s: vAPIC bk page: cpu=%u, addr=%#llx, pa=%#llx\n",
+		 __func__, id, addr,
+		 (unsigned long long) PFN_PHYS(page_to_pfn(page)));
+
+	avic_set_bk_page_entry(svm, APIC_ID, kvm_apic_get_reg(apic, APIC_ID));
+	avic_set_bk_page_entry(svm, APIC_LVR, kvm_apic_get_reg(apic, APIC_LVR));
+	for (i = 0; i < KVM_APIC_LVT_NUM; i++)
+		avic_set_bk_page_entry(svm, APIC_LVTT + 0x10 * i, APIC_LVT_MASKED);
+	avic_set_bk_page_entry(svm, APIC_LVT0,
+			 SET_APIC_DELIVERY_MODE(0, APIC_MODE_EXTINT));
+	avic_set_bk_page_entry(svm, APIC_DFR, 0xffffffffU);
+	avic_set_bk_page_entry(svm, APIC_SPIV, 0xff);
+	avic_set_bk_page_entry(svm, APIC_TASKPRI, 0);
+	avic_set_bk_page_entry(svm, APIC_LDR, kvm_apic_get_reg(apic, APIC_LDR));
+	avic_set_bk_page_entry(svm, APIC_ESR, 0);
+	avic_set_bk_page_entry(svm, APIC_ICR, 0);
+	avic_set_bk_page_entry(svm, APIC_ICR2, 0);
+	avic_set_bk_page_entry(svm, APIC_TDCR, 0);
+	avic_set_bk_page_entry(svm, APIC_TMICT, 0);
+	for (i = 0; i < 8; i++) {
+		avic_set_bk_page_entry(svm, APIC_IRR + 0x10 * i, 0);
+		avic_set_bk_page_entry(svm, APIC_ISR + 0x10 * i, 0);
+		avic_set_bk_page_entry(svm, APIC_TMR + 0x10 * i, 0);
+	}
+
+	avic_init_vmcb(svm);
+
+	return 0;
+}
+
+static inline void avic_unalloc_bk_page(struct kvm_vcpu *vcpu)
+{
+	struct vcpu_svm *svm = to_svm(vcpu);
+
+	if (svm->avic_bk_page)
+		kunmap(svm->avic_bk_page);
+}
+
+static int avic_alloc_bk_page(struct vcpu_svm *svm, int id)
+{
+	int ret = 0, i;
+	bool realloc = false;
+	struct kvm_vcpu *vcpu;
+	struct kvm *kvm = svm->vcpu.kvm;
+	struct svm_vm_data *vm_data = kvm->arch.arch_data;
+
+	mutex_lock(&kvm->slots_lock);
+
+	/* Check if we have already allocated vAPIC backing
+	 * page for this vCPU. If not, we need to realloc
+	 * a new one and re-assign all other vCPU.
+	 */
+	if (kvm->arch.apic_access_page_done &&
+	    (id > vm_data->avic_max_vcpu_id)) {
+		kvm_for_each_vcpu(i, vcpu, kvm)
+			avic_unalloc_bk_page(vcpu);
+
+		__x86_set_memory_region(kvm,
+					APIC_ACCESS_PAGE_PRIVATE_MEMSLOT,
+					0 , 0);
+		realloc = true;
+		vm_data->avic_max_vcpu_id = 0;
+	}
+
+	/*
+	 * We are allocating vAPIC backing page
+	 * upto the max vCPU ID
+	 */
+	if (id >= vm_data->avic_max_vcpu_id) {
+		ret = __x86_set_memory_region(kvm,
+					      APIC_ACCESS_PAGE_PRIVATE_MEMSLOT,
+					      APIC_DEFAULT_PHYS_BASE,
+					      PAGE_SIZE * (id + 1));
+		if (ret)
+			goto out;
+
+		vm_data->avic_max_vcpu_id = id;
+	}
+
+	/* Reinit vAPIC backing page for exisinting vcpus */
+	if (realloc)
+		kvm_for_each_vcpu(i, vcpu, kvm)
+			avic_init_bk_page(vcpu);
+
+	avic_init_bk_page(&svm->vcpu);
+
+	kvm->arch.apic_access_page_done = true;
+
+out:
+	mutex_unlock(&kvm->slots_lock);
+	return ret;
+}
+
+static void avic_vm_uninit(struct kvm *kvm)
+{
+	struct svm_vm_data *vm_data = kvm->arch.arch_data;
+
+	if (!vm_data)
+		return;
+
+	if (vm_data->avic_log_ait_page)
+		__free_page(vm_data->avic_log_ait_page);
+	if (vm_data->avic_phy_ait_page)
+		__free_page(vm_data->avic_phy_ait_page);
+	kfree(vm_data);
+	kvm->arch.arch_data = NULL;
+}
+
+static void avic_vcpu_uninit(struct kvm_vcpu *vcpu)
+{
+	struct svm_vm_data *vm_data = vcpu->kvm->arch.arch_data;
+
+	avic_unalloc_bk_page(vcpu);
+
+	if (vm_data &&
+	    (atomic_read(&vm_data->count) == 0 ||
+	     atomic_dec_and_test(&vm_data->count)))
+		avic_vm_uninit(vcpu->kvm);
+}
+
+static atomic_t avic_tag_gen = ATOMIC_INIT(1);
+
+static inline u32 avic_get_next_tag(void)
+{
+	u32 tag = atomic_read(&avic_tag_gen);
+
+	atomic_inc(&avic_tag_gen);
+	return tag;
+}
+
+static int avic_vm_init(struct kvm *kvm)
+{
+	int err = -ENOMEM;
+	struct svm_vm_data *vm_data;
+	struct page *avic_phy_ait_page;
+	struct page *avic_log_ait_page;
+
+	vm_data = kzalloc(sizeof(struct svm_vm_data),
+				      GFP_KERNEL);
+	if (!vm_data)
+		return err;
+
+	kvm->arch.arch_data = vm_data;
+	atomic_set(&vm_data->count, 0);
+
+	/* Allocating physical APIC ID table (4KB) */
+	avic_phy_ait_page = alloc_page(GFP_KERNEL);
+	if (!avic_phy_ait_page)
+		goto free_avic;
+
+	vm_data->avic_phy_ait_page = avic_phy_ait_page;
+	clear_page(page_address(avic_phy_ait_page));
+
+	/* Allocating logical APIC ID table (4KB) */
+	avic_log_ait_page = alloc_page(GFP_KERNEL);
+	if (!avic_log_ait_page)
+		goto free_avic;
+
+	vm_data->avic_log_ait_page = avic_log_ait_page;
+	clear_page(page_address(avic_log_ait_page));
+
+	vm_data->avic_tag = avic_get_next_tag();
+
+	return 0;
+
+free_avic:
+	avic_vm_uninit(kvm);
+	return err;
+}
+
+static int avic_vcpu_init(struct kvm *kvm, struct vcpu_svm *svm, int id)
+{
+	int err;
+	struct svm_vm_data *vm_data = NULL;
+
+	/* Note: svm_vm_data is per VM */
+	if (!kvm->arch.arch_data) {
+		err = avic_vm_init(kvm);
+		if (err)
+			return err;
+	}
+
+	err = avic_alloc_bk_page(svm, id);
+	if (err) {
+		avic_vcpu_uninit(&svm->vcpu);
+		return err;
+	}
+
+	vm_data = kvm->arch.arch_data;
+	atomic_inc(&vm_data->count);
+
+	return 0;
 }
 
 static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
@@ -1131,6 +1508,9 @@ static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
 
 	kvm_cpuid(vcpu, &eax, &dummy, &dummy, &dummy);
 	kvm_register_write(vcpu, VCPU_REGS_RDX, eax);
+
+	if (avic && !init_event)
+		avic_update_vapic_bar(svm, APIC_DEFAULT_PHYS_BASE);
 }
 
 static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id)
@@ -1169,6 +1549,12 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id)
 	if (!hsave_page)
 		goto free_page3;
 
+	if (avic) {
+		err = avic_vcpu_init(kvm, svm, id);
+		if (err)
+			goto free_page4;
+	}
+
 	svm->nested.hsave = page_address(hsave_page);
 
 	svm->msrpm = page_address(msrpm_pages);
@@ -1187,6 +1573,8 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id)
 
 	return &svm->vcpu;
 
+free_page4:
+	__free_page(hsave_page);
 free_page3:
 	__free_pages(nested_msrpm_pages, MSRPM_ALLOC_ORDER);
 free_page2:
@@ -1209,6 +1597,7 @@ static void svm_free_vcpu(struct kvm_vcpu *vcpu)
 	__free_pages(virt_to_page(svm->msrpm), MSRPM_ALLOC_ORDER);
 	__free_page(virt_to_page(svm->nested.hsave));
 	__free_pages(virt_to_page(svm->nested.msrpm), MSRPM_ALLOC_ORDER);
+	avic_vcpu_uninit(vcpu);
 	kvm_vcpu_uninit(vcpu);
 	kmem_cache_free(kvm_vcpu_cache, svm);
 }
@@ -3372,6 +3761,7 @@ static void dump_vmcb(struct kvm_vcpu *vcpu)
 	pr_err("%-20s%08x\n", "exit_int_info_err:", control->exit_int_info_err);
 	pr_err("%-20s%lld\n", "nested_ctl:", control->nested_ctl);
 	pr_err("%-20s%016llx\n", "nested_cr3:", control->nested_cr3);
+	pr_err("%-20s%016llx\n", "avic_vapic_bar:", control->avic_vapic_bar);
 	pr_err("%-20s%08x\n", "event_inj:", control->event_inj);
 	pr_err("%-20s%08x\n", "event_inj_err:", control->event_inj_err);
 	pr_err("%-20s%lld\n", "lbr_ctl:", control->lbr_ctl);
@@ -3603,7 +3993,17 @@ static void svm_set_virtual_x2apic_mode(struct kvm_vcpu *vcpu, bool set)
 
 static bool svm_get_enable_apicv(void)
 {
-	return false;
+	return avic;
+}
+
+static void svm_hwapic_irr_update(struct kvm_vcpu *vcpu, int max_irr)
+{
+	return;
+}
+
+static void svm_hwapic_isr_update(struct kvm *kvm, int isr)
+{
+	return;
 }
 
 static void svm_refresh_apicv_exec_ctrl(struct kvm_vcpu *vcpu)
@@ -4375,6 +4775,8 @@ static struct kvm_x86_ops svm_x86_ops = {
 	.refresh_apicv_exec_ctrl = svm_refresh_apicv_exec_ctrl,
 	.load_eoi_exitmap = svm_load_eoi_exitmap,
 	.sync_pir_to_irr = svm_sync_pir_to_irr,
+	.hwapic_irr_update = svm_hwapic_irr_update,
+	.hwapic_isr_update = svm_hwapic_isr_update,
 
 	.set_tss_addr = svm_set_tss_addr,
 	.get_tdp_level = get_npt_level,
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1332771 — Re: [PART1 RFC 4/9] KVM: x86: Detect and Initialize AVIC support

On Fri, Feb 12, 2016 at 08:59:29PM +0700, Suravee Suthikulpanit wrote:
> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> 
> This patch introduces AVIC-related data structure, and AVIC
> intitialization 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)
> 
> In order to accommodate the new per-VM tables, we introduce
> a new per-VM arch-specific void pointer, struct kvm_arch.arch_data.
> This will point to the newly introduced struct svm_vm_data.
> 
> This patch also introduces code to detect the new new SVM feature CPUID
> Fn8000_000A_EDX[13], which identifies support for AMD Advance Virtual
> Interrupt Controller (AVIC).
> 
> 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@amd.com>
> ---
>  arch/x86/include/asm/cpufeature.h |   1 +
>  arch/x86/include/asm/kvm_host.h   |   2 +
>  arch/x86/kernel/cpu/scattered.c   |   1 +
>  arch/x86/kvm/svm.c                | 404 +++++++++++++++++++++++++++++++++++++-
>  4 files changed, 407 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
> index 7ad8c94..ee85900 100644
> --- a/arch/x86/include/asm/cpufeature.h
> +++ b/arch/x86/include/asm/cpufeature.h
> @@ -203,6 +203,7 @@
>  
>  #define X86_FEATURE_VMMCALL     ( 8*32+15) /* Prefer vmmcall to vmcall */
>  #define X86_FEATURE_XENPV       ( 8*32+16) /* "" Xen paravirtual guest */
> +#define X86_FEATURE_AVIC        ( 8*32+17) /* AMD Virtual Interrupt Controller support */
>  
>  
>  /* Intel-defined CPU features, CPUID level 0x00000007:0 (ebx), word 9 */
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 44adbb8..7b78328 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -754,6 +754,8 @@ struct kvm_arch {
>  
>  	bool irqchip_split;
>  	u8 nr_reserved_ioapic_pins;
> +
> +	void *arch_data;
>  };
>  
>  struct kvm_vm_stat {
> diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
> index 8cb57df..88cfbe7 100644
> --- a/arch/x86/kernel/cpu/scattered.c
> +++ b/arch/x86/kernel/cpu/scattered.c
> @@ -37,6 +37,7 @@ void init_scattered_cpuid_features(struct cpuinfo_x86 *c)
>  		{ X86_FEATURE_HW_PSTATE,	CR_EDX, 7, 0x80000007, 0 },
>  		{ X86_FEATURE_CPB,		CR_EDX, 9, 0x80000007, 0 },
>  		{ X86_FEATURE_PROC_FEEDBACK,	CR_EDX,11, 0x80000007, 0 },
> +		{ X86_FEATURE_AVIC,		CR_EDX,13, 0x8000000a, 0 },
>  		{ 0, 0, 0, 0, 0 }
>  	};

You need to check tip/master when/before/after touching arch/x86/:

a1ff57260818 ("x86/cpufeature: Add AMD AVIC bit")

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[toc] | [prev] | [next] | [standalone]

#1332821 — Re: [PART1 RFC 4/9] KVM: x86: Detect and Initialize AVIC support

Hi,

On 02/12/2016 09:13 PM, Borislav Petkov wrote:
>> diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
>> >index 8cb57df..88cfbe7 100644
>> >--- a/arch/x86/kernel/cpu/scattered.c
>> >+++ b/arch/x86/kernel/cpu/scattered.c
>> >@@ -37,6 +37,7 @@ void init_scattered_cpuid_features(struct cpuinfo_x86 *c)
>> >  		{ X86_FEATURE_HW_PSTATE,	CR_EDX, 7, 0x80000007, 0 },
>> >  		{ X86_FEATURE_CPB,		CR_EDX, 9, 0x80000007, 0 },
>> >  		{ X86_FEATURE_PROC_FEEDBACK,	CR_EDX,11, 0x80000007, 0 },
>> >+		{ X86_FEATURE_AVIC,		CR_EDX,13, 0x8000000a, 0 },
>> >  		{ 0, 0, 0, 0, 0 }
>> >  	};
> You need to check tip/master when/before/after touching arch/x86/:
>
> a1ff57260818 ("x86/cpufeature: Add AMD AVIC bit")

Ok, I'll check that in the next patch series.

Thanks,
Suravee

[toc] | [prev] | [next] | [standalone]

#1332767 — [PART1 RFC 9/9] svm: Manage vcpu load/unload when enable AVIC

When a vcpu is loaded/unloaded to a physical core, we need to update
information in the Physical APIC-ID table accordingly.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 arch/x86/kvm/svm.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 32da657..41e68d2 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -32,6 +32,7 @@
 #include <linux/trace_events.h>
 #include <linux/slab.h>
 
+#include <asm/apic.h>
 #include <asm/perf_event.h>
 #include <asm/tlbflush.h>
 #include <asm/desc.h>
@@ -1508,6 +1509,61 @@ static int avic_vcpu_init(struct kvm *kvm, struct vcpu_svm *svm, int id)
 	return 0;
 }
 
+static inline int avic_update_iommu(struct kvm_vcpu *vcpu, int cpu,
+				    phys_addr_t pa, bool is_running)
+{
+	if (!kvm_arch_has_assigned_device(vcpu->kvm))
+		return 0;
+
+	/* TODO: We will hook up with IOMMU API at later time */
+	return 0;
+}
+
+static int avic_set_running(struct kvm_vcpu *vcpu, int cpu, bool is_running)
+{
+	int g_phy_apic_id, h_phy_apic_id;
+	struct svm_avic_phy_ait_entry *entry;
+	struct vcpu_svm *svm = to_svm(vcpu);
+	int ret;
+
+	if (!avic)
+		return 0;
+
+	if (!svm)
+		return -EINVAL;
+
+	/* Note: APIC ID = 0xff is used for broadcast.
+	 *       APIC ID > 0xff is reserved.
+	 */
+	g_phy_apic_id = vcpu->vcpu_id;
+	h_phy_apic_id = __default_cpu_present_to_apicid(cpu);
+
+	if ((g_phy_apic_id >= AVIC_PHY_APIC_ID_MAX) ||
+	    (h_phy_apic_id >= AVIC_PHY_APIC_ID_MAX))
+		return -EINVAL;
+
+	entry = avic_get_phy_ait_entry(vcpu, g_phy_apic_id);
+	if (!entry)
+		return -EINVAL;
+
+	if (is_running) {
+		phys_addr_t pa = PFN_PHYS(page_to_pfn(svm->avic_bk_page));
+
+		entry->bk_pg_ptr = (pa >> 12) & 0xffffffffff;
+		entry->valid = 1;
+		entry->host_phy_apic_id = h_phy_apic_id;
+		barrier();
+		entry->is_running = is_running;
+		ret = avic_update_iommu(vcpu, h_phy_apic_id, pa, is_running);
+	} else {
+		ret = avic_update_iommu(vcpu, 0, 0, is_running);
+		barrier();
+		entry->is_running = is_running;
+	}
+
+	return ret;
+}
+
 static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
@@ -1628,6 +1684,8 @@ static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 		mark_all_dirty(svm->vmcb);
 	}
 
+	avic_set_running(vcpu, cpu, true);
+
 #ifdef CONFIG_X86_64
 	rdmsrl(MSR_GS_BASE, to_svm(vcpu)->host.gs_base);
 #endif
@@ -1668,6 +1726,9 @@ static void svm_vcpu_put(struct kvm_vcpu *vcpu)
 #endif
 	for (i = 0; i < NR_HOST_SAVE_USER_MSRS; i++)
 		wrmsrl(host_save_user_msrs[i], svm->host_user_msrs[i]);
+
+	avic_set_running(vcpu, 0, false);
+
 }
 
 static unsigned long svm_get_rflags(struct kvm_vcpu *vcpu)
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1332820 — Re: [PART1 RFC 9/9] svm: Manage vcpu load/unload when enable AVIC

On 12/02/2016 14:59, Suravee Suthikulpanit wrote:
> +
> +	if (is_running) {
> +		phys_addr_t pa = PFN_PHYS(page_to_pfn(svm->avic_bk_page));
> +
> +		entry->bk_pg_ptr = (pa >> 12) & 0xffffffffff;
> +		entry->valid = 1;
> +		entry->host_phy_apic_id = h_phy_apic_id;
> +		barrier();
> +		entry->is_running = is_running;

I'm not sure if you can rely on the compiler doing the right thing here.
 I would prefer something like:

	new_entry = READ_ONCE(entry);
	new_entry.bk_pg_ptr = (pa >> 12) & 0xffffffffff;
	new_entry.valid = 1;
	new_entry.host_phy_apic_id = h_phy_apic_id;
	new_entry.is_running = is_running;
	WRITE_ONCE(entry, new_entry);

Paolo

[toc] | [prev] | [next] | [standalone]

#1332769 — [PART1 RFC 6/9] svm: Add interrupt injection via AVIC

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

VINTR is not supported when enable AVIC. Therefore, we need to inject
interrupt via APIC backing page instead. Also, adding AVIC doorbell
support to signal running vcpu to check IRR for injected interrupts.

This patch also introduces kvm_x86_ops.apicv_intr_pending() to allow SVM
to provide a function hook to query AVIC interrupt pending status.

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 arch/x86/include/asm/kvm_host.h  |   2 +
 arch/x86/include/asm/msr-index.h |   1 +
 arch/x86/kvm/lapic.c             |   3 +-
 arch/x86/kvm/svm.c               | 110 +++++++++++++++++++++++++++++++++++++--
 arch/x86/kvm/x86.c               |   4 +-
 5 files changed, 114 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 7b78328..a7c8852 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -973,6 +973,8 @@ struct kvm_x86_ops {
 	void (*post_block)(struct kvm_vcpu *vcpu);
 	int (*update_pi_irte)(struct kvm *kvm, unsigned int host_irq,
 			      uint32_t guest_irq, bool set);
+
+	bool (*apicv_intr_pending)(struct kvm_vcpu *vcpu);
 };
 
 struct kvm_arch_async_pf {
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index b05402e..605b869 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -284,6 +284,7 @@
 #define MSR_AMD64_TSC_RATIO		0xc0000104
 #define MSR_AMD64_NB_CFG		0xc001001f
 #define MSR_AMD64_PATCH_LOADER		0xc0010020
+#define MSR_AMD64_AVIC_DOORBELL		0xc001011b
 #define MSR_AMD64_OSVW_ID_LENGTH	0xc0010140
 #define MSR_AMD64_OSVW_STATUS		0xc0010141
 #define MSR_AMD64_LS_CFG		0xc0011020
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index fc313a0..f6deb04 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1828,7 +1828,8 @@ int kvm_apic_has_interrupt(struct kvm_vcpu *vcpu)
 	struct kvm_lapic *apic = vcpu->arch.apic;
 	int highest_irr;
 
-	if (!kvm_vcpu_has_lapic(vcpu) || !apic_enabled(apic))
+	if (!kvm_vcpu_has_lapic(vcpu) || !apic_enabled(apic) ||
+	    kvm_x86_ops->apicv_intr_pending)
 		return -1;
 
 	apic_update_ppr(apic);
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index bedf52b..5d7b049 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -169,6 +169,7 @@ struct vcpu_svm {
 	bool nrips_enabled	: 1;
 
 	struct page *avic_bk_page;
+	atomic_t avic_pending_cnt;
 };
 
 struct __attribute__ ((__packed__))
@@ -232,6 +233,8 @@ static bool npt_enabled = true;
 static bool npt_enabled;
 #endif
 
+static struct kvm_x86_ops svm_x86_ops;
+
 /* allow nested paging (virtualized MMU) for all guests */
 static int npt = true;
 module_param(npt, int, S_IRUGO);
@@ -974,6 +977,9 @@ static __init int svm_hardware_setup(void)
 
 	if (avic) {
 		printk(KERN_INFO "kvm: AVIC enabled\n");
+	} else {
+		svm_x86_ops.deliver_posted_interrupt = NULL;
+		svm_x86_ops.apicv_intr_pending = NULL;
 	}
 
 	return 0;
@@ -1188,8 +1194,10 @@ static void init_vmcb(struct vcpu_svm *svm)
 
 	enable_gif(svm);
 
-	if (avic)
+	if (avic) {
 		avic_init_vmcb(svm);
+		atomic_set(&svm->avic_pending_cnt, 0);
+	}
 }
 
 static struct svm_avic_phy_ait_entry *
@@ -3059,8 +3067,10 @@ static int clgi_interception(struct vcpu_svm *svm)
 	disable_gif(svm);
 
 	/* After a CLGI no interrupts should come */
-	svm_clear_vintr(svm);
-	svm->vmcb->control.v_irq = 0;
+	if (!avic) {
+		svm_clear_vintr(svm);
+		svm->vmcb->control.v_irq = 0;
+	}
 
 	mark_dirty(svm->vmcb, VMCB_INTR);
 
@@ -3635,6 +3645,9 @@ static int msr_interception(struct vcpu_svm *svm)
 
 static int interrupt_window_interception(struct vcpu_svm *svm)
 {
+	if (avic)
+		BUG_ON(1);
+
 	kvm_make_request(KVM_REQ_EVENT, &svm->vcpu);
 	svm_clear_vintr(svm);
 	svm->vmcb->control.v_irq = 0;
@@ -4190,7 +4203,7 @@ static inline void svm_inject_irq(struct vcpu_svm *svm, int irq)
 {
 	struct vmcb_control_area *control;
 
-
+	/* The following fields are ignored when AVIC is enabled */
 	control = &svm->vmcb->control;
 	control->int_vector = irq;
 	control->v_intr_prio = 0xf;
@@ -4261,6 +4274,27 @@ static void svm_sync_pir_to_irr(struct kvm_vcpu *vcpu)
 	return;
 }
 
+static void svm_deliver_avic_intr(struct kvm_vcpu *vcpu, int vec)
+{
+	struct vcpu_svm *svm = to_svm(vcpu);
+
+	kvm_lapic_set_vector(vec, avic_get_bk_page_entry(svm, APIC_IRR));
+
+	/* Note:
+	 * This gives us a hint to check for pending interrupts
+	 * during #VMEXIT.
+	 */
+	atomic_inc(&svm->avic_pending_cnt);
+
+	if (vcpu->mode == IN_GUEST_MODE) {
+		wrmsrl(MSR_AMD64_AVIC_DOORBELL,
+		       __default_cpu_present_to_apicid(vcpu->cpu));
+	} else {
+		kvm_make_request(KVM_REQ_EVENT, vcpu);
+		kvm_vcpu_kick(vcpu);
+	}
+}
+
 static int svm_nmi_allowed(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
@@ -4321,6 +4355,9 @@ static void enable_irq_window(struct kvm_vcpu *vcpu)
 	 * get that intercept, this function will be called again though and
 	 * we'll get the vintr intercept.
 	 */
+	if (avic)
+		return;
+
 	if (gif_set(svm) && nested_svm_intr(svm)) {
 		svm_set_vintr(svm);
 		svm_inject_irq(svm, 0x0);
@@ -4462,6 +4499,67 @@ static void svm_cancel_injection(struct kvm_vcpu *vcpu)
 	svm_complete_interrupts(svm);
 }
 
+static bool avic_check_irr_pending(struct kvm_vcpu *vcpu)
+{
+	int i;
+	u32 irr;
+	struct vcpu_svm *svm = to_svm(vcpu);
+
+	for (i = 0; i < 8; i++) {
+		irr = *(avic_get_bk_page_entry(svm,
+					APIC_IRR + (0x10 * i)));
+		if (irr)
+			return true;
+	}
+
+	return false;
+}
+
+static bool svm_avic_check_ppr(struct vcpu_svm *svm)
+{
+	u32 tpr = *(avic_get_bk_page_entry(svm, APIC_TASKPRI));
+	u32 ppr = *(avic_get_bk_page_entry(svm, APIC_PROCPRI));
+
+	if (ppr && (ppr != tpr))
+		return true;
+
+	return false;
+}
+
+/* Note: Returns true means do not block */
+static bool svm_apicv_intr_pending (struct kvm_vcpu *vcpu)
+{
+	struct vcpu_svm *svm = to_svm(vcpu);
+
+	if (!avic)
+		return false;
+
+	if (atomic_read(&svm->avic_pending_cnt))
+		return true;
+
+	return avic_check_irr_pending(vcpu);
+}
+
+static void avic_post_vmrun(struct kvm_vcpu *vcpu)
+{
+	struct vcpu_svm *svm = to_svm(vcpu);
+
+	if (!avic)
+		return;
+
+	if (atomic_read(&svm->avic_pending_cnt)) {
+		if (svm_avic_check_ppr(svm))
+			return;
+		if (avic_check_irr_pending(vcpu))
+			return;
+		/*
+		 * At this point, if there is no interrupt pending.
+		 * So, we decrement the pending count
+		 */
+		atomic_dec(&svm->avic_pending_cnt);
+	}
+}
+
 static void svm_vcpu_run(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
@@ -4588,6 +4686,8 @@ static void svm_vcpu_run(struct kvm_vcpu *vcpu)
 	if (unlikely(svm->vmcb->control.exit_code == SVM_EXIT_NMI))
 		kvm_after_handle_nmi(&svm->vcpu);
 
+	avic_post_vmrun(vcpu);
+
 	sync_cr8_to_lapic(vcpu);
 
 	svm->next_rip = 0;
@@ -5050,7 +5150,9 @@ static struct kvm_x86_ops svm_x86_ops = {
 
 	.sched_in = svm_sched_in,
 
+	.apicv_intr_pending = svm_apicv_intr_pending,
 	.pmu_ops = &amd_pmu_ops,
+	.deliver_posted_interrupt = svm_deliver_avic_intr,
 };
 
 static int __init svm_init(void)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 4244c2b..2def290 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8087,7 +8087,9 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu)
 	if (is_guest_mode(vcpu) && kvm_x86_ops->check_nested_events)
 		kvm_x86_ops->check_nested_events(vcpu, false);
 
-	return kvm_vcpu_running(vcpu) || kvm_vcpu_has_events(vcpu);
+	return (kvm_vcpu_running(vcpu) || kvm_vcpu_has_events(vcpu) ||
+		(kvm_x86_ops->apicv_intr_pending &&
+		 kvm_x86_ops->apicv_intr_pending(vcpu)));
 }
 
 int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu)
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1332774 — Re: [PART1 RFC 6/9] svm: Add interrupt injection via AVIC

On Fri, Feb 12, 2016 at 08:59:31PM +0700, Suravee Suthikulpanit wrote:
> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> 
> VINTR is not supported when enable AVIC. Therefore, we need to inject
> interrupt via APIC backing page instead. Also, adding AVIC doorbell
> support to signal running vcpu to check IRR for injected interrupts.
> 
> This patch also introduces kvm_x86_ops.apicv_intr_pending() to allow SVM
> to provide a function hook to query AVIC interrupt pending status.
> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>  arch/x86/include/asm/kvm_host.h  |   2 +
>  arch/x86/include/asm/msr-index.h |   1 +
>  arch/x86/kvm/lapic.c             |   3 +-
>  arch/x86/kvm/svm.c               | 110 +++++++++++++++++++++++++++++++++++++--
>  arch/x86/kvm/x86.c               |   4 +-
>  5 files changed, 114 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 7b78328..a7c8852 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -973,6 +973,8 @@ struct kvm_x86_ops {
>  	void (*post_block)(struct kvm_vcpu *vcpu);
>  	int (*update_pi_irte)(struct kvm *kvm, unsigned int host_irq,
>  			      uint32_t guest_irq, bool set);
> +
> +	bool (*apicv_intr_pending)(struct kvm_vcpu *vcpu);
>  };
>  
>  struct kvm_arch_async_pf {
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index b05402e..605b869 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -284,6 +284,7 @@
>  #define MSR_AMD64_TSC_RATIO		0xc0000104
>  #define MSR_AMD64_NB_CFG		0xc001001f
>  #define MSR_AMD64_PATCH_LOADER		0xc0010020
> +#define MSR_AMD64_AVIC_DOORBELL		0xc001011b

AFAICT, that MSR is being used only in arch/x86/kvm/lapic.c

Please add it there.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[toc] | [prev] | [next] | [standalone]

#1332833 — Re: [PART1 RFC 6/9] svm: Add interrupt injection via AVIC

Hi,

On 02/12/2016 09:16 PM, Borislav Petkov wrote:
>> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
>> >index b05402e..605b869 100644
>> >--- a/arch/x86/include/asm/msr-index.h
>> >+++ b/arch/x86/include/asm/msr-index.h
>> >@@ -284,6 +284,7 @@
>> >  #define MSR_AMD64_TSC_RATIO		0xc0000104
>> >  #define MSR_AMD64_NB_CFG		0xc001001f
>> >  #define MSR_AMD64_PATCH_LOADER		0xc0010020
>> >+#define MSR_AMD64_AVIC_DOORBELL		0xc001011b
> AFAICT, that MSR is being used only in arch/x86/kvm/lapic.c
>
> Please add it there.

Do you mean in the arch/x86/kvm/svm.c? If so, sure, I'll only put it there.

Suravee

[toc] | [prev] | [next] | [standalone]

#1332930 — Re: [PART1 RFC 6/9] svm: Add interrupt injection via AVIC

On Fri, Feb 12, 2016 at 10:54:43PM +0700, Suravee Suthikulpanit wrote:
> Do you mean in the arch/x86/kvm/svm.c?

Ah, it is svm.c. Yes, where it is being used.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[toc] | [prev] | [next] | [standalone]

#1333005 — Re: [PART1 RFC 6/9] svm: Add interrupt injection via AVIC

On 12/02/2016 18:14, Borislav Petkov wrote:
> On Fri, Feb 12, 2016 at 10:54:43PM +0700, Suravee Suthikulpanit wrote:
>> > Do you mean in the arch/x86/kvm/svm.c?
> Ah, it is svm.c. Yes, where it is being used.

Hmm, currently things such as MSR_VM_HSAVE_PA are defined in msr-index.h.

It's okay for me to move them to kvm_host.h or similar, but they should
all be treated the same.  Right now this means adding the doorbell MSR
to msr-index.h.

Paolo

[toc] | [prev] | [next] | [standalone]

#1333009 — Re: [PART1 RFC 6/9] svm: Add interrupt injection via AVIC

On Fri, Feb 12, 2016 at 07:21:17PM +0100, Paolo Bonzini wrote:
> Hmm, currently things such as MSR_VM_HSAVE_PA are defined in msr-index.h.

But that one is used in 3 files AFAICT.

> It's okay for me to move them to kvm_host.h or similar, but they should
> all be treated the same.  Right now this means adding the doorbell MSR
> to msr-index.h.

Only if it is used in multiple files. The doorbell thing is used once in
svm.c.

We don't want to make msr-index.h an encyclopedia of any MSR ever
defined :-)

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[toc] | [prev] | [next] | [standalone]

#1333042 — Re: [PART1 RFC 6/9] svm: Add interrupt injection via AVIC

On 12/02/2016 19:30, Borislav Petkov wrote:
> On Fri, Feb 12, 2016 at 07:21:17PM +0100, Paolo Bonzini wrote:
>> Hmm, currently things such as MSR_VM_HSAVE_PA are defined in msr-index.h.
> 
> But that one is used in 3 files AFAICT.

Ok, next examples: MSR_VM_CR and MSR_VM_IGNNE. :)

>> It's okay for me to move them to kvm_host.h or similar, but they should
>> all be treated the same.  Right now this means adding the doorbell MSR
>> to msr-index.h.
> 
> Only if it is used in multiple files. The doorbell thing is used once in
> svm.c.
> 
> We don't want to make msr-index.h an encyclopedia of any MSR ever
> defined :-)

Are you okay with moving all the SVM MSRs to virtext.h instead?

Thanks,

Paolo

[toc] | [prev] | [next] | [standalone]

#1333063 — Re: [PART1 RFC 6/9] svm: Add interrupt injection via AVIC

On Fri, Feb 12, 2016 at 07:56:59PM +0100, Paolo Bonzini wrote:
> Ok, next examples: MSR_VM_CR and MSR_VM_IGNNE. :)

I knew you were going to dig out some. :-)

> Are you okay with moving all the SVM MSRs to virtext.h instead?

So I would not move any now and cause unnecessary churn. I think it
should be enough if we agree on a strategy wrt msr-index.h and then
follow it. I think we should do something similar to pci_ids.h.

Let me add tip guys to CC.

---
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 552346598dab..75a5bb61d32f 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -1,7 +1,12 @@
 #ifndef _ASM_X86_MSR_INDEX_H
 #define _ASM_X86_MSR_INDEX_H
 
-/* CPU model specific register (MSR) numbers */
+/*
+ * CPU model specific register (MSR) numbers.
+ *
+ * Do not add new entries to this file unless the definitions are shared
+ * between multiple compilation units.
+ */
 
 /* x86-64 specific MSRs */
 #define MSR_EFER		0xc0000080 /* extended feature register */


-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[toc] | [prev] | [next] | [standalone]

#1335110 — Re: [PART1 RFC 6/9] svm: Add interrupt injection via AVIC

* Borislav Petkov <bp@alien8.de> wrote:

> On Fri, Feb 12, 2016 at 07:56:59PM +0100, Paolo Bonzini wrote:
> > Ok, next examples: MSR_VM_CR and MSR_VM_IGNNE. :)
> 
> I knew you were going to dig out some. :-)
> 
> > Are you okay with moving all the SVM MSRs to virtext.h instead?
> 
> So I would not move any now and cause unnecessary churn. I think it
> should be enough if we agree on a strategy wrt msr-index.h and then
> follow it. I think we should do something similar to pci_ids.h.
> 
> Let me add tip guys to CC.
> 
> ---
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index 552346598dab..75a5bb61d32f 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -1,7 +1,12 @@
>  #ifndef _ASM_X86_MSR_INDEX_H
>  #define _ASM_X86_MSR_INDEX_H
>  
> -/* CPU model specific register (MSR) numbers */
> +/*
> + * CPU model specific register (MSR) numbers.
> + *
> + * Do not add new entries to this file unless the definitions are shared
> + * between multiple compilation units.
> + */

This sounds good to me.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1335146 — [PATCH] x86/msr: Document msr-index.h rule for addition

From: Borislav Petkov <bp@suse.de>

In order to keep this file's size sensible and not cause too much
unnecessary churn, make the rule explicit - similar to pci_ids.h - that
only MSRs which are used in multiple compilation units, should get added
to it.

Signed-off-by: Borislav Petkov <bp@suse.de>
Acked-by: Ingo Molnar <mingo@kernel.org>
Cc: alex.williamson@redhat.com
Cc: gleb@kernel.org
Cc: joro@8bytes.org
Cc: kvm@vger.kernel.org
CC: Paolo Bonzini <pbonzini@redhat.com>
Cc: sherry.hurwitz@amd.com
Cc: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Cc: wei@redhat.com
Cc: x86-ml <x86@kernel.org>
---
 arch/x86/include/asm/msr-index.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index b05402ef3b84..984ab75bf621 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -1,7 +1,12 @@
 #ifndef _ASM_X86_MSR_INDEX_H
 #define _ASM_X86_MSR_INDEX_H
 
-/* CPU model specific register (MSR) numbers */
+/*
+ * CPU model specific register (MSR) numbers.
+ *
+ * Do not add new entries to this file unless the definitions are shared
+ * between multiple compilation units.
+ */
 
 /* x86-64 specific MSRs */
 #define MSR_EFER		0xc0000080 /* extended feature register */
-- 
2.3.5

[toc] | [prev] | [next] | [standalone]

#1332834 — Re: [PART1 RFC 6/9] svm: Add interrupt injection via AVIC

On 12/02/2016 14:59, Suravee Suthikulpanit wrote:
> +static bool avic_check_irr_pending(struct kvm_vcpu *vcpu)
> +{
> +	int i;
> +	u32 irr;
> +	struct vcpu_svm *svm = to_svm(vcpu);
> +
> +	for (i = 0; i < 8; i++) {
> +		irr = *(avic_get_bk_page_entry(svm,
> +					APIC_IRR + (0x10 * i)));
> +		if (irr)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
> +static bool svm_avic_check_ppr(struct vcpu_svm *svm)
> +{
> +	u32 tpr = *(avic_get_bk_page_entry(svm, APIC_TASKPRI));
> +	u32 ppr = *(avic_get_bk_page_entry(svm, APIC_PROCPRI));
> +
> +	if (ppr && (ppr != tpr))
> +		return true;
> +
> +	return false;
> +}
> +
> +/* Note: Returns true means do not block */
> +static bool svm_apicv_intr_pending (struct kvm_vcpu *vcpu)
> +{
> +	struct vcpu_svm *svm = to_svm(vcpu);
> +
> +	if (!avic)
> +		return false;
> +
> +	if (atomic_read(&svm->avic_pending_cnt))
> +		return true;
> +
> +	return avic_check_irr_pending(vcpu);
> +}
> +
> +static void avic_post_vmrun(struct kvm_vcpu *vcpu)
> +{
> +	struct vcpu_svm *svm = to_svm(vcpu);
> +
> +	if (!avic)
> +		return;
> +
> +	if (atomic_read(&svm->avic_pending_cnt)) {
> +		if (svm_avic_check_ppr(svm))
> +			return;
> +		if (avic_check_irr_pending(vcpu))
> +			return;
> +		/*
> +		 * At this point, if there is no interrupt pending.
> +		 * So, we decrement the pending count
> +		 */
> +		atomic_dec(&svm->avic_pending_cnt);
> +	}
> +}
> +
>  static void svm_vcpu_run(struct kvm_vcpu *vcpu)
>  {
>  	struct vcpu_svm *svm = to_svm(vcpu);
> @@ -4588,6 +4686,8 @@ static void svm_vcpu_run(struct kvm_vcpu *vcpu)
>  	if (unlikely(svm->vmcb->control.exit_code == SVM_EXIT_NMI))
>  		kvm_after_handle_nmi(&svm->vcpu);
>  
> +	avic_post_vmrun(vcpu);
> +
>  	sync_cr8_to_lapic(vcpu);
>  
>  	svm->next_rip = 0;
> @@ -5050,7 +5150,9 @@ static struct kvm_x86_ops svm_x86_ops = {
>  
>  	.sched_in = svm_sched_in,
>  
> +	.apicv_intr_pending = svm_apicv_intr_pending,
>  	.pmu_ops = &amd_pmu_ops,
> +	.deliver_posted_interrupt = svm_deliver_avic_intr,
>  };
>  
>  static int __init svm_init(void)
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 4244c2b..2def290 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -8087,7 +8087,9 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu)
>  	if (is_guest_mode(vcpu) && kvm_x86_ops->check_nested_events)
>  		kvm_x86_ops->check_nested_events(vcpu, false);
>  
> -	return kvm_vcpu_running(vcpu) || kvm_vcpu_has_events(vcpu);
> +	return (kvm_vcpu_running(vcpu) || kvm_vcpu_has_events(vcpu) ||
> +		(kvm_x86_ops->apicv_intr_pending &&
> +		 kvm_x86_ops->apicv_intr_pending(vcpu)));
>  }

I think this is not necessary.  What you need is to make kvm_lapic's
regs field point to the backing page.  Then when the processor writes to
IRR, kvm_apic_has_interrupt (called through kvm_vcpu_has_events) will
see it.

avic_pending_cnt shouldn't be necessary either.

Paolo

[toc] | [prev] | [next] | [standalone]

#1332876 — Re: [PART1 RFC 6/9] svm: Add interrupt injection via AVIC

Hi Paolo,

On 02/12/2016 10:55 PM, Paolo Bonzini wrote:
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> >index 4244c2b..2def290 100644
>> >--- a/arch/x86/kvm/x86.c
>> >+++ b/arch/x86/kvm/x86.c
>> >@@ -8087,7 +8087,9 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu)
>> >  	if (is_guest_mode(vcpu) && kvm_x86_ops->check_nested_events)
>> >  		kvm_x86_ops->check_nested_events(vcpu, false);
>> >
>> >-	return kvm_vcpu_running(vcpu) || kvm_vcpu_has_events(vcpu);
>> >+	return (kvm_vcpu_running(vcpu) || kvm_vcpu_has_events(vcpu) ||
>> >+		(kvm_x86_ops->apicv_intr_pending &&
>> >+		 kvm_x86_ops->apicv_intr_pending(vcpu)));
>> >  }
> I think this is not necessary.  What you need is to make kvm_lapic's
> regs field point to the backing page.  Then when the processor writes to
> IRR, kvm_apic_has_interrupt (called through kvm_vcpu_has_events) will
> see it.
>
> avic_pending_cnt shouldn't be necessary either.
>
> Paolo

So, the other thing I am using the avic_pending_cnt for is for the part 
2 of the series (to enable AVIC support in IOMMU) that I am planning to 
send out later. However, it might be good to discuss this at this point.

When the IOMMU cannot inject interrupts into the guest vcpu due to it is 
not running (therefore, it cannot doorbell the vcpu directly), it logs 
the interrupt in the GA log buffer. Then it generates interrupt to 
notify the IOMMU driver that it needs to handle the log entry. Here, the 
IOMMU driver will end up notifying the SVM to scheduling the VCPU in to 
process interrupt.

Here, I have run into issue where the vcpu often goes into idle (i.e. 
scheduled out), and ended up causing IOMMU to generate a lot of the 
entries in the GA log. This really hurts device pass-through performance 
(e.g. for XGBE NIC).

So, what I ended up experimenting with is to set the avic_pending_cnt to 
a larger value (i.e. avic_ga_log_threshold) whenever we processing the 
GA log entry. The intention is to delay the vcpu schedule out in 
expecting that there might be more interrupts coming in soon. I also 
make this threshold value tunable as a module_param.

This actually works well in my experiment, where I can actually get 
about 5% speed up in my netperf test on XGBE NIC pass-through test.
However, I am not sure if this is an acceptable approach. Actually, I 
think it's similar to the halt_poll_ns, but specifically for IOMMU GA 
log in this case.

Let me know what you think.

Thanks,
Suravee

[toc] | [prev] | [next] | [standalone]

#1333003 — Re: [PART1 RFC 6/9] svm: Add interrupt injection via AVIC

On 12/02/2016 17:21, Suravee Suthikulpanit wrote:
> Hi Paolo,
> 
> On 02/12/2016 10:55 PM, Paolo Bonzini wrote:
>>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>>> >index 4244c2b..2def290 100644
>>> >--- a/arch/x86/kvm/x86.c
>>> >+++ b/arch/x86/kvm/x86.c
>>> >@@ -8087,7 +8087,9 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu)
>>> >      if (is_guest_mode(vcpu) && kvm_x86_ops->check_nested_events)
>>> >          kvm_x86_ops->check_nested_events(vcpu, false);
>>> >
>>> >-    return kvm_vcpu_running(vcpu) || kvm_vcpu_has_events(vcpu);
>>> >+    return (kvm_vcpu_running(vcpu) || kvm_vcpu_has_events(vcpu) ||
>>> >+        (kvm_x86_ops->apicv_intr_pending &&
>>> >+         kvm_x86_ops->apicv_intr_pending(vcpu)));
>>> >  }
>> I think this is not necessary.  What you need is to make kvm_lapic's
>> regs field point to the backing page.  Then when the processor writes to
>> IRR, kvm_apic_has_interrupt (called through kvm_vcpu_has_events) will
>> see it.
>>
>> avic_pending_cnt shouldn't be necessary either.
>>
>> Paolo
> 
> So, the other thing I am using the avic_pending_cnt for is for the part
> 2 of the series (to enable AVIC support in IOMMU) that I am planning to
> send out later. However, it might be good to discuss this at this point.

It's better to discuss it later.  For now, I would prefer the AVIC
patches to be as clean as possible, and not know about the IOMMU at all.
 Also, there are a lot of assumptions about how to use kvm_lapic's regs
field for APIC virtualization---dating back to when Intel only
virtualized the TPR field.  Deviating for that would be a recipe for
trouble. :)

Regarding the IOMMU, I'm actually very happy with the way the Intel VT-d
posted interrupts patches worked out, so I would be even more happy if
everything you do fits in the same scheme and reuses the same hooks! :D

> When the IOMMU cannot inject interrupts into the guest vcpu due to it is
> not running (therefore, it cannot doorbell the vcpu directly), it logs
> the interrupt in the GA log buffer.

Where is this documented?

> Then it generates interrupt to
> notify the IOMMU driver that it needs to handle the log entry. Here, the
> IOMMU driver will end up notifying the SVM to scheduling the VCPU in to
> process interrupt.
> 
> Here, I have run into issue where the vcpu often goes into idle (i.e.
> scheduled out), and ended up causing IOMMU to generate a lot of the
> entries in the GA log. This really hurts device pass-through performance
> (e.g. for XGBE NIC).
> 
> So, what I ended up experimenting with is to set the avic_pending_cnt to
> a larger value (i.e. avic_ga_log_threshold) whenever we processing the
> GA log entry. The intention is to delay the vcpu schedule out in
> expecting that there might be more interrupts coming in soon. I also
> make this threshold value tunable as a module_param.
> 
> This actually works well in my experiment, where I can actually get
> about 5% speed up in my netperf test on XGBE NIC pass-through test.
> However, I am not sure if this is an acceptable approach. Actually, I
> think it's similar to the halt_poll_ns, but specifically for IOMMU GA
> log in this case.

Have you retested now that the halt_poll_ns mechanism is dynamic and
enabled by default?  If I read patch 9 right, halt_poll_ns would delay
vcpu_put and IsRunning=0.  Hopefully this is enough to avoid this kind
of notification and make the issue moot.

Paolo

[toc] | [prev] | [next] | [standalone]

#1333059 — Re: [PART1 RFC 6/9] svm: Add interrupt injection via AVIC

Hi

On 2/13/16 01:19, Paolo Bonzini wrote:
>> When the IOMMU cannot inject interrupts into the guest vcpu due to it is
>> >not running (therefore, it cannot doorbell the vcpu directly), it logs
>> >the interrupt in the GA log buffer.
> Where is this documented?
>

http://support.amd.com/TechDocs/48882_IOMMU.pdf

Regards,
Suravee

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web