[v2,7/7] aarch64,arm: Move branch-protection data to targets

Message ID 69a6baa924519053eef92766189d23da1f7afa7c.1699025215.git.szabolcs.nagy@arm.com
State Unresolved
Headers
Series aarch64 GCS preliminary patches |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Szabolcs Nagy Nov. 3, 2023, 3:36 p.m. UTC
  The branch-protection types are target specific, not the same on arm
and aarch64.  This currently affects pac-ret+b-key, but there will be
a new type on aarch64 that is not relevant for arm.

gcc/ChangeLog:

	* config/aarch64/aarch64-opts.h (enum aarch64_key_type): Rename to ...
	(enum aarch_key_type): ... this.
	* config/aarch64/aarch64.cc (aarch_handle_no_branch_protection): Copy.
	(aarch_handle_standard_branch_protection): Copy.
	(aarch_handle_pac_ret_protection): Copy.
	(aarch_handle_pac_ret_leaf): Copy.
	(aarch_handle_pac_ret_b_key): Copy.
	(aarch_handle_bti_protection): Copy.
	* config/arm/aarch-common.cc (aarch_handle_no_branch_protection):
	Remove.
	(aarch_handle_standard_branch_protection): Remove.
	(aarch_handle_pac_ret_protection): Remove.
	(aarch_handle_pac_ret_leaf): Remove.
	(aarch_handle_pac_ret_b_key): Remove.
	(aarch_handle_bti_protection): Remove.
	* config/arm/aarch-common.h (enum aarch_key_type): Remove.
	(struct aarch_branch_protect_type): Declare.
	* config/arm/arm-c.cc (arm_cpu_builtins): Remove aarch_ra_sign_key.
	* config/arm/arm.cc (aarch_handle_no_branch_protection): Copy.
	(aarch_handle_standard_branch_protection): Copy.
	(aarch_handle_pac_ret_protection): Copy.
	(aarch_handle_pac_ret_leaf): Copy.
	(aarch_handle_bti_protection): Copy.
	(arm_configure_build_target): Copy.
	* config/arm/arm.opt: Remove aarch_ra_sign_key.
---
unchanged compared to v1.
---
 gcc/config/aarch64/aarch64-opts.h |  6 ++--
 gcc/config/aarch64/aarch64.cc     | 55 +++++++++++++++++++++++++++++++
 gcc/config/arm/aarch-common.cc    | 55 -------------------------------
 gcc/config/arm/aarch-common.h     | 11 +++----
 gcc/config/arm/arm-c.cc           |  2 --
 gcc/config/arm/arm.cc             | 52 +++++++++++++++++++++++++----
 gcc/config/arm/arm.opt            |  3 --
 7 files changed, 109 insertions(+), 75 deletions(-)
  

Comments

Richard Earnshaw Dec. 7, 2023, 1:13 p.m. UTC | #1
On 03/11/2023 15:36, Szabolcs Nagy wrote:
> The branch-protection types are target specific, not the same on arm
> and aarch64.  This currently affects pac-ret+b-key, but there will be
> a new type on aarch64 that is not relevant for arm.
> 
> gcc/ChangeLog:
> 
> 	* config/aarch64/aarch64-opts.h (enum aarch64_key_type): Rename to ...
> 	(enum aarch_key_type): ... this.
> 	* config/aarch64/aarch64.cc (aarch_handle_no_branch_protection): Copy.
> 	(aarch_handle_standard_branch_protection): Copy.
> 	(aarch_handle_pac_ret_protection): Copy.
> 	(aarch_handle_pac_ret_leaf): Copy.
> 	(aarch_handle_pac_ret_b_key): Copy.
> 	(aarch_handle_bti_protection): Copy.

I think all of the above functions that have been moved back from 
aarch-common should be renamed back to aarch64_..., unless they are 
directly referenced statically by code in aarch-common.c.
> 	* config/arm/aarch-common.cc (aarch_handle_no_branch_protection):
> 	Remove.
> 	(aarch_handle_standard_branch_protection): Remove.
> 	(aarch_handle_pac_ret_protection): Remove.
> 	(aarch_handle_pac_ret_leaf): Remove.
> 	(aarch_handle_pac_ret_b_key): Remove.
> 	(aarch_handle_bti_protection): Remove.
> 	* config/arm/aarch-common.h (enum aarch_key_type): Remove.
> 	(struct aarch_branch_protect_type): Declare.
> 	* config/arm/arm-c.cc (arm_cpu_builtins): Remove aarch_ra_sign_key.
> 	* config/arm/arm.cc (aarch_handle_no_branch_protection): Copy.
> 	(aarch_handle_standard_branch_protection): Copy.
> 	(aarch_handle_pac_ret_protection): Copy.
> 	(aarch_handle_pac_ret_leaf): Copy.
> 	(aarch_handle_bti_protection): Copy.
> 	(arm_configure_build_target): Copy.

And the same here.

> 	* config/arm/arm.opt: Remove aarch_ra_sign_key.
> ---
> unchanged compared to v1.
> ---
>   gcc/config/aarch64/aarch64-opts.h |  6 ++--
>   gcc/config/aarch64/aarch64.cc     | 55 +++++++++++++++++++++++++++++++
>   gcc/config/arm/aarch-common.cc    | 55 -------------------------------
>   gcc/config/arm/aarch-common.h     | 11 +++----
>   gcc/config/arm/arm-c.cc           |  2 --
>   gcc/config/arm/arm.cc             | 52 +++++++++++++++++++++++++----
>   gcc/config/arm/arm.opt            |  3 --
>   7 files changed, 109 insertions(+), 75 deletions(-)
> 
> diff --git a/gcc/config/aarch64/aarch64-opts.h b/gcc/config/aarch64/aarch64-opts.h
> index 831e28ab52a..1abae1442b5 100644
> --- a/gcc/config/aarch64/aarch64-opts.h
> +++ b/gcc/config/aarch64/aarch64-opts.h
> @@ -103,9 +103,9 @@ enum stack_protector_guard {
>   };
>   
>   /* The key type that -msign-return-address should use.  */
> -enum aarch64_key_type {
> -  AARCH64_KEY_A,
> -  AARCH64_KEY_B
> +enum aarch_key_type {
> +  AARCH_KEY_A,
> +  AARCH_KEY_B
>   };
>   
>   /* An enum specifying how to handle load and store pairs using
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index 4f7f707b675..9739223831f 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -18620,6 +18620,61 @@ aarch64_set_asm_isa_flags (aarch64_feature_flags flags)
>     aarch64_set_asm_isa_flags (&global_options, flags);
>   }
>   
> +static void
> +aarch_handle_no_branch_protection (void)
> +{
> +  aarch_ra_sign_scope = AARCH_FUNCTION_NONE;
> +  aarch_enable_bti = 0;
> +}
> +
> +static void
> +aarch_handle_standard_branch_protection (void)
> +{
> +  aarch_ra_sign_scope = AARCH_FUNCTION_NON_LEAF;
> +  aarch_ra_sign_key = AARCH_KEY_A;
> +  aarch_enable_bti = 1;
> +}
> +
> +static void
> +aarch_handle_pac_ret_protection (void)
> +{
> +  aarch_ra_sign_scope = AARCH_FUNCTION_NON_LEAF;
> +  aarch_ra_sign_key = AARCH_KEY_A;
> +}
> +
> +static void
> +aarch_handle_pac_ret_leaf (void)
> +{
> +  aarch_ra_sign_scope = AARCH_FUNCTION_ALL;
> +}
> +
> +static void
> +aarch_handle_pac_ret_b_key (void)
> +{
> +  aarch_ra_sign_key = AARCH_KEY_B;
> +}
> +
> +static void
> +aarch_handle_bti_protection (void)
> +{
> +  aarch_enable_bti = 1;
> +}
> +
> +static const struct aarch_branch_protect_type aarch_pac_ret_subtypes[] = {
> +  { "leaf", false, aarch_handle_pac_ret_leaf, NULL, 0 },
> +  { "b-key", false, aarch_handle_pac_ret_b_key, NULL, 0 },
> +  { NULL, false, NULL, NULL, 0 }
> +};
> +
> +const struct aarch_branch_protect_type aarch_branch_protect_types[] = {

can this be made static now?  And maybe pass the structure as a 
parameter if that's not done already.


> +  { "none", true, aarch_handle_no_branch_protection, NULL, 0 },
> +  { "standard", true, aarch_handle_standard_branch_protection, NULL, 0 },
> +  { "pac-ret", false, aarch_handle_pac_ret_protection, aarch_pac_ret_subtypes,
> +    ARRAY_SIZE (aarch_pac_ret_subtypes) },
> +  { "bti", false, aarch_handle_bti_protection, NULL, 0 },
> +  { NULL, false, NULL, NULL, 0 }
> +};
> +
>   /* Implement TARGET_OPTION_OVERRIDE.  This is called once in the beginning
>      and is used to parse the -m{cpu,tune,arch} strings and setup the initial
>      tuning structs.  In particular it must set selected_tune and
> diff --git a/gcc/config/arm/aarch-common.cc b/gcc/config/arm/aarch-common.cc
> index 159c61b786c..92e1248f83f 100644
> --- a/gcc/config/arm/aarch-common.cc
> +++ b/gcc/config/arm/aarch-common.cc
> @@ -659,61 +659,6 @@ arm_md_asm_adjust (vec<rtx> &outputs, vec<rtx> & /*inputs*/,
>     return saw_asm_flag ? seq : NULL;
>   }
>   
> -static void
> -aarch_handle_no_branch_protection (void)
> -{
> -  aarch_ra_sign_scope = AARCH_FUNCTION_NONE;
> -  aarch_enable_bti = 0;
> -}
> -
> -static void
> -aarch_handle_standard_branch_protection (void)
> -{
> -  aarch_ra_sign_scope = AARCH_FUNCTION_NON_LEAF;
> -  aarch_ra_sign_key = AARCH_KEY_A;
> -  aarch_enable_bti = 1;
> -}
> -
> -static void
> -aarch_handle_pac_ret_protection (void)
> -{
> -  aarch_ra_sign_scope = AARCH_FUNCTION_NON_LEAF;
> -  aarch_ra_sign_key = AARCH_KEY_A;
> -}
> -
> -static void
> -aarch_handle_pac_ret_leaf (void)
> -{
> -  aarch_ra_sign_scope = AARCH_FUNCTION_ALL;
> -}
> -
> -static void
> -aarch_handle_pac_ret_b_key (void)
> -{
> -  aarch_ra_sign_key = AARCH_KEY_B;
> -}
> -
> -static void
> -aarch_handle_bti_protection (void)
> -{
> -  aarch_enable_bti = 1;
> -}
> -
> -static const struct aarch_branch_protect_type aarch_pac_ret_subtypes[] = {
> -  { "leaf", false, aarch_handle_pac_ret_leaf, NULL, 0 },
> -  { "b-key", false, aarch_handle_pac_ret_b_key, NULL, 0 },
> -  { NULL, false, NULL, NULL, 0 }
> -};
> -
> -static const struct aarch_branch_protect_type aarch_branch_protect_types[] = {
> -  { "none", true, aarch_handle_no_branch_protection, NULL, 0 },
> -  { "standard", true, aarch_handle_standard_branch_protection, NULL, 0 },
> -  { "pac-ret", false, aarch_handle_pac_ret_protection, aarch_pac_ret_subtypes,
> -    ARRAY_SIZE (aarch_pac_ret_subtypes) },
> -  { "bti", false, aarch_handle_bti_protection, NULL, 0 },
> -  { NULL, false, NULL, NULL, 0 }
> -};
> -
>   /* In-place split *str at delim, return *str and set *str to the tail
>      of the string or NULL if the end is reached.  */
>   
> diff --git a/gcc/config/arm/aarch-common.h b/gcc/config/arm/aarch-common.h
> index f72e21127fc..90d2112408f 100644
> --- a/gcc/config/arm/aarch-common.h
> +++ b/gcc/config/arm/aarch-common.h
> @@ -44,12 +44,7 @@ enum aarch_function_type {
>     AARCH_FUNCTION_ALL
>   };
>   
> -/* The key type that -msign-return-address should use.  */
> -enum aarch_key_type {
> -  AARCH_KEY_A,
> -  AARCH_KEY_B
> -};
> -
> +/* Specifies a -mbranch-protection= argument.  */
>   struct aarch_branch_protect_type
>   {
>     /* The type's name that the user passes to the branch-protection option
> @@ -64,4 +59,8 @@ struct aarch_branch_protect_type
>     unsigned int num_subtypes;
>   };
>   
> +/* Target specific data and callbacks for parsing -mbranch-protection=.
> +   The first item is used to reset the branch-protection settings.  */
> +extern const struct aarch_branch_protect_type aarch_branch_protect_types[];
> +
>   #endif /* GCC_AARCH_COMMON_H */
> diff --git a/gcc/config/arm/arm-c.cc b/gcc/config/arm/arm-c.cc
> index d3d93ceba00..204403b3ff4 100644
> --- a/gcc/config/arm/arm-c.cc
> +++ b/gcc/config/arm/arm-c.cc
> @@ -248,8 +248,6 @@ arm_cpu_builtins (struct cpp_reader* pfile)
>     {
>       unsigned int pac = 1;
>   
> -    gcc_assert (aarch_ra_sign_key == AARCH_KEY_A);
> -
>       if (aarch_ra_sign_scope == AARCH_FUNCTION_ALL)
>         pac |= 0x4;
>   
> diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc
> index 5681073a948..5cb0f2858b7 100644
> --- a/gcc/config/arm/arm.cc
> +++ b/gcc/config/arm/arm.cc
> @@ -3250,6 +3250,52 @@ static sbitmap isa_all_fpubits_internal;
>   static sbitmap isa_all_fpbits;
>   static sbitmap isa_quirkbits;
>   
> +static void
> +aarch_handle_no_branch_protection (void)
> +{
> +  aarch_ra_sign_scope = AARCH_FUNCTION_NONE;
> +  aarch_enable_bti = 0;
> +}
> +
> +static void
> +aarch_handle_standard_branch_protection (void)
> +{
> +  aarch_ra_sign_scope = AARCH_FUNCTION_NON_LEAF;
> +  aarch_enable_bti = 1;
> +}
> +
> +static void
> +aarch_handle_pac_ret_protection (void)
> +{
> +  aarch_ra_sign_scope = AARCH_FUNCTION_NON_LEAF;
> +}
> +
> +static void
> +aarch_handle_pac_ret_leaf (void)
> +{
> +  aarch_ra_sign_scope = AARCH_FUNCTION_ALL;
> +}
> +
> +static void
> +aarch_handle_bti_protection (void)
> +{
> +  aarch_enable_bti = 1;
> +}
> +
> +static const struct aarch_branch_protect_type aarch_pac_ret_subtypes[] = {
> +  { "leaf", false, aarch_handle_pac_ret_leaf, NULL, 0 },
> +  { NULL, false, NULL, NULL, 0 }
> +};
> +
> +const struct aarch_branch_protect_type aarch_branch_protect_types[] = {
> +  { "none", true, aarch_handle_no_branch_protection, NULL, 0 },
> +  { "standard", true, aarch_handle_standard_branch_protection, NULL, 0 },
> +  { "pac-ret", false, aarch_handle_pac_ret_protection, aarch_pac_ret_subtypes,
> +    ARRAY_SIZE (aarch_pac_ret_subtypes) },
> +  { "bti", false, aarch_handle_bti_protection, NULL, 0 },
> +  { NULL, false, NULL, NULL, 0 }
> +};
> +
>   /* Configure a build target TARGET from the user-specified options OPTS and
>      OPTS_SET.  If WARN_COMPATIBLE, emit a diagnostic if both the CPU and
>      architecture have been specified, but the two are not identical.  */
> @@ -3299,12 +3345,6 @@ arm_configure_build_target (struct arm_build_target *target,
>       {
>         aarch_validate_mbranch_protection (opts->x_arm_branch_protection_string,
>   					 "-mbranch-protection=");
> -
> -      if (aarch_ra_sign_key != AARCH_KEY_A)
> -	{
> -	  warning (0, "invalid key type for %<-mbranch-protection=%>");
> -	  aarch_ra_sign_key = AARCH_KEY_A;
> -	}
>       }
>   
>     if (arm_selected_arch)
> diff --git a/gcc/config/arm/arm.opt b/gcc/config/arm/arm.opt
> index 88299dabc3a..bd8bb00d035 100644
> --- a/gcc/config/arm/arm.opt
> +++ b/gcc/config/arm/arm.opt
> @@ -30,9 +30,6 @@ enum aarch_function_type aarch_ra_sign_scope = AARCH_FUNCTION_NONE
>   TargetVariable
>   unsigned aarch_enable_bti = 0
>   
> -TargetVariable
> -enum aarch_key_type aarch_ra_sign_key = AARCH_KEY_A
> -
>   Enum
>   Name(tls_type) Type(enum arm_tls_type)
>   TLS dialect to use:

It would be nice if, when we raise an error, we could print out the list 
of valid options (and modifiers), much like we do on Arm for -march/-mcpu.

eg.
$ gcc -mcpu=crotex-a8
cc1: error: unrecognised -mcpu target: crotex-a8
cc1: note: valid arguments are: arm8 arm810 strongarm strongarm110 fa526 
[...rest of list]; did you mean ‘cortex-a8’?

R.
  
Szabolcs Nagy Jan. 11, 2024, 2:43 p.m. UTC | #2
The 12/07/2023 13:13, Richard Earnshaw wrote:
> On 03/11/2023 15:36, Szabolcs Nagy wrote:
> > 	* config/aarch64/aarch64.cc (aarch_handle_no_branch_protection): Copy.
> > 	(aarch_handle_standard_branch_protection): Copy.
> > 	(aarch_handle_pac_ret_protection): Copy.
> > 	(aarch_handle_pac_ret_leaf): Copy.
> > 	(aarch_handle_pac_ret_b_key): Copy.
> > 	(aarch_handle_bti_protection): Copy.
> 
> I think all of the above functions that have been moved back from
> aarch-common should be renamed back to aarch64_..., unless they are directly
> referenced statically by code in aarch-common.c.

done.

> > +const struct aarch_branch_protect_type aarch_branch_protect_types[] = {
> 
> can this be made static now?  And maybe pass the structure as a parameter if
> that's not done already.

done in v4.

> It would be nice if, when we raise an error, we could print out the list of
> valid options (and modifiers), much like we do on Arm for -march/-mcpu.
> 
> eg.
> $ gcc -mcpu=crotex-a8
> cc1: error: unrecognised -mcpu target: crotex-a8
> cc1: note: valid arguments are: arm8 arm810 strongarm strongarm110 fa526
> [...rest of list]; did you mean ‘cortex-a8’?

i implemented this with candidates_list_and_hint but it does
not work very well if the typo is in a subtype, so i think
this should be done in a separate patch if at all.
  
Richard Earnshaw (lists) Jan. 11, 2024, 2:49 p.m. UTC | #3
On 11/01/2024 14:43, Szabolcs Nagy wrote:
> The 12/07/2023 13:13, Richard Earnshaw wrote:
>> On 03/11/2023 15:36, Szabolcs Nagy wrote:
>>> 	* config/aarch64/aarch64.cc (aarch_handle_no_branch_protection): Copy.
>>> 	(aarch_handle_standard_branch_protection): Copy.
>>> 	(aarch_handle_pac_ret_protection): Copy.
>>> 	(aarch_handle_pac_ret_leaf): Copy.
>>> 	(aarch_handle_pac_ret_b_key): Copy.
>>> 	(aarch_handle_bti_protection): Copy.
>>
>> I think all of the above functions that have been moved back from
>> aarch-common should be renamed back to aarch64_..., unless they are directly
>> referenced statically by code in aarch-common.c.
> 
> done.
> 
>>> +const struct aarch_branch_protect_type aarch_branch_protect_types[] = {
>>
>> can this be made static now?  And maybe pass the structure as a parameter if
>> that's not done already.
> 
> done in v4.
> 
>> It would be nice if, when we raise an error, we could print out the list of
>> valid options (and modifiers), much like we do on Arm for -march/-mcpu.
>>
>> eg.
>> $ gcc -mcpu=crotex-a8
>> cc1: error: unrecognised -mcpu target: crotex-a8
>> cc1: note: valid arguments are: arm8 arm810 strongarm strongarm110 fa526
>> [...rest of list]; did you mean ‘cortex-a8’?
> 
> i implemented this with candidates_list_and_hint but it does
> not work very well if the typo is in a subtype, so i think
> this should be done in a separate patch if at all.
> 

I'd build the candidates list from all the types + subtypes, so that the suggestion code has a full list to pick from; but fair enough.

R.
  

Patch

diff --git a/gcc/config/aarch64/aarch64-opts.h b/gcc/config/aarch64/aarch64-opts.h
index 831e28ab52a..1abae1442b5 100644
--- a/gcc/config/aarch64/aarch64-opts.h
+++ b/gcc/config/aarch64/aarch64-opts.h
@@ -103,9 +103,9 @@  enum stack_protector_guard {
 };
 
 /* The key type that -msign-return-address should use.  */
-enum aarch64_key_type {
-  AARCH64_KEY_A,
-  AARCH64_KEY_B
+enum aarch_key_type {
+  AARCH_KEY_A,
+  AARCH_KEY_B
 };
 
 /* An enum specifying how to handle load and store pairs using
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 4f7f707b675..9739223831f 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -18620,6 +18620,61 @@  aarch64_set_asm_isa_flags (aarch64_feature_flags flags)
   aarch64_set_asm_isa_flags (&global_options, flags);
 }
 
+static void
+aarch_handle_no_branch_protection (void)
+{
+  aarch_ra_sign_scope = AARCH_FUNCTION_NONE;
+  aarch_enable_bti = 0;
+}
+
+static void
+aarch_handle_standard_branch_protection (void)
+{
+  aarch_ra_sign_scope = AARCH_FUNCTION_NON_LEAF;
+  aarch_ra_sign_key = AARCH_KEY_A;
+  aarch_enable_bti = 1;
+}
+
+static void
+aarch_handle_pac_ret_protection (void)
+{
+  aarch_ra_sign_scope = AARCH_FUNCTION_NON_LEAF;
+  aarch_ra_sign_key = AARCH_KEY_A;
+}
+
+static void
+aarch_handle_pac_ret_leaf (void)
+{
+  aarch_ra_sign_scope = AARCH_FUNCTION_ALL;
+}
+
+static void
+aarch_handle_pac_ret_b_key (void)
+{
+  aarch_ra_sign_key = AARCH_KEY_B;
+}
+
+static void
+aarch_handle_bti_protection (void)
+{
+  aarch_enable_bti = 1;
+}
+
+static const struct aarch_branch_protect_type aarch_pac_ret_subtypes[] = {
+  { "leaf", false, aarch_handle_pac_ret_leaf, NULL, 0 },
+  { "b-key", false, aarch_handle_pac_ret_b_key, NULL, 0 },
+  { NULL, false, NULL, NULL, 0 }
+};
+
+const struct aarch_branch_protect_type aarch_branch_protect_types[] = {
+  { "none", true, aarch_handle_no_branch_protection, NULL, 0 },
+  { "standard", true, aarch_handle_standard_branch_protection, NULL, 0 },
+  { "pac-ret", false, aarch_handle_pac_ret_protection, aarch_pac_ret_subtypes,
+    ARRAY_SIZE (aarch_pac_ret_subtypes) },
+  { "bti", false, aarch_handle_bti_protection, NULL, 0 },
+  { NULL, false, NULL, NULL, 0 }
+};
+
 /* Implement TARGET_OPTION_OVERRIDE.  This is called once in the beginning
    and is used to parse the -m{cpu,tune,arch} strings and setup the initial
    tuning structs.  In particular it must set selected_tune and
diff --git a/gcc/config/arm/aarch-common.cc b/gcc/config/arm/aarch-common.cc
index 159c61b786c..92e1248f83f 100644
--- a/gcc/config/arm/aarch-common.cc
+++ b/gcc/config/arm/aarch-common.cc
@@ -659,61 +659,6 @@  arm_md_asm_adjust (vec<rtx> &outputs, vec<rtx> & /*inputs*/,
   return saw_asm_flag ? seq : NULL;
 }
 
-static void
-aarch_handle_no_branch_protection (void)
-{
-  aarch_ra_sign_scope = AARCH_FUNCTION_NONE;
-  aarch_enable_bti = 0;
-}
-
-static void
-aarch_handle_standard_branch_protection (void)
-{
-  aarch_ra_sign_scope = AARCH_FUNCTION_NON_LEAF;
-  aarch_ra_sign_key = AARCH_KEY_A;
-  aarch_enable_bti = 1;
-}
-
-static void
-aarch_handle_pac_ret_protection (void)
-{
-  aarch_ra_sign_scope = AARCH_FUNCTION_NON_LEAF;
-  aarch_ra_sign_key = AARCH_KEY_A;
-}
-
-static void
-aarch_handle_pac_ret_leaf (void)
-{
-  aarch_ra_sign_scope = AARCH_FUNCTION_ALL;
-}
-
-static void
-aarch_handle_pac_ret_b_key (void)
-{
-  aarch_ra_sign_key = AARCH_KEY_B;
-}
-
-static void
-aarch_handle_bti_protection (void)
-{
-  aarch_enable_bti = 1;
-}
-
-static const struct aarch_branch_protect_type aarch_pac_ret_subtypes[] = {
-  { "leaf", false, aarch_handle_pac_ret_leaf, NULL, 0 },
-  { "b-key", false, aarch_handle_pac_ret_b_key, NULL, 0 },
-  { NULL, false, NULL, NULL, 0 }
-};
-
-static const struct aarch_branch_protect_type aarch_branch_protect_types[] = {
-  { "none", true, aarch_handle_no_branch_protection, NULL, 0 },
-  { "standard", true, aarch_handle_standard_branch_protection, NULL, 0 },
-  { "pac-ret", false, aarch_handle_pac_ret_protection, aarch_pac_ret_subtypes,
-    ARRAY_SIZE (aarch_pac_ret_subtypes) },
-  { "bti", false, aarch_handle_bti_protection, NULL, 0 },
-  { NULL, false, NULL, NULL, 0 }
-};
-
 /* In-place split *str at delim, return *str and set *str to the tail
    of the string or NULL if the end is reached.  */
 
diff --git a/gcc/config/arm/aarch-common.h b/gcc/config/arm/aarch-common.h
index f72e21127fc..90d2112408f 100644
--- a/gcc/config/arm/aarch-common.h
+++ b/gcc/config/arm/aarch-common.h
@@ -44,12 +44,7 @@  enum aarch_function_type {
   AARCH_FUNCTION_ALL
 };
 
-/* The key type that -msign-return-address should use.  */
-enum aarch_key_type {
-  AARCH_KEY_A,
-  AARCH_KEY_B
-};
-
+/* Specifies a -mbranch-protection= argument.  */
 struct aarch_branch_protect_type
 {
   /* The type's name that the user passes to the branch-protection option
@@ -64,4 +59,8 @@  struct aarch_branch_protect_type
   unsigned int num_subtypes;
 };
 
+/* Target specific data and callbacks for parsing -mbranch-protection=.
+   The first item is used to reset the branch-protection settings.  */
+extern const struct aarch_branch_protect_type aarch_branch_protect_types[];
+
 #endif /* GCC_AARCH_COMMON_H */
diff --git a/gcc/config/arm/arm-c.cc b/gcc/config/arm/arm-c.cc
index d3d93ceba00..204403b3ff4 100644
--- a/gcc/config/arm/arm-c.cc
+++ b/gcc/config/arm/arm-c.cc
@@ -248,8 +248,6 @@  arm_cpu_builtins (struct cpp_reader* pfile)
   {
     unsigned int pac = 1;
 
-    gcc_assert (aarch_ra_sign_key == AARCH_KEY_A);
-
     if (aarch_ra_sign_scope == AARCH_FUNCTION_ALL)
       pac |= 0x4;
 
diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc
index 5681073a948..5cb0f2858b7 100644
--- a/gcc/config/arm/arm.cc
+++ b/gcc/config/arm/arm.cc
@@ -3250,6 +3250,52 @@  static sbitmap isa_all_fpubits_internal;
 static sbitmap isa_all_fpbits;
 static sbitmap isa_quirkbits;
 
+static void
+aarch_handle_no_branch_protection (void)
+{
+  aarch_ra_sign_scope = AARCH_FUNCTION_NONE;
+  aarch_enable_bti = 0;
+}
+
+static void
+aarch_handle_standard_branch_protection (void)
+{
+  aarch_ra_sign_scope = AARCH_FUNCTION_NON_LEAF;
+  aarch_enable_bti = 1;
+}
+
+static void
+aarch_handle_pac_ret_protection (void)
+{
+  aarch_ra_sign_scope = AARCH_FUNCTION_NON_LEAF;
+}
+
+static void
+aarch_handle_pac_ret_leaf (void)
+{
+  aarch_ra_sign_scope = AARCH_FUNCTION_ALL;
+}
+
+static void
+aarch_handle_bti_protection (void)
+{
+  aarch_enable_bti = 1;
+}
+
+static const struct aarch_branch_protect_type aarch_pac_ret_subtypes[] = {
+  { "leaf", false, aarch_handle_pac_ret_leaf, NULL, 0 },
+  { NULL, false, NULL, NULL, 0 }
+};
+
+const struct aarch_branch_protect_type aarch_branch_protect_types[] = {
+  { "none", true, aarch_handle_no_branch_protection, NULL, 0 },
+  { "standard", true, aarch_handle_standard_branch_protection, NULL, 0 },
+  { "pac-ret", false, aarch_handle_pac_ret_protection, aarch_pac_ret_subtypes,
+    ARRAY_SIZE (aarch_pac_ret_subtypes) },
+  { "bti", false, aarch_handle_bti_protection, NULL, 0 },
+  { NULL, false, NULL, NULL, 0 }
+};
+
 /* Configure a build target TARGET from the user-specified options OPTS and
    OPTS_SET.  If WARN_COMPATIBLE, emit a diagnostic if both the CPU and
    architecture have been specified, but the two are not identical.  */
@@ -3299,12 +3345,6 @@  arm_configure_build_target (struct arm_build_target *target,
     {
       aarch_validate_mbranch_protection (opts->x_arm_branch_protection_string,
 					 "-mbranch-protection=");
-
-      if (aarch_ra_sign_key != AARCH_KEY_A)
-	{
-	  warning (0, "invalid key type for %<-mbranch-protection=%>");
-	  aarch_ra_sign_key = AARCH_KEY_A;
-	}
     }
 
   if (arm_selected_arch)
diff --git a/gcc/config/arm/arm.opt b/gcc/config/arm/arm.opt
index 88299dabc3a..bd8bb00d035 100644
--- a/gcc/config/arm/arm.opt
+++ b/gcc/config/arm/arm.opt
@@ -30,9 +30,6 @@  enum aarch_function_type aarch_ra_sign_scope = AARCH_FUNCTION_NONE
 TargetVariable
 unsigned aarch_enable_bti = 0
 
-TargetVariable
-enum aarch_key_type aarch_ra_sign_key = AARCH_KEY_A
-
 Enum
 Name(tls_type) Type(enum arm_tls_type)
 TLS dialect to use: