Re: [RFC PATCH v2 2/6] x86/sev: add SVSM vTPM probe/send_command functions

[Borislav Petkov]

On Fri, Feb 28, 2025 at 06:07:16PM +0100, Stefano Garzarella wrote:
> +bool snp_svsm_vtpm_probe(void)
> +{
> +     struct svsm_call call = {};
> +     u64 send_cmd_mask = 0;
> +     u64 platform_cmds;
> +     u64 features;
> +     int ret;
> +
> +     /* The vTPM device is available only if we have a SVSM */

s/if we have a SVSM/if an SVSM is present/

> +     if (!snp_vmpl)
> +             return false;
> +
> +     call.caa = svsm_get_caa();
> +     call.rax = SVSM_VTPM_CALL(SVSM_VTPM_QUERY);
> +
> +     ret = svsm_perform_call_protocol(&call);
> +


^ Superfluous newline.

> +     if (ret != SVSM_SUCCESS)
> +             return false;
> +
> +     features = call.rdx_out;
> +     platform_cmds = call.rcx_out;
> +
> +     /* No feature supported, it should be zero */
> +     if (features)
> +             pr_warn("SNP SVSM vTPM unsupported features: 0x%llx\n",
> +                     features);

So

        return false;

here?

> +
> +     /* TPM_SEND_COMMAND - platform command 8 */
> +     send_cmd_mask = 1 << 8;

        BIT_ULL(8);

> +
> +     return (platform_cmds & send_cmd_mask) == send_cmd_mask;
> +}
> +EXPORT_SYMBOL_GPL(snp_svsm_vtpm_probe);
> +
> +int snp_svsm_vtpm_send_command(u8 *buffer)
> +{
> +     struct svsm_call call = {};
> +
> +     call.caa = svsm_get_caa();
> +     call.rax = SVSM_VTPM_CALL(SVSM_VTPM_CMD);
> +     call.rcx = __pa(buffer);
> +
> +     return svsm_perform_call_protocol(&call);
> +}

In any case, you can zap all those local vars, use comments instead and slim
down the function, diff ontop:

diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
index 3902af4b1385..6d7e97c1f567 100644
--- a/arch/x86/coco/sev/core.c
+++ b/arch/x86/coco/sev/core.c
@@ -2631,12 +2631,9 @@ static int snp_issue_guest_request(struct snp_guest_req 
*req, struct snp_req_dat
 bool snp_svsm_vtpm_probe(void)
 {
        struct svsm_call call = {};
-       u64 send_cmd_mask = 0;
-       u64 platform_cmds;
-       u64 features;
        int ret;
 
-       /* The vTPM device is available only if we have a SVSM */
+       /* The vTPM device is available only if a SVSM is present */
        if (!snp_vmpl)
                return false;
 
@@ -2644,22 +2641,17 @@ bool snp_svsm_vtpm_probe(void)
        call.rax = SVSM_VTPM_CALL(SVSM_VTPM_QUERY);
 
        ret = svsm_perform_call_protocol(&call);
-
        if (ret != SVSM_SUCCESS)
                return false;
 
-       features = call.rdx_out;
-       platform_cmds = call.rcx_out;
-
        /* No feature supported, it should be zero */
-       if (features)
-               pr_warn("SNP SVSM vTPM unsupported features: 0x%llx\n",
-                       features);
-
-       /* TPM_SEND_COMMAND - platform command 8 */
-       send_cmd_mask = 1 << 8;
+       if (call.rdx_out) {
+               pr_warn("SNP SVSM vTPM unsupported features: 0x%llx\n", 
call.rdx_out);
+               return false;
+       }
 
-       return (platform_cmds & send_cmd_mask) == send_cmd_mask;
+       /* Check platform commands is TPM_SEND_COMMAND - platform command 8 */
+       return (call.rcx_out & BIT_ULL(8)) == BIT_ULL(8);
 }
 EXPORT_SYMBOL_GPL(snp_svsm_vtpm_probe);
 

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette