On Tue, Oct 15, 2019 at 04:26:12AM +0000, Chang, Abner (HPS SW/FW Technologist)
wrote:
>
>
> > -----Original Message-----
> > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> > Abner Chang
> > Sent: Tuesday, October 15, 2019 12:03 PM
> > To: devel@edk2.groups.io; leif.lindh...@linaro.org
> > Subject: Re: [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 10/29]
> > MdePkg/BasePeCoff: Add RISC-V PE/Coff related code.
> >
> >
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> > > Leif Lindholm
> > > Sent: Friday, September 27, 2019 7:46 AM
> > > To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist)
> > > <abner.ch...@hpe.com>
> > > Subject: Re: [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 10/29]
> > > MdePkg/BasePeCoff: Add RISC-V PE/Coff related code.
> > >
> > > On Mon, Sep 23, 2019 at 08:31:36AM +0800, Abner Chang wrote:
> > > > Support RISC-V image relocation.
> > > >
> > > > Signed-off-by: Abner Chang <abner.ch...@hpe.com>
> > > > ---
> > > > MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 3 +-
> > > > MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf | 5 +
> > > > MdePkg/Library/BasePeCoffLib/BasePeCoffLib.uni | 2 +
> > > > .../Library/BasePeCoffLib/BasePeCoffLibInternals.h | 1 +
> > > > .../Library/BasePeCoffLib/RiscV/PeCoffLoaderEx.c | 142
> > > +++++++++++++++++++++
> > > > 5 files changed, 152 insertions(+), 1 deletion(-) create mode
> > > > 100644 MdePkg/Library/BasePeCoffLib/RiscV/PeCoffLoaderEx.c
> > > > diff --git a/MdePkg/Library/BasePeCoffLib/RiscV/PeCoffLoaderEx.c
> > > > b/MdePkg/Library/BasePeCoffLib/RiscV/PeCoffLoaderEx.c
> > > > new file mode 100644
> > > > index 0000000..8eb37f9
> > > > --- /dev/null
> > > > +++ b/MdePkg/Library/BasePeCoffLib/RiscV/PeCoffLoaderEx.c
> > > > @@ -0,0 +1,142 @@
> > > > +/** @file
> > > > + PE/Coff loader for RISC-V PE image
> > > > +
> > > > + Copyright (c) 2016, Hewlett Packard Enterprise Development LP.
> > > > +All rights reserved.<BR>
> > > > + SPDX-License-Identifier: BSD-2-Clause-Patent **/ #include
> > > > +"BasePeCoffLibInternals.h"
> > > > +#include <Library/BaseLib.h>
> > > > +
> > > > +//
> > > > +// RISC-V definition.
> > > > +//
> > > > +#define RV_X(x, s, n) (((x) >> (s)) & ((1<<(n))-1)) #define
> > > > +RISCV_IMM_BITS 12 #define RISCV_IMM_REACH
> > > (1LL<<RISCV_IMM_BITS)
> > > > +#define RISCV_CONST_HIGH_PART(VALUE) \
> > > > + (((VALUE) + (RISCV_IMM_REACH/2)) & ~(RISCV_IMM_REACH-1))
> > >
> > > This looked familiar, so I had a look.
> > > This block is copied around - it exists in:
> > > - BaseTools/Source/C/Common/PeCoffLoaderEx.c
> > > - BaseTools/Source/C/GenFw/Elf64Convert.c
> > > - MdePkg/Library/BasePeCoffLib/RiscV/PeCoffLoaderEx.c
> > >
> > > This needs to be moved somewhere central and included elsewhere.
> > > BaseTools and MdePkg unfortunately duplicate a lot of stuff, but this
> > > still belongs in a common header file for either.
> >
> > I can consolidate that macro in two files under BaseTools, but not
> > consolidating macro in files in both MdePkg and BaseTools. BaseTools and
> > edk2 are two separate projects and could be built individually based on my
> > understanding.
> > I have no idea how to leverage one header file from both projects and I
> > don't
> > go that far to address it.
>
> Leif, seem there is no good place and the existing header file to
> put this macro unless I create a new header file under
> BaseTools/Source/C/Include. I would like to keep this duplicate
> macro in both files rather than create an header file in which only
> define this macro. Do you have good idea?
There is never a good reason to duplicate code. You will always end up
changing one and forgetting the other.
I see no problem with creating a header file which contains nothing
else. But I also think it would be valid to put this into
C/Common/PeCoffLib.h - and this file is already included by the
affected .c files.
/
Leif
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#48997): https://edk2.groups.io/g/devel/message/48997
Mute This Topic: https://groups.io/mt/34258204/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
