A set of various fixes and additions

[Ykidia]

1. Usability - added special subspecifier "h" for specifier "X" (hex values print), added corresponding menu option and application setting to UEFITool. After changing option "C-style hex values" user need to reload image file for now or restart the program to take effect.
   NS: NACK pending further discussion.

2. Parser - added some more analysis to raw files and sections: raw files can (or can not) contain sections, raw sections can contain NVAR storage or PE/TE.
   NS: NACK pending further discussion.

3. Parser - improved CPU base address detection and propagation. Now if we have a coreboot image with Intel FSP inside, or a FSP binary itself, or other part of UEFI BIOS, CPU base address more probably will be detected as for standard UEFI image.
   NS: NACK pending further discussion.

4. Parser - improved FIT recognition. FIT does not rely only on VTF found in PEI phase volume, so if VTF was not found, this does not mean that FIT is absent. Now if we have a coreboot image with Intel FSP inside, or some end (higher) part of UEFI BIOS, FIT will be found as for standard Intel image.
   NS: NACK pending further discussion.

5. Parser - small changes to Flash Descriptor parsing to get more cases for valid "Intel image". To get rid of cases when "Intel image" is already in tree but with parse error because of which "UEFI image" appears.
   NS: ACK pending sample images.

6. Parser - added parsing of individual UEFI-files (these can be trimmed from UEFI-volume), displaying it as "UEFI volume part".
   NS: NACK.

7. Usability - added possibility to view/save contents of elements "Free volume space", "Free space" and such, because these can be non-empty. Or they cannot, but now we can edit them, say, in hex (when editing and writing will be unlocked/refactored/restored).
   NS: NACK.

8. Settings - added storage in settings of the following paths: open image file, save image file, open GUIDs file.
   NS: Done in ebf4f83

9. Usability - added last opened files list.
   NS: Done in ebf4f83

10. Usability - added permanent opened file name string to the end of the status bar.
    NS: NACK.

11. Usability - added opened file changes tracking: if the file was modified in other program while it is opened in UEFITool, there are 3 ways to act: a) ignore changes (but mark file path displayed in the status bar with italic font); b) ask user to reopen or ignore (if ignore, mark as in a); c) auto reopen changed file in UEFITool. If changes were in some way ignored, file path displayed in the right of the status bar will be marked with italic font and then become clickable: on click request to reopen appears again.
    NS: NACK.

12. Internals - switched to offset/size instead of byte array storing in each tree item, which results in significant reduction in memory consumption (espesially with latest large images).
    NS: NACK.

13. Debug - added icons to key tree items (compressed and with contents, contents now must be in root item only).
    NS: NACK.

14. Usability - added expanding tree on open image (to depth level 1) and by menu command (expand all).
    NS: Done in ebf4f83 (only the level 1 part)

[NikolajSchlej]

This needs a test file and making the 8 into non-magical value.

[Ykidia]

I do not remember in details, what this about for, but I remember I've experimented with FF-ing maximum in Flash Descriptor, and when masterBase FF-ed, hardware continues to startup normally.

[NikolajSchlej]

Let's discuss the changes 1 by 1. CC @vit9696 for his input.

[NikolajSchlej]

Usability - added special subspecifier "h" for specifier "X" (hex values print), added corresponding menu option and application setting to UEFITool. After changing option "C-style hex values" user need to reload image file for now or restart the program to take effect.

NACK, unclear what problem is getting fixed here, other than "I copy some values from UEFITool and can't paste it into other tools because they don't start with 0x, but have trailing h".
This format was originally chosen to take less screen space, because 0xABCD and other prefix notation for not-base10 numbers in C came from K&R compiler not being able to correctly handle postfixes.
We can discuss replacing the current format with 0xABCD for the sake of "this is how it looks like in most other tools anyway", but it doesn't need to be an option.

[NikolajSchlej]

Usability - fixed QHexView misalignment in Windows and macOS.

Alternative solution to this is adoption of the upcoming QHexView 5.1 that uses direct render. Please open a separate issue with screenshots showing the mentioned misalignment and the fact that your changes fixes it. Notionally ACK when screenshots are provided.

UPD: fixed by f64ba09

[NikolajSchlej]

Parser - added some more analysis to raw files and sections: raw files can (or can not) contain sections, raw sections can contain NVAR storage or PE/TE.

NACK as not needed at the current stage of development.
All parsing of RAW files is ultimately optional and needs to only be done to achieve defined goals or fix existing bugs. RAW is explicitly "please do not parse further, here be dragons" ask by the UEFI spec, and every kind of parser needs to be accomplished with the very same kind of builder when the builder code is written. Adding more parsers of this sort can only be done after that builder code is done and we have barely anything else to improve.

[NikolajSchlej]

Parser - improved CPU base address detection and propagation. Now if we have a coreboot image with Intel FSP inside, or a FSP binary itself, or other part of UEFI BIOS, CPU base address more probably will be detected as for standard UEFI image.
Parser - improved FIT recognition. FIT does not rely only on VTF found in PEI phase volume, so if VTF was not found, this does not mean that FIT is absent. Now if we have a coreboot image with Intel FSP inside, or some end (higher) part of UEFI BIOS, FIT will be found as for standard Intel image.

NACK for now. The need of this is debatable. coreboot images do not confirm to UEFI PI specification, and UEFITool is a tool to parse and edit images confirming to UEFI PI specification. However, we already moved away from the spec by adding support for Intel descriptor (editing of which had not been supported nor planned though, same is true for ME and NVRAM that are getting parsed in NE), there's currently work on adding AMD descriptor support, and an issue regarding Qualcomm images.
If we are to support coreboot images, it's better to create an issue about that, and add FIT detection for such images as part of that effort.

[NikolajSchlej]

Usability - ME region not displayed if it is in unknown format, fixed this because we still want to operate with it.

Also debatable, but OK, I buy it, ACK. UPD: done.

[NikolajSchlej]

Parser - small changes to Flash Descriptor parsing to get more cases for valid "Intel image". To get rid of cases when "Intel image" is already in tree but with parse error because of which "UEFI image" appears.

Sample image(s) or did not happen. Also, this needs a separate issue. TBH, I've seen this bug already, once, and planned to fix it, but haven't found an image and forgot about it. Glad it resurfaced. Notionally ACK if test images are provided.

[NikolajSchlej]

Parser - added parsing of individual UEFI-files (these can be trimmed from UEFI-volume), displaying it as "UEFI volume part".

NACK, the input of UEFITool is a binary confirming with UEFI PI specification, or a wrapper around such an image. If the input doesn't have FFS volumes - it's invalid, no need to parse anything in it further, files or not.

[NikolajSchlej]

Usability - added possibility to view/save contents of elements "Free volume space", "Free space" and such, because these can be non-empty. Or they cannot, but now we can edit them, say, in hex (when editing and writing will be unlocked/refactored/restored).

NACK for now, as Free Space is an item deliberately made uneditable and guaranteed to be free, otherwise it would be called Padding and has a type. There's no need to view any of it (as it's empty) and no need to save it (obtaining an empty file of a given size is a trivial one-liner). We can return to this when we move Hex Preview Window into the main UI, then viewing Free Space item will become necessary.

[NikolajSchlej]

Usability - added info about blocks number and block size (with preliminary and stupid validity check) to volume info.

ACK, UPD: done.

[NikolajSchlej]

Settings - added storage in settings of the following paths: open image file, save image file, open GUIDs file.

ACK if it passively saves the paths without adding anything to the UI, NACK otherwise.

[NikolajSchlej]

Usability - added last opened files list.

Considered implementing that way back in 2014, bailed because the list was ugly on macOS back then. Maybe it's less ugly now, will check. Notionally ACK.

[NikolajSchlej]

Usability - added opened file changes tracking: if the file was modified in other program while it is opened in UEFITool, there are 3 ways to act: a) ignore changes (but mark file path displayed in the status bar with italic font); b) ask user to reopen or ignore (if ignore, mark as in a); c) auto reopen changed file in UEFITool. If changes were in some way ignored, file path displayed in the right of the status bar will be marked with italic font and then become clickable: on click request to reopen appears again.

NACK, unclear what problem this resolves right now. UEFITool is currently not something that support editing an image on the fly, it's a batch processor that takes a file and outputs a file (0.2x that is, NE doesn't output anything yet). We can return to adding that when we add the new builder.

[NikolajSchlej]

Internals - switched to offset/size instead of byte array storing in each tree item, which results in significant reduction in memory consumption (espesially with latest large images).

NACK, Abso-FUCKING-lutely not, excuse me my expression here. We will start doing memory optimization when we are pressed by it, and having copies of all objects handy and separated from one another is an actual design decision way back in UEFITool 0.3.1 times, that I'm not ready to negotiate. We are trading memory consumption (memory is super-cheap and is always a plenty) with simplicity and security (both are super-expensive in practice), and this tradeoff only needs to be made when an actual memory/performance pressure is observed. So far there were zero complaints.

[NikolajSchlej]

Debug - added icons to key tree items (compressed and with contents, contents now must be in root item only).

NACK, as icons in the tree are ugly in different OSes, and we don't need it without the previous change.

[NikolajSchlej]

Usability - added expanding tree on open image (to depth level 1) and by menu command (expand all).

ACK

[Ykidia]

Usability - added special subspecifier "h" for specifier "X" (hex values print), added corresponding menu option and application setting to UEFITool. After changing option "C-style hex values" user need to reload image file for now or restart the program to take effect.

NACK, unclear what problem is getting fixed here, other than "I copy some values from UEFITool and can't paste it into other tools because they don't start with 0x, but have trailing h". This format was originally chosen to take less screen space, because 0xABCD and other prefix notation for not-base10 numbers in C came from K&R compiler not being able to correctly handle postfixes. We can discuss replacing the current format with 0xABCD for the sake of "this is how it looks like in most other tools anyway", but it doesn't need to be an option.

Yes, "I copy some values from UEFITool and can't paste it into other tools because they don't start with 0x, but have trailing h". I have to work for a long time paired with a hex-editor, I often need to copy values ​​from UEFITool to the hex-editor, and constant rewriting suffix "h" to prefix "0x", sometimes in a small screen through a remote - yes, it is very, very inconvenient! Much more conveniently a simple double click selecting ready-to-use value.
I think, UEFITool is a tool for reverse engineering, first of all. Because first and main function - read and examine in details. General users will not use it simply "to see something", therefore they do not need to copy-paste hex values (to programs that not always understand old assembler notation), and they also do not need items information and the program window at all. These users even do not know, what all of it does mean, therefore they does not need to copy-paste hex values...
But profile users run the program many-many times, with other good programs, to make their reversing and experimenting tasks, and not all of these other good programs can understand other notations than "0x".
Just give a choice to the user - you can make in the setting as it used to be in the program, and how is generally accepted for now.

[Ykidia]

Parser - improved CPU base address detection and propagation. Now if we have a coreboot image with Intel FSP inside, or a FSP binary itself, or other part of UEFI BIOS, CPU base address more probably will be detected as for standard UEFI image.
Parser - improved FIT recognition. FIT does not rely only on VTF found in PEI phase volume, so if VTF was not found, this does not mean that FIT is absent. Now if we have a coreboot image with Intel FSP inside, or some end (higher) part of UEFI BIOS, FIT will be found as for standard Intel image.

NACK for now. The need of this is debatable. coreboot images do not confirm to UEFI PI specification, and UEFITool is a tool to parse and edit images confirming to UEFI PI specification. However, we already moved away from the spec by adding support for Intel descriptor (editing of which had not been supported nor planned though, same is true for ME and NVRAM that are getting parsed in NE), there's currently work on adding AMD descriptor support, and an issue regarding Qualcomm images. If we are to support coreboot images, it's better to create an issue about that, and add FIT detection for such images as part of that effort.

So, after all, it was not only about coreboot, it was only one example of several, why it was needed. Okay, I repeat. If I open Intel FSP (I have many variants of the latest platform FSP, and rebased too), I know for sure that all of its components of those TE/PE are agreed in advance, so that the base CPU address is unambiguously calculated. Similarly with other pieces of BIOS, where the code executed "in" the ROM. Forgot about coreboot. If I am (re)constructing (when write and editing will resurrected) some piece of BIOS and base CPU address is "not detected because we do not want do this in alternative ways", then any TE/PE moving will rebase it to 0 or not rebase at all.That's why I wanted to add cases when CPU address is normally detected.

[Ykidia]

Settings - added storage in settings of the following paths: open image file, save image file, open GUIDs file.

ACK if it passively saves the paths without adding anything to the UI, NACK otherwise.

Yes, it saves the paths silently/transparently.

[Ykidia]

Useful images for Birch Stream platform:

1. Gigabyte, motherboard: https://www.gigabyte.com/ru/Enterprise/Server-Motherboard/MS34-CP0-rev-1x, BIOS (file image.bin in "SPI_UPD" subdir inside archive): https://download.gigabyte.com/FileList/BIOS/MS34-CP0_F16.zip?v=132ebbd548928cfcf7f72a22bf6c29ea?v=132ebbd548928cfcf7f72a22bf6c29ea
2. Supermicro, motherboard (or similar): https://www.supermicro.com/en/products/motherboard/x14sbm-tf, BIOS (file BIOS_X14SBM-1D44_20250124_1.2_STDsp.zip inside archive): https://www.supermicro.com/en/support/resources/downloadcenter/firmware/MBD-X14SBM-TF/BIOS

[vit9696]

I generally agree with @NikolajSchlej on the decisions, but to review this we really need separate PRs. Good to have it all in one for reference, but to start merging this we would rather go on one-by-one basis.

NACK, unclear what problem this resolves right now. UEFITool is currently not something that support editing an image on the fly, it's a batch processor that takes a file and outputs a file (0.2x that is, NE doesn't output anything yet). We can return to adding that when we add the new builder.

My Hex Editor offers this:

So I can copy and paste basically anything. I do not really have a problem with UEFITool copied contents, but perhaps we can indeed switch to 0x as a more common format?

NACK, unclear what problem this resolves right now. UEFITool is currently not something that support editing an image on the fly, it's a batch processor that takes a file and outputs a file (0.2x that is, NE doesn't output anything yet). We can return to adding that when we add the new builder.

I think this is about editing image and making sure the changes are correct. I believe what we need here is CMD+R / F5 to work. I.e. a way to reload currently opened image. This is a rather natural thing in browsers / file editors.

[NikolajSchlej]

I do not intend to merge the whole thing, but I promised to @Ykidia via email to organize splitting this effort into smaller chunks myself, so we can continue discussion of everything that is currently NACK here for now.

[NikolajSchlej]

@Ykidia, it looks like several changes are not included in the pull request, i.e. everything regarding UI additions, settings and so on. Those should be in uefitool.cpp, uefitool_main.cpp, uefitool.ui and alike, and there are no changes to the mentioned files here.

[Ykidia]

@Ykidia, it looks like several changes are not included in the pull request, i.e. everything regarding UI additions, settings and so on. Those should be in uefitool.cpp, uefitool_main.cpp, uefitool.ui and alike, and there are no changes to the mentioned files here.

My bad. Fixed (PR).

[NikolajSchlej]

@Ykidia, everything @vit9696 and I agreed to take on had been taken, THANK YOU.

Please continue working on your fork as you see fit, because you certainly have some interesting ideas on how to improve the tool, even if they aren't immediately useful to the mainline.

I will close this PR because we will not be applying it as is, but I promise to think some more about the stuff we NACKed, and maybe bring those one by one, informing you every time it happens.

Looking forward to your future contributions.