Re: [edk2] [platforms PATCH 1/2] Marvell/Drivers: MvFvbDxe: Adjust to new dependencies

[Ard Biesheuvel]

On 17 April 2018 at 07:15, Marcin Wojtas  wrote:
> Hi Laszlo,
>
> 2018-04-16 21:41 GMT+02:00 Laszlo Ersek :
>> On 04/16/18 07:40, Ard Biesheuvel wrote:
>>> (+ Laszlo)
>>>
>>> On 16 April 2018 at 07:09, Marcin Wojtas  wrote:
 Recent changes in the EDK2 mainline resulted in breaking
 of compilation and booting of Armada platforms.
 This patch adjust the MvFvbDxe driver by:

  * installation of gEdkiiNvVarStoreFormattedGuid in order to signal
NvVarStoreFormattedLib to the generic variable runtime driver

>>>
>>> Hello Marcin,
>>>
>>> Installing this GUID is only necessary if you update your platform
>>> .DSC to make the generic variable runtime driver depend on it by
>>> adding a NULL library class resolution using NvVarStoreFormattedLib.
>>> So I think this patch is correct, but you'll need an additional change
>>> to make it work as expected. (Otherwise, the variable runtime driver
>>> could still be dispatched early and invoked for read access before the
>>> variable store is reformatted)
>>
>> I agree.
>>
>> I'd also like to point out another frequent pitfall in this patch:
>>
>>
>> While gBS->InstallProtocolInterface() takes a *pointer* to a handle
>> (because it can *create* a handle, if the handle is NULL on input, and
>> the first protocol interface is installed on it),
>> gBS->UninstallProtocolInterface() takes the handle *itself*. If the last
>> protocol interface is uninstalled from the handle, then the handle is
>> destroyed, but gBS->UninstallProtocolInterface() does not attempt to
>> NULL the handle itself. So, here you should pass "gImageHandle", not
>> "".
>>
>
> Right, I'll correct it.
>

Ah, I missed that. Thanks for spotting that Laszlo

>> There's also a bit of whitespace mangling here that's not compatible
>> with either multiline function call style that we like in edk2, but
>> perhaps edk2-platforms treats that more laxly.
>>
>
> We had some discussions last year - I followed the coding standards:
>
> Function (
>   Argument1,
>   Argument2,
>   Argument3
>   );
>
> But was requested to place Argument1 to the function line and the last
> bracket to Argument3 line:
>
> Function (Argument1,
>   Argument2,
>   Argument3);
>
> Afair, there were some attempts to modify coding standards at the
> time, but I see the original version persisted. In fact I can do
> whatever line-breaking necessary:
>
> Ard - what style do you prefer in future patches?
>

I tend to treat the coding style document as a guideline rather than
rule of law, given that it is not entirely consistent with current
practice to begin with. In my opinion, self consistency and legibility
are more important than adhering to some rule, although I realize
legibility is a subjective quality

Personally, I think the former takes up too much space in general, but
with complex expressions as arguments, it is more readable than the
latter.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [platforms PATCH 1/2] Marvell/Drivers: MvFvbDxe: Adjust to new dependencies

[Marcin Wojtas]

Hi Laszlo,

2018-04-16 21:41 GMT+02:00 Laszlo Ersek :
> On 04/16/18 07:40, Ard Biesheuvel wrote:
>> (+ Laszlo)
>>
>> On 16 April 2018 at 07:09, Marcin Wojtas  wrote:
>>> Recent changes in the EDK2 mainline resulted in breaking
>>> of compilation and booting of Armada platforms.
>>> This patch adjust the MvFvbDxe driver by:
>>>
>>>  * installation of gEdkiiNvVarStoreFormattedGuid in order to signal
>>>NvVarStoreFormattedLib to the generic variable runtime driver
>>>
>>
>> Hello Marcin,
>>
>> Installing this GUID is only necessary if you update your platform
>> .DSC to make the generic variable runtime driver depend on it by
>> adding a NULL library class resolution using NvVarStoreFormattedLib.
>> So I think this patch is correct, but you'll need an additional change
>> to make it work as expected. (Otherwise, the variable runtime driver
>> could still be dispatched early and invoked for read access before the
>> variable store is reformatted)
>
> I agree.
>
> I'd also like to point out another frequent pitfall in this patch:
>
>
> While gBS->InstallProtocolInterface() takes a *pointer* to a handle
> (because it can *create* a handle, if the handle is NULL on input, and
> the first protocol interface is installed on it),
> gBS->UninstallProtocolInterface() takes the handle *itself*. If the last
> protocol interface is uninstalled from the handle, then the handle is
> destroyed, but gBS->UninstallProtocolInterface() does not attempt to
> NULL the handle itself. So, here you should pass "gImageHandle", not
> "".
>

Right, I'll correct it.

> There's also a bit of whitespace mangling here that's not compatible
> with either multiline function call style that we like in edk2, but
> perhaps edk2-platforms treats that more laxly.
>

We had some discussions last year - I followed the coding standards:

Function (
  Argument1,
  Argument2,
  Argument3
  );

But was requested to place Argument1 to the function line and the last
bracket to Argument3 line:

Function (Argument1,
  Argument2,
  Argument3);

Afair, there were some attempts to modify coding standards at the
time, but I see the original version persisted. In fact I can do
whatever line-breaking necessary:

Ard - what style do you prefer in future patches?

Best regards,
Marcin

> Thanks
> Laszlo
>
>>> +ErrorInstallNvVarStoreFormatted:
>>>gBS->UninstallMultipleProtocolInterfaces (>Handle,
>>>   ,
>>>   ,
>>> diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf 
>>> b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf
>>> index 117fe8b..fd3f2f7 100644
>>> --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf
>>> +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf
>>> @@ -63,6 +63,7 @@
>>>UefiRuntimeServicesTableLib
>>>
>>>  [Guids]
>>> +  gEdkiiNvVarStoreFormattedGuid
>>>gEfiAuthenticatedVariableGuid
>>>gEfiEventVirtualAddressChangeGuid
>>>gEfiSystemNvDataFvGuid
>>> @@ -84,8 +85,4 @@
>>>gMarvellTokenSpaceGuid.PcdSpiMemoryBase
>>>
>>>  [Depex]
>>> -  #
>>> -  # MvFvbDxe must be loaded before VariableRuntimeDxe in case empty
>>> -  # flash needs populating with default values.
>>> -  #
>>> -  BEFORE gVariableRuntimeDxeFileGuid
>>> +  gEfiCpuArchProtocolGuid
>>> --
>>> 2.7.4
>>>
>> ___
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
>>
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [edk2-platforms PATCH v2 1/1] Platform/HiKey960: register predefined boot options

[Haojian Zhuang]

Create 4 boot options on HiKey960 platform. They're "Boot from SD",
"Grub", "Android Boot" and "Android Fastboot".

Contributed-under: TianoCore Contribution Agreement 1.1
Cc: Leif Lindholm 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Signed-off-by: Haojian Zhuang 
---
 Platform/Hisilicon/HiKey960/HiKey960.dsc   |   6 +
 .../Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c   | 145 +
 .../Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf |  11 ++
 3 files changed, 162 insertions(+)

diff --git a/Platform/Hisilicon/HiKey960/HiKey960.dsc 
b/Platform/Hisilicon/HiKey960/HiKey960.dsc
index 859ab84f8415..475d39916262 100644
--- a/Platform/Hisilicon/HiKey960/HiKey960.dsc
+++ b/Platform/Hisilicon/HiKey960/HiKey960.dsc
@@ -132,6 +132,12 @@ [PcdsFixedAtBuild.common]
   gEmbeddedTokenSpaceGuid.PcdAndroidFastbootUsbVendorId|0x18d1
   gEmbeddedTokenSpaceGuid.PcdAndroidFastbootUsbProductId|0xd00d
 
+  #
+  # Android Loader
+  #
+  
gHiKey960TokenSpaceGuid.PcdAndroidBootDevicePath|L"VenHw(0D51905B-B77E-452A-A2C0-ECA0CC8D514A,3BFF00)/UFS(0x0,0x3)/HD(7,GPT,D3340696-9B95-4C64-8DF6-E6D4548FBA41,0x12100,0x4000)"
+  
gHiKey960TokenSpaceGuid.PcdSdBootDevicePath|L"VenHw(0D51905B-B77E-452A-A2C0-ECA0CC8D514A,00F037FF00)/SD(0x0)"
+
 

 #
 # Components Section - list of all EDK II Modules needed by this Platform
diff --git a/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c 
b/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c
index 473d61ed384e..84bd76b5cae9 100644
--- a/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c
+++ b/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c
@@ -30,10 +30,16 @@
 #include 
 #include 
 #include 
+#include 
 #include 
+#include 
 #include 
 
+#include 
+#include 
 #include 
+#include 
+#include 
 #include 
 
 #define ADC_ADCIN0   0
@@ -86,6 +92,10 @@
 
 #define DETECT_SW_FASTBOOT   68// GPIO8_4
 
+#define HIKEY960_BOOT_OPTION_NUM 4
+
+#define GRUB_FILE_NAME   L"\\EFI\\BOOT\\GRUBAA64.EFI"
+
 typedef struct {
   UINT64Magic;
   UINT64Data;
@@ -359,6 +369,131 @@ OnEndOfDxe (
   }
 }
 
+STATIC
+EFI_STATUS
+RegisterPlatformBootOptions (
+  OUT EFI_BOOT_MANAGER_LOAD_OPTION   **BootOptions,
+  OUT EFI_INPUT_KEY  **BootKeys
+  )
+{
+  EFI_BOOT_MANAGER_LOAD_OPTION   *Option;
+  EFI_INPUT_KEY  *Key;
+  EFI_DEVICE_PATH*DevicePath;
+  EFI_DEVICE_PATH*FileDevicePath;
+  EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL *DPProtocol;
+  FILEPATH_DEVICE_PATH   *FilePath;
+  EFI_GUID   *FileGuid;
+  EFI_LOADED_IMAGE_PROTOCOL  *LoadedImage;
+  MEDIA_FW_VOL_FILEPATH_DEVICE_PATH  FileNode;
+  CHAR16 *PathStr;
+  EFI_STATUS Status;
+  UINTN  Size;
+
+  if ((BootOptions == NULL) || (BootKeys == NULL)) {
+return EFI_INVALID_PARAMETER;
+  }
+  // Leave the last entry as empty
+  Size = sizeof (EFI_BOOT_MANAGER_LOAD_OPTION) * (HIKEY960_BOOT_OPTION_NUM + 
1);
+  *BootOptions = (EFI_BOOT_MANAGER_LOAD_OPTION *)AllocateZeroPool (Size);
+  if (*BootOptions == NULL) {
+DEBUG ((DEBUG_ERROR, "Failed to allocate memory for BootOptions\n"));
+return EFI_BUFFER_TOO_SMALL;
+  }
+  // Leave the last entry as empty
+  Size = sizeof (EFI_INPUT_KEY) * (HIKEY960_BOOT_OPTION_NUM + 1);
+  *BootKeys = (EFI_INPUT_KEY *)AllocateZeroPool (Size);
+  if (*BootKeys == NULL) {
+DEBUG ((DEBUG_ERROR, "Failed to allocate memory for BootKeys\n"));
+FreePool (*BootOptions);
+return EFI_BUFFER_TOO_SMALL;
+  }
+  Option = *BootOptions;
+  Key = *BootKeys;
+
+  Option[0].Description = L"Boot from SD";
+  PathStr = (CHAR16 *)PcdGetPtr (PcdSdBootDevicePath);
+  ASSERT (PathStr != NULL);
+  Status = gBS->LocateProtocol (,
+  NULL,
+  (VOID **)
+  );
+  ASSERT_EFI_ERROR (Status);
+  DevicePath = (EFI_DEVICE_PATH *)DPProtocol->ConvertTextToDevicePath 
(PathStr);
+  ASSERT (DevicePath != NULL);
+  Option[0].FilePath = DevicePath;
+
+  Option[1].Description = L"Grub";
+  PathStr = (CHAR16 *)PcdGetPtr (PcdAndroidBootDevicePath);
+  ASSERT (PathStr != NULL);
+  Status = gBS->LocateProtocol (,
+  NULL,
+  (VOID **)
+  );
+  ASSERT_EFI_ERROR (Status);
+  DevicePath = (EFI_DEVICE_PATH *)DPProtocol->ConvertTextToDevicePath 
(PathStr);
+  ASSERT (DevicePath != NULL);
+  Size = StrSize (GRUB_FILE_NAME);
+  FileDevicePath = AllocatePool (Size + SIZE_OF_FILEPATH_DEVICE_PATH + 
END_DEVICE_PATH_LENGTH);
+  if (FileDevicePath != NULL) {
+FilePath = (FILEPATH_DEVICE_PATH *) FileDevicePath;
+FilePath->Header.Type 

[edk2] [PATCH v2 1/1] ArmPkg/PlatformBootManagerLib: load boot options from platform

[Haojian Zhuang]

Platform drivers sets up the array of predefined boot options.
ArmPkg/PlatformBootManager converts the array to boot options.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Haojian Zhuang 
---
 ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c | 81 ++
 .../PlatformBootManagerLib.inf |  2 +
 EmbeddedPkg/EmbeddedPkg.dec|  1 +
 EmbeddedPkg/Include/Protocol/PlatformBootManager.h | 39 +++
 4 files changed, 123 insertions(+)
 create mode 100644 EmbeddedPkg/Include/Protocol/PlatformBootManager.h

diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c 
b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
index 61ab61ccc780..68a06f5855d8 100644
--- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
+++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -392,6 +393,84 @@ PlatformRegisterFvBootOption (
 
 STATIC
 VOID
+GetPlatformOptions (
+  VOID
+  )
+{
+  EFI_STATUS  Status;
+  EFI_BOOT_MANAGER_LOAD_OPTIONNewOption;
+  EFI_BOOT_MANAGER_LOAD_OPTION*CurrentBootOptions;
+  EFI_BOOT_MANAGER_LOAD_OPTION*PlatformOption;
+  EFI_BOOT_MANAGER_LOAD_OPTION*PlatformBootOptionArray;
+  EFI_INPUT_KEY   *PlatformKeyArray;
+  EFI_INPUT_KEY   *PlatformKey;
+  PLATFORM_BOOT_MANAGER_PROTOCOL  *PlatformBootManager;
+  UINTN   CurrentBootOptionCount;
+  UINTN   OptionIndex;
+
+  Status = gBS->LocateProtocol (, NULL,
+  (VOID **));
+  if (!EFI_ERROR (Status)) {
+if (PlatformBootManager->Register) {
+  // Last entries of PlatformBootOptionArray and PlatformKeyArray are 
empty.
+  Status = PlatformBootManager->Register (
+ ,
+ 
+ );
+  if (!EFI_ERROR (Status)) {
+PlatformOption = PlatformBootOptionArray;
+PlatformKey = PlatformKeyArray;
+while (PlatformOption->Description != NULL) {
+  Status = EfiBootManagerInitializeLoadOption (
+ ,
+ LoadOptionNumberUnassigned,
+ LoadOptionTypeBoot,
+ LOAD_OPTION_ACTIVE,
+ PlatformOption->Description,
+ PlatformOption->FilePath,
+ NULL,
+ 0
+ );
+  ASSERT_EFI_ERROR (Status);
+  CurrentBootOptions = EfiBootManagerGetLoadOptions (
+ , LoadOptionTypeBoot
+ );
+  OptionIndex = EfiBootManagerFindLoadOption (
+  , CurrentBootOptions, 
CurrentBootOptionCount
+  );
+  if (OptionIndex == -1) {
+// Append the BootLoadOption
+Status = EfiBootManagerAddLoadOptionVariable (, 
MAX_UINTN);
+ASSERT_EFI_ERROR (Status);
+  }
+
+  // If UnicodeChar isn't empty, there's a hot key.
+  if (PlatformKey->UnicodeChar) {
+// The index of Boot Options counts from 1.
+// The last index equals to the count of total Boot Options.
+Status = EfiBootManagerAddKeyOptionVariable (
+   NULL, CurrentBootOptionCount, 0, PlatformKey, NULL
+   );
+  }
+
+  EfiBootManagerFreeLoadOption ();
+  EfiBootManagerFreeLoadOptions (
+CurrentBootOptions, CurrentBootOptionCount
+);
+
+  PlatformOption++;
+  PlatformKey++;
+}
+FreePool (PlatformBootOptionArray);
+FreePool (PlatformKeyArray);
+  }
+}
+  }
+
+}
+
+STATIC
+VOID
 PlatformRegisterOptionsAndKeys (
   VOID
   )
@@ -402,6 +481,8 @@ PlatformRegisterOptionsAndKeys (
   EFI_INPUT_KEYEsc;
   EFI_BOOT_MANAGER_LOAD_OPTION BootOption;
 
+  GetPlatformOptions ();
+
   //
   // Register ENTER as CONTINUE key
   //
diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf 
b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index 71f1d04a87ee..e8cbb10dabdd 100644
--- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -35,6 +35,7 @@ [Sources]
   PlatformBm.c
 
 [Packages]
+  EmbeddedPkg/EmbeddedPkg.dec
   MdeModulePkg/MdeModulePkg.dec
   MdePkg/MdePkg.dec
   ShellPkg/ShellPkg.dec
@@ -84,3 +85,4 @@ [Protocols]
   gEfiPciRootBridgeIoProtocolGuid
   gEfiSimpleFileSystemProtocolGuid
   gEsrtManagementProtocolGuid
+  gPlatformBootManagerProtocolGuid
diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
index eb135340b173..9cd50738363b 100644
--- a/EmbeddedPkg/EmbeddedPkg.dec
+++ b/EmbeddedPkg/EmbeddedPkg.dec
@@ -81,6 +81,7 @@ [Protocols.common]
   

[edk2] [PATCH v2 0/1] load boot options from platform

[Haojian Zhuang]

Changelog:
v2:
  * Avoid to use hardcoding value. Create boot options by functions.

Haojian Zhuang (1):
  ArmPkg/PlatformBootManagerLib: load boot options from platform

 ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c | 81 ++
 .../PlatformBootManagerLib.inf |  2 +
 EmbeddedPkg/EmbeddedPkg.dec|  1 +
 EmbeddedPkg/Include/Protocol/PlatformBootManager.h | 39 +++
 4 files changed, 123 insertions(+)
 create mode 100644 EmbeddedPkg/Include/Protocol/PlatformBootManager.h

-- 
2.7.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] FatPkg/EnhancedFatDxe: Ensure traverse of subtasks is delete-safe

[Hao Wu]

Within function FatQueueTask(), the traverse of FAT subtasks for
executing the disk read/write is not delete-safe.

For the below case:

FatDiskIo(): When non-blocking access, creates subtasks and creates
event (FatOnAccessComplete, NOTIFY level) when subtasks finish.

FatQueueTask(): Traverses the subtasks and submits them one by one at
Tpl lower than NOTIFY.

Disk R/W completes really quick.

FatOnAccessComplete(): Removes the finished subtask, causing the
traverse in FatQueueTask() broken.

This commits will refine the subtask traverse in FatQueueTask() to be
delete-safe.

Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
Reviewed-by: Ruiyu Ni 
---
 FatPkg/EnhancedFatDxe/Misc.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/FatPkg/EnhancedFatDxe/Misc.c b/FatPkg/EnhancedFatDxe/Misc.c
index c035670bf7..af66b1d0e3 100644
--- a/FatPkg/EnhancedFatDxe/Misc.c
+++ b/FatPkg/EnhancedFatDxe/Misc.c
@@ -132,6 +132,7 @@ FatQueueTask (
 {
   EFI_STATUS  Status;
   LIST_ENTRY  *Link;
+  LIST_ENTRY  *NextLink;
   FAT_SUBTASK *Subtask;
 
   //
@@ -149,9 +150,17 @@ FatQueueTask (
   EfiReleaseLock ();
 
   Status = EFI_SUCCESS;
-  for ( Link = GetFirstNode (>Subtasks)
-  ; !IsNull (>Subtasks, Link)
-  ; Link = GetNextNode (>Subtasks, Link)
+  //
+  // Use NextLink to store the next link of the list, because Link might be 
remove from the
+  // doubly-linked list and get freed in the end of current loop.
+  //
+  // Also, list operation APIs like IsNull() and GetNextNode() are avoided 
during the loop, since
+  // they may check the validity of doubly-linked lists by traversing them. 
These APIs cannot
+  // handle list elements being removed during the traverse.
+  //
+  for ( Link = GetFirstNode (>Subtasks), NextLink = GetNextNode 
(>Subtasks, Link)
+  ; Link != >Subtasks
+  ; Link = NextLink, NextLink = Link->ForwardLink
   ) {
 Subtask = CR (Link, FAT_SUBTASK, Link, FAT_SUBTASK_SIGNATURE);
 if (Subtask->Write) {
-- 
2.12.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 2/2] Hisilicon/D0x: Enable tftp command by default

[Guo Heyi]

On Mon, Apr 16, 2018 at 01:33:56PM +0100, Leif Lindholm wrote:
> On Mon, Apr 02, 2018 at 10:51:53AM +0800, Guo Heyi wrote:
> > Hi Ray and Leif,
> > 
> > Any comments?
> > 
> > 
> > On Mon, Mar 26, 2018 at 05:03:10PM +0800, Guo Heyi wrote:
> > > Thanks Ray.
> > > 
> > > Does that mean we need build the dynamic command driver separately and 
> > > store it
> > > in other media instead of UEFI fd image? Right now if I include the 
> > > driver into
> > > the fd image, it will be automatically added to EFI Shell command list; 
> > > we don't
> > > need to run the load command.
> > > 
> > > Hi Leif,
> > > 
> > > Is the policy to forbid including dynamic command driver into UEFI fd 
> > > image?
> 
> Let's just say that I am highly reluctant to have commands not covered
> by the UEFI Shell specification included by default. Especially
> commands that we know from experience have been misused in order to
> not have to figure out a sensible way of doing something properly
> ... and then been copied to new platforms by new developers coming in
> and looking at existing ports for reference.
> 
> > > If we need other media to store tftp command driver, then the command will
> > > become less useful, because it is mainly used to download something else. 
> > > If we
> > > have other media like USB disk, we can use this "other media" instead of 
> > > network
> > > download to store the final target.
> 
> If you are building a development image, by all means specify an
> appropriate build flag and have additional debug features built in.
> 
> But if you are looking to use this for non-development builds, I would
> very much like to hear your use-case.

In our company, network is much more feasible than other media like USB stick,
so that we can easily run some external applications or OS loaders. And USB
stick is also restricted for security concerns.

Anyway, appending a build flag is still acceptable for us, for we can add the
flag in our internal build script for development. But patch 1/2 is still
needed, or else tftp will initialize failed even if we manually set the define
flag.

Thanks,

Heyi



> 
> Best Regards,
> 
> Leif
> 
> > > Thanks,
> > > Heyi
> > > 
> > > On Fri, Mar 23, 2018 at 12:51:45PM +0800, Ni, Ruiyu wrote:
> > > > On 3/20/2018 8:15 PM, Guo Heyi wrote:
> > > > >I've no idea about how to use Driver; let me spend some time to 
> > > > >learn first
> > > > >:)
> > > > 
> > > > Heyi,
> > > > you could use "load xxxDriver.efi" to load the dynamic command in shell.
> > > > After that, you can run "tftp" in shell just as running an internal 
> > > > command.
> > > > 
> > > > >
> > > > >Regards,
> > > > >
> > > > >Heyi
> > > > >
> > > > >On Tue, Mar 20, 2018 at 09:51:32AM +, Leif Lindholm wrote:
> > > > >>Ah, apologies.
> > > > >>
> > > > >>I would be reluctant to add commands not covered by the UEFI Shell
> > > > >>Specification by default.
> > > > >>
> > > > >>Since it is now a dynamic command, is there any way of loading this
> > > > >>dynamically (perhaps via DRIVER) where you feel the need for it?
> > > > >>
> > > > >>/
> > > > >> Leif
> > > > >>
> > > > >>On Tue, Mar 20, 2018 at 03:54:46PM +0800, Guo Heyi wrote:
> > > > >>>Ping :)
> > > > >>>
> > > > >>>
> > > > >>>On Wed, Mar 07, 2018 at 04:02:30PM +, Ard Biesheuvel wrote:
> > > > On 7 March 2018 at 03:03, Heyi Guo  wrote:
> > > > >Since D0x platforms always have network enabled, we would like to
> > > > >enable tftp command by default so that we can download something in
> > > > >EFI Shell.
> > > > >
> > > > >Contributed-under: TianoCore Contribution Agreement 1.1
> > > > >Signed-off-by: Heyi Guo 
> > > > >Cc: Ard Biesheuvel 
> > > > >Cc: Leif Lindholm 
> > > > 
> > > > The first patch looks fine to me, but I would like to give Leif a
> > > > chance to comment on the policy side of this patch.
> > > > 
> > > > Please ping us by the end of next week if we haven't responded by 
> > > > then.
> > > > 
> > > > >---
> > > > >  Platform/Hisilicon/D03/D03.dsc | 2 ++
> > > > >  Platform/Hisilicon/D05/D05.dsc | 1 +
> > > > >  2 files changed, 3 insertions(+)
> > > > >
> > > > >diff --git a/Platform/Hisilicon/D03/D03.dsc 
> > > > >b/Platform/Hisilicon/D03/D03.dsc
> > > > >index cb0669d639d1..fce1e60b1275 100644
> > > > >--- a/Platform/Hisilicon/D03/D03.dsc
> > > > >+++ b/Platform/Hisilicon/D03/D03.dsc
> > > > >@@ -29,6 +29,8 @@ [Defines]
> > > > >SKUID_IDENTIFIER   = DEFAULT
> > > > >FLASH_DEFINITION   = 
> > > > > Platform/Hisilicon/$(PLATFORM_NAME)/$(PLATFORM_NAME).fdf
> > > > >
> > > > >+  DEFINE INCLUDE_TFTP_COMMAND= TRUE
> > > > >+
> > > > >  !include Silicon/Hisilicon/Hisilicon.dsc.inc
> > > > >
> > > > >  [LibraryClasses.common]
> > > > >diff --git 

Re: [edk2] [PATCH edk2-platforms 00/12] Hisilicon/D0x: Switch to generic PciHostBridge

[Guo Heyi]

BTW, there is actually a bug with ATU configuration which will cause IO access
failure, and we need to apply an additional patch (this patch is generated after
PCI host bridge patch series) as attached to fix this.

Regards,
Heyi

On Tue, Apr 17, 2018 at 09:20:44AM +0800, Guo Heyi wrote:
> Hi Ard,
> 
> I tested mm -io on D05, for root bridge 4 with CPU IO address starting from
> 0x8_abff, and it worked; both mm -io 0x8abff and mm 0x8abff 
> provided
> the same output. It seems there is no other limit for 64bit IO address after 
> you
> fixed the issue in EFI shell mm command.
> 
> Thanks and regards,
> 
> Heyi
> 
> On Mon, Apr 16, 2018 at 09:57:09PM +0800, Guo Heyi wrote:
> > Thanks, I will test mm command and let you know the result.
> > 
> > Regards,
> > 
> > Heyi
> > 
> > On Fri, Apr 13, 2018 at 09:19:53AM +0200, Ard Biesheuvel wrote:
> > > On 13 April 2018 at 04:05, Guo Heyi  wrote:
> > > > Hi Ard,
> > > >
> > > > Any comments?
> > > >
> > > 
> > > Apologies for the delay. I have been travelling and am behind on email.
> > > 
> > > > Anyway we can modify the code if you insist on using an intermediate 
> > > > CPU IO
> > > > address space.
> > > >
> > > 
> > > I have not made up my mind yet, to be honest. I agree there is a
> > > certain elegance to merging both translations, but I am concerned that
> > > existing EDK2 code may deal poorly with I/O addresses that require
> > > more than 32 bits to express.
> > > 
> > > Did you try the mm command in the shell for instance? As you know, I
> > > recently removed an artificial address range limit there, but I wonder
> > > if it uses 64-bit variables for I/O ports.
>From guoh...@huawei.com Tue Apr 17 09:40:07 2018
Delivered-To: heyi@linaro.org
Received: by 10.103.107.2 with SMTP id g2csp1147351vsc;
Mon, 16 Apr 2018 18:40:07 -0700 (PDT)
X-Google-Smtp-Source: AIpwx49xa2EjXi3IIuqoYaJ9ZR+KlZePkYWmAvMpJl534IXT0zWt/Jd5UHxMjyCgas2Aluws+S26
X-Received: by 10.101.97.165 with SMTP id i5mr102113pgv.449.1523929207660;
Mon, 16 Apr 2018 18:40:07 -0700 (PDT)
ARC-Seal: i=1; a=rsa-sha256; t=1523929207; cv=none;
d=google.com; s=arc-20160816;
b=JVhwQqc2tHJH/nbb9J4Q5EAe7efNCva8XlNdyeM1AjecnvMtalXxRiIcVSIp2CTTUb
 RCfB400z6ay0s7SW96MDtN0D8jezfXLZXyuMQTqYAvpXu7rxEmXGJtKblXaJ303GTvgS
 kavhZQ7QawTzXzWTjokcARGeyR+mFb9aXT8JKm3jdHVM+Ibsjl0HFYSFrMNFoeyC7EqG
 EPMbjEvcWolADxpywSCM0s0e4EhIVpGrhi1LgdVeATsyrb6GN8sEscVfb+t0b0gMj6XI
 Nqg7mMHnKZGJ4vmkJBRV5Gx+nnQbOWKsOCp8YZcVLFs8hyqWFokYsup3wsY1YtktovxU
 GQUg==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816;
h=mime-version:message-id:date:subject:cc:to:from
 :arc-authentication-results;
bh=MzafS17Rdgthy05RGe4B87hGASmPkGgjBWmjX1ExxLo=;
b=fj+XbZcdH1vGFsHllnQeTc3zRavri5L0Use7rHdCRemtfNojWbY1QJKD6tMVi213fj
 J9IaKL5/Q3Uh0KLfAR848Whbuj4IiFMw1vRsT8BUvu0QjkCsdJo66ypGdQTqkysdtk2X
 fdNfNTE+Dfa1du8Yx9RiFCetXrtgCjDXljVzd/JufSlKtC41HjgWjnqC2Jka4N0MOT1W
 oiBtsqOCjRuVRyb5Rlf7RVstwZ7wz6jXbxj7GYkr85bUFe7LNG8HOKxJYIv1bLyOWw6q
 KvttRU9MKaVex3HApPoMIIgg+sXj3QD1islzlobdLA/OaRKWXCH0fnXH/NuWnKZ7J1jO
 Hl7A==
ARC-Authentication-Results: i=1; mx.google.com;
   spf=pass (google.com: domain of guoh...@huawei.com designates 45.249.212.32 as permitted sender) smtp.mailfrom=guoh...@huawei.com
Return-Path: 
Received: from huawei.com ([45.249.212.32])
by mx.google.com with ESMTPS id t3si10564595pgt.547.2018.04.16.18.40.07
for 
(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
Mon, 16 Apr 2018 18:40:07 -0700 (PDT)
Received-SPF: pass (google.com: domain of guoh...@huawei.com designates 45.249.212.32 as permitted sender) client-ip=45.249.212.32;
Authentication-Results: mx.google.com;
   spf=pass (google.com: domain of guoh...@huawei.com designates 45.249.212.32 as permitted sender) smtp.mailfrom=guoh...@huawei.com
Received: from DGGEMS405-HUB.china.huawei.com (unknown [172.30.72.58])
	by Forcepoint Email with ESMTP id 2B03711A12683
	for ; Tue, 17 Apr 2018 09:40:04 +0800 (CST)
Received: from HGH139998.huawei.com (10.184.68.188) by
 DGGEMS405-HUB.china.huawei.com (10.3.19.205) with Microsoft SMTP Server id
 14.3.361.1; Tue, 17 Apr 2018 09:39:59 +0800
From: Heyi Guo 
To: 
CC: , ,
	
Subject: [PATCH] Hisilicon/Hi161x/PcieInit: fix address overlap
Date: Tue, 17 Apr 2018 09:35:22 +0800
Message-ID: <1523928922-9573-1-git-send-email-guoh...@huawei.com>
X-Mailer: git-send-email 2.8.1
MIME-Version: 1.0
Content-Type: text/plain
X-Originating-IP: [10.184.68.188]
X-CFilter-Loop: Reflected
Content-Length: 2888
Lines: 55

From: Heyi Guo 

PCIe IO address ranges are overlapped by configuration address spaces
when we set 

Re: [edk2] [PATCH edk2-platforms 00/12] Hisilicon/D0x: Switch to generic PciHostBridge

[Guo Heyi]

Hi Ard,

I tested mm -io on D05, for root bridge 4 with CPU IO address starting from
0x8_abff, and it worked; both mm -io 0x8abff and mm 0x8abff provided
the same output. It seems there is no other limit for 64bit IO address after you
fixed the issue in EFI shell mm command.

Thanks and regards,

Heyi

On Mon, Apr 16, 2018 at 09:57:09PM +0800, Guo Heyi wrote:
> Thanks, I will test mm command and let you know the result.
> 
> Regards,
> 
> Heyi
> 
> On Fri, Apr 13, 2018 at 09:19:53AM +0200, Ard Biesheuvel wrote:
> > On 13 April 2018 at 04:05, Guo Heyi  wrote:
> > > Hi Ard,
> > >
> > > Any comments?
> > >
> > 
> > Apologies for the delay. I have been travelling and am behind on email.
> > 
> > > Anyway we can modify the code if you insist on using an intermediate CPU 
> > > IO
> > > address space.
> > >
> > 
> > I have not made up my mind yet, to be honest. I agree there is a
> > certain elegance to merging both translations, but I am concerned that
> > existing EDK2 code may deal poorly with I/O addresses that require
> > more than 32 bits to express.
> > 
> > Did you try the mm command in the shell for instance? As you know, I
> > recently removed an artificial address range limit there, but I wonder
> > if it uses 64-bit variables for I/O ports.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v1 08/18] StandaloneMmPkg/MemLib: AARCH64 Specific instance of memory check library.

[Yao, Jiewen]

Hi
I don't think this lib is generic, because it hardcode the physical address 
bits.

PhysicalAddressBits = 36;

For X86 CPU, we get it from CPUID. :-)

As enhancement, we may put most common C-code logic (such as CopyMem, or memmap 
calculation) to StandaloneMmPkg/MemLib, and only include the PhysicalAddresBit 
calculation under StandaloneMmPkg/MemLib/Arm folder.

As such, we know clearly on which one is ARM specific.

Thank you
Yao Jiewen

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Achin
> Gupta
> Sent: Monday, April 16, 2018 11:13 PM
> To: Supreeth Venkatesh 
> Cc: ard.biesheu...@linaro.org; edk2-devel@lists.01.org;
> leif.lindh...@linaro.org; Yao, Jiewen ; Gao, Liming
> ; Kinney, Michael D ;
> n...@arm.com
> Subject: Re: [edk2] [PATCH v1 08/18] StandaloneMmPkg/MemLib: AARCH64
> Specific instance of memory check library.
> 
> Hi Supreeth,
> 
> On Fri, Apr 06, 2018 at 03:42:13PM +0100, Supreeth Venkatesh wrote:
> > MM memory check library library implementation. This library consumes
> > MM_ACCESS_PROTOCOL to get MMRAM information. In order to use this
> > library instance, the platform should produce all MMRAM range via
> > MM_ACCESS_PROTOCOL, including the range for firmware (like MM Core
> > and MM driver) and/or specific dedicated hardware.
> >
> > This patch provides services for MM Memory Operation.
> > The management mode Mem Library provides function for checking if buffer
> > is outside MMRAM and valid. It also provides functions for copy data
> > from MMRAM to non-MMRAM, from non-MMRAM to MMRAM,
> > from non-MMRAM to non-MMRAM, or set data in non-MMRAM.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Achin Gupta 
> > Signed-off-by: Supreeth Venkatesh 
> > ---
> >  StandaloneMmPkg/Include/Library/MemLib.h| 140 ++
> >  StandaloneMmPkg/Library/MemLib/Arm/MemLib.c | 276
> 
> 
> Why is this Library Arm specific. Apart from cosmetics tweaks, it has not
> changed since it was originally contributed?
> 
> cheers,
> Achin
> 
> >  StandaloneMmPkg/Library/MemLib/MemLib.inf   |  47 +
> >  3 files changed, 463 insertions(+)
> >  create mode 100644 StandaloneMmPkg/Include/Library/MemLib.h
> >  create mode 100644 StandaloneMmPkg/Library/MemLib/Arm/MemLib.c
> >  create mode 100644 StandaloneMmPkg/Library/MemLib/MemLib.inf
> >
> > diff --git a/StandaloneMmPkg/Include/Library/MemLib.h
> b/StandaloneMmPkg/Include/Library/MemLib.h
> > new file mode 100644
> > index 00..3264f10010
> > --- /dev/null
> > +++ b/StandaloneMmPkg/Include/Library/MemLib.h
> > @@ -0,0 +1,140 @@
> > +/** @file
> > +  Provides services for MM Memory Operation.
> > +
> > +  The MM Mem Library provides function for checking if buffer is outside
> MMRAM and valid.
> > +  It also provides functions for copy data from MMRAM to non-MMRAM,
> from non-MMRAM to MMRAM,
> > +  from non-MMRAM to non-MMRAM, or set data in non-MMRAM.
> > +
> > +  Copyright (c) 2015, Intel Corporation. All rights reserved.
> > +  Copyright (c) 2016 - 2017, ARM Limited. All rights reserved.
> > +
> > +  This program and the accompanying materials
> > +  are licensed and made available under the terms and conditions of the BSD
> License
> > +  which accompanies this distribution.  The full text of the license may be
> found at
> > +  http://opensource.org/licenses/bsd-license.php
> > +
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> > +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> > +
> > +**/
> > +
> > +#ifndef _MM_MEM_LIB_H_
> > +#define _MM_MEM_LIB_H_
> > +
> > +/**
> > +  This function check if the buffer is valid per processor architecture 
> > and not
> overlap with MMRAM.
> > +
> > +  @param Buffer  The buffer start address to be checked.
> > +  @param Length  The buffer length to be checked.
> > +
> > +  @retval TRUE  This buffer is valid per processor architecture and not
> overlap with MMRAM.
> > +  @retval FALSE This buffer is not valid per processor architecture or 
> > overlap
> with MMRAM.
> > +**/
> > +BOOLEAN
> > +EFIAPI
> > +MmIsBufferOutsideMmValid (
> > +  IN EFI_PHYSICAL_ADDRESS  Buffer,
> > +  IN UINT64Length
> > +  );
> > +
> > +/**
> > +  Copies a source buffer (non-MMRAM) to a destination buffer (MMRAM).
> > +
> > +  This function copies a source buffer (non-MMRAM) to a destination buffer
> (MMRAM).
> > +  It checks if source buffer is valid per processor architecture and not 
> > overlap
> with MMRAM.
> > +  If the check passes, it copies memory and returns EFI_SUCCESS.
> > +  If the check fails, it return EFI_SECURITY_VIOLATION.
> > +  The implementation must be reentrant.
> > +
> > +  @param  DestinationBuffer   The pointer to the destination buffer of the
> memory 

Re: [edk2] [PATCH] MdePkg: add big-endian MMIO BaseBeIoLib

[Michael Brown]

On 16/04/18 21:42, Laszlo Ersek wrote:

On 04/16/18 16:34, Michael Brown wrote:

On 16/04/18 15:10, Kinney, Michael D wrote:

I think we only need a single lib class and lib
Instance that does the byte swap and we should
not use Le or Be in any of the names of the class,
instance, or APIs.  Just "Swap".


I may have misunderstood, but wouldn't using "Swap" within the API names
effectively encode knowledge of the endianness of the _build_ platform
into the source code?  This would prevent the same source code being
built for both little-endian and big-endian CPUs.


Under this scenario, all drivers meant to be portable to both byte
orders would have to:
- link against both IoLib and IoSwapLib,
- determine at device binding time, from CPU endianness and device
   endianness combined, whether swapping was needed for that device,
- call the IoLib or IoSwapLib APIs through wrapper functions, or
   function pointers.


Given that "all drivers meant to be portable to both byte orders" would 
include almost the complete set of PCI device drivers, that sounds like 
an incredibly large amount of unnecessarily duplicated boilerplate code.


Maybe we need some kind of wrapper library that provides an abstraction 
layer to automatically determine whether or not swapping is needed and 
then call the appropriate IoLib or IoSwapLib API function.  For example, 
the wrapper library could provide a function 
SwapIfNeededForBigEndianDeviceMmioRead16() which would perform a runtime 
check on each call to determine the current CPU endianness and then call 
MmioRead16() or SwapMmioRead16() as appropriate.


Michael
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdePkg: add big-endian MMIO BaseBeIoLib

[Laszlo Ersek]

On 04/16/18 16:34, Michael Brown wrote:
> On 16/04/18 15:10, Kinney, Michael D wrote:
>> I agree that the opposite use case is a BE CPU
>> needing a LE operation.
>>
>> I think we only need a single lib class and lib
>> Instance that does the byte swap and we should
>> not use Le or Be in any of the names of the class,
>> instance, or APIs.  Just "Swap".
> 
> I may have misunderstood, but wouldn't using "Swap" within the API names
> effectively encode knowledge of the endianness of the _build_ platform
> into the source code?  This would prevent the same source code being
> built for both little-endian and big-endian CPUs.

Under this scenario, all drivers meant to be portable to both byte
orders would have to:
- link against both IoLib and IoSwapLib,
- determine at device binding time, from CPU endianness and device
  endianness combined, whether swapping was needed for that device,
- call the IoLib or IoSwapLib APIs through wrapper functions, or
  function pointers.

Thanks,
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Source code debugging of OVMF

[Laszlo Ersek]

On 04/16/18 20:25, Rebecca Cran wrote:
> On 04/16/18 10:13, Laszlo Ersek wrote:
> 
>> Here's another thread that you might find useful:
>>
>> http://edk2-devel.narkive.com/6BRVus92/qestion-about-how-to-debug-ovmf-on-qemu
> 
> I should get my Phabricator wiki running again, which has a
> nicely-formatted version of that - I haven't set it up again after
> moving systems.
> 

Yes please! :)

Thanks!
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] OvmfPkg/PlatformBootManagerLib: add USB keyboard to ConIn

[Laszlo Ersek]

On 04/16/18 07:41, Ard Biesheuvel wrote:
> On 15 April 2018 at 23:15, Laszlo Ersek  wrote:
>> PlatformInitializeConsole() (called by PlatformBootManagerBeforeConsole())
>> adds elements of "gPlatformConsole" to ConIn / ConOut / ErrOut (as
>> requested per element) if at boot at least one of ConIn and ConOut doesn't
>> exist. This typically applies to new VMs, and VMs with freshly recreated
>> varstores.
>>
>> Add a USB keyboard wildcard to ConIn via "gPlatformConsole", so that we
>> not only bind the PS/2 keyboard. (The PS/2 keyboard is added in
>> PrepareLpcBridgeDevicePath()). Explicitly connecting the USB keyboard is
>> necessary after commit 245c643cc8b7.
>>
>> Cc: Ard Biesheuvel 
>> Cc: Jordan Justen 
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Laszlo Ersek 
> 
> Reviewed-by: Ard Biesheuvel 

Thanks Ard, commit 5e0e476a9542.

Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ArmPkg/ArmMmuLib ARM: fix Mva to use idx instead of table base

[Chris Co]

Hi Leif,

> -Original Message-
> From: Leif Lindholm 
> Sent: Monday, April 16, 2018 3:44 AM
> To: Chris Co 
> Cc: edk2-devel@lists.01.org; Ard Biesheuvel 
> Subject: Re: [PATCH] ArmPkg/ArmMmuLib ARM: fix Mva to use idx instead
> of table base
> 
> On Fri, Apr 13, 2018 at 11:43:27PM +, Chris Co wrote:
> > Mva address calculation should use the left-shifted current section
> > index instead of the left-shifted table base address.
> >
> > Using the table base address here has the side-effect of potentially
> > causing an access violation depending on the base address value.
> >
> > Cc: Leif Lindholm 
> > Cc: Ard Biesheuvel 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Christopher Co 
> > ---
> >  ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> > b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> > index 774a7ccf59..9bf4ba03fd 100644
> > --- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> > +++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> > @@ -716,7 +716,7 @@ UpdateSectionEntries (
> >Descriptor |= EntryValue;
> >
> >if (CurrentDescriptor  != Descriptor) {
> > -Mva = (VOID *)(UINTN)(((UINTN)FirstLevelTable) <<
> TT_DESCRIPTOR_SECTION_BASE_SHIFT);
> > +Mva = (VOID *)(UINTN)(((UINTN)FirstLevelIdx + i) <<
> > + TT_DESCRIPTOR_SECTION_BASE_SHIFT);
> 
> So, this clearly looks like you've found a bug - thanks!
> 
> But I am a little bit confused about the patch - should this not need to
> incorporate the descriptor size in some way?
> I.e. something like
>   Mva = (VOID *)(UINTN)(((UINTN)FirstLevelIdx + (i * sizeof(UINTN))) <<
> TT_DESCRIPTOR_SECTION_BASE_SHIFT);
> or
>   ...   [FirstLevelIndex + i] ...
> 
> ?
> 
> Regards,
> 
> Leif
> 
I don't think descriptor size is needed here.  

My understanding is that Mva is the base address of the current section. 

FirstLevelidx is derived by the first section's BaseAddress >> 20.  The current 
section
index is then (FirstLevelIdx + i), which makes the base address of the current 
section (FirstLeveLidx + i) << 20.

Thanks,
Chris
> >
> >  // Clean/invalidate the cache for this section, but only
> >  // if we are modifying the memory type attributes
> > --
> > 2.15.1.gvfs.2.39.g03d366a
> >
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [platforms PATCH 2/2] Marvell/Armada: RealTimeClockLib: Depend on gEfiCpuArchProtocolGuid

[Laszlo Ersek]

On 04/16/18 07:09, Marcin Wojtas wrote:
> Recent changes in the EDK2 mainline resulted in breaking
> RTC functionality of Armada platforms.
> 
> The RealTimeClockLib instance calls gDS->SetMemorySpaceAttributes()
> in the LibRtcInitialize() public function. This DXE service depends
> on the CPU Arch Protocol. Add it to the depex.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas 
> ---
>  Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.inf | 5 
> -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git 
> a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.inf 
> b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.inf
> index 2f842e8..59c71c4 100644
> --- a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.inf
> +++ b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.inf
> @@ -25,7 +25,7 @@
>FILE_GUID  = fa81e889-045b-4c96-9093-742554fd0588
>MODULE_TYPE= BASE
>VERSION_STRING = 1.0
> -  LIBRARY_CLASS  = RealTimeClockLib
> +  LIBRARY_CLASS  = RealTimeClockLib|DXE_RUNTIME_DRIVER
>  
>  [Sources.common]
>RealTimeClockLib.c
> @@ -50,3 +50,6 @@
>  
>  [Pcd]
>gMarvellTokenSpaceGuid.PcdRtcEnabled
> +
> +[Depex.common.DXE_RUNTIME_DRIVER]
> +  gEfiCpuArchProtocolGuid
> 

Acked-by: Laszlo Ersek 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [platforms PATCH 1/2] Marvell/Drivers: MvFvbDxe: Adjust to new dependencies

[Laszlo Ersek]

On 04/16/18 07:40, Ard Biesheuvel wrote:
> (+ Laszlo)
> 
> On 16 April 2018 at 07:09, Marcin Wojtas  wrote:
>> Recent changes in the EDK2 mainline resulted in breaking
>> of compilation and booting of Armada platforms.
>> This patch adjust the MvFvbDxe driver by:
>>
>>  * installation of gEdkiiNvVarStoreFormattedGuid in order to signal
>>NvVarStoreFormattedLib to the generic variable runtime driver
>>
> 
> Hello Marcin,
> 
> Installing this GUID is only necessary if you update your platform
> .DSC to make the generic variable runtime driver depend on it by
> adding a NULL library class resolution using NvVarStoreFormattedLib.
> So I think this patch is correct, but you'll need an additional change
> to make it work as expected. (Otherwise, the variable runtime driver
> could still be dispatched early and invoked for read access before the
> variable store is reformatted)

I agree.

I'd also like to point out another frequent pitfall in this patch:

>>  * making explicit dependency to ArmPkg/Drivers/CpuDxe drivers in order
>>to enable successful calling of gDS->SetMemorySpaceAttributes
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Marcin Wojtas 
>> ---
>>  Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c   | 21 
>>  Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf |  7 ++-
>>  2 files changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c 
>> b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c
>> index 252ef67..6e583a3 100644
>> --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c
>> +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c
>> @@ -26,6 +26,7 @@
>>  #include 
>>  #include 
>>
>> +#include 
>>  #include 
>>  #include 
>>
>> @@ -1076,6 +1077,21 @@ MvFvbEntryPoint (
>>}
>>
>>//
>> +  // The driver implementing the variable read service can now be 
>> dispatched;
>> +  // the varstore headers are in place.
>> +  //
>> +  Status = gBS->InstallProtocolInterface (,
>> +  ,
>> +  EFI_NATIVE_INTERFACE,
>> +  NULL);
>> +  if (EFI_ERROR (Status)) {
>> +DEBUG ((DEBUG_ERROR,
>> +  "%a: Failed to install gEdkiiNvVarStoreFormattedGuid\n",
>> +  __FUNCTION__));
>> +goto ErrorInstallNvVarStoreFormatted;
>> +  }
>> +
>> +  //
>>// Declare the Non-Volatile storage as EFI_MEMORY_RUNTIME
>>//
>>RuntimeMmioRegionSize = mFvbDevice->FvbSize;
>> @@ -1126,6 +1142,11 @@ ErrorSetMemAttr:
>>gDS->RemoveMemorySpace (RegionBaseAddress, RuntimeMmioRegionSize);
>>
>>  ErrorAddSpace:
>> +  gBS->UninstallProtocolInterface (,
>> +  ,
>> +  NULL);
>> +

While gBS->InstallProtocolInterface() takes a *pointer* to a handle
(because it can *create* a handle, if the handle is NULL on input, and
the first protocol interface is installed on it),
gBS->UninstallProtocolInterface() takes the handle *itself*. If the last
protocol interface is uninstalled from the handle, then the handle is
destroyed, but gBS->UninstallProtocolInterface() does not attempt to
NULL the handle itself. So, here you should pass "gImageHandle", not
"".

There's also a bit of whitespace mangling here that's not compatible
with either multiline function call style that we like in edk2, but
perhaps edk2-platforms treats that more laxly.

Thanks
Laszlo

>> +ErrorInstallNvVarStoreFormatted:
>>gBS->UninstallMultipleProtocolInterfaces (>Handle,
>>   ,
>>   ,
>> diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf 
>> b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf
>> index 117fe8b..fd3f2f7 100644
>> --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf
>> +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf
>> @@ -63,6 +63,7 @@
>>UefiRuntimeServicesTableLib
>>
>>  [Guids]
>> +  gEdkiiNvVarStoreFormattedGuid
>>gEfiAuthenticatedVariableGuid
>>gEfiEventVirtualAddressChangeGuid
>>gEfiSystemNvDataFvGuid
>> @@ -84,8 +85,4 @@
>>gMarvellTokenSpaceGuid.PcdSpiMemoryBase
>>
>>  [Depex]
>> -  #
>> -  # MvFvbDxe must be loaded before VariableRuntimeDxe in case empty
>> -  # flash needs populating with default values.
>> -  #
>> -  BEFORE gVariableRuntimeDxeFileGuid
>> +  gEfiCpuArchProtocolGuid
>> --
>> 2.7.4
>>
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdePkg: add big-endian MMIO BaseBeIoLib

[Laszlo Ersek]

On 04/16/18 12:07, Leif Lindholm wrote:
> On Fri, Apr 13, 2018 at 11:32:35PM +, Kinney, Michael D wrote:
>> Leif,
>>
>> I am curious why a Swap class/instances is not sufficient.
>>
>> Currently EDK II follows the UEFI/PI specs, which for all supported
>> CPU architectures use little endian ABI. The BaseIoLib follows the
>> endianness of the CPU.  If UEFI/PI added a CPU that was big endian, I
>> would expect BaseIoLib when built for that CPU would perform big
>> endian operations.
>>
>> Am I missing something?
>
> If you did add a big-endian CPU, you could then find yourself in the
> exact opposite situation and require a little-endian i/o access
> library. Which would be implemented exactly as the contents of
> IoLibSwap.c.
>
> The header file necessarily needs to be endianness-specific, and if
> the coding style had permitted functions in header files, my automatic
> reaction would have been to make all of these static inline helper
> functions (even with the code duplication).

First, to remind myself of the previous discussion (and please correct
me if I remember incorrectly): whether swapping is needed or not depends
on both CPU byte order and device byte order. However, the library API
names that device drivers use should reflect device byte order *only*
(regardless of CPU byte order). This way,

(a) developers that write device drivers can focus on the devices,
regardless of what CPU the driver is compiled for,

(b) library classes for both LE and BE devices can be used together in
the same driver module,

(c) assuming we introduce a CPU with BE byte order, the same driver
source will work (for both LE and BE devices), only the lib
instances will have to be switched around. (This might even happen
dynamically, via function pointers.)

Now, after staring at this patch long and hard, here's my understanding.
Crucially, you take the IoLib class to mean "it doesn't swap". You don't
take it to mean "it talks to LE devices". This is evident from the
source file "IoLibSwap.c", where you add the swapping on top of IoLib.
That's fine, but it will have consequences:

(1) We need a separate library class called IoSwapLib (not IoLibSwap),
implemented under "MdePkg/Library/BaseIoSwapLib/BaseIoSwapLib.c",
and without the preprocessor trickery. There should be one library
instance only, adding nothing but byte swapping on top of IoLib.

(2) We need separate library classes called BeIoLib and LeIoLib. These
provide the ultimate APIs that I describe near the top. In total we
should have four library instances that are explicit about device
endianness. This means four INF files, and a single shared C source
file, *with* the preprocessor trickery.

(2.1) BaseBeIoLib.inf: implements the BeIoLib functions on top of IoLib,
  that is, without swapping -- this means that it is suitable for
  talking to BE devices on BE CPUs.

(2.2) BaseBeIoLibSwap.inf: implements the BeIoLib functions on top of
  IoSwapLib -- talks to BE devices on LE CPUs.

(2.3) BaseLeIoLib.inf: implements LeIoLib functions on top of IoLib --
  talks to LE devices on LE CPUs.

(2.4) BaseLeIoLibSwap.inf: implements LeIoLib functions on top of
  IoSwapLib -- talks to LE devices on BE CPUs.

IMO, it's fine if you only want to add the BeIoLib class now, with its
BaseBeIoLibSwap instance only. But the IoSwapLib and BeIoLib classes
must exist separately nonetheless. And that's because your currently
proposed C source file is unable to express case (2.1): you can generate
the "Be" prefix alright, but the internals will always swap, and do that
on top of IoLib (which *never* swaps). So you will end up swapping once
in the Be*() functions, which is wrong for talking to BE devices on BE
CPUs.

I guess -- relaxing my initial point a bit -- it's also OK if you do not
introduce the BeIoLib class at all (let alone LeIoLib), saying that
drivers are generally expected to compute at startup whether they need
to byte-swap or not (considering both CPU and device byte order), and
then they need to flip a few function pointers between IoLib versus
IoSwapLib for all further use. In that case however the patch should not
say "Be" or "Le" in any lib class, instance, or API names, at all.

Some other random comments I have:

(3) You mention that no UNI file is being duplicated, but I do see one.

(4) Please consider adopting

  
https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-10
  
https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-23

so that patches roughly advance from "abstract" to "concrete".

(5) You skipped the following family of functions:
MmioBitField(Read|Write|Or|And|AndThenOr)(16|32|64). I don't think
that's right: the mask values at the least are expressed in host
byte order, and they'll need conversion. An earlier 

Re: [edk2] Source code debugging of OVMF

[Rebecca Cran]

On 04/16/18 10:13, Laszlo Ersek wrote:

> Here's another thread that you might find useful:
> 
> http://edk2-devel.narkive.com/6BRVus92/qestion-about-how-to-debug-ovmf-on-qemu

I should get my Phabricator wiki running again, which has a
nicely-formatted version of that - I haven't set it up again after
moving systems.

-- 
Rebecca
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v1 1/1] BaseTools: Eot - Remove FvImage file

[Jaben Carsey]

move the single used class from FvImage to Eot
delete the FvImage file
remove FvImage from makefile

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/Eot/Eot.py |   26 +-
 BaseTools/Source/Python/Eot/FvImage.py | 1434 
 BaseTools/Source/Python/Makefile   |1 -
 3 files changed, 24 insertions(+), 1437 deletions(-)

diff --git a/BaseTools/Source/Python/Eot/Eot.py 
b/BaseTools/Source/Python/Eot/Eot.py
index 96c339613476..de32e99bb937 100644
--- a/BaseTools/Source/Python/Eot/Eot.py
+++ b/BaseTools/Source/Python/Eot/Eot.py
@@ -1,7 +1,7 @@
 ## @file
 # This file is used to be the main entrance of EOT tool
 #
-# Copyright (c) 2008 - 2014, Intel Corporation. All rights reserved.
+# Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.
 # This program and the accompanying materials
 # are licensed and made available under the terms and conditions of the BSD 
License
 # which accompanies this distribution.  The full text of the license may be 
found at
@@ -24,13 +24,35 @@ from Common.Misc import GuidStructureStringToGuidString
 from InfParserLite import *
 import c
 import Database
-from FvImage import *
 from array import array
 from Report import Report
 from Common.BuildVersion import gBUILD_VERSION
 from Parser import ConvertGuid
 from Common.LongFilePathSupport import OpenLongFilePath as open
 
+## MultipleFv() class
+#
+#  A class for Multiple FV
+#
+class MultipleFv(FirmwareVolume):
+def __init__(self, FvList):
+FirmwareVolume.__init__(self)
+self.BasicInfo = []
+for FvPath in FvList:
+FvName = os.path.splitext(os.path.split(FvPath)[1])[0]
+Fd = open(FvPath, 'rb')
+Buf = array('B')
+try:
+Buf.fromfile(Fd, os.path.getsize(FvPath))
+except EOFError:
+pass
+
+Fv = FirmwareVolume(FvName)
+Fv.frombuffer(Buf, 0, len(Buf))
+
+self.BasicInfo.append([Fv.Name, Fv.FileSystemGuid, Fv.Size])
+self.FfsDict.append(Fv.FfsDict)
+
 ## Class Eot
 #
 # This class is used to define Eot main entrance
diff --git a/BaseTools/Source/Python/Eot/FvImage.py 
b/BaseTools/Source/Python/Eot/FvImage.py
deleted file mode 100644
index 472ae400506d..
--- a/BaseTools/Source/Python/Eot/FvImage.py
+++ /dev/null
@@ -1,1434 +0,0 @@
-## @file
-# Parse FV image
-#
-# Copyright (c) 2008 - 2014, Intel Corporation. All rights reserved.
-# This program and the accompanying materials
-# are licensed and made available under the terms and conditions of the BSD 
License
-# which accompanies this distribution.  The full text of the license may be 
found at
-# http://opensource.org/licenses/bsd-license.php
-#
-# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-#
-
-## Import Modules
-#
-import Common.LongFilePathOs as os
-import re
-import sys
-import uuid
-import struct
-import codecs
-import copy
-
-from UserDict import IterableUserDict
-from cStringIO import StringIO
-from array import array
-from Common.LongFilePathSupport import OpenLongFilePath as open
-from CommonDataClass import *
-from Common.Misc import sdict, GuidStructureStringToGuidString
-
-import Common.EdkLogger as EdkLogger
-
-import EotGlobalData
-
-# Global definiton
-gFfsPrintTitle  = "%-36s  %-21s %8s %8s %8s  %-4s %-36s" % ("GUID", "TYPE", 
"OFFSET", "SIZE", "FREE", "ALIGN", "NAME")
-gFfsPrintFormat = "%36s  %-21s %8X %8X %8X  %4s %-36s"
-gGuidStringFormat = "%08X-%04X-%04X-%02X%02X-%02X%02X%02X%02X%02X%02X"
-gPeiAprioriFileNameGuid = '1b45cc0a-156a-428a-af62-49864da0e6e6'
-gAprioriGuid = 'fc510ee7-ffdc-11d4-bd41-0080c73c8881'
-gIndention = -4
-
-## Image() class
-#
-#  A class for Image
-#
-class Image(array):
-_HEADER_ = struct.Struct("")
-_HEADER_SIZE_ = _HEADER_.size
-
-def __new__(cls, *args, **kwargs):
-return array.__new__(cls, 'B')
-
-def __init__(m, ID=None):
-if ID is None:
-m._ID_ = str(uuid.uuid1()).upper()
-else:
-m._ID_ = ID
-m._BUF_ = None
-m._LEN_ = None
-m._OFF_ = None
-
-m._SubImages = sdict() # {offset: Image()}
-
-array.__init__(m, 'B')
-
-def __repr__(m):
-return m._ID_
-
-def __len__(m):
-Len = array.__len__(m)
-for Offset in m._SubImages:
-Len += len(m._SubImages[Offset])
-return Len
-
-def _Unpack(m):
-m.extend(m._BUF_[m._OFF_ : m._OFF_ + m._LEN_])
-return len(m)
-
-def _Pack(m, PadByte=0xFF):
-raise NotImplementedError
-
-def frombuffer(m, Buffer, Offset=0, Size=None):
-m._BUF_ = Buffer
-m._OFF_ = Offset
-# we may need the Size information in advance 

Re: [edk2] Source code debugging of OVMF

[Palmer, Thomas]

I use Andrew WIP's stuff often as well.  Just put the GdbSyms.inf in your DSC 
(not FDF) and build.  My .gdbinit script looks like this:

set architecture i386:x86-64:intel
target remote localhost:1234
source DebugPkg/Scripts/gdb_uefi.py
reload-uefi -o 
Build/OvmfX64/DEBUG_GCC5/X64/DebugPkg/GdbSyms/GdbSyms/DEBUG/GdbSyms.dll


Andrew, I owe you a couple of beers if we ever meet.



Regards,

Thomas Palmer

"I have only made this letter longer because I have not had the time to make it 
shorter" - Blaise Pascal


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
Ersek
Sent: Monday, April 16, 2018 11:14 AM
To: Johannes Swoboda 
Cc: Michael Kinney ; edk2-devel@lists.01.org; 
clemens.hlausc...@inso.tuwien.ac.at
Subject: Re: [edk2] Source code debugging of OVMF

Hi Johannes,

On 04/16/18 13:09, Johannes Swoboda wrote:

> How are you ovmf developer debugging it?

In general I add DEBUG statements, grep the tree for protocol / PPI GUIDs, and 
use an editor with good ctags support.

Here's another thread that you might find useful:

http://edk2-devel.narkive.com/6BRVus92/qestion-about-how-to-debug-ovmf-on-qemu

Occasionally I do use gdb with QEMU, but the solution I use is not suitable for 
debugging modules in the SEC and PEI phases, only in DXE. I have some terribly 
rough patches in my local tree that are based on 
.

I don't recall having any luck with SOURCE_DEBUG_ENABLE and the UDK debugger 
,
but admittedly it's been a while (= years?) since I last tried to connect 
debugger VM with debugge VM over a virtual serial port. Others have more 
recently confirmed it works for them. I think Mike uses it successfully, from a 
Windows debugger machine maybe?

Thanks,
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Source code debugging of OVMF

[Laszlo Ersek]

Hi Johannes,

On 04/16/18 13:09, Johannes Swoboda wrote:

> How are you ovmf developer debugging it?

In general I add DEBUG statements, grep the tree for protocol / PPI
GUIDs, and use an editor with good ctags support.

Here's another thread that you might find useful:

http://edk2-devel.narkive.com/6BRVus92/qestion-about-how-to-debug-ovmf-on-qemu

Occasionally I do use gdb with QEMU, but the solution I use is not
suitable for debugging modules in the SEC and PEI phases, only in DXE. I
have some terribly rough patches in my local tree that are based on
.

I don't recall having any luck with SOURCE_DEBUG_ENABLE and the UDK
debugger
,
but admittedly it's been a while (= years?) since I last tried to
connect debugger VM with debugge VM over a virtual serial port. Others
have more recently confirmed it works for them. I think Mike uses it
successfully, from a Windows debugger machine maybe?

Thanks,
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Source code debugging of OVMF

[Blibbet]

On 04/16/2018 08:26 AM, Richardson, Brian wrote:
>
https://github.com/tianocore/tianocore.github.io/wiki/How-to-debug-OVMF-with-QEMU-using-GDB


Also useful for this topic:

https://github.com/tianocore/tianocore.github.io/wiki/Testing-SMM-with-QEMU,-KVM-and-libvirt

HTH,
Lee
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] How-to-run-OVMF wiki page links to outdated sf files area

[Richardson, Brian]

Thanks, I have corrected the links. Please let me know if there are any 
additional issues ... br
---
Brian Richardson, Senior Technical Marketing Engineer, Intel Software
brian.richard...@intel.com -- @intel_brian (Twitter & WeChat)
https://software.intel.com/en-us/meet-the-developers/evangelists/team/brian-richardson
 


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Rebecca 
Cran
Sent: Monday, April 16, 2018 11:31 AM
To: edk2-devel@lists.01.org
Subject: [edk2] How-to-run-OVMF wiki page links to outdated sf files area

There's an outdated link on
https://github.com/tianocore/tianocore.github.io/wiki/How-to-run-OVMF :
the "OVMF downloads area" link points to 
http://sourceforge.net/projects/edk2/files/OVMF, which was last updated in 2014.

--
Rebecca Cran
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] TianoCore UEFI Development Kit build: how to cause the build process to correctly recognize the build platform (Linux)?

[Richardson, Brian]

I recommend you try building MdePkg as a test under any Linux environment. 
NT32Pkg is designed for Windows environments.

https://github.com/tianocore/tianocore.github.io/wiki/UDK2018-How-to-Build#how-to-build-linux-like-system
 

Thanks . br
---
Brian Richardson, Senior Technical Marketing Engineer, Intel Software
brian.richard...@intel.com -- @intel_brian (Twitter & WeChat)
https://software.intel.com/en-us/meet-the-developers/evangelists/team/brian-richardson
 


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Marvin 
Häuser
Sent: Sunday, April 15, 2018 11:31 AM
To: edk2-devel@lists.01.org
Subject: Re: [edk2] TianoCore UEFI Development Kit build: how to cause the 
build process to correctly recognize the build platform (Linux)?

Hey,

As you have found out, Nt32Pkg is for Windows and shouldn't be built on Linux.
You can specify the platform descriptor file to build via the "-p " 
parameter.

Regards,
Marvin.

> -Ursprüngliche Nachricht-
> Von: edk2-devel  Im Auftrag von 
> Aleksey Shevandin
> Gesendet: Sonntag, 15. April 2018 17:23
> An: edk2-devel@lists.01.org
> Betreff: [edk2] TianoCore UEFI Development Kit build: how to cause the 
> build process to correctly recognize the build platform (Linux)?
> 
> Dear members,
> 
> I'm trying to build *UDK2018* on *Ubuntu 17*. After studying the 
> documentation, I had impressed that the platform setup script
> (*sedksetup.sh*) shall configure the build framework to target the 
> correct build platform, the tool chain etc. Unfortunately this is not 
> what actually happens.
> 
> The platform build process (the Build base tool) unexpectedly tries to 
> build some *MS Windows* oriented stuff and fails. How this can be fixed?
> 
> Following the documented recommendations, at the first stage I build 
> the "Base Tools":
> 
> |/make all -C ${EDK_TOOLS_PATH}/|
> 
> Then I run the setup script:
> 
> |/edksetup.sh BaseTools/|
> 
> This stages are finished with success, also the setup script runs some 
> tests that successfully pass.
> 
> On the next stage I'm trying to build the platform:
> 
> |build all -a X64 -t GCC5|
> 
> This last stage fails with the follow error:
> 
> Nt32Pkg/Include/WinNtPeim.h:27:10: fatal error: Common/WinNtInclude.h:
> No such file or directory
> 
> 
> Regards,
> 
> Aleksey
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] How-to-run-OVMF wiki page links to outdated sf files area

[Rebecca Cran]

There's an outdated link on
https://github.com/tianocore/tianocore.github.io/wiki/How-to-run-OVMF :
the "OVMF downloads area" link points to
http://sourceforge.net/projects/edk2/files/OVMF, which was last updated
in 2014.

-- 
Rebecca Cran
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Source code debugging of OVMF

[Richardson, Brian]

This page was recently added to the TianoCore wiki. Please let me know if this 
is useful info:
https://github.com/tianocore/tianocore.github.io/wiki/How-to-debug-OVMF-with-QEMU-using-GDB
 

Thanks ... br
---
Brian Richardson, Senior Technical Marketing Engineer, Intel Software
brian.richard...@intel.com -- @intel_brian (Twitter & WeChat)
https://software.intel.com/en-us/meet-the-developers/evangelists/team/brian-richardson
 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Johannes 
Swoboda
Sent: Monday, April 16, 2018 7:09 AM
To: edk2-devel@lists.01.org
Cc: clemens.hlausc...@inso.tuwien.ac.at
Subject: [edk2] Source code debugging of OVMF

Hello everyone,

I'm doing a research project on efi security. I would like to do source code 
debugging of OVMF. I want to step through the OVMF source code, set break 
points, et cetera, preferably with gdb. I want to debug the overall boot 
process. Is that possible?

I understand I can start qemu with the options -s -S. This gives me a virtual 
machine that awaits connection of a gdb debugger and further instructions.

After connecting with gdb, I can instruct the machine to continue execution. 
Unsurprisingly, there is nothing else I can do, because gdb lacks the relevant 
symbols.
I tried to read in the OVMF.fd file, as i would do with a binary that I would 
debug, but gdb can't handle the file.
It seems to be possible to load an efi app with the file command, but not this 
one.

I found one other person trying to achieve the same five years ago. [3] 
suggests that something like this
> (gdb) add-symbol-file ../edk2/Build/OvmfX64/DEBUG_GCC5/X64/Shell.debug
> The address where ../edk2/Build/OvmfX64/DEBUG_GCC5/X64/Shell.debug has 
> been loaded is missing
might be possible. However, it appears to me this is may contain information 
regarding Shell.efi, an efi-app; but I don't want to debug an app. I want to 
debug the overall boot process.

I'm quoting some terminal output to clarify what I'm trying to achieve:
[johannes@johannes-laptop OVMF_efi_hello_world]$ gdb [...]
(gdb) file OVMF.fd
"/home/johannes/18S/bakk/uefi_virtual/OVMF_efi_hello_world/OVMF.fd": not in 
executable format: File format not recognized
(gdb) target remote localhost:1234
Remote debugging using localhost:1234
warning: No executable has been specified and target does not support 
determining executable automatically.  Try using the "file" command.
0xfff0 in ?? ()
(gdb) step
Cannot find bounds of current function
(gdb) list
No symbol table is loaded.  Use the "file" command.
(gdb) continue
Continuing.
[ovmf loads in qemu window]

I can redirect ovmf debug messages into a text file[1].
As far as I understand, there is a way to do source code level debugging
*with* ovmf, to debug efi-apps that are e.g. run via the efi shell[2], with the 
help of SourceLevelDebugPkg[4], but this package cannot be used to source level 
debug the overall boot process, right?

Is it possible to do source level debugging of ovmf?
Is it possible to step through ovmf, one instruction or function call at a time?
How are you ovmf developer debugging it?

If you could point me to the right direction, that would be great.

Kind regards,
Johannes

[1]: I managed to do so via the qemu options -global
isa-debugcon.iobase=0x402 -debugcon file:qemu.ovmf.log" as demonstrated here 
https://www.linux-kvm.org/downloads/lersek/ovmf-whitepaper-c770f8c.txt
[2]:https://github.com/tianocore/tianocore.github.io/wiki/OVMF-FAQ#how-do-i-enable-source-level-debugging-with-ovmf
[3]: 
http://edk2-devel.narkive.com/LRWe2mSQ/using-gdb-on-ovmf-with-symbols
[4]: 
https://github.com/tianocore/tianocore.github.io/wiki/SourceLevelDebugPkg
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms] Silicon/Socionext/SynQuacer: update PHY reference clock rate

[Masahisa Kojima]

Hi Ard,

> Please confirm that the modification to ogma_config.h is correct.

Thank you very much for your update.
We confirmed and your modification is correct.

Regards,
Masahisa

On 16 April 2018 at 20:00, Ard Biesheuvel  wrote:
> As reported by Kojima-san, the PHY reference clock value we use in our
> ACPI and DT descriptions is out of sync with the hardware. Replace
> 125 MHz with 250 MHz throughout.
>
> Cc: Masahisa Kojima 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
> Kojima-san,
>
> Please confirm that the modification to ogma_config.h is correct.
>
> Thanks,
> Ard.
>
>  Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl  
>| 2 +-
>  Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
>| 4 ++--
>  
> Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/ogma_config.h
>  | 4 ++--
>  3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl 
> b/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl
> index b6f6c4360029..3f73c191d4d6 100644
> --- a/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl
> +++ b/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl
> @@ -162,7 +162,7 @@ DefinitionBlock ("DsdtTable.aml", "DSDT", 1, "SNI", 
> "SYNQUACR",
>Package (2) { "phy-channel", FixedPcdGet32 (PcdNetsecPhyAddress) },
>Package (2) { "max-speed", 1000 },
>Package (2) { "max-frame-size", 9000 },
> -  Package (2) { "socionext,phy-clock-frequency", 12500 },
> +  Package (2) { "socionext,phy-clock-frequency", 25000 },
>  }
>})
>  }
> diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi 
> b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
> index 6e93c6ae16a8..f6887329f6c7 100644
> --- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
> +++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
> @@ -420,9 +420,9 @@
>  reg-shift = <2>;
>  };
>
> -clk_netsec: refclk125mhz {
> +clk_netsec: refclk250mhz {
>  compatible = "fixed-clock";
> -clock-frequency = <12500>;
> +clock-frequency = <25000>;
>  #clock-cells = <0>;
>  };
>
> diff --git 
> a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/ogma_config.h
>  
> b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/ogma_config.h
> index 1caf64e30623..f6ec9b30ec8e 100644
> --- 
> a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/ogma_config.h
> +++ 
> b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/ogma_config.h
> @@ -16,8 +16,8 @@
>  #ifndef OGMA_CONFIG_H
>  #define OGMA_CONFIG_H
>
> -#define OGMA_CONFIG_CLK_HZ 12500UL
> -#define OGMA_CONFIG_GMAC_CLK_HZ 12500UL
> +#define OGMA_CONFIG_CLK_HZ 25000UL
> +#define OGMA_CONFIG_GMAC_CLK_HZ 25000UL
>  #define OGMA_CONFIG_CHECK_CLK_SUPPLY
>
>  #define OGMA_CONFIG_USE_READ_GMAC_STAT
> --
> 2.17.0
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v1 08/18] StandaloneMmPkg/MemLib: AARCH64 Specific instance of memory check library.

[Achin Gupta]

Hi Supreeth,

On Fri, Apr 06, 2018 at 03:42:13PM +0100, Supreeth Venkatesh wrote:
> MM memory check library library implementation. This library consumes
> MM_ACCESS_PROTOCOL to get MMRAM information. In order to use this
> library instance, the platform should produce all MMRAM range via
> MM_ACCESS_PROTOCOL, including the range for firmware (like MM Core
> and MM driver) and/or specific dedicated hardware.
> 
> This patch provides services for MM Memory Operation.
> The management mode Mem Library provides function for checking if buffer
> is outside MMRAM and valid. It also provides functions for copy data
> from MMRAM to non-MMRAM, from non-MMRAM to MMRAM,
> from non-MMRAM to non-MMRAM, or set data in non-MMRAM.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Achin Gupta 
> Signed-off-by: Supreeth Venkatesh 
> ---
>  StandaloneMmPkg/Include/Library/MemLib.h| 140 ++
>  StandaloneMmPkg/Library/MemLib/Arm/MemLib.c | 276 
> 

Why is this Library Arm specific. Apart from cosmetics tweaks, it has not
changed since it was originally contributed?

cheers,
Achin

>  StandaloneMmPkg/Library/MemLib/MemLib.inf   |  47 +
>  3 files changed, 463 insertions(+)
>  create mode 100644 StandaloneMmPkg/Include/Library/MemLib.h
>  create mode 100644 StandaloneMmPkg/Library/MemLib/Arm/MemLib.c
>  create mode 100644 StandaloneMmPkg/Library/MemLib/MemLib.inf
> 
> diff --git a/StandaloneMmPkg/Include/Library/MemLib.h 
> b/StandaloneMmPkg/Include/Library/MemLib.h
> new file mode 100644
> index 00..3264f10010
> --- /dev/null
> +++ b/StandaloneMmPkg/Include/Library/MemLib.h
> @@ -0,0 +1,140 @@
> +/** @file
> +  Provides services for MM Memory Operation.
> +
> +  The MM Mem Library provides function for checking if buffer is outside 
> MMRAM and valid.
> +  It also provides functions for copy data from MMRAM to non-MMRAM, from 
> non-MMRAM to MMRAM,
> +  from non-MMRAM to non-MMRAM, or set data in non-MMRAM.
> +
> +  Copyright (c) 2015, Intel Corporation. All rights reserved.
> +  Copyright (c) 2016 - 2017, ARM Limited. All rights reserved.
> +
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of the BSD 
> License
> +  which accompanies this distribution.  The full text of the license may be 
> found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +
> +**/
> +
> +#ifndef _MM_MEM_LIB_H_
> +#define _MM_MEM_LIB_H_
> +
> +/**
> +  This function check if the buffer is valid per processor architecture and 
> not overlap with MMRAM.
> +
> +  @param Buffer  The buffer start address to be checked.
> +  @param Length  The buffer length to be checked.
> +
> +  @retval TRUE  This buffer is valid per processor architecture and not 
> overlap with MMRAM.
> +  @retval FALSE This buffer is not valid per processor architecture or 
> overlap with MMRAM.
> +**/
> +BOOLEAN
> +EFIAPI
> +MmIsBufferOutsideMmValid (
> +  IN EFI_PHYSICAL_ADDRESS  Buffer,
> +  IN UINT64Length
> +  );
> +
> +/**
> +  Copies a source buffer (non-MMRAM) to a destination buffer (MMRAM).
> +
> +  This function copies a source buffer (non-MMRAM) to a destination buffer 
> (MMRAM).
> +  It checks if source buffer is valid per processor architecture and not 
> overlap with MMRAM.
> +  If the check passes, it copies memory and returns EFI_SUCCESS.
> +  If the check fails, it return EFI_SECURITY_VIOLATION.
> +  The implementation must be reentrant.
> +
> +  @param  DestinationBuffer   The pointer to the destination buffer of the 
> memory copy.
> +  @param  SourceBufferThe pointer to the source buffer of the memory 
> copy.
> +  @param  Length  The number of bytes to copy from SourceBuffer 
> to DestinationBuffer.
> +
> +  @retval EFI_SECURITY_VIOLATION The SourceBuffer is invalid per processor 
> architecture or overlap with MMRAM.
> +  @retval EFI_SUCCESSMemory is copied.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +MmCopyMemToSmram (
> +  OUT VOID   *DestinationBuffer,
> +  IN CONST VOID  *SourceBuffer,
> +  IN UINTN   Length
> +  );
> +
> +/**
> +  Copies a source buffer (MMRAM) to a destination buffer (NON-MMRAM).
> +
> +  This function copies a source buffer (non-MMRAM) to a destination buffer 
> (MMRAM).
> +  It checks if destination buffer is valid per processor architecture and 
> not overlap with MMRAM.
> +  If the check passes, it copies memory and returns EFI_SUCCESS.
> +  If the check fails, it returns EFI_SECURITY_VIOLATION.
> +  The implementation must be reentrant.
> +
> +  @param  DestinationBuffer   The pointer to the destination buffer of the 
> memory copy.
> +  @param  SourceBufferThe pointer to the source buffer of the memory 
> copy.

Re: [edk2] [PATCH v1 07/18] StandaloneMmPkg/FvLib: Add a common FV Library for management mode.

[Achin Gupta]

Hi Supreeth,

On Fri, Apr 06, 2018 at 03:42:12PM +0100, Supreeth Venkatesh wrote:
> This patch implements a firmware volume library that can be used by the
> Standalone management mode core module to parse the firmware volume.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Achin Gupta 

I have not contributed to this patch at all. Could you please remove me? 

Not being an expert, I will wait for Jiewen's feedback cycle to complete.

cheers,
Achin

> Signed-off-by: Supreeth Venkatesh 
> ---
>  StandaloneMmPkg/Include/Library/FvLib.h | 109 ++
>  StandaloneMmPkg/Library/FvLib/FvLib.c   | 366 
> 
>  StandaloneMmPkg/Library/FvLib/FvLib.inf |  57 +
>  3 files changed, 532 insertions(+)
>  create mode 100644 StandaloneMmPkg/Include/Library/FvLib.h
>  create mode 100644 StandaloneMmPkg/Library/FvLib/FvLib.c
>  create mode 100644 StandaloneMmPkg/Library/FvLib/FvLib.inf
> 
> diff --git a/StandaloneMmPkg/Include/Library/FvLib.h 
> b/StandaloneMmPkg/Include/Library/FvLib.h
> new file mode 100644
> index 00..13e1ae2b01
> --- /dev/null
> +++ b/StandaloneMmPkg/Include/Library/FvLib.h
> @@ -0,0 +1,109 @@
> +/** @file
> +
> +Copyright (c) 2015, Intel Corporation. All rights reserved.
> +Copyright (c) 2016 - 2017, ARM Limited. All rights reserved.
> +
> +This program and the accompanying materials
> +are licensed and made available under the terms and conditions of the BSD 
> License
> +which accompanies this distribution.  The full text of the license may be 
> found at
> +http://opensource.org/licenses/bsd-license.php
> +
> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#ifndef _FV_LIB_H_
> +#define _FV_LIB_H_
> +
> +#include 
> +#include 
> +#include 
> +
> +/**
> +  Given the input file pointer, search for the next matching file in the
> +  FFS volume as defined by SearchType. The search starts from FileHeader 
> inside
> +  the Firmware Volume defined by FwVolHeader.
> +
> +  @param  SearchType  Filter to find only files of this type.
> +  Type EFI_FV_FILETYPE_ALL causes no filtering to be 
> done.
> +  @param  FwVolHeader Pointer to the FV header of the volume to search.
> +  This parameter must point to a valid FFS volume.
> +  @param  FileHeader  Pointer to the current file from which to begin 
> searching.
> +  This pointer will be updated upon return to reflect 
> the file found.
> +
> +  @retval EFI_NOT_FOUND  No files matching the search criteria were found
> +  @retval EFI_SUCCESS
> +**/
> +EFI_STATUS
> +EFIAPI
> +FfsFindNextFile (
> +  IN EFI_FV_FILETYPE SearchType,
> +  IN EFI_FIRMWARE_VOLUME_HEADER  *FwVolHeader,
> +  IN OUT EFI_FFS_FILE_HEADER **FileHeader
> +  );
> +
> +/**
> +  Given the input file pointer, search for the next matching section in the
> +  FFS volume.
> +
> +  @param  SearchTypeFilter to find only sections of this type.
> +  @param  FfsFileHeader Pointer to the current file to search.
> +  @param  SectionHeader Pointer to the Section matching SectionType in 
> FfsFileHeader.
> +NULL if section not found
> +
> +  @retval  EFI_NOT_FOUND  No files matching the search criteria were found
> +  @retval  EFI_SUCCESS
> +**/
> +EFI_STATUS
> +FfsFindSection (
> +  IN EFI_SECTION_TYPE  SectionType,
> +  IN EFI_FFS_FILE_HEADER   *FfsFileHeader,
> +  IN OUT EFI_COMMON_SECTION_HEADER **SectionHeader
> +  );
> +
> +/**
> +  Locates a section within a series of sections
> +  with the specified section type.
> +
> +  @param[in]   SectionsThe sections to search
> +  @param[in]   SizeOfSections  Total size of all sections
> +  @param[in]   SectionType The section type to locate
> +  @param[out]  FoundSectionThe FFS section if found
> +
> +  @retval EFI_SUCCESS   The file and section was found
> +  @retval EFI_NOT_FOUND The file and section was not found
> +  @retval EFI_VOLUME_CORRUPTED  The firmware volume was corrupted
> +**/
> +EFI_STATUS
> +EFIAPI
> +FindFfsSectionInSections (
> +  IN  VOID *Sections,
> +  IN  UINTNSizeOfSections,
> +  IN  EFI_SECTION_TYPE SectionType,
> +  OUT EFI_COMMON_SECTION_HEADER**FoundSection
> +  );
> +
> +/**
> +  Given the input file pointer, search for the next matching section in the
> +  FFS volume.
> +
> +  @param  SearchType  Filter to find only sections of this type.
> +  @param  FfsFileHeader   Pointer to the current file to search.
> +  @param  SectionData Pointer to the Section matching SectionType in 
> FfsFileHeader.
> +  NULL if section not found
> +  @param  SectionDataSize The size of SectionData
> +
> +  @retval  EFI_NOT_FOUND  No files matching 

Re: [edk2] [PATCH] MdePkg: add big-endian MMIO BaseBeIoLib

[Michael Brown]

On 16/04/18 15:10, Kinney, Michael D wrote:

I agree that the opposite use case is a BE CPU
needing a LE operation.

I think we only need a single lib class and lib
Instance that does the byte swap and we should
not use Le or Be in any of the names of the class,
instance, or APIs.  Just "Swap".


I may have misunderstood, but wouldn't using "Swap" within the API names 
effectively encode knowledge of the endianness of the _build_ platform 
into the source code?  This would prevent the same source code being 
built for both little-endian and big-endian CPUs.


Michael
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdePkg: add big-endian MMIO BaseBeIoLib

[Kinney, Michael D]

Leif,

I agree that the opposite use case is a BE CPU
needing a LE operation. 

I think we only need a single lib class and lib
Instance that does the byte swap and we should
not use Le or Be in any of the names of the class,
instance, or APIs.  Just "Swap".

The 4 cases

* LE CPU, LE I/O.  Use BaseIoLib
* BE CPU, BE I/O.  Use BaseIoLib
* LE CPU, BE I/O.  Use BaseIoSwapLib
* BE CPU, LE I/O.  Use BaseIoSwapLib

Mike


> -Original Message-
> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> Sent: Monday, April 16, 2018 3:07 AM
> To: Kinney, Michael D 
> Cc: edk2-devel@lists.01.org; Laszlo Ersek
> ; Gao, Liming 
> Subject: Re: [edk2] [PATCH] MdePkg: add big-endian MMIO
> BaseBeIoLib
> 
> On Fri, Apr 13, 2018 at 11:32:35PM +, Kinney,
> Michael D wrote:
> > Leif,
> >
> > I am curious why a Swap class/instances is not
> sufficient.
> >
> > Currently EDK II follows the UEFI/PI specs, which for
> > all supported CPU architectures use little endian
> ABI.
> > The BaseIoLib follows the endianness of the CPU.  If
> > UEFI/PI added a CPU that was big endian, I would
> expect
> > BaseIoLib when built for that CPU would perform big
> endian
> > operations.
> >
> > Am I missing something?
> 
> If you did add a big-endian CPU, you could then find
> yourself in the
> exact opposite situation and require a little-endian
> i/o access
> library. Which would be implemented exactly as the
> contents of
> IoLibSwap.c.
> 
> The header file necessarily needs to be endianness-
> specific, and if
> the coding style had permitted functions in header
> files, my automatic
> reaction would have been to make all of these static
> inline helper
> functions (even with the code duplication).
> 
> /
> Leif
> 
> > Mike
> >
> >
> > > -Original Message-
> > > From: edk2-devel [mailto:edk2-devel-
> > > boun...@lists.01.org] On Behalf Of Leif Lindholm
> > > Sent: Friday, April 13, 2018 12:32 PM
> > > To: Kinney, Michael D 
> > > Cc: edk2-devel@lists.01.org; Laszlo Ersek
> > > ; Gao, Liming
> 
> > > Subject: Re: [edk2] [PATCH] MdePkg: add big-endian
> MMIO
> > > BaseBeIoLib
> > >
> > > On Fri, Apr 13, 2018 at 07:24:06PM +, Kinney,
> > > Michael D wrote:
> > > > Hi Leif,
> > > >
> > > > I think we need to look at the names.  I see a
> mix of
> > > > "Be" and "Swap".  We should pick one and use it
> > > > consistently.
> > >
> > > This was what I meant by the comments:
> > > ---
> > > This modified version introduces a single BeIoLib
> > > instance, backed by
> > > a source-file that could be used also for a
> > > hypothetical LeIoLib.
> > > There is no LeIoLib.h included though.
> > >
> > > While this is arguably overengineered, I do feel
> > > reasonably strongly
> > > that code should be named for what it does, not for
> how
> > > it is used,
> > > and doing it this way lets me follow that rule.
> > > ---
> > >
> > > Clearly this is open for discussion, but the above
> is
> > > my opinion and
> > > the code intentionally reflects that.
> > >
> > > Regards,
> > >
> > > Leif
> > >
> > > > Mike
> > > >
> > > > > -Original Message-
> > > > > From: Leif Lindholm
> > > [mailto:leif.lindh...@linaro.org]
> > > > > Sent: Friday, April 13, 2018 10:42 AM
> > > > > To: edk2-devel@lists.01.org
> > > > > Cc: Kinney, Michael D
> ;
> > > > > Gao, Liming ; Laszlo
> Ersek
> > > > > ; udit.ku...@nxp.com
> > > > > Subject: [PATCH] MdePkg: add big-endian MMIO
> > > > > BaseBeIoLib
> > > > >
> > > > > When performing MMIO to a destination of the
> > > opposite
> > > > > endianness to the
> > > > > executing processor, this library provides
> > > automatic
> > > > > byte order reversal
> > > > > on inputs and outputs.
> > > > >
> > > > > Contributed-under: TianoCore Contribution
> Agreement
> > > 1.1
> > > > > Signed-off-by: Leif Lindholm
> > > 
> > > > > ---
> > > > >
> > > > > Udit, many apologies for this dragging out -
> back-
> > > to-
> > > > > back conferences,
> > > > > holidays, and lots of catching up.
> > > > >
> > > > > This modified version introduces a single
> BeIoLib
> > > > > instance, backed by
> > > > > a source-file that could be used also for a
> > > > > hypothetical LeIoLib.
> > > > > There is no LeIoLib.h included though.
> > > > >
> > > > > While this is arguably overengineered, I do
> feel
> > > > > reasonably strongly
> > > > > that code should be named for what it does, not
> for
> > > how
> > > > > it is used,
> > > > > and doing it this way lets me follow that rule.
> > > > >
> > > > > I have not duplicated the .uni file together
> with
> > > the
> > > > > .inf, since
> > > > > this follows what is done in
> BaseIoLibIntrinsic.
> > > > >
> > > > >  MdePkg/Include/Library/BeIoLib.h
> |
> > > 376
> > > > > +++
> > > > >  

Re: [edk2] [PATCH v1 06/18] StandaloneMmPkg: Add an AArch64 specific entry point library.

[Achin Gupta]

Hi Supreeth,

On Fri, Apr 06, 2018 at 03:42:11PM +0100, Supreeth Venkatesh wrote:
> The Standalone MM environment runs in S-EL0 in AArch64 on ARM Standard
> Platforms and is initialised during the SEC phase. ARM Trusted firmware
> in EL3 is responsible for initialising the architectural context for
> S-EL0 and loading the Standalone MM image. The memory allocated to this
> image is marked as RO+X. Heap memory is marked as RW+XN.
> 
> Certain actions have to be completed prior to executing the generic code
> in the Standalone MM Core module. These are:
> 
> 1. Memory permission attributes for each section of the Standalone MM
>Core module need to be changed prior to accessing any RW data.
> 
> 2. A Hob list has to be created with information that allows the MM
>environment to initialise and dispatch drivers.
> 
> Furthermore, this module is responsible for handing over runtime MM
> events to the Standalone MM CPU driver and returning control to ARM
> Trusted Firmware upon event completion. Hence it needs to know the CPU
> driver entry point.
> 
> This patch implements an entry point module that ARM Trusted Firmware
> jumps to in S-EL0. It then performs the above actions before calling the
> Standalone MM Foundation entry point and handling subsequent MM events.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Achin Gupta 
> Signed-off-by: Supreeth Venkatesh 
> ---
>  .../Library/Arm/StandaloneMmCoreEntryPoint.h   | 232 +
>  .../Include/Library/MmCoreStandaloneEntryPoint.h   | 101 
>  .../StandaloneMmCoreEntryPoint/Arm/CreateHobList.c | 200 +++
>  .../Arm/SetPermissions.c   | 278 
> +
>  .../Arm/StandaloneMmCoreEntryPoint.c   | 264 +++
>  .../StandaloneMmCoreEntryPoint.inf |  53 
>  StandaloneMmPkg => StandaloneMmPkg~HEAD|   0
>  7 files changed, 1128 insertions(+)
>  create mode 100644 
> StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h

The names of this file and the one below are re-arrangements of the same
words. This is confusing. It would be better to stick to one convention. Since
the name of the package starts with StandaloneMm, everything else should follow
suit.

So can this file be renamed to ArmStandaloneMmCoreEntryPoint.h and the one below
to StandaloneMmCoreEntryPoint.h unless you think this can be done differently?

>  create mode 100644 
> StandaloneMmPkg/Include/Library/MmCoreStandaloneEntryPoint.h



>  create mode 100644 
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
>  create mode 100644 
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/SetPermissions.c

PeCoffExtraActionLib and the HobLib are pre-requisites for these two
files. Should they not appear first in the patch stack?

>  create mode 100644 
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
>  create mode 100644 
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf

Can this and the tagged file above go in a separate commit? Cannot see how they
are related to the Arm specific entry point.

>  rename StandaloneMmPkg => StandaloneMmPkg~HEAD (100%)
> 
> diff --git a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h 
> b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
> new file mode 100644
> index 00..029c6c476c
> --- /dev/null
> +++ b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
> @@ -0,0 +1,232 @@
> +/** @file
> +  Entry point to the Standalone MM Foundation when initialised during the SEC
> +  phase on ARM platforms
> +
> +Copyright (c) 2017, ARM Ltd. All rights reserved.

Copyright year needs to be updated.

> +This program and the accompanying materials
> +are licensed and made available under the terms and conditions of the BSD 
> License
> +which accompanies this distribution.  The full text of the license may be 
> found at
> +http://opensource.org/licenses/bsd-license.php
> +
> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#ifndef __MODULE_ENTRY_POINT_H__
> +#define __MODULE_ENTRY_POINT_H__

Name of this define needs to be changed

> +
> +#include 
> +#include 
> +
> +#define CPU_INFO_FLAG_PRIMARY_CPU  0x0001
> +
> +typedef
> +EFI_STATUS
> +(*PI_MM_CPU_TP_FW_ENTRYPOINT) (
> +  IN UINTN EventId,
> +  IN UINTN CpuNumber,
> +  IN UINTN NsCommBufferAddr
> +  );

This typedef is not being used.

> +
> +typedef struct {
> +  UINT8  Type;   /* type of the structure */
> +  UINT8  Version;/* version of this structure */
> +  UINT16 Size;  /* size of this structure in bytes */
> +  UINT32 Attr;  /* attributes: unused bits SBZ */
> +} EFI_PARAM_HEADER;
> +
> +typedef struct {
> +  UINT64 

Re: [edk2] [PATCH edk2-platforms 00/12] Hisilicon/D0x: Switch to generic PciHostBridge

[Guo Heyi]

Thanks, I will test mm command and let you know the result.

Regards,

Heyi

On Fri, Apr 13, 2018 at 09:19:53AM +0200, Ard Biesheuvel wrote:
> On 13 April 2018 at 04:05, Guo Heyi  wrote:
> > Hi Ard,
> >
> > Any comments?
> >
> 
> Apologies for the delay. I have been travelling and am behind on email.
> 
> > Anyway we can modify the code if you insist on using an intermediate CPU IO
> > address space.
> >
> 
> I have not made up my mind yet, to be honest. I agree there is a
> certain elegance to merging both translations, but I am concerned that
> existing EDK2 code may deal poorly with I/O addresses that require
> more than 32 bits to express.
> 
> Did you try the mm command in the shell for instance? As you know, I
> recently removed an artificial address range limit there, but I wonder
> if it uses 64-bit variables for I/O ports.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 2/3] BaseTools: use predefined constants instead of local strings

[Jaben Carsey]

fixed an incorrect import found in Yonghong's review.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/AutoGen/AutoGen.py |  24 +--
 BaseTools/Source/Python/AutoGen/BuildEngine.py |  23 +--
 BaseTools/Source/Python/AutoGen/GenPcdDb.py|  15 +-
 BaseTools/Source/Python/AutoGen/GenVar.py  |  15 +-
 BaseTools/Source/Python/Common/DataType.py |   2 +
 BaseTools/Source/Python/GenFds/AprioriSection.py   |   3 +-
 BaseTools/Source/Python/GenFds/FdfParser.py|  29 ++--
 BaseTools/Source/Python/GenFds/FfsInfStatement.py  |   5 +-
 BaseTools/Source/Python/GenFds/GenFds.py   |  13 +-
 BaseTools/Source/Python/GenFds/GenFdsGlobalVariable.py |   9 +-
 BaseTools/Source/Python/GenFds/Section.py  |   3 +-
 BaseTools/Source/Python/Workspace/DecBuildData.py  |   5 +-
 BaseTools/Source/Python/Workspace/DscBuildData.py  | 154 
++--
 BaseTools/Source/Python/Workspace/InfBuildData.py  |   7 +-
 BaseTools/Source/Python/Workspace/MetaFileParser.py|  34 ++---
 BaseTools/Source/Python/Workspace/MetaFileTable.py |  22 +--
 BaseTools/Source/Python/Workspace/WorkspaceDatabase.py |   6 +-
 17 files changed, 196 insertions(+), 173 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 3384fdb70b7e..abdddec5ed6a 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -45,6 +45,8 @@ import InfSectionParser
 import datetime
 import hashlib
 from GenVar import VariableMgr,var_info
+from Common.DataType import TAB_DEFAULT
+from Common.DataType import TAB_COMMON
 
 ## Regular expression for splitting Dependency Expression string into tokens
 gDepexTokenPattern = re.compile("(\(|\)|\w+| \S+\.inf)")
@@ -253,7 +255,7 @@ class WorkspaceAutoGen(AutoGen):
 self.BuildDatabase  = MetaFileDb
 self.MetaFile   = ActivePlatform
 self.WorkspaceDir   = WorkspaceDir
-self.Platform   = self.BuildDatabase[self.MetaFile, 'COMMON', 
Target, Toolchain]
+self.Platform   = self.BuildDatabase[self.MetaFile, TAB_COMMON, 
Target, Toolchain]
 GlobalData.gActivePlatform = self.Platform
 self.BuildTarget= Target
 self.ToolChain  = Toolchain
@@ -803,7 +805,7 @@ class WorkspaceAutoGen(AutoGen):
 # Here we just need to get FILE_GUID from INF 
file, use 'COMMON' as ARCH attribute. and use 
 # BuildObject from one of AutoGenObjectList is 
enough.
 #
-InfObj = 
self.AutoGenObjectList[0].BuildDatabase.WorkspaceDb.BuildObject[PathClassObj, 
'COMMON', self.BuildTarget, self.ToolChain]
+InfObj = 
self.AutoGenObjectList[0].BuildDatabase.WorkspaceDb.BuildObject[PathClassObj, 
TAB_COMMON, self.BuildTarget, self.ToolChain]
 if not InfObj.Guid.upper() in _GuidDict.keys():
 _GuidDict[InfObj.Guid.upper()] = FfsFile
 else:
@@ -1355,7 +1357,7 @@ class PlatformAutoGen(AutoGen):
 EdkLogger.error("build", FILE_READ_FAILURE, "Can not find 
VPD map file %s to fix up VPD offset." % VpdMapFilePath)
 
 NvStoreOffset = int(NvStoreOffset,16) if 
NvStoreOffset.upper().startswith("0X") else int(NvStoreOffset)
-default_skuobj = 
PcdNvStoreDfBuffer[0].SkuInfoList.get("DEFAULT")
+default_skuobj = 
PcdNvStoreDfBuffer[0].SkuInfoList.get(TAB_DEFAULT)
 maxsize = self.VariableInfo.VpdRegionSize  - NvStoreOffset if 
self.VariableInfo.VpdRegionSize else len(default_skuobj.DefaultValue.split(","))
 var_data = 
self.VariableInfo.PatchNVStoreDefaultMaxSize(maxsize)
 
@@ -1363,7 +1365,7 @@ class PlatformAutoGen(AutoGen):
 default_skuobj.DefaultValue = var_data
 PcdNvStoreDfBuffer[0].DefaultValue = var_data
 PcdNvStoreDfBuffer[0].SkuInfoList.clear()
-PcdNvStoreDfBuffer[0].SkuInfoList['DEFAULT'] = 
default_skuobj
+PcdNvStoreDfBuffer[0].SkuInfoList[TAB_DEFAULT] = 
default_skuobj
 PcdNvStoreDfBuffer[0].MaxDatumSize = 
str(len(default_skuobj.DefaultValue.split(",")))
 
 return OrgVpdFile
@@ -1603,12 +1605,12 @@ class PlatformAutoGen(AutoGen):
PcdKey in VpdPcdDict:
 Pcd = VpdPcdDict[PcdKey]
 SkuValueMap = {}
-DefaultSku = Pcd.SkuInfoList.get('DEFAULT')
+DefaultSku = Pcd.SkuInfoList.get(TAB_DEFAULT)
 if DefaultSku:
 PcdValue = 

Re: [edk2] [PATCH 2/2] Hisilicon/D0x: Enable tftp command by default

[Leif Lindholm]

On Mon, Apr 02, 2018 at 10:51:53AM +0800, Guo Heyi wrote:
> Hi Ray and Leif,
> 
> Any comments?
> 
> 
> On Mon, Mar 26, 2018 at 05:03:10PM +0800, Guo Heyi wrote:
> > Thanks Ray.
> > 
> > Does that mean we need build the dynamic command driver separately and 
> > store it
> > in other media instead of UEFI fd image? Right now if I include the driver 
> > into
> > the fd image, it will be automatically added to EFI Shell command list; we 
> > don't
> > need to run the load command.
> > 
> > Hi Leif,
> > 
> > Is the policy to forbid including dynamic command driver into UEFI fd image?

Let's just say that I am highly reluctant to have commands not covered
by the UEFI Shell specification included by default. Especially
commands that we know from experience have been misused in order to
not have to figure out a sensible way of doing something properly
... and then been copied to new platforms by new developers coming in
and looking at existing ports for reference.

> > If we need other media to store tftp command driver, then the command will
> > become less useful, because it is mainly used to download something else. 
> > If we
> > have other media like USB disk, we can use this "other media" instead of 
> > network
> > download to store the final target.

If you are building a development image, by all means specify an
appropriate build flag and have additional debug features built in.

But if you are looking to use this for non-development builds, I would
very much like to hear your use-case.

Best Regards,

Leif

> > Thanks,
> > Heyi
> > 
> > On Fri, Mar 23, 2018 at 12:51:45PM +0800, Ni, Ruiyu wrote:
> > > On 3/20/2018 8:15 PM, Guo Heyi wrote:
> > > >I've no idea about how to use Driver; let me spend some time to 
> > > >learn first
> > > >:)
> > > 
> > > Heyi,
> > > you could use "load xxxDriver.efi" to load the dynamic command in shell.
> > > After that, you can run "tftp" in shell just as running an internal 
> > > command.
> > > 
> > > >
> > > >Regards,
> > > >
> > > >Heyi
> > > >
> > > >On Tue, Mar 20, 2018 at 09:51:32AM +, Leif Lindholm wrote:
> > > >>Ah, apologies.
> > > >>
> > > >>I would be reluctant to add commands not covered by the UEFI Shell
> > > >>Specification by default.
> > > >>
> > > >>Since it is now a dynamic command, is there any way of loading this
> > > >>dynamically (perhaps via DRIVER) where you feel the need for it?
> > > >>
> > > >>/
> > > >> Leif
> > > >>
> > > >>On Tue, Mar 20, 2018 at 03:54:46PM +0800, Guo Heyi wrote:
> > > >>>Ping :)
> > > >>>
> > > >>>
> > > >>>On Wed, Mar 07, 2018 at 04:02:30PM +, Ard Biesheuvel wrote:
> > > On 7 March 2018 at 03:03, Heyi Guo  wrote:
> > > >Since D0x platforms always have network enabled, we would like to
> > > >enable tftp command by default so that we can download something in
> > > >EFI Shell.
> > > >
> > > >Contributed-under: TianoCore Contribution Agreement 1.1
> > > >Signed-off-by: Heyi Guo 
> > > >Cc: Ard Biesheuvel 
> > > >Cc: Leif Lindholm 
> > > 
> > > The first patch looks fine to me, but I would like to give Leif a
> > > chance to comment on the policy side of this patch.
> > > 
> > > Please ping us by the end of next week if we haven't responded by 
> > > then.
> > > 
> > > >---
> > > >  Platform/Hisilicon/D03/D03.dsc | 2 ++
> > > >  Platform/Hisilicon/D05/D05.dsc | 1 +
> > > >  2 files changed, 3 insertions(+)
> > > >
> > > >diff --git a/Platform/Hisilicon/D03/D03.dsc 
> > > >b/Platform/Hisilicon/D03/D03.dsc
> > > >index cb0669d639d1..fce1e60b1275 100644
> > > >--- a/Platform/Hisilicon/D03/D03.dsc
> > > >+++ b/Platform/Hisilicon/D03/D03.dsc
> > > >@@ -29,6 +29,8 @@ [Defines]
> > > >SKUID_IDENTIFIER   = DEFAULT
> > > >FLASH_DEFINITION   = 
> > > > Platform/Hisilicon/$(PLATFORM_NAME)/$(PLATFORM_NAME).fdf
> > > >
> > > >+  DEFINE INCLUDE_TFTP_COMMAND= TRUE
> > > >+
> > > >  !include Silicon/Hisilicon/Hisilicon.dsc.inc
> > > >
> > > >  [LibraryClasses.common]
> > > >diff --git a/Platform/Hisilicon/D05/D05.dsc 
> > > >b/Platform/Hisilicon/D05/D05.dsc
> > > >index 8373a821a496..f007f3d2d7e8 100644
> > > >--- a/Platform/Hisilicon/D05/D05.dsc
> > > >+++ b/Platform/Hisilicon/D05/D05.dsc
> > > >@@ -29,6 +29,7 @@ [Defines]
> > > >SKUID_IDENTIFIER   = DEFAULT
> > > >FLASH_DEFINITION   = 
> > > > Platform/Hisilicon/$(PLATFORM_NAME)/$(PLATFORM_NAME).fdf
> > > >DEFINE EDK2_SKIP_PEICORE=0
> > > >+  DEFINE INCLUDE_TFTP_COMMAND= TRUE
> > > >DEFINE NETWORK_IP6_ENABLE  = FALSE
> > > >DEFINE HTTP_BOOT_ENABLE= FALSE
> > > >
> > > >--
> > > >2.7.4
> > > >
> > > >___
> > > >edk2-devel 

[edk2] Source code debugging of OVMF

[Johannes Swoboda]

Hello everyone,

I'm doing a research project on efi security. I would like to do source 
code debugging of OVMF. I want to step through the OVMF source code, set 
break points, et cetera, preferably with gdb. I want to debug the 
overall boot process. Is that possible?


I understand I can start qemu with the options -s -S. This gives me a 
virtual machine that awaits connection of a gdb debugger and further 
instructions.


After connecting with gdb, I can instruct the machine to continue 
execution. Unsurprisingly, there is nothing else I can do, because gdb 
lacks the relevant symbols.
I tried to read in the OVMF.fd file, as i would do with a binary that I 
would debug, but gdb can't handle the file.
It seems to be possible to load an efi app with the file command, but 
not this one.


I found one other person trying to achieve the same five years ago. [3] 
suggests that something like this

(gdb) add-symbol-file ../edk2/Build/OvmfX64/DEBUG_GCC5/X64/Shell.debug
The address where ../edk2/Build/OvmfX64/DEBUG_GCC5/X64/Shell.debug has 
been loaded is missing
might be possible. However, it appears to me this is may contain 
information regarding Shell.efi, an efi-app; but I don't want to debug 
an app. I want to debug the overall boot process.


I'm quoting some terminal output to clarify what I'm trying to achieve:
[johannes@johannes-laptop OVMF_efi_hello_world]$ gdb
[...]
(gdb) file OVMF.fd
"/home/johannes/18S/bakk/uefi_virtual/OVMF_efi_hello_world/OVMF.fd": not 
in executable format: File format not recognized

(gdb) target remote localhost:1234
Remote debugging using localhost:1234
warning: No executable has been specified and target does not support
determining executable automatically.  Try using the "file" command.
0xfff0 in ?? ()
(gdb) step
Cannot find bounds of current function
(gdb) list
No symbol table is loaded.  Use the "file" command.
(gdb) continue
Continuing.
[ovmf loads in qemu window]

I can redirect ovmf debug messages into a text file[1].
As far as I understand, there is a way to do source code level debugging 
*with* ovmf, to debug efi-apps that are e.g. run via the efi shell[2], 
with the help of SourceLevelDebugPkg[4], but this package cannot be used 
to source level debug the overall boot process, right?


Is it possible to do source level debugging of ovmf?
Is it possible to step through ovmf, one instruction or function call at 
a time?

How are you ovmf developer debugging it?

If you could point me to the right direction, that would be great.

Kind regards,
Johannes

[1]: I managed to do so via the qemu options -global 
isa-debugcon.iobase=0x402 -debugcon file:qemu.ovmf.log" as demonstrated 
here

https://www.linux-kvm.org/downloads/lersek/ovmf-whitepaper-c770f8c.txt
[2]:https://github.com/tianocore/tianocore.github.io/wiki/OVMF-FAQ#how-do-i-enable-source-level-debugging-with-ovmf
[3]: 
http://edk2-devel.narkive.com/LRWe2mSQ/using-gdb-on-ovmf-with-symbols
[4]: 
https://github.com/tianocore/tianocore.github.io/wiki/SourceLevelDebugPkg

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms] Silicon/AMD/Styx: add PPTT ACPI table

[Ard Biesheuvel]

On 15 March 2018 at 20:07, Leif Lindholm  wrote:
> On Thu, Mar 08, 2018 at 05:03:16PM +, Ard Biesheuvel wrote:
>> Add a ACPI PPTT table describing the cache topology of the Seattle SoC.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel 
>
> Looks plausible:
> Reviewed-by: Leif Lindholm 
>

Pushed as 6d6591a29e52

Thanks

>> ---
>>  Silicon/AMD/Styx/AcpiTables/AcpiTables.inf  |   1 +
>>  Silicon/AMD/Styx/AcpiTables/Pptt.c  | 225 
>> 
>>  Silicon/AMD/Styx/Common/AmdStyxAcpiLib.h|   1 +
>>  Silicon/AMD/Styx/Drivers/AcpiPlatformDxe/AcpiPlatform.c |   3 +-
>>  4 files changed, 229 insertions(+), 1 deletion(-)
>>
>> diff --git a/Silicon/AMD/Styx/AcpiTables/AcpiTables.inf 
>> b/Silicon/AMD/Styx/AcpiTables/AcpiTables.inf
>> index cfffc73894c0..057c52512e4e 100644
>> --- a/Silicon/AMD/Styx/AcpiTables/AcpiTables.inf
>> +++ b/Silicon/AMD/Styx/AcpiTables/AcpiTables.inf
>> @@ -38,6 +38,7 @@ [Sources]
>>Csrt.c
>>Dsdt.c
>>Iort.c
>> +  Pptt.c
>>
>>  [Packages]
>>ArmPkg/ArmPkg.dec
>> diff --git a/Silicon/AMD/Styx/AcpiTables/Pptt.c 
>> b/Silicon/AMD/Styx/AcpiTables/Pptt.c
>> new file mode 100644
>> index ..d9d7c494d86f
>> --- /dev/null
>> +++ b/Silicon/AMD/Styx/AcpiTables/Pptt.c
>> @@ -0,0 +1,225 @@
>> +/** @file
>> +
>> +  Copyright (c) 2018, Linaro Ltd. All rights reserved.
>> +
>> +  This program and the accompanying materials are licensed and made 
>> available
>> +  under the terms and conditions of the BSD License which accompanies this
>> +  distribution.  The full text of the license may be found at
>> +  http://opensource.org/licenses/bsd-license.php
>> +
>> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
>> IMPLIED.
>> +
>> +**/
>> +
>> +#include 
>> +
>> +#include 
>> +
>> +#define FIELD_OFFSET(type, name)__builtin_offsetof(type, name)
>> +
>> +#pragma pack(1)
>> +typedef struct {
>> +  EFI_ACPI_6_2_PPTT_STRUCTURE_PROCESSOR Core;
>> +  UINT32Offset[2];
>> +  EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE DCache;
>> +  EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE ICache;
>> +} STYX_PPTT_CORE;
>> +
>> +typedef struct {
>> +  EFI_ACPI_6_2_PPTT_STRUCTURE_PROCESSOR Cluster;
>> +  UINT32Offset[1];
>> +  EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE L2Cache;
>> +  STYX_PPTT_CORECores[2];
>> +} STYX_PPTT_CLUSTER;
>> +
>> +typedef struct {
>> +  EFI_ACPI_6_2_PPTT_STRUCTURE_PROCESSOR Package;
>> +  UINT32Offset[1];
>> +  EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE L3Cache;
>> +  STYX_PPTT_CLUSTER 
>> Clusters[NUM_CORES / 2];
>> +} STYX_PPTT_PACKAGE;
>> +
>> +typedef struct {
>> +  EFI_ACPI_6_2_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_HEADER   Pptt;
>> +  STYX_PPTT_PACKAGE Packages[1];
>> +} STYX_PPTT_TABLE;
>> +#pragma pack()
>> +
>> +#define PPTT_CORE(pid, cid, id) {   
>>   \
>> +  { 
>>   \
>> +EFI_ACPI_6_2_PPTT_TYPE_PROCESSOR,   
>>   \
>> +FIELD_OFFSET (STYX_PPTT_CORE, DCache),  
>>   \
>> +{}, 
>>   \
>> +{   
>>   \
>> +  0,/* PhysicalPackage */   
>>   \
>> +  EFI_ACPI_6_2_PPTT_PROCESSOR_ID_VALID, /* AcpiProcessorIdValid */  
>>   \
>> +},  
>>   \
>> +FIELD_OFFSET (STYX_PPTT_TABLE,  
>>   \
>> +  Packages[pid].Clusters[cid]), /* Parent */
>>   \
>> +((cid) << 8) + (id),/* AcpiProcessorId */   
>>   \
>> +2,  /* NumberOfPrivateResources 
>> */\
>> +  }, {  
>>   \
>> +FIELD_OFFSET (STYX_PPTT_TABLE,  
>>   \
>> +  Packages[pid].Clusters[cid].Cores[id].DCache),
>>   \
>> +FIELD_OFFSET (STYX_PPTT_TABLE,  
>>   \
>> +  Packages[pid].Clusters[cid].Cores[id].ICache),
>>   \
>> +  }, { 

[edk2] [PATCH edk2-platforms] Silicon/Socionext/SynQuacer: update PHY reference clock rate

[Ard Biesheuvel]

As reported by Kojima-san, the PHY reference clock value we use in our
ACPI and DT descriptions is out of sync with the hardware. Replace
125 MHz with 250 MHz throughout.

Cc: Masahisa Kojima 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
Kojima-san,

Please confirm that the modification to ogma_config.h is correct.

Thanks,
Ard.

 Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl
 | 2 +-
 Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi  
 | 4 ++--
 
Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/ogma_config.h 
| 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl 
b/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl
index b6f6c4360029..3f73c191d4d6 100644
--- a/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl
+++ b/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl
@@ -162,7 +162,7 @@ DefinitionBlock ("DsdtTable.aml", "DSDT", 1, "SNI", 
"SYNQUACR",
   Package (2) { "phy-channel", FixedPcdGet32 (PcdNetsecPhyAddress) },
   Package (2) { "max-speed", 1000 },
   Package (2) { "max-frame-size", 9000 },
-  Package (2) { "socionext,phy-clock-frequency", 12500 },
+  Package (2) { "socionext,phy-clock-frequency", 25000 },
 }
   })
 }
diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi 
b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
index 6e93c6ae16a8..f6887329f6c7 100644
--- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
+++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
@@ -420,9 +420,9 @@
 reg-shift = <2>;
 };
 
-clk_netsec: refclk125mhz {
+clk_netsec: refclk250mhz {
 compatible = "fixed-clock";
-clock-frequency = <12500>;
+clock-frequency = <25000>;
 #clock-cells = <0>;
 };
 
diff --git 
a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/ogma_config.h
 
b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/ogma_config.h
index 1caf64e30623..f6ec9b30ec8e 100644
--- 
a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/ogma_config.h
+++ 
b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/ogma_config.h
@@ -16,8 +16,8 @@
 #ifndef OGMA_CONFIG_H
 #define OGMA_CONFIG_H
 
-#define OGMA_CONFIG_CLK_HZ 12500UL
-#define OGMA_CONFIG_GMAC_CLK_HZ 12500UL
+#define OGMA_CONFIG_CLK_HZ 25000UL
+#define OGMA_CONFIG_GMAC_CLK_HZ 25000UL
 #define OGMA_CONFIG_CHECK_CLK_SUPPLY
 
 #define OGMA_CONFIG_USE_READ_GMAC_STAT
-- 
2.17.0

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [platforms PATCH 3/3] Marvell/Armada7k8k: Hook NvVarStoreFormattedLib into VariableRuntimeDxe

[Ard Biesheuvel]

On 16 April 2018 at 08:15, Marcin Wojtas  wrote:
> As MvFvbDxe driver is ready, we can now link NvVarStoreFormattedLib
> into VariableRuntimeDxe via NULL class resolution for all
> Armada7k8k platforms.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas 
> ---
>  Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc 
> b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> index 535cc3f..cd58107 100644
> --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> @@ -477,6 +477,7 @@
>MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
>  
>
> AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
> +  
> NULL|EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
>NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
>
> TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
>VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
> --
> 2.7.4
>


For the series

Reviewed-by: Ard Biesheuvel 

Pushed as e74f53df8b18..709435c64b47

Thanks!
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] Maintainers.txt: add Laszlo Ersek to stewards

[Ard Biesheuvel]

On 13 April 2018 at 21:26, Leif Lindholm  wrote:
> Cc: Andrew Fish 
> Cc: Laszlo Ersek 
> Cc: Michael D Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Leif Lindholm 
> ---
>  Maintainers.txt | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/Maintainers.txt b/Maintainers.txt
> index 256133810f..7295cd6b83 100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -41,6 +41,7 @@ T: svn (read-only, deprecated) - 
> https://svn.code.sf.net/p/edk2/code/trunk/edk2
>  Tianocore Stewards
>  --
>  M: Andrew Fish 
> +M: Laszlo Ersek 
>  M: Leif Lindholm 
>  M: Michael D Kinney 
>

Excellent news!
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms v2 6/6] Platform/ARM/Sgi: add initial support for ARM SGI platform

[Ard Biesheuvel]

On 12 April 2018 at 20:47, Thomas Abraham  wrote:
> From: Vishwanatha HG 
>
> Add the initial support for ARM's System Guidance for Infrastructure
> (SGI) platforms. SGI-575 is the supported platform in this initial
> implementation and can be extented to include support for upcoming
> SGI platforms as well.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by:  Vishwanatha HG 
> Signed-off-by: Thomas Abraham 
> ---
>  Platform/ARM/SgiPkg/SgiPlatform.dec |  36 
>  Platform/ARM/SgiPkg/SgiPlatform.dsc | 243 +++
>  Platform/ARM/SgiPkg/SgiPlatform.fdf | 320 
> 
>  3 files changed, 599 insertions(+)
>  create mode 100644 Platform/ARM/SgiPkg/SgiPlatform.dec
>  create mode 100644 Platform/ARM/SgiPkg/SgiPlatform.dsc
>  create mode 100644 Platform/ARM/SgiPkg/SgiPlatform.fdf
>
> diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dec 
> b/Platform/ARM/SgiPkg/SgiPlatform.dec
> new file mode 100644
> index 000..b446aa6
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/SgiPlatform.dec
> @@ -0,0 +1,36 @@
> +#
> +#  Copyright (c) 2018, ARM Limited. All rights reserved.
> +#
> +#  This program and the accompanying materials are licensed and made 
> available
> +#  under the terms and conditions of the BSD License which accompanies this
> +#  distribution.  The full text of the license may be found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +#
> +
> +[Defines]
> +  DEC_SPECIFICATION  = 0x00010005
> +  PACKAGE_NAME   = SgiPkg
> +  PACKAGE_GUID   = e6e0f26c-0df9-4f6c-a382-37ded896c6e9
> +  PACKAGE_VERSION= 0.1
> +
> +
> +#
> +# Include Section - list of Include Paths that are provided by this package.
> +#   Comments are used for Keywords and Module Types.
> +#
> +# Supported Module Types:
> +#  BASE SEC PEI_CORE PEIM DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER 
> DXE_SMM_DRIVER DXE_SAL_DRIVER UEFI_DRIVER UEFI_APPLICATION
> +#
> +
> +[Includes.common]
> +  Include# Root include for the package
> +
> +[Guids.common]
> +  gArmSgiTokenSpaceGuid=  { 0x577d6941, 0xaea1, 0x40b4, { 0x90, 0x93, 
> 0x2a, 0x86, 0x61, 0x72, 0x5a, 0x57 } }
> +
> +[PcdsFeatureFlag.common]
> +  # Set this PCD to TRUE to enable virtio support.
> +  gArmSgiTokenSpaceGuid.PcdVirtioSupported|TRUE|BOOLEAN|0x0001
> diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dsc 
> b/Platform/ARM/SgiPkg/SgiPlatform.dsc
> new file mode 100644
> index 000..4f01cb0
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/SgiPlatform.dsc
> @@ -0,0 +1,243 @@
> +#
> +#  Copyright (c) 2018, ARM Limited. All rights reserved.
> +#
> +#  This program and the accompanying materials are licensed and made 
> available
> +#  under the terms and conditions of the BSD License which accompanies this
> +#  distribution.  The full text of the license may be found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +#
> +
> +
> +#
> +# Defines Section - statements that will be processed to create a Makefile.
> +#
> +
> +[Defines]
> +  PLATFORM_NAME  = arm_sgi_platform
> +  PLATFORM_GUID  = 3a6b2eae-0275-4b6e-a5d1-bd2ba1ce1fae
> +  PLATFORM_VERSION   = 0.1
> +  DSC_SPECIFICATION  = 0x00010005
> +  OUTPUT_DIRECTORY   = Build/$(PLATFORM_NAME)
> +  SUPPORTED_ARCHITECTURES= AARCH64|ARM
> +  BUILD_TARGETS  = DEBUG|RELEASE
> +  SKUID_IDENTIFIER   = DEFAULT
> +  FLASH_DEFINITION   = 
> edk2-platforms/Platform/ARM/SgiPkg/SgiPlatform.fdf
> +
> +[LibraryClasses.common]
> +  
> ArmPlatformLib|edk2-platforms/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
> +  ArmHvcLib|ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
> +
> +!include edk2-platforms/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
> +
> +  # ARM Base Library
> +  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
> +  ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> +  ArmSmcLib|ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
> +
> +  BasePathLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
> +
> +  
> ArmPlatformSysConfigLib|Platform/ARM/VExpressPkg/Library/ArmVExpressSysConfigLib/ArmVExpressSysConfigLib.inf
> +  

Re: [edk2] [PATCH] ArmPkg/ArmMmuLib ARM: fix Mva to use idx instead of table base

[Leif Lindholm]

On Fri, Apr 13, 2018 at 11:43:27PM +, Chris Co wrote:
> Mva address calculation should use the left-shifted current
> section index instead of the left-shifted table base address.
> 
> Using the table base address here has the side-effect of potentially
> causing an access violation depending on the base address value.
> 
> Cc: Leif Lindholm 
> Cc: Ard Biesheuvel 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Christopher Co 
> ---
>  ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c 
> b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> index 774a7ccf59..9bf4ba03fd 100644
> --- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> +++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> @@ -716,7 +716,7 @@ UpdateSectionEntries (
>Descriptor |= EntryValue;
>  
>if (CurrentDescriptor  != Descriptor) {
> -Mva = (VOID *)(UINTN)(((UINTN)FirstLevelTable) << 
> TT_DESCRIPTOR_SECTION_BASE_SHIFT);
> +Mva = (VOID *)(UINTN)(((UINTN)FirstLevelIdx + i) << 
> TT_DESCRIPTOR_SECTION_BASE_SHIFT);

So, this clearly looks like you've found a bug - thanks!

But I am a little bit confused about the patch - should this not need
to incorporate the descriptor size in some way?
I.e. something like
  Mva = (VOID *)(UINTN)(((UINTN)FirstLevelIdx + (i * sizeof(UINTN))) << 
TT_DESCRIPTOR_SECTION_BASE_SHIFT);
or
  ...   [FirstLevelIndex + i] ...

?

Regards,

Leif

>  
>  // Clean/invalidate the cache for this section, but only
>  // if we are modifying the memory type attributes
> -- 
> 2.15.1.gvfs.2.39.g03d366a
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdePkg: add big-endian MMIO BaseBeIoLib

[Leif Lindholm]

On Fri, Apr 13, 2018 at 11:32:35PM +, Kinney, Michael D wrote:
> Leif,
> 
> I am curious why a Swap class/instances is not sufficient.
> 
> Currently EDK II follows the UEFI/PI specs, which for
> all supported CPU architectures use little endian ABI.
> The BaseIoLib follows the endianness of the CPU.  If
> UEFI/PI added a CPU that was big endian, I would expect
> BaseIoLib when built for that CPU would perform big endian
> operations.
> 
> Am I missing something?

If you did add a big-endian CPU, you could then find yourself in the
exact opposite situation and require a little-endian i/o access
library. Which would be implemented exactly as the contents of
IoLibSwap.c.

The header file necessarily needs to be endianness-specific, and if
the coding style had permitted functions in header files, my automatic
reaction would have been to make all of these static inline helper
functions (even with the code duplication).

/
Leif

> Mike
> 
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-
> > boun...@lists.01.org] On Behalf Of Leif Lindholm
> > Sent: Friday, April 13, 2018 12:32 PM
> > To: Kinney, Michael D 
> > Cc: edk2-devel@lists.01.org; Laszlo Ersek
> > ; Gao, Liming 
> > Subject: Re: [edk2] [PATCH] MdePkg: add big-endian MMIO
> > BaseBeIoLib
> > 
> > On Fri, Apr 13, 2018 at 07:24:06PM +, Kinney,
> > Michael D wrote:
> > > Hi Leif,
> > >
> > > I think we need to look at the names.  I see a mix of
> > > "Be" and "Swap".  We should pick one and use it
> > > consistently.
> > 
> > This was what I meant by the comments:
> > ---
> > This modified version introduces a single BeIoLib
> > instance, backed by
> > a source-file that could be used also for a
> > hypothetical LeIoLib.
> > There is no LeIoLib.h included though.
> > 
> > While this is arguably overengineered, I do feel
> > reasonably strongly
> > that code should be named for what it does, not for how
> > it is used,
> > and doing it this way lets me follow that rule.
> > ---
> > 
> > Clearly this is open for discussion, but the above is
> > my opinion and
> > the code intentionally reflects that.
> > 
> > Regards,
> > 
> > Leif
> > 
> > > Mike
> > >
> > > > -Original Message-
> > > > From: Leif Lindholm
> > [mailto:leif.lindh...@linaro.org]
> > > > Sent: Friday, April 13, 2018 10:42 AM
> > > > To: edk2-devel@lists.01.org
> > > > Cc: Kinney, Michael D ;
> > > > Gao, Liming ; Laszlo Ersek
> > > > ; udit.ku...@nxp.com
> > > > Subject: [PATCH] MdePkg: add big-endian MMIO
> > > > BaseBeIoLib
> > > >
> > > > When performing MMIO to a destination of the
> > opposite
> > > > endianness to the
> > > > executing processor, this library provides
> > automatic
> > > > byte order reversal
> > > > on inputs and outputs.
> > > >
> > > > Contributed-under: TianoCore Contribution Agreement
> > 1.1
> > > > Signed-off-by: Leif Lindholm
> > 
> > > > ---
> > > >
> > > > Udit, many apologies for this dragging out - back-
> > to-
> > > > back conferences,
> > > > holidays, and lots of catching up.
> > > >
> > > > This modified version introduces a single BeIoLib
> > > > instance, backed by
> > > > a source-file that could be used also for a
> > > > hypothetical LeIoLib.
> > > > There is no LeIoLib.h included though.
> > > >
> > > > While this is arguably overengineered, I do feel
> > > > reasonably strongly
> > > > that code should be named for what it does, not for
> > how
> > > > it is used,
> > > > and doing it this way lets me follow that rule.
> > > >
> > > > I have not duplicated the .uni file together with
> > the
> > > > .inf, since
> > > > this follows what is done in BaseIoLibIntrinsic.
> > > >
> > > >  MdePkg/Include/Library/BeIoLib.h   |
> > 376
> > > > +++
> > > >  MdePkg/Library/BaseIoLibSwap/BaseBeIoLib.inf   |
> > 48
> > > > +++
> > > >  MdePkg/Library/BaseIoLibSwap/BaseIoLibSwap.uni |
> > 23
> > > > ++
> > > >  MdePkg/Library/BaseIoLibSwap/IoLibSwap.c   |
> > 477
> > > > +
> > > >  MdePkg/MdePkg.dec  |
> > 3 +
> > > >  5 files changed, 927 insertions(+)
> > > >  create mode 100644
> > MdePkg/Include/Library/BeIoLib.h
> > > >  create mode 100644
> > > > MdePkg/Library/BaseIoLibSwap/BaseBeIoLib.inf
> > > >  create mode 100644
> > > > MdePkg/Library/BaseIoLibSwap/BaseIoLibSwap.uni
> > > >  create mode 100644
> > > > MdePkg/Library/BaseIoLibSwap/IoLibSwap.c
> > > >
> > > > diff --git a/MdePkg/Include/Library/BeIoLib.h
> > > > b/MdePkg/Include/Library/BeIoLib.h
> > > > new file mode 100644
> > > > index 00..5b2dc1a8e1
> > > > --- /dev/null
> > > > +++ b/MdePkg/Include/Library/BeIoLib.h
> > > > @@ -0,0 +1,376 @@
> > > > +/** @file
> > > > +  Provide byte-swapping services to access MMIO
> > > > registers.
> > > > +
> > > > +Copyright (c) 2006 - 2012, Intel Corporation. 

[edk2] [EDK2] ACPI : S1 deprecated?

[Tiger Liu(BJ-RD)]

Hi, experts:
I have a question about S1 state. (ACPI Spec)

It seems current UEFI BIOS not support S1 state.
For example:
Current desktop PC's UEFI BIOS, usually not report to OS that it support S1 
state.

Thanks


?
?
CONFIDENTIAL NOTE:
This email contains confidential or legally privileged information and is for 
the sole use of its intended recipient. Any unauthorized review, use, copying 
or forwarding of this email or the content of this email is strictly prohibited.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg DxeCapsuleLibFmp: Fix wrong Index is used

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Star
> Zeng
> Sent: Monday, April 16, 2018 4:42 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Zeng, Star 
> Subject: [edk2] [PATCH] MdeModulePkg DxeCapsuleLibFmp: Fix wrong Index is
> used
> 
> DEBUG((
>   DEBUG_ERROR,
>   "ItemOffsetList[%d](0x%lx) < ItemOffsetList[%d](0x%x)\n",
>   Index,
>   ItemOffsetList[Index],
>   Index,   // Should be Index - 1
>   ItemOffsetList[Index - 1]
>   ));
> 
> Cc: Jiewen Yao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng 
> ---
>  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> index 36e8c26aa976..a5dcb76d5aaa 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> @@ -250,7 +250,7 @@ ValidateFmpCapsule (
>  //
>  if (Index > 0) {
>if (ItemOffsetList[Index] <= ItemOffsetList[Index - 1]) {
> -DEBUG((DEBUG_ERROR, "ItemOffsetList[%d](0x%lx) <
> ItemOffsetList[%d](0x%x)\n", Index, ItemOffsetList[Index], Index,
> ItemOffsetList[Index - 1]));
> +DEBUG((DEBUG_ERROR, "ItemOffsetList[%d](0x%lx) <
> ItemOffsetList[%d](0x%x)\n", Index, ItemOffsetList[Index], Index - 1,
> ItemOffsetList[Index - 1]));
>  return EFI_INVALID_PARAMETER;
>}
>  }
> --
> 2.7.0.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] SecurityPkg FmpAuthenticationLibRsa2048Sha256: Remove PCD reference

[Yao, Jiewen]

Good clean up.

Reviewed-by: jiewen@intel.com


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Star
> Zeng
> Sent: Monday, April 16, 2018 4:41 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Zhang, Chao B
> ; Zeng, Star 
> Subject: [edk2] [PATCH] SecurityPkg FmpAuthenticationLibRsa2048Sha256:
> Remove PCD reference
> 
> PcdRsa2048Sha256PublicKeyBuffer is referenced but not used in the
> library, that makes me a little confusing.
> Actually, the PublicKeyData should be from the caller of
> AuthenticateFmpImage() as input parameter, for example
> EdkiiSystemCapsuleLib.
> 
> This patch is to remove the PCD reference in this library instance
> to be aligned with FmpAuthenticationLibPkcs7 that does not reference
> PcdPkcs7CertBuffer.
> 
> Cc: Chao Zhang 
> Cc: Jiewen Yao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng 
> ---
>  .../FmpAuthenticationLibRsa2048Sha256.c  |
> 4 ++--
>  .../FmpAuthenticationLibRsa2048Sha256.inf|
> 5 +
>  2 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git
> a/SecurityPkg/Library/FmpAuthenticationLibRsa2048Sha256/FmpAuthenticatio
> nLibRsa2048Sha256.c
> b/SecurityPkg/Library/FmpAuthenticationLibRsa2048Sha256/FmpAuthenticatio
> nLibRsa2048Sha256.c
> index b40993fd1b00..038e12447782 100644
> ---
> a/SecurityPkg/Library/FmpAuthenticationLibRsa2048Sha256/FmpAuthenticatio
> nLibRsa2048Sha256.c
> +++
> b/SecurityPkg/Library/FmpAuthenticationLibRsa2048Sha256/FmpAuthenticatio
> nLibRsa2048Sha256.c
> @@ -10,7 +10,7 @@
>FmpAuthenticatedHandlerRsa2048Sha256(), AuthenticateFmpImage() will
> receive
>untrusted input and do basic validation.
> 
> -  Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.
> +  Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the BSD
> License
>which accompanies this distribution.  The full text of the license may be
> found at
> @@ -133,7 +133,7 @@ FmpAuthenticatedHandlerRsa2048Sha256 (
>}
> 
>//
> -  // Fail if the PublicKey is not one of the public keys in
> PcdRsa2048Sha256PublicKeyBuffer
> +  // Fail if the PublicKey is not one of the public keys in the input 
> PublicKeyData.
>//
>PublicKey = (VOID *)PublicKeyData;
>PublicKeyBufferSize = PublicKeyDataLength;
> diff --git
> a/SecurityPkg/Library/FmpAuthenticationLibRsa2048Sha256/FmpAuthenticatio
> nLibRsa2048Sha256.inf
> b/SecurityPkg/Library/FmpAuthenticationLibRsa2048Sha256/FmpAuthenticatio
> nLibRsa2048Sha256.inf
> index b190eca8805c..cdd22429c274 100644
> ---
> a/SecurityPkg/Library/FmpAuthenticationLibRsa2048Sha256/FmpAuthenticatio
> nLibRsa2048Sha256.inf
> +++
> b/SecurityPkg/Library/FmpAuthenticationLibRsa2048Sha256/FmpAuthenticatio
> nLibRsa2048Sha256.inf
> @@ -3,7 +3,7 @@
>  #
>  # Instance of FmpAuthentication Library for DXE/PEI post memory phase.
>  #
> -#  Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.
> +#  Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.
>  #  This program and the accompanying materials
>  #  are licensed and made available under the terms and conditions of the BSD
> License
>  #  which accompanies this distribution.  The full text of the license may be
> found at
> @@ -45,9 +45,6 @@ [LibraryClasses]
>MemoryAllocationLib
>BaseCryptLib
> 
> -[Pcd]
> -  gEfiSecurityPkgTokenSpaceGuid.PcdRsa2048Sha256PublicKeyBuffer ##
> CONSUMES
> -
>  [Guids]
>gEfiCertTypeRsa2048Sha256Guid ## SOMETIMES_CONSUMES
> ## GUID # Unique ID for the type of the certificate.
>gEfiHashAlgorithmSha256Guid   ## SOMETIMES_CONSUMES
> ## GUID
> --
> 2.7.0.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] MdeModulePkg DxeCapsuleLibFmp: Fix wrong Index is used

[Star Zeng]

DEBUG((
  DEBUG_ERROR,
  "ItemOffsetList[%d](0x%lx) < ItemOffsetList[%d](0x%x)\n",
  Index,
  ItemOffsetList[Index],
  Index,   // Should be Index - 1
  ItemOffsetList[Index - 1]
  ));

Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c 
b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
index 36e8c26aa976..a5dcb76d5aaa 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
@@ -250,7 +250,7 @@ ValidateFmpCapsule (
 //
 if (Index > 0) {
   if (ItemOffsetList[Index] <= ItemOffsetList[Index - 1]) {
-DEBUG((DEBUG_ERROR, "ItemOffsetList[%d](0x%lx) < 
ItemOffsetList[%d](0x%x)\n", Index, ItemOffsetList[Index], Index, 
ItemOffsetList[Index - 1]));
+DEBUG((DEBUG_ERROR, "ItemOffsetList[%d](0x%lx) < 
ItemOffsetList[%d](0x%x)\n", Index, ItemOffsetList[Index], Index - 1, 
ItemOffsetList[Index - 1]));
 return EFI_INVALID_PARAMETER;
   }
 }
-- 
2.7.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] SecurityPkg FmpAuthenticationLibRsa2048Sha256: Remove PCD reference

[Star Zeng]

PcdRsa2048Sha256PublicKeyBuffer is referenced but not used in the
library, that makes me a little confusing.
Actually, the PublicKeyData should be from the caller of
AuthenticateFmpImage() as input parameter, for example
EdkiiSystemCapsuleLib.

This patch is to remove the PCD reference in this library instance
to be aligned with FmpAuthenticationLibPkcs7 that does not reference
PcdPkcs7CertBuffer.

Cc: Chao Zhang 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 .../FmpAuthenticationLibRsa2048Sha256.c  | 4 ++--
 .../FmpAuthenticationLibRsa2048Sha256.inf| 5 +
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git 
a/SecurityPkg/Library/FmpAuthenticationLibRsa2048Sha256/FmpAuthenticationLibRsa2048Sha256.c
 
b/SecurityPkg/Library/FmpAuthenticationLibRsa2048Sha256/FmpAuthenticationLibRsa2048Sha256.c
index b40993fd1b00..038e12447782 100644
--- 
a/SecurityPkg/Library/FmpAuthenticationLibRsa2048Sha256/FmpAuthenticationLibRsa2048Sha256.c
+++ 
b/SecurityPkg/Library/FmpAuthenticationLibRsa2048Sha256/FmpAuthenticationLibRsa2048Sha256.c
@@ -10,7 +10,7 @@
   FmpAuthenticatedHandlerRsa2048Sha256(), AuthenticateFmpImage() will receive
   untrusted input and do basic validation.
 
-  Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.
+  Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -133,7 +133,7 @@ FmpAuthenticatedHandlerRsa2048Sha256 (
   }
 
   //
-  // Fail if the PublicKey is not one of the public keys in 
PcdRsa2048Sha256PublicKeyBuffer
+  // Fail if the PublicKey is not one of the public keys in the input 
PublicKeyData.
   //
   PublicKey = (VOID *)PublicKeyData;
   PublicKeyBufferSize = PublicKeyDataLength;
diff --git 
a/SecurityPkg/Library/FmpAuthenticationLibRsa2048Sha256/FmpAuthenticationLibRsa2048Sha256.inf
 
b/SecurityPkg/Library/FmpAuthenticationLibRsa2048Sha256/FmpAuthenticationLibRsa2048Sha256.inf
index b190eca8805c..cdd22429c274 100644
--- 
a/SecurityPkg/Library/FmpAuthenticationLibRsa2048Sha256/FmpAuthenticationLibRsa2048Sha256.inf
+++ 
b/SecurityPkg/Library/FmpAuthenticationLibRsa2048Sha256/FmpAuthenticationLibRsa2048Sha256.inf
@@ -3,7 +3,7 @@
 #
 # Instance of FmpAuthentication Library for DXE/PEI post memory phase.
 #
-#  Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.
+#  Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD 
License
 #  which accompanies this distribution.  The full text of the license may be 
found at
@@ -45,9 +45,6 @@ [LibraryClasses]
   MemoryAllocationLib
   BaseCryptLib
 
-[Pcd]
-  gEfiSecurityPkgTokenSpaceGuid.PcdRsa2048Sha256PublicKeyBuffer ## CONSUMES
-
 [Guids]
   gEfiCertTypeRsa2048Sha256Guid ## SOMETIMES_CONSUMES   ## GUID # 
Unique ID for the type of the certificate.
   gEfiHashAlgorithmSha256Guid   ## SOMETIMES_CONSUMES   ## GUID
-- 
2.7.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms v2 6/6] Platform/ARM/Sgi: add initial support for ARM SGI platform

[Ard Biesheuvel]

On 12 April 2018 at 20:47, Thomas Abraham  wrote:
> From: Vishwanatha HG 
>
> Add the initial support for ARM's System Guidance for Infrastructure
> (SGI) platforms. SGI-575 is the supported platform in this initial
> implementation and can be extented to include support for upcoming
> SGI platforms as well.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by:  Vishwanatha HG 
> Signed-off-by: Thomas Abraham 
> ---
>  Platform/ARM/SgiPkg/SgiPlatform.dec |  36 
>  Platform/ARM/SgiPkg/SgiPlatform.dsc | 243 +++
>  Platform/ARM/SgiPkg/SgiPlatform.fdf | 320 
> 
>  3 files changed, 599 insertions(+)
>  create mode 100644 Platform/ARM/SgiPkg/SgiPlatform.dec
>  create mode 100644 Platform/ARM/SgiPkg/SgiPlatform.dsc
>  create mode 100644 Platform/ARM/SgiPkg/SgiPlatform.fdf
>
> diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dec 
> b/Platform/ARM/SgiPkg/SgiPlatform.dec
> new file mode 100644
> index 000..b446aa6
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/SgiPlatform.dec
> @@ -0,0 +1,36 @@
> +#
> +#  Copyright (c) 2018, ARM Limited. All rights reserved.
> +#
> +#  This program and the accompanying materials are licensed and made 
> available
> +#  under the terms and conditions of the BSD License which accompanies this
> +#  distribution.  The full text of the license may be found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +#
> +
> +[Defines]
> +  DEC_SPECIFICATION  = 0x00010005

0x0001001A

> +  PACKAGE_NAME   = SgiPkg
> +  PACKAGE_GUID   = e6e0f26c-0df9-4f6c-a382-37ded896c6e9
> +  PACKAGE_VERSION= 0.1
> +
> +
> +#
> +# Include Section - list of Include Paths that are provided by this package.
> +#   Comments are used for Keywords and Module Types.
> +#
> +# Supported Module Types:
> +#  BASE SEC PEI_CORE PEIM DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER 
> DXE_SMM_DRIVER DXE_SAL_DRIVER UEFI_DRIVER UEFI_APPLICATION
> +#
> +
> +[Includes.common]
> +  Include# Root include for the package
> +
> +[Guids.common]
> +  gArmSgiTokenSpaceGuid=  { 0x577d6941, 0xaea1, 0x40b4, { 0x90, 0x93, 
> 0x2a, 0x86, 0x61, 0x72, 0x5a, 0x57 } }
> +
> +[PcdsFeatureFlag.common]
> +  # Set this PCD to TRUE to enable virtio support.
> +  gArmSgiTokenSpaceGuid.PcdVirtioSupported|TRUE|BOOLEAN|0x0001
> diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dsc 
> b/Platform/ARM/SgiPkg/SgiPlatform.dsc
> new file mode 100644
> index 000..4f01cb0
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/SgiPlatform.dsc
> @@ -0,0 +1,243 @@
> +#
> +#  Copyright (c) 2018, ARM Limited. All rights reserved.
> +#
> +#  This program and the accompanying materials are licensed and made 
> available
> +#  under the terms and conditions of the BSD License which accompanies this
> +#  distribution.  The full text of the license may be found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +#
> +
> +
> +#
> +# Defines Section - statements that will be processed to create a Makefile.
> +#
> +
> +[Defines]
> +  PLATFORM_NAME  = arm_sgi_platform

Please use camel case here like we do everywhere else

> +  PLATFORM_GUID  = 3a6b2eae-0275-4b6e-a5d1-bd2ba1ce1fae
> +  PLATFORM_VERSION   = 0.1
> +  DSC_SPECIFICATION  = 0x00010005

0x0001001B

> +  OUTPUT_DIRECTORY   = Build/$(PLATFORM_NAME)
> +  SUPPORTED_ARCHITECTURES= AARCH64|ARM
> +  BUILD_TARGETS  = DEBUG|RELEASE

Please add NOOPT here as well

> +  SKUID_IDENTIFIER   = DEFAULT
> +  FLASH_DEFINITION   = 
> edk2-platforms/Platform/ARM/SgiPkg/SgiPlatform.fdf

Please drop the edk2-platforms prefix

> +
> +[LibraryClasses.common]
> +  
> ArmPlatformLib|edk2-platforms/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf

and here

> +  ArmHvcLib|ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
> +
> +!include edk2-platforms/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc

and here

> +
> +  # ARM Base Library
> +  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
> +  ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> +  ArmSmcLib|ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
> +
> +  

Re: [edk2] [PATCH edk2-platforms v2 5/6] Platform/ARM/Sgi: add the initial set of acpi tables

[Ard Biesheuvel]

FOn 12 April 2018 at 20:47, Thomas Abraham  wrote:
> From: Daniil Egranov 
>
> Add the initial version of Acpi tables for the SGI-575 platform which
> is required to boot the linux kernel up to a busybox prompt.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Daniil Egranov 
> Signed-off-by: Thomas Abraham 
> ---
>  .../ARM/SgiPkg/AcpiTables/Sgi575/AcpiTables.inf|  50 +++
>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dbg2.aslc|  89 +
>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dsdt.asl |  99 ++
>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Fadt.aslc|  87 +
>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Gtdt.aslc| 144 
> +
>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Madt.aslc| 127 ++
>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Spcr.aslc|  76 +++
>  .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c   |   9 ++
>  Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h|  41 ++
>  9 files changed, 722 insertions(+)
>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/AcpiTables.inf
>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dbg2.aslc
>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dsdt.asl
>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Fadt.aslc
>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Gtdt.aslc
>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Madt.aslc
>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Spcr.aslc
>  create mode 100644 Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h
>
> diff --git a/Platform/ARM/SgiPkg/AcpiTables/Sgi575/AcpiTables.inf 
> b/Platform/ARM/SgiPkg/AcpiTables/Sgi575/AcpiTables.inf
> new file mode 100644
> index 000..a44c76f
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/AcpiTables/Sgi575/AcpiTables.inf
> @@ -0,0 +1,50 @@
> +## @file
> +#
> +#  ACPI table data and ASL sources required to boot the platform.
> +#
> +#  Copyright (c) 2018, ARM Ltd. All rights reserved.
> +#
> +#  This program and the accompanying materials are licensed and made 
> available
> +#  under the terms and conditions of the BSD License which accompanies this
> +#  distribution.  The full text of the license may be found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION= 0x00010005

0x0001001A

> +  BASE_NAME  = Sgi575AcpiTables
> +  FILE_GUID  = c712719a-0aaf-438c-9cdd-35ab4d60207d

Can you use a named GUID and add its name in a comment at the end of
this line ^^^
E.g.,

c712719a-0aaf-438c-9cdd-35ab4d60207d # gSgi575AcpiTablesiFileGuid

> +  MODULE_TYPE= USER_DEFINED
> +  VERSION_STRING = 1.0
> +
> +[Sources]
> +  Dbg2.aslc
> +  Dsdt.asl
> +  Fadt.aslc
> +  Gtdt.aslc
> +  Madt.aslc
> +  Spcr.aslc
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  ArmPlatformPkg/ArmPlatformPkg.dec
> +  Platform/ARM/VExpressPkg/ArmVExpressPkg.dec
> +  edk2-platforms/Platform/ARM/SgiPkg/SgiPlatform.dec

Drop the edk2-platforms prefix

> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +
> +[FixedPcd]
> +  gArmTokenSpaceGuid.PcdArmArchTimerSecIntrNum
> +  gArmTokenSpaceGuid.PcdArmArchTimerIntrNum
> +  gArmTokenSpaceGuid.PcdArmArchTimerHypIntrNum
> +  gArmTokenSpaceGuid.PcdArmArchTimerVirtIntrNum
> +
> +  gArmTokenSpaceGuid.PcdGenericWatchdogControlBase
> +  gArmTokenSpaceGuid.PcdGenericWatchdogRefreshBase
> +  gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase
> +  gArmPlatformTokenSpaceGuid.PL011UartInterrupt
> diff --git a/Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dbg2.aslc 
> b/Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dbg2.aslc
> new file mode 100644
> index 000..d17b7bb
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dbg2.aslc
> @@ -0,0 +1,89 @@
> +/** @file
> +*  DBG2 Table
> +*
> +*  Copyright (c) 2018, ARM Limited. All rights reserved.
> +*
> +*  This program and the accompanying materials are licensed and made 
> available
> +*  under the terms and conditions of the BSD License which accompanies this
> +*  distribution. The full text of the license may be found at
> +*  http://opensource.org/licenses/bsd-license.php
> +*
> +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +*
> +**/
> +
> +#include "SgiAcpiHeader.h"
> +#include 
> +#include 
> +#include 
> +
> +#pragma pack(1)
> +
> +#define DBG2_NUM_DEBUG_PORTS   1
> +#define DBG2_NUMBER_OF_GENERIC_ADDRESS_REGISTERS   1
> +#define DBG2_NAMESPACESTRING_FIELD_SIZE8
> +#define 

Re: [edk2] [PATCH edk2-platforms v2 4/6] Platform/ARM/Sgi: add support for virtio block device

[Ard Biesheuvel]

On 12 April 2018 at 20:47, Thomas Abraham  wrote:
> From: Daniil Egranov 
>
> Add the registration of the virtio block device.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Daniil Egranov 
> Signed-off-by: Thomas Abraham 
> ---
>  .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c   | 18 -
>  .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf |  1 +
>  .../ARM/SgiPkg/Drivers/PlatformDxe/VirtioBlockIo.c | 76 
> ++
>  3 files changed, 94 insertions(+), 1 deletion(-)
>  create mode 100644 Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioBlockIo.c
>
> diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c 
> b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
> index 2da768a..5d54f06 100644
> --- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
> +++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
> @@ -15,11 +15,27 @@
>  #include 
>
>  EFI_STATUS
> +InitVirtioBlockIo (
> +   IN EFI_HANDLE ImageHandle
> +  );
> +

Can this be STATIC?

> +EFI_STATUS
>  EFIAPI
>  ArmSgiPkgEntryPoint (
>IN EFI_HANDLE ImageHandle,
>IN EFI_SYSTEM_TABLE   *SystemTable
>)
>  {
> -  return EFI_SUCCESS;
> +  EFI_STATUS  Status;
> +
> +  // Install Virtio Block IO.
> +  if ( FeaturePcdGet (PcdVirtioSupported) == TRUE ) {

No space after (
No boolean compare with TRUE/FALSE -> FeaturePcdGet() already returns a BOOLEAN

> +Status = InitVirtioBlockIo (ImageHandle);
> +if (EFI_ERROR (Status)) {
> +  DEBUG ((DEBUG_ERROR, "PlatformDxe: Failed to install Virtio Block 
> IO\n"));
> +  return Status;
> +}
> +  }
> +
> +  return Status;
>  }
> diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf 
> b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
> index 379d7f4..534947f 100644
> --- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
> +++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
> @@ -20,6 +20,7 @@
>
>  [Sources.common]
>PlatformDxe.c
> +  VirtioBlockIo.c
>
>  [Packages]
>ArmPkg/ArmPkg.dec
> diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioBlockIo.c 
> b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioBlockIo.c
> new file mode 100644
> index 000..eb00225
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioBlockIo.c
> @@ -0,0 +1,76 @@
> +/** @file
> +
> +  Copyright (c) 2018, ARM Ltd. All rights reserved.
> +
> +  This program and the accompanying materials are licensed and made available
> +  under the terms and conditions of the BSD License which accompanies this
> +  distribution.  The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +
> +**/
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#pragma pack (1)
> +typedef struct {
> +  VENDOR_DEVICE_PATH  Vendor;
> +  EFI_DEVICE_PATH_PROTOCOLEnd;
> +} VIRTIO_BLK_DEVICE_PATH;
> +#pragma pack ()
> +
> +STATIC VIRTIO_BLK_DEVICE_PATH mVirtioBlockDevicePath =
> +{
> +  {
> +{
> +  HARDWARE_DEVICE_PATH,
> +  HW_VENDOR_DP,
> +  {
> +(UINT8)( sizeof (VENDOR_DEVICE_PATH) ),
> +(UINT8)( (sizeof (VENDOR_DEVICE_PATH) ) >> 8)
> +  }
> +},
> +EFI_CALLER_ID_GUID,
> +  },
> +  {
> +END_DEVICE_PATH_TYPE,
> +END_ENTIRE_DEVICE_PATH_SUBTYPE,
> +{
> +  sizeof (EFI_DEVICE_PATH_PROTOCOL),
> +  0
> +}
> +  }
> +};
> +
> +/**
> + * Entrypoint for 'VirtioBlockIo' driver
> + */
> +EFI_STATUS
> +InitVirtioBlockIo (
> +   IN EFI_HANDLE ImageHandle
> +  )
> +{
> +  EFI_STATUS Status = 0;
> +
> +  Status = gBS->InstallProtocolInterface (,
> +   , EFI_NATIVE_INTERFACE,
> +   );
> +
> +  if (EFI_ERROR (Status)) {
> +return Status;
> +  }
> +
> +  // Declare the Virtio BlockIo device
> +  Status = VirtioMmioInstallDevice (SGI_EXP_SYSPH_VIRTIO_BLOCK_BASE, 
> ImageHandle);
> +  if (EFI_ERROR (Status)) {
> +DEBUG ((DEBUG_ERROR, "PlatformDxe: Failed to install Virtio block 
> device\n"));
> +  }
> +
> +  return Status;
> +}
> --
> 2.7.4
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms v2 3/6] Platform/ARM/Sgi: add initial platform dxe driver implementation

[Ard Biesheuvel]

On 12 April 2018 at 20:47, Thomas Abraham  wrote:
> From: Daniil Egranov 
>
> Add a initial platform dxe driver which starts of being almost
> an empty implemenation.

implemenTation

>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Daniil Egranov 
> Signed-off-by: Thomas Abraham 
> ---
>  .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c   | 25 
>  .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf | 74 
> ++
>  2 files changed, 99 insertions(+)
>  create mode 100644 Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
>  create mode 100644 Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
>
> diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c 
> b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
> new file mode 100644
> index 000..2da768a
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
> @@ -0,0 +1,25 @@
> +/** @file
> +*
> +*  Copyright (c) 2018, ARM Limited. All rights reserved.
> +*
> +*  This program and the accompanying materials are licensed and made 
> available
> +*  under the terms and conditions of the BSD License which accompanies this
> +*  distribution.  The full text of the license may be found at
> +*  http://opensource.org/licenses/bsd-license.php
> +*
> +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +*
> +**/
> +
> +#include 
> +
> +EFI_STATUS
> +EFIAPI
> +ArmSgiPkgEntryPoint (
> +  IN EFI_HANDLE ImageHandle,
> +  IN EFI_SYSTEM_TABLE   *SystemTable
> +  )
> +{
> +  return EFI_SUCCESS;
> +}
> diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf 
> b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
> new file mode 100644
> index 000..379d7f4
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
> @@ -0,0 +1,74 @@
> +#
> +#  Copyright (c) 2018, ARM Limited. All rights reserved.
> +#
> +#  This program and the accompanying materials are licensed and made 
> available
> +#  under the terms and conditions of the BSD License which accompanies this
> +#  distribution.  The full text of the license may be found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +#
> +
> +[Defines]
> +  INF_VERSION= 0x00010005

0x0001001A

> +  BASE_NAME  = PlatformDxe
> +  FILE_GUID  = 54cee352-c4cd-4d80-8524-54325c3a528e
> +  MODULE_TYPE= DXE_DRIVER
> +  VERSION_STRING = 1.0
> +  ENTRY_POINT= ArmSgiPkgEntryPoint
> +
> +[Sources.common]
> +  PlatformDxe.c
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  ArmPlatformPkg/ArmPlatformPkg.dec
> +  edk2-platforms/Platform/ARM/SgiPkg/SgiPlatform.dec

Drop the edk2-platforms prefix here

> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdePkg/MdePkg.dec
> +  OvmfPkg/OvmfPkg.dec
> +
> +[LibraryClasses]
> +  AcpiLib
> +  BaseMemoryLib
> +  DebugLib
> +  DxeServicesTableLib
> +  FdtLib
> +  HobLib
> +  IoLib
> +  PcdLib
> +  PrintLib
> +  SerialPortLib
> +  UefiBootServicesTableLib
> +  UefiRuntimeServicesTableLib
> +  UefiLib
> +  UefiDriverEntryPoint
> +  VirtioMmioDeviceLib
> +

Do you really need all these packages and library classes for an empty
implementation?

> +[Guids]
> +  #gArmGlobalVariableGuid
> +  gEfiEndOfDxeEventGroupGuid
> +  gEfiFileInfoGuid
> +  gEfiHobListGuid
> +  gFdtTableGuid
> +  gEfiAcpi10TableGuid
> +  gEfiAcpiTableGuid
> +

Same here

> +[Protocols]
> +  gEfiBlockIoProtocolGuid
> +  gEfiDevicePathFromTextProtocolGuid
> +  gEfiSimpleFileSystemProtocolGuid
> +

and here

> +[FeaturePcd]
> +  gArmSgiTokenSpaceGuid.PcdVirtioSupported
> +
> +[FixedPcd]
> +  gArmTokenSpaceGuid.PcdSystemMemoryBase
> +  gArmTokenSpaceGuid.PcdSystemMemorySize
> +
> +  gArmTokenSpaceGuid.PcdHypFvBaseAddress
> +  gArmTokenSpaceGuid.PcdHypFvSize
> +
> +[Depex]
> +  TRUE

I think it is fine to start out with an empty file and add things as
you go, but this should also cover the dependencies declared in this
file. I.e., add package, library class, guid and protocol dependencies
as needed when you add the functionality to the driver.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms v2 2/6] Platform/ARM/Sgi: add NOR flash platform library implementation

[Ard Biesheuvel]

On 12 April 2018 at 20:47, Thomas Abraham  wrote:
> From: Vishwanatha HG 
>
> Add a initial NOR flash driver platform wrapper as part of the platform
> library. Access to NOR fash 0 is enabled in this initial implementation.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Vishwanatha HG 
> Signed-off-by: Thomas Abraham 
> ---
>  Platform/ARM/SgiPkg/Library/NorFlashLib/NorFlash.c | 60 
> ++
>  .../ARM/SgiPkg/Library/NorFlashLib/NorFlashLib.inf | 33 
>  2 files changed, 93 insertions(+)
>  create mode 100644 Platform/ARM/SgiPkg/Library/NorFlashLib/NorFlash.c
>  create mode 100644 Platform/ARM/SgiPkg/Library/NorFlashLib/NorFlashLib.inf
>
> diff --git a/Platform/ARM/SgiPkg/Library/NorFlashLib/NorFlash.c 
> b/Platform/ARM/SgiPkg/Library/NorFlashLib/NorFlash.c
> new file mode 100644
> index 000..5d7efea
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/Library/NorFlashLib/NorFlash.c
> @@ -0,0 +1,60 @@
> +/** @file
> +
> +  Copyright (c) 2018, ARM Ltd. All rights reserved.
> +
> +  This program and the accompanying materials are licensed and made available
> +  under the terms and conditions of the BSD License which accompanies this
> +  distribution.  The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +
> + **/
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +STATIC NOR_FLASH_DESCRIPTION mNorFlashDevices[] = {
> +  {
> +SGI_EXP_SMC_CS0_BASE,
> +SGI_EXP_SMC_CS0_BASE,
> +SIZE_256KB * 255,
> +SIZE_256KB,
> +{0xEBF0B9DF, 0x17d0, 0x4812, { 0xA9, 0x59, 0xCF, 0xD7, 0x92, 0xEE, 0x31, 
> 0x13} }
> +  },
> +  {
> +SGI_EXP_SMC_CS0_BASE,
> +SGI_EXP_SMC_CS0_BASE + SIZE_256KB * 255,
> +SIZE_64KB * 4,
> +SIZE_64KB,
> +{0x98C111C6, 0xB322, 0x4C33, { 0x95, 0xD5, 0xAF, 0x56, 0xAF, 0x90, 0x18, 
> 0x6A } }
> +  },
> +};
> +
> +EFI_STATUS
> +NorFlashPlatformInitialization (
> +  VOID
> +  )
> +{
> +  return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +NorFlashPlatformGetDevices (
> +  OUT NOR_FLASH_DESCRIPTION   **NorFlashDevices,
> +  OUT UINT32  *Count
> +  )
> +{
> +  if ((NorFlashDevices == NULL) || (Count == NULL)) {
> +return EFI_INVALID_PARAMETER;
> +  }
> +
> +  *NorFlashDevices = mNorFlashDevices;
> +  *Count = sizeof (mNorFlashDevices) / sizeof (NOR_FLASH_DESCRIPTION);

Please use ARRAY_SIZE() here

> +
> +  return EFI_SUCCESS;
> +}
> diff --git a/Platform/ARM/SgiPkg/Library/NorFlashLib/NorFlashLib.inf 
> b/Platform/ARM/SgiPkg/Library/NorFlashLib/NorFlashLib.inf
> new file mode 100644
> index 000..a860e5c
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/Library/NorFlashLib/NorFlashLib.inf
> @@ -0,0 +1,33 @@
> +#/** @file
> +#
> +#  Copyright (c) 2018, ARM Ltd. All rights reserved.
> +
> +#  This program and the accompanying materials are licensed and made 
> available
> +#  under the terms and conditions of the BSD License which accompanies this
> +#  distribution.  The full text of the license may be found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +#
> +#**/
> +
> +[Defines]
> +  INF_VERSION= 0x00010005

0x0001001A

> +  BASE_NAME  = NorFlashSgiLib
> +  FILE_GUID  = 3f021755-6d74-4065-9ee4-98225267b36e
> +  MODULE_TYPE= DXE_DRIVER
> +  VERSION_STRING = 1.0
> +  LIBRARY_CLASS  = NorFlashPlatformLib
> +
> +[Sources.common]
> +  NorFlash.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  ArmPlatformPkg/ArmPlatformPkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  DebugLib
> +  IoLib
> --
> 2.7.4
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms v2 1/6] Platform/ARM/Sgi: Add Platform library implementation

[Ard Biesheuvel]

Hello Thomas,

On 12 April 2018 at 20:47, Thomas Abraham  wrote:
> Add initial SGI platform library support. This includes the virtual
> memory map and helper functions for platform intialization.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Thomas Abraham 
> ---
>  Platform/ARM/SgiPkg/Include/SgiPlatform.h  |  63 
>  .../SgiPkg/Library/PlatformLib/AArch64/Helper.S|  65 
>  Platform/ARM/SgiPkg/Library/PlatformLib/Mem.c  | 111 
> +
>  Platform/ARM/SgiPkg/Library/PlatformLib/Platform.c |  72 +
>  .../ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf |  56 +++
>  5 files changed, 367 insertions(+)
>  create mode 100644 Platform/ARM/SgiPkg/Include/SgiPlatform.h
>  create mode 100644 Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
>  create mode 100644 Platform/ARM/SgiPkg/Library/PlatformLib/Mem.c
>  create mode 100644 Platform/ARM/SgiPkg/Library/PlatformLib/Platform.c
>  create mode 100644 Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
>
> diff --git a/Platform/ARM/SgiPkg/Include/SgiPlatform.h 
> b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
> new file mode 100644
> index 000..86994d5
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
> @@ -0,0 +1,63 @@
> +/** @file
> +*
> +*  Copyright (c) 2018, ARM Limited. All rights reserved.
> +*
> +*  This program and the accompanying materials are licensed and made 
> available
> +*  under the terms and conditions of the BSD License which accompanies this
> +*  distribution. The full text of the license may be found at
> +*  http://opensource.org/licenses/bsd-license.php
> +*
> +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +*
> +**/
> +
> +#ifndef __SGI_PLATFORM_H__
> +#define __SGI_PLATFORM_H__
> +
> +/***
> +// Platform Memory Map
> +/
> +
> +// Expansion AXI - SMC Chip Select 0
> +#define SGI_EXP_SMC_CS0_BASE  0x0800
> +#define SGI_EXP_SMC_CS0_SZSIZE_64MB
> +
> +// Expansion AXI - SMSC 91C111 (Ethernet)
> +#define SGI_EXP_SMSC91X_BASE  0x1800
> +#define SGI_EXP_SMSC91X_SZSIZE_64MB
> +
> +// Expansion AXI - System peripherals
> +#define SGI_EXP_SYS_PERIPH_BASE   0x1C00
> +#define SGI_EXP_SYS_PERIPH_SZ SIZE_2MB
> +
> +#define SGI_EXP_SYSPH_VIRTIO_BLOCK_BASE   0x1c13
> +
> +// Sub System Peripherals - UART0
> +#define SGI_SUBSYS_UART0_BASE 0x2A40
> +#define SGI_SUBSYS_UART0_SZ   0x0001
> +
> +// Sub System Peripherals - UART1
> +#define SGI_SUBSYS_UART1_BASE 0x2A41
> +#define SGI_SUBSYS_UART1_SZ   0x0001
> +
> +// Sub System Peripherals - Generic Watchdog
> +#define SGI_SUBSYS_GENERIC_WDOG_BASE  0x2A44
> +#define SGI_SUBSYS_GENERIC_WDOG_SZSIZE_128KB
> +
> +// Sub System Peripherals - GIC
> +#define SGI_SUBSYS_GENERIC_GIC_BASE   0x3000
> +#define SGI_SUBSYS_GENERIC_GICR_BASE  0x300C
> +#define SGI_SUBSYS_GENERIC_GIC_SZ SIZE_1MB
> +
> +// Expansion AXI - Platform Peripherals - UART0
> +#define SGI_EXP_PLAT_PERIPH_UART0_BASE0x7FF7
> +#define SGI_EXP_PLAT_PERIPH_UART0_SZ  SIZE_64KB
> +
> +// Expansion AXI - Platform Peripherals - UART1
> +#define SGI_EXP_PLAT_PERIPH_UART1_BASE0x7FF8
> +#define SGI_EXP_PLAT_PERIPH_UART1_SZ  SIZE_64KB
> +
> +#define ARM_PLATFORM_WATCHDOG_COUNT  2
> +
> +#endif // __SGI_PLATFORM_H__
> diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S 
> b/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
> new file mode 100644
> index 000..9537bb0
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
> @@ -0,0 +1,65 @@
> +/** @file
> +*
> +*  Copyright (c) 2018, ARM Limited. All rights reserved.
> +*
> +*  This program and the accompanying materials are licensed and made 
> available
> +*  under the terms and conditions of the BSD License which accompanies this
> +*  distribution.  The full text of the license may be found at
> +*  http://opensource.org/licenses/bsd-license.php
> +*
> +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +*
> +**/
> +
> +#include 
> +#include 
> +
> +.text
> +.align 3
> +
> +GCC_ASM_EXPORT(ArmPlatformPeiBootAction)
> +GCC_ASM_EXPORT(ArmPlatformGetCorePosition)
> +GCC_ASM_EXPORT(ArmPlatformGetPrimaryCoreMpId)
> 

[edk2] [platforms PATCH 3/3] Marvell/Armada7k8k: Hook NvVarStoreFormattedLib into VariableRuntimeDxe

[Marcin Wojtas]

As MvFvbDxe driver is ready, we can now link NvVarStoreFormattedLib
into VariableRuntimeDxe via NULL class resolution for all
Armada7k8k platforms.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc 
b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
index 535cc3f..cd58107 100644
--- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
+++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
@@ -477,6 +477,7 @@
   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
 
   
AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
+  
NULL|EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
   NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
   
TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
   VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
-- 
2.7.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [platforms PATCH 1/2] Marvell/Drivers: MvFvbDxe: Adjust to new dependencies

[Marcin Wojtas]

Hi Ard,

2018-04-16 7:40 GMT+02:00 Ard Biesheuvel :
> (+ Laszlo)
>
> On 16 April 2018 at 07:09, Marcin Wojtas  wrote:
>> Recent changes in the EDK2 mainline resulted in breaking
>> of compilation and booting of Armada platforms.
>> This patch adjust the MvFvbDxe driver by:
>>
>>  * installation of gEdkiiNvVarStoreFormattedGuid in order to signal
>>NvVarStoreFormattedLib to the generic variable runtime driver
>>
>
> Hello Marcin,
>
> Installing this GUID is only necessary if you update your platform
> .DSC to make the generic variable runtime driver depend on it by
> adding a NULL library class resolution using NvVarStoreFormattedLib.
> So I think this patch is correct, but you'll need an additional change
> to make it work as expected. (Otherwise, the variable runtime driver
> could still be dispatched early and invoked for read access before the
> variable store is reformatted)

I added NULL class lib hook for VariableRuntimeDxe and it still works.
I'll send third patch on top of already submitted (the branch is
updated as well).

Thanks,
Marcin


>
>>  * making explicit dependency to ArmPkg/Drivers/CpuDxe drivers in order
>>to enable successful calling of gDS->SetMemorySpaceAttributes
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Marcin Wojtas 
>> ---
>>  Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c   | 21 
>>  Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf |  7 ++-
>>  2 files changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c 
>> b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c
>> index 252ef67..6e583a3 100644
>> --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c
>> +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c
>> @@ -26,6 +26,7 @@
>>  #include 
>>  #include 
>>
>> +#include 
>>  #include 
>>  #include 
>>
>> @@ -1076,6 +1077,21 @@ MvFvbEntryPoint (
>>}
>>
>>//
>> +  // The driver implementing the variable read service can now be 
>> dispatched;
>> +  // the varstore headers are in place.
>> +  //
>> +  Status = gBS->InstallProtocolInterface (,
>> +  ,
>> +  EFI_NATIVE_INTERFACE,
>> +  NULL);
>> +  if (EFI_ERROR (Status)) {
>> +DEBUG ((DEBUG_ERROR,
>> +  "%a: Failed to install gEdkiiNvVarStoreFormattedGuid\n",
>> +  __FUNCTION__));
>> +goto ErrorInstallNvVarStoreFormatted;
>> +  }
>> +
>> +  //
>>// Declare the Non-Volatile storage as EFI_MEMORY_RUNTIME
>>//
>>RuntimeMmioRegionSize = mFvbDevice->FvbSize;
>> @@ -1126,6 +1142,11 @@ ErrorSetMemAttr:
>>gDS->RemoveMemorySpace (RegionBaseAddress, RuntimeMmioRegionSize);
>>
>>  ErrorAddSpace:
>> +  gBS->UninstallProtocolInterface (,
>> +  ,
>> +  NULL);
>> +
>> +ErrorInstallNvVarStoreFormatted:
>>gBS->UninstallMultipleProtocolInterfaces (>Handle,
>>   ,
>>   ,
>> diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf 
>> b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf
>> index 117fe8b..fd3f2f7 100644
>> --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf
>> +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf
>> @@ -63,6 +63,7 @@
>>UefiRuntimeServicesTableLib
>>
>>  [Guids]
>> +  gEdkiiNvVarStoreFormattedGuid
>>gEfiAuthenticatedVariableGuid
>>gEfiEventVirtualAddressChangeGuid
>>gEfiSystemNvDataFvGuid
>> @@ -84,8 +85,4 @@
>>gMarvellTokenSpaceGuid.PcdSpiMemoryBase
>>
>>  [Depex]
>> -  #
>> -  # MvFvbDxe must be loaded before VariableRuntimeDxe in case empty
>> -  # flash needs populating with default values.
>> -  #
>> -  BEFORE gVariableRuntimeDxeFileGuid
>> +  gEfiCpuArchProtocolGuid
>> --
>> 2.7.4
>>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel