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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 05364C3DA7F for ; Thu, 15 Aug 2024 18:54:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Type:In-Reply-To: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=IhERV/jL6UlsHTgwO0rZfirsXuvPDZ9y+GMqTvo5SLU=; b=WHUYWZkcMvMdgatjOhm/NuLH99 bxhvMJOwX/Eatx534Sq+8JD2kPqwH7ukr/yHb9Omx5mHm/z3NlP/aG58dCWWJI64dWTUo/0kk8c3N PfDgK6bVEtQopVyOdKZABGd6GFz+BZJqgWCs2r8GATlYNMpCfOjKzxbOt9ZKGP0kBlW+nFbQhCYch sm+0c6zqOM2lPVUXI6b2OOWHjq/y50RtAn1fMKOXGhnEZp4Mn/kobFp/oNPYPZA3Q5sp/iw/gJz51 MsRJLhjseKNTdMn+pVhugOLizMybUAh9AdREKhIgh1GgxDw2WUY+r/TWdMLOXU+9EGvsHtUtl2dMk n+eb2bFg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sefbT-0000000Ap3I-2fg7; Thu, 15 Aug 2024 18:53:43 +0000 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sefap-0000000AotY-0TVn for linux-arm-kernel@lists.infradead.org; Thu, 15 Aug 2024 18:53:04 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1723747982; 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=IhERV/jL6UlsHTgwO0rZfirsXuvPDZ9y+GMqTvo5SLU=; b=BFbTxrKo9/McRJxpw+qQzf+ccyVRCXnvu9fvarenHIVc8v80/W7dyzSaqFTL9aMSQ0wG/5 aeVueHxLx++x6rZQuPk4uU1VpYPOvsFxgjGI9k4I1BG0TfM1Kl+a5S5RELp472MFxcHza5 /n8eKvl9ANV4q2w2DYmFUc6HcI1rcU4= Received: from mail-vs1-f72.google.com (mail-vs1-f72.google.com [209.85.217.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-651-fef2RkR7PpW6yxah6K9ZLA-1; Thu, 15 Aug 2024 14:53:00 -0400 X-MC-Unique: fef2RkR7PpW6yxah6K9ZLA-1 Received: by mail-vs1-f72.google.com with SMTP id ada2fe7eead31-492a17040e1so32217137.0 for ; Thu, 15 Aug 2024 11:53:00 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723747980; x=1724352780; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=IhERV/jL6UlsHTgwO0rZfirsXuvPDZ9y+GMqTvo5SLU=; b=rm0Yp6Pc4FeWUdYRpmjZVx76anSQcTjBS3avnJH+Eu6VBfVU78x6yWYxbdE50n8/Rp olOMOQe8PXmy5m8mLvOLs+2eAROinT2JtrC3JUUhyzdQc5UD8YZwSQTDMMKYH0m/E9y3 T0e0xlDcVx5Xt7hh+JYKlz3z4PmCHtClvqz2Q2RCQYe8bPpG/2GsTBevzICNM2miQA1r wzpJ2Cs/ZJFnDBM9tmxFE/1GBzexiV8Y8Mri9BsWQMuLtUx7GgogLe0JIe+3X9t/U9X+ kE51mYs5byUyxiEskvEQa3gEkQMUQgd53QFZjynClm6INK1K+CsIZGEmLddvgyzWxmyK tmxA== X-Forwarded-Encrypted: i=1; AJvYcCUav6fSRWPkZ3x3JL7URPAeqw8+s85eomGuN0IfhI4Y4bXBegGDZ7RXpREfqinvau4LD3nK0+uuB3wb3K/+gieh@lists.infradead.org X-Gm-Message-State: AOJu0Ywpojne5KN7JCYqpQeeYwyM4M6RYN5yRDr+e+UXBGpfEy96Qaaz JALdgcxEtKp4xODE1hzwX7gaMkHvEZWo35VXJCHP8mNgmbIka5kGmg0UWZDUNGdtF8HoGz4oGE+ ChbcS6GIY+Vxjbu5TP+AXK2R3weJ+4u0AdN75Q86j92UnrFGZzLoxUGQVpxbvcw1I1ScD/NJJ X-Received: by 2002:a05:6102:cc7:b0:48d:aced:abff with SMTP id ada2fe7eead31-497798dde56mr529537137.1.1723747980115; Thu, 15 Aug 2024 11:53:00 -0700 (PDT) X-Google-Smtp-Source: AGHT+IH5mqFNb1PXl4pf/EBUVsWqezNxIo7H5nHPjHSbbxlg0TjpV2kIgUEe/oeqxt6K3fJhgIgYNg== X-Received: by 2002:a05:6102:cc7:b0:48d:aced:abff with SMTP id ada2fe7eead31-497798dde56mr529525137.1.1723747979713; Thu, 15 Aug 2024 11:52:59 -0700 (PDT) Received: from x1n (pool-99-254-121-117.cpe.net.cable.rogers.com. [99.254.121.117]) by smtp.gmail.com with ESMTPSA id af79cd13be357-7a4ff105fe3sm88156085a.109.2024.08.15.11.52.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 15 Aug 2024 11:52:59 -0700 (PDT) Date: Thu, 15 Aug 2024 14:52:56 -0400 From: Peter Xu To: Jason Gunthorpe Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Sean Christopherson , Oscar Salvador , Axel Rasmussen , linux-arm-kernel@lists.infradead.org, x86@kernel.org, Will Deacon , Gavin Shan , Paolo Bonzini , Zi Yan , Andrew Morton , Catalin Marinas , Ingo Molnar , Alistair Popple , Borislav Petkov , David Hildenbrand , Thomas Gleixner , kvm@vger.kernel.org, Dave Hansen , Alex Williamson , Yan Zhao Subject: Re: [PATCH 09/19] mm: New follow_pfnmap API Message-ID: References: <20240809160909.1023470-1-peterx@redhat.com> <20240809160909.1023470-10-peterx@redhat.com> <20240814131954.GK2032816@nvidia.com> <20240814221441.GB2032816@nvidia.com> <20240815161603.GH2032816@nvidia.com> <20240815172445.GK2032816@nvidia.com> MIME-Version: 1.0 In-Reply-To: <20240815172445.GK2032816@nvidia.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240815_115303_256412_C355782D X-CRM114-Status: GOOD ( 41.56 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, Aug 15, 2024 at 02:24:45PM -0300, Jason Gunthorpe wrote: > On Thu, Aug 15, 2024 at 01:21:01PM -0400, Peter Xu wrote: > > > Why? Either the function only returns PFN map no-struct page things or > > > it returns struct page stuff too, in which case why bother to check > > > the VMA flags if the caller already has to be correct for struct page > > > backed results? > > > > > > This function is only safe to use under the proper locking, and under > > > those rules it doesn't matter at all what the result is.. > > > > Do you mean we should drop the PFNMAP|IO check? > > Yeah > > > I didn't see all the > > callers to say that they won't rely on proper failing of !PFNMAP&&!IO vmas > > to work alright. So I assume we should definitely keep them around. > > But as before, if we care about this we should be using vm_normal_page > as that is sort of abusing the PFNMAP flags. I can't say it's abusing.. Taking access_remote_vm() as example again, it can go back as far as 2008 with Rik's commit here: commit 28b2ee20c7cba812b6f2ccf6d722cf86d00a84dc Author: Rik van Riel Date: Wed Jul 23 21:27:05 2008 -0700 access_process_vm device memory infrastructure So it starts with having GUP failing pfnmaps first for remote vm access, as what we also do right now with check_vma_flags(), then this whole walker is a remedy for that. It isn't used at all for normal VMAs, unless it's a private pfnmap mapping which should be extremely rare, or if it's IO+!PFNMAP, which is a world I am not familiar with.. In all cases, I hope we can still leave this alone in the huge pfnmap effort, as they do not yet to be closely relevant. From that POV, this patch as simple as "teach follow_pte() to know huge mappings", while it's just that we can't modify on top when the old interface won't work when stick with pte_t. Most of the rest was inherited from follow_pte(); there're still some trivial changes elsewhere, but here on the vma flag check we stick the same with old. > > > > Any physical address obtained through this API is only valid while > > > the @follow_pfnmap_args. Continuing to use the address after end(), > > > without some other means to synchronize with page table updates > > > will create a security bug. > > > > Some misuse on wordings here (e.g. we don't return PA but PFN), and some > > sentence doesn't seem to be complete.. but I think I get the "scary" part > > of it. How about this, appending the scary part to the end? > > > > * During the start() and end() calls, the results in @args will be valid > > * as proper locks will be held. After the end() is called, all the fields > > * in @follow_pfnmap_args will be invalid to be further accessed. Further > > * use of such information after end() may require proper synchronizations > > * by the caller with page table updates, otherwise it can create a > > * security bug. > > I would specifically emphasis that the pfn may not be used after > end. That is the primary mistake people have made. > > They think it is a PFN so it is safe. I understand your concern. It's just that it seems still legal to me to use it as long as proper action is taken. I hope "require proper synchronizations" would be the best way to phrase this matter, but maybe you have even better suggestion to put this, which I'm definitely open to that too. > > > It sounds like we need some mmu notifiers when mapping the IOMMU pgtables, > > as long as there's MMIO-region / P2P involved. It'll make sure when > > tearing down the BAR mappings, the devices will at least see the same view > > as the processors. > > I think the mmu notifiers can trigger too often for this to be > practical for DMA :( I guess the DMAs are fine as normally the notifier will be no-op, as long as the BAR enable/disable happens rare. But yeah, I see you point, and that is probably a concern if those notifier needs to be kicked off and walk a bunch of MMIO regions, even if 99% of the cases it'll do nothing. Thanks, -- Peter Xu