On Tue, Oct 10, 2023, Maxim Levitsky wrote: > У вт, 2023-10-10 у 07:46 -0700, Sean Christopherson пише: > > On Tue, Oct 10, 2023, Maxim Levitsky wrote: > > > У пн, 2023-10-09 у 14:29 -0700, Sean Christopherson пише: > > > > Note, per the APM, hardware sets the BLOCKING flag when software directly > > > > directly injects an NMI: > > > > > > > > If Event Injection is used to inject an NMI when NMI Virtualization is > > > > enabled, VMRUN sets V_NMI_MASK in the guest state. > > > > > > I think that this comment is not needed in the commit message. It describes > > > a different unrelated concern and can be put somewhere in the code but > > > not in the commit message. > > > > I strongly disagree, this blurb in the APM directly affects the patch. If hardware > > didn't set V_NMI_MASK, then the patch would need to be at least this: > > I don't see how 'the blurb in the APM' relates to the removal of the > IRET intercept, which is what this patch is about. No, it's not *just* about IRET interception. This patch also guards: svm->nmi_masked = true; If the reader doesn't already know that hardware sets V_NMI_BLOCK_MASK on direct injection, as was the case for me when I stumbled upon this issue, it's not at all obvious that not doing something analogous to setting nmi_masked is correct. I mentioned only IRET interception in the shortlog because that's the only practical impact of the change. I can massage the shortlog if it's confusing/misleading, but I really don't want to drop the reference to hardware setting V_NMI_BLOCK_MASK.
