| 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 linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <99c025a17fbf8212c5472eb47e72e1917d812997.camel@intel.com>
Date: Sat, 5 Jun 2021 04:26:18 +0000
From: "Williams, Dan J" <dan.j.williams@...el.com>
To: "sathyanarayanan.kuppuswamy@...ux.intel.com"
<sathyanarayanan.kuppuswamy@...ux.intel.com>,
"peterz@...radead.org" <peterz@...radead.org>,
"Luck, Tony" <tony.luck@...el.com>,
"Hansen, Dave" <dave.hansen@...el.com>,
"luto@...nel.org" <luto@...nel.org>
CC: "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
"Raj, Ashok" <ashok.raj@...el.com>,
"seanjc@...gle.com" <seanjc@...gle.com>,
"ak@...ux.intel.com" <ak@...ux.intel.com>,
"knsathya@...nel.org" <knsathya@...nel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC v2-fix-v1 2/3] x86/tdx: Handle early IO operations
On Wed, 2021-05-26 at 21:23 -0700, Kuppuswamy Sathyanarayanan wrote:
> From: Andi Kleen <ak@...ux.intel.com>
>
> Add an early #VE handler to convert early port IOs into TDCALLs.
>
> TDX cannot do port IO directly. The TDX module triggers a #VE
> exception to let the guest kernel to emulate operations like
> IO ports, by converting them into TDCALLs to call the host.
s,kernel to emulate operations like IO ports,kernel emulate port I/O,
>
> A fully featured #VE handler support for port IO will be added
> later in this patch set (in patch titled "x86/tdx: Handle port
> I/O). But it can be used only at later point in the boot
> process. So to support port IO in early boot code, add a
> minimal support in early exception handler. This is similar to
> what AMD SEV does.
Clarify "fully featured". I naively thought that the notes below about
trace and printk were implying that the full featured #VE handler will
use printk() so it can't also use #VE since printk() would recurse into
the #VE handler if the serial console is using port IO.
...but that does not seem to be the reason since:
http://lore.kernel.org/r/20210527042356.3983284-4-sathyanarayanan.kuppuswamy@linux.intel.com
...is also using #VE for port IO emulation?
> This is mainly to support early_printk's serial driver, as
> well as potentially the VGA driver (although it is expected
> not to be used).
>
> The early handler only does IO calls and nothing else, and
> anything that goes wrong results in a normal early exception
> panic.
>
> It cannot share the code paths with the normal #VE handler
> because it needs to avoid using trace calls or printk.
>
> This early handler allows us to use the normal in*/out*
> macros without patching them for every driver. We don't
> expect IO port IO to be performance critical at all, so an
> extra #VE exception is no problem.
"There is no expectation that early port IO is performance critical, so
the #VE emulation cost is worth the simplicity benefit of not patching
out port IO usage in early code."
> There are also no concerns
> with nesting, since there should be no NMIs this early.
>
> Signed-off-by: Andi Kleen <ak@...ux.intel.com>
Missing a Sathya signed-off-by.
> ---
> arch/x86/include/asm/tdx.h | 6 ++++
> arch/x86/kernel/head64.c | 4 +++
> arch/x86/kernel/tdx.c | 59 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 69 insertions(+)
>
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 53f844200909..e880a9dd40d3 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -72,6 +72,7 @@ u64 __tdx_hypercall(u64 fn, u64 r12, u64 r13, u64 r14, u64 r15,
> struct tdx_hypercall_output *out);
>
> bool tdx_protected_guest_has(unsigned long flag);
> +bool tdg_early_handle_ve(struct pt_regs *regs);
>
> #else // !CONFIG_INTEL_TDX_GUEST
>
> @@ -87,6 +88,11 @@ static inline bool tdx_protected_guest_has(unsigned long flag)
> return false;
> }
>
> +static inline bool tdg_early_handle_ve(struct pt_regs *regs)
> +{
> + return false;
> +}
> +
> #endif /* CONFIG_INTEL_TDX_GUEST */
>
> #ifdef CONFIG_INTEL_TDX_GUEST_KVM
> diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
> index 75f2401cb5db..23d1ff4626aa 100644
> --- a/arch/x86/kernel/head64.c
> +++ b/arch/x86/kernel/head64.c
> @@ -410,6 +410,10 @@ void __init do_early_exception(struct pt_regs *regs, int trapnr)
> trapnr == X86_TRAP_VC && handle_vc_boot_ghcb(regs))
> return;
>
> + if (IS_ENABLED(CONFIG_INTEL_TDX_GUEST) &&
This explicit IS_ENABLED() is unnecessary given tdg_early_handle_ve()
returns false in the CONFIG_INTEL_TDX_GUEST=n case as defined above.
> + trapnr == X86_TRAP_VE && tdg_early_handle_ve(regs))
> + return;
> +
> early_fixup_exception(regs, trapnr);
> }
>
> diff --git a/arch/x86/kernel/tdx.c b/arch/x86/kernel/tdx.c
> index 858e7f3d8f36..ca3442b7accf 100644
> --- a/arch/x86/kernel/tdx.c
> +++ b/arch/x86/kernel/tdx.c
> @@ -13,6 +13,10 @@
> #define TDINFO 1
> #define TDGETVEINFO 3
>
> +#define VE_GET_IO_TYPE(exit_qual) (((exit_qual) & 8) ? 0 : 1)
How about VE_IS_IO_OUT()? To match its usage as a flag below...
> +#define VE_GET_IO_SIZE(exit_qual) (((exit_qual) & 7) + 1)
> +#define VE_GET_PORT_NUM(exit_qual) ((exit_qual) >> 16)
> +
> static struct {
> unsigned int gpa_width;
> unsigned long attributes;
> @@ -256,6 +260,61 @@ int tdg_handle_virtualization_exception(struct pt_regs *regs,
> return ret;
> }
>
> +/*
> + * Handle early IO, mainly for early printks serial output.
> + * This avoids anything that doesn't work early on, like tracing
> + * or printks, by calling the low level functions directly. Any
> + * problems are handled by falling back to a standard early exception.
> + *
> + * Assumes the IO instruction was using ax, which is enforced
> + * by the standard io.h macros.
> + */
> +static __init bool tdx_early_io(struct ve_info *ve, struct pt_regs *regs)
> +{
> + struct tdx_hypercall_output outh;
> + int out = VE_GET_IO_TYPE(ve->exit_qual);
> + int size = VE_GET_IO_SIZE(ve->exit_qual);
> + int port = VE_GET_PORT_NUM(ve->exit_qual);
> + int ret;
> +
...and if @out is a flag then the below can be simplified to:
ret = __tdx_hypercall(EXIT_REASON_IO_INSTRUCTION, size, out, port,
regs->ax, &outh);
if (!out && !ret) {
u64 mask = GENMASK(8 * size, 0);
regs->ax &= ~mask;
regs->ax |= outh.r11 & mask;
}
return !ret;
With the above fixups and clarifications you can add:
Reviewed-by: Dan Williams <dan.j.williams@...el.com>
...but the discussion here about fully featured leaves me confused
about the approach taken in the next patch.
> + if (out) {
> + ret = __tdx_hypercall(EXIT_REASON_IO_INSTRUCTION,
> + size, 1, port,
> + regs->ax,
> + &outh);
> + } else {
> + u64 mask = GENMASK(8 * size, 0);
> +
> + ret = __tdx_hypercall(EXIT_REASON_IO_INSTRUCTION,
> + size, 0, port,
> + regs->ax, &outh);
> + if (!ret) {
> + regs->ax &= ~mask;
> + regs->ax |= outh.r11 & mask;
> + }
> + }
> +
> + return !ret;
> +}
> +
> +/*
> + * Early #VE exception handler. Just used to handle port IOs
> + * for early_printk. If anything goes wrong handle it like
> + * a normal early exception.
> + */
> +__init bool tdg_early_handle_ve(struct pt_regs *regs)
> +{
> + struct ve_info ve;
> +
> + if (tdg_get_ve_info(&ve))
> + return false;
> +
> + if (ve.exit_reason == EXIT_REASON_IO_INSTRUCTION)
> + return tdx_early_io(&ve, regs);
> +
> + return false;
> +}
> +
> void __init tdx_early_init(void)
> {
> if (!cpuid_has_tdx_guest())
Powered by blists - more mailing lists