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::231; helo=mail-wm0-x231.google.com; envelope-from=pete@akeo.ie; receiver=edk2-devel@lists.01.org Received: from mail-wm0-x231.google.com (mail-wm0-x231.google.com [IPv6:2a00:1450:400c:c09::231]) (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 9B84C21B00DC1 for ; Tue, 14 Nov 2017 09:03:25 -0800 (PST) Received: by mail-wm0-x231.google.com with SMTP id r68so18800709wmr.0 for ; Tue, 14 Nov 2017 09:07:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=akeo-ie.20150623.gappssmtp.com; s=20150623; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-language:content-transfer-encoding; bh=TGUkC6YOl9VdYg2rV3cyX/3Vnvr0C8Zb5vePiBEJi8M=; b=vpr+aaHkIomtWHRvtWjqJKnaneyTNByHyzJKv8NiZbVjRc+xaPG/YqPpb5fDyoI/Dh GFVJKD1YkAH1Z2VyVw0pjqdcopivmOUSIyg/kgCq+H/ydnXTFMK/XX8nrYc2y5f0iYek YkTB7ZomGGt9jjI9eWR+HFtz/+9kGoSR0pQMBtW3eHJNPsEt9N2SQZem1F189tE43JZt FBiZeBX+kP2C+Z0Gf3jlyO0Yi8Qn9/KQsLhw9bFqaCdN4O69poTingVd08bECcz1d7Fg Y7wwnfNa+/uB18sy/SdhS8ZputUmjGIOlVlK6/qxRZAiLvayaNTr1WWiZAA3R+osyAKe KkPQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=TGUkC6YOl9VdYg2rV3cyX/3Vnvr0C8Zb5vePiBEJi8M=; b=PGwy1ff3fJRonuINGfhIkD/Q3x5N9m2bTpRq2x97O5LUvBW8SUbKHjrf2/aLnV7p++ 2dUvKSO+Mc58w4g0XzQOSrjwm4xBnLnu9YOiiMU8jCgpe7+rDgoZKil8f4riS00o/Va4 HSamHGC2Y7j+NLCJzNU17gwlLZGZZJ6Tqznfcy67RWCZzu4M9+GuI0hdAFZb5tvjbHta A23BW8iOHsb9fMeUtQKFGFjUeQFC99x0rS1LnpcMwPjIka4uADp65i5pt+nqatpfXezf d7cfl8CbR+vHib1xttEstchff/BUcj3om7hd3n3zOSor6TeKyWzeHXg5WcpDSCKOdvpd i8hA== X-Gm-Message-State: AJaThX6zdBjGUZ+XYTzsh8nk3dvbrncDPLaUQYGEQPgWSuUUZAFT6svG ixJNsaN77cJ64mjKR8SdDMY4jRzsIuo= X-Google-Smtp-Source: AGs4zMZ/Imx04ZqdDMEqPo8d7NgBhlf4OpownRtgmCUOtQ3OwZG7PluJ4w9C4GTXUG3v86LouFiijg== X-Received: by 10.80.216.198 with SMTP id y6mr7932410edj.24.1510679249766; Tue, 14 Nov 2017 09:07:29 -0800 (PST) Received: from [10.0.0.101] ([84.203.45.202]) by smtp.googlemail.com with ESMTPSA id p7sm15032048edj.5.2017.11.14.09.07.28 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 14 Nov 2017 09:07:29 -0800 (PST) To: "Gao, Liming" , "edk2-devel@lists.01.org" References: <20171114123229.3516-1-pete@akeo.ie> <4A89E2EF3DFEDB4C8BFDE51014F606A14E17DE72@SHSMSX104.ccr.corp.intel.com> From: Pete Batard Message-ID: <6018e619-bcd6-2b4c-2e59-7b5ad6e4604e@akeo.ie> Date: Tue, 14 Nov 2017 17:07:28 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <4A89E2EF3DFEDB4C8BFDE51014F606A14E17DE72@SHSMSX104.ccr.corp.intel.com> Subject: Re: [PATCH 0/4] BaseTools: Add VS2017 support, including ARM and AARCH64 X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 14 Nov 2017 17:03:26 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit Hi Liming, On 2017.11.14 16:03, Gao, Liming wrote: > In fact, I have sent one patch serial to add VS2017 tool chain for > IA32 and X64. https://lists.01.org/pipermail/edk2-devel/2017-October/016175.html I missed that proposal, else I would have used it as my base. Thanks for pointing it out. > In my patch, I disable warning 4701&4703, because they are also disabled in VS2015. Okay. Then I see no objections to disabling them. I didn't see the /wd options for those in tools_def.template with VS2015, so I thought these needed to be addressed. But from your looking at your proposal, I realize that these are disabled in ProcessorBind.h. Obviously, I'll update my ARM/AARCH64 proposal so that the warnings for these toolchains are disabled there as well, instead of tools_def.template. > In tools_def.template, to align other VS tool chain, I use VS2017_BIN for 32bit, > VS2017_BINX64 for 64bit. Could you follow this rule to define VS2017_BINARM for > arm, VS2017_BINAARCH64 for AARCH64? I will do that. > On VS2017, I notice it doesn't set VS150COMNTOOLS env by default. > So, I use vswhere.exe to detect VS2017 installation path. When we build source > code with VS tool chain, we open normal cmd instead of VS cmd. I see. I've always been opening a VS command prompt before calling the EDK setup script, so I built my proposal around that. But I agree that it makes sense to be able to build from regular prompt, so I will try to follow your lead. My only concern is that it may be difficult to locate the relevant ARM toolchains without %WindowsSdkVerBinPath% being properly defined. In your proposal, you seem to be hardcoding WINSDK10_PREFIX to something like "%ProgramFiles(x86)%\Windows Kits\10\bin\x86" but that won't work with ARM/ARM64 as we will need to get the actual version of the defayult SDK installed for VS2017 (e.g. "C:\Program Files (x86)\Windows Kits\10\bin\10.0.16299.0\") as the ARM toolchains will not work otherwise. For instance, on my VS2017 installation the "arm64\" under "Windows Kits\10\bin\" does not contain the latest version of the ARM64 tools and libraries, which is problematic as proper ARM64 compilation support was only enabled with the latest update. Only the one under "Windows Kits\10\bin\10.0.16299.0\" does. All in all, rather than try to guess the SDK path, I think we should try to locate and call "%ProgramFiles(x86)%\Microsoft Visual Studio\2017\Community\Common7\Tools\vsdevcmd\core\winsdk.bat", to have the VS environment provide us with the actual SDK version to use, as determined by Microsoft. > In your patch on ARM support, is included in Base.h. > This is a windows header file. So, VS env must be setup to build source > code with ARM arch. Right? That is correct. I had a quick look at removing that header, but didn't see much harm in keeping it. However, now that I understand our constraints better, I will make sure that header is not referenced. I will also do the same for memset_ms.c and memcpy_ms.c, introduced for ARM/ARM64 (In CompilerIntrinsicsLib), as it references for size_t. > Last, I suggest you base on my patch to add VS2017 ARM and AARCH64 arch. > For your patches, I suggest you separate them per package. I will follow both these suggestions and send a v2 when ready. Regards, /Pete