From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mail-pj1-f46.google.com (mail-pj1-f46.google.com [209.85.216.46])
 by mx.groups.io with SMTP id smtpd.web11.1834.1631322932449151691
 for <devel@edk2.groups.io>;
 Fri, 10 Sep 2021 18:15:32 -0700
Authentication-Results: mx.groups.io;
 dkim=pass header.i=@google.com header.s=20210112 header.b=VZszp0Tl;
 spf=pass (domain: google.com, ip: 209.85.216.46, mailfrom: erdemaktas@google.com)
Received: by mail-pj1-f46.google.com with SMTP id m21-20020a17090a859500b00197688449c4so2644176pjn.0
        for <devel@edk2.groups.io>; Fri, 10 Sep 2021 18:15:32 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
        d=google.com; s=20210112;
        h=mime-version:references:in-reply-to:from:date:message-id:subject:to
         :cc;
        bh=oaxbc34wOeZIkkuZbzXoc0BpTwgTalNk6oB0ORjvr6Y=;
        b=VZszp0TlzWl1IqZ80tH2Tb5JEkGuMZEjcyHX+4UypkM90ZKspIX2oJYsXylonvhFrT
         6XLB6JLveWQ4aNCl99ZECvjH/I40tQ1Zh9rxAY50NLIDkQpuzr5YnnoNy4AfcTadWrub
         7SmYON67E9AD9WExBjl5kuktos+ZForhyTAASE4RcZXRsjnxtBEswDJerw6Jx2sRGwO/
         HmICeI0Gydnfcg49vyrgglLoqetPC1p4Dr5CNP7eKeNwNPQN22RDJOY23rex+25dfA8I
         nS9OUToBu7W4tlVjADWEaXwEB0iD8VdLpjX0E2xtCgB1Ru4VPmO+P7ptg+PTIND4vWYW
         ffWw==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
        d=1e100.net; s=20210112;
        h=x-gm-message-state:mime-version:references:in-reply-to:from:date
         :message-id:subject:to:cc;
        bh=oaxbc34wOeZIkkuZbzXoc0BpTwgTalNk6oB0ORjvr6Y=;
        b=RgKMl733OcwFDCePkYcAa2fgtDh8JGcDICNwgUiLrmRK7l0JGV98Ox14tno3YPdaV2
         TjW97EAc2KL1LtnplyVqpKTnPFytIIDPFQXp2L+TfRe5ZEV/xpOVjk4TFv73Cnevv+tP
         ejLCkqxnZYA2eHK/pyP+DSJkk0U+5/pd+bh7BHxNYUdEHrJF5B9NPHYs1e80J7AnK235
         jFXW0UrtIwBVxyABg9X6zNgVdnGXY3elak9ZZDL6gVRBgbJPOW4HIYlOpM9oBY7VlHpX
         IluSako+N4LXE8OjEJQ1+vXzY13Vn0PFYZr6fO0Yh2GBNEEArb72RzPzscAHIsVEDjUB
         2HxA==
X-Gm-Message-State: AOAM531xp039Om1dLAKsn+3ZY87yN9qjQ7ppfBjXSwIfj6NAKydKAZeB
	V/GmGYLDgIy0+HTNsXBs4X+l23lLpIQXknMoY1Z3U16OTnb7PA==
X-Google-Smtp-Source: ABdhPJwWeWxVwgbj78kcktExotLnVJc3aYnY76dpnsz4yelxU4k73ZNKEDAhgC7nzoLYhvVq63TnnqfALHsMrW09WE8=
X-Received: by 2002:a17:90a:1a50:: with SMTP id 16mr527504pjl.164.1631322931714;
 Fri, 10 Sep 2021 18:15:31 -0700 (PDT)
MIME-Version: 1.0
References: <cover.1628767741.git.min.m.xu@intel.com> <ebd4d7ed61e7d8d32c0f12abbc8fc0e88cdf9940.1628767741.git.min.m.xu@intel.com>
In-Reply-To: <ebd4d7ed61e7d8d32c0f12abbc8fc0e88cdf9940.1628767741.git.min.m.xu@intel.com>
From: "Erdem Aktas" <erdemaktas@google.com>
Date: Sat, 11 Sep 2021 04:15:20 +0300
Message-ID: <CAAYXXYyEQZOww20rEZq99UTLy+i0jVQWfwgYCZtufXJ=b0Hxgw@mail.gmail.com>
Subject: Re: [PATCH 06/23] MdePkg: Add TdxLib to wrap Tdx operations
To: Min Xu <min.m.xu@intel.com>
Cc: edk2-devel-groups-io <devel@edk2.groups.io>, Michael D Kinney <michael.d.kinney@intel.com>, 
	Liming Gao <gaoliming@byosoft.com.cn>, Zhiguang Liu <zhiguang.liu@intel.com>, 
	Brijesh Singh <brijesh.singh@amd.com>, James Bottomley <jejb@linux.ibm.com>, 
	Jiewen Yao <jiewen.yao@intel.com>, Tom Lendacky <thomas.lendacky@amd.com>
Content-Type: text/plain; charset="UTF-8"

On Thu, Aug 12, 2021 at 2:57 PM Min Xu <min.m.xu@intel.com> wrote:
>  - TdMaxVCpuNum    : Get the maximum number of virutal CPUs.
s/virutal/virtual

>  - TdVCpuNum       : Get the number of virtual CPUs. (In some case VMM may
>                      add more vCPU in runtime).
s/case/cases
How is this possible considering that once the TD is finalized, there
should not be any new vcpu added, right? Am I missing something here?


> +++ b/MdePkg/Library/TdxLib/X64/Tdcall.nasm
> @@ -0,0 +1,120 @@
> +;------------------------------------------------------------------------------
....
> +%macro tdcall_regs_preamble 2
is this even used in this file?


> +    ; Zero out unused (for standard TDVMCALL) registers to avoid leaking
> +    ; secrets to the VMM.
this is for TDCALL right, there is no leaking to the tdx module.


> +
> +    xor ebx, ebx
> +    xor esi, esi
> +    xor edi, edi
> +
> +    xor edx, edx
> +    xor ebp, ebp
zeroing only the lower 32bit values? why not the higher 32bit value if
leaking is the concern?


> +++ b/MdePkg/Library/TdxLib/X64/Tdvmcall.nasm

> +%define TDVMCALL_EXPOSE_REGS_MASK       0xffec
Should we expose only the minimum number of registers needed for the TDVMCALL?


>
> +%macro tdcall_regs_preamble 2
> +    mov rax, %1
> +
> +    mov ecx, %2
should not we make sure that the higher 32bit of RCX is 0? RCX [63:32]
are reserved and always need to be 0.

> +    ; R10 = 0 (standard TDVMCALL)
> +
> +    xor r10d, r10d
> +
> +    ; Zero out unused (for standard TDVMCALL) registers to avoid leaking
> +    ; secrets to the VMM.

Is not rcx the bitmap of the registers that will be exposed to VMM?
unused registers should be set 0 in the bitmap, why zeroing them?

> +
> +    xor ebx, ebx
> +    xor esi, esi
> +    xor edi, edi

> +    xor edx, edx
> +    xor ebp, ebp
if we are concerned about leaking some data, why xor only the lower 32bits?