Bug 1217704 - ovmf: reproducible builds problem in ovmf-riscv64-code.bin
Summary: ovmf: reproducible builds problem in ovmf-riscv64-code.bin
Status: RESOLVED FIXED
Alias: None
Product: openSUSE Tumbleweed
Classification: openSUSE
Component: Other (show other bugs)
Version: Current
Hardware: Other All
: P5 - None : Normal (vote)
Target Milestone: ---
Assignee: Joey Lee
QA Contact: E-mail List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 1047218
  Show dependency treegraph
 
Reported: 2023-11-30 14:34 UTC by Bernhard Wiedemann
Modified: 2024-05-03 17:01 UTC (History)
1 user (show)

See Also:
Found By: Development
Services Priority:
Business Priority:
Blocker: ---
Marketing QA Status: ---
IT Deployment: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Bernhard Wiedemann 2023-11-30 14:34:09 UTC
While working on reproducible builds for ALP + openSUSE:Factory, I found that
our ovmf package always varies in
/usr/share/qemu/ovmf-riscv64-code.bin probably created by
python3 /home/abuild/rpmbuild/BUILD/edk2-edk2-stable202308/BaseTools/BinWrappers/PosixLike/../../Source/Python/build/build.py -D SECURE_BOOT_ENABLE -D TPM2_ENABLE -D TPM2_CONFIG_ENABLE -D NETWORK_IP6_ENABLE -D NETWORK_HTTP_BOOT_ENABLE -a RISCV64 -p OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc -b DEBUG -t GCC5
Comment 1 Joey Lee 2024-03-27 05:30:18 UTC
I have branched openSUSE:Factory/ovmf to my home project and triggered rebuilding two times. The issue can be reproduced:

RISC-V ovmf:

joeyli@linux-691t:~/tmp/tmp/riscv-ovmf/qemu-uefi-riscv64-202308-Virt.1699.275.1> sha256sum usr/share/qemu/ovmf-riscv64-code.bin
83930b0dfc5250b5810d7b34f432c9f36c581b0ecba419800ae4ebe74c550ad1  usr/share/qemu/ovmf-riscv64-code.bin

joeyli@linux-691t:~/tmp/tmp/riscv-ovmf/qemu-uefi-riscv64-202308-Virt.1699.275.2> sha256sum usr/share/qemu/ovmf-riscv64-code.bin
0197aff5b92fb91473b7be60328337e15c5644017d59b590db2f7b1de5ba6868  usr/share/qemu/ovmf-riscv64-code.bin                <-- hash is different

X64 ovmf:

joeyli@linux-691t:~/tmp/tmp/riscv-ovmf/qemu-ovmf-x86_64-202308-Virt.1699.275.1> sha256sum usr/share/qemu/ovmf-x86_64-code.bin
57549cdcbeab5cfb6302d8684b3563dfc54f62737b7d9667e97af6b6517449a4  usr/share/qemu/ovmf-x86_64-code.bin

joeyli@linux-691t:~/tmp/tmp/riscv-ovmf/qemu-ovmf-x86_64-202308-Virt.1699.275.2> sha256sum usr/share/qemu/ovmf-x86_64-code.bin
57549cdcbeab5cfb6302d8684b3563dfc54f62737b7d9667e97af6b6517449a4  usr/share/qemu/ovmf-x86_64-code.bin           <-- hash is the same

I am looking at it.
Comment 2 Joey Lee 2024-04-02 08:23:43 UTC
Confirmed that the RealTimeClockRuntimeDxe module in DXEFV.Fv causes problem. The RealTimeClockRuntimeDxe binary is changed in every building.
I am checking the detail.
Comment 3 Joey Lee 2024-04-09 12:32:34 UTC
decompiler RealTimeClock.efi and found that only one instruction is changed:

joeyli@localhost:~/source_code-git/edk2/Build/RiscVVirtQemu/DEBUG_GCC5/RISCV64/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe/OUTPUT> diff RealTimeClock.efi.objdump-D ~/RealTimeClock.efi.objdump-D
3027c3027
<     3524:     14b78793                addi    a5,a5,331 # 0x6615014b
---
>     3524:     f3a78793                addi    a5,a5,-198 # 0x6614ff3a

Found that the code assigned BUILD_EPOCH seconds:

joeyli@localhost:~/source_code-git/edk2> objdump -S ./Build/RiscVVirtQemu/DEBUG_GCC5/RISCV64/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe/DEBUG/RealTimeClock.dll | less

000000000000343e <LibGetTime>:
EFIAPI
LibGetTime (
  OUT EFI_TIME               *Time,
  OUT EFI_TIME_CAPABILITIES  *Capabilities
  )
{
    343e:       711d                    addi    sp,sp,-96
[...snip]
    // The following is intended to produce a compilation error on build
    // environments where BUILD_EPOCH can not be set from inline shell.
    // If you are attempting to use this library on such an environment, please
    // contact the edk2 mailing list, so we can try to add support for it.
    //
    EpochSeconds = BUILD_EPOCH;                    // issue statement
    3520:       661507b7                lui     a5,0x66150
    3524:       14b78793                addi    a5,a5,331 # 6615014b <.Ldebug_info0+0x6613884a>      
    3528:       fcf43023                sd      a5,-64(s0)
    DEBUG ((
    352c:       fffff097                auipc   ra,0xfffff
...

The BUILD_EPOCH number be set by date command when building which is defined in VirtualRealTimeClockLib.inf:

EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.inf

# Current usage of this library expects GCC in a UNIX-like shell environment with the date command
[BuildOptions]
  GCC:*_*_*_CC_FLAGS = -DBUILD_EPOCH=`date +%s`

That's why the binary code is always changed when compiler. x86_64 doesn't has this problem.
Comment 4 Joey Lee 2024-04-09 12:34:07 UTC
For reproducible, my plan is using "1970-01-01 00:00" as the build epoch:

diff --git a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.inf b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.inf
index 5d0f867eb6..c76a0ef9a9 100644
--- a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.inf
+++ b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.inf
@@ -34,4 +34,4 @@
 
 # Current usage of this library expects GCC in a UNIX-like shell environment with the date command
 [BuildOptions]
-  GCC:*_*_*_CC_FLAGS = -DBUILD_EPOCH=`date +%s`
+  GCC:*_*_*_CC_FLAGS = -DBUILD_EPOCH=`date --date="1970-01-01 00:00 UTC" +%s`
Comment 5 Bernhard Wiedemann 2024-04-10 13:35:02 UTC
That is equivalent to

-  GCC:*_*_*_CC_FLAGS = -DBUILD_EPOCH=`date +%s`
+  GCC:*_*_*_CC_FLAGS = -DBUILD_EPOCH=0

but you could also use
 -DBUILD_EPOCH=${SOURCE_DATE_EPOCH:-0}

which uses a variable that is consistently set by the build env
and falls back to 0 if unavailable.
Comment 6 Joey Lee 2024-04-11 06:55:10 UTC
(In reply to Bernhard Wiedemann from comment #5)
> That is equivalent to
> 
> -  GCC:*_*_*_CC_FLAGS = -DBUILD_EPOCH=`date +%s`
> +  GCC:*_*_*_CC_FLAGS = -DBUILD_EPOCH=0
> 
> but you could also use
>  -DBUILD_EPOCH=${SOURCE_DATE_EPOCH:-0}
> 
> which uses a variable that is consistently set by the build env
> and falls back to 0 if unavailable.

It's good idea! I will put to RISC-V ovmf.
Comment 7 Joey Lee 2024-04-11 09:05:24 UTC
(In reply to Joey Lee from comment #6)
> (In reply to Bernhard Wiedemann from comment #5)
> > That is equivalent to
> > 
> > -  GCC:*_*_*_CC_FLAGS = -DBUILD_EPOCH=`date +%s`
> > +  GCC:*_*_*_CC_FLAGS = -DBUILD_EPOCH=0
> > 
> > but you could also use
> >  -DBUILD_EPOCH=${SOURCE_DATE_EPOCH:-0}
> > 
> > which uses a variable that is consistently set by the build env
> > and falls back to 0 if unavailable.
> 
> It's good idea! I will put to RISC-V ovmf.

Unfortunately the ${} can not be used in *.inf file:

/home/joeyli/source_code-git/edk2/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c:89:20: error: expected expression before ';' token
   89 |     EpochSeconds = BUILD_EPOCH;
      |                    ^

The BUILD_EPOCH can not be set when using ${SOURCE_DATE_EPOCH}
Comment 8 Joey Lee 2024-04-11 13:16:18 UTC
(In reply to Joey Lee from comment #7)
> (In reply to Joey Lee from comment #6)
> > (In reply to Bernhard Wiedemann from comment #5)
> > > That is equivalent to
> > > 
> > > -  GCC:*_*_*_CC_FLAGS = -DBUILD_EPOCH=`date +%s`
> > > +  GCC:*_*_*_CC_FLAGS = -DBUILD_EPOCH=0
> > > 
> > > but you could also use
> > >  -DBUILD_EPOCH=${SOURCE_DATE_EPOCH:-0}
> > > 
> > > which uses a variable that is consistently set by the build env
> > > and falls back to 0 if unavailable.
> > 
> > It's good idea! I will put to RISC-V ovmf.
> 
> Unfortunately the ${} can not be used in *.inf file:
> 
> /home/joeyli/source_code-git/edk2/EmbeddedPkg/Library/
> VirtualRealTimeClockLib/VirtualRealTimeClockLib.c:89:20: error: expected
> expression before ';' token
>    89 |     EpochSeconds = BUILD_EPOCH;
>       |                    ^
> 
> The BUILD_EPOCH can not be set when using ${SOURCE_DATE_EPOCH}

OK, using printenv with date is good for taking SOURCE_DATE_EPOCH before running date:

+  GCC:*_*_*_CC_FLAGS = -DBUILD_EPOCH=`printenv SOURCE_DATE_EPOCH || date +%s`

Then we can set SOURCE_DATE_EPOCH in spec file before building RISC-V ovmf.

I will send patch to edk2 maineline.
Comment 9 Joey Lee 2024-04-11 16:25:08 UTC
Hi Bernhard,

I have added patch to RISC-V ovmf for supporting SOURCE_DATE_EPOCH. Then we should set SOURCE_DATE_EPOCH in ovmf.spec before building image.

Do you have any suggestion for the value of SOURCE_DATE_EPOCH. Do we just use SOURCE_DATE_EPOCH=`date +%s` or "1970-01-01 00:00". At least we should set a fixed date/time to SOURCE_DATE_EPOCH?

Thanks!
Comment 10 Bernhard Wiedemann 2024-04-12 04:00:14 UTC
OBS already sets SOURCE_DATE_EPOCH for Leap/SLE-15 and later to the last date in .changes, so do not need to do anything in the .spec.
Comment 11 Joey Lee 2024-04-12 05:46:31 UTC
(In reply to Bernhard Wiedemann from comment #10)
> OBS already sets SOURCE_DATE_EPOCH for Leap/SLE-15 and later to the last
> date in .changes, so do not need to do anything in the .spec.

Thanks! I have sent change to openSUSE:Factory/ovmf.
Comment 12 Joey Lee 2024-04-12 07:27:50 UTC
(In reply to Joey Lee from comment #11)
> (In reply to Bernhard Wiedemann from comment #10)
> > OBS already sets SOURCE_DATE_EPOCH for Leap/SLE-15 and later to the last
> > date in .changes, so do not need to do anything in the .spec.
> 
> Thanks! I have sent change to openSUSE:Factory/ovmf.

I have filed submit request on upstream:

https://github.com/tianocore/edk2/pull/5550
Comment 13 Joey Lee 2024-05-03 07:10:55 UTC
(In reply to Joey Lee from comment #11)
> (In reply to Bernhard Wiedemann from comment #10)
> > OBS already sets SOURCE_DATE_EPOCH for Leap/SLE-15 and later to the last
> > date in .changes, so do not need to do anything in the .spec.
> 
> Thanks! I have sent change to openSUSE:Factory/ovmf.

The ovmf-EmbeddedPkg-Library-Support-SOURCE_DATE_EPOCH-in-Vir.patch be merged to factory. 

I am still waiting upstream for reviewing.
Comment 14 Bernhard Wiedemann 2024-05-03 17:01:12 UTC
Let's count it as fixed.