Re: [PATCH v7 RESEND 2/4] kexec: allocate unencrypted control pages for kdump in case SME is enabled

From: Borislav Petkov <bp@suse.de>
To: Lianbo Jiang <lijiang@redhat.com>
Cc: thomas.lendacky@amd.com, jroedel@suse.de, brijesh.singh@amd.com,
	bhe@redhat.com, tiwai@suse.de, x86@kernel.org,
	kexec@lists.infradead.org, linux-kernel@vger.kernel.org,
	mingo@redhat.com, baiyaowei@cmss.chinamobile.com, hpa@zytor.com,
	bhelgaas@google.com, tglx@linutronix.de, dyoung@redhat.com,
	akpm@linux-foundation.org, dan.j.williams@intel.com
Subject: Re: [PATCH v7 RESEND 2/4] kexec: allocate unencrypted control pages for kdump in case SME is enabled
Date: Thu, 27 Sep 2018 18:53:23 +0200	[thread overview]
Message-ID: <20180927165323.GC19779@zn.tnic> (raw)
In-Reply-To: <20180927071954.29615-3-lijiang@redhat.com>

On Thu, Sep 27, 2018 at 03:19:52PM +0800, Lianbo Jiang wrote:
> When SME is enabled in the first kernel, we will allocate unencrypted pages
> for kdump in order to be able to boot the kdump kernel like kexec.

This is not what the commit does - it marks the control pages as
decrypted when SME. Why doesn't the commit message state that and why is
this being done?

> Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  kernel/kexec_core.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index 23a83a4da38a..e7efcd1a977b 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -471,6 +471,16 @@ static struct page *kimage_alloc_crash_control_pages(struct kimage *image,
>  		}
>  	}
>  
> +	if (pages) {
> +		/*
> +		 * For kdump, we need to ensure that these pages are
> +		 * unencrypted pages if SME is enabled.

Remember to always call unencrypted pages "decrypted" - this is the
convention we agreed upon and it should keep the confusion level at
minimum to others staring at this code.

> +		 * By the way, it is unnecessary to call the arch_
> +		 * kexec_pre_free_pages(), which will make the code
> +		 * become more simple.
> +		 */

This second sentence I don't understand...

> +		arch_kexec_post_alloc_pages(page_address(pages), 1 << order, 0);
> +	}
>  	return pages;
>  }
>  
> @@ -867,6 +877,7 @@ static int kimage_load_crash_segment(struct kimage *image,
>  			result  = -ENOMEM;
>  			goto out;
>  		}
> +		arch_kexec_post_alloc_pages(page_address(page), 1, 0);
>  		ptr = kmap(page);
>  		ptr += maddr & ~PAGE_MASK;
>  		mchunk = min_t(size_t, mbytes,
> @@ -884,6 +895,7 @@ static int kimage_load_crash_segment(struct kimage *image,
>  			result = copy_from_user(ptr, buf, uchunk);
>  		kexec_flush_icache_page(page);
>  		kunmap(page);
> +		arch_kexec_pre_free_pages(page_address(page), 1);
>  		if (result) {
>  			result = -EFAULT;
>  			goto out;
> -- 
> 2.17.1
> 

-- 
Regards/Gruss,
    Boris.

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

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec