Re: [edk2-devel] [PATCH v2 4/4] OvmfPkg/Tcg2ConfigPei: Mark TPM MMIO range as unencrypted for SEV-ES

["Lendacky, Thomas" <thomas.lendacky@amd.com>]

On 4/28/21 2:43 PM, Tom Lendacky wrote:
> On 4/28/21 12:51 PM, Laszlo Ersek via groups.io wrote:
>> I'm going to ask for v3 after all:
>>
>> On 04/27/21 18:21, Lendacky, Thomas wrote:
>>> From: Tom Lendacky <thomas.lendacky@amd.com>
>>>
>>> BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3345&amp;data=04%7C01%7Cthomas.lendacky%40amd.com%7C3c65ebfe044e4f3eb5b808d90a6e5455%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637552291252644310%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=G1GwQc6sZqRuNHWC5vbdb78gCOl4YkAq%2BHi0F0ceucg%3D&amp;reserved=0
>>>
>>> During PEI, the MMIO range for the TPM is marked as encrypted when running
>>> as an SEV guest. While this isn't an issue for an SEV guest because of
>>> the way the nested page fault is handled, it does result in an SEV-ES
>>> guest terminating because of a mitigation check in the #VC handler to
>>> prevent MMIO to an encrypted address. For an SEV-ES guest, this range
>>> must be marked as unencrypted.
>>>
>>> Create a new x86 PEIM for TPM support that will map the TPM MMIO range as
>>> unencrypted when SEV-ES is active. The gOvmfTpmMmioAccessiblePpiGuid PPI
>>> will be unconditionally installed before exiting. The PEIM will exit with
>>> the EFI_ABORTED status so that the PEIM does not stay resident.
>>

...

>>> +
>>> +  //
>>> +  // If SEV or SEV-ES is active, MMIO succeeds against an encrypted physical
>>> +  // address because the nested page fault (NPF) that occurs on access does not
>>> +  // include the encryption bit in the guest physical address provided to the
>>> +  // hypervisor.
>>> +  //
>>> +  // However, if SEV-ES is active, before performing the actual MMIO, an
>>> +  // additional MMIO mitigation check is performed in the #VC handler to ensure
>>> +  // that MMIO is being done to an unencrypted address. To prevent guest
>>> +  // termination in this scenario, mark the range unencrypted ahead of access.
>>> +  //
>>
>> Lovely comment, thanks!

I'm going to expand on this a bit more to really show the distinction
between SEV and SEV-ES when it comes to MMIO. Look for a bit more info in v3.

Thanks,
Tom

>>
>>> +  if (MemEncryptSevEsIsEnabled ()) {
>>> +    DEBUG ((DEBUG_INFO, "%a: mapping TPM MMIO address range unencrypted\n", __FUNCTION__));
>>> +
>>> +    DecryptStatus = MemEncryptSevClearPageEncMask (
>>> +                      0,
>>> +                      PcdGet64 (PcdTpmBaseAddress),
>>
>> (11) The INF file says [FixedPcd], so it would be cleanest to say
>> FixedPcdGet64() here.
> 
> Will do.
> 
>>
>>
>> (12) PcdLib is missing from both the [LibraryClasses] section and the
>> #include directives.
> 
> Right, I'll update that.
> 
>>
>>
>>> +                      EFI_SIZE_TO_PAGES ((UINTN) 0x5000),
>>> +                      FALSE
>>> +                      );
>>> +
>>> +    if (RETURN_ERROR (DecryptStatus)) {
>>> +      DEBUG ((DEBUG_INFO, "%a: failed to map TPM MMIO address range unencrypted\n", __FUNCTION__));
>>
>> (13) Overlong line.
> 
> Ok, I'll change that. I though that was ok now since PatchCheck.py didn't
> complain.
> 
>>
>>
>> (14) Please report errors with DEBUG_ERROR.
> 
> Yup, will change.
> 
> Thanks,
> Tom
> 
>>
>>
>>> +      ASSERT_RETURN_ERROR (DecryptStatus);
>>> +    }
>>> +  }
>>> +
>>> +  //
>>> +  // MMIO range available
>>> +  //
>>> +  Status = PeiServicesInstallPpi (&mTpmMmioRangeAccessible);
>>> +  ASSERT_EFI_ERROR (Status);
>>> +
>>> +  return EFI_ABORTED;
>>> +}
>>>
>>
>> Thanks!
>> Laszlo
>>
>>
>>
>> 
>>
>>