linux-kernel - Re: [PATCH 19/26] x86/tdx: Make pages shared in ioremap()

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87c288d6-9bf8-5a94-a628-1e0aaa7de690@amd.com>
Date:   Wed, 22 Dec 2021 11:26:59 -0600
From:   Tom Lendacky <thomas.lendacky@....com>
To:     "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
        dave.hansen@...el.com, luto@...nel.org, peterz@...radead.org
Cc:     sathyanarayanan.kuppuswamy@...ux.intel.com, aarcange@...hat.com,
        ak@...ux.intel.com, dan.j.williams@...el.com, david@...hat.com,
        hpa@...or.com, jgross@...e.com, jmattson@...gle.com,
        joro@...tes.org, jpoimboe@...hat.com, knsathya@...nel.org,
        pbonzini@...hat.com, sdeep@...are.com, seanjc@...gle.com,
        tony.luck@...el.com, vkuznets@...hat.com, wanpengli@...cent.com,
        x86@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 19/26] x86/tdx: Make pages shared in ioremap()

On 12/14/21 9:02 AM, Kirill A. Shutemov wrote:
> In TDX guests, guest memory is protected from host access. If a guest
> performs I/O, it needs to explicitly share the I/O memory with the host.
> 
> Make all ioremap()ed pages that are not backed by normal memory
> (IORES_DESC_NONE or IORES_DESC_RESERVED) mapped as shared.
> 
> Since TDX memory encryption support is similar to AMD SEV architecture,
> reuse the infrastructure from AMD SEV code. Introduce CC_ATTR_GUEST_TDX
> to add TDX-specific changes to the AMD SEV/SME memory encryption code.
> 
> Add tdx_shared_mask() interface to get the TDX guest shared bitmask.
> 
> pgprot_decrypted() is used by drivers (i915, virtio_gpu, vfio). Export
> both pgprot_encrypted() and pgprot_decrypted().
> 

> --- a/arch/x86/mm/mem_encrypt.c
> +++ b/arch/x86/mm/mem_encrypt.c
> @@ -14,6 +14,33 @@
>   #include <linux/mem_encrypt.h>
>   #include <linux/virtio_config.h>
>   
> +#include <asm/tdx.h>
> +
> +/*
> + * Set or unset encryption attribute in vendor agnostic way.
> + */
> +pgprot_t pgprot_cc_encrypted(pgprot_t prot)
> +{
> +	if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT))
> +		return __pgprot(__sme_set(pgprot_val(prot)));
> +	else if (cc_platform_has(CC_ATTR_GUEST_TDX))
> +		return __pgprot(pgprot_val(prot) & ~tdx_shared_mask());
> +

Hmmm... I believe this breaks SEV guests. __sme_set() uses sme_me_mask 
which is used for both SME and SEV. With the current checks, an SEV guest 
will end up never setting an encrypted address through this path. Ditto 
below on the decrypted path.

Thanks,
Tom

> +	return prot;
> +}
> +EXPORT_SYMBOL_GPL(pgprot_cc_encrypted);
> +
> +pgprot_t pgprot_cc_decrypted(pgprot_t prot)
> +{
> +	if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT))
> +		return __pgprot(__sme_clr(pgprot_val(prot)));
> +	else if (cc_platform_has(CC_ATTR_GUEST_TDX))
> +		return __pgprot(pgprot_val(prot) | tdx_shared_mask());
> +
> +	return prot;
> +}
> +EXPORT_SYMBOL_GPL(pgprot_cc_decrypted);
> +
>   /* Override for DMA direct allocation check - ARCH_HAS_FORCE_DMA_UNENCRYPTED */
>   bool force_dma_unencrypted(struct device *dev)
>   {
> diff --git a/include/linux/cc_platform.h b/include/linux/cc_platform.h
> index 63b15108bc85..5fed077cc5f4 100644
> --- a/include/linux/cc_platform.h
> +++ b/include/linux/cc_platform.h
> @@ -82,6 +82,15 @@ enum cc_attr {
>   	 * Examples include TDX Guest.
>   	 */
>   	CC_ATTR_HOTPLUG_DISABLED,
> +
> +	/**
> +	 * @CC_ATTR_GUEST_TDX: Trust Domain Extension Support
> +	 *
> +	 * The platform/OS is running as a TDX guest/virtual machine.
> +	 *
> +	 * Examples include Intel TDX.
> +	 */
> +	CC_ATTR_GUEST_TDX = 0x100,
>   };
>   
>   #ifdef CONFIG_ARCH_HAS_CC_PLATFORM
>