Bug 1217704

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.

Confirmed that the RealTimeClockRuntimeDxe module in DXEFV.Fv causes problem. The RealTimeClockRuntimeDxe binary is changed in every building.
I am checking the detail.

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.

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`

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.

(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.

(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}

(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.

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!

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.

(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.

(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

(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.

Let's count it as fixed.