Re: [edk2-devel] [PATCH V6 1/1] OvmfPkg: Enable TDX in ResetVector

["Min Xu" <min.m.xu@intel.com>]

Thanks Vishal and Brijesh.
Sorry I missed the limitation of smsw. MOV CRx should be used instead. I will fix it in the next version.
BTW, as Vishal mentioned in his comments in Github, we didn't have a chance to test SEV features because of the lack of AMD SEV environment. We're setting up the SEV environment now and will run a sanity test before the patch-set is sent out. But we may still need your help on the possible impact of the changes to the SEV features.
Thanks very much!

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Brijesh
> Singh via groups.io
> Sent: Wednesday, September 15, 2021 3:53 AM
> To: Vishal Annapurve <vannapurve@google.com>; devel@edk2.groups.io; Xu,
> Min M <min.m.xu@intel.com>
> Cc: brijesh.singh@amd.com; Ard Biesheuvel <ardb+tianocore@kernel.org>;
> Justen, Jordan L <jordan.l.justen@intel.com>; Gerd Hoffmann
> <kraxel@redhat.com>; Erdem Aktas <erdemaktas@google.com>; James
> Bottomley <jejb@linux.ibm.com>; Yao, Jiewen <jiewen.yao@intel.com>; Tom
> Lendacky <thomas.lendacky@amd.com>
> Subject: Re: [edk2-devel] [PATCH V6 1/1] OvmfPkg: Enable TDX in ResetVector
> 
> Hi Vishal,
> 
> On 9/14/21 2:00 PM, Vishal Annapurve wrote:
> > Hi Min, Brijesh,
> >
> > Regarding:
> >> diff --git a/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
> >> b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
> >> ...
> >> +%ifdef ARCH_IA32
> >>     nop
> >>     nop
> >>     jmp     EarlyBspInitReal16
> >>
> >>+%else
> >>+
> >>+    smsw    ax
> >
> > We are having intermittent VM crashes with running this code in
> > AMD-SEV enabled VMs. As per the AMD64 manual
> >
> <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fww
> w
> > .amd.com%2Fsystem%2Ffiles%2FTechDocs%2F24593.pdf&data=04%7C01%
> 7Cbrijes
> >
> h.singh%40amd.com%7C652023e953924957972a08d977b2031a%7C3dd8961
> fe4884e6
> >
> 08e11a82d994e183d%7C0%7C0%7C637672430875783281%7CUnknown%7CT
> WFpbGZsb3d
> >
> 8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%
> 3D%7C3000&sdata=VFiIbcV6H4xx5XZd%2F0OZjerSfJwLfUjK7mPU9JHY05E%3D
> &reserved=0> section 15.8.1, executing "smsw" instruction doesn't result in
> bit 63 being set in EXITINFO1 and KVM ends up emulating "smsw" instruction
> by trying to read encrypted guest VM memory as per the code
> <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.k
> ernel.org%2Fpub%2Fscm%2Fvirt%2Fkvm%2Fkvm.git%2Ftree%2Farch%2Fx86
> %2Fkvm%2Fsvm%2Fsvm.c%23n2495&data=04%7C01%7Cbrijesh.singh%40am
> d.com%7C652023e953924957972a08d977b2031a%7C3dd8961fe4884e608e1
> 1a82d994e183d%7C0%7C0%7C637672430875783281%7CUnknown%7CTWFp
> bGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVC
> I6Mn0%3D%7C3000&sdata=jSw7PLfXjhB8utM7Dxx2P%2F5M3fqvO3q3DBaF
> W%2Bu03A8%3D&reserved=0>.
> > Since KVM tries to make sense of different random cipher texts in
> > different boots, it seems to intermittently result in visible issues.
> >
> 
> The smsw does not provide decode assist, in those cases KVM reads the
> guest memory and tries to decode. With encrypted guest, the memory
> contains the ciphertext and hypervisor will not be able to decode the
> instruction.
> 
> But it brings a question to Min, why we are using the smsw ? why cannot
> use mov CRx. The smsw was meant for very old processors (286 or 8086
> etc) and is used for legacy compatibility. The recommendation is to use
> the mov CRx. The mov CRx will provide the decode assist to HV.
> 
> I looked at the Intel architecture manual [1] and it also recommends
> using the mov CRx. The text from the Intel doc.
> 
>    SMSW is only useful in operating-system software. However,
>    it is not a privileged instruction and can be used in
>    application programs if CR4.UMIP = 0. It is provided for
>    compatibility with the Intel 286 processor. Programs and
>    procedures intended to run on IA-32 and Intel 64 processors
>    beginning with the Intel386 processors should use the
>    MOV CR instruction to load the machine status word.
> 
> 
> [1]
> https://www.intel.com/content/dam/www/public/us/en/documents/manual
> s/64-ia-32-architectures-software-developer-instruction-set-reference-
> manual-325383.pdf
> 
> 
> > Is this expected behavior or do we miss some configuration or patches
> > that are recommended by AMD?
> >
> 
> This is expected because the smsw does not provide a decode assist, and
> encrypted guest will have issues with it. Lets understand the reason
> behind using the smsw.
> 
> > Regards,
> > Vishal
> >
> > On Tue, Sep 14, 2021 at 4:54 PM Brijesh Singh via groups.io
> >
> <https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fgrou
> ps.io%2F&data=04%7C01%7Cbrijesh.singh%40amd.com%7C652023e9539249
> 57972a08d977b2031a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0
> %7C637672430875793279%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLj
> AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdat
> a=%2Br4xRTlrqoERthTLJ2YNQAPrKzYX6fid2Q9WNyKybrw%3D&reserved=0>
> > <brijesh.singh=amd.com@groups.io <mailto:amd.com@groups.io>> wrote:
> >
> >     Hi Min,
> >
> >     A quick question below.
> >
> >     On 9/14/21 3:50 AM, Min Xu wrote:
> >      > RFC：
> >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzi
> lla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3429&amp;data=04%7C01%7Cbr
> ijesh.singh%40amd.com%7C2cca2f0a7fb44084da2b08d9775cb220%7C3dd89
> 61fe4884e608e11a82d994e183d%7C0%7C0%7C637672062275443867%7CUn
> known%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6
> Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=4zfuIDvTGDNCt%2BD3u7u
> UR0n6hHDzv%2FI8NkqoUJhsx8Y%3D&amp;reserved=0
> >
> <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbug
> zilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3429&data=04%7C01%7Cbrijes
> h.singh%40amd.com%7C652023e953924957972a08d977b2031a%7C3dd8961
> fe4884e608e11a82d994e183d%7C0%7C0%7C637672430875793279%7CUnkn
> own%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1
> haWwiLCJXVCI6Mn0%3D%7C3000&sdata=E5bfIvEeyWTgUqnmllwKgSwxycqh
> zfNnZ72O0i0UKww%3D&reserved=0>
> >      >
> >      > Intel's Trust Domain Extensions (Intel TDX) refers to an Intel
> >     technology
> >      > that extends Virtual Machines Extensions (VMX) and Multi-Key
> >     Total Memory
> >      > Encryption (MKTME) with a new kind of virutal machines guest called a
> >      > Trust Domain (TD). A TD is desinged to run in a CPU mode that
> >     protects the
> >      > confidentiality of TD memory contents and the TD's CPU state from
> >     other
> >      > software, including the hosting Virtual-Machine Monitor (VMM), unless
> >      > explicitly shared by the TD itself.
> >      >
> >      > Note: Intel TDX is only available on X64, so the Tdx related
> >     changes are
> >      > in X64 path. In IA32 path, there may be null stub to make the build
> >      > success.
> >      >
> >      > This patch includes below major changes.
> >      >
> >      > 1. Definition of BFV & CFV
> >      > Tdx Virtual Firmware (TDVF) includes one Firmware Volume (FV) known
> >      > as the Boot Firmware Volume (BFV). The FV format is defined in the
> >      > UEFI Platform Initialization (PI) spec. BFV includes all TDVF
> >     components
> >      > required during boot.
> >      >
> >      > TDVF also include a configuration firmware volume (CFV) that is
> >     separated
> >      > from the BFV. The reason is because the CFV is measured in RTMR,
> >     while
> >      > the BFV is measured in MRTD.
> >      >
> >      > In practice BFV is the code part of Ovmf image (OVMF_CODE.fd).
> >     CFV is the
> >      > vars part of Ovmf image (OVMF_VARS.fd).
> >      >
> >      > 2. PcdOvmfImageSizeInKb
> >      > PcdOvmfImageSizeInKb indicates the size of Ovmf image. It is used to
> >      > calculate the offset of TdxMetadata in ResetVectorVtf0.asm.
> >
> >     In SEV-SNP v7 series, I implemented the metadata support. I did not see
> >     a need for the PcdOvmfImageSizeInKB. Why do you need it? I think your
> >     calculation below will not work if someone is using the OVMF_CODE.fd
> >     instead of OVMF.fd. Have you tried booting with OVMF_CODE.fd ?
> >
> >     thanks
> >
> >
> >
> >
> >
> >
> >
> 
> 
> 
>