Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v5 07/16] x86/kvm: Use bounce buffers for TD guest
To:     Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@linux.intel.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Peter Zijlstra <peterz@infradead.org>,
        Andy Lutomirski <luto@kernel.org>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Richard Henderson <rth@twiddle.net>,
        Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
        James E J Bottomley <James.Bottomley@HansenPartnership.com>,
        Helge Deller <deller@gmx.de>,
        "David S . Miller" <davem@davemloft.net>,
        Arnd Bergmann <arnd@arndb.de>,
        Jonathan Corbet <corbet@lwn.net>,
        "Michael S . Tsirkin" <mst@redhat.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        David Hildenbrand <david@redhat.com>,
        Andrea Arcangeli <aarcange@redhat.com>,
        Josh Poimboeuf <jpoimboe@redhat.com>
Cc:     Peter H Anvin <hpa@zytor.com>, Dave Hansen <dave.hansen@intel.com>,
        Tony Luck <tony.luck@intel.com>,
        Dan Williams <dan.j.williams@intel.com>,
        Andi Kleen <ak@linux.intel.com>,
        Kirill Shutemov <kirill.shutemov@linux.intel.com>,
        Sean Christopherson <seanjc@google.com>,
        Kuppuswamy Sathyanarayanan <knsathya@kernel.org>,
        x86@kernel.org, linux-kernel@vger.kernel.org,
        linux-pci@vger.kernel.org, linux-alpha@vger.kernel.org,
        linux-mips@vger.kernel.org, linux-parisc@vger.kernel.org,
        sparclinux@vger.kernel.org, linux-arch@vger.kernel.org,
        linux-doc@vger.kernel.org,
        virtualization@lists.linux-foundation.org
References: <20211009003711.1390019-1-sathyanarayanan.kuppuswamy@linux.intel.com>
 <20211009003711.1390019-8-sathyanarayanan.kuppuswamy@linux.intel.com>
From:   Tom Lendacky <thomas.lendacky@amd.com>
Message-ID: <42f17b60-9bd4-a8bc-5164-d960e54cd30b@amd.com>
Date:   Wed, 20 Oct 2021 11:39:01 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.13.0
In-Reply-To: <20211009003711.1390019-8-sathyanarayanan.kuppuswamy@linux.intel.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?UldBZDZQNEdKWFI2Y0k0cHZRcUNwNVoyVjdMTGticmFUMkVGV0lEczN1QUxM?=
 =?utf-8?B?bFh3dVlwdUplZVd1cUdYQTJqZ09iNmpNWUJiUVBvZFN1dmhvQVpVaXpqVUFO?=
 =?utf-8?B?VDFFZnQ5eDJMdGxiUXc0d1VDUVhGcVNCWkhqYTltaVFmM2pJL0lGVEZ6aTIw?=
 =?utf-8?B?ZVVTS2dGb3pqRllXM0RIbTVyYmt4am5Rbkl5a0hVVmVPQ0lMQ3hISmt2SkJ1?=
 =?utf-8?B?YVNVRWVqVExzcFFMOE5XbDROQzlYeG45K3k4d1lwNW12aHBGVEJPNlJFcStx?=
 =?utf-8?B?UlNCZHczeHNIRTdFVlArQS9jalpNSDFTZExvYUEzKzBaRGY0cjBOc2ZpbmFj?=
 =?utf-8?B?WVNxeTJjd0Nkc2lGeVJ5ejFGWWt1REVYUjczUFQvTXgzZVArczg5UEl1aWVm?=
 =?utf-8?B?djkxR0JxM1h2ZGtqQzllMjRQOWIxeHExa1EvUEF4T2drbWI5MkFkUUoyK3ZD?=
 =?utf-8?B?a01NenRHeHFTRnBEQTdpellLVzdqbjZkWk5PZ3VOVXJNcHdaUnJjOGE1ekVL?=
 =?utf-8?B?WXNTcW5DSCs2S2tCSGhhYmI2SjVMeUNJVjhkbm9GRThya1FRdjdmWndaUXRJ?=
 =?utf-8?B?TGtaUGVuczVVVTR4NG4rb0tOS1cybGFZSE5HWEdPQ1FoellNc3dNWWdaeEtH?=
 =?utf-8?B?ckZITFlOVW1pT3FWMFRMdlBHRUFybG1MOXVDbi9OMWlaVGZiSDMwMnVtL2o0?=
 =?utf-8?B?ZGFjN1dlYVd6a0ZuMUJCQndXK2xQZVc4RWIrbUlyclV6VFNRZWFRRWVuanov?=
 =?utf-8?B?MlJzdHFVek9KNVR1eUJKcUM3NlBEUVJ6N2l3VGRZVGdVWStVLzZ2cm04cGNF?=
 =?utf-8?B?Y2NoS2tZMWszK3VMbVEralJaTG1xSjZzL3BlYzh3dks5M05POC9wU3BSeUVC?=
 =?utf-8?B?ZlYydnIyVFRiMHFjZi9GSWpjdmJxZmZ3dGNzbWozZ2E0U1MxSHpnRHFMSTh3?=
 =?utf-8?B?SllRTVU0dmY0REtWTW1tWG4wS1lOc3l4RGlxQVk1OXpZL1dJQlhFaG5wYTlj?=
 =?utf-8?B?bU9wZE9QTlV2dFg1MTVrM0NTS25jVkFvQkxhaTU4MVZPdk80YktKZDdncmFr?=
 =?utf-8?B?TWpqKzZHQ1d5RnczeXdjMVZFMFMyS3A4MXMvU3daVmJFdldtMnMxd3FiZSti?=
 =?utf-8?B?Y3ZYWUFqZGdzMis5S25HUlpEeFM3V0RNeldCWnE4dFVydktDMXl2aCtnMVdZ?=
 =?utf-8?B?SVJFNjhHbmhqUHd2NHZnSURYcmVyRzA2YjR1dkRRb2hMOStPa1ZpNzhJNnVw?=
 =?utf-8?B?bU5sTjFWdWZKVUVOWDcrUGVjZ3h0bkJVN21ZZ3JMbG5yeXZsR1ZiK3pvTjRU?=
 =?utf-8?B?ZlpvZTdmL3VUOE9rdXVaM3J2clkveVlsaDRqVWFLVERvWjBjRDZCaDVtK3N3?=
 =?utf-8?B?VEgvLzFZVTNoMUUxMjdnYTQ4MEROelVwSngzVlRMOWFIbHlTUWZMSTdUdW0w?=
 =?utf-8?B?a0VzRmZpSEhvUUFGV01vVXRnQlFTdkxSdUJidE4rZHdEQ1d6OUJKQzFSWFBp?=
 =?utf-8?B?ZmdoWitRRGdDUTdhbFdxOUFJcFR5dCtLdVVlQTdLQ1BJNnY0d0RHZ2lOZVJl?=
 =?utf-8?B?aUhOb0daS0lxSnFuQnFtUWo3M1F1NlUrOWFNRjRHaGhZcFM0YWVROW01eDV6?=
 =?utf-8?B?RHlaMm0rZ29jalVld3hkdlQ4ZmM2MHptWFFTRW9yMitYMU5DSmhLMlo0S3py?=
 =?utf-8?B?ekU5UnBNcVZ1dEZhd1BxQTJWVTNKZmxma096d3U0N1RINDA1N3dKQnlUZUQx?=
 =?utf-8?Q?AqbbaEDkAi0YGUPIBTKLaO6OQNg6yT/lBreAHfI?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 099ea61d-b18f-4f31-07c0-08d993e8216f
X-MS-Exchange-CrossTenant-AuthSource: DM4PR12MB5229.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Oct 2021 16:39:05.4904
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: tlendack@amd.com
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB5118
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 10/8/21 7:37 PM, Kuppuswamy Sathyanarayanan wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> 
> Intel TDX doesn't allow VMM to directly access guest private memory.
> Any memory that is required for communication with VMM must be shared
> explicitly. The same rule applies for any DMA to and from TDX guest.
> All DMA pages had to marked as shared pages. A generic way to achieve
> this without any changes to device drivers is to use the SWIOTLB
> framework.
> 
> This method of handling is similar to AMD SEV. So extend this support
> for TDX guest as well. Also since there are some common code between
> AMD SEV and TDX guest in mem_encrypt_init(), move it to
> mem_encrypt_common.c and call AMD specific init function from it
> 
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Reviewed-by: Andi Kleen <ak@linux.intel.com>
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> ---
> 
> Changes since v4:
>   * Replaced prot_guest_has() with cc_guest_has().
> 
> Changes since v3:
>   * Rebased on top of Tom Lendacky's protected guest
>     changes (https://lore.kernel.org/patchwork/cover/1468760/)
> 
> Changes since v1:
>   * Removed sme_me_mask check for amd_mem_encrypt_init() in mem_encrypt_init().
> 
>   arch/x86/include/asm/mem_encrypt_common.h |  3 +++
>   arch/x86/kernel/tdx.c                     |  2 ++
>   arch/x86/mm/mem_encrypt.c                 |  5 +----
>   arch/x86/mm/mem_encrypt_common.c          | 14 ++++++++++++++
>   4 files changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/include/asm/mem_encrypt_common.h b/arch/x86/include/asm/mem_encrypt_common.h
> index 697bc40a4e3d..bc90e565bce4 100644
> --- a/arch/x86/include/asm/mem_encrypt_common.h
> +++ b/arch/x86/include/asm/mem_encrypt_common.h
> @@ -8,11 +8,14 @@
>   
>   #ifdef CONFIG_AMD_MEM_ENCRYPT
>   bool amd_force_dma_unencrypted(struct device *dev);
> +void __init amd_mem_encrypt_init(void);
>   #else /* CONFIG_AMD_MEM_ENCRYPT */
>   static inline bool amd_force_dma_unencrypted(struct device *dev)
>   {
>   	return false;
>   }
> +
> +static inline void amd_mem_encrypt_init(void) {}
>   #endif /* CONFIG_AMD_MEM_ENCRYPT */
>   
>   #endif
> diff --git a/arch/x86/kernel/tdx.c b/arch/x86/kernel/tdx.c
> index 433f366ca25c..ce8e3019b812 100644
> --- a/arch/x86/kernel/tdx.c
> +++ b/arch/x86/kernel/tdx.c
> @@ -12,6 +12,7 @@
>   #include <asm/insn.h>
>   #include <asm/insn-eval.h>
>   #include <linux/sched/signal.h> /* force_sig_fault() */
> +#include <linux/swiotlb.h>
>   
>   /* TDX Module call Leaf IDs */
>   #define TDX_GET_INFO			1
> @@ -577,6 +578,7 @@ void __init tdx_early_init(void)
>   	pv_ops.irq.halt = tdx_halt;
>   
>   	legacy_pic = &null_legacy_pic;
> +	swiotlb_force = SWIOTLB_FORCE;
>   
>   	cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "tdx:cpu_hotplug",
>   			  NULL, tdx_cpu_offline_prepare);
> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
> index 5d7fbed73949..8385bc4565e9 100644
> --- a/arch/x86/mm/mem_encrypt.c
> +++ b/arch/x86/mm/mem_encrypt.c
> @@ -438,14 +438,11 @@ static void print_mem_encrypt_feature_info(void)
>   }
>   
>   /* Architecture __weak replacement functions */
> -void __init mem_encrypt_init(void)
> +void __init amd_mem_encrypt_init(void)
>   {
>   	if (!sme_me_mask)
>   		return;
>   
> -	/* Call into SWIOTLB to update the SWIOTLB DMA buffers */
> -	swiotlb_update_mem_attributes();
> -
>   	/*
>   	 * With SEV, we need to unroll the rep string I/O instructions,
>   	 * but SEV-ES supports them through the #VC handler.
> diff --git a/arch/x86/mm/mem_encrypt_common.c b/arch/x86/mm/mem_encrypt_common.c
> index 119a9056efbb..6fe44c6cb753 100644
> --- a/arch/x86/mm/mem_encrypt_common.c
> +++ b/arch/x86/mm/mem_encrypt_common.c
> @@ -10,6 +10,7 @@
>   #include <asm/mem_encrypt_common.h>
>   #include <linux/dma-mapping.h>
>   #include <linux/cc_platform.h>
> +#include <linux/swiotlb.h>
>   
>   /* Override for DMA direct allocation check - ARCH_HAS_FORCE_DMA_UNENCRYPTED */
>   bool force_dma_unencrypted(struct device *dev)
> @@ -24,3 +25,16 @@ bool force_dma_unencrypted(struct device *dev)
>   
>   	return false;
>   }
> +
> +/* Architecture __weak replacement functions */
> +void __init mem_encrypt_init(void)
> +{
> +	/*
> +	 * For TDX guest or SEV/SME, call into SWIOTLB to update
> +	 * the SWIOTLB DMA buffers
> +	 */
> +	if (sme_me_mask || cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))

Can't you just make this:

	if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))

SEV will return true if sme_me_mask is not zero and TDX should only return 
true if it is TDX guest, right?

Thanks,
Tom

> +		swiotlb_update_mem_attributes();
> +
> +	amd_mem_encrypt_init();
> +}
>