Received-SPF: pass (google.com: domain of linux-kernel+bounces-173126-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <b6a30d30-95af-465b-9cc1-867c7bfdc519@amd.com>
Date: Wed, 8 May 2024 16:50:49 +0530
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 1/2] KVM: x86: Print names of apicv inhibit reasons in
 traces
To: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>, kvm@vger.kernel.org,
 seanjc@google.com
Cc: 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: <20240506225321.3440701-1-alejandro.j.jimenez@oracle.com>
 <20240506225321.3440701-2-alejandro.j.jimenez@oracle.com>
Content-Language: en-US
From: Vasant Hegde <vasant.hegde@amd.com>
In-Reply-To: <20240506225321.3440701-2-alejandro.j.jimenez@oracle.com>
Content-Type: text/plain; charset=UTF-8
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?UjlnZzJkbCtkUFhCbE0xUE45STZlOS9BNnRPbDNpQXdyZjBxZ3ViTjJGT2NL?=
 =?utf-8?B?Y1E3RElMekgweWNMcFFLV2dNdll3WnZnZnNIaW9SWndVYWRUN2p4VVg2WmNt?=
 =?utf-8?B?bjQ4b1hiRWlBTm5SQU92U3o5T3BYVEdQdEdYbHRmSXlvUzZjODNGVktJK0sw?=
 =?utf-8?B?ZnhEWm05VWpYRmZRd01IZ25RWmFqajhUWFFHVmJlRCszZCt0UkFJY01uVlo0?=
 =?utf-8?B?bkhVWlZkNVZUWW5iN0NZZVkreG5oUW91czhnQ3BnQWE0WTVLUTBmZDJMUjN2?=
 =?utf-8?B?cFM0RWpJK29YZXNzVjBpcm80bTBxWXpoRGtBTUFFZEl6YWNtQ3ViV0JPd0Rz?=
 =?utf-8?B?bEE3VS8reG1rNEVmWUw2cGZLM0w3RXcvcTZOTU42eHZmNlhubzVOVS9md0VW?=
 =?utf-8?B?cGJxbFV6aDhsUWJlUkdGK2YzNzVyTS9seFFnKys5b3ZiNit2V1dXWUZvWUdr?=
 =?utf-8?B?dGJoMU5xSkFxMHFmOTlVSmJiN0lXVFBQVzM2UnJnMEtPNE00QktoZ1V0Yld3?=
 =?utf-8?B?cVd0MVJYc01DZGRVbDhwdGFWQkZ6UXFQblhlZitIa2ZTYTNmeFN2MnFhaW1R?=
 =?utf-8?B?OVl2aXM5VFNVa0p4QTZpL01JTCtjMTY3ZmdVaUpWdE5WWG1kYk5vSlVQTFFW?=
 =?utf-8?B?Y1dYbVFhVUs0MVhJa2dFdkh0OTBuNUxhbHZPZExlN0l6RmtQWWtiZkRtRnli?=
 =?utf-8?B?R0o3RC92MWU3WUR0b25tbWVSb1daSXZyM3UyaWhZckVuZkl4VStiNCtrdXZm?=
 =?utf-8?B?NnNNS1A1dStXWE9RVklEOERpdGlKTjArQVZpcm5TcE5XWi9HUGFmQWlzd29i?=
 =?utf-8?B?Mmd2TTVVZ09CZjc5THpxb1htUmdMdmhoeVFvUjU2ZVpCMlBpZitBdUF1RFhu?=
 =?utf-8?B?ci9UVmtzQnRWeXRDYTBTSzRhOEJSc1BEbU1sS2NORFh1VWwwSk9zZlA0VXdG?=
 =?utf-8?B?RmtuVnF3QzduWFlXdDk3TE8vcU1td09QRUJQTVVlMUliNWdxZ3VONEtzSmxS?=
 =?utf-8?B?SXNoSkZGd1IzMHdFK1hJYWtpRGJTams3cVVvS25oemp4ZTJ0eDVqMDB1SmxI?=
 =?utf-8?B?WjB4WjRwWFJRQWg1UnUyUTNqMFR0dzVjNlJwZnhvWDB4SVF3WXVWTjZ1RnEv?=
 =?utf-8?B?T2ZtdWl0ZElBTnhCUng1blNNZ3M3R3FJYmNzRmRBcHBDWXk2YUV2dmRPdEVC?=
 =?utf-8?B?NnpUZGxCaHl4ZjZWSGdYZW92N2ZCOUExTXp3Y1BoWnhqYmFzOHZ3c2x3TklK?=
 =?utf-8?B?VHJ1SUZaOGc1dVlJNzk1VC9hMXhtRVd6b3ZieUhrMzkybnRHNVZWM0Jhb05m?=
 =?utf-8?B?emZkRnlTWUhXTEVHaUJCQ0paM0hRc2hzOHBUU25oS0pSc2pGSnFhazNpZ0tG?=
 =?utf-8?B?TzdPbWdkZEZ4MXV2RXd6WXV4SElnNGxKOTZ5dXR0M0U0Y0c2Y0ZQaSt3TDF1?=
 =?utf-8?B?MTFLbStDMVlON1RzMGl1NG9Nek1YRFA2OE1WenNTbGFJL3p0aTkrMUpJeHBs?=
 =?utf-8?B?VXJKUzMvSkpVNStVTDFqWTRlZHZQdzdMTXBZVDQ5clQ0SkVYRStLNlNLeDVa?=
 =?utf-8?B?L1A1d2pQazF1MGVnMlNMaVkrZ3Rid1NiVk5SYXRDQnN3Yml4WUszcS9VUjJK?=
 =?utf-8?B?QjArTXFZOXN0T1lYSmNvZ2I4elFRUkFxdUhJZHc0RWhBazVyUGNmOVovYzlW?=
 =?utf-8?B?TmtXTGUvR2c4R1NQMW5LM1R4b2t4K1RUUmc4QSszRjY1bmxIMHVQR0FCeFpO?=
 =?utf-8?B?eVJXTWdGSzlVLzE4RU5NOE95NWdoeEJ1cXl4MDlIS1FEazlkSUVxaS83cG45?=
 =?utf-8?B?MzNvdUx4anlsa3dpV25aai80SXU5czZpdTRENFpyVVF2S21sT0FFaEdaOUZn?=
 =?utf-8?B?Wk1mVC9PMXNWZmFFekJNQjBobmc2am00ZTkwcFRvOXBuVGoxVlZqUTlaUytD?=
 =?utf-8?B?dlZXMmxHVi9sK21UMkJQU3NCVDRZSDFpaDRtNDJQQXBUWldtNFBNMHE5VHhw?=
 =?utf-8?B?ZFJzbEFBSUFKSE1peUVUUkJlQmQzSjJwL0IvNENDZTlTMDZFQVA4L2g2NzJG?=
 =?utf-8?B?ZkF2S2RHTmFsNGljRGYzaURGemtRQlZTQ1JJUlc4NGVnYm5IbTdPY2R2MEx3?=
 =?utf-8?Q?nasqppjeL5cIi8ZLvugreubcF?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 342df1a6-6639-4bf0-b88c-08dc6f50f0a3
X-MS-Exchange-CrossTenant-AuthSource: DS7PR12MB6048.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 May 2024 11:21:00.8895
 (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: Fzw2eozedeGG4QdDnTel/qrRqYb0JL0q3lFUqghovrqpaTQg0Wd74Tb1DGobGr8zfCyRTNLvhZOGZlSUcD6I4w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR12MB8060


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