Received-SPF: pass (google.com: domain of linux-kernel+bounces-188083-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: <7a7a94db-9019-44bb-95b0-dd3ef3560178@intel.com>
Date: Fri, 24 May 2024 10:42:50 +1200
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 5/6] KVM: x86: Unconditionally set l1tf_flush_l1d
 during vCPU load
To: Sean Christopherson <seanjc@google.com>, Marc Zyngier <maz@kernel.org>,
	Oliver Upton <oliver.upton@linux.dev>, Tianrui Zhao
	<zhaotianrui@loongson.cn>, Bibo Mao <maobibo@loongson.cn>, Huacai Chen
	<chenhuacai@kernel.org>, Michael Ellerman <mpe@ellerman.id.au>, Anup Patel
	<anup@brainfault.org>, Paul Walmsley <paul.walmsley@sifive.com>, "Palmer
 Dabbelt" <palmer@dabbelt.com>, Albert Ou <aou@eecs.berkeley.edu>, "Christian
 Borntraeger" <borntraeger@linux.ibm.com>, Janosch Frank
	<frankja@linux.ibm.com>, Claudio Imbrenda <imbrenda@linux.ibm.com>, "Paolo
 Bonzini" <pbonzini@redhat.com>
CC: <linux-arm-kernel@lists.infradead.org>, <kvmarm@lists.linux.dev>,
	<kvm@vger.kernel.org>, <loongarch@lists.linux.dev>,
	<linux-mips@vger.kernel.org>, <linuxppc-dev@lists.ozlabs.org>,
	<kvm-riscv@lists.infradead.org>, <linux-riscv@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>
References: <20240522014013.1672962-1-seanjc@google.com>
 <20240522014013.1672962-6-seanjc@google.com>
Content-Language: en-US
From: "Huang, Kai" <kai.huang@intel.com>
In-Reply-To: <20240522014013.1672962-6-seanjc@google.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?N3ZXb0h4bGs2KytsUXhVMWFPR25YVGFGZmp6bFJmMVhlWEY5eDU2aWZJTnE0?=
 =?utf-8?B?SzEzZGdGNUhMTHpRRlRBNTB5SWwwd0tSYnB6QmJYUzluNzIrUUNUMGlLakdR?=
 =?utf-8?B?enlDN3R6QTVHYlhQMUZzUFE4NjVHRkVlejdOVkpsbTJuRzJrcnJFL0VuaEJB?=
 =?utf-8?B?ZFJtNUsycnBZZ3FGejhzaUt4NGtDNXc4S2tacDRuNG1BMXQ1N3VBaVRxODFL?=
 =?utf-8?B?MlBqcG9sUEJhQkRRK0RPb1NwL2FOczJsdFI3MWpEWStBMEdHSlNzM3ZCdVRJ?=
 =?utf-8?B?TFQ3UFJpRGl1NEZRVk51R2hvaFNEOFNzZ2xqeFBKZkM5eUV4TC90Z05tNURl?=
 =?utf-8?B?c0R3NFVodVBsUmFYT2tXMWROcHhnZkJ0WXowa21Hd0VrSmgwUUptRXA3TkRt?=
 =?utf-8?B?aTlhQlk4djY2d1pnRDIwdnZHNmx1SHUvQktER2pNZVErMTE3RkQxWCsrNFlH?=
 =?utf-8?B?MWZWS2UvdHMwNFFqYmh1QW0yQWx5Q01Tc1k1M0dBOG1jRVhLdHdIdWIwOEky?=
 =?utf-8?B?MElxT2xVa1lka0Y5Z29QYkJkWU8vL2x5cHVNL0ZOdlAzQmJUZVFnOHB6c001?=
 =?utf-8?B?RXZGWVd1V0sydkpUNzBLam1jcTBBNVE0OTJFVGE3Rlp4WFBpVk9aQ3BEUnlK?=
 =?utf-8?B?NVR3RUU1b0h1dituYlRFRk9sUXpxeldoN2EyWHNNZllxWDhzMUxzQm1uS3dD?=
 =?utf-8?B?b2x6T2lXSmhaNHd2N0xSbFJ5Zzh0YTJVaFJiOS9rUGI0VXZDcUtUUStIS0Fm?=
 =?utf-8?B?V3BNSm1iU0hQalZ3U2pTYUNBbnllSGQxWkd3d1J5bFhpc3VFZnY0TUR1emUy?=
 =?utf-8?B?aDZpTGZ0cjEzV3FockovUnVVYUxxalFIcUZzM25LZ2lBTmc2NDRObW5nbkJB?=
 =?utf-8?B?dHpDU3lMUGl3S3gxWWQra0YrY21lK2paWTlmbEFYRVpQTk1JUEx2OVF5YzBX?=
 =?utf-8?B?OTNWcGhwUXJVc2RiVlhSV2xlL3p4T25IRm1mTURaVlJ3eHdTL0pDV1UyK2Za?=
 =?utf-8?B?Qk5vN05KQkVkWUtwdVo0blh4cHlQMzQzbG9DT2Z6QjF4eXo5YTd3b2ZjeURL?=
 =?utf-8?B?ang0VFBRR1RvNTNwOUwvTm5oVkdHbXBmS2dhN1ROY1BJaG9SQ011TVdZWmoy?=
 =?utf-8?B?a3U2cG1uRys4STN3ZktnWE42SW5NY2NQeTFPejNFOVpUVmhIbDBPamxDVFI2?=
 =?utf-8?B?eVkreWp1a0pwMHRKMDYva280bTFMZlltTVlBYkRRWTI4OVZEZ2RCWVhKRm5v?=
 =?utf-8?B?ZTF4Zi82TUp6UzNZdEZWT2NYdWpYK05GSXNDSCtVTHRvSFo4ejdsalZMb2JY?=
 =?utf-8?B?ejlhNmUzWUQzZTJZZVpqWEhrWC8wcjZZMC8wUzNjKzdlWTN3b0RtNWRjcU03?=
 =?utf-8?B?MHVJeDF6NDNIODFkWmNGd0tpOEtKVHY4Rzg0cmRhdjNaUTh2MUpGWncxbDBY?=
 =?utf-8?B?YW50MnFOZVN2ZnVIakU3SitQZGVHRFVWR2pZTDVNTVA2UktrU3dvWjZTbUho?=
 =?utf-8?B?b3A2RWNrUUphclZhL0p4ODlZeWsxWFladWZuWkhKWXJSTFAyUmxQN3RacXlL?=
 =?utf-8?B?aDF2RENuMTZCMWJaejVodVUzQW5uODN5SUszejRIek5EY0p3VTlyM2lteFd0?=
 =?utf-8?B?MkltNjhpTUhjaU4rSlVENzV2NDFLR3ZoNHlNSFcrQng5cUROWHUxNmFDL1Jw?=
 =?utf-8?B?NUY0RFdXNDBYbzh1TC9CMHJtZ1N5RXBqdjZCdWVtYStwczlEK0dCUlFvOGRv?=
 =?utf-8?B?V2hHOEZkNkZIQ2VmU3UvTnRUQitVTjI1MGgzaU1ydnZWZ3hJdmo2TVZCc052?=
 =?utf-8?B?d1NFdzVZcFN2QkQ1N1pjUXpxUnd1NGM4VGgzdDRHd2o4Q2xOdW1vK1BaQS9B?=
 =?utf-8?B?c1FXY1RPMEd0QkoxRHhGcVZUdkpUMmtudGd0dStXWjZEZldjclN3Mm9ERVRw?=
 =?utf-8?B?U3pzYUh5dm5YbS83cWxHYzBBdzAwQ3ZQUGFaSzkwL293cUVaZTRXZCtORUtx?=
 =?utf-8?B?ZEk4TkZqby8wdER5RVdSaTZkdnFxZklDUzI4MWRaZUlka1NXMVNObVlBVkQ0?=
 =?utf-8?B?N3VnVi9mMit2dVU2eFhQUWJxOGl1WXBvZG1OZS9LWTE4c3VLOHdRb3ZQVnFy?=
 =?utf-8?Q?A0+D+llZK6ckhQQ0hQWfwRBR8?=
X-MS-Exchange-CrossTenant-Network-Message-Id: be9cd3fe-8843-49aa-7d96-08dc7b79b472
X-MS-Exchange-CrossTenant-AuthSource: BL1PR11MB5978.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 May 2024 22:43:03.0290
 (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: YUtgevdcadRemJRGGODK62Pb1RDpA/XzcxctB8TajOuZi8EiUC2y5fRBXLouBXNHWGE+mIaLVHRAeI5z2TkJHQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR11MB5170
X-OriginatorOrg: intel.com


On 22/05/2024 1:40 pm, Sean Christopherson wrote:
> Always set l1tf_flush_l1d during kvm_arch_vcpu_load() instead of setting
> it only when the vCPU is being scheduled back in.  The flag is processed
> only when VM-Enter is imminent, and KVM obviously needs to load the vCPU
> before VM-Enter, so attempting to precisely set l1tf_flush_l1d provides no
> meaningful value.  I.e. the flag _will_ be set either way, it's simply a
> matter of when.

Seems reasonable.

> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---

Acked-by: Kai Huang <kai.huang@intel.com>

>   arch/x86/kvm/x86.c | 11 +++++------
>   1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 59aa772af755..60fea297f91f 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -5006,12 +5006,11 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
>   {
>   	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
>   
> -	if (vcpu->scheduled_out) {
> -		vcpu->arch.l1tf_flush_l1d = true;
> -		if (pmu->version && unlikely(pmu->event_count)) {
> -			pmu->need_cleanup = true;
> -			kvm_make_request(KVM_REQ_PMU, vcpu);
> -		}
> +	vcpu->arch.l1tf_flush_l1d = true;
> +
> +	if (vcpu->scheduled_out && pmu->version && pmu->event_count) {
> +		pmu->need_cleanup = true;
> +		kvm_make_request(KVM_REQ_PMU, vcpu);
>   	}

Nit, the unlikely() is lost, but I guess it is OK?