ArmPkg/Drivers/ArmGic/ArmGicLib.inf | 2 +- ArmPkg/Library/ArmExceptionLib/Arm/ExceptionSupport.asm | 30 +++++++++++++++++------------- ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf | 2 +- ArmPkg/Library/ArmExceptionLib/ArmRelocateExceptionLib.inf | 2 +- ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf | 2 +- ArmPkg/Library/ArmLib/Arm/ArmV7Support.asm | 6 ++++-- ArmPkg/Library/ArmLib/ArmBaseLib.inf | 8 ++++---- ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf | 4 ++-- ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf | 2 +- ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf | 2 +- ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf | 2 +- ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf | 2 +- ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf | 2 +- ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf | 6 +++--- ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf | 6 +++--- ArmPlatformPkg/PrePi/PeiMPCore.inf | 2 +- ArmPlatformPkg/PrePi/PeiUniCore.inf | 2 +- MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm | 18 +++++++++--------- MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf | 2 +- MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf | 20 ++++++++++---------- MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf | 2 +- 21 files changed, 65 insertions(+), 59 deletions(-)
We are currently making an effort to make ARM (and AARCH64 eventually) builds using Microsoft's Visual Studio Compiler (aka MSVC/MSFT). These 3 patches correspond to an effort to make the assembler work with MSFT, which entails : - Feeding MSFT the RVCT .asm files, since they share syntax requirements. - Fixing some instructions syntax in those .asm files, in order to make them palatable for MSFT. - Fixing some minor formatting issue in INF files, while we're at it. This set enables the assembler, meanwhile the C also require changes, which will come in a set later. This set makes the RVCT toolchain family and profiles obsolete, unblocking : BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1750 As mentioned in the above bug, dropping RVCT would entail orphanating the .asm files that powered the RVCT build. Since Visual Studio uses the same file syntax, those can be reused to power the VS build. These patches have been tested on VS2019 (v15.9.11) and VS2017 (v16.0.1) Baptiste GERONDEAU (3): ArmPkg/MdePkg : Unify INF files format ARM/Assembler: Correct syntax from RVCT for MSFT ARM/Assembler: Reuse RVCT assembler for MSFT build ArmPkg/Drivers/ArmGic/ArmGicLib.inf | 2 +- ArmPkg/Library/ArmExceptionLib/Arm/ExceptionSupport.asm | 30 +++++++++++++++++------------- ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf | 2 +- ArmPkg/Library/ArmExceptionLib/ArmRelocateExceptionLib.inf | 2 +- ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf | 2 +- ArmPkg/Library/ArmLib/Arm/ArmV7Support.asm | 6 ++++-- ArmPkg/Library/ArmLib/ArmBaseLib.inf | 8 ++++---- ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf | 4 ++-- ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf | 2 +- ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf | 2 +- ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf | 2 +- ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf | 2 +- ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf | 2 +- ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf | 6 +++--- ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf | 6 +++--- ArmPlatformPkg/PrePi/PeiMPCore.inf | 2 +- ArmPlatformPkg/PrePi/PeiUniCore.inf | 2 +- MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm | 18 +++++++++--------- MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf | 2 +- MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf | 20 ++++++++++---------- MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf | 2 +- 21 files changed, 65 insertions(+), 59 deletions(-) -- 2.23.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47478): https://edk2.groups.io/g/devel/message/47478 Mute This Topic: https://groups.io/mt/34187297/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Thanks Baptiste,
Ard: I would appreciate if you could sanity check the syntax
conversion in 2/3 - it looks correct to me.
From my point of view, for the series:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
But we also need a nod from the MdePkg maintainers.
/
Leif
On Wed, Sep 18, 2019 at 06:05:21PM +0200, Baptiste Gerondeau wrote:
> EDIT: Resending the series since I mistakenly used the wrong email,
> sorry !
>
> We are currently making an effort to make ARM (and AARCH64 eventually)
> builds using Microsoft's Visual Studio Compiler (aka MSVC/MSFT).
>
> These 3 patches correspond to an effort to make the assembler work with
> MSFT, which entails :
> - Feeding MSFT the RVCT .asm files, since they share syntax
> requirements.
> - Fixing some instructions syntax in those .asm files, in order to make
> them palatable for MSFT.
> - Fixing some minor formatting issue in INF files, while we're at it.
>
> This set enables the assembler, meanwhile the C also require changes,
> which will come in a set later. This set makes the RVCT toolchain family
> and profiles obsolete, unblocking :
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1750
>
> As mentioned in the above bug, dropping RVCT would entail orphanating
> the .asm files that powered the RVCT build. Since Visual Studio uses the
> same file syntax, those can be reused to power the VS build.
>
> These patches have been tested on VS2019 (v15.9.11) and VS2017 (v16.0.1)
>
> Baptiste GERONDEAU (3):
> ArmPkg/MdePkg : Unify INF files format
> ARM/Assembler: Correct syntax from RVCT for MSFT
> ARM/Assembler: Reuse RVCT assembler for MSFT build
>
> ArmPkg/Drivers/ArmGic/ArmGicLib.inf | 2 +-
> ArmPkg/Library/ArmExceptionLib/Arm/ExceptionSupport.asm | 30 +++++++++++++++++-------------
> ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf | 2 +-
> ArmPkg/Library/ArmExceptionLib/ArmRelocateExceptionLib.inf | 2 +-
> ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf | 2 +-
> ArmPkg/Library/ArmLib/Arm/ArmV7Support.asm | 6 ++++--
> ArmPkg/Library/ArmLib/ArmBaseLib.inf | 8 ++++----
> ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf | 4 ++--
> ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf | 2 +-
> ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf | 2 +-
> ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf | 2 +-
> ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf | 2 +-
> ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf | 2 +-
> ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf | 6 +++---
> ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf | 6 +++---
> ArmPlatformPkg/PrePi/PeiMPCore.inf | 2 +-
> ArmPlatformPkg/PrePi/PeiUniCore.inf | 2 +-
> MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm | 18 +++++++++---------
> MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf | 2 +-
> MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf | 20 ++++++++++----------
> MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf | 2 +-
> 21 files changed, 65 insertions(+), 59 deletions(-)
>
> --
> 2.23.0
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#47490): https://edk2.groups.io/g/devel/message/47490
Mute This Topic: https://groups.io/mt/34187297/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
I add my comments. >-----Original Message----- >From: Baptiste Gerondeau [mailto:baptiste.gerondeau@linaro.org] >Sent: Thursday, September 19, 2019 12:05 AM >To: devel@edk2.groups.io >Cc: ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; Kinney, Michael D ><michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Zhang, >Shenglei <shenglei.zhang@intel.com>; Baptiste Gerondeau ><baptiste.gerondeau@linaro.org> >Subject: [PATCH 0/3] Arm builds on Visual Studio > >EDIT: Resending the series since I mistakenly used the wrong email, >sorry ! > >We are currently making an effort to make ARM (and AARCH64 eventually) >builds using Microsoft's Visual Studio Compiler (aka MSVC/MSFT). > >These 3 patches correspond to an effort to make the assembler work with >MSFT, which entails : >- Feeding MSFT the RVCT .asm files, since they share syntax > requirements. Please separate the patch. Each patch is for each package, can't cross packages. If so, the package maintainer can easy review the change. >- Fixing some instructions syntax in those .asm files, in order to make > them palatable for MSFT. >- Fixing some minor formatting issue in INF files, while we're at it. > >This set enables the assembler, meanwhile the C also require changes, >which will come in a set later. This set makes the RVCT toolchain family >and profiles obsolete, unblocking : >BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1750 With this change, can we continue to work on BZ 1750? > >As mentioned in the above bug, dropping RVCT would entail orphanating >the .asm files that powered the RVCT build. Since Visual Studio uses the >same file syntax, those can be reused to power the VS build. > >These patches have been tested on VS2019 (v15.9.11) and VS2017 (v16.0.1) Do you mean you verify this change with new VS2019 tool chain? Thanks Liming > >Baptiste GERONDEAU (3): > ArmPkg/MdePkg : Unify INF files format > ARM/Assembler: Correct syntax from RVCT for MSFT > ARM/Assembler: Reuse RVCT assembler for MSFT build > > ArmPkg/Drivers/ArmGic/ArmGicLib.inf | 2 +- > ArmPkg/Library/ArmExceptionLib/Arm/ExceptionSupport.asm | 30 >+++++++++++++++++------------- > ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf | 2 +- > ArmPkg/Library/ArmExceptionLib/ArmRelocateExceptionLib.inf | 2 +- > ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf | 2 +- > ArmPkg/Library/ArmLib/Arm/ArmV7Support.asm | 6 ++++-- > ArmPkg/Library/ArmLib/ArmBaseLib.inf | 8 ++++---- > ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf | 4 ++-- > ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf | 2 +- > ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf >| 2 +- > ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf | 2 +- > ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf | 2 +- > ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf | 2 >+- > ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf | 6 +++--- > ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf | 6 +++--- > ArmPlatformPkg/PrePi/PeiMPCore.inf | 2 +- > ArmPlatformPkg/PrePi/PeiUniCore.inf | 2 +- > MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm | 18 >+++++++++--------- > MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf | 2 +- > MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf | >20 ++++++++++---------- > MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf | 2 +- > 21 files changed, 65 insertions(+), 59 deletions(-) > >-- >2.23.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47558): https://edk2.groups.io/g/devel/message/47558 Mute This Topic: https://groups.io/mt/34187297/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi Liming,
On Thu, Sep 19, 2019 at 06:19:42AM +0000, Gao, Liming wrote:
> I add my comments.
>
> >-----Original Message-----
> >From: Baptiste Gerondeau [mailto:baptiste.gerondeau@linaro.org]
> >Sent: Thursday, September 19, 2019 12:05 AM
> >To: devel@edk2.groups.io
> >Cc: ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; Kinney, Michael D
> ><michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Zhang,
> >Shenglei <shenglei.zhang@intel.com>; Baptiste Gerondeau
> ><baptiste.gerondeau@linaro.org>
> >Subject: [PATCH 0/3] Arm builds on Visual Studio
> >
> >EDIT: Resending the series since I mistakenly used the wrong email,
> >sorry !
> >
> >We are currently making an effort to make ARM (and AARCH64 eventually)
> >builds using Microsoft's Visual Studio Compiler (aka MSVC/MSFT).
> >
> >These 3 patches correspond to an effort to make the assembler work with
> >MSFT, which entails :
> >- Feeding MSFT the RVCT .asm files, since they share syntax
> > requirements.
>
> Please separate the patch. Each patch is for each package, can't cross packages.
> If so, the package maintainer can easy review the change.
I agree with this as a general rule, but for this (hopefully never to
be repeated) operation, it makes sense to me to keep each change in
this set as one patch.
For the simple reason that the alternative leaves several unusable
commits in sequence in the repository. There is simply no way to
bisect through this change on a per-package basis.
This is after all a horrible horrible hack that lets us keep using the
.asm files provided for one toolchain family (RVCT) in a different
toolchain family (MSFT), without having to delete and re-add, losing
history in the process.
Would you be OK with an exception for this extremely unusual
situation?
> >- Fixing some instructions syntax in those .asm files, in order to make
> > them palatable for MSFT.
> >- Fixing some minor formatting issue in INF files, while we're at it.
> >
> >This set enables the assembler, meanwhile the C also require changes,
> >which will come in a set later. This set makes the RVCT toolchain family
> >and profiles obsolete, unblocking :
> >BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1750
>
> With this change, can we continue to work on BZ 1750?
Yes.
/
Leif
> >As mentioned in the above bug, dropping RVCT would entail orphanating
> >the .asm files that powered the RVCT build. Since Visual Studio uses the
> >same file syntax, those can be reused to power the VS build.
> >
> >These patches have been tested on VS2019 (v15.9.11) and VS2017 (v16.0.1)
>
> Do you mean you verify this change with new VS2019 tool chain?
>
> Thanks
> Liming
> >
> >Baptiste GERONDEAU (3):
> > ArmPkg/MdePkg : Unify INF files format
> > ARM/Assembler: Correct syntax from RVCT for MSFT
> > ARM/Assembler: Reuse RVCT assembler for MSFT build
> >
> > ArmPkg/Drivers/ArmGic/ArmGicLib.inf | 2 +-
> > ArmPkg/Library/ArmExceptionLib/Arm/ExceptionSupport.asm | 30
> >+++++++++++++++++-------------
> > ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf | 2 +-
> > ArmPkg/Library/ArmExceptionLib/ArmRelocateExceptionLib.inf | 2 +-
> > ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf | 2 +-
> > ArmPkg/Library/ArmLib/Arm/ArmV7Support.asm | 6 ++++--
> > ArmPkg/Library/ArmLib/ArmBaseLib.inf | 8 ++++----
> > ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf | 4 ++--
> > ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf | 2 +-
> > ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
> >| 2 +-
> > ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf | 2 +-
> > ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf | 2 +-
> > ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf | 2
> >+-
> > ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf | 6 +++---
> > ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf | 6 +++---
> > ArmPlatformPkg/PrePi/PeiMPCore.inf | 2 +-
> > ArmPlatformPkg/PrePi/PeiUniCore.inf | 2 +-
> > MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm | 18
> >+++++++++---------
> > MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf | 2 +-
> > MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf |
> >20 ++++++++++----------
> > MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf | 2 +-
> > 21 files changed, 65 insertions(+), 59 deletions(-)
> >
> >--
> >2.23.0
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#47581): https://edk2.groups.io/g/devel/message/47581
Mute This Topic: https://groups.io/mt/34187297/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Leif: For this special case of the single patch to include the changes in cross packages, I include Laszlo, Fish and Mike for the discussion. Thanks Liming > -----Original Message----- > From: Leif Lindholm [mailto:leif.lindholm@linaro.org] > Sent: Thursday, September 19, 2019 5:45 PM > To: Gao, Liming <liming.gao@intel.com> > Cc: Baptiste Gerondeau <baptiste.gerondeau@linaro.org>; devel@edk2.groups.io; ard.biesheuvel@linaro.org; Kinney, Michael D > <michael.d.kinney@intel.com>; Zhang, Shenglei <shenglei.zhang@intel.com> > Subject: Re: [PATCH 0/3] Arm builds on Visual Studio > > Hi Liming, > > On Thu, Sep 19, 2019 at 06:19:42AM +0000, Gao, Liming wrote: > > I add my comments. > > > > >-----Original Message----- > > >From: Baptiste Gerondeau [mailto:baptiste.gerondeau@linaro.org] > > >Sent: Thursday, September 19, 2019 12:05 AM > > >To: devel@edk2.groups.io > > >Cc: ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; Kinney, Michael D > > ><michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Zhang, > > >Shenglei <shenglei.zhang@intel.com>; Baptiste Gerondeau > > ><baptiste.gerondeau@linaro.org> > > >Subject: [PATCH 0/3] Arm builds on Visual Studio > > > > > >EDIT: Resending the series since I mistakenly used the wrong email, > > >sorry ! > > > > > >We are currently making an effort to make ARM (and AARCH64 eventually) > > >builds using Microsoft's Visual Studio Compiler (aka MSVC/MSFT). > > > > > >These 3 patches correspond to an effort to make the assembler work with > > >MSFT, which entails : > > >- Feeding MSFT the RVCT .asm files, since they share syntax > > > requirements. > > > > Please separate the patch. Each patch is for each package, can't cross packages. > > If so, the package maintainer can easy review the change. > > I agree with this as a general rule, but for this (hopefully never to > be repeated) operation, it makes sense to me to keep each change in > this set as one patch. > > For the simple reason that the alternative leaves several unusable > commits in sequence in the repository. There is simply no way to > bisect through this change on a per-package basis. > > This is after all a horrible horrible hack that lets us keep using the > .asm files provided for one toolchain family (RVCT) in a different > toolchain family (MSFT), without having to delete and re-add, losing > history in the process. > > Would you be OK with an exception for this extremely unusual > situation? > > > >- Fixing some instructions syntax in those .asm files, in order to make > > > them palatable for MSFT. > > >- Fixing some minor formatting issue in INF files, while we're at it. > > > > > >This set enables the assembler, meanwhile the C also require changes, > > >which will come in a set later. This set makes the RVCT toolchain family > > >and profiles obsolete, unblocking : > > >BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1750 > > > > With this change, can we continue to work on BZ 1750? > > Yes. > > / > Leif > > > >As mentioned in the above bug, dropping RVCT would entail orphanating > > >the .asm files that powered the RVCT build. Since Visual Studio uses the > > >same file syntax, those can be reused to power the VS build. > > > > > >These patches have been tested on VS2019 (v15.9.11) and VS2017 (v16.0.1) > > > > Do you mean you verify this change with new VS2019 tool chain? > > > > Thanks > > Liming > > > > > >Baptiste GERONDEAU (3): > > > ArmPkg/MdePkg : Unify INF files format > > > ARM/Assembler: Correct syntax from RVCT for MSFT > > > ARM/Assembler: Reuse RVCT assembler for MSFT build > > > > > > ArmPkg/Drivers/ArmGic/ArmGicLib.inf | 2 +- > > > ArmPkg/Library/ArmExceptionLib/Arm/ExceptionSupport.asm | 30 > > >+++++++++++++++++------------- > > > ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf | 2 +- > > > ArmPkg/Library/ArmExceptionLib/ArmRelocateExceptionLib.inf | 2 +- > > > ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf | 2 +- > > > ArmPkg/Library/ArmLib/Arm/ArmV7Support.asm | 6 ++++-- > > > ArmPkg/Library/ArmLib/ArmBaseLib.inf | 8 ++++---- > > > ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf | 4 ++-- > > > ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf | 2 +- > > > ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf > > >| 2 +- > > > ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf | 2 +- > > > ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf | 2 +- > > > ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf | 2 > > >+- > > > ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf | 6 +++--- > > > ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf | 6 +++--- > > > ArmPlatformPkg/PrePi/PeiMPCore.inf | 2 +- > > > ArmPlatformPkg/PrePi/PeiUniCore.inf | 2 +- > > > MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm | 18 > > >+++++++++--------- > > > MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf | 2 +- > > > MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf | > > >20 ++++++++++---------- > > > MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf | 2 +- > > > 21 files changed, 65 insertions(+), 59 deletions(-) > > > > > >-- > > >2.23.0 > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47602): https://edk2.groups.io/g/devel/message/47602 Mute This Topic: https://groups.io/mt/34187297/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 09/19/19 11:44, Leif Lindholm wrote: > Hi Liming, > > On Thu, Sep 19, 2019 at 06:19:42AM +0000, Gao, Liming wrote: >> I add my comments. >> >>> -----Original Message----- >>> From: Baptiste Gerondeau [mailto:baptiste.gerondeau@linaro.org] >>> Sent: Thursday, September 19, 2019 12:05 AM >>> To: devel@edk2.groups.io >>> Cc: ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; Kinney, Michael D >>> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Zhang, >>> Shenglei <shenglei.zhang@intel.com>; Baptiste Gerondeau >>> <baptiste.gerondeau@linaro.org> >>> Subject: [PATCH 0/3] Arm builds on Visual Studio >>> >>> EDIT: Resending the series since I mistakenly used the wrong email, >>> sorry ! >>> >>> We are currently making an effort to make ARM (and AARCH64 eventually) >>> builds using Microsoft's Visual Studio Compiler (aka MSVC/MSFT). >>> >>> These 3 patches correspond to an effort to make the assembler work with >>> MSFT, which entails : >>> - Feeding MSFT the RVCT .asm files, since they share syntax >>> requirements. >> >> Please separate the patch. Each patch is for each package, can't cross packages. >> If so, the package maintainer can easy review the change. > > I agree with this as a general rule, but for this (hopefully never to > be repeated) operation, it makes sense to me to keep each change in > this set as one patch. > > For the simple reason that the alternative leaves several unusable > commits in sequence in the repository. There is simply no way to > bisect through this change on a per-package basis. > > This is after all a horrible horrible hack that lets us keep using the > .asm files provided for one toolchain family (RVCT) in a different > toolchain family (MSFT), without having to delete and re-add, losing > history in the process. > > Would you be OK with an exception for this extremely unusual > situation? (The question was posed to Liming, but I'm going to follow up here with my own thoughts, after getting CC'd by Liming. Thanks for that BTW.) Let's assume the changes are split up with a fine granularity. Under that assumption, consider two cases: (1) ARM builds on Visual Studio are broken until the last patch is applied, but all other toolchains continue working fine throughout the series. versus (2) ARM builds are broken on Visual Studio *and* on at least one other -- preexistent -- toolchain, until the last patch in the series is applied. Which case reflects reality? If it's case (1), then I prefer the fine-grained patch series structure. Nothing regresses with existent toolchains (so bisection works with them), we just have to advertise the last patch in the series as the one that enables Visual Studio. If it's case (2), then I agree with the larger (multi-package) patch, as a rare exception. (We can also put it like this: if it's *possible* to write this series such that it enables (1), then we should strive for that.) Thanks, Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47632): https://edk2.groups.io/g/devel/message/47632 Mute This Topic: https://groups.io/mt/34187297/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
> On Sep 19, 2019, at 12:24 PM, Laszlo Ersek <lersek@redhat.com> wrote: > > On 09/19/19 11:44, Leif Lindholm wrote: >> Hi Liming, >> >> On Thu, Sep 19, 2019 at 06:19:42AM +0000, Gao, Liming wrote: >>> I add my comments. >>> >>>> -----Original Message----- >>>> From: Baptiste Gerondeau [mailto:baptiste.gerondeau@linaro.org] >>>> Sent: Thursday, September 19, 2019 12:05 AM >>>> To: devel@edk2.groups.io >>>> Cc: ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; Kinney, Michael D >>>> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Zhang, >>>> Shenglei <shenglei.zhang@intel.com>; Baptiste Gerondeau >>>> <baptiste.gerondeau@linaro.org> >>>> Subject: [PATCH 0/3] Arm builds on Visual Studio >>>> >>>> EDIT: Resending the series since I mistakenly used the wrong email, >>>> sorry ! >>>> >>>> We are currently making an effort to make ARM (and AARCH64 eventually) >>>> builds using Microsoft's Visual Studio Compiler (aka MSVC/MSFT). >>>> >>>> These 3 patches correspond to an effort to make the assembler work with >>>> MSFT, which entails : >>>> - Feeding MSFT the RVCT .asm files, since they share syntax >>>> requirements. >>> >>> Please separate the patch. Each patch is for each package, can't cross packages. >>> If so, the package maintainer can easy review the change. >> >> I agree with this as a general rule, but for this (hopefully never to >> be repeated) operation, it makes sense to me to keep each change in >> this set as one patch. >> >> For the simple reason that the alternative leaves several unusable >> commits in sequence in the repository. There is simply no way to >> bisect through this change on a per-package basis. >> >> This is after all a horrible horrible hack that lets us keep using the >> .asm files provided for one toolchain family (RVCT) in a different >> toolchain family (MSFT), without having to delete and re-add, losing >> history in the process. >> >> Would you be OK with an exception for this extremely unusual >> situation? > > (The question was posed to Liming, but I'm going to follow up here with > my own thoughts, after getting CC'd by Liming. Thanks for that BTW.) > > Let's assume the changes are split up with a fine granularity. Under > that assumption, consider two cases: > > (1) ARM builds on Visual Studio are broken until the last patch is > applied, but all other toolchains continue working fine throughout the > series. > > versus > > (2) ARM builds are broken on Visual Studio *and* on at least one other > -- preexistent -- toolchain, until the last patch in the series is applied. > > > Which case reflects reality? > > If it's case (1), then I prefer the fine-grained patch series structure. > Nothing regresses with existent toolchains (so bisection works with > them), we just have to advertise the last patch in the series as the one > that enables Visual Studio. > > If it's case (2), then I agree with the larger (multi-package) patch, as > a rare exception. > > (We can also put it like this: if it's *possible* to write this series > such that it enables (1), then we should strive for that.) > Laszlo, I agree with your logic around (1) and (2). I'd also point out we can do the review using (1) and squash into a single commit if we need to take path (2). Thanks, Andrew Fish > Thanks, > Laszlo > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47674): https://edk2.groups.io/g/devel/message/47674 Mute This Topic: https://groups.io/mt/34187297/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On Thu, Sep 19, 2019 at 09:24:15PM +0200, Laszlo Ersek wrote:
> On 09/19/19 11:44, Leif Lindholm wrote:
> > I agree with this as a general rule, but for this (hopefully never to
> > be repeated) operation, it makes sense to me to keep each change in
> > this set as one patch.
> >
> > For the simple reason that the alternative leaves several unusable
> > commits in sequence in the repository. There is simply no way to
> > bisect through this change on a per-package basis.
> >
> > This is after all a horrible horrible hack that lets us keep using the
> > .asm files provided for one toolchain family (RVCT) in a different
> > toolchain family (MSFT), without having to delete and re-add, losing
> > history in the process.
> >
> > Would you be OK with an exception for this extremely unusual
> > situation?
>
> (The question was posed to Liming, but I'm going to follow up here with
> my own thoughts, after getting CC'd by Liming. Thanks for that BTW.)
Yes, I should have thought of that myself - Baptiste did exactly what
I asked him to.
> Let's assume the changes are split up with a fine granularity. Under
> that assumption, consider two cases:
>
> (1) ARM builds on Visual Studio are broken until the last patch is
> applied, but all other toolchains continue working fine throughout the
> series.
>
> versus
>
> (2) ARM builds are broken on Visual Studio *and* on at least one other
> -- preexistent -- toolchain, until the last patch in the series is applied.
>
>
> Which case reflects reality?
>
> If it's case (1), then I prefer the fine-grained patch series structure.
> Nothing regresses with existent toolchains (so bisection works with
> them), we just have to advertise the last patch in the series as the one
> that enables Visual Studio.
>
> If it's case (2), then I agree with the larger (multi-package) patch, as
> a rare exception.
>
> (We can also put it like this: if it's *possible* to write this series
> such that it enables (1), then we should strive for that.)
I agree with your logic.
It's just that I'm not even aware of anyone who has *tried* building
with RVCT for several years. So we leave the "probably not working"
toolchain potentially working for a few more commits, as a tradeoff
against enabling the new one in one go.
(Now, fair, there are other issues with the VS ARM port that as per
the cover letter will also need to be addressed before the port is
fully functional.)
/
Leif
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#47675): https://edk2.groups.io/g/devel/message/47675
Mute This Topic: https://groups.io/mt/34187297/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Leif: >-----Original Message----- >From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Leif >Lindholm >Sent: Friday, September 20, 2019 4:27 AM >To: Laszlo Ersek <lersek@redhat.com> >Cc: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>; Baptiste >Gerondeau <baptiste.gerondeau@linaro.org>; ard.biesheuvel@linaro.org; >Kinney, Michael D <michael.d.kinney@intel.com>; Zhang, Shenglei ><shenglei.zhang@intel.com> >Subject: Re: [edk2-devel] [PATCH 0/3] Arm builds on Visual Studio > >On Thu, Sep 19, 2019 at 09:24:15PM +0200, Laszlo Ersek wrote: >> On 09/19/19 11:44, Leif Lindholm wrote: >> > I agree with this as a general rule, but for this (hopefully never to >> > be repeated) operation, it makes sense to me to keep each change in >> > this set as one patch. >> > >> > For the simple reason that the alternative leaves several unusable >> > commits in sequence in the repository. There is simply no way to >> > bisect through this change on a per-package basis. >> > >> > This is after all a horrible horrible hack that lets us keep using the >> > .asm files provided for one toolchain family (RVCT) in a different >> > toolchain family (MSFT), without having to delete and re-add, losing >> > history in the process. >> > >> > Would you be OK with an exception for this extremely unusual >> > situation? >> >> (The question was posed to Liming, but I'm going to follow up here with >> my own thoughts, after getting CC'd by Liming. Thanks for that BTW.) > >Yes, I should have thought of that myself - Baptiste did exactly what >I asked him to. > >> Let's assume the changes are split up with a fine granularity. Under >> that assumption, consider two cases: >> >> (1) ARM builds on Visual Studio are broken until the last patch is >> applied, but all other toolchains continue working fine throughout the >> series. >> >> versus >> >> (2) ARM builds are broken on Visual Studio *and* on at least one other >> -- preexistent -- toolchain, until the last patch in the series is applied. >> >> >> Which case reflects reality? >> >> If it's case (1), then I prefer the fine-grained patch series structure. >> Nothing regresses with existent toolchains (so bisection works with >> them), we just have to advertise the last patch in the series as the one >> that enables Visual Studio. >> >> If it's case (2), then I agree with the larger (multi-package) patch, as >> a rare exception. >> >> (We can also put it like this: if it's *possible* to write this series >> such that it enables (1), then we should strive for that.) > >I agree with your logic. >It's just that I'm not even aware of anyone who has *tried* building >with RVCT for several years. So we leave the "probably not working" >toolchain potentially working for a few more commits, as a tradeoff >against enabling the new one in one go. > >(Now, fair, there are other issues with the VS ARM port that as per >the cover letter will also need to be addressed before the port is >fully functional.) > If RVCT doesn't work, this change will follow into case (1). Right? Thanks Liming >/ > Leif > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47886): https://edk2.groups.io/g/devel/message/47886 Mute This Topic: https://groups.io/mt/34187297/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.