2024-01-26 22:16:55

by Tom Lendacky

[permalink] [raw]
Subject: [PATCH 01/11] x86/sev: Rename snp_init() in the boot/compressed/sev.c file

The snp_init() function is local to the boot/compressed/sev.c file and is
not called from outside of the file. Change the name so that it is not
tied to the function definition in arch/x86/include/asm/sev.h. Move the
renamed snp_init() and related functions up in the file to avoid having to
add a forward declaration and make the function static, too.

This will allow the snp_init() function in arch/x86/kernel/sev.c to be
changed without having to make the same change in boot/compressed/sev.c.

Signed-off-by: Tom Lendacky <[email protected]>
---
arch/x86/boot/compressed/sev.c | 162 ++++++++++++++++-----------------
1 file changed, 81 insertions(+), 81 deletions(-)

diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
index 454acd7a2daf..c3030cfb6484 100644
--- a/arch/x86/boot/compressed/sev.c
+++ b/arch/x86/boot/compressed/sev.c
@@ -405,6 +405,85 @@ void snp_check_features(void)
}
}

+/* Search for Confidential Computing blob in the EFI config table. */
+static struct cc_blob_sev_info *find_cc_blob_efi(struct boot_params *bp)
+{
+ unsigned long cfg_table_pa;
+ unsigned int cfg_table_len;
+ int ret;
+
+ ret = efi_get_conf_table(bp, &cfg_table_pa, &cfg_table_len);
+ if (ret)
+ return NULL;
+
+ return (struct cc_blob_sev_info *)efi_find_vendor_table(bp, cfg_table_pa,
+ cfg_table_len,
+ EFI_CC_BLOB_GUID);
+}
+
+/*
+ * Initial set up of SNP relies on information provided by the
+ * Confidential Computing blob, which can be passed to the boot kernel
+ * by firmware/bootloader in the following ways:
+ *
+ * - via an entry in the EFI config table
+ * - via a setup_data structure, as defined by the Linux Boot Protocol
+ *
+ * Scan for the blob in that order.
+ */
+static struct cc_blob_sev_info *find_cc_blob(struct boot_params *bp)
+{
+ struct cc_blob_sev_info *cc_info;
+
+ cc_info = find_cc_blob_efi(bp);
+ if (cc_info)
+ goto found_cc_info;
+
+ cc_info = find_cc_blob_setup_data(bp);
+ if (!cc_info)
+ return NULL;
+
+found_cc_info:
+ if (cc_info->magic != CC_BLOB_SEV_HDR_MAGIC)
+ sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED);
+
+ return cc_info;
+}
+
+/*
+ * Indicate SNP based on presence of SNP-specific CC blob. Subsequent checks
+ * will verify the SNP CPUID/MSR bits.
+ */
+static bool snp_setup(struct boot_params *bp)
+{
+ struct cc_blob_sev_info *cc_info;
+
+ if (!bp)
+ return false;
+
+ cc_info = find_cc_blob(bp);
+ if (!cc_info)
+ return false;
+
+ /*
+ * If a SNP-specific Confidential Computing blob is present, then
+ * firmware/bootloader have indicated SNP support. Verifying this
+ * involves CPUID checks which will be more reliable if the SNP
+ * CPUID table is used. See comments over snp_setup_cpuid_table() for
+ * more details.
+ */
+ setup_cpuid_table(cc_info);
+
+ /*
+ * Pass run-time kernel a pointer to CC info via boot_params so EFI
+ * config table doesn't need to be searched again during early startup
+ * phase.
+ */
+ bp->cc_blob_address = (u32)(unsigned long)cc_info;
+
+ return true;
+}
+
/*
* sev_check_cpu_support - Check for SEV support in the CPU capabilities
*
@@ -455,7 +534,7 @@ void sev_enable(struct boot_params *bp)
bp->cc_blob_address = 0;

/*
- * Do an initial SEV capability check before snp_init() which
+ * Do an initial SEV capability check before snp_setup() which
* loads the CPUID page and the same checks afterwards are done
* without the hypervisor and are trustworthy.
*
@@ -470,7 +549,7 @@ void sev_enable(struct boot_params *bp)
* Setup/preliminary detection of SNP. This will be sanity-checked
* against CPUID/MSR values later.
*/
- snp = snp_init(bp);
+ snp = snp_setup(bp);

/* Now repeat the checks with the SNP CPUID table. */

@@ -527,85 +606,6 @@ u64 sev_get_status(void)
return m.q;
}

-/* Search for Confidential Computing blob in the EFI config table. */
-static struct cc_blob_sev_info *find_cc_blob_efi(struct boot_params *bp)
-{
- unsigned long cfg_table_pa;
- unsigned int cfg_table_len;
- int ret;
-
- ret = efi_get_conf_table(bp, &cfg_table_pa, &cfg_table_len);
- if (ret)
- return NULL;
-
- return (struct cc_blob_sev_info *)efi_find_vendor_table(bp, cfg_table_pa,
- cfg_table_len,
- EFI_CC_BLOB_GUID);
-}
-
-/*
- * Initial set up of SNP relies on information provided by the
- * Confidential Computing blob, which can be passed to the boot kernel
- * by firmware/bootloader in the following ways:
- *
- * - via an entry in the EFI config table
- * - via a setup_data structure, as defined by the Linux Boot Protocol
- *
- * Scan for the blob in that order.
- */
-static struct cc_blob_sev_info *find_cc_blob(struct boot_params *bp)
-{
- struct cc_blob_sev_info *cc_info;
-
- cc_info = find_cc_blob_efi(bp);
- if (cc_info)
- goto found_cc_info;
-
- cc_info = find_cc_blob_setup_data(bp);
- if (!cc_info)
- return NULL;
-
-found_cc_info:
- if (cc_info->magic != CC_BLOB_SEV_HDR_MAGIC)
- sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED);
-
- return cc_info;
-}
-
-/*
- * Indicate SNP based on presence of SNP-specific CC blob. Subsequent checks
- * will verify the SNP CPUID/MSR bits.
- */
-bool snp_init(struct boot_params *bp)
-{
- struct cc_blob_sev_info *cc_info;
-
- if (!bp)
- return false;
-
- cc_info = find_cc_blob(bp);
- if (!cc_info)
- return false;
-
- /*
- * If a SNP-specific Confidential Computing blob is present, then
- * firmware/bootloader have indicated SNP support. Verifying this
- * involves CPUID checks which will be more reliable if the SNP
- * CPUID table is used. See comments over snp_setup_cpuid_table() for
- * more details.
- */
- setup_cpuid_table(cc_info);
-
- /*
- * Pass run-time kernel a pointer to CC info via boot_params so EFI
- * config table doesn't need to be searched again during early startup
- * phase.
- */
- bp->cc_blob_address = (u32)(unsigned long)cc_info;
-
- return true;
-}
-
void sev_prep_identity_maps(unsigned long top_level_pgt)
{
/*
--
2.42.0



2024-01-27 00:36:37

by Dionna Amalie Glaze

[permalink] [raw]
Subject: Re: [PATCH 01/11] x86/sev: Rename snp_init() in the boot/compressed/sev.c file

On Fri, Jan 26, 2024 at 2:16 PM Tom Lendacky <[email protected]> wrote:
>
> The snp_init() function is local to the boot/compressed/sev.c file and is
> not called from outside of the file. Change the name so that it is not
> tied to the function definition in arch/x86/include/asm/sev.h. Move the
> renamed snp_init() and related functions up in the file to avoid having to
> add a forward declaration and make the function static, too.
>
> This will allow the snp_init() function in arch/x86/kernel/sev.c to be
> changed without having to make the same change in boot/compressed/sev.c.
>
> Signed-off-by: Tom Lendacky <[email protected]>
> ---
> arch/x86/boot/compressed/sev.c | 162 ++++++++++++++++-----------------
> 1 file changed, 81 insertions(+), 81 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
> index 454acd7a2daf..c3030cfb6484 100644
> --- a/arch/x86/boot/compressed/sev.c
> +++ b/arch/x86/boot/compressed/sev.c
> @@ -405,6 +405,85 @@ void snp_check_features(void)
> }
> }
>
> +/* Search for Confidential Computing blob in the EFI config table. */
> +static struct cc_blob_sev_info *find_cc_blob_efi(struct boot_params *bp)
> +{
> + unsigned long cfg_table_pa;
> + unsigned int cfg_table_len;
> + int ret;
> +
> + ret = efi_get_conf_table(bp, &cfg_table_pa, &cfg_table_len);
> + if (ret)
> + return NULL;
> +
> + return (struct cc_blob_sev_info *)efi_find_vendor_table(bp, cfg_table_pa,
> + cfg_table_len,
> + EFI_CC_BLOB_GUID);
> +}
> +
> +/*
> + * Initial set up of SNP relies on information provided by the
> + * Confidential Computing blob, which can be passed to the boot kernel
> + * by firmware/bootloader in the following ways:
> + *
> + * - via an entry in the EFI config table
> + * - via a setup_data structure, as defined by the Linux Boot Protocol
> + *
> + * Scan for the blob in that order.
> + */
> +static struct cc_blob_sev_info *find_cc_blob(struct boot_params *bp)
> +{
> + struct cc_blob_sev_info *cc_info;
> +
> + cc_info = find_cc_blob_efi(bp);
> + if (cc_info)
> + goto found_cc_info;
> +
> + cc_info = find_cc_blob_setup_data(bp);
> + if (!cc_info)
> + return NULL;
> +
> +found_cc_info:
> + if (cc_info->magic != CC_BLOB_SEV_HDR_MAGIC)
> + sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED);
> +
> + return cc_info;
> +}
> +
> +/*
> + * Indicate SNP based on presence of SNP-specific CC blob. Subsequent checks
> + * will verify the SNP CPUID/MSR bits.
> + */
> +static bool snp_setup(struct boot_params *bp)
> +{
> + struct cc_blob_sev_info *cc_info;
> +
> + if (!bp)
> + return false;
> +
> + cc_info = find_cc_blob(bp);
> + if (!cc_info)
> + return false;
> +
> + /*
> + * If a SNP-specific Confidential Computing blob is present, then
> + * firmware/bootloader have indicated SNP support. Verifying this
> + * involves CPUID checks which will be more reliable if the SNP
> + * CPUID table is used. See comments over snp_setup_cpuid_table() for
> + * more details.
> + */
> + setup_cpuid_table(cc_info);
> +
> + /*
> + * Pass run-time kernel a pointer to CC info via boot_params so EFI
> + * config table doesn't need to be searched again during early startup
> + * phase.
> + */
> + bp->cc_blob_address = (u32)(unsigned long)cc_info;
> +
> + return true;
> +}
> +
> /*
> * sev_check_cpu_support - Check for SEV support in the CPU capabilities
> *
> @@ -455,7 +534,7 @@ void sev_enable(struct boot_params *bp)
> bp->cc_blob_address = 0;
>
> /*
> - * Do an initial SEV capability check before snp_init() which
> + * Do an initial SEV capability check before snp_setup() which
> * loads the CPUID page and the same checks afterwards are done
> * without the hypervisor and are trustworthy.
> *
> @@ -470,7 +549,7 @@ void sev_enable(struct boot_params *bp)
> * Setup/preliminary detection of SNP. This will be sanity-checked
> * against CPUID/MSR values later.
> */
> - snp = snp_init(bp);
> + snp = snp_setup(bp);
>
> /* Now repeat the checks with the SNP CPUID table. */
>
> @@ -527,85 +606,6 @@ u64 sev_get_status(void)
> return m.q;
> }
>
> -/* Search for Confidential Computing blob in the EFI config table. */
> -static struct cc_blob_sev_info *find_cc_blob_efi(struct boot_params *bp)
> -{
> - unsigned long cfg_table_pa;
> - unsigned int cfg_table_len;
> - int ret;
> -
> - ret = efi_get_conf_table(bp, &cfg_table_pa, &cfg_table_len);
> - if (ret)
> - return NULL;
> -
> - return (struct cc_blob_sev_info *)efi_find_vendor_table(bp, cfg_table_pa,
> - cfg_table_len,
> - EFI_CC_BLOB_GUID);
> -}
> -
> -/*
> - * Initial set up of SNP relies on information provided by the
> - * Confidential Computing blob, which can be passed to the boot kernel
> - * by firmware/bootloader in the following ways:
> - *
> - * - via an entry in the EFI config table
> - * - via a setup_data structure, as defined by the Linux Boot Protocol
> - *
> - * Scan for the blob in that order.
> - */
> -static struct cc_blob_sev_info *find_cc_blob(struct boot_params *bp)
> -{
> - struct cc_blob_sev_info *cc_info;
> -
> - cc_info = find_cc_blob_efi(bp);
> - if (cc_info)
> - goto found_cc_info;
> -
> - cc_info = find_cc_blob_setup_data(bp);
> - if (!cc_info)
> - return NULL;
> -
> -found_cc_info:
> - if (cc_info->magic != CC_BLOB_SEV_HDR_MAGIC)
> - sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED);
> -
> - return cc_info;
> -}
> -
> -/*
> - * Indicate SNP based on presence of SNP-specific CC blob. Subsequent checks
> - * will verify the SNP CPUID/MSR bits.
> - */
> -bool snp_init(struct boot_params *bp)
> -{
> - struct cc_blob_sev_info *cc_info;
> -
> - if (!bp)
> - return false;
> -
> - cc_info = find_cc_blob(bp);
> - if (!cc_info)
> - return false;
> -
> - /*
> - * If a SNP-specific Confidential Computing blob is present, then
> - * firmware/bootloader have indicated SNP support. Verifying this
> - * involves CPUID checks which will be more reliable if the SNP
> - * CPUID table is used. See comments over snp_setup_cpuid_table() for
> - * more details.
> - */
> - setup_cpuid_table(cc_info);
> -
> - /*
> - * Pass run-time kernel a pointer to CC info via boot_params so EFI
> - * config table doesn't need to be searched again during early startup
> - * phase.
> - */
> - bp->cc_blob_address = (u32)(unsigned long)cc_info;
> -
> - return true;
> -}
> -

Did some kind of whitespace replacement happen accidentally? There's a
lot that isn't changed but the diff is quite big.

> void sev_prep_identity_maps(unsigned long top_level_pgt)
> {
> /*
> --
> 2.42.0
>
>


--
-Dionna Glaze, PhD (she/her)

2024-01-27 14:38:22

by Tom Lendacky

[permalink] [raw]
Subject: Re: [PATCH 01/11] x86/sev: Rename snp_init() in the boot/compressed/sev.c file

On 1/26/24 18:05, Dionna Amalie Glaze wrote:
> On Fri, Jan 26, 2024 at 2:16 PM Tom Lendacky <[email protected]> wrote:
>>
>> The snp_init() function is local to the boot/compressed/sev.c file and is
>> not called from outside of the file. Change the name so that it is not
>> tied to the function definition in arch/x86/include/asm/sev.h. Move the
>> renamed snp_init() and related functions up in the file to avoid having to
>> add a forward declaration and make the function static, too.
>>
>> This will allow the snp_init() function in arch/x86/kernel/sev.c to be
>> changed without having to make the same change in boot/compressed/sev.c.
>>
>> Signed-off-by: Tom Lendacky <[email protected]>
>> ---
>> arch/x86/boot/compressed/sev.c | 162 ++++++++++++++++-----------------
>> 1 file changed, 81 insertions(+), 81 deletions(-)
>>
>> diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
>> index 454acd7a2daf..c3030cfb6484 100644
>> --- a/arch/x86/boot/compressed/sev.c
>> +++ b/arch/x86/boot/compressed/sev.c

>
> Did some kind of whitespace replacement happen accidentally? There's a
> lot that isn't changed but the diff is quite big.

No whitespace damage. As mentioned in the commit log, the functions were
moved within the file to avoid forward declarations.

Thanks,
Tom

>
>> void sev_prep_identity_maps(unsigned long top_level_pgt)
>> {
>> /*
>> --
>> 2.42.0
>>
>>
>
>