Messages in this thread | | | Date | Thu, 26 May 2022 23:13:54 +0300 | From | "Kirill A. Shutemov" <> | Subject | Re: [PATCHv3 2/3] x86/tdx: Clarify RIP adjustments in #VE handler |
| |
On Wed, May 25, 2022 at 09:02:48AM -0700, Dave Hansen wrote: > On 5/24/22 15:10, Kirill A. Shutemov wrote: > > +static int ve_instr_len(struct ve_info *ve) > > +{ > > + /* > > + * If the #VE happened due to instruction execution, GET_VEINFO > > + * provides info on the instruction. > > + * > > + * For #VE due to EPT violation, info provided by GET_VEINFO not usable > > + * and kernel has to decode instruction manually to find out its > > + * length. Catch such cases. > > + */ > > + if (WARN_ON_ONCE(ve->exit_reason == EXIT_REASON_EPT_VIOLATION)) > > + return 0; > > + > > + return ve->instr_len; > > +} > > I'm not super happy with how this comment ended up. First, let's put > the comment next to the code to which it applies, like: > > /* > * ve->instr_len is not defined for EPT violations. For those, > * the kernel must decode instructions manually and should not > * be using this function. > */ > if (WARN_ON_ONCE(ve->exit_reason == EXIT_REASON_EPT_VIOLATION)) > return 0; > > /* > * Assume that the #VE occurred due to instruction execution. > */ > return ve->instr_len;
Would it be helpful if the function has a whitelist of exit resons where using ve->instr_len is safe? WARN_ONCE() and return 0 otherwise.
-- Kirill A. Shutemov
| |