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%2Fwww.amd.com%2Fsystem%2Ffiles%2FTechDocs%2F24593.pdf&data=04%7C01%7Cbrijesh.singh%40amd.com%7C652023e953924957972a08d977b2031a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637672430875783281%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%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.kernel.org%2Fpub%2Fscm%2Fvirt%2Fkvm%2Fkvm.git%2Ftree%2Farch%2Fx86%2Fkvm%2Fsvm%2Fsvm.c%23n2495&data=04%7C01%7Cbrijesh.singh%40amd.com%7C652023e953924957972a08d977b2031a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637672430875783281%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=jSw7PLfXjhB8utM7Dxx2P%2F5M3fqvO3q3DBaFW%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/manuals/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%2Fgroups.io%2F&data=04%7C01%7Cbrijesh.singh%40amd.com%7C652023e953924957972a08d977b2031a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637672430875793279%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=%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%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3429&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7C2cca2f0a7fb44084da2b08d9775cb220%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637672062275443867%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=4zfuIDvTGDNCt%2BD3u7uUR0n6hHDzv%2FI8NkqoUJhsx8Y%3D&amp;reserved=0
    
<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3429&data=04%7C01%7Cbrijesh.singh%40amd.com%7C652023e953924957972a08d977b2031a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637672430875793279%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=E5bfIvEeyWTgUqnmllwKgSwxycqhzfNnZ72O0i0UKww%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 (#80669): https://edk2.groups.io/g/devel/message/80669
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]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to