Jump to content
ErmaC

Clover Bug/Issue Report and Patch

2,792 posts in this topic

Recommended Posts

On 5/19/2018 at 7:59 PM, Zenith432 said:

@Slice:

How about making Patches_for_UDK2018 using "svn cp" from Patches_for_EDK2 and using that for UDK2018 while keeping Patches_for_EDK2 for EDK2?

 

PS: If it's acceptable, I can make the change.

Yes, acceptable.

Share this post


Link to post
Share on other sites
Advertisement
20 hours ago, Zenith432 said:

@apianti:

Not everything can be handled this way, because some of the patches are to include files that are included in many project in EDK2, not to just to local source files.  Moreover, keeping the patches in EDK2 tree makes the patches easy to keep up to date with git-rebase, but I'm the only one doing that.  If patches are moved to Clover tree, they need more complex merge.

 

No, you've misunderstood. Take the entire source module and move it into clover, modify with the patches, then use that module instead of whatever module was originally being used from the edk. It's so that you don't need to use patches at all, you can easily diff and rebase only that module, I'm confused as to why it would be any different....? It's certainly what you're supposed to do when using the edk or it wouldn't have library classes where you can choose different modules to satisfy the class reference. Clover does this for some modules already so unsure why it wouldn't do so for every module that it needs patched, you can also have a variable when defined build with the unpatched edk modules instead as well.

Share this post


Link to post
Share on other sites
3 hours ago, apianti said:

 

No, you've misunderstood. Take the entire source module and move it into clover, modify with the patches, then use that module instead of whatever module was originally being used from the edk. It's so that you don't need to use patches at all, you can easily diff and rebase only that module, I'm confused as to why it would be any different....? It's certainly what you're supposed to do when using the edk or it wouldn't have library classes where you can choose different modules to satisfy the class reference. Clover does this for some modules already so unsure why it wouldn't do so for every module that it needs patched, you can also have a variable when defined build with the unpatched edk modules instead as well.

Yes, it is a way. Take all headers from EDK2 and place them into Clover/Include. Take all Libraries and all Modules, including cross dependencies. 

It just requires efforts.

Share this post


Link to post
Share on other sites
On 5/21/2018 at 8:27 AM, Slice said:

Take all headers from EDK2 and place them into Clover/Include. Take all Libraries and all Modules, including cross dependencies.

includepath for a module is taken from .dec file specified in .inf file for module.  It is why PeCoffLib.h ended up being patched in edk2 tree, remember?  So headers cannot be moved under Clover unless all modules that use them also get moved and their .inf file modified.

Share this post


Link to post
Share on other sites
35 minutes ago, Zenith432 said:

includepath for a module is taken from .dec file specified in .inf file for module.  It is why PeCoffLib.h ended up being patched in edk2 tree, remember?  So headers cannot be moved under Clover unless all modules that use them also get moved and their .inf file modified.

Yes, all modules and all libraries should be modified.

I partially made the job. DUET is a part of Clover tree now being very old and modified version instead of patches_for_recent_duet.

 

32 minutes ago, STLVNUB said:

Why doesn't Clover go with the flow and use EDK2 as is, no need for patches IMHO

EDK2 as is? You are laughing.

Share this post


Link to post
Share on other sites
3 hours ago, Slice said:

EDK2 as is? You are laughing.

I went quite well without commenting out ASSERTs, adding extern declarations to headers not defining that GUID, or effectively removing safety checks from functions declared "safe" in their very names while adding false comments.

But this GetBestLanguage ([...], "", [...]); hack really got to me, you should submit that.

 

On 5/20/2018 at 2:06 AM, apianti said:

A better choice would be to remove the patches all together, move the modified source modules into clover and modify the dsc/dec/inf files to use that source module instead of the one in the edks. Then it will build without having to copy patches or worry about the edk version.

 

not even Base.h was safe, do you really want to fork MdePkg? :P

 

@Zenith432 Didn't I see you submit va-args fixes on edk2-devel? Those look like the only changes that might ("might" because idk about the validity) be worth anything.

Edited by Download-Fritz

Share this post


Link to post
Share on other sites

I also have to mention that LoadImage() is allowed only from FV, not from HDD as Clover did.

There are other discrepancies with pure EDK2.

There is a bug with Thunk.s which is not correctly compiled by clang. Other clang problems still not submitted to EDK2.

There was a strange bug with SafeString which made Clover not working. We resolve it but a hacking way.

Share this post


Link to post
Share on other sites

I'm aware, that is an actual security feature implemented this way so it is not accidentially bypassed or the unsafe variant is used because it is more convenient. Except for maybe va-args and one variable assigned but not used, the rest looks like garbage.

Share this post


Link to post
Share on other sites
4 hours ago, Slice said:

There is a bug with Thunk.s which is not correctly compiled by clang.

If one follows edk2 mailing list, then there are patches in pipe to remove all non nasm assembler sources.

Edited by nms
missing words

Share this post


Link to post
Share on other sites
20 hours ago, Download-Fritz said:

@Zenith432 Didn't I see you submit va-args fixes on edk2-devel? Those look like the only changes that might ("might" because idk about the validity) be worth anything.

That was a long time ago - there was a bug of iterating a VA_LIST twice without intervening VA_END+reVA_START, plus other (less-lethal) cases of not using VA_END properly.

The more recent submits were fixes to problems in BaseTools.

The problem with GetBestLanguage/VariableGetBestLanguage are fixed in EDK2 by using -Wno-varargs globally with clang.  This isn't such a good idea, because it silences all clang -Wvarargs which may detect potential bugs.  However, the package maintainer seems content with it - and fixing all cases in EDK2 of disallowed type of 2nd argument to VA_START requires a change in the official PEI documentation.  I'm also unsure that the solution in Clover of naming the first variadic parameter works in all cases in EDK2, because the first variadic parameter may also have a type which is not a result of default arument promotion (this by itself is also UB :)).  I don't have patience to sort it all out.

Anyways, I recently changed the implementation in Clover of GetBestLanguage so adding the redundant "" is not needed.  The named (formerly first variadic) parameter is processed along all the variadic.

Edited by Zenith432

Share this post


Link to post
Share on other sites
That was a long time ago - there was a bug of iterating a VA_LIST twice without intervening VA_END+reVA_START, plus other (less-lethal) cases of not using VA_END properly.
The more recent submits were fixes to problems in BaseTools.
The problem with GetBestLanguage/VariableGetBestLanguage are fixed in EDK2 by using -Wno-varargs globally with clang.  This isn't such a good idea, because it silences all clang -Wvarargs which may detect potential bugs.  However, the package maintainer seems content with it - and fixing all cases in EDK2 of disallowed type of 2nd argument to VA_START requires a change in the official PEI documentation.  I'm also unsure that the solution in Clover of naming the first variadic parameter works in all cases in EDK2, because the first variadic parameter may also have a type which is not a result of default arument promotion (this by itself is also UB ).  I don't have patience to sort it all out.
Anyways, I recently changed the implementation in Clover of GetBestLanguage so adding the redundant "" is not needed.  The named (formerly first variadic) parameter is processed along all the variadic.
Oh sorry, I meant the changes in Base.h which had something about "MS no builtin vaargs" (don't have access atm). GetBestLanguage will be fixed once the PI spec is updated.

Share this post


Link to post
Share on other sites
1 hour ago, Download-Fritz said:

Oh sorry, I meant the changes in Base.h which had something about "MS no builtin vaargs" (don't have access atm). GetBestLanguage will be fixed once the PI spec is updated.

I see lgao4 just fixed it...

Quote

77ca824c (origin/master, origin/HEAD) MdePkg/IndustryStandard: Add header file for SPMI ACPI table
1d4f84f9 BaseTools/Workspace: Fix ValueChain set
180ac200 MdeModulePkg Variable: Fix XCODE5 varargs warning
cb96e7d4 IntelFrameworkPkg UefiLib: Fix XCODE5 varargs warning
d2aafe1e MdePkg UefiLib: Fix XCODE5 varargs warning

And you also patched something in 1d4f84.

EDK2 fixed it by changing the BOOLEAN to UINTN.

Anyhow, EDK2 doesn't need the changes I made to MdePkg/Include/Base.h.

They have a macro NO_MSABI_VA_FUNCS to disable the __builtin_ms_va_* variants.  It is used for XCOODE5 in tools_def.template.

In GCC, __builtin_ms_va_* always work the same (as Microsoft) whether you use mixed ABI or not.

In clang, with x86_64-windows-macho, they can't be used except on some specific older versions identified in Clover mod to Base.h.  The implementation of __builtin_va_* works (= is STDC compliant), but is incompatible with Microsoft.

In Clover, we have XCODE5 which uses x86_64-windows-macho, and XCODE8 which uses x86_64-apple-darwin, and also some of the macOS support apps shipped with Clover end up including Base.h even though they're not EDK2 EFI binaries.  They also use x86_64-apple-darwin.  So Clover Base.h needs to handle cases that EDK2 doesn't.

Edited by Zenith432

Share this post


Link to post
Share on other sites

  • Recently Browsing   0 members

    No registered users viewing this page.

  • Similar Content

    • By zebity
      Hi Hypervisors,
       
      I have been working to move my Late 2009 Xserve MacOS Server onto Ubuntu 18.04 LTS QEMU/KVM based virtual machine using OVMF and Clover.
       
      After much effort and testing based on the following information from: Kraxel's, Kholia, Gordon Turner and Clover site:
      https://www.kraxel.org/blog/2017/09/running-macos-as-guest-in-kvm/
      https://github.com/kholia/OSX-KVM
      https://gist.github.com/gordonturner/2a2e5ecde5e7860b52e2
      https://clover-wiki.zetam.org/Home
       
      I have now managed to get OVMF/Clover boot and install of MacOS Sierra.
       
      I started with Ubuntu 16.04 initially but this required download and compile of QEMU to get update of machines to: pc-q35-2.9 or better, so I moved to Ubuntu 18.04, which has pc-q35.2.11 available as standard.
       
      So I am now working with standard Ubuntu 18.04 LTS based systems with following packages: kvm/qemu/libvirt/bridge-utils/ovmf/virt-manager
       
      As I want to use PCIe Passthrough for a number of PCIe card currently installed in the server (SmallTree 10GbE & Areca ARC-1883 SAS RAID) I set up my machine with linux kernel boot configuration (/etc/default/grub) of: 
      GRUB_CMDLINE_LINUX_DEFAULT="iommu=1 intel_iommu=on" (as my machine is intel VT-d based HW virtualisation)
       
      This resulted in creation of a number of iommu groups (see /sys/kernel/iommu_group directory for this and this posting for information: https://forum.level1techs.com/t/ubuntu-17-04-vfio-pcie-passthrough-kernel-update-4-14-rc1/119639 , noting that Ubuntu 18.04 LTS has kernel version: 4.15.0-22-generic so there is no need to do kernel update for iommu to work).
       
      Using bare minimal Clover config.plist:
       
       
      And following Clover UEFI drivers:
       
      I can boot MacOS, but it is very very slow... it sits on the apple boot logo for over a minutes before finally moving onto the progress bar.
      The other problem I have is that no PCI or Network devices appear in the "About This Mac" System Information Report and I cannot get any network connectivity from bridged E1000 network device.
       
      Here is my virtlib.xml dump for the virtual machine:
       

      Can any one advise of whether I need to update the CLOVER config.plist to add extra items in to allow network and PCI Passthrough to work.
       
      Thank you.
       
      Regards,
       
      Zebity
       
    • By frankiee
      Hi there, time for my first guide after asking so many things
       
      Updated for newer Clover versions and with additional instructions for more apps: gdisk and RU.
       
      So what may be overlooked sometimes in the whole boot loader discussion is the ability of UEFI to do more than just loading an OS. This is why I see ".efi" files as "apps" because in fact they just are, built on a special kind of API: UEFI. So, you can actually do stuff like editing, diagnosing, testing certain stuff at the lowest possible system level without booting to any OS.
       
      DISCLAIMER: Use this guide at your own risk! For educational purposes only! Should do not any harm, but remember you are dealing with rather low level stuff. But at least for me everything went flawless.
       
      Prerequisites:
      Working clover installation Clover Configurator or some app to edit the config.plist  
      1) Memtest
       
      This is how to install the UEFI version of Memtest with Clover:
      Download memtest, I used this link: Image for creating boot-able USB Drive Open the archive by double clicking on it. Mount the file memtest86-usb.img within by double clicking again. Now, you should see a folder named EFI in this volume, containing a folder BOOT. Open it. The files containing it are the actual app, in 32 and 64 bit versions. Think in almost all cases we want to use the 64 bit version. Now, mount your EFI partition, using Clover configurator. Create a folder named MemTest86 in the EFI folder on your main drive. Copy all files in the EFI/BOOT folder from the mounted image to your newly created folder. That ends the basic setup! Now, we add the correct Menu entry to the clover boot menu, so that we can actually start this app.
       
      Get into clover configurator, open your standard config.plist from your boot drive and go to the GUI section. Under "Custom Entries", add an entry like this:
       

       
      Please note while the path you enter is not case sensitive, you must make sure you use the backslash "\" for entering paths, and not a slash. So basically what we do is tell clover where it can find the app, give it a name, and tell clover it is a "Windows" app. Note: with older Clover versions we had to set this to "Linux", but for now "Windows" is the setting that works.
       
      If you prefer to do a manual config, add this entry to your config.plist, under GUI/Custom/Entries:
      <dict> <key>CustomLogo</key> <true/> <key>Disabled</key> <false/> <key>FullTitle</key> <string>MemTest86</string> <key>Hidden</key> <false/> <key>Ignore</key> <false/> <key>Image</key> <string>\EFI\CLOVER\themes\Icons\os_mint.png</string> <key>Path</key> <string>\EFI\MemTest86\BOOTX64.efi</string> <key>Type</key> <string>Windows</string> <key>VolumeType</key> <string>Internal</string> </dict> Now, save and reboot and you should see the newly created entry. Note that I also used a custom Icon - this is optional - in case you don't you will just see a generic windows icon instead:
       

       
      And this is how it looks like:
       

       

       
      PS: also noticeably faster and you get also more features with this version than with the older version, for example it does now write a log file. For more Information, see: http://www.passmark.com/forum/showthread.php?4315-Memtest86-Version-5-Beta-%28Pure-UEFI%29
       
       
      2) gdisk
       
      Another app you can use is the UEFI version of gdisk (aka GPT fdisk), which you can use to edit and change your partitions.
       
      Warning! Install and use this app at your own risk! If you do something wrong here you might nuke your drive(s), so only use that if you know what you are doing.
       
      This is how to do it (even easier):
      Download here: https://sourceforge.net/projects/gptfdisk/files/gptfdisk/1.0.1/gdisk-binaries/gdisk-efi-1.0.1.zip/download Mount your EFI partition Unzip the file and copy the resulting folder to your EFI Folder, so your path goes like this "/Volumes/EFI/EFI/gdisk-efi" Get into clover configurator, open your standard config.plist from your boot drive and go to the GUI section. Add an entry like this:  
      If you prefer to do a manual config, add this entry to your config.plist, under GUI/Custom/Entries:
      <dict> <key>CustomLogo</key> <false/> <key>Disabled</key> <false/> <key>FullTitle</key> <string>GDisk</string> <key>Hidden</key> <false/> <key>Ignore</key> <false/> <key>Image</key> <string></string> <key>Path</key> <string>\EFI\gdisk-efi\gdisk_x64.efi</string> <key>Type</key> <string>Windows</string> <key>VolumeType</key> <string>Internal</string> </dict> When everything went OK you should see this after selecting the respective entry in Clover:
       

       
       
      3) RU
       
      I recently found one more app that might be interesting to some, and thats "Read Universal" or in short "RU". This is a tool that enables you to debug your BIOS and read (and modify!) every imaginable data, including UEFI variables, ACPI Tables etc.
       
      Warning, again! I think blindly messing around with this app is even more dangerous, but on the other hand it gives you access to a lot of potentially interesting stuff.
      Download here: http://ruexe.blogspot.de/ The next steps are basically the same as above so make a folder in your EFI partition, copy the .EFI files and make another custom entry in your config.plist Here are some screens:
       

       

       

       
       
      So this is what you also can do with UEFI and clover. I am curious which other useful applications might exist!
       
      Anybody who knows, maybe post it here?
    • By TimNeumann
      MB: Asus X99 A-II
      RAM: G.Skill Ripjaws V F4-3200C16D-32GVK (3200MHz, 4x 16GB DIMMs, so Total 64GB) in quad-channel configuration.
      CPU: i7 6800k @4.0Ghz
      GPUs: GTX 1080 Ti and GTX 1060 3GB
      (each one a monitor, because I can't get DVI to run parallel with DP on the 1080Ti, and my second monitor only has DVI/VGA)
      OS: High Sierra 10.13.2 (17C88) / Clover revision: 4392
       
      Hello everybody,
      I occasionally have system crashes and suspect that it is related to RAM. However, this RAM is completely new, and a run with MemTest86+ from the USB stick showed no errors. However, if I use Memtest on MacOS, I get a bunch of error messages, but sometimes none (I restart the computer between tests).  
      As I just discovered, my RAM is not on the recommended vendor list of my motherboard, nor is it on the manufacturer side of the RAM. I have read several times that the RAM may not have been tested by the manufacturer on this platform. Not a single X99 motherboard is listed for this RAM.
      So I also suspect that my RAM is not compatible.
      But if that were the case, how could it sometimes work completely error-free?
       
      During RAM tests in summer 2017 (a slightly different setup, 4x8GB but still G.Skill Ripjaws V) I had no errors at Memtest under MacOS, if I remember correctly. At that time it ran under Sierra 10.12.6. AptioMemoryFix.efi could also cause problems.
      I am appreciative for any help!
       
      Here is an excerpt from a faulty test.
      It is noticeable that it is always the same address during the test, but a different one after each test.
      Memtest version 4.22 (64-bit) Copyright (C) 2004 Charles Cazabon Copyright (C) 2004-2008 Tony Scaminaci (Macintosh port) Licensed under the GNU General Public License version 2 only Mac OS X 10.13.2 (17C88) running in multiuser mode Memory Page Size: 4096 System has 12 Intel core(s) with SSE Requested memory: 57232MB (60012990464 bytes) Available memory: 57232MB (60012990464 bytes) Allocated memory: 57232MB (60012990464 bytes) at local address 0x00000001033e1000 Attempting memory lock... locked successfully Partitioning memory into 2 comparison buffers... Buffer A: 28616MB (30006495232 bytes) starts at local address 0x00000001033e1000 Buffer B: 28616MB (30006495232 bytes) starts at local address 0x00000007ffc4d800 Running 1 test sequence... (CTRL-C to quit) Test sequence 1 of 1: Running tests on full 57232MB region... Stuck Address : ok Linear PRN : testing 7 of 16 FAILURE! Data mismatch at local address 0x0000000e88133d30 Expected Data: 0xc4c340079ac9a645, Actual Data: 0xc4c3400788468846 Running comparison tests using 28616MB buffers... Random Value : ok Compare XOR : ok Compare SUB : ok Compare MUL : ok Compare DIV : ok Compare OR : ok Compare AND : ok Sequential Increment: ok Solid Bits : ok Block Sequential : testing 80 of 256 FAILURE! Data mismatch at local BUFA address 0x000000078b8c7530, BUFB address 0x0000000e88133d30 BUFA Data: 0x4f4f4f4f4f4f4f4f, BUFB Data: 0x4f4f4f4f88468846 Checkerboard : testing 3 of 64 FAILURE! Data mismatch at local BUFA address 0x000000078b8c7530, BUFB address 0x0000000e88133d30 BUFA Data: 0x5555555555555555, BUFB Data: 0x5555555588468846 Bit Spread : testing 1 of 128 FAILURE! Data mismatch at local BUFA address 0x000000078b8c7530, BUFB address 0x0000000e88133d30 BUFA Data: 0x0000000000000005, BUFB Data: 0x0000000088468846 Bit Flip : testing 74 of 512 FAILURE! Data mismatch at local BUFA address 0x000000078b8c7530, BUFB address 0x0000000e88133d30 BUFA Data: 0x0000000000000200, BUFB Data: 0x0000000088468846 Walking Ones : testing 40 of 128 Clover Files attached
      config.plist
      drivers64UEFI.zip
      kexts.zip
      ACPI.zip
    • By BurpSuite
      screenshot:
       

       
      description:
      github: https://github.com/burpsuite/clover_theme
×