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=-5.6 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 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 360A3C433DB for ; Sun, 31 Jan 2021 18:55:49 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (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 DCE4A64E53 for ; Sun, 31 Jan 2021 18:55:48 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DCE4A64E53 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.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=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=J4f2nMxrW67FjaCoH2680DQTqcKnVnSmh0ex3EDrFWI=; b=b5ZIIW13B7yeIq/UMW1sRanuh YteTDggeXt8QYrrQ+VVh7O0TV0lti84XtIt0sXfYaFHmgModooXmIzAAE7yUMsNFcor5uuhPT+6Cs PECgzvMb7vJUKeVW0zIAEgABmr7SHEIGsMoBtluY1t9VmUcJ9hCnC9VdaD+1b+iKtAsV7dA+665dt X2SrvnrDMgmYV02jLTlt1cpwCRPEj+h5MudQTmpLF8/4vQQAoOosQOVq4zA2Cv0fJ+5qF1swPnUO4 2bIVUCxB2w9XgXYTk50xaddKyiEQO4qBaBxu0b37OmZq7XwmaYFuqNj0WMOuUrpEToow97Obv4mnJ bMWWZ/K3A==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1l6Hs3-0008G1-5V; Sun, 31 Jan 2021 18:54:51 +0000 Received: from mail.kernel.org ([198.145.29.99]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1l6Hs1-0008FX-5P for linux-arm-kernel@lists.infradead.org; Sun, 31 Jan 2021 18:54:49 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id 9B1DB64E31; Sun, 31 Jan 2021 18:54:46 +0000 (UTC) Date: Sun, 31 Jan 2021 18:54:44 +0000 From: Catalin Marinas To: Linus Torvalds Subject: Re: [GIT PULL] arm64 fixes for 5.11-rc6 Message-ID: <20210131185443.GA29083@gaia> References: <20210129190322.GA4590@gaia> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210131_135449_267285_37468E3E X-CRM114-Status: GOOD ( 17.56 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Lorenzo Pieralisi , Will Deacon , Linux Kernel Mailing List , Linux ARM 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 Fri, Jan 29, 2021 at 02:09:05PM -0800, Linus Torvalds wrote: > On Fri, Jan 29, 2021 at 11:03 AM Catalin Marinas > wrote: > > > > arm64 fixes: > > > > - Fix the virt_addr_valid() returning true for < PAGE_OFFSET addresses. > > That's a really odd fix. > > It went from an incorrect bitwise operation (masking) to an _odd_ > bitwise operation (xor). > > Yes, PAGE_OFFSET has the bit pattern of all upper bits set, so "(addr > ^ PAGE_OFFSET)" by definition reverses the upper bits - and for a > valid case turns them to zero. > > But isn't the *logical* thing to do to use a subtract instead? For the > valid cases, the two do the same thing (clear the upper bits), but > just conceptually, isn't the operation that you actually want to do > "(addr - PAGE_OFFSET)"? > > IOW, why is it using that odd xor pattern that doesn't make much > sense? I believe it _works_, but it looks very strange to me. This macro used to test a single bit and it evolved into a bitmask. So, yes, basically what we need is: #define __is_lm_address(addr) ((u64)(addr) >= PAGE_OFFSET && \ (u64)(addr) < PAGE_END) I wasn't sure whether the code generation with two comparisons is similar to the xor variant but the compiler should probably be smart enough to use CMP and CCMP. In the grand scheme, it probably doesn't even matter. Unless I miss something, I don't see any overflow issues even if we do (((u64)addr - PAGE_OFFSET) < (PAGE_END - PAGE_OFFSET)). We can backport the fix already upstream and clean-up the code in mainline going forward (after some sanity check on the code generation). It would be easier to parse in the future. > Also, shouldn't _lm_to_phys() do the same? It does that "mask upper > bits" too that was problematic in __is_lm_address(). Again, shouldn't > that logically be a subtract op? Yes, that's similar and a subtract should do. -- Catalin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel