lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 7 Oct 2021 19:25:44 -0700 From: "Kuppuswamy, Sathyanarayanan" <sathyanarayanan.kuppuswamy@...ux.intel.com> To: Josh Poimboeuf <jpoimboe@...hat.com> Cc: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, x86@...nel.org, Paolo Bonzini <pbonzini@...hat.com>, David Hildenbrand <david@...hat.com>, Andrea Arcangeli <aarcange@...hat.com>, Juergen Gross <jgross@...e.com>, Deep Shah <sdeep@...are.com>, VMware Inc <pv-drivers@...are.com>, Vitaly Kuznetsov <vkuznets@...hat.com>, Wanpeng Li <wanpengli@...cent.com>, Jim Mattson <jmattson@...gle.com>, Joerg Roedel <joro@...tes.org>, Peter H Anvin <hpa@...or.com>, Dave Hansen <dave.hansen@...el.com>, Tony Luck <tony.luck@...el.com>, Dan Williams <dan.j.williams@...el.com>, Andi Kleen <ak@...ux.intel.com>, Kirill Shutemov <kirill.shutemov@...ux.intel.com>, Sean Christopherson <seanjc@...gle.com>, Kuppuswamy Sathyanarayanan <knsathya@...nel.org>, linux-kernel@...r.kernel.org Subject: Re: [PATCH v8 11/11] x86/tdx: Handle CPUID via #VE On 10/6/21 1:26 PM, Josh Poimboeuf wrote: > On Mon, Oct 04, 2021 at 07:52:05PM -0700, Kuppuswamy Sathyanarayanan wrote: >> +static u64 tdx_handle_cpuid(struct pt_regs *regs) >> +{ >> + struct tdx_hypercall_output out = {0}; >> + u64 ret; >> + >> + /* >> + * Emulate CPUID instruction via hypercall. More info about >> + * ABI can be found in TDX Guest-Host-Communication Interface >> + * (GHCI), section titled "VP.VMCALL<Instruction.CPUID>". >> + */ >> + ret = _tdx_hypercall(EXIT_REASON_CPUID, regs->ax, regs->cx, 0, 0, &out); >> + >> + /* >> + * As per TDX GHCI CPUID ABI, r12-r15 registers contains contents of >> + * EAX, EBX, ECX, EDX registers after CPUID instruction execution. >> + * So copy the register contents back to pt_regs. >> + */ >> + regs->ax = out.r12; >> + regs->bx = out.r13; >> + regs->cx = out.r14; >> + regs->dx = out.r15; > > Does it still make sense to save the regs if _tdx_hypercall() returns an > error? We don't need to save it in failure case. I will add check for error case in next version. > >> + >> + return ret; > > Also I'm wondering about error handling for all these _tdx_hypercall() > wrapper functions which are called by the #VE handler. > > First, there are some inconsistencies in whether and how they return the > r10 error. Since we have only cared about zero/non-zero return value, we did not check for consistency. May be I can convert all handler return values to bool. > > - _tdx_halt() warns and doesn't return anything. Since tdx_halt handler is shared with pv_ops, we can't return anything back (so we use WARN_ON to report the error). > > - tdx_read_msr_safe() and tdx_write_msr_safe() convert all errors to -EIO. Return value does not matter. we only check for zero/non-zero value in tdx_handle_virtualization_exception(). we have used -EIO to convey that it is an IO error. > > - tdx_handle_cpuid() returns the raw vmcall error. > > Second, as far as I can tell, the #VE handler doesn't check the actual > return code value, other than checking for non-zero. Should it at least > be printed in a warning? I don't think this is required. We can use trace to check the error code or argument details in failure case. Since we don't really use the error value, I am planning to change the #VE handler return type to bool. > -- Sathyanarayanan Kuppuswamy Linux Kernel Developer
Powered by blists - more mailing lists