Re: [edk2-devel] [PATCH v1 1/1] ShellPkg: Acpivew/GTDT: Print timer flags information.

[PierreGondois]

Hello Levi,
The patch looks good to me, also:

Tested-by: Pierre Gondois 

Regards,
Pierre

On 7/12/23 13:44, levi.yun wrote:

Currently, GTDT only prints the value of timer flags in hex.
This change prints the detail informaiton about Timer flags in GTDT.

before:
 Shell> acpiview -s GTDT
 ...
 Non-Secure EL1 timer FLAGS : 0x2
 Virtual timer GSIV : 0x1B
 Virtual timer FLAGS: 0x2
 ...

after:
 Shell> acpiview -s GTDT
 ...
 Non-Secure EL1 timer FLAGS : 0x2
 Timer Interrupt Mode : Level Trigger(0)
 Timer Interrupt Polarity : Active Low(1)
 Always-on Capability : 0
 Reserved : 0

 Virtual timer GSIV : 0x1B
 Virtual timer FLAGS: 0x2

Signed-off-by: levi.yun 
---
The changes can be seen at 
https://github.com/LeviYeoReum/edk2/compare/master...LeviYeoReum:edk2:refs.geads/2711_gtdt_flags_v1
  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c | 111 
+---
  1 file changed, 98 insertions(+), 13 deletions(-)

diff --git 
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
index 
e62927098a010a0e1dad8361dcfc6559d32dcebf..0001d4231e88c03fa1e296539e9fbc76eb5dd7e1
 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
@@ -84,33 +84,118 @@ ValidateGtFrameNumber (
}
  }
  
+/**

+  This function prints Triggermode information in timer flags.
+
+  @param [in] Format Print format.
+  @param [in] PtrPointer to the start of the field data.
+**/
+STATIC
+VOID
+EFIAPI
+PrintTimerInterruptMode (
+  IN CONST CHAR16  *Format OPTIONAL,
+  IN UINT8 *Ptr
+  )
+{
+  UINT8  TriggerMode;
+
+  TriggerMode = (*(UINT8 *)Ptr);
+
+  Print (
+L"%s(%d)",
+((TriggerMode) ? L"Edge Trigger" : L"Level Trigger"),
+TriggerMode
+);
+}
+
+/**
+  This function prints Polarity information in timer flags.
+
+  @param [in] Format Print format.
+  @param [in] PtrPointer to the start of the field data.
+**/
+STATIC
+VOID
+EFIAPI
+PrintTimerInterruptPolarity (
+  IN CONST CHAR16  *Format OPTIONAL,
+  IN UINT8 *Ptr
+  )
+{
+  UINT8  Polarity;
+
+  Polarity = (*(UINT8 *)Ptr);
+
+  Print (
+L"%s(%d)",
+((Polarity) ? L"Active Low" : L"Active High"),
+Polarity
+);
+}
+
+/**
+  An ACPI_PARSER array describing the Timer Flags Field in GTDT Table.
+**/
+STATIC CONST ACPI_PARSER  TimerFlagsParser[] = {
+  { L"Timer Interrupt Mode", 1,  0, NULL,  PrintTimerInterruptMode, 
NULL, NULL, NULL },
+  { L"Timer Interrupt Polarity", 1,  1, NULL,  PrintTimerInterruptPolarity, 
NULL, NULL, NULL },
+  { L"Always-on Capability", 1,  2, L"%d", NULL,
NULL, NULL, NULL },
+  { L"Reserved", 29, 3, L"%d", NULL,
NULL, NULL, NULL },
+};
+
+/**
+  This function parses the Timer Flags.
+
+  @param [in]   Format  Print format.
+  @param [in]   Ptr Pointer to the start of the Timer flags.
+ **/
+STATIC
+VOID
+EFIAPI
+DumpTimerFlags (
+  IN CONST CHAR16  *Format OPTIONAL,
+  IN UINT8 *Ptr
+  )
+{
+  DumpUint32 (L"0x%x\n", Ptr);
+  ParseAcpiBitFields (
+TRUE,
+2,
+NULL,
+Ptr,
+4,
+PARSER_PARAMS (TimerFlagsParser)
+);
+}
+
  /**
An ACPI_PARSER array describing the ACPI GTDT Table.
  **/
  STATIC CONST ACPI_PARSER  GtdtParser[] = {
PARSE_ACPI_HEADER (&AcpiHdrInfo),
-  { L"CntControlBase Physical Address",8,  36,  L"0x%lx", NULL, NULL,
+  { L"CntControlBase Physical Address",8,  36,  L"0x%lx", NULL,   
NULL,
  NULL, NULL },
-  { L"Reserved",  4,  44,  L"0x%x",  NULL, NULL,NULL,  
NULL },
-  { L"Secure EL1 timer GSIV", 4,  48,  L"0x%x",  NULL, NULL,NULL,  
NULL },
-  { L"Secure EL1 timer FLAGS",4,  52,  L"0x%x",  NULL, NULL,NULL,  
NULL },
+  { L"Reserved",  4,  44,  L"0x%x",  NULL,   
NULL,NULL,  NULL },
+  { L"Secure EL1 timer GSIV", 4,  48,  L"0x%x",  NULL,   
NULL,NULL,  NULL },
+  { L"Secure EL1 timer FLAGS",4,  52,  NULL, DumpTimerFlags, 
NULL,NULL,  NULL },
  
-  { L"Non-Secure EL1 timer GSIV", 4,  56,  L"0x%x",  NULL, NULL,NULL,  NULL },

-  { L"Non-Secure EL1 timer FLAGS",4,  60,  L"0x%x",  NULL, NULL,NULL,  
NULL },
+  { L"Non-Secure EL1 timer GSIV", 4,  56,  L"0x%x",  NULL,   
NULL,NULL,  NULL },
+  { L"Non-Secure EL1 timer FLAGS",4,  60,  NULL, DumpTimerFlags, 
NULL,NULL,  NULL },
  
-  { L"Virtual timer GSIV",4,  64,  L"0x%x",  NULL, NULL,NULL,  NULL },

-  { L"Virtual timer FLAGS",   4,  68,  L"0x%x",  NULL, NULL,NULL,  
NULL },

Re: [edk2-devel] [PATCH] IntelSiliconPkg/Vtd: Resolve parameter transfer errors

[Robert Kowalewski]

Reviewed by Robert Kowalewski 

-Original Message-
From: Huang, Jenny  
Sent: Thursday, July 13, 2023 8:01 AM
To: Sheng, W ; devel@edk2.groups.io
Cc: Ni, Ray ; Chaganty, Rangasai V 
; Kowalewski, Robert 

Subject: RE: [PATCH] IntelSiliconPkg/Vtd: Resolve parameter transfer errors

Reviewed by Jenny Huang 

-Original Message-
From: Sheng, W  
Sent: Wednesday, July 12, 2023 10:52 PM
To: devel@edk2.groups.io
Cc: Ni, Ray ; Chaganty, Rangasai V 
; Huang, Jenny ; 
Kowalewski, Robert 
Subject: [PATCH] IntelSiliconPkg/Vtd: Resolve parameter transfer errors

Fix the capsule update assert caused by function call errors.

Cc: Ray Ni 
Cc: Rangasai V Chaganty 
Cc: Jenny Huang 
Cc: Robert Kowalewski 
Signed-off-by: Sheng Wei 
---
 .../Intel/IntelSiliconPkg/Feature/VTd/IntelVTdCoreDxe/VtdReg.c  | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdCoreDxe/VtdReg.c 
b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdCoreDxe/VtdReg.c
index dd0c49698..f05ca6ae5 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdCoreDxe/VtdReg.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdCoreDxe/VtdReg.c
@@ -737,7 +737,7 @@ DumpVtdIfError (
 if (HasError) {
   REPORT_STATUS_CODE (EFI_ERROR_CODE, PcdGet32 (PcdErrorCodeVTdError));
   DEBUG((DEBUG_INFO, "\n ERROR \n"));
-  DumpVtdRegs (Num);
+  DumpVtdRegs (mVtdUnitInformation[Num].VtdUnitBaseAddress);
   DEBUG((DEBUG_INFO, " ERROR \n\n"));
   //
   // Clear
-- 
2.26.2.windows.1

-
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial 
Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | 
Kapital zakladowy 200.000 PLN.
Spolka oswiadcza, ze posiada status duzego przedsiebiorcy w rozumieniu ustawy z 
dnia 8 marca 2013 r. o przeciwdzialaniu nadmiernym opoznieniom w transakcjach 
handlowych.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i 
moze zawierac informacje poufne. W razie przypadkowego otrzymania tej 
wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; 
jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole 
use of the intended recipient(s). If you are not the intended recipient, please 
contact the sender and delete all copies; any review or distribution by others 
is strictly prohibited.



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106898): https://edk2.groups.io/g/devel/message/106898
Mute This Topic: https://groups.io/mt/100115481/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 0/4] Add support for generating ACPI ThermalZones

[PierreGondois]

Hello Jeff,
Thanks for the new version:

Reviewed-by: Pierre Gondois 

On 7/11/23 00:25, Jeff Brasen wrote:

Add APIs needed to create thermal zones dynamically.
Does not add a generator for this as creating the TMP method generically may
be difficult.

Change log:

v2 - renamed NameString function and added goto dones in a couple error cases

Jeff Brasen (4):
   DynamicTablesPkg: Add ThermalZone CodeGen function
   DynamicTablesPkg: Add support for simple method invocation.
   DynamicTablesPkg: Add support to add Strings to package
   DynamicTablesPkg: Add Aml NameUnicodeString API

  .../Include/Library/AmlLib/AmlLib.h   | 130 +
  .../Common/AmlLib/CodeGen/AmlCodeGen.c| 508 ++
  2 files changed, 638 insertions(+)




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

[edk2-devel] [PATCH] RedfishPkg/RedfishRestExDxe: reset session when TCP timeout happens

[Nickle Wang via groups.io]

Call ResetHttpTslSession() to reset HTTP session when TCP timeout
failure happens. So that application can perform retry to the same URI.

Signed-off-by: Nickle Wang 
Cc: Abner Chang 
Cc: Igor Kulchytskyy 
Cc: Nick Ramirez 
---
 .../RedfishRestExDxe/RedfishRestExInternal.h   | 14 ++
 RedfishPkg/RedfishRestExDxe/RedfishRestExImpl.c|  4 ++--
 .../RedfishRestExDxe/RedfishRestExProtocol.c   |  4 
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/RedfishPkg/RedfishRestExDxe/RedfishRestExInternal.h 
b/RedfishPkg/RedfishRestExDxe/RedfishRestExInternal.h
index bca679e2ccc4..c146f4a647cb 100644
--- a/RedfishPkg/RedfishRestExDxe/RedfishRestExInternal.h
+++ b/RedfishPkg/RedfishRestExDxe/RedfishRestExInternal.h
@@ -55,6 +55,20 @@ RedfishCheckHttpReceiveStatus (
   IN EFI_STATUS   HttpIoReceiveStatus
   );
 
+/**
+  Create a new TLS session because the previous one is closed.
+
+  @param[in]  InstancePointer to EFI_REST_EX_PROTOCOL instance for 
a particular
+  REST service.
+  @retval EFI_SUCCESS operation succeeded.
+  @retval EFI_ERROR   Other errors.
+
+**/
+EFI_STATUS
+ResetHttpTslSession (
+  IN   RESTEX_INSTANCE  *Instance
+  );
+
 /**
   This function send the HTTP request without body to see
   if the write to URL is permitted by Redfish service. This function
diff --git a/RedfishPkg/RedfishRestExDxe/RedfishRestExImpl.c 
b/RedfishPkg/RedfishRestExDxe/RedfishRestExImpl.c
index 838e24f7e7ef..b2961424de80 100644
--- a/RedfishPkg/RedfishRestExDxe/RedfishRestExImpl.c
+++ b/RedfishPkg/RedfishRestExDxe/RedfishRestExImpl.c
@@ -4,6 +4,7 @@
   Copyright (c) 2019, Intel Corporation. All rights reserved.
   (C) Copyright 2020 Hewlett Packard Enterprise Development LP
   Copyright (c) 2023, American Megatrends International LLC.
+  Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -12,8 +13,7 @@
 #include "RedfishRestExInternal.h"
 
 /**
-  Create a new TLS session becuase the previous on is closed.
-  status.
+  Create a new TLS session because the previous one is closed.
 
   @param[in]  InstancePointer to EFI_REST_EX_PROTOCOL instance for 
a particular
   REST service.
diff --git a/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c 
b/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c
index d8f2c73f8ef0..90973619f2bc 100644
--- a/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c
+++ b/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c
@@ -188,6 +188,10 @@ ReSendRequest:;
   }
 
   if (EFI_ERROR (Status)) {
+//
+// Communication failure happens. Reset the session.
+//
+ResetHttpTslSession (Instance);
 goto ON_EXIT;
   }
 
-- 
2.17.1



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

[edk2-devel] [PATCH v4 0/1] MdePkg:Implement RISCV CMO

[Dhaval Sharma]

Implementing code to support Cache Management Operations (CMO) defined by RV 
spec https://github.com/riscv/riscv-CMOs

Notes:

CMO only supports block based Operations. Meaning complete cache 
flush/invd/clean Operations are not available. In that case we fallback on 
fence.i instructions.
Rely on the fact that platform init has initialized CMO and this implementation 
just checks if it is enabled.
In order to avoid compiler dependency injecting byte code.
Test:

Ensured correct instructions are refelecting in asm
Able to boot platform with RiscVVirtQemu config
Not able to verify actual instruction in HW as Qemu ignores any actual cache 
operations.

Dhaval Sharma (1):
  MdePkg:Implement RISCV CMO

 MdePkg/Library/BaseLib/BaseLib.inf  |   2 +-
 MdePkg/Include/Register/RiscV64/RiscVEncoding.h |   4 +
 MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c | 200 ++--
 MdePkg/Library/BaseLib/RiscV64/FlushCache.S |  21 --
 MdePkg/Library/BaseLib/RiscV64/RiscVCacheMgmt.S |  64 +++
 5 files changed, 254 insertions(+), 37 deletions(-)
 delete mode 100644 MdePkg/Library/BaseLib/RiscV64/FlushCache.S
 create mode 100644 MdePkg/Library/BaseLib/RiscV64/RiscVCacheMgmt.S

-- 
2.34.1



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

[edk2-devel] [PATCH v4 1/1] MdePkg:Implement RISCV CMO

[Dhaval Sharma]

From: Dhaval Sharma 

Implementing code to support Cache Management Operations
(CMO) defined by RV spec https://github.com/riscv/riscv-CMOs

Notes:
1. CMO only supports block based Operations. Meaning complete
   cache flush/invd/clean Operations are not available. In that case
   we fallback on fence.i instructions.
2. Rely on the fact that platform init has initialized CMO and this
   implementation just checks if it is enabled.
3. In order to avoid compiler dependency injecting byte code.

Test:
1. Ensured correct instructions are refelecting in asm
2. Able to boot platform with RiscVVirtQemu config
3. Not able to verify actual instruction in HW as Qemu ignores
any actual cache operations.

Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc: Gerd Hoffmann 
Cc: Sunil V L 
Cc: Andrei Warkentin 
Signed-off-by: Dhaval Sharma 
---

Notes:
v4:
- Removed CMO specific directory in Base Lib
- Implemented compiler independent code for CMO
- Merged CMO implementation with fence.i
- Added logic to confirm CMO is enabled

 MdePkg/Library/BaseLib/BaseLib.inf  |   2 +-
 MdePkg/Include/Register/RiscV64/RiscVEncoding.h |   4 +
 MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c | 200 ++--
 MdePkg/Library/BaseLib/RiscV64/FlushCache.S |  21 --
 MdePkg/Library/BaseLib/RiscV64/RiscVCacheMgmt.S |  64 +++
 5 files changed, 254 insertions(+), 37 deletions(-)

diff --git a/MdePkg/Library/BaseLib/BaseLib.inf 
b/MdePkg/Library/BaseLib/BaseLib.inf
index 03c7b02e828b..53389389448c 100644
--- a/MdePkg/Library/BaseLib/BaseLib.inf
+++ b/MdePkg/Library/BaseLib/BaseLib.inf
@@ -400,7 +400,7 @@ [Sources.RISCV64]
   RiscV64/RiscVCpuBreakpoint.S  | GCC
   RiscV64/RiscVCpuPause.S   | GCC
   RiscV64/RiscVInterrupt.S  | GCC
-  RiscV64/FlushCache.S  | GCC
+  RiscV64/RiscVCacheMgmt.S  | GCC
   RiscV64/CpuScratch.S  | GCC
   RiscV64/ReadTimer.S   | GCC
   RiscV64/RiscVMmu.S| GCC
diff --git a/MdePkg/Include/Register/RiscV64/RiscVEncoding.h 
b/MdePkg/Include/Register/RiscV64/RiscVEncoding.h
index 5c2989b797bf..ea1493578bd5 100644
--- a/MdePkg/Include/Register/RiscV64/RiscVEncoding.h
+++ b/MdePkg/Include/Register/RiscV64/RiscVEncoding.h
@@ -85,6 +85,10 @@
 /* Supervisor Configuration */
 #define CSR_SENVCFG  0x10a
 
+/* Defined CBO bits*/
+#define SENVCFG_CBCFE  0x40UL
+#define SENVCFG_CBIE   0x30UL
+
 /* Supervisor Trap Handling */
 #define CSR_SSCRATCH  0x140
 #define CSR_SEPC  0x141
diff --git a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c 
b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c
index d08fb9f193ca..8b853e5b69fa 100644
--- a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c
+++ b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c
@@ -1,7 +1,8 @@
 /** @file
-  RISC-V specific functionality for cache.
+  Implement Risc-V Cache Management Operations
 
   Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights 
reserved.
+  Copyright (c) 2023, Rivos Inc. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
@@ -10,13 +11,21 @@
 #include 
 #include 
 
+#define RV64_CACHE_BLOCK_SIZE  64
+
+typedef enum {
+  Clean,
+  Flush,
+  Invld,
+} CACHE_OP;
+
 /**
   RISC-V invalidate instruction cache.
 
 **/
 VOID
 EFIAPI
-RiscVInvalidateInstCacheAsm (
+RiscVInvalidateInstCacheAsm_Fence (
   VOID
   );
 
@@ -26,13 +35,144 @@ RiscVInvalidateInstCacheAsm (
 **/
 VOID
 EFIAPI
-RiscVInvalidateDataCacheAsm (
+RiscVInvalidateDataCacheAsm_Fence (
   VOID
   );
 
+/**
+  RISC-V flush cache block. Atomically perform a clean operation
+  followed by an invalidate operation
+
+**/
+VOID
+EFIAPI
+RiscVCpuCacheFlushAsm_Cbo (
+  UINTN
+  );
+
+/**
+Perform a write transfer to another cache or to memory if the
+data in the copy of the cache block have been modified by a store
+operation
+
+**/
+VOID
+EFIAPI
+RiscVCpuCacheCleanAsm_Cbo (
+  UINTN
+  );
+
+/**
+Deallocate the copy of the cache block
+
+**/
+VOID
+EFIAPI
+RiscVCpuCacheInvalAsm_Cbo (
+  UINTN
+  );
+
+/**
+Verify CBOs are supported by this HW
+CBCFE == Cache Block Clean and Flush instruction Enable
+CBIE == Cache Block Invalidate instruction Enable
+
+**/
+UINTN
+RiscvIsCbcfeEnabledAsm (
+  VOID
+  );
+
+UINTN
+RiscvIsCbiEnabledAsm (
+  VOID
+  );
+
+/**
+  Performs required opeartion on cache lines in the cache coherency domain
+  of the calling CPU. If Address is not aligned on a cache line boundary,
+  then entire cache line containing Address is operated. If Address + Length
+  is not aligned on a cache line boundary, then the entire cache line
+  containing Address + Length -1 is operated.
+
+  If Length is greater than (MAX_ADDRESS - Address + 1), then ASSERT().
+
+  @param  Address The base address of the cache lines to
+  invalidate. If the CPU is in a physical addressing mode,
+  then Address is a physical address. If the CPU is in a virtual
+  addressing mo

Re: [edk2-devel] [PATCH v1 1/1] ShellPkg: Acpivew/GTDT: Print timer flags information.

[Pedro Falcato]

Nit: AcpiView, not Acpivew

On Wed, Jul 12, 2023 at 4:16 PM levi.yun  wrote:
>
> Currently, GTDT only prints the value of timer flags in hex.
> This change prints the detail informaiton about Timer flags in GTDT.
Nit: information

>
> before:
> Shell> acpiview -s GTDT
> ...
> Non-Secure EL1 timer FLAGS : 0x2
> Virtual timer GSIV : 0x1B
> Virtual timer FLAGS: 0x2
> ...
>
> after:
> Shell> acpiview -s GTDT
> ...
> Non-Secure EL1 timer FLAGS : 0x2
> Timer Interrupt Mode : Level Trigger(0)
> Timer Interrupt Polarity : Active Low(1)
> Always-on Capability : 0
> Reserved : 0
>
> Virtual timer GSIV : 0x1B
> Virtual timer FLAGS: 0x2
>
> Signed-off-by: levi.yun 
> ---
> The changes can be seen at 
> https://github.com/LeviYeoReum/edk2/compare/master...LeviYeoReum:edk2:refs.geads/2711_gtdt_flags_v1
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c | 111 
> +---
>  1 file changed, 98 insertions(+), 13 deletions(-)
>
> diff --git 
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c 
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
> index 
> e62927098a010a0e1dad8361dcfc6559d32dcebf..0001d4231e88c03fa1e296539e9fbc76eb5dd7e1
>  100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
> @@ -84,33 +84,118 @@ ValidateGtFrameNumber (
>}
>
>  }
>
>
>
> +/**
>
> +  This function prints Triggermode information in timer flags.
typo: trigger mode
>
> +
>
> +  @param [in] Format Print format.
>
> +  @param [in] PtrPointer to the start of the field data.
>
> +**/
>
> +STATIC
>
> +VOID
>
> +EFIAPI
>
> +PrintTimerInterruptMode (
>
> +  IN CONST CHAR16  *Format OPTIONAL,
>
> +  IN UINT8 *Ptr
>
> +  )
>
> +{
>
> +  UINT8  TriggerMode;
>
> +
>
> +  TriggerMode = (*(UINT8 *)Ptr);
You don't need parenthesis here, nor a cast.
>
> +
>
> +  Print (
>
> +L"%s(%d)",
Personal opinion, but a space between %s and (%d) would look better maybe?
>
> +((TriggerMode) ? L"Edge Trigger" : L"Level Trigger"),
You don't need parenthesis around TriggerMode either AFAIK (unless
this is some uncrustify BS).
>
> +TriggerMode
>
> +);
>
> +}
>
> +
>
> +/**
>
> +  This function prints Polarity information in timer flags.
polarity (lowercase)
>
> +
>
> +  @param [in] Format Print format.
>
> +  @param [in] PtrPointer to the start of the field data.
>
> +**/
>
> +STATIC
>
> +VOID
>
> +EFIAPI
>
> +PrintTimerInterruptPolarity (
>
> +  IN CONST CHAR16  *Format OPTIONAL,
>
> +  IN UINT8 *Ptr
>
> +  )
>
> +{
>
> +  UINT8  Polarity;
>
> +
>
> +  Polarity = (*(UINT8 *)Ptr);
>
> +
>
> +  Print (
>
> +L"%s(%d)",
>
> +((Polarity) ? L"Active Low" : L"Active High"),
>
> +Polarity
>
> +);
Same comments as the above function
>
> +}
>
> +
>
> +/**
>
> +  An ACPI_PARSER array describing the Timer Flags Field in GTDT Table.
>
> +**/
>
> +STATIC CONST ACPI_PARSER  TimerFlagsParser[] = {
>
> +  { L"Timer Interrupt Mode", 1,  0, NULL,  PrintTimerInterruptMode, 
> NULL, NULL, NULL },
>
> +  { L"Timer Interrupt Polarity", 1,  1, NULL,  PrintTimerInterruptPolarity, 
> NULL, NULL, NULL },
>
> +  { L"Always-on Capability", 1,  2, L"%d", NULL,
> NULL, NULL, NULL },
>
> +  { L"Reserved", 29, 3, L"%d", NULL,
> NULL, NULL, NULL },
>
> +};
>
> +
>
> +/**
>
> +  This function parses the Timer Flags.
>
> +
>
> +  @param [in]   Format  Print format.
>
> +  @param [in]   Ptr Pointer to the start of the Timer flags.
>
> + **/
>
> +STATIC
>
> +VOID
>
> +EFIAPI
>
> +DumpTimerFlags (
>
> +  IN CONST CHAR16  *Format OPTIONAL,
>
> +  IN UINT8 *Ptr
>
> +  )
>
> +{
>
> +  DumpUint32 (L"0x%x\n", Ptr);
>
> +  ParseAcpiBitFields (
>
> +TRUE,
>
> +2,
>
> +NULL,
>
> +Ptr,
>
> +4,
>
> +PARSER_PARAMS (TimerFlagsParser)
>
> +);
>
> +}
>
> +
>
>  /**
>
>An ACPI_PARSER array describing the ACPI GTDT Table.
>
>  **/
>
>  STATIC CONST ACPI_PARSER  GtdtParser[] = {
>
>PARSE_ACPI_HEADER (&AcpiHdrInfo),
>
> -  { L"CntControlBase Physical Address",8,  36,  L"0x%lx", NULL, NULL,
>
> +  { L"CntControlBase Physical Address",8,  36,  L"0x%lx", NULL,  
>  NULL,
>
>  NULL, NULL },
>
> -  { L"Reserved",  4,  44,  L"0x%x",  NULL, 
> NULL,NULL,  NULL },
>
> -  { L"Secure EL1 timer GSIV", 4,  48,  L"0x%x",  NULL, 
> NULL,NULL,  NULL },
>
> -  { L"Secure EL1 timer FLAGS",4,  52,  L"0x%x",  NULL, 
> NULL,NULL,  NULL },
>
> +  { L"Reserved",  4,  44,  L"0x%x",  NULL,   
> NULL,NULL,  NULL },
>
> +  { L"Secure EL1 timer GSIV", 

[edk2-devel] [PATCH] OvmfPkg/OvmfXen: Fix S3

[Xenia Ragiadakou via groups.io]

Currently, resuming an S3 suspended guest results in the following
assertion failure:
ASSERT 
MdePkg/Library/PeiResourcePublicationLib/PeiResourcePublicationLib.c(41): 
MemoryLength > 0
This happens because some parts of the S3 suspend and resume paths
are missing in order for S3 to work. For instance, the variables
mS3AcpiReservedMemoryBase and mS3AcpiReservedMemoryBase are not
initialized, regions that are used on S3 resume are either missing
or not marked as ACPI NVS memory and can be corrupted by the OS.
This patch adds the missing parts based heavily on the existing S3
implementation of other virtual platforms.

For S3 support, the provision of fw_cfg is required in order for
suspend states to be retrieved.

Another issue noticed is that when CalibrateLapicTimer() is called
on S3 resume path, the shared info page is remapped to a different
guest physical address. This remapping happens under guest's feet,
so any subsequent attempt of the guest to access the shared info
page results in nested page faults. This patch removes any local
APIC timer initializion and calibration from S3 resume path.

Signed-off-by: Xenia Ragiadakou 
---
 OvmfPkg/XenPlatformPei/Fv.c   |  2 +-
 OvmfPkg/XenPlatformPei/MemDetect.c| 60 ++-
 OvmfPkg/XenPlatformPei/Platform.c | 11 -
 OvmfPkg/XenPlatformPei/Platform.h |  2 +
 OvmfPkg/XenPlatformPei/XenPlatformPei.inf |  7 +++
 5 files changed, 78 insertions(+), 4 deletions(-)

diff --git a/OvmfPkg/XenPlatformPei/Fv.c b/OvmfPkg/XenPlatformPei/Fv.c
index 871a2c1c5b..37ecb3cfee 100644
--- a/OvmfPkg/XenPlatformPei/Fv.c
+++ b/OvmfPkg/XenPlatformPei/Fv.c
@@ -37,7 +37,7 @@ PeiFvInitialization (
   BuildMemoryAllocationHob (

 PcdGet32 (PcdOvmfPeiMemFvBase),

 PcdGet32 (PcdOvmfPeiMemFvSize),

-EfiBootServicesData

+mS3Supported ? EfiACPIMemoryNVS : EfiBootServicesData

 );

 

   //

diff --git a/OvmfPkg/XenPlatformPei/MemDetect.c 
b/OvmfPkg/XenPlatformPei/MemDetect.c
index e552e7a55e..1724a4988f 100644
--- a/OvmfPkg/XenPlatformPei/MemDetect.c
+++ b/OvmfPkg/XenPlatformPei/MemDetect.c
@@ -283,6 +283,19 @@ PublishPeiMemory (
 

   LowerMemorySize = GetSystemMemorySizeBelow4gb ();

 

+  //

+  // If S3 is supported, then the S3 permanent PEI memory is placed next,

+  // downwards. Its size is primarily dictated by CpuMpPei. The formula below

+  // is an approximation.

+  //

+  if (mS3Supported) {

+mS3AcpiReservedMemorySize = SIZE_512KB +

+PcdGet32 (PcdCpuMaxLogicalProcessorNumber) *

+PcdGet32 (PcdCpuApStackSize);

+mS3AcpiReservedMemoryBase = LowerMemorySize - mS3AcpiReservedMemorySize;

+LowerMemorySize   = mS3AcpiReservedMemoryBase;

+  }

+

   if (mBootMode == BOOT_ON_S3_RESUME) {

 MemoryBase = mS3AcpiReservedMemoryBase;

 MemorySize = mS3AcpiReservedMemorySize;

@@ -328,6 +341,51 @@ InitializeRamRegions (
 {

   XenPublishRamRegions ();

 

+  if (mS3Supported && (mBootMode != BOOT_ON_S3_RESUME)) {

+//

+// This is the memory range that will be used for PEI on S3 resume

+//

+BuildMemoryAllocationHob (

+  mS3AcpiReservedMemoryBase,

+  mS3AcpiReservedMemorySize,

+  EfiACPIMemoryNVS

+  );

+

+//

+// Cover the initial RAM area used as stack and temporary PEI heap.

+//

+// This is reserved as ACPI NVS so it can be used on S3 resume.

+//

+BuildMemoryAllocationHob (

+  PcdGet32 (PcdOvmfSecPeiTempRamBase),

+  PcdGet32 (PcdOvmfSecPeiTempRamSize),

+  EfiACPIMemoryNVS

+  );

+

+//

+// SEC stores its table of GUIDed section handlers here.

+//

+BuildMemoryAllocationHob (

+  PcdGet64 (PcdGuidedExtractHandlerTableAddress),

+  PcdGet32 (PcdGuidedExtractHandlerTableSize),

+  EfiACPIMemoryNVS

+  );

+

+ #ifdef MDE_CPU_X64

+//

+// Reserve the initial page tables built by the reset vector code.

+//

+// Since this memory range will be used by the Reset Vector on S3

+// resume, it must be reserved as ACPI NVS.

+//

+BuildMemoryAllocationHob (

+  (EFI_PHYSICAL_ADDRESS)(UINTN)PcdGet32 (PcdOvmfSecPageTablesBase),

+  (UINT64)(UINTN)PcdGet32 (PcdOvmfSecPageTablesSize),

+  EfiACPIMemoryNVS

+  );

+ #endif

+  }

+

   if (mBootMode != BOOT_ON_S3_RESUME) {

 //

 // Reserve the lock box storage area

@@ -346,7 +404,7 @@ InitializeRamRegions (
 BuildMemoryAllocationHob (

   (EFI_PHYSICAL_ADDRESS)(UINTN)PcdGet32 (PcdOvmfLockBoxStorageBase),

   (UINT64)(UINTN)PcdGet32 (PcdOvmfLockBoxStorageSize),

-  EfiBootServicesData

+  mS3Supported ? EfiACPIMemoryNVS : EfiBootServicesData

   );

   }

 }

diff --git a/OvmfPkg/XenPlatformPei/Platform.c 
b/OvmfPkg/XenPlatformPei/Platform.c
index c3fdf3d0b8..1b074cff33 100644
--- a/OvmfPkg/XenPlatformPei/Platform.c
+++ b/OvmfPkg/XenPlatformPei/Platform.c
@@ -60,6 +60,8 @@ UINT16  mHostBridgeD

[edk2-devel] Event: TianoCore Community Meeting EMEA/NAMO - Thursday, July 13, 2023 #cal-reminder

[Group Notification]

*Reminder: TianoCore Community Meeting EMEA/NAMO*

*When:*
Thursday, July 13, 2023
8:00am to 9:00am
(UTC-07: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=1940331 )

*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&ivr=teams&d=conf.intel.com&test=test_call
 )

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




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106905): https://edk2.groups.io/g/devel/message/106905
Mute This Topic: https://groups.io/mt/100101253/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] Now: TianoCore Community Meeting EMEA/NAMO - Thursday, July 13, 2023 #cal-notice

[Group Notification]

*TianoCore Community Meeting EMEA/NAMO*

*When:*
Thursday, July 13, 2023
8:00am to 9:00am
(UTC-07: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=1940331 )

*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&ivr=teams&d=conf.intel.com&test=test_call
 )

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




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

Re: [edk2-devel] [edk2-platforms][PATCH V1 09/20] StandaloneMmPkg: parse SP manifest and populate new boot information

[Girish Mahadevan via groups.io]

I had one comment , in-line.

Thanks
Girish

On 7/11/2023 8:36 AM, Nishant Sharma via groups.io wrote:

External email: Use caution opening links or attachments


From: Achin Gupta 

This patch discovers the SP manifest in DT format passed by the SPMC. It
then parses it to obtain the boot information required to initialise the
SP.

Signed-off-by: Achin Gupta 
Signed-off-by: Sayanta Pattanayak 
Signed-off-by: Nishant Sharma 
---
  StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h  
  |   2 +-
  
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
 | 389 +++-
  2 files changed, 381 insertions(+), 10 deletions(-)

diff --git a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h 
b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
index c965192c702e..90d67a2f25b5 100644
--- a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
+++ b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
@@ -2,7 +2,7 @@
Entry point to the Standalone MM Foundation when initialized during the SEC
phase on ARM platforms

-Copyright (c) 2017 - 2021, Arm Ltd. All rights reserved.
+Copyright (c) 2017 - 2023, Arm Ltd. All rights reserved.
  SPDX-License-Identifier: BSD-2-Clause-Patent

  **/
diff --git 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
index 9f6af55c86c4..505786aff07c 100644
--- 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
+++ 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
@@ -38,6 +38,10 @@ SPDX-License-Identifier: BSD-2-Clause-Patent

  #define BOOT_PAYLOAD_VERSION  1

+#define FFA_PAGE_4K 0
+#define FFA_PAGE_16K 1
+#define FFA_PAGE_64K 2
+
  PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT  CpuDriverEntryPoint = NULL;

  /**
@@ -106,6 +110,7 @@ GetAndPrintBootinformation (
}

return PayloadBootInfo;
+}

  /**
An StMM SP implements partial support for FF-A v1.0. The FF-A ABIs are used 
to
@@ -266,6 +271,308 @@ DelegatedEventLoop (
}
  }

+STATIC
+BOOLEAN
+CheckDescription (
+IN VOID   * DtbAddress,
+IN INT32Offset,
+OUT CHAR8 * Description,
+OUT UINT32  Size
+)
+{
+  CONST CHAR8 * Property;
+  INT32 LenP;
+
+  Property = fdt_getprop (DtbAddress, Offset, "description", &LenP);
+  if (Property == NULL) {
+return FALSE;
+  }
+
+ return CompareMem (Description, Property, MIN(Size, (UINT32)LenP)) == 0;
+
+}
+
+STATIC
+EFI_STATUS
+ReadProperty32 (
+IN  VOID   * DtbAddress,
+IN  INT32Offset,
+IN  CHAR8  * Property,
+OUT UINT32 * Value
+)
+{
+  CONST UINT32 * Property32;
+
+  Property32 =  fdt_getprop (DtbAddress, Offset, Property, NULL);
+  if (Property32 == NULL) {
+DEBUG ((
+  DEBUG_ERROR,
+  "%s: Missing in FF-A boot information manifest\n",
+  Property
+  ));
+return EFI_INVALID_PARAMETER;
+  }
+
+  *Value = fdt32_to_cpu (*Property32);
+
+  return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+ReadProperty64 (
+IN  VOID   * DtbAddress,
+IN  INT32Offset,
+IN  CHAR8  * Property,
+OUT UINT64 * Value
+)
+{
+  CONST UINT64 * Property64;
+
+  Property64 =  fdt_getprop (DtbAddress, Offset, Property, NULL);
+  if (Property64 == NULL) {
+DEBUG ((
+  DEBUG_ERROR,
+  "%s: Missing in FF-A boot information manifest\n",
+  Property
+  ));
+return EFI_INVALID_PARAMETER;
+  }
+
+  *Value = fdt64_to_cpu (*Property64);
+
+  return EFI_SUCCESS;
+}
+
+STATIC
+BOOLEAN
+ReadRegionInfo (
+IN VOID  *DtbAddress,
+IN INT32  Node,
+IN CHAR8 *Region,
+IN UINTN  RegionStrSize,
+IN UINT32 PageSize,
+OUT UINT64 *Address,
+OUT UINT64 *Size
+)
+{
+  BOOLEAN FoundBuffer;
+  INTN Status = 0;
+
+  FoundBuffer = CheckDescription (
+  DtbAddress,
+  Node,
+  Region,
+  RegionStrSize
+  );
+  if (!FoundBuffer) {
+return FALSE;
+  }
+
+  DEBUG ((DEBUG_INFO, "Found Node: %a\n", Region));
+  Status = ReadProperty64 (
+  DtbAddress,
+  Node,
+  "base-address",
+  Address
+  );
+  if (Status != EFI_SUCCESS) {
+DEBUG ((DEBUG_ERROR, "base-address missing in DTB"));
+return FALSE;
+  }
+  DEBUG ((
+DEBUG_INFO,
+"base = 0x%llx\n",
+*Address
+));
+
+  Status = ReadProperty32 (
+  DtbAddress,
+  Node,
+  "pages-count",
+  (UINT32*)Size
+  );
+  if (Status != EFI_SUCCESS) {
+DEBUG ((DEBUG_ERROR, "pages-count missing in DTB"));
+return FALSE;
+  }
+
+  DEBUG ((DEBUG_ERROR, "pages-count: 0x%lx\n", *Size));
+
+  *Size = *Size * PageSize;
+  DEBUG ((
+DEBUG_INFO,
+"Size = 0x%llx\n",
+*Size
+));
+
+  return TRUE;
+}
+
+/**
+
+  Populates FF-A boot information structure.
+
+  This function receives the address of a D

[edk2-devel] [PATCH v3 0/2] Automatically set NXCOMPAT bit if requirements are met

[Joey Vagedes via groups.io]

v3: Updates function to be Doxygen compliant
v3: Updates commit message

v2: Adds --nonxcompat flag to GenFw; updates man page
v2: Updates PeImage.h to reference spec 9.3 rather then 8.3

Utilize GenFw to automatically set the NXCOMPAT bit of the DLL Characteristics
field of the Optional Header if the following requirements are met:

1. It is a 64bit PE
2. The section alignment is evently divisible by 4K
3. No section is both EFI_IMAGE_SCN_MEM_EXECUTE and EFI_IMAGE_SCN_MEM_WRITE

Cc: Rebecca Cran 
Cc: Liming Gao 
Cc: Bob Feng 
Cc: Yuwei Chen 

Joey Vagedes (2):
  MdePkg: IndustryStandard: Add DLL Characteristics
  BaseTools: GenFw: auto-set nxcompat flag

 MdePkg/Include/IndustryStandard/PeImage.h|  17 +-
 BaseTools/Source/C/GenFw/GenFw.c |  69 
 BaseTools/UserManuals/GenFw_Utility_Man_Page.rtf | 420 +++-
 3 files changed, 308 insertions(+), 198 deletions(-)

-- 
2.41.0.windows.2



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

[edk2-devel] [PATCH v3 1/2] MdePkg: IndustryStandard: Add DLL Characteristics

[Joey Vagedes via groups.io]

Add the bit masks for DLL Characteristics, used within the optional
header of a PE, to the PeImage.h header file.

Update the Visual Studio, Microsoft Portable Executable and Common
Object File Format Specification, and the PE/COFF Specification to the
latest version.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
Signed-off-by: Joey Vagedes 

Reviewed-by: Michael D Kinney 
---
 MdePkg/Include/IndustryStandard/PeImage.h | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/IndustryStandard/PeImage.h 
b/MdePkg/Include/IndustryStandard/PeImage.h
index 47037049348c..9fdbfb9c4944 100644
--- a/MdePkg/Include/IndustryStandard/PeImage.h
+++ b/MdePkg/Include/IndustryStandard/PeImage.h
@@ -4,7 +4,7 @@
   EFI_IMAGE_NT_HEADERS64 is for PE32+.
 
   This file is coded to the Visual Studio, Microsoft Portable Executable and
-  Common Object File Format Specification, Revision 8.3 - February 6, 2013.
+  Common Object File Format Specification, Revision 9.3 - December 29, 2015.
   This file also includes some definitions in PI Specification, Revision 1.0.
 
 Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
@@ -269,6 +269,21 @@ typedef struct {
 #define EFI_IMAGE_SUBSYSTEM_OS2_CUI  5
 #define EFI_IMAGE_SUBSYSTEM_POSIX_CUI7
 
+//
+// DLL Characteristics
+//
+#define IMAGE_DLLCHARACTERISTICS_HIGH_ENTROPY_VA0x0020
+#define IMAGE_DLLCHARACTERISTICS_DYNAMIC_BASE   0x0040
+#define IMAGE_DLLCHARACTERISTICS_FORCE_INTEGRITY0x0080
+#define IMAGE_DLLCHARACTERISTICS_NX_COMPAT  0x0100
+#define IMAGE_DLLCHARACTERISTICS_NO_ISOLATION   0x0200
+#define IMAGE_DLLCHARACTERISTICS_NO_SEH 0x0400
+#define IMAGE_DLLCHARACTERISTICS_NO_BIND0x0800
+#define IMAGE_DLLCHARACTERISTICS_APPCONTAINER   0x1000
+#define IMAGE_DLLCHARACTERISTICS_WDM_DRIVER 0x2000
+#define IMAGE_DLLCHARACTERISTICS_GUARD_CF   0x4000
+#define IMAGE_DLLCHARACTERISTICS_TERMINAL_SERVER_AWARE  0x8000
+
 ///
 /// Length of ShortName.
 ///
-- 
2.41.0.windows.2



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

[edk2-devel] [PATCH v3 2/2] BaseTools: GenFw: auto-set nxcompat flag

[Joey Vagedes via groups.io]

Automatically set the nxcompat flag in the DLL Characteristics field of
the Optional Header of the PE32+ image. For this flag to be set
automatically, the section alignment must be evenly divisible
by 4K (EFI_PAGE_SIZE) and no section must be executable and writable.

Adds a command line flag to GenFw, --nonxcompat, to ensure the
IMAGE_DLLCHARACTERISTICS_NX_COMPAT bit is not set, even if all
requirements are met. Updates the manual for GenFw to include the new
flag.

Cc: Rebecca Cran 
Cc: Liming Gao 
Cc: Bob Feng 
Cc: Yuwei Chen 
Signed-off-by: Joey Vagedes 
---
 BaseTools/Source/C/GenFw/GenFw.c |  69 
 BaseTools/UserManuals/GenFw_Utility_Man_Page.rtf | 420 +++-
 2 files changed, 292 insertions(+), 197 deletions(-)

diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c
index 0289c8ef8a5c..bd635b375a99 100644
--- a/BaseTools/Source/C/GenFw/GenFw.c
+++ b/BaseTools/Source/C/GenFw/GenFw.c
@@ -86,6 +86,7 @@ UINT32 mImageSize = 0;
 UINT32 mOutImageType = FW_DUMMY_IMAGE;
 BOOLEAN mIsConvertXip = FALSE;
 BOOLEAN mExportFlag = FALSE;
+BOOLEAN mNoNxCompat = FALSE;
 
 STATIC
 EFI_STATUS
@@ -281,6 +282,9 @@ Returns:
 write export table into PE-COFF.\n\
 This option can be used together with -e.\n\
 It doesn't work for other options.\n");
+  fprintf (stdout, "  --nonxcompat  Do not set the 
IMAGE_DLLCHARACTERISTICS_NX_COMPAT bit \n\
+of the optional header in the PE header even if the \n\
+requirements are met.\n");
   fprintf (stdout, "  -v, --verbose Turn on verbose output with 
informational messages.\n");
   fprintf (stdout, "  -q, --quiet   Disable all messages except key 
message and fatal error\n");
   fprintf (stdout, "  -d, --debug level Enable debug messages, at input 
debug level.\n");
@@ -441,6 +445,59 @@ Returns:
   return STATUS_SUCCESS;
 }
 
+/**
+
+  Checks if the Pe image is nxcompat compliant.
+
+  Must meet the following conditions:
+  1. The PE is 64bit
+  2. The section alignment is evenly divisible by 4k
+  3. No section is writable and executable.
+
+  @param  PeHdr - The PE header
+
+  @retval TRUE  - The PE is nx compat compliant
+  @retval FALSE - The PE is not nx compat compliant
+
+**/
+STATIC
+BOOLEAN
+IsNxCompatCompliant (
+  EFI_IMAGE_OPTIONAL_HEADER_UNION  *PeHdr
+  )
+{
+  EFI_IMAGE_SECTION_HEADER *SectionHeader;
+  UINT32   Index;
+  UINT32   Mask;
+
+  // Must have an optional header to perform verification
+  if (PeHdr->Pe32.FileHeader.SizeOfOptionalHeader == 0) {
+return FALSE;
+  }
+
+  // Verify PE is 64 bit
+  if (!(PeHdr->Pe32.OptionalHeader.Magic == 
EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC)) {
+return FALSE;
+  }
+
+  // Verify Section Alignment is divisible by 4K
+  if (!((PeHdr->Pe32Plus.OptionalHeader.SectionAlignment % EFI_PAGE_SIZE) == 
0)) {
+return FALSE;
+  }
+
+  // Verify sections are not Write & Execute
+  Mask = EFI_IMAGE_SCN_MEM_EXECUTE | EFI_IMAGE_SCN_MEM_WRITE;
+  SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) 
&(PeHdr->Pe32Plus.OptionalHeader) + 
PeHdr->Pe32Plus.FileHeader.SizeOfOptionalHeader);
+  for (Index = 0; Index < PeHdr->Pe32Plus.FileHeader.NumberOfSections; Index 
++, SectionHeader ++) {
+if ((SectionHeader->Characteristics & Mask) == Mask) {
+  return FALSE;
+}
+  }
+
+  // Passed all requirements, return TRUE
+  return TRUE;
+}
+
 VOID
 SetHiiResourceHeader (
   UINT8   *HiiBinData,
@@ -1452,6 +1509,13 @@ Returns:
   continue;
 }
 
+if (stricmp (argv[0], "--nonxcompat") == 0) {
+  mNoNxCompat = TRUE;
+  argc --;
+  argv ++;
+  continue;
+}
+
 if (argv[0][0] == '-') {
   Error (NULL, 0, 1000, "Unknown option", argv[0]);
   goto Finish;
@@ -2458,6 +2522,11 @@ Returns:
 TEImageHeader.BaseOfCode  = Optional64->BaseOfCode;
 TEImageHeader.ImageBase   = (UINT64) (Optional64->ImageBase);
 
+// Set NxCompat flag
+if (IsNxCompatCompliant (PeHdr) && !mNoNxCompat) {
+  Optional64->DllCharacteristics |= IMAGE_DLLCHARACTERISTICS_NX_COMPAT;
+}
+
 if (Optional64->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC) 
{
   
TEImageHeader.DataDirectory[EFI_TE_IMAGE_DIRECTORY_ENTRY_BASERELOC].VirtualAddress
 = 
Optional64->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC].VirtualAddress;
   TEImageHeader.DataDirectory[EFI_TE_IMAGE_DIRECTORY_ENTRY_BASERELOC].Size 
= Optional64->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC].Size;
diff --git a/BaseTools/UserManuals/GenFw_Utility_Man_Page.rtf 
b/BaseTools/UserManuals/GenFw_Utility_Man_Page.rtf
index f4235b77fbce..6046b9fefd65 100644
--- a/BaseTools/UserManuals/GenFw_Utility_Man_Page.rtf
+++ b/BaseTools/UserManuals/GenFw_Utility_Man_Page.rtf
@@ -1,28 +1,29 @@
-{\rtf1\adeflang1025\ansi\ansicpg1252\uc2\adeff0\deff0\stshfdbch31505\sts

Re: [edk2-devel] [PATCH v1 1/1] BaseTools: BinToPcd: Resolve xdrlib deprecation

[Joey Vagedes via groups.io]

Thank you for the review Michael. @Rebecca Cran , @Liming
Gao  have you had time to take a look at this? It
is a fairly simple change, following the same logic as xdrlib with a few
modifications to use some newer python functionality.

Thanks,
Joey

On Tue, Jun 27, 2023 at 10:21 AM Kinney, Michael D <
michael.d.kin...@intel.com> wrote:

> Thank you for fixing this.
>
> Reviewed-by: Michael D Kinney 
>
> > -Original Message-
> > From: Joey Vagedes 
> > Sent: Tuesday, June 27, 2023 9:27 AM
> > To: devel@edk2.groups.io
> > Cc: Rebecca Cran ; Gao, Liming
> > ; Feng, Bob C ; Chen,
> > Christine ; Kinney, Michael D
> > 
> > Subject: [PATCH v1 1/1] BaseTools: BinToPcd: Resolve xdrlib deprecation
> >
> > Removes the dependency on xdrlib and replaces it with custom logic to
> > pack a per the xdr requirements. Necessary as xdrlib is being deprecated
> > in python 3.13.
> >
> > Cc: Rebecca Cran 
> > Cc: Liming Gao 
> > Cc: Bob Feng 
> > Cc: Yuwei Chen 
> > Cc: Michael D Kinney 
> > Signed-off-by: Joey Vagedes 
> > ---
> >  BaseTools/Scripts/BinToPcd.py | 19 +++
> >  1 file changed, 15 insertions(+), 4 deletions(-)
> >
> > diff --git a/BaseTools/Scripts/BinToPcd.py
> b/BaseTools/Scripts/BinToPcd.py
> > index 3bc557b8412c..460c08b7f7cd 100644
> > --- a/BaseTools/Scripts/BinToPcd.py
> > +++ b/BaseTools/Scripts/BinToPcd.py
> > @@ -14,6 +14,9 @@ import sys
> >  import argparse
> >
> >  import re
> >
> >  import xdrlib
> >
> > +import io
> >
> > +import struct
> >
> > +import math
> >
> >
> >
> >  #
> >
> >  # Globals for help information
> >
> > @@ -46,16 +49,24 @@ if __name__ == '__main__':
> >  raise argparse.ArgumentTypeError (Message)
> >
> >  return Argument
> >
> >
> >
> > +def XdrPackBuffer (buffer):
> >
> > +packed_bytes = io.BytesIO()
> >
> > +for unpacked_bytes in buffer:
> >
> > +n = len(unpacked_bytes)
> >
> > +packed_bytes.write(struct.pack('>L',n))
> >
> > +data = unpacked_bytes[:n]
> >
> > +n = math.ceil(n/4)*4
> >
> > +data = data + (n - len(data)) * b'\0'
> >
> > +packed_bytes.write(data)
> >
> > +return packed_bytes.getvalue()
> >
> > +
> >
> >  def ByteArray (Buffer, Xdr = False):
> >
> >  if Xdr:
> >
> >  #
> >
> >  # If Xdr flag is set then encode data using the Variable-
> > Length Opaque
> >
> >  # Data format of RFC 4506 External Data Representation
> > Standard (XDR).
> >
> >  #
> >
> > -XdrEncoder = xdrlib.Packer ()
> >
> > -for Item in Buffer:
> >
> > -XdrEncoder.pack_bytes (Item)
> >
> > -Buffer = bytearray (XdrEncoder.get_buffer ())
> >
> > +Buffer = bytearray (XdrPackBuffer (Buffer))
> >
> >  else:
> >
> >  #
> >
> >  # If Xdr flag is not set, then concatenate all the data
> >
> > --
> > 2.41.0.windows.1
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106911): https://edk2.groups.io/g/devel/message/106911
Mute This Topic: https://groups.io/mt/99813913/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] BaseTools: BinToPcd: Resolve xdrlib deprecation

[Rebecca Cran]

Sorry I haven’t had a chance to look at it. I’ll try and find some time this 
weekend.

On Thu, Jul 13, 2023, at 9:26 AM, Joey Vagedes via groups.io wrote:
> Thank you for the review Michael. @Rebecca Cran 
> , @Liming Gao 
>  have you had time to take a look at 
> this? It is a fairly simple change, following the same logic as xdrlib 
> with a few modifications to use some newer python functionality.
>
> Thanks,
> Joey
>
> On Tue, Jun 27, 2023 at 10:21 AM Kinney, Michael D 
>  wrote:
>> Thank you for fixing this.
>> 
>> Reviewed-by: Michael D Kinney 
>> 
>> > -Original Message-
>> > From: Joey Vagedes 
>> > Sent: Tuesday, June 27, 2023 9:27 AM
>> > To: devel@edk2.groups.io
>> > Cc: Rebecca Cran ; Gao, Liming
>> > ; Feng, Bob C ; Chen,
>> > Christine ; Kinney, Michael D
>> > 
>> > Subject: [PATCH v1 1/1] BaseTools: BinToPcd: Resolve xdrlib deprecation
>> > 
>> > Removes the dependency on xdrlib and replaces it with custom logic to
>> > pack a per the xdr requirements. Necessary as xdrlib is being deprecated
>> > in python 3.13.
>> > 
>> > Cc: Rebecca Cran 
>> > Cc: Liming Gao 
>> > Cc: Bob Feng 
>> > Cc: Yuwei Chen 
>> > Cc: Michael D Kinney 
>> > Signed-off-by: Joey Vagedes 
>> > ---
>> >  BaseTools/Scripts/BinToPcd.py | 19 +++
>> >  1 file changed, 15 insertions(+), 4 deletions(-)
>> > 
>> > diff --git a/BaseTools/Scripts/BinToPcd.py b/BaseTools/Scripts/BinToPcd.py
>> > index 3bc557b8412c..460c08b7f7cd 100644
>> > --- a/BaseTools/Scripts/BinToPcd.py
>> > +++ b/BaseTools/Scripts/BinToPcd.py
>> > @@ -14,6 +14,9 @@ import sys
>> >  import argparse
>> > 
>> >  import re
>> > 
>> >  import xdrlib
>> > 
>> > +import io
>> > 
>> > +import struct
>> > 
>> > +import math
>> > 
>> > 
>> > 
>> >  #
>> > 
>> >  # Globals for help information
>> > 
>> > @@ -46,16 +49,24 @@ if __name__ == '__main__':
>> >  raise argparse.ArgumentTypeError (Message)
>> > 
>> >  return Argument
>> > 
>> > 
>> > 
>> > +def XdrPackBuffer (buffer):
>> > 
>> > +packed_bytes = io.BytesIO()
>> > 
>> > +for unpacked_bytes in buffer:
>> > 
>> > +n = len(unpacked_bytes)
>> > 
>> > +packed_bytes.write(struct.pack('>L',n))
>> > 
>> > +data = unpacked_bytes[:n]
>> > 
>> > +n = math.ceil(n/4)*4
>> > 
>> > +data = data + (n - len(data)) * b'\0'
>> > 
>> > +packed_bytes.write(data)
>> > 
>> > +return packed_bytes.getvalue()
>> > 
>> > +
>> > 
>> >  def ByteArray (Buffer, Xdr = False):
>> > 
>> >  if Xdr:
>> > 
>> >  #
>> > 
>> >  # If Xdr flag is set then encode data using the Variable-
>> > Length Opaque
>> > 
>> >  # Data format of RFC 4506 External Data Representation
>> > Standard (XDR).
>> > 
>> >  #
>> > 
>> > -XdrEncoder = xdrlib.Packer ()
>> > 
>> > -for Item in Buffer:
>> > 
>> > -XdrEncoder.pack_bytes (Item)
>> > 
>> > -Buffer = bytearray (XdrEncoder.get_buffer ())
>> > 
>> > +Buffer = bytearray (XdrPackBuffer (Buffer))
>> > 
>> >  else:
>> > 
>> >  #
>> > 
>> >  # If Xdr flag is not set, then concatenate all the data
>> > 
>> > --
>> > 2.41.0.windows.1
>> 
> 


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

Re: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver binding start issue.

[Saloni Kasbekar]

Mike,

Would you be able to help us merge the patch?

Thanks,
Saloni

-Original Message-
From: Nickle Wang  
Sent: Wednesday, July 12, 2023 11:54 PM
To: devel@edk2.groups.io; Kasbekar, Saloni ; 
Clark-williams, Zachary 
Cc: Abner Chang ; Igor Kulchytskyy ; Nick 
Ramirez 
Subject: RE: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver binding 
start issue.

Hi Saloni,

Could you please help to merge this fix since there is no objection during past 
weeks?

Thanks,
Nickle

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Saloni 
> Kasbekar via groups.io
> Sent: Friday, June 30, 2023 6:28 AM
> To: Nickle Wang ; devel@edk2.groups.io
> Cc: Maciej Rabeda ; Siyuan Fu 
> ; Abner Chang ; Igor 
> Kulchytskyy ; Nick Ramirez 
> Subject: Re: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver 
> binding start issue.
> 
> External email: Use caution opening links or attachments
> 
> 
> Hi Nickle,
> 
> That makes sense. Thanks for the clarification.
> 
> Reviewed-by: Saloni Kasbekar 
> 
> Thanks,
> Saloni
> 
> -Original Message-
> From: Nickle Wang 
> Sent: Wednesday, June 28, 2023 3:30 PM
> To: Kasbekar, Saloni ; devel@edk2.groups.io
> Cc: Maciej Rabeda ; Siyuan Fu 
> ; Abner Chang ; Igor 
> Kulchytskyy ; Nick Ramirez 
> Subject: RE: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver 
> binding start issue.
> 
> Hi Saloni,
> 
> Thanks for your review.
> 
> When uninstall fails, per UEFI specification, the protocol will be 
> installed again and will be visible to UEFI drivers.
> 
> Page 190, UEFI spec. 2.10:
> "If any errors are generated while the protocol interfaces are being 
> uninstalled, then the protocols uninstalled prior to the error will be 
> reinstalled with the boot service
> EFI_BOOT_SERVICES.InstallProtocolInterface() and the status code 
> EFI_INVALID_PARAMETER is returned."
> 
> In this case, if we do FreePool while driver still can locate 
> gEfiHttpServiceBindingProtocolGuid. Driver will access to the memory 
> that is released to system. Memory issue may happen.
> 
> Regards,
> Nickle
> 
> > -Original Message-
> > From: Kasbekar, Saloni 
> > Sent: Thursday, June 29, 2023 3:07 AM
> > To: devel@edk2.groups.io; Nickle Wang 
> > Cc: Maciej Rabeda ; Siyuan Fu 
> > ; Abner Chang ; Igor 
> > Kulchytskyy ; Nick Ramirez 
> > Subject: RE: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver 
> > binding start issue.
> >
> > External email: Use caution opening links or attachments
> >
> >
> > Hi Nickle,
> >
> > We would want to do the FreePool even if the Uninstall fails (like 
> > in the case where we failed to install the multiple protocol 
> > interfaces and then went to ON_ERROR). Do you think it's better if 
> > we change it to -
> >
> >   if (HttpService != NULL) {
> > HttpCleanService (HttpService, UsingIpv6);
> > Status = gBS->UninstallMultipleProtocolInterfaces (
> > &ControllerHandle,
> > &gEfiHttpServiceBindingProtocolGuid,
> > &HttpService->ServiceBinding,
> > NULL
> > );
> > if ((HttpService->Tcp4ChildHandle == NULL) && 
> > (HttpService->Tcp6ChildHandle == NULL)) {
> > FreePool (HttpService);
> > }
> >   }
> >
> > Thanks,
> > Saloni
> >
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of 
> > Nickle Wang via groups.io
> > Sent: Tuesday, June 27, 2023 5:56 PM
> > To: devel@edk2.groups.io; Nickle Wang 
> > Cc: Maciej Rabeda ; Siyuan Fu 
> > ; Abner Chang ; Igor 
> > Kulchytskyy ; Nick Ramirez 
> > Subject: Re: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver 
> > binding start issue.
> >
> > May I know if someone can help to review this patch?
> >
> > Thanks,
> > Nickle
> >
> > > -Original Message-
> > > From: devel@edk2.groups.io  On Behalf Of 
> > > Nickle Wang via groups.io
> > > Sent: Friday, February 10, 2023 8:34 PM
> > > To: devel@edk2.groups.io
> > > Cc: Maciej Rabeda ; Siyuan Fu 
> > > ; Abner Chang ; Igor 
> > > Kulchytskyy ; Nick Ramirez 
> > > Subject: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver 
> > > binding start issue.
> > >
> > > External email: Use caution opening links or attachments
> > >
> > >
> > > When failure happens in HttpDxeStart, the error handling code 
> > > release the memory buffer but it does not uninstall HTTP service 
> > > bindnig protocol. As the result, application can still locate this 
> > > protocol and invoke service binding fucntions in released memory pool.
> > >
> > > Signed-off-by: Nickle Wang 
> > > Cc: Maciej Rabeda 
> > > Cc: Siyuan Fu 
> > > Cc: Abner Chang 
> > > Cc: Igor Kulchytskyy 
> > > Cc: Nick Ramirez 
> > > ---
> > >  NetworkPkg/HttpDxe/HttpDriver.c | 13 +++--
> > >  1 file changed, 11 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/NetworkPkg/HttpDxe/HttpDriver.c 
> > > b/NetworkPkg/HttpDxe/HttpDriver.c index 5d918d3c4d..f6d1263cad
> > > 100644
> > > --- a/NetworkPkg/HttpDxe/HttpDriver.c
> > 

Re: [edk2-devel] [edk2-platforms][PATCH V1 09/20] StandaloneMmPkg: parse SP manifest and populate new boot information

[Chris Fernald]

Achin/Nishant, could you explain the motivation behind falling back to 
device tree for some of the secure partition information? It seems like 
we have this large abstraction framework using FF-A and it seems a bit 
odd to have the secure partition have to directly read device tree for 
some of this information when everything else is query-able from the 
framework itself.


Thanks,

Chris Fernald

On 7/13/2023 8:24 AM, Girish Mahadevan via groups.io wrote:

I had one comment , in-line.

Thanks
Girish

On 7/11/2023 8:36 AM, Nishant Sharma via groups.io wrote:

External email: Use caution opening links or attachments


From: Achin Gupta 

This patch discovers the SP manifest in DT format passed by the SPMC. It
then parses it to obtain the boot information required to initialise the
SP.

Signed-off-by: Achin Gupta 
Signed-off-by: Sayanta Pattanayak 
Signed-off-by: Nishant Sharma 
---
StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h |   
2 +-
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c 
| 389 +++-

  2 files changed, 381 insertions(+), 10 deletions(-)

diff --git 
a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h 
b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h

index c965192c702e..90d67a2f25b5 100644
--- a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
+++ b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
@@ -2,7 +2,7 @@
    Entry point to the Standalone MM Foundation when initialized 
during the SEC

    phase on ARM platforms

-Copyright (c) 2017 - 2021, Arm Ltd. All rights reserved.
+Copyright (c) 2017 - 2023, Arm Ltd. All rights reserved.
  SPDX-License-Identifier: BSD-2-Clause-Patent

  **/
diff --git 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c 


index 9f6af55c86c4..505786aff07c 100644
--- 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
+++ 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c

@@ -38,6 +38,10 @@ SPDX-License-Identifier: BSD-2-Clause-Patent

  #define BOOT_PAYLOAD_VERSION  1

+#define FFA_PAGE_4K 0
+#define FFA_PAGE_16K 1
+#define FFA_PAGE_64K 2
+
  PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT  CpuDriverEntryPoint = NULL;

  /**
@@ -106,6 +110,7 @@ GetAndPrintBootinformation (
    }

    return PayloadBootInfo;
+}

  /**
    An StMM SP implements partial support for FF-A v1.0. The FF-A 
ABIs are used to

@@ -266,6 +271,308 @@ DelegatedEventLoop (
    }
  }

+STATIC
+BOOLEAN
+CheckDescription (
+    IN VOID   * DtbAddress,
+    IN INT32    Offset,
+    OUT CHAR8 * Description,
+    OUT UINT32  Size
+    )
+{
+  CONST CHAR8 * Property;
+  INT32 LenP;
+
+  Property = fdt_getprop (DtbAddress, Offset, "description", &LenP);
+  if (Property == NULL) {
+    return FALSE;
+  }
+
+ return CompareMem (Description, Property, MIN(Size, (UINT32)LenP)) 
== 0;

+
+}
+
+STATIC
+EFI_STATUS
+ReadProperty32 (
+    IN  VOID   * DtbAddress,
+    IN  INT32    Offset,
+    IN  CHAR8  * Property,
+    OUT UINT32 * Value
+    )
+{
+  CONST UINT32 * Property32;
+
+  Property32 =  fdt_getprop (DtbAddress, Offset, Property, NULL);
+  if (Property32 == NULL) {
+    DEBUG ((
+  DEBUG_ERROR,
+  "%s: Missing in FF-A boot information manifest\n",
+  Property
+  ));
+    return EFI_INVALID_PARAMETER;
+  }
+
+  *Value = fdt32_to_cpu (*Property32);
+
+  return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+ReadProperty64 (
+    IN  VOID   * DtbAddress,
+    IN  INT32    Offset,
+    IN  CHAR8  * Property,
+    OUT UINT64 * Value
+    )
+{
+  CONST UINT64 * Property64;
+
+  Property64 =  fdt_getprop (DtbAddress, Offset, Property, NULL);
+  if (Property64 == NULL) {
+    DEBUG ((
+  DEBUG_ERROR,
+  "%s: Missing in FF-A boot information manifest\n",
+  Property
+  ));
+    return EFI_INVALID_PARAMETER;
+  }
+
+  *Value = fdt64_to_cpu (*Property64);
+
+  return EFI_SUCCESS;
+}
+
+STATIC
+BOOLEAN
+ReadRegionInfo (
+    IN VOID  *DtbAddress,
+    IN INT32  Node,
+    IN CHAR8 *Region,
+    IN UINTN  RegionStrSize,
+    IN UINT32 PageSize,
+    OUT UINT64 *Address,
+    OUT UINT64 *Size
+    )
+{
+  BOOLEAN FoundBuffer;
+  INTN Status = 0;
+
+  FoundBuffer = CheckDescription (
+  DtbAddress,
+  Node,
+  Region,
+  RegionStrSize
+  );
+  if (!FoundBuffer) {
+    return FALSE;
+  }
+
+  DEBUG ((DEBUG_INFO, "Found Node: %a\n", Region));
+  Status = ReadProperty64 (
+  DtbAddress,
+  Node,
+  "base-address",
+  Address
+  );
+  if (Status != EFI_SUCCESS) {
+    DEBUG ((DEBUG_ERROR, "base-address missing in DTB"));
+    return FALSE;
+  }
+  DEBUG ((
+    DEBUG_INFO,
+    "base = 0x%llx\n",
+    *Address
+    ));
+
+  Status = ReadProperty32 (
+  DtbAddress,
+  Node,
+  "pages-coun

Re: [edk2-devel] [edk2-platforms][PATCH V1 14/20] ArmPkg/MmCommunicationDxe: Introduce FF-A version check

[Chris Fernald]

Might as well use MAKE_FFA_VERSION from ArmFfaSvc.h for consistency.

-Chris

On 7/11/2023 7:36 AM, Nishant Sharma wrote:

From: Achin Gupta 

This patch adds support for querying whether FF-A v1.1 is supported by the
FF-A impplementation.

Signed-off-by: Achin Gupta 
Signed-off-by: Nishant Sharma 
---
  ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf |  3 +++
  ArmPkg/Drivers/MmCommunicationDxe/MmCommunicate.h |  7 ++-
  ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c   | 17 -
  3 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf 
b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
index 05b6de73ff34..c15b1a7a86ae 100644
--- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
@@ -31,6 +31,9 @@
ArmPkg/ArmPkg.dec
MdePkg/MdePkg.dec
  
+[FixedPcd]

+  gArmTokenSpaceGuid.PcdFfaEnable
+
  [LibraryClasses]
ArmLib
ArmSmcLib
diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunicate.h 
b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunicate.h
index 5c5fcb576856..71edf7f49174 100644
--- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunicate.h
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunicate.h
@@ -16,7 +16,12 @@
  #define MM_MAJOR_VER(x)  (((x) & MM_MAJOR_VER_MASK) >> MM_MAJOR_VER_SHIFT)
  #define MM_MINOR_VER(x)  ((x) & MM_MINOR_VER_MASK)
  
+#if (FixedPcdGet32 (PcdFfaEnable) == 1)

  #define MM_CALLER_MAJOR_VER  0x1UL
-#define MM_CALLER_MINOR_VER  0x0
+#define MM_CALLER_MINOR_VER  0x1UL
+#else
+#define MM_CALLER_MAJOR_VER  0x1UL
+#define MM_CALLER_MINOR_VER  0x0UL
+#endif
  
  #endif /* MM_COMMUNICATE_H_ */

diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c 
b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
index 85d9034555f0..a6fcd590a65b 100644
--- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
@@ -18,6 +18,7 @@
  
  #include 
  
+#include 

  #include 
  
  #include "MmCommunicate.h"

@@ -250,14 +251,20 @@ GetMmCompatibility (
  {
EFI_STATUSStatus;
UINT32MmVersion;
-  ARM_SMC_ARGS  MmVersionArgs;
+  ARM_SMC_ARGS  SmcArgs = {0};
  
-  // MM_VERSION uses SMC32 calling conventions

-  MmVersionArgs.Arg0 = ARM_SMC_ID_MM_VERSION_AARCH32;
+  if (FixedPcdGet32 (PcdFfaEnable) != 0) {
+SmcArgs.Arg0 = ARM_SVC_ID_FFA_VERSION_AARCH32;
+SmcArgs.Arg1 = MM_CALLER_MAJOR_VER << MM_MAJOR_VER_SHIFT;
+SmcArgs.Arg1 |= MM_CALLER_MINOR_VER;
+  } else {
+// MM_VERSION uses SMC32 calling conventions
+SmcArgs.Arg0 = ARM_SMC_ID_MM_VERSION_AARCH32;
+  }
  
-  ArmCallSmc (&MmVersionArgs);

+  ArmCallSmc (&SmcArgs);
  
-  MmVersion = MmVersionArgs.Arg0;

+  MmVersion = SmcArgs.Arg0;
  
if ((MM_MAJOR_VER (MmVersion) == MM_CALLER_MAJOR_VER) &&

(MM_MINOR_VER (MmVersion) >= MM_CALLER_MINOR_VER))



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

Re: [edk2-devel] ArmVirtPkg: non-executable EFI_LOADER_DATA breaks GRUB on Ubuntu 22.04

[Pedro Falcato]

On Tue, Jul 11, 2023 at 7:58 AM Gerd Hoffmann  wrote:
>
> On Mon, Jul 10, 2023 at 04:58:15PM +0100, Pedro Falcato wrote:
> > On Mon, Jul 10, 2023 at 2:28 PM  wrote:
> > >
> > > I have an existing install of Ubuntu 22.04 on a QEMU virtual machine 
> > > which I've decided to update the UEFI firmware. After doing so, GRUB no 
> > > longer boots ("Synchronous Exception" message seen). After a git bisect 
> > > session, I found the problematic 
> > > 2997ae38739756ecba9b0de19e86032ebc689ef9. The comment says GRUB should 
> > > have been fixed in 2017, but for one reason or another, my VM which was 
> > > built in 2022 still had the issue. Regardless, I don't think it's a good 
> > > idea to break GRUB, even if it's fixed in 2017. In the very least, a 
> > > better error message would be preferable to crashing with an "Synchronous 
> > > Exception." Googling this error message shows that other people may be 
> > > hitting this issue as well but the vague error symptom means its 
> > > impossible to know if it's the same issue or not.
> >
> > +CC Some of the folks involved in the original discussion
> >
> > In the original thread, people discussed some alternative behavior to
> > just crashing on a NX fault. Is this still an alternative?
>
> The idea is: Improve page fault handler to (a) print a big'n'fat
> warning, and (b) loosening up memory permissions for the faulting
> page address.
>
> No patch for that emerged (yet?).

Ack. I can work on that.

> > I'm kind of thinking this should be addressed by distros anyway
> > How is $CURRENT_YEAR Ubuntu still shipping bad GRUBs? I know the
> > situation around GRUB and distro patching is complicated but...
> > Do we have any idea of how many distros/GRUBs are affected by this?
>
> Too many :(

Ugh, even the latest releases?

> > Personally, I would like to avoid loosening up memory permissions.
>
> Well, you can't have both.  You have to pick between strict nx handling
> and grub bug compatibility ...

Yes. IMO it should be ok to add a hack around NX handling if there's a
solid plan for dealing with this from the distros' side (and phasing
this out). And I'm assuming upstream GRUB has this fixed.
This whole situation is kind of messy as firmware people add new
restrictions that weren't really there in the first place.

Also, what's the situation on this for x86? I assume it's a lot worse there?

-- 
Pedro


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

[edk2-devel] [PATCH v3 0/2] MdeModulePkg/Bus/Ata/AtaAtapiPassThru: Fix

[Ranbir Singh]

v2 -> v3:
  - [Patch 2] Update as per review comments

v1 -> v2:
  - Retain outer cast
  - Add error check instead of Status storage removal

Ranbir Singh (2):
  MdeModulePkg/Bus/Ata/AtaAtapiPassThru: Fix SIGN_EXTENSION Coverity
issue
  MdeModulePkg/Bus/Ata/AtaAtapiPassThru: Fix UNUSED_VALUE Coverity issue

 MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c |  2 +-
 MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c  | 10 +-
 2 files changed, 10 insertions(+), 2 deletions(-)

-- 
2.34.1



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

[edk2-devel] [PATCH v3 1/2] MdeModulePkg/Bus/Ata/AtaAtapiPassThru: Fix SIGN_EXTENSION Coverity issue

[Ranbir Singh]

From: Ranbir Singh 

Line number 1348 does contain a typecast with UINT32, but it is after
all the operations (16-bit left shift followed by OR'ing) are over.
To avoid any SIGN_EXTENSION, typecast the intermediate result after
16-bit left shift operation immediately with UINT32.

Cc: Hao A Wu 
Cc: Ray Ni 
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4204
Signed-off-by: Ranbir Singh 
Signed-off-by: Ranbir Singh 
Reviewed-by: Hao A Wu 
---

 MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c 
b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
index 50406fe0270d..f39c909d0631 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
@@ -1345,7 +1345,7 @@ AtaPassThruPassThru (
 // Check logical block size
 //
 if ((IdentifyData->AtaData.phy_logic_sector_support & BIT12) != 0) {
-  BlockSize = (UINT32)(((IdentifyData->AtaData.logic_sector_size_hi << 16) 
| IdentifyData->AtaData.logic_sector_size_lo) * sizeof (UINT16));
+  BlockSize = 
(UINT32)(((UINT32)(IdentifyData->AtaData.logic_sector_size_hi << 16) | 
IdentifyData->AtaData.logic_sector_size_lo) * sizeof (UINT16));
 }
   }
 
-- 
2.34.1



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

[edk2-devel] [PATCH v3 2/2] MdeModulePkg/Bus/Ata/AtaAtapiPassThru: Fix UNUSED_VALUE Coverity issue

[Ranbir Singh]

From: Ranbir Singh 

The return value stored in Status after call to SetDriveParameters
is not made of any use thereafter and hence it remains as UNUSED.

Add error check as is done after calls to SetDeviceTransferMode.

Cc: Hao A Wu 
Cc: Ray Ni 
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4204
Signed-off-by: Ranbir Singh 
Signed-off-by: Ranbir Singh 
---
 MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c 
b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
index 75403886e44a..af022139cf02 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
@@ -2549,13 +2549,21 @@ DetectAndConfigIdeDevice (
 //
 if (DeviceType == EfiIdeHarddisk) {
   //
-  // Init driver parameters
+  // Init drive parameters
   //
   DriveParameters.Sector = (UINT8)((ATA5_IDENTIFY_DATA 
*)(&Buffer.AtaData))->sectors_per_track;
   DriveParameters.Heads  = (UINT8)(((ATA5_IDENTIFY_DATA 
*)(&Buffer.AtaData))->heads - 1);
   DriveParameters.MultipleSector = (UINT8)((ATA5_IDENTIFY_DATA 
*)(&Buffer.AtaData))->multi_sector_cmd_max_sct_cnt;
 
   Status = SetDriveParameters (Instance, IdeChannel, IdeDevice, 
&DriveParameters, NULL);
+
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_WARN, "Set Drive Parameters Fail, Status = %r\n", 
Status));
+//
+// Ignore warning and proceed normally
+//
+Status = EFI_SUCCESS;
+  }
 }
 
 //
-- 
2.34.1



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

Re: [edk2-devel] [edk2-platforms][PATCH V1 18/20] ArmPkg/MmCommunicationDxe: Discover the StMM SP

[Chris Fernald]

Should |EFI_FFA_PART_INFO_DESC be packed since its used across an ABI? 
Also there are a few immediately actionable TODO's in this commit.

|

|-Chris
|

On 7/11/2023 7:36 AM, Nishant Sharma wrote:

From: Achin Gupta 

This patch adds support for discovering the presence of the SP using the
EFI_MM_COMMUNICATION_PROTOCOL GUID that implements Standalone MM
drivers. This is done by querying the framework through
FFA_PARTITION_INFO_GET whether any partition that implements the
EFI_MM_COMMUNICATION_PROTOCOL is present or not. The partition ID and
its properties are stashed for use in subsequent communication with the
StMM SP.

Signed-off-by: Achin Gupta 
Signed-off-by: Nishant Sharma 
---
  ArmPkg/Include/IndustryStandard/ArmFfaSvc.h | 24 +
  ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 93 +++-
  2 files changed, 114 insertions(+), 3 deletions(-)

diff --git a/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h 
b/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
index f78442a465e1..530af8bd3c2e 100644
--- a/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
+++ b/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
@@ -19,6 +19,9 @@
  #define ARM_SVC_ID_FFA_VERSION_AARCH32   0x8463
  #define ARM_SVC_ID_FFA_RXTX_MAP_AARCH32  0x8466
  #define ARM_SVC_ID_FFA_RXTX_MAP_AARCH64  0xC466
+#define ARM_SVC_ID_FFA_RX_RELEASE_AARCH320x8465
+#define ARM_SVC_ID_FFA_RXTX_UNMAP_AARCH320x8467
+#define ARM_SVC_ID_FFA_PARTITION_INFO_GET_AARCH320x8468
  #define ARM_SVC_ID_FFA_ID_GET_AARCH320x8469
  #define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH32   0x846F
  #define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP_AARCH32  0x8470
@@ -154,4 +157,25 @@ typedef struct {
UINT64 Reserved;
  } EFI_FFA_BOOT_INFO_HEADER;
  
+// FF-A partition information descriptor

+typedef struct {
+  UINT16 PartId;
+  UINT16 EcCnt;
+  UINT32 PartProps;
+  UINT32 PartGuid[4];
+} EFI_FFA_PART_INFO_DESC;
+
+#define PART_INFO_PROP_MASK0x3f
+#define PART_INFO_PROP_SHIFT   0
+#define PART_INFO_PROP_DIR_MSG_RECV_BIT(1u << 0)
+#define PART_INFO_PROP_DIR_MSG_SEND_BIT(1u << 1)
+#define PART_INFO_PROP_INDIR_MSG_BIT   (1u << 2)
+#define PART_INFO_PROP_NOTIFICATIONS_BIT   (1u << 3)
+#define PART_INFO_PROP_EP_TYPE_MASK0x3
+#define PART_INFO_PROP_EP_TYPE_SHIFT   4
+#define PART_INFO_PROP_EP_PE   0
+#define PART_INFO_PROP_EP_SEPID_IND1
+#define PART_INFO_PROP_EP_SEPID_DEP2
+#define PART_INFO_PROP_EP_AUX  3
+
  #endif // ARM_FFA_SVC_H_
diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c 
b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
index 39a1b329b9ea..94a5d96c051d 100644
--- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
@@ -8,6 +8,7 @@
  
  #include 

  #include 
+#include 
  #include 
  #include 
  #include 
@@ -28,6 +29,11 @@
  //
  STATIC UINT16  mFfaPartId;
  
+// Partition information of the StMM SP if FF-A support is enabled

+// TODO: Revisit assumption that there is only a single StMM SP
+//
+STATIC EFI_FFA_PART_INFO_DESC mStmmPartInfo;
+
  //
  // RX/TX pair if FF-A support is enabled
  //
@@ -298,7 +304,9 @@ GetMmCompatibility (
  {
EFI_STATUSStatus;
UINT32MmVersion;
+  UINT32SmccUuid[4];
ARM_SMC_ARGS  SmcArgs = {0};
+  EFI_GUID  MmCommProtGuid = EFI_MM_COMMUNICATION_PROTOCOL_GUID;
  
if (FixedPcdGet32 (PcdFfaEnable) != 0) {

  SmcArgs.Arg0 = ARM_SVC_ID_FFA_VERSION_AARCH32;
@@ -335,14 +343,21 @@ GetMmCompatibility (
  Status = EFI_UNSUPPORTED;
}
  
-  // If FF-A is supported then discover our ID and register our RX/TX buffers.

+  // If FF-A is supported then discover the StMM SP's presence, ID, our ID and
+  // register our RX/TX buffers.
if (FixedPcdGet32 (PcdFfaEnable) != 0) {
+EFI_FFA_PART_INFO_DESC *StmmPartInfo;
+
  // Get our ID
  ZeroMem(&SmcArgs, sizeof(SmcArgs));
  SmcArgs.Arg0 = ARM_SVC_ID_FFA_ID_GET_AARCH32;
  ArmCallSmc (&SmcArgs);
  if (SmcArgs.Arg0 == ARM_SVC_ID_FFA_ERROR_AARCH32) {
-  DEBUG ((DEBUG_ERROR, "Unable to retrieve FF-A partition ID (%d).\n", 
SmcArgs.Arg2));
+  DEBUG ((
+DEBUG_ERROR,
+"Unable to retrieve FF-A partition ID (%d).\n",
+SmcArgs.Arg2
+));
return EFI_UNSUPPORTED;
  }
  DEBUG ((DEBUG_INFO, "FF-A partition ID = 0x%lx.\n", SmcArgs.Arg2));
@@ -355,11 +370,83 @@ GetMmCompatibility (
  SmcArgs.Arg3 = EFI_PAGE_SIZE / SIZE_4KB;
  ArmCallSmc (&SmcArgs);
  if (SmcArgs.Arg0 == ARM_SVC_ID_FFA_ERROR_AARCH32) {
-  DEBUG ((DEBUG_ERROR, "Unable to register FF-A RX/TX buffers (%d).\n", 
SmcArgs.Arg2));
+  DEBUG ((
+DEBUG_ERROR,
+"Unable to register FF-A RX/TX buffers (%d).\n",
+SmcArgs.Arg2
+));
return EFI_UN

Re: [edk2-devel] ArmVirtPkg: non-executable EFI_LOADER_DATA breaks GRUB on Ubuntu 22.04

[Ard Biesheuvel]

On Thu, 13 Jul 2023 at 18:57, Pedro Falcato  wrote:
>
> On Tue, Jul 11, 2023 at 7:58 AM Gerd Hoffmann  wrote:
> >
> > On Mon, Jul 10, 2023 at 04:58:15PM +0100, Pedro Falcato wrote:
> > > On Mon, Jul 10, 2023 at 2:28 PM  wrote:
> > > >
> > > > I have an existing install of Ubuntu 22.04 on a QEMU virtual machine 
> > > > which I've decided to update the UEFI firmware. After doing so, GRUB no 
> > > > longer boots ("Synchronous Exception" message seen). After a git bisect 
> > > > session, I found the problematic 
> > > > 2997ae38739756ecba9b0de19e86032ebc689ef9. The comment says GRUB should 
> > > > have been fixed in 2017, but for one reason or another, my VM which was 
> > > > built in 2022 still had the issue. Regardless, I don't think it's a 
> > > > good idea to break GRUB, even if it's fixed in 2017. In the very least, 
> > > > a better error message would be preferable to crashing with an 
> > > > "Synchronous Exception." Googling this error message shows that other 
> > > > people may be hitting this issue as well but the vague error symptom 
> > > > means its impossible to know if it's the same issue or not.
> > >
> > > +CC Some of the folks involved in the original discussion
> > >
> > > In the original thread, people discussed some alternative behavior to
> > > just crashing on a NX fault. Is this still an alternative?
> >
> > The idea is: Improve page fault handler to (a) print a big'n'fat
> > warning, and (b) loosening up memory permissions for the faulting
> > page address.
> >
> > No patch for that emerged (yet?).
>
> Ack. I can work on that.
>
> > > I'm kind of thinking this should be addressed by distros anyway
> > > How is $CURRENT_YEAR Ubuntu still shipping bad GRUBs? I know the
> > > situation around GRUB and distro patching is complicated but...
> > > Do we have any idea of how many distros/GRUBs are affected by this?
> >
> > Too many :(
>
> Ugh, even the latest releases?
>
> > > Personally, I would like to avoid loosening up memory permissions.
> >
> > Well, you can't have both.  You have to pick between strict nx handling
> > and grub bug compatibility ...
>
> Yes. IMO it should be ok to add a hack around NX handling if there's a
> solid plan for dealing with this from the distros' side (and phasing
> this out). And I'm assuming upstream GRUB has this fixed.
> This whole situation is kind of messy as firmware people add new
> restrictions that weren't really there in the first place.
>
> Also, what's the situation on this for x86? I assume it's a lot worse there?
>

To be honest, I have little sympathy for the gigantic mess that the
distros have created for themselves with GRUB, shim, etc. Mainline
GRUB works fine with mainline EDK2, and secure boot in a arm64 VM is
rather pointless, given that the [emulated] NOR flash is writable by
the guest OS. The breakage is in the downstream GRUB changes that make
it interoperate with shim, and its hacked up PE loader.

If we are going to accommodate every broken GRUB build that the
distros ever released, we won't be able to make any progress on this
front. I understand that the distros need to support their existing
user bases, so I am willing to consider facilities that make it easier
to create builds that work around such issues.

However, just turning off NX support is not one of the options.
Upstream is not what the distros are shipping, this applies to GRUB
and shim as well as EDK2: so if their downstream GRUB breaks EDK2,
they can fix it in their EDK2 builds, either by carrying a code
change, or by enabling an upstream build flag that is off by default.


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

Re: [edk2-devel] ArmVirtPkg: non-executable EFI_LOADER_DATA breaks GRUB on Ubuntu 22.04

[Pedro Falcato]

On Thu, Jul 13, 2023 at 6:20 PM Ard Biesheuvel  wrote:
>
> On Thu, 13 Jul 2023 at 18:57, Pedro Falcato  wrote:
> >
> > On Tue, Jul 11, 2023 at 7:58 AM Gerd Hoffmann  wrote:
> > >
> > > On Mon, Jul 10, 2023 at 04:58:15PM +0100, Pedro Falcato wrote:
> > > > On Mon, Jul 10, 2023 at 2:28 PM  wrote:
> > > > >
> > > > > I have an existing install of Ubuntu 22.04 on a QEMU virtual machine 
> > > > > which I've decided to update the UEFI firmware. After doing so, GRUB 
> > > > > no longer boots ("Synchronous Exception" message seen). After a git 
> > > > > bisect session, I found the problematic 
> > > > > 2997ae38739756ecba9b0de19e86032ebc689ef9. The comment says GRUB 
> > > > > should have been fixed in 2017, but for one reason or another, my VM 
> > > > > which was built in 2022 still had the issue. Regardless, I don't 
> > > > > think it's a good idea to break GRUB, even if it's fixed in 2017. In 
> > > > > the very least, a better error message would be preferable to 
> > > > > crashing with an "Synchronous Exception." Googling this error message 
> > > > > shows that other people may be hitting this issue as well but the 
> > > > > vague error symptom means its impossible to know if it's the same 
> > > > > issue or not.
> > > >
> > > > +CC Some of the folks involved in the original discussion
> > > >
> > > > In the original thread, people discussed some alternative behavior to
> > > > just crashing on a NX fault. Is this still an alternative?
> > >
> > > The idea is: Improve page fault handler to (a) print a big'n'fat
> > > warning, and (b) loosening up memory permissions for the faulting
> > > page address.
> > >
> > > No patch for that emerged (yet?).
> >
> > Ack. I can work on that.
> >
> > > > I'm kind of thinking this should be addressed by distros anyway
> > > > How is $CURRENT_YEAR Ubuntu still shipping bad GRUBs? I know the
> > > > situation around GRUB and distro patching is complicated but...
> > > > Do we have any idea of how many distros/GRUBs are affected by this?
> > >
> > > Too many :(
> >
> > Ugh, even the latest releases?
> >
> > > > Personally, I would like to avoid loosening up memory permissions.
> > >
> > > Well, you can't have both.  You have to pick between strict nx handling
> > > and grub bug compatibility ...
> >
> > Yes. IMO it should be ok to add a hack around NX handling if there's a
> > solid plan for dealing with this from the distros' side (and phasing
> > this out). And I'm assuming upstream GRUB has this fixed.
> > This whole situation is kind of messy as firmware people add new
> > restrictions that weren't really there in the first place.
> >
> > Also, what's the situation on this for x86? I assume it's a lot worse there?
> >
>
> To be honest, I have little sympathy for the gigantic mess that the
> distros have created for themselves with GRUB, shim, etc. Mainline
> GRUB works fine with mainline EDK2, and secure boot in a arm64 VM is
> rather pointless, given that the [emulated] NOR flash is writable by
> the guest OS. The breakage is in the downstream GRUB changes that make
> it interoperate with shim, and its hacked up PE loader.
>
> If we are going to accommodate every broken GRUB build that the
> distros ever released, we won't be able to make any progress on this
> front. I understand that the distros need to support their existing
> user bases, so I am willing to consider facilities that make it easier
> to create builds that work around such issues.
>
> However, just turning off NX support is not one of the options.
> Upstream is not what the distros are shipping, this applies to GRUB
> and shim as well as EDK2: so if their downstream GRUB breaks EDK2,
> they can fix it in their EDK2 builds, either by carrying a code
> change, or by enabling an upstream build flag that is off by default.

I understand your sentiment, but I don't see how this can work. Let's
say Fedora has a fixed GRUB (I have no idea if this is the case), so
they have a fully-NX'd edk2. Then someone tries to boot Ubuntu 22.04 -
it crashes because Ubuntu's GRUB is borked; what now? I don't know if
this case is super prevalent in virtualization, but it's definitely a
problem (and it seems to have happened to osy here?).

I agree that turning off NX sucks, but so does not being able to boot
into distros as recent as Ubuntu 22.04. It might be that a single
Sleep(10); +  a nice loud error message gets the idea across? maybe
over a stable tag or so, then we remove the hack and add full-NX. What
do you think?

-- 
Pedro


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

Re: [edk2-devel] ArmVirtPkg: non-executable EFI_LOADER_DATA breaks GRUB on Ubuntu 22.04

[Oliver Smith-Denny]

On 7/13/2023 10:41 AM, Pedro Falcato wrote:

On Thu, Jul 13, 2023 at 6:20 PM Ard Biesheuvel  wrote:


On Thu, 13 Jul 2023 at 18:57, Pedro Falcato  wrote:


On Tue, Jul 11, 2023 at 7:58 AM Gerd Hoffmann  wrote:


On Mon, Jul 10, 2023 at 04:58:15PM +0100, Pedro Falcato wrote:

On Mon, Jul 10, 2023 at 2:28 PM  wrote:


I have an existing install of Ubuntu 22.04 on a QEMU virtual machine which I've decided to update 
the UEFI firmware. After doing so, GRUB no longer boots ("Synchronous Exception" message 
seen). After a git bisect session, I found the problematic 
2997ae38739756ecba9b0de19e86032ebc689ef9. The comment says GRUB should have been fixed in 2017, but 
for one reason or another, my VM which was built in 2022 still had the issue. Regardless, I don't 
think it's a good idea to break GRUB, even if it's fixed in 2017. In the very least, a better error 
message would be preferable to crashing with an "Synchronous Exception." Googling this 
error message shows that other people may be hitting this issue as well but the vague error symptom 
means its impossible to know if it's the same issue or not.


+CC Some of the folks involved in the original discussion

In the original thread, people discussed some alternative behavior to
just crashing on a NX fault. Is this still an alternative?


The idea is: Improve page fault handler to (a) print a big'n'fat
warning, and (b) loosening up memory permissions for the faulting
page address.

No patch for that emerged (yet?).


Ack. I can work on that.


I'm kind of thinking this should be addressed by distros anyway
How is $CURRENT_YEAR Ubuntu still shipping bad GRUBs? I know the
situation around GRUB and distro patching is complicated but...
Do we have any idea of how many distros/GRUBs are affected by this?


Too many :(


Ugh, even the latest releases?


Personally, I would like to avoid loosening up memory permissions.


Well, you can't have both.  You have to pick between strict nx handling
and grub bug compatibility ...


Yes. IMO it should be ok to add a hack around NX handling if there's a
solid plan for dealing with this from the distros' side (and phasing
this out). And I'm assuming upstream GRUB has this fixed.
This whole situation is kind of messy as firmware people add new
restrictions that weren't really there in the first place.

Also, what's the situation on this for x86? I assume it's a lot worse there?



To be honest, I have little sympathy for the gigantic mess that the
distros have created for themselves with GRUB, shim, etc. Mainline
GRUB works fine with mainline EDK2, and secure boot in a arm64 VM is
rather pointless, given that the [emulated] NOR flash is writable by
the guest OS. The breakage is in the downstream GRUB changes that make
it interoperate with shim, and its hacked up PE loader.

If we are going to accommodate every broken GRUB build that the
distros ever released, we won't be able to make any progress on this
front. I understand that the distros need to support their existing
user bases, so I am willing to consider facilities that make it easier
to create builds that work around such issues.

However, just turning off NX support is not one of the options.
Upstream is not what the distros are shipping, this applies to GRUB
and shim as well as EDK2: so if their downstream GRUB breaks EDK2,
they can fix it in their EDK2 builds, either by carrying a code
change, or by enabling an upstream build flag that is off by default.


I understand your sentiment, but I don't see how this can work. Let's
say Fedora has a fixed GRUB (I have no idea if this is the case), so
they have a fully-NX'd edk2. Then someone tries to boot Ubuntu 22.04 -
it crashes because Ubuntu's GRUB is borked; what now? I don't know if
this case is super prevalent in virtualization, but it's definitely a
problem (and it seems to have happened to osy here?).

I agree that turning off NX sucks, but so does not being able to boot
into distros as recent as Ubuntu 22.04. It might be that a single
Sleep(10); +  a nice loud error message gets the idea across? maybe
over a stable tag or so, then we remove the hack and add full-NX. What
do you think?



I agree with Ard here. If other software is buggy and outdated, our
general approach should be fix your outdated and buggy software,
especially when there are security and safety implications to bending
backwards for broken code.

A downstream platform may well need to support buggy OSes, etc. for the
lifespan of the device, but upstream edk2 is not the right place to add
hacks for buggy external software. edk2 is our opportunity to do the
right thing and help encourage the community to the right place.

When distros have the maintenance burden of working with downstream
platforms to support their lack of updating grub etc., they may decide
it is worthwhile to actually update grub...

Thanks,
Oliver


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

Re: [edk2-devel] [PATCH v4 5/7] OvmfPkg/RiscVVirt: Add VirtNorFlashDxe to APRIORI list

[Tuan Phan]

On Tue, Jul 4, 2023 at 12:01 AM Sunil V L  wrote:

> On Mon, Jul 03, 2023 at 11:45:45PM -0700, Tuan Phan wrote:
> > As i said, VirtNorFlashDxe needed to be loaded before VariableRuntimeDxe
> so
> > your suggestion will not work.
> >
> Okay, at least for me, by removing APRIORI patch and adding this depex,
> edk2 boots fine with your series. I am not sure what won't work.
>
> Hi Ard, any thoughts? If no better way, may be we have to use APRIORI.
>
It doesn't work as your workaround trying to make CpuDxe depends on
variable protocol which has nothing to do with it. Also, CpuDxe is an
essential module and should not depend on anything, what happens if the
variable driver before generating the protocol tries to use CPU protocol?
It is worse than having APRIORI workaround.

>
> Thanks,
> Sunil
> > On Mon, Jul 3, 2023 at 10:07 PM Sunil V L 
> wrote:
> >
> > > On Wed, Jun 28, 2023 at 02:27:10PM -0700, Tuan Phan wrote:
> > > > On Wed, Jun 28, 2023 at 9:47 AM Sunil V L 
> > > wrote:
> > > >
> > > > > On Fri, Jun 23, 2023 at 11:39:32AM -0700, Tuan Phan wrote:
> > > > > > Make sure VirtNorFlashDxe loaded before VariableRuntimeDxe as it
> > > > > > is the backend flash driver.
> > > > > >
> > > > > > Signed-off-by: Tuan Phan 
> > > > > > ---
> > > > > >  OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf | 10 ++
> > > > > >  1 file changed, 10 insertions(+)
> > > > > >
> > > > > > diff --git a/OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf
> > > > > b/OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf
> > > > > > index 21e4ba67379f..9ab8eb3ba7d8 100644
> > > > > > --- a/OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf
> > > > > > +++ b/OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf
> > > > > > @@ -53,6 +53,16 @@ READ_STATUS= TRUE
> > > > > >  READ_LOCK_CAP  = TRUE
> > > > > >  READ_LOCK_STATUS   = TRUE
> > > > > >
> > > > > > +APRIORI DXE {
> > > > > > +  INF  MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
> > > > > > +  INF  MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
> > > > > > +  INF
> > > > >
> > >
> MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/ReportStatusCodeRouterRuntimeDxe.inf
> > > > > > +  INF
> > > > >
> > >
> MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.inf
> > > > > > +  INF  EmbeddedPkg/Drivers/FdtClientDxe/FdtClientDxe.inf
> > > > > > +  INF  UefiCpuPkg/CpuDxeRiscV64/CpuDxeRiscV64.inf
> > > > > > +  INF  OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf
> > > > > > +}
> > > > > > +
> > > > > Hi Tuan,
> > > > >
> > > > > Actually, Ard had recommended not to use APRIORI and hence we
> avoided
> > > > > it when we upstreamed RiscVVirt. So, I am wondering whether this
> can be
> > > > > avoided by using depex in CpuDxe on gEfiVariableArchProtocolGuid?
> > > > >
> > > > > Hi Sunil,
> > > > Not sure what the reason behind avoiding APRIORI besides it is a
> > > workaround
> > > > for broken DEPEX. BTW, what we need is to put VirtNorFlashDxe loaded
> > > before
> > > > VariableRuntimeDxe which doesn't depend on any modules. I don't see
> any
> > > > other clearer way than modifying VirNorFlashDxe as shown in the first
> > > > version of this series.
> > > >
> > > > The CpuDxeRiscV64 in the aprioriy list as VirNorFlashDxe depends on
> it.
> > > >
> > > Hi Tuan,
> > >
> > > I couldn't locate old mail from Ard recommending to remove APRIORI in
> > > RISC-V. But here is the recent mail on different context but those
> > > reasons are still valid in any case.
> > > https://edk2.groups.io/g/devel/message/104543
> > >
> > > IMO, there is no dependency between VirtNorFlashDxe and
> > > VariableRuntimeDxe. I think what we need is CpuDxeRiscV64 loaded after
> > > VariableRuntimeDxe and before VirtNorFlashDxe. A simple depex like I
> > > suggested in previous mail should work. I still prefer this than
> > > introducing APRIORI unless there are other issues I am now aware of.
> > > What do you think?
> > >
> > > Thanks!
> > > Sunil
> > >
>


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

Re: [edk2-devel] [edk2-platforms][PATCH V1 09/20] StandaloneMmPkg: parse SP manifest and populate new boot information

[Achin Gupta]

Hi Chris,

Prior to FF-A, an impdef structure populated by TF-A was passed to a
SPM_MM based StMM SP. The data structures used for this are
EFI_SECURE_PARTITION_BOOT_INFO and EFI_SECURE_PARTITION_CPU_INFO.

With FF-A, we have reduced the amount of information that is passed
e.g. the per-cpu information is not passed. The remaining information
has been generalized so that it is not specific to StMM SPs.

This information is only used at boot time. Hence, passing it once via
the FF-A boot information protocol made sense.

The FF-A spec does not specify the format in which boot information is
passed. An SP manifest is specified in a DT by default. So, this was
the natural choice. The same information could be passed using a HOB
list but that would require additional support in TF-A.

I hope this helps.

cheers,
Achin


On Thu, 2023-07-13 at 09:48 -0700, Chris Fernald wrote:
> Achin/Nishant, could you explain the motivation behind falling back
> to
> device tree for some of the secure partition information? It seems
> like
> we have this large abstraction framework using FF-A and it seems a
> bit
> odd to have the secure partition have to directly read device tree
> for
> some of this information when everything else is query-able from the
> framework itself.
>
> Thanks,
>
> Chris Fernald
>
> On 7/13/2023 8:24 AM, Girish Mahadevan via groups.io wrote:
> > I had one comment , in-line.
> >
> > Thanks
> > Girish
> >
> > On 7/11/2023 8:36 AM, Nishant Sharma via groups.io wrote:
> > > External email: Use caution opening links or attachments
> > >
> > >
> > > From: Achin Gupta 
> > >
> > > This patch discovers the SP manifest in DT format passed by the
> > > SPMC. It
> > > then parses it to obtain the boot information required to
> > > initialise the
> > > SP.
> > >
> > > Signed-off-by: Achin Gupta 
> > > Signed-off-by: Sayanta Pattanayak 
> > > Signed-off-by: Nishant Sharma 
> > > ---
> > > StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
> > > |
> > > 2 +-
> > > StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/Standalone
> > > MmCoreEntryPoint.c
> > > > 389 +++-
> > >   2 files changed, 381 insertions(+), 10 deletions(-)
> > >
> > > diff --git
> > > a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.
> > > h
> > > b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.
> > > h
> > > index c965192c702e..90d67a2f25b5 100644
> > > ---
> > > a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.
> > > h
> > > +++
> > > b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.
> > > h
> > > @@ -2,7 +2,7 @@
> > > Entry point to the Standalone MM Foundation when initialized
> > > during the SEC
> > > phase on ARM platforms
> > >
> > > -Copyright (c) 2017 - 2021, Arm Ltd. All rights reserved.
> > > +Copyright (c) 2017 - 2023, Arm Ltd. All rights reserved.
> > >   SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > >   **/
> > > diff --git
> > > a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/Standalo
> > > neMmCoreEntryPoint.c
> > > b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/Standalo
> > > neMmCoreEntryPoint.c
> > >
> > > index 9f6af55c86c4..505786aff07c 100644
> > > ---
> > > a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/Standalo
> > > neMmCoreEntryPoint.c
> > > +++
> > > b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/Standalo
> > > neMmCoreEntryPoint.c
> > > @@ -38,6 +38,10 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > >   #define BOOT_PAYLOAD_VERSION  1
> > >
> > > +#define FFA_PAGE_4K 0
> > > +#define FFA_PAGE_16K 1
> > > +#define FFA_PAGE_64K 2
> > > +
> > >   PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT  CpuDriverEntryPoint = NULL;
> > >
> > >   /**
> > > @@ -106,6 +110,7 @@ GetAndPrintBootinformation (
> > > }
> > >
> > > return PayloadBootInfo;
> > > +}
> > >
> > >   /**
> > > An StMM SP implements partial support for FF-A v1.0. The FF-A
> > > ABIs are used to
> > > @@ -266,6 +271,308 @@ DelegatedEventLoop (
> > > }
> > >   }
> > >
> > > +STATIC
> > > +BOOLEAN
> > > +CheckDescription (
> > > +IN VOID   * DtbAddress,
> > > +IN INT32Offset,
> > > +OUT CHAR8 * Description,
> > > +OUT UINT32  Size
> > > +)
> > > +{
> > > +  CONST CHAR8 * Property;
> > > +  INT32 LenP;
> > > +
> > > +  Property = fdt_getprop (DtbAddress, Offset, "description",
> > > &LenP);
> > > +  if (Property == NULL) {
> > > +return FALSE;
> > > +  }
> > > +
> > > + return CompareMem (Description, Property, MIN(Size,
> > > (UINT32)LenP))
> > > == 0;
> > > +
> > > +}
> > > +
> > > +STATIC
> > > +EFI_STATUS
> > > +ReadProperty32 (
> > > +IN  VOID   * DtbAddress,
> > > +IN  INT32Offset,
> > > +IN  CHAR8  * Property,
> > > +OUT UINT32 * Value
> > > +)
> > > +{
> > > +  CONST UINT32 * Property32;
> > > +
> > > +  Property32 =  fdt_getprop (DtbAddress, Offset, Property,
> > > NULL);
> > > +  if (Property32 == NULL) {
> > > +DEBUG ((
> > > +

Re: [edk2-devel] ArmVirtPkg: non-executable EFI_LOADER_DATA breaks GRUB on Ubuntu 22.04

[Michael D Kinney]

A general approach to this type of issues is to enable the new feature by 
default
and optionally provide a setup option to disable the new feature.

If there is a way to detect the failure case and fail gracefully back to the 
boot manager with an error message that indicates the type of issue and also
indicates if there is a setup option to disable a feature to allow that OS
to boot, then the user can choose to opt-in to disabling the feature.

Can also consider a warning message when a system is in a defeatured state
so the user knows that every time they boot so they can re-enable if they
only needed to disable for a short period of time.

Mike

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Oliver
> Smith-Denny
> Sent: Thursday, July 13, 2023 11:23 AM
> To: devel@edk2.groups.io; pedro.falc...@gmail.com; Ard Biesheuvel
> 
> Cc: Gerd Hoffmann ; o...@turing.llc; Leif Lindholm
> ; dann frazier 
> Subject: Re: [edk2-devel] ArmVirtPkg: non-executable EFI_LOADER_DATA
> breaks GRUB on Ubuntu 22.04
> 
> On 7/13/2023 10:41 AM, Pedro Falcato wrote:
> > On Thu, Jul 13, 2023 at 6:20 PM Ard Biesheuvel  wrote:
> >>
> >> On Thu, 13 Jul 2023 at 18:57, Pedro Falcato 
> wrote:
> >>>
> >>> On Tue, Jul 11, 2023 at 7:58 AM Gerd Hoffmann 
> wrote:
> 
>  On Mon, Jul 10, 2023 at 04:58:15PM +0100, Pedro Falcato wrote:
> > On Mon, Jul 10, 2023 at 2:28 PM  wrote:
> >>
> >> I have an existing install of Ubuntu 22.04 on a QEMU virtual
> machine which I've decided to update the UEFI firmware. After doing so,
> GRUB no longer boots ("Synchronous Exception" message seen). After a git
> bisect session, I found the problematic
> 2997ae38739756ecba9b0de19e86032ebc689ef9. The comment says GRUB should
> have been fixed in 2017, but for one reason or another, my VM which was
> built in 2022 still had the issue. Regardless, I don't think it's a good
> idea to break GRUB, even if it's fixed in 2017. In the very least, a
> better error message would be preferable to crashing with an
> "Synchronous Exception." Googling this error message shows that other
> people may be hitting this issue as well but the vague error symptom
> means its impossible to know if it's the same issue or not.
> >
> > +CC Some of the folks involved in the original discussion
> >
> > In the original thread, people discussed some alternative behavior
> to
> > just crashing on a NX fault. Is this still an alternative?
> 
>  The idea is: Improve page fault handler to (a) print a big'n'fat
>  warning, and (b) loosening up memory permissions for the faulting
>  page address.
> 
>  No patch for that emerged (yet?).
> >>>
> >>> Ack. I can work on that.
> >>>
> > I'm kind of thinking this should be addressed by distros
> anyway
> > How is $CURRENT_YEAR Ubuntu still shipping bad GRUBs? I know the
> > situation around GRUB and distro patching is complicated but...
> > Do we have any idea of how many distros/GRUBs are affected by
> this?
> 
>  Too many :(
> >>>
> >>> Ugh, even the latest releases?
> >>>
> > Personally, I would like to avoid loosening up memory permissions.
> 
>  Well, you can't have both.  You have to pick between strict nx
> handling
>  and grub bug compatibility ...
> >>>
> >>> Yes. IMO it should be ok to add a hack around NX handling if there's
> a
> >>> solid plan for dealing with this from the distros' side (and phasing
> >>> this out). And I'm assuming upstream GRUB has this fixed.
> >>> This whole situation is kind of messy as firmware people add new
> >>> restrictions that weren't really there in the first place.
> >>>
> >>> Also, what's the situation on this for x86? I assume it's a lot
> worse there?
> >>>
> >>
> >> To be honest, I have little sympathy for the gigantic mess that the
> >> distros have created for themselves with GRUB, shim, etc. Mainline
> >> GRUB works fine with mainline EDK2, and secure boot in a arm64 VM is
> >> rather pointless, given that the [emulated] NOR flash is writable by
> >> the guest OS. The breakage is in the downstream GRUB changes that
> make
> >> it interoperate with shim, and its hacked up PE loader.
> >>
> >> If we are going to accommodate every broken GRUB build that the
> >> distros ever released, we won't be able to make any progress on this
> >> front. I understand that the distros need to support their existing
> >> user bases, so I am willing to consider facilities that make it
> easier
> >> to create builds that work around such issues.
> >>
> >> However, just turning off NX support is not one of the options.
> >> Upstream is not what the distros are shipping, this applies to GRUB
> >> and shim as well as EDK2: so if their downstream GRUB breaks EDK2,
> >> they can fix it in their EDK2 builds, either by carrying a code
> >> change, or by enabling an upstream build flag that is off by default.
> >
> > I understand your sentiment, but I don't see how this can work. Let's
> > say Fedo

[edk2-devel] Event: TianoCore Community Meeting - APAC/NAMO - Thursday, July 13, 2023 #cal-reminder

[Group Notification]

*Reminder: TianoCore Community Meeting - APAC/NAMO*

*When:*
Thursday, July 13, 2023
7:30pm to 8:30pm
(UTC-07: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=1940328 )

*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&ivr=teams&d=conf.intel.com&test=test_call
 )

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




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106927): https://edk2.groups.io/g/devel/message/106927
Mute This Topic: https://groups.io/mt/100113362/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] [PATCH v4 5/7] OvmfPkg/RiscVVirt: Add VirtNorFlashDxe to APRIORI list

[Sunil V L]

On Thu, Jul 13, 2023 at 12:08:02PM -0700, Tuan Phan wrote:
> On Tue, Jul 4, 2023 at 12:01 AM Sunil V L  wrote:
> 
> > On Mon, Jul 03, 2023 at 11:45:45PM -0700, Tuan Phan wrote:
> > > As i said, VirtNorFlashDxe needed to be loaded before VariableRuntimeDxe
> > so
> > > your suggestion will not work.
> > >
> > Okay, at least for me, by removing APRIORI patch and adding this depex,
> > edk2 boots fine with your series. I am not sure what won't work.
> >
> > Hi Ard, any thoughts? If no better way, may be we have to use APRIORI.
> >
> It doesn't work as your workaround trying to make CpuDxe depends on
> variable protocol which has nothing to do with it. Also, CpuDxe is an
> essential module and should not depend on anything, what happens if the
> variable driver before generating the protocol tries to use CPU protocol?
> It is worse than having APRIORI workaround.
> 
Okay. Thanks!. Let's go with this solution for now.  But let me do some
tests to ensure this builds fine with CLANG and then merge.

Thanks,
Sunil


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

Re: [edk2-devel] [PATCH v3 2/2] MdeModulePkg/Bus/Ata/AtaAtapiPassThru: Fix UNUSED_VALUE Coverity issue

[Wu, Hao A]

> -Original Message-
> From: Ranbir Singh 
> Sent: Friday, July 14, 2023 12:47 AM
> To: devel@edk2.groups.io; rsi...@ventanamicro.com
> Cc: Wu, Hao A ; Ni, Ray 
> Subject: [PATCH v3 2/2] MdeModulePkg/Bus/Ata/AtaAtapiPassThru: Fix
> UNUSED_VALUE Coverity issue
> 
> From: Ranbir Singh 
> 
> The return value stored in Status after call to SetDriveParameters is not made
> of any use thereafter and hence it remains as UNUSED.
> 
> Add error check as is done after calls to SetDeviceTransferMode.
> 
> Cc: Hao A Wu 
> Cc: Ray Ni 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4204
> Signed-off-by: Ranbir Singh 
> Signed-off-by: Ranbir Singh 
> ---
>  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
> b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
> index 75403886e44a..af022139cf02 100644
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
> @@ -2549,13 +2549,21 @@ DetectAndConfigIdeDevice (
>  //
> 
>  if (DeviceType == EfiIdeHarddisk) {
> 
>//
> 
> -  // Init driver parameters
> 
> +  // Init drive parameters
> 
>//
> 
>DriveParameters.Sector = (UINT8)((ATA5_IDENTIFY_DATA
> *)(&Buffer.AtaData))->sectors_per_track;
> 
>DriveParameters.Heads  = (UINT8)(((ATA5_IDENTIFY_DATA
> *)(&Buffer.AtaData))->heads - 1);
> 
>DriveParameters.MultipleSector = (UINT8)((ATA5_IDENTIFY_DATA
> *)(&Buffer.AtaData))->multi_sector_cmd_max_sct_cnt;
> 
> 
> 
>Status = SetDriveParameters (Instance, IdeChannel, IdeDevice,
> &DriveParameters, NULL);
> 
> +
> 
> +  if (EFI_ERROR (Status)) {
> 
> +DEBUG ((DEBUG_WARN, "Set Drive Parameters Fail, Status = %r\n",
> + Status));
> 
> +//
> 
> +// Ignore warning and proceed normally
> 
> +//
> 
> +Status = EFI_SUCCESS;


To please both sides, how about:
1. Remove the 'Status' assignment of the return value from SetDriveParameters()
Based on my findings (https://edk2.groups.io/g/devel/message/106844), the
successful execution of SetDriveParameters() is not mandatory for initializing
IDE mode hard disk device.

2. Add DEBUG_WARN level debug message within SetDriveParameters() function
In function SetDriveParameters, for the 2 calls of AtaNonDataCommandIn (one
for the INITIALIZE DEVICE PARAMETERS command and the other for SET MULTIPLE
MODE command), if the return status is not EFI_SUCCESS, add debug message to
display the information.

Best Regards,
Hao Wu


> 
> +  }
> 
>  }
> 
> 
> 
>  //
> 
> --
> 2.34.1



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