[edk2-devel] [PATCH RFC v2 07/28] OvmfPkg: Use MemEncryptSevClearMmioPageEncMask() to clear EncMask from Mmio

Brijesh Singh posted 28 patches 1 month, 2 weeks ago

[edk2-devel] [PATCH RFC v2 07/28] OvmfPkg: Use MemEncryptSevClearMmioPageEncMask() to clear EncMask from Mmio

Posted by Brijesh Singh 1 month, 2 weeks ago
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

Use the MemEncryptSevClearMmioPageEncMask() to clear memory encryption mask
for the Mmio address range from the current page table context.

Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 OvmfPkg/AmdSevDxe/AmdSevDxe.c                              | 10 ++++------
 OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c |  5 ++---
 OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c      |  5 ++---
 3 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
index 689bfb376d..80831b81fa 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
@@ -53,11 +53,10 @@ AmdSevDxeEntryPoint (
       Desc = &AllDescMap[Index];
       if (Desc->GcdMemoryType == EfiGcdMemoryTypeMemoryMappedIo ||
           Desc->GcdMemoryType == EfiGcdMemoryTypeNonExistent) {
-        Status = MemEncryptSevClearPageEncMask (
+        Status = MemEncryptSevClearMmioPageEncMask (
                    0,
                    Desc->BaseAddress,
-                   EFI_SIZE_TO_PAGES (Desc->Length),
-                   FALSE
+                   EFI_SIZE_TO_PAGES (Desc->Length)
                    );
         ASSERT_EFI_ERROR (Status);
       }
@@ -73,11 +72,10 @@ AmdSevDxeEntryPoint (
   // the range.
   //
   if (PcdGet16 (PcdOvmfHostBridgePciDevId) == INTEL_Q35_MCH_DEVICE_ID) {
-    Status = MemEncryptSevClearPageEncMask (
+    Status = MemEncryptSevClearMmioPageEncMask (
                0,
                FixedPcdGet64 (PcdPciExpressBaseAddress),
-               EFI_SIZE_TO_PAGES (SIZE_256MB),
-               FALSE
+               EFI_SIZE_TO_PAGES (SIZE_256MB)
                );
 
     ASSERT_EFI_ERROR (Status);
diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c
index 1f285e0083..ab40087a84 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c
@@ -205,11 +205,10 @@ MarkIoMemoryRangeForRuntimeAccess (
   // memory range.
   //
   if (MemEncryptSevIsEnabled ()) {
-    Status = MemEncryptSevClearPageEncMask (
+    Status = MemEncryptSevClearMmioPageEncMask (
                0,
                BaseAddress,
-               EFI_SIZE_TO_PAGES (Length),
-               FALSE
+               EFI_SIZE_TO_PAGES (Length)
                );
     ASSERT_EFI_ERROR (Status);
   }
diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c
index 7eb80bfeff..ea75b489c7 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c
@@ -38,11 +38,10 @@ QemuFlashBeforeProbe (
   // C-bit on flash ranges from SMM page table.
   //
 
-  Status = MemEncryptSevClearPageEncMask (
+  Status = MemEncryptSevClearMmioPageEncMask (
              0,
              BaseAddress,
-             EFI_SIZE_TO_PAGES (FdBlockSize * FdBlockCount),
-             FALSE
+             EFI_SIZE_TO_PAGES (FdBlockSize * FdBlockCount)
              );
   ASSERT_EFI_ERROR (Status);
 }
-- 
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#74634): https://edk2.groups.io/g/devel/message/74634
Mute This Topic: https://groups.io/mt/82479054/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH RFC v2 07/28] OvmfPkg: Use MemEncryptSevClearMmioPageEncMask() to clear EncMask from Mmio

Posted by Laszlo Ersek 1 month, 1 week ago
On 04/30/21 13:51, Brijesh Singh wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275
> 
> Use the MemEncryptSevClearMmioPageEncMask() to clear memory encryption mask
> for the Mmio address range from the current page table context.
> 
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Min Xu <min.m.xu@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  OvmfPkg/AmdSevDxe/AmdSevDxe.c                              | 10 ++++------
>  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c |  5 ++---
>  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c      |  5 ++---
>  3 files changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> index 689bfb376d..80831b81fa 100644
> --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> @@ -53,11 +53,10 @@ AmdSevDxeEntryPoint (
>        Desc = &AllDescMap[Index];
>        if (Desc->GcdMemoryType == EfiGcdMemoryTypeMemoryMappedIo ||
>            Desc->GcdMemoryType == EfiGcdMemoryTypeNonExistent) {
> -        Status = MemEncryptSevClearPageEncMask (
> +        Status = MemEncryptSevClearMmioPageEncMask (
>                     0,
>                     Desc->BaseAddress,
> -                   EFI_SIZE_TO_PAGES (Desc->Length),
> -                   FALSE
> +                   EFI_SIZE_TO_PAGES (Desc->Length)
>                     );
>          ASSERT_EFI_ERROR (Status);
>        }
> @@ -73,11 +72,10 @@ AmdSevDxeEntryPoint (
>    // the range.
>    //
>    if (PcdGet16 (PcdOvmfHostBridgePciDevId) == INTEL_Q35_MCH_DEVICE_ID) {
> -    Status = MemEncryptSevClearPageEncMask (
> +    Status = MemEncryptSevClearMmioPageEncMask (
>                 0,
>                 FixedPcdGet64 (PcdPciExpressBaseAddress),
> -               EFI_SIZE_TO_PAGES (SIZE_256MB),
> -               FALSE
> +               EFI_SIZE_TO_PAGES (SIZE_256MB)
>                 );
>  
>      ASSERT_EFI_ERROR (Status);
> diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c
> index 1f285e0083..ab40087a84 100644
> --- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c
> +++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c
> @@ -205,11 +205,10 @@ MarkIoMemoryRangeForRuntimeAccess (
>    // memory range.
>    //
>    if (MemEncryptSevIsEnabled ()) {
> -    Status = MemEncryptSevClearPageEncMask (
> +    Status = MemEncryptSevClearMmioPageEncMask (
>                 0,
>                 BaseAddress,
> -               EFI_SIZE_TO_PAGES (Length),
> -               FALSE
> +               EFI_SIZE_TO_PAGES (Length)
>                 );
>      ASSERT_EFI_ERROR (Status);
>    }
> diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c
> index 7eb80bfeff..ea75b489c7 100644
> --- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c
> +++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c
> @@ -38,11 +38,10 @@ QemuFlashBeforeProbe (
>    // C-bit on flash ranges from SMM page table.
>    //
>  
> -  Status = MemEncryptSevClearPageEncMask (
> +  Status = MemEncryptSevClearMmioPageEncMask (
>               0,
>               BaseAddress,
> -             EFI_SIZE_TO_PAGES (FdBlockSize * FdBlockCount),
> -             FALSE
> +             EFI_SIZE_TO_PAGES (FdBlockSize * FdBlockCount)
>               );
>    ASSERT_EFI_ERROR (Status);
>  }
> 

The contents of this patch are sound, but they are incomplete, and
incorrectly structured too.

(1) Please provide a separate patch for each modified module.

(2) You missed the MemEncryptSevClearPageEncMask() call in
TpmMmioSevDecryptPeimEntryPoint()
[OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPeim.c] -- probably
because you worked on this series in parallel with Tom working on the
SEV-ES TPM fixes.

In the end, this patch should be split into three patches (because the
change is needed for three modules).

Thanks
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#74792): https://edk2.groups.io/g/devel/message/74792
Mute This Topic: https://groups.io/mt/82479054/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH RFC v2 07/28] OvmfPkg: Use MemEncryptSevClearMmioPageEncMask() to clear EncMask from Mmio

Posted by Brijesh Singh 1 month, 1 week ago
On 5/6/21 5:50 AM, Laszlo Ersek wrote:
> On 04/30/21 13:51, Brijesh Singh wrote:
>> BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3275&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cce4b852d83a14265d48f08d9107cd16f%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637558950553380286%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=CgY57XQGI9QBvj7vipJoJLVZLiEvpfySW17TLLx%2BZm8%3D&amp;reserved=0
>>
>> Use the MemEncryptSevClearMmioPageEncMask() to clear memory encryption mask
>> for the Mmio address range from the current page table context.
>>
>> Cc: James Bottomley <jejb@linux.ibm.com>
>> Cc: Min Xu <min.m.xu@intel.com>
>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Erdem Aktas <erdemaktas@google.com>
>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>> ---
>>  OvmfPkg/AmdSevDxe/AmdSevDxe.c                              | 10 ++++------
>>  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c |  5 ++---
>>  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c      |  5 ++---
>>  3 files changed, 8 insertions(+), 12 deletions(-)
>>
>> diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
>> index 689bfb376d..80831b81fa 100644
>> --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
>> +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
>> @@ -53,11 +53,10 @@ AmdSevDxeEntryPoint (
>>        Desc = &AllDescMap[Index];
>>        if (Desc->GcdMemoryType == EfiGcdMemoryTypeMemoryMappedIo ||
>>            Desc->GcdMemoryType == EfiGcdMemoryTypeNonExistent) {
>> -        Status = MemEncryptSevClearPageEncMask (
>> +        Status = MemEncryptSevClearMmioPageEncMask (
>>                     0,
>>                     Desc->BaseAddress,
>> -                   EFI_SIZE_TO_PAGES (Desc->Length),
>> -                   FALSE
>> +                   EFI_SIZE_TO_PAGES (Desc->Length)
>>                     );
>>          ASSERT_EFI_ERROR (Status);
>>        }
>> @@ -73,11 +72,10 @@ AmdSevDxeEntryPoint (
>>    // the range.
>>    //
>>    if (PcdGet16 (PcdOvmfHostBridgePciDevId) == INTEL_Q35_MCH_DEVICE_ID) {
>> -    Status = MemEncryptSevClearPageEncMask (
>> +    Status = MemEncryptSevClearMmioPageEncMask (
>>                 0,
>>                 FixedPcdGet64 (PcdPciExpressBaseAddress),
>> -               EFI_SIZE_TO_PAGES (SIZE_256MB),
>> -               FALSE
>> +               EFI_SIZE_TO_PAGES (SIZE_256MB)
>>                 );
>>  
>>      ASSERT_EFI_ERROR (Status);
>> diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c
>> index 1f285e0083..ab40087a84 100644
>> --- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c
>> +++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c
>> @@ -205,11 +205,10 @@ MarkIoMemoryRangeForRuntimeAccess (
>>    // memory range.
>>    //
>>    if (MemEncryptSevIsEnabled ()) {
>> -    Status = MemEncryptSevClearPageEncMask (
>> +    Status = MemEncryptSevClearMmioPageEncMask (
>>                 0,
>>                 BaseAddress,
>> -               EFI_SIZE_TO_PAGES (Length),
>> -               FALSE
>> +               EFI_SIZE_TO_PAGES (Length)
>>                 );
>>      ASSERT_EFI_ERROR (Status);
>>    }
>> diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c
>> index 7eb80bfeff..ea75b489c7 100644
>> --- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c
>> +++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c
>> @@ -38,11 +38,10 @@ QemuFlashBeforeProbe (
>>    // C-bit on flash ranges from SMM page table.
>>    //
>>  
>> -  Status = MemEncryptSevClearPageEncMask (
>> +  Status = MemEncryptSevClearMmioPageEncMask (
>>               0,
>>               BaseAddress,
>> -             EFI_SIZE_TO_PAGES (FdBlockSize * FdBlockCount),
>> -             FALSE
>> +             EFI_SIZE_TO_PAGES (FdBlockSize * FdBlockCount)
>>               );
>>    ASSERT_EFI_ERROR (Status);
>>  }
>>
> The contents of this patch are sound, but they are incomplete, and
> incorrectly structured too.
>
> (1) Please provide a separate patch for each modified module.

Noted.


> (2) You missed the MemEncryptSevClearPageEncMask() call in
> TpmMmioSevDecryptPeimEntryPoint()
> [OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPeim.c] -- probably
> because you worked on this series in parallel with Tom working on the
> SEV-ES TPM fixes.


I guess Tom's patches were not accepted when I rebased the the SNP
patches. Will pick those changes in next rev.


>
> In the end, this patch should be split into three patches (because the
> change is needed for three modules).
>
> Thanks
> Laszlo
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#74803): https://edk2.groups.io/g/devel/message/74803
Mute This Topic: https://groups.io/mt/82479054/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-