Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 28 Sep 2022 19:26:18 +0800
From:   Chao Gao <chao.gao@intel.com>
To:     Sean Christopherson <seanjc@google.com>
CC:     <kvm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <jon@nutanix.com>, <kevin.tian@intel.com>,
        Paolo Bonzini <pbonzini@redhat.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>
Subject: Re: [RFC v2] KVM: x86/vmx: Suppress posted interrupt notification
 when CPU is in host
Message-ID: <YzQvWhr9nAeesRKy@gao-cwp>
References: <20220923085806.384344-1-chao.gao@intel.com>
 <YzHRKO1v5N/BIQl6@google.com>
 <YzKZExaU2k7qfcS9@gao-cwp>
 <YzNubGQf0yvODjZC@google.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <YzNubGQf0yvODjZC@google.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?MTyUFtPkQ3VLzpD8bGpnP/dKm+ECp0dTfDsZml1DaTxeVBsLpsn/j5fsYQaL?=
 =?us-ascii?Q?/cDE/oXe0pHJnaNPcBb6m6LtkHhYbI/VoVe8u/ihXDq5J0elKxJetNhekj3o?=
 =?us-ascii?Q?IC3eozWD9KFhsp/yxzLnjcIn9HKvkv/0EoFLZShHH69CqZbGYc/rFVIGFwhH?=
 =?us-ascii?Q?LobnNaf6U2zouaKZ2aNeEZCFqLI18aOGzUwgeWo847NqO24I839ZQoTRUByr?=
 =?us-ascii?Q?SY/r0VxN5kgGQZ9Wf4LFxQVkAKZGaAX3S99Tq8CBS3h/9AiIlVCli9/BdduT?=
 =?us-ascii?Q?0pGbVaaf0WaaUgloN8+jYoxNJoxZJcmSAFEKL51xQ6XVKBtyF+L2gcctZRsV?=
 =?us-ascii?Q?R427z7JH+vKwRpGfk/A6poLgfOlv6vM9mAp07GQf5ONSOf4xWt6UEMJuGVbV?=
 =?us-ascii?Q?90cVKfb8TdENqgwRQiiSUjOf0NlZnbd1iQPgvOccWxfYpRWdUJGFZGuG0hoH?=
 =?us-ascii?Q?lqmjNUfelDqXqyibccC4H7AwIw0uQLG7X1lgElYGVPD5yC6l+aNHu6Ap01TP?=
 =?us-ascii?Q?L/66k9Jv4UlLpwLwbH2SFAvdAPsuC7HTFvt8srMZwchIkuGn9K4fdox25H16?=
 =?us-ascii?Q?raHgANXXiUlsSBilBWw5DfkJMCnepdjZqOpMB/+m5VMwOsX3QpwUvNOapRCC?=
 =?us-ascii?Q?yqF+FYnoCzOrhZltSNXOLqQ7mETBgHmhN8jgQjDf+znW/lk/Z3g13seroW1W?=
 =?us-ascii?Q?nXEQ/eVEb6aD5GhgmUewhv2WitfFRplvvYplsI1BtUqgLR93dwVZ+470udD6?=
 =?us-ascii?Q?KZ7W6KoUMHKdHPmJnew+qgr2Eavso3ld6WG0wGSRs52WcPU1mlJvxV0JEZi3?=
 =?us-ascii?Q?HDQcdtBxrXrgllaGHCLcglb5EaoRMNwOwJPtRoah5xflMK+zSf8eqNskxvmE?=
 =?us-ascii?Q?L/rlULM8CyAitKbB1PWv8Zd57z2N8RsZuowodcK9V054mMxcNSzfHz+EThxF?=
 =?us-ascii?Q?SvfTAL8jNyiO4kjEMi0MqCNlRlF1OGIdxjxKBG+PZiMsv/3kqnbObOzEZa1Z?=
 =?us-ascii?Q?noYNBv9Rtk2VwkEuGnwt3xaYYzHbvMZPyeDqpX5y/QPakvH2AWXQ+gMm8oiD?=
 =?us-ascii?Q?yd1NYfptWvVUHSYXC9lsYVe0WeCV/lXIneh6Z2J/DZchsPro2QJO3Clef9xr?=
 =?us-ascii?Q?p8BQCDG1VpnY4CKBzQ+7LCPp/5P3prjBCCVhMX7rptDP0gECCsN8Pif0229c?=
 =?us-ascii?Q?1r7AYWs/JmqyWNfRL/sZEUtBPFUasNAJ8wlXf+Gv8sdksCa5o6jbjRRQzfOx?=
 =?us-ascii?Q?46JtiGSTxBbe9/ZEIX9Gd8Bc81o0gBdZ9a2bRLCGKjjKmTTdrxbR8EfRp0CV?=
 =?us-ascii?Q?nfaL1F7I0I1UGPLlZMhbm+/QOzTwIgZ6F76pb4myGZPb+iI7bGHwUjodR/he?=
 =?us-ascii?Q?W4NhZDjvSnpq9ZtyCObkVfWg5PbcAZkGts4wtURq2avniHITVlOLc9sEEP8I?=
 =?us-ascii?Q?e6odEApmzSc0qKCUFxIcHls/ryVY0dC9/N3WE7tsx+Ccx9D60FdUl7iNI277?=
 =?us-ascii?Q?56Mn8LbWORdTXBrNz03becQjQ4SeEEt/c1C6DP024QqpnBZneYQHyleodJtM?=
 =?us-ascii?Q?T8BATn8vYvzgpJp5O48kj8CKHHbZSri3wfjHFXtQ?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 74ca3753-6742-4176-32c1-08daa14442dc
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1101MB2221.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Sep 2022 11:26:17.9327
 (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: YhGj8ETUR1OwhZdo5bFMnO/yEjoZEboHL87JuJsqU38emjufC4aK8l0/V+Y9cWDLLiZKSW6Lsq19mSN28T57Yw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR11MB5828
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Sep 27, 2022 at 09:43:08PM +0000, Sean Christopherson wrote:
>The benefits I see are:
>
>  1. The code is very explicit.  When clearing SN, check PIR and set ON=1 to
>     ensure IRQs aren't ignored.
>
>  2. pi_enable_wakeup_handler() is only responsible for changing the vector,
>     i.e. there's no clearing of SN "hidden" in the CMPXCHG loop.
>
>  3. Same for vmx_vcpu_pi_load(), it's only responsible for updating the pCPU
>     and the vector, it doesn't touch SN.
>
>  4. The logic is common to all paths that clear SN, i.e. the same helper can
>     be used for both VM-Enter and vCPU blocking.
>
>E.g. the VMX hook for both VM-Enter and vCPU blocking could be:
>
>  static void vmx_no_idea_what_to_call_this(struct kvm_vcpu *vcpu)
>  {
>	pi_clear_sn(&vmx->pi_desc);
>
>	/*
>	 * Clear SN before reading the bitmap.  The VT-d firmware writes the
>	 * bitmap and reads SN atomically (5.2.3 in the spec), so it doesn't
>	 * really have a memory barrier that pairs with this, but we cannot do
>	 * that and we need one.
>	 */
>	smp_mb__after_atomic();
>
>	/* blah blah blah */
>	if (!pi_test_on(&vmx->pi_desc) && !pi_is_pir_empty(&vmx->pi_desc))
>		pi_set_on(&vmx->pi_desc);
>  }
>
>One related thought thing I want to capture:
>
>The proposed approach is having vmx_sync_pir_to_irr() check PIR if ON=1 || SN=1
>is effectively _required_ to avoid breaking halt-polling.  I was thinking we could
>keep that path optimized to check only ON=1, but with that approach, KVM won't detect
>the pending IRQ until it actually starts to block the vCPU, i.e. until it clears SN
>and manually checks the PIR.  Clearing SN in kvm_arch_vcpu_pending() would be better
>than waiting until the vCPU is "put", but even that is too late from a halt-polling
>perspective.
>
>A comment in vmx_sync_pir_to_irr() is likely needed to capture that.

All your comments make sense to me. I just posted v3 with all issues of v2
addressed.