Received-SPF: pass (google.com: domain of linux-kernel+bounces-135058-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <39de903d-384c-4e4d-b020-3f9f0c7c2c0e@amd.com>
Date: Mon, 8 Apr 2024 14:13:14 +0530
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 1/1] KVM: x86: Print names of apicv inhibit reasons in
 traces
To: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>, kvm@vger.kernel.org
Cc: seanjc@google.com, pbonzini@redhat.com, linux-kernel@vger.kernel.org,
 joao.m.martins@oracle.com, boris.ostrovsky@oracle.com,
 suravee.suthikulpanit@amd.com, mlevitsk@redhat.com
References: <20240214223554.1033154-1-alejandro.j.jimenez@oracle.com>
Content-Language: en-US
From: Vasant Hegde <vashegde@amd.com>
In-Reply-To: <20240214223554.1033154-1-alejandro.j.jimenez@oracle.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?ZlhrNWE2U2kvVkpoQnRhc1RlM3NsNTN4NzgydXMxeHBWVkxLVzNBdVV5c0F5?=
 =?utf-8?B?OGRKZDVLUkJMT3B0cys1UEpXNW9weWt1YnRFVTdsZjBDeTA4dU5XMllRaWhG?=
 =?utf-8?B?bHlaY3gzUnhBamMzRi8wMG5NVVpnNk9LLzVQKzBIRlFjeTUvVmpPcDk1WVEr?=
 =?utf-8?B?TUo2RUt6Rm9ESS82TzhtMG1NQ1NrbnZ0WjlMRTJsRmJuYmJHUlRuWDRGbUlI?=
 =?utf-8?B?UjRGbnNaWE16QVgrVUk3eUpWbXhZUWZRRnR0UlZVTDZpL1kzV1FheEk3N1FQ?=
 =?utf-8?B?R0NOWkViTGhLSHo2N2J1Si9TY2lmMGxTL2JrQTJBeVBvaEd0disyaU9lUHEy?=
 =?utf-8?B?K3lQTVM5N2ZseXFzb0JiWW91R1Y5L0JQcVUvYzJudDZENy9mWW1oSjZadFVW?=
 =?utf-8?B?SWJnMklHMkFZUlVmVklpYWVUWXNnV29IRzlTK3hCRWpXRUVQS1ppYWdTdC9B?=
 =?utf-8?B?SUlNWDY0eW5Vemg5U1FGUlZURjF4MzhuRTByR1RsaWhDRGwwelhsdkNuSXk3?=
 =?utf-8?B?T3pKTXYrSzZvVitrZDRDcmR5aTUzZ04vUitRY2lUNVpNV1hwTHQ3RUtTWngx?=
 =?utf-8?B?YmdkRWltRzBaa2wzeHB4ZHM2aEV0cDBVUHJ3dEtUM0Y4dENvbVQ4U0J3cWpy?=
 =?utf-8?B?d2VLNHVLamJWK3JvZ1YvQzNPa1B3WUthSkpURHR4VXZpY3JMekFiWE9oQ1h2?=
 =?utf-8?B?aW9OaG9LNWxhbWtQUGFjYjdaWGRvdGszbTk0eXZSR1NrYVc4NmhQVXc3WjBs?=
 =?utf-8?B?VzMyUkFnRWd4aUh3Sm5INlk1aUtQNjJQMVVETk9aOGNvNXBTbFQzbmRQdHZV?=
 =?utf-8?B?M3R4SVJSTEVROHZVMG1IUTlkNzAwV2l2VVJyQ2QxWnRiWG1nQ2c4eSs4NWZv?=
 =?utf-8?B?SmJVdFdJUkN3Z0hncXBxOHNGa3RkZ3RmeWw4blZ6emhoN25Ic1BKUGJXOEFu?=
 =?utf-8?B?RUtYZEdCSGErUEJMbVVjSjFLeXJ6eWdEWDA2ZitmaG5mdmRnWFJrZStKd0Nq?=
 =?utf-8?B?RzQ5SVhvVFFlK0dZWFZWUXUvcHczZDhUMEp6cVJrS2hDdlZBd2xFalZ0RzNw?=
 =?utf-8?B?TFNVUTEvQ01iMFhhTE1EcDR6WmtjUEl6cnVjNElVazRZdTFnS2hJVE52VGhh?=
 =?utf-8?B?VlZzN1pxL1F0d25WUi9hT21qalAzREp4aDVoL0NzQmZXbnVrcHVMbTgydVpz?=
 =?utf-8?B?UTQzT1VWNVFabEIvc01makdyWWxIb2sxWGlsUE1sSitZYXdmLzIzYVNKMkxV?=
 =?utf-8?B?cW9aUFJpU0Rvbm9KRkxNMUlCK2l0LzFvcjFQL3VIYktOVTNLM0pmNUREcFlS?=
 =?utf-8?B?Y3JqajRNM3IyVStJNDdIRjFRbGJwUmhvYTNGTjBQUERGTFgxQ3g1WXFacUN5?=
 =?utf-8?B?T1dPM2VLbWRJVEpGMXJtbkpDUG1SVlB0SlBLeDhudEMrdTBiRkN5THhJQ0Z3?=
 =?utf-8?B?S0JSVlZKR0NKd2UybWQwb0ZTZHFobmJBaVVlcXgrMFZhdUFHUVJORTBRak9Q?=
 =?utf-8?B?N0pKbHlFZmdzM0hzTGs2TGJ1anlJamR5M1l5SFZHbVNZY0NzN2hUdWdVWFFY?=
 =?utf-8?B?L3pONGVkdHE3cjhYZ2lNQVozd2lleVlqOEhLck5EU3UzcFloSmNNUFpPZDg1?=
 =?utf-8?B?clR3bFJ2UExoUVVoalkvdHphZXJkS0Z6cncwcVVWeGdhZFVsTmJ2ZC9DRm9o?=
 =?utf-8?B?NkF2T0NYSThXeXo5RW85bXNMVDdvSUkvQ2NrUTFjTHBxWU1kUEttbjd6S29N?=
 =?utf-8?B?VnltUUs5aytMMGlnZ1dRQVlLeFdEVzBhQ1M5S3FjcHNTUUlMQ1VYTGJua29q?=
 =?utf-8?B?R1lBZ2h1ODRzbWhNOGtOREFaSnBRN0FHbjNBbG0rc2t3c1dJd3YranpOK3p3?=
 =?utf-8?B?SHowbkQwUnNESlNpaURHZFZsVkdLL2FlWnlDbllBMUhsQWdOUG93cWxjMG5P?=
 =?utf-8?B?dThGRUlCTzNuZklRZWFjTjlsTHkyZU5sbVROdjFvOW1lSEMxRGdyS3NPY1NL?=
 =?utf-8?B?cWFQMWY2bnFULzd3QVZJMTBMR2lZblZaeWhjekkwTEloeHozcWdxRU42aVdn?=
 =?utf-8?B?SVhHUnU2QkI4UExQeUY1OXJCZWR4M0VCUm9HdkVZVENLd21RZi9KWEdSRFdo?=
 =?utf-8?Q?Kp0BAlR+TlTBqqLdyzXDOgr2M?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 0fda206d-8b9e-4f93-fa18-08dc57a7f345
X-MS-Exchange-CrossTenant-AuthSource: DS7PR12MB6048.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Apr 2024 08:43:23.8328
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: X96r2EOU4A+AeGlZl0DFAV+adzwE1ANkQGufuy3Ou3FdsudSFuAzfyS5/NJ1TE91QRyvrzR8kemojBtf9Cdnvw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ2PR12MB8012

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