Clover Problems and Solutions

[apianti]

9 hours ago, Slice said:

OK, my changes was before your commit so why there are some conflicts. For example BOOLEAN... Usually (a<b) is already boolean. But we have old C (not C++, not C#) which has no boolean type and work with integer. I think (a>0) if better then (BOOLEAN)a.

 

3 hours ago, Download-Fritz said:

Comparisons always yield the type "int", just new compilers are smart enough to know the cast will not truncate. The only compilers I know having issues are <= VS2005, which are being dropped soon (or have they already been?) from edk2.

 

Yes, "long" is not stable across ABIs. What was the actual error? Should never resort to using non-UEFI types imo

 

Nah, it's actually like DF says, but in my edit I suggest that we use method to satisfy all, which is (BOOLEAN)(a != 0).

 

9 hours ago, Slice said:

About cbuild.bat be free to remake it. I need SVNREVISION to be known during compilation because Version.h is created and included into sources. In macOS I can use (svn info ...) but in windows I use TortoiseSVN and see no possibility to get actual revision.

cbuild.bat -r 4817

looks to be a good way to set SVNREVISION.

 

It already did that. That's what the lines that you commented out and replaced did, ran svnversion -n to get the current revision. I also use TortoiseSVN, did you not install the subversion binaries when you ran the installer? You must make a commit before the revision changes.

rem # get the current revision number
:getrevision
  cd "%CURRENTDIR%"
  rem  get svn revision number
  svnversion -n>%F_VER_TXT%
  set /P s=<%F_VER_TXT%
  rem del %F_VER_TXT%
  set SVNREVISION=

rem # get the current revision number
:fixrevision
  if ["%s%"] == [""] goto init
  set c=%s:~0,1%
  set s=%s:~1%
  if ["%c::=%"] == [""] goto init
  if ["%c:M=%"] == [""] goto init
  if ["%c:S=%"] == [""] goto init
  if ["%c:P=%"] == [""] goto init
  set SVNREVISION=%SVNREVISION%%c%
  goto fixrevision

 

9 hours ago, Slice said:

About unsigned long...

I will not make nonsense changes if not required. It was VS2013 forced me to do anything claiming an error in UINT64 usage. I quick fixed it just replacing UINT64 -> unsigned long long, and UINT32 -> unsigned long.

The second replace is good for VS2013 but again wrong for Clang. So now I replaced to unsigned int which is in agree with clang. Windows compatibility will check at Monday.

May be you may propose better way?

 

3 hours ago, Download-Fritz said:

Yes, "long" is not stable across ABIs. What was the actual error? Should never resort to using non-UEFI types imo

 

This was a typecasting error that I fixed by using the correct type casting. As DF suggests we should not be using non-UEFI types. They should be changed back, and it will be ok again. I think you did a merge with your current working copy and it caused problems with my changes.

 

9 hours ago, Slice said:

MapBiosKey. I don't remember initially why it is external. The warning said that external declaration inside the function and should be moved out of it. I did this. But then I got your commit where you just exclude it. I don't know. It is more safe to declare this function to be external. May be because of ABI.

 

Nah, you can remove the line. The problem was the function was written below where it was called and was also declared file scope by using the declaration inside of a function which is an error in visual studio without disabling it. It was just easier to remove the declaration and move the function above where it was called.

[Slice]

d:\projects\edk2\Clover\rEFIt_UEFI\Platform\AcpiPatcher.c(1539) : error C2275: 'UINT64' : illegal use of this type as an expression
        d:\projects\edk2\MdePkg\Include\X64\ProcessorBind.h(141) : see declaration of 'UINT64'

At the line 1539

UINT64 *Ptr = XsdtEntryPtrFromIndex(0);

 

[Slice]

svnversion is not installed although I made full installation of TortoiseSVN.

[apianti]

2 hours ago, Slice said:

d:\projects\edk2\Clover\rEFIt_UEFI\Platform\AcpiPatcher.c(1539) : error C2275: 'UINT64' : illegal use of this type as an expression
        d:\projects\edk2\MdePkg\Include\X64\ProcessorBind.h(141) : see declaration of 'UINT64'

At the line 1539

UINT64 *Ptr = XsdtEntryPtrFromIndex(0);

 

 

You can't declare a variable after an expression in any scope. So you need to move the definition to the beginning of the scope, most likely the function. I thought I had done that but I think you had a merge issue with my changes or something.

1 hour ago, Slice said:

svnversion is not installed although I made full installation of TortoiseSVN.

 

Hmmmm, that's weird, it works for me, are you sure the binaries were selected? I am not sure if they are by default like TortoiseGit, so you have to explicitly select them.

[Slice]

Clover compiled with VS2013 is not fully working

So there are more issues then warnings.

[apianti]

That's very strange since everything else seems to be fine including the selection. Also why would it be showing the icon not present, should it not have fallen back to the embedded theme just like the scrollbar did? There must be something wrong with the code getting the icon that is just being exposed by visual studio, I've found many bugs that way.

[Zenith432]

@Slice:

EDK2 removed MdeModulePkg/Universal/Variable/EmuRuntimeDxe in svn r28854

Instead, this functionality has been incorporated into

MdeModulePkg/Universal/Variable/RuntimeDxe

and turned on with a PCD

gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable

The change is documented here

https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Notes

including an example has to compile with this PCD set to TRUE in the .dsc file.

In addition to setting the PCD, we need to somehow rename the resulting Dxe to EmuRuntimeDxe, because we also build RuntimeDxe as a true nvram driver, and there is a name collision for building the Dxe twice, once with PCD on, once with PCD off.  Unfortunately, I don't know how to do this output rename in the .dsc file.  Please help.

[apianti]

There are three things to do here.

 

You can only change the name of a binary by editing the .inf. Create a copy of the .inf for RuntimeDxe and modify it, then rename it, put it in patches_for_edk2/MdeModulePkg/Universal/Variable/RuntimeDxe. Then in the .dsc file include the correct .inf for the module like the example. I think that should work, if you understand what I mean.

 

You can just comment out the .inf for EmuRuntimeDxe since the source was copied into the repo and is built as well, unsure why the same module is built twice but one is disregarded.

 

Build only one of the drivers by default and the other by specifying a command line switch, probably for the actual nvram driver since it's not as useful because it most likely won't work in legacy and is unneeded for UEFI since it's already supported.

[Zenith432]

7 hours ago, apianti said:

You can just comment out the .inf for EmuRuntimeDxe since the source was copied into the repo and is built as well, unsure why the same module is built twice but one is disregarded.

Clover/Protocols/EmuVariableUefi is installed by ebuild.sh as a standalone dxe for using with native UEFI firmware that has faulty nvram.

MdePkg/Universal/Variable/EmuRuntimeDxe is built in Clover.dsc and inserted by Clover.fdf into Clover EFI firmware to be used for legacy boot.

So they do different things and both are needed.

MdePkg/Universal/Variable/RuntimeDxe (the real-nvram variant) is also inserted by Clover.fdf for legacy boot.

Maybe this last one is not needed and we can just build the Emu variant of RuntimeDxe.

[apianti]

Sorry, I meant building the real nvram driver and the emulated nvram driver, sorry, was unclear, but you only want one or the other. You should edit .fdf to:

!ifdef REAL_NVRAM
  INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
!endif
  INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf

and change the .dsc to:

!ifdef REAL_NVRAM
  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
    <LibraryClasses>
      NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
  }
!else
  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
    <PcdsFixedAtBuild>
      gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable|TRUE
    <LibraryClasses>
      AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
      TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
      VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
  }
!endif

Then actually that should be a better solution than I suggested before. And won't build the same driver twice.

 

EDIT: Seems a lot of these recent edk changes are deviating the code base significantly from the previous udk. I'm not sure how much longer there won't have to be branches created if both want to be maintained because this alone will actually break building with udk right? So, there still needs to be that separate copy of the .inf for the emulated driver in patches_for_edk2 and the correct .inf names above, but there may be no need to modify the .fdf then.

Edited February 21, 2019 by apianti

[Slice]

There was an idea to include VariableRuntimeDxe into CloverEFI supposing it will work with real NVRAM on UEFI BIOS computer if start with legacy Clover. 

But I think nobody need it.

 

May be we should forget EDK2 and stay with UDK2018? What achievements we can expect from new EDK2? Except new bugs.

Or we can monitor some security fixes in EDK2 and apply them to our Patches_for_UDK2018.

[Zenith432]

Problem with EmuRuntimeDxe having been removed in edk2 svn r28854 is resolved in Clover r4884 based on apianti's suggestion.  Works for both EDK2 and UDK2018 (tested both).  There is no need for gloom about maintaining compatibility with both.

cbuild.bat still needs to be changed to do something similar to what ebuild.sh change in r4884 is doing.

 

EDK2 have began tagging their git build every 3 months to designate stable releases

edk2-stable201808

edk2-stable201811

 

So in the future I can stick to updating Patches_for_EDK2 only for the versions with these tags.

 

PS: I'm not sure whether Clover's use of UDK2018 is officially based on the tag vUDK2018 which is the initial release of UDK2018, or branch UDK2018 which is occasionally updated with cherry picks from master, but not very often because its last update was in Sep 2018.

Edited February 22, 2019 by Zenith432

[mhaeuser]

One cannot just use the edk2 "real" NVRAM driver, somewhere in the chain it requires implementation of the chipset-specific (well, more or less stable) SPI commands. There is not generic implementation that could be embedded into CloverEFI

[Slice]

3 hours ago, Zenith432 said:

PS: I'm not sure whether Clover's use of UDK2018 is officially based on the tag vUDK2018 which is the initial release of UDK2018, or branch UDK2018 which is occasionally updated with cherry picks from master, but not very often because its last update was in Sep 2018.

I am just too lazy to update UDK2018 staying at initial release. It still works.

May be there is a reason to update drivers for SATA, NVME, USB, XHCI but usually new computers uses such drivers from UEFI BIOS and old legacy computers uses our old drivers. There must be careful testing for updating them as EDK2 often breaks working codes.

[apianti]

I believe that staying on EDK2 is the correct course and trying to maintain compatibility with the previous udk for as long as possible because eventually they will release a new udk, and it will be based on edk2. Do we plan on never having any new features or bug/security fixes? Or are we just going to create a massive patches repo, lol? Also, I agree with DF that the nvram driver is not useful unless you also have a device specific SPI flash driver.... However, the intel spec for spi flash is open and there is a linux driver for intel SPI flash that could easily ported, that would support most users since that driver can be pretty generic and support a large amount of cpus/chipsets (since haswell pulled all these device onto the cpu instead of the pch).

 

EDIT: Truthfully, it may be possible to take the nvram source and modify it to check if there is a flash protocol, if so use real nvram, if not use emulated nvram.

Edited March 1, 2019 by apianti

[apianti]

@Slice

Hey, are you able to build the newest edk versions in windows? I can build the 32bit but not the 64bit, I keep getting an error that the include Nasm.inc can't be found no matter which version of VS I use to build:

        "C:\Program Files (x86)\Microsoft Visual Studio 12.0\Vc\bin\x86_amd64\cl.exe" /nologo /E /TC /FIAutoGen.h /Ic:\development\edk2\MdePkg\Library\BaseLib\X64  /Ic:\development\edk2\MdePkg\Library\BaseLib  /Ic:\development\edk2\Build\Clover\RELEASE_VS2013x86\X64\MdePkg\Library\BaseLib\BaseLib\DEBUG  /Ic:\development\edk2\MdePkg  /Ic:\development\edk2\MdePkg\Include  /Ic:\development\edk2\MdePkg\Include\X64 c:\development\edk2\MdePkg\Library\BaseLib\X64\LongJump.nasm > c:\development\edk2\Build\Clover\RELEASE_VS2013x86\X64\MdePkg\Library\BaseLib\BaseLib\OUTPUT\X64\LongJump.i
LongJump.nasm
        Trim --trim-long --source-code -o c:\development\edk2\Build\Clover\RELEASE_VS2013x86\X64\MdePkg\Library\BaseLib\BaseLib\OUTPUT\X64\LongJump.iii c:\development\edk2\Build\Clover\RELEASE_VS2013x86\X64\MdePkg\Library\BaseLib\BaseLib\OUTPUT\X64\LongJump.i
        "C:\Program Files\NASM\nasm" -Ic:\development\edk2\MdePkg\Library\BaseLib\X64\ -Ox -f win64 -o c:\development\edk2\Build\Clover\RELEASE_VS2013x86\X64\MdePkg\Library\BaseLib\BaseLib\OUTPUT\X64\LongJump.obj c:\development\edk2\Build\Clover\RELEASE_VS2013x86\X64\MdePkg\Library\BaseLib\BaseLib\OUTPUT\X64\LongJump.iii
c:\development\edk2\Build\Clover\RELEASE_VS2013x86\X64\MdePkg\Library\BaseLib\BaseLib\OUTPUT\X64\LongJump.iii:22: fatal: unable to open include file `Nasm.inc'
NMAKE : fatal error U1077: '"C:\Program Files\NASM\nasm.EXE"' : return code '0x1'
Stop.


build.py...
 : error 7000: Failed to execute command
        C:\Program Files (x86)\Microsoft Visual Studio 12.0\Vc\bin\nmake.exe /nologo tbuild [c:\development\edk2\Build\Clover\RELEASE_VS2013x86\X64\MdePkg\Library\BaseLib\BaseLib]


build.py...
 : error F002: Failed to build module
        c:\development\edk2\MdePkg\Library\BaseLib\BaseLib.inf [X64, VS2013x86, RELEASE]

Clearly, all the include paths are not being passed to nasm. When did this happen?

 

EDIT: I can build 64bit but not 32bit in macOS... lol.

Edited March 3, 2019 by apianti

[Zenith432]

@apianti:

You need the change to build_rule.txt that I synced in r4893, but I only tested it works on macOS and Linux, don't know about Windows

@@ -62,7 +62,8 @@
 #   $(BIN_DIR)          Common directory for executable files
 #   $(FV_DIR)           Directory to store flash image files
 #   $(INC)              Search path of current module
-#   $(INC_LIST)         A file containing search pathes of current module
+#   $(NASM_INC)         Search nasm file path of current module
+#   $(INC_LIST)         A file containing search paths of current module
 #   $(LIBS)             Static library files of current module
 #   $(<tool>_FLAGS)     Tools flags of current module
 #   $(MODULE_NAME)      Current module name
@@ -229,7 +230,7 @@
     <Command>
         @"$(PP)" $(PP_FLAGS) $(INC) ${src} > ${d_path}(+)${s_base}.i
         @Trim --trim-long --source-code -o ${d_path}(+)${s_base}.iii ${d_path}(+)${s_base}.i
-        @"$(NASM)" -I${s_path}(+) $(NASM_FLAGS) -o $dst ${d_path}(+)${s_base}.iii
+        @"$(NASM)" -I${s_path}(+) $(NASM_INC) $(NASM_FLAGS) -o $dst ${d_path}(+)${s_base}.iii
         @"echo" [NASM] ${s_base}
 
 [Device-Tree-Source-File]

The variable NASM_INC is set by the python script

BaseTools/Source/Python/AutoGen/GenMake.py

and Nasm.inc is under

MdePkg/Include/<Arch>

 

so you need to figure this out for Windows.

[apianti]

Oh ok, I see the problem is with the windows build itself. Why is it so messed up now? It used to work then what is this crazy template double configuration thing, and why does none of the post build work anymore? Annoyed.

 

EDIT: Now, I get build errors for PCDs not being defined for X64 and IA32 in windows and IA32 in macOS.

Edited March 3, 2019 by apianti

[Slice]

I can build Clover in windows with VS2013 toolset and with nasm

D:\Projects\edk2\Clover>nasm -v
NASM version 2.13.03 compiled on Feb  7 2018

D:\Projects\edk2\Clover>

But I have an error with svnversion so why I corrected cbuild.bat script to not use this command.

\Build\Clover\RELEASE_VS2013x86\X64\Clover\rEFIt_UEFI\refit\OUTPUT
d:\projects\edk2\Build\Clover\RELEASE_VS2013x86\X64\Clover\rEFIt_UEFI\refit\DEBUG\CLOVER.map
Скопировано файлов:         1.
        copy /y d:\projects\edk2\Build\Clover\RELEASE_VS2013x86\X64\Clover\rEFIt_UEFI\refit\DEBUG\*.pdb d:\projects\edk2\Build\Clover\RELEASE_VS2013x86\X64\Clover\rEFIt_UEFI\refit\OUTPUT
d:\projects\edk2\Build\Clover\RELEASE_VS2013x86\X64\Clover\rEFIt_UEFI\refit\DEBUG\*.pdb
НСе удается найти указанный файл.
копировано файлов:         0.

Generating DUETEFIMAINFVX64 FV
##########
GUID cross reference file can be found at d:\projects\edk2\Build\Clover\RELEASE_VS2013x86\FV\Guid.xref

FV Space Information
DUETEFIMAINFVX64 [61%Full] 2752512 total, 1693288 used, 1059224 free

- Done -
Build end time: 10:26:41, Mar.04 2019
Build total time: 00:00:38


Performing post build operations ...

Compressing DUETEFIMainFv.FV (X64) ...
Compressing DxeMain.efi (X64) ...
Compressing DxeIpl.efi (X64) ...
Generating Loader Image (X64) ...
Created D:\Projects\edk2\Build\Clover\RELEASE_VS2013x86\FV\Efildr64
warning: boot file bigger than low-ebda permits, switching to --std-ebda

Start copying:

Mandatory (UEFI) drivers:

-> "FSInject-64.efi"
-> "OsxFatBinaryDrv-64.efi"
-> "VBoxHfs-64.efi"

Optional (UEFI) drivers:

-> "CsmVideoDxe-64.efi"
-> "DataHubDxe-64.efi"
-> "EmuVariableUefi-64.efi"
-> "OsxAptioFixDrv-64.efi"
-> "OsxLowMemFixDrv-64.efi"
-> "PartitionDxe-64.efi"

Optional drivers:

-> "NvmExpressDxe-64.efi"
-> "Ps2MouseDxe-64.efi"
-> "UsbMouseDxe-64.efi"
-> "VBoxIso9600-64.efi"
-> "VBoxExt2-64.efi"
-> "VBoxExt4-64.efi"
-> "XhciDxe-64.efi"

CloverEFI + Applications:

-> "boot6"
-> "bdmesg-64.efi"
-> "CLOVERX64.efi"
-> "BOOTX64.efi"

End copying ...

Generating BootSectors ...

"make" не является внутренней или внешней
командой, исполняемой программой или пакетным файлом.

### Build dir: "D:\Projects\edk2\Build\Clover\RELEASE_VS2013x86"
### EFI dir: "D:\Projects\edk2\Clover\CloverPackage\CloverV2\EFI"

Done!

D:\Projects\edk2\Clover>

 

[Slice]

To make NASM working I have to change cbuild.bat with my pathes

set DEFAULT_CYGWIN_HOME=D:\Projects\edk2\BaseTools\Bin\CYGWIN_NT-5.1-i686
set DEFAULT_PYTHONHOME=D:\Projects\edk2\BaseTools\Source\Python
set DEFAULT_PYTHON_FREEZER_PATH=%PYTHON_HOME%\Scripts
set DEFAULT_NASM_PREFIX=%DEFAULT_CYGWIN_HOME%\bin

And manually copy binnasm.exe into the cygwin\bin folder.

[apianti]

You are not using the script correctly. You should set the environment variables CYGWIN_HOME, PYTHONHOME, PYTHON_FREEZER_PATH, and NASM_PREFIX, those are the default values if those are not found so changing the script is changing the default install locations. You also don't need cygwin anymore, the base tools will build without it since that set of binaries was removed for legacy stuff. Also, I just realized that if you installed tortoisesvn, you may have to add the bin directory to your PATH so that the subversion binaries are found. If they aren't in there then you didn't select them, there was an update a few days ago so you can upgrade and select them to install, if needed. Setting environment variable:

https://superuser.com/questions/949560/how-do-i-set-system-environment-variables-in-windows-10

 

Now onto the fact that the build does not work in windows at all, or IA32 for any because it says there are PCDs that are undefined. They seem to be coming from NASM sources. Why does the windows build not do postbuild anymore and why does it have the weird double configuration? Who did that? Building's broken.

[Slice]

I remembered I added C:/../NASM to Path variable but I didn't set other variables.

Anyway I see no error with PCD_***.

[Slice]

18 hours ago, apianti said:

You are not using the script correctly. You should set the environment variables CYGWIN_HOME, PYTHONHOME, PYTHON_FREEZER_PATH, and NASM_PREFIX, those are the default values if those are not found so changing the script is changing the default install locations. You also don't need cygwin anymore, the base tools will build without it since that set of binaries was removed for legacy stuff. Also, I just realized that if you installed tortoisesvn, you may have to add the bin directory to your PATH so that the subversion binaries are found. If they aren't in there then you didn't select them, there was an update a few days ago so you can upgrade and select them to install, if needed. Setting environment variable:

https://superuser.com/questions/949560/how-do-i-set-system-environment-variables-in-windows-10

 

Now onto the fact that the build does not work in windows at all, or IA32 for any because it says there are PCDs that are undefined. They seem to be coming from NASM sources. Why does the windows build not do postbuild anymore and why does it have the weird double configuration? Who did that? Building's broken.

No, Tortoisesvn doesn't contains something with name "svnversion"!

As well clover build script should not depend on system pathes like CYGWIN_HOME, PYTHONHOME, PYTHON_FREEZER_PATH. It should set it!

The script should use

D:\Projects\edk2\Clover>SubWCRev.exe .
SubWCRev: 'D:\Projects\edk2\Clover'
Last committed at revision 4896
Updated to revision 4896
Local modifications found
Unversioned items found

D:\Projects\edk2\Clover>

 

[apianti]

How could it set those paths? They are actually set by the python installer, and cygwin needs that defined to function. The variables you changed aren't even those variables anyway, those are the values that are the default installation locations which those variables are set to if they are not defined. You should absolutely set those path variables. Here is svnversion included with tortoisesvn, you have to select to install the subversion binaries in the installer, they are not installed by default:

As for the PCD it only happened like a few revisions ago, and the windows build didn't even work, I had to make changes. Now it has undefined PCDs. But truthfully who modified the windows build to be completely insane? Why is it creating a templated double copy of the configuration?

[Slice]

OK, I installed TortoiseSVN with all utility and set variables in Windows. Now the script cbuild.bat begins to work but an error

NMAKE : fatal error U1077: '"c:\cygwin\binnasm' : return code '0x1'

It is probably somewhere in python script, or somewhere in NMAKE.