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;
Date:   Thu, 11 May 2023 14:56:22 +0800
From:   Yan Zhao <yan.y.zhao@intel.com>
To:     Jason Gunthorpe <jgg@nvidia.com>
CC:     Alex Williamson <alex.williamson@redhat.com>,
        <kvm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <kevin.tian@intel.com>, <yishaih@nvidia.com>,
        <shameerali.kolothum.thodi@huawei.com>,
        =?iso-8859-1?Q?C=E9dric?= Le Goater <clg@redhat.com>
Subject: Re: [PATCH] vfio/pci: take mmap write lock for io_remap_pfn_range
Message-ID: <ZFyRlnXmEy6Enk4w@yzhao56-desk.sh.intel.com>
Reply-To: Yan Zhao <yan.y.zhao@intel.com>
References: <20230508125842.28193-1-yan.y.zhao@intel.com>
 <ZFkn3q45RUJXMS+P@nvidia.com>
 <20230508145715.630fe3ae.alex.williamson@redhat.com>
 <ZFwBYtjL1V0r5WW3@nvidia.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <ZFwBYtjL1V0r5WW3@nvidia.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?b7kDvM1d5xm+SEtpwHy0DKOdeQVBX5to/m9QNC28UWnD2BwIBn2SHRDOi0Gt?=
 =?us-ascii?Q?Y2IH16g0HGjrfwdyO/LwMeUou4GE6uz786pqGr0k48OBRmqeHrjvG7v3n6gg?=
 =?us-ascii?Q?vFqdlIRofhazI+vsOP3nfCXsonJmpvnc+1GdOSvLHPpttMeoyUlch0XtedGJ?=
 =?us-ascii?Q?i2nTvEmjUi2QVVxfEBk9Mn2wbnbaGTiX0zQq1G+x91BRewQkOl+Rtzjcty75?=
 =?us-ascii?Q?BtfxqoAAcnFBsvyMti13El8NqXezby//63rFYpEYg++YfXRiJieP9R6OAT4U?=
 =?us-ascii?Q?rpVvIbN8fErJmUrv49NNzx9UZ3Fmr4ZSpFP6c3lahppAMQ9uO8woDssCcIQJ?=
 =?us-ascii?Q?rpl6elaqHnaCBfRz90EqQPu2K2cNxO2CxJcEzkcPv4tiqXPznclpm2cTAi74?=
 =?us-ascii?Q?iY4k5iC76xmYn2m+VCZaTr3U793hzMrfTDJb+BVYS2GKa6vx67iCltRzVJC8?=
 =?us-ascii?Q?QeRUJvRjKNFRevYERHmnGK3chNYUg1MffZiy1FjmgYMCDPrHINQwkW5MUnFf?=
 =?us-ascii?Q?t3lcye1AAThkk7oOodmaVI7hxNScaHpb7zLacn0gLzwTsQ+z9ME9/PZJXbxD?=
 =?us-ascii?Q?VILvcCa7wTnbSXoGPlRqse79iag3qYnAlrQXtnWEwKJMei/hTA6mlyttfLYZ?=
 =?us-ascii?Q?4nXeuJHvY5kNsdLnTZe9Nt//hO6uvrXdt03XTA5nPhCMVbq09ZJhxAZpTGUJ?=
 =?us-ascii?Q?m4VhRl4evbA6whkfaGMk/3EF8caurPr0t8AtChwT8bzMf0gm6BPJHv8DoIJw?=
 =?us-ascii?Q?u/c4Aieex7ii1w/3a1DIyzs/gB4iWDF6b0Nn5AEaRRd0obNRR9K/3RhhgAoS?=
 =?us-ascii?Q?Mebjgfi7YSPThVFL72/AVYA6CXJehGJm5yT6hsbajE3eMfXsXDJ9IIloQTaQ?=
 =?us-ascii?Q?/W+hE+CAbeBEH0v65PnembSx7QSKBRyqWBjLq7QY7XVsKePdck/EtjrhhaTv?=
 =?us-ascii?Q?nBV54eTTCNn6XpyckKWwwZ4tvBD22qFDWbNtReR3Qgt+g37BDqE0WQgHsvv4?=
 =?us-ascii?Q?yuvZNi3QPGm8Vcx3cbPE/wbDs+WgJiKGGW/clp/HxiWTwdhS6J9yk8N9eWHY?=
 =?us-ascii?Q?qiGNkHJb0+nHzGVlNOFgTzDQ6awPvBIfGrqNvcj7PeLp+gImrMY5rYqwLtQ3?=
 =?us-ascii?Q?7uJUeyadRRlU+xkl5iZyxcywCy7Qhn+iOZx/mOzKYkSi8Volx0cXgyd3Y8AA?=
 =?us-ascii?Q?2ibI/0WJqOl7OBsHQ+nw4vMK5hVdoT6MVy4MUiSE9dFSlNSgfZo/Pxo5CRhk?=
 =?us-ascii?Q?b5/9+ElzUr+iZrh8GoIlZy1smBSZ9DwKQtsvxHBVGsV7vAfxqgG2wgqtRQC0?=
 =?us-ascii?Q?v4tsC8Mq/5+GqzIn/lwebXYgacKKocAqtYLR+Rc33LxbSWAjfsjj1dvtlOoB?=
 =?us-ascii?Q?oovPl1rEsu4Ge8/kKS7TOei15aN3o0Yrb/hPfxWSppDiKaD0Swo/PB4Nq0KW?=
 =?us-ascii?Q?zt+qrFk5ED1Rf4PkyqgD8/dM3PdyE7NB6BaeGW8o8Snaju11oUZpdg+ZgI0/?=
 =?us-ascii?Q?+Q0IMAdPcrqXZQC8OK5JAYprauGsSDdXz5i64b788ew5Dk7hiGzJ2DpSjd+2?=
 =?us-ascii?Q?y6wUG+HNgREpQ6g6Vm3a6snf5G8YRjWInZMCglLc?=
X-MS-Exchange-CrossTenant-Network-Message-Id: f471657e-4cea-4da9-aeac-08db51f0564b
X-MS-Exchange-CrossTenant-AuthSource: DS7PR11MB5966.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 May 2023 07:21:28.8460
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: jxj5gvGBcQiSlqVCepGeGVB7kdalMVr58/vKTml0PgZfYgdBFXX2IDDsnDF93jSn+jU+5/4P2sL0cHOA12qt7A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR11MB6942
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE 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 Wed, May 10, 2023 at 05:41:06PM -0300, Jason Gunthorpe wrote:
> On Mon, May 08, 2023 at 02:57:15PM -0600, Alex Williamson wrote:
> 
> > We already try to set the flags in advance, but there are some
> > architectural flags like VM_PAT that make that tricky.  Cedric has been
> > looking at inserting individual pages with vmf_insert_pfn(), but that
> > incurs a lot more faults and therefore latency vs remapping the entire
> > vma on fault.  I'm not convinced that we shouldn't just attempt to
> > remove the fault handler entirely, but I haven't tried it yet to know
> > what gotchas are down that path.  Thanks,
> 
> I thought we did it like this because there were races otherwise with
> PTE insertion and zapping? I don't remember well anymore.
> 
> I vaugely remember the address_space conversion might help remove the
> fault handler?
>
What about calling vmf_insert_pfn() in bulk as below?
And what is address_space conversion?


diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index a5ab416cf476..1476e537f593 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -1686,6 +1686,7 @@ static vm_fault_t vfio_pci_mmap_fault(struct vm_fault *vmf)
        struct vfio_pci_core_device *vdev = vma->vm_private_data;
        struct vfio_pci_mmap_vma *mmap_vma;
        vm_fault_t ret = VM_FAULT_NOPAGE;
+       unsigned long base_pfn, offset, i;

        mutex_lock(&vdev->vma_lock);
        down_read(&vdev->memory_lock);
@@ -1710,12 +1711,15 @@ static vm_fault_t vfio_pci_mmap_fault(struct vm_fault *vmf)
                        goto up_out;
        }

-       if (io_remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
-                              vma->vm_end - vma->vm_start,
-                              vma->vm_page_prot)) {
-               ret = VM_FAULT_SIGBUS;
-               zap_vma_ptes(vma, vma->vm_start, vma->vm_end - vma->vm_start);
-               goto up_out;
+       base_pfn = (vmf->address - vma->vm_start) >> PAGE_SHIFT;
+       base_pfn += vma->vm_pgoff;
+       for (i = vma->vm_start; i < vma->vm_end; i += PAGE_SIZE) {
+               offset = (i - vma->vm_start) >> PAGE_SHIFT;
+               ret = vmf_insert_pfn(vma, i, base_pfn + offset);
+               if (ret != VM_FAULT_NOPAGE) {
+                       zap_vma_ptes(vma, vma->vm_start, vma->vm_end - vma->vm_start);
+                       goto up_out;
+               }
        }

        if (__vfio_pci_add_vma(vdev, vma)) {