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

Groups > linux.kernel > #1360358 > unrolled thread

[PART1 RFC v3 00/12] KVM: x86: Introduce SVM AVIC support

Back to article view | Back to linux.kernel

Contents

  [PART1 RFC v3 00/12] KVM: x86: Introduce SVM AVIC support Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-03-18 07:20 +0100
    [PART1 RFC v3 11/12] svm: Do not intercept CR8 when enable AVIC Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-03-18 07:20 +0100
      Re: [PART1 RFC v3 11/12] svm: Do not intercept CR8 when enable AVIC Radim Krčmář <rkrcmar@redhat.com> - 2016-03-18 22:20 +0100
    [PART1 RFC v3 03/12] KVM: x86: Introducing kvm_x86_ops VCPU blocking/unblocking hooks Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-03-18 07:20 +0100
      Re: [PART1 RFC v3 03/12] KVM: x86: Introducing kvm_x86_ops VCPU  blocking/unblocking hooks Paolo Bonzini <pbonzini@redhat.com> - 2016-03-18 11:20 +0100
    [PART1 RFC v3 05/12] svm: Introduce new AVIC VMCB registers Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-03-18 07:20 +0100
    [PART1 RFC v3 01/12] KVM: x86: Misc LAPIC changes to expose helper functions Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-03-18 07:20 +0100
      Re: [PART1 RFC v3 01/12] KVM: x86: Misc LAPIC changes to expose  helper functions Paolo Bonzini <pbonzini@redhat.com> - 2016-03-18 12:20 +0100
    [PART1 RFC v3 12/12] svm: Manage vcpu load/unload when enable AVIC Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-03-18 07:20 +0100
      Re: [PART1 RFC v3 12/12] svm: Manage vcpu load/unload when enable  AVIC Radim Krčmář <rkrcmar@redhat.com> - 2016-03-18 22:50 +0100
        Re: [PART1 RFC v3 12/12] svm: Manage vcpu load/unload when enable  AVIC Radim Krčmář <rkrcmar@redhat.com> - 2016-03-31 16:20 +0200
    [PART1 RFC v3 06/12] KVM: x86: Detect and Initialize AVIC support Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-03-18 07:20 +0100
      Re: [PART1 RFC v3 06/12] KVM: x86: Detect and Initialize AVIC support Paolo Bonzini <pbonzini@redhat.com> - 2016-03-18 12:30 +0100
    [PART1 RFC v3 10/12] svm: Do not expose x2APIC when enable AVIC Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-03-18 07:20 +0100
      Re: [PART1 RFC v3 10/12] svm: Do not expose x2APIC when enable AVIC Radim Krčmář <rkrcmar@redhat.com> - 2016-03-18 22:10 +0100
        Re: [PART1 RFC v3 10/12] svm: Do not expose x2APIC when enable AVIC Paolo Bonzini <pbonzini@redhat.com> - 2016-03-31 13:30 +0200

#1360358 — [PART1 RFC v3 00/12] KVM: x86: Introduce SVM AVIC support

CHANGES FROM RFCv2:
==================
(https://lkml.org/lkml/2016/3/4/746)
    * Do not use bit-field in VMCB.
    * Get rid of struct svm_vm_data, and consolidate the struct members
      into struct kvm_arch.
    * Do not abbreviate AVIC vmexit function and structure names.
    * Adding VM init/uninit function hooks and use it to initialize /
      uninitialize per-VM data structures.  
    * Simplify AVIC backing page allocation to use the emulated lapic
      register page.
    * Introduce kvm_vcpu_wake_up() (from Radim).
    * Clean up AVIC vmexit handling functions.
    * Replace pr_debug with new AVIC tracepoints.
    * Use per-vcpu AVIC enable check (besides the global avic flag).
    * Add smb_mb__after_atomic() in svm_deliver_avic_intr.
    * Get rid of iommu related code for now.

CHANGES FROM RFCv1:
==================
(https://lkml.org/lkml/2016/2/12/225)
    * Rebased from tip.git
    * Use the vAPIC backing page as the emulated LAPIC register page.
    * Clarified with HW engineer that Avic sets the IRR of all the cores
      targeted whether they are running or not. It sends doorbells to the
      ones that are running to get them to evaluate their new state.
      If one or more weren't running it does the VMEXIT to indicate that
      the host needs to go run the not running guests. It gives it the
      ID of one of the guests that it observed not running which should
      be enough of a hint for the hypervisor to track them all down.
    * Rewrite the logic for handling AVIC_INCOMPLETE_IPI #vmexit for
      IPI target not running case based on information above.
    * Rewrite the logic in avic_vcpu_load() and avic_set_running().
    * Rewrite the interrupt injection to remove the avic_pending_cnt
      variable.
    * Adding vcpu blocking/unblocking function hooks

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

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 fields

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 pass-through 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
===============

* BENCHMARK 1: HACKBENCH
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 |  248.53 (~10%) |  155.01 (~20%)

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 vcpu scheduling.

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.

* BENCHMARK 1: TAR DECOMPRESSION
This test measures the average running time (of 10 runs) of the following
tar decompression command with 1, 2, and 4 vcpus.

   tar xf linux-4.3.3.tar.xz

                |      4 vcpus 
 ---------------------------------
         Vanila |  10.26  
  AVIC disabled |  10.10 (~1.5%)
           AVIC |  10.07 (~1.8%)

Note: The unit of result below is in seconds (lower is better). 

* BENCHMARK 2: NETPERF w/ virtual network
This test creates a virtual network by setting up bridge and tap device
on the host and pass it into the VM as virtio-net-pci device w/ vhost. 
Then it sets up netserver in the host machine, and run netperf
in the VM with following option:

  netperf -H <netserver ip> -l 60 -t TCP_RR -D 2

                |    1 vcpu      
 ------------------------------------
         Vanila |  21623.887
  AVIC disabled |  21538.09 (~-.4%)
           AVIC |  21712.68 (~0.4%)

Note: The unit of result below is trans/sec (higher is better).

Preliminary result of both benchmarks show AVIC performance are slightly
better than the other two cases.

CURRENT UNTESTED USE-CASES
===========================
    - VM Migration (work in progress)
    - Nested VM

Any feedback and comments are very much appreciated.

Thank you,
Suravee

Radim Krčmář (1):
  KVM: split kvm_vcpu_wake_up from kvm_vcpu_kick

Suravee Suthikulpanit (11):
  KVM: x86: Misc LAPIC changes to expose helper functions
  KVM: x86: Introducing kvm_x86_ops VM init/uninit hooks
  KVM: x86: Introducing kvm_x86_ops VCPU blocking/unblocking hooks
  svm: Introduce new AVIC VMCB registers
  KVM: x86: Detect and Initialize AVIC support
  svm: Add interrupt injection via AVIC
  KVM: x86: Add trace events for AVIC
  svm: Add VMEXIT handlers for 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/kvm_host.h |  28 +-
 arch/x86/include/asm/svm.h      |  12 +-
 arch/x86/include/uapi/asm/svm.h |   9 +-
 arch/x86/kvm/lapic.c            | 110 ++++---
 arch/x86/kvm/lapic.h            |  11 +
 arch/x86/kvm/svm.c              | 707 +++++++++++++++++++++++++++++++++++++++-
 arch/x86/kvm/trace.h            |  57 ++++
 arch/x86/kvm/x86.c              |  12 +-
 include/linux/kvm_host.h        |   1 +
 virt/kvm/kvm_main.c             |  27 +-
 10 files changed, 891 insertions(+), 83 deletions(-)

-- 
1.9.1

[toc] | [next] | [standalone]

#1360359 — [PART1 RFC v3 11/12] svm: Do not intercept CR8 when enable AVIC

When enable AVIC:
    * Do not intercept CR8 since this should be handled by AVIC HW.
    * Also update TPR in APIC backing page when syncing CR8 before VMRUN

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

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index ba84d57..d5418c3 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -984,6 +984,8 @@ static __init int svm_hardware_setup(void)
 
 	if (avic) {
 		printk(KERN_INFO "kvm: AVIC enabled\n");
+
+		svm_x86_ops.update_cr8_intercept = NULL;
 	} else {
 		svm_x86_ops.deliver_posted_interrupt = NULL;
 	}
@@ -1097,7 +1099,8 @@ static void init_vmcb(struct vcpu_svm *svm)
 	set_cr_intercept(svm, INTERCEPT_CR0_WRITE);
 	set_cr_intercept(svm, INTERCEPT_CR3_WRITE);
 	set_cr_intercept(svm, INTERCEPT_CR4_WRITE);
-	set_cr_intercept(svm, INTERCEPT_CR8_WRITE);
+	if (!svm_vcpu_avic_enabled(svm))
+		set_cr_intercept(svm, INTERCEPT_CR8_WRITE);
 
 	set_dr_intercepts(svm);
 
@@ -4080,7 +4083,8 @@ static void update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 
-	if (is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK))
+	if ((is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK)) ||
+	     svm_vcpu_avic_enabled(svm))
 		return;
 
 	clr_cr_intercept(svm, INTERCEPT_CR8_WRITE);
@@ -4271,9 +4275,12 @@ static inline void sync_lapic_to_cr8(struct kvm_vcpu *vcpu)
 	if (is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK))
 		return;
 
-	cr8 = kvm_get_cr8(vcpu);
+	cr8 = kvm_get_cr8(vcpu) & V_TPR_MASK;
 	svm->vmcb->control.int_ctl &= ~V_TPR_MASK;
-	svm->vmcb->control.int_ctl |= cr8 & V_TPR_MASK;
+	svm->vmcb->control.int_ctl |= cr8;
+
+	if (svm_vcpu_avic_enabled(svm))
+		kvm_lapic_set_reg(svm->vcpu.arch.apic, APIC_TASKPRI, (u32)cr8 << 4);
 }
 
 static void svm_complete_interrupts(struct vcpu_svm *svm)
-- 
1.9.1

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

#1360933 — Re: [PART1 RFC v3 11/12] svm: Do not intercept CR8 when enable AVIC

2016-03-18 01:09-0500, Suravee Suthikulpanit:
> When enable AVIC:
>     * Do not intercept CR8 since this should be handled by AVIC HW.
>     * Also update TPR in APIC backing page when syncing CR8 before VMRUN
> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>  arch/x86/kvm/svm.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index ba84d57..d5418c3 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -984,6 +984,8 @@ static __init int svm_hardware_setup(void)
>  
>  	if (avic) {
>  		printk(KERN_INFO "kvm: AVIC enabled\n");
> +
> +		svm_x86_ops.update_cr8_intercept = NULL;

This doesn't look right.

>  	} else {
>  		svm_x86_ops.deliver_posted_interrupt = NULL;
>  	}
> @@ -1097,7 +1099,8 @@ static void init_vmcb(struct vcpu_svm *svm)
>  	set_cr_intercept(svm, INTERCEPT_CR0_WRITE);
>  	set_cr_intercept(svm, INTERCEPT_CR3_WRITE);
>  	set_cr_intercept(svm, INTERCEPT_CR4_WRITE);
> -	set_cr_intercept(svm, INTERCEPT_CR8_WRITE);
> +	if (!svm_vcpu_avic_enabled(svm))
> +		set_cr_intercept(svm, INTERCEPT_CR8_WRITE);
>  
>  	set_dr_intercepts(svm);
>  
> @@ -4080,7 +4083,8 @@ static void update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr)
>  {
>  	struct vcpu_svm *svm = to_svm(vcpu);
>  
> -	if (is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK))
> +	if ((is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK)) ||
> +	     svm_vcpu_avic_enabled(svm))
>  		return;
>  
>  	clr_cr_intercept(svm, INTERCEPT_CR8_WRITE);
> @@ -4271,9 +4275,12 @@ static inline void sync_lapic_to_cr8(struct kvm_vcpu *vcpu)
>  	if (is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK))
>  		return;

I think we can exit early with svm_vcpu_avic_enabled().

>  
> -	cr8 = kvm_get_cr8(vcpu);
> +	cr8 = kvm_get_cr8(vcpu) & V_TPR_MASK;
>  	svm->vmcb->control.int_ctl &= ~V_TPR_MASK;
> -	svm->vmcb->control.int_ctl |= cr8 & V_TPR_MASK;
> +	svm->vmcb->control.int_ctl |= cr8;
> +
> +	if (svm_vcpu_avic_enabled(svm))
> +		kvm_lapic_set_reg(svm->vcpu.arch.apic, APIC_TASKPRI, (u32)cr8 << 4);

kvm_get_cr8() == kvm_lapic_get_reg(APIC_TASKPRI) >> 4.

>  }
>  
>  static void svm_complete_interrupts(struct vcpu_svm *svm)
> -- 
> 1.9.1
> 

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

#1360360 — [PART1 RFC v3 03/12] KVM: x86: Introducing kvm_x86_ops VCPU blocking/unblocking hooks

Adding new function pointer in struct kvm_x86_ops, and calling them
from the kvm_arch_vcpu[blocking/unblocking].

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 arch/x86/include/asm/kvm_host.h | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 4b0dd0f..87eac2a 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -972,6 +972,10 @@ struct kvm_x86_ops {
 	 */
 	int (*pre_block)(struct kvm_vcpu *vcpu);
 	void (*post_block)(struct kvm_vcpu *vcpu);
+
+	void (*vcpu_blocking)(struct kvm_vcpu *vcpu);
+	void (*vcpu_unblocking)(struct kvm_vcpu *vcpu);
+
 	int (*update_pi_irte)(struct kvm *kvm, unsigned int host_irq,
 			      uint32_t guest_irq, bool set);
 };
@@ -1323,7 +1327,16 @@ bool kvm_intr_is_single_vcpu(struct kvm *kvm, struct kvm_lapic_irq *irq,
 void kvm_set_msi_irq(struct kvm_kernel_irq_routing_entry *e,
 		     struct kvm_lapic_irq *irq);
 
-static inline void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu) {}
-static inline void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu) {}
+static inline void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu)
+{
+	if (kvm_x86_ops->vcpu_blocking)
+		kvm_x86_ops->vcpu_blocking(vcpu);
+}
+
+static inline void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu)
+{
+	if (kvm_x86_ops->vcpu_unblocking)
+		kvm_x86_ops->vcpu_unblocking(vcpu);
+}
 
 #endif /* _ASM_X86_KVM_HOST_H */
-- 
1.9.1

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

#1360496 — Re: [PART1 RFC v3 03/12] KVM: x86: Introducing kvm_x86_ops VCPU blocking/unblocking hooks

On 18/03/2016 07:09, Suravee Suthikulpanit wrote:
> Adding new function pointer in struct kvm_x86_ops, and calling them
> from the kvm_arch_vcpu[blocking/unblocking].
> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>  arch/x86/include/asm/kvm_host.h | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 4b0dd0f..87eac2a 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -972,6 +972,10 @@ struct kvm_x86_ops {
>  	 */
>  	int (*pre_block)(struct kvm_vcpu *vcpu);
>  	void (*post_block)(struct kvm_vcpu *vcpu);
> +
> +	void (*vcpu_blocking)(struct kvm_vcpu *vcpu);
> +	void (*vcpu_unblocking)(struct kvm_vcpu *vcpu);
> +
>  	int (*update_pi_irte)(struct kvm *kvm, unsigned int host_irq,
>  			      uint32_t guest_irq, bool set);
>  };
> @@ -1323,7 +1327,16 @@ bool kvm_intr_is_single_vcpu(struct kvm *kvm, struct kvm_lapic_irq *irq,
>  void kvm_set_msi_irq(struct kvm_kernel_irq_routing_entry *e,
>  		     struct kvm_lapic_irq *irq);
>  
> -static inline void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu) {}
> -static inline void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu) {}
> +static inline void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu)
> +{
> +	if (kvm_x86_ops->vcpu_blocking)
> +		kvm_x86_ops->vcpu_blocking(vcpu);
> +}
> +
> +static inline void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu)
> +{
> +	if (kvm_x86_ops->vcpu_unblocking)
> +		kvm_x86_ops->vcpu_unblocking(vcpu);
> +}
>  
>  #endif /* _ASM_X86_KVM_HOST_H */
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

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

#1360361 — [PART1 RFC v3 05/12] svm: Introduce new AVIC VMCB registers

Introduce new AVIC VMCB registers.

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 arch/x86/include/asm/svm.h | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 6136d99..66e26a0 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -78,7 +78,8 @@ struct __attribute__ ((__packed__)) vmcb_control_area {
 	u32 exit_int_info;
 	u32 exit_int_info_err;
 	u64 nested_ctl;
-	u8 reserved_4[16];
+	u64 avic_vapic_bar;
+	u8 reserved_4[8];
 	u32 event_inj;
 	u32 event_inj_err;
 	u64 nested_cr3;
@@ -88,7 +89,11 @@ struct __attribute__ ((__packed__)) vmcb_control_area {
 	u64 next_rip;
 	u8 insn_len;
 	u8 insn_bytes[15];
-	u8 reserved_6[800];
+	u64 avic_backing_page;	/* Offset 0xe0 */
+	u8 reserved_6[8];	/* Offset 0xe8 */
+	u64 avic_log_apic_id;	/* Offset 0xf0 */
+	u64 avic_phy_apic_id;	/* Offset 0xf8 */
+	u8 reserved_7[768];
 };
 
 
-- 
1.9.1

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

#1360362 — [PART1 RFC v3 01/12] KVM: x86: Misc LAPIC changes to expose helper functions

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

Exporting LAPIC utility functions and macros for re-use in SVM code.

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 arch/x86/kvm/lapic.c | 110 ++++++++++++++++++++++++++-------------------------
 arch/x86/kvm/lapic.h |   8 ++++
 2 files changed, 65 insertions(+), 53 deletions(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 3a045f3..6da1628 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -59,9 +59,8 @@
 /* #define apic_debug(fmt,arg...) printk(KERN_WARNING fmt,##arg) */
 #define apic_debug(fmt, arg...)
 
-#define APIC_LVT_NUM			6
 /* 14 is the version for Xeon and Pentium 8.4.8*/
-#define APIC_VERSION			(0x14UL | ((APIC_LVT_NUM - 1) << 16))
+#define APIC_VERSION			(0x14UL | ((KVM_APIC_LVT_NUM - 1) << 16))
 #define LAPIC_MMIO_LENGTH		(1 << 12)
 /* followed define is not in apicdef.h */
 #define APIC_SHORT_MASK			0xc0000
@@ -76,10 +75,11 @@
 #define VEC_POS(v) ((v) & (32 - 1))
 #define REG_POS(v) (((v) >> 5) << 4)
 
-static inline void apic_set_reg(struct kvm_lapic *apic, int reg_off, u32 val)
+void kvm_lapic_set_reg(struct kvm_lapic *apic, int reg_off, u32 val)
 {
 	*((u32 *) (apic->regs + reg_off)) = val;
 }
+EXPORT_SYMBOL_GPL(kvm_lapic_set_reg);
 
 static inline int apic_test_vector(int vec, void *bitmap)
 {
@@ -94,10 +94,11 @@ bool kvm_apic_pending_eoi(struct kvm_vcpu *vcpu, int vector)
 		apic_test_vector(vector, apic->regs + APIC_IRR);
 }
 
-static inline void apic_set_vector(int vec, void *bitmap)
+void kvm_lapic_set_vector(int vec, void *bitmap)
 {
 	set_bit(VEC_POS(vec), (bitmap) + REG_POS(vec));
 }
+EXPORT_SYMBOL_GPL(kvm_lapic_set_vector);
 
 static inline void apic_clear_vector(int vec, void *bitmap)
 {
@@ -212,7 +213,7 @@ static inline void apic_set_spiv(struct kvm_lapic *apic, u32 val)
 {
 	bool enabled = val & APIC_SPIV_APIC_ENABLED;
 
-	apic_set_reg(apic, APIC_SPIV, val);
+	kvm_lapic_set_reg(apic, APIC_SPIV, val);
 
 	if (enabled != apic->sw_enabled) {
 		apic->sw_enabled = enabled;
@@ -226,13 +227,13 @@ static inline void apic_set_spiv(struct kvm_lapic *apic, u32 val)
 
 static inline void kvm_apic_set_id(struct kvm_lapic *apic, u8 id)
 {
-	apic_set_reg(apic, APIC_ID, id << 24);
+	kvm_lapic_set_reg(apic, APIC_ID, id << 24);
 	recalculate_apic_map(apic->vcpu->kvm);
 }
 
 static inline void kvm_apic_set_ldr(struct kvm_lapic *apic, u32 id)
 {
-	apic_set_reg(apic, APIC_LDR, id);
+	kvm_lapic_set_reg(apic, APIC_LDR, id);
 	recalculate_apic_map(apic->vcpu->kvm);
 }
 
@@ -240,8 +241,8 @@ static inline void kvm_apic_set_x2apic_id(struct kvm_lapic *apic, u8 id)
 {
 	u32 ldr = ((id >> 4) << 16) | (1 << (id & 0xf));
 
-	apic_set_reg(apic, APIC_ID, id << 24);
-	apic_set_reg(apic, APIC_LDR, ldr);
+	kvm_lapic_set_reg(apic, APIC_ID, id << 24);
+	kvm_lapic_set_reg(apic, APIC_LDR, ldr);
 	recalculate_apic_map(apic->vcpu->kvm);
 }
 
@@ -287,10 +288,10 @@ void kvm_apic_set_version(struct kvm_vcpu *vcpu)
 	feat = kvm_find_cpuid_entry(apic->vcpu, 0x1, 0);
 	if (feat && (feat->ecx & (1 << (X86_FEATURE_X2APIC & 31))))
 		v |= APIC_LVR_DIRECTED_EOI;
-	apic_set_reg(apic, APIC_LVR, v);
+	kvm_lapic_set_reg(apic, APIC_LVR, v);
 }
 
-static const unsigned int apic_lvt_mask[APIC_LVT_NUM] = {
+static const unsigned int apic_lvt_mask[KVM_APIC_LVT_NUM] = {
 	LVT_MASK ,      /* part LVTT mask, timer mode mask added at runtime */
 	LVT_MASK | APIC_MODE_MASK,	/* LVTTHMR */
 	LVT_MASK | APIC_MODE_MASK,	/* LVTPC */
@@ -351,7 +352,7 @@ EXPORT_SYMBOL_GPL(kvm_apic_update_irr);
 
 static inline void apic_set_irr(int vec, struct kvm_lapic *apic)
 {
-	apic_set_vector(vec, apic->regs + APIC_IRR);
+	kvm_lapic_set_vector(vec, apic->regs + APIC_IRR);
 	/*
 	 * irr_pending must be true if any interrupt is pending; set it after
 	 * APIC_IRR to avoid race with apic_clear_irr
@@ -569,7 +570,7 @@ static void apic_update_ppr(struct kvm_lapic *apic)
 		   apic, ppr, isr, isrv);
 
 	if (old_ppr != ppr) {
-		apic_set_reg(apic, APIC_PROCPRI, ppr);
+		kvm_lapic_set_reg(apic, APIC_PROCPRI, ppr);
 		if (ppr < old_ppr)
 			kvm_make_request(KVM_REQ_EVENT, apic->vcpu);
 	}
@@ -577,7 +578,7 @@ static void apic_update_ppr(struct kvm_lapic *apic)
 
 static void apic_set_tpr(struct kvm_lapic *apic, u32 tpr)
 {
-	apic_set_reg(apic, APIC_TASKPRI, tpr);
+	kvm_lapic_set_reg(apic, APIC_TASKPRI, tpr);
 	apic_update_ppr(apic);
 }
 
@@ -674,6 +675,7 @@ bool kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
 		return false;
 	}
 }
+EXPORT_SYMBOL_GPL(kvm_apic_match_dest);
 
 bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
 		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map)
@@ -844,7 +846,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 
 		if (apic_test_vector(vector, apic->regs + APIC_TMR) != !!trig_mode) {
 			if (trig_mode)
-				apic_set_vector(vector, apic->regs + APIC_TMR);
+				kvm_lapic_set_vector(vector, apic->regs + APIC_TMR);
 			else
 				apic_clear_vector(vector, apic->regs + APIC_TMR);
 		}
@@ -1109,7 +1111,7 @@ static inline struct kvm_lapic *to_lapic(struct kvm_io_device *dev)
 	return container_of(dev, struct kvm_lapic, dev);
 }
 
-static int apic_reg_read(struct kvm_lapic *apic, u32 offset, int len,
+int kvm_lapic_reg_read(struct kvm_lapic *apic, u32 offset, int len,
 		void *data)
 {
 	unsigned char alignment = offset & 0xf;
@@ -1146,6 +1148,7 @@ static int apic_reg_read(struct kvm_lapic *apic, u32 offset, int len,
 	}
 	return 0;
 }
+EXPORT_SYMBOL_GPL(kvm_lapic_reg_read);
 
 static int apic_mmio_in_range(struct kvm_lapic *apic, gpa_t addr)
 {
@@ -1163,7 +1166,7 @@ static int apic_mmio_read(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
 	if (!apic_mmio_in_range(apic, address))
 		return -EOPNOTSUPP;
 
-	apic_reg_read(apic, offset, len, data);
+	kvm_lapic_reg_read(apic, offset, len, data);
 
 	return 0;
 }
@@ -1348,7 +1351,7 @@ static void apic_manage_nmi_watchdog(struct kvm_lapic *apic, u32 lvt0_val)
 	}
 }
 
-static int apic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
+int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
 {
 	int ret = 0;
 
@@ -1380,7 +1383,7 @@ static int apic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
 
 	case APIC_DFR:
 		if (!apic_x2apic_mode(apic)) {
-			apic_set_reg(apic, APIC_DFR, val | 0x0FFFFFFF);
+			kvm_lapic_set_reg(apic, APIC_DFR, val | 0x0FFFFFFF);
 			recalculate_apic_map(apic->vcpu->kvm);
 		} else
 			ret = 1;
@@ -1395,10 +1398,10 @@ static int apic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
 			int i;
 			u32 lvt_val;
 
-			for (i = 0; i < APIC_LVT_NUM; i++) {
+			for (i = 0; i < KVM_APIC_LVT_NUM; i++) {
 				lvt_val = kvm_apic_get_reg(apic,
 						       APIC_LVTT + 0x10 * i);
-				apic_set_reg(apic, APIC_LVTT + 0x10 * i,
+				kvm_lapic_set_reg(apic, APIC_LVTT + 0x10 * i,
 					     lvt_val | APIC_LVT_MASKED);
 			}
 			apic_update_lvtt(apic);
@@ -1409,14 +1412,14 @@ static int apic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
 	}
 	case APIC_ICR:
 		/* No delay here, so we always clear the pending bit */
-		apic_set_reg(apic, APIC_ICR, val & ~(1 << 12));
+		kvm_lapic_set_reg(apic, APIC_ICR, val & ~(1 << 12));
 		apic_send_ipi(apic);
 		break;
 
 	case APIC_ICR2:
 		if (!apic_x2apic_mode(apic))
 			val &= 0xff000000;
-		apic_set_reg(apic, APIC_ICR2, val);
+		kvm_lapic_set_reg(apic, APIC_ICR2, val);
 		break;
 
 	case APIC_LVT0:
@@ -1430,7 +1433,7 @@ static int apic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
 			val |= APIC_LVT_MASKED;
 
 		val &= apic_lvt_mask[(reg - APIC_LVTT) >> 4];
-		apic_set_reg(apic, reg, val);
+		kvm_lapic_set_reg(apic, reg, val);
 
 		break;
 
@@ -1438,7 +1441,7 @@ static int apic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
 		if (!kvm_apic_sw_enabled(apic))
 			val |= APIC_LVT_MASKED;
 		val &= (apic_lvt_mask[0] | apic->lapic_timer.timer_mode_mask);
-		apic_set_reg(apic, APIC_LVTT, val);
+		kvm_lapic_set_reg(apic, APIC_LVTT, val);
 		apic_update_lvtt(apic);
 		break;
 
@@ -1447,14 +1450,14 @@ static int apic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
 			break;
 
 		hrtimer_cancel(&apic->lapic_timer.timer);
-		apic_set_reg(apic, APIC_TMICT, val);
+		kvm_lapic_set_reg(apic, APIC_TMICT, val);
 		start_apic_timer(apic);
 		break;
 
 	case APIC_TDCR:
 		if (val & 4)
 			apic_debug("KVM_WRITE:TDCR %x\n", val);
-		apic_set_reg(apic, APIC_TDCR, val);
+		kvm_lapic_set_reg(apic, APIC_TDCR, val);
 		update_divide_count(apic);
 		break;
 
@@ -1467,7 +1470,7 @@ static int apic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
 
 	case APIC_SELF_IPI:
 		if (apic_x2apic_mode(apic)) {
-			apic_reg_write(apic, APIC_ICR, 0x40000 | (val & 0xff));
+			kvm_lapic_reg_write(apic, APIC_ICR, 0x40000 | (val & 0xff));
 		} else
 			ret = 1;
 		break;
@@ -1479,6 +1482,7 @@ static int apic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
 		apic_debug("Local APIC Write to read-only register %x\n", reg);
 	return ret;
 }
+EXPORT_SYMBOL_GPL(kvm_lapic_reg_write);
 
 static int apic_mmio_write(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
 			    gpa_t address, int len, const void *data)
@@ -1508,7 +1512,7 @@ static int apic_mmio_write(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
 		apic_debug("%s: offset 0x%x with length 0x%x, and value is "
 			   "0x%x\n", __func__, offset, len, val);
 
-	apic_reg_write(apic, offset & 0xff0, val);
+	kvm_lapic_reg_write(apic, offset & 0xff0, val);
 
 	return 0;
 }
@@ -1516,7 +1520,7 @@ static int apic_mmio_write(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
 void kvm_lapic_set_eoi(struct kvm_vcpu *vcpu)
 {
 	if (kvm_vcpu_has_lapic(vcpu))
-		apic_reg_write(vcpu->arch.apic, APIC_EOI, 0);
+		kvm_lapic_reg_write(vcpu->arch.apic, APIC_EOI, 0);
 }
 EXPORT_SYMBOL_GPL(kvm_lapic_set_eoi);
 
@@ -1528,10 +1532,10 @@ void kvm_apic_write_nodecode(struct kvm_vcpu *vcpu, u32 offset)
 	/* hw has done the conditional check and inst decode */
 	offset &= 0xff0;
 
-	apic_reg_read(vcpu->arch.apic, offset, 4, &val);
+	kvm_lapic_reg_read(vcpu->arch.apic, offset, 4, &val);
 
 	/* TODO: optimize to just emulate side effect w/o one more write */
-	apic_reg_write(vcpu->arch.apic, offset, val);
+	kvm_lapic_reg_write(vcpu->arch.apic, offset, val);
 }
 EXPORT_SYMBOL_GPL(kvm_apic_write_nodecode);
 
@@ -1670,28 +1674,28 @@ void kvm_lapic_reset(struct kvm_vcpu *vcpu, bool init_event)
 		kvm_apic_set_id(apic, vcpu->vcpu_id);
 	kvm_apic_set_version(apic->vcpu);
 
-	for (i = 0; i < APIC_LVT_NUM; i++)
-		apic_set_reg(apic, APIC_LVTT + 0x10 * i, APIC_LVT_MASKED);
+	for (i = 0; i < KVM_APIC_LVT_NUM; i++)
+		kvm_lapic_set_reg(apic, APIC_LVTT + 0x10 * i, APIC_LVT_MASKED);
 	apic_update_lvtt(apic);
 	if (kvm_check_has_quirk(vcpu->kvm, KVM_X86_QUIRK_LINT0_REENABLED))
-		apic_set_reg(apic, APIC_LVT0,
+		kvm_lapic_set_reg(apic, APIC_LVT0,
 			     SET_APIC_DELIVERY_MODE(0, APIC_MODE_EXTINT));
 	apic_manage_nmi_watchdog(apic, kvm_apic_get_reg(apic, APIC_LVT0));
 
-	apic_set_reg(apic, APIC_DFR, 0xffffffffU);
+	kvm_lapic_set_reg(apic, APIC_DFR, 0xffffffffU);
 	apic_set_spiv(apic, 0xff);
-	apic_set_reg(apic, APIC_TASKPRI, 0);
+	kvm_lapic_set_reg(apic, APIC_TASKPRI, 0);
 	if (!apic_x2apic_mode(apic))
 		kvm_apic_set_ldr(apic, 0);
-	apic_set_reg(apic, APIC_ESR, 0);
-	apic_set_reg(apic, APIC_ICR, 0);
-	apic_set_reg(apic, APIC_ICR2, 0);
-	apic_set_reg(apic, APIC_TDCR, 0);
-	apic_set_reg(apic, APIC_TMICT, 0);
+	kvm_lapic_set_reg(apic, APIC_ESR, 0);
+	kvm_lapic_set_reg(apic, APIC_ICR, 0);
+	kvm_lapic_set_reg(apic, APIC_ICR2, 0);
+	kvm_lapic_set_reg(apic, APIC_TDCR, 0);
+	kvm_lapic_set_reg(apic, APIC_TMICT, 0);
 	for (i = 0; i < 8; i++) {
-		apic_set_reg(apic, APIC_IRR + 0x10 * i, 0);
-		apic_set_reg(apic, APIC_ISR + 0x10 * i, 0);
-		apic_set_reg(apic, APIC_TMR + 0x10 * i, 0);
+		kvm_lapic_set_reg(apic, APIC_IRR + 0x10 * i, 0);
+		kvm_lapic_set_reg(apic, APIC_ISR + 0x10 * i, 0);
+		kvm_lapic_set_reg(apic, APIC_TMR + 0x10 * i, 0);
 	}
 	apic->irr_pending = vcpu->arch.apicv_active;
 	apic->isr_count = vcpu->arch.apicv_active ? 1 : 0;
@@ -2073,8 +2077,8 @@ int kvm_x2apic_msr_write(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 
 	/* if this is ICR write vector before command */
 	if (reg == APIC_ICR)
-		apic_reg_write(apic, APIC_ICR2, (u32)(data >> 32));
-	return apic_reg_write(apic, reg, (u32)data);
+		kvm_lapic_reg_write(apic, APIC_ICR2, (u32)(data >> 32));
+	return kvm_lapic_reg_write(apic, reg, (u32)data);
 }
 
 int kvm_x2apic_msr_read(struct kvm_vcpu *vcpu, u32 msr, u64 *data)
@@ -2091,10 +2095,10 @@ int kvm_x2apic_msr_read(struct kvm_vcpu *vcpu, u32 msr, u64 *data)
 		return 1;
 	}
 
-	if (apic_reg_read(apic, reg, 4, &low))
+	if (kvm_lapic_reg_read(apic, reg, 4, &low))
 		return 1;
 	if (reg == APIC_ICR)
-		apic_reg_read(apic, APIC_ICR2, 4, &high);
+		kvm_lapic_reg_read(apic, APIC_ICR2, 4, &high);
 
 	*data = (((u64)high) << 32) | low;
 
@@ -2110,8 +2114,8 @@ int kvm_hv_vapic_msr_write(struct kvm_vcpu *vcpu, u32 reg, u64 data)
 
 	/* if this is ICR write vector before command */
 	if (reg == APIC_ICR)
-		apic_reg_write(apic, APIC_ICR2, (u32)(data >> 32));
-	return apic_reg_write(apic, reg, (u32)data);
+		kvm_lapic_reg_write(apic, APIC_ICR2, (u32)(data >> 32));
+	return kvm_lapic_reg_write(apic, reg, (u32)data);
 }
 
 int kvm_hv_vapic_msr_read(struct kvm_vcpu *vcpu, u32 reg, u64 *data)
@@ -2122,10 +2126,10 @@ int kvm_hv_vapic_msr_read(struct kvm_vcpu *vcpu, u32 reg, u64 *data)
 	if (!kvm_vcpu_has_lapic(vcpu))
 		return 1;
 
-	if (apic_reg_read(apic, reg, 4, &low))
+	if (kvm_lapic_reg_read(apic, reg, 4, &low))
 		return 1;
 	if (reg == APIC_ICR)
-		apic_reg_read(apic, APIC_ICR2, 4, &high);
+		kvm_lapic_reg_read(apic, APIC_ICR2, 4, &high);
 
 	*data = (((u64)high) << 32) | low;
 
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index 41bdb35..7bf8184 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -7,6 +7,7 @@
 
 #define KVM_APIC_INIT		0
 #define KVM_APIC_SIPI		1
+#define KVM_APIC_LVT_NUM	6
 
 struct kvm_timer {
 	struct hrtimer timer;
@@ -56,6 +57,12 @@ void kvm_lapic_set_eoi(struct kvm_vcpu *vcpu);
 void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 value);
 u64 kvm_lapic_get_base(struct kvm_vcpu *vcpu);
 void kvm_apic_set_version(struct kvm_vcpu *vcpu);
+void kvm_lapic_set_vector(int vec, void *bitmap);
+int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val);
+int kvm_lapic_reg_read(struct kvm_lapic *apic, u32 offset, int len,
+		       void *data);
+bool kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
+			   int short_hand, unsigned int dest, int dest_mode);
 
 void __kvm_apic_update_irr(u32 *pir, void *regs);
 void kvm_apic_update_irr(struct kvm_vcpu *vcpu, u32 *pir);
@@ -96,6 +103,7 @@ static inline bool kvm_hv_vapic_assist_page_enabled(struct kvm_vcpu *vcpu)
 int kvm_lapic_enable_pv_eoi(struct kvm_vcpu *vcpu, u64 data);
 void kvm_lapic_init(void);
 
+void kvm_lapic_set_reg(struct kvm_lapic *apic, int reg_off, u32 val);
 static inline u32 kvm_apic_get_reg(struct kvm_lapic *apic, int reg_off)
 {
 	        return *((u32 *) (apic->regs + reg_off));
-- 
1.9.1

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

#1360537 — Re: [PART1 RFC v3 01/12] KVM: x86: Misc LAPIC changes to expose helper functions

On 18/03/2016 07:09, Suravee Suthikulpanit wrote:
> From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> 
> Exporting LAPIC utility functions and macros for re-use in SVM code.
> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>  arch/x86/kvm/lapic.c | 110 ++++++++++++++++++++++++++-------------------------
>  arch/x86/kvm/lapic.h |   8 ++++
>  2 files changed, 65 insertions(+), 53 deletions(-)
> 
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 3a045f3..6da1628 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -59,9 +59,8 @@
>  /* #define apic_debug(fmt,arg...) printk(KERN_WARNING fmt,##arg) */
>  #define apic_debug(fmt, arg...)
>  
> -#define APIC_LVT_NUM			6
>  /* 14 is the version for Xeon and Pentium 8.4.8*/
> -#define APIC_VERSION			(0x14UL | ((APIC_LVT_NUM - 1) << 16))
> +#define APIC_VERSION			(0x14UL | ((KVM_APIC_LVT_NUM - 1) << 16))
>  #define LAPIC_MMIO_LENGTH		(1 << 12)
>  /* followed define is not in apicdef.h */
>  #define APIC_SHORT_MASK			0xc0000
> @@ -76,10 +75,11 @@
>  #define VEC_POS(v) ((v) & (32 - 1))
>  #define REG_POS(v) (((v) >> 5) << 4)
>  
> -static inline void apic_set_reg(struct kvm_lapic *apic, int reg_off, u32 val)
> +void kvm_lapic_set_reg(struct kvm_lapic *apic, int reg_off, u32 val)
>  {
>  	*((u32 *) (apic->regs + reg_off)) = val;
>  }
> +EXPORT_SYMBOL_GPL(kvm_lapic_set_reg);
>  
>  static inline int apic_test_vector(int vec, void *bitmap)
>  {

Please keep the function inline, moving it to lapic.h

> @@ -94,10 +94,11 @@ bool kvm_apic_pending_eoi(struct kvm_vcpu *vcpu, int vector)
>  		apic_test_vector(vector, apic->regs + APIC_IRR);
>  }
>  
> -static inline void apic_set_vector(int vec, void *bitmap)
> +void kvm_lapic_set_vector(int vec, void *bitmap)
>  {
>  	set_bit(VEC_POS(vec), (bitmap) + REG_POS(vec));
>  }
> +EXPORT_SYMBOL_GPL(kvm_lapic_set_vector);

Same here, and also move apic_set_irr to lapic.h so that you don't have
to use the lower-level apic_set_vector directly.  (Of course renaming it
to kvm_lapic_set_irr).

Otherwise okay.

Paolo

>  static inline void apic_clear_vector(int vec, void *bitmap)
>  {
> @@ -212,7 +213,7 @@ static inline void apic_set_spiv(struct kvm_lapic *apic, u32 val)
>  {
>  	bool enabled = val & APIC_SPIV_APIC_ENABLED;
>  
> -	apic_set_reg(apic, APIC_SPIV, val);
> +	kvm_lapic_set_reg(apic, APIC_SPIV, val);
>  
>  	if (enabled != apic->sw_enabled) {
>  		apic->sw_enabled = enabled;
> @@ -226,13 +227,13 @@ static inline void apic_set_spiv(struct kvm_lapic *apic, u32 val)
>  
>  static inline void kvm_apic_set_id(struct kvm_lapic *apic, u8 id)
>  {
> -	apic_set_reg(apic, APIC_ID, id << 24);
> +	kvm_lapic_set_reg(apic, APIC_ID, id << 24);
>  	recalculate_apic_map(apic->vcpu->kvm);
>  }
>  
>  static inline void kvm_apic_set_ldr(struct kvm_lapic *apic, u32 id)
>  {
> -	apic_set_reg(apic, APIC_LDR, id);
> +	kvm_lapic_set_reg(apic, APIC_LDR, id);
>  	recalculate_apic_map(apic->vcpu->kvm);
>  }
>  
> @@ -240,8 +241,8 @@ static inline void kvm_apic_set_x2apic_id(struct kvm_lapic *apic, u8 id)
>  {
>  	u32 ldr = ((id >> 4) << 16) | (1 << (id & 0xf));
>  
> -	apic_set_reg(apic, APIC_ID, id << 24);
> -	apic_set_reg(apic, APIC_LDR, ldr);
> +	kvm_lapic_set_reg(apic, APIC_ID, id << 24);
> +	kvm_lapic_set_reg(apic, APIC_LDR, ldr);
>  	recalculate_apic_map(apic->vcpu->kvm);
>  }
>  
> @@ -287,10 +288,10 @@ void kvm_apic_set_version(struct kvm_vcpu *vcpu)
>  	feat = kvm_find_cpuid_entry(apic->vcpu, 0x1, 0);
>  	if (feat && (feat->ecx & (1 << (X86_FEATURE_X2APIC & 31))))
>  		v |= APIC_LVR_DIRECTED_EOI;
> -	apic_set_reg(apic, APIC_LVR, v);
> +	kvm_lapic_set_reg(apic, APIC_LVR, v);
>  }
>  
> -static const unsigned int apic_lvt_mask[APIC_LVT_NUM] = {
> +static const unsigned int apic_lvt_mask[KVM_APIC_LVT_NUM] = {
>  	LVT_MASK ,      /* part LVTT mask, timer mode mask added at runtime */
>  	LVT_MASK | APIC_MODE_MASK,	/* LVTTHMR */
>  	LVT_MASK | APIC_MODE_MASK,	/* LVTPC */
> @@ -351,7 +352,7 @@ EXPORT_SYMBOL_GPL(kvm_apic_update_irr);
>  
>  static inline void apic_set_irr(int vec, struct kvm_lapic *apic)
>  {
> -	apic_set_vector(vec, apic->regs + APIC_IRR);
> +	kvm_lapic_set_vector(vec, apic->regs + APIC_IRR);
>  	/*
>  	 * irr_pending must be true if any interrupt is pending; set it after
>  	 * APIC_IRR to avoid race with apic_clear_irr
> @@ -569,7 +570,7 @@ static void apic_update_ppr(struct kvm_lapic *apic)
>  		   apic, ppr, isr, isrv);
>  
>  	if (old_ppr != ppr) {
> -		apic_set_reg(apic, APIC_PROCPRI, ppr);
> +		kvm_lapic_set_reg(apic, APIC_PROCPRI, ppr);
>  		if (ppr < old_ppr)
>  			kvm_make_request(KVM_REQ_EVENT, apic->vcpu);
>  	}
> @@ -577,7 +578,7 @@ static void apic_update_ppr(struct kvm_lapic *apic)
>  
>  static void apic_set_tpr(struct kvm_lapic *apic, u32 tpr)
>  {
> -	apic_set_reg(apic, APIC_TASKPRI, tpr);
> +	kvm_lapic_set_reg(apic, APIC_TASKPRI, tpr);
>  	apic_update_ppr(apic);
>  }
>  
> @@ -674,6 +675,7 @@ bool kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
>  		return false;
>  	}
>  }
> +EXPORT_SYMBOL_GPL(kvm_apic_match_dest);
>  
>  bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
>  		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map)
> @@ -844,7 +846,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
>  
>  		if (apic_test_vector(vector, apic->regs + APIC_TMR) != !!trig_mode) {
>  			if (trig_mode)
> -				apic_set_vector(vector, apic->regs + APIC_TMR);
> +				kvm_lapic_set_vector(vector, apic->regs + APIC_TMR);
>  			else
>  				apic_clear_vector(vector, apic->regs + APIC_TMR);
>  		}
> @@ -1109,7 +1111,7 @@ static inline struct kvm_lapic *to_lapic(struct kvm_io_device *dev)
>  	return container_of(dev, struct kvm_lapic, dev);
>  }
>  
> -static int apic_reg_read(struct kvm_lapic *apic, u32 offset, int len,
> +int kvm_lapic_reg_read(struct kvm_lapic *apic, u32 offset, int len,
>  		void *data)
>  {
>  	unsigned char alignment = offset & 0xf;
> @@ -1146,6 +1148,7 @@ static int apic_reg_read(struct kvm_lapic *apic, u32 offset, int len,
>  	}
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(kvm_lapic_reg_read);
>  
>  static int apic_mmio_in_range(struct kvm_lapic *apic, gpa_t addr)
>  {
> @@ -1163,7 +1166,7 @@ static int apic_mmio_read(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
>  	if (!apic_mmio_in_range(apic, address))
>  		return -EOPNOTSUPP;
>  
> -	apic_reg_read(apic, offset, len, data);
> +	kvm_lapic_reg_read(apic, offset, len, data);
>  
>  	return 0;
>  }
> @@ -1348,7 +1351,7 @@ static void apic_manage_nmi_watchdog(struct kvm_lapic *apic, u32 lvt0_val)
>  	}
>  }
>  
> -static int apic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
> +int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
>  {
>  	int ret = 0;
>  
> @@ -1380,7 +1383,7 @@ static int apic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
>  
>  	case APIC_DFR:
>  		if (!apic_x2apic_mode(apic)) {
> -			apic_set_reg(apic, APIC_DFR, val | 0x0FFFFFFF);
> +			kvm_lapic_set_reg(apic, APIC_DFR, val | 0x0FFFFFFF);
>  			recalculate_apic_map(apic->vcpu->kvm);
>  		} else
>  			ret = 1;
> @@ -1395,10 +1398,10 @@ static int apic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
>  			int i;
>  			u32 lvt_val;
>  
> -			for (i = 0; i < APIC_LVT_NUM; i++) {
> +			for (i = 0; i < KVM_APIC_LVT_NUM; i++) {
>  				lvt_val = kvm_apic_get_reg(apic,
>  						       APIC_LVTT + 0x10 * i);
> -				apic_set_reg(apic, APIC_LVTT + 0x10 * i,
> +				kvm_lapic_set_reg(apic, APIC_LVTT + 0x10 * i,
>  					     lvt_val | APIC_LVT_MASKED);
>  			}
>  			apic_update_lvtt(apic);
> @@ -1409,14 +1412,14 @@ static int apic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
>  	}
>  	case APIC_ICR:
>  		/* No delay here, so we always clear the pending bit */
> -		apic_set_reg(apic, APIC_ICR, val & ~(1 << 12));
> +		kvm_lapic_set_reg(apic, APIC_ICR, val & ~(1 << 12));
>  		apic_send_ipi(apic);
>  		break;
>  
>  	case APIC_ICR2:
>  		if (!apic_x2apic_mode(apic))
>  			val &= 0xff000000;
> -		apic_set_reg(apic, APIC_ICR2, val);
> +		kvm_lapic_set_reg(apic, APIC_ICR2, val);
>  		break;
>  
>  	case APIC_LVT0:
> @@ -1430,7 +1433,7 @@ static int apic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
>  			val |= APIC_LVT_MASKED;
>  
>  		val &= apic_lvt_mask[(reg - APIC_LVTT) >> 4];
> -		apic_set_reg(apic, reg, val);
> +		kvm_lapic_set_reg(apic, reg, val);
>  
>  		break;
>  
> @@ -1438,7 +1441,7 @@ static int apic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
>  		if (!kvm_apic_sw_enabled(apic))
>  			val |= APIC_LVT_MASKED;
>  		val &= (apic_lvt_mask[0] | apic->lapic_timer.timer_mode_mask);
> -		apic_set_reg(apic, APIC_LVTT, val);
> +		kvm_lapic_set_reg(apic, APIC_LVTT, val);
>  		apic_update_lvtt(apic);
>  		break;
>  
> @@ -1447,14 +1450,14 @@ static int apic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
>  			break;
>  
>  		hrtimer_cancel(&apic->lapic_timer.timer);
> -		apic_set_reg(apic, APIC_TMICT, val);
> +		kvm_lapic_set_reg(apic, APIC_TMICT, val);
>  		start_apic_timer(apic);
>  		break;
>  
>  	case APIC_TDCR:
>  		if (val & 4)
>  			apic_debug("KVM_WRITE:TDCR %x\n", val);
> -		apic_set_reg(apic, APIC_TDCR, val);
> +		kvm_lapic_set_reg(apic, APIC_TDCR, val);
>  		update_divide_count(apic);
>  		break;
>  
> @@ -1467,7 +1470,7 @@ static int apic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
>  
>  	case APIC_SELF_IPI:
>  		if (apic_x2apic_mode(apic)) {
> -			apic_reg_write(apic, APIC_ICR, 0x40000 | (val & 0xff));
> +			kvm_lapic_reg_write(apic, APIC_ICR, 0x40000 | (val & 0xff));
>  		} else
>  			ret = 1;
>  		break;
> @@ -1479,6 +1482,7 @@ static int apic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
>  		apic_debug("Local APIC Write to read-only register %x\n", reg);
>  	return ret;
>  }
> +EXPORT_SYMBOL_GPL(kvm_lapic_reg_write);
>  
>  static int apic_mmio_write(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
>  			    gpa_t address, int len, const void *data)
> @@ -1508,7 +1512,7 @@ static int apic_mmio_write(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
>  		apic_debug("%s: offset 0x%x with length 0x%x, and value is "
>  			   "0x%x\n", __func__, offset, len, val);
>  
> -	apic_reg_write(apic, offset & 0xff0, val);
> +	kvm_lapic_reg_write(apic, offset & 0xff0, val);
>  
>  	return 0;
>  }
> @@ -1516,7 +1520,7 @@ static int apic_mmio_write(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
>  void kvm_lapic_set_eoi(struct kvm_vcpu *vcpu)
>  {
>  	if (kvm_vcpu_has_lapic(vcpu))
> -		apic_reg_write(vcpu->arch.apic, APIC_EOI, 0);
> +		kvm_lapic_reg_write(vcpu->arch.apic, APIC_EOI, 0);
>  }
>  EXPORT_SYMBOL_GPL(kvm_lapic_set_eoi);
>  
> @@ -1528,10 +1532,10 @@ void kvm_apic_write_nodecode(struct kvm_vcpu *vcpu, u32 offset)
>  	/* hw has done the conditional check and inst decode */
>  	offset &= 0xff0;
>  
> -	apic_reg_read(vcpu->arch.apic, offset, 4, &val);
> +	kvm_lapic_reg_read(vcpu->arch.apic, offset, 4, &val);
>  
>  	/* TODO: optimize to just emulate side effect w/o one more write */
> -	apic_reg_write(vcpu->arch.apic, offset, val);
> +	kvm_lapic_reg_write(vcpu->arch.apic, offset, val);
>  }
>  EXPORT_SYMBOL_GPL(kvm_apic_write_nodecode);
>  
> @@ -1670,28 +1674,28 @@ void kvm_lapic_reset(struct kvm_vcpu *vcpu, bool init_event)
>  		kvm_apic_set_id(apic, vcpu->vcpu_id);
>  	kvm_apic_set_version(apic->vcpu);
>  
> -	for (i = 0; i < APIC_LVT_NUM; i++)
> -		apic_set_reg(apic, APIC_LVTT + 0x10 * i, APIC_LVT_MASKED);
> +	for (i = 0; i < KVM_APIC_LVT_NUM; i++)
> +		kvm_lapic_set_reg(apic, APIC_LVTT + 0x10 * i, APIC_LVT_MASKED);
>  	apic_update_lvtt(apic);
>  	if (kvm_check_has_quirk(vcpu->kvm, KVM_X86_QUIRK_LINT0_REENABLED))
> -		apic_set_reg(apic, APIC_LVT0,
> +		kvm_lapic_set_reg(apic, APIC_LVT0,
>  			     SET_APIC_DELIVERY_MODE(0, APIC_MODE_EXTINT));
>  	apic_manage_nmi_watchdog(apic, kvm_apic_get_reg(apic, APIC_LVT0));
>  
> -	apic_set_reg(apic, APIC_DFR, 0xffffffffU);
> +	kvm_lapic_set_reg(apic, APIC_DFR, 0xffffffffU);
>  	apic_set_spiv(apic, 0xff);
> -	apic_set_reg(apic, APIC_TASKPRI, 0);
> +	kvm_lapic_set_reg(apic, APIC_TASKPRI, 0);
>  	if (!apic_x2apic_mode(apic))
>  		kvm_apic_set_ldr(apic, 0);
> -	apic_set_reg(apic, APIC_ESR, 0);
> -	apic_set_reg(apic, APIC_ICR, 0);
> -	apic_set_reg(apic, APIC_ICR2, 0);
> -	apic_set_reg(apic, APIC_TDCR, 0);
> -	apic_set_reg(apic, APIC_TMICT, 0);
> +	kvm_lapic_set_reg(apic, APIC_ESR, 0);
> +	kvm_lapic_set_reg(apic, APIC_ICR, 0);
> +	kvm_lapic_set_reg(apic, APIC_ICR2, 0);
> +	kvm_lapic_set_reg(apic, APIC_TDCR, 0);
> +	kvm_lapic_set_reg(apic, APIC_TMICT, 0);
>  	for (i = 0; i < 8; i++) {
> -		apic_set_reg(apic, APIC_IRR + 0x10 * i, 0);
> -		apic_set_reg(apic, APIC_ISR + 0x10 * i, 0);
> -		apic_set_reg(apic, APIC_TMR + 0x10 * i, 0);
> +		kvm_lapic_set_reg(apic, APIC_IRR + 0x10 * i, 0);
> +		kvm_lapic_set_reg(apic, APIC_ISR + 0x10 * i, 0);
> +		kvm_lapic_set_reg(apic, APIC_TMR + 0x10 * i, 0);
>  	}
>  	apic->irr_pending = vcpu->arch.apicv_active;
>  	apic->isr_count = vcpu->arch.apicv_active ? 1 : 0;
> @@ -2073,8 +2077,8 @@ int kvm_x2apic_msr_write(struct kvm_vcpu *vcpu, u32 msr, u64 data)
>  
>  	/* if this is ICR write vector before command */
>  	if (reg == APIC_ICR)
> -		apic_reg_write(apic, APIC_ICR2, (u32)(data >> 32));
> -	return apic_reg_write(apic, reg, (u32)data);
> +		kvm_lapic_reg_write(apic, APIC_ICR2, (u32)(data >> 32));
> +	return kvm_lapic_reg_write(apic, reg, (u32)data);
>  }
>  
>  int kvm_x2apic_msr_read(struct kvm_vcpu *vcpu, u32 msr, u64 *data)
> @@ -2091,10 +2095,10 @@ int kvm_x2apic_msr_read(struct kvm_vcpu *vcpu, u32 msr, u64 *data)
>  		return 1;
>  	}
>  
> -	if (apic_reg_read(apic, reg, 4, &low))
> +	if (kvm_lapic_reg_read(apic, reg, 4, &low))
>  		return 1;
>  	if (reg == APIC_ICR)
> -		apic_reg_read(apic, APIC_ICR2, 4, &high);
> +		kvm_lapic_reg_read(apic, APIC_ICR2, 4, &high);
>  
>  	*data = (((u64)high) << 32) | low;
>  
> @@ -2110,8 +2114,8 @@ int kvm_hv_vapic_msr_write(struct kvm_vcpu *vcpu, u32 reg, u64 data)
>  
>  	/* if this is ICR write vector before command */
>  	if (reg == APIC_ICR)
> -		apic_reg_write(apic, APIC_ICR2, (u32)(data >> 32));
> -	return apic_reg_write(apic, reg, (u32)data);
> +		kvm_lapic_reg_write(apic, APIC_ICR2, (u32)(data >> 32));
> +	return kvm_lapic_reg_write(apic, reg, (u32)data);
>  }
>  
>  int kvm_hv_vapic_msr_read(struct kvm_vcpu *vcpu, u32 reg, u64 *data)
> @@ -2122,10 +2126,10 @@ int kvm_hv_vapic_msr_read(struct kvm_vcpu *vcpu, u32 reg, u64 *data)
>  	if (!kvm_vcpu_has_lapic(vcpu))
>  		return 1;
>  
> -	if (apic_reg_read(apic, reg, 4, &low))
> +	if (kvm_lapic_reg_read(apic, reg, 4, &low))
>  		return 1;
>  	if (reg == APIC_ICR)
> -		apic_reg_read(apic, APIC_ICR2, 4, &high);
> +		kvm_lapic_reg_read(apic, APIC_ICR2, 4, &high);
>  
>  	*data = (((u64)high) << 32) | low;
>  
> diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
> index 41bdb35..7bf8184 100644
> --- a/arch/x86/kvm/lapic.h
> +++ b/arch/x86/kvm/lapic.h
> @@ -7,6 +7,7 @@
>  
>  #define KVM_APIC_INIT		0
>  #define KVM_APIC_SIPI		1
> +#define KVM_APIC_LVT_NUM	6
>  
>  struct kvm_timer {
>  	struct hrtimer timer;
> @@ -56,6 +57,12 @@ void kvm_lapic_set_eoi(struct kvm_vcpu *vcpu);
>  void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 value);
>  u64 kvm_lapic_get_base(struct kvm_vcpu *vcpu);
>  void kvm_apic_set_version(struct kvm_vcpu *vcpu);
> +void kvm_lapic_set_vector(int vec, void *bitmap);
> +int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val);
> +int kvm_lapic_reg_read(struct kvm_lapic *apic, u32 offset, int len,
> +		       void *data);
> +bool kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
> +			   int short_hand, unsigned int dest, int dest_mode);
>  
>  void __kvm_apic_update_irr(u32 *pir, void *regs);
>  void kvm_apic_update_irr(struct kvm_vcpu *vcpu, u32 *pir);
> @@ -96,6 +103,7 @@ static inline bool kvm_hv_vapic_assist_page_enabled(struct kvm_vcpu *vcpu)
>  int kvm_lapic_enable_pv_eoi(struct kvm_vcpu *vcpu, u64 data);
>  void kvm_lapic_init(void);
>  
> +void kvm_lapic_set_reg(struct kvm_lapic *apic, int reg_off, u32 val);
>  static inline u32 kvm_apic_get_reg(struct kvm_lapic *apic, int reg_off)
>  {
>  	        return *((u32 *) (apic->regs + reg_off));
> 

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

#1360363 — [PART1 RFC v3 12/12] svm: Manage vcpu load/unload when enable AVIC

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

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

Also, when vCPU is blocking/un-blocking (due to halt instruction),
we need to make sure that the is-running bit in set accordingly in the
physical APIC-ID table.

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

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index d5418c3..c5e8100 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -35,6 +35,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>
@@ -1334,6 +1335,110 @@ free_avic:
 	return err;
 }
 
+static int avic_vcpu_load(struct kvm_vcpu *vcpu, int cpu, bool is_load)
+{
+	int h_phy_apic_id;
+	u64 *entry, new_entry;
+	struct vcpu_svm *svm = to_svm(vcpu);
+	int ret = 0;
+
+	if (!svm_vcpu_avic_enabled(svm))
+		return 0;
+
+	if (!svm)
+		return -EINVAL;
+
+	/* Note: APIC ID = 0xff is used for broadcast.
+	 *       APIC ID > 0xff is reserved.
+	 */
+	h_phy_apic_id = __default_cpu_present_to_apicid(cpu);
+
+	if (h_phy_apic_id >= AVIC_PHY_APIC_ID_MAX)
+		return -EINVAL;
+
+	entry = svm->avic_phy_apic_id_cache;
+	if (!entry)
+		return -EINVAL;
+
+	if (is_load) {
+		new_entry = READ_ONCE(*entry);
+
+		BUG_ON(new_entry & AVIC_PHY_APIC_ID__IS_RUN_MSK);
+
+		new_entry &= ~AVIC_PHY_APIC_ID__HOST_PHY_APIC_ID_MSK;
+		new_entry |= (h_phy_apic_id & AVIC_PHY_APIC_ID__HOST_PHY_APIC_ID_MSK);
+
+		/**
+		 * Restore AVIC running flag if it was set during
+		 * vcpu unload.
+		 */
+		if (svm->avic_was_running)
+			new_entry |= AVIC_PHY_APIC_ID__IS_RUN_MSK;
+		else
+			new_entry &= ~AVIC_PHY_APIC_ID__IS_RUN_MSK;
+
+		WRITE_ONCE(*entry, new_entry);
+
+	} else {
+		new_entry = READ_ONCE(*entry);
+
+		/**
+		 * This handles the case when vcpu is scheduled out
+		 * and has not yet not called blocking. We save the
+		 * AVIC running flag so that we can restore later.
+		 */
+		if (new_entry & AVIC_PHY_APIC_ID__IS_RUN_MSK) {
+			svm->avic_was_running = true;
+			new_entry &= ~AVIC_PHY_APIC_ID__IS_RUN_MSK;
+			WRITE_ONCE(*entry, new_entry);
+		} else {
+			svm->avic_was_running = false;
+		}
+	}
+
+	return ret;
+}
+
+/**
+ * This function is called during VCPU halt/unhalt.
+ */
+static int avic_set_running(struct kvm_vcpu *vcpu, bool is_run)
+{
+	int ret = 0;
+	int h_phy_apic_id;
+	u64 *entry, new_entry;
+	struct vcpu_svm *svm = to_svm(vcpu);
+
+	if (!svm_vcpu_avic_enabled(svm))
+		return 0;
+
+	/* Note: APIC ID = 0xff is used for broadcast.
+	 *       APIC ID > 0xff is reserved.
+	 */
+	h_phy_apic_id = __default_cpu_present_to_apicid(vcpu->cpu);
+
+	if (h_phy_apic_id >= AVIC_PHY_APIC_ID_MAX)
+		return -EINVAL;
+
+	entry = svm->avic_phy_apic_id_cache;
+	if (!entry)
+		return -EINVAL;
+
+	if (is_run) {
+		/* Handle vcpu unblocking after HLT */
+		new_entry = READ_ONCE(*entry);
+		new_entry |= AVIC_PHY_APIC_ID__IS_RUN_MSK;
+		WRITE_ONCE(*entry, new_entry);
+	} else {
+		/* Handle vcpu blocking due to HLT */
+		new_entry = READ_ONCE(*entry);
+		new_entry &= ~AVIC_PHY_APIC_ID__IS_RUN_MSK;
+		WRITE_ONCE(*entry, new_entry);
+	}
+
+	return ret;
+}
+
 static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
@@ -1476,6 +1581,8 @@ static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 	/* This assumes that the kernel never uses MSR_TSC_AUX */
 	if (static_cpu_has(X86_FEATURE_RDTSCP))
 		wrmsrl(MSR_TSC_AUX, svm->tsc_aux);
+
+	avic_vcpu_load(vcpu, cpu, true);
 }
 
 static void svm_vcpu_put(struct kvm_vcpu *vcpu)
@@ -1483,6 +1590,8 @@ static void svm_vcpu_put(struct kvm_vcpu *vcpu)
 	struct vcpu_svm *svm = to_svm(vcpu);
 	int i;
 
+	avic_vcpu_load(vcpu, 0, false);
+
 	++vcpu->stat.host_state_reload;
 	kvm_load_ldt(svm->host.ldt);
 #ifdef CONFIG_X86_64
@@ -1498,6 +1607,16 @@ static void svm_vcpu_put(struct kvm_vcpu *vcpu)
 		wrmsrl(host_save_user_msrs[i], svm->host_user_msrs[i]);
 }
 
+static void svm_vcpu_blocking(struct kvm_vcpu *vcpu)
+{
+	avic_set_running(vcpu, false);
+}
+
+static void svm_vcpu_unblocking(struct kvm_vcpu *vcpu)
+{
+	avic_set_running(vcpu, true);
+}
+
 static unsigned long svm_get_rflags(struct kvm_vcpu *vcpu)
 {
 	return to_svm(vcpu)->vmcb->save.rflags;
@@ -4876,6 +4995,8 @@ static struct kvm_x86_ops svm_x86_ops = {
 	.prepare_guest_switch = svm_prepare_guest_switch,
 	.vcpu_load = svm_vcpu_load,
 	.vcpu_put = svm_vcpu_put,
+	.vcpu_blocking = svm_vcpu_blocking,
+	.vcpu_unblocking = svm_vcpu_unblocking,
 
 	.update_bp_intercept = update_bp_intercept,
 	.get_msr = svm_get_msr,
-- 
1.9.1

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

#1360948 — Re: [PART1 RFC v3 12/12] svm: Manage vcpu load/unload when enable AVIC

2016-03-18 01:09-0500, Suravee Suthikulpanit:
> From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> 
> When a vcpu is loaded/unloaded to a physical core, we need to update
> host physical APIC ID information in the Physical APIC-ID table
> accordingly.
> 
> Also, when vCPU is blocking/un-blocking (due to halt instruction),
> we need to make sure that the is-running bit in set accordingly in the
> physical APIC-ID table.
> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>  arch/x86/kvm/svm.c | 121 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 121 insertions(+)
> 
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> +static int avic_vcpu_load(struct kvm_vcpu *vcpu, int cpu, bool is_load)
> +{
> +	int h_phy_apic_id;

(Paolo said a lot about those names.)

> +	u64 *entry, new_entry;
> +	struct vcpu_svm *svm = to_svm(vcpu);
> +	int ret = 0;
> +	if (!svm_vcpu_avic_enabled(svm))
> +		return 0;

(The check for NULL below feels weird when it has already been used.)

> +
> +	if (!svm)
> +		return -EINVAL;

!svm means that KVM completely blew up ... don't check for it.
(See implementation of to_svm.)

> +
> +	/* Note: APIC ID = 0xff is used for broadcast.
> +	 *       APIC ID > 0xff is reserved.
> +	 */
> +	h_phy_apic_id = __default_cpu_present_to_apicid(cpu);
> +
> +	if (h_phy_apic_id >= AVIC_PHY_APIC_ID_MAX)
> +		return -EINVAL;
> +
> +	entry = svm->avic_phy_apic_id_cache;

The naming is confusing ... can avic_phy_apic_id_cache change during
execution of this function?
If yes, then add READ_ONCE and distinguish the pointer name.
If not, then use svm->avic_phy_apic_id_cache directly.

entry would be ok name for current new_entry.

> +	if (!entry)
> +		return -EINVAL;
> +
> +	if (is_load) {
> +		new_entry = READ_ONCE(*entry);

Please move this before the if.

> +		BUG_ON(new_entry & AVIC_PHY_APIC_ID__IS_RUN_MSK);
> +
> +		new_entry &= ~AVIC_PHY_APIC_ID__HOST_PHY_APIC_ID_MSK;
> +		new_entry |= (h_phy_apic_id & AVIC_PHY_APIC_ID__HOST_PHY_APIC_ID_MSK);
> +
> +		/**
> +		 * Restore AVIC running flag if it was set during
> +		 * vcpu unload.
> +		 */
> +		if (svm->avic_was_running)
> +			new_entry |= AVIC_PHY_APIC_ID__IS_RUN_MSK;
> +		else
> +			new_entry &= ~AVIC_PHY_APIC_ID__IS_RUN_MSK;

You even BUG_ON when AVIC_PHY_APIC_ID__IS_RUN_MSK is set, so there is no
reason to clear it.

(Also, don't BUG.)

> +
> +		WRITE_ONCE(*entry, new_entry);

This will translate to two writes in 32 bit mode and we need to write
physical ID first to avoid spurious doorbells ...
is the order guaranteed?

> +	} else {
> +		new_entry = READ_ONCE(*entry);
> +		/**
> +		 * This handles the case when vcpu is scheduled out
> +		 * and has not yet not called blocking. We save the
> +		 * AVIC running flag so that we can restore later.
> +		 */

is_running must be disabled in between ...blocking and ...unblocking,
because we don't want to miss interrupts and block forever.
I somehow don't get it from the comment. :)

> +		if (new_entry & AVIC_PHY_APIC_ID__IS_RUN_MSK) {
> +			svm->avic_was_running = true;
> +			new_entry &= ~AVIC_PHY_APIC_ID__IS_RUN_MSK;
> +			WRITE_ONCE(*entry, new_entry);
> +		} else {
> +			svm->avic_was_running = false;
> +		}

(This can be shorter by setting avic_was_running first.)

> +	}
> +
> +	return ret;

(return 0;)

> +}
> +
> +/**
> + * This function is called during VCPU halt/unhalt.
> + */
> +static int avic_set_running(struct kvm_vcpu *vcpu, bool is_run)
> +{
> +	int ret = 0;
> +	int h_phy_apic_id;
> +	u64 *entry, new_entry;
> +	struct vcpu_svm *svm = to_svm(vcpu);
> +
> +	if (!svm_vcpu_avic_enabled(svm))
> +		return 0;
> +
> +	/* Note: APIC ID = 0xff is used for broadcast.
> +	 *       APIC ID > 0xff is reserved.
> +	 */
> +	h_phy_apic_id = __default_cpu_present_to_apicid(vcpu->cpu);
> +
> +	if (h_phy_apic_id >= AVIC_PHY_APIC_ID_MAX)
> +		return -EINVAL;

The cache should be valid only if this condition is true.
We can get rid of it in both function.

> +
> +	entry = svm->avic_phy_apic_id_cache;
> +	if (!entry)
> +		return -EINVAL;
> +
> +	if (is_run) {

Both READ_ONCE and WRITE_ONCE belong outside of the if.

> +		/* Handle vcpu unblocking after HLT */
> +		new_entry = READ_ONCE(*entry);
> +		new_entry |= AVIC_PHY_APIC_ID__IS_RUN_MSK;
> +		WRITE_ONCE(*entry, new_entry);
> +	} else {
> +		/* Handle vcpu blocking due to HLT */
> +		new_entry = READ_ONCE(*entry);
> +		new_entry &= ~AVIC_PHY_APIC_ID__IS_RUN_MSK;
> +		WRITE_ONCE(*entry, new_entry);
> +	}
> +
> +	return ret;
> +}
> +
>  static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>  {
>  	struct vcpu_svm *svm = to_svm(vcpu);

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

#1368340 — Re: [PART1 RFC v3 12/12] svm: Manage vcpu load/unload when enable AVIC

2016-03-31 15:52+0700, Suravee Suthikulpanit:
> On 03/19/2016 04:44 AM, Radim Krčmář wrote:
>>2016-03-18 01:09-0500, Suravee Suthikulpanit:
>>>+	} else {
>>>+		new_entry = READ_ONCE(*entry);
>>>+		/**
>>>+		 * This handles the case when vcpu is scheduled out
>>>+		 * and has not yet not called blocking. We save the
>>>+		 * AVIC running flag so that we can restore later.
>>>+		 */
>>
>>is_running must be disabled in between ...blocking and ...unblocking,
>>because we don't want to miss interrupts and block forever.
>>I somehow don't get it from the comment. :)
> 
> Not sure if I understand your concern.  However, the is_running bit
> setting/clearing should be handled in the avic_set_running below. This part
> only handles othe case when the is_running bit still set when calling
> vcpu_put (and later on loading some other vcpus). This way, when we are
> re-loading this vcpu, we can restore the is_running bit accordingly.

I think that the comment is misleading.  The saved is_running flag only
matters after svm_vcpu_blocking, yet the comment says that it handles
the irrelevant case before.

Another minor bug is that was_running isn't initialized to 1, so we need
to halt before is_running gets set.
It would be clearer to toggle a flag in svm_vcpu_(un)blocking and set
is_running = !is_blocking.  Doing so will also be immeasurably faster,
because avic_vcpu_load is called far more than svm_vcpu_(un)blocking.

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

#1360364 — [PART1 RFC v3 06/12] KVM: x86: Detect and Initialize AVIC support

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@amd.com>
---
 arch/x86/include/asm/kvm_host.h |   8 ++
 arch/x86/include/asm/svm.h      |   3 +
 arch/x86/kvm/svm.c              | 232 +++++++++++++++++++++++++++++++++++++++-
 3 files changed, 242 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 87eac2a..2698e40 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -754,6 +754,14 @@ struct kvm_arch {
 
 	bool irqchip_split;
 	u8 nr_reserved_ioapic_pins;
+
+	/* Struct members for AVIC */
+	struct hlist_node hnode;
+	struct kvm *kvm;
+	u32 ldr_mode;
+	u32 avic_tag;
+	struct page *avic_log_apic_id_table_page;
+	struct page *avic_phy_apic_id_table_page;
 };
 
 struct kvm_vm_stat {
diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 66e26a0..287e635 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -116,6 +116,9 @@ struct __attribute__ ((__packed__)) vmcb_control_area {
 #define V_INTR_MASKING_SHIFT 24
 #define V_INTR_MASKING_MASK (1 << V_INTR_MASKING_SHIFT)
 
+#define AVIC_ENABLE_SHIFT 31
+#define AVIC_ENABLE_MASK (1 << AVIC_ENABLE_SHIFT)
+
 #define SVM_INTERRUPT_SHADOW_MASK 1
 
 #define SVM_IOIO_STR_SHIFT 2
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index c13a64b..621c948 100644
--- 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_PHY_APIC_ID_MAX	0xFF
+
 static bool erratum_383_found __read_mostly;
 
 static const u32 host_save_user_msrs[] = {
@@ -162,8 +170,20 @@ struct vcpu_svm {
 
 	/* cached guest cpuid flags for faster access */
 	bool nrips_enabled	: 1;
+
+	struct page *avic_backing_page;
+	u64 *avic_phy_apic_id_cache;
+	bool avic_was_running;
 };
 
+#define AVIC_LOG_APIC_ID__GUEST_PHY_APIC_ID_MSK	(0xFF)
+#define AVIC_LOG_APIC_ID__VALID_MSK		(1 << 31)
+
+#define AVIC_PHY_APIC_ID__HOST_PHY_APIC_ID_MSK	(0xFFULL)
+#define AVIC_PHY_APIC_ID__BACKING_PAGE_MSK	(0xFFFFFFFFFFULL << 12)
+#define AVIC_PHY_APIC_ID__IS_RUN_MSK		(1ULL << 62)
+#define AVIC_PHY_APIC_ID__VALID_MSK		(1ULL << 63)
+
 static DEFINE_PER_CPU(u64, current_tsc_ratio);
 #define TSC_RATIO_DEFAULT	0x0100000000ULL
 
@@ -205,6 +225,10 @@ module_param(npt, int, S_IRUGO);
 static int nested = true;
 module_param(nested, int, S_IRUGO);
 
+/* enable / disable AVIC */
+static int avic;
+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 +258,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));
+}
+
+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 +959,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 +1043,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_log_apic_id_table_page);
+	phys_addr_t ppa = page_to_phys(vm_data->avic_phy_apic_id_table_page);
+
+	if (!vmcb)
+		return;
+
+	vmcb->control.avic_backing_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;
+	vmcb->control.int_ctl |= AVIC_ENABLE_MASK;
+}
+
 static void init_vmcb(struct vcpu_svm *svm)
 {
 	struct vmcb_control_area *control = &svm->vmcb->control;
@@ -1113,6 +1174,139 @@ static void init_vmcb(struct vcpu_svm *svm)
 	mark_all_dirty(svm->vmcb);
 
 	enable_gif(svm);
+
+	if (avic)
+		avic_init_vmcb(svm);
+}
+
+static u64 *avic_get_phy_apic_id_entry(struct kvm_vcpu *vcpu, int index)
+{
+	u64 *avic_phy_ait;
+	struct kvm_arch *vm_data = &vcpu->kvm->arch;
+
+	/* 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_apic_id_table_page);
+
+	return &avic_phy_ait[index];
+}
+
+static int avic_init_backing_page(struct kvm_vcpu *vcpu)
+{
+	u64 *entry, new_entry;
+	int id = vcpu->vcpu_id;
+	struct vcpu_svm *svm = to_svm(vcpu);
+
+	if (id >= AVIC_PHY_APIC_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);
+
+	/* Setting AVIC backing page address in the phy APIC ID table */
+	entry = avic_get_phy_apic_id_entry(vcpu, id);
+	if (!entry)
+		return -EINVAL;
+
+	new_entry = READ_ONCE(*entry);
+	new_entry = (page_to_phys(svm->avic_backing_page) &
+		     AVIC_PHY_APIC_ID__BACKING_PAGE_MSK) |
+		     AVIC_PHY_APIC_ID__VALID_MSK;
+	WRITE_ONCE(*entry, new_entry);
+
+	svm->avic_phy_apic_id_cache = entry;
+
+	return 0;
+}
+
+static void avic_vm_uninit(struct kvm *kvm)
+{
+	unsigned long flags;
+	struct kvm_arch *vm_data = &kvm->arch;
+
+	if (vm_data->avic_log_apic_id_table_page)
+		__free_page(vm_data->avic_log_apic_id_table_page);
+	if (vm_data->avic_phy_apic_id_table_page)
+		__free_page(vm_data->avic_phy_apic_id_table_page);
+}
+
+static void avic_vcpu_uninit(struct kvm_vcpu *vcpu)
+{
+	struct vcpu_svm *svm = to_svm(vcpu);
+
+	if (!avic)
+		return;
+
+	if (svm->avic_phy_apic_id_cache)
+		svm->avic_phy_apic_id_cache = NULL;
+}
+
+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 kvm_arch *vm_data = &kvm->arch;
+	struct page *p_page;
+	struct page *l_page;
+
+	/**
+	 * Note:
+	 * AVIC hardware walks the nested page table to check permissions,
+	 * but does not use the SPA address specified in the leaf page
+	 * table entry since it uses  address in the AVIC_BACKING_PAGE pointer
+	 * field of the VMCB. Therefore, we set up the
+	 * APIC_ACCESS_PAGE_PRIVATE_MEMSLOT (4KB) here.
+	 */
+	mutex_lock(&kvm->slots_lock);
+	err = __x86_set_memory_region(kvm,
+				      APIC_ACCESS_PAGE_PRIVATE_MEMSLOT,
+				      APIC_DEFAULT_PHYS_BASE,
+				      PAGE_SIZE);
+	mutex_unlock(&kvm->slots_lock);
+	if (err)
+		goto free_avic;
+	kvm->arch.apic_access_page_done = true;
+
+	/* Allocating physical APIC ID table (4KB) */
+	p_page = alloc_page(GFP_KERNEL);
+	if (!p_page)
+		goto free_avic;
+
+	vm_data->avic_phy_apic_id_table_page = p_page;
+	clear_page(page_address(p_page));
+
+	/* Allocating logical APIC ID table (4KB) */
+	l_page = alloc_page(GFP_KERNEL);
+	if (!l_page)
+		goto free_avic;
+
+	vm_data->avic_log_apic_id_table_page = l_page;
+	clear_page(page_address(l_page));
+
+	vm_data->avic_tag = avic_get_next_tag();
+
+	return 0;
+
+free_avic:
+	avic_vm_uninit(kvm);
+	return err;
 }
 
 static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
@@ -1131,6 +1325,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 (svm_vcpu_avic_enabled(svm) && !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 +1366,14 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id)
 	if (!hsave_page)
 		goto free_page3;
 
+	if (avic) {
+		err = avic_init_backing_page(&svm->vcpu);
+		if (err) {
+			avic_vcpu_uninit(&svm->vcpu);
+			goto free_page4;
+		}
+	}
+
 	svm->nested.hsave = page_address(hsave_page);
 
 	svm->msrpm = page_address(msrpm_pages);
@@ -1187,6 +1392,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 +1416,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);
 }
@@ -3371,6 +3579,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);
@@ -3602,11 +3811,27 @@ 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)
+{
+}
+
+static void svm_hwapic_isr_update(struct kvm *kvm, int isr)
+{
+}
+
+/* Note: Currently only used by Hyper-V. */
 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;
 }
 
 static void svm_load_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap)
@@ -4318,6 +4543,9 @@ static struct kvm_x86_ops svm_x86_ops = {
 	.vcpu_free = svm_free_vcpu,
 	.vcpu_reset = svm_vcpu_reset,
 
+	.vm_init = avic_vm_init,
+	.vm_uninit = avic_vm_uninit,
+
 	.prepare_guest_switch = svm_prepare_guest_switch,
 	.vcpu_load = svm_vcpu_load,
 	.vcpu_put = svm_vcpu_put,
@@ -4375,6 +4603,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]

#1360546 — Re: [PART1 RFC v3 06/12] KVM: x86: Detect and Initialize AVIC support

On 18/03/2016 07:09, Suravee Suthikulpanit wrote:
> 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@amd.com>
> ---
>  arch/x86/include/asm/kvm_host.h |   8 ++
>  arch/x86/include/asm/svm.h      |   3 +
>  arch/x86/kvm/svm.c              | 232 +++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 242 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 87eac2a..2698e40 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -754,6 +754,14 @@ struct kvm_arch {
>  
>  	bool irqchip_split;
>  	u8 nr_reserved_ioapic_pins;
> +
> +	/* Struct members for AVIC */
> +	struct hlist_node hnode;

Unused, please remove.

> +	struct kvm *kvm;

Not needed, please use container_of instead like

	struct kvm_arch *ka;
	...
	struct kvm *kvm = container_of(ka, struct kvm, arch)

> +	u32 ldr_mode;
> +	u32 avic_tag;

Where is avic_tag used?

> +	struct page *avic_log_apic_id_table_page;

Remove apic_ if you want to keep the variable name shorter, but please
spell logical and physical in full.

> +	struct page *avic_phy_apic_id_table_page;
>  };
>  
>  struct kvm_vm_stat {
> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
> index 66e26a0..287e635 100644
> --- a/arch/x86/include/asm/svm.h
> +++ b/arch/x86/include/asm/svm.h
> @@ -116,6 +116,9 @@ struct __attribute__ ((__packed__)) vmcb_control_area {
>  #define V_INTR_MASKING_SHIFT 24
>  #define V_INTR_MASKING_MASK (1 << V_INTR_MASKING_SHIFT)
>  
> +#define AVIC_ENABLE_SHIFT 31
> +#define AVIC_ENABLE_MASK (1 << AVIC_ENABLE_SHIFT)
> +
>  #define SVM_INTERRUPT_SHADOW_MASK 1
>  
>  #define SVM_IOIO_STR_SHIFT 2
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index c13a64b..621c948 100644
> --- 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_PHY_APIC_ID_MAX	0xFF
> +
>  static bool erratum_383_found __read_mostly;
>  
>  static const u32 host_save_user_msrs[] = {
> @@ -162,8 +170,20 @@ struct vcpu_svm {
>  
>  	/* cached guest cpuid flags for faster access */
>  	bool nrips_enabled	: 1;
> +
> +	struct page *avic_backing_page;
> +	u64 *avic_phy_apic_id_cache;

Spelling same as above.

> +	bool avic_was_running;
>  };
>  
> +#define AVIC_LOG_APIC_ID__GUEST_PHY_APIC_ID_MSK	(0xFF)

MSK->MASK
LOG_APIC->LOGICAL
PHY_APIC->PHYSICAL

> +#define AVIC_LOG_APIC_ID__VALID_MSK		(1 << 31)
> +
> +#define AVIC_PHY_APIC_ID__HOST_PHY_APIC_ID_MSK	(0xFFULL)
> +#define AVIC_PHY_APIC_ID__BACKING_PAGE_MSK	(0xFFFFFFFFFFULL << 12)
> +#define AVIC_PHY_APIC_ID__IS_RUN_MSK		(1ULL << 62)

IS_RUN->IS_RUNNING

> +#define AVIC_PHY_APIC_ID__VALID_MSK		(1ULL << 63)
> +
>  static DEFINE_PER_CPU(u64, current_tsc_ratio);
>  #define TSC_RATIO_DEFAULT	0x0100000000ULL
>  
> @@ -205,6 +225,10 @@ module_param(npt, int, S_IRUGO);
>  static int nested = true;
>  module_param(nested, int, S_IRUGO);
>  
> +/* enable / disable AVIC */
> +static int avic;
> +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 +258,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));
> +}
> +
> +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 +959,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 +1043,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_log_apic_id_table_page);
> +	phys_addr_t ppa = page_to_phys(vm_data->avic_phy_apic_id_table_page);
> +
> +	if (!vmcb)
> +		return;
> +
> +	vmcb->control.avic_backing_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;
> +	vmcb->control.int_ctl |= AVIC_ENABLE_MASK;
> +}
> +
>  static void init_vmcb(struct vcpu_svm *svm)
>  {
>  	struct vmcb_control_area *control = &svm->vmcb->control;
> @@ -1113,6 +1174,139 @@ static void init_vmcb(struct vcpu_svm *svm)
>  	mark_all_dirty(svm->vmcb);
>  
>  	enable_gif(svm);
> +
> +	if (avic)
> +		avic_init_vmcb(svm);
> +}
> +
> +static u64 *avic_get_phy_apic_id_entry(struct kvm_vcpu *vcpu, int index)
> +{
> +	u64 *avic_phy_ait;

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)
> +		return NULL;
> +
> +	avic_phy_ait = page_address(vm_data->avic_phy_apic_id_table_page);
> +
> +	return &avic_phy_ait[index];
> +}
> +
> +static int avic_init_backing_page(struct kvm_vcpu *vcpu)
> +{
> +	u64 *entry, new_entry;
> +	int id = vcpu->vcpu_id;
> +	struct vcpu_svm *svm = to_svm(vcpu);
> +
> +	if (id >= AVIC_PHY_APIC_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);
> +
> +	/* Setting AVIC backing page address in the phy APIC ID table */
> +	entry = avic_get_phy_apic_id_entry(vcpu, id);
> +	if (!entry)
> +		return -EINVAL;
> +
> +	new_entry = READ_ONCE(*entry);
> +	new_entry = (page_to_phys(svm->avic_backing_page) &
> +		     AVIC_PHY_APIC_ID__BACKING_PAGE_MSK) |
> +		     AVIC_PHY_APIC_ID__VALID_MSK;
> +	WRITE_ONCE(*entry, new_entry);
> +
> +	svm->avic_phy_apic_id_cache = entry;
> +
> +	return 0;
> +}
> +
> +static void avic_vm_uninit(struct kvm *kvm)
> +{
> +	unsigned long flags;
> +	struct kvm_arch *vm_data = &kvm->arch;
> +
> +	if (vm_data->avic_log_apic_id_table_page)
> +		__free_page(vm_data->avic_log_apic_id_table_page);
> +	if (vm_data->avic_phy_apic_id_table_page)
> +		__free_page(vm_data->avic_phy_apic_id_table_page);
> +}
> +
> +static void avic_vcpu_uninit(struct kvm_vcpu *vcpu)
> +{
> +	struct vcpu_svm *svm = to_svm(vcpu);
> +
> +	if (!avic)
> +		return;
> +
> +	if (svm->avic_phy_apic_id_cache)
> +		svm->avic_phy_apic_id_cache = NULL;
> +}
> +
> +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;

This should be atomic_inc_return, otherwise it's not thread-safe.

> +}
> +
> +static int avic_vm_init(struct kvm *kvm)
> +{
> +	int err = -ENOMEM;
> +	struct kvm_arch *vm_data = &kvm->arch;
> +	struct page *p_page;
> +	struct page *l_page;
> +
> +	/**
> +	 * Note:
> +	 * AVIC hardware walks the nested page table to check permissions,
> +	 * but does not use the SPA address specified in the leaf page
> +	 * table entry since it uses  address in the AVIC_BACKING_PAGE pointer
> +	 * field of the VMCB. Therefore, we set up the
> +	 * APIC_ACCESS_PAGE_PRIVATE_MEMSLOT (4KB) here.
> +	 */
> +	mutex_lock(&kvm->slots_lock);
> +	err = __x86_set_memory_region(kvm,
> +				      APIC_ACCESS_PAGE_PRIVATE_MEMSLOT,
> +				      APIC_DEFAULT_PHYS_BASE,
> +				      PAGE_SIZE);
> +	mutex_unlock(&kvm->slots_lock);

Please use x86_set_memory_region, since you don't have to do anything
else while the lock is taken.

Paolo

> +	if (err)
> +		goto free_avic;
> +	kvm->arch.apic_access_page_done = true;
> +
> +	/* Allocating physical APIC ID table (4KB) */
> +	p_page = alloc_page(GFP_KERNEL);
> +	if (!p_page)
> +		goto free_avic;
> +
> +	vm_data->avic_phy_apic_id_table_page = p_page;
> +	clear_page(page_address(p_page));
> +
> +	/* Allocating logical APIC ID table (4KB) */
> +	l_page = alloc_page(GFP_KERNEL);
> +	if (!l_page)
> +		goto free_avic;
> +
> +	vm_data->avic_log_apic_id_table_page = l_page;
> +	clear_page(page_address(l_page));
> +
> +	vm_data->avic_tag = avic_get_next_tag();
> +
> +	return 0;
> +
> +free_avic:
> +	avic_vm_uninit(kvm);
> +	return err;
>  }
>  
>  static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
> @@ -1131,6 +1325,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 (svm_vcpu_avic_enabled(svm) && !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 +1366,14 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id)
>  	if (!hsave_page)
>  		goto free_page3;
>  
> +	if (avic) {
> +		err = avic_init_backing_page(&svm->vcpu);
> +		if (err) {
> +			avic_vcpu_uninit(&svm->vcpu);
> +			goto free_page4;
> +		}
> +	}
> +
>  	svm->nested.hsave = page_address(hsave_page);
>  
>  	svm->msrpm = page_address(msrpm_pages);
> @@ -1187,6 +1392,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 +1416,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);
>  }
> @@ -3371,6 +3579,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);
> @@ -3602,11 +3811,27 @@ 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)
> +{
> +}
> +
> +static void svm_hwapic_isr_update(struct kvm *kvm, int isr)
> +{
> +}
> +
> +/* Note: Currently only used by Hyper-V. */
>  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;
>  }
>  
>  static void svm_load_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap)
> @@ -4318,6 +4543,9 @@ static struct kvm_x86_ops svm_x86_ops = {
>  	.vcpu_free = svm_free_vcpu,
>  	.vcpu_reset = svm_vcpu_reset,
>  
> +	.vm_init = avic_vm_init,
> +	.vm_uninit = avic_vm_uninit,
> +
>  	.prepare_guest_switch = svm_prepare_guest_switch,
>  	.vcpu_load = svm_vcpu_load,
>  	.vcpu_put = svm_vcpu_put,
> @@ -4375,6 +4603,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,
> 

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

#1360365 — [PART1 RFC v3 10/12] svm: Do not expose x2APIC when enable AVIC

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

Since AVIC only virtualizes xAPIC hardware for the guest, we need to:
    * Intercept APIC BAR msr accesses to disable x2APIC
    * Intercept CPUID access to not advertise x2APIC support
    * Hide x2APIC support when checking via KVM ioctl

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

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 6303147..ba84d57 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -209,6 +209,7 @@ static const struct svm_direct_access_msrs {
 	{ .index = MSR_IA32_LASTBRANCHTOIP,		.always = false },
 	{ .index = MSR_IA32_LASTINTFROMIP,		.always = false },
 	{ .index = MSR_IA32_LASTINTTOIP,		.always = false },
+	{ .index = MSR_IA32_APICBASE,			.always = false },
 	{ .index = MSR_INVALID,				.always = false },
 };
 
@@ -841,7 +842,7 @@ static void set_msr_interception(u32 *msrpm, unsigned msr,
 	msrpm[offset] = tmp;
 }
 
-static void svm_vcpu_init_msrpm(u32 *msrpm)
+static void svm_vcpu_init_msrpm(struct vcpu_svm *svm, u32 *msrpm)
 {
 	int i;
 
@@ -853,6 +854,9 @@ static void svm_vcpu_init_msrpm(u32 *msrpm)
 
 		set_msr_interception(msrpm, direct_access_msrs[i].index, 1, 1);
 	}
+
+	if (svm_vcpu_avic_enabled(svm))
+		set_msr_interception(msrpm, MSR_IA32_APICBASE, 1, 1);
 }
 
 static void add_msr_offset(u32 offset)
@@ -1394,18 +1398,18 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id)
 
 	svm->nested.hsave = page_address(hsave_page);
 
-	svm->msrpm = page_address(msrpm_pages);
-	svm_vcpu_init_msrpm(svm->msrpm);
-
-	svm->nested.msrpm = page_address(nested_msrpm_pages);
-	svm_vcpu_init_msrpm(svm->nested.msrpm);
-
 	svm->vmcb = page_address(page);
 	clear_page(svm->vmcb);
 	svm->vmcb_pa = page_to_pfn(page) << PAGE_SHIFT;
 	svm->asid_generation = 0;
 	init_vmcb(svm);
 
+	svm->msrpm = page_address(msrpm_pages);
+	svm_vcpu_init_msrpm(svm, svm->msrpm);
+
+	svm->nested.msrpm = page_address(nested_msrpm_pages);
+	svm_vcpu_init_msrpm(svm, svm->nested.msrpm);
+
 	svm_init_osvw(&svm->vcpu);
 
 	return &svm->vcpu;
@@ -3308,6 +3312,18 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 			msr_info->data = 0x1E;
 		}
 		break;
+	case MSR_IA32_APICBASE:
+		if (svm_vcpu_avic_enabled(svm)) {
+			/* Note:
+			 * For AVIC, we need to disable X2APIC
+			 * and enable XAPIC
+			 */
+			kvm_get_msr_common(vcpu, msr_info);
+			msr_info->data &= ~X2APIC_ENABLE;
+			msr_info->data |= XAPIC_ENABLE;
+			break;
+		}
+		/* Follow through if not AVIC */
 	default:
 		return kvm_get_msr_common(vcpu, msr_info);
 	}
@@ -3436,6 +3452,10 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
 	case MSR_VM_IGNNE:
 		vcpu_unimpl(vcpu, "unimplemented wrmsr: 0x%x data 0x%llx\n", ecx, data);
 		break;
+	case MSR_IA32_APICBASE:
+		if (svm_vcpu_avic_enabled(svm))
+			avic_update_vapic_bar(to_svm(vcpu), data);
+		/* Follow through */
 	default:
 		return kvm_set_msr_common(vcpu, msr);
 	}
@@ -4554,11 +4574,26 @@ static void svm_cpuid_update(struct kvm_vcpu *vcpu)
 
 	/* Update nrips enabled cache */
 	svm->nrips_enabled = !!guest_cpuid_has_nrips(&svm->vcpu);
+
+	/* Do not support X2APIC when enable AVIC */
+	if (svm_vcpu_avic_enabled(svm)) {
+		int i;
+
+		for (i = 0 ; i < vcpu->arch.cpuid_nent ; i++) {
+			if (vcpu->arch.cpuid_entries[i].function == 1)
+				vcpu->arch.cpuid_entries[i].ecx &= ~(1 << 21);
+		}
+	}
 }
 
 static void svm_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry)
 {
 	switch (func) {
+	case 0x00000001:
+		/* Do not support X2APIC when enable AVIC */
+		if (avic)
+			entry->ecx &= ~(1 << 21);
+		break;
 	case 0x80000001:
 		if (nested)
 			entry->ecx |= (1 << 2); /* Set SVM bit */
-- 
1.9.1

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

#1360930 — Re: [PART1 RFC v3 10/12] svm: Do not expose x2APIC when enable AVIC

2016-03-18 01:09-0500, Suravee Suthikulpanit:
> From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> 
> Since AVIC only virtualizes xAPIC hardware for the guest, we need to:
>     * Intercept APIC BAR msr accesses to disable x2APIC
>     * Intercept CPUID access to not advertise x2APIC support
>     * Hide x2APIC support when checking via KVM ioctl
> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>  arch/x86/kvm/svm.c | 49 ++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 42 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 6303147..ba84d57 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -209,6 +209,7 @@ static const struct svm_direct_access_msrs {
>  	{ .index = MSR_IA32_LASTBRANCHTOIP,		.always = false },
>  	{ .index = MSR_IA32_LASTINTFROMIP,		.always = false },
>  	{ .index = MSR_IA32_LASTINTTOIP,		.always = false },
> +	{ .index = MSR_IA32_APICBASE,			.always = false },
>  	{ .index = MSR_INVALID,				.always = false },
>  };
>  
> @@ -853,6 +854,9 @@ static void svm_vcpu_init_msrpm(u32 *msrpm)
>  
>  		set_msr_interception(msrpm, direct_access_msrs[i].index, 1, 1);
>  	}
> +
> +	if (svm_vcpu_avic_enabled(svm))
> +		set_msr_interception(msrpm, MSR_IA32_APICBASE, 1, 1);

AVIC really won't exit on writes to MSR_IA32_APICBASE otherwise?

> @@ -3308,6 +3312,18 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  			msr_info->data = 0x1E;
>  		}
>  		break;
> +	case MSR_IA32_APICBASE:
> +		if (svm_vcpu_avic_enabled(svm)) {
> +			/* Note:
> +			 * For AVIC, we need to disable X2APIC
> +			 * and enable XAPIC
> +			 */
> +			kvm_get_msr_common(vcpu, msr_info);
> +			msr_info->data &= ~X2APIC_ENABLE;
> +			msr_info->data |= XAPIC_ENABLE;
> +			break;

No.  This won't make the guest switch to xAPIC.
x2APIC can only be enabled if CPUID has that flag and it's impossible to
toggle that CPUID flag it during runtime.

> +		}
> +		/* Follow through if not AVIC */
>  	default:
>  		return kvm_get_msr_common(vcpu, msr_info);
>  	}
> @@ -3436,6 +3452,10 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
>  	case MSR_VM_IGNNE:
>  		vcpu_unimpl(vcpu, "unimplemented wrmsr: 0x%x data 0x%llx\n", ecx, data);
>  		break;
> +	case MSR_IA32_APICBASE:
> +		if (svm_vcpu_avic_enabled(svm))
> +			avic_update_vapic_bar(to_svm(vcpu), data);

There is no connection to x2APIC, please do it in a different patch.

> +		/* Follow through */
>  	default:
>  		return kvm_set_msr_common(vcpu, msr);
>  	}
> @@ -4554,11 +4574,26 @@ static void svm_cpuid_update(struct kvm_vcpu *vcpu)
>  
>  	/* Update nrips enabled cache */
>  	svm->nrips_enabled = !!guest_cpuid_has_nrips(&svm->vcpu);
> +
> +	/* Do not support X2APIC when enable AVIC */
> +	if (svm_vcpu_avic_enabled(svm)) {
> +		int i;
> +
> +		for (i = 0 ; i < vcpu->arch.cpuid_nent ; i++) {
> +			if (vcpu->arch.cpuid_entries[i].function == 1)

Please use kvm_find_cpuid_entry for the search.

> +				vcpu->arch.cpuid_entries[i].ecx &= ~(1 << 21);

and X86_FEATURE_X2APIC (or something with X2APIC in name) for the bit.

The code will become so obvious that the comment can be removed. :)

> +		}
> +	}
>  }
>  
>  static void svm_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry)
>  {
>  	switch (func) {
> +	case 0x00000001:
> +		/* Do not support X2APIC when enable AVIC */
> +		if (avic)
> +			entry->ecx &= ~(1 << 21);

I think this might be the right place for the code you have in
svm_cpuid_update.

Btw. how does x2APIC behave under AVIC?
We definitely shouldn't recommend/expose x2APIC with AVIC as AVIC
doesn't accelerate x2APIC guest-facing interface, but the MSR interface
is going to exit and host-side interrupt delivery will probably still
work, so I don't see a huge problem with it.

> +		break;
>  	case 0x80000001:
>  		if (nested)
>  			entry->ecx |= (1 << 2); /* Set SVM bit */
> -- 
> 1.9.1
> 

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

#1368199 — Re: [PART1 RFC v3 10/12] svm: Do not expose x2APIC when enable AVIC

On 31/03/2016 06:15, Suravee Suthikulpanit wrote:
>>> +                vcpu->arch.cpuid_entries[i].ecx &= ~(1 << 21);
>> 
>> and X86_FEATURE_X2APIC (or something with X2APIC in name) for the
>> bit.
>> 
>> The code will become so obvious that the comment can be removed.
>> :)
> 
> Good point. I can only find example of using (X86_FEATURE_X2APIC %
> 32) == 21.

You can use bit(X86_FEATURE_X2APIC), it is defined in arch/x86/kvm/x86.h.

>> but the MSR interface is going to exit and host-side interrupt 
>> delivery will probably still work, so I don't see a huge problem
>> with it.
> 
> Agree that it will still work. However, in such case, the guest code
> would likely default to using x2APIC interface, which will not be
> handled by the AVIC hardware, and resulting in no performance
> improvement that we are trying to introduce.

You would still get some improvement from exit-free interrupt delivery.

Paolo

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web