From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: None (no SPF record) identity=mailfrom; client-ip=2a00:1450:400c:c09::22d; helo=mail-wm0-x22d.google.com; envelope-from=pete@akeo.ie; receiver=edk2-devel@lists.01.org Received: from mail-wm0-x22d.google.com (mail-wm0-x22d.google.com [IPv6:2a00:1450:400c:c09::22d]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 3B1C021E25685 for ; Thu, 25 Jan 2018 03:26:28 -0800 (PST) Received: by mail-wm0-x22d.google.com with SMTP id t74so14086327wme.3 for ; Thu, 25 Jan 2018 03:31:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=akeo-ie.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=jvS57GhEIE5M328/rC9cfu5zrG0GlpiOzx1I191GZKU=; b=tkgdSjtVwuM8G9To3pyrLuZ6Fx05QPFSUe5UdoUL2JXj5SHlV72FFOUqN86YITicvG kBEGxBzJtabNwd7bU/4s5Q7MnJScdkzY8XyyoI8g8jUT5lAAfhiOU9lS03banT6Wkr1D Y0C4iAawVNV8UXXsQxXVNgpTu1Bc8vzqjePPO1xsuQa8z0xqVGCwAPQJfWAZjTjc9Tfx MBPBxBLsm3hq+UGaDZCzQ6F9Io6G/8W8I/BKaLBYQ0qgDWvR9ouBlCS5uTAcUhq/AEHT GBIAomNmIaSSrBdpbU5QQsllR6/qtoW5MCJUP51MOZ6dmS2FSWC6tSx/PERwlMJHFZ3n RMWw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=jvS57GhEIE5M328/rC9cfu5zrG0GlpiOzx1I191GZKU=; b=hdAQ8iLA+3e7u02XTLdCsl7CstjJQI6rVYEug21YnnJZ9/u8f00Kp5SAmKKv0FKyqS BXEf8d/PSOGIr+GkvaJfbM9bmBiZThpLg6sYqHR5mIF0mhCQdlu5iwV/lnipe1yQeUUd ISmk0bSWMxG5TG1icEpnncKFb7umiF0X29y1zFYjDkYRN7Ms9dQGzP9yUzIwSZxTS/pt QGMllFfdsNkoFnD1rh9ocdoOCVg1cImQe6TPm2qWo7sIVVUZPhYTrpZxmOt2tg/s2BxO o7M41Nafp2TmrZEYVE4umHPi9w8lIAOwVrcGnA/HjHLKJbc73Wl+a4hZPDA3dmkY0BOX DTRQ== X-Gm-Message-State: AKwxytedErEMP9z72luh6G+cDYaesMofsECQK4wVDuIk27tJrK3EIq2T l0q7PUvPrTaaA/JMP/qESRRH/w== X-Google-Smtp-Source: AH8x224HacPxXlPNVCkne7tPG89kF0/sQY7jJ0+IVxgTokhC9Unq2s6tZyuyyAS1+j97z+IvOi859A== X-Received: by 10.80.161.167 with SMTP id 36mr28522333edk.38.1516879916422; Thu, 25 Jan 2018 03:31:56 -0800 (PST) Received: from [10.0.0.101] ([84.203.95.87]) by smtp.googlemail.com with ESMTPSA id x20sm1282524edm.47.2018.01.25.03.31.55 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 25 Jan 2018 03:31:55 -0800 (PST) To: "Gao, Liming" , "edk2-devel@lists.01.org" Cc: "ard.biesheuvel@linaro.org" , "eugene@hp.com" References: <20180112133331.7776-1-pete@akeo.ie> <4A89E2EF3DFEDB4C8BFDE51014F606A14E1AC788@SHSMSX104.ccr.corp.intel.com> From: Pete Batard Message-ID: <281c2328-6bba-fd80-0ae8-b4de0ce668b2@akeo.ie> Date: Thu, 25 Jan 2018 11:31:54 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <4A89E2EF3DFEDB4C8BFDE51014F606A14E1AC788@SHSMSX104.ccr.corp.intel.com> Subject: Re: [PATCH v5 0/6] Add ARM support for VS2017 X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 25 Jan 2018 11:26:29 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit Thanks Liming. From the way you framed your comment, I'm not completely sure if there's action still needed on my side... Do I actually need to resubmit a patch set with "Reviewed By"? Or, now that each patch should have flagged as reviewed, are we simply supposed to wait for formal integration? Regards, /Pete On 2018.01.25 10:27, Gao, Liming wrote: > Pete: > The changes in BaseTools and MdePkg are good to me. You can add my R-B. > >> -----Original Message----- >> From: Pete Batard [mailto:pete@akeo.ie] >> Sent: Friday, January 12, 2018 9:33 PM >> To: edk2-devel@lists.01.org >> Cc: Gao, Liming ; ard.biesheuvel@linaro.org; >> eugene@hp.com >> Subject: [PATCH v5 0/6] Add ARM support for VS2017 >> >> (Same as v4, except for the AREA names where we replaced the RVCT macros, >> in >> patch 4/6, so that it matches what would have been produced with the >> macros) >> >> >> The following series adds ARM compilation support for the VS2017 toolchain. >> * PATCH 1 targets the disabling of VS Level 4 warnings. The disabled warnings >> for ARM are now aligned with IA32 and X64. >> * PATCH 2 adds a NULL handler for the base stack check, since this is a GCC >> functionality. >> * PATCH 3 updates MdePkg/Library/BaseLib so that the RVCT assembly >> sources >> are also used for MSFT. >> * PATCH 4 adds the required compiler intrinsics replacements for division, >> shift, by reusing the RVCT code, as well as memset/memcpy. >> * PATCH 5 adds variable argument handlers for print output. Note that this >> is done without relying on any external headers, with the VA_ARG macro >> having been reverse engineered from MSFT ARM assembly output. >> * PATCH 6 enables the selection of ARM in the conf templates. >> >> With these patches, VS2017 toolchain users should be able to compile >> regular UEFI ARM applications using EDK2. Note that, unlike ARM64 support, >> ARM support does not require a specific update of Visual Studio 2017, as >> the ARM toolchain has been available from the very first release. >> >> Additional notes: >> >> We tested compiling and running the full UEFI Shell with this series, as >> well as a small set of applications and drivers, and found no issues. >> With an additional patch [1], it is also possible to use this proposal to >> compile a complete QEMU ARM firmware. As the patch shows, the changes >> that >> need to be applied to the EDK2 sources to achieve this are actually very >> minimal. >> >> However, the generated firmware does not currently boot, possibly because >> of the following warnings being generated by the MS compiler: >> - ArmCpuDxe.dll : warning LNK4072: section count 118 exceeds max (96); >> image may not run >> - UiApp.dll : warning LNK4072: section count 113 exceeds max (96); image may >> not run >> >> As far as I could see, the section count max is hardcoded so a workaround >> would be needed to address those. >> >> Also, because the VS2017 ARM compiler forces a section alignment of 4096 >> bytes (which in turn forces use to use /FILEALIGN:4096 as a linker option >> for the firmware generation), the generated firmware exceeds 2MB and we >> had to double its size to 4MB. >> >> At this stage, since the goal of this series is to allow users to compile >> regular ARM UEFI applications using the VS2017 toolchain, I have no plans >> to spend more time on the QEMU firmware issues, especially as I suspect >> that reducing the firmware size back to 2 MB may not be achievable without >> Microsoft altering their compiler. I am however hopeful that ARM >> specialists can take this matter over eventually... >> >> Regards, >> >> /Pete >> >> [1] >> https://github.com/pbatard/edk2/commit/c4ce41094a46f4f3dc7ccc64a906048 >> 13f037b13 >> >> Pete Batard (6): >> MdePkg: Disable some Level 4 warnings for VS2017/ARM >> MdePkg/Library/BaseStackCheckLib: Add Null handler for VS2017/ARM >> MdePkg/Library/BaseLib: Enable VS2017/ARM builds >> ArmPkg/Library/CompilerIntrinsicsLib: Enable VS2017/ARM builds >> MdePkg/Include: Add VA list support for VS2017/ARM >> BaseTools/Conf: Add VS2017/ARM support >> >> ArmPkg/Library/CompilerIntrinsicsLib/Arm/div.asm | 43 +++++++-- >> ArmPkg/Library/CompilerIntrinsicsLib/Arm/ldivmod.asm | 40 ++++++-- >> ArmPkg/Library/CompilerIntrinsicsLib/Arm/llsr.asm | 22 +++-- >> ArmPkg/Library/CompilerIntrinsicsLib/Arm/uldiv.asm | 29 +++++- >> ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf | 16 +++- >> ArmPkg/Library/CompilerIntrinsicsLib/memcpy_ms.c | 34 +++++++ >> ArmPkg/Library/CompilerIntrinsicsLib/memset_ms.c | 33 +++++++ >> BaseTools/Conf/build_rule.template | 31 ++++++- >> BaseTools/Conf/tools_def.template | 31 +++++++ >> MdePkg/Include/Arm/ProcessorBind.h | 96 >> +++++++++++++++----- >> MdePkg/Include/Base.h | 13 +++ >> MdePkg/Library/BaseLib/Arm/CpuBreakpoint.asm | 5 +- >> MdePkg/Library/BaseLib/BaseLib.inf | 16 +++- >> MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf | 5 +- >> MdePkg/Library/BaseStackCheckLib/BaseStackCheckNull.c | 18 ++++ >> 15 files changed, 372 insertions(+), 60 deletions(-) >> create mode 100644 ArmPkg/Library/CompilerIntrinsicsLib/memcpy_ms.c >> create mode 100644 ArmPkg/Library/CompilerIntrinsicsLib/memset_ms.c >> create mode 100644 >> MdePkg/Library/BaseStackCheckLib/BaseStackCheckNull.c >> >> -- >> 2.9.3.windows.2 >