Re: [edk2-devel] [PATCH v3 2/7] OvmfPkg: Add the way of HOBs in QemuFwCfgLibMmio

2024-04-25 Thread Chao Li

Hi Ard,


Thanks,
Chao
On 2024/4/26 09:20, Chao Li wrote:


Hi Ard,

On 2024/4/25 21:02, Ard Biesheuvel wrote:

On Thu, 25 Apr 2024 at 14:13, Chao Li  wrote:

Added the HOB methods to load and store the QEMU firmware configure
address, data address and DMA address, which are not enabled during the
DXE stage.

Build-tested only (with "ArmVirtQemu.dsc and RiscVVirtQemu.dsc").

BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=4755

Cc: Ard Biesheuvel
Cc: Jiewen Yao
Cc: Gerd Hoffmann
Cc: Leif Lindholm
Cc: Sami Mujawar
Cc: Sunil V L
Cc: Andrei Warkentin
Signed-off-by: Chao Li
---
  .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.c   | 81 +++--
  .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf |  1 +
  .../QemuFwCfgLib/QemuFwCfgLibMmioInternal.h   | 74 +++
  .../Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c   | 91 ---
  4 files changed, 226 insertions(+), 21 deletions(-)

diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c 
b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c
index dc949c8e26..b5dbc5e4b5 100644
--- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c
@@ -8,11 +8,16 @@
SPDX-License-Identifier: BSD-2-Clause-Patent
  **/

+#include 
  #include 

+#include 
+#include 
+
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -21,6 +26,62 @@

  #include "QemuFwCfgLibMmioInternal.h"

+EFI_GUID  mFwCfgResourceGuid = FW_CONFIG_RESOURCE_HOB_GUID;
+
+/**
+  Build firmware configure resource address HOB.
+
+  @param[in]   FwCfgResource  A pointer to firmware configure resource.
+
+  @retval  NULL
+**/
+VOID
+QemuBuildFwCfgResourceHob (
+  IN QEMU_FW_CFG_RESOURCE  *FwCfgResource
+  )
+{
+  UINT64  Data64;
+
+  Data64 = (UINT64)(UINTN)FwCfgResource;
+
+  BuildGuidDataHob (
+,
+(VOID *),

This looks wrong: why are you taking the address of the stack variable
rather than the address of the resource descriptor?


It only saves the pointer of FwCfgResource, and the memory space has 
been created in the PEI constructor.  Do you mean saving all contents 
of FwCfgResource in the HOB?


The following line is indeed wrong. if only save the pointer, the size 
should be "sizeof (UINT64)".



I will save the real HOB data in the next version.



+sizeof (QEMU_FW_CFG_RESOURCE)
+);
+}
+
+/**
+  Get firmware configure resource in HOB.
+
+  @param VOID
+
+  @retval  FwCfgResourceThe firmware configure resouce in HOB.

resource

All right.

+   NULL The firmware configure resouce not found.
+**/
+QEMU_FW_CFG_RESOURCE *
+QemuGetFwCfgResourceHob (
+  VOID
+  )
+{
+  EFI_HOB_GUID_TYPE *GuidHob;
+  VOID  *DataInHob;
+  QEMU_FW_CFG_RESOURCE  *FwCfgResource;
+
+  GuidHob   = NULL;
+  DataInHob = NULL;
+
+  GuidHob = GetFirstGuidHob ();

Please define this GUID in the package .DEC file and add it to the
[Guids] section in the .INF so that you can refer to its name
directly.

OK.

+  if (GuidHob == NULL) {
+return NULL;
+  }
+
+  DataInHob = GET_GUID_HOB_DATA (GuidHob);
+  FwCfgResource = (QEMU_FW_CFG_RESOURCE *)(*(UINTN *)DataInHob);
+
+  return FwCfgResource;
+}
+
  /**
Returns a boolean indicating if the firmware configuration interface
is available or not.
@@ -37,7 +98,7 @@ QemuFwCfgIsAvailable (
VOID
)
  {
-  return (BOOLEAN)(mFwCfgSelectorAddress != 0 && mFwCfgDataAddress != 0);
+  return (BOOLEAN)(QemuGetFwCfgSelectorAddress () != 0 && 
QemuGetFwCfgDataAddress () != 0);
  }

  /**
@@ -56,7 +117,7 @@ QemuFwCfgSelectItem (
)
  {
if (QemuFwCfgIsAvailable ()) {
-MmioWrite16 (mFwCfgSelectorAddress, SwapBytes16 ((UINT16)QemuFwCfgItem));
+MmioWrite16 (QemuGetFwCfgSelectorAddress (), SwapBytes16 
((UINT16)QemuFwCfgItem));
}
  }

@@ -86,30 +147,30 @@ MmioReadBytes (

   #if defined (MDE_CPU_AARCH64) || defined (MDE_CPU_RISCV64) || defined 
(MDE_CPU_LOONGARCH64)
while (Ptr < End) {
-*(UINT64 *)Ptr = MmioRead64 (mFwCfgDataAddress);
+*(UINT64 *)Ptr = MmioRead64 (QemuGetFwCfgDataAddress ());
  Ptr   += 8;
}

if (Left & 4) {
-*(UINT32 *)Ptr = MmioRead32 (mFwCfgDataAddress);
+*(UINT32 *)Ptr = MmioRead32 (QemuGetFwCfgDataAddress ());
  Ptr   += 4;
}

   #else
while (Ptr < End) {
-*(UINT32 *)Ptr = MmioRead32 (mFwCfgDataAddress);
+*(UINT32 *)Ptr = MmioRead32 (QemuGetFwCfgDataAddress ());
  Ptr   += 4;
}

   #endif

if (Left & 2) {
-*(UINT16 *)Ptr = MmioRead16 (mFwCfgDataAddress);
+*(UINT16 *)Ptr = MmioRead16 (QemuGetFwCfgDataAddress ());
  Ptr   += 2;
}

if (Left & 1) {
-*Ptr = MmioRead8 (mFwCfgDataAddress);
+*Ptr = MmioRead8 (QemuGetFwCfgDataAddress ());
}
  }

@@ -162,9 +223,9 @@ DmaTransferBytes (
// This will fire off the transfer.
//
   #if defined (MDE_CPU_AARCH64) || defined (MDE_CPU_RISCV64) || defined 
(MDE_CPU_LOONGARCH64)
-  MmioWrite64 (mFwCfgDmaAddress, SwapBytes64 ((UINT64)));

Re: [edk2-devel] [PATCH v4 00/10] Add DeviceSecurity feature based on PFP 1.06 spec

2024-04-25 Thread Wenxing Hou
Hi EDKII stewards,

Could you please review the libspdm license?

The libspdm(https://github.com/DMTF/libspdm) is a implementation that follows 
the DMTF SPDM(https://www.dmtf.org/standards/spdm) spec.

And the libspdm library is under DMTF repo.
The license is: https://github.com/DMTF/libspdm/blob/main/LICENSE.md



Thanks,
Wenxing


-Original Message-
From: Yao, Jiewen  
Sent: Sunday, April 21, 2024 10:31 AM
To: Hou, Wenxing ; devel@edk2.groups.io; Andrew Fish 
; Leif Lindholm ; Kinney, Michael D 
; Liming Gao ; Sean 
Brogan ; Joey Vagedes ; Liu, 
Zhiguang ; Kumar, Rahul R 
Subject: RE: [edk2-devel] [PATCH v4 00/10] Add DeviceSecurity feature based on 
PFP 1.06 spec

All series: Reviewed-by: Jiewen Yao 

Dear Steward member
Do you have any concern on adding libspdm (https://github.com/DMTF/libspdm) as 
one more submodule?

Thank you
Yao, Jiewen

> -Original Message-
> From: Hou, Wenxing 
> Sent: Thursday, April 18, 2024 6:16 PM
> To: devel@edk2.groups.io; Andrew Fish ; Leif Lindholm 
> ; Kinney, Michael D 
> ; Liming Gao ; 
> Sean Brogan ; Joey Vagedes 
> ; Liu, Zhiguang ; 
> Kumar, Rahul R ; Yao, Jiewen 
> 
> Subject: RE: [edk2-devel] [PATCH v4 00/10] Add DeviceSecurity feature 
> based on PFP 1.06 spec
> 
> Dear EDKII reviewers:
> 
> Thank you for your previous review of this patch set.
> Currently, five patches have been reviewed by.
> 
> But there are five patches need review.
>   Patch1:  MdePkg: Add SPDM1.2 support.
>   Patch2:  MdePkg: Add TCG PFP 1.06 support.
>   Patch4:  MdeModulePkg/Variable: Add TCG SPDM device measurement 
> update
>   Patch8:  .gitmodule: Add libspdm submodule for EDKII
>   Patch10: ReadMe.rst: Add libspdm submodule license
> 
> Could you please review the PATCH v4?
> 
> PS: Jiewen has reviewed all the PATCH. And I have fixed his feedback in PATCH 
> v4.
> Jiewen has no questions about all the patches anymore.
> 
> Thanks,
> Wenxing
> 
> 
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Wenxing 
> Hou
> Sent: Thursday, April 18, 2024 5:28 PM
> To: devel@edk2.groups.io
> Cc: Andrew Fish ; Leif Lindholm 
> ; Kinney, Michael D 
> ; Liming Gao ; 
> Sean Brogan ; Joey Vagedes 
> ; Liu, Zhiguang ; 
> Kumar, Rahul R ; Yao, Jiewen 
> 
> Subject: [edk2-devel] [PATCH v4 00/10] Add DeviceSecurity feature 
> based on PFP
> 1.06 spec
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2479
> 
> In PFP spec 1.06, platform firmware records the device certificate and 
> device measurement for each SPDM responder.
> This PATCH set implement the DeviceSecurityLib to support spdm device 
> Authentication and Measurement.
> 
> Libspdm as submodule is to support DeviceSecurity feature:
> https://github.com/DMTF/libspdm
> 
> TCG PFP spec 1.06:
> https://trustedcomputinggroup.org/resource/pc-client-specific-platform
> -
> firmware-profile-specification/
> 
> The POC branch:
> https://github.com/tianocore/edk2-staging/tree/DeviceSecurity
> 
> And the PATCH set has passed the EDKII CI:
> https://github.com/tianocore/edk2/pull/5508
> 
> v2 changes:
>  - Fix typo: PcdEnableSpdmDeviceAuthenticaion -> 
> PcdEnableSpdmDeviceAuthentication
> v3 changes:
>  - Add new patch 10: Update ReadMe.rst for libspdm submodule license
> v4 changes:
>  - Update submodule libspdm to latest tag
> 
> PATCH 3: Reviewed-by: Liming Gao  PATCH 5:
> Reviewed-by: Jiewen Yao  PATCH 6: Reviewed-by:
> Jiewen Yao  PATCH 7: Reviewed-by: Joey Vagedes 
>  PATCH 9: Reviewed-by: Jiewen Yao 
> 
> 
> Cc: Andrew Fish 
> Cc: Leif Lindholm 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Sean Brogan 
> Cc: Joey Vagedes 
> Cc: Zhiguang Liu 
> Cc: Rahul Kumar 
> Cc: Jiewen Yao 
> Signed-off-by: Wenxing Hou 
> 
> Wenxing Hou (10):
>   MdePkg: Add SPDM1.2 support.
>   MdePkg: Add TCG PFP 1.06 support.
>   MdePkg: Add devAuthBoot GlobalVariable
>   MdeModulePkg/Variable: Add TCG SPDM device measurement update
>   SecurityPkg: Add TCG PFP 1.06 support.
>   SecurityPkg: add DeviceSecurity support
>   .pytool/CISettings.py: add libspdm submodule.
>   .gitmodule: Add libspdm submodule for EDKII
>   SecurityPkg: Add libspdm submodule
>   ReadMe.rst: Add libspdm submodule license
> 
>  .gitmodules   |3 +
>  .pytool/CISettings.py |2 +
>  MdeModulePkg/MdeModulePkg.dec |5 +
>  .../Variable/RuntimeDxe/Measurement.c |   38 +-
>  .../RuntimeDxe/VariableRuntimeDxe.inf |3 +
>  .../RuntimeDxe/VariableSmmRuntimeDxe.inf  |3 +
>  MdePkg/Include/Guid/GlobalVariable.h  |8 +-
>  MdePkg/Include/Guid/ImageAuthentication.h |5 +-
>  MdePkg/Include/IndustryStandard/Spdm.h| 1112 -
>  .../IndustryStandard/UefiTcgPlatform.h|  186 ++-
>  ReadMe.rst|1 +
>  .../OsStub/CryptlibWrapper/CryptlibWrapper.c  |  970 ++
>  .../CryptlibWrapper/CryptlibWrapper.inf   |   38 +
>  

Re: [edk2-devel] [PATCH v3 2/7] OvmfPkg: Add the way of HOBs in QemuFwCfgLibMmio

2024-04-25 Thread Chao Li

Hi Ard,


Thanks,
Chao
On 2024/4/25 21:02, Ard Biesheuvel wrote:

On Thu, 25 Apr 2024 at 14:13, Chao Li  wrote:

Added the HOB methods to load and store the QEMU firmware configure
address, data address and DMA address, which are not enabled during the
DXE stage.

Build-tested only (with "ArmVirtQemu.dsc and RiscVVirtQemu.dsc").

BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=4755

Cc: Ard Biesheuvel
Cc: Jiewen Yao
Cc: Gerd Hoffmann
Cc: Leif Lindholm
Cc: Sami Mujawar
Cc: Sunil V L
Cc: Andrei Warkentin
Signed-off-by: Chao Li
---
  .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.c   | 81 +++--
  .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf |  1 +
  .../QemuFwCfgLib/QemuFwCfgLibMmioInternal.h   | 74 +++
  .../Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c   | 91 ---
  4 files changed, 226 insertions(+), 21 deletions(-)

diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c 
b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c
index dc949c8e26..b5dbc5e4b5 100644
--- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c
@@ -8,11 +8,16 @@
SPDX-License-Identifier: BSD-2-Clause-Patent
  **/

+#include 
  #include 

+#include 
+#include 
+
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -21,6 +26,62 @@

  #include "QemuFwCfgLibMmioInternal.h"

+EFI_GUID  mFwCfgResourceGuid = FW_CONFIG_RESOURCE_HOB_GUID;
+
+/**
+  Build firmware configure resource address HOB.
+
+  @param[in]   FwCfgResource  A pointer to firmware configure resource.
+
+  @retval  NULL
+**/
+VOID
+QemuBuildFwCfgResourceHob (
+  IN QEMU_FW_CFG_RESOURCE  *FwCfgResource
+  )
+{
+  UINT64  Data64;
+
+  Data64 = (UINT64)(UINTN)FwCfgResource;
+
+  BuildGuidDataHob (
+,
+(VOID *),

This looks wrong: why are you taking the address of the stack variable
rather than the address of the resource descriptor?


It only saves the pointer of FwCfgResource, and the memory space has 
been created in the PEI constructor.  Do you mean saving all contents of 
FwCfgResource in the HOB?


The following line is indeed wrong. if only save the pointer, the size 
should be "sizeof (UINT64)".





+sizeof (QEMU_FW_CFG_RESOURCE)
+);
+}
+
+/**
+  Get firmware configure resource in HOB.
+
+  @param VOID
+
+  @retval  FwCfgResourceThe firmware configure resouce in HOB.

resource

All right.



+   NULL The firmware configure resouce not found.
+**/
+QEMU_FW_CFG_RESOURCE *
+QemuGetFwCfgResourceHob (
+  VOID
+  )
+{
+  EFI_HOB_GUID_TYPE *GuidHob;
+  VOID  *DataInHob;
+  QEMU_FW_CFG_RESOURCE  *FwCfgResource;
+
+  GuidHob   = NULL;
+  DataInHob = NULL;
+
+  GuidHob = GetFirstGuidHob ();

Please define this GUID in the package .DEC file and add it to the
[Guids] section in the .INF so that you can refer to its name
directly.

OK.



+  if (GuidHob == NULL) {
+return NULL;
+  }
+
+  DataInHob = GET_GUID_HOB_DATA (GuidHob);
+  FwCfgResource = (QEMU_FW_CFG_RESOURCE *)(*(UINTN *)DataInHob);
+
+  return FwCfgResource;
+}
+
  /**
Returns a boolean indicating if the firmware configuration interface
is available or not.
@@ -37,7 +98,7 @@ QemuFwCfgIsAvailable (
VOID
)
  {
-  return (BOOLEAN)(mFwCfgSelectorAddress != 0 && mFwCfgDataAddress != 0);
+  return (BOOLEAN)(QemuGetFwCfgSelectorAddress () != 0 && 
QemuGetFwCfgDataAddress () != 0);
  }

  /**
@@ -56,7 +117,7 @@ QemuFwCfgSelectItem (
)
  {
if (QemuFwCfgIsAvailable ()) {
-MmioWrite16 (mFwCfgSelectorAddress, SwapBytes16 ((UINT16)QemuFwCfgItem));
+MmioWrite16 (QemuGetFwCfgSelectorAddress (), SwapBytes16 
((UINT16)QemuFwCfgItem));
}
  }

@@ -86,30 +147,30 @@ MmioReadBytes (

   #if defined (MDE_CPU_AARCH64) || defined (MDE_CPU_RISCV64) || defined 
(MDE_CPU_LOONGARCH64)
while (Ptr < End) {
-*(UINT64 *)Ptr = MmioRead64 (mFwCfgDataAddress);
+*(UINT64 *)Ptr = MmioRead64 (QemuGetFwCfgDataAddress ());
  Ptr   += 8;
}

if (Left & 4) {
-*(UINT32 *)Ptr = MmioRead32 (mFwCfgDataAddress);
+*(UINT32 *)Ptr = MmioRead32 (QemuGetFwCfgDataAddress ());
  Ptr   += 4;
}

   #else
while (Ptr < End) {
-*(UINT32 *)Ptr = MmioRead32 (mFwCfgDataAddress);
+*(UINT32 *)Ptr = MmioRead32 (QemuGetFwCfgDataAddress ());
  Ptr   += 4;
}

   #endif

if (Left & 2) {
-*(UINT16 *)Ptr = MmioRead16 (mFwCfgDataAddress);
+*(UINT16 *)Ptr = MmioRead16 (QemuGetFwCfgDataAddress ());
  Ptr   += 2;
}

if (Left & 1) {
-*Ptr = MmioRead8 (mFwCfgDataAddress);
+*Ptr = MmioRead8 (QemuGetFwCfgDataAddress ());
}
  }

@@ -162,9 +223,9 @@ DmaTransferBytes (
// This will fire off the transfer.
//
   #if defined (MDE_CPU_AARCH64) || defined (MDE_CPU_RISCV64) || defined 
(MDE_CPU_LOONGARCH64)
-  MmioWrite64 (mFwCfgDmaAddress, SwapBytes64 ((UINT64)));
+  MmioWrite64 (QemuGetFwCfgDmaAddress (), SwapBytes64 ((UINT64)));
   #else
-  MmioWrite32 

Re: [edk2-devel] [PATCH v3 1/7] OvmfPkg: Separate QemuFwCfgLibMmio.c into two files

2024-04-25 Thread Chao Li

Hi Ard,


Thanks,
Chao
On 2024/4/25 20:58, Ard Biesheuvel wrote:

On Thu, 25 Apr 2024 at 14:13, Chao Li  wrote:

Separate QemuFwCfgLibMmio.c into two files named QemuFwCfgLibMmio.c and
QemuFwCfgLibMmioDxe.c, added a new header named
QemuFwCfgLibMmioInternal.h for MMIO version.

Build-tested only (with "ArmVirtQemu.dsc and RiscVVirtQemu.dsc").

BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=4755

Cc: Ard Biesheuvel
Cc: Jiewen Yao
Cc: Gerd Hoffmann
Cc: Leif Lindholm
Cc: Sami Mujawar
Cc: Sunil V L
Cc: Andrei Warkentin
Signed-off-by: Chao Li
---
  .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.c   | 194 +-
  .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf |   4 +-
  .../QemuFwCfgLib/QemuFwCfgLibMmioInternal.h   | 179 
  .../Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c   | 153 ++
  4 files changed, 340 insertions(+), 190 deletions(-)
  create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmioInternal.h
  create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c

diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c 
b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c
index 115a210759..dc949c8e26 100644
--- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c
@@ -1,10 +1,9 @@
  /** @file

-  Stateful and implicitly initialized fw_cfg library implementation.
-
Copyright (C) 2013 - 2014, Red Hat, Inc.
Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.
(C) Copyright 2021 Hewlett Packard Enterprise Development LP
+  Copyright (c) 2024 Loongson Technology Corporation Limited. All rights 
reserved.


Please only claim copyright for code that you wrote, not for code that
you just moved between files.
OK, I will remove Loongson copyright in this patch and add it in patch 2 
or 3.



SPDX-License-Identifier: BSD-2-Clause-Patent
  **/
@@ -20,63 +19,7 @@

  #include 

-STATIC UINTN  mFwCfgSelectorAddress;
-STATIC UINTN  mFwCfgDataAddress;
-STATIC UINTN  mFwCfgDmaAddress;
-
-/**
-  Reads firmware configuration bytes into a buffer
-
-  @param[in] SizeSize in bytes to read
-  @param[in] Buffer  Buffer to store data into  (OPTIONAL if Size is 0)
-
-**/
-typedef
-VOID(EFIAPI READ_BYTES_FUNCTION)(
-  IN UINTN Size,
-  IN VOID  *Buffer OPTIONAL
-  );
-
-/**
-  Writes bytes from a buffer to firmware configuration
-
-  @param[in] SizeSize in bytes to write
-  @param[in] Buffer  Buffer to transfer data from (OPTIONAL if Size is 0)
-
-**/
-typedef
-VOID(EFIAPI WRITE_BYTES_FUNCTION)(
-  IN UINTN Size,
-  IN VOID  *Buffer OPTIONAL
-  );
-
-/**
-  Skips bytes in firmware configuration
-
-  @param[in] Size  Size in bytes to skip
-
-**/
-typedef
-VOID(EFIAPI SKIP_BYTES_FUNCTION)(
-  IN UINTN Size
-  );
-
-//
-// Forward declaration of the two implementations we have.
-//
-STATIC READ_BYTES_FUNCTION   MmioReadBytes;
-STATIC WRITE_BYTES_FUNCTION  MmioWriteBytes;
-STATIC SKIP_BYTES_FUNCTION   MmioSkipBytes;
-STATIC READ_BYTES_FUNCTION   DmaReadBytes;
-STATIC WRITE_BYTES_FUNCTION  DmaWriteBytes;
-STATIC SKIP_BYTES_FUNCTION   DmaSkipBytes;
-
-//
-// These correspond to the implementation we detect at runtime.
-//
-STATIC READ_BYTES_FUNCTION   *InternalQemuFwCfgReadBytes  = MmioReadBytes;
-STATIC WRITE_BYTES_FUNCTION  *InternalQemuFwCfgWriteBytes = MmioWriteBytes;
-STATIC SKIP_BYTES_FUNCTION   *InternalQemuFwCfgSkipBytes  = MmioSkipBytes;
+#include "QemuFwCfgLibMmioInternal.h"

  /**
Returns a boolean indicating if the firmware configuration interface
@@ -97,126 +40,6 @@ QemuFwCfgIsAvailable (
return (BOOLEAN)(mFwCfgSelectorAddress != 0 && mFwCfgDataAddress != 0);
  }

-RETURN_STATUS
-EFIAPI
-QemuFwCfgInitialize (
-  VOID
-  )
-{
-  EFI_STATUS   Status;
-  FDT_CLIENT_PROTOCOL  *FdtClient;
-  CONST UINT64 *Reg;
-  UINT32   RegSize;
-  UINTNAddressCells, SizeCells;
-  UINT64   FwCfgSelectorAddress;
-  UINT64   FwCfgSelectorSize;
-  UINT64   FwCfgDataAddress;
-  UINT64   FwCfgDataSize;
-  UINT64   FwCfgDmaAddress;
-  UINT64   FwCfgDmaSize;
-
-  Status = gBS->LocateProtocol (
-  ,
-  NULL,
-  (VOID **)
-  );
-  ASSERT_EFI_ERROR (Status);
-
-  Status = FdtClient->FindCompatibleNodeReg (
-FdtClient,
-"qemu,fw-cfg-mmio",
-(CONST VOID **),
-,
-,
-
-);
-  if (EFI_ERROR (Status)) {
-DEBUG ((
-  DEBUG_WARN,
-  "%a: No 'qemu,fw-cfg-mmio' compatible DT node found (Status == %r)\n",
-  __func__,
-  Status
-  ));
-return EFI_SUCCESS;
-  }
-
-  ASSERT (AddressCells == 2);
-  ASSERT (SizeCells == 2);
-  ASSERT (RegSize == 2 * sizeof (UINT64));
-
-  FwCfgDataAddress = SwapBytes64 (Reg[0]);
-  FwCfgDataSize= 8;
-  

Re: [edk2-devel] 回复: [PATCH 0/7] General Updates based on UEFI 2.10 and PI 1.8 Specification

2024-04-25 Thread Sachin Ganesh via groups.io
Hi Felix/ Liming,

Thank you for your comments. Patch 6 has been updated to only focus on 
consolidating the revision macros.

Patch Link: 
https://edk2.groups.io/g/devel/message/118246?p=%2C%2C%2C20%2C0%2C0%2C0%3A%3Arecentpostdate%2Fsticky%2C%2Csachin%2C20%2C2%2C0%2C105721897
PR Link: https://github.com/tianocore/edk2/pull/5569

Thank you,
Sachin.

-Original Message-
From: Felix Polyudov 
Sent: Tuesday, April 23, 2024 10:58 PM
To: gaoliming ; Sachin Ganesh ; 
devel@edk2.groups.io
Cc: zhiguang@intel.com; michael.d.kin...@intel.com; 
ardb+tianoc...@kernel.org; kra...@redhat.com; jiewen@intel.com; 
erdemak...@google.com; min.m...@intel.com; thomas.lenda...@amd.com; Dhanaraj V 

Subject: RE: [EXTERNAL] 回复: [PATCH 0/7] General Updates based on UEFI 2.10 and 
PI 1.8 Specification

I think patch 6 can be updated to introduce unified PI specification versioning 
macros without incrementing the minor revision, by changing 
PI_SPECIFICATION_MINOR_REVISION back to 70.
This will ensure compliance with the versioning schema introduced in PI 1.7B 
without changing the PI support level.
(the macros were introduced by PIWG mantis 2101)

-Original Message-
From: gaoliming 
Sent: Tuesday, April 23, 2024 10:49 AM
To: Sachin Ganesh ; devel@edk2.groups.io
Cc: zhiguang@intel.com; michael.d.kin...@intel.com; 
ardb+tianoc...@kernel.org; kra...@redhat.com; jiewen@intel.com; 
erdemak...@google.com; min.m...@intel.com; thomas.lenda...@amd.com; Felix 
Polyudov ; Dhanaraj V 
Subject: [EXTERNAL] 回复: [PATCH 0/7] General Updates based on UEFI 2.10 and PI 
1.8 Specification


**CAUTION: The e-mail below is from an external source. Please exercise caution 
before opening attachments, clicking links, or following guidance.**

Except for Patch 6/7, others are good to me. Reviewed-by: Liming Gao 


I suggest to merge others first. The patch 6/7 to update PI version from 1.7 to 
1.8 can be discussed first.

Thanks
Liming
> -邮件原件-
> 发件人: Sachin Ganesh 
> 发送时间: 2024年4月20日 5:46
> 收件人: devel@edk2.groups.io
> 抄送: gaolim...@byosoft.com.cn; zhiguang@intel.com;
> michael.d.kin...@intel.com; ardb+tianoc...@kernel.org;
> kra...@redhat.com; jiewen@intel.com; erdemak...@google.com;
> min.m...@intel.com; thomas.lenda...@amd.com; Felix Polyudov
> ; Dhanaraj V ; Sachin Ganesh
> 
> 主题: [PATCH 0/7] General Updates based on UEFI 2.10 and PI 1.8
> Specification
>
> This series of patches are for general updates to MdePkg and
> MdeModulePkg based on UEFI 2.10 and PI 1.8 Specifications
>
> Sachin Ganesh (7):
>   MdePkg: Add definition for NVMe Over Fabric Device Path
>   MdePkg: Add new Resource Attributes defined in PI 1.8 Spec
>   MdePkg: Define Unaccepted Memory Type
>   MdeModulePkg: Use newly defined Unaccepted Memory Type
>   MdePkg: Update Delayed Dispatch PPI as per PI 1.8 Spec
>   MdePkg: Update to PI 1.8 Revision
>   OvmfPkg: Use newly defined Unaccepted Memory Type
>
>  MdeModulePkg/Core/Dxe/Gcd/Gcd.c  | 10 +++---
>  MdeModulePkg/Core/Dxe/Mem/Page.c | 38
> ++--
>  MdeModulePkg/Include/Pi/PrePiDxeCis.h| 25 -
>  MdeModulePkg/Include/Pi/PrePiHob.h   | 20 ---
>  MdePkg/Include/Pi/PiDxeCis.h | 19 +-
>  MdePkg/Include/Pi/PiHob.h| 14 +++-
>  MdePkg/Include/Pi/PiMmCis.h  |  6 ++--
>  MdePkg/Include/Pi/PiMultiPhase.h |  6 
>  MdePkg/Include/Pi/PiPeiCis.h |  6 ++--
>  MdePkg/Include/Pi/PiSmmCis.h |  2 +-
>  MdePkg/Include/Ppi/DelayedDispatch.h | 24 -
>  MdePkg/Include/Protocol/DevicePath.h | 22 
>  OvmfPkg/AmdSevDxe/AmdSevDxe.c|  4 +--
>  OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c |  8 ++---
>  OvmfPkg/Library/PeilessStartupLib/Hob.c  |  4 +--
>  OvmfPkg/Library/PlatformInitLib/IntelTdx.c   |  8 ++---
>  OvmfPkg/PlatformPei/AmdSev.c |  4 +--
>  17 files changed, 108 insertions(+), 112 deletions(-)  delete mode
> 100644 MdeModulePkg/Include/Pi/PrePiDxeCis.h
>  delete mode 100644 MdeModulePkg/Include/Pi/PrePiHob.h
>
> --
> 2.24.1.windows.2
> -The information contained in this message may be confidential and
> proprietary to American Megatrends (AMI). This communication is
> intended
to
> be read only by the individual or entity to whom it is addressed or by
their
> designee. If the reader of this message is not the intended recipient,
> you
are
> on notice that any distribution of this message, in any form, is
> strictly prohibited. Please promptly notify the sender by reply e-mail
> or by
telephone
> at 770-246-8600, and then delete or destroy all copies of the
transmission=


-The information contained in this message may be confidential and proprietary 
to American Megatrends (AMI). This communication is intended to be read only by 
the individual or entity to whom it is addressed or by their designee. If the 
reader of this message is not the intended 

[edk2-devel] Cannot access SerialPortPoll function

2024-04-25 Thread Ba Gia Bao Phan
Hello Everyone,

I'm working on a project with edk2, and these guys are trying to port edk2
to STM32MP25 platforms. I have a problem with the module SerialPortLib,
specifically the function *SerialPortPoll*. I made a test that adds a line
of debug in this function and adds a MmioWrite8 to a forbidden address  of
my platform but I don't see anything. This demonstrates that I cannot
access this function (this function is not loaded). Do you know why and how
to solve this?

Here is my function SerialPortPoll
BOOLEAN
EFIAPI
SerialPortPoll (
  VOID
  )
{
  DEBUG ((DEBUG_INFO, "In SerialPortPo\n"));
  MmioWrite8(0x8000, 0xff);
  return FALSE;
}

And here is my function SerialPortWrite that works well because I can see
the boot log message
UINTN
EFIAPI
SerialPortWrite (
  IN UINT8 *Buffer,
  IN UINTN NumberOfBytes
)
{
  UINT8 *CONST  Final = [NumberOfBytes];
  while (Buffer < Final) {
// Wait until UART able to accept another char
while (!(MmioRead32 (USART2_BASE + R_UART_ISR) & (1<<7))) {
}
MmioWrite8 (USART2_BASE + R_UART_TDR, *Buffer++);
  }
  DEBUG ((DEBUG_INFO, "end SerialPortWrite\n"));

  return NumberOfBytes;
}

Thanks
-- 
PHAN Ba Gia Bao
Etudiant en 5A STI - INSA Centre Val de Loire


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




Re: [edk2-devel] Synchronous Exception at ArmGicDxe

2024-04-25 Thread Ba Gia Bao Phan
It is near to the GicDistributorBase (0x4ac1) of STM32MP25

Le jeu. 25 avr. 2024 à 02:21, Andrew (EFI) Fish  a écrit :

> The fault address is 0x0004AC14. Is that in the address range of
> the GIC for this platform? What does that Physical address map to you on
> the STM32MP25?
>
> Thanks,
>
> Andrew Fish
>
> On Apr 21, 2024, at 10:07 PM, Ba Gia Bao Phan 
> wrote:
>
> Hi Everyone,
>
> I'm working on a project with edk2, and these guys are trying to port edk2
> to STM32MP25 platforms. I had no issue compiling and booting the image on
> my device. Then I've come across an issue at very early stages of booting.
> I was given a Synchronous Exception listed briefly below.
>
> Synchronous Exception at *0x00010A63501C*
> PC 0x00010A63501C (0x00010A633000+0x201C) [ 0] ArmGicDxe.dll
> PC 0x00010A6350CC (0x00010A633000+0x20CC) [ 0] ArmGicDxe.dll
> PC 0x00010A63657C (0x00010A633000+0x357C) [ 0] ArmGicDxe.dll
> PC 0x00010A85853C (0x00010A851000+0x753C) [ 1] DxeCore.dll
> PC 0x00010A8666DC (0x00010A851000+0x000156DC) [ 1] DxeCore.dll
> PC 0x00010A85BF14 (0x00010A851000+0xAF14) [ 1] DxeCore.dll
> PC 0x84006EBC
> PC 0x8400700C
>
> [ 0]
> /local/home/phanbagi/Documents/RPi3/Build/STM32MP25/DEBUG_GCC5/AARCH64/ArmPkg/Drivers/ArmGic/ArmGicDxe/DEBUG/ArmGicDxe.dll
> [ 1]
> /local/home/phanbagi/Documents/RPi3/Build/STM32MP25/DEBUG_GCC5/AARCH64/MdeModulePkg/Core/Dxe/DxeMain/DEBUG/DxeCore.dll
>
>   X0 0x0004AC14   X1 0x   X2 0x
> X3 0x4F46A76E3FDDA605
>   X4 0x083D1B53F41229AD   X5 0x003CF238   X6 0x0080
> X7 0x
>   X8 0x00010A850688   X9 0x0004  X10 0x000109E9F000
>  X11 0x00010A297FFF
>  X12 0x  X13 0x000E  X14 0x00FF
>  X15 0x0002
>  X16 0x00010AFFFAD0  X17 0x00395790  X18 0x
>  X19 0x0004AC14
>  X20 0x00010A638000  X21 0x00010A638000  X22 0x00010A638000
>  X23 0x00010A6380A8
>  X24 0x00010A876448  X25 0x00010A873026  X26 0x00010A877000
>  X27 0x800E
>  X28 0x0001   FP 0x00010AFFFA50   LR 0x00010A6350CC
>
>   V0 0xAFAFAFAFAFAFAFAF AFAFAFAFAFAFAFAF   V1 0xFF80FFD0
> 00010AFFFA30
>   V2 0x0007 0E001858   V3 0x1000
> 0100
>   V4 0x    V5 0x
> 
>   V6 0x    V7 0x
> 
>   V8 0x    V9 0x
> 
>  V10 0x   V11 0x
> 
>  V12 0x   V13 0x
> 
>  V14 0x   V15 0x
> 
>  V16 0x6145998C70863CB0 9004002B4AC68640  V17 0x646586E820A5D596
> AA30249EC11F1469
>  V18 0x5756252D20E5B421 336EEAEC2122B3AB  V19 0x6285C083AAECD063
> 020A72C37700322A
>  V20 0x40846C68BA2085FB 20966526601C44BB  V21 0x818208204CD0A89D
> 6D84E94813E14B24
>  V22 0x2362CBA9962AF073 E83ED49CD4A13F98  V23 0x0C1123CD73F5C5B0
> C3E9222891467015
>  V24 0x02B1194895858177 1B028BE1A860D73B  V25 0x542AB00936183D90
> 752F1D1CCC4D2345
>  V26 0x03D02013962356F3 0CCA6F83842045EA  V27 0x9DF9D2A28D44915E
> 2892460B223FD84C
>  V28 0x61A2CC120A809CD8 AC33034803D74C10  V29 0x7A1089709810B030
> 88B3298EEB68B450
>  V30 0xD20F81310E0896C0 20E2BA370E2AC6F9  V31 0x6C3BBEA030B2BB50
> 2C2A91AAF201EF50
>
>   SP 0x00010AFFFA50  ELR 0x00010A63501C  SPSR 0x43C5  FPSR
> 0x
>  ESR 0x9605  FAR 0x0004AC14
>
>  ESR : EC 0x25  IL 0x1  ISS 0x0005
>
> Data abort: Translation fault, first level
>
>
> By using objdump to determine where the wrong source code is, I found out
> that the program stopped around the code below.
>
> UINT32
> EFIAPI
> MmioRead32 (
>   IN  UINTN  Address
>   )
> {
>   UINT32   Value;
>   BOOLEAN  Flag;
>
>   ASSERT ((Address & 3) == 0);
>
>   Flag = FilterBeforeMmIoRead (FilterWidth32, Address, );
>   if (Flag) {
> MemoryFence ();
>
> if (IsTdxGuest ()) {
>   Value = TdMmioRead32 (Address);
> } else {
>   Value = *(volatile UINT32 *)Address;
> }
>
> MemoryFence ();
>   }
>
>   FilterAfterMmIoRead (FilterWidth32, Address, );
>
>   return Value;
> }
>
> Output of objdump:
> UINT32
> EFIAPI
> MmioRead32 (
>   IN  UINTN  Address
>   )
> {
> 1fec: a9be7bfd stp x29, x30, [sp, #-32]!
> 1ff0: 910003fd mov x29, sp
> 1ff4: f9000bf3 str x19, [sp, #16]
> 1ff8: aa0003f3 mov x19, x0
>   UINT32   Value;
>   BOOLEAN  Flag;
>
>   ASSERT ((Address & 3) == 0);
> 1ffc: f240041f tst x0, #0x3
> 2000: 54e0 b.eq 201c   // b.none
> 2004: b002 adrp x2, 3000 
> 2008: b000 adrp x0, 3000 
> 200c: 913ac442 add x2, x2, #0xeb1
> 2010: 913b1000 add x0, x0, #0xec4
> 2014: 

Re: [edk2-devel] [PATCH] OvmfPkg: Set PcdCpuMaxLogicalProcessorNumber in OvmfXen

2024-04-25 Thread Anthony PERARD via groups.io
On Thu, Apr 25, 2024 at 11:42:01AM +0100, Alejandro Vallejo wrote:
> Hi,
> 
> On 25/04/2024 08:31, Gerd Hoffmann wrote:
> > On Wed, Apr 24, 2024 at 02:36:32PM +0100, Alejandro Vallejo wrote:
> >> Bump the compile-time constant for maximum processor count from 64 to 128
> >> in order to allow that many vCPUs to be brought online on Xen guests with
> >> the default OVMF configuration.
> > 
> >> +  # UefiCpuPkg PCDs related to initial AP bringup and general AP 
> >> management.
> >> +  gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|128
> > 
> > Note that this is a dynamic PCD, so you can set it at runtime to the
> > number of vcpus present in the VM.  See MaxCpuCountInitialization() in
> > OvmfPkg/PlatformPei/Platform.c for example.
> > 
> > take care,
> >   Gerd
> > 
> 
> Thanks for the heads up. Do you mean setting it at runtime through
> fw_cfg? I saw PlatformMaxCpuCountInitialization() providing some
> customizability, but Xen's toolstack doesn't provide fw_cfg at the
> moment so it can't (as far as I've seen) use it.

We don't need fw_cfg to set a PCD at runtime.

It's a bit more complicated than setting it at build time, but we can
always ask Xen how many vcpu we have and set the PCD accordingly. This
is something that can happen in OvmfPkg/XenPlatformPei module.

But to be honest, I don't know if it's worth it, because I don't know the
downside of having a higher value for PcdCpuMaxLogicalProcessorNumber.

Cheers,

-- 
Anthony PERARD


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




Re: [edk2-devel] [PATCH] OvmfPkg: Set PcdCpuMaxLogicalProcessorNumber in OvmfXen

2024-04-25 Thread Gerd Hoffmann
  Hi,
 
> It's a bit more complicated than setting it at build time, but we can
> always ask Xen how many vcpu we have and set the PCD accordingly. This
> is something that can happen in OvmfPkg/XenPlatformPei module.

Exactly.

> But to be honest, I don't know if it's worth it, because I don't know the
> downside of having a higher value for PcdCpuMaxLogicalProcessorNumber.

Well, the downside is that you have to keep ovmf and xen in sync
manually.  Your call.  If you prefer to do it that way I have no
objections, just wanted point out the alternative approach.

take care,
  Gerd



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




Re: [edk2-devel] [PATCH v3 2/7] OvmfPkg: Add the way of HOBs in QemuFwCfgLibMmio

2024-04-25 Thread Ard Biesheuvel
On Thu, 25 Apr 2024 at 14:13, Chao Li  wrote:
>
> Added the HOB methods to load and store the QEMU firmware configure
> address, data address and DMA address, which are not enabled during the
> DXE stage.
>
> Build-tested only (with "ArmVirtQemu.dsc and RiscVVirtQemu.dsc").
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4755
>
> Cc: Ard Biesheuvel 
> Cc: Jiewen Yao 
> Cc: Gerd Hoffmann 
> Cc: Leif Lindholm 
> Cc: Sami Mujawar 
> Cc: Sunil V L 
> Cc: Andrei Warkentin 
> Signed-off-by: Chao Li 
> ---
>  .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.c   | 81 +++--
>  .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf |  1 +
>  .../QemuFwCfgLib/QemuFwCfgLibMmioInternal.h   | 74 +++
>  .../Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c   | 91 ---
>  4 files changed, 226 insertions(+), 21 deletions(-)
>
> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c 
> b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c
> index dc949c8e26..b5dbc5e4b5 100644
> --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c
> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c
> @@ -8,11 +8,16 @@
>SPDX-License-Identifier: BSD-2-Clause-Patent
>  **/
>
> +#include 
>  #include 
>
> +#include 
> +#include 
> +
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -21,6 +26,62 @@
>
>  #include "QemuFwCfgLibMmioInternal.h"
>
> +EFI_GUID  mFwCfgResourceGuid = FW_CONFIG_RESOURCE_HOB_GUID;
> +
> +/**
> +  Build firmware configure resource address HOB.
> +
> +  @param[in]   FwCfgResource  A pointer to firmware configure resource.
> +
> +  @retval  NULL
> +**/
> +VOID
> +QemuBuildFwCfgResourceHob (
> +  IN QEMU_FW_CFG_RESOURCE  *FwCfgResource
> +  )
> +{
> +  UINT64  Data64;
> +
> +  Data64 = (UINT64)(UINTN)FwCfgResource;
> +
> +  BuildGuidDataHob (
> +,
> +(VOID *),

This looks wrong: why are you taking the address of the stack variable
rather than the address of the resource descriptor?

> +sizeof (QEMU_FW_CFG_RESOURCE)
> +);
> +}
> +
> +/**
> +  Get firmware configure resource in HOB.
> +
> +  @param VOID
> +
> +  @retval  FwCfgResourceThe firmware configure resouce in HOB.

resource

> +   NULL The firmware configure resouce not found.
> +**/
> +QEMU_FW_CFG_RESOURCE *
> +QemuGetFwCfgResourceHob (
> +  VOID
> +  )
> +{
> +  EFI_HOB_GUID_TYPE *GuidHob;
> +  VOID  *DataInHob;
> +  QEMU_FW_CFG_RESOURCE  *FwCfgResource;
> +
> +  GuidHob   = NULL;
> +  DataInHob = NULL;
> +
> +  GuidHob = GetFirstGuidHob ();

Please define this GUID in the package .DEC file and add it to the
[Guids] section in the .INF so that you can refer to its name
directly.

> +  if (GuidHob == NULL) {
> +return NULL;
> +  }
> +
> +  DataInHob = GET_GUID_HOB_DATA (GuidHob);
> +  FwCfgResource = (QEMU_FW_CFG_RESOURCE *)(*(UINTN *)DataInHob);
> +
> +  return FwCfgResource;
> +}
> +
>  /**
>Returns a boolean indicating if the firmware configuration interface
>is available or not.
> @@ -37,7 +98,7 @@ QemuFwCfgIsAvailable (
>VOID
>)
>  {
> -  return (BOOLEAN)(mFwCfgSelectorAddress != 0 && mFwCfgDataAddress != 0);
> +  return (BOOLEAN)(QemuGetFwCfgSelectorAddress () != 0 && 
> QemuGetFwCfgDataAddress () != 0);
>  }
>
>  /**
> @@ -56,7 +117,7 @@ QemuFwCfgSelectItem (
>)
>  {
>if (QemuFwCfgIsAvailable ()) {
> -MmioWrite16 (mFwCfgSelectorAddress, SwapBytes16 ((UINT16)QemuFwCfgItem));
> +MmioWrite16 (QemuGetFwCfgSelectorAddress (), SwapBytes16 
> ((UINT16)QemuFwCfgItem));
>}
>  }
>
> @@ -86,30 +147,30 @@ MmioReadBytes (
>
>   #if defined (MDE_CPU_AARCH64) || defined (MDE_CPU_RISCV64) || defined 
> (MDE_CPU_LOONGARCH64)
>while (Ptr < End) {
> -*(UINT64 *)Ptr = MmioRead64 (mFwCfgDataAddress);
> +*(UINT64 *)Ptr = MmioRead64 (QemuGetFwCfgDataAddress ());
>  Ptr   += 8;
>}
>
>if (Left & 4) {
> -*(UINT32 *)Ptr = MmioRead32 (mFwCfgDataAddress);
> +*(UINT32 *)Ptr = MmioRead32 (QemuGetFwCfgDataAddress ());
>  Ptr   += 4;
>}
>
>   #else
>while (Ptr < End) {
> -*(UINT32 *)Ptr = MmioRead32 (mFwCfgDataAddress);
> +*(UINT32 *)Ptr = MmioRead32 (QemuGetFwCfgDataAddress ());
>  Ptr   += 4;
>}
>
>   #endif
>
>if (Left & 2) {
> -*(UINT16 *)Ptr = MmioRead16 (mFwCfgDataAddress);
> +*(UINT16 *)Ptr = MmioRead16 (QemuGetFwCfgDataAddress ());
>  Ptr   += 2;
>}
>
>if (Left & 1) {
> -*Ptr = MmioRead8 (mFwCfgDataAddress);
> +*Ptr = MmioRead8 (QemuGetFwCfgDataAddress ());
>}
>  }
>
> @@ -162,9 +223,9 @@ DmaTransferBytes (
>// This will fire off the transfer.
>//
>   #if defined (MDE_CPU_AARCH64) || defined (MDE_CPU_RISCV64) || defined 
> (MDE_CPU_LOONGARCH64)
> -  MmioWrite64 (mFwCfgDmaAddress, SwapBytes64 ((UINT64)));
> +  MmioWrite64 (QemuGetFwCfgDmaAddress (), SwapBytes64 ((UINT64)));
>   #else
> -  MmioWrite32 ((UINT32)(mFwCfgDmaAddress + 4), SwapBytes32 
> ((UINT32)));
> +  MmioWrite32 

Re: [edk2-devel] [PATCH v3 1/7] OvmfPkg: Separate QemuFwCfgLibMmio.c into two files

2024-04-25 Thread Ard Biesheuvel
On Thu, 25 Apr 2024 at 14:13, Chao Li  wrote:
>
> Separate QemuFwCfgLibMmio.c into two files named QemuFwCfgLibMmio.c and
> QemuFwCfgLibMmioDxe.c, added a new header named
> QemuFwCfgLibMmioInternal.h for MMIO version.
>
> Build-tested only (with "ArmVirtQemu.dsc and RiscVVirtQemu.dsc").
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4755
>
> Cc: Ard Biesheuvel 
> Cc: Jiewen Yao 
> Cc: Gerd Hoffmann 
> Cc: Leif Lindholm 
> Cc: Sami Mujawar 
> Cc: Sunil V L 
> Cc: Andrei Warkentin 
> Signed-off-by: Chao Li 
> ---
>  .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.c   | 194 +-
>  .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf |   4 +-
>  .../QemuFwCfgLib/QemuFwCfgLibMmioInternal.h   | 179 
>  .../Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c   | 153 ++
>  4 files changed, 340 insertions(+), 190 deletions(-)
>  create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmioInternal.h
>  create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c
>
> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c 
> b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c
> index 115a210759..dc949c8e26 100644
> --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c
> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c
> @@ -1,10 +1,9 @@
>  /** @file
>
> -  Stateful and implicitly initialized fw_cfg library implementation.
> -
>Copyright (C) 2013 - 2014, Red Hat, Inc.
>Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.
>(C) Copyright 2021 Hewlett Packard Enterprise Development LP
> +  Copyright (c) 2024 Loongson Technology Corporation Limited. All rights 
> reserved.
>

Please only claim copyright for code that you wrote, not for code that
you just moved between files.

>SPDX-License-Identifier: BSD-2-Clause-Patent
>  **/
> @@ -20,63 +19,7 @@
>
>  #include 
>
> -STATIC UINTN  mFwCfgSelectorAddress;
> -STATIC UINTN  mFwCfgDataAddress;
> -STATIC UINTN  mFwCfgDmaAddress;
> -
> -/**
> -  Reads firmware configuration bytes into a buffer
> -
> -  @param[in] SizeSize in bytes to read
> -  @param[in] Buffer  Buffer to store data into  (OPTIONAL if Size is 0)
> -
> -**/
> -typedef
> -VOID(EFIAPI READ_BYTES_FUNCTION)(
> -  IN UINTN Size,
> -  IN VOID  *Buffer OPTIONAL
> -  );
> -
> -/**
> -  Writes bytes from a buffer to firmware configuration
> -
> -  @param[in] SizeSize in bytes to write
> -  @param[in] Buffer  Buffer to transfer data from (OPTIONAL if Size is 0)
> -
> -**/
> -typedef
> -VOID(EFIAPI WRITE_BYTES_FUNCTION)(
> -  IN UINTN Size,
> -  IN VOID  *Buffer OPTIONAL
> -  );
> -
> -/**
> -  Skips bytes in firmware configuration
> -
> -  @param[in] Size  Size in bytes to skip
> -
> -**/
> -typedef
> -VOID(EFIAPI SKIP_BYTES_FUNCTION)(
> -  IN UINTN Size
> -  );
> -
> -//
> -// Forward declaration of the two implementations we have.
> -//
> -STATIC READ_BYTES_FUNCTION   MmioReadBytes;
> -STATIC WRITE_BYTES_FUNCTION  MmioWriteBytes;
> -STATIC SKIP_BYTES_FUNCTION   MmioSkipBytes;
> -STATIC READ_BYTES_FUNCTION   DmaReadBytes;
> -STATIC WRITE_BYTES_FUNCTION  DmaWriteBytes;
> -STATIC SKIP_BYTES_FUNCTION   DmaSkipBytes;
> -
> -//
> -// These correspond to the implementation we detect at runtime.
> -//
> -STATIC READ_BYTES_FUNCTION   *InternalQemuFwCfgReadBytes  = MmioReadBytes;
> -STATIC WRITE_BYTES_FUNCTION  *InternalQemuFwCfgWriteBytes = MmioWriteBytes;
> -STATIC SKIP_BYTES_FUNCTION   *InternalQemuFwCfgSkipBytes  = MmioSkipBytes;
> +#include "QemuFwCfgLibMmioInternal.h"
>
>  /**
>Returns a boolean indicating if the firmware configuration interface
> @@ -97,126 +40,6 @@ QemuFwCfgIsAvailable (
>return (BOOLEAN)(mFwCfgSelectorAddress != 0 && mFwCfgDataAddress != 0);
>  }
>
> -RETURN_STATUS
> -EFIAPI
> -QemuFwCfgInitialize (
> -  VOID
> -  )
> -{
> -  EFI_STATUS   Status;
> -  FDT_CLIENT_PROTOCOL  *FdtClient;
> -  CONST UINT64 *Reg;
> -  UINT32   RegSize;
> -  UINTNAddressCells, SizeCells;
> -  UINT64   FwCfgSelectorAddress;
> -  UINT64   FwCfgSelectorSize;
> -  UINT64   FwCfgDataAddress;
> -  UINT64   FwCfgDataSize;
> -  UINT64   FwCfgDmaAddress;
> -  UINT64   FwCfgDmaSize;
> -
> -  Status = gBS->LocateProtocol (
> -  ,
> -  NULL,
> -  (VOID **)
> -  );
> -  ASSERT_EFI_ERROR (Status);
> -
> -  Status = FdtClient->FindCompatibleNodeReg (
> -FdtClient,
> -"qemu,fw-cfg-mmio",
> -(CONST VOID **),
> -,
> -,
> -
> -);
> -  if (EFI_ERROR (Status)) {
> -DEBUG ((
> -  DEBUG_WARN,
> -  "%a: No 'qemu,fw-cfg-mmio' compatible DT node found (Status == %r)\n",
> -  __func__,
> -  Status
> -  ));
> -return EFI_SUCCESS;
> -  }
> -
> -  ASSERT (AddressCells == 2);
> -  ASSERT (SizeCells 

[edk2-devel] [PATCH v3 6/7] OvmfPkg/RiscVVirt: Enable QemuFwCfgMmioDxeLib.inf

2024-04-25 Thread Chao Li
Enable QemuFwCfgMmioDxeLib.inf in RiscVVirtQemu.dsc

Build-tested only (with "RiscVVirtQemu.dsc").

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4755

Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Gerd Hoffmann 
Cc: Sunil V L 
Cc: Andrei Warkentin 
Signed-off-by: Chao Li 
---
 OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc 
b/OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc
index 27f24648e8..e0ed6fb9bc 100644
--- a/OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc
+++ b/OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc
@@ -78,7 +78,7 @@ [LibraryClasses.common]
   # Virtio Support
   VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
   
VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf
-  QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf
+  QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxeLib.inf
   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf
   
QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
   
QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
-- 
2.27.0



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




[edk2-devel] [PATCH v3 7/7] OvmfPkg: Remove QemuFwCfgLibMmio.inf

2024-04-25 Thread Chao Li
All of platforms are switching to QemuFwCfgMmioDxeLib.inf, remove
QemuFwCfgLibMmio.inf now.

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4755

Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Gerd Hoffmann 
Signed-off-by: Chao Li 
---
 .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf | 51 ---
 1 file changed, 51 deletions(-)
 delete mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf

diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf 
b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf
deleted file mode 100644
index 8e191f2d22..00
--- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf
+++ /dev/null
@@ -1,51 +0,0 @@
-## @file
-#
-#  Stateful, implicitly initialized fw_cfg library.
-#
-#  Copyright (C) 2013 - 2014, Red Hat, Inc.
-#  Copyright (c) 2008 - 2012, Intel Corporation. All rights reserved.
-#  Copyright (c) 2024 Loongson Technology Corporation Limited. All rights 
reserved.
-#
-#  SPDX-License-Identifier: BSD-2-Clause-Patent
-#
-##
-
-[Defines]
-  INF_VERSION= 0x00010005
-  BASE_NAME  = QemuFwCfgLib
-  FILE_GUID  = B271F41F-B841-48A9-BA8D-545B4BC2E2BF
-  MODULE_TYPE= BASE
-  VERSION_STRING = 1.0
-  LIBRARY_CLASS  = QemuFwCfgLib|DXE_DRIVER UEFI_DRIVER
-
-  CONSTRUCTOR= QemuFwCfgInitialize
-
-#
-# The following information is for reference only and not required by the build
-# tools.
-#
-#  VALID_ARCHITECTURES   = ARM AARCH64 RISCV64 LOONGARCH64
-#
-
-[Sources]
-  QemuFwCfgLibMmio.c
-  QemuFwCfgMmioDxe.c
-
-[Packages]
-  MdePkg/MdePkg.dec
-  OvmfPkg/OvmfPkg.dec
-  EmbeddedPkg/EmbeddedPkg.dec
-
-[LibraryClasses]
-  BaseLib
-  BaseMemoryLib
-  DebugLib
-  HobLib
-  IoLib
-  UefiBootServicesTableLib
-
-[Protocols]
-  gFdtClientProtocolGuid## CONSUMES
-
-[Depex]
-  gFdtClientProtocolGuid
-- 
2.27.0



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




[edk2-devel] [PATCH v3 5/7] ArmVirtPkg: Enable QemuFwCfgMmioDxeLib.inf

2024-04-25 Thread Chao Li
Enable QemuFwCfgMmioDxeLib.inf in ArmVirtQemu.dsc and
ArmVirtQemuKernel.dsc.

Build-tested only (with "ArmVirtQemu.dsc").

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4755

Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Gerd Hoffmann 
Cc: Leif Lindholm 
Cc: Sami Mujawar 
Signed-off-by: Chao Li 
---
 ArmVirtPkg/ArmVirtQemu.dsc   | 2 +-
 ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index e48c75b5e9..987b8bb238 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -60,7 +60,7 @@ [LibraryClasses.common]
   # Virtio Support
   VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
   
VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf
-  QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf
+  QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxeLib.inf
   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf
   
QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
   
QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index 668a65ba64..efe2df97bd 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -57,7 +57,7 @@ [LibraryClasses.common]
   # Virtio Support
   VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
   
VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf
-  QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf
+  QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxeLib.inf
   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf
   
QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
   
QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
-- 
2.27.0



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




[edk2-devel] [PATCH v3 4/7] OvmfPkg: Copy the same new INF as QemuFwCfgLibMmio.inf

2024-04-25 Thread Chao Li
Copy QemuFwCfgLibMmio.inf to QemuFwCfgMmioDxeLib.inf,
QemuFwCfgLibMmio.inf will be deleted when all platforms switching is
completed.

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4755

Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Gerd Hoffmann 
Signed-off-by: Chao Li 
---
 .../QemuFwCfgLib/QemuFwCfgMmioDxeLib.inf  | 51 +++
 1 file changed, 51 insertions(+)
 create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxeLib.inf

diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxeLib.inf 
b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxeLib.inf
new file mode 100644
index 00..8e191f2d22
--- /dev/null
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxeLib.inf
@@ -0,0 +1,51 @@
+## @file
+#
+#  Stateful, implicitly initialized fw_cfg library.
+#
+#  Copyright (C) 2013 - 2014, Red Hat, Inc.
+#  Copyright (c) 2008 - 2012, Intel Corporation. All rights reserved.
+#  Copyright (c) 2024 Loongson Technology Corporation Limited. All rights 
reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION= 0x00010005
+  BASE_NAME  = QemuFwCfgLib
+  FILE_GUID  = B271F41F-B841-48A9-BA8D-545B4BC2E2BF
+  MODULE_TYPE= BASE
+  VERSION_STRING = 1.0
+  LIBRARY_CLASS  = QemuFwCfgLib|DXE_DRIVER UEFI_DRIVER
+
+  CONSTRUCTOR= QemuFwCfgInitialize
+
+#
+# The following information is for reference only and not required by the build
+# tools.
+#
+#  VALID_ARCHITECTURES   = ARM AARCH64 RISCV64 LOONGARCH64
+#
+
+[Sources]
+  QemuFwCfgLibMmio.c
+  QemuFwCfgMmioDxe.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  BaseMemoryLib
+  DebugLib
+  HobLib
+  IoLib
+  UefiBootServicesTableLib
+
+[Protocols]
+  gFdtClientProtocolGuid## CONSUMES
+
+[Depex]
+  gFdtClientProtocolGuid
-- 
2.27.0



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




[edk2-devel] [PATCH v3 3/7] OvmfPkg: Add the QemuFwCfgMmioLib PEI stage version

2024-04-25 Thread Chao Li
Added the PEI stage library for QemuFwCfgMmioLib, which uses the FDT to
find the fw_cfg and parse it.

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4755

Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Gerd Hoffmann 
Co-authored-by: Xianglai Li 
Signed-off-by: Chao Li 
---
 .../Library/QemuFwCfgLib/QemuFwCfgMmioPei.c   | 228 ++
 .../QemuFwCfgLib/QemuFwCfgMmioPeiLib.inf  |  49 
 2 files changed, 277 insertions(+)
 create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioPei.c
 create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioPeiLib.inf

diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioPei.c 
b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioPei.c
new file mode 100644
index 00..fa6f531e3a
--- /dev/null
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioPei.c
@@ -0,0 +1,228 @@
+/** @file
+
+  Stateful and implicitly initialized fw_cfg library implementation.
+
+  Copyright (C) 2013 - 2014, Red Hat, Inc.
+  Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.
+  (C) Copyright 2021 Hewlett Packard Enterprise Development LP
+  Copyright (c) 2024 Loongson Technology Corporation Limited. All rights 
reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include "QemuFwCfgLibMmioInternal.h"
+
+//
+// These correspond to the implementation we detect at runtime.
+//
+READ_BYTES_FUNCTION   *InternalQemuFwCfgReadBytes  = MmioReadBytes;
+WRITE_BYTES_FUNCTION  *InternalQemuFwCfgWriteBytes = MmioWriteBytes;
+SKIP_BYTES_FUNCTION   *InternalQemuFwCfgSkipBytes  = MmioSkipBytes;
+
+/**
+  To get firmware configure selector address.
+
+  @param VOID
+
+  @retval  firmware configure selector address
+**/
+UINTN
+EFIAPI
+QemuGetFwCfgSelectorAddress (
+  VOID
+  )
+{
+  QEMU_FW_CFG_RESOURCE  *FwCfgResource;
+
+  FwCfgResource = QemuGetFwCfgResourceHob ();
+  ASSERT (FwCfgResource != NULL);
+
+  return FwCfgResource->FwCfgSelectorAddress;
+}
+
+/**
+  To get firmware configure Data address.
+
+  @param VOID
+
+  @retval  firmware configure data address
+**/
+UINTN
+EFIAPI
+QemuGetFwCfgDataAddress (
+  VOID
+  )
+{
+  QEMU_FW_CFG_RESOURCE  *FwCfgResource;
+
+  FwCfgResource = QemuGetFwCfgResourceHob ();
+  ASSERT (FwCfgResource != NULL);
+
+  return FwCfgResource->FwCfgDataAddress;
+}
+
+/**
+  To get firmware DMA address.
+
+  @param VOID
+
+  @retval  firmware DMA address
+**/
+UINTN
+EFIAPI
+QemuGetFwCfgDmaAddress (
+  VOID
+  )
+{
+  QEMU_FW_CFG_RESOURCE  *FwCfgResource;
+
+  FwCfgResource = QemuGetFwCfgResourceHob ();
+  ASSERT (FwCfgResource != NULL);
+
+  return FwCfgResource->FwCfgDmaAddress;
+}
+
+RETURN_STATUS
+EFIAPI
+QemuFwCfgInitialize (
+  VOID
+  )
+{
+  VOID  *DeviceTreeBase;
+  INT32 Node;
+  INT32 Prev;
+  CONST CHAR8   *Type;
+  INT32 Len;
+  CONST UINT64  *Reg;
+  UINT64FwCfgSelectorAddress;
+  UINT64FwCfgSelectorSize;
+  UINT64FwCfgDataAddress;
+  UINT64FwCfgDataSize;
+  UINT64FwCfgDmaAddress;
+  UINT64FwCfgDmaSize;
+  QEMU_FW_CFG_RESOURCE  *FwCfgResource;
+  VOID  *Buffer;
+
+  //
+  // Check whether the Qemu firmware configure resources HOB has been created,
+  // if so use the resources in the HOB.
+  //
+  FwCfgResource = QemuGetFwCfgResourceHob ();
+  if (FwCfgResource != NULL) {
+return RETURN_SUCCESS;
+  }
+
+  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeInitialBaseAddress);
+  ASSERT (DeviceTreeBase != NULL);
+  //
+  // Make sure we have a valid device tree blob
+  //
+  ASSERT (fdt_check_header (DeviceTreeBase) == 0);
+
+  //
+  // Create resouce memory
+  //
+  Buffer = AllocatePages(EFI_SIZE_TO_PAGES (sizeof (QEMU_FW_CFG_RESOURCE)));
+  ASSERT (Buffer != NULL);
+  ZeroMem (Buffer, sizeof (QEMU_FW_CFG_RESOURCE));
+
+  FwCfgResource = (QEMU_FW_CFG_RESOURCE *)Buffer;
+
+  for (Prev = 0; ; Prev = Node) {
+Node = fdt_next_node (DeviceTreeBase, Prev, NULL);
+if (Node < 0) {
+  break;
+}
+
+//
+// Check for memory node
+//
+Type = fdt_getprop (DeviceTreeBase, Node, "compatible", );
+if ((Type) &&
+(AsciiStrnCmp (Type, "qemu,fw-cfg-mmio", Len) == 0))
+{
+  //
+  // Get the 'reg' property of this node. For now, we will assume
+  // two 8 byte quantities for base and size, respectively.
+  //
+  Reg = fdt_getprop (DeviceTreeBase, Node, "reg", );
+  if ((Reg != 0) && (Len == (2 * sizeof (UINT64 {
+FwCfgDataAddress = SwapBytes64 (Reg[0]);
+FwCfgDataSize= 8;
+FwCfgSelectorAddress = FwCfgDataAddress + FwCfgDataSize;
+FwCfgSelectorSize= 2;
+
+//
+// The following ASSERT()s express
+//
+//   Address + Size - 1 <= MAX_UINTN
+//
+// for both registers, that is, that the last byte in each MMIO range 
is
+ 

[edk2-devel] [PATCH v3 2/7] OvmfPkg: Add the way of HOBs in QemuFwCfgLibMmio

2024-04-25 Thread Chao Li
Added the HOB methods to load and store the QEMU firmware configure
address, data address and DMA address, which are not enabled during the
DXE stage.

Build-tested only (with "ArmVirtQemu.dsc and RiscVVirtQemu.dsc").

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4755

Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Gerd Hoffmann 
Cc: Leif Lindholm 
Cc: Sami Mujawar 
Cc: Sunil V L 
Cc: Andrei Warkentin 
Signed-off-by: Chao Li 
---
 .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.c   | 81 +++--
 .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf |  1 +
 .../QemuFwCfgLib/QemuFwCfgLibMmioInternal.h   | 74 +++
 .../Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c   | 91 ---
 4 files changed, 226 insertions(+), 21 deletions(-)

diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c 
b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c
index dc949c8e26..b5dbc5e4b5 100644
--- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c
@@ -8,11 +8,16 @@
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
 
+#include 
 #include 
 
+#include 
+#include 
+
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -21,6 +26,62 @@
 
 #include "QemuFwCfgLibMmioInternal.h"
 
+EFI_GUID  mFwCfgResourceGuid = FW_CONFIG_RESOURCE_HOB_GUID;
+
+/**
+  Build firmware configure resource address HOB.
+
+  @param[in]   FwCfgResource  A pointer to firmware configure resource.
+
+  @retval  NULL
+**/
+VOID
+QemuBuildFwCfgResourceHob (
+  IN QEMU_FW_CFG_RESOURCE  *FwCfgResource
+  )
+{
+  UINT64  Data64;
+
+  Data64 = (UINT64)(UINTN)FwCfgResource;
+
+  BuildGuidDataHob (
+,
+(VOID *),
+sizeof (QEMU_FW_CFG_RESOURCE)
+);
+}
+
+/**
+  Get firmware configure resource in HOB.
+
+  @param VOID
+
+  @retval  FwCfgResourceThe firmware configure resouce in HOB.
+   NULL The firmware configure resouce not found.
+**/
+QEMU_FW_CFG_RESOURCE *
+QemuGetFwCfgResourceHob (
+  VOID
+  )
+{
+  EFI_HOB_GUID_TYPE *GuidHob;
+  VOID  *DataInHob;
+  QEMU_FW_CFG_RESOURCE  *FwCfgResource;
+
+  GuidHob   = NULL;
+  DataInHob = NULL;
+
+  GuidHob = GetFirstGuidHob ();
+  if (GuidHob == NULL) {
+return NULL;
+  }
+
+  DataInHob = GET_GUID_HOB_DATA (GuidHob);
+  FwCfgResource = (QEMU_FW_CFG_RESOURCE *)(*(UINTN *)DataInHob);
+
+  return FwCfgResource;
+}
+
 /**
   Returns a boolean indicating if the firmware configuration interface
   is available or not.
@@ -37,7 +98,7 @@ QemuFwCfgIsAvailable (
   VOID
   )
 {
-  return (BOOLEAN)(mFwCfgSelectorAddress != 0 && mFwCfgDataAddress != 0);
+  return (BOOLEAN)(QemuGetFwCfgSelectorAddress () != 0 && 
QemuGetFwCfgDataAddress () != 0);
 }
 
 /**
@@ -56,7 +117,7 @@ QemuFwCfgSelectItem (
   )
 {
   if (QemuFwCfgIsAvailable ()) {
-MmioWrite16 (mFwCfgSelectorAddress, SwapBytes16 ((UINT16)QemuFwCfgItem));
+MmioWrite16 (QemuGetFwCfgSelectorAddress (), SwapBytes16 
((UINT16)QemuFwCfgItem));
   }
 }
 
@@ -86,30 +147,30 @@ MmioReadBytes (
 
  #if defined (MDE_CPU_AARCH64) || defined (MDE_CPU_RISCV64) || defined 
(MDE_CPU_LOONGARCH64)
   while (Ptr < End) {
-*(UINT64 *)Ptr = MmioRead64 (mFwCfgDataAddress);
+*(UINT64 *)Ptr = MmioRead64 (QemuGetFwCfgDataAddress ());
 Ptr   += 8;
   }
 
   if (Left & 4) {
-*(UINT32 *)Ptr = MmioRead32 (mFwCfgDataAddress);
+*(UINT32 *)Ptr = MmioRead32 (QemuGetFwCfgDataAddress ());
 Ptr   += 4;
   }
 
  #else
   while (Ptr < End) {
-*(UINT32 *)Ptr = MmioRead32 (mFwCfgDataAddress);
+*(UINT32 *)Ptr = MmioRead32 (QemuGetFwCfgDataAddress ());
 Ptr   += 4;
   }
 
  #endif
 
   if (Left & 2) {
-*(UINT16 *)Ptr = MmioRead16 (mFwCfgDataAddress);
+*(UINT16 *)Ptr = MmioRead16 (QemuGetFwCfgDataAddress ());
 Ptr   += 2;
   }
 
   if (Left & 1) {
-*Ptr = MmioRead8 (mFwCfgDataAddress);
+*Ptr = MmioRead8 (QemuGetFwCfgDataAddress ());
   }
 }
 
@@ -162,9 +223,9 @@ DmaTransferBytes (
   // This will fire off the transfer.
   //
  #if defined (MDE_CPU_AARCH64) || defined (MDE_CPU_RISCV64) || defined 
(MDE_CPU_LOONGARCH64)
-  MmioWrite64 (mFwCfgDmaAddress, SwapBytes64 ((UINT64)));
+  MmioWrite64 (QemuGetFwCfgDmaAddress (), SwapBytes64 ((UINT64)));
  #else
-  MmioWrite32 ((UINT32)(mFwCfgDmaAddress + 4), SwapBytes32 ((UINT32)));
+  MmioWrite32 ((UINT32)(QemuGetFwCfgDmaAddress () + 4), SwapBytes32 
((UINT32)));
  #endif
 
   //
@@ -233,7 +294,7 @@ MmioWriteBytes (
   UINTN  Idx;
 
   for (Idx = 0; Idx < Size; ++Idx) {
-MmioWrite8 (mFwCfgDataAddress, ((UINT8 *)Buffer)[Idx]);
+MmioWrite8 (QemuGetFwCfgDataAddress (), ((UINT8 *)Buffer)[Idx]);
   }
 }
 
diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf 
b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf
index f2596f270e..8e191f2d22 100644
--- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf
@@ -40,6 +40,7 @@ [LibraryClasses]
   BaseLib
   BaseMemoryLib
   

[edk2-devel] [PATCH v3 1/7] OvmfPkg: Separate QemuFwCfgLibMmio.c into two files

2024-04-25 Thread Chao Li
Separate QemuFwCfgLibMmio.c into two files named QemuFwCfgLibMmio.c and
QemuFwCfgLibMmioDxe.c, added a new header named
QemuFwCfgLibMmioInternal.h for MMIO version.

Build-tested only (with "ArmVirtQemu.dsc and RiscVVirtQemu.dsc").

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4755

Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Gerd Hoffmann 
Cc: Leif Lindholm 
Cc: Sami Mujawar 
Cc: Sunil V L 
Cc: Andrei Warkentin 
Signed-off-by: Chao Li 
---
 .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.c   | 194 +-
 .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf |   4 +-
 .../QemuFwCfgLib/QemuFwCfgLibMmioInternal.h   | 179 
 .../Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c   | 153 ++
 4 files changed, 340 insertions(+), 190 deletions(-)
 create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmioInternal.h
 create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c

diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c 
b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c
index 115a210759..dc949c8e26 100644
--- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c
@@ -1,10 +1,9 @@
 /** @file
 
-  Stateful and implicitly initialized fw_cfg library implementation.
-
   Copyright (C) 2013 - 2014, Red Hat, Inc.
   Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.
   (C) Copyright 2021 Hewlett Packard Enterprise Development LP
+  Copyright (c) 2024 Loongson Technology Corporation Limited. All rights 
reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
@@ -20,63 +19,7 @@
 
 #include 
 
-STATIC UINTN  mFwCfgSelectorAddress;
-STATIC UINTN  mFwCfgDataAddress;
-STATIC UINTN  mFwCfgDmaAddress;
-
-/**
-  Reads firmware configuration bytes into a buffer
-
-  @param[in] SizeSize in bytes to read
-  @param[in] Buffer  Buffer to store data into  (OPTIONAL if Size is 0)
-
-**/
-typedef
-VOID(EFIAPI READ_BYTES_FUNCTION)(
-  IN UINTN Size,
-  IN VOID  *Buffer OPTIONAL
-  );
-
-/**
-  Writes bytes from a buffer to firmware configuration
-
-  @param[in] SizeSize in bytes to write
-  @param[in] Buffer  Buffer to transfer data from (OPTIONAL if Size is 0)
-
-**/
-typedef
-VOID(EFIAPI WRITE_BYTES_FUNCTION)(
-  IN UINTN Size,
-  IN VOID  *Buffer OPTIONAL
-  );
-
-/**
-  Skips bytes in firmware configuration
-
-  @param[in] Size  Size in bytes to skip
-
-**/
-typedef
-VOID(EFIAPI SKIP_BYTES_FUNCTION)(
-  IN UINTN Size
-  );
-
-//
-// Forward declaration of the two implementations we have.
-//
-STATIC READ_BYTES_FUNCTION   MmioReadBytes;
-STATIC WRITE_BYTES_FUNCTION  MmioWriteBytes;
-STATIC SKIP_BYTES_FUNCTION   MmioSkipBytes;
-STATIC READ_BYTES_FUNCTION   DmaReadBytes;
-STATIC WRITE_BYTES_FUNCTION  DmaWriteBytes;
-STATIC SKIP_BYTES_FUNCTION   DmaSkipBytes;
-
-//
-// These correspond to the implementation we detect at runtime.
-//
-STATIC READ_BYTES_FUNCTION   *InternalQemuFwCfgReadBytes  = MmioReadBytes;
-STATIC WRITE_BYTES_FUNCTION  *InternalQemuFwCfgWriteBytes = MmioWriteBytes;
-STATIC SKIP_BYTES_FUNCTION   *InternalQemuFwCfgSkipBytes  = MmioSkipBytes;
+#include "QemuFwCfgLibMmioInternal.h"
 
 /**
   Returns a boolean indicating if the firmware configuration interface
@@ -97,126 +40,6 @@ QemuFwCfgIsAvailable (
   return (BOOLEAN)(mFwCfgSelectorAddress != 0 && mFwCfgDataAddress != 0);
 }
 
-RETURN_STATUS
-EFIAPI
-QemuFwCfgInitialize (
-  VOID
-  )
-{
-  EFI_STATUS   Status;
-  FDT_CLIENT_PROTOCOL  *FdtClient;
-  CONST UINT64 *Reg;
-  UINT32   RegSize;
-  UINTNAddressCells, SizeCells;
-  UINT64   FwCfgSelectorAddress;
-  UINT64   FwCfgSelectorSize;
-  UINT64   FwCfgDataAddress;
-  UINT64   FwCfgDataSize;
-  UINT64   FwCfgDmaAddress;
-  UINT64   FwCfgDmaSize;
-
-  Status = gBS->LocateProtocol (
-  ,
-  NULL,
-  (VOID **)
-  );
-  ASSERT_EFI_ERROR (Status);
-
-  Status = FdtClient->FindCompatibleNodeReg (
-FdtClient,
-"qemu,fw-cfg-mmio",
-(CONST VOID **),
-,
-,
-
-);
-  if (EFI_ERROR (Status)) {
-DEBUG ((
-  DEBUG_WARN,
-  "%a: No 'qemu,fw-cfg-mmio' compatible DT node found (Status == %r)\n",
-  __func__,
-  Status
-  ));
-return EFI_SUCCESS;
-  }
-
-  ASSERT (AddressCells == 2);
-  ASSERT (SizeCells == 2);
-  ASSERT (RegSize == 2 * sizeof (UINT64));
-
-  FwCfgDataAddress = SwapBytes64 (Reg[0]);
-  FwCfgDataSize= 8;
-  FwCfgSelectorAddress = FwCfgDataAddress + FwCfgDataSize;
-  FwCfgSelectorSize= 2;
-
-  //
-  // The following ASSERT()s express
-  //
-  //   Address + Size - 1 <= MAX_UINTN
-  //
-  // for both registers, that is, that the last byte in each MMIO range is
-  // expressible as a MAX_UINTN. The form below is 

[edk2-devel] [PATCH v3 0/7] Adjust the QemuFwCfgLibMmio and add PEI stage

2024-04-25 Thread Chao Li
Patch1: Added three PCDs for QemuFwCfgLibMmio
Patch2: Sparate QemuFwCfgLibMmio.c into two files and default as DXE
stage library.
Patch3: Added QemuFwCfgMmiLib PEI version
Patch4: Rename QemuFwCfgLibMmio.inf to QemuFwCfgMmioDxeLib.inf and
enable it in AARCH64 and RISCV64.

V1 -> V2:
1. Use HOBs instead of PCD.
2. The old patch2 is divided into two parts, one is code splitting, and
the other is functional changes.
3. add two patches to keep the safe when change the platform DSC file.

V2 -> V3:
1. Merge three HOBs into a single HOB.
2. Remove the dynamic global variables in PEI.

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4755

PR: https://github.com/tianocore/edk2/pull/5568

Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Gerd Hoffmann 
Cc: Leif Lindholm 
Cc: Sami Mujawar 
Cc: Sunil V L 
Cc: Andrei Warkentin 



Chao Li (7):
  OvmfPkg: Separate QemuFwCfgLibMmio.c into two files
  OvmfPkg: Add the way of HOBs in QemuFwCfgLibMmio
  OvmfPkg: Add the QemuFwCfgMmioLib PEI stage version
  OvmfPkg: Copy the same new INF as QemuFwCfgLibMmio.inf
  ArmVirtPkg: Enable QemuFwCfgMmioDxeLib.inf
  OvmfPkg/RiscVVirt: Enable QemuFwCfgMmioDxeLib.inf
  OvmfPkg: Remove QemuFwCfgLibMmio.inf

 ArmVirtPkg/ArmVirtQemu.dsc|   2 +-
 ArmVirtPkg/ArmVirtQemuKernel.dsc  |   2 +-
 .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.c   | 249 +
 .../QemuFwCfgLib/QemuFwCfgLibMmioInternal.h   | 253 ++
 .../Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c   | 222 +++
 ...CfgLibMmio.inf => QemuFwCfgMmioDxeLib.inf} |   5 +-
 .../Library/QemuFwCfgLib/QemuFwCfgMmioPei.c   | 228 
 .../QemuFwCfgLib/QemuFwCfgMmioPeiLib.inf  |  49 
 OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc   |   2 +-
 9 files changed, 822 insertions(+), 190 deletions(-)
 create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmioInternal.h
 create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioDxe.c
 rename OvmfPkg/Library/QemuFwCfgLib/{QemuFwCfgLibMmio.inf => 
QemuFwCfgMmioDxeLib.inf} (81%)
 create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioPei.c
 create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgMmioPeiLib.inf

-- 
2.27.0



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




[edk2-devel] [PATCH edk2-platforms WIP 2/3] SbsaQemu: describe PCIe buses in SSDT tables

2024-04-25 Thread Marcin Juszkiewicz
We can have more than one PCI Express bus. So instead of having static
description in DSDT we create SSDT table for each existing PCIe bus.

Signed-off-by: Marcin Juszkiewicz 
---
 Platform/Qemu/SbsaQemu/SbsaQemu.dsc |   2 +
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf |  37 +-
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiPcie.h  |  23 +
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c   |  87 ++-
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiPcie.c  | 576 
 Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl   | 302 --
 .../Drivers/SbsaQemuAcpiDxe/SsdtTemplate.asl|  82 +++
 7 files changed, 790 insertions(+), 319 deletions(-)

diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc 
b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
index e246db8b0a23..b012eaa34147 100644
--- a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
+++ b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
@@ -173,6 +173,8 @@ [LibraryClasses.common]
   
ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf
 
   AcpiLib|EmbeddedPkg/Library/AcpiLib/AcpiLib.inf
+  AcpiHelperLib|DynamicTablesPkg/Library/Common/AcpiHelperLib/AcpiHelperLib.inf
+  AmlLib|DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf
 
   ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
   ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf 
b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
index 727c8e82d16e..6de1073e6ac2 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
@@ -18,18 +18,25 @@ [Defines]
 
 [Sources]
   SbsaQemuAcpiDxe.c
+  SbsaQemuAcpiDxe.h
+  SbsaQemuAcpiPcie.c
+  SbsaQemuAcpiPcie.h
+  SsdtTemplate.asl
 
 [Packages]
   ArmPkg/ArmPkg.dec
   ArmPlatformPkg/ArmPlatformPkg.dec
   ArmVirtPkg/ArmVirtPkg.dec
+  DynamicTablesPkg/DynamicTablesPkg.dec
   EmbeddedPkg/EmbeddedPkg.dec
   MdeModulePkg/MdeModulePkg.dec
   MdePkg/MdePkg.dec
   Silicon/Qemu/SbsaQemu/SbsaQemu.dec
 
 [LibraryClasses]
+  AcpiHelperLib
   AcpiLib
+  AmlLib
   ArmLib
   BaseMemoryLib
   BaseLib
@@ -37,6 +44,7 @@ [LibraryClasses]
   DxeServicesLib
   HardwareInfoLib
   PcdLib
+  PciLib
   PrintLib
   UefiDriverEntryPoint
   UefiLib
@@ -54,14 +62,39 @@ [Pcd]
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdSmmuBase
 
 [Depex]
-  gEfiAcpiTableProtocolGuid   ## CONSUMES
+  # We want all PCIe buses to be scanned first
+  gEfiPciIoProtocolGuid ## CONSUMES
 
 [Guids]
   gEdkiiPlatformHasAcpiGuid
 
 [Protocols]
   gEfiAcpiSdtProtocolGuid
-  gEfiAcpiTableProtocolGuid   ## CONSUMES
+  gEfiAcpiTableProtocolGuid
+  gEfiPciRootBridgeIoProtocolGuid
+
+
+[Pcd]
+  gArmTokenSpaceGuid.PcdPciBusMin
+  gArmTokenSpaceGuid.PcdPciBusMax
+  gArmTokenSpaceGuid.PcdPciIoBase
+  gArmTokenSpaceGuid.PcdPciIoSize
+  gEfiMdePkgTokenSpaceGuid.PcdPciIoTranslation
+  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPciIoLimit
+
+  gArmTokenSpaceGuid.PcdPciMmio32Base
+  gArmTokenSpaceGuid.PcdPciMmio32Size
+  gEfiMdePkgTokenSpaceGuid.PcdPciMmio32Translation
+  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPciMmio32Limit
+
+  gArmTokenSpaceGuid.PcdPciMmio64Base
+  gArmTokenSpaceGuid.PcdPciMmio64Size
+  gEfiMdePkgTokenSpaceGuid.PcdPciMmio64Translation
+  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPciMmio64Limit
+
+  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
+  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPciExpressBarSize
+  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPciExpressBarLimit
 
 [FixedPcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiPcie.h 
b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiPcie.h
new file mode 100644
index ..56cc6f1381da
--- /dev/null
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiPcie.h
@@ -0,0 +1,23 @@
+/** @file
+*  This file is an ACPI driver for the Qemu SBSA platform.
+*
+*  Copyright (c) 2024, Linaro Ltd. All rights reserved.
+*
+*  SPDX-License-Identifier: BSD-2-Clause-Patent
+*
+**/
+
+#ifndef SBSAQEMU_ACPI_PCIE_H
+#define SBSAQEMU_ACPI_PCIE_H
+
+#pragma pack(1)
+
+/* AML bytecode generated from SsdtTemplate.asl */
+extern CHAR8  ssdttemplate_aml_code[];
+
+EFI_STATUS
+AddPcieHostBridges (
+  AML_OBJECT_NODE_HANDLE  ScopeNode
+  );
+
+#endif
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c 
b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
index 30239e7dca0d..f3d5dc9e9ba7 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
@@ -6,28 +6,27 @@
 *  SPDX-License-Identifier: BSD-2-Clause-Patent
 *
 **/
-#include 
-#include 
 #include 
 #include 
 #include 
+
 #include 
+#include 
 #include 
 #include 
 #include 
-#include 
-#include 
-#include 
 #include 

[edk2-devel] [PATCH edk2-platforms WIP 3/3] SbsaQemu: generate MCFG table

2024-04-25 Thread Marcin Juszkiewicz
We want to have dynaminc PCI Express variables. Which forces us to
generate MCFG from C code.

Signed-off-by: Marcin Juszkiewicz 
---
 Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf  |  1 -
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c| 83 
 Silicon/Qemu/SbsaQemu/AcpiTables/Mcfg.aslc   | 43 --
 3 files changed, 83 insertions(+), 44 deletions(-)

diff --git a/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf 
b/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
index 8d4905362edc..37abf2f4c512 100644
--- a/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
+++ b/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
@@ -19,7 +19,6 @@ [Sources]
   Dbg2.aslc
   Dsdt.asl
   Fadt.aslc
-  Mcfg.aslc
   Spcr.aslc
 
 [Packages]
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c 
b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
index f3d5dc9e9ba7..6c7913eead81 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
@@ -7,6 +7,7 @@
 *
 **/
 #include 
+#include 
 #include 
 #include 
 
@@ -883,6 +884,83 @@ AddSsdtPcieTable (
   return EFI_SUCCESS;
 }
 
+/** Adds the MCFG ACPI table.
+
+  @param AcpiTableThe ACPI Table.
+  @param PcieCfgData  PCIe configuration data.
+  @param NumPcieSegments  Number of PCIe segments.
+
+  @return EFI_SUCCESS on success, or an error code.
+
+**/
+STATIC
+EFI_STATUS
+AddMcfgTable (
+  IN EFI_ACPI_TABLE_PROTOCOL  *AcpiTable
+  )
+{
+  EFI_STATUSStatus;
+  UINTN TableHandle;
+  UINT32TableSize;
+  EFI_PHYSICAL_ADDRESS  PageAddress;
+  UINT8 *New;
+
+  EFI_ACPI_DESCRIPTION_HEADER  Header =
+SBSAQEMU_ACPI_HEADER (
+  
EFI_ACPI_6_3_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE,
+  EFI_ACPI_DESCRIPTION_HEADER,
+  EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_SPACE_ACCESS_TABLE_REVISION
+  );
+
+  TableSize = sizeof 
(EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER) +
+  sizeof 
(EFI_ACPI_MEMORY_MAPPED_ENHANCED_CONFIGURATION_SPACE_BASE_ADDRESS_ALLOCATION_STRUCTURE);
+
+  Status = gBS->AllocatePages (
+  AllocateAnyPages,
+  EfiACPIReclaimMemory,
+  EFI_SIZE_TO_PAGES (TableSize),
+  
+  );
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Failed to allocate pages for MCFG table\n"));
+return EFI_OUT_OF_RESOURCES;
+  }
+
+  New = (UINT8 *)(UINTN)PageAddress;
+  ZeroMem (New, TableSize);
+
+  // Add the  ACPI Description table header
+  CopyMem (New, , sizeof (EFI_ACPI_DESCRIPTION_HEADER));
+  ((EFI_ACPI_DESCRIPTION_HEADER *)New)->Length = TableSize;
+  New += sizeof 
(EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER);
+
+  
EFI_ACPI_MEMORY_MAPPED_ENHANCED_CONFIGURATION_SPACE_BASE_ADDRESS_ALLOCATION_STRUCTURE
  *CfgPtr;
+
+  CfgPtr = (VOID *)New;
+
+  CfgPtr->BaseAddress   = PcdGet64 (PcdPciExpressBaseAddress);
+  CfgPtr->PciSegmentGroupNumber = 0;
+  CfgPtr->StartBusNumber= PcdGet32 (PcdPciBusMin);
+  CfgPtr->EndBusNumber  = PcdGet32 (PcdPciBusMax);
+
+  New += sizeof 
(EFI_ACPI_MEMORY_MAPPED_ENHANCED_CONFIGURATION_SPACE_BASE_ADDRESS_ALLOCATION_STRUCTURE);
+
+  // Perform Checksum
+  AcpiTableChecksum ((UINT8 *)PageAddress, TableSize);
+
+  Status = AcpiTable->InstallAcpiTable (
+AcpiTable,
+(EFI_ACPI_COMMON_HEADER *)PageAddress,
+TableSize,
+
+);
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Failed to install MCFG table\n"));
+  }
+
+  return Status;
+}
+
 
 EFI_STATUS
 EFIAPI
@@ -951,6 +1029,11 @@ InitializeSbsaQemuAcpiDxe (
 DEBUG ((DEBUG_ERROR, "Failed to add SSDT table\n"));
   }
 
+  Status = AddMcfgTable (AcpiTable);
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Failed to add MCFG table\n"));
+  }
+
 
   return EFI_SUCCESS;
 }
diff --git a/Silicon/Qemu/SbsaQemu/AcpiTables/Mcfg.aslc 
b/Silicon/Qemu/SbsaQemu/AcpiTables/Mcfg.aslc
deleted file mode 100644
index 289f4ad4ea3a..
--- a/Silicon/Qemu/SbsaQemu/AcpiTables/Mcfg.aslc
+++ /dev/null
@@ -1,43 +0,0 @@
-/** @file
-*  ACPI Memory mapped configuration space base address Description Table 
(MCFG).
-*
-*  Copyright (c) 2020, Linaro Limited. All rights reserved.
-*
-*  SPDX-License-Identifier: BSD-2-Clause-Patent
-**/
-
-#include 
-#include 
-#include 
-
-#pragma pack(push, 1)
-
-typedef struct {
-  EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER Header;
-  
EFI_ACPI_MEMORY_MAPPED_ENHANCED_CONFIGURATION_SPACE_BASE_ADDRESS_ALLOCATION_STRUCTURE
 Structure[1];
-} EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_SPACE_ACCESS_DESCRIPTION_TABLE;
-
-EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_SPACE_ACCESS_DESCRIPTION_TABLE 

[edk2-devel] [PATCH WIP edk2-platforms 0/3] SbsaQemu: add support for multiple PCI Express buses

2024-04-25 Thread Marcin Juszkiewicz
QEMU allows to have NUMA setup where each node has own cpus, memory and
i/o. We already handle cpus and memory. This patchset adds support for
having multiple PCI Express buses.

SbsaQemu assumed that there is only bus 0. First patch does PCIe bus
scan to find all host bridges (bus 0 one and additional 'pxb-pcie'
ones).

Second patch moves description of PCIe from DSDT to SSDT (one per each
PCIe bus). So Operating System will know about all of them.

Third patch moves generation of MCFG table to C. It is preparation to
move PCIe Pcds from being fixed to dynamic ones.

There are some booting issues with assigning resources for cards:

pci :00:03.0: BAR 15: no space for [mem size 0x0020 64bit pref]
pci :00:03.0: BAR 15: failed to assign [mem size 0x0020 64bit pref]
pci :00:01.0: BAR 6: no space for [mem size 0x0004 pref]
pci :00:01.0: BAR 6: failed to assign [mem size 0x0004 pref]
pci :00:03.0: BAR 13: no space for [io  size 0x1000]
pci :00:03.0: BAR 13: failed to assign [io  size 0x1000]

Boot log (Linux + lspci + ACPI tables dump):
https://people.linaro.org/~marcin.juszkiewicz/sbsa-ref/boot-linux-with-numa-multiple-pcie-buses.txt

I am wondering where I made mistakes in handling PCIe buses.

Thanks go to Leif for pointing me to use of Aml to generate SSDT tables.

Cc: Leif Lindholm 
Cc: Ard Biesheuvel 
Cc: Graeme Gregory 
Cc: Ray Ni 
To: devel@edk2.groups.io

Signed-off-by: Marcin Juszkiewicz 
---
Marcin Juszkiewicz (3):
  SbsaQemu: scan for PCIe buses
  SbsaQemu: describe PCIe buses in SSDT tables
  SbsaQemu: generate MCFG table

 Platform/Qemu/SbsaQemu/SbsaQemu.dsc |   2 +
 Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf |   1 -
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf |  37 +-
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiPcie.h  |  23 +
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c   | 170 +-
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiPcie.c  | 576 
 .../SbsaQemuPciHostBridgeLib.c  | 185 ---
 Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl   | 302 --
 Silicon/Qemu/SbsaQemu/AcpiTables/Mcfg.aslc  |  43 --
 .../Drivers/SbsaQemuAcpiDxe/SsdtTemplate.asl|  82 +++
 10 files changed, 982 insertions(+), 439 deletions(-)
---
base-commit: 73cfdc4afff3e641be217b31b985761ef8338412
change-id: 20240425-review-multiple-pcie-0425-54ce3817fd3f

Best regards,
-- 
Marcin Juszkiewicz 



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




[edk2-devel] [PATCH edk2-platforms WIP 1/3] SbsaQemu: scan for PCIe buses

2024-04-25 Thread Marcin Juszkiewicz
SbsaQemu assumes that there is only one PCI Express bus. But there can
be multiple PCIe buses as NUMA systems can get 'pxb-pcie' HostBridge
devices added.

Let scan for all PCIe buses and report them back so EDK2 will be able to
find all expansions.

Signed-off-by: Marcin Juszkiewicz 
---
 .../SbsaQemuPciHostBridgeLib.c  | 185 
 1 file changed, 109 insertions(+), 76 deletions(-)

diff --git 
a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuPciHostBridgeLib/SbsaQemuPciHostBridgeLib.c
 
b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuPciHostBridgeLib/SbsaQemuPciHostBridgeLib.c
index 9739c7500def..1c4ed1c74e52 100644
--- 
a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuPciHostBridgeLib/SbsaQemuPciHostBridgeLib.c
+++ 
b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuPciHostBridgeLib/SbsaQemuPciHostBridgeLib.c
@@ -6,10 +6,14 @@
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
+#include 
+#include 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -52,76 +56,49 @@ CHAR16 *mPciHostBridgeLibAcpiAddressSpaceTypeStr[] = {
   L"Mem", L"I/O", L"Bus"
 };
 
-STATIC PCI_ROOT_BRIDGE mRootBridge = {
-  /* UINT32 Segment; Segment number */
-  0,
-
-  /* UINT64 Supports; Supported attributes */
-  EFI_PCI_ATTRIBUTE_ISA_IO_16 | EFI_PCI_ATTRIBUTE_ISA_MOTHERBOARD_IO |
-  EFI_PCI_ATTRIBUTE_VGA_IO_16 | EFI_PCI_ATTRIBUTE_VGA_PALETTE_IO_16,
-
-  /* UINT64 Attributes; Initial attributes */
-  EFI_PCI_ATTRIBUTE_ISA_IO_16 | EFI_PCI_ATTRIBUTE_ISA_MOTHERBOARD_IO |
-  EFI_PCI_ATTRIBUTE_VGA_IO_16 | EFI_PCI_ATTRIBUTE_VGA_PALETTE_IO_16,
-
-  /* BOOLEAN DmaAbove4G; DMA above 4GB memory */
-  TRUE,
-
-  /* BOOLEAN NoExtendedConfigSpace; When FALSE, the root bridge supports
- Extended (4096-byte) Configuration Space.  When TRUE, the root bridge
- supports 256-byte Configuration Space only. */
-  FALSE,
-
-  /* BOOLEAN ResourceAssigned; Resource assignment status of the root bridge.
- Set to TRUE if Bus/IO/MMIO resources for root bridge have been assigned */
-  FALSE,
-
-  /* UINT64 AllocationAttributes; Allocation attributes. */
-  EFI_PCI_HOST_BRIDGE_COMBINE_MEM_PMEM |
-  EFI_PCI_HOST_BRIDGE_MEM64_DECODE, /* as Mmio64Size > 0 */
-
-  {
- /* PCI_ROOT_BRIDGE_APERTURE Bus; Bus aperture which can be used by the
-  * root bridge. */
- FixedPcdGet32 (PcdPciBusMin),
- FixedPcdGet32 (PcdPciBusMax)
-  },
-
-  /* PCI_ROOT_BRIDGE_APERTURE Io; IO aperture which can be used by the root
- bridge */
-  {
- FixedPcdGet64 (PcdPciIoBase),
- FixedPcdGet64 (PcdPciIoBase) + FixedPcdGet64 (PcdPciIoSize) - 1
-  },
-
-  /* PCI_ROOT_BRIDGE_APERTURE Mem; MMIO aperture below 4GB which can be used by
- the root bridge
- (gEfiMdePkgTokenSpaceGuid.PcdPciMmio32Translation as 0x0) */
-  {
-FixedPcdGet32 (PcdPciMmio32Base),
-FixedPcdGet32 (PcdPciMmio32Base) + FixedPcdGet32 (PcdPciMmio32Size) - 1,
-  },
-
-  /* PCI_ROOT_BRIDGE_APERTURE MemAbove4G; MMIO aperture above 4GB which can be
- used by the root bridge.
- (gEfiMdePkgTokenSpaceGuid.PcdPciMmio64Translation as 0x0) */
-  {
-FixedPcdGet64 (PcdPciMmio64Base),
-FixedPcdGet64 (PcdPciMmio64Base) + FixedPcdGet64 (PcdPciMmio64Size) - 1
-  },
-
-  /* PCI_ROOT_BRIDGE_APERTURE PMem; Prefetchable MMIO aperture below 4GB which
- can be used by the root bridge.
- In our case, there are no separate ranges for prefetchable and
- non-prefetchable BARs */
-  { MAX_UINT64, 0 },
-
-  /* PCI_ROOT_BRIDGE_APERTURE PMemAbove4G; Prefetchable MMIO aperture above 4GB
- which can be used by the root bridge. */
-  { MAX_UINT64, 0 },
-  /* EFI_DEVICE_PATH_PROTOCOL *DevicePath; Device path. */
-  (EFI_DEVICE_PATH_PROTOCOL *),
-};
+EFI_STATUS
+EFIAPI
+PciHostBridgeUtilityInitRootBridge (
+  IN UINTN RootBusNumber,
+  OUT PCI_ROOT_BRIDGE  *RootBus
+  )
+{
+  EFI_PCI_ROOT_BRIDGE_DEVICE_PATH  *DevicePath;
+  UINTNMaxSubBusNumber = 255;
+
+  DevicePath = AllocateCopyPool (
+ sizeof mEfiPciRootBridgeDevicePath,
+ 
+ );
+  if (DevicePath == NULL) {
+DEBUG ((DEBUG_ERROR, "%a: %r\n", __func__, EFI_OUT_OF_RESOURCES));
+return EFI_OUT_OF_RESOURCES;
+  }
+
+  DevicePath->AcpiDevicePath.UID = RootBusNumber;
+
+  RootBus->Segment   = 0;
+  RootBus->Supports  = 0;
+  RootBus->Attributes= 0;
+  RootBus->DmaAbove4G= TRUE;
+  RootBus->AllocationAttributes  = EFI_PCI_HOST_BRIDGE_COMBINE_MEM_PMEM | 
EFI_PCI_HOST_BRIDGE_MEM64_DECODE, /* as Mmio64Size > 0 */
+  RootBus->Bus.Base  = RootBusNumber;
+  RootBus->Bus.Limit = MaxSubBusNumber;
+  RootBus->Io.Base   = PcdGet64 (PcdPciIoBase);
+  RootBus->Io.Limit  = PcdGet64 (PcdPciIoBase) + PcdGet64 
(PcdPciIoSize) - 1;
+  RootBus->Mem.Base  = PcdGet32 (PcdPciMmio32Base);
+  RootBus->Mem.Limit = PcdGet32 (PcdPciMmio32Base) + PcdGet32 
(PcdPciMmio32Size) - 1;
+  

Re: [edk2-devel] [PATCH] OvmfPkg: Set PcdCpuMaxLogicalProcessorNumber in OvmfXen

2024-04-25 Thread Alejandro Vallejo via groups.io
Hi,

On 25/04/2024 08:31, Gerd Hoffmann wrote:
> On Wed, Apr 24, 2024 at 02:36:32PM +0100, Alejandro Vallejo wrote:
>> Bump the compile-time constant for maximum processor count from 64 to 128
>> in order to allow that many vCPUs to be brought online on Xen guests with
>> the default OVMF configuration.
> 
>> +  # UefiCpuPkg PCDs related to initial AP bringup and general AP management.
>> +  gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|128
> 
> Note that this is a dynamic PCD, so you can set it at runtime to the
> number of vcpus present in the VM.  See MaxCpuCountInitialization() in
> OvmfPkg/PlatformPei/Platform.c for example.
> 
> take care,
>   Gerd
> 

Thanks for the heads up. Do you mean setting it at runtime through
fw_cfg? I saw PlatformMaxCpuCountInitialization() providing some
customizability, but Xen's toolstack doesn't provide fw_cfg at the
moment so it can't (as far as I've seen) use it.

I'm currently forced to do the override at compile time passing this...

--pcd gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber=128

... to build.sh, but I'd rather have the default max match Xen's idea of
max unless there's a strong reason not to.

Cheers,
Alejandro


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




Re: [edk2-devel] [PATCH V3 1/1] MdeModulePkg/XhciDxe: Add PCD for the delay of HCRST

2024-04-25 Thread Xianglei Cai
Hi @gaoliming,

Could you provide the eta when the patch can be merged?

Thanks,
Xianglei

-Original Message-
From: Cai, Xianglei 
Sent: Thursday, April 18, 2024 5:00 PM
To: gaoliming ; devel@edk2.groups.io
Cc: Ni, Ray ; Lewandowski, Krzysztof 
; Huang, Jenny ; Shih, 
More ; Chiu, Ian 
Subject: RE: [PATCH V3 1/1] MdeModulePkg/XhciDxe: Add PCD for the delay of HCRST

Hi Liming,

Could you help pick the change to master branch?

Thanks,
Xianglei

-Original Message-
From: gaoliming 
Sent: Monday, April 15, 2024 2:36 PM
To: Cai, Xianglei ; devel@edk2.groups.io
Cc: Ni, Ray ; Lewandowski, Krzysztof 
; Huang, Jenny ; Shih, 
More ; Chiu, Ian 
Subject: 回复: [PATCH V3 1/1] MdeModulePkg/XhciDxe: Add PCD for the delay of HCRST

Reviewed-by: Liming Gao 

> -邮件原件-
> 发件人: Xianglei Cai 
> 发送时间: 2024年4月15日 14:34
> 收件人: devel@edk2.groups.io
> 抄送: Xianglei Cai ; Ray Ni ; 
> Liming Gao ; Krzysztof Lewandowski 
> ; Jenny Huang 
> ; More Shih ; Ian Chiu 
> 
> 主题: [PATCH V3 1/1] MdeModulePkg/XhciDxe: Add PCD for the delay of 
> HCRST
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=4727
> 
> Recently some of XHCI host controllers require to have extra 1ms delay 
> before accessing any MMIO register during reset. PHY transition from
> P3 to P0 can take around 1.3ms and the xHCI reset can take around 
> 1.5ms.
> Add PCD to control the delay, the default is 2 ms.
> 
> Cc: Ray Ni   
> Cc: Liming Gao   
> Cc: Krzysztof Lewandowski
> Cc: Jenny Huang  
> Cc: More Shih
> Cc: Ian Chiu 
> Signed-off-by: Xianglei Cai 
> Reviewed-by: Krzysztof Lewandowski 
> ---
>  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h  | 1 +
>  MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf | 4 
>  MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c   | 2 +-
>  MdeModulePkg/MdeModulePkg.dec| 5 +
>  4 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
> b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
> index 44016758724c..c9a12095c29e 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
> @@ -28,6 +28,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent 
> #include   #include 
>   #include 
> +#include 
> 
>  #include 
> 
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf
> b/MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf
> index 18ef87916ae4..e6c1ac8a6346 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf
> @@ -56,6 +56,7 @@
>DebugLib
>ReportStatusCodeLib
>TimerLib
> +  PcdLib
> 
>  [Guids]
>gEfiEventExitBootServicesGuid ##
> SOMETIMES_CONSUMES ## Event
> @@ -64,6 +65,9 @@
>gEfiPciIoProtocolGuid ## TO_START
>gEfiUsb2HcProtocolGuid## BY_START
> 
> +[Pcd]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdDelayXhciHCReset  ##
> CONSUMES
> +
>  # [Event]
>  # EVENT_TYPE_PERIODIC_TIMER   ## CONSUMES
>  #
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> index 40f2f1f22766..525942a167b0 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> @@ -864,7 +864,7 @@ XhcResetHC (
>  // Otherwise there may have the timeout case happened.
>  // The below is a workaround to solve such problem.
>  //
> -gBS->Stall (XHC_1_MILLISECOND);
> +gBS->Stall (PcdGet16 (PcdDelayXhciHCReset));
>  Status = XhcWaitOpRegBit (Xhc, XHC_USBCMD_OFFSET, 
> XHC_USBCMD_RESET, FALSE, Timeout);
> 
>  if (!EFI_ERROR (Status)) {
> diff --git a/MdeModulePkg/MdeModulePkg.dec 
> b/MdeModulePkg/MdeModulePkg.dec index a91058e5b5df..d9e2e724df9e
> 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -1148,6 +1148,11 @@
># @Prompt Enable large address image loading.
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdImageLargeAddressLoad|TRUE|BOO
> LEAN|0x30001059
> 
> +  ## Indicates time delay for XHCI registers access after it issues
HCRST.
> +  # Default is 2000, it represent delay is 2 ms.
> +  # @Prompt Delay access XHCI register after it issues HCRST (us)
> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdDelayXhciHCReset|2000|UINT16|0x3
> 0001060
> +
>  [PcdsFixedAtBuild, PcdsPatchableInModule]
>## Dynamic type PCD can be registered callback function for Pcd 
> setting action.
>#  PcdMaxPeiPcdCallBackNumberPerPcdEntry indicates the maximum 
> number of callback function
> --
> 2.42.0.windows.2





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




Re: [edk2-devel] [PATCH V2 1/1] MdeModulePkg/XhciDxe: Reset endpoint while USB Transaction error

2024-04-25 Thread Xianglei Cai
Hi @gaoliming

Could you provide the eta when the patch can be merged?

Thanks,
Xianglei

-Original Message-
From: Cai, Xianglei 
Sent: Thursday, April 18, 2024 4:59 PM
To: gaoliming ; devel@edk2.groups.io
Cc: Ni, Ray ; Lewandowski, Krzysztof 
; Huang, Jenny ; Shih, 
More 
Subject: RE: [edk2-devel] [PATCH V2 1/1] MdeModulePkg/XhciDxe: Reset endpoint 
while USB Transaction error

Hi Liming,

Could you help pick the change to the master branch?

Thanks,
Xianglei

-Original Message-
From: gaoliming 
Sent: Monday, April 15, 2024 5:52 PM
To: devel@edk2.groups.io; Cai, Xianglei 
Cc: Ni, Ray ; Lewandowski, Krzysztof 
; Huang, Jenny ; Shih, 
More 
Subject: 回复: [edk2-devel] [PATCH V2 1/1] MdeModulePkg/XhciDxe: Reset endpoint 
while USB Transaction error

Reviewed-by: Liming Gao 

> -邮件原件-
> 发件人: devel@edk2.groups.io  代表 Xianglei Cai
> 发送时间: 2024年4月15日 14:55
> 收件人: devel@edk2.groups.io
> 抄送: Xianglei Cai ; Ray Ni ; 
> Liming Gao ; Krzysztof Lewandowski 
> ; Jenny Huang 
> ; More Shih 
> 主题: [edk2-devel] [PATCH V2 1/1] MdeModulePkg/XhciDxe: Reset endpoint 
> while USB Transaction error
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=4556
> 
> Based on XHCI spec 4.8.3, software should do the reset endpoint while 
> USB Transaction occur.
> Add the error code for USB Transaction error since UEFI spec don't 
> have the related definition.
> 
> Cc: Ray Ni  
> Cc: Liming Gao  
> Cc: Krzysztof Lewandowski   
> Cc: Jenny Huang 
> Cc: More Shih   
> Signed-off-by: Xianglei Cai 
> Reviewed-by: Krzysztof Lewandowski 
> ---
>  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c  | 5 -
>  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 5 - 
> MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h | 7 +++
>  3 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> index f4e61d223c1b..cf6b32959e68 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> @@ -825,7 +825,10 @@ XhcTransfer (
>*TransferResult = Urb->Result;
>*DataLength = Urb->Completed;
> 
> -  if ((*TransferResult == EFI_USB_ERR_STALL) || (*TransferResult ==
> EFI_USB_ERR_BABBLE)) {
> +  //
> +  // Based on XHCI spec 4.8.3, software should do the reset endpoint
while
> USB Transaction occur.
> +  //
> +  if ((*TransferResult == EFI_USB_ERR_STALL) || (*TransferResult ==
> EFI_USB_ERR_BABBLE) || (*TransferResult ==
> EDKII_USB_ERR_TRANSACTION)) {
>  ASSERT (Status == EFI_DEVICE_ERROR);
>  RecoveryStatus = XhcRecoverHaltedEndpoint (Xhc, Urb);
>  if (EFI_ERROR (RecoveryStatus)) { diff --git 
> a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> index 5d735008ba31..a97ed44dbfc3 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> @@ -1192,8 +1192,11 @@ XhcCheckUrbResult (
>  DEBUG ((DEBUG_ERROR, "XhcCheckUrbResult: ERR_BUFFER!
> Completecode = %x\n", EvtTrb->Completecode));
>  goto EXIT;
> 
> +  //
> +  // Based on XHCI spec 4.8.3, software should do the reset 
> + endpoint
> while USB Transaction occur.
> +  //
>case TRB_COMPLETION_USB_TRANSACTION_ERROR:
> -CheckedUrb->Result  |= EFI_USB_ERR_TIMEOUT;
> +CheckedUrb->Result  |= EDKII_USB_ERR_TRANSACTION;
>  CheckedUrb->Finished = TRUE;
>  DEBUG ((DEBUG_ERROR, "XhcCheckUrbResult:
> TRANSACTION_ERROR! Completecode = %x\n", EvtTrb->Completecode));
>  goto EXIT;
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h
> b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h
> index 7c85f7993b5c..e606e212a1d3 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h
> @@ -78,6 +78,13 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #define TRB_COMPLETION_STOPPED 26
>  #define TRB_COMPLETION_STOPPED_LENGTH_INVALID  27
> 
> +//
> +// USB Transfer Results Internal Definition // Based on XHCI spec 
> +4.8.3, software should do the reset endpoint while
> USB Transaction occur.
> +// Add the error code for USB Transaction error since UEFI spec don't
have
> the related definition.
> +//
> +#define EDKII_USB_ERR_TRANSACTION  0x200
> +
>  //
>  // The topology string used to present usb device location  //
> --
> 2.42.0.windows.2
> 
> 
> 
> 
> 





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




Re: [edk2-devel] [PATCH v2 3/7] OvmfPkg: Add the QemuFwCfgMmioLib PEI stage version

2024-04-25 Thread Chao Li

Hi Gerd,

I get it, I will refactor the code as soon as I can, it looks like 
there's still some work and will take some time. I will try to send the 
V3 tonight if possible.



Thanks,
Chao
On 2024/4/25 17:02, Gerd Hoffmann wrote:

On Thu, Apr 25, 2024 at 04:06:13PM +0800, Chao Li wrote:

Hi Gerd,


Thanks,
Chao
On 2024/4/25 15:53, Gerd Hoffmann wrote:

Hi,


+UINTN  mFwCfgSelectorAddress;
+UINTN  mFwCfgDataAddress;
+UINTN  mFwCfgDmaAddress;

Hmm, global variables for PEI?  I think the point of storing these in
the HOB is to avoid the need for global variables?  Also does that work
when running PEI in-place from flash?

I think it would be useful if some platforms(not LoongArch) could use the
global variables in PEI, because the global variables are faster.

Performance isn't my main concern here, I very much prefer code which is
easy to maintain.  Taking the same code path on all platforms is good
for that.  It's less code and it also makes testing easier.  The risk of
breaking loongarch when changing something for riscv or arm is much
lower if all platforms work the same way.

I'd suggest to first refactor the existing DXE code to use a HOB instead
of global variables.  Have a helper function which looks up the HOB and
returns a pointer to the configuration struct.  That helper function can
be slightly different for DXE/PEI, the DXE variant can cache the pointer
to the struct in a global variable so it needs to do the lookup only
once.


+  UINT64FwCfgDataSize;
+  UINT64FwCfgDmaAddress;
+  UINT64FwCfgDmaSize;

First thing this function should do is check whenever the HOB already
exists.  Should that be the case there is no need to parse the device
tree.

This is a constructor in PEI, that has to parse the device tree and then
build the HOBs.

This is a library, so it can be linked into multiple PEI and DXE
modules.  So it must be prepared to run multiple times.  On the
second and all following runs the HOB will already exist.

The DXE variant will need the check for sure.  I'd strongly suggest to
add it to the PEI variant too, even though it might not be needed right
now because PlatformPei is the only PEI module using the library.

take care,
   Gerd








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




Re: [edk2-devel] [PATCH v2 3/7] OvmfPkg: Add the QemuFwCfgMmioLib PEI stage version

2024-04-25 Thread Gerd Hoffmann
On Thu, Apr 25, 2024 at 04:06:13PM +0800, Chao Li wrote:
> Hi Gerd,
> 
> 
> Thanks,
> Chao
> On 2024/4/25 15:53, Gerd Hoffmann wrote:
> >Hi,
> > 
> > > +UINTN  mFwCfgSelectorAddress;
> > > +UINTN  mFwCfgDataAddress;
> > > +UINTN  mFwCfgDmaAddress;
> > Hmm, global variables for PEI?  I think the point of storing these in
> > the HOB is to avoid the need for global variables?  Also does that work
> > when running PEI in-place from flash?
> I think it would be useful if some platforms(not LoongArch) could use the
> global variables in PEI, because the global variables are faster.

Performance isn't my main concern here, I very much prefer code which is
easy to maintain.  Taking the same code path on all platforms is good
for that.  It's less code and it also makes testing easier.  The risk of
breaking loongarch when changing something for riscv or arm is much
lower if all platforms work the same way.

I'd suggest to first refactor the existing DXE code to use a HOB instead
of global variables.  Have a helper function which looks up the HOB and
returns a pointer to the configuration struct.  That helper function can
be slightly different for DXE/PEI, the DXE variant can cache the pointer
to the struct in a global variable so it needs to do the lookup only
once.

> > > +  UINT64FwCfgDataSize;
> > > +  UINT64FwCfgDmaAddress;
> > > +  UINT64FwCfgDmaSize;
> > First thing this function should do is check whenever the HOB already
> > exists.  Should that be the case there is no need to parse the device
> > tree.
> This is a constructor in PEI, that has to parse the device tree and then
> build the HOBs.

This is a library, so it can be linked into multiple PEI and DXE
modules.  So it must be prepared to run multiple times.  On the
second and all following runs the HOB will already exist.

The DXE variant will need the check for sure.  I'd strongly suggest to
add it to the PEI variant too, even though it might not be needed right
now because PlatformPei is the only PEI module using the library.

take care,
  Gerd



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




Re: [edk2-devel] [PATCH v2 2/7] OvmfPkg: Add the way of HOBs in QemuFwCfgLibMmio

2024-04-25 Thread Chao Li

Hi Ard and Gerd,


Thanks,
Chao
On 2024/4/25 16:11, Ard Biesheuvel wrote:

On Thu, 25 Apr 2024 at 10:10, Chao Li  wrote:

Hi Gerd,


Thanks,
Chao
On 2024/4/25 15:40, Gerd Hoffmann wrote:

   Hi,

+EFI_GUID  mFwCfgSelectorAddressGuid = FW_CONFIG_SELECTOR_ADDRESS_HOB_GUID;
+EFI_GUID  mFwCfgDataAddressGuid = FW_CONFIG_DATA_ADDRESS_HOB_GUID;
+EFI_GUID  mFwCfgDmaAddressGuid  = FW_CONFIG_DMA_ADDRESS_HOB_GUID;

Oh.  I assumed that would be obvious (because it's common practice for
HOBs), but I was thinking about a single HOB containing a struct with
all three values instead of a separate HOB for each value.

If uses a single HOB, it must define a structure in this library, and it may be 
more complicated to build and parse the HOB.


Please use a single HOB, and avoid global variables in PEI (PEI
modules may execute in place from read-only NOR flash, so global
variables are not allowed in PEI)

OK, I will adjust it now, and send the V3.








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




Re: [edk2-devel] [PATCH v2 2/7] OvmfPkg: Add the way of HOBs in QemuFwCfgLibMmio

2024-04-25 Thread Ard Biesheuvel
On Thu, 25 Apr 2024 at 10:10, Chao Li  wrote:
>
> Hi Gerd,
>
>
> Thanks,
> Chao
> On 2024/4/25 15:40, Gerd Hoffmann wrote:
>
>   Hi,
>
> +EFI_GUID  mFwCfgSelectorAddressGuid = FW_CONFIG_SELECTOR_ADDRESS_HOB_GUID;
> +EFI_GUID  mFwCfgDataAddressGuid = FW_CONFIG_DATA_ADDRESS_HOB_GUID;
> +EFI_GUID  mFwCfgDmaAddressGuid  = FW_CONFIG_DMA_ADDRESS_HOB_GUID;
>
> Oh.  I assumed that would be obvious (because it's common practice for
> HOBs), but I was thinking about a single HOB containing a struct with
> all three values instead of a separate HOB for each value.
>
> If uses a single HOB, it must define a structure in this library, and it may 
> be more complicated to build and parse the HOB.
>

Please use a single HOB, and avoid global variables in PEI (PEI
modules may execute in place from read-only NOR flash, so global
variables are not allowed in PEI)


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




Re: [edk2-devel] [PATCH v2 2/7] OvmfPkg: Add the way of HOBs in QemuFwCfgLibMmio

2024-04-25 Thread Chao Li

Hi Gerd,


Thanks,
Chao
On 2024/4/25 15:40, Gerd Hoffmann wrote:

   Hi,


+EFI_GUID  mFwCfgSelectorAddressGuid = FW_CONFIG_SELECTOR_ADDRESS_HOB_GUID;
+EFI_GUID  mFwCfgDataAddressGuid = FW_CONFIG_DATA_ADDRESS_HOB_GUID;
+EFI_GUID  mFwCfgDmaAddressGuid  = FW_CONFIG_DMA_ADDRESS_HOB_GUID;

Oh.  I assumed that would be obvious (because it's common practice for
HOBs), but I was thinking about a single HOB containing a struct with
all three values instead of a separate HOB for each value.
If uses a single HOB, it must define a structure in this library, and it 
may be more complicated to build and parse the HOB.


take care,
   Gerd








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




Re: [edk2-devel] [PATCH v2 3/7] OvmfPkg: Add the QemuFwCfgMmioLib PEI stage version

2024-04-25 Thread Chao Li

Hi Gerd,


Thanks,
Chao
On 2024/4/25 15:53, Gerd Hoffmann wrote:

   Hi,


+UINTN  mFwCfgSelectorAddress;
+UINTN  mFwCfgDataAddress;
+UINTN  mFwCfgDmaAddress;

Hmm, global variables for PEI?  I think the point of storing these in
the HOB is to avoid the need for global variables?  Also does that work
when running PEI in-place from flash?
I think it would be useful if some platforms(not LoongArch) could use 
the global variables in PEI, because the global variables are faster.



+RETURN_STATUS
+EFIAPI
+QemuFwCfgInitialize (
+  VOID
+  )
+{
+  VOID  *DeviceTreeBase;
+  INT32 Node;
+  INT32 Prev;
+  CONST CHAR8   *Type;
+  INT32 Len;
+  CONST UINT64  *Reg;
+  UINT64FwCfgSelectorAddress;
+  UINT64FwCfgSelectorSize;
+  UINT64FwCfgDataAddress;
+  UINT64FwCfgDataSize;
+  UINT64FwCfgDmaAddress;
+  UINT64FwCfgDmaSize;

First thing this function should do is check whenever the HOB already
exists.  Should that be the case there is no need to parse the device
tree.
This is a constructor in PEI, that has to parse the device tree and then 
build the HOBs.


take care,
   Gerd








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




Re: [edk2-devel] [PATCH v2 3/7] OvmfPkg: Add the QemuFwCfgMmioLib PEI stage version

2024-04-25 Thread Gerd Hoffmann
  Hi,

> +UINTN  mFwCfgSelectorAddress;
> +UINTN  mFwCfgDataAddress;
> +UINTN  mFwCfgDmaAddress;

Hmm, global variables for PEI?  I think the point of storing these in
the HOB is to avoid the need for global variables?  Also does that work
when running PEI in-place from flash?

> +RETURN_STATUS
> +EFIAPI
> +QemuFwCfgInitialize (
> +  VOID
> +  )
> +{
> +  VOID  *DeviceTreeBase;
> +  INT32 Node;
> +  INT32 Prev;
> +  CONST CHAR8   *Type;
> +  INT32 Len;
> +  CONST UINT64  *Reg;
> +  UINT64FwCfgSelectorAddress;
> +  UINT64FwCfgSelectorSize;
> +  UINT64FwCfgDataAddress;
> +  UINT64FwCfgDataSize;
> +  UINT64FwCfgDmaAddress;
> +  UINT64FwCfgDmaSize;

First thing this function should do is check whenever the HOB already
exists.  Should that be the case there is no need to parse the device
tree.

take care,
  Gerd



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




Re: [edk2-devel] [PATCH v3 08/13] OvmfPkg/PlatformInitLib: Create gEfiSmmSmramMemoryGuid

2024-04-25 Thread Wu, Jiaxin
> 
> Creating the EFI_SMM_SMRAM_MEMORY_GUID HOB should be moved to its
> own
> function.  Also move over the comments from SmmAccess describing the
> regions please.
> 
> Adding a reference to the PI spec section describing this would be good
> too.
> 

Got it, I will refine the patch to V4 based on your comments.

Thanks,
Jiaxin 


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




Re: [edk2-devel] [PATCH v2 2/7] OvmfPkg: Add the way of HOBs in QemuFwCfgLibMmio

2024-04-25 Thread Gerd Hoffmann
  Hi,

> +EFI_GUID  mFwCfgSelectorAddressGuid = FW_CONFIG_SELECTOR_ADDRESS_HOB_GUID;
> +EFI_GUID  mFwCfgDataAddressGuid = FW_CONFIG_DATA_ADDRESS_HOB_GUID;
> +EFI_GUID  mFwCfgDmaAddressGuid  = FW_CONFIG_DMA_ADDRESS_HOB_GUID;

Oh.  I assumed that would be obvious (because it's common practice for
HOBs), but I was thinking about a single HOB containing a struct with
all three values instead of a separate HOB for each value.

take care,
  Gerd



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




Re: [edk2-devel] [PATCH] OvmfPkg: Set PcdCpuMaxLogicalProcessorNumber in OvmfXen

2024-04-25 Thread Gerd Hoffmann
On Wed, Apr 24, 2024 at 02:36:32PM +0100, Alejandro Vallejo wrote:
> Bump the compile-time constant for maximum processor count from 64 to 128
> in order to allow that many vCPUs to be brought online on Xen guests with
> the default OVMF configuration.

> +  # UefiCpuPkg PCDs related to initial AP bringup and general AP management.
> +  gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|128

Note that this is a dynamic PCD, so you can set it at runtime to the
number of vcpus present in the VM.  See MaxCpuCountInitialization() in
OvmfPkg/PlatformPei/Platform.c for example.

take care,
  Gerd



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




Re: [edk2-devel] [PATCH v3 08/13] OvmfPkg/PlatformInitLib: Create gEfiSmmSmramMemoryGuid

2024-04-25 Thread Gerd Hoffmann
  Hi,

> Let me explain more why need this change:
> 
> 1. The EFI_SMM_SMRAM_MEMORY_GUID HOB, as defined in the PI specification, is 
> used to describe the SMRAM memory regions supported by the platform. This HOB 
> should be produced during the memory detection phase to align with the PI 
> spec.
> 
> 2. In addition to the memory reserved for ACPI S3 resume, an increasing 
> number of features require reserving SMRAM for specific purposes, such as 
> SmmRelocation. Other advanced features in Intel platforms also necessitate 
> this. The implementation of these features varies and is entirely dependent 
> on the platform. This is why an increasing number of platforms are adopting 
> the EFI_SMM_SMRAM_MEMORY_GUID HOB for SMRAM description.
> 
> 3. It is crucial that the SMRAM information remains consistent when retrieved 
> from the platform, whether through the SMM ACCESS PPI/Protocol or the 
> EFI_SMM_SMRAM_MEMORY_GUID HOB. Inconsistencies can lead to unexpected issues, 
> most commonly memory region conflicts.
> 
> 4. The SMM ACCESS PPI/Protocol can be naturally implemented for general use. 
> The common approach is to utilize the EFI_SMM_SMRAM_MEMORY_GUID HOB. For 
> reference, see the existing implementation in the EDK2 repository at 
> edk2/UefiPayloadPkg/SmmAccessDxe/SmmAccessDxe.inf and 
> edk2-platforms/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLib/PeiSmmAccessLib.inf.
>  
> 
> For the reasons mentioned, we are moving the SMRAM memory regions to HOBs and 
> allowing SMM access to consume these HOBs.
> 
> I will add the above info into commit message.

Thanks.

Creating the EFI_SMM_SMRAM_MEMORY_GUID HOB should be moved to its own
function.  Also move over the comments from SmmAccess describing the
regions please.

Adding a reference to the PI spec section describing this would be good
too.

> > Storing anything SMM related outside SMRAM makes me nervous.
> > I'd strongly suggest to avoid that.
> > 
> > It might be that in this specific case it is not a problem.  But it
> > needs very careful review of the implications (which I have not done)
> > and you have to hope you don't miss a possible attack vector, such as
> > someone modifying the HOB and the firmware then storing SMM data + code
> > outside SMRAM.
> 
> Understand, but here is the case we can record the info in non-smram
> since PI spec exposes that, there is no difference the info retrieved
> from PPI/ non-smm Protocol or the non-smram.

Good point.

take care,
  Gerd



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




Re: [edk2-devel] [PATCH v3 00/13] Add SmmRelocationLib

2024-04-25 Thread Gerd Hoffmann
  Hi,

> That means the SMMRevId is 0_xx64h for AMD64 processor. But I am not
> sure what the value is for AMD32 processor. Maybe 0 according to the
> OVMF logic.

The smm emulation in the linux kernel uses 0 and 0x64.

> But, I am very suspicious about the logic in AMD's version as below:
> --- AMD's version
>   SmmSaveStateRegisterLma = (UINT8)EFI_MM_SAVE_STATE_REGISTER_LMA_32BIT;
> 
>   LMAValue = (UINT32)AsmReadMsr64 (EFER_ADDRESS) & LMA;
>   if (LMAValue) {
> SmmSaveStateRegisterLma = (UINT8)EFI_MM_SAVE_STATE_REGISTER_LMA_64BIT;
>   }
> ---
> The above logic detects the current CPU mode and 64bit save state area layout 
> is used if it's running in 64bit.

> But if a AMD64 CPU runs in 32bit mode, the above logic causes the
> 32bit save state area layout is used. It's not right!  The save state
> area layout does not depend on the CPU running mode, but whether it's
> a legacy CPU or a 64-capable CPU.

Well, that is not entirely clear to me.  Could it be 64-bit processors
support both 32-bit and 64-bit format, for backward compatibility
reasons?

So OvmfPkgIa32 builds could use the 32-bit format, OvmfPkgX64 builds use
the 64-bit format, everything works fine?

The tricky corner case is OvmfPkgIa32X64, where (after applying this
series) 32-bit PEI should setup things for 64-bit SMM/DXE, and checking
the current processor mode will not give use the result we need.

> Jiaxin, I agree that the confusion should be cleaned up by AMD
> experts. Let's not change any existing behavior.

Agree.  Tom?

take care,
  Gerd



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




Re: [edk2-devel] [PATCH v2] Add SHA3/SM3 functions with openssl for Mbedtls

2024-04-25 Thread Li, Yi


Looks good to me.
Reviewed-by: Yi Li 

-Original Message-
From: Hou, Wenxing  
Sent: Wednesday, April 24, 2024 4:25 PM
To: devel@edk2.groups.io
Cc: Yao, Jiewen ; Li, Yi1 
Subject: [PATCH v2] Add SHA3/SM3 functions with openssl for Mbedtls

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



Because the Mbedlts 3.3.0 doesn't have SHA3 and Sm3, the SHA3 and Sm3

implementaion based on Openssl.

And the implementaion has passed build check.



Cc: Jiewen Yao 

Cc: Yi Li 

Signed-off-by: Wenxing Hou 

---

 CryptoPkg/Library/BaseCryptLibMbedTls/BaseCryptLib.inf   | 9 +++--

 CryptoPkg/Library/BaseCryptLibMbedTls/PeiCryptLib.inf| 9 +++--

 .../Library/BaseCryptLibMbedTls/RuntimeCryptLib.inf  | 3 ++-

 CryptoPkg/Library/BaseCryptLibMbedTls/SmmCryptLib.inf| 9 +++--

 .../Library/BaseCryptLibMbedTls/TestBaseCryptLib.inf | 4 +++-

 CryptoPkg/Library/MbedTlsLib/MbedTlsLib.inf  | 6 ++

 CryptoPkg/Library/MbedTlsLib/MbedTlsLibFull.inf  | 6 ++

 7 files changed, 38 insertions(+), 8 deletions(-)



diff --git a/CryptoPkg/Library/BaseCryptLibMbedTls/BaseCryptLib.inf 
b/CryptoPkg/Library/BaseCryptLibMbedTls/BaseCryptLib.inf

index 16def792c5..999054500f 100644

--- a/CryptoPkg/Library/BaseCryptLibMbedTls/BaseCryptLib.inf

+++ b/CryptoPkg/Library/BaseCryptLibMbedTls/BaseCryptLib.inf

@@ -18,6 +18,7 @@

   MODULE_TYPE= DXE_DRIVER

   VERSION_STRING = 1.0

   LIBRARY_CLASS  = BaseCryptLib|DXE_DRIVER DXE_CORE 
UEFI_APPLICATION UEFI_DRIVER

+  DEFINE BASE_CRYPT_PATH = ../BaseCryptLib

 

 #

 # The following information is for reference only and not required by the 
build tools.

@@ -31,10 +32,14 @@

   Cipher/CryptAes.c

   Hash/CryptSha256.c

   Hash/CryptSha512.c

-  Hash/CryptParallelHashNull.c

-  Hash/CryptSm3Null.c

   Hash/CryptMd5.c

   Hash/CryptSha1.c

+  $(BASE_CRYPT_PATH)/Hash/CryptCShake256.c

+  $(BASE_CRYPT_PATH)/Hash/CryptDispatchApDxe.c

+  $(BASE_CRYPT_PATH)/Hash/CryptParallelHash.c

+  $(BASE_CRYPT_PATH)/Hash/CryptSha3.c

+  $(BASE_CRYPT_PATH)/Hash/CryptSm3.c

+  $(BASE_CRYPT_PATH)/Hash/CryptXkcp.c

   Hmac/CryptHmac.c

   Kdf/CryptHkdf.c

   Pk/CryptRsaBasic.c

diff --git a/CryptoPkg/Library/BaseCryptLibMbedTls/PeiCryptLib.inf 
b/CryptoPkg/Library/BaseCryptLibMbedTls/PeiCryptLib.inf

index 72b22a24e8..a153c0c8e4 100644

--- a/CryptoPkg/Library/BaseCryptLibMbedTls/PeiCryptLib.inf

+++ b/CryptoPkg/Library/BaseCryptLibMbedTls/PeiCryptLib.inf

@@ -26,6 +26,7 @@

   MODULE_TYPE= PEIM

   VERSION_STRING = 1.0

   LIBRARY_CLASS  = BaseCryptLib|PEIM PEI_CORE

+  DEFINE BASE_CRYPT_PATH = ../BaseCryptLib

 

 #

 # The following information is for reference only and not required by the 
build tools.

@@ -38,9 +39,13 @@

   Hash/CryptMd5.c

   Hash/CryptSha1.c

   Hash/CryptSha256.c

-  Hash/CryptSm3Null.c

   Hash/CryptSha512.c

-  Hash/CryptParallelHashNull.c

+  $(BASE_CRYPT_PATH)/Hash/CryptCShake256.c

+  $(BASE_CRYPT_PATH)/Hash/CryptDispatchApPei.c

+  $(BASE_CRYPT_PATH)/Hash/CryptParallelHash.c

+  $(BASE_CRYPT_PATH)/Hash/CryptSha3.c

+  $(BASE_CRYPT_PATH)/Hash/CryptSm3.c

+  $(BASE_CRYPT_PATH)/Hash/CryptXkcp.c

   Hmac/CryptHmac.c

   Kdf/CryptHkdf.c

   Cipher/CryptAes.c

diff --git a/CryptoPkg/Library/BaseCryptLibMbedTls/RuntimeCryptLib.inf 
b/CryptoPkg/Library/BaseCryptLibMbedTls/RuntimeCryptLib.inf

index 9f17ef00bf..1b33dbdaad 100644

--- a/CryptoPkg/Library/BaseCryptLibMbedTls/RuntimeCryptLib.inf

+++ b/CryptoPkg/Library/BaseCryptLibMbedTls/RuntimeCryptLib.inf

@@ -25,6 +25,7 @@

   VERSION_STRING = 1.0

   LIBRARY_CLASS  = BaseCryptLib|DXE_RUNTIME_DRIVER

   CONSTRUCTOR= RuntimeCryptLibConstructor

+  DEFINE BASE_CRYPT_PATH = ../BaseCryptLib

 

 #

 # The following information is for reference only and not required by the 
build tools.

@@ -37,9 +38,9 @@

   Hash/CryptMd5.c

   Hash/CryptSha1.c

   Hash/CryptSha256.c

-  Hash/CryptSm3Null.c

   Hash/CryptSha512.c

   Hash/CryptParallelHashNull.c

+  $(BASE_CRYPT_PATH)/Hash/CryptSm3.c

   Hmac/CryptHmac.c

   Kdf/CryptHkdf.c

   Cipher/CryptAes.c

diff --git a/CryptoPkg/Library/BaseCryptLibMbedTls/SmmCryptLib.inf 
b/CryptoPkg/Library/BaseCryptLibMbedTls/SmmCryptLib.inf

index 40c56d1b7d..d9a9cb8d10 100644

--- a/CryptoPkg/Library/BaseCryptLibMbedTls/SmmCryptLib.inf

+++ b/CryptoPkg/Library/BaseCryptLibMbedTls/SmmCryptLib.inf

@@ -24,6 +24,7 @@

   VERSION_STRING = 1.0

   PI_SPECIFICATION_VERSION   = 0x0001000A

   LIBRARY_CLASS  = BaseCryptLib|DXE_SMM_DRIVER SMM_CORE 
MM_STANDALONE

+  DEFINE BASE_CRYPT_PATH = ../BaseCryptLib

 

 #

 # The following information is for reference only and not required by the 
build tools.

@@ -36,9 +37,13 @@

   Hash/CryptMd5.c

   Hash/CryptSha1.c

   Hash/CryptSha256.c

-  Hash/CryptSm3Null.c

   Hash/CryptSha512.c

-