Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <9ab1abd1-4f00-8412-a926-7ef617af80e7@suse.com>
Date:   Wed, 9 Aug 2023 12:56:34 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [RFC][PATCH 13/17] objtool/x86: Add arch_is_offset_insn()
Content-Language: en-US
To:     Peter Zijlstra <peterz@infradead.org>, x86@kernel.org
Cc:     linux-kernel@vger.kernel.org, David.Kaplan@amd.com,
        Andrew.Cooper3@citrix.com, jpoimboe@kernel.org,
        gregkh@linuxfoundation.org
References: <20230809071218.000335006@infradead.org>
 <20230809072201.326960605@infradead.org>
From:   Nikolay Borisov <nik.borisov@suse.com>
In-Reply-To: <20230809072201.326960605@infradead.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?aDk2cUsvS1V0VFJzYUV0MmxETFAyeE1SNmtjRXZyeHVmZHgxajEyeHR5OVJ4?=
 =?utf-8?B?UmwwdjVaVHhNRk56Nlc0cDJsbEJHK1AyWWR4NFRxSEwxWkExazBiOHh3NFQy?=
 =?utf-8?B?SkFnRjM0TCt0Uk1ibE5HSHBSSGtVNmNPa1V2ODZacnB3ZnZkUUdzVEVoUk5S?=
 =?utf-8?B?LzhTbzA2NEl6NDhqaCtpTzNkVHVrOVBJOHdVVGs1THhkVzFkOE9qZWVKMUdp?=
 =?utf-8?B?SDVaU0lCeHc1c0JTU1Y2SkErNGk1Mk95WXJHMHU5TWx5aXNhOHpHYjVnUWJh?=
 =?utf-8?B?eFJYbzNxTDVJRXVwL3hiSW5BUkpFL3IrQWtXU2NYR3Y2QWM3aFRvSWdqb1ZX?=
 =?utf-8?B?R1Z4MWQxaHRHVlc2NFg1dVA5cFE5YzVJT0RlVmVSNXRBbkt6R3BYK1VvMWFL?=
 =?utf-8?B?dDdiZnlCUTFnNERrTG1RMW5XYitoSG1yZStrYSsyc280eEVFZ2xOZzdSZlJo?=
 =?utf-8?B?eGV6RjRzemJHNEM1QkN6K1ZnL0NKcS9rZ2FUeDFwejBoN0ZOdStwSmtZVnll?=
 =?utf-8?B?enp6OWNuRFNHUGVPZjJaWHFDcG91ODdtSDZjQUNhQXIyYkRsM1NRdFA0UjNV?=
 =?utf-8?B?bWliajZMbUl0V0ZDYUFPSUdvakV1alVna3ZmaC9Va2VJS1FJaGI4emZlK3dN?=
 =?utf-8?B?MVBYekY1K3JCRnJtdmptU3lJRnR2N0k3S1Nkc3lTU2ttZk5jOHVVUGErd1Vj?=
 =?utf-8?B?VXQ0bWtDRm5QWlJkZ3BDeElaV0VVbGxFb3Z2dVdUMUJ2cnJNVEhYak1WUFVh?=
 =?utf-8?B?UjFWNUxLT29McHpkRnI1TzJGYkxlTHVKdUpxcFBUVHJZcEhoRkZSbC93bDE5?=
 =?utf-8?B?NENGTzdvRUZKMm5jVmhSQXNoZE81dzdWbnhUcGpNS0dORlZGbnRraWFuZ0Yz?=
 =?utf-8?B?aWgvSzFLSmt0RHNkR0Z5akIzaklCTThEbnBXS2pKREtvd0c0U1VVVy9uRUw2?=
 =?utf-8?B?cENOWEcwTEx3OWdLa3h5aHJ4Wmh0RHEvN201dmdtZ1FNTGU5N1BhNEk4and6?=
 =?utf-8?B?d3RBYTFhZGgyemdXSW9QcGs5ZDdabkNMYVNTdzVaRmp1WjF1Zml2a3RRRStX?=
 =?utf-8?B?MnZWeVl4dmlwaExkWmFabkk4Z3Vjanhvck1JdndKVnlzN0I1U3AwOER5UnY2?=
 =?utf-8?B?OXExT2pLVGoxR21WM3RwZnJSQ21kOU9tbTczQTlTRDVBMUtiRkhSWFZXdklB?=
 =?utf-8?B?MUJiOHZlK2pObEtyR1piSVRnUDR6SjVCSzJ1TStBUDUrWGNOallTWUhyUFdH?=
 =?utf-8?B?WEJuclpUcWMzaEdqOGI1Mm1rZTVtTDRoT0padUFpeXI1cFVLS2g0NUpYZ2Qx?=
 =?utf-8?B?UHpBN0VIWjV1TE1MWElmVzVXNkpkRG5pSFBaTk9HQlZmTUVVanNMOS9iVDFQ?=
 =?utf-8?B?TnhxT0ptKzYyYzYxcE1jMDRRMDRaVGdXL1Z1Z3o4T2RJQzFUUjRnQmdBVU01?=
 =?utf-8?B?NGkrQm5aU1RNekNUVEpoSDg1RUpsWUxTSFJ0cEtKcy9xTHVZSUJMRzNvTXI5?=
 =?utf-8?B?d1Boc1puKzg4YmtuZ2NXN3p0eG5IYnVTVWE0VHZ1VFdLb0hsVC9hbmVnL0hQ?=
 =?utf-8?B?N2dzQnZBOERqVUR6cjNkRk5LWW5RbEdFWWZzYXB2aURRVmNjdVk2Ry8wV3ZI?=
 =?utf-8?B?NG1ENEdOUENUV3IxY1g4eExiUm84TkVFbGErRG95SFpSWXJMcVNIdnAvY1Qr?=
 =?utf-8?B?WERaRjR1RWRrL2Fmb0YrSE0wU1lBOWNhbjdRUCsxRC94YlpmaUZ4SmJ2WXp4?=
 =?utf-8?B?RG5yL2NlN0UrK1dCeUluZlQ1MDVwK0Z5bHU3RGs0K0prUlk2ZlRPMVQvOEhV?=
 =?utf-8?B?c1JYcFY0ZSthNjM0Y3pXV3g4eVJMTFlnYWxiM1VWMkRyYVh1NWhMTWxqL1Mv?=
 =?utf-8?B?OVI5a2NxcEUwcFNPUXpWLzlxWDNaZmkzeE0wa25rai84OWJGbXdHRzVuUE0r?=
 =?utf-8?B?aURSbGozOXRJMUdmN0t3dzA2VkNwUkNFcEp4clZjWGpxNEVSa004NU93OWVO?=
 =?utf-8?B?blczTU8zay9SNVZ0WVdnZS83UkQvQ2RQTFVrM1FQcWpsenh2Wmc1T1E2UHpw?=
 =?utf-8?B?THcwSXRrVEdYUEQ2OENoZk8rN1JGZEtTck1zUlBiQVBsRW4wNnhDRnFwenhN?=
 =?utf-8?Q?Xxowk1L/4UkGN/oPCrJLnoi7k?=
X-OriginatorOrg: suse.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 9bcb5b40-b100-44e2-f64f-08db98beebbf
X-MS-Exchange-CrossTenant-AuthSource: PA4PR04MB7790.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Aug 2023 09:56:37.0515
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 3hwYTXocxLJ622xV5TeudfxA/tUTi9Hdo8qMUS4W9a8EQY/fAWOIV4NGoymo4eB2j9o6RJA+M2rMDqbJxFxEmA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PAXPR04MB8458
X-Spam-Status: No, score=-2.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_BLOCKED,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,
        URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 9.08.23 г. 10:12 ч., Peter Zijlstra wrote:
> Add a little wrappery to identify the magic symbols that are actually
> inside another instruction -- yay for variable length instruction
> encoding.
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>   tools/objtool/arch/x86/decode.c      |    6 ++++++
>   tools/objtool/check.c                |   13 ++++++++++---
>   tools/objtool/include/objtool/arch.h |    1 +
>   tools/objtool/include/objtool/elf.h  |    1 +
>   4 files changed, 18 insertions(+), 3 deletions(-)
> 
> --- a/tools/objtool/arch/x86/decode.c
> +++ b/tools/objtool/arch/x86/decode.c
> @@ -826,3 +826,9 @@ bool arch_is_rethunk(struct symbol *sym)
>   {
>   	return !strcmp(sym->name, "__x86_return_thunk");
>   }
> +
> +bool arch_is_offset_insn(struct symbol *sym)
> +{
> +	return !strcmp(sym->name, "zen_return_thunk") ||
> +	       !strcmp(sym->name, "srso_safe_ret");
> +}
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -459,8 +459,7 @@ static int decode_instructions(struct ob
>   			 * Both zen_return_thunk() and srso_safe_ret() are embedded inside
>   			 * another instruction and objtool doesn't grok that. Skip validating them.
>   			 */
> -			if (!strcmp(func->name, "zen_return_thunk") ||
> -			    !strcmp(func->name, "srso_safe_ret") || func->alias != func)
> +			if (func->offset_insn || func->alias != func)
>   				continue;
>   
>   			if (!find_insn(file, sec, func->offset)) {
> @@ -1303,6 +1302,11 @@ __weak bool arch_is_rethunk(struct symbo
>   	return false;
>   }
>   
> +__weak bool arch_is_offset_insn(struct symbol *sym)
> +{
> +	return false;
> +}
> +
>   static struct reloc *insn_reloc(struct objtool_file *file, struct instruction *insn)
>   {
>   	struct reloc *reloc;
> @@ -1588,7 +1592,7 @@ static int add_jump_destinations(struct
>   			 * middle of another instruction.  Objtool only
>   			 * knows about the outer instruction.
>   			 */
> -			if (sym && !strcmp(sym->name, "zen_return_thunk")) {
> +			if (sym && sym->offset_insn) {
>   				add_return_call(file, insn, false);
>   				continue;
>   			}
> @@ -2507,6 +2511,9 @@ static int classify_symbols(struct objto
>   		if (arch_is_rethunk(func))
>   			func->return_thunk = true;
>   
> +		if (arch_is_offset_insn(func))
> +			func->offset_insn = true;
> +
>   		if (arch_ftrace_match(func->name))
>   			func->fentry = true;

nit: Why go through this if when one can simply do:

func->foo = arch_is_foo(bar) ?

>   
> --- a/tools/objtool/include/objtool/arch.h
> +++ b/tools/objtool/include/objtool/arch.h
> @@ -90,6 +90,7 @@ int arch_decode_hint_reg(u8 sp_reg, int
>   
>   bool arch_is_retpoline(struct symbol *sym);
>   bool arch_is_rethunk(struct symbol *sym);
> +bool arch_is_offset_insn(struct symbol *sym);
>   
>   int arch_rewrite_retpolines(struct objtool_file *file);
>   
> --- a/tools/objtool/include/objtool/elf.h
> +++ b/tools/objtool/include/objtool/elf.h
> @@ -66,6 +66,7 @@ struct symbol {
>   	u8 fentry            : 1;
>   	u8 profiling_func    : 1;
>   	u8 warned	     : 1;
> +	u8 offset_insn       : 1;
>   	struct list_head pv_target;
>   	struct reloc *relocs;
>   };
> 
>