[PATCH 0/4] Export APICv-related state via binary stats interface

[PATCH 0/4] Export APICv-related state via binary stats interface

[Alejandro Jimenez]

After discussion in the RFC thread[0], the following items were identified as
desirable to expose via the stats interface:

- APICv status: (apicv_enabled, boolean, per-vCPU)

- Guest using SynIC's AutoEOI: (synic_auto_eoi_used, boolean, per-VM)

- KVM PIT in reinject mode inhibits AVIC: (pit_reinject_mode, boolean, per-VM)

- APICv unaccelerated injections causing a vmexit (i.e. AVIC_INCOMPLETE_IPI,
  AVIC_UNACCELERATED_ACCESS, APIC_WRITE): (apicv_unaccelerated_inj, counter,
  per-vCPU)

Example retrieving the newly introduced stats for guest running on AMD Genoa
host, with AVIC enabled:

(QEMU) query-stats target=vcpu vcpus=['/machine/unattached/device[0]'] providers=[{'provider':'kvm','names':['apicv_unaccelerated_inj','apicv_active']}]
{
    "return": [
        {
            "provider": "kvm",
            "qom-path": "/machine/unattached/device[0]",
            "stats": [
                {
                    "name": "apicv_unaccelerated_inj",
                    "value": 2561
                },
                {
                    "name": "apicv_active",
                    "value": true
                }
            ]
        }
    ]
}
(QEMU) query-stats target=vm providers=[{'provider':'kvm','names':['pit_reinject_mode','synic_auto_eoi_used']}]
{
    "return": [
        {
            "provider": "kvm",
            "stats": [
                {
                    "name": "pit_reinject_mode",
                    "value": false
                },
                {
                    "name": "synic_auto_eoi_used",
                    "value": false
                }
            ]
        }
    ]
}

Changes were also sanity tested on Intel Sapphire Rapids platform, with/without
IPI virtualization.

Regards,
Alejandro

[0] https://lore.kernel.org/all/20240215160136.1256084-1-alejandro.j.jimenez@oracle.com/

Alejandro Jimenez (4):
  KVM: x86: Expose per-vCPU APICv status
  KVM: x86: Add a VM stat exposing when SynIC AutoEOI is in use
  KVM: x86: Add a VM stat exposing when KVM PIT is set to reinject mode
  KVM: x86: Add vCPU stat for APICv interrupt injections causing #VMEXIT

 arch/x86/include/asm/kvm_host.h | 4 ++++
 arch/x86/kvm/hyperv.c           | 2 ++
 arch/x86/kvm/i8254.c            | 2 ++
 arch/x86/kvm/lapic.c            | 1 +
 arch/x86/kvm/svm/avic.c         | 7 +++++++
 arch/x86/kvm/vmx/vmx.c          | 2 ++
 arch/x86/kvm/x86.c              | 7 ++++++-
 7 files changed, 24 insertions(+), 1 deletion(-)


base-commit: 7b076c6a308ec5bce9fc96e2935443ed228b9148
-- 
2.39.3

[PATCH 1/4] KVM: x86: Expose per-vCPU APICv status

[Alejandro Jimenez]

Expose the APICv activation status of individual vCPUs via the stats
subsystem. In special cases a vCPU's APICv can be deactivated/disabled
even though there are no VM-wide inhibition reasons. The only current
example of this is AVIC for a vCPU running in nested mode. This type of
inhibition is not recorded in the VM inhibit reasons or visible in
current tracepoints.

Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
---
 arch/x86/include/asm/kvm_host.h | 1 +
 arch/x86/kvm/lapic.c            | 1 +
 arch/x86/kvm/x86.c              | 2 ++
 3 files changed, 4 insertions(+)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 1d13e3cd1dc5..12f30cb5c842 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1573,6 +1573,7 @@ struct kvm_vcpu_stat {
 	u64 preemption_other;
 	u64 guest_mode;
 	u64 notify_window_exits;
+	u64 apicv_active;
 };
 
 struct x86_instruction_info;
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index cf37586f0466..37fe75a5db8c 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -2872,6 +2872,7 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu, int timer_advance_ns)
 	 */
 	if (enable_apicv) {
 		apic->apicv_active = true;
+		vcpu->stat.apicv_active = apic->apicv_active;
 		kvm_make_request(KVM_REQ_APICV_UPDATE, vcpu);
 	}
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index e9ef1fa4b90b..0451c4c8d731 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -304,6 +304,7 @@ const struct _kvm_stats_desc kvm_vcpu_stats_desc[] = {
 	STATS_DESC_COUNTER(VCPU, preemption_other),
 	STATS_DESC_IBOOLEAN(VCPU, guest_mode),
 	STATS_DESC_COUNTER(VCPU, notify_window_exits),
+	STATS_DESC_IBOOLEAN(VCPU, apicv_active),
 };
 
 const struct kvm_stats_header kvm_vcpu_stats_header = {
@@ -10625,6 +10626,7 @@ void __kvm_vcpu_update_apicv(struct kvm_vcpu *vcpu)
 		goto out;
 
 	apic->apicv_active = activate;
+	vcpu->stat.apicv_active = apic->apicv_active;
 	kvm_apic_update_apicv(vcpu);
 	static_call(kvm_x86_refresh_apicv_exec_ctrl)(vcpu);
 
-- 
2.39.3

[PATCH 2/4] KVM: x86: Add a VM stat exposing when SynIC AutoEOI is in use

[Alejandro Jimenez]

APICv/AVIC is inhibited for guests using Hyper-V's synthetic interrupt
controller (SynIC) enlightenment, and specifically SynIC's AutoEOI feature.
It is recommended that guests do not enable AutoEOI (see flag
HV_DEPRECATING_AEOI_RECOMMENDED and commit 0f250a646382 ("KVM: x86:
hyper-v: Deactivate APICv only when AutoEOI feature is in use")), but it
can still be used in legacy configurations.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
---
 arch/x86/include/asm/kvm_host.h | 1 +
 arch/x86/kvm/hyperv.c           | 2 ++
 arch/x86/kvm/x86.c              | 3 ++-
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 12f30cb5c842..f3b40cfebec4 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1534,6 +1534,7 @@ struct kvm_vm_stat {
 	u64 nx_lpage_splits;
 	u64 max_mmu_page_hash_collisions;
 	u64 max_mmu_rmap_size;
+	u64 synic_auto_eoi_used;
 };
 
 struct kvm_vcpu_stat {
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 8a47f8541eab..4263b4ad71c5 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -149,6 +149,8 @@ static void synic_update_vector(struct kvm_vcpu_hv_synic *synic,
 					 APICV_INHIBIT_REASON_HYPERV,
 					 !!hv->synic_auto_eoi_used);
 
+	vcpu->kvm->stat.synic_auto_eoi_used = !!hv->synic_auto_eoi_used;
+
 	up_write(&vcpu->kvm->arch.apicv_update_lock);
 }
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 0451c4c8d731..27e339133068 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -256,7 +256,8 @@ const struct _kvm_stats_desc kvm_vm_stats_desc[] = {
 	STATS_DESC_ICOUNTER(VM, pages_1g),
 	STATS_DESC_ICOUNTER(VM, nx_lpage_splits),
 	STATS_DESC_PCOUNTER(VM, max_mmu_rmap_size),
-	STATS_DESC_PCOUNTER(VM, max_mmu_page_hash_collisions)
+	STATS_DESC_PCOUNTER(VM, max_mmu_page_hash_collisions),
+	STATS_DESC_IBOOLEAN(VM, synic_auto_eoi_used)
 };
 
 const struct kvm_stats_header kvm_vm_stats_header = {
-- 
2.39.3

[PATCH 3/4] KVM: x86: Add a VM stat exposing when KVM PIT is set to reinject mode

[Alejandro Jimenez]

Add a stat to query when PIT is in reinject mode, which can have a large
performance impact due to disabling SVM AVIC.
When using in-kernel irqchip, QEMU and KVM default to creating a PIT in
reinject mode, since this is necessary for old guest operating systems that
use the PIT for timing. Unfortunately, reinject mode relies on EOI
interception and so SVM AVIC must be inhibited when the PIT is set up using
this mode.

Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
---
 arch/x86/include/asm/kvm_host.h | 1 +
 arch/x86/kvm/i8254.c            | 2 ++
 arch/x86/kvm/x86.c              | 3 ++-
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index f3b40cfebec4..e7e3213cefae 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1535,6 +1535,7 @@ struct kvm_vm_stat {
 	u64 max_mmu_page_hash_collisions;
 	u64 max_mmu_rmap_size;
 	u64 synic_auto_eoi_used;
+	u64 pit_reinject_mode;
 };
 
 struct kvm_vcpu_stat {
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index cd57a517d04a..44e593e909a1 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -316,6 +316,8 @@ void kvm_pit_set_reinject(struct kvm_pit *pit, bool reinject)
 		kvm_unregister_irq_mask_notifier(kvm, 0, &pit->mask_notifier);
 	}
 
+	kvm->stat.pit_reinject_mode = reinject;
+
 	atomic_set(&ps->reinject, reinject);
 }
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 27e339133068..03cb933920cb 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -257,7 +257,8 @@ const struct _kvm_stats_desc kvm_vm_stats_desc[] = {
 	STATS_DESC_ICOUNTER(VM, nx_lpage_splits),
 	STATS_DESC_PCOUNTER(VM, max_mmu_rmap_size),
 	STATS_DESC_PCOUNTER(VM, max_mmu_page_hash_collisions),
-	STATS_DESC_IBOOLEAN(VM, synic_auto_eoi_used)
+	STATS_DESC_IBOOLEAN(VM, synic_auto_eoi_used),
+	STATS_DESC_IBOOLEAN(VM, pit_reinject_mode)
 };
 
 const struct kvm_stats_header kvm_vm_stats_header = {
-- 
2.39.3

[PATCH 4/4] KVM: x86: Add vCPU stat for APICv interrupt injections causing #VMEXIT

[Alejandro Jimenez]

Even when APICv/AVIC is active, certain guest accesses to its local APIC(s)
cannot be fully accelerated, and cause a #VMEXIT to allow the VMM to
emulate the behavior and side effects. Expose a counter stat for these
specific #VMEXIT types.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
---
 arch/x86/include/asm/kvm_host.h | 1 +
 arch/x86/kvm/svm/avic.c         | 7 +++++++
 arch/x86/kvm/vmx/vmx.c          | 2 ++
 arch/x86/kvm/x86.c              | 1 +
 4 files changed, 11 insertions(+)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index e7e3213cefae..388979dfe9f3 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1576,6 +1576,7 @@ struct kvm_vcpu_stat {
 	u64 guest_mode;
 	u64 notify_window_exits;
 	u64 apicv_active;
+	u64 apicv_unaccelerated_inj;
 };
 
 struct x86_instruction_info;
diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
index 4b74ea91f4e6..274041d3cf66 100644
--- a/arch/x86/kvm/svm/avic.c
+++ b/arch/x86/kvm/svm/avic.c
@@ -517,6 +517,8 @@ int avic_incomplete_ipi_interception(struct kvm_vcpu *vcpu)
 			kvm_apic_write_nodecode(vcpu, APIC_ICR);
 		else
 			kvm_apic_send_ipi(apic, icrl, icrh);
+
+		++vcpu->stat.apicv_unaccelerated_inj;
 		break;
 	case AVIC_IPI_FAILURE_TARGET_NOT_RUNNING:
 		/*
@@ -525,6 +527,8 @@ int avic_incomplete_ipi_interception(struct kvm_vcpu *vcpu)
 		 * vcpus. So, we just need to kick the appropriate vcpu.
 		 */
 		avic_kick_target_vcpus(vcpu->kvm, apic, icrl, icrh, index);
+
+		++vcpu->stat.apicv_unaccelerated_inj;
 		break;
 	case AVIC_IPI_FAILURE_INVALID_BACKING_PAGE:
 		WARN_ONCE(1, "Invalid backing page\n");
@@ -704,6 +708,9 @@ int avic_unaccelerated_access_interception(struct kvm_vcpu *vcpu)
 
 	trace_kvm_avic_unaccelerated_access(vcpu->vcpu_id, offset,
 					    trap, write, vector);
+
+	++vcpu->stat.apicv_unaccelerated_inj;
+
 	if (trap) {
 		/* Handling Trap */
 		WARN_ONCE(!write, "svm: Handling trap read.\n");
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index f10b5f8f364b..a7487f12ded1 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -5657,6 +5657,8 @@ static int handle_apic_write(struct kvm_vcpu *vcpu)
 {
 	unsigned long exit_qualification = vmx_get_exit_qual(vcpu);
 
+	++vcpu->stat.apicv_unaccelerated_inj;
+
 	/*
 	 * APIC-write VM-Exit is trap-like, KVM doesn't need to advance RIP and
 	 * hardware has done any necessary aliasing, offset adjustments, etc...
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 03cb933920cb..c8730b0fac87 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -307,6 +307,7 @@ const struct _kvm_stats_desc kvm_vcpu_stats_desc[] = {
 	STATS_DESC_IBOOLEAN(VCPU, guest_mode),
 	STATS_DESC_COUNTER(VCPU, notify_window_exits),
 	STATS_DESC_IBOOLEAN(VCPU, apicv_active),
+	STATS_DESC_COUNTER(VCPU, apicv_unaccelerated_inj),
 };
 
 const struct kvm_stats_header kvm_vcpu_stats_header = {
-- 
2.39.3

Re: [PATCH 0/4] Export APICv-related state via binary stats interface

[Alejandro Jimenez]

On 4/29/24 11:57, Alejandro Jimenez wrote:
> After discussion in the RFC thread[0], the following items were identified as
> desirable to expose via the stats interface:
> 
> - APICv status: (apicv_enabled, boolean, per-vCPU)
> 
> - Guest using SynIC's AutoEOI: (synic_auto_eoi_used, boolean, per-VM)
> 
> - KVM PIT in reinject mode inhibits AVIC: (pit_reinject_mode, boolean, per-VM)
> 
> - APICv unaccelerated injections causing a vmexit (i.e. AVIC_INCOMPLETE_IPI,
>    AVIC_UNACCELERATED_ACCESS, APIC_WRITE): (apicv_unaccelerated_inj, counter,
>    per-vCPU)
> 
> Example retrieving the newly introduced stats for guest running on AMD Genoa
> host, with AVIC enabled:
> 
> (QEMU) query-stats target=vcpu vcpus=['/machine/unattached/device[0]'] providers=[{'provider':'kvm','names':['apicv_unaccelerated_inj','apicv_active']}]
> {
>      "return": [
>          {
>              "provider": "kvm",
>              "qom-path": "/machine/unattached/device[0]",
>              "stats": [
>                  {
>                      "name": "apicv_unaccelerated_inj",
>                      "value": 2561
>                  },
>                  {
>                      "name": "apicv_active",
>                      "value": true
>                  }
>              ]
>          }
>      ]
> }
> (QEMU) query-stats target=vm providers=[{'provider':'kvm','names':['pit_reinject_mode','synic_auto_eoi_used']}]
> {
>      "return": [
>          {
>              "provider": "kvm",
>              "stats": [
>                  {
>                      "name": "pit_reinject_mode",
>                      "value": false
>                  },
>                  {
>                      "name": "synic_auto_eoi_used",
>                      "value": false
>                  }
>              ]
>          }
>      ]
> }
> 
> Changes were also sanity tested on Intel Sapphire Rapids platform, with/without
> IPI virtualization.

ping...

This series implements the suggestions from earlier RFC discussion [0]. Additional comments/reviews are appreciated.

Thank you,
Alejandro

> 
> [0] https://lore.kernel.org/all/20240215160136.1256084-1-alejandro.j.jimenez@oracle.com/
> 
> Alejandro Jimenez (4):
>    KVM: x86: Expose per-vCPU APICv status
>    KVM: x86: Add a VM stat exposing when SynIC AutoEOI is in use
>    KVM: x86: Add a VM stat exposing when KVM PIT is set to reinject mode
>    KVM: x86: Add vCPU stat for APICv interrupt injections causing #VMEXIT
> 
>   arch/x86/include/asm/kvm_host.h | 4 ++++
>   arch/x86/kvm/hyperv.c           | 2 ++
>   arch/x86/kvm/i8254.c            | 2 ++
>   arch/x86/kvm/lapic.c            | 1 +
>   arch/x86/kvm/svm/avic.c         | 7 +++++++
>   arch/x86/kvm/vmx/vmx.c          | 2 ++
>   arch/x86/kvm/x86.c              | 7 ++++++-
>   7 files changed, 24 insertions(+), 1 deletion(-)
> 
> 
> base-commit: 7b076c6a308ec5bce9fc96e2935443ed228b9148

Re: [PATCH 0/4] Export APICv-related state via binary stats interface

[Vasant Hegde]

Hi,



On 4/29/2024 9:27 PM, Alejandro Jimenez wrote:
> After discussion in the RFC thread[0], the following items were identified as
> desirable to expose via the stats interface:
> 
> - APICv status: (apicv_enabled, boolean, per-vCPU)
> 
> - Guest using SynIC's AutoEOI: (synic_auto_eoi_used, boolean, per-VM)
> 
> - KVM PIT in reinject mode inhibits AVIC: (pit_reinject_mode, boolean, per-VM)
> 
> - APICv unaccelerated injections causing a vmexit (i.e. AVIC_INCOMPLETE_IPI,
>   AVIC_UNACCELERATED_ACCESS, APIC_WRITE): (apicv_unaccelerated_inj, counter,
>   per-vCPU)
> 
> Example retrieving the newly introduced stats for guest running on AMD Genoa
> host, with AVIC enabled:


I have reviewed generic and AMD driver related code. Also tested this series on
AMD systems and its working fine.

Tested-by: Vasant Hegde <vasant.hegde@amd.com>

-Vasant


> 
> (QEMU) query-stats target=vcpu vcpus=['/machine/unattached/device[0]'] providers=[{'provider':'kvm','names':['apicv_unaccelerated_inj','apicv_active']}]
> {
>     "return": [
>         {
>             "provider": "kvm",
>             "qom-path": "/machine/unattached/device[0]",
>             "stats": [
>                 {
>                     "name": "apicv_unaccelerated_inj",
>                     "value": 2561
>                 },
>                 {
>                     "name": "apicv_active",
>                     "value": true
>                 }
>             ]
>         }
>     ]
> }
> (QEMU) query-stats target=vm providers=[{'provider':'kvm','names':['pit_reinject_mode','synic_auto_eoi_used']}]
> {
>     "return": [
>         {
>             "provider": "kvm",
>             "stats": [
>                 {
>                     "name": "pit_reinject_mode",
>                     "value": false
>                 },
>                 {
>                     "name": "synic_auto_eoi_used",
>                     "value": false
>                 }
>             ]
>         }
>     ]
> }
> 
> Changes were also sanity tested on Intel Sapphire Rapids platform, with/without
> IPI virtualization.
> 
> Regards,
> Alejandro
> 
> [0] https://lore.kernel.org/all/20240215160136.1256084-1-alejandro.j.jimenez@oracle.com/
> 
> Alejandro Jimenez (4):
>   KVM: x86: Expose per-vCPU APICv status
>   KVM: x86: Add a VM stat exposing when SynIC AutoEOI is in use
>   KVM: x86: Add a VM stat exposing when KVM PIT is set to reinject mode
>   KVM: x86: Add vCPU stat for APICv interrupt injections causing #VMEXIT
> 
>  arch/x86/include/asm/kvm_host.h | 4 ++++
>  arch/x86/kvm/hyperv.c           | 2 ++
>  arch/x86/kvm/i8254.c            | 2 ++
>  arch/x86/kvm/lapic.c            | 1 +
>  arch/x86/kvm/svm/avic.c         | 7 +++++++
>  arch/x86/kvm/vmx/vmx.c          | 2 ++
>  arch/x86/kvm/x86.c              | 7 ++++++-
>  7 files changed, 24 insertions(+), 1 deletion(-)
> 
> 
> base-commit: 7b076c6a308ec5bce9fc96e2935443ed228b9148

Re: [PATCH 4/4] KVM: x86: Add vCPU stat for APICv interrupt injections causing #VMEXIT

[Vasant Hegde]

Hi Alejandro,


On 4/29/2024 9:27 PM, Alejandro Jimenez wrote:
> Even when APICv/AVIC is active, certain guest accesses to its local APIC(s)
> cannot be fully accelerated, and cause a #VMEXIT to allow the VMM to
> emulate the behavior and side effects. Expose a counter stat for these
> specific #VMEXIT types.
> 
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>

Reviewed-by: Vasant Hegde <vasant.hegde@amd.com> # AMD

-Vasant

> ---
>  arch/x86/include/asm/kvm_host.h | 1 +
>  arch/x86/kvm/svm/avic.c         | 7 +++++++
>  arch/x86/kvm/vmx/vmx.c          | 2 ++
>  arch/x86/kvm/x86.c              | 1 +
>  4 files changed, 11 insertions(+)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index e7e3213cefae..388979dfe9f3 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1576,6 +1576,7 @@ struct kvm_vcpu_stat {
>  	u64 guest_mode;
>  	u64 notify_window_exits;
>  	u64 apicv_active;
> +	u64 apicv_unaccelerated_inj;
>  };
>  
>  struct x86_instruction_info;
> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> index 4b74ea91f4e6..274041d3cf66 100644
> --- a/arch/x86/kvm/svm/avic.c
> +++ b/arch/x86/kvm/svm/avic.c
> @@ -517,6 +517,8 @@ int avic_incomplete_ipi_interception(struct kvm_vcpu *vcpu)
>  			kvm_apic_write_nodecode(vcpu, APIC_ICR);
>  		else
>  			kvm_apic_send_ipi(apic, icrl, icrh);
> +
> +		++vcpu->stat.apicv_unaccelerated_inj;
>  		break;
>  	case AVIC_IPI_FAILURE_TARGET_NOT_RUNNING:
>  		/*
> @@ -525,6 +527,8 @@ int avic_incomplete_ipi_interception(struct kvm_vcpu *vcpu)
>  		 * vcpus. So, we just need to kick the appropriate vcpu.
>  		 */
>  		avic_kick_target_vcpus(vcpu->kvm, apic, icrl, icrh, index);
> +
> +		++vcpu->stat.apicv_unaccelerated_inj;
>  		break;
>  	case AVIC_IPI_FAILURE_INVALID_BACKING_PAGE:
>  		WARN_ONCE(1, "Invalid backing page\n");
> @@ -704,6 +708,9 @@ int avic_unaccelerated_access_interception(struct kvm_vcpu *vcpu)
>  
>  	trace_kvm_avic_unaccelerated_access(vcpu->vcpu_id, offset,
>  					    trap, write, vector);
> +
> +	++vcpu->stat.apicv_unaccelerated_inj;
> +
>  	if (trap) {
>  		/* Handling Trap */
>  		WARN_ONCE(!write, "svm: Handling trap read.\n");
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index f10b5f8f364b..a7487f12ded1 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -5657,6 +5657,8 @@ static int handle_apic_write(struct kvm_vcpu *vcpu)
>  {
>  	unsigned long exit_qualification = vmx_get_exit_qual(vcpu);
>  
> +	++vcpu->stat.apicv_unaccelerated_inj;
> +
>  	/*
>  	 * APIC-write VM-Exit is trap-like, KVM doesn't need to advance RIP and
>  	 * hardware has done any necessary aliasing, offset adjustments, etc...
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 03cb933920cb..c8730b0fac87 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -307,6 +307,7 @@ const struct _kvm_stats_desc kvm_vcpu_stats_desc[] = {
>  	STATS_DESC_IBOOLEAN(VCPU, guest_mode),
>  	STATS_DESC_COUNTER(VCPU, notify_window_exits),
>  	STATS_DESC_IBOOLEAN(VCPU, apicv_active),
> +	STATS_DESC_COUNTER(VCPU, apicv_unaccelerated_inj),
>  };
>  
>  const struct kvm_stats_header kvm_vcpu_stats_header = {

Re: [PATCH 3/4] KVM: x86: Add a VM stat exposing when KVM PIT is set to reinject mode

[Vasant Hegde]

On 4/29/2024 9:27 PM, Alejandro Jimenez wrote:
> Add a stat to query when PIT is in reinject mode, which can have a large
> performance impact due to disabling SVM AVIC.
> When using in-kernel irqchip, QEMU and KVM default to creating a PIT in
> reinject mode, since this is necessary for old guest operating systems that
> use the PIT for timing. Unfortunately, reinject mode relies on EOI
> interception and so SVM AVIC must be inhibited when the PIT is set up using
> this mode.
> 
> Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>

Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>

-Vasant

> ---
>  arch/x86/include/asm/kvm_host.h | 1 +
>  arch/x86/kvm/i8254.c            | 2 ++
>  arch/x86/kvm/x86.c              | 3 ++-
>  3 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index f3b40cfebec4..e7e3213cefae 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1535,6 +1535,7 @@ struct kvm_vm_stat {
>  	u64 max_mmu_page_hash_collisions;
>  	u64 max_mmu_rmap_size;
>  	u64 synic_auto_eoi_used;
> +	u64 pit_reinject_mode;
>  };
>  
>  struct kvm_vcpu_stat {
> diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
> index cd57a517d04a..44e593e909a1 100644
> --- a/arch/x86/kvm/i8254.c
> +++ b/arch/x86/kvm/i8254.c
> @@ -316,6 +316,8 @@ void kvm_pit_set_reinject(struct kvm_pit *pit, bool reinject)
>  		kvm_unregister_irq_mask_notifier(kvm, 0, &pit->mask_notifier);
>  	}
>  
> +	kvm->stat.pit_reinject_mode = reinject;
> +
>  	atomic_set(&ps->reinject, reinject);
>  }
>  
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 27e339133068..03cb933920cb 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -257,7 +257,8 @@ const struct _kvm_stats_desc kvm_vm_stats_desc[] = {
>  	STATS_DESC_ICOUNTER(VM, nx_lpage_splits),
>  	STATS_DESC_PCOUNTER(VM, max_mmu_rmap_size),
>  	STATS_DESC_PCOUNTER(VM, max_mmu_page_hash_collisions),
> -	STATS_DESC_IBOOLEAN(VM, synic_auto_eoi_used)
> +	STATS_DESC_IBOOLEAN(VM, synic_auto_eoi_used),
> +	STATS_DESC_IBOOLEAN(VM, pit_reinject_mode)
>  };
>  
>  const struct kvm_stats_header kvm_vm_stats_header = {

Re: [PATCH 1/4] KVM: x86: Expose per-vCPU APICv status

[Vasant Hegde]

On 4/29/2024 9:27 PM, Alejandro Jimenez wrote:
> Expose the APICv activation status of individual vCPUs via the stats
> subsystem. In special cases a vCPU's APICv can be deactivated/disabled
> even though there are no VM-wide inhibition reasons. The only current
> example of this is AVIC for a vCPU running in nested mode. This type of
> inhibition is not recorded in the VM inhibit reasons or visible in
> current tracepoints.
> 
> Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>


Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>

-Vasant

> ---
>  arch/x86/include/asm/kvm_host.h | 1 +
>  arch/x86/kvm/lapic.c            | 1 +
>  arch/x86/kvm/x86.c              | 2 ++
>  3 files changed, 4 insertions(+)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 1d13e3cd1dc5..12f30cb5c842 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1573,6 +1573,7 @@ struct kvm_vcpu_stat {
>  	u64 preemption_other;
>  	u64 guest_mode;
>  	u64 notify_window_exits;
> +	u64 apicv_active;
>  };
>  
>  struct x86_instruction_info;
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index cf37586f0466..37fe75a5db8c 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -2872,6 +2872,7 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu, int timer_advance_ns)
>  	 */
>  	if (enable_apicv) {
>  		apic->apicv_active = true;
> +		vcpu->stat.apicv_active = apic->apicv_active;
>  		kvm_make_request(KVM_REQ_APICV_UPDATE, vcpu);
>  	}
>  
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index e9ef1fa4b90b..0451c4c8d731 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -304,6 +304,7 @@ const struct _kvm_stats_desc kvm_vcpu_stats_desc[] = {
>  	STATS_DESC_COUNTER(VCPU, preemption_other),
>  	STATS_DESC_IBOOLEAN(VCPU, guest_mode),
>  	STATS_DESC_COUNTER(VCPU, notify_window_exits),
> +	STATS_DESC_IBOOLEAN(VCPU, apicv_active),
>  };
>  
>  const struct kvm_stats_header kvm_vcpu_stats_header = {
> @@ -10625,6 +10626,7 @@ void __kvm_vcpu_update_apicv(struct kvm_vcpu *vcpu)
>  		goto out;
>  
>  	apic->apicv_active = activate;
> +	vcpu->stat.apicv_active = apic->apicv_active;
>  	kvm_apic_update_apicv(vcpu);
>  	static_call(kvm_x86_refresh_apicv_exec_ctrl)(vcpu);
>  

Re: [PATCH 4/4] KVM: x86: Add vCPU stat for APICv interrupt injections causing #VMEXIT

[Sean Christopherson]

On Mon, Apr 29, 2024, Alejandro Jimenez wrote:
> Even when APICv/AVIC is active, certain guest accesses to its local APIC(s)
> cannot be fully accelerated, and cause a #VMEXIT to allow the VMM to
> emulate the behavior and side effects. Expose a counter stat for these
> specific #VMEXIT types.
> 
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
> ---
>  arch/x86/include/asm/kvm_host.h | 1 +
>  arch/x86/kvm/svm/avic.c         | 7 +++++++
>  arch/x86/kvm/vmx/vmx.c          | 2 ++
>  arch/x86/kvm/x86.c              | 1 +
>  4 files changed, 11 insertions(+)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index e7e3213cefae..388979dfe9f3 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1576,6 +1576,7 @@ struct kvm_vcpu_stat {
>  	u64 guest_mode;
>  	u64 notify_window_exits;
>  	u64 apicv_active;
> +	u64 apicv_unaccelerated_inj;

The stat name doesn't match the changelog or the code.  The AVIC updates in
avic_incomplete_ipi_interception() are unaccelerated _injection_, they're
unaccelarated _delivery_.  And in those cases, the fact that delivery wasn't
accelerated is relatively uninteresting in most cases.

And avic_unaccelerated_access_interception() and handle_apic_write() don't
necessarily have anything to do with injection.

On the flip side, the slow paths for {svm,vmx}_deliver_interrupt() are very
explicitly unnaccelerated injection.

It's not entirely clear from the changelog what the end goal of this stat is.
A singular stat for all APICv/AVIC access VM-Exits seems uninteresting, as such
a stat essentially just captures that the guest is active.  Maaaybe someone could
glean info from comparing two VMs, but even that is dubious.  E.g. if a guest is
doing something function and generating a lot of avic_incomplete_ipi_interception()
exits, those will likely be in the noise due to the total volume of other AVIC
exits.

>  };
>  
>  struct x86_instruction_info;
> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> index 4b74ea91f4e6..274041d3cf66 100644
> --- a/arch/x86/kvm/svm/avic.c
> +++ b/arch/x86/kvm/svm/avic.c
> @@ -517,6 +517,8 @@ int avic_incomplete_ipi_interception(struct kvm_vcpu *vcpu)
>  			kvm_apic_write_nodecode(vcpu, APIC_ICR);
>  		else
>  			kvm_apic_send_ipi(apic, icrl, icrh);
> +
> +		++vcpu->stat.apicv_unaccelerated_inj;
>  		break;
>  	case AVIC_IPI_FAILURE_TARGET_NOT_RUNNING:
>  		/*
> @@ -525,6 +527,8 @@ int avic_incomplete_ipi_interception(struct kvm_vcpu *vcpu)
>  		 * vcpus. So, we just need to kick the appropriate vcpu.
>  		 */
>  		avic_kick_target_vcpus(vcpu->kvm, apic, icrl, icrh, index);
> +
> +		++vcpu->stat.apicv_unaccelerated_inj;
>  		break;
>  	case AVIC_IPI_FAILURE_INVALID_BACKING_PAGE:
>  		WARN_ONCE(1, "Invalid backing page\n");
> @@ -704,6 +708,9 @@ int avic_unaccelerated_access_interception(struct kvm_vcpu *vcpu)
>  
>  	trace_kvm_avic_unaccelerated_access(vcpu->vcpu_id, offset,
>  					    trap, write, vector);
> +
> +	++vcpu->stat.apicv_unaccelerated_inj;
> +
>  	if (trap) {
>  		/* Handling Trap */
>  		WARN_ONCE(!write, "svm: Handling trap read.\n");
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index f10b5f8f364b..a7487f12ded1 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -5657,6 +5657,8 @@ static int handle_apic_write(struct kvm_vcpu *vcpu)
>  {
>  	unsigned long exit_qualification = vmx_get_exit_qual(vcpu);
>  
> +	++vcpu->stat.apicv_unaccelerated_inj;
> +
>  	/*
>  	 * APIC-write VM-Exit is trap-like, KVM doesn't need to advance RIP and
>  	 * hardware has done any necessary aliasing, offset adjustments, etc...
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 03cb933920cb..c8730b0fac87 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -307,6 +307,7 @@ const struct _kvm_stats_desc kvm_vcpu_stats_desc[] = {
>  	STATS_DESC_IBOOLEAN(VCPU, guest_mode),
>  	STATS_DESC_COUNTER(VCPU, notify_window_exits),
>  	STATS_DESC_IBOOLEAN(VCPU, apicv_active),
> +	STATS_DESC_COUNTER(VCPU, apicv_unaccelerated_inj),
>  };
>  
>  const struct kvm_stats_header kvm_vcpu_stats_header = {
> -- 
> 2.39.3
> 

Re: [PATCH 1/4] KVM: x86: Expose per-vCPU APICv status

[Sean Christopherson]

On Mon, Apr 29, 2024, Alejandro Jimenez wrote:
> Expose the APICv activation status of individual vCPUs via the stats
> subsystem. In special cases a vCPU's APICv can be deactivated/disabled
> even though there are no VM-wide inhibition reasons. The only current
> example of this is AVIC for a vCPU running in nested mode. This type of
> inhibition is not recorded in the VM inhibit reasons or visible in
> current tracepoints.
> 
> Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
> ---
>  arch/x86/include/asm/kvm_host.h | 1 +
>  arch/x86/kvm/lapic.c            | 1 +
>  arch/x86/kvm/x86.c              | 2 ++
>  3 files changed, 4 insertions(+)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 1d13e3cd1dc5..12f30cb5c842 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1573,6 +1573,7 @@ struct kvm_vcpu_stat {
>  	u64 preemption_other;
>  	u64 guest_mode;
>  	u64 notify_window_exits;
> +	u64 apicv_active;

Hrm, do we really have no way to effectively symlink stats to internal state?
That seems like a flaw.  It's obviously not the end of the world, but having to
burn 8 bytes per vCPU for boolean stats that are 1:1 reflections of KVM state is
going to be a deterrent for future stats.

>  };
>  
>  struct x86_instruction_info;
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index cf37586f0466..37fe75a5db8c 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -2872,6 +2872,7 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu, int timer_advance_ns)
>  	 */
>  	if (enable_apicv) {
>  		apic->apicv_active = true;
> +		vcpu->stat.apicv_active = apic->apicv_active;
>  		kvm_make_request(KVM_REQ_APICV_UPDATE, vcpu);
>  	}
>  
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index e9ef1fa4b90b..0451c4c8d731 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -304,6 +304,7 @@ const struct _kvm_stats_desc kvm_vcpu_stats_desc[] = {
>  	STATS_DESC_COUNTER(VCPU, preemption_other),
>  	STATS_DESC_IBOOLEAN(VCPU, guest_mode),
>  	STATS_DESC_COUNTER(VCPU, notify_window_exits),
> +	STATS_DESC_IBOOLEAN(VCPU, apicv_active),
>  };
>  
>  const struct kvm_stats_header kvm_vcpu_stats_header = {
> @@ -10625,6 +10626,7 @@ void __kvm_vcpu_update_apicv(struct kvm_vcpu *vcpu)
>  		goto out;
>  
>  	apic->apicv_active = activate;
> +	vcpu->stat.apicv_active = apic->apicv_active;
>  	kvm_apic_update_apicv(vcpu);
>  	static_call(kvm_x86_refresh_apicv_exec_ctrl)(vcpu);
>  
> -- 
> 2.39.3
> 

Re: [PATCH 4/4] KVM: x86: Add vCPU stat for APICv interrupt injections causing #VMEXIT

[Alejandro Jimenez]

On 6/3/24 20:14, Sean Christopherson wrote:
> On Mon, Apr 29, 2024, Alejandro Jimenez wrote:
>> Even when APICv/AVIC is active, certain guest accesses to its local APIC(s)
>> cannot be fully accelerated, and cause a #VMEXIT to allow the VMM to
>> emulate the behavior and side effects. Expose a counter stat for these
>> specific #VMEXIT types.
>>
>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
>> ---
>>   arch/x86/include/asm/kvm_host.h | 1 +
>>   arch/x86/kvm/svm/avic.c         | 7 +++++++
>>   arch/x86/kvm/vmx/vmx.c          | 2 ++
>>   arch/x86/kvm/x86.c              | 1 +
>>   4 files changed, 11 insertions(+)
>>
>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>> index e7e3213cefae..388979dfe9f3 100644
>> --- a/arch/x86/include/asm/kvm_host.h
>> +++ b/arch/x86/include/asm/kvm_host.h
>> @@ -1576,6 +1576,7 @@ struct kvm_vcpu_stat {
>>   	u64 guest_mode;
>>   	u64 notify_window_exits;
>>   	u64 apicv_active;
>> +	u64 apicv_unaccelerated_inj;
> 
> The stat name doesn't match the changelog or the code.  The AVIC updates in
> avic_incomplete_ipi_interception() are unaccelerated _injection_, they're
> unaccelarated _delivery_.  And in those cases, the fact that delivery wasn't
> accelerated is relatively uninteresting in most cases.
> 

Yeah, this was my flawed attempt to interpret/implement Paolo's comment in the RFC thread:

"... for example I'd add an interrupt_injections stat for unaccelerated injections causing a vmexit or otherwise hitting lapic.c"

so I incorrectly bundled together APIC accesses that result in #VMEXIT and end up requiring additional emulation (while managing to miss the handle_apic_access() case).


> And avic_unaccelerated_access_interception() and handle_apic_write() don't
> necessarily have anything to do with injection.

apicv_unaccelerated_acccess is perhaps a better name (assuming stat is updated in handle_apic_access() as well)?

> 
> On the flip side, the slow paths for {svm,vmx}_deliver_interrupt() are very
> explicitly unnaccelerated injection.

Now that you highlight this, I think it might be closer to Paolo's idea. i.e. a stat for the slow path on these can be contrasted/compared with the kvm_apicv_accept_irq tracepoint that is hit on the fast path.
My initial reaction would be to update a stat for the fast path, as a confirmation that apicv is active which is how/why I typically use the kvm_apicv_accept_irq tracepoint, but that becomes redundant by having the apicv_active stat on PATCH 1.

So, if you don't think it is useful to have a general apicv_unaccelerated_acccess counter, I can drop this patch.

Thank you,

Alejandro

> 
> It's not entirely clear from the changelog what the end goal of this stat is.
> A singular stat for all APICv/AVIC access VM-Exits seems uninteresting, as such
> a stat essentially just captures that the guest is active.  Maaaybe someone could
> glean info from comparing two VMs, but even that is dubious.  E.g. if a guest is
> doing something function and generating a lot of avic_incomplete_ipi_interception()
> exits, those will likely be in the noise due to the total volume of other AVIC
> exits.
> 
>>   };
>>   
>>   struct x86_instruction_info;
>> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
>> index 4b74ea91f4e6..274041d3cf66 100644
>> --- a/arch/x86/kvm/svm/avic.c
>> +++ b/arch/x86/kvm/svm/avic.c
>> @@ -517,6 +517,8 @@ int avic_incomplete_ipi_interception(struct kvm_vcpu *vcpu)
>>   			kvm_apic_write_nodecode(vcpu, APIC_ICR);
>>   		else
>>   			kvm_apic_send_ipi(apic, icrl, icrh);
>> +
>> +		++vcpu->stat.apicv_unaccelerated_inj;
>>   		break;
>>   	case AVIC_IPI_FAILURE_TARGET_NOT_RUNNING:
>>   		/*
>> @@ -525,6 +527,8 @@ int avic_incomplete_ipi_interception(struct kvm_vcpu *vcpu)
>>   		 * vcpus. So, we just need to kick the appropriate vcpu.
>>   		 */
>>   		avic_kick_target_vcpus(vcpu->kvm, apic, icrl, icrh, index);
>> +
>> +		++vcpu->stat.apicv_unaccelerated_inj;
>>   		break;
>>   	case AVIC_IPI_FAILURE_INVALID_BACKING_PAGE:
>>   		WARN_ONCE(1, "Invalid backing page\n");
>> @@ -704,6 +708,9 @@ int avic_unaccelerated_access_interception(struct kvm_vcpu *vcpu)
>>   
>>   	trace_kvm_avic_unaccelerated_access(vcpu->vcpu_id, offset,
>>   					    trap, write, vector);
>> +
>> +	++vcpu->stat.apicv_unaccelerated_inj;
>> +
>>   	if (trap) {
>>   		/* Handling Trap */
>>   		WARN_ONCE(!write, "svm: Handling trap read.\n");
>> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>> index f10b5f8f364b..a7487f12ded1 100644
>> --- a/arch/x86/kvm/vmx/vmx.c
>> +++ b/arch/x86/kvm/vmx/vmx.c
>> @@ -5657,6 +5657,8 @@ static int handle_apic_write(struct kvm_vcpu *vcpu)
>>   {
>>   	unsigned long exit_qualification = vmx_get_exit_qual(vcpu);
>>   
>> +	++vcpu->stat.apicv_unaccelerated_inj;
>> +
>>   	/*
>>   	 * APIC-write VM-Exit is trap-like, KVM doesn't need to advance RIP and
>>   	 * hardware has done any necessary aliasing, offset adjustments, etc...
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 03cb933920cb..c8730b0fac87 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -307,6 +307,7 @@ const struct _kvm_stats_desc kvm_vcpu_stats_desc[] = {
>>   	STATS_DESC_IBOOLEAN(VCPU, guest_mode),
>>   	STATS_DESC_COUNTER(VCPU, notify_window_exits),
>>   	STATS_DESC_IBOOLEAN(VCPU, apicv_active),
>> +	STATS_DESC_COUNTER(VCPU, apicv_unaccelerated_inj),
>>   };
>>   
>>   const struct kvm_stats_header kvm_vcpu_stats_header = {
>> -- 
>> 2.39.3
>>

Re: [PATCH 4/4] KVM: x86: Add vCPU stat for APICv interrupt injections causing #VMEXIT

[Sean Christopherson]

On Thu, Jun 06, 2024, Alejandro Jimenez wrote:
> On 6/3/24 20:14, Sean Christopherson wrote:
> > On Mon, Apr 29, 2024, Alejandro Jimenez wrote:
> > > Even when APICv/AVIC is active, certain guest accesses to its local APIC(s)
> > > cannot be fully accelerated, and cause a #VMEXIT to allow the VMM to
> > > emulate the behavior and side effects. Expose a counter stat for these
> > > specific #VMEXIT types.
> > > 
> > > Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> > > Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
> > > ---
> > >   arch/x86/include/asm/kvm_host.h | 1 +
> > >   arch/x86/kvm/svm/avic.c         | 7 +++++++
> > >   arch/x86/kvm/vmx/vmx.c          | 2 ++
> > >   arch/x86/kvm/x86.c              | 1 +
> > >   4 files changed, 11 insertions(+)
> > > 
> > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > > index e7e3213cefae..388979dfe9f3 100644
> > > --- a/arch/x86/include/asm/kvm_host.h
> > > +++ b/arch/x86/include/asm/kvm_host.h
> > > @@ -1576,6 +1576,7 @@ struct kvm_vcpu_stat {
> > >   	u64 guest_mode;
> > >   	u64 notify_window_exits;
> > >   	u64 apicv_active;
> > > +	u64 apicv_unaccelerated_inj;
> > 
> > The stat name doesn't match the changelog or the code.  The AVIC updates in
> > avic_incomplete_ipi_interception() are unaccelerated _injection_, they're
> > unaccelarated _delivery_.  And in those cases, the fact that delivery wasn't
> > accelerated is relatively uninteresting in most cases.
> > 
> 
> Yeah, this was my flawed attempt to interpret/implement Paolo's comment in
> the RFC thread:
> 
> "... for example I'd add an interrupt_injections stat for unaccelerated
> injections causing a vmexit or otherwise hitting lapic.c"

KVM essentially already has this stat, irq_injections.  Sort of.  The problem is
that the stat isn't bumped when APICv is enabled because the IRQ isn't *directly*
injected.  KVM does "inject" the IRQ into the IRR (and RVI on Intel), but KVM
doesn't go through .inject_irq().

For APICv, KVM could bump the stat when manually moving the IRQ from the IRR to
RVI, but that'd be more than a bit misleading with respect to AVIC.  With AVIC,
the CPU itself processes the IRR on VMRUN, i.e. there's no software intervention
needed to get the CPU to inject the IRQ.  But practically speaking, there's no
meaningful difference between the two flows; in both cases an IRQ arrived while
the target vCPU wasn't actively running the guest.  And that means KVM would need
to parse the IRR on AMD just to bump a stat.

It'd also be misleading to some extent in general, because when the target vCPU
is in its inner run loop, but not actually post-VM-Enter, KVM doesn't kick the
vCPU because either KVM or the CPU will automatically process the pending IRQ.
I.e. KVM would bump the stat cases where the injection isn't fully accelerated,
but that's somewhat disingenuous because KVM didn't need to slow down the vCPU
in order to deliver the interrupt.

And KVM already has an irq_exits stat, which can be used to get a rough feel for
how often KVM is kicking a vCPU (though timer ticks likely dominate the stat).

> > And avic_unaccelerated_access_interception() and handle_apic_write() don't
> > necessarily have anything to do with injection.
> 
> apicv_unaccelerated_acccess is perhaps a better name (assuming stat is
> updated in handle_apic_access() as well)?

This is again not super interesting.  If we were to add this stat, I would lobby
hard for turning "exits" into an array that accounts each individual VM-Exit,
though with some massaging to reconcile differences between VMX and SVM.

Unaccelerated APIC exits aren't completely uninteresting, but they suffer similar
issues to the "exits" stat: a few flavors of APIC exits would dominate the stats,
and _those_ exits aren't very interesting.

> > On the flip side, the slow paths for {svm,vmx}_deliver_interrupt() are very
> > explicitly unnaccelerated injection.
> 
> Now that you highlight this, I think it might be closer to Paolo's idea. i.e.
> a stat for the slow path on these can be contrasted/compared with the
> kvm_apicv_accept_irq tracepoint that is hit on the fast path.  My initial
> reaction would be to update a stat for the fast path, as a confirmation that
> apicv is active which is how/why I typically use the kvm_apicv_accept_irq
> tracepoint, but that becomes redundant by having the apicv_active stat on
> PATCH 1.
> 
> So, if you don't think it is useful to have a general
> apicv_unaccelerated_acccess counter, I can drop this patch.

The one thing I can think of that might be somewhat interesting is when
kvm_apic_send_ipi() is invoked to deliver an IPI.  If KVM manually sends the IPI,
and IPI virtualization is enabled (on-by-default in AVIC, and an add-on feature
for APICv), then it means IPI virtualization isn't doing it's job for whatever
reason.  But even then, I'm doubt it's worth a stat, because it likely just means
the guest is doing something weird, not that there's a problem in KVM.