[edk2-devel] [PATCH v1 1/1] RISCV: Fix InternalLongJump to return correct value

[Leif Lindholm]

On 2023-09-19 15:31, Ard Biesheuvel wrote:
> Hello Andrei,
> 
> On Tue, 19 Sept 2023 at 04:43, Andrei Warkentin
> <andrei.warkentin at intel.com> wrote:
>>
>> InternalLongJump was not returning the 2nd parameter passed
>> to LongJmp (Value) as the return value from SetJmp.
>>
>> Seen with code compiled with -Os, where an LongJmp (Buffer, -1)
>> somehow translated to SetJmp returning 0...
>>
>> Cc: Yong Li <yong.li at intel.com>
>> Cc: Sunil V L <sunilvl at ventanamicro.com>
>> Cc: Tuan Phan <tphan at ventanamicro.com>
>> Cc: Daniel Schaefer <git at danielschaefer.me>
>> Signed-off-by: Andrei Warkentin <andrei.warkentin at intel.com>
>> ---
>>   CryptoPkg/Library/OpensslLib/openssl                  | 2 +-
>>   MdePkg/Library/BaseLib/RiscV64/RiscVSetJumpLongJump.S | 7 ++-----
>>   2 files changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/CryptoPkg/Library/OpensslLib/openssl b/CryptoPkg/Library/OpensslLib/openssl
>> index de90e54bbe82..830bf8e1e474 160000
>> --- a/CryptoPkg/Library/OpensslLib/openssl
>> +++ b/CryptoPkg/Library/OpensslLib/openssl
>> @@ -1 +1 @@
>> -Subproject commit de90e54bbe82e5be4fb9608b6f5c308bb837d355
>> +Subproject commit 830bf8e1e4749ad65c51b6a1d0d769ae689404ba
> 
> This does not belong here
> 
>> diff --git a/MdePkg/Library/BaseLib/RiscV64/RiscVSetJumpLongJump.S b/MdePkg/Library/BaseLib/RiscV64/RiscVSetJumpLongJump.S
>> index 34486eabba4c..e97a7d0727b8 100644
>> --- a/MdePkg/Library/BaseLib/RiscV64/RiscVSetJumpLongJump.S
>> +++ b/MdePkg/Library/BaseLib/RiscV64/RiscVSetJumpLongJump.S
>> @@ -3,6 +3,7 @@
>>   // Set/Long jump for RISC-V
>>   //
>>   // Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
>> +// Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>
> 
> I suppose there is some internal policy at Intel that tells you to
> claim copyright, but do you really think fixing existing HP code by
> removing 4 instructions and adding one back is sufficient for claiming
> copyright on the entire file?
> 
> Note that I am not objecting to this in principle: I am just curious
> (and I have objected in the past to patches that only removed lines
> from existing code and then added a copyright line)
> 
> Should we have some project/community wide guidance on this?

I reacted early on when joining this project that copyright was 
frequently added/bumped for trivial changes, including things like 
fixing typos in comments. So I think the custom for this project is that 
the bar is a lot lower than for projects like Linux, grub, etc.

So I see this as on par for tianocore.

> (The problem is that claiming copyright gives the right to distribute
> the code without being bound by the terms of the license)

Wouldn't all the other copyright holders also need to agree?

/
     Leif

>>   //
>>   // SPDX-License-Identifier: BSD-2-Clause-Patent
>>   //
>> @@ -47,9 +48,5 @@ InternalLongJump:
>>       REG_L s10, 11*SZREG(a0)
>>       REG_L s11, 12*SZREG(a0)
>>       REG_L sp,  13*SZREG(a0)
>> -
>> -    add   a0, s0, 0
>> -    add   a1, s1, 0
>> -    add   a2, s2, 0
>> -    add   a3, s3, 0
>> +    mv    a0, a1
>>       ret
>> --
>> 2.34.1
>>
>>
>>
>> 
>>
>>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108844): https://edk2.groups.io/g/devel/message/108844
Mute This Topic: https://groups.io/mt/101450445/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3943202/1813853/130120423/xyzzy [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-