[PATCH v2 0/2] Print names of apicv inhibit reasons in traces

[PATCH v2 0/2] Print names of apicv inhibit reasons in traces

[Alejandro Jimenez]

v2:
- Use Sean's implementation/patch from v1: https://lore.kernel.org/all/ZjVQOFLXWrZvoa-Y@google.com/
- Fix typo in commit message (s/inhbit/inhibit).
- Add patch renaming APICV_INHIBIT_REASON_DISABLE to APICV_INHIBIT_REASON_DISABLED.
- Drop Vasant's R-b from v1 since implementation was refined, even though the
general approach and behavior remains the same.

v1: https://lore.kernel.org/all/20240214223554.1033154-1-alejandro.j.jimenez@oracle.com/

Tested on Genoa system. With the proposed changes, the tracepoint output looks
like the following examples:

 qemu-system-x86-7068    [194] .....  1397.647770: kvm_apicv_inhibit_changed: set reason=2, inhibits=0x4 ABSENT
 qemu-system-x86-7068    [003] .....  1397.676703: kvm_apicv_inhibit_changed: cleared reason=2, inhibits=0x0
 qemu-system-x86-7074    [247] .....  1397.701398: kvm_apicv_inhibit_changed: cleared reason=4, inhibits=0x0

 qemu-system-x86-7074    [008] .....  1408.697413: kvm_apicv_inhibit_changed: set reason=8, inhibits=0x100 IRQWIN
 qemu-system-x86-7074    [008] .....  1408.697420: kvm_apicv_inhibit_changed: cleared reason=8, inhibits=0x0

[...]

 qemu-system-x86-7173    [056] .....  1570.541372: kvm_apicv_inhibit_changed: set reason=8, inhibits=0x300 IRQWIN|PIT_REINJ
 qemu-system-x86-7173    [056] .....  1570.541380: kvm_apicv_inhibit_changed: cleared reason=8, inhibits=0x200 PIT_REINJ


Alejandro Jimenez (2):
  KVM: x86: Print names of apicv inhibit reasons in traces
  KVM: x86: Keep consistent naming for APICv/AVIC inhibit reasons

 arch/x86/include/asm/kvm_host.h | 21 ++++++++++++++++++++-
 arch/x86/kvm/svm/svm.h          |  2 +-
 arch/x86/kvm/trace.h            |  9 +++++++--
 arch/x86/kvm/vmx/main.c         |  2 +-
 arch/x86/kvm/x86.c              |  6 +++++-
 5 files changed, 34 insertions(+), 6 deletions(-)


base-commit: d91a9cc16417b8247213a0144a1f0fd61dc855dd
-- 
2.39.3

[PATCH v2 1/2] KVM: x86: Print names of apicv inhibit reasons in traces

[Alejandro Jimenez]

Use the tracing infrastructure helper __print_flags() for printing flag
bitfields, to enhance the trace output by displaying a string describing
each of the inhibit reasons set.

The kvm_apicv_inhibit_changed tracepoint currently shows the raw bitmap
value, requiring the user to consult the source file where the inhibit
reasons are defined to decode the trace output.

Co-developed-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
---
 arch/x86/include/asm/kvm_host.h | 19 +++++++++++++++++++
 arch/x86/kvm/trace.h            |  9 +++++++--
 arch/x86/kvm/x86.c              |  4 ++++
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 1d13e3cd1dc5..08f83efd12ff 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1273,8 +1273,27 @@ enum kvm_apicv_inhibit {
 	 * mapping between logical ID and vCPU.
 	 */
 	APICV_INHIBIT_REASON_LOGICAL_ID_ALIASED,
+
+	NR_APICV_INHIBIT_REASONS,
 };
 
+#define __APICV_INHIBIT_REASON(reason)			\
+	{ BIT(APICV_INHIBIT_REASON_##reason), #reason }
+
+#define APICV_INHIBIT_REASONS				\
+	__APICV_INHIBIT_REASON(DISABLE),		\
+	__APICV_INHIBIT_REASON(HYPERV),			\
+	__APICV_INHIBIT_REASON(ABSENT),			\
+	__APICV_INHIBIT_REASON(BLOCKIRQ),		\
+	__APICV_INHIBIT_REASON(PHYSICAL_ID_ALIASED),	\
+	__APICV_INHIBIT_REASON(APIC_ID_MODIFIED),	\
+	__APICV_INHIBIT_REASON(APIC_BASE_MODIFIED),	\
+	__APICV_INHIBIT_REASON(NESTED),			\
+	__APICV_INHIBIT_REASON(IRQWIN),			\
+	__APICV_INHIBIT_REASON(PIT_REINJ),		\
+	__APICV_INHIBIT_REASON(SEV),			\
+	__APICV_INHIBIT_REASON(LOGICAL_ID_ALIASED)
+
 struct kvm_arch {
 	unsigned long n_used_mmu_pages;
 	unsigned long n_requested_mmu_pages;
diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index 9d0b02ef307e..f23fb9a6776e 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -1375,6 +1375,10 @@ TRACE_EVENT(kvm_hv_stimer_cleanup,
 		  __entry->vcpu_id, __entry->timer_index)
 );
 
+#define kvm_print_apicv_inhibit_reasons(inhibits)	\
+	(inhibits), (inhibits) ? " " : "",		\
+	(inhibits) ? __print_flags(inhibits, "|", APICV_INHIBIT_REASONS) : ""
+
 TRACE_EVENT(kvm_apicv_inhibit_changed,
 	    TP_PROTO(int reason, bool set, unsigned long inhibits),
 	    TP_ARGS(reason, set, inhibits),
@@ -1391,9 +1395,10 @@ TRACE_EVENT(kvm_apicv_inhibit_changed,
 		__entry->inhibits = inhibits;
 	),
 
-	TP_printk("%s reason=%u, inhibits=0x%lx",
+	TP_printk("%s reason=%u, inhibits=0x%lx%s%s",
 		  __entry->set ? "set" : "cleared",
-		  __entry->reason, __entry->inhibits)
+		  __entry->reason,
+		  kvm_print_apicv_inhibit_reasons(__entry->inhibits))
 );
 
 TRACE_EVENT(kvm_apicv_accept_irq,
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index b389129d59a9..597ff748f955 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -10011,6 +10011,10 @@ EXPORT_SYMBOL_GPL(kvm_vcpu_apicv_activated);
 static void set_or_clear_apicv_inhibit(unsigned long *inhibits,
 				       enum kvm_apicv_inhibit reason, bool set)
 {
+	const struct trace_print_flags apicv_inhibits[] = { APICV_INHIBIT_REASONS };
+
+	BUILD_BUG_ON(ARRAY_SIZE(apicv_inhibits) != NR_APICV_INHIBIT_REASONS);
+
 	if (set)
 		__set_bit(reason, inhibits);
 	else
-- 
2.39.3

[PATCH v2 2/2] KVM: x86: Keep consistent naming for APICv/AVIC inhibit reasons

[Alejandro Jimenez]

Keep kvm_apicv_inhibit enum naming consistent with the current pattern by
renaming the reason/enumerator defined as APICV_INHIBIT_REASON_DISABLE to
APICV_INHIBIT_REASON_DISABLED.

No functional change intended.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
---
 arch/x86/include/asm/kvm_host.h | 4 ++--
 arch/x86/kvm/svm/svm.h          | 2 +-
 arch/x86/kvm/vmx/main.c         | 2 +-
 arch/x86/kvm/x86.c              | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 08f83efd12ff..8910a8ac23b0 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1203,7 +1203,7 @@ enum kvm_apicv_inhibit {
 	 * APIC acceleration is disabled by a module parameter
 	 * and/or not supported in hardware.
 	 */
-	APICV_INHIBIT_REASON_DISABLE,
+	APICV_INHIBIT_REASON_DISABLED,
 
 	/*
 	 * APIC acceleration is inhibited because AutoEOI feature is
@@ -1281,7 +1281,7 @@ enum kvm_apicv_inhibit {
 	{ BIT(APICV_INHIBIT_REASON_##reason), #reason }
 
 #define APICV_INHIBIT_REASONS				\
-	__APICV_INHIBIT_REASON(DISABLE),		\
+	__APICV_INHIBIT_REASON(DISABLED),		\
 	__APICV_INHIBIT_REASON(HYPERV),			\
 	__APICV_INHIBIT_REASON(ABSENT),			\
 	__APICV_INHIBIT_REASON(BLOCKIRQ),		\
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 323901782547..7ba09aca2342 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -634,7 +634,7 @@ extern struct kvm_x86_nested_ops svm_nested_ops;
 /* avic.c */
 #define AVIC_REQUIRED_APICV_INHIBITS			\
 (							\
-	BIT(APICV_INHIBIT_REASON_DISABLE) |		\
+	BIT(APICV_INHIBIT_REASON_DISABLED) |		\
 	BIT(APICV_INHIBIT_REASON_ABSENT) |		\
 	BIT(APICV_INHIBIT_REASON_HYPERV) |		\
 	BIT(APICV_INHIBIT_REASON_NESTED) |		\
diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c
index 7c546ad3e4c9..5418feb17e13 100644
--- a/arch/x86/kvm/vmx/main.c
+++ b/arch/x86/kvm/vmx/main.c
@@ -7,7 +7,7 @@
 #include "pmu.h"
 
 #define VMX_REQUIRED_APICV_INHIBITS				\
-	(BIT(APICV_INHIBIT_REASON_DISABLE)|			\
+	(BIT(APICV_INHIBIT_REASON_DISABLED) |			\
 	 BIT(APICV_INHIBIT_REASON_ABSENT) |			\
 	 BIT(APICV_INHIBIT_REASON_HYPERV) |			\
 	 BIT(APICV_INHIBIT_REASON_BLOCKIRQ) |			\
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 597ff748f955..9747c3bc52f9 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -10026,7 +10026,7 @@ static void set_or_clear_apicv_inhibit(unsigned long *inhibits,
 static void kvm_apicv_init(struct kvm *kvm)
 {
 	enum kvm_apicv_inhibit reason = enable_apicv ? APICV_INHIBIT_REASON_ABSENT :
-						       APICV_INHIBIT_REASON_DISABLE;
+						       APICV_INHIBIT_REASON_DISABLED;
 
 	set_or_clear_apicv_inhibit(&kvm->arch.apicv_inhibit_reasons, reason, true);
 
-- 
2.39.3

Re: [PATCH v2 1/2] KVM: x86: Print names of apicv inhibit reasons in traces

[Vasant Hegde]

On 5/7/2024 4:23 AM, Alejandro Jimenez wrote:
> Use the tracing infrastructure helper __print_flags() for printing flag
> bitfields, to enhance the trace output by displaying a string describing
> each of the inhibit reasons set.
> 
> The kvm_apicv_inhibit_changed tracepoint currently shows the raw bitmap
> value, requiring the user to consult the source file where the inhibit
> reasons are defined to decode the trace output.
> 
> Co-developed-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>

Looks good to me.

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

-Vasant

> ---
>  arch/x86/include/asm/kvm_host.h | 19 +++++++++++++++++++
>  arch/x86/kvm/trace.h            |  9 +++++++--
>  arch/x86/kvm/x86.c              |  4 ++++
>  3 files changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 1d13e3cd1dc5..08f83efd12ff 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1273,8 +1273,27 @@ enum kvm_apicv_inhibit {
>  	 * mapping between logical ID and vCPU.
>  	 */
>  	APICV_INHIBIT_REASON_LOGICAL_ID_ALIASED,
> +
> +	NR_APICV_INHIBIT_REASONS,
>  };
>  
> +#define __APICV_INHIBIT_REASON(reason)			\
> +	{ BIT(APICV_INHIBIT_REASON_##reason), #reason }
> +
> +#define APICV_INHIBIT_REASONS				\
> +	__APICV_INHIBIT_REASON(DISABLE),		\
> +	__APICV_INHIBIT_REASON(HYPERV),			\
> +	__APICV_INHIBIT_REASON(ABSENT),			\
> +	__APICV_INHIBIT_REASON(BLOCKIRQ),		\
> +	__APICV_INHIBIT_REASON(PHYSICAL_ID_ALIASED),	\
> +	__APICV_INHIBIT_REASON(APIC_ID_MODIFIED),	\
> +	__APICV_INHIBIT_REASON(APIC_BASE_MODIFIED),	\
> +	__APICV_INHIBIT_REASON(NESTED),			\
> +	__APICV_INHIBIT_REASON(IRQWIN),			\
> +	__APICV_INHIBIT_REASON(PIT_REINJ),		\
> +	__APICV_INHIBIT_REASON(SEV),			\
> +	__APICV_INHIBIT_REASON(LOGICAL_ID_ALIASED)
> +
>  struct kvm_arch {
>  	unsigned long n_used_mmu_pages;
>  	unsigned long n_requested_mmu_pages;
> diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
> index 9d0b02ef307e..f23fb9a6776e 100644
> --- a/arch/x86/kvm/trace.h
> +++ b/arch/x86/kvm/trace.h
> @@ -1375,6 +1375,10 @@ TRACE_EVENT(kvm_hv_stimer_cleanup,
>  		  __entry->vcpu_id, __entry->timer_index)
>  );
>  
> +#define kvm_print_apicv_inhibit_reasons(inhibits)	\
> +	(inhibits), (inhibits) ? " " : "",		\
> +	(inhibits) ? __print_flags(inhibits, "|", APICV_INHIBIT_REASONS) : ""
> +
>  TRACE_EVENT(kvm_apicv_inhibit_changed,
>  	    TP_PROTO(int reason, bool set, unsigned long inhibits),
>  	    TP_ARGS(reason, set, inhibits),
> @@ -1391,9 +1395,10 @@ TRACE_EVENT(kvm_apicv_inhibit_changed,
>  		__entry->inhibits = inhibits;
>  	),
>  
> -	TP_printk("%s reason=%u, inhibits=0x%lx",
> +	TP_printk("%s reason=%u, inhibits=0x%lx%s%s",
>  		  __entry->set ? "set" : "cleared",
> -		  __entry->reason, __entry->inhibits)
> +		  __entry->reason,
> +		  kvm_print_apicv_inhibit_reasons(__entry->inhibits))
>  );
>  
>  TRACE_EVENT(kvm_apicv_accept_irq,
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index b389129d59a9..597ff748f955 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -10011,6 +10011,10 @@ EXPORT_SYMBOL_GPL(kvm_vcpu_apicv_activated);
>  static void set_or_clear_apicv_inhibit(unsigned long *inhibits,
>  				       enum kvm_apicv_inhibit reason, bool set)
>  {
> +	const struct trace_print_flags apicv_inhibits[] = { APICV_INHIBIT_REASONS };
> +
> +	BUILD_BUG_ON(ARRAY_SIZE(apicv_inhibits) != NR_APICV_INHIBIT_REASONS);
> +
>  	if (set)
>  		__set_bit(reason, inhibits);
>  	else

Re: [PATCH v2 0/2] Print names of apicv inhibit reasons in traces

[Sean Christopherson]

On Mon, 06 May 2024 22:53:19 +0000, Alejandro Jimenez wrote:
> v2:
> - Use Sean's implementation/patch from v1: https://lore.kernel.org/all/ZjVQOFLXWrZvoa-Y@google.com/
> - Fix typo in commit message (s/inhbit/inhibit).
> - Add patch renaming APICV_INHIBIT_REASON_DISABLE to APICV_INHIBIT_REASON_DISABLED.
> - Drop Vasant's R-b from v1 since implementation was refined, even though the
> general approach and behavior remains the same.
> 
> [...]

Applied to kvm-x86 misc, thanks!

[1/2] KVM: x86: Print names of apicv inhibit reasons in traces
      https://github.com/kvm-x86/linux/commit/8b5bf6b80eb3
[2/2] KVM: x86: Keep consistent naming for APICv/AVIC inhibit reasons
      https://github.com/kvm-x86/linux/commit/f9979c52eb02

--
https://github.com/kvm-x86/linux/tree/next

Re: [PATCH v2 0/2] Print names of apicv inhibit reasons in traces

[Sean Christopherson]

On Tue, Jun 04, 2024, Sean Christopherson wrote:
> On Mon, 06 May 2024 22:53:19 +0000, Alejandro Jimenez wrote:
> > v2:
> > - Use Sean's implementation/patch from v1: https://lore.kernel.org/all/ZjVQOFLXWrZvoa-Y@google.com/
> > - Fix typo in commit message (s/inhbit/inhibit).
> > - Add patch renaming APICV_INHIBIT_REASON_DISABLE to APICV_INHIBIT_REASON_DISABLED.
> > - Drop Vasant's R-b from v1 since implementation was refined, even though the
> > general approach and behavior remains the same.
> > 
> > [...]
> 
> Applied to kvm-x86 misc, thanks!
> 
> [1/2] KVM: x86: Print names of apicv inhibit reasons in traces
>       https://github.com/kvm-x86/linux/commit/8b5bf6b80eb3
> [2/2] KVM: x86: Keep consistent naming for APICv/AVIC inhibit reasons
>       https://github.com/kvm-x86/linux/commit/f9979c52eb02

FYI, hashes changed due to dropping an unrelated commit.

[1/2] KVM: x86: Print names of apicv inhibit reasons in traces
      https://github.com/kvm-x86/linux/commit/69148ccec679
[2/2] KVM: x86: Keep consistent naming for APICv/AVIC inhibit reasons
      https://github.com/kvm-x86/linux/commit/f992572120fb