From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_ADSP_CUSTOM_MED,DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8D62CC47082 for ; Tue, 8 Jun 2021 22:02:20 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 4D9DE61184 for ; Tue, 8 Jun 2021 22:02:20 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4D9DE61184 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=S4A3GJJlcOMeVRGoKsb0I9DaCfN0TIPmQgjdxQ6GJFg=; b=Os6S/hn8B1cxCI tVgXhBqfrQH7btOdcTbTmPG9iKF1IxgGs8CzcLN3xQv5qs51wQSrUEYJqmdbkNWpWfAw9XOmQBqvm jYGSUx3bkvoCkCGhjFcRzk0QxQ4Yhd9WT/ItNaj33Ae0gfIcjO55GpvGFV/dzK8CRuwB4cs003JAG yBbL5I1nfS4En/Ny+oKt1zFIvyVPYdfsyaoS1LdMmiR/6smOU78NmETg4PD4/2kVWcpwoWUq5ruy8 Qui19jo7uttndyz+ESTs9ylaiyRSo4kAUSziiEvh45Vy1XwfYR+BgKIKGkPksRm3qyyNjfjC+fAvb hmGvIEExzKO4y8G8X/dg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1lqjjf-00Acv3-4p; Tue, 08 Jun 2021 21:58:11 +0000 Received: from mail-vs1-xe2a.google.com ([2607:f8b0:4864:20::e2a]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1lqjjb-00AbKA-4r for linux-arm-kernel@lists.infradead.org; Tue, 08 Jun 2021 21:58:08 +0000 Received: by mail-vs1-xe2a.google.com with SMTP id z15so11748851vsn.13 for ; Tue, 08 Jun 2021 14:56:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=pNhsu4RFXMFW8mqpRlhWry39BLUT2LJW6xrf+UIarrs=; b=cm4BQRiGIrMiLyxw1qx3hiJtzOIoB12T3CdKLp6Bull8sybpdZ3cF8UHlRDe2wdoKG 8VbQ2Wf8K0iJxCeF0gPVMKdGQaVppY1thf/wu4Kez7/ho9ssbcv3CvZOjUSUPAEZZTgT iokVZXxsqwgFm05r40lU7B1ZcexH4WkeSXpdYz9geZMuIqE5PKebIbsT9fuvVblkbTXj +MruT9TCMw6YQmAqwoSikGi4ibPe06U3AlXN837hY+n08JEY70OiBz6uMd6xJPzFuCPx pZy6U+tLQ+AVr0Vgp7Lj9e0OgDrjN7kayDd4IAR4/4vPAJoWlPQePK+7EdcuOhgAPJGo fF4A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=pNhsu4RFXMFW8mqpRlhWry39BLUT2LJW6xrf+UIarrs=; b=bplg9freJph21niCMGyOg5Yb8vv2wY60I3iy+ki5BwG/+P7acrsQB7Zla2NLEYQI8I ldpIl9iDoDyrkoVNEKQD+rCVRbg3VLs1qXV0ik/8qo3JQcNq6Hd4LmpNG1mRZIZrFXNE K/1Nl/6t8sMmB7s6goQfDsj+5Byj0Idkw99a5xxcv1yi1Jo8ejsltt+IXMkBdDRdsku6 pZG7e3G5Roz4ul+e+UBg/WlnKAbw1DNEEHERxMzKPPl5WUZx8Ac74R7Fhnw59go2vrXA 25/qUFroFLyz2DMrU74nhJrqglQMu/mc70/mxnuuaY7LA5uw9GQ3Gw6U63F89CNUSCSG 9Xug== X-Gm-Message-State: AOAM531yxajMRNPAxK9Tm5IXJFGt2WwzwJb2C7y/F+hRnxL59Yc7PFB3 kj5OxJhEYLQzC+ePtTllPF4/fz7NLiVb3cir+NbsHA== X-Google-Smtp-Source: ABdhPJw84Syx7XG7zELXeNuHBO/8/9FYD8mSD4+m9rH79uippk5VxsIknfinKQKwOffg0VOPyQ1QL1TMgGQhJLfFRks= X-Received: by 2002:a05:6102:2221:: with SMTP id d1mr2550489vsb.28.1623189360907; Tue, 08 Jun 2021 14:56:00 -0700 (PDT) MIME-Version: 1.0 References: <20210602232445.3829248-1-pcc@google.com> <5ee9d9a1-5b13-ea21-67df-e713c76fc163@arm.com> <20210608144353.GF17957@arm.com> In-Reply-To: From: Evgenii Stepanov Date: Tue, 8 Jun 2021 14:55:49 -0700 Message-ID: Subject: Re: [PATCH] arm64: mte: allow async MTE to be upgraded to sync on a per-CPU basis To: Peter Collingbourne Cc: Catalin Marinas , Vincenzo Frascino , Will Deacon , Linux ARM X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210608_145807_240245_5835B448 X-CRM114-Status: GOOD ( 66.29 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Jun 8, 2021 at 12:54 PM Peter Collingbourne wrote: > > On Tue, Jun 8, 2021 at 7:44 AM Catalin Marinas wrote: > > > > On Thu, Jun 03, 2021 at 10:49:24AM -0700, Peter Collingbourne wrote: > > > On Thu, Jun 3, 2021 at 6:01 AM Vincenzo Frascino > > > wrote: > > > > On 6/3/21 12:24 AM, Peter Collingbourne wrote: > > > > > On some CPUs the performance of MTE in synchronous mode is the same > > > > > as that of asynchronous mode. This makes it worthwhile to enable > > > > > synchronous mode on those CPUs when asynchronous mode is requested, > > > > > in order to gain the error detection benefits of synchronous mode > > > > > without the performance downsides. Therefore, make it possible for CPUs > > > > > to opt into upgrading to synchronous mode via a new mte-prefer-sync > > > > > device tree attribute. > > > > > > > > > > > > > I had a look at your patch and I think that there are few points that are worth > > > > mentioning: > > > > 1) The approach you are using is per-CPU hence we might end up with a system > > > > that has some PE configured as sync and some configured as async. We currently > > > > support only a system wide setting. > > > > > > This is the intent. On e.g. a big/little system this means that we > > > would effectively have sampling of sync MTE faults at a higher rate > > > than a pure userspace implementation could achieve, at zero cost. > > > > > > > 2) async and sync have slightly different semantics (e.g. in sync mode the > > > > access does not take place and it requires emulation) this means that a mixed > > > > configuration affects the ABI. > > > > > > We considered the ABI question and think that is somewhat academic. > > > While it's true that we would prevent the first access from succeeding > > > (and, more visibly, use SEGV_MTESERR in the signal rather than > > > SEGV_MTEAERR) I'm not aware of a reasonable way that a userspace > > > program could depend on the access succeeding. > > > > It's more about whether some software relies on the async mode only for > > logging without any intention of handling the synchronous faults. In > > such scenario, the async signal is fairly simple, it logs and continues > > safely (well, as "safe" as before MTE). With the sync mode, however, the > > signal handler will have to ensure the access took place either before > > continuing, either by emulating, restarting the instruction with > > PSTATE.TCO set or by falling back to the async mode. > > > > IOW, I don't expect all programs making use of MTE to simply die on an > > MTE fault (though I guess they'll be in a minority but we still need to > > allow such scenarios). > > Okay, allowing such programs seems reasonable to me. The question is > then whether the new behavior should be an opt-in or an opt-out. > > > > While it's slightly > > > more plausible that there could be a dependency on the signal type, we > > > don't depend on that in Android, at least not in a way that would lead > > > to worse outcomes if we get MTESERR instead of MTEAERR (it would lead > > > to better outcomes, in the form of a more accurate/detailed crash > > > report, which is what motivates this change). I also checked glibc and > > > they don't appear to have any dependencies on the signal type, or > > > indeed have any detailed crash reporting at all as far as I can tell. > > > Furthermore, basically nobody has hardware at the moment so I don't > > > think we would be breaking any actual users by doing this. > > > > While there's no user-space out there yet, given that MTE was merged in > > 5.10 and that's an LTS kernel, we'll see software running on this kernel > > version at some point in the next few years. So any fix will need > > backporting but an ABI change for better performance on specific SoCs > > hardly counts as a fix. > > Okay, seems reasonable enough. > > > I'm ok with improving the ABI but not breaking the current one, hence > > the suggestion for a new PR_MTE_TCF_* field or maybe a new bit that > > allows the mode to be "upgraded", for some definition of this (for > > example, if TCF_NONE is as fast as TCF_ASYNC, should NONE be upgraded?) > > I think the possibility of upgrading NONE to ASYNC is a good reason to > make this an opt-in, since it alters the behavior in a more > significant way than ASYNC to SYNC. > > > > > 3) In your patch you use DT to enforce sync mode on a CPU, probably it is better > > > > to have an MIDR scheme to mark these CPUs. > > > > > > Okay, so in your scheme we would say that e.g. all Cortex-A510 CPUs > > > should be subject to this treatment. Can we guarantee that all > > > Cortex-A510 CPUs would have the same performance for sync and async or > > > could the system designer tweak some aspect of the system such that > > > they could get different performance? The possibility of the latter is > > > what led me to specify the information via DT. > > > > While it's more of a CPU microarchitecture issue, there's indeed a good > > chance that the SoC has a non-trivial impact on the performance of the > > synchronous mode, so it may tip the balance one way or another. > > > > Another idea would be to introduce a PR_MTE_TCF_DEFAULT mode together > > with some /sys/*/cpu*/ entries to control the default MTE mode per CPU. > > We'd leave it entirely to user-space (privileged user), maybe it even > > wants to run some checks before tuning the default mode per CPU. > > That's an interesting idea, but it sounds like it wouldn't permit > upgrading from NONE as a separate feature from upgrading from ASYNC. For the userspace API, I like the idea of a new bit flag with the meaning "upgrade to a stricter mode with similar performance". On a hypothetical CPU where SYNC mode has negligible overhead this could upgrade NONE all the way to SYNC. It would also allow transparent opt-in to any future modes, like asymmetric. A /sys interface in addition to DT would be nice to have, but with this API we already have an escape hatch in the userspace - remove the bit, or even have libc forcibly remove the bit for all prctl calls. > > > I'm pretty sure the sync vs async decision is not a clear cut (e.g. sync > > may still be slower but within a tolerable margin for certain > > benchmarks). Leaving the decision to the hardware vendor, hard-coded in > > the DT, is probably not the best option (nor is the MIDR). Some future > > benchmark with a weird memory access pattern could expose slowness in > > the sync mode and vendors will scramble on how to change the DT already > > deployed (maybe they can do this OTA already). > > Perhaps we can let the default upgrading settings be specified via DT, > and allow them to be overridden later via sysfs. That seems like the > best of both worlds, since I think that in most cases DT should be > enough, while the ability to override should satisfy the remaining > cases. It also means that ACPI users have a way to use this feature > until it gets added to the spec. > > By the way, if we want to allow upgrading from NONE in the future, or > upgrading to asymmetric mode, perhaps we should spell the option > "mte-upgrade-async" and have it take an argument in the range 0-3 > (corresponding to the SCTLR_EL1.TCF bits) specifying the desired mode > to upgrade to. Then we could have e.g. "mte-upgrade-none" later with > the same semantics. > > Peter _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel