Received-SPF: pass (google.com: domain of linux-kernel+bounces-122570-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Thu, 28 Mar 2024 16:12:36 +0800
From: Chao Gao <chao.gao@intel.com>
To: <isaku.yamahata@intel.com>
CC: <kvm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<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>
Subject: Re: [PATCH v19 091/130] KVM: TDX: remove use of struct vcpu_vmx from
 posted_interrupt.c
Message-ID: <ZgUmdIM67dybDTCn@chao-email>
References: <cover.1708933498.git.isaku.yamahata@intel.com>
 <6c7774a44515d6787c9512cb05c3b305e9b5855c.1708933498.git.isaku.yamahata@intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <6c7774a44515d6787c9512cb05c3b305e9b5855c.1708933498.git.isaku.yamahata@intel.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?MQzZmwKgoDIw35b+jtUD6ksnoNq9U+/uB23DLF9tKckR1mg30vRDJpr8kcyO?=
 =?us-ascii?Q?oyhOtoEjZA0yhB53J1y8ipByFexTW42a2xukroDyT0d5KE7d/AffXS1NadmS?=
 =?us-ascii?Q?ae2XzJKBSrtKq2BTBimA0NOmHMeuXIyW14PDayVGiyk4lDYnC+2hjbbXKXom?=
 =?us-ascii?Q?v85O67yBNpwg933kPAFu3G4Pmg0mEVacx3dr+6FZ4t6TZZF53ZHA0P3unQQx?=
 =?us-ascii?Q?lBcpQb5ppV6RjwTfTZWlrWRxA6MzMt/EYfUa9h6C3q9cki8fbcyuy/3ZA8Mx?=
 =?us-ascii?Q?TyM/cypvUs9XdptCm4bY9drE+cjFBQnDMTeMftHCcdLYaYosvHyaTtsLA/TH?=
 =?us-ascii?Q?kfXLxr3i05oQsaj9CVU4NIPR7AxSB5uonWpSlt1H8mFx51PwCIZDUP3zUJFD?=
 =?us-ascii?Q?nNHNDzSPhAVfzNR1Nh8GwtZoTvhsU+b3lNsg+4Gykptv9e/aZ0cVqXqh0aQD?=
 =?us-ascii?Q?X6qclm4RUjIybM8EsBZXUA+vjCgI0dBER5tr3TDeZl75D0JdmkFIBoirgNWp?=
 =?us-ascii?Q?GBZ8q8us5ifpHSY8POSnM1SD83qlnsPiWK0r+j89QcKoHO0cj7R+S2JNcYDN?=
 =?us-ascii?Q?7PTtNXGcCZaCRRo0lXTgK/8WYCE2tPoVxeoOVyEil50pu1GKi/WUAKY6BWQk?=
 =?us-ascii?Q?iOND30o7TmSXVF5YIYvKavnHJ9fhvyApru4+WBe1yMynXoyOuxIx0GkC7cGD?=
 =?us-ascii?Q?nDPLpjxAXC2cv9IcmvF+5jBYvMjmJ6yykBeUpRp4376sHx8GhSymZo7gE2km?=
 =?us-ascii?Q?C6VVC4czruRdQ/392l7qVP9/zCkLHDSHTHPmOdspAJyOy3E3UklKvpf5D2oG?=
 =?us-ascii?Q?RWoGsWceXEmE3s2Pa7SUGfQD+Lhi2Vkojbszbn2LaTTaCksC4OPJ2iSc1wXi?=
 =?us-ascii?Q?sgqbe3aIfT7K/pUsY2M1cpvzubQnoZyO1VPqrfVh1urlCmkXKUhpK/jfHb8F?=
 =?us-ascii?Q?OeWMsIXTD2L++DyjGDCq6sjBia+WkTxhMPe4Tf2FApPKM3P987PNRmdpGoP+?=
 =?us-ascii?Q?RGNU7fV4VTivUJH/7RmZClxMkGMPrUhhflR2H5t7y/dBbACJqiYx9E9HOT73?=
 =?us-ascii?Q?OKTa/NCYEEnMrYZzETkuwDF41jLb1Bn3dQyTVumNOfrTH1auz/yeTd3HoPtO?=
 =?us-ascii?Q?OnoH/rH14rYfLsFFl4nZ1rWpqewdY5lBdKzzPuSU2AFecxiWq98mO8iJnYy9?=
 =?us-ascii?Q?yXG0Fp6TOtsfgeT9AsllSi7PEt8tcVqsn+IT2lVC9ZUNOmXpTrao2E9BHhvy?=
 =?us-ascii?Q?LC960IjC2IauZAF6wDlTbfhfGpm03R8rAPhi2vvGbG2mkdppj3wCkyc7bIAS?=
 =?us-ascii?Q?GwIy3P5NA86qATqMf8WWZUjB6jWJPrM3uWXc+QWcewwKxIGDqID8TJdNOHkE?=
 =?us-ascii?Q?BNj8dHy4Y/rXwH7jgLIZhv3Nlrnsha+gvgJIfY+MmqP8h/I4HYm/8OWgEuCB?=
 =?us-ascii?Q?4nCchd8oPewxir/nbtrUMzL7pf3HCWiKO9wmJ00UwVpbwaSPUNl1+bJ9FB41?=
 =?us-ascii?Q?urd2YH6m/ctoIIkIfrQcFp/xF3cCUp2sKO/lQSkj4XSgrAScVp21kpi5ETmz?=
 =?us-ascii?Q?X5xXpFvcs7gbx6GdS+fXfUK0bMw8yLmHonJMRXq4?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 1f90f63b-1350-4a8e-ad00-08dc4efed9b1
X-MS-Exchange-CrossTenant-AuthSource: CH3PR11MB8660.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Mar 2024 08:12:46.2097
 (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: i7soK+zwdkOLf1hX2577do7+VAl+bQcXDJC9C0W7F8yKZgdY5nAY8HEHZqnVMUOou4EXlgd1LJd+M/2assGxtw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB5996
X-OriginatorOrg: intel.com

On Mon, Feb 26, 2024 at 12:26:33AM -0800, isaku.yamahata@intel.com wrote:
>@@ -190,7 +211,8 @@ static bool vmx_needs_pi_wakeup(struct kvm_vcpu *vcpu)
> 	 * notification vector is switched to the one that calls
> 	 * back to the pi_wakeup_handler() function.
> 	 */
>-	return vmx_can_use_ipiv(vcpu) || vmx_can_use_vtd_pi(vcpu->kvm);
>+	return (vmx_can_use_ipiv(vcpu) && !is_td_vcpu(vcpu)) ||
>+		vmx_can_use_vtd_pi(vcpu->kvm);

It is better to separate this functional change from the code refactoring.

> }
> 
> void vmx_vcpu_pi_put(struct kvm_vcpu *vcpu)
>@@ -200,7 +222,8 @@ void vmx_vcpu_pi_put(struct kvm_vcpu *vcpu)
> 	if (!vmx_needs_pi_wakeup(vcpu))
> 		return;
> 
>-	if (kvm_vcpu_is_blocking(vcpu) && !vmx_interrupt_blocked(vcpu))
>+	if (kvm_vcpu_is_blocking(vcpu) &&
>+	    (is_td_vcpu(vcpu) || !vmx_interrupt_blocked(vcpu)))

Ditto.

This looks incorrect to me. here we assume interrupt is always enabled for TD.
But on TDVMCALL(HLT), the guest tells KVM if hlt is called with interrupt
disabled. KVM can just check that interrupt status passed from the guest.

> 		pi_enable_wakeup_handler(vcpu);
>