linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Borislav Petkov <bp@suse.de>
To: Brijesh Singh <brijesh.singh@amd.com>
Cc: x86@kernel.org, linux-kernel@vger.kernel.org,
	kvm@vger.kernel.org, "Tom Lendacky" <thomas.lendacky@amd.com>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"H. Peter Anvin" <hpa@zytor.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Sean Christopherson" <sean.j.christopherson@intel.com>,
	"Radim Krčmář" <rkrcmar@redhat.com>
Subject: Re: [PATCH v8 1/2] x86/mm: add .bss..decrypted section to hold shared variables
Date: Fri, 14 Sep 2018 09:10:56 +0200	[thread overview]
Message-ID: <20180914071056.GA4747@zn.tnic> (raw)
In-Reply-To: <1536875471-17391-2-git-send-email-brijesh.singh@amd.com>

On Thu, Sep 13, 2018 at 04:51:10PM -0500, Brijesh Singh wrote:
> kvmclock defines few static variables which are shared with the
> hypervisor during the kvmclock initialization.

...

> diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
> index 8047379..c16af27 100644
> --- a/arch/x86/kernel/head64.c
> +++ b/arch/x86/kernel/head64.c
> @@ -112,6 +112,7 @@ static bool __head check_la57_support(unsigned long physaddr)
>  unsigned long __head __startup_64(unsigned long physaddr,
>  				  struct boot_params *bp)
>  {
> +	unsigned long vaddr, vaddr_end;
>  	unsigned long load_delta, *p;
>  	unsigned long pgtable_flags;
>  	pgdval_t *pgd;
> @@ -235,6 +236,21 @@ unsigned long __head __startup_64(unsigned long physaddr,
>  	sme_encrypt_kernel(bp);
>  
>  	/*
> +	 * Clear the memory encryption mask from the .bss..decrypted section.
> +	 * The bss section will be memset to zero later in the initialization so
> +	 * there is no need to zero it after changing the memory encryption
> +	 * attribute.
> +	 */
> +	if (mem_encrypt_active()) {
> +		vaddr = (unsigned long)__start_bss_decrypted;
> +		vaddr_end = (unsigned long)__end_bss_decrypted;
> +		for (; vaddr < vaddr_end; vaddr += PMD_SIZE) {
> +			i = pmd_index(vaddr);
> +			pmd[i] -= sme_get_me_mask();
> +		}
> +	}

Why isn't this chunk at the end of sme_encrypt_kernel() instead of here?

> +	/*
>  	 * Return the SME encryption mask (if SME is active) to be used as a
>  	 * modifier for the initial pgdir entry programmed into CR3.
>  	 */
> diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
> index 9c77d2d..0d618ee 100644
> --- a/arch/x86/kernel/vmlinux.lds.S
> +++ b/arch/x86/kernel/vmlinux.lds.S
> @@ -65,6 +65,23 @@ jiffies_64 = jiffies;
>  #define ALIGN_ENTRY_TEXT_BEGIN	. = ALIGN(PMD_SIZE);
>  #define ALIGN_ENTRY_TEXT_END	. = ALIGN(PMD_SIZE);
>  
> +/*
> + * This section contains data which will be mapped as decrypted. Memory
> + * encryption operates on a page basis. Make this section PMD-aligned
> + * to avoid splitting the pages while mapping the section early.
> + *
> + * Note: We use a separate section so that only this section gets
> + * decrypted to avoid exposing more than we wish.
> + */
> +#define BSS_DECRYPTED						\
> +	. = ALIGN(PMD_SIZE);					\
> +	__start_bss_decrypted = .;				\
> +	*(.bss..decrypted);					\
> +	. = ALIGN(PAGE_SIZE);					\
> +	__start_bss_decrypted_unused = .;			\
> +	. = ALIGN(PMD_SIZE);					\
> +	__end_bss_decrypted = .;				\
> +
>  #else
>  
>  #define X86_ALIGN_RODATA_BEGIN
> @@ -74,6 +91,7 @@ jiffies_64 = jiffies;
>  
>  #define ALIGN_ENTRY_TEXT_BEGIN
>  #define ALIGN_ENTRY_TEXT_END
> +#define BSS_DECRYPTED
>  
>  #endif
>  
> @@ -345,6 +363,7 @@ SECTIONS
>  		__bss_start = .;
>  		*(.bss..page_aligned)
>  		*(.bss)
> +		BSS_DECRYPTED
>  		. = ALIGN(PAGE_SIZE);
>  		__bss_stop = .;

Putting it in the BSS would need a bit of care in the future as it poses
a certain ordering on the calls in x86_64_start_kernel() (not that there
isn't any now :)):

You have:

x86_64_start_kernel:

	...

	clear_bss();

	...

	x86_64_start_reservations() -> ... -> setup_arch() -> kvmclock_init()

so it would be prudent to put at least a comment somewhere, say, over
the definition of BSS_DECRYPTED or so, that attention should be paid to
the clear_bss() call early.

Thx.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

  parent reply	other threads:[~2018-09-14  7:11 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-13 21:51 [PATCH v8 0/2] x86: Fix SEV guest regression Brijesh Singh
2018-09-13 21:51 ` [PATCH v8 1/2] x86/mm: add .bss..decrypted section to hold shared variables Brijesh Singh
2018-09-13 23:24   ` Thomas Gleixner
2018-09-14  2:14     ` Brijesh Singh
2018-09-14  7:10   ` Borislav Petkov [this message]
2018-09-14 11:58     ` Brijesh Singh
2018-09-14 12:17       ` Thomas Gleixner
2018-09-14 14:12         ` Borislav Petkov
2018-09-14 14:27           ` Brijesh Singh
2018-09-14 14:45             ` Borislav Petkov
2018-09-14 14:50           ` Tom Lendacky
2018-09-14 14:55             ` Thomas Gleixner
2018-09-13 21:51 ` [PATCH v8 2/2] x86/kvm: use __bss_decrypted attribute in " Brijesh Singh
2018-09-13 23:31   ` Thomas Gleixner
2018-09-14  0:25   ` kbuild test robot

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=20180914071056.GA4747@zn.tnic \
    --to=bp@suse.de \
    --cc=brijesh.singh@amd.com \
    --cc=hpa@zytor.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=rkrcmar@redhat.com \
    --cc=sean.j.christopherson@intel.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).