public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Leif Lindholm <leif.lindholm@linaro.org>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: edk2-devel-01 <edk2-devel@lists.01.org>,
	"Cohen, Eugene" <eugene@hp.com>, Laszlo Ersek <lersek@redhat.com>
Subject: Re: [PATCH 03/26] ArmPkg/AsmMacroIoLib: remove unused obsolete MMIO and other asm macros
Date: Thu, 11 Aug 2016 09:23:21 +0100	[thread overview]
Message-ID: <20160811082321.GN31760@bivouac.eciton.net> (raw)
In-Reply-To: <CAKv+Gu-OfpC8CHTBVq5Vk=KrQ-n-w7udsH54utE3+n=X1Jg-gA@mail.gmail.com>

On Wed, Aug 10, 2016 at 07:26:46PM +0200, Ard Biesheuvel wrote:
> On 10 August 2016 at 19:04, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Wed, Aug 10, 2016 at 05:17:39PM +0200, Ard Biesheuvel wrote:
> >> This removes the various Mmio ASM macros that are not used anywhere in
> >> the code, and removes some variants of LoadConstant... () that are not
> >> used anywhere either.
> >
> > If you say something about how the Mmio* functions are redundant due
> > to the MdePkg implementations:
> 
> No, they are not. These are asm implementations, and completely unused.

Yes, but due to the unfortunate naming of these macros conflicting
with real functions existing in IoLib, this patch looks potentially
horrifying at first glance. Hence I would prefer that to be mentioned
in the commit message.

/
    Leif

> > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> >
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> ---
> >>  ArmPkg/Include/AsmMacroIoLib.h   | 213 --------------------
> >>  ArmPkg/Include/AsmMacroIoLib.inc |  54 -----
> >>  2 files changed, 267 deletions(-)
> >>
> >> diff --git a/ArmPkg/Include/AsmMacroIoLib.h b/ArmPkg/Include/AsmMacroIoLib.h
> >> index f94dcc619f7a..551b87803d19 100644
> >> --- a/ArmPkg/Include/AsmMacroIoLib.h
> >> +++ b/ArmPkg/Include/AsmMacroIoLib.h
> >> @@ -24,88 +24,6 @@
> >>  //  ldr reg, =expr does not work with current Apple tool chain. So do the work our selves
> >>  //
> >>
> >> -// returns _Data in R0 and _Address in R1
> >> -#define MmioWrite32(_Address, _Data) \
> >> -  ldr  r1, [pc, #8]     ;            \
> >> -  ldr  r0, [pc, #8]     ;            \
> >> -  str  r0, [r1]         ;            \
> >> -  b    1f               ;            \
> >> -  .long (_Address)      ;            \
> >> -  .long (_Data) ;                    \
> >> -1:
> >> -
> >> -// returns _Data in R0 and _Address in R1, and _OrData in r2
> >> -#define MmioOr32(_Address, _OrData) \
> >> -  ldr  r1, [pc, #16]    ;           \
> >> -  ldr  r2, [pc, #16]    ;           \
> >> -  ldr  r0, [r1]         ;           \
> >> -  orr  r0, r0, r2       ;           \
> >> -  str  r0, [r1]         ;           \
> >> -  b    1f               ;           \
> >> -  .long (_Address)      ;           \
> >> -  .long (_OrData)       ;           \
> >> -1:
> >> -
> >> -// returns _Data in R0 and _Address in R1, and _OrData in r2
> >> -#define MmioAnd32(_Address, _AndData) \
> >> -  ldr  r1, [pc, #16]    ;             \
> >> -  ldr  r2, [pc, #16]    ;             \
> >> -  ldr  r0, [r1]         ;             \
> >> -  and  r0, r0, r2       ;             \
> >> -  str  r0, [r1]         ;             \
> >> -  b    1f               ;             \
> >> -  .long (_Address)      ;             \
> >> -  .long (_AndData)       ;             \
> >> -1:
> >> -
> >> -// returns result in R0, _Address in R1, and _OrData in r2
> >> -#define MmioAndThenOr32(_Address, _AndData, _OrData)  \
> >> -  ldr  r1, [pc, #24]    ;                             \
> >> -  ldr  r0, [r1]         ;                             \
> >> -  ldr  r2, [pc, #20]    ;                             \
> >> -  and  r0, r0, r2       ;                             \
> >> -  ldr  r2, [pc, #16]    ;                             \
> >> -  orr  r0, r0, r2       ;                             \
> >> -  str  r0, [r1]         ;                             \
> >> -  b    1f               ;                             \
> >> -  .long (_Address)      ;                             \
> >> -  .long (_AndData)      ;                             \
> >> -  .long (_OrData)       ;                             \
> >> -1:
> >> -
> >> -// returns _Data in _Reg and _Address in R1
> >> -#define MmioWriteFromReg32(_Address, _Reg) \
> >> -  ldr  r1, [pc, #4]     ;                  \
> >> -  str  _Reg, [r1]       ;                  \
> >> -  b    1f               ;                  \
> >> -  .long (_Address)      ;                  \
> >> -1:
> >> -
> >> -
> >> -// returns _Data in R0 and _Address in R1
> >> -#define MmioRead32(_Address)   \
> >> -  ldr  r1, [pc, #4]     ;      \
> >> -  ldr  r0, [r1]         ;      \
> >> -  b    1f               ;      \
> >> -  .long (_Address)      ;      \
> >> -1:
> >> -
> >> -// returns _Data in Reg and _Address in R1
> >> -#define MmioReadToReg32(_Address, _Reg) \
> >> -  ldr  r1, [pc, #4]     ;               \
> >> -  ldr  _Reg, [r1]       ;               \
> >> -  b    1f               ;               \
> >> -  .long (_Address)      ;               \
> >> -1:
> >> -
> >> -
> >> -// load R0 with _Data
> >> -#define LoadConstant(_Data)  \
> >> -  ldr  r0, [pc, #0]     ;    \
> >> -  b    1f               ;    \
> >> -  .long (_Data)         ;    \
> >> -1:
> >> -
> >>  // load _Reg with _Data
> >>  #define LoadConstantToReg(_Data, _Reg)  \
> >>    ldr  _Reg, [pc, #0]   ;               \
> >> @@ -113,91 +31,8 @@
> >>    .long (_Data)         ;               \
> >>  1:
> >>
> >> -// load _Reg with _Data if eq
> >> -#define LoadConstantToRegIfEq(_Data, _Reg)  \
> >> -  ldreq  _Reg, [pc, #0]   ;                 \
> >> -  b    1f                 ;                 \
> >> -  .long (_Data)           ;                 \
> >> -1:
> >> -
> >> -// Reserve a region at the top of the Primary Core stack
> >> -// for Global variables for the XIP phase
> >> -#define SetPrimaryStack(StackTop, GlobalSize, Tmp)  \
> >> -  and     Tmp, GlobalSize, #7         ;             \
> >> -  rsbne   Tmp, Tmp, #8                ;             \
> >> -  add     GlobalSize, GlobalSize, Tmp ;             \
> >> -  sub     sp, StackTop, GlobalSize    ;             \
> >> -                                      ;             \
> >> -  mov     Tmp, sp                     ;             \
> >> -  mov     GlobalSize, #0x0            ;             \
> >> -_SetPrimaryStackInitGlobals:          ;             \
> >> -  cmp     Tmp, StackTop               ;             \
> >> -  beq     _SetPrimaryStackEnd         ;             \
> >> -  str     GlobalSize, [Tmp], #4       ;             \
> >> -  b       _SetPrimaryStackInitGlobals ;             \
> >> -_SetPrimaryStackEnd:
> >> -
> >> -// Initialize the Global Variable with '0'
> >> -#define InitializePrimaryStack(GlobalSize, Tmp1)    \
> >> -  and     Tmp1, GlobalSize, #7        ;             \
> >> -  rsbne   Tmp1, Tmp1, #8              ;             \
> >> -  add     GlobalSize, GlobalSize, Tmp1 ;            \
> >> -                                      ;             \
> >> -  mov     Tmp1, sp                    ;             \
> >> -  sub     sp, GlobalSize              ;             \
> >> -  mov     GlobalSize, #0x0            ;             \
> >> -_InitializePrimaryStackLoop:          ;             \
> >> -  cmp     Tmp1, sp                    ;             \
> >> -  bls     _InitializePrimaryStackEnd  ;             \
> >> -  str     GlobalSize, [Tmp1, #-4]!    ;             \
> >> -  b       _InitializePrimaryStackLoop ;             \
> >> -_InitializePrimaryStackEnd:
> >> -
> >>  #elif defined (__GNUC__)
> >>
> >> -#define MmioWrite32(Address, Data) \
> >> -  ldr  r1, =Address ;              \
> >> -  ldr  r0, =Data    ;              \
> >> -  str  r0, [r1]
> >> -
> >> -#define MmioOr32(Address, OrData) \
> >> -  ldr  r1, =Address ;             \
> >> -  ldr  r2, =OrData  ;             \
> >> -  ldr  r0, [r1]     ;             \
> >> -  orr  r0, r0, r2   ;             \
> >> -  str  r0, [r1]
> >> -
> >> -#define MmioAnd32(Address, AndData) \
> >> -  ldr  r1, =Address ;               \
> >> -  ldr  r2, =AndData ;               \
> >> -  ldr  r0, [r1]     ;               \
> >> -  and  r0, r0, r2   ;               \
> >> -  str  r0, [r1]
> >> -
> >> -#define MmioAndThenOr32(Address, AndData, OrData) \
> >> -  ldr  r1, =Address ;                             \
> >> -  ldr  r0, [r1]     ;                             \
> >> -  ldr  r2, =AndData ;                             \
> >> -  and  r0, r0, r2   ;                             \
> >> -  ldr  r2, =OrData  ;                             \
> >> -  orr  r0, r0, r2   ;                             \
> >> -  str  r0, [r1]
> >> -
> >> -#define MmioWriteFromReg32(Address, Reg) \
> >> -  ldr  r1, =Address ;                    \
> >> -  str  Reg, [r1]
> >> -
> >> -#define MmioRead32(Address) \
> >> -  ldr  r1, =Address ;       \
> >> -  ldr  r0, [r1]
> >> -
> >> -#define MmioReadToReg32(Address, Reg) \
> >> -  ldr  r1, =Address ;                 \
> >> -  ldr  Reg, [r1]
> >> -
> >> -#define LoadConstant(Data) \
> >> -  ldr  r0, =Data
> >> -
> >>  #define LoadConstantToReg(Data, Reg) \
> >>    ldr  Reg, =Data
> >>
> >> @@ -209,59 +44,11 @@ _InitializePrimaryStackEnd:
> >>  //  Less magic in the macros if ldr reg, =expr works
> >>  //
> >>
> >> -// returns _Data in R0 and _Address in R1
> >> -
> >> -
> >> -
> >> -#define MmioWrite32(Address, Data) MmioWrite32Macro Address, Data
> >> -
> >> -
> >> -
> >> -
> >> -// returns Data in R0 and Address in R1, and OrData in r2
> >> -#define MmioOr32(Address, OrData) MmioOr32Macro Address, OrData
> >> -
> >> -
> >> -// returns _Data in R0 and _Address in R1, and _OrData in r2
> >> -
> >> -
> >> -#define MmioAnd32(Address, AndData)  MmioAnd32Macro Address, AndData
> >> -
> >> -// returns result in R0, _Address in R1, and _OrData in r2
> >> -
> >> -
> >> -#define MmioAndThenOr32(Address, AndData, OrData) MmioAndThenOr32Macro Address, AndData, OrData
> >> -
> >> -
> >> -// returns _Data in _Reg and _Address in R1
> >> -
> >> -
> >> -#define MmioWriteFromReg32(Address, Reg) MmioWriteFromReg32Macro Address, Reg
> >> -
> >> -// returns _Data in R0 and _Address in R1
> >> -
> >> -
> >> -#define MmioRead32(Address)  MmioRead32Macro Address
> >> -
> >> -// returns _Data in Reg and _Address in R1
> >> -
> >> -
> >> -#define MmioReadToReg32(Address, Reg) MmioReadToReg32Macro Address, Reg
> >> -
> >> -
> >> -// load R0 with _Data
> >> -
> >> -
> >> -#define LoadConstant(Data)  LoadConstantMacro Data
> >> -
> >>  // load _Reg with _Data
> >>
> >>
> >>  #define LoadConstantToReg(Data, Reg)  LoadConstantToRegMacro Data, Reg
> >>
> >> -// conditional load testing eq flag
> >> -#define LoadConstantToRegIfEq(Data, Reg)  LoadConstantToRegIfEqMacro Data, Reg
> >> -
> >>  #endif
> >>
> >>  #endif
> >> diff --git a/ArmPkg/Include/AsmMacroIoLib.inc b/ArmPkg/Include/AsmMacroIoLib.inc
> >> index 95dc640d6fc3..c9cad5230c94 100644
> >> --- a/ArmPkg/Include/AsmMacroIoLib.inc
> >> +++ b/ArmPkg/Include/AsmMacroIoLib.inc
> >> @@ -17,60 +17,6 @@
> >>
> >>
> >>    MACRO
> >> -  MmioWrite32Macro $Address, $Data
> >> -  ldr  r1, = ($Address)
> >> -  ldr  r0, = ($Data)
> >> -  str  r0, [r1]
> >> -  MEND
> >> -
> >> -  MACRO
> >> -  MmioOr32Macro $Address, $OrData
> >> -  ldr  r1, =($Address)
> >> -  ldr  r2, =($OrData)
> >> -  ldr  r0, [r1]
> >> -  orr  r0, r0, r2
> >> -  str  r0, [r1]
> >> -  MEND
> >> -
> >> -  MACRO
> >> -  MmioAnd32Macro $Address, $AndData
> >> -  ldr  r1, =($Address)
> >> -  ldr  r2, =($AndData)
> >> -  ldr  r0, [r1]
> >> -  and  r0, r0, r2
> >> -  str  r0, [r1]
> >> -  MEND
> >> -
> >> -  MACRO
> >> -  MmioAndThenOr32Macro $Address, $AndData, $OrData
> >> -  ldr  r1, =($Address)
> >> -  ldr  r0, [r1]
> >> -  ldr  r2, =($AndData)
> >> -  and  r0, r0, r2
> >> -  ldr  r2, =($OrData)
> >> -  orr  r0, r0, r2
> >> -  str  r0, [r1]
> >> -  MEND
> >> -
> >> -  MACRO
> >> -  MmioWriteFromReg32Macro $Address, $Reg
> >> -  ldr  r1, =($Address)
> >> -  str  $Reg, [r1]
> >> -  MEND
> >> -
> >> -  MACRO
> >> -  MmioRead32Macro $Address
> >> -  ldr  r1, =($Address)
> >> -  ldr  r0, [r1]
> >> -  MEND
> >> -
> >> -  MACRO
> >> -  MmioReadToReg32Macro $Address, $Reg
> >> -  ldr  r1, =($Address)
> >> -  ldr  $Reg, [r1]
> >> -  MEND
> >> -
> >> -  MACRO
> >>    LoadConstantMacro $Data
> >>    ldr  r0, =($Data)
> >>    MEND
> >> --
> >> 2.7.4
> >>


  reply	other threads:[~2016-08-11  8:23 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-10 15:17 [PATCH 00/26] ARM assembler cleanup series Ard Biesheuvel
2016-08-10 15:17 ` [PATCH 01/26] ArmLib: remove ArmReplaceLiveTranslationEntry() implementation Ard Biesheuvel
2016-08-10 16:56   ` Leif Lindholm
2016-08-10 17:31     ` Ard Biesheuvel
2016-08-10 15:17 ` [PATCH 02/26] ArmPkg: add missing ArmMmuLib resolution to ArmPkg.dsc Ard Biesheuvel
2016-08-10 15:17 ` [PATCH 03/26] ArmPkg/AsmMacroIoLib: remove unused obsolete MMIO and other asm macros Ard Biesheuvel
2016-08-10 17:04   ` Leif Lindholm
2016-08-10 17:26     ` Ard Biesheuvel
2016-08-11  8:23       ` Leif Lindholm [this message]
2016-08-10 15:17 ` [PATCH 04/26] ArmPlatformPkg RVCT: drop dependency on GCC macro library Ard Biesheuvel
2016-08-10 15:17 ` [PATCH 05/26] ArmPkg: introduce ASM_FUNC, MOV32/MOV64 and ADRL/LDRL macros Ard Biesheuvel
2016-08-10 18:26   ` Cohen, Eugene
2016-08-10 18:29     ` Ard Biesheuvel
2016-08-10 18:48       ` Cohen, Eugene
2016-08-10 15:17 ` [PATCH 06/26] ArmVirt/PrePi: make jump to CEntryPoint relative Ard Biesheuvel
2016-08-10 15:17 ` [PATCH 07/26] ArmVirtPkg: clean up assembly source files Ard Biesheuvel
2016-08-10 15:17 ` [PATCH 08/26] ArmPkg/ArmSmcLibNull: move to generic C implementation Ard Biesheuvel
2016-08-10 15:17 ` [PATCH 09/26] ArmPkg/ArmCpuLib: switch to ASM_FUNC() asm macro Ard Biesheuvel
2016-08-10 15:17 ` [PATCH 10/26] ArmPkg/ArmGicV3: " Ard Biesheuvel
2016-08-10 15:17 ` [PATCH 11/26] ArmPkg/ArmHvcLib: " Ard Biesheuvel
2016-08-10 15:17 ` [PATCH 12/26] ArmPkg/ArmLib: " Ard Biesheuvel
2016-08-10 19:00   ` Leif Lindholm
2016-08-10 15:17 ` [PATCH 13/26] ArmPkg/ArmMmuLib: " Ard Biesheuvel
2016-08-10 15:17 ` [PATCH 14/26] ArmPkg/ArmSmcLib: " Ard Biesheuvel
2016-08-10 15:17 ` [PATCH 15/26] ArmPkg/BaseMemoryLibSm: " Ard Biesheuvel
2016-08-10 15:17 ` [PATCH 16/26] ArmPkg/BaseMemoryLibVstm: " Ard Biesheuvel
2016-08-10 15:17 ` [PATCH 17/26] ArmPkg/CompilerIntrinsicsLib: " Ard Biesheuvel
2016-08-10 15:17 ` [PATCH 18/26] ArmPkg/SemihostLib: " Ard Biesheuvel
2016-08-10 15:17 ` [PATCH 19/26] BeagleBoardPkg: remove unused Sec.inf module Ard Biesheuvel
2016-08-11  8:34   ` Leif Lindholm
2016-08-10 15:17 ` [PATCH 20/26] BeagleBoardPkg: add missing ArmMmuLib resolution Ard Biesheuvel
2016-08-10 15:17 ` [PATCH 21/26] ArmPlatformPkg/ArmJunoLib: switch to ASM_FUNC() asm macro Ard Biesheuvel
2016-08-11  8:37   ` Leif Lindholm
2016-08-10 15:17 ` [PATCH 22/26] ArmPlatformPkg/PrePi: " Ard Biesheuvel
2016-08-11  8:38   ` Leif Lindholm
2016-08-31  4:33     ` Michael Zimmermann
2016-08-31  9:14       ` Ard Biesheuvel
2016-08-31  9:43         ` Michael Zimmermann
2016-08-31  9:45           ` Ard Biesheuvel
2016-08-31  9:47       ` Evan Lloyd
2016-08-31  9:52         ` Michael Zimmermann
2016-08-31  9:53           ` Ard Biesheuvel
2016-09-07 11:10       ` Ryan Harkin
2016-09-07 11:59         ` Ard Biesheuvel
2016-09-07 12:18           ` Ryan Harkin
2016-08-10 15:17 ` [PATCH 23/26] ArmPlatformPkg/PrePeiCore: " Ard Biesheuvel
2016-08-11  8:38   ` Leif Lindholm
2016-08-10 15:18 ` [PATCH 24/26] ArmPlatformPkg/ArmVExpressPkg: " Ard Biesheuvel
2016-08-11  8:39   ` Leif Lindholm
2016-08-10 15:18 ` [PATCH 25/26] ArmPlatformPkg/ArmPlatformLibNull: " Ard Biesheuvel
2016-08-11  8:39   ` Leif Lindholm
2016-08-10 15:18 ` [PATCH 26/26] ArmPlatformPkg/ArmPlatformStackLib: " Ard Biesheuvel
2016-08-11  8:42   ` Leif Lindholm
2016-08-11 10:18 ` [PATCH 00/26] ARM assembler cleanup series Leif Lindholm
2016-08-11 11:27   ` Ard Biesheuvel
2016-08-11 11:31     ` Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160811082321.GN31760@bivouac.eciton.net \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox