Messages in this thread | | | Date | Fri, 9 Dec 2022 07:45:34 -0800 | Subject | Re: [PATCH 3/4] x86/tdx: Relax SEPT_VE_DISABLE check for debug TD | From | Sathyanarayanan Kuppuswamy <> |
| |
On 12/9/22 5:25 AM, 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. > > 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.");
Why add this change part of TD debug check? Should this be a separate patch?
> return handle_mmio(regs, ve); > case EXIT_REASON_IO_INSTRUCTION: > return handle_io(regs, ve);
-- Sathyanarayanan Kuppuswamy Linux Kernel Developer
| |