[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6555a4d8-7825-4078-b15b-4bd4b1a9d3f8@oracle.com>
Date: Tue, 26 Mar 2024 12:47:17 -0400
From: Alejandro Jimenez <alejandro.j.jimenez@...cle.com>
To: kvm@...r.kernel.org
Cc: seanjc@...gle.com, pbonzini@...hat.com, linux-kernel@...r.kernel.org,
joao.m.martins@...cle.com, boris.ostrovsky@...cle.com,
suravee.suthikulpanit@....com, mlevitsk@...hat.com
Subject: Re: [PATCH 1/1] KVM: x86: Print names of apicv inhibit reasons in
traces
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@...cle.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
Powered by blists - more mailing lists