All of lore.kernel.org
 help / color / mirror / Atom feed
From: Borislav Petkov <bp@alien8.de>
To: Joerg Roedel <joro@8bytes.org>
Cc: x86@kernel.org, Joerg Roedel <jroedel@suse.de>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Andy Lutomirski <luto@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Kees Cook <keescook@chromium.org>,
	Arvind Sankar <nivedita@alum.mit.edu>,
	Martin Radev <martin.b.radev@gmail.com>,
	Tom Lendacky <thomas.lendacky@amd.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 3/5] x86/boot/compressed/64: Check SEV encryption in 64-bit boot-path
Date: Tue, 27 Oct 2020 12:08:12 +0100	[thread overview]
Message-ID: <20201027110812.GC15580@zn.tnic> (raw)
In-Reply-To: <20201021123938.3696-4-joro@8bytes.org>

On Wed, Oct 21, 2020 at 02:39:36PM +0200, Joerg Roedel wrote:
> diff --git a/arch/x86/kernel/sev_verify_cbit.S b/arch/x86/kernel/sev_verify_cbit.S
> new file mode 100644
> index 000000000000..5075458ecad0
> --- /dev/null
> +++ b/arch/x86/kernel/sev_verify_cbit.S

Why a separate file? You're using it just like verify_cpu.S and this is
kinda verifying CPU so you could simply add the functionality there...

> @@ -0,0 +1,90 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + *	sev_verify_cbit.S - Code for verification of the C-bit position reported
> + *			    by the Hypervisor when running with SEV enabled.
> + *
> + *	Copyright (c) 2020  Joerg Roedel (jroedel@suse.de)
> + *
> + * Implements sev_verify_cbit() which is called before switching to a new
> + * long-mode page-table at boot.
> + *
> + * It verifies that the C-bit position is correct by writing a random value to
> + * an encrypted memory location while on the current page-table. Then it
> + * switches to the new page-table to verify the memory content is still the
> + * same. After that it switches back to the current page-table and when the
> + * check succeeded it returns. If the check failed the code invalidates the
> + * stack pointer and goes into a hlt loop. The stack-pointer is invalidated to
> + * make sure no interrupt or exception can get the CPU out of the hlt loop.
> + *
> + * New page-table pointer is expected in %rdi (first parameter)
> + *
> + */
> +SYM_FUNC_START(sev_verify_cbit)
> +#ifdef CONFIG_AMD_MEM_ENCRYPT

Yeah, can you please use the callee-clobbered registers in the order as
they're used by the ABI, see arch/x86/entry/calling.h.

Because I'm looking at this and wondering are rsi, rdx and rcx somehow
live here and you're avoiding them...

Otherwise nice commenting - I like when it is properly explained what
the asm does and what it expects as input, cool.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

  reply	other threads:[~2020-10-27 11:08 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-21 12:39 [PATCH v3 0/5] x86/sev-es: Mitigate some HV attack vectors Joerg Roedel
2020-10-21 12:39 ` [PATCH v3 1/5] x86/boot/compressed/64: Introduce sev_status Joerg Roedel
2020-10-26 18:27   ` Borislav Petkov
2020-10-28  8:23     ` Joerg Roedel
2020-10-28 16:50       ` Arvind Sankar
2020-10-28 16:55         ` Joerg Roedel
2020-10-21 12:39 ` [PATCH v3 2/5] x86/boot/compressed/64: Add CPUID sanity check to early #VC handler Joerg Roedel
2020-10-27 10:38   ` Borislav Petkov
2020-10-28  8:31     ` Joerg Roedel
2020-10-21 12:39 ` [PATCH v3 3/5] x86/boot/compressed/64: Check SEV encryption in 64-bit boot-path Joerg Roedel
2020-10-27 11:08   ` Borislav Petkov [this message]
2020-10-28  8:38     ` Joerg Roedel
2020-10-27 11:09   ` Borislav Petkov
2020-10-21 12:39 ` [PATCH v3 4/5] x86/head/64: Check SEV encryption before switching to kernel page-table Joerg Roedel
2020-10-27 11:20   ` Borislav Petkov
2020-10-21 12:39 ` [PATCH v3 5/5] x86/sev-es: Do not support MMIO to/from encrypted memory Joerg Roedel
2020-10-27 11:26   ` Borislav Petkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201027110812.GC15580@zn.tnic \
    --to=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=joro@8bytes.org \
    --cc=jroedel@suse.de \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=martin.b.radev@gmail.com \
    --cc=mingo@redhat.com \
    --cc=nivedita@alum.mit.edu \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=thomas.lendacky@amd.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.