2023-10-10 14:52:48

by Alexander Shishkin

[permalink] [raw]
Subject: [PATCH v3 0/2] x86/sev: Generalize sev_setup_arch()

Hi,

v3:
* rebased
* moved mem_encrypt_setup_arch under a common #ifdef CONFIG_X86_MEM_ENCRYPT
per Tom's comment

v2: [1]
* added 2/2, which is a minor cleanup

v1: [0]

The main intention of this is to move sev_setup_arch() to mem_encrypt.c
to reflect the fact that it's not SEV-specific, but covers TDX as well,
although unintentionally.

While looking at it, I also noticed that mem_encrypt_amd.c still
includes virtio_config.h, which was needed for the code that since got
moved to a different place (and even there doesn't require the include
any more).

[0] https://lore.kernel.org/all/[email protected]/
[1] https://lore.kernel.org/all/[email protected]/

Alexander Shishkin (2):
x86/sev: Move sev_setup_arch() to mem_encrypt.c
x86/sev: Drop unneeded include

arch/x86/include/asm/mem_encrypt.h | 4 ++--
arch/x86/kernel/setup.c | 2 +-
arch/x86/mm/mem_encrypt.c | 34 ++++++++++++++++++++++++++++
arch/x86/mm/mem_encrypt_amd.c | 36 ------------------------------
4 files changed, 37 insertions(+), 39 deletions(-)

--
2.40.1


2023-10-10 14:53:00

by Alexander Shishkin

[permalink] [raw]
Subject: [PATCH v3 1/2] x86/sev: Move sev_setup_arch() to mem_encrypt.c

Since commit 4d96f9109109b ("x86/sev: Replace occurrences of
sev_active() with cc_platform_has()"), the SWIOTLB bounce buffer size
adjustment and restricted virtio memory setting also inadvertently apply
to TDX: the code is using cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT) as
a gatekeeping condition, which is also true for TDX, and this is also what
we want.

To reflect this, move the corresponding code to generic mem_encrypt.c.
No functional changes intended.

Signed-off-by: Alexander Shishkin <[email protected]>
---
arch/x86/include/asm/mem_encrypt.h | 4 ++--
arch/x86/kernel/setup.c | 2 +-
arch/x86/mm/mem_encrypt.c | 34 +++++++++++++++++++++++++++++
arch/x86/mm/mem_encrypt_amd.c | 35 ------------------------------
4 files changed, 37 insertions(+), 38 deletions(-)

diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
index 473b16d73b47..359ada486fa9 100644
--- a/arch/x86/include/asm/mem_encrypt.h
+++ b/arch/x86/include/asm/mem_encrypt.h
@@ -19,8 +19,10 @@

#ifdef CONFIG_X86_MEM_ENCRYPT
void __init mem_encrypt_init(void);
+void __init mem_encrypt_setup_arch(void);
#else
static inline void mem_encrypt_init(void) { }
+static inline void __init mem_encrypt_setup_arch(void) { }
#endif

#ifdef CONFIG_AMD_MEM_ENCRYPT
@@ -43,7 +45,6 @@ void __init sme_map_bootdata(char *real_mode_data);
void __init sme_unmap_bootdata(char *real_mode_data);

void __init sme_early_init(void);
-void __init sev_setup_arch(void);

void __init sme_encrypt_kernel(struct boot_params *bp);
void __init sme_enable(struct boot_params *bp);
@@ -73,7 +74,6 @@ static inline void __init sme_map_bootdata(char *real_mode_data) { }
static inline void __init sme_unmap_bootdata(char *real_mode_data) { }

static inline void __init sme_early_init(void) { }
-static inline void __init sev_setup_arch(void) { }

static inline void __init sme_encrypt_kernel(struct boot_params *bp) { }
static inline void __init sme_enable(struct boot_params *bp) { }
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 3e05ecf8cf8d..ccd3ad29a1dc 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1120,7 +1120,7 @@ void __init setup_arch(char **cmdline_p)
* Needs to run after memblock setup because it needs the physical
* memory size.
*/
- sev_setup_arch();
+ mem_encrypt_setup_arch();

efi_fake_memmap();
efi_find_mirror();
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index 9f27e14e185f..c290c55b632b 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -12,6 +12,7 @@
#include <linux/swiotlb.h>
#include <linux/cc_platform.h>
#include <linux/mem_encrypt.h>
+#include <linux/virtio_anchor.h>

/* Override for DMA direct allocation check - ARCH_HAS_FORCE_DMA_UNENCRYPTED */
bool force_dma_unencrypted(struct device *dev)
@@ -86,3 +87,36 @@ void __init mem_encrypt_init(void)

print_mem_encrypt_feature_info();
}
+
+void __init mem_encrypt_setup_arch(void)
+{
+ phys_addr_t total_mem = memblock_phys_mem_size();
+ unsigned long size;
+
+ if (!cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
+ return;
+
+ /*
+ * For SEV and TDX, all DMA has to occur via shared/unencrypted pages.
+ * Kernel uses SWIOTLB to make this happen without changing device
+ * drivers. However, depending on the workload being run, the
+ * default 64MB of SWIOTLB may not be enough and SWIOTLB may
+ * run out of buffers for DMA, resulting in I/O errors and/or
+ * performance degradation especially with high I/O workloads.
+ *
+ * Adjust the default size of SWIOTLB using a percentage of guest
+ * memory for SWIOTLB buffers. Also, as the SWIOTLB bounce buffer
+ * memory is allocated from low memory, ensure that the adjusted size
+ * is within the limits of low available memory.
+ *
+ * The percentage of guest memory used here for SWIOTLB buffers
+ * is more of an approximation of the static adjustment which
+ * 64MB for <1G, and ~128M to 256M for 1G-to-4G, i.e., the 6%
+ */
+ size = total_mem * 6 / 100;
+ size = clamp_val(size, IO_TLB_DEFAULT_SIZE, SZ_1G);
+ swiotlb_adjust_size(size);
+
+ /* Set restricted memory access for virtio. */
+ virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc);
+}
diff --git a/arch/x86/mm/mem_encrypt_amd.c b/arch/x86/mm/mem_encrypt_amd.c
index 6faea41e99b6..62dde75d41fa 100644
--- a/arch/x86/mm/mem_encrypt_amd.c
+++ b/arch/x86/mm/mem_encrypt_amd.c
@@ -20,7 +20,6 @@
#include <linux/bitops.h>
#include <linux/dma-mapping.h>
#include <linux/virtio_config.h>
-#include <linux/virtio_anchor.h>
#include <linux/cc_platform.h>

#include <asm/tlbflush.h>
@@ -215,40 +214,6 @@ void __init sme_map_bootdata(char *real_mode_data)
__sme_early_map_unmap_mem(__va(cmdline_paddr), COMMAND_LINE_SIZE, true);
}

-void __init sev_setup_arch(void)
-{
- phys_addr_t total_mem = memblock_phys_mem_size();
- unsigned long size;
-
- if (!cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
- return;
-
- /*
- * For SEV, all DMA has to occur via shared/unencrypted pages.
- * SEV uses SWIOTLB to make this happen without changing device
- * drivers. However, depending on the workload being run, the
- * default 64MB of SWIOTLB may not be enough and SWIOTLB may
- * run out of buffers for DMA, resulting in I/O errors and/or
- * performance degradation especially with high I/O workloads.
- *
- * Adjust the default size of SWIOTLB for SEV guests using
- * a percentage of guest memory for SWIOTLB buffers.
- * Also, as the SWIOTLB bounce buffer memory is allocated
- * from low memory, ensure that the adjusted size is within
- * the limits of low available memory.
- *
- * The percentage of guest memory used here for SWIOTLB buffers
- * is more of an approximation of the static adjustment which
- * 64MB for <1G, and ~128M to 256M for 1G-to-4G, i.e., the 6%
- */
- size = total_mem * 6 / 100;
- size = clamp_val(size, IO_TLB_DEFAULT_SIZE, SZ_1G);
- swiotlb_adjust_size(size);
-
- /* Set restricted memory access for virtio. */
- virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc);
-}
-
static unsigned long pg_level_to_pfn(int level, pte_t *kpte, pgprot_t *ret_prot)
{
unsigned long pfn = 0;
--
2.40.1

2023-10-10 14:53:08

by Alexander Shishkin

[permalink] [raw]
Subject: [PATCH v3 2/2] x86/sev: Drop unneeded include

Commit 20f07a044a76 ("x86/sev: Move common memory encryption code to
mem_encrypt.c") forgot to remove the include of virtio_config.h from
mem_encrypt_amd.c when it moved the related code to mem_encrypt.c (from
where this include subsequently got removed by a later commit).

Remove it now. No functional changes.

Signed-off-by: Alexander Shishkin <[email protected]>
---
arch/x86/mm/mem_encrypt_amd.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/arch/x86/mm/mem_encrypt_amd.c b/arch/x86/mm/mem_encrypt_amd.c
index 62dde75d41fa..a68f2dda0948 100644
--- a/arch/x86/mm/mem_encrypt_amd.c
+++ b/arch/x86/mm/mem_encrypt_amd.c
@@ -19,7 +19,6 @@
#include <linux/kernel.h>
#include <linux/bitops.h>
#include <linux/dma-mapping.h>
-#include <linux/virtio_config.h>
#include <linux/cc_platform.h>

#include <asm/tlbflush.h>
--
2.40.1

2023-10-10 20:47:12

by Tom Lendacky

[permalink] [raw]
Subject: Re: [PATCH v3 0/2] x86/sev: Generalize sev_setup_arch()

On 10/10/23 09:52, Alexander Shishkin wrote:
> Hi,
>
> v3:
> * rebased
> * moved mem_encrypt_setup_arch under a common #ifdef CONFIG_X86_MEM_ENCRYPT
> per Tom's comment
>
> v2: [1]
> * added 2/2, which is a minor cleanup
>
> v1: [0]
>
> The main intention of this is to move sev_setup_arch() to mem_encrypt.c
> to reflect the fact that it's not SEV-specific, but covers TDX as well,
> although unintentionally.
>
> While looking at it, I also noticed that mem_encrypt_amd.c still
> includes virtio_config.h, which was needed for the code that since got
> moved to a different place (and even there doesn't require the include
> any more).
>
> [0] https://lore.kernel.org/all/[email protected]/
> [1] https://lore.kernel.org/all/[email protected]/
>
> Alexander Shishkin (2):
> x86/sev: Move sev_setup_arch() to mem_encrypt.c
> x86/sev: Drop unneeded include
>
> arch/x86/include/asm/mem_encrypt.h | 4 ++--
> arch/x86/kernel/setup.c | 2 +-
> arch/x86/mm/mem_encrypt.c | 34 ++++++++++++++++++++++++++++
> arch/x86/mm/mem_encrypt_amd.c | 36 ------------------------------
> 4 files changed, 37 insertions(+), 39 deletions(-)
>

For the series:

Reviewed-by: Tom Lendacky <[email protected]>

2023-10-11 08:30:42

by tip-bot2 for Tony Luck

[permalink] [raw]
Subject: [tip: x86/mm] x86/sev: Move sev_setup_arch() to mem_encrypt.c

The following commit has been merged into the x86/mm branch of tip:

Commit-ID: 6e74b125155dc8c747d76fb45d8e6d20e9e4fb4d
Gitweb: https://git.kernel.org/tip/6e74b125155dc8c747d76fb45d8e6d20e9e4fb4d
Author: Alexander Shishkin <[email protected]>
AuthorDate: Tue, 10 Oct 2023 17:52:19 +03:00
Committer: Ingo Molnar <[email protected]>
CommitterDate: Wed, 11 Oct 2023 10:15:47 +02:00

x86/sev: Move sev_setup_arch() to mem_encrypt.c

Since commit:

4d96f9109109b ("x86/sev: Replace occurrences of sev_active() with cc_platform_has()")

... the SWIOTLB bounce buffer size adjustment and restricted virtio memory
setting also inadvertently apply to TDX: the code is using
cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT) as a gatekeeping condition,
which is also true for TDX, and this is also what we want.

To reflect this, move the corresponding code to generic mem_encrypt.c.

No functional changes intended.

Signed-off-by: Alexander Shishkin <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Reviewed-by: Tom Lendacky <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/include/asm/mem_encrypt.h | 4 +--
arch/x86/kernel/setup.c | 2 +-
arch/x86/mm/mem_encrypt.c | 34 ++++++++++++++++++++++++++++-
arch/x86/mm/mem_encrypt_amd.c | 35 +-----------------------------
4 files changed, 37 insertions(+), 38 deletions(-)

diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
index 473b16d..359ada4 100644
--- a/arch/x86/include/asm/mem_encrypt.h
+++ b/arch/x86/include/asm/mem_encrypt.h
@@ -19,8 +19,10 @@

#ifdef CONFIG_X86_MEM_ENCRYPT
void __init mem_encrypt_init(void);
+void __init mem_encrypt_setup_arch(void);
#else
static inline void mem_encrypt_init(void) { }
+static inline void __init mem_encrypt_setup_arch(void) { }
#endif

#ifdef CONFIG_AMD_MEM_ENCRYPT
@@ -43,7 +45,6 @@ void __init sme_map_bootdata(char *real_mode_data);
void __init sme_unmap_bootdata(char *real_mode_data);

void __init sme_early_init(void);
-void __init sev_setup_arch(void);

void __init sme_encrypt_kernel(struct boot_params *bp);
void __init sme_enable(struct boot_params *bp);
@@ -73,7 +74,6 @@ static inline void __init sme_map_bootdata(char *real_mode_data) { }
static inline void __init sme_unmap_bootdata(char *real_mode_data) { }

static inline void __init sme_early_init(void) { }
-static inline void __init sev_setup_arch(void) { }

static inline void __init sme_encrypt_kernel(struct boot_params *bp) { }
static inline void __init sme_enable(struct boot_params *bp) { }
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index b9145a6..ec44dc5 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1124,7 +1124,7 @@ void __init setup_arch(char **cmdline_p)
* Needs to run after memblock setup because it needs the physical
* memory size.
*/
- sev_setup_arch();
+ mem_encrypt_setup_arch();

efi_fake_memmap();
efi_find_mirror();
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index 9f27e14..c290c55 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -12,6 +12,7 @@
#include <linux/swiotlb.h>
#include <linux/cc_platform.h>
#include <linux/mem_encrypt.h>
+#include <linux/virtio_anchor.h>

/* Override for DMA direct allocation check - ARCH_HAS_FORCE_DMA_UNENCRYPTED */
bool force_dma_unencrypted(struct device *dev)
@@ -86,3 +87,36 @@ void __init mem_encrypt_init(void)

print_mem_encrypt_feature_info();
}
+
+void __init mem_encrypt_setup_arch(void)
+{
+ phys_addr_t total_mem = memblock_phys_mem_size();
+ unsigned long size;
+
+ if (!cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
+ return;
+
+ /*
+ * For SEV and TDX, all DMA has to occur via shared/unencrypted pages.
+ * Kernel uses SWIOTLB to make this happen without changing device
+ * drivers. However, depending on the workload being run, the
+ * default 64MB of SWIOTLB may not be enough and SWIOTLB may
+ * run out of buffers for DMA, resulting in I/O errors and/or
+ * performance degradation especially with high I/O workloads.
+ *
+ * Adjust the default size of SWIOTLB using a percentage of guest
+ * memory for SWIOTLB buffers. Also, as the SWIOTLB bounce buffer
+ * memory is allocated from low memory, ensure that the adjusted size
+ * is within the limits of low available memory.
+ *
+ * The percentage of guest memory used here for SWIOTLB buffers
+ * is more of an approximation of the static adjustment which
+ * 64MB for <1G, and ~128M to 256M for 1G-to-4G, i.e., the 6%
+ */
+ size = total_mem * 6 / 100;
+ size = clamp_val(size, IO_TLB_DEFAULT_SIZE, SZ_1G);
+ swiotlb_adjust_size(size);
+
+ /* Set restricted memory access for virtio. */
+ virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc);
+}
diff --git a/arch/x86/mm/mem_encrypt_amd.c b/arch/x86/mm/mem_encrypt_amd.c
index 6faea41..62dde75 100644
--- a/arch/x86/mm/mem_encrypt_amd.c
+++ b/arch/x86/mm/mem_encrypt_amd.c
@@ -20,7 +20,6 @@
#include <linux/bitops.h>
#include <linux/dma-mapping.h>
#include <linux/virtio_config.h>
-#include <linux/virtio_anchor.h>
#include <linux/cc_platform.h>

#include <asm/tlbflush.h>
@@ -215,40 +214,6 @@ void __init sme_map_bootdata(char *real_mode_data)
__sme_early_map_unmap_mem(__va(cmdline_paddr), COMMAND_LINE_SIZE, true);
}

-void __init sev_setup_arch(void)
-{
- phys_addr_t total_mem = memblock_phys_mem_size();
- unsigned long size;
-
- if (!cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
- return;
-
- /*
- * For SEV, all DMA has to occur via shared/unencrypted pages.
- * SEV uses SWIOTLB to make this happen without changing device
- * drivers. However, depending on the workload being run, the
- * default 64MB of SWIOTLB may not be enough and SWIOTLB may
- * run out of buffers for DMA, resulting in I/O errors and/or
- * performance degradation especially with high I/O workloads.
- *
- * Adjust the default size of SWIOTLB for SEV guests using
- * a percentage of guest memory for SWIOTLB buffers.
- * Also, as the SWIOTLB bounce buffer memory is allocated
- * from low memory, ensure that the adjusted size is within
- * the limits of low available memory.
- *
- * The percentage of guest memory used here for SWIOTLB buffers
- * is more of an approximation of the static adjustment which
- * 64MB for <1G, and ~128M to 256M for 1G-to-4G, i.e., the 6%
- */
- size = total_mem * 6 / 100;
- size = clamp_val(size, IO_TLB_DEFAULT_SIZE, SZ_1G);
- swiotlb_adjust_size(size);
-
- /* Set restricted memory access for virtio. */
- virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc);
-}
-
static unsigned long pg_level_to_pfn(int level, pte_t *kpte, pgprot_t *ret_prot)
{
unsigned long pfn = 0;

2023-10-11 08:31:21

by tip-bot2 for Tony Luck

[permalink] [raw]
Subject: [tip: x86/mm] x86/sev: Drop unneeded #include

The following commit has been merged into the x86/mm branch of tip:

Commit-ID: d6f274b7c8ac52abc81e898b62c3ff63fbeb11b8
Gitweb: https://git.kernel.org/tip/d6f274b7c8ac52abc81e898b62c3ff63fbeb11b8
Author: Alexander Shishkin <[email protected]>
AuthorDate: Tue, 10 Oct 2023 17:52:20 +03:00
Committer: Ingo Molnar <[email protected]>
CommitterDate: Wed, 11 Oct 2023 10:15:47 +02:00

x86/sev: Drop unneeded #include

Commit:

20f07a044a76 ("x86/sev: Move common memory encryption code to mem_encrypt.c")

... forgot to remove the include of virtio_config.h from mem_encrypt_amd.c
when it moved the related code to mem_encrypt.c (from where this include
subsequently got removed by a later commit).

Remove it now.

Signed-off-by: Alexander Shishkin <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Reviewed-by: Tom Lendacky <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/mm/mem_encrypt_amd.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/arch/x86/mm/mem_encrypt_amd.c b/arch/x86/mm/mem_encrypt_amd.c
index 62dde75..a68f2dd 100644
--- a/arch/x86/mm/mem_encrypt_amd.c
+++ b/arch/x86/mm/mem_encrypt_amd.c
@@ -19,7 +19,6 @@
#include <linux/kernel.h>
#include <linux/bitops.h>
#include <linux/dma-mapping.h>
-#include <linux/virtio_config.h>
#include <linux/cc_platform.h>

#include <asm/tlbflush.h>