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;
From:   "Li, Xin3" <xin3.li@intel.com>
To:     Peter Zijlstra <peterz@infradead.org>
CC:     Paolo Bonzini <pbonzini@redhat.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "x86@kernel.org" <x86@kernel.org>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        "tglx@linutronix.de" <tglx@linutronix.de>,
        "mingo@redhat.com" <mingo@redhat.com>,
        "bp@alien8.de" <bp@alien8.de>,
        "dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
        "hpa@zytor.com" <hpa@zytor.com>,
        "Christopherson,, Sean" <seanjc@google.com>,
        "Tian, Kevin" <kevin.tian@intel.com>
Subject: RE: [RESEND PATCH 5/6] KVM: x86/VMX: add kvm_vmx_reinject_nmi_irq()
 for NMI/IRQ reinjection
Thread-Topic: [RESEND PATCH 5/6] KVM: x86/VMX: add kvm_vmx_reinject_nmi_irq()
 for NMI/IRQ reinjection
Thread-Index: AQHY9M9YytlyKV7ob0GK/oJbeBhPY6433PwAgACmfxCAAO8aAIAAL0qAgAAENoCAAAgUAIAAGnuAgAA1qHCAACERAIADuBAAgABQYICAAXpoEIAAGnIAgAK/reA=
Date:   Thu, 17 Nov 2022 03:37:40 +0000
Message-ID: <DM5PR1101MB2172D7D7BC49255DB3752802A8069@DM5PR1101MB2172.namprd11.prod.outlook.com>
References: <Y24SoNKZtj/NPSGy@hirez.programming.kicks-ass.net>
 <6097036e-063f-5175-72b2-8935b12af853@redhat.com>
 <Y24908NWCdzUNqI0@hirez.programming.kicks-ass.net>
 <6fd26a70-3774-6ae7-73ea-4653aee106f0@redhat.com>
 <Y25a0Z2tOMWYZs4j@hirez.programming.kicks-ass.net>
 <BN6PR1101MB216141A21353AB84CEA541DFA8009@BN6PR1101MB2161.namprd11.prod.outlook.com>
 <Y26jkHfK9INwU7Yy@hirez.programming.kicks-ass.net>
 <BN6PR1101MB2161E8217F50D18C56E5864EA8059@BN6PR1101MB2161.namprd11.prod.outlook.com>
 <Y3IFo9NrAcYalBzM@hirez.programming.kicks-ass.net>
 <BN6PR1101MB2161299749E12D484DE9302BA8049@BN6PR1101MB2161.namprd11.prod.outlook.com>
 <Y3NZQBJugRt07udw@hirez.programming.kicks-ass.net>
In-Reply-To: <Y3NZQBJugRt07udw@hirez.programming.kicks-ass.net>
Accept-Language: en-US
Content-Language: en-US
dlp-product: dlpe-windows
dlp-version: 11.6.500.17
dlp-reaction: no-action
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?pOZ77kbfwR9m6n8740N0CLPRFE3IJXIBkPVVA2excRqkFeKtmItlCs42R4AI?=
 =?us-ascii?Q?nnPN0hnpKDLvHx8+Rfsnmk5UJLXMBI78lGGlEiULW0oStrmboLNObdWVL9Ez?=
 =?us-ascii?Q?06MOivdwTuLzRL57A/7QoS2QlbuCpEHG4C8/v55ZS/tlQYPHkif0HFsUJfpc?=
 =?us-ascii?Q?nIPNXzy96Sl/6u3aE/S9f4Pqkq/fLhQdoaKJJeUBcmZUj7Mhazk4r12Mh5pa?=
 =?us-ascii?Q?FaWyY3tPijx/qn4Uvpoih4CKY5HABX6O8bTe6bBXUeh6jIlFgYQqPq8mCeGx?=
 =?us-ascii?Q?W6C91c7MKP/FUy8o6LA64krwvA7T9zPD01zdRdbhqfCr12/r2kqvtbGTvJIt?=
 =?us-ascii?Q?P0v9oo+/345iBoz1tvgZdBLy9iADVe/t5ob0f5tEODzW19vJS+8VjpuH8bZ7?=
 =?us-ascii?Q?WdecyUGx/advlFTrBPtLecJViECb5kj5bZBnRYiWblB6HTsYkUCOK9gPnEHR?=
 =?us-ascii?Q?oQwxgRziCf+9RwLLe+fWzEPUz2qRRFkWaoEEqF+VLaCePNPQb27vhCAwogRT?=
 =?us-ascii?Q?5KlkuuFz3i82SLgbNvATKFj1Ru78Md0oMOaQQPdF9Icey1YTqUFIqFqCFTnb?=
 =?us-ascii?Q?wSa46SGrOMX0PVmR9gosquBox19YWDugdSoHgTUhL1JPicuJ1rCBGehO+7KJ?=
 =?us-ascii?Q?G+y8CTwo7Dl0jq1nDZbvbYmBfyjJ0IPmm3IMYixt4+AkcnHqeMm6zETmWxZS?=
 =?us-ascii?Q?8y0BKBl6XrlkGQP4ne6reiD49tnTuFAHEaNvK8OfMZIiMzAQlDKsxUGNQO6I?=
 =?us-ascii?Q?cwvFlyw75cycgfbQQYu9NNz32HOgndOntI/fD84LAaT+sGopmNUih+ScYOvJ?=
 =?us-ascii?Q?BTBVxnCZV/0nIrAe29Bc5C2PC9eB5kLozVUq84HPSLa2fx+cBl5B/Drfe3YX?=
 =?us-ascii?Q?etJeM5DVNAN84HwwYPgMGoQuwVVocMlPN6YfZdc1W571kpURfUSFsyi4K2fB?=
 =?us-ascii?Q?ZRXJm/lRoM8tcNLMKEYuzR55HQF7oOZ8qBOq8XwajC5Dv1X0V5aTucO9T3VU?=
 =?us-ascii?Q?gWfX9PwZ08oUQxkS4ArftfV0sGJmfbZwuuqoovFvlKeTODE/byQIEZRy81qY?=
 =?us-ascii?Q?OTCZ8qI2H6yWPc27r09Yg56gkAhk4e76nme/kegCUcueRmdA33M8jSONWNQH?=
 =?us-ascii?Q?c6ajoHUKTOKorKcDaOnkCchKljovykVfeRot5YgXOS7mQVVf63SXOkRTWClV?=
 =?us-ascii?Q?GgYRaSAgIWxiTmZmH9so1f6tR2wi+ZSSN6aun7zdDyACZIAAMsAxZVEczxLp?=
 =?us-ascii?Q?hWe9FQsvKRYrn1iOzy0w9rAg7oGqlWE2Z1SjDI6tACtT+ELSIdsZxxK9IZI1?=
 =?us-ascii?Q?lZykE8h9kj+blHVJleKOvOkK4Vugc9stZnX54YunMWHZ9FxISiWxIslmln1b?=
 =?us-ascii?Q?8HpIaajhw5f7Js+pVOoWjiV9f9WWZE3xIu/cf1qOfItVW1O1cDGgOB31lrHY?=
 =?us-ascii?Q?W9pok8+ZUB5ZbgkRKpvWySrBe9lg2OKP5AJmx478Pf/NwS+lmm/1Ugf3GKEZ?=
 =?us-ascii?Q?KsHX1VxDLlzdYI7KJT93Gb72ErQ3E8hZhDmrc6ZmavVSkJqb7V+H7HG4IHpY?=
 =?us-ascii?Q?lIu9HPgi63M4RM7kv5k=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DM5PR1101MB2172.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 03a2a8bc-1392-4634-41fb-08dac84d1478
X-MS-Exchange-CrossTenant-originalarrivaltime: 17 Nov 2022 03:37:40.7652
 (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: 55OuwNiX8/qepFbN2iDSEyYCu8IL2sgRjaBEnDNFPEsfr7D+/P3Ch64zou4DqxOQ5LXoy8maX1GrwRB0aoqvRw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ2PR11MB7647
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-7.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI,
        RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,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


> > > > > But what about NMIs, afaict this is all horribly broken for NMIs.
> > > > >
> > > > > So the whole VMX thing latches the NMI (which stops NMI
> > > > > recursion),
> > > right?
> > > > >
> > > > > But then you drop out of noinstr code, which means any random
> > > > > exception can happen (kprobes #BP, hw_breakpoint #DB, or even
> > > > > #PF due to random nonsense like *SAN). This exception will do
> > > > > IRET and clear the NMI latch, all before you get to run any of th=
e NMI
> code.
> > > >
> > > > What you said here implies that we have this problem in the existin=
g
> code.
> > > > Because a fake iret stack is created to call the NMI handler in
> > > > the IDT NMI descriptor, which lastly executes the IRET instruction.
> > >
> > > I can't follow; of course the IDT handler terminates with IRET, it ha=
s to no?
> >
> > With FRED, ERETS/ERETU replace IRET, and use bit 28 of the popped CS
> > field to control whether to unblock NMI. If bit 28 of the field (above
> > the selector) is 1, ERETS/ERETU unblocks NMIs.
>=20
> Yes, I know that. It is one of the many improvements FRED brings.
> Ideally the IBT WAIT-FOR-ENDBR state also gets squirreled away in the
> hardware exception frame, but that's still up in the air I believe :/
>=20
> Anyway.. given there is interrupt priority and NMI is pretty much on top =
of
> everything else the reinject crap *should* run NMI first. That way NMI ru=
ns
> with the latch disabled and whatever other pending interrupts will run la=
ter.
>=20
> But that all is still broken because afaict the current code also leaves =
noinstr --
> and once you leave noinstr (or use a static_key, static_call or anything =
else that
> *can* change at runtime) you can't guarantee nothing.

For NMI, HPA asked me to use "int $2", as it switches to the NMI IST stack =
to
execute the NMI handler, essentially like how HW deals with a NMI in host. =
and
I tested it with NMI watchdog, it looks working fine.

For IRQs, we still use the dispatch table, but with a new func added in
DEFINE_IDTENTRY_SYSVEC with the noinstr entry/exit code removed:

#define DEFINE_IDTENTRY_SYSVEC(func)                                    \
static void __##func(struct pt_regs *regs);                             \
                                                                        \
__visible noinstr void func(struct pt_regs *regs)                       \
{                                                                       \
        irqentry_state_t state =3D irqentry_enter(regs);                  \
                                                                        \
        instrumentation_begin();                                        \
        kvm_set_cpu_l1tf_flush_l1d();                                   \
        run_sysvec_on_irqstack_cond(__##func, regs);                    \
        instrumentation_end();                                          \
        irqentry_exit(regs, state);                                     \
}                                                                       \
                                                                        \
+void dispatch_table_##func(struct pt_regs *regs)                        \
+{                                                                       \
+        kvm_set_cpu_l1tf_flush_l1d();                                   \
+        run_sysvec_on_irqstack_cond(__##func, regs);                    \
+}
+                                                                       \
                                                                        \
static noinline void __##func(struct pt_regs *regs)

How do you think?