Received: by 2002:a05:6359:6284:b0:131:369:b2a3 with SMTP id se4csp1407879rwb; Sat, 5 Aug 2023 14:36:48 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHL+lqrgAissQaOxBxZLahahz1oAIS1kjywCqwmmo/qek2Hv4pc/Le0EDrPTAwa7SHberYr X-Received: by 2002:a05:6a21:32a6:b0:13f:e0b0:3578 with SMTP id yt38-20020a056a2132a600b0013fe0b03578mr5492815pzb.49.1691271407852; Sat, 05 Aug 2023 14:36:47 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1691271407; cv=none; d=google.com; s=arc-20160816; b=DgDFklxXgINmCJb8XmOjSmC/K6fpudnQxexhEdBsp1QoJDEomvaE5uDX1rV5lPrlK8 rlj+MndKYAhaX5AyyS8blOG0Gex1VuXsub24/jBSrTxCf5eQMY4onFq3dJqk5mXpdu0Q YuM2oeOxJOB1d54CyZEbKznTL2FkMk2ESeFc/V/ILqkVv+2LY39xlMI93VIuTZdqW53f wGFNeepDEBBm6E1rNdlGrDR2NyQnr2EOotAYh5nO2PMJqAAHwbiAuvjPeCTUgewwTWmf dqHTUPXC5+7IX3WdzD982uZ+zXioVJ2ntFNbhXbhhBgABbGO5zjxKvQh5qeaxgRjuh7d JMSQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:mime-version:message-id:date:references :in-reply-to:subject:cc:to:dkim-signature:dkim-signature:from; bh=gSbKeXLBsgKqGj/TwqTJ9+PojQPLuVRP6JDwmEEGuoE=; fh=k521Y10Xq6Npd4Lu2K/1b6VnEmYyYEoA9hG26CJsKoI=; b=Ktna1zJZBlX5NnwPiwNuBHqaIh23GKXe2/hdC8JHcO++pggJmKVeKQKNUMpyzGy4lO AVkhyUZJog28O1Q0N8PK6wF/2mCxxS5bO4tJAa54S3OMuEIgTDtcwOoHqQDBtfwzcaWE 9F9SGXWSocDVzSoY9IEgXNZvG8rj/gxNSv1fIvgGdAArG6uKKfrWjT/6yB2Ti93kmuk4 uKQ8qsEhkYcQzE9sTfswhcXscI+i9SqPSqLJ0+dPdOvzidUoA/wb7D9I53bF2NoXY9ak 5l0hNNb0x6HhkaP6d/xBM4jxKRm+tNbEGFTytG98pVvD3tl+b0OcmpRkMUSVWoeXQuY0 Cf0w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linutronix.de header.s=2020 header.b=G1L7HlLW; dkim=neutral (no key) header.i=@linutronix.de header.s=2020e header.b=tJDGqJG9; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=linutronix.de Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id 85-20020a630058000000b0055b7319a311si3523058pga.356.2023.08.05.14.36.34; Sat, 05 Aug 2023 14:36:47 -0700 (PDT) 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; Authentication-Results: mx.google.com; dkim=pass header.i=@linutronix.de header.s=2020 header.b=G1L7HlLW; dkim=neutral (no key) header.i=@linutronix.de header.s=2020e header.b=tJDGqJG9; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=linutronix.de Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229978AbjHEVDE (ORCPT + 99 others); Sat, 5 Aug 2023 17:03:04 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46300 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229542AbjHEVDD (ORCPT ); Sat, 5 Aug 2023 17:03:03 -0400 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 71AC4E7 for ; Sat, 5 Aug 2023 14:03:02 -0700 (PDT) From: Thomas Gleixner DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1691269380; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=gSbKeXLBsgKqGj/TwqTJ9+PojQPLuVRP6JDwmEEGuoE=; b=G1L7HlLWpNbkQsJ5hzO4fWWiZ+FtA0lxn/PDsGsygDHKd82Z1Dlrc+/V4zug84FWYwNTBe /MysUkkx5IjU9x62KFTC8slW5q048LdNDfzZMBpZeqJSQy/30me7wEOUXlVjdPRelQbtal PO6QKSTMLwFh1ZFT3SgL5IaxVVsyZYXcDeEnOPSs4sdPvE0UEVxtoBueWh4tk9uHJSamI+ i5R7rN+6OmoKEx9+5sfv1irx9EVO+J6UgKEYv/QSZ6F+D6Pqc9F5j/CSbBB3bfKsrm+o38 IuwmqsyUupUM8g7ufi9tYo9fYSwRTi7bhGAE78K3uoeRUpbdCG8xwWbO1DDugQ== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1691269380; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=gSbKeXLBsgKqGj/TwqTJ9+PojQPLuVRP6JDwmEEGuoE=; b=tJDGqJG9ZGL0jQfrmZogiEnjYq0oxLwLymgfLwSzBNsVGW5UDfDiT9E9gA4oG6BZEn37Qr KJ2/cN0ZZLuWxOCA== To: Peter Zijlstra , "Li, Xin3" Cc: "linux-kernel@vger.kernel.org" , "xen-devel@lists.xenproject.org" , "Lutomirski, Andy" , Ingo Molnar , Borislav Petkov , Dave Hansen , "x86@kernel.org" , "H . Peter Anvin" , "Gross, Jurgen" , "Ostrovsky, Boris" Subject: Re: [RFC PATCH 1/1] x86/traps: Get rid of exception handlers' second argument error code In-Reply-To: <20230804190120.GP212435@hirez.programming.kicks-ass.net> References: <20230804075734.8372-1-xin3.li@intel.com> <20230804101321.GH214207@hirez.programming.kicks-ass.net> <20230804190120.GP212435@hirez.programming.kicks-ass.net> Date: Sat, 05 Aug 2023 23:02:59 +0200 Message-ID: <87o7jlmccc.ffs@tglx> MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED, SPF_HELO_NONE,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: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Aug 04 2023 at 21:01, Peter Zijlstra wrote: > On Fri, Aug 04, 2023 at 05:35:11PM +0000, Li, Xin3 wrote: >> > > The commit d99015b1abbad ("x86: move entry_64.S register saving out of >> > > the macros") introduced the changes to set orig_ax to -1, but I can't >> > > see why it's required. Our tests on x86_64 and x86_32 seem fine if >> > > orig_ax is left unchanged instead of set to -1. >> > >> > That means that SYSCALL_NUM(regs) get to be garbage; or something like that. >> >> I find SYSCALL_NUM(regs) in tools/testing/selftests/seccomp/seccomp_bpf.c, >> but nothing obvious to me. >> >> I think it's clear that once exceptions and IRQs are handled, the original >> context will be fully recovered in a normal case. >> >> Is it related to preemption after such a event? >> >> I must have missed something; can you please elaborate it? > > arch/x86/include/asm/syscall.h > > syscall_get_nr() syscall_rollback() Specifically it breaks signal handling. See arch_do_signal_or_restart() and handle_signal(). So, no. This cannot be changed nilly willy especially not because orig_ax is part of the UABI. Even if it would work, this patch is completely unreviewable. There are smarter ways to make such a change in digestable chunks. I also looked at the latest FRED series and that's broken vs. orig_ax in the same way. Aside of it the series is not applying to any tree I'm aware of. It fails because its based on a tree which has IRQ_MOVE_CLEANUP_VECTOR removed, but that's nowhere in mainline or tip. This is not the way it works, really. Back to the problem at hand. I'm absolutely not understanding why this all is so complicated. FRED pushs the error code or 0 (in case there is no error code) right into orig_ax. That's where the ERET[US] frame starts. ERT[US] do not care at all about the error code location, they skip it by adding 8 to RSP. So the obvious thing to do is: fred_entry_from_xxxx(struct pt_regs *regs) { unsigned long error_code = regs->orig_ax; regs->orig_ax = -1; dispatch_1st_level(regs->type, regs, error_code); } Now the dispatching code in your series looks nice and shiny with all those tables, but in practice it's not so shiny at all. For one this code forces indirect calls even in places where there are no indirect calls required. But what's worse it causes you to make everything uniform one way or the other and adding all this dispatch_table_##func muck, which is fully duplicated code for absolutely zero reason. That's error prone and a maintainence mess. The vast majority of exceptions, interrupts whatever is not special at all neither on FRED nor on IDT. The ones which need special treatment are those which store additional information in the stack frame and these are the exception - not the rule. And those require special treatment anyway whether you make the rest look uniform or not. IOW, your approach of making everything uniform is completely wrong. There is a reason why some system vectors are not using DEFINE_IDTENTRY_SYSVEC(). Particularly the RESCHEDULE_VECTOR. This has been performance optimized with a lot of effort and no, we are not going to sacrifice that just because it makes it sooo simple for FRED. This is not about simplicity. This is about sane and maintainable code which preserves the carefully optimized code paths which are hotpath no matter whether there is IDT or FRED. I'm going to reply on your last failed resend attempt after creating a sane mail thread out of the mess you created (including the insanely large and inappropriate cc list). Thanks, tglx