Messages in this thread | | | Date | Fri, 25 Feb 2022 11:46:04 -0800 | From | Dave Hansen <> | Subject | Re: [PATCHv4 07/30] x86/traps: Add #VE support for TDX guest |
| |
On 2/25/22 11:30, Kirill A. Shutemov wrote: > On Thu, Feb 24, 2022 at 10:36:02AM -0800, Dave Hansen wrote: >> On 2/24/22 07:56, Kirill A. Shutemov wrote: >>> Virtualization Exceptions (#VE) are delivered to TDX guests due to >>> specific guest actions which may happen in either user space or the >>> kernel: >>> >>> * Specific instructions (WBINVD, for example) >>> * Specific MSR accesses >>> * Specific CPUID leaf accesses >>> * Access to unmapped pages (EPT violation) >> >> Considering that you're talking partly about userspace, it would be nice >> to talk about what "unmapped" really means here. > > I'm not sure what you want to see here. Doesn't EPT violation describe it? > > It can happen to userspace too, but we don't expect it to be use used and > SIGSEGV the process if it happens.
How about just:
* Access to specific guest physical addresses
That makes it clear that we're not really talking about userspace unmapped pages.
... >>> + * module also treats virtual NMIs as inhibited if the #VE valid flag is >>> + * set, e.g. so that NMI=>#VE will not result in a #DF. >>> + */ >> >> Are we missing anything valuable if we just trim the comment down to >> something like: >> >> /* >> * Called during #VE handling to retrieve the #VE info from the >> * TDX module. >> * >> * This should called done early in #VE handling. A "nested" >> * #VE which occurs before this will raise a #DF and is not >> * recoverable. >> */ > > This variant of the comment lost information about #VE-valid flag and > doesn't describe how NMI is inhibited.
IMNHO, the "#VE valid" flag is a super-fine implementation detail. I'd personally deal with that in Documentation or the changelog instead of a comment.
>>> +#ifdef CONFIG_INTEL_TDX_GUEST >>> + >>> +#define VE_FAULT_STR "VE fault" >>> + >>> +static void ve_raise_fault(struct pt_regs *regs, long error_code) >>> +{ >>> + if (user_mode(regs)) { >>> + gp_user_force_sig_segv(regs, X86_TRAP_VE, error_code, VE_FAULT_STR); >>> + return; >>> + } >>> + >>> + if (gp_try_fixup_and_notify(regs, X86_TRAP_VE, error_code, VE_FAULT_STR)) >>> + return; >>> + >>> + die_addr(VE_FAULT_STR, regs, error_code, 0); >>> +} >>> + >>> +/* >>> + * Virtualization Exceptions (#VE) are delivered to TDX guests due to >>> + * specific guest actions which may happen in either user space or the >>> + * kernel: >>> + * >>> + * * Specific instructions (WBINVD, for example) >>> + * * Specific MSR accesses >>> + * * Specific CPUID leaf accesses >>> + * * Access to unmapped pages (EPT violation) >>> + * >>> + * In the settings that Linux will run in, virtualization exceptions are >>> + * never generated on accesses to normal, TD-private memory that has been >>> + * accepted. >> >> This actually makes a lot more sense as a code comment than changelog. >> It would be really nice to circle back here and actually refer to the >> functions that accept memory. > > We don't have such functions at this point in the patchset. Do you want > the comment to be updated once we get them introduced?
Yes, please. Supplement the comment when the functions are introduced later.
| |