Messages in this thread |  | | Date | Thu, 15 Dec 2022 18:40:18 +0300 | From | "Kirill A. Shutemov" <> | Subject | Re: [PATCH 3/4] x86/tdx: Relax SEPT_VE_DISABLE check for debug TD |
| |
On Tue, Dec 13, 2022 at 03:13:43PM -0800, Dave Hansen wrote: > On 12/9/22 05:25, Kirill A. Shutemov wrote: > > SEPT_VE_DISABLE check is required to keep the TD protected from VMM > > attacks, but it makes harder to debug guest kernel bugs. If guest > > touches unaccepted memory the TD will get terminated without any > > traces on what has happened. > > This is a bit sparse. > > -- > > A "SEPT #VE" occurs when a TDX guest touches memory that is not properly > mapped into the "secure EPT". This can be the result of hypervisor > attacks or bugs, *OR* guest bugs. Most notably, buggy guests might > touch unaccepted memory for lots of different memory safety bugs like > buffer overflows. > > TDX guests do not want to continue in the face of hypervisor attacks or > hypervisor bugs. They want to terminate as fast and safely as possible. > SEPT_VE_DISABLE ensures that TDX guests *can't* continue in the face of > these kinds of issues. > > But, that causes a problem. TDX guests that can't continue can't spit > out oopses or other debugging info. In essence SEPT_VE_DISABLE=1 guests > are not debuggable. That's a problem. > > -- > > Eh?
Thanks!
> > Relax the SEPT_VE_DISABLE check to warning on debug TD and panic() in > > the #VE handler on EPT-violation on private memory. It will produce > > useful backtrace. > > > > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > > --- > > arch/x86/coco/tdx/tdx.c | 14 ++++++++++++-- > > 1 file changed, 12 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c > > index 8ad04d101270..0e47846ff8ff 100644 > > --- a/arch/x86/coco/tdx/tdx.c > > +++ b/arch/x86/coco/tdx/tdx.c > > @@ -38,6 +38,7 @@ > > #define VE_GET_PORT_NUM(e) ((e) >> 16) > > #define VE_IS_IO_STRING(e) ((e) & BIT(4)) > > > > +#define ATTR_DEBUG BIT(0) > > #define ATTR_SEPT_VE_DISABLE BIT(28) > > > > /* TDX Module call error codes */ > > @@ -207,8 +208,15 @@ static void tdx_parse_tdinfo(u64 *cc_mask) > > * TD-private memory. Only VMM-shared memory (MMIO) will #VE. > > */ > > td_attr = out.rdx; > > - if (!(td_attr & ATTR_SEPT_VE_DISABLE)) > > - tdx_panic("TD misconfiguration: SEPT_VE_DISABLE attribute must be set."); > > + if (!(td_attr & ATTR_SEPT_VE_DISABLE)) { > > + const char *msg = "TD misconfiguration: SEPT_VE_DISABLE attribute must be set."; > > + > > + /* Relax SEPT_VE_DISABLE check for debug TD. */ > > + if (td_attr & ATTR_DEBUG) > > + pr_warn("%s\n", msg); > > + else > > + tdx_panic(msg); > > + } > > } > > > > /* > > @@ -682,6 +690,8 @@ static int virt_exception_kernel(struct pt_regs *regs, struct ve_info *ve) > > case EXIT_REASON_CPUID: > > return handle_cpuid(regs, ve); > > case EXIT_REASON_EPT_VIOLATION: > > + if (ve->gpa != cc_mkdec(ve->gpa)) > > + panic("Unexpected EPT-violation on private memory."); > > What's the cc_mkdec() doing?
Checks if the GPA is private. I will move it to helper. Like this:
static inline bool is_private_gpa(u64 gpa) { return gpa == cc_mkenc(gpa); }
-- Kiryl Shutsemau / Kirill A. Shutemov
|  |