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 <vannapu...@google.com>; devel@edk2.groups.io; Xu, > Min M <min.m...@intel.com> > Cc: brijesh.si...@amd.com; Ard Biesheuvel <ardb+tianoc...@kernel.org>; > Justen, Jordan L <jordan.l.jus...@intel.com>; Gerd Hoffmann > <kra...@redhat.com>; Erdem Aktas <erdemak...@google.com>; James > Bottomley <j...@linux.ibm.com>; Yao, Jiewen <jiewen....@intel.com>; Tom > Lendacky <thomas.lenda...@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....@groups.io <mailto:amd....@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&data=04%7C01%7Cbr > ijesh.singh%40amd.com%7C2cca2f0a7fb44084da2b08d9775cb220%7C3dd89 > 61fe4884e608e11a82d994e183d%7C0%7C0%7C637672062275443867%7CUn > known%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6 > Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=4zfuIDvTGDNCt%2BD3u7u > UR0n6hHDzv%2FI8NkqoUJhsx8Y%3D&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 > > > > > > > > > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#80687): https://edk2.groups.io/g/devel/message/80687 Mute This Topic: https://groups.io/mt/85597386/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
