Received-SPF: pass (google.com: domain of linux-kernel+bounces-175475-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <0867c527-6fe5-4f54-adcc-0344e9416f9b@intel.com>
Date: Fri, 10 May 2024 17:03:28 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 13/15] KVM: x86: Kill cur_tsc_{nsec,offset,write}
 fields
To: David Woodhouse <dwmw2@infradead.org>, <kvm@vger.kernel.org>
CC: Paolo Bonzini <pbonzini@redhat.com>, Jonathan Corbet <corbet@lwn.net>,
	Sean Christopherson <seanjc@google.com>, Thomas Gleixner
	<tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, Borislav Petkov
	<bp@alien8.de>, Dave Hansen <dave.hansen@linux.intel.com>, <x86@kernel.org>,
	"H. Peter Anvin" <hpa@zytor.com>, Paul Durrant <paul@xen.org>, Shuah Khan
	<shuah@kernel.org>, <linux-doc@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <linux-kselftest@vger.kernel.org>, "Oliver
 Upton" <oliver.upton@linux.dev>, Marcelo Tosatti <mtosatti@redhat.com>,
	<jalliste@amazon.co.uk>, <sveith@amazon.de>, <zide.chen@intel.com>, "Dongli
 Zhang" <dongli.zhang@oracle.com>
References: <20240427111929.9600-1-dwmw2@infradead.org>
 <20240427111929.9600-14-dwmw2@infradead.org>
From: Chenyi Qiang <chenyi.qiang@intel.com>
Content-Language: en-US
In-Reply-To: <20240427111929.9600-14-dwmw2@infradead.org>
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?azJZbXkzYzNuRDZBVnJGaWNYRG5YcjhDQ0Z2UU9uUjR5bFdSMDVYR1lyVkx4?=
 =?utf-8?B?OXlNdzFDODNhaHhFQVFVang0ODFmS2hPbFhaSEU0ZjNVUHpCMWxPZ1l2N3JF?=
 =?utf-8?B?NFk3VGIyOFZHN2hiZFA2cHdaWkJnenFTLzRCU1JONm55NkpXaU5XTGpKVE1z?=
 =?utf-8?B?RjgrYlN1QjREakhmOUQzK3M3cWJsNTBzamlrZm1MRTVOb0NDN2c0TEdrVkpM?=
 =?utf-8?B?STVBWklySTI1VTAxbkpCY0FBZXZsYVFwUmIzTnprVzVLbnAvZTdnZ2hGTlFa?=
 =?utf-8?B?KzZJRndMVjJldDkxTVhYMGZVNkhTSG5kT2w3VjJkZ3RnRlB2bmpvZG54QjU5?=
 =?utf-8?B?TlJXMEpjVURHc2ttOWNBejM5RTBwNjR2ZSsrWVVHLzhlQms1Ymt2OTQxMllL?=
 =?utf-8?B?MGFOaUxJdzFTUVpWWlMya0EwK2dvSXNvSEVIamY0WUhBVWVtemVaTXRXSkZR?=
 =?utf-8?B?SlBrbm9hZnpYeWFsK3lrbWcrQXJnZUVmcXVPYXplV241SU0yd2JXQWxRT0JC?=
 =?utf-8?B?dmV2RjFhN0pDNzlTU3MxcktkRy84L2QwMk40WFF6R0lCR1VQUTY0QUxQK2Vr?=
 =?utf-8?B?QzMwNnRTL0VMSldjZWEvajlEQmx5dGM3bTNxMlV5S2xXUUJRNGFsTGQ1aHpv?=
 =?utf-8?B?anBRNU95WG9xR0xXOVBWM1VsRzIxendYMzVZYnhIeHQySkFtYlFFV0dsN2Jp?=
 =?utf-8?B?aVlXZE9hdE5mUUVtMUhkZlhjS2ZNVHpFMGVzSHZFWFR3LzRTNXdjeFJDVGc1?=
 =?utf-8?B?L2N6Ui93akhscXRXTHlpekp4NWpLbEIxODVWVjlDUktvL1ZXd05XazhVMm5v?=
 =?utf-8?B?dVQvNnFDMVh0cWUwc3dDc1BxNkRSYVlWTE4vSWdQZW8zWGJUdkhNNldYMW5u?=
 =?utf-8?B?Rm42MTFUQ1d3bkYrWHF5c1JjZlBpdHpiQVFwL1Vjb2ZRNlJWUldHNmNyWTFk?=
 =?utf-8?B?eXlqLytpcWdlNjEyZXFmaVZkUWVEVVhZWFEzQkVVYU5UdzFmWmV3UVJBbjNi?=
 =?utf-8?B?VTk3ZVZlR1crME9rQ0pNM0ZwZVBDaGw1cHNYRzJIL3JoKzJoVWNkZDhBY3dB?=
 =?utf-8?B?Q3RPcDRzU2FYeDNWL0RNRWhHbm5XaFJ4UzRiNFFRVUcwWEFtRnlKS1V4SHk3?=
 =?utf-8?B?Zi9ublJQRFhkV2FwaFVtVHVFUUJlMmQrdXg3YTVlN2Fqb3RwRjZ1WE4rYzI5?=
 =?utf-8?B?dVpXdGdnRHpYTGlqcnI3a2JjdU00a1FaN28yV0dWeitxck1oY2VlUVhqekhV?=
 =?utf-8?B?UGVwSE9sbldrY0NreVpZNXVnbHppbVRmTWpsSzdSOVVNWHdad0xvZG9JYlB2?=
 =?utf-8?B?UDJPT0ZmWUJMM2FXanUzRU1jcDZqam01WjB6ekdDalVSczJ6Y1AzeXlkR2V5?=
 =?utf-8?B?Mk9YbE5KaHFlRDUyVURtV3RTM0Q4NzN2Y0NFUzZwL1hxK0trQjZuWExCRm1I?=
 =?utf-8?B?cHBoYXRNZHR4Uk1FcXB3RHFzQzFSVnM2V1JkWXRWd0VPQm82dEQwYk8xVmlB?=
 =?utf-8?B?eFEvdGVCYWlIUUFPelFGYS8xT2hSZ2Z6Q2NRM0dtUWlzZ0paTGJVUEVyM1Ar?=
 =?utf-8?B?dGdSRkcyS09GaUw0djROYU5Zd0lhSVh2WVVrbks4cElCOXVoSERseVhsQWRm?=
 =?utf-8?B?aHFUekVGRHFkK1ljNlVoWmlzQ3c4eGxSQlFJR1NIcStJTUJnT3NETUZHWVZw?=
 =?utf-8?B?SXBUTmM0M0gxVklWS3VMTkI5YmRFZ0F3YldlZklGWllWQ3VqSDM2Rmg2S2VU?=
 =?utf-8?B?Q2hoSmFDbklvTXZ0MjJTL1l0Y1Y4U1ZKM044SThneVV5TTBBUXVkZWFUa2FI?=
 =?utf-8?B?bGhhMGczMWJ3TXN3bW96d1RMbmlJd0tzYWsvN1VnK2hwcll0TVJXek9icDlT?=
 =?utf-8?B?cWQ2MEM4RUdlSTZIeGtueWRLV1Jpem5uV3gyUzFjWHNTdVBjQ2JRbGE3Tlhm?=
 =?utf-8?B?T1lHdW1iT0Rhd0Y0dlhmZDBNODlkTVBqQURXMVVhbUFOSnpMaHFIZlp2NDQ1?=
 =?utf-8?B?S0VTZUl5NEloSnBWMjFQY1RIbk9jWVFCVUM3MXRxdWxMeXpoV2l0WVNJZmJ3?=
 =?utf-8?B?MEMyaTl6SUNUTHZNZWQ1MElxVmtKZ2ZZZjhQbG1vOVBVdksrRVJqdFZBQWo4?=
 =?utf-8?B?dGRHb0hkMTE5eEFjQnBaR3NCNDVvdlJNd1F4RUxXRkxkQzVNRWtTRjlCMXRi?=
 =?utf-8?B?THc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: aa05751d-8d43-4a6c-63c1-08dc70d01664
X-MS-Exchange-CrossTenant-AuthSource: DM3PR11MB8735.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 May 2024 09:03:41.3817
 (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: N5RlBJjT0YySIZK15/TUEsfExtsIzhjAyVVLWWW3ZrwIaSIdOKzGQw0QLXDPydd0OJ31du/HKIL16kiXoHybBw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA3PR11MB8047
X-OriginatorOrg: intel.com


On 4/27/2024 7:05 PM, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
> 
> These pointlessly duplicate of the last_tsc_{nsec,offset,write} values.
> 
> The only place they were used was where the TSC is stable and a new vCPU
> is being synchronized to the previous setting, in which case the 'last_'
> value is definitely identical.
> 
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
>  arch/x86/include/asm/kvm_host.h | 3 ---
>  arch/x86/kvm/x86.c              | 9 ++-------
>  2 files changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index b01c1d000fff..7d06f389a607 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1354,9 +1354,6 @@ struct kvm_arch {
>  	u32 last_tsc_khz;
>  	u64 last_tsc_offset;
>  	u64 last_tsc_scaling_ratio;
> -	u64 cur_tsc_nsec;
> -	u64 cur_tsc_write;
> -	u64 cur_tsc_offset;
>  	u64 cur_tsc_generation;
>  	int nr_vcpus_matched_tsc;
>  
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 6ec43f39bdb0..92e81bfca25a 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2737,9 +2737,6 @@ static void __kvm_synchronize_tsc(struct kvm_vcpu *vcpu, u64 offset, u64 tsc,
>  		 * These values are tracked in kvm->arch.cur_xxx variables.
>  		 */
>  		kvm->arch.cur_tsc_generation++;
> -		kvm->arch.cur_tsc_nsec = ns;
> -		kvm->arch.cur_tsc_write = tsc;
> -		kvm->arch.cur_tsc_offset = offset;
>  		kvm->arch.nr_vcpus_matched_tsc = 0;
>  	} else if (vcpu->arch.this_tsc_generation != kvm->arch.cur_tsc_generation) {
>  		kvm->arch.nr_vcpus_matched_tsc++;
> @@ -2747,8 +2744,6 @@ static void __kvm_synchronize_tsc(struct kvm_vcpu *vcpu, u64 offset, u64 tsc,
>  
>  	/* Keep track of which generation this VCPU has synchronized to */
>  	vcpu->arch.this_tsc_generation = kvm->arch.cur_tsc_generation;
> -	vcpu->arch.this_tsc_nsec = kvm->arch.cur_tsc_nsec;
> -	vcpu->arch.this_tsc_write = kvm->arch.cur_tsc_write;

Do we need to track vcpu->arch.this_tsc_nsec/this_tsc_write? At least
they are still used in compute_guest_tsc() to calculate the guest tsc.

>  
>  	kvm_track_tsc_matching(vcpu);
>  }
> @@ -2825,8 +2820,8 @@ static void kvm_synchronize_tsc(struct kvm_vcpu *vcpu, u64 *user_value)
>  		data = kvm->arch.last_tsc_write;
>  
>  		if (!kvm_check_tsc_unstable()) {
> -			offset = kvm->arch.cur_tsc_offset;
> -			ns = kvm->arch.cur_tsc_nsec;
> +			offset = kvm->arch.last_tsc_offset;
> +			ns = kvm->arch.last_tsc_nsec;
>  		} else {
>  			/*
>  			 * ... unless the TSC is unstable and has to be