Re: [edk2-devel] [PATCH v3 0/5] StarFive/VisionFive2: Add VisionFive 2 platform

[Sunil V L]

On Tue, Oct 31, 2023 at 06:17:05PM -0700, John Chew wrote:
> Hi Sunil,
> 
> Okay, I will update the "Maintainers.txt" file in patch v4.
> 
> By the way, I wonder if you also review this series?
> 
> [PATCH v2 0/5] Designware MMCDXE changes and enhancement
> 
Hi John,

Ard and Leif are maintainers for this module and will have better
knowledge. So, you need them to review the Designware series.

Thanks,
Sunil


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

[edk2-devel] [edk2-platforms PATCH v2 0/1] Marvell package restructure

[Narinder Dhillon]

From: Narinder Dhillon 

Current Silicon/Marvell package structure does not allow sharing of
components that are common to different SoC's. This restructure will
increase shared code and better seperation.

Credit to Leif Lindholm for providing this new structure.

v2:
 - Remove three lines for libraries that were erroneously introduced
   into MarvellSiliconPkg.dec file. These libraries are present in
   Armada7k8k.dsc.inc
 - Removed trailing new line at the end of MarvellSiliconPkg.dec
 - Squashed patches 2,3,4 into 1 as they don't build separately

v1:
 - Original patch series restructuring Marvell package


Narinder Dhillon (1):
  Silicon/Marvell: Retructure package

 .../Marvell/Armada70x0Db/Armada70x0Db.dsc | 108 -
 .../Armada70x0DbBoardDescLib.inf  |   2 +-
 .../NonDiscoverableInitLib.inf|   2 +-
 .../Marvell/Armada80x0Db/Armada80x0Db.dsc | 133 ++-
 .../Armada80x0DbBoardDescLib.inf  |   2 +-
 .../NonDiscoverableInitLib.inf|   2 +-
 .../Cn9130DbABoardDescLib.inf |   2 +-
 .../Cn9132DbABoardDescLib.inf |   2 +-
 Platform/Marvell/Cn913xDb/Cn9130DbA.dsc.inc   | 100 -
 Platform/Marvell/Cn913xDb/Cn9131DbA.dsc.inc   |  66 +++---
 Platform/Marvell/Cn913xDb/Cn9132DbA.dsc.inc   |  66 +++---
 Platform/Marvell/Cn913xDb/Cn913xDbA.dsc   |   8 +-
 .../NonDiscoverableInitLib.inf|   2 +-
 .../Armada80x0McBin/Armada80x0McBin.dsc   | 116 +-
 .../Armada80x0McBinBoardDescLib.inf   |   2 +-
 .../NonDiscoverableInitLib.inf|   2 +-
 .../BoardDescriptionLib.inf   |   2 +-
 .../Cn913xCEx7Eval/Cn9130Eval.dsc.inc |  40 ++--
 .../Cn913xCEx7Eval/Cn9131Eval.dsc.inc |  56 ++---
 .../Cn913xCEx7Eval/Cn9132Eval.dsc.inc |  56 ++---
 .../Cn913xCEx7Eval/Cn913xCEx7.dsc.inc |  60 ++---
 .../Cn913xCEx7Eval/Cn913xCEx7Eval.dsc |   6 +-
 .../NonDiscoverableInitLib.inf|   2 +-
 .../Applications/EepromCmd/EepromCmd.inf  |   2 +-
 .../Applications/FirmwareUpdate/FUpdate.inf   |   6 +-
 .../Applications/SpiTool/SpiFlashCmd.inf  |   6 +-
 .../Armada7k8k/AcpiTables/Armada70x0Db.inf|   2 +-
 .../Armada7k8k/AcpiTables/Armada80x0Db.inf|   2 +-
 .../Armada7k8k/AcpiTables/Armada80x0McBin.inf |   2 +-
 Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc |  22 +-
 .../Armada7k8kRngDxe/Armada7k8kRngDxe.inf |   4 +-
 .../Drivers/PlatInitDxe/PlatInitDxe.inf   |   6 +-
 .../PlatformFlashAccessLib.inf|   6 +-
 .../Library/Armada7k8kLib/Armada7k8kLib.inf   |   4 +-
 .../Armada7k8kMemoryInitPeiLib.inf|  14 +-
 .../PciHostBridgeLib.inf  |   2 +-
 .../Armada7k8kPciSegmentLib/PciSegmentLib.inf |   2 +-
 .../Armada7k8kSampleAtResetLib.inf|   2 +-
 .../Armada7k8kSoCDescLib.inf  |   4 +-
 .../RealTimeClockLib/RealTimeClockLib.inf |   4 +-
 .../Marvell/Documentation/PortingGuide.txt| 114 +-
 .../Drivers/BoardDesc/MvBoardDescDxe.inf  |  18 +-
 .../Drivers/Gpio/MvGpioDxe/MvGpioDxe.inf  |   2 +-
 .../Gpio/MvPca95xxDxe/MvPca95xxDxe.inf|   2 +-
 .../Drivers/I2c/MvEepromDxe/MvEepromDxe.inf   |   6 +-
 .../Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf |  14 +-
 .../Drivers/Net/MvMdioDxe/MvMdioDxe.inf   |   2 +-
 .../Marvell/Drivers/Net/MvPhyDxe/MvPhyDxe.inf |  12 +-
 Silicon/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf |  16 +-
 .../NonDiscoverableDxe/NonDiscoverableDxe.inf |   2 +-
 .../Drivers/SdMmc/XenonDxe/XenonDxe.inf   |   2 +-
 .../SmbiosPlatformDxe/SmbiosPlatformDxe.inf   |  14 +-
 .../Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf |   8 +-
 .../Spi/MvSpiFlashDxe/MvSpiFlashDxe.inf   |   2 +-
 .../Spi/MvSpiOrionDxe/MvSpiOrionDxe.inf   |   8 +-
 .../Marvell/Library/ComPhyLib/ComPhyLib.inf   |  28 +--
 Silicon/Marvell/Library/IcuLib/IcuLib.inf |   4 +-
 Silicon/Marvell/Library/MppLib/MppLib.inf |  94 
 .../Marvell/Library/MvGpioLib/MvGpioLib.inf   |   2 +-
 .../Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf |   2 +-
 Silicon/Marvell/Marvell.dec   | 208 --
 .../Include/IndustryStandard/MvSmc.h  |   0
 .../Include/Library/ArmadaBoardDescLib.h  |   0
 .../Include/Library/ArmadaIcuLib.h|   0
 .../Include/Library/ArmadaSoCDescLib.h|   0
 .../Include/Library/MppLib.h  |   0
 .../Include/Library/MvComPhyLib.h |   0
 .../Include/Library/MvGpioLib.h   |   0
 .../Include/Library/NonDiscoverableInitLib.h  |   0
 .../Include/Library/SampleAtResetLib.h|   0
 .../Include/Library/UtmiPhyLib.h  |   0
 .../Include/Protocol/BoardDesc.h  |   0
 .../Include/Protocol/Eeprom.h |   0
 .../Include/Protocol/Mdio.h   |   0
 .../Include/Protocol/MvI2c.h  |   0
 .../Include/Protocol/MvPhy.h  |   0
 

Re: [edk2-devel] [PATCH v2] RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4 installed after RestEx

[Nickle Wang via groups.io]

This change looks good to me. Just minor comments:

> DEBUG ((DEBUG_INFO, "%a: all required protocols are found on this controller 
> handle: %p.\n", __func__, ControllerHandle));

Could we use DEBUG_MANAGEABILITY to replace DEBUG_INFO in Redfish driver? This 
gives us the way to show manageability information only while debugging.

And it looks like we don't run uncrustify to this patch. May I know if it is 
possible to create a PR on edk2 repository and edk2 CI can test it? Edk2 CI 
helps me to catch some formatting issue that I am not aware of.

Thanks,
Nickle

> -Original Message-
> From: Igor Kulchytskyy 
> Sent: Thursday, November 2, 2023 4:18 AM
> To: devel@edk2.groups.io
> Cc: Abner Chang ; Nickle Wang ;
> Mike Maslenkin 
> Subject: [PATCH v2] RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4 
> installed
> after RestEx
> 
> External email: Use caution opening links or attachments
> 
> 
> Supported function of the driver changed to wait for all newtwork
> interface to be installed.
> Filer out the network interfaces which are not supported by
> Redfish Host Interface.
> 
> Cc: Abner Chang 
> Cc: Nickle Wang 
> Cc: Mike Maslenkin 
> Signed-off-by: Igor Kulchytskyy 
> ---
>  RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c | 131 ++---
> ---
>  1 file changed, 95 insertions(+), 36 deletions(-)
> 
> diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> index 23da3b968f..c67b8acf12 100644
> --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> @@ -322,9 +322,15 @@ GetTargetNetworkInterfaceInternal (
>  {
>EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL
> *ThisNetworkInterface;
> 
> +  if (IsListEmpty ()) {
> +return NULL;
> +  }
> +
>ThisNetworkInterface =
> (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL *)GetFirstNode
> ();
>while (TRUE) {
> -if (CompareMem ((VOID *)>MacAddress,
> >MacAddress, ThisNetworkInterface->HwAddressSize)
> == 0) {
> +if (CompareMem ((VOID *)>MacAddress,
> >MacAddress, ThisNetworkInterface->HwAddressSize)
> == 0 &&
> +((TargetNetworkInterface->IsIpv6 && ThisNetworkInterface-
> >NetworkProtocolType == ProtocolTypeTcp6) ||
> +(!TargetNetworkInterface->IsIpv6 && ThisNetworkInterface-
> >NetworkProtocolType == ProtocolTypeTcp4))) {
>return ThisNetworkInterface;
>  }
> 
> @@ -354,6 +360,10 @@ GetTargetNetworkInterfaceInternalByController (
>  {
>EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL
> *ThisNetworkInterface;
> 
> +  if (IsListEmpty ()) {
> +return NULL;
> +  }
> +
>ThisNetworkInterface =
> (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL *)GetFirstNode
> ();
>while (TRUE) {
>  if (ThisNetworkInterface->OpenDriverControllerHandle == 
> ControllerHandle) {
> @@ -476,6 +486,39 @@ CheckIsIpVersion6 (
>return FALSE;
>  }
> 
> +/**
> +  This function returns the  IP type supported by the Host Interface
> +
> +  @retval IP Type
> +  //  Unknown=00h,
> +  //  Ipv4=01h,
> +  //  Ipv6=02h,
> +
> +**/
> +STATIC
> +UINT8
> +GetHiIpProtocolType()
> +{
> +  EFI_STATUS Status;
> +  REDFISH_OVER_IP_PROTOCOL_DATA  *Data;
> +  REDFISH_INTERFACE_DATA *DeviceDescriptor;
> +
> +  Data = NULL;
> +  DeviceDescriptor = NULL;
> +  if (mSmbios == NULL) {
> +Status = gBS->LocateProtocol (, NULL, (VOID
> **));
> +if (EFI_ERROR (Status)) {
> +  return
> REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_UNKNOWN;
> +}
> +  }
> +  Status = RedfishGetHostInterfaceProtocolData (mSmbios, ,
> ); // Search for SMBIOS type 42h
> +  if (!EFI_ERROR (Status) && (Data != NULL) &&
> +  (Data->HostIpAssignmentType == RedfishHostIpAssignmentStatic)) {
> +  return Data->HostIpAddressFormat;
> +  }
> +  return REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_UNKNOWN;
> +}
> +
>  /**
>This function discover Redfish service through SMBIOS host interface.
> 
> @@ -512,6 +555,15 @@ DiscoverRedfishHostInterface (
> 
>Status = RedfishGetHostInterfaceProtocolData (mSmbios, ,
> ); // Search for SMBIOS type 42h
>if (!EFI_ERROR (Status) && (Data != NULL) && (DeviceDescriptor != NULL)) {
> +
> +if (Instance->NetworkInterface->NetworkProtocolType == ProtocolTypeTcp4
> && Data->HostIpAddressFormat !=
> REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP4) { // IPv4 case
> +  DEBUG ((DEBUG_ERROR, "%a: Network Interface is IPv4, but Host Interface
> requires Ipv6\n", __func__));
> +  return EFI_UNSUPPORTED;
> +}
> +else if (Instance->NetworkInterface->NetworkProtocolType ==
> ProtocolTypeTcp6 && Data->HostIpAddressFormat !=
> REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP6) { // IPv6 case
> +  DEBUG ((DEBUG_ERROR, "%a: Network Interface is IPv6, but Host Interface
> requires IPv4\n", __func__));
> +  return EFI_UNSUPPORTED;
> +}
>  //
>  // Check if we can reach out Redfish service using this 

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

[Group Notification]

*Reminder: TianoCore Community Meeting - APAC/NAMO*

*When:*
Thursday, November 2, 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=2072475 )

*Description:*



Microsoft Teams meeting

*Join on your computer or mobile app*

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

Meeting ID: 283 318 374 436
Passcode: 633zLo

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

*Join with a video conferencing device*

te...@conf.intel.com

Video Conference ID: 119 493 012 8

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

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




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110497): https://edk2.groups.io/g/devel/message/110497
Mute This Topic: https://groups.io/mt/102334672/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 v3 2/2] BaseTools: GenFw: auto-set nxcompat flag

[Rebecca Cran]

I'm still working on this! The last hurdle is fixing PatchCheck.py to 
allow .rtf files written by Notepad etc. to be committed as-is, without 
requiring line endings to be fixed, trailing whitespace to be removed etc.



Once that's in, I'll merge these two patches.


--

Rebecca Cran


On 10/17/23 13:56, Joey Vagedes wrote:
While I’m not a maintainer, so I don’t have much say - I don’t see an 
issue with your solution that rewrites the entire file as the small 
change I made to add the nonxcompat flag already creates a large diff 
to the rtf manual. The rtf format does not really support git diff/ 
readability.


Thanks,
Joey

On Tue, Oct 17, 2023 at 3:34 PM Rebecca Cran  wrote:

Unfortunately the patch doesn't pass CI because
BaseTools/UserManuals/GenFw_Utility_Man_Page.rtf contains trailing
whitespace.

I'm unsure whether I can manually fix it, or if I should open it in
LibreOffice and re-save it? Doing so appears to rewrite the entire
file,
which might not be what's desired.


-- 


Rebecca Cran


On 8/1/23 09:57, Joey Vagedes wrote:
> Hello BaseTools maintainers. I'm still looking for feedback and a
> review for the changes made to GenFw to automatically set
the NXCOMPAT
> flag if the requirements are met. Drivers can opt out of the flag
> regardless, with the --nonxcompat flag. Please let me know if
you have
> any questions.
>
> Thanks,
> Joey
>
> On Thu, Jul 13, 2023 at 8:24 AM Joey Vagedes

> wrote:
>
>     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) {

[edk2-devel] [PATCH 1/1] BaseTools: Update PatchCheck.py to allow whitespace issues in .rtf files

[Rebecca Cran]

Allow .rtf files created by applications such as Notepad to be committed
as-is without further manual editing by skipping the requirements for
CRLF, no tabs and no trailing whitespace.

Signed-off-by: Rebecca Cran 
---
 BaseTools/Scripts/PatchCheck.py | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py
index 900226f18fe5..7f372d40b570 100755
--- a/BaseTools/Scripts/PatchCheck.py
+++ b/BaseTools/Scripts/PatchCheck.py
@@ -363,6 +363,9 @@ class GitDiffCheck:
 self.is_newfile = False
 self.force_crlf = True
 self.force_notabs = True
+if self.filename.endswith('.rtf'):
+self.force_crlf = False
+self.force_notabs = False
 if self.filename.endswith('.sh') or \
 
self.filename.startswith('BaseTools/BinWrappers/PosixLike/') or \
 
self.filename.startswith('BaseTools/BinPipWrappers/PosixLike/') or \
@@ -416,7 +419,7 @@ class GitDiffCheck:
 self.format_error("didn't find diff hunk marker (@@)")
 self.line_num += 1
 elif self.state == PATCH:
-if self.binary:
+if self.binary or self.filename.endswith(".rtf"):
 pass
 elif line.startswith('-'):
 pass
-- 
2.34.1



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

Re: [edk2-devel] [RFC] Ordering of Arm PCI ECAM and MMIO operations

[Pedro Falcato]

On Wed, Nov 1, 2023 at 9:51 PM Michael Brown  wrote:
>
> On 01/11/2023 20:17, Joe L wrote:
> > Thanks for the discussion,
> >
> > Are we saying that DataSynchronizationBarrier () before the return
> > from RootBridgeIoPciAccess() is not strong enough to determine that
> > the final ECAM write would have completed? According to Learn the
> > architecture - ARMv8-A memory systems
> >  the
> > DSB should block "execution of any further instructions, not just
> > loads or stores, until synchronization is complete". To me this means
> > that for Arm the DSB will stall any subsequent instructions until the
> > completion for the ECAM write is received by the processor.
>
> I honestly can't tell from the wording there whether or not DSB would
> guarantee that the configuration space write has completed all the way
> to the target PCIe device (as opposed to e.g. completing as far as the
> host bridge, or to an intermediate PCIe bridge).
>
> I don't know the answer, and it feels like the kind of messy area where
> adding a barrier will definitely reduce the probability of the issue
> occurring but might not guarantee a fix, which is a bad situation to be
> left in.

Given we are working with Device nGnRnE memory, and the documentation
says stuff like

" This determines whether an intermediate write buffer between the
processor and the slave device being accessed is allowed to send an
acknowledgement of a write completion. If the address is marked as non
Early Write Acknowledgement (nE), then the write response must come
from the peripheral. If the address is marked as Early Write
Acknowledgement (E), then it is permissible for a buffer in the
interconnect logic to signal write acceptance, in advance of the write
actually being received by the end device. This is essentially a
message to the external memory system."

I would expect the write ACK to come from the device itself (I can't
wait for the obscure ARM doc that proves me wrong!)

-- 
Pedro


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

Re: [edk2-devel] [RFC] Ordering of Arm PCI ECAM and MMIO operations

[Michael Brown]

On 01/11/2023 20:17, Joe L wrote:

Thanks for the discussion,

Are we saying that DataSynchronizationBarrier () before the return 
from RootBridgeIoPciAccess() is not strong enough to determine that 
the final ECAM write would have completed? According to Learn the 
architecture - ARMv8-A memory systems 
 the 
DSB should block "execution of any further instructions, not just 
loads or stores, until synchronization is complete". To me this means

that for Arm the DSB will stall any subsequent instructions until the
completion for the ECAM write is received by the processor.


I honestly can't tell from the wording there whether or not DSB would
guarantee that the configuration space write has completed all the way 
to the target PCIe device (as opposed to e.g. completing as far as the 
host bridge, or to an intermediate PCIe bridge).


I don't know the answer, and it feels like the kind of messy area where 
adding a barrier will definitely reduce the probability of the issue 
occurring but might not guarantee a fix, which is a bad situation to be 
left in.


Though if an architecture-agnostic solution is desired the readback 
before returning from RootBridgeIoPciAccess() does make sense.


Personally I like having an architecture-agnostic solution, and I'll be
updating the ECAM driver in iPXE to use the readback approach.

If the access spanned multiple DWORDs then should a read from the 
final aligned DWORD in the "buffer" be sufficient?


Quite possibly.  Given that PCIe configuration space accesses are never 
performance-critical, I'm not sure it's worth the optimisation, but I'll 
defer to the people who actually have to implement it in EDK2.


Thanks,

Michael


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

[edk2-devel] TianoCore Community Meeting Agenda Topics

[Michael D Kinney]

Any agenda topics for the meeting this week?

If no topics, we will cancel.

Mike


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

Re: [edk2-devel] [PATCH v7 5/5] OvmfPkg/RiscVVirt: Override for RV CPU Features

[Andrei Warkentin]

Hi Dhaval,

Yes, that is what I’m suggesting. But I’m not telling you that your current 
implementation needs to be adjusted, but I am providing my feedback, that IMHO 
continued /use and evolution/ of the added PCD may to be messy because it’s 
untyped. I already gave you a Reviewed-by, so you’re welcome to do whatever you 
want with the additional feedback.

As far as an example goes… see:
  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialPciDeviceInfo|{0xFF}|VOID*|0x00010067
  …
  DeviceInfo = (PCI_UART_DEVICE_INFO *)PcdGetPtr (PcdSerialPciDeviceInfo);

So a PCD can be of type VOID *.

A

From: Dhaval Sharma 
Sent: Wednesday, November 1, 2023 12:05 PM
To: Warkentin, Andrei ; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH v7 5/5] OvmfPkg/RiscVVirt: Override for RV CPU 
Features

Hi Andrei,
Are you suggesting:

  1.  We have a generic PCD to store the address of RVFeatures bitfield.
  2.  It gets populated at some point during initialization with let's say some 
kind of global variable address which keeps this bitfield.
  3.  SEC/DXE phases deref this address and use where needed?
Is there a reference I can take a look at?

Assuming my above understanding is correct, the whole idea was to keep 
implementation based on simple PCD such that it can be used easily as an 
override mechanism. The reason to keep it "enabled" by default in MDE is that 
if m-mode decides to keep it disabled it is anyways going to remain that way. 
So practically this PCD is going to be useful only in cases where the user 
wants to "Override Disable". LMK if you still think we should modify the 
implementation.

=D

On Tue, Oct 31, 2023 at 10:31 PM Warkentin, Andrei 
mailto:andrei.warken...@intel.com>> wrote:
I think I misunderstood the intent. Reviewing the full patchset, it seems this 
is necessary to avoid using the new CMO path in the Virt platform (since the 
default value is all FFs). Shouldn’t the default Pcd value here be all 0’s – 
i.e. CMO or any other feature use becomes “opt in” instead of “opt out”?

It also seems that encoding the meaning inside the bit positions is a bit… 
obscure. Have you considered storing a pointer to a struct with bitfields 
instead? You could then change the logic to be something like “If PcdPtrValue 
!= NULL && ((struct cast *) PcdPtrValue)->LegibleFieldName”. I think this would 
do wonders for code maintainability. The cost of course is in having to 
initialize the Pcd now at runtime, and the additional dereference, but that 
seems like a low cost all things considered.

From: Dhaval Sharma mailto:dha...@rivosinc.com>>
Sent: Tuesday, October 31, 2023 1:13 AM
To: Warkentin, Andrei 
mailto:andrei.warken...@intel.com>>; 
devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH v7 5/5] OvmfPkg/RiscVVirt: Override for RV CPU 
Features

Thanks. This PCD is for Virt platform only. Or maybe I am missing the point.


--
Thanks!
=D


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

[edk2-devel] [PATCH v2] RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4 installed after RestEx

[Igor Kulchytskyy via groups.io]

Supported function of the driver changed to wait for all newtwork
interface to be installed.
Filer out the network interfaces which are not supported by
Redfish Host Interface.

Cc: Abner Chang 
Cc: Nickle Wang 
Cc: Mike Maslenkin 
Signed-off-by: Igor Kulchytskyy 
---
 RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c | 131 ++--
 1 file changed, 95 insertions(+), 36 deletions(-)

diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c 
b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
index 23da3b968f..c67b8acf12 100644
--- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
+++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
@@ -322,9 +322,15 @@ GetTargetNetworkInterfaceInternal (
 {
   EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL  *ThisNetworkInterface;

+  if (IsListEmpty ()) {
+return NULL;
+  }
+
   ThisNetworkInterface = (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL 
*)GetFirstNode ();
   while (TRUE) {
-if (CompareMem ((VOID *)>MacAddress, 
>MacAddress, ThisNetworkInterface->HwAddressSize) == 0) 
{
+if (CompareMem ((VOID *)>MacAddress, 
>MacAddress, ThisNetworkInterface->HwAddressSize) == 0 
&&
+((TargetNetworkInterface->IsIpv6 && 
ThisNetworkInterface->NetworkProtocolType == ProtocolTypeTcp6) ||
+(!TargetNetworkInterface->IsIpv6 && 
ThisNetworkInterface->NetworkProtocolType == ProtocolTypeTcp4))) {
   return ThisNetworkInterface;
 }

@@ -354,6 +360,10 @@ GetTargetNetworkInterfaceInternalByController (
 {
   EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL  *ThisNetworkInterface;

+  if (IsListEmpty ()) {
+return NULL;
+  }
+
   ThisNetworkInterface = (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL 
*)GetFirstNode ();
   while (TRUE) {
 if (ThisNetworkInterface->OpenDriverControllerHandle == ControllerHandle) {
@@ -476,6 +486,39 @@ CheckIsIpVersion6 (
   return FALSE;
 }

+/**
+  This function returns the  IP type supported by the Host Interface
+
+  @retval IP Type
+  //  Unknown=00h,
+  //  Ipv4=01h,
+  //  Ipv6=02h,
+
+**/
+STATIC
+UINT8
+GetHiIpProtocolType()
+{
+  EFI_STATUS Status;
+  REDFISH_OVER_IP_PROTOCOL_DATA  *Data;
+  REDFISH_INTERFACE_DATA *DeviceDescriptor;
+
+  Data = NULL;
+  DeviceDescriptor = NULL;
+  if (mSmbios == NULL) {
+Status = gBS->LocateProtocol (, NULL, (VOID 
**));
+if (EFI_ERROR (Status)) {
+  return REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_UNKNOWN;
+}
+  }
+  Status = RedfishGetHostInterfaceProtocolData (mSmbios, , 
); // Search for SMBIOS type 42h
+  if (!EFI_ERROR (Status) && (Data != NULL) &&
+  (Data->HostIpAssignmentType == RedfishHostIpAssignmentStatic)) {
+  return Data->HostIpAddressFormat;
+  }
+  return REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_UNKNOWN;
+}
+
 /**
   This function discover Redfish service through SMBIOS host interface.

@@ -512,6 +555,15 @@ DiscoverRedfishHostInterface (

   Status = RedfishGetHostInterfaceProtocolData (mSmbios, , 
); // Search for SMBIOS type 42h
   if (!EFI_ERROR (Status) && (Data != NULL) && (DeviceDescriptor != NULL)) {
+
+if (Instance->NetworkInterface->NetworkProtocolType == ProtocolTypeTcp4 && 
Data->HostIpAddressFormat != REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP4) 
{ // IPv4 case
+  DEBUG ((DEBUG_ERROR, "%a: Network Interface is IPv4, but Host Interface 
requires Ipv6\n", __func__));
+  return EFI_UNSUPPORTED;
+}
+else if (Instance->NetworkInterface->NetworkProtocolType == 
ProtocolTypeTcp6 && Data->HostIpAddressFormat != 
REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP6) { // IPv6 case
+  DEBUG ((DEBUG_ERROR, "%a: Network Interface is IPv6, but Host Interface 
requires IPv4\n", __func__));
+  return EFI_UNSUPPORTED;
+}
 //
 // Check if we can reach out Redfish service using this network interface.
 // Check with MAC address using Device Descriptor Data Device Type 04 and 
Type 05.
@@ -1102,6 +1154,7 @@ RedfishServiceGetNetworkInterface (
   OUT EFI_REDFISH_DISCOVER_NETWORK_INTERFACE  **NetworkIntfInstances
   )
 {
+  EFI_STATUS   Status;
   EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL  *ThisNetworkInterfaceIntn;
   EFI_REDFISH_DISCOVER_NETWORK_INTERFACE   *ThisNetworkInterface;
   EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL   *RestExInstance;
@@ -1141,6 +1194,16 @@ RedfishServiceGetNetworkInterface (

   ThisNetworkInterfaceIntn = (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL 
*)GetFirstNode ();
   while (TRUE) {
+// If Get Subnet Info failed then skip this interface
+Status = NetworkInterfaceGetSubnetInfo (ThisNetworkInterfaceIntn, 
ImageHandle); // Get subnet info
+if (EFI_ERROR(Status)) {
+  if (IsNodeAtEnd (, 
>Entry)) {
+break;
+  }
+  ThisNetworkInterfaceIntn = 
(EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL *)GetNextNode 
(, >Entry);
+  continue;
+}
+
 ThisNetworkInterface->IsIpv6 = FALSE;
 if 

Re: [edk2-devel] [RFC] Ordering of Arm PCI ECAM and MMIO operations

[Joe L]

Thanks for the discussion,

Are we saying that DataSynchronizationBarrier () before the return from 
RootBridgeIoPciAccess() is not strong enough to determine that the final ECAM 
write would have completed? According to Learn the architecture - ARMv8-A 
memory systems ( https://developer.arm.com/documentation/100941/0101/Barriers ) 
the DSB should block "execution of any further instructions, not just loads or 
stores, until synchronization is complete". To me this means that for Arm the 
DSB will stall any subsequent instructions until the completion for the ECAM 
write is received by the processor.

Though if an architecture-agnostic solution is desired the readback before 
returning from RootBridgeIoPciAccess() does make sense. If the access spanned 
multiple DWORDs then should a read from the final aligned DWORD in the "buffer" 
be sufficient?


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

Re: [edk2-devel] [PATCH v4 11/14] UefiCpuPkg: Use Attribute From SMM MemoryAttributesTable if Nonzero

[Taylor Beebe]

This patch is the final in this series which needs a review. Can someone 
take a look?


On 8/4/2023 12:46 PM, Taylor Beebe via groups.io wrote:

From: Taylor Beebe 

The function EnforceMemoryMapAttribute() in the SMM MAT logic will
ensure that the CODE and DATA memory types have the desired attributes.
The consumer of the SMM MAT should only override the Attributes field
in the MAT if it is nonzero. This also allows the UEFI and SMM MAT
logic to use ImagePropertiesRecordLib instead of carrying two copies
of the image properties record manipulation.

Signed-off-by: Taylor Beebe 
Cc: Eric Dong 
Cc: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
---
  UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 19 +++
  1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index 6f498666157e..d302a9b0cbcf 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -1062,14 +1062,17 @@ SetMemMapAttributes (
MemoryMap = MemoryMapStart;
for (Index = 0; Index < MemoryMapEntryCount; Index++) {
  DEBUG ((DEBUG_VERBOSE, "SetAttribute: Memory Entry - 0x%lx, 0x%x\n", 
MemoryMap->PhysicalStart, MemoryMap->NumberOfPages));
-if (MemoryMap->Type == EfiRuntimeServicesCode) {
-  MemoryAttribute = EFI_MEMORY_RO;
-} else {
-  ASSERT ((MemoryMap->Type == EfiRuntimeServicesData) || (MemoryMap->Type 
== EfiConventionalMemory));
-  //
-  // Set other type memory as NX.
-  //
-  MemoryAttribute = EFI_MEMORY_XP;
+MemoryAttribute = MemoryMap->Attribute & EFI_MEMORY_ACCESS_MASK;
+if (MemoryAttribute == 0) {
+  if (MemoryMap->Type == EfiRuntimeServicesCode) {
+MemoryAttribute = EFI_MEMORY_RO;
+  } else {
+ASSERT ((MemoryMap->Type == EfiRuntimeServicesData) || 
(MemoryMap->Type == EfiConventionalMemory));
+//
+// Set other type memory as NX.
+//
+MemoryAttribute = EFI_MEMORY_XP;
+  }
  }
  
  //



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

Re: [edk2-devel] [PATCH v7 5/5] OvmfPkg/RiscVVirt: Override for RV CPU Features

[Dhaval Sharma]

Hi Andrei,
Are you suggesting:

   1. We have a generic PCD to store the address of RVFeatures bitfield.
   2. It gets populated at some point during initialization with let's say
   some kind of global variable address which keeps this bitfield.
   3. SEC/DXE phases deref this address and use where needed?

Is there a reference I can take a look at?

Assuming my above understanding is correct, the whole idea was to keep
implementation based on simple PCD such that it can be used easily as an
override mechanism. The reason to keep it "enabled" by default in MDE is
that if m-mode decides to keep it disabled it is anyways going to remain
that way. So practically this PCD is going to be useful only in cases where
the user wants to "Override Disable". LMK if you still think we should
modify the implementation.

=D

On Tue, Oct 31, 2023 at 10:31 PM Warkentin, Andrei <
andrei.warken...@intel.com> wrote:

> I think I misunderstood the intent. Reviewing the full patchset, it seems
> this is necessary to avoid using the new CMO path in the Virt platform
> (since the default value is all FFs). Shouldn’t the default Pcd value here
> be all 0’s – i.e. CMO or any other feature use becomes “opt in” instead of
> “opt out”?
>
>
>
> It also seems that encoding the meaning inside the bit positions is a bit…
> obscure. Have you considered storing a pointer to a struct with bitfields
> instead? You could then change the logic to be something like “If
> PcdPtrValue != NULL && ((struct cast *) PcdPtrValue)->LegibleFieldName”. I
> think this would do wonders for code maintainability. The cost of course is
> in having to initialize the Pcd now at runtime, and the additional
> dereference, but that seems like a low cost all things considered.
>
>
>
> *From:* Dhaval Sharma 
> *Sent:* Tuesday, October 31, 2023 1:13 AM
> *To:* Warkentin, Andrei ; devel@edk2.groups.io
> *Subject:* Re: [edk2-devel] [PATCH v7 5/5] OvmfPkg/RiscVVirt: Override
> for RV CPU Features
>
>
>
> Thanks. This PCD is for Virt platform only. Or maybe I am missing the
> point.
>


-- 
Thanks!
=D


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

Re: [edk2-devel] [RFC] Ordering of Arm PCI ECAM and MMIO operations

[Ard Biesheuvel]

On Wed, 1 Nov 2023 at 14:24, Michael Brown  wrote:
>
> On 01/11/2023 12:51, Ard Biesheuvel wrote:
> > On Wed, 1 Nov 2023 at 13:25, Michael Brown  wrote:
> >> By my reading, the PCIe specification seems to therefore require
> >> something stronger than an ordering guarantee: it requires the ability
> >> for software to make a standalone determination that the write has
> >> *completed*, independent of the existence of any subsequent I/O operations.
> >
> > indeed, thanks for bringing this up.
> >
> >> The PCIe specification does not mandate that any particular mechanism be
> >> used, but it does require that the processor and/or host bridge provides
> >> *some* mechanism for software to determine that the ECAM write has
> >> completed.
> >>
> >> What mechanism does ARM (or the host bridge) provide to determine
> >> completion of an ECAM write?
> >
> > A MMIO read of the same location should ensure that the MMIO write has
> > completed by the time the read returns. Not sure whether or not there
> > are any other requirements (e.g., wrt.the size of the read vs the size
> > of the write).
>
> That seems to suggest that a logical PCIe configuration space write
> operation using ECAM should probably always comprise:
>
> 1. ECAM write
> 2. ECAM read from the same location (using the same size)
>
> If reads are not allowed to have side effects (e.g. read-clear
> registers) then this seems safe.  The PCIe specification "Configuration
> Register Types" list comprises (in version 3.0, at least):
>
>HwInit - read-only, no read side effects
>
>RO - read-only, no read side effects
>
>RW - read-write, no read side effects
>
>RW1C - write 1 to clear bits, no read side effects
>
>ROS - read-only, no read side effects
>
>RWS - read-write, no read side effects
>
>RW1CS - write 1 to clear bits, no read side effects
>
>RsvdP - read-write, no read side effects
>
>RsvdZ - read-write, no read side effects
>
> So, unless newer versions of the PCIe specification have allowed for the
> existence of configuration register types with read side effects, then
> the approach of always reading back from ECAM seems to be safe for any
> conforming PCIe device.
>
> I would therefore suggest that all ECAM driver implementation functions
> in EDK2 (e.g. PciExpressWrite32(), PciExpressOr32(),
> PciSegmentWrite32(), etc) be updated to add the relevant ECAM read
> following the write operation.
>
> PCI configuration space writes are never fast-path operations (in any
> sane hardware), and so the delay introduced by the read should not be
> significant.
>
> Does this seem like a sensible solution?
>

I think that would be reasonable, but it needs to be implemented at
the correct level of abstraction. We have some plumbing to iterate
over ranges to do what basically comes down to memcpy and memset on
MMIO ranges, and I don't think we want the readback on every store in
a sequence of multiple.

RootBridgeIoPciAccess() looks like a reasonable location to insert a
readback of the last access if it was a write.


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

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

[Group Notification]

*Reminder: TianoCore Community Meeting EMEA/NAMO*

*When:*
Thursday, November 2, 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=2072479 )

*Description:*



Microsoft Teams meeting

*Join on your computer or mobile app*

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

Meeting ID: 226 323 011 029
Passcode: hMRCj6

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

*Join with a video conferencing device*

te...@conf.intel.com

Video Conference ID: 112 716 814 3

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

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




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110485): https://edk2.groups.io/g/devel/message/110485
Mute This Topic: https://groups.io/mt/102321186/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] Event: TianoCore edk2-test Bug Triage Meeting - Thursday, November 2, 2023 #cal-reminder

[Group Notification]

*Reminder: TianoCore edk2-test Bug Triage Meeting*

*When:*
Thursday, November 2, 2023
10:00pm to 11:00pm
(UTC+08:00) Asia/Shanghai

*Where:*
https://armltd.zoom.us/j/91247522013?pwd=ei9nUndTbG9oWEROS2M1aVREZkpiQT09=addon

*Organizer:* Edhaya Chandran edhaya.chand...@arm.com ( 
edhaya.chand...@arm.com?subject=Re:%20Event:%20TianoCore%20edk2-test%20Bug%20Triage%20Meeting
 )

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

*Description:*


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110484): https://edk2.groups.io/g/devel/message/110484
Mute This Topic: https://groups.io/mt/102319709/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] [edk2-redfish-client][PATCH 3/3] RedfishClientPkg/RedfishFeatureUtilityLib: fix issues and enhancement

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

Reviewed-by: Abner Chang 

> -Original Message-
> From: Nickle Wang 
> Sent: Tuesday, October 31, 2023 3:45 PM
> To: devel@edk2.groups.io
> Cc: Chang, Abner ; Igor Kulchytskyy
> ; Nick Ramirez 
> Subject: [edk2-redfish-client][PATCH 3/3]
> RedfishClientPkg/RedfishFeatureUtilityLib: fix issues and enhancement
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> -Add RedfishDebugLib to print Redfish request and response
> details when PATCH and POST request return error.
> -Use "%a:" in debug print to align with the style in EDK2.
> -Enhance GetConfigureLang() to handle pending resource URI.
> Pending resource share ths same schema as its active resource.
> So we can simply remove "/Settings" from URI and search again.
> -Enhancement to GetPropertyVagueValue(). This function stops
> to handle remaining property while error happens. We like to
> process as much as properties as we can and skip the problematic
> property.
> -Enhancement to MatchPropertyWithJsonContext(). For string type
> of BIOS attribute, it is possible that the attribute value is
> empty by default. This does not means that the resource is not
> provisioned yet.
> -Fix typo.
>
> Signed-off-by: Nickle Wang 
> Cc: Abner Chang 
> Cc: Igor Kulchytskyy 
> Cc: Nick Ramirez 
> ---
>  RedfishClientPkg/RedfishClientLibs.dsc.inc|   1 +
>  .../RedfishFeatureUtilityLib.inf  |   1 +
>  .../RedfishFeatureUtilityInternal.h   |   2 +
>  .../RedfishFeatureUtilityLib.c| 272 ++
>  4 files changed, 154 insertions(+), 122 deletions(-)
>
> diff --git a/RedfishClientPkg/RedfishClientLibs.dsc.inc
> b/RedfishClientPkg/RedfishClientLibs.dsc.inc
> index 5ea38015..5eae6711 100644
> --- a/RedfishClientPkg/RedfishClientLibs.dsc.inc
> +++ b/RedfishClientPkg/RedfishClientLibs.dsc.inc
> @@ -39,3 +39,4 @@
>
> RedfishEventLib|RedfishClientPkg/Library/RedfishEventLib/RedfishEventLib.i
> nf
>
> RedfishVersionLib|RedfishClientPkg/Library/RedfishVersionLib/RedfishVersio
> nLib.inf
>
> RedfishAddendumLib|RedfishClientPkg/Library/RedfishAddendumLib/Redfis
> hAddendumLib.inf
> +  RedfishDebugLib|RedfishPkg/Library/RedfishDebugLib/RedfishDebugLib.inf
> diff --git
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.i
> nf
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.i
> nf
> index 66d5dce6..fd66b8ac 100644
> ---
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.i
> nf
> +++
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.i
> nf
> @@ -45,6 +45,7 @@
>UefiRuntimeServicesTableLib
>PrintLib
>HttpLib
> +  RedfishDebugLib
>
>  [Protocols]
>gEdkIIRedfishETagProtocolGuid   ## CONSUMED ##
> diff --git
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityInte
> rnal.h
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityInte
> rnal.h
> index d2bd6507..5d39984c 100644
> ---
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityInte
> rnal.h
> +++
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityInte
> rnal.h
> @@ -2,6 +2,7 @@
>Common header file for RedfishFeatureUtilityLib driver.
>
>(C) Copyright 2020-2022 Hewlett Packard Enterprise Development LP
> +  Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights
> reserved.
>
>SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -28,6 +29,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #include 
>
> diff --git
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> index e1899878..13e29902 100644
> ---
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> +++
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> @@ -133,7 +133,7 @@ SetEtagWithUri (
>
>Status = RedfishLocateProtocol ((VOID **),
> );
>if (EFI_ERROR (Status)) {
> -DEBUG ((DEBUG_ERROR, "%a, fail to locate gEdkIIRedfishETagProtocolGuid:
> %r\n", __func__, Status));
> +DEBUG ((DEBUG_ERROR, "%a: fail to locate
> gEdkIIRedfishETagProtocolGuid: %r\n", __func__, Status));
>  return Status;
>}
>
> @@ -175,7 +175,7 @@ GetEtagWithUri (
>
>Status = RedfishLocateProtocol ((VOID **),
> );
>if (EFI_ERROR (Status)) {
> -DEBUG ((DEBUG_ERROR, "%a, fail to locate gEdkIIRedfishETagProtocolGuid:
> %r\n", __func__, Status));
> +DEBUG ((DEBUG_ERROR, "%a: fail to locate
> gEdkIIRedfishETagProtocolGuid: %r\n", __func__, Status));
>  return NULL;
>}
>
> @@ -306,10 +306,10 @@ ApplyFeatureSettingsStringType (
>//
>Status = RedfishPlatformConfigGetValue (Schema, Version, ConfigureLang,
> );
>if (EFI_ERROR (Status)) {
> -

Re: [edk2-devel] [edk2-redfish-client][PATCH 2/3] RedfishClientPkg/RedfishConfigLangMapDxe: uninitialized variable issue

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

Reviewed-by: Abner Chang 

> -Original Message-
> From: Nickle Wang 
> Sent: Tuesday, October 31, 2023 3:45 PM
> To: devel@edk2.groups.io
> Cc: Chang, Abner ; Igor Kulchytskyy
> ; Nick Ramirez 
> Subject: [edk2-redfish-client][PATCH 2/3]
> RedfishClientPkg/RedfishConfigLangMapDxe: uninitialized variable issue
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> -RedfishConfigLangMapDxe relies on variable arch protocol.
> -Status variable is not initialized.
>
> Signed-off-by: Nickle Wang 
> Cc: Abner Chang 
> Cc: Igor Kulchytskyy 
> Cc: Nick Ramirez 
> ---
>  .../RedfishConfigLangMapDxe/RedfishConfigLangMapDxe.inf   | 4 ++--
>  .../RedfishConfigLangMapDxe/RedfishConfigLangMapDxe.c | 3 ++-
>  2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git
> a/RedfishClientPkg/RedfishConfigLangMapDxe/RedfishConfigLangMapDxe.in
> f
> b/RedfishClientPkg/RedfishConfigLangMapDxe/RedfishConfigLangMapDxe.in
> f
> index 821f0552..42d9daf2 100644
> ---
> a/RedfishClientPkg/RedfishConfigLangMapDxe/RedfishConfigLangMapDxe.in
> f
> +++
> b/RedfishClientPkg/RedfishConfigLangMapDxe/RedfishConfigLangMapDxe.in
> f
> @@ -1,7 +1,7 @@
>  ## @file
>  #
>  #  (C) Copyright 2022 Hewlett Packard Enterprise Development LP
> -#  Copyright (c) 2022, NVIDIA CORPORATION & AFFILIATES. All rights
> reserved.
> +#  Copyright (c) 2022-2023, NVIDIA CORPORATION & AFFILIATES. All rights
> reserved.
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -45,4 +45,4 @@
>gEfiRedfishClientVariableGuid  ## CONSUMED ##
>
>  [Depex]
> -  TRUE
> +  gEfiVariableArchProtocolGuid
> diff --git
> a/RedfishClientPkg/RedfishConfigLangMapDxe/RedfishConfigLangMapDxe.c
> b/RedfishClientPkg/RedfishConfigLangMapDxe/RedfishConfigLangMapDxe.c
> index 6a72afed..97f29549 100644
> ---
> a/RedfishClientPkg/RedfishConfigLangMapDxe/RedfishConfigLangMapDxe.c
> +++
> b/RedfishClientPkg/RedfishConfigLangMapDxe/RedfishConfigLangMapDxe.c
> @@ -1,7 +1,7 @@
>  /** @file
>
>(C) Copyright 2022 Hewlett Packard Enterprise Development LP
> -  Copyright (c) 2022, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
> +  Copyright (c) 2022-2023, NVIDIA CORPORATION & AFFILIATES. All rights
> reserved.
>
>SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -756,6 +756,7 @@ RedfishConfigLangMapDriverEntryPoint (
>InitializeListHead (
> >ConfigLangList.Listheader);
>mRedfishConfigLangMapPrivate->VariableName = AllocateCopyPool (StrSize
> (CONFIG_LANG_MAP_VARIABLE_NAME),
> CONFIG_LANG_MAP_VARIABLE_NAME);
>if (mRedfishConfigLangMapPrivate->VariableName == NULL) {
> +Status = EFI_OUT_OF_RESOURCES;
>  goto ON_ERROR;
>}
>
> --
> 2.17.1



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

Re: [edk2-devel] [edk2-redfish-client][PATCH 1/3] RedfishClientPkg/RedfishETagDxe: fix uninitialized variable issue

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

Reviewed-by: Abner Chang 

> -Original Message-
> From: Nickle Wang 
> Sent: Tuesday, October 31, 2023 3:45 PM
> To: devel@edk2.groups.io
> Cc: Chang, Abner ; Igor Kulchytskyy
> ; Nick Ramirez 
> Subject: [edk2-redfish-client][PATCH 1/3] RedfishClientPkg/RedfishETagDxe:
> fix uninitialized variable issue
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Status variable is not initialized.
>
> Signed-off-by: Nickle Wang 
> Cc: Abner Chang 
> Cc: Igor Kulchytskyy 
> Cc: Nick Ramirez 
> ---
>  RedfishClientPkg/RedfishETagDxe/RedfishETagDxe.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/RedfishClientPkg/RedfishETagDxe/RedfishETagDxe.c
> b/RedfishClientPkg/RedfishETagDxe/RedfishETagDxe.c
> index a892ced9..f731d39d 100644
> --- a/RedfishClientPkg/RedfishETagDxe/RedfishETagDxe.c
> +++ b/RedfishClientPkg/RedfishETagDxe/RedfishETagDxe.c
> @@ -1,7 +1,7 @@
>  /** @file
>
>(C) Copyright 2021-2022 Hewlett Packard Enterprise Development LP
> -  Copyright (c) 2022, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
> +  Copyright (c) 2022-2023, NVIDIA CORPORATION & AFFILIATES. All rights
> reserved.
>
>SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -728,6 +728,7 @@ RedfishETagDriverEntryPoint (
>InitializeListHead (>ETagList.Listheader);
>mRedfishETagPrivate->VariableName = AllocateCopyPool (StrSize
> (ETAG_VARIABLE_NAME), ETAG_VARIABLE_NAME);
>if (mRedfishETagPrivate->VariableName == NULL) {
> +Status = EFI_OUT_OF_RESOURCES;
>  goto ON_ERROR;
>}
>
> --
> 2.17.1



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

Re: [edk2-devel] [RFC] Ordering of Arm PCI ECAM and MMIO operations

[Michael Brown]

On 01/11/2023 12:51, Ard Biesheuvel wrote:

On Wed, 1 Nov 2023 at 13:25, Michael Brown  wrote:

By my reading, the PCIe specification seems to therefore require
something stronger than an ordering guarantee: it requires the ability
for software to make a standalone determination that the write has
*completed*, independent of the existence of any subsequent I/O operations.


indeed, thanks for bringing this up.


The PCIe specification does not mandate that any particular mechanism be
used, but it does require that the processor and/or host bridge provides
*some* mechanism for software to determine that the ECAM write has
completed.

What mechanism does ARM (or the host bridge) provide to determine
completion of an ECAM write?


A MMIO read of the same location should ensure that the MMIO write has
completed by the time the read returns. Not sure whether or not there
are any other requirements (e.g., wrt.the size of the read vs the size
of the write).


That seems to suggest that a logical PCIe configuration space write 
operation using ECAM should probably always comprise:


1. ECAM write
2. ECAM read from the same location (using the same size)

If reads are not allowed to have side effects (e.g. read-clear 
registers) then this seems safe.  The PCIe specification "Configuration 
Register Types" list comprises (in version 3.0, at least):


  HwInit - read-only, no read side effects

  RO - read-only, no read side effects

  RW - read-write, no read side effects

  RW1C - write 1 to clear bits, no read side effects

  ROS - read-only, no read side effects

  RWS - read-write, no read side effects

  RW1CS - write 1 to clear bits, no read side effects

  RsvdP - read-write, no read side effects

  RsvdZ - read-write, no read side effects

So, unless newer versions of the PCIe specification have allowed for the 
existence of configuration register types with read side effects, then 
the approach of always reading back from ECAM seems to be safe for any 
conforming PCIe device.


I would therefore suggest that all ECAM driver implementation functions 
in EDK2 (e.g. PciExpressWrite32(), PciExpressOr32(), 
PciSegmentWrite32(), etc) be updated to add the relevant ECAM read 
following the write operation.


PCI configuration space writes are never fast-path operations (in any 
sane hardware), and so the delay introduced by the read should not be 
significant.


Does this seem like a sensible solution?

Thanks,

Michael



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

Re: [edk2-devel] [PATCH] IntelFsp2Pkg/PatchFv: Fix syntax issue in markdown manual

[Ashraf Ali S]

Reviewed-by: Ashraf Ali S 

Thanks.,
S, Ashraf Ali

-Original Message-
From: Ni, Ray  
Sent: Wednesday, November 1, 2023 3:00 PM
To: devel@edk2.groups.io
Cc: Chiu, Chasel ; Desimone, Nathaniel L 
; Duggapu, Chinni B 
; Ng, Ray Han Lim ; Zeng, 
Star ; Kuo, Ted ; S, Ashraf Ali 
; Mohapatra, Susovan 
Subject: [PATCH] IntelFsp2Pkg/PatchFv: Fix syntax issue in markdown manual

According to the markdown language syntax, headings should be after number 
signs (#). The number of number signs correspond to the heading level.
But current PatchFvUserManual.md doesn't insert a space between the number 
signs and the heading title, resulting the markdown file is not rendered well 
in markdown viewers.

The patch doesn't change any content but only adds spaces to ensure the 
headings are correctly recognized.

Signed-off-by: Ray Ni 
Cc: Chasel Chiu 
Cc: Nate DeSimone 
Cc:  Duggapu Chinni B 
Cc: Ray Han Lim Ng 
Cc: Star Zeng 
Cc: Ted Kuo 
Cc: Ashraf Ali S 
Cc: Susovan Mohapatra 
---
 .../Tools/UserManuals/PatchFvUserManual.md| 38 +--
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/IntelFsp2Pkg/Tools/UserManuals/PatchFvUserManual.md 
b/IntelFsp2Pkg/Tools/UserManuals/PatchFvUserManual.md
index f28eedf625..205ad57773 100644
--- a/IntelFsp2Pkg/Tools/UserManuals/PatchFvUserManual.md
+++ b/IntelFsp2Pkg/Tools/UserManuals/PatchFvUserManual.md
@@ -1,9 +1,9 @@
-#Name+# Name **_PatchFv.py_** - The python script that patches the firmware 
volumes (**FV**) with in the flash device (**FD**) file post FSP build. From 
version 0.60, script is capable of patching flash device (**FD**) directly. 
-#Synopsis+# Synopsis  ``` PatchFv FvBuildDir 
[FvFileBaseNames:]FdFileBaseNameToPatch ["Offset, Value"]+@@ -18,32 +18,32 @@ 
PatchFv FdFileDir FdFileName ["Offset, Value"]+
   | ["Offset, Value, $Command, @Comment"]+ ``` -#Description+# Description The 
**_PatchFv.py_** tool allows the developer to fix up FD images to follow the 
Intel FSP Architecture specification.  It also makes the FD image relocatable. 
The tool is written in Python and uses Python 2.7 or later to run. Consider 
using the tool in a build script. -#FvBuildDir (Argument 1)+# FvBuildDir 
(Argument 1) This is the first argument that **_PatchFv.py_** requires.  It is 
the build directory for all firmware volumes created during the FSP build. The 
path must be either an absolute path or a relevant path, relevant to the top 
level of the FSP tree. -Example usage:+ Example usage: ```  
Build\YouPlatformFspPkg\%BD_TARGET%_%VS_VERSION%%VS_X86%\FV ```  The example 
used contains Windows batch script %VARIABLES%. -#FvFileBaseNames (Argument 2: 
Optional Part 1)+# FvFileBaseNames (Argument 2: Optional Part 1) The firmware 
volume file base names (**_FvFileBaseNames_**) are the independent-Fv?s that 
are to be patched within the FD. (0 or more in the form-**FVFILEBASENAME:**) 
The colon **:** is used for delimiting the single+FVs that are to be patched 
within the FD. (0 or more in the form+**FvFileBaseNames:**) The colon **:** is 
used for delimiting the single argument and must be appended to the end of each 
(**_FvFileBaseNames_**). -Example usage:+ Example usage: ``` 
STAGE1:STAGE2:MANIFEST:YOURPLATFORM ```@@ -55,14 +55,14 @@ In the example 
**STAGE1** is **STAGE1.Fv** in **YOURPLATFORM.fd**.
 Firmware device file name to patch (**_FdFileNameToPatch_**) is the base name 
of the FD file that is to be patched. (1 only, in the form **YOURPLATFORM**) 
-Example usage:+ Example usage: ``` STAGE1:STAGE2:MANIFEST:YOURPLATFORM 
```  In the example **YOURPLATFORM** is from **_YOURPLATFORM.fd_** -#"Offset, 
Value[, Command][, Comment]" (Argument 3)+# "Offset, Value[, Command][, 
Comment]" (Argument 3) The **_Offset_** can be a positive or negative number 
and represents where the **_Value_** to be patched is located within the FD. 
The **_Value_** is what will be written at the given **_Offset_** in the FD. 
Constants may be used for@@ -79,10 +79,10 @@ The entire argument includes the 
quote marks like in the example argument below:
 0xFFC0, SomeCore:__EntryPoint - [0x00F0],@SomeCore Entry ``` 
-###Constants:+### Constants:  Hexadecimal (use **0x** as prefix) | Decimal 
-Examples:+ Examples:  | **Positive Hex** | **Negative Hex** | 
**Positive Decimal** | **Negative Decimal** | | ---: | 
---: | ---: | ---: |@@ -93,7 +93,7 
@@ ModuleName:FunctionName | ModuleName:GlobalVariableName  ModuleGuid:Offset 
``` -###Operators:+### Operators:  ``` @@ -113,7 +113,7 @@ From version 0.60 
tool allows to pass flash device file path as Argument 1 and  flash device name 
as Argument 2 and rules for passing offset & value are same as explained in the 
previous sections. -Example usage:+ Example usage: Argument 1 ```  
YouPlatformFspBinPkg\@@ -123,21 +123,21 @@ Argument 2
  Fsp_Rebased_T ``` -###Special Commands:+### Special Commands: Special 
commands must use 

Re: [edk2-devel] [RFC] Ordering of Arm PCI ECAM and MMIO operations

[Ard Biesheuvel]

On Wed, 1 Nov 2023 at 13:25, Michael Brown  wrote:
>
> On 01/11/2023 09:56, Ard Biesheuvel wrote:
> > On Wed, 1 Nov 2023 at 03:09, Pedro Falcato  wrote:
> >> On Wed, Nov 1, 2023 at 12:40 AM Joe L  wrote:
> >>> Our CMN TRM showcases an example where ECAM and MMIO are two different 
> >>> regions in the HN-I SAM. The implication is that we would expect a DSB 
> >>> between the ECAM write and MMIO read. I'm asking our Open Source Software 
> >>> group to confirm that standard PCIe software is generally expected to be 
> >>> aware of the need for a DSB--but my impression from talking to some of 
> >>> our hardware engineers is that that is indeed the expectation.
> >
> > 
> >
> > 1) as per the architecture (as interpreted by the ARM architects), a
> > DSB is required to ensure that the side effects of enabling a MMIO BAR
> > in the PCI config space are sufficiently observable to memory accesses
> > to that BAR that appear after the PCI config space access in the
> > program.
>
> It's possibly worth mentioning what the PCIe specification requires in
> terms of ECAM ordering:
>
>"As an example, software may wish to configure a device Function’s
> Base Address register by writing to the device using the ECAM,
> and then read a location in the memory-mapped range described by
> this Base Address register. If the software were to issue the
> memory-mapped read before the ECAM write was completed, it would
> be possible for the memory-mapped read to be re-ordered and arrive
> at the device before the Configuration Write Request, thus causing
> unpredictable results.
>
> To avoid this problem, processor and host bridge implementations must
> ensure that a method exists for the software to determine when the
> write using the ECAM is completed by the completer."
>
> By my reading, the PCIe specification seems to therefore require
> something stronger than an ordering guarantee: it requires the ability
> for software to make a standalone determination that the write has
> *completed*, independent of the existence of any subsequent I/O operations.
>

indeed, thanks for bringing this up.

> As a practical example of when this might be relevant: software could be
> writing to device configuration space to disable bus mastering as part
> of a reset or shutdown sequence, in order to guarantee that the device
> will initiate no further DMA operations and that any DMA buffers
> allocated to the device can be freed and reused.  In this situation,
> there may be no subsequent MMIO read or write to the device, and so
> there is no way to rely upon an ordering guarantee to satisfy the
> requirement.
>
> Any solution involving ordering guarantees can therefore mask the
> problem in some situations, but cannot solve it.
>
> The PCIe specification does not mandate that any particular mechanism be
> used, but it does require that the processor and/or host bridge provides
> *some* mechanism for software to determine that the ECAM write has
> completed.
>
> What mechanism does ARM (or the host bridge) provide to determine
> completion of an ECAM write?
>

A MMIO read of the same location should ensure that the MMIO write has
completed by the time the read returns. Not sure whether or not there
are any other requirements (e.g., wrt.the size of the read vs the size
of the write).


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

Re: [edk2-devel] [RFC] Ordering of Arm PCI ECAM and MMIO operations

[Michael Brown]

On 01/11/2023 09:56, Ard Biesheuvel wrote:

On Wed, 1 Nov 2023 at 03:09, Pedro Falcato  wrote:

On Wed, Nov 1, 2023 at 12:40 AM Joe L  wrote:

Our CMN TRM showcases an example where ECAM and MMIO are two different regions 
in the HN-I SAM. The implication is that we would expect a DSB between the ECAM 
write and MMIO read. I'm asking our Open Source Software group to confirm that 
standard PCIe software is generally expected to be aware of the need for a 
DSB--but my impression from talking to some of our hardware engineers is that 
that is indeed the expectation.




1) as per the architecture (as interpreted by the ARM architects), a
DSB is required to ensure that the side effects of enabling a MMIO BAR
in the PCI config space are sufficiently observable to memory accesses
to that BAR that appear after the PCI config space access in the
program.


It's possibly worth mentioning what the PCIe specification requires in 
terms of ECAM ordering:


  "As an example, software may wish to configure a device Function’s
   Base Address register by writing to the device using the ECAM,
   and then read a location in the memory-mapped range described by
   this Base Address register. If the software were to issue the
   memory-mapped read before the ECAM write was completed, it would
   be possible for the memory-mapped read to be re-ordered and arrive
   at the device before the Configuration Write Request, thus causing
   unpredictable results.

   To avoid this problem, processor and host bridge implementations must
   ensure that a method exists for the software to determine when the
   write using the ECAM is completed by the completer."

By my reading, the PCIe specification seems to therefore require 
something stronger than an ordering guarantee: it requires the ability 
for software to make a standalone determination that the write has 
*completed*, independent of the existence of any subsequent I/O operations.


As a practical example of when this might be relevant: software could be 
writing to device configuration space to disable bus mastering as part 
of a reset or shutdown sequence, in order to guarantee that the device 
will initiate no further DMA operations and that any DMA buffers 
allocated to the device can be freed and reused.  In this situation, 
there may be no subsequent MMIO read or write to the device, and so 
there is no way to rely upon an ordering guarantee to satisfy the 
requirement.


Any solution involving ordering guarantees can therefore mask the 
problem in some situations, but cannot solve it.


The PCIe specification does not mandate that any particular mechanism be 
used, but it does require that the processor and/or host bridge provides 
*some* mechanism for software to determine that the ECAM write has 
completed.


What mechanism does ARM (or the host bridge) provide to determine 
completion of an ECAM write?


Thanks,

Michael



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110477): https://edk2.groups.io/g/devel/message/110477
Mute This Topic: https://groups.io/mt/102310377/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] DynamicTablesPkg: Fix assert in CmObject parser

[levi.yun]

Reviewed-by: levi.yun (yeoreum@arm.com)


From: Sami Mujawar 
Sent: 01 November 2023 10:28
To: devel@edk2.groups.io
Cc: Sami Mujawar; Pierre Gondois; Yeo Reum Yun; quic_llind...@quicinc.com; 
Akanksha Jain; Sibel Allinson; nd
Subject: [PATCH v1 1/1] DynamicTablesPkg: Fix assert in CmObject parser

The patch "f81ee47513e5 DynamicTablesPkg: Add an ET info
object parser" updates the Configuration Manager object
parser to add support for parsing CM_ARM_ET_INFO object.

However, the GicC info structure also has an ET Reference
token that points to the CM_ARM_ET_INFO object. Therefore,
update the GICC info object parser to add an entry to parse
the ET reference token. Without this change an assert
stating that the RemainingSize != 0 will be triggered.

Signed-off-by: Sami Mujawar 
---
 
DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git 
a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
 
b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
index 
127675d4cec435e6a076c4466b86a31160bf9de1..ce494816ed884f14af56fb32e7bf6bbba8595521
 100644
--- 
a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
+++ 
b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
@@ -84,7 +84,8 @@ STATIC CONST CM_OBJ_PARSER  CmArmGicCInfoParser[] = {
   { "ClockDomain",   4,"0x%x",   NULL 
},
   { "AffinityFlags", 4,"0x%x",   NULL 
},
   { "CpcToken",  sizeof (CM_OBJECT_TOKEN), "0x%p",   NULL 
},
-  { "TRBEInterrupt", 2,"0x%x",   NULL }
+  { "TRBEInterrupt", 2,"0x%x",   NULL 
},
+  { "EtToken",   sizeof (CM_OBJECT_TOKEN), "0x%p",   NULL }
 };

 /** A parser for EArmObjGicDInfo.
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

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


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

Re: [edk2-devel] CodeQL and Apache Licensed Files

[Leif Lindholm]

On 2023-10-31 19:49, Pedro Falcato wrote:

On Tue, Oct 31, 2023 at 7:43 PM Kinney, Michael D
 wrote:


Hi Pedro,

SPDX is only for licenses, not copyrights.


IANAL, but several FOSS projects (including Linux) have generally
replaced the "Copyright (c) ..." verbiage with SPDX.


They may have decided to get rid of the copyright statements at the same 
time as they switched to SPDX instead of full boilerplate licenses, but 
that is not the same thing.


/
Leif


I assume there has to be some legal basis for this, although I don't
know if it depends on the license, etc
(IIRC I /think/ one could state that copyright holders are stored in
git information, but, again, I'm not a lawyer)





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

[edk2-devel] [PATCH v1 1/1] DynamicTablesPkg: Fix assert in CmObject parser

[Sami Mujawar]

The patch "f81ee47513e5 DynamicTablesPkg: Add an ET info
object parser" updates the Configuration Manager object
parser to add support for parsing CM_ARM_ET_INFO object.

However, the GicC info structure also has an ET Reference
token that points to the CM_ARM_ET_INFO object. Therefore,
update the GICC info object parser to add an entry to parse
the ET reference token. Without this change an assert
stating that the RemainingSize != 0 will be triggered.

Signed-off-by: Sami Mujawar 
---
 
DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git 
a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
 
b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
index 
127675d4cec435e6a076c4466b86a31160bf9de1..ce494816ed884f14af56fb32e7bf6bbba8595521
 100644
--- 
a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
+++ 
b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
@@ -84,7 +84,8 @@ STATIC CONST CM_OBJ_PARSER  CmArmGicCInfoParser[] = {
   { "ClockDomain",   4,"0x%x",   NULL 
},
   { "AffinityFlags", 4,"0x%x",   NULL 
},
   { "CpcToken",  sizeof (CM_OBJECT_TOKEN), "0x%p",   NULL 
},
-  { "TRBEInterrupt", 2,"0x%x",   NULL }
+  { "TRBEInterrupt", 2,"0x%x",   NULL 
},
+  { "EtToken",   sizeof (CM_OBJECT_TOKEN), "0x%p",   NULL }
 };
 
 /** A parser for EArmObjGicDInfo.
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'



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

Re: [edk2-devel] [RFC] Ordering of Arm PCI ECAM and MMIO operations

[Ard Biesheuvel]

(cc Ray)

On Wed, 1 Nov 2023 at 03:09, Pedro Falcato  wrote:
>
> +CC ARM maintainers
>
> On Wed, Nov 1, 2023 at 12:40 AM Joe L  wrote:
> >
> > Hello,
> >
> > During experimentation on an AARCH64 platform with a PCIe endpoint that 
> > supports option ROM, it was found that the ordering of transactions between 
> > ECAM (Cfg-Write) and MMIO (Mem-Read) is not preserved by the CHI HN-I node. 
> > The observed sequence is as follows:
> > 1. EFI issues a Cfg-Write to endpoint Memory Space Enable bit to enable 
> > memory access
> > 2. EFI issues a Mem-Read to the endpoint Option ROM memory space and 
> > receives back 0x (unsupported request)
> > If the memory ordering between the two accesses is explicitly preserved via 
> > memory barrier (DMB instruction), 2. will return back valid data instead of 
> > UR. Analyzing the transactions with a protocol analyzer, we found that the 
> > Mem-Read was being issued before the completion for the Cfg-Write is 
> > received.
> >
> > On this system, the HN-I node is configured to separate the ECAM and MMIO 
> > into different SAM regions. Both regions are assigned Decice-nGnRnE 
> > attributes. According to this snippet from Arm "DEN0114 PCIe AMBA 
> > Integration Guide", the ordering of even Device-nGnRnE memory regions 
> > cannot be guaranteed if they belong to separate PCIe address spaces
> >
> > 4.8.2
> >
> > Multiple PCIe address spaces mapped as Device-nGnRnE or Device-nGnRE Arm 
> > memory model does not give any ordering guarantees between accesses to 
> > different Device-nGnRnE or Device-nGnRE peripherals. Additionally, there is 
> > no restriction on mapping various PCIe address spaces of the same PCIe 
> > function as different Device-nGnRnE or Device-nGnRE peripherals. 
> > Consequently, software cannot assume that program order will be maintained 
> > between accesses to two different PCIe address spaces, even though both 
> > spaces are mapped as Device-nGnRnE or Device-nGnRE. Therefore, for maximum 
> > software portability, ordering requirements between accesses to different 
> > PCIe address spaces must be handled explicitly in software using 
> > appropriate ordering instructions."
> >
> > We requested a comment from an Arm representative and received a similar 
> > response, confirming that a memory barrier is needed to ensure ordering 
> > between accesses to ECAM and MMIO regions (or between any two ranges that 
> > are assigned to a separate SAM address region)
> >
> > When they are to two different order regions, the read will not wait for 
> > the write to complete, and can return data before the write does anything. 
> > The HN-I only preserves ordering between reads and writes to the same Order 
> > Region (which implies the same Address Region). Likewise, the HN-I will 
> > only preserve ordering between multiple reads and between multiple writes 
> > within the same Order Region, and it accomplishes this by issuing the reads 
> > with the same ARID and the writes with the same AWID (i.e. it relies on the 
> > downstream device to follow AXI ordering rules). Issuing a CHI request with 
> > REQ.Order=EndpointOrder only guarantees ordering to the same “endpoint 
> > address range,” which the HN-I defines as an Order Region (within an 
> > Address Region).
> >
> > Our CMN TRM showcases an example where ECAM and MMIO are two different 
> > regions in the HN-I SAM. The implication is that we would expect a DSB 
> > between the ECAM write and MMIO read. I'm asking our Open Source Software 
> > group to confirm that standard PCIe software is generally expected to be 
> > aware of the need for a DSB--but my impression from talking to some of our 
> > hardware engineers is that that is indeed the expectation.
> >
> >
> > I am requesting that EDK2 consumes or produces a change to the current 
> > PciExpressLib that will ensure ordering on Arm architectures after 
> > Cfg-Writes which may or may not have side effects. For example, in 
> > MdePkg/Library/BasePciExpressLib/PciExpressLib.c,
> >
> > UINT8
> > EFIAPI
> > PciExpressWrite8 (
> >   IN  UINTN  Address,
> >   IN  UINT8  Value
> >   )
> > {
> >   ASSERT_INVALID_PCI_ADDRESS (Address);
> >   if (Address >= PcdPciExpressBaseSize ()) {
> > return (UINT8)-1;
> >   }
> >
> >   return MmioWrite8 ((UINTN)GetPciExpressBaseAddress () + Address, Value);
> > }
> >
> > should become
> >
> >
> >
> > UINT8
> > EFIAPI
> > PciExpressWrite8 (
> >   IN  UINTN  Address,
> >   IN  UINT8  Value
> >   )
> > {
> >   ASSERT_INVALID_PCI_ADDRESS (Address);
> >   if (Address >= PcdPciExpressBaseSize ()) {
> > return (UINT8)-1;
> >   }
> >
> >   UINT8 ReturnValue = MmioWrite8 ((UINTN)GetPciExpressBaseAddress () + 
> > Address, Value);
> >   #if defined (MDE_CPU_AARCH64)
> >  MemoryFence (); // DMB sy or DSB
> >   #endif
> >
> >   return ReturnValue;
> > }
> >
> > Please let me know your thoughts and if this is the correct implementation 
> > change needed to enforce memory ordering between 

Re: [edk2-devel] [PATCH] RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4 installed after RestEx

[Mike Maslenkin]

On Wed, Nov 1, 2023 at 6:24 AM Igor Kulchytskyy  wrote:
>
> Hi Mike,
> Thank you for review.
> Please see my answers below the text.
>
> -Original Message-
> From: Mike Maslenkin 
> Sent: Tuesday, October 31, 2023 9:00 PM
> To: devel@edk2.groups.io; Igor Kulchytskyy 
> Cc: Abner Chang ; Nickle Wang 
> Subject: [EXTERNAL] Re: [edk2-devel] [PATCH] RedfishPkg: RedfishDiscoverDxe: 
> Fix issue if IPv4 installed after RestEx
>
>
> **CAUTION: The e-mail below is from an external source. Please exercise 
> caution before opening attachments, clicking links, or following guidance.**
>
> Hi Igor
>
> please find my comments below.
>
> On Tue, Oct 31, 2023 at 8:56 PM Igor Kulchytskyy via groups.io
>  wrote:
> >
> > Supported function of the driver changed to wait for all newtwork
> > interface to be installed.
> > Filer out the network interfaces which are not supported by
> > Redfish Host Interface.
> >
> > Cc: Abner Chang 
> > Cc: Nickle Wang 
> > Cc: Igor Kulchytskyy 
> > Signed-off-by: Igor Kulchytskyy 
> > ---
> >  RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c | 96 
> > ++--
> >  1 file changed, 89 insertions(+), 7 deletions(-)
> >
> > diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c 
> > b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > index 23da3b968f..a88ad55938 100644
> > --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > @@ -322,9 +322,15 @@ GetTargetNetworkInterfaceInternal (
> >  {
> >EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL  *ThisNetworkInterface;
> >
> > +  if (IsListEmpty()) {
> > +return NULL;
> > +  }
> > +
> >ThisNetworkInterface = (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL 
> > *)GetFirstNode ();
> >while (TRUE) {
> > -if (CompareMem ((VOID *)>MacAddress, 
> > >MacAddress, ThisNetworkInterface->HwAddressSize) 
> > == 0) {
> > +if (CompareMem ((VOID *)>MacAddress, 
> > >MacAddress, ThisNetworkInterface->HwAddressSize) 
> > == 0 &&
> > +((TargetNetworkInterface->IsIpv6 && 
> > ThisNetworkInterface->NetworkProtocolType == ProtocolTypeTcp6) ||
> > +(!TargetNetworkInterface->IsIpv6 && 
> > ThisNetworkInterface->NetworkProtocolType == ProtocolTypeTcp4))) {
> >return ThisNetworkInterface;
> >  }
> >
> > @@ -354,6 +360,10 @@ GetTargetNetworkInterfaceInternalByController (
> >  {
> >EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL  *ThisNetworkInterface;
> >
> > +  if (IsListEmpty()) {
> > +return NULL;
> > +  }
> > +
>
> I also have these two hunks in my pending list.
> But I suggest to add ASSERT to GetTargetNetworkInterfaceInternal, just
> because currently it is really impossible situation,
> and mEfiRedfishDiscoverNetworkInterface was checked before in scope of
>  ValidateTargetNetworkInterface().
>
> Igor: Agree.
> I also just noticed that even if GetTargetNetworkInterfaceInternal function 
> may return NULL, the return value is not verified on NULL in 
> RedfishServiceAcquireService function.
> I think we should add this verification.

Mike: Agreed. I missed that the check in ValidateTargetNetworkInterface is:
if (IsListEmpty () &&
(TargetNetworkInterface == NULL))
not a just a IsListEmpty.

And ValidateTargetNetworkInterface is called before
GetTargetNetworkInterfaceInternal()...
But for now don't we need to add additional check for the NULL value
returned from GetTargetNetworkInterfaceInternal() in a caller ?
In RedfishServiceAcquireService():
   TargetNetworkInterfaceInternal = GetTargetNetworkInterfaceInternal
(TargetNetworkInterface);

The NULL value of TargetNetworkInterfaceInternal is propagated down.
So on some code branch it can not cause a problems (hopefully because
of same IsListEmpty checks), but it can on another.

>
>
> Also I wonder why check patch doesn't complain about missed spaces in
> "IsListEmpty ()" call for example.
>
> >ThisNetworkInterface = (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL 
> > *)GetFirstNode ();
> >while (TRUE) {
> >  if (ThisNetworkInterface->OpenDriverControllerHandle == 
> > ControllerHandle) {
> > @@ -476,6 +486,38 @@ CheckIsIpVersion6 (
> >return FALSE;
> >  }
> >
> > +/**
> > +  This function returns the  IP type supported by the Host Interface
> > +
> > +  @retval IP Type
> > +  //  Unknown=00h,
> > +  //  Ipv4=01h,
> > +  //  Ipv6=02h,
> > +
> > +**/
> > +UINT8
>
> STATIC ?
> Igor: WHY?

Mike: Just because it is local function,  No need to grep over
RedfishPkg to find all its implementations if any.

>
> > +GetHiIpProtocolType()
> > +{
> > +  EFI_STATUS Status;
> > +  REDFISH_OVER_IP_PROTOCOL_DATA  *Data;
> > +  REDFISH_INTERFACE_DATA *DeviceDescriptor;
> > +
> > +  Data = NULL;
> > +  DeviceDescriptor = NULL;
> > +  if (mSmbios == NULL) {
> > +Status = gBS->LocateProtocol (, NULL, (VOID 
> > **));
> > +if (EFI_ERROR (Status)) {
> > +  return 0;
>
> In this driver 

[edk2-devel] [PATCH] IntelFsp2Pkg/PatchFv: Fix syntax issue in markdown manual

[Ni, Ray]

According to the markdown language syntax, headings should be after
number signs (#). The number of number signs correspond to the heading
level.
But current PatchFvUserManual.md doesn't insert a space between the
number signs and the heading title, resulting the markdown file is not
rendered well in markdown viewers.

The patch doesn't change any content but only adds spaces to ensure
the headings are correctly recognized.

Signed-off-by: Ray Ni 
Cc: Chasel Chiu 
Cc: Nate DeSimone 
Cc:  Duggapu Chinni B 
Cc: Ray Han Lim Ng 
Cc: Star Zeng 
Cc: Ted Kuo 
Cc: Ashraf Ali S 
Cc: Susovan Mohapatra 
---
 .../Tools/UserManuals/PatchFvUserManual.md| 38 +--
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/IntelFsp2Pkg/Tools/UserManuals/PatchFvUserManual.md 
b/IntelFsp2Pkg/Tools/UserManuals/PatchFvUserManual.md
index f28eedf625..205ad57773 100644
--- a/IntelFsp2Pkg/Tools/UserManuals/PatchFvUserManual.md
+++ b/IntelFsp2Pkg/Tools/UserManuals/PatchFvUserManual.md
@@ -1,9 +1,9 @@
-#Name
+# Name
 **_PatchFv.py_** - The python script that patches the firmware volumes (**FV**)
 with in the flash device (**FD**) file post FSP build.
 From version 0.60, script is capable of patching flash device (**FD**) 
directly.
 
-#Synopsis
+# Synopsis
 
 ```
 PatchFv FvBuildDir [FvFileBaseNames:]FdFileBaseNameToPatch ["Offset, Value"]+
@@ -18,32 +18,32 @@ PatchFv FdFileDir FdFileName ["Offset, Value"]+
   | ["Offset, Value, $Command, @Comment"]+
 ```
 
-#Description
+# Description
 The **_PatchFv.py_** tool allows the developer to fix up FD images to follow 
the
 Intel FSP Architecture specification.  It also makes the FD image relocatable.
 The tool is written in Python and uses Python 2.7 or later to run.
 Consider using the tool in a build script.
 
-#FvBuildDir (Argument 1)
+# FvBuildDir (Argument 1)
 This is the first argument that **_PatchFv.py_** requires.  It is the build
 directory for all firmware volumes created during the FSP build. The path must
 be either an absolute path or a relevant path, relevant to the top level of the
 FSP tree.
 
-Example usage:
+ Example usage:
 ```
  Build\YouPlatformFspPkg\%BD_TARGET%_%VS_VERSION%%VS_X86%\FV
 ```
 
 The example used contains Windows batch script %VARIABLES%.
 
-#FvFileBaseNames (Argument 2: Optional Part 1)
+# FvFileBaseNames (Argument 2: Optional Part 1)
 The firmware volume file base names (**_FvFileBaseNames_**) are the independent
-Fv?s that are to be patched within the FD. (0 or more in the form
-**FVFILEBASENAME:**) The colon **:** is used for delimiting the single
+FVs that are to be patched within the FD. (0 or more in the form
+**FvFileBaseNames:**) The colon **:** is used for delimiting the single
 argument and must be appended to the end of each (**_FvFileBaseNames_**).
 
-Example usage:
+ Example usage:
 ```
 STAGE1:STAGE2:MANIFEST:YOURPLATFORM
 ```
@@ -55,14 +55,14 @@ In the example **STAGE1** is **STAGE1.Fv** in 
**YOURPLATFORM.fd**.
 Firmware device file name to patch (**_FdFileNameToPatch_**) is the base name 
of
 the FD file that is to be patched. (1 only, in the form **YOURPLATFORM**)
 
-Example usage:
+ Example usage:
 ```
 STAGE1:STAGE2:MANIFEST:YOURPLATFORM
 ```
 
 In the example **YOURPLATFORM** is from **_YOURPLATFORM.fd_**
 
-#"Offset, Value[, Command][, Comment]" (Argument 3)
+# "Offset, Value[, Command][, Comment]" (Argument 3)
 The **_Offset_** can be a positive or negative number and represents where the
 **_Value_** to be patched is located within the FD. The **_Value_** is what
 will be written at the given **_Offset_** in the FD. Constants may be used for
@@ -79,10 +79,10 @@ The entire argument includes the quote marks like in the 
example argument below:
 0xFFC0, SomeCore:__EntryPoint - [0x00F0],@SomeCore Entry
 ```
 
-###Constants:
+### Constants:
  Hexadecimal (use **0x** as prefix) | Decimal
 
-Examples:
+ Examples:
 
 | **Positive Hex** | **Negative Hex** | **Positive Decimal** | **Negative 
Decimal** |
 | ---: | ---: | ---: | 
---: |
@@ -93,7 +93,7 @@ ModuleName:FunctionName | ModuleName:GlobalVariableName
 ModuleGuid:Offset
 ```
 
-###Operators:
+### Operators:
 
 ```
 
@@ -113,7 +113,7 @@ From version 0.60 tool allows to pass flash device file 
path as Argument 1 and
 flash device name as Argument 2 and rules for passing offset & value are same
 as explained in the previous sections.
 
-Example usage:
+ Example usage:
 Argument 1
 ```
  YouPlatformFspBinPkg\
@@ -123,21 +123,21 @@ Argument 2
  Fsp_Rebased_T
 ```
 
-###Special Commands:
+### Special Commands:
 Special commands must use the **$** symbol as a prefix to the command itself.
 There is only one command available at this time.
 
 ```
-$COPY ? Copy a binary block from source to destination.
+$COPY   Copy a binary block from source to destination.
 ```
 
-Example:
+ Example:
 
 ```
 0x94, [PlatformInit:__gPcd_BinPatch_FvRecOffset] + 0x94, 

Re: [edk2-devel] [PATCH v7 3/5] MdePkg: Implement RISC-V Cache Management Operations

[Jingyu Li via groups.io]

On Tue, Oct 31, 2023 at 05:55 PM, Dhaval Sharma wrote:

> 
> I am posting an update on behalf of Jingyu as he had trouble with posting.
> CC'ing him here:
> In summary what we have verified so far:
> * I have verified that instructions/op codes are okay. I have also
> verified on Qemu that functionally it seems to be calling correct
> instructions. Ensured with negative test cases that any other op codes do
> cause exceptions as expected.
> * Jingyu was able to verify the CpuFlushCpuDataCache function with this
> framework (he had to use custom op code based on his soc implementation)
> on SG2042. There is one issue that he is debugging now which is related to
> other cache instructions and he will get back with more data. P.S. SG2042
> does not implement the exact same CMO opcodes but equivalent ones. So this
> experiment is just an additional data point that helps verify the
> framework and not CMO itself.
> * In general it sounds like framework flows are alright and as long as
> instructions do their job as claimed in the spec, it is lower risk.
> Guess this is what we have so far. If it makes sense to everyone, we could
> go ahead with merging with this *feature disabled by default* after Jingyu
> provides clarity reg failures on SG2042 platform. Otherwise we can wait
> until newer Si is available where these exact instructions can be tested
> and then upstreamed.
> 
> [From Jingyu]
> I verified this CMO framework on an actual HW platform.
> 
> SW:
> edk2: https://github.com/rivosinc/edk2/tree/dev-rv-cmo-v7 branch:
> dev-rv-cmo-v7
> edk2-platforms: https://github.com/sophgo/edk2-platforms branch: sg2042-dev
> 
> 
> HW:
> Milk-V Pioneer Box, a developer motherboard based on SG2042 with 64-Core
> T-HEAD C920.
> 
> Attention:
> The T-HEAD C920 implemented its own CMO Extension and is different from
> the standard CMO Extension.
> 
> Test steps:
> 1. Modified the opcodes in RiscVasm.inc to accommodate the C920 CMO
> feature.

Update the test status.
The InvalidateInstructionCacheRange and the InvalidateDataCacheRange execute 
the same instruction "cbo.inval", but the T-HEAD C920 executes different 
instructions for the two functions. Please see the form below for details.
I replaced ".long 0x02a5000b" with "fence.i", which solved unexpected 
exceptions that occurred yesterday.

RISC-V standard CMO Extension

C920 CMO Extension

WriteBackInvalidateDataCache

WriteBackInvalidateDataCacheRange

cbo. flush

".long 0x02b5000b"

WriteBackDataCache

WriteBackDataCacheRange

cbo. clean

".long 0x02b5000b"

InvalidateDataCache

fence

fence

InvalidateDataCacheRange

cbo. inval

".long 0x02a5000b"

InvalidateInstructionCache

fence. i

fence. i

InvalidateInstructionCacheRange

cbo. inval

fence. i

Now, I enable CMO in the entire edk2 phase, not just during PCIe inbound. No 
exceptions found so far. Hope to give you some reference.

Update the patches:
diff --git a/MdePkg/Include/RiscV64/RiscVasm.inc 
b/MdePkg/Include/RiscV64/RiscVasm.inc
index 29de735885..c2e573eb3d 100644
--- a/MdePkg/Include/RiscV64/RiscVasm.inc
+++ b/MdePkg/Include/RiscV64/RiscVasm.inc
@@ -7,13 +7,13 @@
*/

.macro RISCVCMOFLUSH
-    .word 0x25200f
+    .long 0x02b5000b^M
.endm

.macro RISCVCMOINVALIDATE
-    .word 0x05200f
+    .long 0x02a5000b^M
.endm

.macro RISCVCMOCLEAN
-    .word 0x15200f
+    .long 0x02b5000b^M
.endm

diff --git a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c 
b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c
index 5b3104afb6..ee85d0548c 100644
--- a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c
+++ b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c
@@ -89,7 +89,7 @@ CacheOpCacheRange (
Start &= ~((UINTN)CacheLineSize - 1);

DEBUG (
-    (DEBUG_INFO,
+    (DEBUG_VERBOSE,^M
"CacheOpCacheRange:\
Performing Cache Management Operation %d \n", Op)
);
@@ -163,7 +163,8 @@ InvalidateInstructionCacheRange (
)
{
if (RiscVIsCMOEnabled ()) {
-    CacheOpCacheRange (Address, Length, Invld);
+    // CacheOpCacheRange (Address, Length, Invld);^M
+    InvalidateInstructionCache ();^M
} else {
DEBUG (
(DEBUG_VERBOSE,

diff --git a/UefiCpuPkg/CpuDxeRiscV64/CpuDxe.c 
b/UefiCpuPkg/CpuDxeRiscV64/CpuDxe.c
index 2af3b62234..824667bc87 100644
--- a/UefiCpuPkg/CpuDxeRiscV64/CpuDxe.c
+++ b/UefiCpuPkg/CpuDxeRiscV64/CpuDxe.c
@@ -9,6 +9,7 @@
**/

#include "CpuDxe.h"
+#include ^M

//
// Global Variables
@@ -59,7 +60,7 @@ EFI_CPU_ARCH_PROTOCOL  gCpu = {
CpuGetTimerValue,
CpuSetMemoryAttributes,
1,                          // NumberOfTimers
-  4                           // DmaBufferAlignment
+  64                          // DmaBufferAlignment^M
};

//
@@ -90,6 +91,20 @@ CpuFlushCpuDataCache (
IN EFI_CPU_FLUSH_TYPE     FlushType
)
{
+  switch (FlushType) {^M
+    case EfiCpuFlushTypeWriteBack:^M
+      WriteBackDataCacheRange ((VOID *)(UINTN)Start, (UINTN)Length);^M
+      break;^M
+    case EfiCpuFlushTypeInvalidate:^M
+      InvalidateDataCacheRange ((VOID *)(UINTN)Start, (UINTN)Length);^M
+      break;^M
+    case 

[edk2-devel] edk2-platforms: developing Whitley Lake package

[Alireza Banejad]

Hello everyone,
I am trying to port edk2-platforms for an IceLake-SP board which uses
a C621a chipset. digging into the repo I found out that the most
compatible board inside WhitelyLakePkg is the WilsonCityRvp (The
ReadMe file inside Platform/Intel explicitly says it is IceLake-SP ).
I built it but it didn't work at all. looking into the Uba/Pei files I
found this gpio table:
#define GPIO_SKL_H_GPP_A0 0x0100
#define GPIO_SKL_H_GPP_A1 0x0101
#define GPIO_SKL_H_GPP_A2 0x0102
#define GPIO_SKL_H_GPP_A3 0x0103
#define GPIO_SKL_H_GPP_A4 0x0104
#define GPIO_SKL_H_GPP_A5 0x0105
#define GPIO_SKL_H_GPP_A6 0x0106
#define GPIO_SKL_H_GPP_A7 0x0107
#define GPIO_SKL_H_GPP_A8 0x0108
#define GPIO_SKL_H_GPP_A9 0x0109
#define GPIO_SKL_H_GPP_A10 0x010A
#define GPIO_SKL_H_GPP_A11 0x010B
#define GPIO_SKL_H_GPP_A12 0x010C
#define GPIO_SKL_H_GPP_A13 0x010D
#define GPIO_SKL_H_GPP_A14 0x010E
#define GPIO_SKL_H_GPP_A15 0x010F
#define GPIO_SKL_H_GPP_A16 0x0110
#define GPIO_SKL_H_GPP_A17 0x0111
...
Looking at the macro names I think this is meant for the SkyLake-H
platform but why does WilsonCityRvp(IceLake-SP) use this table as
well? Also, I am having a hard time understanding the values assigned
for these macros? are they the offset for each gpio register? because
in the c620 series datasheet the offset of gpio registers (A to L)
starts from 0x400.


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

Re: [edk2-devel] SSL handshake in HTTPS boot if the certificate was signed with a root certificate

[jacopo . r00ta]

Hi Laszlo,

Now I see, using the script it works perfectly fine and your explanation was 
very clear! Thank you very much, I appreciated!

Cheers,

Jacopo


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