Received-SPF: pass (google.com: domain of linux-kernel+bounces-147380-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <9785680f-1c30-456f-af49-b6d26dd38fdf@intel.com>
Date: Tue, 16 Apr 2024 11:24:16 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v19 086/130] KVM: TDX: restore debug store when TD exit
To: <isaku.yamahata@intel.com>, <kvm@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
CC: <isaku.yamahata@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>,
	<erdemaktas@google.com>, Sean Christopherson <seanjc@google.com>, Sagi Shahar
	<sagis@google.com>, Kai Huang <kai.huang@intel.com>, <chen.bo@intel.com>,
	<hang.yuan@intel.com>, <tina.zhang@intel.com>
References: <cover.1708933498.git.isaku.yamahata@intel.com>
 <b7a96654fa46593417afa6153959b2989a507842.1708933498.git.isaku.yamahata@intel.com>
Content-Language: en-US
From: Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <b7a96654fa46593417afa6153959b2989a507842.1708933498.git.isaku.yamahata@intel.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?SWFHVllIUmhsUjZ5ZGN6UWhCa21RODZyd3hsaFVqQkE5VTBIZ0V0M1J5aW9o?=
 =?utf-8?B?MTZzejBFdkVlVjh5TjNYdWEya0RmMlRRY1N0RSt1Wm1VUjVMMGpCSXhQM2dU?=
 =?utf-8?B?eFpqczhwOEg4V2k1Unp0dzI1MDNaRjh5cDF2NE8wK3Z2SG5IRHVtcUorSStE?=
 =?utf-8?B?dDhVeVFoRG1NNWlhelU4Tld6cjI0WWUrZUczT2VSWlcyc1M3WU04b0owMkU3?=
 =?utf-8?B?N29FOXlqTiszcTB0ZjcremUzdm1vQ2Qya25PY0VDRFU5Zm1pTUpYKzFIU1Ey?=
 =?utf-8?B?MHcwU25HbkZCd0d4a1ovT2RTdVByRExGaGhtMVBpcHc5RlRwT2RlL1JKMThJ?=
 =?utf-8?B?aGNZOTBIYzg2N0NxcHVGeUF2d2VYU1lxakt6VGowM3Joc29kbTlnSHpWcUd4?=
 =?utf-8?B?R0FhdzNKd3BhMGduQzNGd1RVRFRxbG1SNkVGMzdNMCt0OXVkNGhEbER0YkpD?=
 =?utf-8?B?cldNMHovRFZjSWdCY2lsN0czQmhUUEFPNWJFVTg4K2p4LzFnN0dvNlR5OFFG?=
 =?utf-8?B?U0FvVW1kS0hvSDdzRUpoaVJ0QU9vWGRXQWtYaUVGdFBJcGplUmVwVHJ3bm9W?=
 =?utf-8?B?RDF0RXpHSkxsd0tkOHRCWnFTc0VFUWU1RitPTDV3Y1RhdmZGREV1bUQrOHdx?=
 =?utf-8?B?bFBXS0o3S09kNUdLRjM5djdQN2xVM2RuVy9ZQWIrSEtqYkowcmZUcGtadWZp?=
 =?utf-8?B?VmxSTkljdHNEWFAxV3FXWGY1ejErclZNMUpoZ1VLR3Y5YkpvN3I4ZG9zQ0l5?=
 =?utf-8?B?WG03dWpXL2xBdDN0T3U2SFBEYVowZ3QvRlIxbkJ6UlJmYi9PUTB5bTFQR0hi?=
 =?utf-8?B?QzFIUVl6eVo3Szh0Zy9NUHVmcm9IUXh3YjEzclYrQ09QMFNiaEhBK0YwVERm?=
 =?utf-8?B?SUo2SGNMMHg2c09qWXRvT3VSWkQwZG9lL2JrK1B2ZzloSG1XNFpEUlgzb0NV?=
 =?utf-8?B?a3U1elFaTTUra1B1QWpidmU0aG9YUHhsMGgyQmo0SUs2ZVR1K0Q5bWUxaGVF?=
 =?utf-8?B?NW9keU9BQjFYNmhJTjhXQzR1SHJJdDhvRVlrSVRwSUpKa0xUMWU5eElUclhO?=
 =?utf-8?B?UkxLY0syY1R6anRyem9jMkN6aEFhK3lVdzE1dlBOMzJCczZxRWZSWUpoc2M3?=
 =?utf-8?B?VlR5RStrMHNWYk1PcnFqa2ZWMWUyVUl6YWd0NkVUQnQ3elQ4ZjVWbnJIcW04?=
 =?utf-8?B?dmk1QTFuSnQ5RmRMQVFEbjRyUzN1ZEY0ZWJHMlVCS1oyb1NJOVkzWVRINXZP?=
 =?utf-8?B?cFFVeVZMUmtmRHhQbTlOMDVCT0Nzdy9RemhPL0d0MzZzaDltbUdrYXJNdVp1?=
 =?utf-8?B?U21zcnRaSVozMnVCZGRvRmVRRm15Y3MxV3JFTHF3dUlEWnlGSndGUTM2YnFM?=
 =?utf-8?B?LzZ0cysvL29PanhuT25iQ1dpcDJ1cklrSlM2Rk82dFRVemxMenRpTFltamgx?=
 =?utf-8?B?R3hmL0RZTUF1aDEza3RCSEw4bU9jcHcyNW0wd0J4c1VQdjRWaGY0bk0xbFVx?=
 =?utf-8?B?TlhsT2V6QmJFTTc5K3VMZlBIS2YxRVp1VWtrT0FJWjJzTmR4eFBBME5nUHU5?=
 =?utf-8?B?QnlpdjA4am5Ddk1ENUhETzFBQVE2dk5QbjB3TzUxKzRWM1dFc3ZtWkJ4TXp3?=
 =?utf-8?B?MWd2RCtkK2htN2x3M3BsRHFBOWk3a0pZVXFleWgxUEhMeWNFZDMyVGxVVEw0?=
 =?utf-8?B?QlV3dzc2MkNhWWVuM1RLbEw3SHZYaU40TFRIVGZPOE9zVkpJdUJEOS9pUU4y?=
 =?utf-8?B?Uk8yTkNPTWYrMTFjK3Q3NjdZeG1XTXp4UWdKU3dTZG5nd2FWbVBEOGlKcitT?=
 =?utf-8?B?Z3M5alFwM2x1eGlCb2JVVzNFRjMrL29Ha1V1cTJGcWJ5ZFdxQm0rWDdTY2NB?=
 =?utf-8?B?VDA4T0cwTGRVWnhXQ0V4dTJabDdRSThtTnBRaDE2bWpwc0U4TmpqVzRQK0Iw?=
 =?utf-8?B?VDlGc3JqeHoxdEVEV09KTFBHd2NPWkgzVUZNMDN5VmdKYUd2blR6S25KUUlR?=
 =?utf-8?B?TUt3aC9BZHdyenVzbXBuQ2VwWEdGb3pRTFgwbCsxMzE0L2NNSWw5QmNydFFp?=
 =?utf-8?B?NXA4YzlDNTFHMCsrckhKbHl4bm56bjc5cFFXUEZja3pkc1ptRTBuN0tmWXk4?=
 =?utf-8?B?SEgzQmtvV3VhU1NLU2JzY0FiTnMrZERpQkgxeVp6ZkhpMUNacFRIV2dnYnRP?=
 =?utf-8?B?cHc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 8380b567-008b-4959-3b5f-08dc5e426dc6
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Apr 2024 18:24:18.3872
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: g4lgiZA6dDnK4s+7d2d0Wjm7yF3nz4wyhOD/AZF+JRKdfBSwxvzq+vZ4b6sKvOX4Uv9oD7D/6UWlZeENa+8bAsWDWMR7UBrm9sRZlSIQGFc=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR11MB4647
X-OriginatorOrg: intel.com

Hi Isaku,

On 2/26/2024 12:26 AM, isaku.yamahata@intel.com wrote:
> From: Isaku Yamahata <isaku.yamahata@intel.com>
> 
> Because debug store is clobbered, restore it on TD exit.
> 

I am trying to understand why this is needed and finding a few
places that seem to indicate that this is not needed.

For example, in the TX base spec, section 15.2 "On-TD Performance
Monitoring", subsection "15.2.1 Overview":
 * IA32_DS_AREA MSR is context-switched across TD entry and exit transitions.

To confirm I peeked at the TDX module code and found (if I understand this
correctly) that IA32_DS_AREA is saved on TD entry (see [1]) and restored on 
TD exit (see [2]).


> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/events/intel/ds.c | 1 +
>  arch/x86/kvm/vmx/tdx.c     | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
> index d49d661ec0a7..25670d8a485b 100644
> --- a/arch/x86/events/intel/ds.c
> +++ b/arch/x86/events/intel/ds.c
> @@ -2428,3 +2428,4 @@ void perf_restore_debug_store(void)
>  
>  	wrmsrl(MSR_IA32_DS_AREA, (unsigned long)ds);
>  }
> +EXPORT_SYMBOL_GPL(perf_restore_debug_store);
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index b8b168f74dfe..ad4d3d4eaf6c 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -665,6 +665,7 @@ fastpath_t tdx_vcpu_run(struct kvm_vcpu *vcpu)
>  	tdx_vcpu_enter_exit(tdx);
>  
>  	tdx_user_return_update_cache(vcpu);
> +	perf_restore_debug_store();
>  	tdx_restore_host_xsave_state(vcpu);
>  	tdx->host_state_need_restore = true;
>  

Reinette

[1] https://github.com/intel/tdx-module/blob/tdx_1.5/src/td_transitions/tdh_vp_enter.c#L719
[2] https://github.com/intel/tdx-module/blob/tdx_1.5/src/td_transitions/td_exit.c#L130