[edk2-devel] [edk2-platforms][PATCH v1 1/1] MinPlatformPkg/Acpi/AcpiSmm: Add Standalone MM support

2021-03-05 Thread Michael Kubacki
From: Michael Kubacki 

Adds a new module called AcpiStandaloneMm that serves the same role
as AcpiSmm but in a Standalone MM environment.

This change follows a similar pattern to other changes that have
added Standalone MM support to a SMM module. The SMM INF name and
file path remain unaltered to allow backward compatibility and much
of the code is shared between the driver instances with unique entry
points for each respective module type.

Cc: Chasel Chiu 
Cc: Nate DeSimone 
Cc: Liming Gao 
Cc: Eric Dong 
Signed-off-by: Michael Kubacki 
---
 Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/{AcpiSmm.c => AcpiMm.c} 
  | 33 +--
 Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiStandaloneMm.c  
  | 34 
 Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiTraditionalMm.c 
  | 34 
 Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiMm.h
  | 23 +
 Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiSmm.h   
  | 24 --
 Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiSmm.inf 
  | 21 ++--
 Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/{AcpiSmm.inf => 
AcpiStandaloneMm.inf} | 32 +-
 Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc   
  |  2 ++
 8 files changed, 133 insertions(+), 70 deletions(-)

diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiSmm.c 
b/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiMm.c
similarity index 81%
rename from Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiSmm.c
rename to Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiMm.c
index 809f75d3c588..2cf559f3fe09 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiSmm.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiMm.c
@@ -1,12 +1,20 @@
 /** @file
-  Acpi Smm driver.
+  Functions shared between driver instances.
 
 Copyright (c) 2017, Intel Corporation. All rights reserved.
+Copyright (c) Microsoft Corporation.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
-#include "AcpiSmm.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "AcpiMm.h"
 
 /**
   Enable SCI
@@ -53,20 +61,13 @@ DisableAcpiCallback (
 }
 
 /**
-  Initializes the Acpi Smm Driver
-
-  @param[in] ImageHandle   - Pointer to the loaded image protocol for this 
driver
-  @param[in] SystemTable   - Pointer to the EFI System Table
-
-  @retval Status   - EFI_SUCCESS
-  @retval Assert, otherwise.
+  ACPI initialization logic shared between the Traditional MM and
+  Standalone MM driver instances.
 
 **/
-EFI_STATUS
-EFIAPI
-InitializeAcpiSmm (
-  IN EFI_HANDLEImageHandle,
-  IN EFI_SYSTEM_TABLE  *SystemTable
+VOID
+InitializeAcpiMm (
+  VOID
   )
 {
   EFI_STATUSStatus;
@@ -77,7 +78,7 @@ InitializeAcpiSmm (
   //
   // Locate the ICH SMM SW dispatch protocol
   //
-  Status = gSmst->SmmLocateProtocol (&gEfiSmmSwDispatch2ProtocolGuid, NULL, 
(VOID**)&SwDispatch);
+  Status = gMmst->MmLocateProtocol (&gEfiSmmSwDispatch2ProtocolGuid, NULL, 
(VOID**) &SwDispatch);
   ASSERT_EFI_ERROR (Status);
 
   //
@@ -103,6 +104,4 @@ InitializeAcpiSmm (
  &SwHandle
  );
   ASSERT_EFI_ERROR (Status);
-
-  return EFI_SUCCESS;
 }
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiStandaloneMm.c 
b/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiStandaloneMm.c
new file mode 100644
index ..f378942fdc07
--- /dev/null
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiStandaloneMm.c
@@ -0,0 +1,34 @@
+/** @file
+  Standalone MM driver for ACPI initialization.
+
+Copyright (c) 2017, Intel Corporation. All rights reserved.
+Copyright (c) Microsoft Corporation.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+
+#include "AcpiMm.h"
+
+/**
+  The Standalone MM driver entry point.
+
+  @param[in] ImageHandle   - Pointer to the loaded image protocol for this 
driver
+  @param[in] SystemTable   - Pointer to the EFI MM System Table
+
+  @retval Status   - EFI_SUCCESS
+  @retval Assert, otherwise.
+
+**/
+EFI_STATUS
+EFIAPI
+AcpiStandaloneMmEntryPoint (
+  IN EFI_HANDLE ImageHandle,
+  IN EFI_MM_SYSTEM_TABLE*MmSystemTable
+  )
+{
+  InitializeAcpiMm ();
+
+  return EFI_SUCCESS;
+}
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiTraditionalMm.c 
b/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiTraditionalMm.c
new file mode 100644
index ..9512926b9e2e
--- /dev/null
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiTraditionalMm.c
@@ -0,0 +1,34 @@
+/** @file
+  Traditional MM driver for ACPI initialization.
+
+Copyright (c) 2017, Intel Corporation. All rights reserved.
+Copyright (c) Microsoft Corporation.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+
+#include "AcpiMm.h"
+
+/**
+  The Traditional MM driver entry point.
+
+  @param[in

Re: [edk2-devel] [edk2-platforms][PATCH V1 4/5] Platform/ARM/Morello: Add Configuration Manager for Morello

2021-03-05 Thread Sami Mujawar
Hi Chandni,

Just remembered one more thing.

+  // Processor Node Id Info
+  {
+// A unique token used to identify this object
+REFERENCE_TOKEN (ProcNodeIdInfo),
+// Vendor ID (as described in ACPI ID registry)
+SIGNATURE_32('A', 'R', 'M', 'H'),
+// First level unique node ID
+0,
+// Second level unique node ID
+0,
+// Major revision of the node
+0,
+// Minor revision of the node
+0,
+// Spin revision of the node
+0
+  },

The 'PPTT Type 2 - Processor ID' is deprecated so don’t add this information.

Regards,

Sami Mujawar




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72509): https://edk2.groups.io/g/devel/message/72509
Mute This Topic: https://groups.io/mt/80885830/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live Migration for AMD SEV

2021-03-05 Thread Tobin Feldman-Fitzthum




On Fri, Mar 05, 2021 at 10:44:23AM +, Ashish Kalra wrote:

On Wed, Mar 03, 2021 at 01:25:40PM -0500, Tobin Feldman-Fitzthum wrote:

Hi Tobin,

On 03/02/21 21:48, Tobin Feldman-Fitzthum wrote:

This is a demonstration of fast migration for encrypted virtual machines
using a Migration Handler that lives in OVMF. This demo uses AMD SEV,
but the ideas may generalize to other confidential computing platforms.
With AMD SEV, guest memory is encrypted and the hypervisor cannot access
or move it. This makes migration tricky. In this demo, we show how the
HV can ask a Migration Handler (MH) in the firmware for an encrypted
page. The MH encrypts the page with a transport key prior to releasing
it to the HV. The target machine also runs an MH that decrypts the page
once it is passed in by the target HV. These patches are not ready for
production, but the are a full end-to-end solution that facilitates a
fast live migration between two SEV VMs.

Corresponding patches for QEMU have been posted my colleague Dov Murik
on qemu-devel. Our approach needs little kernel support, requiring only
one hypercall that the guest can use to mark a page as encrypted or
shared. This series includes updated patches from Ashish Kalra and
Brijesh Singh that allow OVMF to use this hypercall.

The MH runs continuously in the guest, waiting for communication from
the HV. The HV starts an additional vCPU for the MH but does not expose
it to the guest OS via ACPI. We use the MpService to start the MH. The
MpService is only available at runtime and processes that are started by
it are usually cleaned up on ExitBootServices. Since we need the MH to
run continuously, we had to make some modifications. Ideally a feature
could be added to the MpService to allow for the starting of
long-running processes. Besides migration, this could support other
background processes that need to operate within the encryption
boundary. For now, we have included a handful of patches that modify the
MpService to allow the MH to keep running after ExitBootServices. These
are temporary.

I plan to do a lightweight review for this series. (My understanding is
that it's an RFC and not actually being proposed for merging.)

Regarding the MH's availability at runtime -- does that necessarily
require the isolation of an AP? Because in the current approach,
allowing the MP Services to survive into OS runtime (in some form or
another) seems critical, and I don't think it's going to fly.

I agree that the UefiCpuPkg patches have been well separated from the
rest of the series, but I'm somewhat doubtful the "firmware-initiated
background process" idea will be accepted. Have you investigated
exposing a new "runtime service" (a function pointer) via the UEFI
Configuration table, and calling that (perhaps periodically?) from the
guest kernel? It would be a form of polling I guess. Or maybe, poll the
mailbox directly in the kernel, and call the new firmware runtime
service when there's an actual command to process.

Continuous runtime availability for the MH is almost certainly the most
controversial part of this proposal, which is why I put it in the cover
letter and why it's good to discuss.

(You do spell out "little kernel support", and I'm not sure if that's a
technical benefit, or a political / community benefit.)

As you allude to, minimal kernel support is really one of the main things
that shapes our approach. This is partly a political and practical benefit,
but there are also technical benefits. Having the MH in firmware likely
leads to higher availability. It can be accessed when the OS is unreachable,
perhaps during boot or when the OS is hung. There are also potential
portability advantages although we do currently require support for one
hypercall. The cost of implementing this hypercall is low.

Generally speaking, our task is to find a home for functionality that was
traditionally provided by the hypervisor, but that needs to be inside the
trust domain, but that isn't really part of a guest. A meta-goal of this
project is to figure out the best way to do this.


I'm quite uncomfortable with an attempt to hide a CPU from the OS via
ACPI. The OS has other ways to learn (for example, a boot loader could
use the MP services itself, stash the information, and hand it to the OS
kernel -- this would minimally allow for detecting an inconsistency in
the OS). What about "all-but-self" IPIs too -- the kernel might think
all the processors it's poking like that were under its control.

This might be the second most controversial piece. Here's a question: if we
could successfully hide the MH vCPU from the OS, would it still make you
uncomfortable? In other words, is the worry that there might be some
inconsistency or more generally that there is something hidden from the OS?
One thing to think about is that the guest owner should generally be aware
that there is a migration handler running. The way I see it, a guest owner
of an SEV VM would need to opt-in to migration 

Re: [edk2-devel] [PATCH v6 0/7] Add MmUnblockMemoryLib Interface and Usages

2021-03-05 Thread Kun Qin
Hi Laszlo,

Thanks for the help. Will close the BZ tickets and see what is up with the 
email settings. (this one is sent through webpage.)

Regards,
Kun


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72507): https://edk2.groups.io/g/devel/message/72507
Mute This Topic: https://groups.io/mt/81095803/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH 2/2 v5] StMMRpmb: Add support for building StandaloneMm image for OP-TEE

2021-03-05 Thread PierreGondois
Hi Ilias,
Thanks for the answer.
Is it necessary to have the 'COMPRESSION_TOOL_GUID' define ? I could not find 
any use of it. If this is coming from StandaloneMmPkg/StandaloneMmPkg.dsc we 
might want to remove it there aswel.

Regards,
Pierre


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72506): https://edk2.groups.io/g/devel/message/72506
Mute This Topic: https://groups.io/mt/80588995/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH 1/2 v5] Drivers/OpTeeRpmb: Add an OP-TEE backed RPMB driver

2021-03-05 Thread PierreGondois

Hi Ilias,
Here is the rest of the review. Sorry to do it in 2 times.

Regards,

Pierre




+/**

+  Fixup the Pcd values for variable storage

+

+  Since the upper layers of EDK2 expect a memory mapped interface and 
we can't


+  offer that from an RPMB, the driver allocates memory on init and 
passes that


+  on the upper layers. Since the memory is dynamically allocated and 
we can't set the


+  PCD is StMM context, we need to patch it correctly on each access

+

+  @retval EFI_SUCCESS Protocol was found and PCDs patched up

The error codes are missing.


+

+ **/

+EFI_STATUS

+EFIAPI

+FixPcdMemory (

+  VOID

+  )

+{

+  EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL  *FvbProtocol;

+  MEM_INSTANCE    *Instance;

+  EFI_STATUS  Status;

+

+  //

+  // Locate SmmFirmwareVolumeBlockProtocol

+  //

+  Status = gMmst->MmLocateProtocol (

+ &gEfiSmmFirmwareVolumeBlockProtocolGuid,

+    NULL,

+    (VOID **) &FvbProtocol

+    );

+  ASSERT_EFI_ERROR (Status);

+

+  Instance = INSTANCE_FROM_FVB_THIS (FvbProtocol);

+  // The Pcd is user defined, so make sure we don't overflow

+  if (Instance->MemBaseAddress > MAX_UINT64 - PcdGet32 
(PcdFlashNvStorageVariableSize)) {

I think this can be removed since the next condition is more strict.


+    return EFI_INVALID_PARAMETER;

+  }

+



[...]

+STATIC

+EFI_STATUS

+ReadWriteRpmb (

+  UINTN  SvcAct,

+  UINTN  Addr,

+  UINTN  NumBytes,

+  UINTN  Offset

+  )

+{

+  ARM_SVC_ARGS  SvcArgs;

+  EFI_STATUS    Status;

+

+  ZeroMem (&SvcArgs, sizeof (SvcArgs));

+

+  SvcArgs.Arg0 = ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH64;


If this is an FFA call, is it possible to:
 - put a reference in the header to the spec (it should be similar to 
the one at 
edk2/ArmPkg/Library/StandaloneMmMmuLib/AArch64/ArmMmuStandaloneMmLib.c)
 - check the return status of the SVC call against the ones available 
at edk2/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h

 - if possible, remove the dependency to 



+  SvcArgs.Arg1 = mStorageId;

+  SvcArgs.Arg2 = 0;

+  SvcArgs.Arg3 = SvcAct;

+  SvcArgs.Arg4 = Addr;

+  SvcArgs.Arg5 = NumBytes;

+  SvcArgs.Arg6 = Offset;

+

+  ArmCallSvc (&SvcArgs);

+  if (SvcArgs.Arg3) {

+    DEBUG ((DEBUG_ERROR, "%a: Svc Call 0x%08x Addr: 0x%08x len: 0x%x 
Offset: 0x%x failed with 0x%x\n",


+  __func__, SvcAct, Addr, NumBytes, Offset, SvcArgs.Arg3));

+  }

+

+  switch (SvcArgs.Arg3) {

+  case ARM_SVC_SPM_RET_SUCCESS:

+    Status = EFI_SUCCESS;

+    break;

+

+  case ARM_SVC_SPM_RET_NOT_SUPPORTED:

+    Status = EFI_UNSUPPORTED;

+    break;

+

+  case ARM_SVC_SPM_RET_INVALID_PARAMS:

+    Status = EFI_INVALID_PARAMETER;

+    break;

+

+  case ARM_SVC_SPM_RET_DENIED:

+    Status = EFI_ACCESS_DENIED;

+    break;

+

+  case ARM_SVC_SPM_RET_NO_MEMORY:

+    Status = EFI_OUT_OF_RESOURCES;

+    break;

+

+  default:

+    Status = EFI_ACCESS_DENIED;

+  }

+

+  return Status;

+}


[...]


+STATIC

+EFI_STATUS

+EFIAPI

+ValidateFvHeader (

+  IN EFI_FIRMWARE_VOLUME_HEADER    *FwVolHeader

+  )

+{

+  UINT16  Checksum;

+  VARIABLE_STORE_HEADER   *VariableStoreHeader;

+  UINTN   VariableStoreLength;

+  UINTN   FvLength;

+

+  FvLength = PcdGet32(PcdFlashNvStorageVariableSize) +

+ PcdGet32(PcdFlashNvStorageFtwWorkingSize) +

+ PcdGet32(PcdFlashNvStorageFtwSpareSize);

+

+  // Verify the header revision, header signature, length

+  // Length of FvBlock cannot be 2**64-1

+  // HeaderLength cannot be an odd number

+  //

+  if (   (FwVolHeader->Revision  != EFI_FVH_REVISION)

+  || (FwVolHeader->Signature != EFI_FVH_SIGNATURE)

+  || (FwVolHeader->FvLength  != FvLength)

+  )

could be on the same line -> ') {'


+  {

+    DEBUG ((DEBUG_INFO, "%a: No Firmware Volume header present\n",

+  __FUNCTION__));

+    return EFI_NOT_FOUND;

+  }

+

+  // Check the Firmware Volume Guid

+  if (!CompareGuid (&FwVolHeader->FileSystemGuid, 
&gEfiSystemNvDataFvGuid)) {


+    DEBUG ((DEBUG_INFO, "%a: Firmware Volume Guid non-compatible\n",

+  __FUNCTION__));

+    return EFI_VOLUME_CORRUPTED;

+  }

+

+  // Verify the header checksum

+  Checksum = CalculateSum16((UINT16*)FwVolHeader, 
FwVolHeader->HeaderLength);


+  if (Checksum != 0) {

+    DEBUG ((DEBUG_INFO, "%a: FV checksum is invalid (Checksum:0x%X)\n",

+  __FUNCTION__, Checksum));

+    return EFI_VOLUME_CORRUPTED;

+  }

+

+  VariableStoreHeader = (VARIABLE_STORE_HEADER*)((UINTN)FwVolHeader +

+ FwVolHeader->HeaderLength);

+

+  // Check the Variable Store Guid

+  if (!CompareGuid (&VariableStoreHeader->Signature, 
&gEfiVariableGuid) &&


+  !CompareGuid (&VariableStoreHeader->Signature, 
&gEfiAuthenticatedVariableGuid)) {


+    DEBUG ((DEBUG_INFO, "%a: Variable Store Guid non-compatible\n", 
__FUNCTION__));


+    return EFI_VOLUME_CORRUP

[edk2-devel] New Year, New PR Thread

2021-03-05 Thread Bret Barkelew via groups.io
Can we move to PRs yet? Maybe after the stabilization?

- Bret



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72504): https://edk2.groups.io/g/devel/message/72504
Mute This Topic: https://groups.io/mt/81108115/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [EXTERNAL] Re: [edk2-devel] [PATCH v6 0/7] Add MmUnblockMemoryLib Interface and Usages

2021-03-05 Thread Bret Barkelew via groups.io
“I had to jump through hoops to collect the individual patches from my list 
folder”
Man, it’s almost like you had Outlook. *shudder*

- Bret

From: Laszlo Ersek via groups.io
Sent: Friday, March 5, 2021 7:28 AM
To: devel@edk2.groups.io; 
ku...@outlook.com
Cc: Kinney, Michael D; Liming 
Gao; Zhiguang 
Liu; Yao, Jiewen; 
Jian J Wang; Hao A Wu; 
Ard Biesheuvel; Jordan 
Justen; Qi Zhang; 
Rahul Kumar
Subject: [EXTERNAL] Re: [edk2-devel] [PATCH v6 0/7] Add MmUnblockMemoryLib 
Interface and Usages

On 03/05/21 05:59, Kun Qin wrote:
> Hi,
>
> Thanks to all the reviewers helping through this patch series. Each 
> individual patch has received reviewed-by tag in this v6 version. It has also 
> just passed all CI build tests here: Unblock mem v6 by kuqin12 · Pull Request 
> #1473 · tianocore/edk2 
> (github.com)
>  Could one of the maintainers help to merge these patches into the mainline 
> when you have a chance?
>
> Please let me know if there is anything needed from me to merge in these 
> patches. Thanks in advance!

(1) Series merged as commit range c5740f360636..59a3ccb09e7a, via
.


(2) I couldn't tell if there was a TianoCore BZ specifically associated
with this series. Some patches in the series do not reference any BZs,
while some other patches reference two different BZs, namely #3168 and
#3169.

Neither #3168 nor #3169 contains links to *all six* postings (versions)
of the patch series. So I can't decide if now, with the v6 series
merged, I should close these tickets, or not. (IOW, if other tasks
remain, for solving the BZs.)

In case the tickets should be closed at this point, please go ahead and
close them yourself, as RESOLVED|FIXED. Please also include a new
comment in each ticket, repeating my point (1) above, verbatim -- each
solved BZ should highlight the commit range and the pull request that
solved it.


(3) For the future, please fix up your email setup. I'm not sure what's
happening -- it looks like whatever SMTP server you use throws away the
Message-Id headers generated by git-send-email, and generates new
Message-Ids. Or something similar -- FWIW, the In-Reply-To headers look
questionable as well.

Whatever the background, the threading in your posted patch set is
broken; I had to jump through hoops to collect the individual patches
from my list folder. Please fix this issue for your next contribution.

Thanks,
Laszlo


>
> Regards,
> Kun
>
> From: Kun Qin
> Sent: Thursday, March 4, 2021 20:13
> To: devel@edk2.groups.io
> Cc: Michael D Kinney; Liming 
> Gao; Zhiguang 
> Liu; Jiewen Yao; 
> Jian J Wang; Hao A 
> Wu; Laszlo Ersek; Ard 
> Biesheuvel; Jordan 
> Justen; Qi 
> Zhang; Rahul Kumar
> Subject: [edk2-devel] [PATCH v6 0/7] Add MmUnblockMemoryLib Interface and 
> Usages
>
> This patch series is a follow up of previous submission:
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F72442&data=04%7C01%7Cbret.barkelew%40microsoft.com%7C043a8827bad043bd7e9d08d8dfeb49ff%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637505548948688125%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=cNexu92PbyNbxrUN7l%2BdZqVcwfvHfKFohUuWPvO2Py4%3D&reserved=0
>
> v6 patches mainly focus on feedback for reviewed commits in v5 patches,
> including:
> a. Adding "Reviewed-by" and "Acked-by" tags for a

Re: [edk2-devel] [PATCH 1/1] EmbeddedPkg: Only print LibGetTime message about compile time epoch once

2021-03-05 Thread Ard Biesheuvel
On Fri, 5 Mar 2021 at 17:11, Rebecca Cran  wrote:
>
> The message "LibGetTime: RtcEpochSeconds non volatile variable was not
> found - Using compilation time epoch." can be printed a very large
> number of times, causing log files to become excessively large. This is
> because the RtcEpochSeconds variable only gets set if LibSetTime is
> called, for example by running 'time 12:00' in the UEFI Shell.
>
> Avoid this by setting RtcEpochSeconds to BUILD_EPOCH (EpochSeconds)
> after printing the message. It's set to a volatile variable so the
> message will be displayed on future boots and not hidden.
>
> Commit 44ae214591e58af468eacb7b873eaa0bc187c4fa reduced the verbosity of
> the message to DEBUG_VERBOSE. Revert it back to DEBUG_INFO so it's more
> prominent now that it doesn't get printed so frequently.
>
> Signed-off-by: Rebecca Cran 

Acked-by: Ard Biesheuvel 

> ---
>  EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c | 10 
> +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git 
> a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c 
> b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
> index 4210708cff36..de6fbb40e61b 100644
> --- a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
> +++ b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
> @@ -88,10 +88,18 @@ LibGetTime (
>  //
>  EpochSeconds = BUILD_EPOCH;
>  DEBUG ((
> -  DEBUG_VERBOSE,
> +  DEBUG_INFO,
>"LibGetTime: %s non volatile variable was not found - Using 
> compilation time epoch.\n",
>mEpochVariableName
>));
> +
> +EfiSetVariable (
> +  (CHAR16 *)mEpochVariableName,
> +  &gEfiCallerIdGuid,
> +  EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
> +  sizeof (EpochSeconds),
> +  &EpochSeconds
> +  );
>}
>Counter = GetPerformanceCounter ();
>EpochSeconds += DivU64x64Remainder (Counter, Freq, &Remainder);
> --
> 2.26.2
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72502): https://edk2.groups.io/g/devel/message/72502
Mute This Topic: https://groups.io/mt/81106280/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH 1/1] EmbeddedPkg: Only print LibGetTime message about compile time epoch once

2021-03-05 Thread Rebecca Cran
The message "LibGetTime: RtcEpochSeconds non volatile variable was not
found - Using compilation time epoch." can be printed a very large
number of times, causing log files to become excessively large. This is
because the RtcEpochSeconds variable only gets set if LibSetTime is
called, for example by running 'time 12:00' in the UEFI Shell.

Avoid this by setting RtcEpochSeconds to BUILD_EPOCH (EpochSeconds)
after printing the message. It's set to a volatile variable so the
message will be displayed on future boots and not hidden.

Commit 44ae214591e58af468eacb7b873eaa0bc187c4fa reduced the verbosity of
the message to DEBUG_VERBOSE. Revert it back to DEBUG_INFO so it's more
prominent now that it doesn't get printed so frequently.

Signed-off-by: Rebecca Cran 
---
 EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c | 10 
+-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git 
a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c 
b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
index 4210708cff36..de6fbb40e61b 100644
--- a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
+++ b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
@@ -88,10 +88,18 @@ LibGetTime (
 //
 EpochSeconds = BUILD_EPOCH;
 DEBUG ((
-  DEBUG_VERBOSE,
+  DEBUG_INFO,
   "LibGetTime: %s non volatile variable was not found - Using compilation 
time epoch.\n",
   mEpochVariableName
   ));
+
+EfiSetVariable (
+  (CHAR16 *)mEpochVariableName,
+  &gEfiCallerIdGuid,
+  EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
+  sizeof (EpochSeconds),
+  &EpochSeconds
+  );
   }
   Counter = GetPerformanceCounter ();
   EpochSeconds += DivU64x64Remainder (Counter, Freq, &Remainder);
-- 
2.26.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72501): https://edk2.groups.io/g/devel/message/72501
Mute This Topic: https://groups.io/mt/81106280/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live Migration for AMD SEV

2021-03-05 Thread Ashish Kalra
On Fri, Mar 05, 2021 at 10:44:23AM +, Ashish Kalra wrote:
> On Wed, Mar 03, 2021 at 01:25:40PM -0500, Tobin Feldman-Fitzthum wrote:
> > 
> > > Hi Tobin,
> > > 
> > > On 03/02/21 21:48, Tobin Feldman-Fitzthum wrote:
> > > > This is a demonstration of fast migration for encrypted virtual machines
> > > > using a Migration Handler that lives in OVMF. This demo uses AMD SEV,
> > > > but the ideas may generalize to other confidential computing platforms.
> > > > With AMD SEV, guest memory is encrypted and the hypervisor cannot access
> > > > or move it. This makes migration tricky. In this demo, we show how the
> > > > HV can ask a Migration Handler (MH) in the firmware for an encrypted
> > > > page. The MH encrypts the page with a transport key prior to releasing
> > > > it to the HV. The target machine also runs an MH that decrypts the page
> > > > once it is passed in by the target HV. These patches are not ready for
> > > > production, but the are a full end-to-end solution that facilitates a
> > > > fast live migration between two SEV VMs.
> > > > 
> > > > Corresponding patches for QEMU have been posted my colleague Dov Murik
> > > > on qemu-devel. Our approach needs little kernel support, requiring only
> > > > one hypercall that the guest can use to mark a page as encrypted or
> > > > shared. This series includes updated patches from Ashish Kalra and
> > > > Brijesh Singh that allow OVMF to use this hypercall.
> > > > 
> > > > The MH runs continuously in the guest, waiting for communication from
> > > > the HV. The HV starts an additional vCPU for the MH but does not expose
> > > > it to the guest OS via ACPI. We use the MpService to start the MH. The
> > > > MpService is only available at runtime and processes that are started by
> > > > it are usually cleaned up on ExitBootServices. Since we need the MH to
> > > > run continuously, we had to make some modifications. Ideally a feature
> > > > could be added to the MpService to allow for the starting of
> > > > long-running processes. Besides migration, this could support other
> > > > background processes that need to operate within the encryption
> > > > boundary. For now, we have included a handful of patches that modify the
> > > > MpService to allow the MH to keep running after ExitBootServices. These
> > > > are temporary.
> > > I plan to do a lightweight review for this series. (My understanding is
> > > that it's an RFC and not actually being proposed for merging.)
> > > 
> > > Regarding the MH's availability at runtime -- does that necessarily
> > > require the isolation of an AP? Because in the current approach,
> > > allowing the MP Services to survive into OS runtime (in some form or
> > > another) seems critical, and I don't think it's going to fly.
> > > 
> > > I agree that the UefiCpuPkg patches have been well separated from the
> > > rest of the series, but I'm somewhat doubtful the "firmware-initiated
> > > background process" idea will be accepted. Have you investigated
> > > exposing a new "runtime service" (a function pointer) via the UEFI
> > > Configuration table, and calling that (perhaps periodically?) from the
> > > guest kernel? It would be a form of polling I guess. Or maybe, poll the
> > > mailbox directly in the kernel, and call the new firmware runtime
> > > service when there's an actual command to process.
> > Continuous runtime availability for the MH is almost certainly the most
> > controversial part of this proposal, which is why I put it in the cover
> > letter and why it's good to discuss.
> > > (You do spell out "little kernel support", and I'm not sure if that's a
> > > technical benefit, or a political / community benefit.)
> > 
> > As you allude to, minimal kernel support is really one of the main things
> > that shapes our approach. This is partly a political and practical benefit,
> > but there are also technical benefits. Having the MH in firmware likely
> > leads to higher availability. It can be accessed when the OS is unreachable,
> > perhaps during boot or when the OS is hung. There are also potential
> > portability advantages although we do currently require support for one
> > hypercall. The cost of implementing this hypercall is low.
> > 
> > Generally speaking, our task is to find a home for functionality that was
> > traditionally provided by the hypervisor, but that needs to be inside the
> > trust domain, but that isn't really part of a guest. A meta-goal of this
> > project is to figure out the best way to do this.
> > 
> > > 
> > > I'm quite uncomfortable with an attempt to hide a CPU from the OS via
> > > ACPI. The OS has other ways to learn (for example, a boot loader could
> > > use the MP services itself, stash the information, and hand it to the OS
> > > kernel -- this would minimally allow for detecting an inconsistency in
> > > the OS). What about "all-but-self" IPIs too -- the kernel might think
> > > all the processors it's poking like that were under its control.
> > 
> > This might

Re: [edk2-devel] [PATCH 1/2 v5] Drivers/OpTeeRpmb: Add an OP-TEE backed RPMB driver

2021-03-05 Thread PierreGondois

Hi Ilias,

Thanks for the answers, I am re-reviewing the patch because I think I 
missed some things. I answered to your comments (inlined),


Regards,

Pierre


On 3/3/21 8:02 PM, Ilias Apalodimas wrote:

Hi Pierre,

On Wed, Mar 03, 2021 at 10:20:17AM +, Pierre wrote:

Hello Ilias,

My review is mostly about coding style, but I would have some (inlined)
remarks about your patch,

Regards,

Pierre

On 3/2/21 3:35 PM, Pierre Gondois wrote:




[...]


+� offer that from an RPMB, the driver allocates memory on init and
passes that

+� on the upper layers. Since the memory is dynamically allocated and we
can't set the

+� PCD is StMM context, we need to patch it correctly on each access

I think: is -> in

Ok


+

+� @retval EFI_SUCCESS Protocol was found and PCDs patched up

+

+ **/

I think there should not be a space here.

Ok


+EFI_STATUS

+EFIAPI

+FixPcdMemory (


[...]


+

+� // Patch PCDs with the the correct values

I think it s
'the the' -> the

Yea


+� PatchPcdSet64 (PcdFlashNvStorageVariableBase64,
Instance->MemBaseAddress);

[...]


+� @retval���������� EFI_OUT_OF_RESOURCES op-tee out of 
memory

+**/

+STATIC

+EFI_STATUS

+ReadWriteRpmb (

+� UINTN� SvcAct,

+� UINTN� Addr,

+� UINTN� NumBytes,

+� UINTN� Offset

+� )

I think the parameters should have IN/OUT indications, and the function
header aswell (cf 
https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/6_documenting_software/62_comments#6-2-1-only-use-c-style-comments-on-the-same-line-as-pre-processor-directives-and-in-doxygen-style-file-and-function-header-comment-blocks).
There are other functions with missing IN/OUT parameters.

Sure, (did you c/p the wrong link?)

Yes it seems so ...



+{


[...]


+OpTeeRpmbFvbSetAttributes (

+� IN CONST� EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,

+� IN OUT��� EFI_FVB_ATTRIBUTES_2 *Attributes

Parameters should be aligned. (I think this is valid at other places, like
for OpTeeRpmbFvbGetPhysicalAddress())

Ok


+� )

[...]


+� The GetBlockSize() function retrieves the size of the requested

+� block. It also returns the number of additional blocks with

+� the identical size. The GetBlockSize() function is used to

+� retrieve the block map (see EFI_FIRMWARE_VOLUME_HEADER).

+

+

+� @param This���������� Indicates the
EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL instance.

+

+� @param Lba����������� Indicates the block for which 
to return the size.

+

+� @param BlockSize����� Pointer to a caller-allocated UINTN in 
which

+����������������������� the size 
of the block is returned.

+

+� @param NumberOfBlocks Pointer to a caller-allocated UINTN in

+����������������������� which 
the number of consecutive blocks,

+����������������������� starting 
with Lba, is returned. All

+����������������������� blocks 
in this range have a size of

+����������������������� 
BlockSize.

+

+

+� @retval EFI_SUCCESS������������ The firmware 
volume base address was
returned.

+

+� @retval EFI_INVALID_PARAMETER�� The requested LBA is out of range.

+

+**/

+STATIC

+EFI_STATUS

+OpTeeRpmbFvbGetBlockSize (

+� IN CONST� EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,

+� IN������� 
EFI_LBA���������������������������
 Lba,

+� OUT������ UINTN *BlockSize,

+� OUT������ UINTN *NumberOfBlocks

+� )

+{

+� MEM_INSTANCE *Instance;

+

+� Instance = INSTANCE_FROM_FVB_THIS (This);

+� if (Lba > Instance->NBlocks) {

+��� return EFI_INVALID_PARAMETER;

+� }

+

+� *NumberOfBlocks = Instance->NBlocks - (UINTN)Lba;

+� *BlockSize = Instance->BlockSize;

+

+� return EFI_SUCCESS;

+}

+

+/**

+� Reads the specified number of bytes into a buffer from the specified
block.

+

+� The Read() function reads the requested number of bytes from the

+� requested block and stores them in the provided buffer.

+� Implementations should be mindful that the firmware volume

+� might be in the ReadDisabled state. If it is in this state,

+� the Read() function must return the status code

+� EFI_ACCESS_DENIED without modifying the contents of the

+� buffer. The Read() function must also prevent spanning block

+� boundaries. If a read is requested that would span a block

+� boundary, the read must read up to the boundary but not

+� beyond. The output parameter NumBytes must be set to correctly

+� indicate the number of bytes actually read. The caller must be

+� aware that a read may be partially completed.

+

+� @param This���� Indicates the EFI_FIRMWARE_VOLUME_BLOCK

Re: [edk2-devel] [PATCH v6 0/7] Add MmUnblockMemoryLib Interface and Usages

2021-03-05 Thread Laszlo Ersek
On 03/05/21 05:59, Kun Qin wrote:
> Hi,
> 
> Thanks to all the reviewers helping through this patch series. Each 
> individual patch has received reviewed-by tag in this v6 version. It has also 
> just passed all CI build tests here: Unblock mem v6 by kuqin12 · Pull Request 
> #1473 · tianocore/edk2 
> (github.com) Could one of the 
> maintainers help to merge these patches into the mainline when you have a 
> chance?
> 
> Please let me know if there is anything needed from me to merge in these 
> patches. Thanks in advance!

(1) Series merged as commit range c5740f360636..59a3ccb09e7a, via
.


(2) I couldn't tell if there was a TianoCore BZ specifically associated
with this series. Some patches in the series do not reference any BZs,
while some other patches reference two different BZs, namely #3168 and
#3169.

Neither #3168 nor #3169 contains links to *all six* postings (versions)
of the patch series. So I can't decide if now, with the v6 series
merged, I should close these tickets, or not. (IOW, if other tasks
remain, for solving the BZs.)

In case the tickets should be closed at this point, please go ahead and
close them yourself, as RESOLVED|FIXED. Please also include a new
comment in each ticket, repeating my point (1) above, verbatim -- each
solved BZ should highlight the commit range and the pull request that
solved it.


(3) For the future, please fix up your email setup. I'm not sure what's
happening -- it looks like whatever SMTP server you use throws away the
Message-Id headers generated by git-send-email, and generates new
Message-Ids. Or something similar -- FWIW, the In-Reply-To headers look
questionable as well.

Whatever the background, the threading in your posted patch set is
broken; I had to jump through hoops to collect the individual patches
from my list folder. Please fix this issue for your next contribution.

Thanks,
Laszlo


> 
> Regards,
> Kun
> 
> From: Kun Qin
> Sent: Thursday, March 4, 2021 20:13
> To: devel@edk2.groups.io
> Cc: Michael D Kinney; Liming 
> Gao; Zhiguang 
> Liu; Jiewen Yao; 
> Jian J Wang; Hao A 
> Wu; Laszlo Ersek; Ard 
> Biesheuvel; Jordan 
> Justen; Qi 
> Zhang; Rahul Kumar
> Subject: [edk2-devel] [PATCH v6 0/7] Add MmUnblockMemoryLib Interface and 
> Usages
> 
> This patch series is a follow up of previous submission:
> https://edk2.groups.io/g/devel/message/72442
> 
> v6 patches mainly focus on feedback for reviewed commits in v5 patches,
> including:
> a. Adding "Reviewed-by" and "Acked-by" tags for applicable patch;
> b. Updating library class description for newly added interface;
> 
> Patch v6 branch: https://github.com/kuqin12/edk2/tree/unblock_mem_v6
> 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Zhiguang Liu 
> Cc: Jiewen Yao 
> Cc: Jian J Wang 
> Cc: Hao A Wu 
> Cc: Laszlo Ersek 
> Cc: Ard Biesheuvel 
> Cc: Jordan Justen 
> Cc: Qi Zhang 
> Cc: Rahul Kumar 
> 
> Kun Qin (7):
>   MdePkg: MmUnblockMemoryLib: Added definition and null instance
>   OvmfPkg: resolve MmUnblockMemoryLib (mainly for VariableSmmRuntimeDxe)
>   MdeModulePkg: VariableSmmRuntimeDxe: Added request unblock memory
> interface
>   SecurityPkg: Tcg2Smm: Switching from gSmst to gMmst
>   SecurityPkg: Tcg2Smm: Separate Tcg2Smm into 2 modules
>   SecurityPkg: Tcg2Smm: Added support for Standalone Mm
>   SecurityPkg: Tcg2Acpi: Added unblock memory interface for NVS region
> 
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c   |  42 +
>  MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.c   |  44 +
>  SecurityPkg/Tcg/{Tcg2Smm/Tcg2Smm.c => Tcg2Acpi/Tcg2Acpi.c}   | 362 
> -
>  SecurityPkg/Tcg/Tcg2Smm/Tcg2MmDependencyDxe.c|  48 ++
>  SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c| 857 
> 
>  SecurityPkg/Tcg/Tcg2Smm/Tcg2StandaloneMm.c   |  71 ++
>  SecurityPkg/Tcg/Tcg2Smm/Tcg2TraditionalMm.c  |  82 ++
>  MdeModulePkg/MdeModulePkg.dsc|   1 +
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf |   1 +
>  MdePkg/Include/Library/MmUnblockMemoryLib.h  |  44 +
>  MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf |  34 +
>  MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.uni |  21 +
>  MdePkg/MdePkg.dec|   5 +
>  MdePkg/MdePkg.dsc|   

Re: [edk2-devel] [PATCH v3 0/5] UefiCpuPkg/StandaloneMmCpuFeaturesLib: Add Standalone MM support

2021-03-05 Thread Laszlo Ersek
Eric, Ray,

On 02/17/21 22:32, Michael Kubacki wrote:
> From: Michael Kubacki 
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3218
> 
> The present SmmCpuFeaturesLib implementation in UefiCpuPkg can be
> useful for IA32/X64 platforms that need a library instance for
> a Standalone MM environment. Much of the logic can be reused and a
> new INF can isolate the differences unique to Standalone MM.
> 
> This patch series contains an initial set of changes for cleaning
> up pre-existing design issues in the library. The final two patches
> contain changes needed for Standalone MM support.
> 
> Here's an overview of how the three library instances are organized
> that may be a useful reference (provided by Laszlo):
> 
> Traditional,  Traditional,  Standalone,
> no STMSTM   no STM
> 
> Entry point type   DXE   DXE   MM
> 
> Lib inst. init.basic STM   basic
> 
> Processor init.basic STM   basic
> 
> PCD access any   any   fixed
> 
> * Traditional no STM = SmmCpuFeaturesLib.inf
> * Traditional STM = SmmCpuFeaturesLibStm.inf
> * Standalone no STM = StandaloneMmCpuFeaturesLib.inf

do you have any comments please?

Thanks,
Laszlo


> 
> V3 changes:
> 
>   PATCH v3 2/5 is a new patch in the series that renames the file
>   SmmCpuFeaturesLib.c to SmmCpuFeaturesLibCommon.c to more clearly
>   identify implementation in the file as shared between all library
>   instances.
> 
>   PATCH v3 3/5 adds a new source file SmmCpuFeaturesLib.c that
>   contains the constructor specific to the Traditional MM no
>   STM library instance. This was previously implemented in a
>   file built by the Standalone MM instance and while not
>   harmful, it was not clean.
> 
>   PATCH v3 4/5 updates "@retval" to "@return" in the documentation
>   for GetCpuMaxLogicalProcessorNumber() since it is not a constant
>   return value.
>   
>   PATCH v3 5/5 contains a commit message update to note that all
>   instances of "PiSmm.h" in the library source files have been
>   updated to "PiMm.h" for consistency throughout the library.
> 
> V2 changes:
> 
>   Due to some pre-existing design issues in the library that
>   affected a single v1 patch that add Standalone MM support,
>   it was suggested to first address those issues and then add the
>   new INF StandaloneMmCpuFeaturesLib.inf.
> 
>   To address these concerns, the following v1 patch was converted
>   into a v2 patch series:
>   https://edk2.groups.io/g/devel/message/71626
> 
>   The first two patches in v2 primarily addressed those concerns.
> 
>   PATCH v2 1/4 and PATCH v2 2/4 focused on fixing pre-existing
>   design issues.
> 
>   PATCH v2 3/4 and PATCH v2 4/4 focused on the changes needed to add
>   Standalone MM support.
> 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Cc: Rahul Kumar 
> Signed-off-by: Michael Kubacki 
> 
> Michael Kubacki (5):
>   UefiCpuPkg/SmmCpuFeaturesLib: Move multi-instance function decl to
> header
>   UefiCpuPkg/SmmCpuFeaturesLib: Rename SmmCpuFeaturesLib.c
>   UefiCpuPkg/SmmCpuFeaturesLib: Cleanup library constructors
>   UefiCpuPkg/SmmCpuFeaturesLib: Abstract PcdCpuMaxLogicalProcessorNumber
>   UefiCpuPkg/SmmCpuFeaturesLib: Add Standalone MM support
> 
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmmStmSupport.c
>   |   2 +-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c 
>   | 608 +---
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/{SmmCpuFeaturesLib.c => 
> SmmCpuFeaturesLibCommon.c}|  36 +-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibNoStm.c
>   |   3 +-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c
>   |  26 +-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.c
>   |  50 ++
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/TraditionalMmCpuFeaturesLib.c   
>   |  28 +
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmmStmSupport.c 
>   |   2 +-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
>   |  48 ++
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf   
>   |   3 +
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
>   |   4 +-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/{SmmCpuFeaturesLib.inf => 
> StandaloneMmCpuFeaturesLib.inf} |  22 +-
>  UefiCpuPkg/UefiCpuPkg.dsc
>   |   1 +
>  13 files changed, 172 insertions(+), 661 deletions(-)
>  copy UefiCpuPkg/Library/SmmCpuFeaturesLib/{SmmCpuFeaturesLib.c => 
> SmmCpuFeaturesLibCommon.c} (93%)
>  create mode 100644 
> UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuF

Re: [edk2-devel] [edk2-platforms][PATCH V2 0/2] Enable SMMUv3 for Arm SGI/RD platforms

2021-03-05 Thread Sami Mujawar
Hi Vivek,

Thank you for this patch.

For this series.
Reviewed-by: Sami Mujawar 

I will merge this series once the merge window opens.

Regards,

Sami Mujawar

-Original Message-
From: Vivek Kumar Gautam  
Sent: 05 March 2021 01:21 PM
To: devel@edk2.groups.io
Cc: ardb+tianoc...@kernel.org; l...@nuviainc.com; Sami Mujawar 

Subject: Re: [edk2-devel] [edk2-platforms][PATCH V2 0/2] Enable SMMUv3 for Arm 
SGI/RD platforms



On 3/5/21 6:44 PM, Vivek Kumar Gautam via groups.io wrote:
> Arm's SMMUv3 present in various SGI/RD platforms provides address
> translation support for devices such as the ones present over PCIe.
> SMMUv3 also supports Address Translation Service (ATS) and Page
> Request Interface (PRI) to work with PCIe devices.
> ATS allows PCIe devices to request translation from a translation
> agent such as SMMU, and then cache these translation in their private
> cache called as Address Translation Cache (ATC).
> Devices that support PRI can also enable the feature when ATS is
> enabled as ATS is a prerequisite for PRI.
> 
> The I/O topology on SGI/RD platforms includes I/O devices (or PCIe
> devices) connected to a SMMU-v3, and an GIC ITS block that facilitates
> interrupt translations for message signaled interrupts. A typical view
> of this topology is as below -
> 
>---      
>   |  PCIe device  |>|  SMMUv3|>|   ITS  |
>   | (RequesterID) | | (StreamID) | | (DeviceID) |
>---      
> 
> This patch series adds the SMMU-v3 node in iort table, and sets up the
> connection between these iort nodes to forward the traffic in the right
> manner.

After applying these patches, the resulting updated Iort Acpi table 
looks like below:


Shell> Acpiview -s iort





  --- IORT Table ---



Address  : 0xF98DF598

Length   : 236



 : 49 4F 52 54 EC 00 00 00 - 00 A5 41 52 4D 4C 54 44 
IORT..ARMLTD

0010 : 41 52 4D 53 47 49 20 20 - 27 07 14 20 41 52 4D 20   ARMSGI 
'.. ARM

0020 : 99 00 00 00 03 00 00 00 - 30 00 00 00 00 00 00 00 
0...

0030 : 00 18 00 00 00 00 00 00 - 00 00 00 00 00 00 00 00 


0040 : 01 00 00 00 00 00 00 00 - 04 6C 00 02 00 00 00 00 
.l..

0050 : 02 00 00 00 44 00 00 00 - 00 00 00 4F 00 00 00 00 
D..O

0060 : 01 00 00 00 00 00 00 00 - 00 00 00 00 00 00 00 00 


0070 : 00 00 00 00 04 01 00 00 - 00 00 00 00 06 01 00 00 


0080 : 05 01 00 00 00 00 00 00 - 01 00 00 00 00 00 00 00 


0090 : FF FF 00 00 00 00 00 00 - 30 00 00 00 00 00 00 00 
0...

00A0 : 00 00 00 00 01 00 00 00 - 00 00 01 00 30 00 00 00 
0...

00B0 : 01 00 00 00 02 38 00 00 - 00 00 00 00 01 00 00 00 
.8..

00C0 : 24 00 00 00 01 00 00 00 - 00 00 00 00 01 00 00 00 
$...

00D0 : 00 00 00 00 00 00 00 00 - 00 00 00 00 FF FF 00 00 


00E0 : 00 00 00 00 48 00 00 00 - 00 00 00 00   H...



Table Checksum : OK



IORT :

   Signature  : IORT

   Length : 236

   Revision   : 0

   Checksum   : 0xA5

   Oem ID : ARMLTD

   Oem Table ID   : ARMSGI

   Oem Revision   : 0x20140727

   Creator ID : ARM

   Creator Revision   : 0x99

   Number of IORT Nodes   : 3

   Offset to Array of IORT Nodes  : 0x30

   Reserved   : 0x0

   * Node Offset *: 0x30

   ITS Node   :

 Type : 0

 Length   : 24

 Revision : 0

 Reserved : 0x0

 Number of ID mappings: 0

 Reference to ID Array: 0x0

 Number of ITSs   : 1

 GIC ITS Identifier Array [0] :

   GIC ITS Identifier : 0

   * Node Offset *: 0x48

   SMMUV3 Node:

 Type : 4

 Length   : 108

 Revision : 2

 Reserved : 0x0

 Number of ID mappings: 2

 Reference to ID Array: 0x44

 Base Address : 0x4F00

 Flags: 0x1

 Reserved : 0x0

 VATOS Address: 0x0

 Model: 0

 Event: 0x104

 PRI  : 0x0

 GERR   

Re: [edk2-devel] [edk2-platforms][PATCH V2 0/2] Enable SMMUv3 for Arm SGI/RD platforms

2021-03-05 Thread Vivek Kumar Gautam




On 3/5/21 6:44 PM, Vivek Kumar Gautam via groups.io wrote:

Arm's SMMUv3 present in various SGI/RD platforms provides address
translation support for devices such as the ones present over PCIe.
SMMUv3 also supports Address Translation Service (ATS) and Page
Request Interface (PRI) to work with PCIe devices.
ATS allows PCIe devices to request translation from a translation
agent such as SMMU, and then cache these translation in their private
cache called as Address Translation Cache (ATC).
Devices that support PRI can also enable the feature when ATS is
enabled as ATS is a prerequisite for PRI.

The I/O topology on SGI/RD platforms includes I/O devices (or PCIe
devices) connected to a SMMU-v3, and an GIC ITS block that facilitates
interrupt translations for message signaled interrupts. A typical view
of this topology is as below -

   ---      
  |  PCIe device  |>|  SMMUv3|>|   ITS  |
  | (RequesterID) | | (StreamID) | | (DeviceID) |
   ---      

This patch series adds the SMMU-v3 node in iort table, and sets up the
connection between these iort nodes to forward the traffic in the right
manner.


After applying these patches, the resulting updated Iort Acpi table
looks like below:


Shell> Acpiview -s iort





 --- IORT Table ---



Address  : 0xF98DF598

Length   : 236



 : 49 4F 52 54 EC 00 00 00 - 00 A5 41 52 4D 4C 54 44
IORT..ARMLTD

0010 : 41 52 4D 53 47 49 20 20 - 27 07 14 20 41 52 4D 20   ARMSGI
'.. ARM

0020 : 99 00 00 00 03 00 00 00 - 30 00 00 00 00 00 00 00
0...

0030 : 00 18 00 00 00 00 00 00 - 00 00 00 00 00 00 00 00


0040 : 01 00 00 00 00 00 00 00 - 04 6C 00 02 00 00 00 00
.l..

0050 : 02 00 00 00 44 00 00 00 - 00 00 00 4F 00 00 00 00
D..O

0060 : 01 00 00 00 00 00 00 00 - 00 00 00 00 00 00 00 00


0070 : 00 00 00 00 04 01 00 00 - 00 00 00 00 06 01 00 00


0080 : 05 01 00 00 00 00 00 00 - 01 00 00 00 00 00 00 00


0090 : FF FF 00 00 00 00 00 00 - 30 00 00 00 00 00 00 00
0...

00A0 : 00 00 00 00 01 00 00 00 - 00 00 01 00 30 00 00 00
0...

00B0 : 01 00 00 00 02 38 00 00 - 00 00 00 00 01 00 00 00
.8..

00C0 : 24 00 00 00 01 00 00 00 - 00 00 00 00 01 00 00 00
$...

00D0 : 00 00 00 00 00 00 00 00 - 00 00 00 00 FF FF 00 00


00E0 : 00 00 00 00 48 00 00 00 - 00 00 00 00   H...



Table Checksum : OK



IORT :

  Signature  : IORT

  Length : 236

  Revision   : 0

  Checksum   : 0xA5

  Oem ID : ARMLTD

  Oem Table ID   : ARMSGI

  Oem Revision   : 0x20140727

  Creator ID : ARM

  Creator Revision   : 0x99

  Number of IORT Nodes   : 3

  Offset to Array of IORT Nodes  : 0x30

  Reserved   : 0x0

  * Node Offset *: 0x30

  ITS Node   :

Type : 0

Length   : 24

Revision : 0

Reserved : 0x0

Number of ID mappings: 0

Reference to ID Array: 0x0

Number of ITSs   : 1

GIC ITS Identifier Array [0] :

  GIC ITS Identifier : 0

  * Node Offset *: 0x48

  SMMUV3 Node:

Type : 4

Length   : 108

Revision : 2

Reserved : 0x0

Number of ID mappings: 2

Reference to ID Array: 0x44

Base Address : 0x4F00

Flags: 0x1

Reserved : 0x0

VATOS Address: 0x0

Model: 0

Event: 0x104

PRI  : 0x0

GERR : 0x106

Sync : 0x105

Proximity domain : 0x0

Device ID mapping index  : 1

ID Mapping [0]   :

  Input base : 0x0

  Number of IDs  : 0x

  Output base: 0x0

  Output reference   : 0x30

  Flags  : 0x0

ID Mapping [1]   :

  Input base : 0x0

  Number of IDs  : 

[edk2-devel] [edk2-platforms][PATCH V2 0/2] Enable SMMUv3 for Arm SGI/RD platforms

2021-03-05 Thread Vivek Kumar Gautam
Arm's SMMUv3 present in various SGI/RD platforms provides address
translation support for devices such as the ones present over PCIe.
SMMUv3 also supports Address Translation Service (ATS) and Page
Request Interface (PRI) to work with PCIe devices.
ATS allows PCIe devices to request translation from a translation
agent such as SMMU, and then cache these translation in their private
cache called as Address Translation Cache (ATC).
Devices that support PRI can also enable the feature when ATS is
enabled as ATS is a prerequisite for PRI.

The I/O topology on SGI/RD platforms includes I/O devices (or PCIe
devices) connected to a SMMU-v3, and an GIC ITS block that facilitates
interrupt translations for message signaled interrupts. A typical view
of this topology is as below -

 ---      
|  PCIe device  |>|  SMMUv3|>|   ITS  |
| (RequesterID) | | (StreamID) | | (DeviceID) |
 ---      

This patch series adds the SMMU-v3 node in iort table, and sets up the
connection between these iort nodes to forward the traffic in the right
manner.

Changes since v1:
- Addressed review comments given by Sami:
  - Replaced __builtin_offsetof() with OFFSET_OF() as suggested by Sami.
  - Updated the commit message for patches.

Vivek Gautam (2):
  Platform/Sgi: Add smmu-v3 node in the iort acpi table
  Platform/Sgi: Enable ATS mode over PCI root complex

 Platform/ARM/SgiPkg/AcpiTables/Iort.aslc | 60 ++--
 1 file changed, 55 insertions(+), 5 deletions(-)

-- 
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72492): https://edk2.groups.io/g/devel/message/72492
Mute This Topic: https://groups.io/mt/81102021/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [edk2-platforms][PATCH V2 1/2] Platform/Sgi: Add smmu-v3 node in the iort acpi table

2021-03-05 Thread Vivek Kumar Gautam
Arm's SMMU-v3 present in various SGI/RD platforms provides address
translation support for devices such as the ones present over PCIe
bus. SMMU-v3 also supports Address Translation Service (ATS) and
Page Request Interface (PRI) to work with PCIe devices.

The overall system topology looks as below:
 ---      
|  PCIe device  |>|  SMMUv3|>|   ITS  |
| (RequesterID) | | (StreamID) | | (DeviceID) |
 ---      

SMMU-v3 accepts requests coming from the PCIe device, and forwards
the traffic to the GIC ITS block that can provide the translation for
interrupts coming from LPI sources.

Add this generic SMMUv3 type node in the iort table and setup the
rid->stream-id->device-id mapping accordingly.

Signed-off-by: Vivek Gautam 
---
 Platform/ARM/SgiPkg/AcpiTables/Iort.aslc | 58 ++--
 1 file changed, 54 insertions(+), 4 deletions(-)

diff --git a/Platform/ARM/SgiPkg/AcpiTables/Iort.aslc 
b/Platform/ARM/SgiPkg/AcpiTables/Iort.aslc
index 58ec31ddc837..ce8eefc585ea 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/Iort.aslc
+++ b/Platform/ARM/SgiPkg/AcpiTables/Iort.aslc
@@ -20,6 +20,12 @@ typedef struct
   UINT32   ItsIdentifiers;
 } ARM_EFI_ACPI_6_0_IO_REMAPPING_ITS_NODE;
 
+typedef struct
+{
+  EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE SmmuNode;
+  EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE   SmmuIdMap[2];
+} ARM_EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE;
+
 typedef struct
 {
   EFI_ACPI_6_0_IO_REMAPPING_RC_NODERcNode;
@@ -30,6 +36,7 @@ typedef struct
 {
   EFI_ACPI_6_0_IO_REMAPPING_TABLE  Header;
   ARM_EFI_ACPI_6_0_IO_REMAPPING_ITS_NODE   ItsNode;
+  ARM_EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE SmmuNode;
   ARM_EFI_ACPI_6_0_IO_REMAPPING_RC_NODERcNode;
 } ARM_EFI_ACPI_6_0_IO_REMAPPING_TABLE;
 
@@ -45,7 +52,7 @@ ARM_EFI_ACPI_6_0_IO_REMAPPING_TABLE Iort =
ARM_EFI_ACPI_6_0_IO_REMAPPING_TABLE,
EFI_ACPI_IO_REMAPPING_TABLE_REVISION
  ),
- 2,  // NumNodes
+ 3,  // NumNodes
  sizeof (EFI_ACPI_6_0_IO_REMAPPING_TABLE),  // NodeOffset
  0,  // Reserved
   },
@@ -62,9 +69,52 @@ ARM_EFI_ACPI_6_0_IO_REMAPPING_TABLE Iort =
 0,  // NumIdMappings
 0,  // IdReference
   },
-  1,// GIC ITS Identifiers
+  1,// ITS count
+},
+0,  // GIC ITS Identifiers
+  },
+  // SMMU
+  {
+// EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE
+{
+  // EFI_ACPI_6_0_IO_REMAPPING_NODE
+  {
+EFI_ACPI_IORT_TYPE_SMMUv3, // Type
+sizeof (ARM_EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE), // Length
+2, // Revision
+0, // Reserved
+2, // NumIdMapping
+OFFSET_OF (ARM_EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE, SmmuIdMap), // 
IdReference
+  },
+  0x4F00, // Base address
+  EFI_ACPI_IORT_SMMUv3_FLAG_COHAC_OVERRIDE, // Flags
+  0,   // Reserved
+  0,   // VATOS address
+  EFI_ACPI_IORT_SMMUv3_MODEL_GENERIC, // SMMUv3 Model
+  260, // Event
+  0,   // Pri
+  262, // Gerror
+  261, // Sync
+  0,   // Proximity domain
+  1,   // DevIDMappingIndex
+},
+// EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE
+{
+  {
+0x, // InputBase
+0x, // NumIds
+0x, // OutputBase
+OFFSET_OF (ARM_EFI_ACPI_6_0_IO_REMAPPING_TABLE, ItsNode), // 
OutputReference
+0, // Flags
+  },
+  {
+0x0, // InputBase
+0x1, // NumIds
+0x1, // OutputBase
+OFFSET_OF (ARM_EFI_ACPI_6_0_IO_REMAPPING_TABLE, ItsNode), // 
OutputReference
+EFI_ACPI_IORT_ID_MAPPING_FLAGS_SINGLE, // Flags
+  },
 },
-0,
   },
   // ARM_EFI_ACPI_6_0_IO_REMAPPING_RC_NODE
   {
@@ -91,7 +141,7 @@ ARM_EFI_ACPI_6_0_IO_REMAPPING_TABLE Iort =
   0x,  // InputBase
   0x,  // NumIds
   0x,  // OutputBase
-  OFFSET_OF (ARM_EFI_ACPI_6_0_IO_REMAPPING_TABLE, ItsNode),  // 
OutputReference
+  OFFSET_OF (ARM_EFI_ACPI_6_0_IO_REMAPPING_TABLE, SmmuNode),  // 
OutputReference
   0,   // Flags
 }
   }
-- 
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72493): https://edk2.groups.io/g/devel/message/72493
Mute This Topic: https://groups.io/mt/81102022/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [edk2-platforms][PATCH V2 2/2] Platform/Sgi: Enable ATS mode over PCI root complex

2021-03-05 Thread Vivek Kumar Gautam
Enable Address Translation Service (ATS) support for the PCI root
complex listed in the iort table.
ATS allows PCIe devices to request an address translation before
starting the dma transaction, so that devices can cache these
translations in their private cache that is called as Address
Translation Cache (ATC).
Devices that support Page Request Interface (PRI) can also enable
the feature when ATS is enabled as ATS is a prerequisite for PRI.

Signed-off-by: Vivek Gautam 
---
 Platform/ARM/SgiPkg/AcpiTables/Iort.aslc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Platform/ARM/SgiPkg/AcpiTables/Iort.aslc 
b/Platform/ARM/SgiPkg/AcpiTables/Iort.aslc
index ce8eefc585ea..fcc28a71c82e 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/Iort.aslc
+++ b/Platform/ARM/SgiPkg/AcpiTables/Iort.aslc
@@ -133,7 +133,7 @@ ARM_EFI_ACPI_6_0_IO_REMAPPING_TABLE Iort =
   0,  // AllocationHints
   0,  // Reserved
   0,  // MemoryAccessFlags
-  EFI_ACPI_IORT_ROOT_COMPLEX_ATS_UNSUPPORTED,  // AtsAttribute
+  EFI_ACPI_IORT_ROOT_COMPLEX_ATS_SUPPORTED,// AtsAttribute
   0x0, // PciSegmentNumber
 },
 // EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE
-- 
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72491): https://edk2.groups.io/g/devel/message/72491
Mute This Topic: https://groups.io/mt/81102020/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [edk2-platforms][PATCH V1 1/2] Platform/Sgi: Add smmu node in the iort acpi table

2021-03-05 Thread Vivek Kumar Gautam

Hi Sami,


On 3/1/21 8:08 PM, Sami Mujawar via Groups.Io wrote:

Hi Vivek,

Can you include the description you included in the cover letter in the commit 
message for this patch, please?


Sure, I will update the commit message.



On Mon, Feb 15, 2021 at 01:00 PM, Vivek Kumar Gautam wrote:



+ __builtin_offsetof (ARM_EFI_ACPI_6_0_IO_REMAPPING_TABLE, ItsNode=


Use OFFSET_OF() macro as __builtin_offsetof is compiler specific.


Sure. Thanks for the input and the review.

Best regards
Vivek



Regards,

Sami Mujawar


IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72490): https://edk2.groups.io/g/devel/message/72490
Mute This Topic: https://groups.io/mt/80664134/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [edk2-platforms] [PATCH v1 0/9] IpmiFeaturePkg: Add IPMI transport drivers

2021-03-05 Thread Nhi Pham via groups.io

Hi Isaac,

We - Ampere Computing  are implementing IPMI over SSIF transport (per 
IPMI 2.0 spec). We also have this ready for submission, soon.
However, in light of this review, we would be interested in co-work with 
Intel so the same design can be used on non-KCS platform, instead of 
keeping each implementation proprietary.


Thanks,
Nhi

On 3/5/21 06:33, Oram, Isaac W via groups.io wrote:

Michael,

Thanks for the feedback.

I was torn between aligning with the proprietary version and cleaning it up.
My concern is if we do too much cleanup, it will delay adoption.

I did the minimum that we know is required for ECC tool to pass coding style 
tool and avoid EFI prefixes.

I would like delay refactoring and cleaning up until it is in the open where 
people can easily follow the code evolution from proprietary to open source.  I 
am looking to develop some maintainers for this feature package, and the 
cleanup would be a good way to ramp them into active open participation and put 
them on the path to becoming maintainers.

Anyway, your feedback is noted and I will put those on the to do list for 
completing this.

Regards,
Isaac

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Michael Kubacki
Sent: Wednesday, March 3, 2021 11:23 AM
To: devel@edk2.groups.io; Desimone, Nathaniel L 
Cc: Oram, Isaac W ; Chaganty, Rangasai V 
; Liming Gao ; Michael Kubacki 

Subject: Re: [edk2-devel] [edk2-platforms] [PATCH v1 0/9] IpmiFeaturePkg: Add 
IPMI transport drivers

Looked over the series at a high-level and the feature structure looks fine. 
For the series:
Acked-by: Michael Kubacki 

I didn't look closely at implementation but there's a few things I noticed.

There's a few typos. I'd suggest running a spell check to clean those up. Some 
quick examples:

ServerManagement.h:

   "Generic Definations for Server Management Header File."

typedef struct {
  LINERIZATION_TYPE Linearization;  // L

IpmiTransportPei.h:

"IPMI Ttransport PPI Header File."

There was a mix of function documentation styles though that might be something 
to clean up later.

I saw some globals that did not appear necessary. For example, mIpmiTransport 
in SmmIpmiBaseLib.c seems to be located before every access.

Thanks,
Michael

On 3/1/2021 6:27 PM, Nate DeSimone wrote:

From: Isaac Oram 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3242

Implement an open source generic IPMI transport driver.
Provides the ability to communicate with a BMC over IPMI in
MinPlatform board packages.

New changes:
1. Added GenericIpmi
2. Added IPMI base libs
3. Added transport PPI and protocol
4. Updated IPMI command request and response data size from
   UINT8 to UINT32 in IPMI transport layer to be compatible
   with EDK2 industry standard IPMI commands.
6. Added the completion code in the first byte of all IPMI
   response data to be compatible with EDK2 industry standard
   IPMI commands.

Cc: Sai Chaganty 
Cc: Liming Gao 
Cc: Michael Kubacki 
Signed-off-by: Isaac Oram 
Co-authored-by: Nate DeSimone 

Isaac Oram (9):
IpmiFeaturePkg: Add IPMI driver Include headers
IpmiFeaturePkg: Add IpmiBaseLib and IpmiCommandLib
IpmiFeaturePkg: Add IpmiInit drivers
IpmiFeaturePkg: Add GenericIpmi driver common code
IpmiFeaturePkg: Add GenericIpmi PEIM
IpmiFeaturePkg: Add GenericIpmi DXE Driver
IpmiFeaturePkg: Add GenericIpmi SMM Driver
IpmiFeaturePkg: Add IPMI driver build files
Maintainers.txt: Add IpmiFeaturePkg maintainers

   .../GenericIpmi/Common/IpmiBmc.c  | 297 +++
   .../GenericIpmi/Common/IpmiBmc.h  |  44 ++
   .../GenericIpmi/Common/IpmiBmcCommon.h| 179 +++
   .../GenericIpmi/Common/IpmiHooks.c|  96 
   .../GenericIpmi/Common/IpmiHooks.h|  81 +++
   .../GenericIpmi/Common/IpmiPhysicalLayer.h|  29 ++
   .../GenericIpmi/Common/KcsBmc.c   | 483 ++
   .../GenericIpmi/Common/KcsBmc.h   | 236 +
   .../GenericIpmi/Dxe/GenericIpmi.c |  46 ++
   .../GenericIpmi/Dxe/GenericIpmi.inf   |  66 +++
   .../IpmiFeaturePkg/GenericIpmi/Dxe/IpmiInit.c | 452 
   .../GenericIpmi/Pei/PeiGenericIpmi.c  | 362 +
   .../GenericIpmi/Pei/PeiGenericIpmi.h  | 138 +
   .../GenericIpmi/Pei/PeiGenericIpmi.inf|  58 +++
   .../GenericIpmi/Pei/PeiIpmiBmc.c  | 277 ++
   .../GenericIpmi/Pei/PeiIpmiBmc.h  |  38 ++
   .../GenericIpmi/Pei/PeiIpmiBmcDef.h   | 156 ++
   .../GenericIpmi/Smm/SmmGenericIpmi.c  | 208 
   .../GenericIpmi/Smm/SmmGenericIpmi.inf|  53 ++
   .../IpmiFeaturePkg/Include/IpmiFeature.dsc|   9 +-
   .../Include/Library/IpmiBaseLib.h |  50 ++
   .../Include/Library/IpmiCommandLib.h  |  19 +-
   .../Include/Ppi/IpmiTransportPpi.h|  68 +++
   .../Include/Protocol/Ipm

Re: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live Migration for AMD SEV

2021-03-05 Thread Ashish Kalra
On Wed, Mar 03, 2021 at 01:25:40PM -0500, Tobin Feldman-Fitzthum wrote:
> 
> > Hi Tobin,
> > 
> > On 03/02/21 21:48, Tobin Feldman-Fitzthum wrote:
> > > This is a demonstration of fast migration for encrypted virtual machines
> > > using a Migration Handler that lives in OVMF. This demo uses AMD SEV,
> > > but the ideas may generalize to other confidential computing platforms.
> > > With AMD SEV, guest memory is encrypted and the hypervisor cannot access
> > > or move it. This makes migration tricky. In this demo, we show how the
> > > HV can ask a Migration Handler (MH) in the firmware for an encrypted
> > > page. The MH encrypts the page with a transport key prior to releasing
> > > it to the HV. The target machine also runs an MH that decrypts the page
> > > once it is passed in by the target HV. These patches are not ready for
> > > production, but the are a full end-to-end solution that facilitates a
> > > fast live migration between two SEV VMs.
> > > 
> > > Corresponding patches for QEMU have been posted my colleague Dov Murik
> > > on qemu-devel. Our approach needs little kernel support, requiring only
> > > one hypercall that the guest can use to mark a page as encrypted or
> > > shared. This series includes updated patches from Ashish Kalra and
> > > Brijesh Singh that allow OVMF to use this hypercall.
> > > 
> > > The MH runs continuously in the guest, waiting for communication from
> > > the HV. The HV starts an additional vCPU for the MH but does not expose
> > > it to the guest OS via ACPI. We use the MpService to start the MH. The
> > > MpService is only available at runtime and processes that are started by
> > > it are usually cleaned up on ExitBootServices. Since we need the MH to
> > > run continuously, we had to make some modifications. Ideally a feature
> > > could be added to the MpService to allow for the starting of
> > > long-running processes. Besides migration, this could support other
> > > background processes that need to operate within the encryption
> > > boundary. For now, we have included a handful of patches that modify the
> > > MpService to allow the MH to keep running after ExitBootServices. These
> > > are temporary.
> > I plan to do a lightweight review for this series. (My understanding is
> > that it's an RFC and not actually being proposed for merging.)
> > 
> > Regarding the MH's availability at runtime -- does that necessarily
> > require the isolation of an AP? Because in the current approach,
> > allowing the MP Services to survive into OS runtime (in some form or
> > another) seems critical, and I don't think it's going to fly.
> > 
> > I agree that the UefiCpuPkg patches have been well separated from the
> > rest of the series, but I'm somewhat doubtful the "firmware-initiated
> > background process" idea will be accepted. Have you investigated
> > exposing a new "runtime service" (a function pointer) via the UEFI
> > Configuration table, and calling that (perhaps periodically?) from the
> > guest kernel? It would be a form of polling I guess. Or maybe, poll the
> > mailbox directly in the kernel, and call the new firmware runtime
> > service when there's an actual command to process.
> Continuous runtime availability for the MH is almost certainly the most
> controversial part of this proposal, which is why I put it in the cover
> letter and why it's good to discuss.
> > (You do spell out "little kernel support", and I'm not sure if that's a
> > technical benefit, or a political / community benefit.)
> 
> As you allude to, minimal kernel support is really one of the main things
> that shapes our approach. This is partly a political and practical benefit,
> but there are also technical benefits. Having the MH in firmware likely
> leads to higher availability. It can be accessed when the OS is unreachable,
> perhaps during boot or when the OS is hung. There are also potential
> portability advantages although we do currently require support for one
> hypercall. The cost of implementing this hypercall is low.
> 
> Generally speaking, our task is to find a home for functionality that was
> traditionally provided by the hypervisor, but that needs to be inside the
> trust domain, but that isn't really part of a guest. A meta-goal of this
> project is to figure out the best way to do this.
> 
> > 
> > I'm quite uncomfortable with an attempt to hide a CPU from the OS via
> > ACPI. The OS has other ways to learn (for example, a boot loader could
> > use the MP services itself, stash the information, and hand it to the OS
> > kernel -- this would minimally allow for detecting an inconsistency in
> > the OS). What about "all-but-self" IPIs too -- the kernel might think
> > all the processors it's poking like that were under its control.
> 
> This might be the second most controversial piece. Here's a question: if we
> could successfully hide the MH vCPU from the OS, would it still make you
> uncomfortable? In other words, is the worry that there might be some
> incons

Re: [edk2-devel] [PATCH V5] ShellPkg: add more items for smbiosview -t 3 .

2021-03-05 Thread Mars CC Lin
Thank you for help and advice. I will review the process again.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72487): https://edk2.groups.io/g/devel/message/72487
Mute This Topic: https://groups.io/mt/81047033/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live Migration for AMD SEV

2021-03-05 Thread Paolo Bonzini

On 04/03/21 21:45, Laszlo Ersek wrote:

On 03/04/21 10:21, Paolo Bonzini wrote:

Hi Tobin,

as mentioned in the reply to the QEMU patches posted by Tobin, I
think the firmware helper approach is very good, but there are some
disadvantages in the idea of auxiliary vCPUs. These are especially
true in the VMM, where it's much nicer to have a separate VM that
goes through a specialized run loop; however, even in the firmware
level there are some complications (as you pointed out) in letting
MpService workers run after ExitBootServices.

My idea would be that the firmware would start the VM as usual using
the same launch data; then, the firmware would detect it was running
as a migration helper VM during the SEC or PEI phases (for example
via the GHCB or some other unencrypted communication area), and
divert execution to the migration helper instead of proceeding to the
next boot phase. This would be somewhat similar in spirit to how edk2
performs S3 resume, if my memory serves correctly.


Very cool. You'd basically warm-reboot the virtual machine into a new
boot mode (cf. BOOT_WITH_FULL_CONFIGURATION vs. BOOT_ON_S3_RESUME in
OvmfPkg/PlatformPei).

To me that's much more attractive than a "background job".

The S3 parallel is great. What I'm missing is:

- Is it possible to warm-reboot an SEV VM? (I vaguely recall that it's
not possible for SEV-ES at least.) Because, that's how we'd transfer
control to the early parts of the firmware again, IIUC your idea, while
preserving the memory contents.


It's not exactly a warm reboot.  It's two VMs booted at the same time, 
with exactly the same contents as far as encrypted RAM goes, but 
different unencrypted RAM.  The difference makes one VM boot regularly 
and the other end up in the migration helper.  The migration helper can 
be entirely contained in PEI, or it can even be its own OS, stored as a 
flat binary in the firmware.  Whatever is easier.


The divergence would happen much earlier than S3 though.  It would have 
to happen before the APs are brought up, for example, and essentially 
before the first fw_cfg access if (as is likely) the migration helper VM 
does not have fw_cfg at all.  That's why I brought up the possibility of 
diverging as soon as SEC.



- Who would initiate this process? S3 suspend is guest-initiated. (Not
that we couldn't use the guest agent, if needed.)

(In case the idea is really about a separate VM, and not about rebooting
the already running VM, then I don't understand -- how would a separate
VM access the guest RAM that needs to be migrated?)


Answering the other message:


(For some unsolicited personal information, now I feel less bad about
this idea never occurring to me -- I never knew about the KVM patch set
that would enable encryption context sharing. (TBH I thought that was
prevented, by design, in the SEV hardware...))


As far as the SEV hardware is concerned, a "VM" is defined by the ASID.

The VM would be separate at the KVM level, but it would share the ASID 
(and thus the guest RAM) with the primary VM.  So as far as the SEV 
hardware and the processor are concerned, the separate VM would be just 
one more VMCB that runs with that ASID.  Only KVM knows that they are 
backed by different file descriptors etc.


In fact, another advantage is that it would be much easier to scale the 
migration helper to multiple vCPUs.  This is probably also a case for 
diverging much earlier than PEI, because a multi-processor migration 
helper running in PEI or DXE would require ACPI tables and a lot of 
infrastructure that is probably undesirable.


Paolo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72486): https://edk2.groups.io/g/devel/message/72486
Mute This Topic: https://groups.io/mt/81036365/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] EDK II Stable Tag release edk2-stable202102 completed

2021-03-05 Thread Laszlo Ersek
Hi Liming,

On 03/05/21 09:29, gaoliming wrote:
> Hi, all
> 
>  
> 
> The tag edk2-stable202102 has been created.
> https://github.com/tianocore/edk2/releases/tag/edk2-stable202102
> 
>   git clone -b edk2-stable202102 https://github.com/tianocore/edk2.git
> 
>  
> 
> The tag edk2-stable202102 has been added into the main EDK II Wiki page.
> 
>   https://github.com/tianocore/tianocore.github.io/wiki/EDK-II
> 
>  
> 
> The quiet period has now ended. Thank you for your cooperation and
> patience. Normal commits can now be resumed.

thanks for having managed this release!

> 
>  
> 
> Next edk2 stable tag (edk2-stable202105) planning has been added into
> wiki page.
> 
> https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning.
> 
>  
> 
> If you have ideas for features in the next stable tag, please enter a
> Bugzilla for evaluation. Please let me know if there are existing open
> Bugzilla entries that should be targeted at this next stable tag.

Yes, could you please re-add
?

Thanks!
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72485): https://edk2.groups.io/g/devel/message/72485
Mute This Topic: https://groups.io/mt/81098568/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v4] MdeModulePkg/UfsPassThruDxe: Improve Device initialization polling Loop

2021-03-05 Thread Wu, Hao A
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Wu, Hao
> A
> Sent: Tuesday, March 2, 2021 2:04 PM
> To: Bandaru, Purna Chandra Rao ;
> devel@edk2.groups.io
> Cc: Albecki, Mateusz ; Ni, Ray
> 
> Subject: Re: [edk2-devel] [PATCH v4] MdeModulePkg/UfsPassThruDxe:
> Improve Device initialization polling Loop
> 
> > -Original Message-
> > From: Bandaru, Purna Chandra Rao
> 
> > Sent: Tuesday, March 2, 2021 1:18 PM
> > To: devel@edk2.groups.io
> > Cc: Bandaru, Purna Chandra Rao ;
> > Albecki, Mateusz ; Ni, Ray
> > ; Wu, Hao A 
> > Subject: [PATCH v4] MdeModulePkg/UfsPassThruDxe: Improve Device
> > initialization polling Loop
> >
> > https://bugzilla.tianocore.org/show_bug.cgi?id=3217
> >
> > Current Ufs Pass thru driver polls for 5us and return success even
> > when the timeout occurs.
> > There are cards that can take upto 600ms for Init and hence increased
> > the time out for fDeviceInit polling loop.
> >
> > Change-Id: I7b38e6f6aa46d93f07621caf29ad7c43f57df021
> 
> 
> Reviewed-by: Hao A Wu 
> 
> I will remove the above 'Change-Id' information when pushing the patch.
> I will hold this patch until the edk2-stable202102 is announced.


Pushed via commit:
https://github.com/tianocore/edk2/commit/c5740f360636479fb91681093b1dee1cc366075c

Best Regards,
Hao Wu


> 
> Best Regards,
> Hao Wu
> 
> 
> > Signed-off-by: Bandaru 
> > Cc: Mateusz Albecki 
> > Cc: Ray Ni 
> > Cc: Hao A Wu 
> > ---
> >  MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c | 17
> > +
> > MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h |  3 ++-
> >  2 files changed, 15 insertions(+), 5 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> > b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> > index 9768c2e6fb..92ff958f16 100644
> > --- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> > +++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> > @@ -1,6 +1,6 @@
> >  /** @file
> >
> > -  Copyright (c) 2014 - 2019, Intel Corporation. All rights
> > reserved.
> > +  Copyright (c) 2014 - 2021, Intel Corporation. All rights
> > + reserved.
> >Copyright (c) Microsoft Corporation.
> >SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > @@ -749,7 +749,7 @@ UfsFinishDeviceInitialization (  {
> >EFI_STATUS  Status;
> >UINT8  DeviceInitStatus;
> > -  UINT8  Timeout;
> > +  UINT32 Timeout;
> >
> >DeviceInitStatus = 0xFF;
> >
> > @@ -761,7 +761,10 @@ UfsFinishDeviceInitialization (
> >  return Status;
> >}
> >
> > -  Timeout = 5;
> > +  //
> > +  // There are cards that can take upto 600ms to clear fDeviceInit flag.
> > +  //
> > +  Timeout = UFS_INIT_COMPLETION_TIMEOUT;
> >do {
> >  Status = UfsReadFlag (Private, UfsFlagDevInit, &DeviceInitStatus);
> >  if (EFI_ERROR (Status)) {
> > @@ -771,7 +774,13 @@ UfsFinishDeviceInitialization (
> >  Timeout--;
> >} while (DeviceInitStatus != 0 && Timeout != 0);
> >
> > -  return EFI_SUCCESS;
> > +  if (Timeout == 0) {
> > +DEBUG ((DEBUG_ERROR, "UfsFinishDeviceInitialization
> > DeviceInitStatus=%x EFI_TIMEOUT \n", DeviceInitStatus));
> > +return EFI_TIMEOUT;
> > +  } else {
> > +DEBUG ((DEBUG_INFO, "UfsFinishDeviceInitialization Timeout
> > + left=%x
> > EFI_SUCCESS \n", Timeout));
> > +return EFI_SUCCESS;
> > +  }
> >  }
> >
> >  /**
> > diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
> > b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
> > index ef33250c89..79b86f7e6b 100644
> > --- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
> > +++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
> > @@ -1,6 +1,6 @@
> >  /** @file
> >
> > -  Copyright (c) 2014 - 2019, Intel Corporation. All rights
> > reserved.
> > +  Copyright (c) 2014 - 2021, Intel Corporation. All rights
> > + reserved.
> >Copyright (c) Microsoft Corporation.
> >SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > @@ -40,6 +40,7 @@
> >  //
> >  #define UFS_MAX_LUNS12
> >  #define UFS_WLUN_PREFIX 0xC1
> > +#define UFS_INIT_COMPLETION_TIMEOUT 60
> >
> >  typedef struct {
> >UINT8Lun[UFS_MAX_LUNS];
> > --
> > 2.16.2.windows.1
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72484): https://edk2.groups.io/g/devel/message/72484
Mute This Topic: https://groups.io/mt/81019242/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] EDK II Stable Tag release edk2-stable202102 completed

2021-03-05 Thread gaoliming
Hi, all

 

The tag edk2-stable202102 has been created.
https://github.com/tianocore/edk2/releases/tag/edk2-stable202102

  git clone -b edk2-stable202102 https://github.com/tianocore/edk2.git

 

The tag edk2-stable202102 has been added into the main EDK II Wiki page.

  https://github.com/tianocore/tianocore.github.io/wiki/EDK-II

 

The quiet period has now ended. Thank you for your cooperation and patience.
Normal commits can now be resumed.

 

Next edk2 stable tag (edk2-stable202105) planning has been added into wiki
page.

https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Plannin
g.

 

If you have ideas for features in the next stable tag, please enter a
Bugzilla for evaluation. Please let me know if there are existing open
Bugzilla entries that should be targeted at this next stable tag.

 

Thanks

Liming



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72483): https://edk2.groups.io/g/devel/message/72483
Mute This Topic: https://groups.io/mt/81098568/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-