diff mbox series

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

Message ID 20240214223554.1033154-1-alejandro.j.jimenez@oracle.com (mailing list archive)
State New, archived
Headers show
Series [1/1] KVM: x86: Print names of apicv inhibit reasons in traces | expand

Commit Message

Alejandro Jimenez Feb. 14, 2024, 10:35 p.m. UTC 
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 inhbit
reasons are defined to decode the trace output.

Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>

---
checkpatch reports an error:
ERROR: Macros with complex values should be enclosed in parentheses

but that seems common for other patches that also use a macro to define an array
of struct trace_print_flags used by __print_flags().

I did not include an example of the new traces in the commit message since they
are longer than 80 columns, but perhaps that is desirable. e.g.:

qemu-system-x86-6961    [055] .....  1779.344065: kvm_apicv_inhibit_changed: set reason=2, inhibits=0x4 ABSENT
qemu-system-x86-6961    [055] .....  1779.356710: kvm_apicv_inhibit_changed: cleared reason=2, inhibits=0x0

qemu-system-x86-9912    [137] ..... 57106.196107: kvm_apicv_inhibit_changed: set reason=8, inhibits=0x300 IRQWIN|PIT_REINJ
qemu-system-x86-9912    [137] ..... 57106.196115: kvm_apicv_inhibit_changed: cleared reason=8, inhibits=0x200 PIT_REINJ
---
 arch/x86/kvm/trace.h | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)


base-commit: 7455665a3521aa7b56245c0a2810f748adc5fdd4

Comments

Alejandro Jimenez March 8, 2024, 4:06 p.m. UTC | #1 
On 2/14/24 17:35, 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 inhbit
> reasons are defined to decode the trace output.
> 
> Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
> 
> ---
> checkpatch reports an error:
> ERROR: Macros with complex values should be enclosed in parentheses
> 
> but that seems common for other patches that also use a macro to define an array
> of struct trace_print_flags used by __print_flags().
> 
> I did not include an example of the new traces in the commit message since they
> are longer than 80 columns, but perhaps that is desirable. e.g.:
> 
> qemu-system-x86-6961    [055] .....  1779.344065: kvm_apicv_inhibit_changed: set reason=2, inhibits=0x4 ABSENT
> qemu-system-x86-6961    [055] .....  1779.356710: kvm_apicv_inhibit_changed: cleared reason=2, inhibits=0x0
> 
> qemu-system-x86-9912    [137] ..... 57106.196107: kvm_apicv_inhibit_changed: set reason=8, inhibits=0x300 IRQWIN|PIT_REINJ
> qemu-system-x86-9912    [137] ..... 57106.196115: kvm_apicv_inhibit_changed: cleared reason=8, inhibits=0x200 PIT_REINJ
> ---
>   arch/x86/kvm/trace.h | 28 ++++++++++++++++++++++++++--
>   1 file changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
> index b82e6ed4f024..8469e59dfce2 100644
> --- a/arch/x86/kvm/trace.h
> +++ b/arch/x86/kvm/trace.h
> @@ -1372,6 +1372,27 @@ TRACE_EVENT(kvm_hv_stimer_cleanup,
>   		  __entry->vcpu_id, __entry->timer_index)
>   );
>   
> +/*
> + * The inhibit flags in this flag array must be kept in sync with the
> + * kvm_apicv_inhibit enum members in <asm/kvm_host.h>.
> + */
> +#define APICV_INHIBIT_FLAGS \
> +	{ BIT(APICV_INHIBIT_REASON_DISABLE),		 "DISABLED" }, \
> +	{ BIT(APICV_INHIBIT_REASON_HYPERV),		 "HYPERV" }, \
> +	{ BIT(APICV_INHIBIT_REASON_ABSENT),		 "ABSENT" }, \
> +	{ BIT(APICV_INHIBIT_REASON_BLOCKIRQ),		 "BLOCKIRQ" }, \
> +	{ BIT(APICV_INHIBIT_REASON_PHYSICAL_ID_ALIASED), "PHYS_ID_ALIASED" }, \
> +	{ BIT(APICV_INHIBIT_REASON_APIC_ID_MODIFIED),	 "APIC_ID_MOD" }, \
> +	{ BIT(APICV_INHIBIT_REASON_APIC_BASE_MODIFIED),	 "APIC_BASE_MOD" }, \
> +	{ BIT(APICV_INHIBIT_REASON_NESTED),		 "NESTED" }, \
> +	{ BIT(APICV_INHIBIT_REASON_IRQWIN),		 "IRQWIN" }, \
> +	{ BIT(APICV_INHIBIT_REASON_PIT_REINJ),		 "PIT_REINJ" }, \
> +	{ BIT(APICV_INHIBIT_REASON_SEV),		 "SEV" }, \
> +	{ BIT(APICV_INHIBIT_REASON_LOGICAL_ID_ALIASED),	 "LOG_ID_ALIASED" } \
> +
> +#define show_inhibit_reasons(inhibits) \
> +	__print_flags(inhibits, "|", APICV_INHIBIT_FLAGS)
> +
>   TRACE_EVENT(kvm_apicv_inhibit_changed,
>   	    TP_PROTO(int reason, bool set, unsigned long inhibits),
>   	    TP_ARGS(reason, set, inhibits),
> @@ -1388,9 +1409,12 @@ 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, __entry->inhibits,
> +		  __entry->inhibits ? " " : "",
> +		  __entry->inhibits ?
> +		  show_inhibit_reasons(__entry->inhibits) : "")
>   );
>   
>   TRACE_EVENT(kvm_apicv_accept_irq,
> 
> base-commit: 7455665a3521aa7b56245c0a2810f748adc5fdd4
ping..
Alejandro Jimenez March 26, 2024, 4:47 p.m. UTC | #2 
On 3/8/24 11:06, Alejandro Jimenez wrote:
> 
> 
> On 2/14/24 17:35, 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 inhbit
>> reasons are defined to decode the trace output.
>>
>> Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
>>
>> ---
>> checkpatch reports an error:
>> ERROR: Macros with complex values should be enclosed in parentheses
>>
>> but that seems common for other patches that also use a macro to define an array
>> of struct trace_print_flags used by __print_flags().
>>
>> I did not include an example of the new traces in the commit message since they
>> are longer than 80 columns, but perhaps that is desirable. e.g.:
>>
>> qemu-system-x86-6961    [055] .....  1779.344065: kvm_apicv_inhibit_changed: set reason=2, inhibits=0x4 ABSENT
>> qemu-system-x86-6961    [055] .....  1779.356710: kvm_apicv_inhibit_changed: cleared reason=2, inhibits=0x0
>>
>> qemu-system-x86-9912    [137] ..... 57106.196107: kvm_apicv_inhibit_changed: set reason=8, inhibits=0x300 IRQWIN|PIT_REINJ
>> qemu-system-x86-9912    [137] ..... 57106.196115: kvm_apicv_inhibit_changed: cleared reason=8, inhibits=0x200 PIT_REINJ
>> ---
>>   arch/x86/kvm/trace.h | 28 ++++++++++++++++++++++++++--
>>   1 file changed, 26 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
>> index b82e6ed4f024..8469e59dfce2 100644
>> --- a/arch/x86/kvm/trace.h
>> +++ b/arch/x86/kvm/trace.h
>> @@ -1372,6 +1372,27 @@ TRACE_EVENT(kvm_hv_stimer_cleanup,
>>             __entry->vcpu_id, __entry->timer_index)
>>   );
>> +/*
>> + * The inhibit flags in this flag array must be kept in sync with the
>> + * kvm_apicv_inhibit enum members in <asm/kvm_host.h>.
>> + */
>> +#define APICV_INHIBIT_FLAGS \
>> +    { BIT(APICV_INHIBIT_REASON_DISABLE),         "DISABLED" }, \
>> +    { BIT(APICV_INHIBIT_REASON_HYPERV),         "HYPERV" }, \
>> +    { BIT(APICV_INHIBIT_REASON_ABSENT),         "ABSENT" }, \
>> +    { BIT(APICV_INHIBIT_REASON_BLOCKIRQ),         "BLOCKIRQ" }, \
>> +    { BIT(APICV_INHIBIT_REASON_PHYSICAL_ID_ALIASED), "PHYS_ID_ALIASED" }, \
>> +    { BIT(APICV_INHIBIT_REASON_APIC_ID_MODIFIED),     "APIC_ID_MOD" }, \
>> +    { BIT(APICV_INHIBIT_REASON_APIC_BASE_MODIFIED),     "APIC_BASE_MOD" }, \
>> +    { BIT(APICV_INHIBIT_REASON_NESTED),         "NESTED" }, \
>> +    { BIT(APICV_INHIBIT_REASON_IRQWIN),         "IRQWIN" }, \
>> +    { BIT(APICV_INHIBIT_REASON_PIT_REINJ),         "PIT_REINJ" }, \
>> +    { BIT(APICV_INHIBIT_REASON_SEV),         "SEV" }, \
>> +    { BIT(APICV_INHIBIT_REASON_LOGICAL_ID_ALIASED),     "LOG_ID_ALIASED" } \
>> +
>> +#define show_inhibit_reasons(inhibits) \
>> +    __print_flags(inhibits, "|", APICV_INHIBIT_FLAGS)
>> +
>>   TRACE_EVENT(kvm_apicv_inhibit_changed,
>>           TP_PROTO(int reason, bool set, unsigned long inhibits),
>>           TP_ARGS(reason, set, inhibits),
>> @@ -1388,9 +1409,12 @@ 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, __entry->inhibits,
>> +          __entry->inhibits ? " " : "",
>> +          __entry->inhibits ?
>> +          show_inhibit_reasons(__entry->inhibits) : "")
>>   );
>>   TRACE_EVENT(kvm_apicv_accept_irq,
>>
>> base-commit: 7455665a3521aa7b56245c0a2810f748adc5fdd4
> ping..
> 

ping...

This is a trivial but useful usability improvement, and I don't expect it to be controversial (other than perhaps formatting of the string), but I'd appreciate any comments.

Alejandro
Vasant Hegde April 8, 2024, 8:43 a.m. UTC | #3 
On 2/15/2024 4:05 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 inhbit
> reasons are defined to decode the trace output.
> 
> Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>

I have reviewed and tested this patch on AMD Genoa system. It looks good 
to me.

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

-Vasant

> 
> ---
> checkpatch reports an error:
> ERROR: Macros with complex values should be enclosed in parentheses
> 
> but that seems common for other patches that also use a macro to define an array
> of struct trace_print_flags used by __print_flags().
> 
> I did not include an example of the new traces in the commit message since they
> are longer than 80 columns, but perhaps that is desirable. e.g.:
> 
> qemu-system-x86-6961    [055] .....  1779.344065: kvm_apicv_inhibit_changed: set reason=2, inhibits=0x4 ABSENT
> qemu-system-x86-6961    [055] .....  1779.356710: kvm_apicv_inhibit_changed: cleared reason=2, inhibits=0x0
> 
> qemu-system-x86-9912    [137] ..... 57106.196107: kvm_apicv_inhibit_changed: set reason=8, inhibits=0x300 IRQWIN|PIT_REINJ
> qemu-system-x86-9912    [137] ..... 57106.196115: kvm_apicv_inhibit_changed: cleared reason=8, inhibits=0x200 PIT_REINJ
> ---
>   arch/x86/kvm/trace.h | 28 ++++++++++++++++++++++++++--
>   1 file changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
> index b82e6ed4f024..8469e59dfce2 100644
> --- a/arch/x86/kvm/trace.h
> +++ b/arch/x86/kvm/trace.h
> @@ -1372,6 +1372,27 @@ TRACE_EVENT(kvm_hv_stimer_cleanup,
>   		  __entry->vcpu_id, __entry->timer_index)
>   );
>   
> +/*
> + * The inhibit flags in this flag array must be kept in sync with the
> + * kvm_apicv_inhibit enum members in <asm/kvm_host.h>.
> + */
> +#define APICV_INHIBIT_FLAGS \
> +	{ BIT(APICV_INHIBIT_REASON_DISABLE),		 "DISABLED" }, \
> +	{ BIT(APICV_INHIBIT_REASON_HYPERV),		 "HYPERV" }, \
> +	{ BIT(APICV_INHIBIT_REASON_ABSENT),		 "ABSENT" }, \
> +	{ BIT(APICV_INHIBIT_REASON_BLOCKIRQ),		 "BLOCKIRQ" }, \
> +	{ BIT(APICV_INHIBIT_REASON_PHYSICAL_ID_ALIASED), "PHYS_ID_ALIASED" }, \
> +	{ BIT(APICV_INHIBIT_REASON_APIC_ID_MODIFIED),	 "APIC_ID_MOD" }, \
> +	{ BIT(APICV_INHIBIT_REASON_APIC_BASE_MODIFIED),	 "APIC_BASE_MOD" }, \
> +	{ BIT(APICV_INHIBIT_REASON_NESTED),		 "NESTED" }, \
> +	{ BIT(APICV_INHIBIT_REASON_IRQWIN),		 "IRQWIN" }, \
> +	{ BIT(APICV_INHIBIT_REASON_PIT_REINJ),		 "PIT_REINJ" }, \
> +	{ BIT(APICV_INHIBIT_REASON_SEV),		 "SEV" }, \
> +	{ BIT(APICV_INHIBIT_REASON_LOGICAL_ID_ALIASED),	 "LOG_ID_ALIASED" } \
> +
> +#define show_inhibit_reasons(inhibits) \
> +	__print_flags(inhibits, "|", APICV_INHIBIT_FLAGS)
> +
>   TRACE_EVENT(kvm_apicv_inhibit_changed,
>   	    TP_PROTO(int reason, bool set, unsigned long inhibits),
>   	    TP_ARGS(reason, set, inhibits),
> @@ -1388,9 +1409,12 @@ 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, __entry->inhibits,
> +		  __entry->inhibits ? " " : "",
> +		  __entry->inhibits ?
> +		  show_inhibit_reasons(__entry->inhibits) : "")
>   );
>   
>   TRACE_EVENT(kvm_apicv_accept_irq,
> 
> base-commit: 7455665a3521aa7b56245c0a2810f748adc5fdd4
Sean Christopherson May 3, 2024, 8:59 p.m. UTC | #4 
On Wed, Feb 14, 2024, 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 inhbit
> reasons are defined to decode the trace output.
> 
> Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
> 
> ---
> checkpatch reports an error:
> ERROR: Macros with complex values should be enclosed in parentheses

Yeah, checkpatch is a tool/guide, there are situations where it's best to use
common sense and ignore checkpatch.

> +/*
> + * The inhibit flags in this flag array must be kept in sync with the
> + * kvm_apicv_inhibit enum members in <asm/kvm_host.h>.

Comments are nice, code is better.  And in this case, it's quite easy to assert
that all inhibits have an entry via BUILD_BUG_ON(), e.g. by defining a throwaway
array and asserting that ARRAY_SIZE == NR_APICV_INHIBIT_REASONS.

It's not so easy to add such an assert for the VMX exit reason because the numbers
aren't arbitrary and there are gaps, but for APICv inhibits, the numbers truly
don't matter.

> + */
> +#define APICV_INHIBIT_FLAGS \
> +	{ BIT(APICV_INHIBIT_REASON_DISABLE),		 "DISABLED" }, \

To reduce copy+paste and especially to minimize the number of typos, use another
macro, e.g.

#define __APICV_INHIBIT_REASON(reason)			\
	{ BIT(APICV_INHIBIT_REASON_##reason), #reason }

If the DISABLE vs. DISABLED annoys you enough, feel free to tack on add a patch
to rename the enum itself.

> +	{ BIT(APICV_INHIBIT_REASON_HYPERV),		 "HYPERV" }, \
> +	{ BIT(APICV_INHIBIT_REASON_ABSENT),		 "ABSENT" }, \
> +	{ BIT(APICV_INHIBIT_REASON_BLOCKIRQ),		 "BLOCKIRQ" }, \
> +	{ BIT(APICV_INHIBIT_REASON_PHYSICAL_ID_ALIASED), "PHYS_ID_ALIASED" }, \
> +	{ BIT(APICV_INHIBIT_REASON_APIC_ID_MODIFIED),	 "APIC_ID_MOD" }, \
> +	{ BIT(APICV_INHIBIT_REASON_APIC_BASE_MODIFIED),	 "APIC_BASE_MOD" }, \
> +	{ BIT(APICV_INHIBIT_REASON_NESTED),		 "NESTED" }, \
> +	{ BIT(APICV_INHIBIT_REASON_IRQWIN),		 "IRQWIN" }, \
> +	{ BIT(APICV_INHIBIT_REASON_PIT_REINJ),		 "PIT_REINJ" }, \
> +	{ BIT(APICV_INHIBIT_REASON_SEV),		 "SEV" }, \
> +	{ BIT(APICV_INHIBIT_REASON_LOGICAL_ID_ALIASED),	 "LOG_ID_ALIASED" } \
> +
> +#define show_inhibit_reasons(inhibits) \

s/show/print, no reason to introduce yet more terminology.  And it probably makes
sense to use a fully verbose name, e.g. kvm_print_apicv_inhibit_reasons().

> +	__print_flags(inhibits, "|", APICV_INHIBIT_FLAGS)
> +
>  TRACE_EVENT(kvm_apicv_inhibit_changed,
>  	    TP_PROTO(int reason, bool set, unsigned long inhibits),
>  	    TP_ARGS(reason, set, inhibits),
> @@ -1388,9 +1409,12 @@ 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, __entry->inhibits,
> +		  __entry->inhibits ? " " : "",
> +		  __entry->inhibits ?
> +		  show_inhibit_reasons(__entry->inhibits) : "")

Use the inner macro to print the _entire_ sequence.  The fmt is obviously going
to be coupled with the macro regardless, and so having the macro handle everything
makes it a bit more obvious that the entirety of 0x%lx%s%s of being taken care of.

All in all, sans the potential s/DISABLE/DISABLED rename, this?  Compile tested only.

---
 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..8d9d9a0e9685 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..ea1ef901fb8c 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 apic_inhibits[] = { APICV_INHIBIT_REASONS };
+
+	BUILD_BUG_ON(ARRAY_SIZE(apic_inhibits) != NR_APICV_INHIBIT_REASONS);
+
 	if (set)
 		__set_bit(reason, inhibits);
 	else

base-commit: 04b1c6b4841dc319810db044e887219e05fae27f
--
diff mbox series

Patch

diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index b82e6ed4f024..8469e59dfce2 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -1372,6 +1372,27 @@  TRACE_EVENT(kvm_hv_stimer_cleanup,
 		  __entry->vcpu_id, __entry->timer_index)
 );
 
+/*
+ * The inhibit flags in this flag array must be kept in sync with the
+ * kvm_apicv_inhibit enum members in <asm/kvm_host.h>.
+ */
+#define APICV_INHIBIT_FLAGS \
+	{ BIT(APICV_INHIBIT_REASON_DISABLE),		 "DISABLED" }, \
+	{ BIT(APICV_INHIBIT_REASON_HYPERV),		 "HYPERV" }, \
+	{ BIT(APICV_INHIBIT_REASON_ABSENT),		 "ABSENT" }, \
+	{ BIT(APICV_INHIBIT_REASON_BLOCKIRQ),		 "BLOCKIRQ" }, \
+	{ BIT(APICV_INHIBIT_REASON_PHYSICAL_ID_ALIASED), "PHYS_ID_ALIASED" }, \
+	{ BIT(APICV_INHIBIT_REASON_APIC_ID_MODIFIED),	 "APIC_ID_MOD" }, \
+	{ BIT(APICV_INHIBIT_REASON_APIC_BASE_MODIFIED),	 "APIC_BASE_MOD" }, \
+	{ BIT(APICV_INHIBIT_REASON_NESTED),		 "NESTED" }, \
+	{ BIT(APICV_INHIBIT_REASON_IRQWIN),		 "IRQWIN" }, \
+	{ BIT(APICV_INHIBIT_REASON_PIT_REINJ),		 "PIT_REINJ" }, \
+	{ BIT(APICV_INHIBIT_REASON_SEV),		 "SEV" }, \
+	{ BIT(APICV_INHIBIT_REASON_LOGICAL_ID_ALIASED),	 "LOG_ID_ALIASED" } \
+
+#define show_inhibit_reasons(inhibits) \
+	__print_flags(inhibits, "|", APICV_INHIBIT_FLAGS)
+
 TRACE_EVENT(kvm_apicv_inhibit_changed,
 	    TP_PROTO(int reason, bool set, unsigned long inhibits),
 	    TP_ARGS(reason, set, inhibits),
@@ -1388,9 +1409,12 @@  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, __entry->inhibits,
+		  __entry->inhibits ? " " : "",
+		  __entry->inhibits ?
+		  show_inhibit_reasons(__entry->inhibits) : "")
 );
 
 TRACE_EVENT(kvm_apicv_accept_irq,