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 7278E2095606F for ; Fri, 16 Mar 2018 09:29:18 -0700 (PDT) Received: by mail-wm0-x231.google.com with SMTP id t3so4124439wmc.2 for ; Fri, 16 Mar 2018 09:35:44 -0700 (PDT) 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=S048B2CD40Oea4Vm3u5N9E1REMsqrfHbZyIFJ5F2SYo=; b=f+ohMkXNC7MYrbVp4qqeoae2Cy7Owv5TNvNytxkX5euqLobx7RxIRVUPVNka1+K6Hj Ssd2x6STjj1zDosvB+raxfvt/tn1hRjG8gtyN0WOiPXg/XIm2sGpVJw9915DwLdNKJki TmDIX0gG/SZHTdMwCYFCjJy957OmzJkcfWIR06yWIjxEtXIgQjgY9wIFeBuifTGfhpT1 VuBuSmZVvPqi2IC9RN84++3ekvyYl2T85KQ1SEgA1mS35Ag+wKe7+J0+WjN+73OmwPhf fIOjMfafpzEtjSqGlDPVHyMuVSpPIFDgBIn1bB0K/DAoPhsYVZCiZsoM+dI7puFokWLL o/iQ== 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=S048B2CD40Oea4Vm3u5N9E1REMsqrfHbZyIFJ5F2SYo=; b=LNBUkPNnyOYN7uLFvn9XS3ZHRheeNXkDRrSL03R3loDeNPv0nTwHyTyTZf5ibeSF9+ 8yUZCIR4y+5p88I02ey4b/ASYus9c1liixntjxkhgSBPyPLZPosScxvRfs371Wn1qd6F +4Faq4oRs4JDJgx/SB3+ZOYIQONICwYWKRgeaKw8tOGbzInDUWojrt0Gg8CMObBBKWyy VYl5wTrikPQT5noI8YyBIunl2RAAl9jc9RSckOd+sLQcJ0SRuarBIkJcaJ4moZYZDDJi VDkdDG2/izYzQ9ogxm6uTnnSbcGxnVN3CikltmVkIBJv1j6hH5METG4nEGGHW+fMQ+BI 2RRQ== X-Gm-Message-State: AElRT7GE0xxoFcTomxOV8KdDuv+S3iAx1LzluKoKLgWtui84XO0iWaqW q0FqWpJ/HgtvS95egfPykAxsWw== X-Google-Smtp-Source: AG47ELsLp5+Npf8f2v6EH8eY4pHQwZOMD2zFiEX9ykA20izDvreoGUVR8Wi3hBCNKBpizvmyHQNOzA== X-Received: by 10.80.212.43 with SMTP id t43mr3184171edh.53.1521218142736; Fri, 16 Mar 2018 09:35:42 -0700 (PDT) Received: from [10.0.0.101] ([84.203.73.68]) by smtp.googlemail.com with ESMTPSA id s10sm4289106edc.63.2018.03.16.09.35.41 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 16 Mar 2018 09:35:42 -0700 (PDT) To: "Gao, Liming" , "edk2-devel@lists.01.org" Cc: Ard Biesheuvel References: <20180223095003.6012-1-pete@akeo.ie> <4A89E2EF3DFEDB4C8BFDE51014F606A14E1E7104@SHSMSX104.ccr.corp.intel.com> <71d6b134-d875-2c24-6687-f3b01d0ff9ea@akeo.ie> <4A89E2EF3DFEDB4C8BFDE51014F606A14E1E7EEB@SHSMSX104.ccr.corp.intel.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14E1E8137@SHSMSX104.ccr.corp.intel.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14E1E81C2@SHSMSX104.ccr.corp.intel.com> From: Pete Batard Message-ID: Date: Fri, 16 Mar 2018 16:35:41 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <4A89E2EF3DFEDB4C8BFDE51014F606A14E1E81C2@SHSMSX104.ccr.corp.intel.com> Subject: Re: [PATCH 0/4] Add ARM64 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: Fri, 16 Mar 2018 16:29:20 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit Thanks Liming, much appreciated! I'll send the comment harmonization patch as soon as I see the VS2017/ARM64 changes in edk2 mainline. Regards, /Pete On 2018.03.16 16:31, Gao, Liming wrote: > Yes. This is a minor issue. So, I think the effort is small. If it is a big task to you, you can separate it into another patch. > > And, I don't expect this minor issue break your patches. I give my Reviewed-by: Liming Gao > > Thanks > Liming >> -----Original Message----- >> From: Pete Batard [mailto:pete@akeo.ie] >> Sent: Saturday, March 17, 2018 12:12 AM >> To: edk2-devel@lists.01.org; Gao, Liming >> Cc: Ard Biesheuvel >> Subject: Re: [edk2] [PATCH 0/4] Add ARM64 support for VS2017 >> >> I understand where you're coming from, but that means I have to recreate >> this patch set, and then create a new patch for the .S (because it makes >> zero sense to require the same comment style on the .asm and not request >> a follow through for the .S). >> >> My time being limited, I'd rather only have to produce one new patch, >> that will harmonize the comments for both .S and .asm at the same time. >> >> The end result will be exactly the same, so I'm going to have to insist >> that we split the comment harmonization (which is a very minor issue and >> should hardly be seen as a showstopper for the patch series in the first >> place) into a subsequent patch. >> >> Thank you, >> >> /Pete >> >> On 2018.03.16 15:56, Gao, Liming wrote: >>> Pete: >>> I understand the existing .S file has the inconsistent comment style. I also know new added ASM files are converted from .S files. >> But, my comment is for this patch that adds new ASM files. I expect new added ASM files have the same style. If you check ARM arch >> ASM files, you will find they all have the same style. >>> >>> Thanks >>> Liming >>>> -----Original Message----- >>>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Pete Batard >>>> Sent: Friday, March 16, 2018 7:04 PM >>>> To: edk2-devel@lists.01.org >>>> Cc: Gao, Liming ; Ard Biesheuvel >>>> Subject: Re: [edk2] [PATCH 0/4] Add ARM64 support for VS2017 >>>> >>>> On 2018.03.16 08:24, Gao, Liming wrote: >>>>> Pete: >>>>> .S for GCC assembly, .asm for MSFT assembly. They can have the different comment style. >>>> >>>> Yes, but as I explained, the actual original issue is that our current >>>> .S files do *not* have the same comment styles in the first place. >>>> >>>> If you look at MdePkg/Library/BaseLib/AArch64/SwitchStack.S, you'll see >>>> that is uses '//' for comments, whereas other .S files, such as >>>> MdePkg/Library/BaseLib/AArch64/SetJumpLongJump.S, use '#'. >>>> >>>> So that is our actual issue here: Regardless of VS2017, the GCC assembly >>>> files for AARCH64 we currently have do not use the same comment style. >>>> >>>> Thus, the only reason why the .asm don't have the same comment style in >>>> our proposal is because the .S, which we derived the .asm from, don't. >>>> This means that either we should fix the .S too, or we shouldn't fix >>>> anything at all. >>>> >>>>> Here, my comment is to make sure .asm files have the same comment style. I don't request to change .S file. >>>> >>>> And what I am saying is that it makes little sense to harmonize the >>>> comment style for the .asm files, if we're not going to do the same for >>>> the .S files as well. It just doesn't seem fair in my book to have the >>>> VS2017 assembly files held to a higher standard than the GCC ones. So >>>> either we need to fix both, or we fix none at all. >>>> >>>> But as I indicated in my last e-mail, I am planning to send an >>>> additional patch that does comment harmonization, for both .S and .asm, >>>> *after* this VS2017 series has been applied to mainline. So the change >>>> you request will happen. Just not as part of this patch series. >>>> >>>> And the reason I have insist on splitting these changes is because, if >>>> we have to alter the .S files to be consistent, then this comment >>>> harmonization request should logically be handled separately from the >>>> VS2017 effort. >>>> >>>> Please let me know if you still think having a future separate patch, >>>> that will do .S and .asm comment harmonization, does not make sense. >>>> >>>> Regards, >>>> >>>> /Pete >>>> >>>>> >>>>>> -----Original Message----- >>>>>> From: Pete Batard [mailto:pete@akeo.ie] >>>>>> Sent: Thursday, March 15, 2018 5:28 PM >>>>>> To: Gao, Liming ; edk2-devel@lists.01.org >>>>>> Cc: ard.biesheuvel@linaro.org >>>>>> Subject: Re: [PATCH 0/4] Add ARM64 support for VS2017 >>>>>> >>>>>> Hi Liming, >>>>>> >>>>>> Thanks for reviewing the patches. >>>>>> >>>>>> On 2018.03.15 06:15, Gao, Liming wrote: >>>>>>> Pete: >>>>>>> For new added ASM file in BaseLib, could you use the same comment style >>>>>>> for them? ASM use ; for the comment. Most of new files uses ; as the >>>>>>> comment, but switchstack is not. >>>>>> >>>>>> This is because SwitchStack.asm is simply SwitchStack.S, with the GCC >>>>>> assembler specifics removed, and MSVC assembler specifics added. >>>>>> >>>>>> I did not change the comment style from the original files, so the real >>>>>> issue here is that our GCC assembly files for AARCH64 do not use the >>>>>> same comment style. >>>>>> >>>>>> I'm fine with trying to harmonize the comment styles, but seeing as this >>>>>> needs to be done for both the .S and .asm, I'd rather send a patch to do >>>>>> that *after* these VS2017 changes have been applied, as I don't consider >>>>>> this correction to in scope of this patch series (because logically, the >>>>>> introduction of VS2017 should not alter any of the .S files, unless we >>>>>> reuse them, which we don't). >>>>>> >>>>>> If you agree to apply this series, I'll make sure to send a non >>>>>> VS2017-specific additional patch, that does what you request for both >>>>>> the .S and .asm. >>>>>> >>>>>>> Besides, compared to Arm arch assembly >>>>>>> file, I don't find CpuPause.asm. Is it required? >>>>>> >>>>>> That file doesn't exist for GCC (as you will see there is no >>>>>> MdePkg/Library/BaseLib/AArch64/CpuPause.S), so we don't have one for >>>>>> VS2017 either. >>>>>> >>>>>> Regards, >>>>>> >>>>>> /Pete >>>> >>>> _______________________________________________ >>>> edk2-devel mailing list >>>> edk2-devel@lists.01.org >>>> https://lists.01.org/mailman/listinfo/edk2-devel >