From: Thomas Preudhomme <thomas.preudhomme@foss.arm.com>
To: Kyrill Tkachov <kyrylo.tkachov@foss.arm.com>,
"Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com>,
Ramana Radhakrishnan <ramana.radhakrishnan@arm.com>,
"gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH, GCC/ARM, ping] Remove ARMv8-M code for D17-D31
Date: Thu, 28 Sep 2017 16:13:00 -0000 [thread overview]
Message-ID: <49cae04c-8969-e720-d144-be07e1ba61b5@foss.arm.com> (raw)
In-Reply-To: <59AFADD3.10303@foss.arm.com>
Committed (sorry for delay).
Best regards,
Thomas
On 06/09/17 09:12, Kyrill Tkachov wrote:
> Hi Thomas,
>
> On 05/09/17 10:04, Thomas Preudhomme wrote:
>> Ping?
>>
>
> This is ok if a bootstrap and test run on arm-none-linux-gnueabihf shows no
> problems.
> Thanks,
> Kyrill
>
>> Best regards,
>>
>> Thomas
>>
>> On 25/08/17 12:18, Thomas Preudhomme wrote:
>>> Hi,
>>>
>>> I've now also added a couple more changes:
>>>
>>> * size to_clear_bitmap according to maxregno to be consistent with its use
>>> * use directly TARGET_HARD_FLOAT instead of clear_vfpregs
>>>
>>>
>>> Original message below (ChangeLog unchanged):
>>>
>>> Function cmse_nonsecure_entry_clear_before_return has code to deal with
>>> high VFP register (D16-D31) while ARMv8-M Baseline and Mainline both do
>>> not support more than 16 double VFP registers (D0-D15). This makes this
>>> security-sensitive code harder to read for not much benefit since
>>> libcall for cmse_nonsecure_call functions do not deal with those high
>>> VFP registers anyway.
>>>
>>> This commit gets rid of this code for simplicity and fixes 2 issues in
>>> the same function:
>>>
>>> - stop the first loop when reaching maxregno to avoid dealing with VFP
>>> registers if targetting Thumb-1 or using -mfloat-abi=soft
>>> - include maxregno in that loop
>>>
>>> ChangeLog entry is as follows:
>>>
>>> *** gcc/ChangeLog ***
>>>
>>> 2017-06-13 Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>
>>> * config/arm/arm.c (arm_option_override): Forbid ARMv8-M Security
>>> Extensions with more than 16 double VFP registers.
>>> (cmse_nonsecure_entry_clear_before_return): Remove second entry of
>>> to_clear_mask and all code related to it. Replace the remaining
>>> entry by a sbitmap and adapt code accordingly.
>>>
>>> Testing: Testsuite shows no regression when run for ARMv8-M Baseline and
>>> ARMv8-M Mainline.
>>>
>>> Is this ok for trunk?
>>>
>>> Best regards,
>>>
>>> Thomas
>>>
>>> On 23/08/17 11:56, Thomas Preudhomme wrote:
>>>> Ping?
>>>>
>>>> Best regards,
>>>>
>>>> Thomas
>>>>
>>>> On 17/07/17 17:25, Thomas Preudhomme wrote:
>>>>> My bad, found an off-by-one error in the sizing of bitmaps. Please find
>>>>> fixed patch in attachment.
>>>>>
>>>>> ChangeLog entry is unchanged:
>>>>>
>>>>> *** gcc/ChangeLog ***
>>>>>
>>>>> 2017-06-13 Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>>>
>>>>> * config/arm/arm.c (arm_option_override): Forbid ARMv8-M Security
>>>>> Extensions with more than 16 double VFP registers.
>>>>> (cmse_nonsecure_entry_clear_before_return): Remove second entry of
>>>>> to_clear_mask and all code related to it. Replace the remaining
>>>>> entry by a sbitmap and adapt code accordingly.
>>>>>
>>>>> Best regards,
>>>>>
>>>>> Thomas
>>>>>
>>>>> On 17/07/17 09:52, Thomas Preudhomme wrote:
>>>>>> Ping?
>>>>>>
>>>>>> Best regards,
>>>>>>
>>>>>> Thomas
>>>>>>
>>>>>> On 12/07/17 09:59, Thomas Preudhomme wrote:
>>>>>>> Hi Richard,
>>>>>>>
>>>>>>> On 07/07/17 15:19, Richard Earnshaw (lists) wrote:
>>>>>>>>
>>>>>>>> Hmm, I think that's because really this is a partial conversion. It
>>>>>>>> looks like doing this properly would involve moving that existing code
>>>>>>>> to use sbitmaps as well. I think doing that would be better for
>>>>>>>> long-term maintenance perspectives, but I'm not going to insist that you
>>>>>>>> do it now.
>>>>>>>
>>>>>>> There's also the assert later but I've found a way to improve it
>>>>>>> slightly. While switching to auto_sbitmap I also changed the code
>>>>>>> slightly to allocate directly bitmaps to the right size. Since the change
>>>>>>> is probably bigger than what you had in mind I'd appreciate if you can
>>>>>>> give me an OK again. See updated patch in attachment. ChangeLog entry is
>>>>>>> unchanged:
>>>>>>>
>>>>>>> 2017-06-13 Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>>>>>
>>>>>>> * config/arm/arm.c (arm_option_override): Forbid ARMv8-M Security
>>>>>>> Extensions with more than 16 double VFP registers.
>>>>>>> (cmse_nonsecure_entry_clear_before_return): Remove second entry of
>>>>>>> to_clear_mask and all code related to it. Replace the remaining
>>>>>>> entry by a sbitmap and adapt code accordingly.
>>>>>>>
>>>>>>>>
>>>>>>>> As a result I'll let you take the call as to whether you keep this
>>>>>>>> version or go back to your earlier patch. If you do decide to keep this
>>>>>>>> version, then see the comment below.
>>>>>>>
>>>>>>> Given the changes I'm more happy with how the patch looks now and making
>>>>>>> it go in can be a nice incentive to change other ARMv8-M Security
>>>>>>> Extension related code later on.
>>>>>>>
>>>>>>> Best regards,
>>>>>>>
>>>>>>> Thomas
>
prev parent reply other threads:[~2017-09-28 16:13 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-20 15:01 [PATCH, GCC/ARM] " Thomas Preudhomme
2017-06-28 15:12 ` [PATCH, GCC/ARM, ping] " Thomas Preudhomme
2017-06-28 15:57 ` [PATCH, GCC/ARM] " Richard Earnshaw (lists)
2017-06-29 8:44 ` Thomas Preudhomme
2017-07-06 12:36 ` Thomas Preudhomme
2017-07-07 14:19 ` Richard Earnshaw (lists)
2017-07-12 8:59 ` Thomas Preudhomme
2017-07-17 8:52 ` [PATCH, GCC/ARM, ping] " Thomas Preudhomme
2017-07-17 16:25 ` Thomas Preudhomme
2017-08-23 11:01 ` Thomas Preudhomme
2017-08-25 13:23 ` [PATCH, GCC/ARM] " Thomas Preudhomme
2017-09-05 9:04 ` [PATCH, GCC/ARM, ping] " Thomas Preudhomme
2017-09-06 8:12 ` Kyrill Tkachov
2017-09-28 16:13 ` Thomas Preudhomme [this message]
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-all 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=49cae04c-8969-e720-d144-be07e1ba61b5@foss.arm.com \
--to=thomas.preudhomme@foss.arm.com \
--cc=Richard.Earnshaw@arm.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=kyrylo.tkachov@foss.arm.com \
--cc=ramana.radhakrishnan@arm.com \
/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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).