Re: [edk2-devel] [PATCH 1/1] MdePkg: Update the Label definitions of the EFI_NVDIMM_LABEL

2024-01-10 Thread Junfeng Guan
Hi Liming,
   Thanks! I created a PR: https://github.com/tianocore/edk2/pull/5191
   Could you help push it?

B.R.
Junfeng
-Original Message-
From: gaoliming  
Sent: Thursday, January 11, 2024 10:19 AM
To: devel@edk2.groups.io; Guan, JunfengX 
Cc: Kinney, Michael D ; Liu, Zhiguang 
; Li, Yi1 
Subject: 回复: [edk2-devel] [PATCH 1/1] MdePkg: Update the Label definitions of 
the EFI_NVDIMM_LABEL

Reviewed-by: Liming Gao 

> -邮件原件-
> 发件人: devel@edk2.groups.io  代表 Junfeng Guan
> 发送时间: 2023年12月27日 9:57
> 收件人: devel@edk2.groups.io
> 抄送: Michael D Kinney ; Liming Gao 
> ; Zhiguang Liu ; Yi 
> Li 
> 主题: [edk2-devel] [PATCH 1/1] MdePkg: Update the Label definitions of 
> the EFI_NVDIMM_LABEL
> 
> Refer to Uefi spec 2.10 section 13.19.5, update the label definitions 
> for NVDIMM SPA location cookie.
> 
> Signed-off-by: Junfeng Guan 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Zhiguang Liu 
> Cc: Yi Li 
> ---
>  MdePkg/Include/Protocol/NvdimmLabel.h | 16 +++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/MdePkg/Include/Protocol/NvdimmLabel.h
> b/MdePkg/Include/Protocol/NvdimmLabel.h
> index e46999a3ab4a..2352c9bd1652 100644
> --- a/MdePkg/Include/Protocol/NvdimmLabel.h
> +++ b/MdePkg/Include/Protocol/NvdimmLabel.h
> @@ -122,6 +122,12 @@ typedef struct {
>  ///
>  #define EFI_NVDIMM_LABEL_FLAGS_UPDATING  0x0008
> 
> +///
> +/// When set, the SPALocationCookie in the namespace label is valid 
> +and
> should match the
> +/// current value in the NFIT SPA Range Structure.
> +///
> +#define EFI_NVDIMM_LABEL_FLAGS_SPACOOKIE_BOUND  0x0010
> +
>  typedef struct {
>///
>/// Unique Label Identifier UUID per RFC 4122.
> @@ -196,10 +202,18 @@ typedef struct {
>///
>EFI_GUIDAddressAbstractionGuid;
> 
> +  ///
> +  /// When creating the label, this value is set to the value from 
> + the
NFIT
> SPA Range Structure if the
> +  /// SPALocationCookie flag (bit 2) is set. If
> EFI_NVDIMM_LABEL_FLAGS_SPACOOKIE_BOUND is set, the SPALocationCookie
> +  /// value stored in the namespace label should match the current 
> + value
in
> the NFIT SPA Range Structure.
> +  /// Otherwise, the data may not be read correctly.
> +  ///
> +  UINT64  SPALocationCookie;
> +
>///
>/// Shall be 0.
>///
> -  UINT8   Reserved1[88];
> +  UINT8   Reserved1[80];
> 
>///
>/// 64-bit Fletcher64 checksum of all fields in this Label.
> --
> 2.26.2.windows.1
> 
> 
> 
> 
> 





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




Re: [edk2-devel] [PATCH 5/5] Platform/RaspberryPi: Update PCIe MMIO window for DT

2024-01-10 Thread Ard Biesheuvel
On Thu, 11 Jan 2024 at 00:52, Jeremy Linton  wrote:
>
> Since we are updating the DT memory map and telling it how
> we have configured the PCIe, there isn't a reason for moving the
> MMIO window. In fact this appears to fix OpenBSD+DT as well as
> it makes the linux XHCI reset sequence happier.
>
> Signed-off-by: Jeremy Linton 
> ---
>  Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c | 24 
>  1 file changed, 24 insertions(+)
>
> diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c 
> b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> index dd4fc0a05e..e6942df6a3 100644
> --- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> +++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> @@ -375,6 +375,30 @@ SyncPcie (
>  return EFI_NOT_FOUND;
>}
>
> +  // move the MMIO window too
> +  DmaRanges[0] = cpu_to_fdt32 (0x0200); //non prefech 32-bit
> +  DmaRanges[1] = cpu_to_fdt32 (0x); //bus addr @ 0x0f800
> +  DmaRanges[2] = cpu_to_fdt32 (0xf800);
> +  DmaRanges[3] = cpu_to_fdt32 (0x0006); //cpu addr @ 0x6
> +  DmaRanges[4] = cpu_to_fdt32 (0x);
> +  DmaRanges[5] = cpu_to_fdt32 (0x);
> +  DmaRanges[6] = cpu_to_fdt32 (0x0400); // len = 0x4000 
> +

Is there any way we can use symbolic constants here?

> +  DEBUG ((DEBUG_INFO, "%a: Updating PCIe ranges\n",  __func__));
> +
> +  /*
> +   * Match dma-ranges with the EDK2+ACPI setup we are using.  This works
> +   * around a failure in Linux and OpenBSD to reset the PCIe/XHCI correctly
> +   * when in DT mode.
> +   */

Please sync the comment with the code.

> +  Retval = fdt_setprop (mFdtImage, Node, "ranges",
> +DmaRanges,  sizeof DmaRanges);
> +  if (Retval != 0) {
> +DEBUG ((DEBUG_ERROR, "%a: failed to locate PCIe MMIO 'ranges' property 
> (%d)\n",
> +  __func__, Retval));
> +return EFI_NOT_FOUND;
> +  }
> +
>if (PcdGet32 (PcdXhciReload) != 1) {
>  return EFI_SUCCESS;
>}
> --
> 2.43.0
>


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




Re: [edk2-devel] [PATCH 4/5] Platform/RaspberryPi: Give the user control over the XHCI mailbox

2024-01-10 Thread Ard Biesheuvel
On Thu, 11 Jan 2024 at 00:52, Jeremy Linton  wrote:
>
> Its a complete tossup whether removing the mailbox call after we have
> set up the XHCI works for a given kernel+distro in DT mode. So lets
> give users which want to try DT the option of flipping this on/off.
>
> Users that don't want to have to deal with DT, can use ACPI.
>
> Signed-off-by: Jeremy Linton 
> ---
>  .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 10 ++
>  .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf   |  1 +
>  .../Drivers/ConfigDxe/ConfigDxeHii.uni|  5 +
>  .../Drivers/ConfigDxe/ConfigDxeHii.vfr| 15 +++
>  Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c  |  4 
>  Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf|  1 +
>  Platform/RaspberryPi/RPi3/RPi3.dsc|  6 ++
>  Platform/RaspberryPi/RPi4/RPi4.dsc|  7 +++
>  Platform/RaspberryPi/RaspberryPi.dec  |  1 +
>  9 files changed, 50 insertions(+)
>

This looks ok to me but it doesn't appear to apply on today's edk2-platforms.

> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c 
> b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> index 3dcf2bac0d..2484787982 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> @@ -298,6 +298,16 @@ SetupVariables (
>  Status = PcdSet32S (PcdXhciPci, 1);
>  ASSERT_EFI_ERROR (Status);
>}
> +
> +  Size = sizeof (UINT32);
> +  Status = gRT->GetVariable (L"XhciReload",
> + ,
> + NULL, , );
> +  if (EFI_ERROR (Status)) {
> +Status = PcdSet32S (PcdXhciReload, PcdGet32 (PcdXhciReload));
> +ASSERT_EFI_ERROR (Status);
> +  }
> +
>  }
>} else {
>  /*
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf 
> b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> index 6f6e8f42ac..475e645537 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> @@ -96,6 +96,7 @@
>gRaspberryPiTokenSpaceGuid.PcdUartInUse
>gRaspberryPiTokenSpaceGuid.PcdXhciPci
>gRaspberryPiTokenSpaceGuid.PcdMiniUartClockRate
> +  gRaspberryPiTokenSpaceGuid.PcdXhciReload
>
>  [Depex]
>gPcdProtocolGuid AND gRaspberryPiFirmwareProtocolGuid
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni 
> b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> index 5ec17072c3..8130638876 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> @@ -62,6 +62,11 @@
>  #string STR_ADVANCED_XHCIPCI_XHCI #language en-US "XHCI"
>  #string STR_ADVANCED_XHCIPCI_PCIE #language en-US "PCIe"
>
> +#string STR_ADVANCED_XHCIRELOAD_PROMPT  #language en-US "DT Reload XHCI 
> firmware"
> +#string STR_ADVANCED_XHCIRELOAD_HELP#language en-US "OS should reload 
> XHCI firmware on reset"
> +#string STR_ADVANCED_XHCIRELOAD_DISABLE #language en-US "Disabled"
> +#string STR_ADVANCED_XHCIRELOAD_RELOAD  #language en-US "Reload"
> +
>  #string STR_ADVANCED_ASSET_TAG_PROMPT #language en-US "Asset Tag"
>  #string STR_ADVANCED_ASSET_TAG_HELP   #language en-US "Set the system Asset 
> Tag"
>
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr 
> b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> index f668b7a553..f13b70711d 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> @@ -61,6 +61,11 @@ formset
>name  = XhciPci,
>guid  = CONFIGDXE_FORM_SET_GUID;
>
> +efivarstore ADVANCED_XHCIPCI_VARSTORE_DATA,
> +  attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | 
> EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
> +  name  = XhciReload,
> +  guid  = CONFIGDXE_FORM_SET_GUID;
> +
>  efivarstore SYSTEM_TABLE_MODE_VARSTORE_DATA,
>attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | 
> EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
>name  = SystemTableMode,
> @@ -228,6 +233,16 @@ formset
>option text = STRING_TOKEN(STR_ADVANCED_XHCIPCI_PCIE), value = 
> 1, flags = 0;
>  endoneof;
>endif;
> +
> +  grayoutif ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_ACPI;
> +oneof varid = XhciReload.Value,
> +  prompt  = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_PROMPT),
> +  help= STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_HELP),
> +  flags   = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
> +  option text = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_DISABLE), 
> value = 0, flags = DEFAULT;
> +  option text = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_RELOAD), 
> value = 1, flags = 0;
> +endoneof;
> +  endif;
>  

Re: [edk2-devel] [PATCH 1/5] Platform/RaspberryPi/DualSerialPortLib: Always configure the pl011

2024-01-10 Thread Ard Biesheuvel
Hi Jeremy,

Thanks for the patches.


On Thu, 11 Jan 2024 at 00:52, Jeremy Linton  wrote:
>
> The rpi's config.txt controls which uart (pl011, or miniuart) is
> selected as the console. TFA and edk2 follow its lead, but if the
> miniuart is selected as the primary and the machine is booted in ACPI
> mode the baud/etc is never configured for the pl011. The linux kernel
> won't reconfigure it either as its listed as a "SBSA" uart, so it
> simply won't work.
>
> This re-enables BT on the pl011 in ACPI mode, and it somewhat starts
> to work again.
>
> Signed-off-by: Jeremy Linton 
> ---
>  .../DualSerialPortLib/DualSerialPortLib.c | 37 +++
>  1 file changed, 22 insertions(+), 15 deletions(-)
>
> diff --git 
> a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c 
> b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
> index d2f983bf0a..79545d93d6 100644
> --- a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
> +++ b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
> @@ -76,6 +76,8 @@ SerialPortInitialize (
>EFI_PARITY_TYPE Parity;
>UINT8   DataBits;
>EFI_STOP_BITS_TYPE  StopBits;
> +  RETURN_STATUS   Ret;
> +  UINTN   Timeout;

What is this for?


>
>//
>// First thing we need to do is determine which of PL011 or miniUART is 
> selected
> @@ -85,23 +87,27 @@ SerialPortInitialize (
>  UsePl011UartSet = TRUE;
>}
>
> -  if (UsePl011Uart) {
> -BaudRate = FixedPcdGet64 (PcdUartDefaultBaudRate);
> +  // always init the pl011 on the pi4, linux expects a SBSA uart to be at 
> 115200
> +  // this means we need to set the baud/etc even if we arn't using it as a 
> console
> +  if ((UsePl011Uart) || (RPI_MODEL == 4)) {
>  ReceiveFifoDepth = 0; // Use default FIFO depth
> +BaudRate = FixedPcdGet64 (PcdUartDefaultBaudRate);

Shouldn't we hardcode 115200 here if !UsePl011Uart?

>  Parity = (EFI_PARITY_TYPE)FixedPcdGet8 (PcdUartDefaultParity);
>  DataBits = FixedPcdGet8 (PcdUartDefaultDataBits);
>  StopBits = (EFI_STOP_BITS_TYPE) FixedPcdGet8 (PcdUartDefaultStopBits);
>
> -return PL011UartInitializePort (
> - PL011_UART_REGISTER_BASE,
> - PL011UartClockGetFreq(),
> - ,
> - ,
> - ,
> - ,
> - 
> - );
> -  } else {
> +Ret = PL011UartInitializePort (
> +   PL011_UART_REGISTER_BASE,
> +   PL011UartClockGetFreq(),
> +   ,
> +   ,
> +   ,
> +   ,
> +   
> +   );
> +  }
> +
> +  if (!UsePl011Uart) {
>  SerialRegisterBase = MINI_UART_REGISTER_BASE;
>  Divisor = SerialPortGetDivisor (PcdGet32 (PcdSerialBaudRate));
>
> @@ -127,7 +133,8 @@ SerialPortInitialize (
>  // Wait for the serial port to be ready.
>  // Verify that both the transmit FIFO and the shift register are empty.
>  //
> -while ((SerialPortReadRegister (SerialRegisterBase, R_UART_LSR) & 
> (B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) != (B_UART_LSR_TEMT | 
> B_UART_LSR_TXRDY));
> +Timeout = 1000;
> +while (((SerialPortReadRegister (SerialRegisterBase, R_UART_LSR) & 
> (B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) != (B_UART_LSR_TEMT | 
> B_UART_LSR_TXRDY)) && (Timeout--));
>
>  //
>  // Configure baud rate
> @@ -158,9 +165,9 @@ SerialPortInitialize (
>  // Put Modem Control Register(MCR) into its reset state of 0x00.
>  //
>  SerialPortWriteRegister (SerialRegisterBase, R_UART_MCR, 0x00);
> -
> -return RETURN_SUCCESS;
> +Ret = RETURN_SUCCESS;
>}
> +  return Ret;
>  }
>
>  /**
> --
> 2.43.0
>


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




[edk2-devel] [PATCH] MdePkg: Update ReceiveData and SendData function description

2024-01-10 Thread Tan, Ming
From: Qingyu Shang 

Refer to Uefi spec 2.10 section 13.14, update the parameter 'MediaId'
description for EFI_STORAGE_SECURITY_COMMAND_PROTOCOL function ReceiveData
and SendData.

Signed-off-by: Qingyu Shang 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
---
 MdePkg/Include/Protocol/StorageSecurityCommand.h | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/Protocol/StorageSecurityCommand.h 
b/MdePkg/Include/Protocol/StorageSecurityCommand.h
index 810af59b85..206ae79aff 100644
--- a/MdePkg/Include/Protocol/StorageSecurityCommand.h
+++ b/MdePkg/Include/Protocol/StorageSecurityCommand.h
@@ -59,7 +59,9 @@ typedef struct _EFI_STORAGE_SECURITY_COMMAND_PROTOCOL 
EFI_STORAGE_SECURITY_COMMA
   function shall return EFI_DEVICE_ERROR.
 
   @param  This Indicates a pointer to the calling 
context.
-  @param  MediaId  ID of the medium to receive data from.
+  @param  MediaId  ID of the medium to receive data from. 
If there is no
+   block IO protocol supported by the 
physical device, the
+   value of MediaId is undefined.
   @param  Timeout  The timeout, in 100ns units, to use for 
the execution
of the security protocol command. A 
Timeout value of 0
means that this function will wait 
indefinitely for the
@@ -138,7 +140,9 @@ EFI_STATUS
   shall return EFI_DEVICE_ERROR.
 
   @param  This Indicates a pointer to the calling 
context.
-  @param  MediaId  ID of the medium to receive data from.
+  @param  MediaId  ID of the medium to receive data from. 
If there is no
+   block IO protocol supported by the 
physical device, the
+   value of MediaId is undefined.
   @param  Timeout  The timeout, in 100ns units, to use for 
the execution
of the security protocol command. A 
Timeout value of 0
means that this function will wait 
indefinitely for the
-- 
2.39.1.windows.1



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




Re: [edk2-devel] [PATCH v1 2/4] StandaloneMmPkg/Hob: Integer Overflow in CreateHob()

2024-01-10 Thread Ard Biesheuvel
On Thu, 11 Jan 2024 at 07:52, Ard Biesheuvel  wrote:
>
> On Thu, 11 Jan 2024 at 06:15,  wrote:
> >
> > From: Gerd Hoffmann 
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4166
> >
> > Fix integer overflow in various CreateHob instances.
> > Fixes: CVE-2022-36765
> >
> > The CreateHob() function aligns the requested size to 8
> > performing the following operation:
> > ```
> > HobLength = (UINT16)((HobLength + 0x7) & (~0x7));
> > ```
> >
> > No checks are performed to ensure this value doesn't
> > overflow, and could lead to CreateHob() returning a smaller
> > HOB than requested, which could lead to OOB HOB accesses.
> >
> > Reported-by: Marc Beatove 
> > Cc: Ard Biesheuvel 
> > Cc: Sami Mujawar 
> > Cc: Ray Ni 
> > Cc: John Mathew 
> > Signed-off-by: Gerd Hoffmann 
>
> Reviewed-by: Ard Biesheuvel 
>

Same as the other patch: this needs a signoff from the sender, not the
author of the patch.

> > ---
> >  .../StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c | 6 ++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git 
> > a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c
> >  
> > b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c
> > index 1550e1babc..29ade2e4ef 100644
> > --- 
> > a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c
> > +++ 
> > b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c
> > @@ -34,6 +34,12 @@ CreateHob (
> >
> >HandOffHob = GetHobList ();
> >
> > +  //
> > +  // Check Length to avoid data overflow.
> > +  //
> > +  if (HobLength > MAX_UINT16 - 0x7) {
> > +return NULL;
> > +  }
> >HobLength = (UINT16)((HobLength + 0x7) & (~0x7));
> >
> >FreeMemory = HandOffHob->EfiFreeMemoryTop - 
> > HandOffHob->EfiFreeMemoryBottom;
> > --
> > 2.39.2.windows.1
> >


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




[edk2-devel] [PATCH 1/2] CloudHv: Add CloudHv support to PlatformScanE820 utility function.

2024-01-10 Thread Thomas Barrett
Signed-off-by: Thomas Barrett 
---
 OvmfPkg/Library/PlatformInitLib/MemDetect.c | 95 ++---
 1 file changed, 65 insertions(+), 30 deletions(-)

diff --git a/OvmfPkg/Library/PlatformInitLib/MemDetect.c 
b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
index 662e7e85bb..76a9dc9211 100644
--- a/OvmfPkg/Library/PlatformInitLib/MemDetect.c
+++ b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
@@ -248,6 +248,67 @@ PlatformReservationConflictCB (
   PlatformInfoHob->PcdPciMmio64Base = NewBase;

 }

 

+/**

+  Returns PVH memmap

+  @param Entries  Pointer to PVH memmap

+  @param CountNumber of entries

+  @return EFI_STATUS

+**/

+EFI_STATUS

+GetPvhMemmapEntries (

+  struct hvm_memmap_table_entry  **Entries,

+  UINT32 *Count

+  )

+{

+  UINT32 *PVHResetVectorData;

+  struct hvm_start_info  *pvh_start_info;

+

+  PVHResetVectorData = (VOID *)(UINTN)PcdGet32 (PcdXenPvhStartOfDayStructPtr);

+  if (PVHResetVectorData == 0) {

+return EFI_NOT_FOUND;

+  }

+

+  pvh_start_info = (struct hvm_start_info *)(UINTN)PVHResetVectorData[0];

+

+  *Entries = (struct hvm_memmap_table_entry 
*)(UINTN)pvh_start_info->memmap_paddr;

+  *Count   = pvh_start_info->memmap_entries;

+

+  return EFI_SUCCESS;

+}

+

+STATIC

+EFI_STATUS

+PlatformScanE820Pvh (

+  IN  E820_SCAN_CALLBACK Callback,

+  IN OUT  EFI_HOB_PLATFORM_INFO  *PlatformInfoHob

+  )

+{

+  struct hvm_memmap_table_entry  *Memmap;

+  UINT32 MemmapEntriesCount;

+  struct hvm_memmap_table_entry  *Entry;

+  EFI_E820_ENTRY64   E820Entry;

+  EFI_STATUS Status;

+  UINT32 Loop;

+

+  Status = GetPvhMemmapEntries (, );

+  if (EFI_ERROR (Status)) {

+return Status;

+  }

+

+  for (Loop = 0; Loop < MemmapEntriesCount; Loop++) {

+Entry = Memmap + Loop;

+

+if (Entry->type == XEN_HVM_MEMMAP_TYPE_RAM) {

+  E820Entry.BaseAddr = Entry->addr;

+  E820Entry.Length   = Entry->size;

+  E820Entry.Type = Entry->type;

+  Callback (, PlatformInfoHob);

+}

+  }

+

+  return EFI_SUCCESS;

+}

+

 /**

   Iterate over the entries in QEMU's fw_cfg E820 RAM map, call the

   passed callback for each entry.

@@ -279,6 +340,10 @@ PlatformScanE820 (
   EFI_E820_ENTRY64  E820Entry;

   UINTN Processed;

 

+  if (PlatformInfoHob->HostBridgeDevId == CLOUDHV_DEVICE_ID) {

+return PlatformScanE820Pvh (Callback, PlatformInfoHob);

+  }

+

   Status = QemuFwCfgFindFile ("etc/e820", , );

   if (EFI_ERROR (Status)) {

 return Status;

@@ -297,36 +362,6 @@ PlatformScanE820 (
   return EFI_SUCCESS;

 }

 

-/**

-  Returns PVH memmap

-

-  @param Entries  Pointer to PVH memmap

-  @param CountNumber of entries

-

-  @return EFI_STATUS

-**/

-EFI_STATUS

-GetPvhMemmapEntries (

-  struct hvm_memmap_table_entry  **Entries,

-  UINT32 *Count

-  )

-{

-  UINT32 *PVHResetVectorData;

-  struct hvm_start_info  *pvh_start_info;

-

-  PVHResetVectorData = (VOID *)(UINTN)PcdGet32 (PcdXenPvhStartOfDayStructPtr);

-  if (PVHResetVectorData == 0) {

-return EFI_NOT_FOUND;

-  }

-

-  pvh_start_info = (struct hvm_start_info *)(UINTN)PVHResetVectorData[0];

-

-  *Entries = (struct hvm_memmap_table_entry 
*)(UINTN)pvh_start_info->memmap_paddr;

-  *Count   = pvh_start_info->memmap_entries;

-

-  return EFI_SUCCESS;

-}

-

 STATIC

 UINT64

 GetHighestSystemMemoryAddressFromPvhMemmap (

-- 
2.34.1

Disclaimer

The information contained in this communication from the sender is 
confidential. It is intended solely for use by the recipient and others 
authorized to receive it. If you are not the recipient, you are hereby notified 
that any disclosure, copying, distribution or taking action in relation of the 
contents of this information is strictly prohibited and may be unlawful.

This email has been scanned for viruses and malware, and may have been 
automatically archived by Mimecast, a leader in email security and cyber 
resilience. Mimecast integrates email defenses with brand protection, security 
awareness training, web security, compliance and other essential capabilities. 
Mimecast helps protect large and small organizations from malicious activity, 
human error and technology failure; and to lead the movement toward building a 
more resilient world. To find out more, visit our website.


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




[edk2-devel] [PATCH 2/2] CloudHv: Update PlatformAddressWidthInitialization logic

2024-01-10 Thread Thomas Barrett
When booting CloudHv guests with greater than 1TiB of high
memory, the PlatformAddressWidthInitialization function
incorrect calculates the address width using the overflowed
24-bit CMOS register rather than the E820 entries.

Signed-off-by: Thomas Barrett 
---
 OvmfPkg/CloudHv/CloudHvX64.dsc  |  2 ++
 OvmfPkg/Library/PlatformInitLib/MemDetect.c | 12 
 2 files changed, 14 insertions(+)

diff --git a/OvmfPkg/CloudHv/CloudHvX64.dsc b/OvmfPkg/CloudHv/CloudHvX64.dsc
index af594959a9..b522fa1059 100644
--- a/OvmfPkg/CloudHv/CloudHvX64.dsc
+++ b/OvmfPkg/CloudHv/CloudHvX64.dsc
@@ -566,6 +566,8 @@
   # Point to the MdeModulePkg/Application/UiApp/UiApp.inf

   gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 
0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }

 

+  gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable|TRUE

+

 


 #

 # Pcd Dynamic Section - list of all EDK II PCD Entries defined by this Platform

diff --git a/OvmfPkg/Library/PlatformInitLib/MemDetect.c 
b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
index 76a9dc9211..f042517bb6 100644
--- a/OvmfPkg/Library/PlatformInitLib/MemDetect.c
+++ b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
@@ -873,6 +873,18 @@ PlatformAddressWidthInitialization (
 

   if (PlatformInfoHob->HostBridgeDevId == 0x /* microvm */) {

 PlatformAddressWidthFromCpuid (PlatformInfoHob, FALSE);

+return;

+  } else if (PlatformInfoHob->HostBridgeDevId == CLOUDHV_DEVICE_ID) {

+PlatformInfoHob->FirstNonAddress = BASE_4GB;

+Status   = PlatformScanE820 
(PlatformGetFirstNonAddressCB, PlatformInfoHob);

+if (EFI_ERROR (Status)) {

+  PlatformInfoHob->FirstNonAddress = BASE_4GB + 
PlatformGetSystemMemorySizeAbove4gb ();

+}

+

+PlatformInfoHob->PcdPciMmio64Base = PlatformInfoHob->FirstNonAddress;

+PlatformAddressWidthFromCpuid (PlatformInfoHob, FALSE);

+PlatformInfoHob->PcdPciMmio64Size = PlatformInfoHob->FirstNonAddress - 
PlatformInfoHob->PcdPciMmio64Base;

+

 return;

   }

 

-- 
2.34.1

Disclaimer

The information contained in this communication from the sender is 
confidential. It is intended solely for use by the recipient and others 
authorized to receive it. If you are not the recipient, you are hereby notified 
that any disclosure, copying, distribution or taking action in relation of the 
contents of this information is strictly prohibited and may be unlawful.

This email has been scanned for viruses and malware, and may have been 
automatically archived by Mimecast, a leader in email security and cyber 
resilience. Mimecast integrates email defenses with brand protection, security 
awareness training, web security, compliance and other essential capabilities. 
Mimecast helps protect large and small organizations from malicious activity, 
human error and technology failure; and to lead the movement toward building a 
more resilient world. To find out more, visit our website.


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




Re: [edk2-devel] [PATCH v1 3/4] EmbeddedPkg/Hob: Integer Overflow in CreateHob()

2024-01-10 Thread Ard Biesheuvel
On Thu, 11 Jan 2024 at 06:15,  wrote:
>
> From: Gerd Hoffmann 
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4166
>
> Fix integer overflow in various CreateHob instances.
> Fixes: CVE-2022-36765
>
> The CreateHob() function aligns the requested size to 8
> performing the following operation:
> ```
> HobLength = (UINT16)((HobLength + 0x7) & (~0x7));
> ```
>
> No checks are performed to ensure this value doesn't
> overflow, and could lead to CreateHob() returning a smaller
> HOB than requested, which could lead to OOB HOB accesses.
>
> Reported-by: Marc Beatove 
> Cc: Leif Lindholm 
> Cc: Ard Biesheuvel 
> Cc: Abner Chang 
> Cc: John Mathew 
> Signed-off-by: Gerd Hoffmann 

This is missing a signed-off line from the sender.

(the signoff is not a statement of authorship, it is a promise by the
sender that the contribution complies with the license)

With that fixed:

Reviewed-by: Ard Biesheuvel 


> ---
>  EmbeddedPkg/Library/PrePiHobLib/Hob.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/EmbeddedPkg/Library/PrePiHobLib/Hob.c 
> b/EmbeddedPkg/Library/PrePiHobLib/Hob.c
> index 8eb175aa96..ee2e3176be 100644
> --- a/EmbeddedPkg/Library/PrePiHobLib/Hob.c
> +++ b/EmbeddedPkg/Library/PrePiHobLib/Hob.c
> @@ -110,6 +110,12 @@ CreateHob (
>
>HandOffHob = GetHobList ();
>
> +  //
> +  // Check Length to avoid data overflow.
> +  //
> +  if (HobLength > MAX_UINT16 - 0x7) {
> +return NULL;
> +  }
>HobLength = (UINT16)((HobLength + 0x7) & (~0x7));
>
>FreeMemory = HandOffHob->EfiFreeMemoryTop - 
> HandOffHob->EfiFreeMemoryBottom;
> --
> 2.39.2.windows.1
>


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




[edk2-devel] Cancelled Event: TianoCore Community Meeting EMEA/NAMO - Thursday, January 11, 2024 #cal-cancelled

2024-01-10 Thread Group Notification
BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//Groups.io Inc//Groups.io Calendar//EN
METHOD:CANCELLED
REFRESH-INTERVAL;VALUE=DURATION:PT1H
X-PUBLISHED-TTL:PT1H
CALSCALE:GREGORIAN
BEGIN:VTIMEZONE
TZID:America/Los_Angeles
LAST-MODIFIED:20231222T233358Z
TZURL:https://www.tzurl.org/zoneinfo-outlook/America/Los_Angeles
X-LIC-LOCATION:America/Los_Angeles
BEGIN:DAYLIGHT
TZNAME:PDT
TZOFFSETFROM:-0800
TZOFFSETTO:-0700
DTSTART:19700308T02
RRULE:FREQ=YEARLY;BYMONTH=3;BYDAY=2SU
END:DAYLIGHT
BEGIN:STANDARD
TZNAME:PST
TZOFFSETFROM:-0700
TZOFFSETTO:-0800
DTSTART:19701101T02
RRULE:FREQ=YEARLY;BYMONTH=11;BYDAY=1SU
END:STANDARD
END:VTIMEZONE
BEGIN:VEVENT
X-GIOIDS:Event:2148598 
UID:u7v8.1660766102568797982.u...@groups.io
DTSTAMP:20240111T065313Z
ORGANIZER;CN=Miki Demeter:mailto:devel@edk2.groups.io
DTSTART:20240111T16Z
DTEND:20240111T17Z
SUMMARY:TianoCore Community Meeting EMEA/NAMO
DESCRIPTION:_
 ___\n\nMicrosoft Teams meeting\n\n*Join on your computer 
 or mobile app*\n\nClick here to join the meeting ( https://teams.microsof
 t.com/l/meetup-join/19%3ameeting_MTAyZGJhNjMtYzQ4Mi00MTUxLWFlMWMtOGU0MWNl
 ZDk4NjY5%40thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-
 4ed7712e255d%22%2c%22Oid%22%3a%226e4ce4c4-1242-431b-9a51-92cd01a5df3c%22%
 7d )\n\nMeeting ID: 226 323 011 029\nPasscode: hMRCj6\n\nDownload Teams (
  https://www.microsoft.com/en-us/microsoft-teams/download-app ) | Join on
  the web ( https://www.microsoft.com/microsoft-teams/join-a-meeting )\n\n
 *Join with a video conferencing device*\n\nte...@conf.intel.com\n\nVideo 
 Conference ID: 112 716 814 3\n\nAlternate VTC instructions ( https://conf
 .intel.com/teams/?conf=1127168143=teams=conf.intel.com=test_ca
 ll )\n\nLearn More ( https://aka.ms/JoinTeamsMeeting ) | Meeting options 
 ( https://teams.microsoft.com/meetingOptions/?organizerId=6e4ce4c4-1242-4
 31b-9a51-92cd01a5df3c=46c98d88-e344-4ed4-8496-4ed7712e255d
 dId=19_meeting_MTAyZGJhNjMtYzQ4Mi00MTUxLWFlMWMtOGU0MWNlZDk4NjY5@thread.v2
 =0=en-US )\n\n
 
LOCATION:Microsoft Teams meeting  Join on your computer or mobile app  Cl
 ick here to join the meeting  Meeting ID: 226 323 011 029  Passcode: hMRC
 j6  Download Teams | Join on the web Join with a video conferencing devic
 e  te...@conf.intel.com  Video Conference ID: 112 716 814 3  Alternate VT
 C instructions  Learn More | Meeting options
RECURRENCE-ID:20240104T16Z
SEQUENCE:2
STATUS:CANCELLED
END:VEVENT
END:VCALENDAR


invite.ics
Description: application/ics


Re: [edk2-devel] TianoCore Community Meeting - call for topics

2024-01-10 Thread Michael D Kinney
Meeting canceled.

Mike


> -Original Message-
> From: Kinney, Michael D 
> Sent: Wednesday, January 10, 2024 10:37 AM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D 
> Subject: TianoCore Community Meeting - call for topics
> 
> Any topics for the TianoCore Community meeting this week?
> 
> Mike


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




Re: [edk2-devel] [PATCH v1 2/4] StandaloneMmPkg/Hob: Integer Overflow in CreateHob()

2024-01-10 Thread Ard Biesheuvel
On Thu, 11 Jan 2024 at 06:15,  wrote:
>
> From: Gerd Hoffmann 
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4166
>
> Fix integer overflow in various CreateHob instances.
> Fixes: CVE-2022-36765
>
> The CreateHob() function aligns the requested size to 8
> performing the following operation:
> ```
> HobLength = (UINT16)((HobLength + 0x7) & (~0x7));
> ```
>
> No checks are performed to ensure this value doesn't
> overflow, and could lead to CreateHob() returning a smaller
> HOB than requested, which could lead to OOB HOB accesses.
>
> Reported-by: Marc Beatove 
> Cc: Ard Biesheuvel 
> Cc: Sami Mujawar 
> Cc: Ray Ni 
> Cc: John Mathew 
> Signed-off-by: Gerd Hoffmann 

Reviewed-by: Ard Biesheuvel 

> ---
>  .../StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git 
> a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c 
> b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c
> index 1550e1babc..29ade2e4ef 100644
> --- 
> a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c
> +++ 
> b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c
> @@ -34,6 +34,12 @@ CreateHob (
>
>HandOffHob = GetHobList ();
>
> +  //
> +  // Check Length to avoid data overflow.
> +  //
> +  if (HobLength > MAX_UINT16 - 0x7) {
> +return NULL;
> +  }
>HobLength = (UINT16)((HobLength + 0x7) & (~0x7));
>
>FreeMemory = HandOffHob->EfiFreeMemoryTop - 
> HandOffHob->EfiFreeMemoryBottom;
> --
> 2.39.2.windows.1
>


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




[edk2-devel] [PATCH v1 4/4] MdeModulePkg/Hob: Integer Overflow in CreateHob()

2024-01-10 Thread Guo, Gua
From: Gerd Hoffmann 

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

Fix integer overflow in various CreateHob instances.
Fixes: CVE-2022-36765

The CreateHob() function aligns the requested size to 8
performing the following operation:
```
HobLength = (UINT16)((HobLength + 0x7) & (~0x7));
```

No checks are performed to ensure this value doesn't
overflow, and could lead to CreateHob() returning a smaller
HOB than requested, which could lead to OOB HOB accesses.

Reported-by: Marc Beatove 
Cc: Liming Gao 
Cc: John Mathew 
Signed-off-by: Gerd Hoffmann 
---
 MdeModulePkg/Core/Pei/Hob/Hob.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/Pei/Hob/Hob.c b/MdeModulePkg/Core/Pei/Hob/Hob.c
index c4882a23cd..985da50995 100644
--- a/MdeModulePkg/Core/Pei/Hob/Hob.c
+++ b/MdeModulePkg/Core/Pei/Hob/Hob.c
@@ -85,7 +85,7 @@ PeiCreateHob (
   //
   // Check Length to avoid data overflow.
   //
-  if (0x1 - Length <= 0x7) {
+  if (MAX_UINT16 - Length < 0x7) {
 return EFI_INVALID_PARAMETER;
   }
 
-- 
2.39.2.windows.1



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




[edk2-devel] [PATCH v1 3/4] EmbeddedPkg/Hob: Integer Overflow in CreateHob()

2024-01-10 Thread Guo, Gua
From: Gerd Hoffmann 

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

Fix integer overflow in various CreateHob instances.
Fixes: CVE-2022-36765

The CreateHob() function aligns the requested size to 8
performing the following operation:
```
HobLength = (UINT16)((HobLength + 0x7) & (~0x7));
```

No checks are performed to ensure this value doesn't
overflow, and could lead to CreateHob() returning a smaller
HOB than requested, which could lead to OOB HOB accesses.

Reported-by: Marc Beatove 
Cc: Leif Lindholm 
Cc: Ard Biesheuvel 
Cc: Abner Chang 
Cc: John Mathew 
Signed-off-by: Gerd Hoffmann 
---
 EmbeddedPkg/Library/PrePiHobLib/Hob.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/EmbeddedPkg/Library/PrePiHobLib/Hob.c 
b/EmbeddedPkg/Library/PrePiHobLib/Hob.c
index 8eb175aa96..ee2e3176be 100644
--- a/EmbeddedPkg/Library/PrePiHobLib/Hob.c
+++ b/EmbeddedPkg/Library/PrePiHobLib/Hob.c
@@ -110,6 +110,12 @@ CreateHob (
 
   HandOffHob = GetHobList ();
 
+  //
+  // Check Length to avoid data overflow.
+  //
+  if (HobLength > MAX_UINT16 - 0x7) {
+return NULL;
+  }
   HobLength = (UINT16)((HobLength + 0x7) & (~0x7));
 
   FreeMemory = HandOffHob->EfiFreeMemoryTop - HandOffHob->EfiFreeMemoryBottom;
-- 
2.39.2.windows.1



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




[edk2-devel] [PATCH v1 2/4] StandaloneMmPkg/Hob: Integer Overflow in CreateHob()

2024-01-10 Thread Guo, Gua
From: Gerd Hoffmann 

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

Fix integer overflow in various CreateHob instances.
Fixes: CVE-2022-36765

The CreateHob() function aligns the requested size to 8
performing the following operation:
```
HobLength = (UINT16)((HobLength + 0x7) & (~0x7));
```

No checks are performed to ensure this value doesn't
overflow, and could lead to CreateHob() returning a smaller
HOB than requested, which could lead to OOB HOB accesses.

Reported-by: Marc Beatove 
Cc: Ard Biesheuvel 
Cc: Sami Mujawar 
Cc: Ray Ni 
Cc: John Mathew 
Signed-off-by: Gerd Hoffmann 
---
 .../StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git 
a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c 
b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c
index 1550e1babc..29ade2e4ef 100644
--- 
a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c
+++ 
b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c
@@ -34,6 +34,12 @@ CreateHob (
 
   HandOffHob = GetHobList ();
 
+  //
+  // Check Length to avoid data overflow.
+  //
+  if (HobLength > MAX_UINT16 - 0x7) {
+return NULL;
+  }
   HobLength = (UINT16)((HobLength + 0x7) & (~0x7));
 
   FreeMemory = HandOffHob->EfiFreeMemoryTop - HandOffHob->EfiFreeMemoryBottom;
-- 
2.39.2.windows.1



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




[edk2-devel] [PATCH v1 1/4] UefiPayloadPkg/Hob: Integer Overflow in CreateHob()

2024-01-10 Thread Guo, Gua
From: Gerd Hoffmann 

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

Fix integer overflow in various CreateHob instances.
Fixes: CVE-2022-36765

The CreateHob() function aligns the requested size to 8
performing the following operation:
```
HobLength = (UINT16)((HobLength + 0x7) & (~0x7));
```

No checks are performed to ensure this value doesn't
overflow, and could lead to CreateHob() returning a smaller
HOB than requested, which could lead to OOB HOB accesses.

Reported-by: Marc Beatove 
Cc: Guo Dong 
Cc: Sean Rhodes 
Cc: James Lu 
Reviewed-by: Gua Guo 
Cc: John Mathew 
Signed-off-by: Gerd Hoffmann 
---
 UefiPayloadPkg/Library/PayloadEntryHobLib/Hob.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/UefiPayloadPkg/Library/PayloadEntryHobLib/Hob.c 
b/UefiPayloadPkg/Library/PayloadEntryHobLib/Hob.c
index 2c3acbbc19..f2bd2650b6 100644
--- a/UefiPayloadPkg/Library/PayloadEntryHobLib/Hob.c
+++ b/UefiPayloadPkg/Library/PayloadEntryHobLib/Hob.c
@@ -110,6 +110,12 @@ CreateHob (
 
   HandOffHob = GetHobList ();
 
+  //
+  // Check Length to avoid data overflow.
+  //
+  if (HobLength > MAX_UINT16 - 0x7) {
+return NULL;
+  }
   HobLength = (UINT16)((HobLength + 0x7) & (~0x7));
 
   FreeMemory = HandOffHob->EfiFreeMemoryTop - HandOffHob->EfiFreeMemoryBottom;
-- 
2.39.2.windows.1



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




[edk2-devel] [PATCH v1 0/4] Bz4166: Integer Overflow in CreateHob()

2024-01-10 Thread Guo, Gua
From: Gua Guo 

Fix Integer Overflow for CVE-2022-36765
1. UefiPayloadPkg/Hob: Integer Overflow in CreateHob()
2. StandaloneMmPkg/Hob: Integer Overflow in CreateHob()
3. EmbeddedPkg/Hob: Integer Overflow in CreateHob()
4. MdeModulePkg/Hob: Integer Overflow in CreateHob()


Gerd Hoffmann (4):
  UefiPayloadPkg/Hob: Integer Overflow in CreateHob()
  StandaloneMmPkg/Hob: Integer Overflow in CreateHob()
  EmbeddedPkg/Hob: Integer Overflow in CreateHob()
  MdeModulePkg/Hob: Integer Overflow in CreateHob()

 EmbeddedPkg/Library/PrePiHobLib/Hob.c   | 6 ++
 MdeModulePkg/Core/Pei/Hob/Hob.c | 2 +-
 .../StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c | 6 ++
 UefiPayloadPkg/Library/PayloadEntryHobLib/Hob.c | 6 ++
 4 files changed, 19 insertions(+), 1 deletion(-)

--
2.39.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113576): https://edk2.groups.io/g/devel/message/113576
Mute This Topic: https://groups.io/mt/103657270/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] UefiCpuPkg:Limit PhysicalAddressBits in speicial case

2024-01-10 Thread Ni, Ray
Reviewed-by: Ray Ni 

Thanks,
Ray
> -Original Message-
> From: Tan, Dun 
> Sent: Thursday, January 11, 2024 10:11 AM
> To: devel@edk2.groups.io
> Cc: Ni, Ray ; Laszlo Ersek ; Kumar,
> Rahul R ; Gerd Hoffmann 
> Subject: [Patch V2] UefiCpuPkg:Limit PhysicalAddressBits in speicial case
> 
> When creating smm page table, limit maximum
> supported physical address bits returned by
> CalculateMaximumSupportAddress() to 47 if
> 5-Level Paging is disabled.
> When 5-Level Paging is disabled and the
> PhysicalAddressBits retrived from CPU HOB or
> CpuId is bigger than 47, and since virtual
> addresses are sign-extended, only [0, 2^47-1]
> range in 52-bit physical address is mapped
> in page table.
> 
> Signed-off-by: Dun Tan 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Cc: Rahul Kumar 
> Cc: Gerd Hoffmann 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 15 +--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> index ddd9be66b5..35c282a771 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> @@ -137,11 +137,13 @@ GetSubEntriesNum (
>  /**
>Calculate the maximum support address.
> 
> +  @param[in] Is5LevelPagingNeededIf 5-level paging enabling is
> needed.
> +
>@return the maximum support address.
>  **/
>  UINT8
>  CalculateMaximumSupportAddress (
> -  VOID
> +  BOOLEAN  Is5LevelPagingNeeded
>)
>  {
>UINT32  RegEax;
> @@ -164,6 +166,15 @@ CalculateMaximumSupportAddress (
>  }
>}
> 
> +  //
> +  // Only [0, 2^47 -1] in 52-bit physical addresses is mapped in page table
> +  // when 5-Level Paging is disabled.
> +  //
> +  ASSERT (PhysicalAddressBits <= 52);
> +  if (!Is5LevelPagingNeeded && (PhysicalAddressBits > 47)) {
> +PhysicalAddressBits = 47;
> +  }
> +
>return PhysicalAddressBits;
>  }
> 
> @@ -197,7 +208,7 @@ SmmInitPageTable (
>mCpuSmmRestrictedMemoryAccess = PcdGetBool
> (PcdCpuSmmRestrictedMemoryAccess);
>m1GPageTableSupport   = Is1GPageSupport ();
>m5LevelPagingNeeded   = Is5LevelPagingNeeded ();
> -  mPhysicalAddressBits  = CalculateMaximumSupportAddress ();
> +  mPhysicalAddressBits  = CalculateMaximumSupportAddress
> (m5LevelPagingNeeded);
>PatchInstructionX86 (gPatch5LevelPagingNeeded, m5LevelPagingNeeded,
> 1);
>if (m5LevelPagingNeeded) {
>  mPagingMode = m1GPageTableSupport ? Paging5Level1GB :
> Paging5Level;
> --
> 2.31.1.windows.1



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




[edk2-devel] Event: TianoCore Community Meeting - APAC/NAMO - Thursday, January 11, 2024 #cal-reminder

2024-01-10 Thread Group Notification
*Reminder: TianoCore Community Meeting - APAC/NAMO*

*When:*
Thursday, January 11, 2024
7:30pm to 8:30pm
(UTC-08:00) America/Los Angeles

*Where:*
https://teams.microsoft.com/l/meetup-join/19%3ameeting_Y2M1NDE3ODYtN2M3Yy00MDMxLTk3OWYtMTlkNjhlNWFlMjA2%40thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%226e4ce4c4-1242-431b-9a51-92cd01a5df3c%22%7d

*Organizer:* Miki Demeter

View Event ( https://edk2.groups.io/g/devel/viewevent?eventid=2148595 )

*Description:*



Microsoft Teams meeting

*Join on your computer or mobile app*

Click here to join the meeting ( 
https://teams.microsoft.com/l/meetup-join/19%3ameeting_Y2M1NDE3ODYtN2M3Yy00MDMxLTk3OWYtMTlkNjhlNWFlMjA2%40thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%226e4ce4c4-1242-431b-9a51-92cd01a5df3c%22%7d
 )

Meeting ID: 283 318 374 436
Passcode: 633zLo

Download Teams ( https://www.microsoft.com/en-us/microsoft-teams/download-app ) 
| Join on the web ( https://www.microsoft.com/microsoft-teams/join-a-meeting )

*Join with a video conferencing device*

te...@conf.intel.com

Video Conference ID: 119 493 012 8

Alternate VTC instructions ( 
https://conf.intel.com/teams/?conf=1194930128=teams=conf.intel.com=test_call
 )

Learn More ( https://aka.ms/JoinTeamsMeeting ) | Meeting options ( 
https://teams.microsoft.com/meetingOptions/?organizerId=6e4ce4c4-1242-431b-9a51-92cd01a5df3c=46c98d88-e344-4ed4-8496-4ed7712e255d=19_meeting_Y2M1NDE3ODYtN2M3Yy00MDMxLTk3OWYtMTlkNjhlNWFlMjA2@thread.v2=0=en-US
 )




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




回复: [edk2-devel] [PATCH 1/1] MdePkg: Update GetHealthStatus function description

2024-01-10 Thread gaoliming via groups.io
Junfeng:
  I add push label for this PR.

Thanks
Liming
> -邮件原件-
> 发件人: devel@edk2.groups.io  代表 Junfeng Guan
> 发送时间: 2024年1月11日 9:14
> 收件人: Gao, Liming ; devel@edk2.groups.io
> 抄送: Kinney, Michael D ; Liu, Zhiguang
> ; Li, Yi1 
> 主题: Re: [edk2-devel] [PATCH 1/1] MdePkg: Update GetHealthStatus
> function description
> 
> Hi Liming,
>Thanks! I created a PR: https://github.com/tianocore/edk2/pull/5248
>Could you help push it?
> 
> B.R.
> Junfeng
> -Original Message-
> From: gaoliming 
> Sent: Wednesday, January 10, 2024 2:19 PM
> To: devel@edk2.groups.io; Guan, JunfengX 
> Cc: Kinney, Michael D ; Liu, Zhiguang
> ; Li, Yi1 
> Subject: 回复: [edk2-devel] [PATCH 1/1] MdePkg: Update GetHealthStatus
> function description
> 
> Reviewed-by: Liming Gao 
> 
> > -邮件原件-
> > 发件人: devel@edk2.groups.io  代表 Junfeng
> Guan
> > 发送时间: 2023年12月28日 15:54
> > 收件人: devel@edk2.groups.io
> > 抄送: Michael D Kinney ; Liming Gao
> > ; Zhiguang Liu ; Yi
> > Li 
> > 主题: [edk2-devel] [PATCH 1/1] MdePkg: Update GetHealthStatus function
> > description
> >
> > Refer to Uefi spec 2.10 section 11.10.2, update the return value for
> > EFI_DRIVER_HEALTH_PROTOCOL.GetHealthStatus.
> >
> > Signed-off-by: Junfeng Guan 
> > Cc: Michael D Kinney 
> > Cc: Liming Gao 
> > Cc: Zhiguang Liu 
> > Cc: Yi Li 
> > ---
> >  MdePkg/Include/Protocol/DriverHealth.h | 32
> > +++---
> >  1 file changed, 8 insertions(+), 24 deletions(-)
> >
> > diff --git a/MdePkg/Include/Protocol/DriverHealth.h
> > b/MdePkg/Include/Protocol/DriverHealth.h
> > index 9de025434ef3..9e1bd903f4be 100644
> > --- a/MdePkg/Include/Protocol/DriverHealth.h
> > +++ b/MdePkg/Include/Protocol/DriverHealth.h
> > @@ -137,35 +137,19 @@ EFI_STATUS
> >will only be returned with a
> > HealthStatus value of
> >
> > EfiDriverHealthStatusConfigurationRequired.
> >
> > -  @retval EFI_SUCCESS   ControllerHandle is NULL, and all the
> > controllers
> > -managed by this driver specified by
> > This have a health
> > -status of
> > EfiDriverHealthStatusHealthy with no warning
> > -messages to be returned.  The
> > ChildHandle, HealthStatus,
> > -MessageList, and FormList
> > parameters are ignored.
> > -
> > -  @retval EFI_DEVICE_ERROR  ControllerHandle is NULL, and one
> or
> > more of the
> > -controllers managed by this driver
> > specified by This
> > -do not have a health status of
> > EfiDriverHealthStatusHealthy.
> > -The ChildHandle, HealthStatus,
> > MessageList, and
> > -FormList parameters are ignored.
> > -
> > -  @retval EFI_DEVICE_ERROR  ControllerHandle is NULL, and one
> or
> > more of the
> > -controllers managed by this driver
> > specified by This
> > -have one or more warning and/or
> > error messages.
> > -The ChildHandle, HealthStatus,
> > MessageList, and
> > -FormList parameters are ignored.
> > -
> > -  @retval EFI_SUCCESS   ControllerHandle is not NULL and
> the
> > health status
> > -of the controller specified by
> > ControllerHandle and
> > -ChildHandle was returned in
> > HealthStatus.  A list
> > -of warning and error messages
> may
> > be optionally
> > -returned in MessageList, and a list
> of
> > HII Forms
> > -may be optionally returned in
> > FormList.
> > +  @retval EFI_SUCCESS   The health status of the controller
> > specified by
> > +ControllerHandle and ChildHandle
> > was returned in HealthStatus.
> > +A list of warning and error
> messages
> > may be optionally
> > +returned in MessageList, and an
> HII
> > Form may be optionally
> > +specified by FormHiiHandle.
> >
> >@retval EFI_UNSUPPORTED   ControllerHandle is not NULL, and
> > the controller
> >  specified by ControllerHandle and
> > ChildHandle is not
> >  currently being managed by the
> driver
> > specified by This.
> >
> > +  @retval EFI_UNSUPPORTED   ControllerHandle is NULL and
> there
> > are no devices being
> > +managed by the driver.
> > +
> >@retval EFI_INVALID_PARAMETER HealthStatus is NULL.
> >
> >@retval EFI_OUT_OF_RESOURCES  MessageList is not NULL, and
> there
> > are not enough
> > --
> > 2.26.2.windows.1
> >
> >
> >
> >
> >
> 
> 
> 
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all 

回复: [edk2-devel] [PATCH 1/1] MdePkg: Update the Label definitions of the EFI_NVDIMM_LABEL

2024-01-10 Thread gaoliming via groups.io
Reviewed-by: Liming Gao 

> -邮件原件-
> 发件人: devel@edk2.groups.io  代表 Junfeng Guan
> 发送时间: 2023年12月27日 9:57
> 收件人: devel@edk2.groups.io
> 抄送: Michael D Kinney ; Liming Gao
> ; Zhiguang Liu ; Yi Li
> 
> 主题: [edk2-devel] [PATCH 1/1] MdePkg: Update the Label definitions of the
> EFI_NVDIMM_LABEL
> 
> Refer to Uefi spec 2.10 section 13.19.5, update the label definitions
> for NVDIMM SPA location cookie.
> 
> Signed-off-by: Junfeng Guan 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Zhiguang Liu 
> Cc: Yi Li 
> ---
>  MdePkg/Include/Protocol/NvdimmLabel.h | 16 +++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/MdePkg/Include/Protocol/NvdimmLabel.h
> b/MdePkg/Include/Protocol/NvdimmLabel.h
> index e46999a3ab4a..2352c9bd1652 100644
> --- a/MdePkg/Include/Protocol/NvdimmLabel.h
> +++ b/MdePkg/Include/Protocol/NvdimmLabel.h
> @@ -122,6 +122,12 @@ typedef struct {
>  ///
>  #define EFI_NVDIMM_LABEL_FLAGS_UPDATING  0x0008
> 
> +///
> +/// When set, the SPALocationCookie in the namespace label is valid and
> should match the
> +/// current value in the NFIT SPA Range Structure.
> +///
> +#define EFI_NVDIMM_LABEL_FLAGS_SPACOOKIE_BOUND  0x0010
> +
>  typedef struct {
>///
>/// Unique Label Identifier UUID per RFC 4122.
> @@ -196,10 +202,18 @@ typedef struct {
>///
>EFI_GUIDAddressAbstractionGuid;
> 
> +  ///
> +  /// When creating the label, this value is set to the value from the
NFIT
> SPA Range Structure if the
> +  /// SPALocationCookie flag (bit 2) is set. If
> EFI_NVDIMM_LABEL_FLAGS_SPACOOKIE_BOUND is set, the
> SPALocationCookie
> +  /// value stored in the namespace label should match the current value
in
> the NFIT SPA Range Structure.
> +  /// Otherwise, the data may not be read correctly.
> +  ///
> +  UINT64  SPALocationCookie;
> +
>///
>/// Shall be 0.
>///
> -  UINT8   Reserved1[88];
> +  UINT8   Reserved1[80];
> 
>///
>/// 64-bit Fletcher64 checksum of all fields in this Label.
> --
> 2.26.2.windows.1
> 
> 
> 
> 
> 





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




回复: [edk2-devel] [PATCH 1/1] MdePkg: Updated the comments of EFI_SYSTEM_TABLE and ReadKeyStroke

2024-01-10 Thread gaoliming via groups.io
Reviewed-by: Liming Gao 

> -邮件原件-
> 发件人: devel@edk2.groups.io  代表 Junfeng Guan
> 发送时间: 2023年12月21日 10:02
> 收件人: devel@edk2.groups.io
> 抄送: Michael D Kinney ; Liming Gao
> ; Zhiguang Liu ; Yi Li
> 
> 主题: [edk2-devel] [PATCH 1/1] MdePkg: Updated the comments of
> EFI_SYSTEM_TABLE and ReadKeyStroke
> 
> 1. Refer to Uefi spec 2.10 section 4.3.1, Updated the comments of
> EFI_SYSTEM_TABLE to satisfy ConIn/ConOut/StdErr requirements.
> 2. Refer to Uefi spec 2.10 section 13.3.3, Add a new retval
> EFI_UNSUPPORTED to
> EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL.ReadKeyStrokeEx
> and EFI_SIMPLE_TEXT_INPUT_PROTOCOL.ReadKeyStroke().
> 
> Signed-off-by: Junfeng Guan 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Zhiguang Liu 
> Cc: Yi Li 
> ---
>  MdePkg/Include/Protocol/SimpleTextIn.h   |  1 +
>  MdePkg/Include/Protocol/SimpleTextInEx.h |  1 +
>  MdePkg/Include/Uefi/UefiSpec.h   | 10 +++---
>  3 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/MdePkg/Include/Protocol/SimpleTextIn.h
> b/MdePkg/Include/Protocol/SimpleTextIn.h
> index 838fae279e71..b9d48472dd7f 100644
> --- a/MdePkg/Include/Protocol/SimpleTextIn.h
> +++ b/MdePkg/Include/Protocol/SimpleTextIn.h
> @@ -100,6 +100,7 @@ EFI_STATUS
>@retval EFI_NOT_READYThere was no keystroke data available.
>@retval EFI_DEVICE_ERROR The keystroke information was not returned
> due to
> hardware errors.
> +  @retval EFI_UNSUPPORTED  The device does not support the ability to
> read keystroke data.
> 
>  **/
>  typedef
> diff --git a/MdePkg/Include/Protocol/SimpleTextInEx.h
> b/MdePkg/Include/Protocol/SimpleTextInEx.h
> index 8317325d9b82..f33893768efb 100644
> --- a/MdePkg/Include/Protocol/SimpleTextInEx.h
> +++ b/MdePkg/Include/Protocol/SimpleTextInEx.h
> @@ -186,6 +186,7 @@ typedef struct {
>@retval EFI_NOT_READYThere was no keystroke data available.
>@retval EFI_DEVICE_ERROR The keystroke information was not returned
> due to
> hardware errors.
> +  @retval EFI_UNSUPPORTED  The device does not support the ability to
> read keystroke data.
> 
> 
>  **/
> diff --git a/MdePkg/Include/Uefi/UefiSpec.h
> b/MdePkg/Include/Uefi/UefiSpec.h
> index 5de00e8ea2af..c4952bd5f0d3 100644
> --- a/MdePkg/Include/Uefi/UefiSpec.h
> +++ b/MdePkg/Include/Uefi/UefiSpec.h
> @@ -2006,7 +2006,8 @@ typedef struct {
>UINT32 FirmwareRevision;
>///
>/// The handle for the active console input device. This handle must
> support
> -  /// EFI_SIMPLE_TEXT_INPUT_PROTOCOL and
> EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL.
> +  /// EFI_SIMPLE_TEXT_INPUT_PROTOCOL and
> EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL. If
> +  /// there is no active console, these protocols must still be present.
>///
>EFI_HANDLE ConsoleInHandle;
>///
> @@ -2015,7 +2016,9 @@ typedef struct {
>///
>EFI_SIMPLE_TEXT_INPUT_PROTOCOL *ConIn;
>///
> -  /// The handle for the active console output device.
> +  /// The handle for the active console output device. This handle must
> support the
> +  /// EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL. If there is no active console,
> these protocols
> +  /// must still be present.
>///
>EFI_HANDLE ConsoleOutHandle;
>///
> @@ -2025,7 +2028,8 @@ typedef struct {
>EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL*ConOut;
>///
>/// The handle for the active standard error console device.
> -  /// This handle must support the EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.
> +  /// This handle must support the EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.
> If there
> +  /// is no active console, this protocol must still be present.
>///
>EFI_HANDLE StandardErrorHandle;
>///
> --
> 2.26.2.windows.1
> 
> 
> 
> 
> 





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




[edk2-devel] [Patch V2] UefiCpuPkg:Limit PhysicalAddressBits in speicial case

2024-01-10 Thread duntan
When creating smm page table, limit maximum
supported physical address bits returned by
CalculateMaximumSupportAddress() to 47 if
5-Level Paging is disabled.
When 5-Level Paging is disabled and the
PhysicalAddressBits retrived from CPU HOB or
CpuId is bigger than 47, and since virtual
addresses are sign-extended, only [0, 2^47-1]
range in 52-bit physical address is mapped
in page table.

Signed-off-by: Dun Tan 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
index ddd9be66b5..35c282a771 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
@@ -137,11 +137,13 @@ GetSubEntriesNum (
 /**
   Calculate the maximum support address.
 
+  @param[in] Is5LevelPagingNeededIf 5-level paging enabling is needed.
+
   @return the maximum support address.
 **/
 UINT8
 CalculateMaximumSupportAddress (
-  VOID
+  BOOLEAN  Is5LevelPagingNeeded
   )
 {
   UINT32  RegEax;
@@ -164,6 +166,15 @@ CalculateMaximumSupportAddress (
 }
   }
 
+  //
+  // Only [0, 2^47 -1] in 52-bit physical addresses is mapped in page table
+  // when 5-Level Paging is disabled.
+  //
+  ASSERT (PhysicalAddressBits <= 52);
+  if (!Is5LevelPagingNeeded && (PhysicalAddressBits > 47)) {
+PhysicalAddressBits = 47;
+  }
+
   return PhysicalAddressBits;
 }
 
@@ -197,7 +208,7 @@ SmmInitPageTable (
   mCpuSmmRestrictedMemoryAccess = PcdGetBool (PcdCpuSmmRestrictedMemoryAccess);
   m1GPageTableSupport   = Is1GPageSupport ();
   m5LevelPagingNeeded   = Is5LevelPagingNeeded ();
-  mPhysicalAddressBits  = CalculateMaximumSupportAddress ();
+  mPhysicalAddressBits  = CalculateMaximumSupportAddress 
(m5LevelPagingNeeded);
   PatchInstructionX86 (gPatch5LevelPagingNeeded, m5LevelPagingNeeded, 1);
   if (m5LevelPagingNeeded) {
 mPagingMode = m1GPageTableSupport ? Paging5Level1GB : Paging5Level;
-- 
2.31.1.windows.1



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




Re: [edk2-devel] [PATCH] UefiCpuPkg/CpuMpPei: Don't write CR3 in ConvertMemoryPageToNotPresent

2024-01-10 Thread Ni, Ray


Thanks,
Ray
> -Original Message-
> From: Laszlo Ersek 
> Sent: Wednesday, January 10, 2024 7:57 PM
> To: Liu, Zhiguang ; devel@edk2.groups.io
> Cc: Ni, Ray ; Kumar, Rahul R ;
> Gerd Hoffmann 
> Subject: Re: [PATCH] UefiCpuPkg/CpuMpPei: Don't write CR3 in
> ConvertMemoryPageToNotPresent
> 
> On 1/10/24 06:43, Zhiguang Liu wrote:
> > After ConvertMemoryPageToNotPresent, there is always a flush TLB
> > function. So, to improve performance, there is no need to write CR3
> > inside ConvertMemoryPageToNotPresent
> >
> > Cc: Ray Ni 
> > Cc: Laszlo Ersek 
> > Cc: Rahul Kumar 
> > Cc: Gerd Hoffmann 
> > Signed-off-by: Zhiguang Liu 
> > ---
> >  UefiCpuPkg/CpuMpPei/CpuPaging.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c
> b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> > index 15c7015fb8..c6894458f7 100644
> > --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
> > +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> > @@ -167,7 +167,6 @@ ConvertMemoryPageToNotPresent (
> >}
> >
> >ASSERT_EFI_ERROR (Status);
> > -  AsmWriteCr3 (PageTable);
> >return Status;
> >  }
> >
> 
> (1) I mostly understand the point that the commit message makes, but the
> message is not clear enough. The real point is that we have two
> ConvertMemoryPageToNotPresent() calls altogether, and each of those
> takes place in a *loop*, and each of those loops is followed by a
> CpuFlushTlb() call.
> 
> The loops matter. If there were no loops, then we might be motivated to
> choose a different solution (for example, to move centralize the
> CpuFlushTlb() calls *inside* ConvertMemoryPageToNotPresent(), or
> something similar).
> 
> So, please update the commit message; mention the loops.
> 
> (2) I can't easily see why this change is actually correct. IIRC,
> writing CR3 has a "side effect" of flushing the TLB. But obviously,
> that's not the *only* effect of writing CR3. You could say that
> CpuFlushTlb() performs a *strict subset* of what AsmWriteCr3() performs.
> Therefore, in order to replace AsmWriteCr3() with just CpuFlushTlb(),
> you need to demonstrate that the functionality that now is *not* going
> to be done has always been superfluous. In more direct terms, you need
> to show that the AsmWriteCr3() write call that's being removed here
> never actuall changes the *value* of CR3.
> 
> And I cannot show that myself very easily.
> ConvertMemoryPageToNotPresent(). In ConvertMemoryPageToNotPresent(),
> "PageTable" is first set from AsmReadCr3(), then passed twice to
> PageTableMap() by reference (!), and then it is written back to CR3. If
> at least one of those PageTableMap() calls change "PageTable", then the
> AsmWriteCr3() call at the end that's now being removed actually changes
> the value of CR3, and then the patch would be wrong.
> 
> It's possible that PageTableMap() never changes the value of
> "PageTable", but it must be proved, and the evidence should be included
> in the commit message.
> 
> (3) Furthermore, with the patch applied, ConvertMemoryPageToNotPresent()
> will no longer flush the TLB itself -- that will always be left to the
> caller. This new caller responsibility should be documented in the
> leading comment of ConvertMemoryPageToNotPresent(). We already have a
> paragraph there starting with "Caller should make sure..."
> 
> Sorry for making such a big deal out of this, but such simple-looking
> changes can sometimes case obscure (and rarely occurring) bugs down the
> road. If you already have evidence that CR3 does not change here, that's
> great, but then please don't think it's "obvious"; just go ahead and
> document it.

Laszlo,
Happy to see these comments! All make sense!

PageTableMap() only modifies the PageTable root pointer when creating from zero.
Since there is only one instance of the PageTableLib, I think we could update 
the
PageTableLib API comments to explicitly mention that. So point (2) will be 
resolved.

> 
> Laszlo



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




Re: [edk2-devel] [PATCH] UefiCpuPkg: Fix issue that IsModified is wrongly set in PageTableMap

2024-01-10 Thread Ni, Ray
> This function is incredibly complicated, so reviewing this patch is
> hard, even after reading the bugzilla ticket.
> 
> The commit message is useless. It should contain a brief description of
> the problem, and how the fix resolves the problem.
> 
> The documentation of the PageTableLibMapInLevel() function is wrong,
> even before this patch. It documents the "IsModified" output-only
> parameter as follows:
> 
> "TRUE means page table is modified. FALSE means page table is not
> modified."
> 
> This states that "IsModified" is always set on output, to either FALSE
> or TRUE. Which is an incorrect statement; in reality the caller is
> expected to pre-set (*IsModified) to FALSE, and PageTableLibMapInLevel()
> will (conditionally!) perform a FALSE->TRUE transition only.
> 
> Now, this patch may fix a bug, but it makes the above-described
> documentation issue worse, by further restricting the condition for said
> FALSE->TRUE transition.

Laszlo, thanks for the comments!
Though the fixing looks simple, Zhiguang and I did have several rounds of 
offline discussions
regarding how to fix it.

When the lib accesses the page table content, CPU would set the "Access" bit in 
the page entry
that points to the page table memory being accessed by the lib.

So, even when the "Modify" is FALSE (indicating caller doesn't want the lib to 
modify the page table),
lib code should not modify the page table but CPU still sets the "Access" bit 
in some of the entries due to
the reasons above.
I agree it will be better that the commit message carries above details.

Zhiguang,
Can we update the code to always assign "IsModified"? I thought we did that but 
it seems not.

> 
> The fix per se looks vaguely reasonable to me (really the function is so
> complicated that verifying this change from scratch would take me ages),
> but minimally, the documentation of "IsModified" should certainly be
> updated too. To something like this:
> 
>   @param[out] IsModified  If "Modify" is TRUE on input and the function
>   has actually modified the page table, then
> set
>   to TRUE on output. Not overwritten
> otherwise.
> 
> Laszlo



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




Re: [edk2-devel] [PATCH] UefiCpuPkg:Limit PhysicalAddressBits in speicial case

2024-01-10 Thread duntan
Hi Gerd,

Thanks for your comments. I agree with your opinion. Will change the code and 
related comments in V2 patch.

Thanks,
Dun

-Original Message-
From: Gerd Hoffmann  
Sent: Wednesday, January 10, 2024 6:55 PM
To: Tan, Dun 
Cc: devel@edk2.groups.io; Ni, Ray ; Laszlo Ersek 
; Kumar, Rahul R 
Subject: Re: [PATCH] UefiCpuPkg:Limit PhysicalAddressBits in speicial case

On Wed, Jan 10, 2024 at 04:05:44PM +0800, Dun Tan wrote:
> When creating smm page table, limit maximum supported physical address 
> bits returned by
> CalculateMaximumSupportAddress() to 48 if 5-Level Paging is disabled.
> When 5-Level Paging is disabled and the PhysicalAddressBits retrived 
> from CPU HOB or CpuId is bigger than 48, only [0, 2^48 -1] range in 
> 52-bit physical address is mapped in page table.

I think this is wrong.  Virtual addresses are sign-extended, i.e. the virtual 
address space without 5-level paging is:

 0x -> 0x7fff and
 0x8000 -> 0x

Therefore identity-mapping works for [0, 2^47-1] only.

take care,
  Gerd



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




[edk2-devel] [PATCH v1 1/1] PrmPkg/PrmInfo: Drop -r parameter

2024-01-10 Thread Michael Kubacki
From: Michael Kubacki 

The "-r" parameter was not added to the application so remove it from
the help string.

The standards section is also updated to point to the current
specification location on uefi.org.

Cc: Nate DeSimone 
Cc: Ankit Sinha 
Signed-off-by: Michael Kubacki 
---
 PrmPkg/Application/PrmInfo/PrmInfoStrings.uni | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/PrmPkg/Application/PrmInfo/PrmInfoStrings.uni 
b/PrmPkg/Application/PrmInfo/PrmInfoStrings.uni
index 9385fd848344..756cf97c7c37 100644
--- a/PrmPkg/Application/PrmInfo/PrmInfoStrings.uni
+++ b/PrmPkg/Application/PrmInfo/PrmInfoStrings.uni
@@ -61,7 +61,7 @@
 "Display and test Platform Runtime Mechanism (PRM) modules.\r\n"
 ".SH SYNOPSIS\r\n"
 "\r\n"
-"PRMINFO [[-?] | [-b] [-l] [-r] [-t (guid | all)]]\r\n"
+"PRMINFO [[-?] | [-b] [-l] [-t (guid | all)]]\r\n"
 ".SH OPTIONS\r\n"
 " \r\n"
 "  -? - Show help.\r\n"
@@ -107,18 +107,14 @@
 ".SH STANDARDS\r\n"
 " \r\n"
 "STANDARDS:\r\n"
-"  Platform Runtime Mechanism (PRM) is currently in a draft state and the\r\n"
-"  specification is not yet publicly available. A reference to the 
publicly\r\n"
-"  available document will replace this text when it is available.\r\n"
+"  The Platform Runtime Mechanism (PRM) specification is available at:\r\n"
+"  
https://uefi.org/sites/default/files/resources/Platform%20Runtime%20Mechanism%20-%20with%20legal%20notice.pdf\r\n;
 ".SH EXAMPLES\r\n"
 " \r\n"
 "EXAMPLES:\r\n"
 "  * To display a list of the installed PRM modules and PRM handlers:\r\n"
 "fs0:\> prminfo -l\r\n"
 " \r\n"
-"  * To validate the installed PRMT ACPI table:\r\n"
-"fs0:\> prminfo -r\r\n"
-" \r\n"
 "  * To call a PRM handler by GUID:\r\n"
 "fs0:\> prminfo -t e1466081-7562-430f-896b-b0e523dc335a\r\n"
 " \r\n"
-- 
2.43.0.windows.1



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




Re: [edk2-devel] [PATCH 1/1] MdePkg: Update GetHealthStatus function description

2024-01-10 Thread Junfeng Guan
Hi Liming,
   Thanks! I created a PR: https://github.com/tianocore/edk2/pull/5248
   Could you help push it?

B.R.
Junfeng
-Original Message-
From: gaoliming  
Sent: Wednesday, January 10, 2024 2:19 PM
To: devel@edk2.groups.io; Guan, JunfengX 
Cc: Kinney, Michael D ; Liu, Zhiguang 
; Li, Yi1 
Subject: 回复: [edk2-devel] [PATCH 1/1] MdePkg: Update GetHealthStatus function 
description

Reviewed-by: Liming Gao 

> -邮件原件-
> 发件人: devel@edk2.groups.io  代表 Junfeng Guan
> 发送时间: 2023年12月28日 15:54
> 收件人: devel@edk2.groups.io
> 抄送: Michael D Kinney ; Liming Gao 
> ; Zhiguang Liu ; Yi 
> Li 
> 主题: [edk2-devel] [PATCH 1/1] MdePkg: Update GetHealthStatus function 
> description
> 
> Refer to Uefi spec 2.10 section 11.10.2, update the return value for 
> EFI_DRIVER_HEALTH_PROTOCOL.GetHealthStatus.
> 
> Signed-off-by: Junfeng Guan 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Zhiguang Liu 
> Cc: Yi Li 
> ---
>  MdePkg/Include/Protocol/DriverHealth.h | 32 
> +++---
>  1 file changed, 8 insertions(+), 24 deletions(-)
> 
> diff --git a/MdePkg/Include/Protocol/DriverHealth.h
> b/MdePkg/Include/Protocol/DriverHealth.h
> index 9de025434ef3..9e1bd903f4be 100644
> --- a/MdePkg/Include/Protocol/DriverHealth.h
> +++ b/MdePkg/Include/Protocol/DriverHealth.h
> @@ -137,35 +137,19 @@ EFI_STATUS
>will only be returned with a 
> HealthStatus value of
> 
> EfiDriverHealthStatusConfigurationRequired.
> 
> -  @retval EFI_SUCCESS   ControllerHandle is NULL, and all the
> controllers
> -managed by this driver specified by
> This have a health
> -status of
> EfiDriverHealthStatusHealthy with no warning
> -messages to be returned.  The
> ChildHandle, HealthStatus,
> -MessageList, and FormList
> parameters are ignored.
> -
> -  @retval EFI_DEVICE_ERROR  ControllerHandle is NULL, and one or
> more of the
> -controllers managed by this driver
> specified by This
> -do not have a health status of
> EfiDriverHealthStatusHealthy.
> -The ChildHandle, HealthStatus,
> MessageList, and
> -FormList parameters are ignored.
> -
> -  @retval EFI_DEVICE_ERROR  ControllerHandle is NULL, and one or
> more of the
> -controllers managed by this driver
> specified by This
> -have one or more warning and/or
> error messages.
> -The ChildHandle, HealthStatus,
> MessageList, and
> -FormList parameters are ignored.
> -
> -  @retval EFI_SUCCESS   ControllerHandle is not NULL and the
> health status
> -of the controller specified by
> ControllerHandle and
> -ChildHandle was returned in
> HealthStatus.  A list
> -of warning and error messages may
> be optionally
> -returned in MessageList, and a list of
> HII Forms
> -may be optionally returned in
> FormList.
> +  @retval EFI_SUCCESS   The health status of the controller
> specified by
> +ControllerHandle and ChildHandle
> was returned in HealthStatus.
> +A list of warning and error messages
> may be optionally
> +returned in MessageList, and an HII
> Form may be optionally
> +specified by FormHiiHandle.
> 
>@retval EFI_UNSUPPORTED   ControllerHandle is not NULL, and
> the controller
>  specified by ControllerHandle and 
> ChildHandle is not
>  currently being managed by the driver 
> specified by This.
> 
> +  @retval EFI_UNSUPPORTED   ControllerHandle is NULL and there
> are no devices being
> +managed by the driver.
> +
>@retval EFI_INVALID_PARAMETER HealthStatus is NULL.
> 
>@retval EFI_OUT_OF_RESOURCES  MessageList is not NULL, and there 
> are not enough
> --
> 2.26.2.windows.1
> 
> 
> 
> 
> 





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




[edk2-devel] [PATCH] Platform/RaspberryPi: Enable FPDT data on RPi4

2024-01-10 Thread Jeremy Linton
FPDT data gives an approximation of how long each stage
of the firmware boot takes. Since the RPi has a generic
timer this is a fairly accurate timestamp from the point
where edk2 takes over, which is a few ms from actual
power on. While its not perfect its sufficiently granular
that a command like `systemd-analyze` shows the debug boot
to be quite slow.

Ex:
[root@rpi4 ~]# systemd-analyze
Startup finished in 24.313s (firmware) + 7.771s (loader) + 2.442s (kernel) + 
5.810s (initrd) + 20.371s (userspace) = 1min 708ms
multi-user.target reached after 17.343s in userspace.

Signed-off-by: Jeremy Linton 
---
 Platform/RaspberryPi/RPi4/RPi4.dsc | 28 ++--
 Platform/RaspberryPi/RPi4/RPi4.fdf |  7 +++
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc 
b/Platform/RaspberryPi/RPi4/RPi4.dsc
index 170e45ae7e..e9ca9463ce 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -69,7 +69,8 @@
   BmpSupportLib|MdeModulePkg/Library/BaseBmpSupportLib/BaseBmpSupportLib.inf
   
SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
   
PerformanceLib|MdePkg/Library/BasePerformanceLibNull/BasePerformanceLibNull.inf
-  
ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf
+  
ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
+
   PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
   
PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf
   PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
@@ -279,13 +280,25 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
   gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport|FALSE
 
+  ## Indicates if S3 performance data will be supported in ACPI FPDT table.
+  #   TRUE  - S3 performance data will be supported in ACPI FPDT table.
+  #   FALSE - S3 performance data will not be supported in ACPI FPDT table.
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwarePerformanceDataTableS3Support|FALSE
+
 [PcdsFixedAtBuild.common]
   gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength|100
   gEfiMdePkgTokenSpaceGuid.PcdMaximumAsciiStringLength|100
   gEfiMdePkgTokenSpaceGuid.PcdMaximumLinkedListLength|100
   gEfiMdePkgTokenSpaceGuid.PcdSpinLockTimeout|1000
   gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue|0xAF
-  gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask|1
+  #define PERFORMANCE_LIBRARY_PROPERTY_MEASUREMENT_ENABLED  0x0001
+  #define PERF_CORE_START_IMAGE  0x0002
+  #define PERF_CORE_LOAD_IMAGE   0x0004
+  #define PERF_CORE_DB_SUPPORT   0x0008
+  #define PERF_CORE_DB_START 0x0010
+  #define PERF_CORE_DB_STOP  0x0020
+  #define PERF_GENERAL_TYPE  0x0040
+  gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask|0x1
   gEfiMdePkgTokenSpaceGuid.PcdPostCodePropertyMask|0
   gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|320
 
@@ -324,6 +337,7 @@
   gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|$(DEBUG_PRINT_ERROR_LEVEL)
 
   gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
+  gEfiMdeModulePkgTokenSpaceGuid.PcdExtFpdtBootRecordPadSize|0x2000
 
   #
   # Optional feature to help prevent EFI memory map fragments
@@ -812,6 +826,16 @@
   Silicon/Broadcom/Drivers/I2cDxe/I2cDxe.inf
 
 
+  #
+  # Firmware Performance Data Table (FPDT)
+  #
+  
MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/ReportStatusCodeRouterRuntimeDxe.inf
+  
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
 {
+
+  LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf
+  }
+
+  #
   # UEFI application (Shell Embedded Boot Loader)
   #
   ShellPkg/Application/Shell/Shell.inf {
diff --git a/Platform/RaspberryPi/RPi4/RPi4.fdf 
b/Platform/RaspberryPi/RPi4/RPi4.fdf
index 989d99a49f..321639fc64 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.fdf
+++ b/Platform/RaspberryPi/RPi4/RPi4.fdf
@@ -326,6 +326,13 @@ READ_LOCK_STATUS   = TRUE
   #
   INF Platform/RaspberryPi/Drivers/LogoDxe/LogoDxe.inf
 
+  #
+  # Firmware Performance Data Table (FPDT)
+  #
+  INF 
MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/ReportStatusCodeRouterRuntimeDxe.inf
+  INF 
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
+
+
 [FV.FVMAIN_COMPACT]
 FvAlignment= 16
 ERASE_POLARITY = 1
-- 
2.43.0



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




[edk2-devel] [RFC 3/6] Platform/RasberryPi: Create I2C driver bound to RTC

2024-01-10 Thread Jeremy Linton
Now that we have a generic Bcm I2C driver lets instantiate one
against a possible RTC hat on the pi4.

Signed-off-by: Jeremy Linton 
---
 .../Drivers/BcmI2CPlatform/BcmI2CPlatform.c   | 127 ++
 .../Drivers/BcmI2CPlatform/BcmI2CPlatform.inf |  54 
 2 files changed, 181 insertions(+)
 create mode 100644 Platform/RaspberryPi/Drivers/BcmI2CPlatform/BcmI2CPlatform.c
 create mode 100644 
Platform/RaspberryPi/Drivers/BcmI2CPlatform/BcmI2CPlatform.inf

diff --git a/Platform/RaspberryPi/Drivers/BcmI2CPlatform/BcmI2CPlatform.c 
b/Platform/RaspberryPi/Drivers/BcmI2CPlatform/BcmI2CPlatform.c
new file mode 100644
index 00..11f927b848
--- /dev/null
+++ b/Platform/RaspberryPi/Drivers/BcmI2CPlatform/BcmI2CPlatform.c
@@ -0,0 +1,127 @@
+/** @file
+  Brcm/Rpi I2C DXE platform driver.
+
+  Copyright 2018-2020 NXP
+  Sourced and reworked from edk2/NXP I2C stack
+  Copyright 2022 Arm, Jeremy Linton
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+  This thing binds a I2C driver to a RTC..
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+typedef struct {
+  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR StartDesc;
+  UINT8 EndDesc;
+} ADDRESS_SPACE_DESCRIPTOR;
+
+#define BCM_I2C_NUM_CONTROLLERS 1 //actually 6 on the bcm2711, hack for now
+
+STATIC ADDRESS_SPACE_DESCRIPTOR mI2cDesc[BCM_I2C_NUM_CONTROLLERS];
+
+STATIC
+EFI_STATUS
+RegisterDevice (
+  IN  EFI_GUID*TypeGuid,
+  IN  ADDRESS_SPACE_DESCRIPTOR*Desc,
+  OUT EFI_HANDLE  *Handle
+  )
+{
+  NON_DISCOVERABLE_DEVICE *Device;
+  EFI_STATUS  Status;
+
+  Device = (NON_DISCOVERABLE_DEVICE *)AllocateZeroPool (sizeof (*Device));
+  if (Device == NULL) {
+return EFI_OUT_OF_RESOURCES;
+  }
+
+  Device->Type = TypeGuid;
+  Device->DmaType = NonDiscoverableDeviceDmaTypeNonCoherent;
+  Device->Resources = (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)Desc;
+
+  Status = gBS->InstallMultipleProtocolInterfaces (Handle,
+  , Device,
+  NULL);
+  if (EFI_ERROR (Status)) {
+goto FreeDevice;
+  }
+  return EFI_SUCCESS;
+
+FreeDevice:
+  FreePool (Device);
+
+  return Status;
+}
+
+VOID
+PopulateI2cInformation (
+  IN VOID
+  )
+{
+  UINT32 Index;
+
+  for (Index = 0; Index < ARRAY_SIZE (mI2cDesc); Index++) {
+mI2cDesc[Index].StartDesc.Desc = ACPI_ADDRESS_SPACE_DESCRIPTOR;
+mI2cDesc[Index].StartDesc.Len = sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) 
- 3;
+mI2cDesc[Index].StartDesc.ResType = ACPI_ADDRESS_SPACE_TYPE_MEM;
+mI2cDesc[Index].StartDesc.GenFlag = 0;
+mI2cDesc[Index].StartDesc.SpecificFlag = 0;
+mI2cDesc[Index].StartDesc.AddrSpaceGranularity = 32;
+mI2cDesc[Index].StartDesc.AddrRangeMin = BCM2836_I2C1_BASE_ADDRESS;
+mI2cDesc[Index].StartDesc.AddrRangeMax = 
mI2cDesc[Index].StartDesc.AddrRangeMin + BCM2836_I2C1_LENGTH;
+mI2cDesc[Index].StartDesc.AddrTranslationOffset = 0;
+mI2cDesc[Index].StartDesc.AddrLen = BCM2836_I2C1_LENGTH;
+
+mI2cDesc[Index].EndDesc = ACPI_END_TAG_DESCRIPTOR;
+  }
+}
+
+EFI_STATUS
+EFIAPI
+BcmI2CPlatformDxeEntryPoint (
+  IN EFI_HANDLE ImageHandle,
+  IN EFI_SYSTEM_TABLE   *SystemTable
+  )
+{
+  EFI_STATUS  Status;
+  EFI_HANDLE  Handle;
+
+  Handle = NULL;
+
+  PopulateI2cInformation ();
+
+  if (PcdGet32 (PcdHwRtc))
+  {
+DEBUG ((DEBUG_ERROR, "RTC I2C enable\n"));
+// If we don't register this, the second rtc won't get enabled
+// leaving the emulator in place.
+Status = RegisterDevice (,
+   [0], );
+ASSERT_EFI_ERROR (Status);
+
+//
+// Install the DS1307 I2C Master protocol on this handle so the RTC driver
+// can identify it as the I2C master it can invoke directly.
+//
+Status = gBS->InstallProtocolInterface (,
+,
+EFI_NATIVE_INTERFACE, NULL);
+ASSERT_EFI_ERROR (Status);
+  }  else  {
+DEBUG ((DEBUG_ERROR, "RTC I2C disabled\n"));
+  }
+
+  return EFI_SUCCESS;
+}
diff --git a/Platform/RaspberryPi/Drivers/BcmI2CPlatform/BcmI2CPlatform.inf 
b/Platform/RaspberryPi/Drivers/BcmI2CPlatform/BcmI2CPlatform.inf
new file mode 100644
index 00..aa5c1b51b2
--- /dev/null
+++ b/Platform/RaspberryPi/Drivers/BcmI2CPlatform/BcmI2CPlatform.inf
@@ -0,0 +1,54 @@
+## @file
+#
+#  Component description file for Bcm/Rpi I2C driver.
+#
+#  Copyright 2018-2020 NXP
+#  Sourced and reworked from edk2/NXP I2C stack
+#  Copyright 2022 Arm, Jeremy Linton
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION= 0x0001001A
+  BASE_NAME  = BcmI2CPlatformDxe
+  FILE_GUID  = 1a23fe23-39bc-4bee-859d-ecb5bbb60484
+  MODULE_TYPE= DXE_DRIVER
+  VERSION_STRING = 1.0
+  ENTRY_POINT= BcmI2CPlatformDxeEntryPoint
+
+[Sources]
+  BcmI2CPlatform.c
+
+[Packages]

[edk2-devel] [RFC 4/6] Silicon/Maxim: Fix runtime issues

2024-01-10 Thread Jeremy Linton
The Ds1307 is ideally a runtime RTC but its use of the I2C
protocol might make it better if it updated the I2C_MASTER_PROTOCOL
callbacks it uses, although... hmmm maybe think about this
a bit more.

Signed-off-by: Jeremy Linton 
---
 .../Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.c | 36 +--
 .../Library/Ds1307RtcLib/Ds1307RtcLib.inf |  7 +++-
 2 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/Silicon/Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.c 
b/Silicon/Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.c
index 444e011248..ede890448f 100644
--- a/Silicon/Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.c
+++ b/Silicon/Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.c
@@ -18,14 +18,40 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "Ds1307Rtc.h"
 
 STATIC VOID   *mDriverEventRegistration;
 STATIC EFI_HANDLE mI2cMasterHandle;
+STATIC EFI_EVENT  mRtcVirtualAddrChangeEvent;
 STATIC EFI_I2C_MASTER_PROTOCOL*mI2cMaster;
 
+
+/**
+  Fixup internal data so that EFI can be call in virtual mode.
+  Call the passed in Child Notify event and convert any pointers in
+  lib to virtual mode.
+
+  @param[in]Event   The Event that is being processed
+  @param[in]Context Event Context
+**/
+VOID
+EFIAPI
+LibRtcVirtualNotifyEvent (
+  IN EFI_EVENTEvent,
+  IN VOID *Context
+  )
+{
+  EfiConvertPointer (0x0, (VOID **)>SetBusFrequency);
+  EfiConvertPointer (0x0, (VOID **)>Reset);
+  EfiConvertPointer (0x0, (VOID **)>StartRequest);
+  EfiConvertPointer (0x0, (VOID **)>I2cControllerCapabilities);
+  EfiConvertPointer (0x0, (VOID **));
+}
+
+
 /**
   Read RTC register.
   Data Read-Slave Transmitter Mode
@@ -54,7 +80,7 @@ RtcRead (
 
   Req.OperationCount = 2;
 
-  Req.SetAddressOp.Flags = 0;
+  Req.SetAddressOp.Flags = 0; //I2C_FLAG_WRITE
   Req.SetAddressOp.LengthInBytes = sizeof (RtcRegAddr);
   Req.SetAddressOp.Buffer = 
 
@@ -98,7 +124,7 @@ RtcWrite (
   Buffer[0] = RtcRegAddr;
   Buffer[1] = Val;
 
-  Req.SetAddressOp.Flags = 0;
+  Req.SetAddressOp.Flags = 0; //I2C_FLAG_WRITE
   Req.SetAddressOp.LengthInBytes = sizeof (Buffer);
   Req.SetAddressOp.Buffer = Buffer;
 
@@ -375,5 +401,11 @@ LibRtcInitialize (
 NULL,
 );
 
+  Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_NOTIFY,
+  LibRtcVirtualNotifyEvent, NULL,
+  ,
+  );
+  ASSERT_EFI_ERROR (Status);
+
   return EFI_SUCCESS;
 }
diff --git a/Silicon/Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.inf 
b/Silicon/Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.inf
index b92f658bfc..9d8cf16d28 100644
--- a/Silicon/Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.inf
+++ b/Silicon/Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.inf
@@ -28,6 +28,9 @@
   UefiBootServicesTableLib
   UefiLib
 
+[Guids]
+  gEfiEventVirtualAddressChangeGuid
+
 [Protocols]
   gEfiI2cMasterProtocolGuid  ## CONSUMES
   gDs1307RealTimeClockLibI2cMasterProtocolGuid   ## CONSUMES
@@ -36,5 +39,7 @@
   gDs1307RtcLibTokenSpaceGuid.PcdI2cSlaveAddress
   gDs1307RtcLibTokenSpaceGuid.PcdI2cBusFrequency
 
+# 2.15 EDK II INF, .. the generic (i.e depex).. are not permitted
+# yet it ANDs this into the parent's depex
 [Depex]
-  gDs1307RealTimeClockLibI2cMasterProtocolGuid
+  gDs1307RealTimeClockLibI2cMasterProtocolGuid AND gEfiI2cMasterProtocolGuid
-- 
2.43.0



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




[edk2-devel] [RFC 2/6] Silicon/Bcm283x: Add core I2C drivers

2024-01-10 Thread Jeremy Linton
There are a number of I2C interfaces on the rpi, some of which are
available on the GPIO connector and are utilized by various HATs. In
particular we are interested in the RTCs (usually based on DS1037)
which are attached to GPIO2/3 which can be pin muxed to I2C #1.

This commit adds a basic runtime utilizable I2C Dxe in preparation
for a platform driver which can bind it to the ds1307 RTC driver.

Signed-off-by: Jeremy Linton 
---
 .../Broadcom/Drivers/I2cDxe/ComponentName.c   | 181 ++
 .../Broadcom/Drivers/I2cDxe/DriverBinding.c   | 237 ++
 Silicon/Broadcom/Drivers/I2cDxe/I2cDxe.c  | 309 ++
 Silicon/Broadcom/Drivers/I2cDxe/I2cDxe.h  |  55 
 Silicon/Broadcom/Drivers/I2cDxe/I2cDxe.inf|  56 
 5 files changed, 838 insertions(+)
 create mode 100644 Silicon/Broadcom/Drivers/I2cDxe/ComponentName.c
 create mode 100644 Silicon/Broadcom/Drivers/I2cDxe/DriverBinding.c
 create mode 100644 Silicon/Broadcom/Drivers/I2cDxe/I2cDxe.c
 create mode 100644 Silicon/Broadcom/Drivers/I2cDxe/I2cDxe.h
 create mode 100644 Silicon/Broadcom/Drivers/I2cDxe/I2cDxe.inf

diff --git a/Silicon/Broadcom/Drivers/I2cDxe/ComponentName.c 
b/Silicon/Broadcom/Drivers/I2cDxe/ComponentName.c
new file mode 100644
index 00..75b4fdb3fe
--- /dev/null
+++ b/Silicon/Broadcom/Drivers/I2cDxe/ComponentName.c
@@ -0,0 +1,181 @@
+/** @file
+
+  Copyright 2018-2019 NXP
+  Sourced and reworked from edk2/NXP I2C stack
+  Copyright 2022 Arm, Jeremy Linton
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "I2cDxe.h"
+
+STATIC EFI_UNICODE_STRING_TABLE mBcmI2cDriverNameTable[] = {
+  {
+"en",
+(CHAR16 *)L"Bcm I2C Driver"
+  },
+  { }
+};
+
+STATIC EFI_UNICODE_STRING_TABLE mBcmI2cControllerNameTable[] = {
+  {
+"en",
+(CHAR16 *)L"Bcm I2C Controller"
+  },
+  { }
+};
+
+/**
+  Retrieves a Unicode string that is the user readable name of the driver.
+
+  This function retrieves the user readable name of a driver in the form of a
+  Unicode string. If the driver specified by This has a user readable name in
+  the language specified by Language, then a pointer to the driver name is
+  returned in DriverName, and EFI_SUCCESS is returned. If the driver specified
+  by This does not support the language specified by Language,
+  then EFI_UNSUPPORTED is returned.
+
+  @param  This[in]  A pointer to the EFI_COMPONENT_NAME2_PROTOCOL 
or
+EFI_COMPONENT_NAME_PROTOCOL instance.
+
+  @param  Language[in]  A pointer to a Null-terminated ASCII string
+array indicating the language. This is the
+language of the driver name that the caller is
+requesting, and it must match one of the
+languages specified in SupportedLanguages. The
+number of languages supported by a driver is up
+to the driver writer. Language is specified
+in RFC 4646 or ISO 639-2 language code format.
+
+  @param  DriverName[out]   A pointer to the Unicode string to return.
+This Unicode string is the name of the
+driver specified by This in the language
+specified by Language.
+
+  @retval EFI_SUCCESS   The Unicode string for the Driver specified by
+This and the language specified by Language was
+returned in DriverName.
+
+  @retval EFI_INVALID_PARAMETER Language is NULL.
+
+  @retval EFI_INVALID_PARAMETER DriverName is NULL.
+
+  @retval EFI_UNSUPPORTED   The driver specified by This does not support
+the language specified by Language.
+
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+BcmI2cGetDriverName (
+  IN  EFI_COMPONENT_NAME2_PROTOCOL  *This,
+  IN  CHAR8 *Language,
+  OUT CHAR16**DriverName
+  )
+{
+  return LookupUnicodeString2 (Language,
+   This->SupportedLanguages,
+   mBcmI2cDriverNameTable,
+   DriverName,
+   FALSE);
+}
+
+/**
+  Retrieves a Unicode string that is the user readable name of the controller
+  that is being managed by a driver.
+
+  This function retrieves the user readable name of the controller specified by
+  ControllerHandle and ChildHandle in the form of a Unicode string. If the
+  driver specified by This has a user readable name in the language specified 
by
+  Language, then a pointer to the controller name is returned in 
ControllerName,
+  and EFI_SUCCESS is returned.  If the driver specified by This is not 
currently
+  managing the controller specified by ControllerHandle and ChildHandle,
+  then EFI_UNSUPPORTED is returned.  If the driver specified by 

[edk2-devel] [RFC 5/6] Platform/RasberryPi: Add I2C1 to uefi runtime memory map

2024-01-10 Thread Jeremy Linton
We intend to allow the OS to update the RTC after exit boot services.
THis means we need to assure that the I2C IO map is marked correctly.

Signed-off-by: Jeremy Linton 
---
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index 377ef438ff..fec8f63ea8 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -505,6 +505,7 @@ ApplyVariables (
   }
 
   if (mModelFamily == 4) {
+// add memoryspaces for the runtime flash/variable store
 Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo, 
BCM2836_SPI0_BASE_ADDRESS,
   SIZE_4KB, EFI_MEMORY_UC | 
EFI_MEMORY_RUNTIME);
 ASSERT_EFI_ERROR (Status);
@@ -517,6 +518,13 @@ ApplyVariables (
 Status = gDS->SetMemorySpaceAttributes (GPIO_BASE_ADDRESS,
 SIZE_4KB, 
EFI_MEMORY_UC|EFI_MEMORY_RUNTIME);
 
+// add memoryspace for the runtime rtc/i2c
+Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo, 
BCM2836_I2C1_BASE_ADDRESS,
+  SIZE_4KB, EFI_MEMORY_UC | 
EFI_MEMORY_RUNTIME);
+ASSERT_EFI_ERROR (Status);
+Status = gDS->SetMemorySpaceAttributes (BCM2836_I2C1_BASE_ADDRESS,
+SIZE_4KB, 
EFI_MEMORY_UC|EFI_MEMORY_RUNTIME);
+
 ASSERT_EFI_ERROR (Status);
   }
 
-- 
2.43.0



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




[edk2-devel] [RFC 6/6] Platform/RaspberryPi: Add menu and build options for HW RTC

2024-01-10 Thread Jeremy Linton
Now that the i2c drivers, config setup, and DS1307 driver are
in place, lets add the drivers to the build. We also add a menu
item to enable/disable it since the device I have:

https://www.amazon.com/Makerfire%C2%AE-Raspberry-Module-DS1307-Battery/dp/B00ZOXWHK4

Doesn't utilize the HAT protocol so it cannot be detected.

Signed-off-by: Jeremy Linton 
---
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 22 ++
 .../Drivers/ConfigDxe/ConfigDxe.inf   |  1 +
 .../Drivers/ConfigDxe/ConfigDxeHii.uni|  6 
 .../Drivers/ConfigDxe/ConfigDxeHii.vfr| 16 ++
 Platform/RaspberryPi/Include/ConfigVars.h |  4 +++
 Platform/RaspberryPi/RPi3/RPi3.dsc|  7 +
 Platform/RaspberryPi/RPi4/RPi4.dsc| 30 +++
 Platform/RaspberryPi/RPi4/RPi4.fdf| 19 
 Platform/RaspberryPi/RaspberryPi.dec  |  2 ++
 9 files changed, 107 insertions(+)

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index fec8f63ea8..0397941a06 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -319,6 +319,16 @@ SetupVariables (
   ASSERT_EFI_ERROR (Status);
 }
 
+Size = sizeof (UINT32);
+Status = gRT->GetVariable (L"HwRtc",
+   ,
+   NULL, , );
+if (EFI_ERROR (Status)) {
+  Status = PcdSet32S (PcdHwRtc, PcdGet32 (PcdHwRtc));
+  ASSERT_EFI_ERROR (Status);
+}
+
+
   } else {
 /*
  * Disable PCIe and XHCI
@@ -716,6 +726,18 @@ ApplyVariables (
 GpioPinFuncSet (33, GPIO_FSEL_ALT3);
   }
 
+  // Assure I2C1 is selected on header
+  if (PcdGet32 (PcdHwRtc)) {
+UINT32 ClockRate;
+DEBUG ((DEBUG_INFO, "Enable SDA1\n"));
+GpioPinFuncSet (2, GPIO_FSEL_ALT0);
+GpioPinFuncSet (3, GPIO_FSEL_ALT0);
+
+mFwProtocol->GetClockRate (RPI_MBOX_CLOCK_RATE_CORE, );
+ClockRate/=5; //50Khz slow it down a bit initially
+
+MmioWrite32 (BCM2836_I2C1_OFFSET + BCM2836_SOC_REGISTERS + 
BCM2835_I2C_DIV, ClockRate ); //was 5dc which assumes a 150Mhz clock, when we 
are usually at 500Mhz?
+  }
 }
 
 
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
index e422e5ba5c..4c213174ce 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
@@ -98,6 +98,7 @@
   gRaspberryPiTokenSpaceGuid.PcdMiniUartClockRate
   gRaspberryPiTokenSpaceGuid.PcdXhciReload
   gRaspberryPiTokenSpaceGuid.PcdEnableGpio
+  gRaspberryPiTokenSpaceGuid.PcdHwRtc
 
 [Depex]
   gPcdProtocolGuid AND gRaspberryPiFirmwareProtocolGuid
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
index fb06d46a61..8da143d519 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
@@ -72,6 +72,12 @@
 #string STR_ADVANCED_ENABLEGPIO_DISABLE #language en-US "Disabled"
 #string STR_ADVANCED_ENABLEGPIO_ENABLE  #language en-US "Enable"
 
+#string STR_ADVANCED_ENABLEHWRTC_PROMPT  #language en-US "Enable Hardware RTC"
+#string STR_ADVANCED_ENABLEHWRTC_HELP#language en-US "A DS1307 hardware 
real time clock is attached on I2C1."
+#string STR_ADVANCED_ENABLEHWRTC_DISABLE #language en-US "Disabled"
+#string STR_ADVANCED_ENABLEHWRTC_ENABLE  #language en-US "Enable"
+
+
 #string STR_ADVANCED_ASSET_TAG_PROMPT #language en-US "Asset Tag"
 #string STR_ADVANCED_ASSET_TAG_HELP   #language en-US "Set the system Asset 
Tag"
 
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
index 04eb0a15a2..b7146b7e8c 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
@@ -71,6 +71,11 @@ formset
   name  = EnableGpio,
   guid  = CONFIGDXE_FORM_SET_GUID;
 
+efivarstore ADVANCED_ENABLEHWRTC_VARSTORE_DATA,
+  attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | 
EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
+  name  = HwRtc,
+  guid  = CONFIGDXE_FORM_SET_GUID;
+
 efivarstore SYSTEM_TABLE_MODE_VARSTORE_DATA,
   attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | 
EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
   name  = SystemTableMode,
@@ -259,6 +264,17 @@ formset
 option text = STRING_TOKEN(STR_ADVANCED_ENABLEGPIO_ENABLE), value 
= 1, flags = 0;
   endoneof;
 endif;
+
+grayoutif ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_DT;
+  oneof varid = HwRtc.Value,
+  prompt  = STRING_TOKEN(STR_ADVANCED_ENABLEHWRTC_PROMPT),
+  help= STRING_TOKEN(STR_ADVANCED_ENABLEHWRTC_HELP),
+  flags   = NUMERIC_SIZE_4 | INTERACTIVE | 

[edk2-devel] [RFC 0/6] RasberryPi: RTC HAT support

2024-01-10 Thread Jeremy Linton
There are a number of Dallas 1307 based RTC HATs available for the RPi
that don't follow the HAT ID protocol but are simple, inexpensive and
widely available. Lets add an I2C driver and then allow the user to
enable them in the advanced menu. Once enabled and detected we then
dynamically swap them in to provide UEFI runtime RTC.

Its this latter bit of logic which seems to be unique to this platform
and probably should be closely reviewed.

Jeremy Linton (6):
  Silicon/Bcm283x: Document the I2C registers
  Silicon/Bcm283x: Add core I2C drivers
  Platform/RasberryPi: Create I2C driver bound to RTC
  Silicon/Maxim: Fix runtime issues
  Platform/RasberryPi: Add I2C1 to uefi runtime memory map
  Platform/RaspberryPi: Add menu and build options for HW RTC

 .../Drivers/BcmI2CPlatform/BcmI2CPlatform.c   | 127 +++
 .../Drivers/BcmI2CPlatform/BcmI2CPlatform.inf |  54 +++
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c |  30 ++
 .../Drivers/ConfigDxe/ConfigDxe.inf   |   1 +
 .../Drivers/ConfigDxe/ConfigDxeHii.uni|   6 +
 .../Drivers/ConfigDxe/ConfigDxeHii.vfr|  16 +
 Platform/RaspberryPi/Include/ConfigVars.h |   4 +
 Platform/RaspberryPi/RPi3/RPi3.dsc|   7 +
 Platform/RaspberryPi/RPi4/RPi4.dsc|  30 ++
 Platform/RaspberryPi/RPi4/RPi4.fdf|  19 ++
 Platform/RaspberryPi/RaspberryPi.dec  |   2 +
 .../Include/IndustryStandard/Bcm2836.h|  34 ++
 .../Broadcom/Drivers/I2cDxe/ComponentName.c   | 181 ++
 .../Broadcom/Drivers/I2cDxe/DriverBinding.c   | 237 ++
 Silicon/Broadcom/Drivers/I2cDxe/I2cDxe.c  | 309 ++
 Silicon/Broadcom/Drivers/I2cDxe/I2cDxe.h  |  55 
 Silicon/Broadcom/Drivers/I2cDxe/I2cDxe.inf|  56 
 .../Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.c |  36 +-
 .../Library/Ds1307RtcLib/Ds1307RtcLib.inf |   7 +-
 19 files changed, 1208 insertions(+), 3 deletions(-)
 create mode 100644 Platform/RaspberryPi/Drivers/BcmI2CPlatform/BcmI2CPlatform.c
 create mode 100644 
Platform/RaspberryPi/Drivers/BcmI2CPlatform/BcmI2CPlatform.inf
 create mode 100644 Silicon/Broadcom/Drivers/I2cDxe/ComponentName.c
 create mode 100644 Silicon/Broadcom/Drivers/I2cDxe/DriverBinding.c
 create mode 100644 Silicon/Broadcom/Drivers/I2cDxe/I2cDxe.c
 create mode 100644 Silicon/Broadcom/Drivers/I2cDxe/I2cDxe.h
 create mode 100644 Silicon/Broadcom/Drivers/I2cDxe/I2cDxe.inf

-- 
2.43.0



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




[edk2-devel] [RFC 1/6] Silicon/Bcm283x: Document the I2C registers

2024-01-10 Thread Jeremy Linton
Since we are now using I2C in edk2, its helpful if we document
the register names.

Signed-off-by: Jeremy Linton 
---
 .../Include/IndustryStandard/Bcm2836.h| 34 +++
 1 file changed, 34 insertions(+)

diff --git a/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h 
b/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h
index 55a446a86c..6dc8921346 100644
--- a/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h
+++ b/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h
@@ -97,6 +97,40 @@
 #define BCM2836_I2C2_BASE_ADDRESS   
(BCM2836_SOC_REGISTERS + BCM2836_I2C2_OFFSET)
 #define BCM2836_I2C2_LENGTH 0x0020
 
+#define BCM2836_I2C3_OFFSET 0x00205600
+#define BCM2836_I2C3_BASE_ADDRESS   
(BCM2836_SOC_REGISTERS + BCM2836_I2C3_OFFSET)
+#define BCM2836_I2C3_LENGTH 0x0020
+
+#define BCM2836_I2C4_OFFSET 0x00205800
+#define BCM2836_I2C4_BASE_ADDRESS   
(BCM2836_SOC_REGISTERS + BCM2836_I2C4_OFFSET)
+#define BCM2836_I2C4_LENGTH 0x0020
+
+#define BCM2836_I2C5_OFFSET 0x00205a00 //2711 
doc says 205a80?!
+#define BCM2836_I2C5_BASE_ADDRESS   
(BCM2836_SOC_REGISTERS + BCM2836_I2C5_OFFSET)
+#define BCM2836_I2C5_LENGTH 0x0020
+
+#define BCM2836_I2C6_OFFSET 0x00205c00
+#define BCM2836_I2C6_BASE_ADDRESS   
(BCM2836_SOC_REGISTERS + BCM2836_I2C6_OFFSET)
+#define BCM2836_I2C6_LENGTH 0x0020
+
+#define BCM2836_I2C20_OFFSET0x00f04500 //2711 
DCC0
+#define BCM2836_I2C20_BASE_ADDRESS  
(BCM2836_SOC_REGISTERS + BCM2836_I2C20_OFFSET)
+#define BCM2836_I2C20_LENGTH0x0020
+
+#define BCM2836_I2C21_OFFSET0x00f09500 //2711 
DCC1
+#define BCM2836_I2C21_BASE_ADDRESS  
(BCM2836_SOC_REGISTERS + BCM2836_I2C21_OFFSET)
+#define BCM2836_I2C21_LENGTH0x0020
+
+/* I2C register offsets */
+#define BCM2835_I2C_C  0x00
+#define BCM2835_I2C_S  0x04
+#define BCM2835_I2C_DLEN   0x08
+#define BCM2835_I2C_A  0x0c
+#define BCM2835_I2C_FIFO   0x10
+#define BCM2835_I2C_DIV0x14
+#define BCM2835_I2C_DEL0x18
+#define BCM2835_I2C_CLKT   0x1c
+
 #define BCM2836_SPI0_OFFSET 0x00204000
 #define BCM2836_SPI0_BASE_ADDRESS   
(BCM2836_SOC_REGISTERS + BCM2836_SPI0_OFFSET)
 #define BCM2836_SPI0_LENGTH 0x0020
-- 
2.43.0



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




[edk2-devel] [PATCH 3/7] Platform/RaspberryPi: Add constants for controlling SPI

2024-01-10 Thread Jeremy Linton
Add the #defines needed to access the SPI interface
documented in the BCM2711 Peripheral guide chapter 8.

Signed-off-by: Jeremy Linton 
---
 .../Include/IndustryStandard/Bcm2836.h| 34 +++
 1 file changed, 34 insertions(+)

diff --git a/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h 
b/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h
index a930c64af3..55a446a86c 100644
--- a/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h
+++ b/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h
@@ -109,6 +109,40 @@
 #define BCM2836_SPI2_LENGTH 0x0040
 #define BCM2836_SPI2_BASE_ADDRESS   
(BCM2836_SOC_REGISTERS + BCM2836_SPI2_OFFSET)
 
+/* SPI register offsets */
+#define BCM2835_SPI_CS  0x00
+#define BCM2835_SPI_FIFO0x04
+#define BCM2835_SPI_CLK 0x08
+#define BCM2835_SPI_DLEN0x0c
+#define BCM2835_SPI_LTOH0x10
+#define BCM2835_SPI_DC  0x14
+
+/* Bitfields in CS */
+#define BCM2835_SPI_CS_LEN_LONG 0x0200
+#define BCM2835_SPI_CS_DMA_LEN  0x0100
+#define BCM2835_SPI_CS_CSPOL2   0x0080
+#define BCM2835_SPI_CS_CSPOL1   0x0040
+#define BCM2835_SPI_CS_CSPOL0   0x0020
+#define BCM2835_SPI_CS_RXF  0x0010
+#define BCM2835_SPI_CS_RXR  0x0008
+#define BCM2835_SPI_CS_TXD  0x0004
+#define BCM2835_SPI_CS_RXD  0x0002
+#define BCM2835_SPI_CS_DONE 0x0001
+#define BCM2835_SPI_CS_LEN  0x2000
+#define BCM2835_SPI_CS_REN  0x1000
+#define BCM2835_SPI_CS_ADCS 0x0800
+#define BCM2835_SPI_CS_INTR 0x0400
+#define BCM2835_SPI_CS_INTD 0x0200
+#define BCM2835_SPI_CS_DMAEN0x0100
+#define BCM2835_SPI_CS_TA   0x0080
+#define BCM2835_SPI_CS_CSPOL0x0040
+#define BCM2835_SPI_CS_CLEAR_RX 0x0020
+#define BCM2835_SPI_CS_CLEAR_TX 0x0010
+#define BCM2835_SPI_CS_CPOL 0x0008
+#define BCM2835_SPI_CS_CPHA 0x0004
+#define BCM2835_SPI_CS_CS_100x0002
+#define BCM2835_SPI_CS_CS_010x0001
+
 /* dma constants */
 #define BCM2836_DMA0_OFFSET 0x7000
 #define BCM2836_DMA0_BASE_ADDRESS   
(BCM2836_SOC_REGISTERS + BCM2836_DMA0_OFFSET)
-- 
2.43.0



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




[edk2-devel] [PATCH 5/7] Platform/RaspberryPi: Add SPI/GPIO to memory map

2024-01-10 Thread Jeremy Linton
A large reason for using the SPI flash on this platform is that
it can be updated without OS interference at rutime. In order for
that to happen we need both the SPI, as well as the GPIO
which is used to change the pinmux from the PWM device to SPI added
to the UEFI memory map as being used by the runtime service.

Signed-off-by: Jeremy Linton 
---
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c| 16 
 1 file changed, 16 insertions(+)

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index 151f3cd801..377ef438ff 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -504,6 +504,22 @@ ApplyVariables (
 DEBUG ((DEBUG_INFO, "Current CPU speed is %u MHz\n", Rate / FREQ_1_MHZ));
   }
 
+  if (mModelFamily == 4) {
+Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo, 
BCM2836_SPI0_BASE_ADDRESS,
+  SIZE_4KB, EFI_MEMORY_UC | 
EFI_MEMORY_RUNTIME);
+ASSERT_EFI_ERROR (Status);
+Status = gDS->SetMemorySpaceAttributes (BCM2836_SPI0_BASE_ADDRESS,
+SIZE_4KB, 
EFI_MEMORY_UC|EFI_MEMORY_RUNTIME);
+
+Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo, 
GPIO_BASE_ADDRESS,
+  SIZE_4KB, EFI_MEMORY_UC | 
EFI_MEMORY_RUNTIME);
+ASSERT_EFI_ERROR (Status);
+Status = gDS->SetMemorySpaceAttributes (GPIO_BASE_ADDRESS,
+SIZE_4KB, 
EFI_MEMORY_UC|EFI_MEMORY_RUNTIME);
+
+ASSERT_EFI_ERROR (Status);
+  }
+
   if (mModelFamily >= 4 && PcdGet32 (PcdRamMoreThan3GB) != 0 &&
   PcdGet32 (PcdRamLimitTo3GB) == 0) {
 UINT64 SystemMemorySize;
-- 
2.43.0



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




[edk2-devel] [PATCH 6/7] Platform/RaspberryPi: Allow pin function selection at runtime

2024-01-10 Thread Jeremy Linton
Update GpioLib slightly so that we can change the GPIO pin
muxing at runtime. For the moment only the GpioPinFuncGet/Set()
routines are used at runtime, and only by the Variable service.

Signed-off-by: Jeremy Linton 
---
 .../Broadcom/Bcm283x/Include/Library/GpioLib.h   |  6 ++
 .../Broadcom/Bcm283x/Library/GpioLib/GpioLib.c   | 16 ++--
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h 
b/Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h
index 1f7d2204e0..79765be4fb 100644
--- a/Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h
+++ b/Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h
@@ -45,4 +45,10 @@ GpioSetPull (
   IN  UINTN   Pud
 );
 
+VOID
+GpioSetupRuntime (
+  VOID
+);
+
+
 #endif /* __GPIO_LIB__ */
diff --git a/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c 
b/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c
index eaf53e5369..fc1f928e6b 100644
--- a/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c
+++ b/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c
@@ -15,10 +15,22 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 
+
+STATIC EFI_PHYSICAL_ADDRESS GpioGfpSel0 = GPIO_GPFSEL0;
+
+VOID
+GpioSetupRuntime (
+  VOID
+  )
+{
+  EfiConvertPointer (0x0, (VOID**));
+}
+
 STATIC
 VOID
 GpioFSELModify (
@@ -30,7 +42,7 @@ GpioFSELModify (
   UINT32 Val;
   EFI_PHYSICAL_ADDRESS Reg;
 
-  Reg = RegIndex * sizeof (UINT32) + GPIO_GPFSEL0;
+  Reg = RegIndex * sizeof (UINT32) + GpioGfpSel0;
 
   ASSERT (Reg <= GPIO_GPFSEL5);
   ASSERT ((~ModifyMask & FunctionMask) == 0);
@@ -77,7 +89,7 @@ GpioPinFuncGet (
 
   RegIndex = Pin / 10;
   SelIndex = Pin % 10;
-  Reg = RegIndex * sizeof (UINT32) + GPIO_GPFSEL0;
+  Reg = RegIndex * sizeof (UINT32) + GpioGfpSel0;
 
   Val = MmioRead32 (Reg);
   Val >>= SelIndex * GPIO_FSEL_BITS_PER_PIN;
-- 
2.43.0



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




[edk2-devel] [PATCH 7/7] Platform/RaspberryPi: Add SPI flash variable store.

2024-01-10 Thread Jeremy Linton
The RPi4 has a 512KB SPI flash, which depending on RPi
and firmware revision has 300-180K free. We can use this
storage to persist variables when the OS is running or
over firmware upgrades. The problem is that the SPI is pin
mux'ed with the PWM audio, so we want to leave the PWM
configured for OS use. And of course there is the problem
of sharing the GPIO block with OS's that are aware of it.
Hence a previous patch set which moves the GPIO and some of
the low level i2c/etc devices into its own SSDT, and
disables them by default.

This patch, adds a few SPI access functions directly to
the variable store rather than creating another runtime
service since the early boot ordering is critical. These
functions are of the form ReadSpi(), WriteSpi(),
DisableSpiWp(), etc all with Spi in the name. On top of that
a few "Flash" routines are created which provide high level
functions for reading/writing and walking the portion of the
SPI flash we use to clone the variable store region.
Importantly WalkFlashVolume() walks the entire SPI flash
region, which has a simple header structure containing
filename+len for each region of the flash, to return how
much of the capacity is being utilized by the existing
bootloader/etc firmware.

So, if this is a RPi4, and there is sufficient space, and
that space doesn't have a valid varstore header we erase
it and flush the RPI's varstore region to the SPI. Then
we note its starting offset in mFvInstance->FlashOffset.
>From then on, writes to the EFI_FW_VOL_BLOCK_DEVICE are
written to both the RAM copy as well as the SPI flash. If
the empty region has a valid header we read the entire
region overtop of the one being passed as part of the RPi's
BL33, and continue as above.

At ready to boot we re-enable the LDO, and then during the
dump vars check, we check for DT or the GPIO being enabled
and disable the runtime SPI updates because we can't be sure
of what the OS might be doing with the GPIO. The dual ACPI
and DT mode, leaves it enabled (if GPIO is disabled) so
care should be taken.

Now, one of the problems here is that with the LDO enabled
any SPI accesses can be heard over the speakers as pops,
buzzes, or scratchy tones. This is happening even without
this patch because TFA and/or the rpi low level firmware
doesn't itself assure the LDO is disabled during resets, so
the early SoC startup is quite noisy. We add to this, but
alongside that a couple fairly trivial TFA patchs, to mute
it before reset, and again assure its off before releasing
to us solve a large part of this problem. That said, this
can now happen during runtime as well. Generally the OS's
aren't doing a lot of variable updates, but when they do
its generally barely noticable clicks since we aren't going
through the long process of muting/unmuting the LDO which
itself causes a pop.

So, this patch fixes a whole bunch of bugs on github that
exist because the variable store isn't persisted. It also
fixes a rather large bug in the existing variable store
code caused by the FaultTolerantWriteDxe erasing the entire
variable store region when it garbage collects during startup.
That latter bug is the result of FvbGetLbaAddress reading
recently erased data from the VolumeHeader before its been
recreated, and results in random UEFI crashes.

Signed-off-by: Jeremy Linton 
---
 .../Drivers/VarBlockServiceDxe/FvbInfo.c  |   8 +-
 .../VarBlockServiceDxe/VarBlockService.c  | 669 +-
 .../VarBlockServiceDxe/VarBlockService.h  |  10 +
 .../VarBlockServiceDxe/VarBlockServiceDxe.c   |  69 +-
 .../VarBlockServiceDxe/VarBlockServiceDxe.inf |   7 +
 Platform/RaspberryPi/RPi4/RPi4.dsc|   5 +-
 6 files changed, 743 insertions(+), 25 deletions(-)

diff --git a/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/FvbInfo.c 
b/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/FvbInfo.c
index 0e0c108dba..ee18f327e6 100644
--- a/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/FvbInfo.c
+++ b/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/FvbInfo.c
@@ -11,12 +11,7 @@
 #include 
 #include 
 #include 
-
-typedef struct {
-  UINT64  FvLength;
-  EFI_FIRMWARE_VOLUME_HEADER  FvbInfo;
-  EFI_FV_BLOCK_MAP_ENTRY  End[1];
-} EFI_FVB_MEDIA_INFO;
+#include "VarBlockService.h"
 
 EFI_FVB_MEDIA_INFO  mPlatformFvbMediaInfo[] = {
   //
@@ -38,6 +33,7 @@ EFI_FVB_MEDIA_INFO  mPlatformFvbMediaInfo[] = {
   FixedPcdGet32 (PcdNvStorageEventLogSize),
   EFI_FVH_SIGNATURE,
   EFI_FVB2_MEMORY_MAPPED |
+EFI_FVB2_STICKY_WRITE |
 EFI_FVB2_READ_ENABLED_CAP |
 EFI_FVB2_READ_STATUS |
 EFI_FVB2_WRITE_ENABLED_CAP |
diff --git a/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/VarBlockService.c 
b/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/VarBlockService.c
index 572309439a..3b83d7ead8 100644
--- a/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/VarBlockService.c
+++ b/Platform/RaspberryPi/Drivers/VarBlockServiceDxe/VarBlockService.c
@@ -7,17 +7,28 @@
  *
  **/
 

[edk2-devel] [PATCH 1/7] Platform/RaspberryPi: Move GPIO/SPI/I2C to SSDT

2024-01-10 Thread Jeremy Linton
The UEFI firmware uses the GPIO port for the fan and
real soon now the runtime SPI variable store. As such
we need to be able to either isolate those devices from
the OS or we risk clashing with OS's that reconfigure
the GPIO pins. Ideally we would just rip this out
and use _DSM() or just individual device power
on/off methods to adjust the GPIO pins when needed.

For now, lets leave it since windows at least knows
about it. In the future we will decide whether the
firmware is controlling something (SPI!) based on
whether the user has enabled the GPIO block.

Signed-off-by: Jeremy Linton 
---
 .../RaspberryPi/AcpiTables/AcpiTables.inf |   1 +
 Platform/RaspberryPi/AcpiTables/Dsdt.asl  |   7 -
 Platform/RaspberryPi/AcpiTables/GpuDevs.asl   | 126 --
 Platform/RaspberryPi/AcpiTables/SsdtGpio.asl  | 159 ++
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c |   6 +
 5 files changed, 166 insertions(+), 133 deletions(-)
 create mode 100644 Platform/RaspberryPi/AcpiTables/SsdtGpio.asl

diff --git a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf 
b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
index da2a6db85f..3894d00565 100644
--- a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
+++ b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
@@ -40,6 +40,7 @@
   SsdtThermal.asl
   Xhci.asl
   Pci.asl
+  SsdtGpio.asl
 
 [Packages]
   ArmPkg/ArmPkg.dec
diff --git a/Platform/RaspberryPi/AcpiTables/Dsdt.asl 
b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
index b594d50bdf..08acc81d57 100644
--- a/Platform/RaspberryPi/AcpiTables/Dsdt.asl
+++ b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
@@ -21,13 +21,6 @@
 
 #include "AcpiTables.h"
 
-#define BCM_ALT0 0x4
-#define BCM_ALT1 0x5
-#define BCM_ALT2 0x6
-#define BCM_ALT3 0x7
-#define BCM_ALT4 0x3
-#define BCM_ALT5 0x2
-
 //
 // The ASL compiler does not support argument arithmetic in functions
 // like QWordMemory (). So we need to instantiate dummy qword regions
diff --git a/Platform/RaspberryPi/AcpiTables/GpuDevs.asl 
b/Platform/RaspberryPi/AcpiTables/GpuDevs.asl
index 9750dc25c0..3399f0fc9a 100644
--- a/Platform/RaspberryPi/AcpiTables/GpuDevs.asl
+++ b/Platform/RaspberryPi/AcpiTables/GpuDevs.asl
@@ -203,56 +203,6 @@ Device (VCSM)
   }
 }
 
-// Description: GPIO
-Device (GPI0)
-{
-  Name (_HID, "BCM2845")
-  Name (_CID, "BCM2845")
-  Name (_UID, 0x0)
-  Name (_CCA, 0x0)
-  Method (_STA)
-  {
-Return(0xf)
-  }
-  Name (RBUF, ResourceTemplate ()
-  {
-MEMORY32FIXED (ReadWrite, 0, GPIO_LENGTH, RMEM)
-Interrupt (ResourceConsumer, Level, ActiveHigh, Shared)
-{
-  BCM2386_GPIO_INTERRUPT0, BCM2386_GPIO_INTERRUPT1,
-  BCM2386_GPIO_INTERRUPT2, BCM2386_GPIO_INTERRUPT3
-}
-  })
-  Method (_CRS, 0x0, Serialized)
-  {
-MEMORY32SETBASE (RBUF, RMEM, RBAS, GPIO_OFFSET)
-Return (^RBUF)
-  }
-}
-
-// Description: I2C
-Device (I2C1)
-{
-  Name (_HID, "BCM2841")
-  Name (_CID, "BCM2841")
-  Name (_UID, 0x1)
-  Name (_CCA, 0x0)
-  Method (_STA)
-  {
-Return(0xf)
-  }
-  Name (RBUF, ResourceTemplate ()
-  {
-MEMORY32FIXED (ReadWrite, 0, BCM2836_I2C1_LENGTH, RMEM)
-Interrupt (ResourceConsumer, Level, ActiveHigh, Shared) { 
BCM2836_I2C1_INTERRUPT }
-PinFunction (Exclusive, PullUp, BCM_ALT0, "\\_SB.GDV0.GPI0", 0, 
ResourceConsumer, , ) { 2, 3 }
-  })
-  Method (_CRS, 0x0, Serialized)
-  {
-MEMORY32SETBASE (RBUF, RMEM, RBAS, BCM2836_I2C1_OFFSET)
-Return (^RBUF)
-  }
-}
 
 // I2C2 is the HDMI DDC connection
 Device (I2C2)
@@ -278,81 +228,6 @@ Device (I2C2)
   }
 }
 
-// SPI
-Device (SPI0)
-{
-  Name (_HID, "BCM2838")
-  Name (_CID, "BCM2838")
-  Name (_UID, 0x0)
-  Name (_CCA, 0x0)
-  Method (_STA)
-  {
-Return (0xf)
-  }
-  Name (RBUF, ResourceTemplate ()
-  {
-MEMORY32FIXED (ReadWrite, 0, BCM2836_SPI0_LENGTH, RMEM)
-Interrupt (ResourceConsumer, Level, ActiveHigh, Shared) { 
BCM2836_SPI0_INTERRUPT }
-PinFunction (Exclusive, PullDown, BCM_ALT0, "\\_SB.GDV0.GPI0", 0, 
ResourceConsumer, , ) { 9, 10, 11 } // MISO, MOSI, SCLK
-PinFunction (Exclusive, PullUp, BCM_ALT0, "\\_SB.GDV0.GPI0", 0, 
ResourceConsumer, , ) { 8 } // CE0
-PinFunction (Exclusive, PullUp, BCM_ALT0, "\\_SB.GDV0.GPI0", 0, 
ResourceConsumer, , ) { 7 } // CE1
-  })
-
-  Method (_CRS, 0x0, Serialized)
-  {
-MEMORY32SETBASE (RBUF, RMEM, RBAS, BCM2836_SPI0_OFFSET)
-Return (^RBUF)
-  }
-}
-
-Device (SPI1)
-{
-  Name (_HID, "BCM2839")
-  Name (_CID, "BCM2839")
-  Name (_UID, 0x1)
-  Name (_CCA, 0x0)
-  Name (_DEP, Package() { \_SB.GDV0.RPIQ })
-  Method (_STA)
-  {
-Return (0xf)
-  }
-  Name (RBUF, ResourceTemplate ()
-  {
-MEMORY32FIXED (ReadWrite, 0, BCM2836_SPI1_LENGTH, RMEM)
-Interrupt (ResourceConsumer, Level, ActiveHigh, Shared,) { 
BCM2836_SPI1_INTERRUPT }
-PinFunction (Exclusive, PullDown, BCM_ALT4, "\\_SB.GDV0.GPI0", 0, 
ResourceConsumer, , ) { 19, 20, 21 } // MISO, MOSI, SCLK
-PinFunction (Exclusive, PullDown, BCM_ALT4, "\\_SB.GDV0.GPI0", 0, 
ResourceConsumer, , ) { 16 } // CE2
-  })
-
-  Method (_CRS, 0x0, 

[edk2-devel] [PATCH 2/7] Platform/RaspberryPi: Add menu item to enable/disable GPIO

2024-01-10 Thread Jeremy Linton
Now that the GPIO devices are in their own SSDT lets add
a menu item for the rpi4 to enable/disable it. For the
rpi3 the SSDT is always exported.

Signed-off-by: Jeremy Linton 
---
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c   | 17 -
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf |  1 +
 .../Drivers/ConfigDxe/ConfigDxeHii.uni  |  5 +
 .../Drivers/ConfigDxe/ConfigDxeHii.vfr  | 15 +++
 Platform/RaspberryPi/Include/ConfigVars.h   |  4 
 Platform/RaspberryPi/RPi3/RPi3.dsc  |  6 ++
 Platform/RaspberryPi/RPi4/RPi4.dsc  |  7 +++
 Platform/RaspberryPi/RaspberryPi.dec|  1 +
 8 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index dd01e9a8dc..151f3cd801 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -309,12 +309,27 @@ SetupVariables (
   }
 
 }
+
+Size = sizeof (UINT32);
+Status = gRT->GetVariable (L"EnableGpio",
+   ,
+   NULL, , );
+if (EFI_ERROR (Status)) {
+  Status = PcdSet32S (PcdEnableGpio, PcdGet32 (PcdEnableGpio));
+  ASSERT_EFI_ERROR (Status);
+}
+
   } else {
 /*
  * Disable PCIe and XHCI
  */
 Status = PcdSet32S (PcdXhciPci, 0);
 ASSERT_EFI_ERROR (Status);
+/*
+ * Enable GPIO
+ */
+Status = PcdSet32S (PcdEnableGpio, 1);
+ASSERT_EFI_ERROR (Status);
   }
 
   Size = sizeof (AssetTagVar);
@@ -842,7 +857,7 @@ STATIC CONST NAMESPACE_TABLES SdtTables[] = {
 #endif
   {
 SIGNATURE_64 ('R', 'P', 'I', '3', 'G', 'P', 'I', 'O'),
-0,
+PcdToken (PcdEnableGpio),
 0,
 NULL
   },
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
index 475e645537..e422e5ba5c 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
@@ -97,6 +97,7 @@
   gRaspberryPiTokenSpaceGuid.PcdXhciPci
   gRaspberryPiTokenSpaceGuid.PcdMiniUartClockRate
   gRaspberryPiTokenSpaceGuid.PcdXhciReload
+  gRaspberryPiTokenSpaceGuid.PcdEnableGpio
 
 [Depex]
   gPcdProtocolGuid AND gRaspberryPiFirmwareProtocolGuid
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
index 8130638876..fb06d46a61 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
@@ -67,6 +67,11 @@
 #string STR_ADVANCED_XHCIRELOAD_DISABLE #language en-US "Disabled"
 #string STR_ADVANCED_XHCIRELOAD_RELOAD  #language en-US "Reload"
 
+#string STR_ADVANCED_ENABLEGPIO_PROMPT  #language en-US "Export GPIO devices 
to OS"
+#string STR_ADVANCED_ENABLEGPIO_HELP#language en-US "OS can see the GPIO 
device and some low level SPI and I2C interfaces. Enabling this option will 
disable runtime variable support."
+#string STR_ADVANCED_ENABLEGPIO_DISABLE #language en-US "Disabled"
+#string STR_ADVANCED_ENABLEGPIO_ENABLE  #language en-US "Enable"
+
 #string STR_ADVANCED_ASSET_TAG_PROMPT #language en-US "Asset Tag"
 #string STR_ADVANCED_ASSET_TAG_HELP   #language en-US "Set the system Asset 
Tag"
 
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
index f13b70711d..04eb0a15a2 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
@@ -66,6 +66,11 @@ formset
   name  = XhciReload,
   guid  = CONFIGDXE_FORM_SET_GUID;
 
+efivarstore ADVANCED_ENABLEGPIO_VARSTORE_DATA,
+  attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | 
EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
+  name  = EnableGpio,
+  guid  = CONFIGDXE_FORM_SET_GUID;
+
 efivarstore SYSTEM_TABLE_MODE_VARSTORE_DATA,
   attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | 
EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
   name  = SystemTableMode,
@@ -244,6 +249,16 @@ formset
 endoneof;
   endif;
 endif;
+
+grayoutif ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_DT;
+  oneof varid = EnableGpio.Value,
+prompt  = STRING_TOKEN(STR_ADVANCED_ENABLEGPIO_PROMPT),
+help= STRING_TOKEN(STR_ADVANCED_ENABLEGPIO_HELP),
+flags   = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
+option text = STRING_TOKEN(STR_ADVANCED_ENABLEGPIO_DISABLE), value 
= 0, flags = DEFAULT;
+option text = STRING_TOKEN(STR_ADVANCED_ENABLEGPIO_ENABLE), value 
= 1, flags = 0;
+  endoneof;
+endif;
 #endif
 string varid = AssetTag.AssetTag,
 prompt  = STRING_TOKEN(STR_ADVANCED_ASSET_TAG_PROMPT),
diff --git 

[edk2-devel] [PATCH 4/7] Platform/RaspberryPi: Add mailbox cmd to control audio amp

2024-01-10 Thread Jeremy Linton
The lower level firmware can enable/disable a LDO audio
amp, which allows us to mute/unmute audio output while
the firmware is running.

Signed-off-by: Jeremy Linton 
---
 .../Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c   | 60 ++-
 .../Include/IndustryStandard/RpiMbox.h|  1 +
 .../Include/Protocol/RpiFirmware.h|  7 +++
 3 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c 
b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
index 4edec0ad04..ea0ac1eefe 100644
--- a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
+++ b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
@@ -1532,6 +1532,63 @@ RpiFirmwareNotifyGpioSetCfg (
   return Status;
 }
 
+#pragma pack()
+typedef struct {
+  UINT32   State;
+} RPI_FW_SET_LDO_REG_TAG;
+
+typedef struct {
+  RPI_FW_BUFFER_HEAD   BufferHead;
+  RPI_FW_TAG_HEAD  TagHead;
+  RPI_FW_SET_LDO_REG_TAG   TagBody;
+  UINT32   EndTag;
+} RPI_FW_SET_LDO_REG_CMD;
+#pragma pack()
+
+
+STATIC
+EFI_STATUS
+EFIAPI
+RpiFirmwareSetLdoRegState (
+  IN UINTN State
+  )
+{
+  RPI_FW_SET_LDO_REG_CMD   *Cmd;
+  EFI_STATUS   Status;
+  UINT32   Result;
+
+  if (!AcquireSpinLockOrFail ()) {
+DEBUG ((DEBUG_ERROR, "%a: failed to acquire spinlock\n", __func__));
+return EFI_DEVICE_ERROR;
+  }
+
+  Cmd = mDmaBuffer;
+  ZeroMem (Cmd, sizeof (*Cmd));
+
+  Cmd->BufferHead.BufferSize  = sizeof (*Cmd);
+  Cmd->BufferHead.Response= 0;
+  Cmd->TagHead.TagId  = RPI_MBOX_SET_LDO_REGULATOR;
+  Cmd->TagHead.TagSize= sizeof (Cmd->TagBody);
+  Cmd->TagBody.State  = State;
+
+  Cmd->TagHead.TagValueSize   = 0;
+  Cmd->EndTag = 0;
+
+  Status = MailboxTransaction (Cmd->BufferHead.BufferSize, 
RPI_MBOX_VC_CHANNEL, );
+
+  if (EFI_ERROR (Status) ||
+  Cmd->BufferHead.Response != RPI_MBOX_RESP_SUCCESS) {
+DEBUG ((DEBUG_ERROR,
+  "%a: mailbox  transaction error: Status == %r, Response == 0x%x\n",
+  __func__, Status, Cmd->BufferHead.Response));
+  }
+
+  ReleaseSpinLock ();
+
+  return Status;
+}
+
+
 STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL mRpiFirmwareProtocol = {
   RpiFirmwareSetPowerState,
   RpiFirmwareGetMacAddress,
@@ -1557,7 +1614,8 @@ STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL 
mRpiFirmwareProtocol = {
   RpiFirmwareNotifyXhciReset,
   RpiFirmwareGetCurrentClockState,
   RpiFirmwareSetClockState,
-  RpiFirmwareNotifyGpioSetCfg
+  RpiFirmwareNotifyGpioSetCfg,
+  RpiFirmwareSetLdoRegState
 };
 
 /**
diff --git a/Platform/RaspberryPi/Include/IndustryStandard/RpiMbox.h 
b/Platform/RaspberryPi/Include/IndustryStandard/RpiMbox.h
index 551c2b82e5..f36aaafaf8 100644
--- a/Platform/RaspberryPi/Include/IndustryStandard/RpiMbox.h
+++ b/Platform/RaspberryPi/Include/IndustryStandard/RpiMbox.h
@@ -92,6 +92,7 @@
 #define RPI_MBOX_NOTIFY_REBOOT0x00030048
 #define RPI_MBOX_GET_POE_HAT_VAL  0x00030049
 #define RPI_MBOX_SET_POE_HAT_VAL  0x00030050
+#define RPI_MBOX_SET_LDO_REGULATOR0x00030056
 #define RPI_MBOX_NOTIFY_XHCI_RESET0x00030058
 
 #define RPI_MBOX_SET_CLOCK_STATE  0x00038001
diff --git a/Platform/RaspberryPi/Include/Protocol/RpiFirmware.h 
b/Platform/RaspberryPi/Include/Protocol/RpiFirmware.h
index c48bb6e434..175894e37a 100644
--- a/Platform/RaspberryPi/Include/Protocol/RpiFirmware.h
+++ b/Platform/RaspberryPi/Include/Protocol/RpiFirmware.h
@@ -171,6 +171,12 @@ EFI_STATUS
   UINTN State
   );
 
+typedef
+EFI_STATUS
+(EFIAPI *SET_LDO_REGULATOR) (
+  UINTN State
+  );
+
 typedef struct {
   SET_POWER_STATESetPowerState;
   GET_MAC_ADDRESSGetMacAddress;
@@ -197,6 +203,7 @@ typedef struct {
   GET_CLOCK_STATEGetClockState;
   SET_CLOCK_STATESetClockState;
   GPIO_SET_CFG   SetGpioConfig;
+  SET_LDO_REGULATOR  SetLdoRegState;
 } RASPBERRY_PI_FIRMWARE_PROTOCOL;
 
 extern EFI_GUID gRaspberryPiFirmwareProtocolGuid;
-- 
2.43.0



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




[edk2-devel] [PATCH 0/7] RaspberryPi: SPI Variables, again

2024-01-10 Thread Jeremy Linton
There is a 512KB SPI flash EEPROM on the RPi4, which is used to hold
the early boot firmware. Versions of this this firmware which predate
the network recovery have 160-300+KB unused, which is a perfect place
to drop a UEFI variable store.

This code scans the region and makes the decision at runtime if there
is sufficient space available and falls back to updating the SD/USB
firmware image if not. This behavior is now enabled/disabled at
compile time with -DSPI_VARIABLES=1/0

This is a rehash of a previous set I posted from a few years back, and
I'm largely re-posting it because its still useful for people who are
willing to back that firmware down to gain the functionality, as well
as fixing a crash caused by the variable store code garbage collecting
the storage region when it starts to get full

Beyond providing some example SPI programming on the platform, it also
moves I2C/SPI/GPIO devices which are debatably usable on Linux in
ACPI mode into its own SSDT.

Jeremy Linton (7):
  Platform/RaspberryPi: Move GPIO/SPI/I2C to SSDT
  Platform/RaspberryPi: Add menu item to enable/disable GPIO
  Platform/RaspberryPi: Add constants for controlling SPI
  Platform/RaspberryPi: Add mailbox cmd to control audio amp
  Platform/RaspberryPi: Add SPI/GPIO to memory map
  Platform/RaspberryPi: Allow pin function selection at runtime
  Platform/RaspberryPi: Add SPI flash variable store.

 .../RaspberryPi/AcpiTables/AcpiTables.inf |   1 +
 Platform/RaspberryPi/AcpiTables/Dsdt.asl  |   7 -
 Platform/RaspberryPi/AcpiTables/GpuDevs.asl   | 126 
 Platform/RaspberryPi/AcpiTables/SsdtGpio.asl  | 159 +
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c |  37 +
 .../Drivers/ConfigDxe/ConfigDxe.inf   |   1 +
 .../Drivers/ConfigDxe/ConfigDxeHii.uni|   5 +
 .../Drivers/ConfigDxe/ConfigDxeHii.vfr|  15 +
 .../Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c   |  60 +-
 .../Drivers/VarBlockServiceDxe/FvbInfo.c  |   8 +-
 .../VarBlockServiceDxe/VarBlockService.c  | 669 +-
 .../VarBlockServiceDxe/VarBlockService.h  |  10 +
 .../VarBlockServiceDxe/VarBlockServiceDxe.c   |  69 +-
 .../VarBlockServiceDxe/VarBlockServiceDxe.inf |   7 +
 Platform/RaspberryPi/Include/ConfigVars.h |   4 +
 .../Include/IndustryStandard/RpiMbox.h|   1 +
 .../Include/Protocol/RpiFirmware.h|   7 +
 Platform/RaspberryPi/RPi3/RPi3.dsc|   6 +
 Platform/RaspberryPi/RPi4/RPi4.dsc|  12 +-
 Platform/RaspberryPi/RaspberryPi.dec  |   1 +
 .../Include/IndustryStandard/Bcm2836.h|  34 +
 .../Bcm283x/Include/Library/GpioLib.h |   6 +
 .../Bcm283x/Library/GpioLib/GpioLib.c |  16 +-
 23 files changed, 1100 insertions(+), 161 deletions(-)
 create mode 100644 Platform/RaspberryPi/AcpiTables/SsdtGpio.asl

-- 
2.43.0



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




[edk2-devel] [PATCH 1/5] Platform/RaspberryPi/DualSerialPortLib: Always configure the pl011

2024-01-10 Thread Jeremy Linton
The rpi's config.txt controls which uart (pl011, or miniuart) is
selected as the console. TFA and edk2 follow its lead, but if the
miniuart is selected as the primary and the machine is booted in ACPI
mode the baud/etc is never configured for the pl011. The linux kernel
won't reconfigure it either as its listed as a "SBSA" uart, so it
simply won't work.

This re-enables BT on the pl011 in ACPI mode, and it somewhat starts
to work again.

Signed-off-by: Jeremy Linton 
---
 .../DualSerialPortLib/DualSerialPortLib.c | 37 +++
 1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c 
b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
index d2f983bf0a..79545d93d6 100644
--- a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
+++ b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
@@ -76,6 +76,8 @@ SerialPortInitialize (
   EFI_PARITY_TYPE Parity;
   UINT8   DataBits;
   EFI_STOP_BITS_TYPE  StopBits;
+  RETURN_STATUS   Ret;
+  UINTN   Timeout;

   //
   // First thing we need to do is determine which of PL011 or miniUART is 
selected
@@ -85,23 +87,27 @@ SerialPortInitialize (
 UsePl011UartSet = TRUE;
   }

-  if (UsePl011Uart) {
-BaudRate = FixedPcdGet64 (PcdUartDefaultBaudRate);
+  // always init the pl011 on the pi4, linux expects a SBSA uart to be at 
115200
+  // this means we need to set the baud/etc even if we arn't using it as a 
console
+  if ((UsePl011Uart) || (RPI_MODEL == 4)) {
 ReceiveFifoDepth = 0; // Use default FIFO depth
+BaudRate = FixedPcdGet64 (PcdUartDefaultBaudRate);
 Parity = (EFI_PARITY_TYPE)FixedPcdGet8 (PcdUartDefaultParity);
 DataBits = FixedPcdGet8 (PcdUartDefaultDataBits);
 StopBits = (EFI_STOP_BITS_TYPE) FixedPcdGet8 (PcdUartDefaultStopBits);

-return PL011UartInitializePort (
- PL011_UART_REGISTER_BASE,
- PL011UartClockGetFreq(),
- ,
- ,
- ,
- ,
- 
- );
-  } else {
+Ret = PL011UartInitializePort (
+   PL011_UART_REGISTER_BASE,
+   PL011UartClockGetFreq(),
+   ,
+   ,
+   ,
+   ,
+   
+   );
+  }
+
+  if (!UsePl011Uart) {
 SerialRegisterBase = MINI_UART_REGISTER_BASE;
 Divisor = SerialPortGetDivisor (PcdGet32 (PcdSerialBaudRate));

@@ -127,7 +133,8 @@ SerialPortInitialize (
 // Wait for the serial port to be ready.
 // Verify that both the transmit FIFO and the shift register are empty.
 //
-while ((SerialPortReadRegister (SerialRegisterBase, R_UART_LSR) & 
(B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) != (B_UART_LSR_TEMT | B_UART_LSR_TXRDY));
+Timeout = 1000;
+while (((SerialPortReadRegister (SerialRegisterBase, R_UART_LSR) & 
(B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) != (B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) 
&& (Timeout--));

 //
 // Configure baud rate
@@ -158,9 +165,9 @@ SerialPortInitialize (
 // Put Modem Control Register(MCR) into its reset state of 0x00.
 //
 SerialPortWriteRegister (SerialRegisterBase, R_UART_MCR, 0x00);
-
-return RETURN_SUCCESS;
+Ret = RETURN_SUCCESS;
   }
+  return Ret;
 }

 /**
--
2.43.0



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




[edk2-devel] [PATCH 5/5] Platform/RaspberryPi: Update PCIe MMIO window for DT

2024-01-10 Thread Jeremy Linton
Since we are updating the DT memory map and telling it how
we have configured the PCIe, there isn't a reason for moving the
MMIO window. In fact this appears to fix OpenBSD+DT as well as
it makes the linux XHCI reset sequence happier.

Signed-off-by: Jeremy Linton 
---
 Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c 
b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
index dd4fc0a05e..e6942df6a3 100644
--- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
+++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
@@ -375,6 +375,30 @@ SyncPcie (
 return EFI_NOT_FOUND;
   }

+  // move the MMIO window too
+  DmaRanges[0] = cpu_to_fdt32 (0x0200); //non prefech 32-bit
+  DmaRanges[1] = cpu_to_fdt32 (0x); //bus addr @ 0x0f800
+  DmaRanges[2] = cpu_to_fdt32 (0xf800);
+  DmaRanges[3] = cpu_to_fdt32 (0x0006); //cpu addr @ 0x6
+  DmaRanges[4] = cpu_to_fdt32 (0x);
+  DmaRanges[5] = cpu_to_fdt32 (0x);
+  DmaRanges[6] = cpu_to_fdt32 (0x0400); // len = 0x4000 
+
+  DEBUG ((DEBUG_INFO, "%a: Updating PCIe ranges\n",  __func__));
+
+  /*
+   * Match dma-ranges with the EDK2+ACPI setup we are using.  This works
+   * around a failure in Linux and OpenBSD to reset the PCIe/XHCI correctly
+   * when in DT mode.
+   */
+  Retval = fdt_setprop (mFdtImage, Node, "ranges",
+DmaRanges,  sizeof DmaRanges);
+  if (Retval != 0) {
+DEBUG ((DEBUG_ERROR, "%a: failed to locate PCIe MMIO 'ranges' property 
(%d)\n",
+  __func__, Retval));
+return EFI_NOT_FOUND;
+  }
+
   if (PcdGet32 (PcdXhciReload) != 1) {
 return EFI_SUCCESS;
   }
--
2.43.0



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




[edk2-devel] [PATCH 4/5] Platform/RaspberryPi: Give the user control over the XHCI mailbox

2024-01-10 Thread Jeremy Linton
Its a complete tossup whether removing the mailbox call after we have
set up the XHCI works for a given kernel+distro in DT mode. So lets
give users which want to try DT the option of flipping this on/off.

Users that don't want to have to deal with DT, can use ACPI.

Signed-off-by: Jeremy Linton 
---
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 10 ++
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf   |  1 +
 .../Drivers/ConfigDxe/ConfigDxeHii.uni|  5 +
 .../Drivers/ConfigDxe/ConfigDxeHii.vfr| 15 +++
 Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c  |  4 
 Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf|  1 +
 Platform/RaspberryPi/RPi3/RPi3.dsc|  6 ++
 Platform/RaspberryPi/RPi4/RPi4.dsc|  7 +++
 Platform/RaspberryPi/RaspberryPi.dec  |  1 +
 9 files changed, 50 insertions(+)

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index 3dcf2bac0d..2484787982 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -298,6 +298,16 @@ SetupVariables (
 Status = PcdSet32S (PcdXhciPci, 1);
 ASSERT_EFI_ERROR (Status);
   }
+
+  Size = sizeof (UINT32);
+  Status = gRT->GetVariable (L"XhciReload",
+ ,
+ NULL, , );
+  if (EFI_ERROR (Status)) {
+Status = PcdSet32S (PcdXhciReload, PcdGet32 (PcdXhciReload));
+ASSERT_EFI_ERROR (Status);
+  }
+
 }
   } else {
 /*
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
index 6f6e8f42ac..475e645537 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
@@ -96,6 +96,7 @@
   gRaspberryPiTokenSpaceGuid.PcdUartInUse
   gRaspberryPiTokenSpaceGuid.PcdXhciPci
   gRaspberryPiTokenSpaceGuid.PcdMiniUartClockRate
+  gRaspberryPiTokenSpaceGuid.PcdXhciReload

 [Depex]
   gPcdProtocolGuid AND gRaspberryPiFirmwareProtocolGuid
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
index 5ec17072c3..8130638876 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
@@ -62,6 +62,11 @@
 #string STR_ADVANCED_XHCIPCI_XHCI #language en-US "XHCI"
 #string STR_ADVANCED_XHCIPCI_PCIE #language en-US "PCIe"

+#string STR_ADVANCED_XHCIRELOAD_PROMPT  #language en-US "DT Reload XHCI 
firmware"
+#string STR_ADVANCED_XHCIRELOAD_HELP#language en-US "OS should reload XHCI 
firmware on reset"
+#string STR_ADVANCED_XHCIRELOAD_DISABLE #language en-US "Disabled"
+#string STR_ADVANCED_XHCIRELOAD_RELOAD  #language en-US "Reload"
+
 #string STR_ADVANCED_ASSET_TAG_PROMPT #language en-US "Asset Tag"
 #string STR_ADVANCED_ASSET_TAG_HELP   #language en-US "Set the system Asset 
Tag"

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
index f668b7a553..f13b70711d 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
@@ -61,6 +61,11 @@ formset
   name  = XhciPci,
   guid  = CONFIGDXE_FORM_SET_GUID;

+efivarstore ADVANCED_XHCIPCI_VARSTORE_DATA,
+  attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | 
EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
+  name  = XhciReload,
+  guid  = CONFIGDXE_FORM_SET_GUID;
+
 efivarstore SYSTEM_TABLE_MODE_VARSTORE_DATA,
   attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | 
EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
   name  = SystemTableMode,
@@ -228,6 +233,16 @@ formset
   option text = STRING_TOKEN(STR_ADVANCED_XHCIPCI_PCIE), value = 
1, flags = 0;
 endoneof;
   endif;
+
+  grayoutif ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_ACPI;
+oneof varid = XhciReload.Value,
+  prompt  = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_PROMPT),
+  help= STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_HELP),
+  flags   = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
+  option text = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_DISABLE), 
value = 0, flags = DEFAULT;
+  option text = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_RELOAD), 
value = 1, flags = 0;
+endoneof;
+  endif;
 endif;
 #endif
 string varid = AssetTag.AssetTag,
diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c 
b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
index cbbc2ad30d..dd4fc0a05e 100644
--- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
+++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
@@ -375,6 +375,10 @@ SyncPcie (
 return 

[edk2-devel] [PATCH 0/5] Platform/RaspberryPi: Various minor fixes

2024-01-10 Thread Jeremy Linton
This includes a change to always initialize the PL011 to the
configured baud (which should be 115200 for the SBSA UART), which
fixes linux's assumption that SBSA UARTs are pre-programmed for
115200. This in turn (re)enables the PL011 when the console is on the
miniuart per the config.txt file.

Also included is another spin with the DT/XHCI reset patch which puts
removal of the DT node that causes linux to reset the XHCI controller,
as well as an additional patch that updates the DT to match the PCIe
MMIO window we have programmed. This cures much of the problem with
the PCIe/XHCI configuration when booted in DT mode on linux.

There is also a few menu visibility/section tweaks to assure ACPI/DT
specific settings show up at the appropriate time.

As well as a minor fix to work around a bogus compiler warning.

Jeremy Linton (5):
  Platform/RaspberryPi/DualSerialPortLib: Always configure the pl011
  Silicon/Broadcom/BcmGenetDxe: Suppress some bogus compiler warnings
  Platform/RaspberryPi: Cleanup menu visibility
  Platform/RaspberryPi: Give the user control over the XHCI mailbox
  Platform/RaspberryPi: Update PCIe MMIO window for DT

 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 10 +
 .../Drivers/ConfigDxe/ConfigDxe.inf   |  1 +
 .../Drivers/ConfigDxe/ConfigDxeHii.uni|  5 +++
 .../Drivers/ConfigDxe/ConfigDxeHii.vfr| 21 +--
 Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c  | 28 ++
 .../RaspberryPi/Drivers/FdtDxe/FdtDxe.inf |  1 +
 .../DualSerialPortLib/DualSerialPortLib.c | 37 +++
 Platform/RaspberryPi/RPi3/RPi3.dsc|  6 +++
 Platform/RaspberryPi/RPi4/RPi4.dsc|  7 
 Platform/RaspberryPi/RaspberryPi.dec  |  1 +
 .../Drivers/Net/BcmGenetDxe/GenericPhy.c  |  2 +
 .../Drivers/Net/BcmGenetDxe/SimpleNetwork.c   |  3 ++
 12 files changed, 104 insertions(+), 18 deletions(-)

--
2.43.0



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




[edk2-devel] [PATCH 2/5] Silicon/Broadcom/BcmGenetDxe: Suppress some bogus compiler warnings

2024-01-10 Thread Jeremy Linton
Some recent GCC revisions will throw warnings about values being used
before being initialized. But in the case where the lack of initialization
is the result of the called function returning error status the EFI_ERROR()
macro/error seems to confuse the compiler about the fact that the value
is then never used.

So, while the code appears to be fine, lets just zero the variables
anyway to make the compiler happy.

Signed-off-by: Jeremy Linton 
---
 Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenericPhy.c| 2 ++
 Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenericPhy.c 
b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenericPhy.c
index 9e5d30fafd..2d5f70170e 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenericPhy.c
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenericPhy.c
@@ -381,6 +381,8 @@ GenericPhyUpdateConfig (
   BOOLEAN LinkUp;

   Status = GenericPhyGetLinkStatus (Phy);
+  Speed = 0;
+  Duplex = 0;
   LinkUp = EFI_ERROR (Status) ? FALSE : TRUE;

   if (Phy->LinkUp != LinkUp) {
diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c 
b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
index 3b51a86d65..7a7c398b1f 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
@@ -731,6 +731,9 @@ GenetSimpleNetworkReceive (
   UINT8   *Frame;
   UINTN   FrameLength;

+  DescIndex = 0;
+  FrameLength = 0;
+
   if (This == NULL || Buffer == NULL) {
 DEBUG ((DEBUG_ERROR, "%a: Invalid parameter (missing handle or buffer)\n",
   __FUNCTION__));
--
2.43.0



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




[edk2-devel] [PATCH 3/5] Platform/RaspberryPi: Cleanup menu visibility

2024-01-10 Thread Jeremy Linton
Lets allow some of these options to change when the
system is in ACPI+DT mode. Plus the fan temp should
be disabled when ACPI isn't enabled.

Signed-off-by: Jeremy Linton 
---
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
index e8bf16312d..f668b7a553 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
@@ -196,7 +196,7 @@ formset
 endoneof;

 #if (RPI_MODEL == 4)
-grayoutif NOT ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_ACPI;
+grayoutif ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_DT;
   oneof varid = FanOnGpio.Enabled,
   prompt  = STRING_TOKEN(STR_ADVANCED_FANONGPIO_PROMPT),
   help= STRING_TOKEN(STR_ADVANCED_FANONGPIO_HELP),
@@ -207,7 +207,7 @@ formset
   endoneof;
 endif;

-grayoutif ideqval FanOnGpio.Enabled == 0;
+grayoutif ideqval FanOnGpio.Enabled == 0 OR ideqval 
SystemTableMode.Mode == SYSTEM_TABLE_MODE_DT;
   numeric varid = FanTemp.Value,
   prompt  = STRING_TOKEN(STR_ADVANCED_FANTEMP_PROMPT),
   help= STRING_TOKEN(STR_ADVANCED_FANTEMP_HELP),
@@ -219,7 +219,7 @@ formset
 endif;

 suppressif ideqval XhciPci.Value == 2;
-  grayoutif NOT ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_ACPI;
+  grayoutif ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_DT;
 oneof varid = XhciPci.Value,
   prompt  = STRING_TOKEN(STR_ADVANCED_XHCIPCI_PROMPT),
   help= STRING_TOKEN(STR_ADVANCED_XHCIPCI_HELP),
--
2.43.0



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




Re: [edk2-devel] [RFC PATCH v1 00/20] DynamicTablesPkg: Prepare to add RISC-V support

2024-01-10 Thread Jeshua Smith via groups.io
> > It looks like instead of moving the common code to
> EObjNameSpaceStandard namespace or a new (Arch? Common?) namespace,
> you're renaming the entire EObjNameSpaceArm namespace to
> EObjNameSpaceArch. It seems to me that if ARM code vs. common code is
> being separated out, then the EObjNameSpaceArm namespace should
> continue to be used for the ARM-specific code and a common namespace
> should be used for the common code.
> 
> I agree. I started with separating common things into new common space and
> create one for risc-v. However, I dropped that approach for two reasons.
> 
> 1) The commit "b2bbe3df5470 DynamicTablesPkg: Remove PPTT ID structure
> from ACPI 6.4 generator" when removed one of the enums from
> ArmObjectID, didn't change the other values for other enums but reserved the
> removed one. So, I thought there may be some assumptions which will break
> if the enum value changes.

I'm not familiar with why that was done. Hopefully someone else can comment. I 
do know that 
edk2/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
 has arrays (StdNamespaceObjectParser and ArmNamespaceObjectParser) that need 
to be kept in sync with all of the namespace enums, but other than that I'm not 
aware of any places that need to be changes when the enums are changed.

> 2) DynamicPlatformRepositoryInfo structure has ArmCmObjList and
> ArmCmObjArray. With separate spaces for Arm, RiscV and Common, list
> management needs some redesign and I was not sure it is worth it.
> 
> Hence, I thought a single list of all possible Obj Ids for all architectures 
> and
> common things would be a good trade off. But I can go back to that approach
> in v2 if above issues are fine.

Hopefully ARM can give input on the best direction before you make more 
changes. The DynamicPlatRepo currently only supports the ARM namespace, but 
comments such as "only Arm objects are supported for now." (line 144) seem to 
imply that support for more namespaces was considered.


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




Re: [edk2-devel] Memory Attribute for depex section

2024-01-10 Thread Pedro Falcato
On Wed, Jan 10, 2024 at 1:45 PM Laszlo Ersek  wrote:
>
> (+ Andrew)
>
> On 1/10/24 14:09, Laszlo Ersek wrote:
>
> > I think that keeping the depex section read-only is valuable, so I'd
> > rule out #2. I'd also not start with option #1 -- copying the depex to
> > heap memory, just so this optimization can succeed. I'd simply start by
> > removing the optimization, and measuring how much driver dispatch slows
> > down in practice, on various platforms.
> >
> > Can you try this? (I have only build-tested and "uncrustified" it.)
> >
> > The patch removes the EFI_DEP_REPLACE_TRUE handling altogether, plus it
> > CONST-ifies the Iterator pointer (which points into the DEPEX section),
> > so that the compiler catch any possible accesses at *build time* that
> > would write to the write-protected DEPEX memory area.
>
> On a tangent: the optimization in question highlights a more general
> problem, namely that the DXE (and possibly MM/SMM) protocol databases
> are considered slow, for some access patterns.
>
> Edk2 implements those protocol databases with linked lists, where lookup
> costs O(n) operations (average and worst cases). And protocol lookups
> are quite frequent (for example, in depex evaluations, they could be
> considered "particularly frequent").
>
> (1) The "Tasks" wiki page mentions something *similar* (but not the
> same); see
>
> https://github.com/tianocore/tianocore.github.io/wiki/Tasks#datahub--gcd-scalability
>
> The description is: "The DXE core's DataHub and GCD (Global Coherency
> Domain) layers don't scale well as the number of data items gets large,
> since they are based on simple linked lists. Find better data structures."

How large do they usually get? What's the worst case?

>
> The same might apply more or less to the protocol database implementation.
>
> (2) More to the point, Andrew Fish reported earlier that at Apple, they
> had rewritten the DXE protocol database, using the red-black tree
> OrderedCollectionLib that I had contributed previously to edk2 -- and
> they saw significant performance improvements.
>
> So upstreaming that feature to edk2 could be very valuable. (Red-black
> trees have O(log(n)) time cost (worst case) for lookup, insertion and
> deletion, and O(n) cost for iterating through the whole data structure.)

FWIW, can we do better than an RB tree? They're notoriously cache unfriendly...

>
> Let me see if I can find the bugzilla ticket...
>
> Ah, I got it. Apologies, I misremembered: Andrew's comment was not about
> the protocol database, but about the handle database. Here it is:
>
> https://bugzilla.tianocore.org/show_bug.cgi?id=988#c7
>
> (the BZ is still CONFIRMED btw...)
>
> Still, I think it must be related in a way. Namely, an EFI handle exists
> if and only if at least one protocol interface is installed on it. If
> you uninstall the last protocol interface from a handle, then the handle
> is destroyed -- in fact that's the *only* way to destroy a handle, to my
> understanding. See EFI_BOOT_SERVICES.UninstallProtocolInterface() in the
> UEFI spec: "If the last protocol interface is removed from a handle, the
> handle is freed and is no longer valid". Furthermore, calling
> InstallProtocolInterface() and InstallMultipleProtocolInterfaces() is
> how one *creates* new handles.
>
> So given how handles and protocol interfaces are conceptually
> interlinked, an rbtree-based protocol DB might have to maintain multiple
> rbtrees internally (for the ability to search the database quickly with
> different types of "keys"). I don't have a design ready in my mind at
> all (I'm not that familiar with the *current*, list-based implementation
> to begin with!). Upstreaming Apple's (experimental?) code could prove
> very helpful.

Ok, some thoughts:

For the handle database, if we made EFI_HANDLE an integer instead, we
could very easily use something similar to a radix tree (see Linux's
xarray). This would provide O(log(n)) lookups and insertion with a
much higher fan-out, without needing CoreValidateHandle's awful O(n)
lookup every time it gets an EFI_HANDLE. You'd just look up the handle
in the tree and if NULL, it's invalid. [1]

For the protocol database, you'd replace the linked list with a simple
hashtable, hashed by protocol. Something as simple as LIST_ENTRY
mProtocolHashtable[64]; would probably be enough to fan out most of
the problems (I think? How many different protocols are there?)

-- 
Pedro

[1] One could be wary of doing this lookup for every EFI_HANDLE
instead of having the pointer directly. But this is much more
efficient than needing to iterate a linked list to validate a pointer.
Considering an xarray-like radix tree as previously described, two
levels with a fan-out of 64 could describe indices 0 - 4096 (64 * 64),
which is much more efficient than chasing through pointers (worst
case) 4096 times until we find the handle.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113541): 

Re: [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg: Initialise serial port early in StandaloneMmEntryPoint

2024-01-10 Thread Brian J. Johnson

On 1/10/24 11:16, levi.yun wrote:

Hi, Laszlo.

This will keep initing the serial port upon every API call until the
global variable becomes writeable, and then the next API call will init
the serial port for one last time, and also prevent further page table
checks.

The CheckWritable() function is an implementation detail. In the DXE
phase, it could be implemented (I think?) with the
GetMemorySpaceDescriptor() DXE service, or perhaps even the
EFI_MEMORY_ATTRIBUTE_PROTOCOL.GetMemoryAttributes() UEFI protocol member
function. In the standalone MM core, CheckWritable() could walk the page
tables explicitly. The idea is, either way, to *predict* whether writing
to "mInitialized" would trap. >> I think it wouldn't proper, to DxeCore and 
StMM too.

IIUC,  before CoreInitializeMemoryServices is called, we couldn't use
that method in case DxeCore.
And the problem now I face is also StMM before populating memory
information (done in LibConstructor).



Now I think that speculative / out of order execution could actually
trigger the trap *before* GlobalsWriteable is calculated; however, I
think such a trap should be architecturally hidden (i.e., invisible). I
think at worst we could need a compiler barrier (maybe throw in some
"volatile" for GlobalsWriteable and mInitialized), so that the
*compiler* not try to reorder the accesses. But even that sounds like a
stretch.

Agree if we develop CheckPerm??

Currently, (In my narrow thinking) there is no more generic solution
than create new interface SerialPortInitilizeEarly.

Am I missing?

Many Thanks.


Sincerely,
Levi.


Levi,

FWIW I prefer your original approach:  explicitly call 
SerialPortInitialize() early enough that you don't need to worry about 
output occurring before that point.  Then you can also use a 
SerialPortLib instance which assumes that this has already been done and 
doesn't try to re-initialize the port, which saves some overhead. 
Constructors in DebugLib, SerialPortLib, and other very low-level 
libraries are problematic due to the issue you ran in to, so it seems 
best to just avoid them altogether.


Ard didn't want a SerialPortInitialize() call directly in the 
all-platform StandaloneMmCore _ModuleEntryPoint() function, which is 
understandable.  So perhaps you could either:


1. Propose a platform-specific callout at that point and a library class 
to implement it, with an empty instance for general use and your own 
platform-specific instance which calls SerialPortInitialize().


or

2. Write your own platform-specific version of StandaloneMmCoreEntryPoint.

--
Brian J. Johnson
Enterprise X86 Lab
Hewlett Packard Enterprise



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




[edk2-devel] TianoCore Community Meeting - call for topics

2024-01-10 Thread Michael D Kinney
Any topics for the TianoCore Community meeting this week?

Mike


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




Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Fix raw strings containing valid escape characters

2024-01-10 Thread Rebecca Cran

On 1/9/2024 11:52 PM, Huang, Yanbo wrote:


@Gao, Liming  @'Rebecca Cran' 



Could you please help to merge this PR?

BaseTools: Fix raw strings containing valid escape characters by 
YuweiChen1110 · Pull Request #5238 · tianocore/edk2 (github.com) 



Thanks in advance!


It's been merged.


--

Rebecca Cran



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




Re: [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg: Initialise serial port early in StandaloneMmEntryPoint

2024-01-10 Thread levi.yun

Hi, Laszlo.

This will keep initing the serial port upon every API call until the
global variable becomes writeable, and then the next API call will init
the serial port for one last time, and also prevent further page table
checks.

The CheckWritable() function is an implementation detail. In the DXE
phase, it could be implemented (I think?) with the
GetMemorySpaceDescriptor() DXE service, or perhaps even the
EFI_MEMORY_ATTRIBUTE_PROTOCOL.GetMemoryAttributes() UEFI protocol member
function. In the standalone MM core, CheckWritable() could walk the page
tables explicitly. The idea is, either way, to *predict* whether writing
to "mInitialized" would trap.

I think it wouldn't proper, to DxeCore and StMM too.
IIUC,  before CoreInitializeMemoryServices is called, we couldn't use
that method in case DxeCore.
And the problem now I face is also StMM before populating memory
information (done in LibConstructor).



Now I think that speculative / out of order execution could actually
trigger the trap *before* GlobalsWriteable is calculated; however, I
think such a trap should be architecturally hidden (i.e., invisible). I
think at worst we could need a compiler barrier (maybe throw in some
"volatile" for GlobalsWriteable and mInitialized), so that the
*compiler* not try to reorder the accesses. But even that sounds like a
stretch.

Agree if we develop CheckPerm??

Currently, (In my narrow thinking) there is no more generic solution
than create new interface SerialPortInitilizeEarly.

Am I missing?

Many Thanks.


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


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




Re: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache Disable should not be set by default in CR0

2024-01-10 Thread West, Catharine
Disabling cache by default results in violation of BTG protections (if BTG 
enabled).
 
BIOS cannot assume that cache is disabled before it executes as ACM may be 
required to enable NEM.

Whatever solution needs to be done here cannot evict ACM-enabled NEM.

Why is boot time increasing?  In this failing case was ACM executed / cache 
enabled by ACM?  If not, then CD should be 0 by hardware default right?

thanks
Catharine

-Original Message-
From: Xu, Min M  
Sent: Tuesday, January 9, 2024 11:52 PM
To: devel@edk2.groups.io; Ni, Ray ; Wu, MingliangX 

Cc: Yao, Jiewen ; Xue, Shengfeng 
; Dong, Eric ; Kumar, Rahul R 
; kra...@redhat.com; De, Debkumar 
; West, Catharine ; Xu, Min M 

Subject: RE: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache Disable 
should not be set by default in CR0

This patch causes a regression when launching a vm guest with below command:

$ /usr/libexec/qemu-kvm  \
-name guestVM1 -machine q35 -accel kvm -m 10240 -smp 8 -cpu host -monitor pty \ 
-drive format=raw,file=/home/tdvf/centos-stream-9.img  \ -bios 
/home/tdvf/OVMF.fd \ -nic user,hostfwd=tcp::-:22 -nographic \ -object 
iommufd,id=iommufd0 \ -device 
intel-iommu,caching-mode=on,dma-drain=on,x-scalable-mode="modern",x-pasid-mode=true,device-iotlb=on,iommufd=iommufd0
 \ -device 
vfio-pci,sysfsdev=/sys/bus/dsa/devices/vdev0.0,iommufd=iommufd0,bypass-iommu=false

Commit e8aa4c6546 (this patch has been merged) clear the CD bit in CR0 when 
transferring from real16 mode to 32bit protect mode. After the patch is 
applied,  it costs about 60s in DecompressMemFvs@SecMain.c. 

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Ni, Ray
> Sent: Thursday, August 3, 2023 4:14 PM
> To: devel@edk2.groups.io; Ni, Ray ; Xue, Shengfeng 
> ; Dong, Eric ; 
> Kumar, Rahul R ; kra...@redhat.com; De, 
> Debkumar ; West, Catharine 
> 
> Cc: Wu, MingliangX 
> Subject: Re: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache 
> Disable should not be set by default in CR0
> 
> The patch resolves an issue in Boot Guard enabled system that NEM is 
> already enabled by Boot Guard, disabling cache evicts all cache 
> content which is unexpected.
> 
> Reviewed-by: Ray Ni 
> 
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of Ni, 
> > Ray
> > Sent: Wednesday, July 26, 2023 5:56 PM
> > To: Xue, Shengfeng ; 
> > devel@edk2.groups.io; Dong, Eric ; Kumar, Rahul 
> > R ; kra...@redhat.com; De, Debkumar 
> > ; West, Catharine 
> > Cc: Wu, MingliangX 
> > Subject: Re: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: 
> > Cache Disable should not be set by default in CR0
> >
> > This patch is not right.
> >
> > Intel SDM explicitly says the initial CR0 value is 6000_0010. CD bit is set.
> >
> > So the ResetVector code that still sets CD bit should be good.
> >
> > If you are facing NEM enable failure, can you change your NEM enable 
> > logic to explicitly clear CD bit instead of changing here?
> >
> > Thanks,
> > Ray
> >
> >
> > > -Original Message-
> > > From: xueshengfeng 
> > > Sent: Wednesday, July 26, 2023 5:48 PM
> > > To: devel@edk2.groups.io; Dong, Eric ; Ni, 
> > > Ray ; Kumar, Rahul R ; 
> > > kra...@redhat.com; De, Debkumar ; West, 
> > > Catharine 
> > > Cc: Wu, MingliangX ; Wu
> > > Subject: [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache Disable 
> > > should not be set by default in CR0
> > >
> > > From: "Wu, MingliangX" 
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4511
> > >
> > > With 64 bit build we are seeing the CD in control register CR 0 set.
> > > This causes the NEM to disabled for some specific bios profiles.
> > >
> > > Cc: Eric Dong 
> > > Cc: Ray Ni 
> > > Cc: Rahul Kumar 
> > > Cc: Gerd Hoffmann 
> > > Cc: Debkumar De 
> > > Cc: Catharine West 
> > > Signed-off-by: Wu, Mingliang 
> > > ---
> > >  UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
> > > b/UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
> > > index f59fc6ead4ba..4af2e875c31c 100644
> > > --- a/UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
> > > +++ b/UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
> > > @@ -7,7 +7,7 @@
> > >  ;
> > >
> > > ;-
> > > --
> > > ---
> > >
> > > -%define SEC_DEFAULT_CR0  0x4023
> > > +%define SEC_DEFAULT_CR0  0x0023
> > >  %define SEC_DEFAULT_CR4  0x640
> > >
> > >  BITS16
> > > --
> > > 2.26.2.windows.1
> > >
> >
> >
> >
> >
> >
> 
> 
> 
> 
> 



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




Re: [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg: Initialise serial port early in StandaloneMmEntryPoint

2024-01-10 Thread Laszlo Ersek
On 1/10/24 17:13, levi.yun wrote:
>> My personal conclusion in that thread was [1], and correspondingly,
>> commit 5087a0773645 ("ArmVirtPkg/FdtPL011SerialPortLib: initialize
>> implicitly", 2023-10-07). In the end, the only tractable solution was to
>> initialize the serial port (hardware, and library instance) exactly
>> once, in (a) the constructor, or (b) the explicit SerialPortInitialize()
>> call, or (c) any SerialPortLib API, whichever occurred first. (And (a)
>> and (b) can be coalesced, because SerialPortInitialize() can be marked
>> as the constructor for the lib instance.)
>>
>> [1]
>> http://mid.mail-archive.com/542db9e1-cd28-27a2-3a98-5b0c85cd7c79@redhat.com
>>  https://edk2.groups.io/g/devel/message/109235
>>
>> Laszlo
>>
> In my personal thinking, It's better to make new interface like
> 
> RETURN_STATUS
> EFIAPI
> SerialPortInitializeEarly (
> VOID
>   );
> 
> to solve this problem.
> 
> Because, It makes a memory permission fault
> when we call SerialPortInitialize like
> ArmVirtPkg/Library/FdtPL011SerialPortLib
> where try to modify global variable.
> 
> At the _ModuleEntryPoint of StandAloneMmCore which is FIRST entry from TF-A
> All of Image area is mapped as RO+X, so before load StandaloneMmCore,
> We couldn't write global variable.
> 
> the purposes of above interface are:
>     - Initalize serial port to use in early environment only (like
> StandAloneMmCore Entry Point) where couldn't write global variable by
> static information (FixedPcd).
>     - It presume that all setting configured by it will be overwritten
> by SerialPortInitialize.

This is not a scalable solution (assuming I understand your proposal
right). It would require the SerialPortLib *class* (i.e., header) to
grow a new API called SerialPortInitializeEarly(), and then all existent
SerialPortLib *instances* -- and there are too many of them -- would
have to implement that function, even if the new function were empty in
several particular library instances.

If you don't have writeable global variables at the time
SerialPortInitialize() is called, then there are two options:

(a) the common option is to just not use global variables for caching
state, and to perform the serial port init on every API call.

(b) A somewhat nasty / limited, but functional approach could be to
check the page protection covering the global variable. Something like this:

STATIC BOOLEAN  mInitialized;

...

  UINTNAddressOfGlobal;
  BOOLEAN  GlobalsWriteable;

  if (mInitialized) {
//
// we're done, nothing to be done
//
return RETURN_SUCCESS;
  }

  //
  // here, perform the serial port initialization
  //
  ...

  //
  // finally:
  //
  AddressOfGlobal = (UINTN)
  GlobalsWriteable = CheckWritable (AddressOfGlobal);
  if (GlobalsWriteable) {
mInitialized = TRUE;
  }

This will keep initing the serial port upon every API call until the
global variable becomes writeable, and then the next API call will init
the serial port for one last time, and also prevent further page table
checks.

The CheckWritable() function is an implementation detail. In the DXE
phase, it could be implemented (I think?) with the
GetMemorySpaceDescriptor() DXE service, or perhaps even the
EFI_MEMORY_ATTRIBUTE_PROTOCOL.GetMemoryAttributes() UEFI protocol member
function. In the standalone MM core, CheckWritable() could walk the page
tables explicitly. The idea is, either way, to *predict* whether writing
to "mInitialized" would trap.

Now I think that speculative / out of order execution could actually
trigger the trap *before* GlobalsWriteable is calculated; however, I
think such a trap should be architecturally hidden (i.e., invisible). I
think at worst we could need a compiler barrier (maybe throw in some
"volatile" for GlobalsWriteable and mInitialized), so that the
*compiler* not try to reorder the accesses. But even that sounds like a
stretch.

Laszlo



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




Re: [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg: Initialise serial port early in StandaloneMmEntryPoint

2024-01-10 Thread levi.yun

My personal conclusion in that thread was [1], and correspondingly,
commit 5087a0773645 ("ArmVirtPkg/FdtPL011SerialPortLib: initialize
implicitly", 2023-10-07). In the end, the only tractable solution was to
initialize the serial port (hardware, and library instance) exactly
once, in (a) the constructor, or (b) the explicit SerialPortInitialize()
call, or (c) any SerialPortLib API, whichever occurred first. (And (a)
and (b) can be coalesced, because SerialPortInitialize() can be marked
as the constructor for the lib instance.)

[1] http://mid.mail-archive.com/542db9e1-cd28-27a2-3a98-5b0c85cd7c79@redhat.com
 https://edk2.groups.io/g/devel/message/109235

Laszlo


In my personal thinking, It's better to make new interface like

RETURN_STATUS
EFIAPI
SerialPortInitializeEarly (
VOID
  );

to solve this problem.

Because, It makes a memory permission fault
when we call SerialPortInitialize like
ArmVirtPkg/Library/FdtPL011SerialPortLib
where try to modify global variable.

At the _ModuleEntryPoint of StandAloneMmCore which is FIRST entry from TF-A
All of Image area is mapped as RO+X, so before load StandaloneMmCore,
We couldn't write global variable.

the purposes of above interface are:
- Initalize serial port to use in early environment only (like
StandAloneMmCore Entry Point) where couldn't write global variable by
static information (FixedPcd).
- It presume that all setting configured by it will be overwritten
by SerialPortInitialize.




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


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




[edk2-devel] Event: TianoCore Community Meeting EMEA/NAMO - Thursday, January 11, 2024 #cal-reminder

2024-01-10 Thread Group Notification
*Reminder: TianoCore Community Meeting EMEA/NAMO*

*When:*
Thursday, January 11, 2024
8:00am to 9:00am
(UTC-08:00) America/Los Angeles

*Where:*
Microsoft Teams meeting Join on your computer or mobile app Click here to join 
the meeting Meeting ID: 226 323 011 029 Passcode: hMRCj6 Download Teams | Join 
on the web Join with a video conferencing device te...@conf.intel.com Video 
Conference ID: 112 716 814 3 Alternate VTC instructions Learn More | Meeting 
options

*Organizer:* Miki Demeter

View Event ( https://edk2.groups.io/g/devel/viewevent?eventid=2148598 )

*Description:*



Microsoft Teams meeting

*Join on your computer or mobile app*

Click here to join the meeting ( 
https://teams.microsoft.com/l/meetup-join/19%3ameeting_MTAyZGJhNjMtYzQ4Mi00MTUxLWFlMWMtOGU0MWNlZDk4NjY5%40thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%226e4ce4c4-1242-431b-9a51-92cd01a5df3c%22%7d
 )

Meeting ID: 226 323 011 029
Passcode: hMRCj6

Download Teams ( https://www.microsoft.com/en-us/microsoft-teams/download-app ) 
| Join on the web ( https://www.microsoft.com/microsoft-teams/join-a-meeting )

*Join with a video conferencing device*

te...@conf.intel.com

Video Conference ID: 112 716 814 3

Alternate VTC instructions ( 
https://conf.intel.com/teams/?conf=1127168143=teams=conf.intel.com=test_call
 )

Learn More ( https://aka.ms/JoinTeamsMeeting ) | Meeting options ( 
https://teams.microsoft.com/meetingOptions/?organizerId=6e4ce4c4-1242-431b-9a51-92cd01a5df3c=46c98d88-e344-4ed4-8496-4ed7712e255d=19_meeting_MTAyZGJhNjMtYzQ4Mi00MTUxLWFlMWMtOGU0MWNlZDk4NjY5@thread.v2=0=en-US
 )




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




Re: [edk2-devel] Memory Attribute for depex section

2024-01-10 Thread Laszlo Ersek
(+ Andrew)

On 1/10/24 14:09, Laszlo Ersek wrote:

> I think that keeping the depex section read-only is valuable, so I'd
> rule out #2. I'd also not start with option #1 -- copying the depex to
> heap memory, just so this optimization can succeed. I'd simply start by
> removing the optimization, and measuring how much driver dispatch slows
> down in practice, on various platforms.
> 
> Can you try this? (I have only build-tested and "uncrustified" it.)
> 
> The patch removes the EFI_DEP_REPLACE_TRUE handling altogether, plus it
> CONST-ifies the Iterator pointer (which points into the DEPEX section),
> so that the compiler catch any possible accesses at *build time* that
> would write to the write-protected DEPEX memory area.

On a tangent: the optimization in question highlights a more general
problem, namely that the DXE (and possibly MM/SMM) protocol databases
are considered slow, for some access patterns.

Edk2 implements those protocol databases with linked lists, where lookup
costs O(n) operations (average and worst cases). And protocol lookups
are quite frequent (for example, in depex evaluations, they could be
considered "particularly frequent").

(1) The "Tasks" wiki page mentions something *similar* (but not the
same); see

https://github.com/tianocore/tianocore.github.io/wiki/Tasks#datahub--gcd-scalability

The description is: "The DXE core's DataHub and GCD (Global Coherency
Domain) layers don't scale well as the number of data items gets large,
since they are based on simple linked lists. Find better data structures."

The same might apply more or less to the protocol database implementation.

(2) More to the point, Andrew Fish reported earlier that at Apple, they
had rewritten the DXE protocol database, using the red-black tree
OrderedCollectionLib that I had contributed previously to edk2 -- and
they saw significant performance improvements.

So upstreaming that feature to edk2 could be very valuable. (Red-black
trees have O(log(n)) time cost (worst case) for lookup, insertion and
deletion, and O(n) cost for iterating through the whole data structure.)

Let me see if I can find the bugzilla ticket...

Ah, I got it. Apologies, I misremembered: Andrew's comment was not about
the protocol database, but about the handle database. Here it is:

https://bugzilla.tianocore.org/show_bug.cgi?id=988#c7

(the BZ is still CONFIRMED btw...)

Still, I think it must be related in a way. Namely, an EFI handle exists
if and only if at least one protocol interface is installed on it. If
you uninstall the last protocol interface from a handle, then the handle
is destroyed -- in fact that's the *only* way to destroy a handle, to my
understanding. See EFI_BOOT_SERVICES.UninstallProtocolInterface() in the
UEFI spec: "If the last protocol interface is removed from a handle, the
handle is freed and is no longer valid". Furthermore, calling
InstallProtocolInterface() and InstallMultipleProtocolInterfaces() is
how one *creates* new handles.

So given how handles and protocol interfaces are conceptually
interlinked, an rbtree-based protocol DB might have to maintain multiple
rbtrees internally (for the ability to search the database quickly with
different types of "keys"). I don't have a design ready in my mind at
all (I'm not that familiar with the *current*, list-based implementation
to begin with!). Upstreaming Apple's (experimental?) code could prove
very helpful.

Laszlo



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




Re: [edk2-devel] Memory Attribute for depex section

2024-01-10 Thread Laszlo Ersek
Hi,

On 1/8/24 11:11, Nhi Pham via groups.io wrote:
> Hi Ard, all,
>
> Could you please help explain how the depex section in an image is
> mapped in terms of memory attribute?
>
> As my observation, dispatcher locates[1] the depex section inside the
> module image and write[2] an evaluated data to the depex if necessary
> for scheduled boot order. The problem that the depex section is now in
> RO+X memory due to a part of the module image, so a writing to depex
> would cause data abort. I'm unsure whether this issue is generic in
> EDK2 or not.
>
> I think of two approaches:
>
> #1 Relocate the depex section to heap memory for dependency
> #evaluation?
>
> #2 EDK2 build tool to support granting write permission for depex
> #section.
>
> [1] StandaloneMmPkg/Core/FwVol.c:236
> [2] StandaloneMmPkg/Core/Dependency.c:256

here's my understanding of the issue.

In Platform Init 1.8, section II-10.7 describes the depex instruction
set for the DXE DISPATCHER. In particular, section II-10.7.3 describes
the PUSH opcode as follows:

  PUSH 

  Pushes a Boolean value onto the stack. If the GUID is present in the
  handle database, then a TRUE is pushed onto the stack. If the GUID is
  not present in the handle database, then a FALSE is pushed onto the
  stack. The test for the GUID in the handle database may be performed
  with the Boot Service LocateProtocol().

This basically says that every time the dispatcher sees a PUSH opcode in
a depex, it has to look up the protocol GUID in the protocol database.

Now, as far as I can tell, edk2's DXE Core implementation wanted to
optimize this. (And this goes back to ancient commit 28a00297189c, from
2007.) Here's the idea AIUI:

- Assume the protocol is not found, i.e. we push a FALSE. This will
likely mean that the driver whose DEPEX contains this PUSH opcode cannot
be dispatched just yet. In other words, we're going to have to
re-evaluate this PUSH at least once more, possibly multiple times. We
cannot optimize anything here, because the needed protocol is not
present yet, but it may become available later. When we next evaluate
the DEPEX for this driver, we'll check again if the protocol has become
available by then.

- Assume the protocol is found, i.e. we push a TRUE. The optimization
here is that we assume the protocol will not *disappear*, once
available. The evaluation of the driver's *entire* DEPEX may still
result in FALSE, so it's not guaranteed that the driver can be
dispatched right now. However, given our assumption that the protocol
we've just found will not disappear during DXE driver dispatch, we might
want to "cache" the current successful protocol lookup, and when we
*next* evaluate the DEPEX for this same driver (assuming we cannot
dispatch it right now due to something else missing from its DEPEX), we
don't want to perform the *same* protocol lookup again -- we'll just
want to remember it was already there last time.

And the way the DXE core seems to implement this optimization (i.e., how
it "remembers success") is that it patches the DEPEX in-place: it
replaces the PUSH opcode with a special (edk2-specific) opcode called
EFI_DEP_REPLACE_TRUE (in addition to pushing the TRUE of course, to the
eval stack). This opcode is functionally identical to the plain (and
standard) TRUE opcode, so the next time the dispatcher evaluates the
same depex and sees EFI_DEP_REPLACE_TRUE it will just push TRUE, the
difference with the TRUE opcode is that EFI_DEP_REPLACE_TRUE also
provides "room" for the -- otherwise useless -- original protocol GUID
that used to be the argument of the PUSH opcode. The dispatcher ignores
the protocol GUID on EFI_DEP_REPLACE_TRUE, beyond logging it.

EFI_DEP_REPLACE_TRUE is documented in the code as follows
("MdeModulePkg/Core/Dxe/DxeMain.h"):

///
/// EFI_DEP_REPLACE_TRUE - Used to dynamically patch the dependency expression
///to save time.  A EFI_DEP_PUSH is evaluated one an
///replaced with EFI_DEP_REPLACE_TRUE. If PI spec's Vol 
2
///Driver Execution Environment Core Interface use 0xff
///as new DEPEX opcode. EFI_DEP_REPLACE_TRUE should be
///defined to a new value that is not conflicting with 
PI spec.
///
#define EFI_DEP_REPLACE_TRUE  0xff

This documentation is hard to understand due to English grammar errors.
It means to say:

"Used to dynamically patch the dependency expression to save time.  An
EFI_DEP_PUSH opcode is evaluated *once*, *and* replaced with
EFI_DEP_REPLACE_TRUE. If PI spec's Vol 2 Driver Execution Environment
Core Interface *starts using* 0xff as new DEPEX opcode *in the future*,
*then* EFI_DEP_REPLACE_TRUE should be *redefined* to a new value that is
not conflicting with *said new* PI spec."

Over time, this optimization (hack) has made its way into the
traditional PiSmmCore, and finally into the standalone SMM core,
apparently.

In summary, this seems like a performance optimization, and should 

Re: [edk2-devel] [PATCH v2] CryptoPkg: Fix redefinition error of int defines

2024-01-10 Thread Li, Yi



Looks good to me.
Reviewed-by: Yi Li 


-Original Message-
From: Hou, Wenxing  
Sent: Wednesday, January 10, 2024 7:36 PM
To: devel@edk2.groups.io
Cc: Yao, Jiewen ; Li, Yi1 ; Jiang, 
Guomin 
Subject: [PATCH v2] CryptoPkg: Fix redefinition error of int defines

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

Move the define to stdint and add MACRO to prevent duplicate inclusion.

Cc: Jiewen Yao 
Cc: Yi Li 
Cc: Guomin Jiang 
Signed-off-by: Wenxing Hou 
---
 CryptoPkg/Library/Include/CrtLibSupport.h | 15 ---
 CryptoPkg/Library/Include/stdint.h| 19 +++
 2 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/CryptoPkg/Library/Include/CrtLibSupport.h 
b/CryptoPkg/Library/Include/CrtLibSupport.h
index 76591f12cb..f36fe08f0c 100644
--- a/CryptoPkg/Library/Include/CrtLibSupport.h
+++ b/CryptoPkg/Library/Include/CrtLibSupport.h
@@ -424,19 +424,4 @@ strcpy (
 #define atoi(nptr)  AsciiStrDecimalToUintn(nptr)

 #define gettimeofday(tvp, tz)   do { (tvp)->tv_sec = time(NULL); 
(tvp)->tv_usec = 0; } while (0)

 

-//

-// only use in Mbedlts. The Openssl has defined them internally.

-//

-#ifndef OPENSSL_SYS_UEFI

-typedef INT8   int8_t;

-typedef UINT8  uint8_t;

-typedef INT16  int16_t;

-typedef UINT16 uint16_t;

-typedef INT32  int32_t;

-typedef UINT32 uint32_t;

-typedef INT64  int64_t;

-typedef UINT64 uint64_t;

-typedef UINTN  uintptr_t;

-#endif

-

 #endif

diff --git a/CryptoPkg/Library/Include/stdint.h 
b/CryptoPkg/Library/Include/stdint.h
index 786d57e8d5..e1f54b412e 100644
--- a/CryptoPkg/Library/Include/stdint.h
+++ b/CryptoPkg/Library/Include/stdint.h
@@ -6,4 +6,23 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 

 **/

 

+#ifndef CRYPTO_CRT_STDIO_H_

+#define CRYPTO_CRT_STDIO_H_

 #include 

+

+//

+// only use in Mbedlts. The Openssl has defined them internally.

+//

+#ifndef OPENSSL_SYS_UEFI

+typedef INT8   int8_t;

+typedef UINT8  uint8_t;

+typedef INT16  int16_t;

+typedef UINT16 uint16_t;

+typedef INT32  int32_t;

+typedef UINT32 uint32_t;

+typedef INT64  int64_t;

+typedef UINT64 uint64_t;

+typedef UINTN  uintptr_t;

+#endif

+

+#endif

-- 
2.26.2.windows.1



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




Re: [edk2-devel] [PATCH] UefiCpuPkg:Limit PhysicalAddressBits in speicial case

2024-01-10 Thread Laszlo Ersek
On 1/10/24 11:54, Gerd Hoffmann wrote:
> On Wed, Jan 10, 2024 at 04:05:44PM +0800, Dun Tan wrote:
>> When creating smm page table, limit maximum
>> supported physical address bits returned by
>> CalculateMaximumSupportAddress() to 48 if
>> 5-Level Paging is disabled.
>> When 5-Level Paging is disabled and the
>> PhysicalAddressBits retrived from CPU HOB or
>> CpuId is bigger than 48, only [0, 2^48 -1]
>> range in 52-bit physical address is mapped
>> in page table.
> 
> I think this is wrong.  Virtual addresses are sign-extended,
> i.e. the virtual address space without 5-level paging is:
> 
>  0x -> 0x7fff and
>  0x8000 -> 0x
> 
> Therefore identity-mapping works for [0, 2^47-1] only.

I'd have never noticed this. I'll happily defer reviewing this patch to
you then! :)

Thanks!
Laszlo



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




Re: [edk2-devel] [PATCH] UefiCpuPkg: Fix issue that IsModified is wrongly set in PageTableMap

2024-01-10 Thread Laszlo Ersek
On 1/10/24 06:38, Zhiguang Liu wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4614
> 
> Fix issue that IsModified is wrongly set in PageTableMap.
> 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Cc: Rahul Kumar 
> Cc: Gerd Hoffmann 
> Cc: Crystal Lee 
> Signed-off-by: Zhiguang Liu 
> ---
>  UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c 
> b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> index 36b2c4e6a3..164187f151 100644
> --- a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> +++ b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> @@ -567,7 +567,10 @@ PageTableLibMapInLevel (
>  OriginalCurrentPagingEntry.Uint64 = CurrentPagingEntry->Uint64;
>  PageTableLibSetPle (Level, CurrentPagingEntry, Offset, Attribute, 
> );
>  
> -if (OriginalCurrentPagingEntry.Uint64 != CurrentPagingEntry->Uint64) 
> {
> +if (Modify && (OriginalCurrentPagingEntry.Uint64 != 
> CurrentPagingEntry->Uint64)) {
> +  //
> +  // The page table entry can be changed by this function only when 
> Modify is true.
> +  //
>*IsModified = TRUE;
>  }
>}
> @@ -609,7 +612,10 @@ PageTableLibMapInLevel (
>// Check if ParentPagingEntry entry is modified here is enough. Except the 
> changes happen in leaf PagingEntry during
>// the while loop, if there is any other change happens in page table, the 
> ParentPagingEntry must has been modified.
>//
> -  if (OriginalParentPagingEntry.Uint64 != ParentPagingEntry->Uint64) {
> +  if (Modify && (OriginalParentPagingEntry.Uint64 != 
> ParentPagingEntry->Uint64)) {
> +//
> +// The page table entry can be changed by this function only when Modify 
> is true.
> +//
>  *IsModified = TRUE;
>}
>  

This function is incredibly complicated, so reviewing this patch is
hard, even after reading the bugzilla ticket.

The commit message is useless. It should contain a brief description of
the problem, and how the fix resolves the problem.

The documentation of the PageTableLibMapInLevel() function is wrong,
even before this patch. It documents the "IsModified" output-only
parameter as follows:

"TRUE means page table is modified. FALSE means page table is not modified."

This states that "IsModified" is always set on output, to either FALSE
or TRUE. Which is an incorrect statement; in reality the caller is
expected to pre-set (*IsModified) to FALSE, and PageTableLibMapInLevel()
will (conditionally!) perform a FALSE->TRUE transition only.

Now, this patch may fix a bug, but it makes the above-described
documentation issue worse, by further restricting the condition for said
FALSE->TRUE transition.

The fix per se looks vaguely reasonable to me (really the function is so
complicated that verifying this change from scratch would take me ages),
but minimally, the documentation of "IsModified" should certainly be
updated too. To something like this:

  @param[out] IsModified  If "Modify" is TRUE on input and the function
  has actually modified the page table, then set
  to TRUE on output. Not overwritten otherwise.

Laszlo



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




Re: [edk2-devel] [PATCH] UefiCpuPkg/CpuMpPei: Don't write CR3 in ConvertMemoryPageToNotPresent

2024-01-10 Thread Laszlo Ersek
On 1/10/24 06:43, Zhiguang Liu wrote:
> After ConvertMemoryPageToNotPresent, there is always a flush TLB
> function. So, to improve performance, there is no need to write CR3
> inside ConvertMemoryPageToNotPresent
> 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Cc: Rahul Kumar 
> Cc: Gerd Hoffmann 
> Signed-off-by: Zhiguang Liu 
> ---
>  UefiCpuPkg/CpuMpPei/CpuPaging.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> index 15c7015fb8..c6894458f7 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
> +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> @@ -167,7 +167,6 @@ ConvertMemoryPageToNotPresent (
>}
>  
>ASSERT_EFI_ERROR (Status);
> -  AsmWriteCr3 (PageTable);
>return Status;
>  }
>  

(1) I mostly understand the point that the commit message makes, but the
message is not clear enough. The real point is that we have two
ConvertMemoryPageToNotPresent() calls altogether, and each of those
takes place in a *loop*, and each of those loops is followed by a
CpuFlushTlb() call.

The loops matter. If there were no loops, then we might be motivated to
choose a different solution (for example, to move centralize the
CpuFlushTlb() calls *inside* ConvertMemoryPageToNotPresent(), or
something similar).

So, please update the commit message; mention the loops.

(2) I can't easily see why this change is actually correct. IIRC,
writing CR3 has a "side effect" of flushing the TLB. But obviously,
that's not the *only* effect of writing CR3. You could say that
CpuFlushTlb() performs a *strict subset* of what AsmWriteCr3() performs.
Therefore, in order to replace AsmWriteCr3() with just CpuFlushTlb(),
you need to demonstrate that the functionality that now is *not* going
to be done has always been superfluous. In more direct terms, you need
to show that the AsmWriteCr3() write call that's being removed here
never actuall changes the *value* of CR3.

And I cannot show that myself very easily.
ConvertMemoryPageToNotPresent(). In ConvertMemoryPageToNotPresent(),
"PageTable" is first set from AsmReadCr3(), then passed twice to
PageTableMap() by reference (!), and then it is written back to CR3. If
at least one of those PageTableMap() calls change "PageTable", then the
AsmWriteCr3() call at the end that's now being removed actually changes
the value of CR3, and then the patch would be wrong.

It's possible that PageTableMap() never changes the value of
"PageTable", but it must be proved, and the evidence should be included
in the commit message.

(3) Furthermore, with the patch applied, ConvertMemoryPageToNotPresent()
will no longer flush the TLB itself -- that will always be left to the
caller. This new caller responsibility should be documented in the
leading comment of ConvertMemoryPageToNotPresent(). We already have a
paragraph there starting with "Caller should make sure..."

Sorry for making such a big deal out of this, but such simple-looking
changes can sometimes case obscure (and rarely occurring) bugs down the
road. If you already have evidence that CR3 does not change here, that's
great, but then please don't think it's "obvious"; just go ahead and
document it.

Laszlo



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




[edk2-devel] [PATCH v2] CryptoPkg: Fix redefinition error of int defines

2024-01-10 Thread Wenxing Hou
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4632

Move the define to stdint and add MACRO to prevent duplicate inclusion.

Cc: Jiewen Yao 
Cc: Yi Li 
Cc: Guomin Jiang 
Signed-off-by: Wenxing Hou 
---
 CryptoPkg/Library/Include/CrtLibSupport.h | 15 ---
 CryptoPkg/Library/Include/stdint.h| 19 +++
 2 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/CryptoPkg/Library/Include/CrtLibSupport.h 
b/CryptoPkg/Library/Include/CrtLibSupport.h
index 76591f12cb..f36fe08f0c 100644
--- a/CryptoPkg/Library/Include/CrtLibSupport.h
+++ b/CryptoPkg/Library/Include/CrtLibSupport.h
@@ -424,19 +424,4 @@ strcpy (
 #define atoi(nptr)  AsciiStrDecimalToUintn(nptr)
 #define gettimeofday(tvp, tz)   do { (tvp)->tv_sec = time(NULL); 
(tvp)->tv_usec = 0; } while (0)
 
-//
-// only use in Mbedlts. The Openssl has defined them internally.
-//
-#ifndef OPENSSL_SYS_UEFI
-typedef INT8   int8_t;
-typedef UINT8  uint8_t;
-typedef INT16  int16_t;
-typedef UINT16 uint16_t;
-typedef INT32  int32_t;
-typedef UINT32 uint32_t;
-typedef INT64  int64_t;
-typedef UINT64 uint64_t;
-typedef UINTN  uintptr_t;
-#endif
-
 #endif
diff --git a/CryptoPkg/Library/Include/stdint.h 
b/CryptoPkg/Library/Include/stdint.h
index 786d57e8d5..e1f54b412e 100644
--- a/CryptoPkg/Library/Include/stdint.h
+++ b/CryptoPkg/Library/Include/stdint.h
@@ -6,4 +6,23 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
+#ifndef CRYPTO_CRT_STDIO_H_
+#define CRYPTO_CRT_STDIO_H_
 #include 
+
+//
+// only use in Mbedlts. The Openssl has defined them internally.
+//
+#ifndef OPENSSL_SYS_UEFI
+typedef INT8   int8_t;
+typedef UINT8  uint8_t;
+typedef INT16  int16_t;
+typedef UINT16 uint16_t;
+typedef INT32  int32_t;
+typedef UINT32 uint32_t;
+typedef INT64  int64_t;
+typedef UINT64 uint64_t;
+typedef UINTN  uintptr_t;
+#endif
+
+#endif
-- 
2.26.2.windows.1



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




Re: [edk2-devel] [PATCH] CryptoPkg: change the define

2024-01-10 Thread Li, Yi
Hi, this title is too simple.
Please change it to CryptoPkg: Fix redefinition error of int defines

-Original Message-
From: Hou, Wenxing  
Sent: Wednesday, January 10, 2024 7:16 PM
To: devel@edk2.groups.io
Cc: Yao, Jiewen ; Li, Yi1 ; Jiang, 
Guomin 
Subject: [PATCH] CryptoPkg: change the define

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

Move the define to stdint and add MACRO to prevent duplicate inclusion.

Cc: Jiewen Yao 
Cc: Yi Li 
Cc: Guomin Jiang 
Signed-off-by: Wenxing Hou 
---
 CryptoPkg/Library/Include/CrtLibSupport.h | 15 ---
 CryptoPkg/Library/Include/stdint.h| 19 +++
 2 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/CryptoPkg/Library/Include/CrtLibSupport.h 
b/CryptoPkg/Library/Include/CrtLibSupport.h
index 76591f12cb..f36fe08f0c 100644
--- a/CryptoPkg/Library/Include/CrtLibSupport.h
+++ b/CryptoPkg/Library/Include/CrtLibSupport.h
@@ -424,19 +424,4 @@ strcpy (
 #define atoi(nptr)  AsciiStrDecimalToUintn(nptr)

 #define gettimeofday(tvp, tz)   do { (tvp)->tv_sec = time(NULL); 
(tvp)->tv_usec = 0; } while (0)

 

-//

-// only use in Mbedlts. The Openssl has defined them internally.

-//

-#ifndef OPENSSL_SYS_UEFI

-typedef INT8   int8_t;

-typedef UINT8  uint8_t;

-typedef INT16  int16_t;

-typedef UINT16 uint16_t;

-typedef INT32  int32_t;

-typedef UINT32 uint32_t;

-typedef INT64  int64_t;

-typedef UINT64 uint64_t;

-typedef UINTN  uintptr_t;

-#endif

-

 #endif

diff --git a/CryptoPkg/Library/Include/stdint.h 
b/CryptoPkg/Library/Include/stdint.h
index 786d57e8d5..e1f54b412e 100644
--- a/CryptoPkg/Library/Include/stdint.h
+++ b/CryptoPkg/Library/Include/stdint.h
@@ -6,4 +6,23 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 

 **/

 

+#ifndef CRYPTO_CRT_STDIO_H_

+#define CRYPTO_CRT_STDIO_H_

 #include 

+

+//

+// only use in Mbedlts. The Openssl has defined them internally.

+//

+#ifndef OPENSSL_SYS_UEFI

+typedef INT8   int8_t;

+typedef UINT8  uint8_t;

+typedef INT16  int16_t;

+typedef UINT16 uint16_t;

+typedef INT32  int32_t;

+typedef UINT32 uint32_t;

+typedef INT64  int64_t;

+typedef UINT64 uint64_t;

+typedef UINTN  uintptr_t;

+#endif

+

+#endif

-- 
2.26.2.windows.1



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




[edk2-devel] [PATCH] CryptoPkg: change the define

2024-01-10 Thread Wenxing Hou
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4632

Move the define to stdint and add MACRO to prevent duplicate inclusion.

Cc: Jiewen Yao 
Cc: Yi Li 
Cc: Guomin Jiang 
Signed-off-by: Wenxing Hou 
---
 CryptoPkg/Library/Include/CrtLibSupport.h | 15 ---
 CryptoPkg/Library/Include/stdint.h| 19 +++
 2 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/CryptoPkg/Library/Include/CrtLibSupport.h 
b/CryptoPkg/Library/Include/CrtLibSupport.h
index 76591f12cb..f36fe08f0c 100644
--- a/CryptoPkg/Library/Include/CrtLibSupport.h
+++ b/CryptoPkg/Library/Include/CrtLibSupport.h
@@ -424,19 +424,4 @@ strcpy (
 #define atoi(nptr)  AsciiStrDecimalToUintn(nptr)
 #define gettimeofday(tvp, tz)   do { (tvp)->tv_sec = time(NULL); 
(tvp)->tv_usec = 0; } while (0)
 
-//
-// only use in Mbedlts. The Openssl has defined them internally.
-//
-#ifndef OPENSSL_SYS_UEFI
-typedef INT8   int8_t;
-typedef UINT8  uint8_t;
-typedef INT16  int16_t;
-typedef UINT16 uint16_t;
-typedef INT32  int32_t;
-typedef UINT32 uint32_t;
-typedef INT64  int64_t;
-typedef UINT64 uint64_t;
-typedef UINTN  uintptr_t;
-#endif
-
 #endif
diff --git a/CryptoPkg/Library/Include/stdint.h 
b/CryptoPkg/Library/Include/stdint.h
index 786d57e8d5..e1f54b412e 100644
--- a/CryptoPkg/Library/Include/stdint.h
+++ b/CryptoPkg/Library/Include/stdint.h
@@ -6,4 +6,23 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
+#ifndef CRYPTO_CRT_STDIO_H_
+#define CRYPTO_CRT_STDIO_H_
 #include 
+
+//
+// only use in Mbedlts. The Openssl has defined them internally.
+//
+#ifndef OPENSSL_SYS_UEFI
+typedef INT8   int8_t;
+typedef UINT8  uint8_t;
+typedef INT16  int16_t;
+typedef UINT16 uint16_t;
+typedef INT32  int32_t;
+typedef UINT32 uint32_t;
+typedef INT64  int64_t;
+typedef UINT64 uint64_t;
+typedef UINTN  uintptr_t;
+#endif
+
+#endif
-- 
2.26.2.windows.1



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




Re: [edk2-devel] [PATCH] UefiCpuPkg:Limit PhysicalAddressBits in speicial case

2024-01-10 Thread Gerd Hoffmann
On Wed, Jan 10, 2024 at 04:05:44PM +0800, Dun Tan wrote:
> When creating smm page table, limit maximum
> supported physical address bits returned by
> CalculateMaximumSupportAddress() to 48 if
> 5-Level Paging is disabled.
> When 5-Level Paging is disabled and the
> PhysicalAddressBits retrived from CPU HOB or
> CpuId is bigger than 48, only [0, 2^48 -1]
> range in 52-bit physical address is mapped
> in page table.

I think this is wrong.  Virtual addresses are sign-extended,
i.e. the virtual address space without 5-level paging is:

 0x -> 0x7fff and
 0x8000 -> 0x

Therefore identity-mapping works for [0, 2^47-1] only.

take care,
  Gerd



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




Re: [edk2-devel] [PATCH v6 26/36] OvmfPkg/LoongArchVirt: Add a NULL library named CollectApResouceLibNull

2024-01-10 Thread maobibo




On 2024/1/10 上午10:47, Chao Li wrote:

Hi Bibo,


Thanks,
Chao
On 2024/1/10 09:24, maobibo wrote:



On 2024/1/5 下午5:45, Chao Li wrote:

This Library is used to collect APs resources, but is currently NULL
for OvmfPkg, because it is not used by the LoongArch virtual machine.

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

Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc: Gerd Hoffmann 
Cc: Bibo Mao 
Cc: Dongyan Qian 
Signed-off-by: Chao Li 
---
  .../CollectApResourceLibNull.c    | 38 +++
  .../CollectApResourceLibNull.inf  | 31 +++
  .../CollectApResourceLibNull.uni  |  9 +
  3 files changed, 78 insertions(+)
  create mode 100644 
OvmfPkg/LoongArchVirt/Library/CollectApResouceLibNull/CollectApResourceLibNull.c
  create mode 100644 
OvmfPkg/LoongArchVirt/Library/CollectApResouceLibNull/CollectApResourceLibNull.inf
  create mode 100644 
OvmfPkg/LoongArchVirt/Library/CollectApResouceLibNull/CollectApResourceLibNull.uni


diff --git 
a/OvmfPkg/LoongArchVirt/Library/CollectApResouceLibNull/CollectApResourceLibNull.c 
b/OvmfPkg/LoongArchVirt/Library/CollectApResouceLibNull/CollectApResourceLibNull.c 


new file mode 100644
index 00..19995c1193
--- /dev/null
+++ 
b/OvmfPkg/LoongArchVirt/Library/CollectApResouceLibNull/CollectApResourceLibNull.c

@@ -0,0 +1,38 @@
+/** @file
+  LoongArch64 CPU Collect AP resource NULL Library functions.
+
+  Copyright (c) 2024, Loongson Technology Corporation Limited. All 
rights reserved.

+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "../../../UefiCpuPkg/Library/LoongArch64MpInitLib/MpLib.h"
The included path is a little strange, should we put 
CollectApResouceLibNull library in ovmf package or 
UefiCpuPkg/Library/LoongArch64MpInitLib package?
This library is a private because virtual-matchines collect AP resouces 
differently from physical machines, so I thought would be fine if it was 
located in OvmfPkg/LoongArchVirt/Library.

Ok, that sounds good to me.

Reviewed-by: Bibo Mao 


Regards
Bibo Mao


+
+VOID
+SaveProcessorResourceData (
+  IN PROCESSOR_RESOURCE_DATA *
+  );
+
+VOID
+EFIAPI
+SaveProcessorResource (
+  PROCESSOR_RESOURCE_DATA  *mProcessorResource
+  )
+{
+  SaveProcessorResourceData (mProcessorResource);
+}
+
+VOID
+EFIAPI
+CollectAllProcessorResource (
+  VOID
+  )
+{
+  return;
+}
diff --git 
a/OvmfPkg/LoongArchVirt/Library/CollectApResouceLibNull/CollectApResourceLibNull.inf 
b/OvmfPkg/LoongArchVirt/Library/CollectApResouceLibNull/CollectApResourceLibNull.inf 


new file mode 100644
index 00..c166df6bbd
--- /dev/null
+++ 
b/OvmfPkg/LoongArchVirt/Library/CollectApResouceLibNull/CollectApResourceLibNull.inf

@@ -0,0 +1,31 @@
+## @file
+#  LoongArch64 CPU Collect AP resource NULL Library.
+#
+#  Copyright (c) 2024, Loongson Technology Corporation Limited. All 
rights reserved.

+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION    = 1.29
+  BASE_NAME  = CollectApResourceLibNull
+  MODULE_UNI_FILE    = CollectApResourceLibNull.uni
+  FILE_GUID  = 8C3B54BF-6A9F-E8B4-4D57-67B3AB578DD6
+  MODULE_TYPE    = PEIM
+  VERSION_STRING = 1.1
+  LIBRARY_CLASS  = PEIM
+
+[Sources.common]
+  CollectApResourceLibNull.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  UefiCpuPkg/UefiCpuPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  HobLib
+  MemoryAllocationLib
+
+[Pcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber
diff --git 
a/OvmfPkg/LoongArchVirt/Library/CollectApResouceLibNull/CollectApResourceLibNull.uni 
b/OvmfPkg/LoongArchVirt/Library/CollectApResouceLibNull/CollectApResourceLibNull.uni 


new file mode 100644
index 00..d1638ab11e
--- /dev/null
+++ 
b/OvmfPkg/LoongArchVirt/Library/CollectApResouceLibNull/CollectApResourceLibNull.uni

@@ -0,0 +1,9 @@
+// @file
+//  LoongArch64 CPU Collect AP resource NULL Library.
+//
+//  Copyright (c) 2024, Loongson Technology Corporation Limited. All 
rights reserved.

+//  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+#string STR_MODULE_ABSTRACT #language en-US "CPU Collect 
AP resource NULL Library."

+
+#string STR_MODULE_DESCRIPTION  #language en-US "CPU Collect 
AP resource NULL Library."












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




Re: [edk2-devel] [PATCH] UefiCpuPkg: Fix issue that IsModified is wrongly set in PageTableMap

2024-01-10 Thread Ni, Ray
Reviewed-by: Ray Ni 

Thanks,
Ray
> -Original Message-
> From: Liu, Zhiguang 
> Sent: Wednesday, January 10, 2024 1:38 PM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang ; Ni, Ray ; Laszlo
> Ersek ; Kumar, Rahul R ; Gerd
> Hoffmann ; Lee, Crystal 
> Subject: [PATCH] UefiCpuPkg: Fix issue that IsModified is wrongly set in
> PageTableMap
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4614
> 
> Fix issue that IsModified is wrongly set in PageTableMap.
> 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Cc: Rahul Kumar 
> Cc: Gerd Hoffmann 
> Cc: Crystal Lee 
> Signed-off-by: Zhiguang Liu 
> ---
>  UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> index 36b2c4e6a3..164187f151 100644
> --- a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> +++ b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> @@ -567,7 +567,10 @@ PageTableLibMapInLevel (
>  OriginalCurrentPagingEntry.Uint64 = CurrentPagingEntry->Uint64;
>  PageTableLibSetPle (Level, CurrentPagingEntry, Offset, Attribute,
> );
> 
> -if (OriginalCurrentPagingEntry.Uint64 != CurrentPagingEntry->Uint64) 
> {
> +if (Modify && (OriginalCurrentPagingEntry.Uint64 != 
> CurrentPagingEntry-
> >Uint64)) {
> +  //
> +  // The page table entry can be changed by this function only when
> Modify is true.
> +  //
>*IsModified = TRUE;
>  }
>}
> @@ -609,7 +612,10 @@ PageTableLibMapInLevel (
>// Check if ParentPagingEntry entry is modified here is enough. Except the
> changes happen in leaf PagingEntry during
>// the while loop, if there is any other change happens in page table, the
> ParentPagingEntry must has been modified.
>//
> -  if (OriginalParentPagingEntry.Uint64 != ParentPagingEntry->Uint64) {
> +  if (Modify && (OriginalParentPagingEntry.Uint64 != ParentPagingEntry-
> >Uint64)) {
> +//
> +// The page table entry can be changed by this function only when Modify
> is true.
> +//
>  *IsModified = TRUE;
>}
> 
> --
> 2.31.1.windows.1



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




Re: [edk2-devel] [PATCH] UefiCpuPkg/CpuMpPei: Don't write CR3 in ConvertMemoryPageToNotPresent

2024-01-10 Thread Ni, Ray
Reviewed-by: Ray Ni 

Thanks,
Ray
> -Original Message-
> From: Liu, Zhiguang 
> Sent: Wednesday, January 10, 2024 1:43 PM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang ; Ni, Ray ; Laszlo
> Ersek ; Kumar, Rahul R ; Gerd
> Hoffmann 
> Subject: [PATCH] UefiCpuPkg/CpuMpPei: Don't write CR3 in
> ConvertMemoryPageToNotPresent
> 
> After ConvertMemoryPageToNotPresent, there is always a flush TLB
> function. So, to improve performance, there is no need to write CR3
> inside ConvertMemoryPageToNotPresent
> 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Cc: Rahul Kumar 
> Cc: Gerd Hoffmann 
> Signed-off-by: Zhiguang Liu 
> ---
>  UefiCpuPkg/CpuMpPei/CpuPaging.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c
> b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> index 15c7015fb8..c6894458f7 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
> +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> @@ -167,7 +167,6 @@ ConvertMemoryPageToNotPresent (
>}
> 
>ASSERT_EFI_ERROR (Status);
> -  AsmWriteCr3 (PageTable);
>return Status;
>  }
> 
> --
> 2.31.1.windows.1



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




Re: [edk2-devel] [PATCH] UefiCpuPkg:Limit PhysicalAddressBits in speicial case

2024-01-10 Thread Ni, Ray
Reviewed-by: Ray Ni 

Thanks,
Ray
> -Original Message-
> From: Tan, Dun 
> Sent: Wednesday, January 10, 2024 4:06 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray ; Laszlo Ersek ; Kumar,
> Rahul R ; Gerd Hoffmann 
> Subject: [PATCH] UefiCpuPkg:Limit PhysicalAddressBits in speicial case
> 
> When creating smm page table, limit maximum
> supported physical address bits returned by
> CalculateMaximumSupportAddress() to 48 if
> 5-Level Paging is disabled.
> When 5-Level Paging is disabled and the
> PhysicalAddressBits retrived from CPU HOB or
> CpuId is bigger than 48, only [0, 2^48 -1]
> range in 52-bit physical address is mapped
> in page table.
> 
> Signed-off-by: Dun Tan 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Cc: Rahul Kumar 
> Cc: Gerd Hoffmann 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 15 +--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> index ddd9be66b5..e6f174ca10 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> @@ -137,11 +137,13 @@ GetSubEntriesNum (
>  /**
>Calculate the maximum support address.
> 
> +  @param[in] Is5LevelPagingNeededIf 5-level paging enabling is needed.
> +
>@return the maximum support address.
>  **/
>  UINT8
>  CalculateMaximumSupportAddress (
> -  VOID
> +  BOOLEAN  Is5LevelPagingNeeded
>)
>  {
>UINT32  RegEax;
> @@ -164,6 +166,15 @@ CalculateMaximumSupportAddress (
>  }
>}
> 
> +  //
> +  // Only [0, 2^48 -1] in 52-bit physical addresses is mapped in page table
> +  // when 5-Level Paging is disabled.
> +  //
> +  ASSERT (PhysicalAddressBits <= 52);
> +  if (!Is5LevelPagingNeeded && (PhysicalAddressBits > 48)) {
> +PhysicalAddressBits = 48;
> +  }
> +
>return PhysicalAddressBits;
>  }
> 
> @@ -197,7 +208,7 @@ SmmInitPageTable (
>mCpuSmmRestrictedMemoryAccess = PcdGetBool
> (PcdCpuSmmRestrictedMemoryAccess);
>m1GPageTableSupport   = Is1GPageSupport ();
>m5LevelPagingNeeded   = Is5LevelPagingNeeded ();
> -  mPhysicalAddressBits  = CalculateMaximumSupportAddress ();
> +  mPhysicalAddressBits  = CalculateMaximumSupportAddress
> (m5LevelPagingNeeded);
>PatchInstructionX86 (gPatch5LevelPagingNeeded, m5LevelPagingNeeded,
> 1);
>if (m5LevelPagingNeeded) {
>  mPagingMode = m1GPageTableSupport ? Paging5Level1GB : Paging5Level;
> --
> 2.31.1.windows.1



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




[edk2-devel] [PATCH] UefiCpuPkg:Limit PhysicalAddressBits in speicial case

2024-01-10 Thread duntan
When creating smm page table, limit maximum
supported physical address bits returned by
CalculateMaximumSupportAddress() to 48 if
5-Level Paging is disabled.
When 5-Level Paging is disabled and the
PhysicalAddressBits retrived from CPU HOB or
CpuId is bigger than 48, only [0, 2^48 -1]
range in 52-bit physical address is mapped
in page table.

Signed-off-by: Dun Tan 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
index ddd9be66b5..e6f174ca10 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
@@ -137,11 +137,13 @@ GetSubEntriesNum (
 /**
   Calculate the maximum support address.
 
+  @param[in] Is5LevelPagingNeededIf 5-level paging enabling is needed.
+
   @return the maximum support address.
 **/
 UINT8
 CalculateMaximumSupportAddress (
-  VOID
+  BOOLEAN  Is5LevelPagingNeeded
   )
 {
   UINT32  RegEax;
@@ -164,6 +166,15 @@ CalculateMaximumSupportAddress (
 }
   }
 
+  //
+  // Only [0, 2^48 -1] in 52-bit physical addresses is mapped in page table
+  // when 5-Level Paging is disabled.
+  //
+  ASSERT (PhysicalAddressBits <= 52);
+  if (!Is5LevelPagingNeeded && (PhysicalAddressBits > 48)) {
+PhysicalAddressBits = 48;
+  }
+
   return PhysicalAddressBits;
 }
 
@@ -197,7 +208,7 @@ SmmInitPageTable (
   mCpuSmmRestrictedMemoryAccess = PcdGetBool (PcdCpuSmmRestrictedMemoryAccess);
   m1GPageTableSupport   = Is1GPageSupport ();
   m5LevelPagingNeeded   = Is5LevelPagingNeeded ();
-  mPhysicalAddressBits  = CalculateMaximumSupportAddress ();
+  mPhysicalAddressBits  = CalculateMaximumSupportAddress 
(m5LevelPagingNeeded);
   PatchInstructionX86 (gPatch5LevelPagingNeeded, m5LevelPagingNeeded, 1);
   if (m5LevelPagingNeeded) {
 mPagingMode = m1GPageTableSupport ? Paging5Level1GB : Paging5Level;
-- 
2.31.1.windows.1



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