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

[Dionna Amalie Glaze]

On Fri, Jan 26, 2024 at 2:16 PM Tom Lendacky <thomas.lendacky@xxxxxxx> 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 <thomas.lendacky@xxxxxxx>
> ---
>  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)