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=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham 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 01B15C433B4 for ; Sat, 8 May 2021 07:36:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C36C661448 for ; Sat, 8 May 2021 07:36:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230004AbhEHHhN (ORCPT ); Sat, 8 May 2021 03:37:13 -0400 Received: from szxga07-in.huawei.com ([45.249.212.35]:18795 "EHLO szxga07-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229583AbhEHHhL (ORCPT ); Sat, 8 May 2021 03:37:11 -0400 Received: from DGGEMS412-HUB.china.huawei.com (unknown [172.30.72.58]) by szxga07-in.huawei.com (SkyGuard) with ESMTP id 4FcfF53gzYzBt7y; Sat, 8 May 2021 15:33:29 +0800 (CST) Received: from [10.174.187.224] (10.174.187.224) by DGGEMS412-HUB.china.huawei.com (10.3.19.212) with Microsoft SMTP Server id 14.3.498.0; Sat, 8 May 2021 15:35:58 +0800 Subject: Re: [RFC PATCH v4 01/13] iommu: Introduce dirty log tracking framework To: Lu Baolu , , , , Robin Murphy , Will Deacon , "Joerg Roedel" , Jean-Philippe Brucker , Yi Sun , Tian Kevin References: <20210507102211.8836-1-zhukeqian1@huawei.com> <20210507102211.8836-2-zhukeqian1@huawei.com> CC: Alex Williamson , Kirti Wankhede , Cornelia Huck , Jonathan Cameron , , , , From: Keqian Zhu Message-ID: <18ac787a-179e-71f7-728b-c43feda80a16@huawei.com> Date: Sat, 8 May 2021 15:35:57 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.174.187.224] X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Baolu, On 2021/5/8 11:46, Lu Baolu wrote: > Hi Keqian, > > On 5/7/21 6:21 PM, Keqian Zhu wrote: >> Some types of IOMMU are capable of tracking DMA dirty log, such as >> ARM SMMU with HTTU or Intel IOMMU with SLADE. This introduces the >> dirty log tracking framework in the IOMMU base layer. >> >> Four new essential interfaces are added, and we maintaince the status >> of dirty log tracking in iommu_domain. >> 1. iommu_support_dirty_log: Check whether domain supports dirty log tracking >> 2. iommu_switch_dirty_log: Perform actions to start|stop dirty log tracking >> 3. iommu_sync_dirty_log: Sync dirty log from IOMMU into a dirty bitmap >> 4. iommu_clear_dirty_log: Clear dirty log of IOMMU by a mask bitmap >> >> Note: Don't concurrently call these interfaces with other ops that >> access underlying page table. >> >> Signed-off-by: Keqian Zhu >> Signed-off-by: Kunkun Jiang >> --- >> drivers/iommu/iommu.c | 201 +++++++++++++++++++++++++++++++++++ >> include/linux/iommu.h | 63 +++++++++++ >> include/trace/events/iommu.h | 63 +++++++++++ >> 3 files changed, 327 insertions(+) >> >> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c >> index 808ab70d5df5..0d15620d1e90 100644 >> --- a/drivers/iommu/iommu.c >> +++ b/drivers/iommu/iommu.c >> @@ -1940,6 +1940,7 @@ static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus, >> domain->type = type; >> /* Assume all sizes by default; the driver may override this later */ >> domain->pgsize_bitmap = bus->iommu_ops->pgsize_bitmap; >> + mutex_init(&domain->switch_log_lock); >> return domain; >> } >> @@ -2703,6 +2704,206 @@ int iommu_set_pgtable_quirks(struct iommu_domain *domain, >> } >> EXPORT_SYMBOL_GPL(iommu_set_pgtable_quirks); >> +bool iommu_support_dirty_log(struct iommu_domain *domain) >> +{ >> + const struct iommu_ops *ops = domain->ops; >> + >> + return ops->support_dirty_log && ops->support_dirty_log(domain); >> +} >> +EXPORT_SYMBOL_GPL(iommu_support_dirty_log); > > I suppose this interface is to ask the vendor IOMMU driver to check > whether each device/iommu in the domain supports dirty bit tracking. > But what will happen if new devices with different tracking capability > are added afterward? Yep, this is considered in the vfio part. We will query again after attaching or detaching devices from the domain. When the domain becomes capable, we enable dirty log for it. When it becomes not capable, we disable dirty log for it. > > To make things simple, is it possible to support this tracking only when > all underlying IOMMUs support dirty bit tracking? IIUC, all underlying IOMMUs you refer is of system wide. I think this idea may has two issues. 1) The target domain may just contains part of system IOMMUs. 2) The dirty tracking capability can be related to the capability of devices. For example, we can track dirty log based on IOPF, which needs the capability of devices. That's to say, we can make this framework more common. > > Or, the more crazy idea is that we don't need to check this capability > at all. If dirty bit tracking is not supported by hardware, just mark > all pages dirty? Yeah, I think this idea is nice :). Still one concern is that we may have other dirty tracking methods in the future, if we can't track dirty through iommu, we can still try other methods. If there is no interface to check this capability, we have no chance to try other methods. What do you think? > >> + >> +int iommu_switch_dirty_log(struct iommu_domain *domain, bool enable, >> + unsigned long iova, size_t size, int prot) >> +{ >> + const struct iommu_ops *ops = domain->ops; >> + unsigned long orig_iova = iova; >> + unsigned int min_pagesz; >> + size_t orig_size = size; >> + bool flush = false; >> + int ret = 0; >> + >> + if (unlikely(!ops->switch_dirty_log)) >> + return -ENODEV; >> + >> + min_pagesz = 1 << __ffs(domain->pgsize_bitmap); >> + if (!IS_ALIGNED(iova | size, min_pagesz)) { >> + pr_err("unaligned: iova 0x%lx size 0x%zx min_pagesz 0x%x\n", >> + iova, size, min_pagesz); >> + return -EINVAL; >> + } >> + >> + mutex_lock(&domain->switch_log_lock); >> + if (enable && domain->dirty_log_tracking) { >> + ret = -EBUSY; >> + goto out; >> + } else if (!enable && !domain->dirty_log_tracking) { >> + ret = -EINVAL; >> + goto out; >> + } >> + >> + pr_debug("switch_dirty_log %s for: iova 0x%lx size 0x%zx\n", >> + enable ? "enable" : "disable", iova, size); >> + >> + while (size) { >> + size_t pgsize = iommu_pgsize(domain, iova, size); >> + >> + flush = true; >> + ret = ops->switch_dirty_log(domain, enable, iova, pgsize, prot); > > Per minimal page callback is much expensive. How about using (pagesize, > count), so that all pages with the same page size could be handled in a > single indirect call? I remember I commented this during last review, > but I don't mind doing it again. Thanks for reminding me again :). I'll do that in next version. Thanks, Keqian 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=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_RED, USER_AGENT_SANE_1 autolearn=unavailable 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 64D34C433ED for ; Sat, 8 May 2021 07:36:17 +0000 (UTC) Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (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 DB3AA61458 for ; Sat, 8 May 2021 07:36:16 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DB3AA61458 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=huawei.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=iommu-bounces@lists.linux-foundation.org Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 8AD3440FAD; Sat, 8 May 2021 07:36:16 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id NzqnbtWq7gjO; Sat, 8 May 2021 07:36:15 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp4.osuosl.org (Postfix) with ESMTP id EAF8A40E76; Sat, 8 May 2021 07:36:14 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id CE99DC000E; Sat, 8 May 2021 07:36:14 +0000 (UTC) Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 3915CC0001 for ; Sat, 8 May 2021 07:36:14 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 1EB2A60718 for ; Sat, 8 May 2021 07:36:14 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id BF5G2tzKcWkH for ; Sat, 8 May 2021 07:36:12 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from szxga07-in.huawei.com (szxga07-in.huawei.com [45.249.212.35]) by smtp3.osuosl.org (Postfix) with ESMTPS id 96CCF606E3 for ; Sat, 8 May 2021 07:36:12 +0000 (UTC) Received: from DGGEMS412-HUB.china.huawei.com (unknown [172.30.72.58]) by szxga07-in.huawei.com (SkyGuard) with ESMTP id 4FcfF53gzYzBt7y; Sat, 8 May 2021 15:33:29 +0800 (CST) Received: from [10.174.187.224] (10.174.187.224) by DGGEMS412-HUB.china.huawei.com (10.3.19.212) with Microsoft SMTP Server id 14.3.498.0; Sat, 8 May 2021 15:35:58 +0800 Subject: Re: [RFC PATCH v4 01/13] iommu: Introduce dirty log tracking framework To: Lu Baolu , , , , Robin Murphy , Will Deacon , "Joerg Roedel" , Jean-Philippe Brucker , Yi Sun , Tian Kevin References: <20210507102211.8836-1-zhukeqian1@huawei.com> <20210507102211.8836-2-zhukeqian1@huawei.com> From: Keqian Zhu Message-ID: <18ac787a-179e-71f7-728b-c43feda80a16@huawei.com> Date: Sat, 8 May 2021 15:35:57 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: X-Originating-IP: [10.174.187.224] X-CFilter-Loop: Reflected Cc: jiangkunkun@huawei.com, Cornelia Huck , Kirti Wankhede , lushenming@huawei.com, Alex Williamson , wanghaibin.wang@huawei.com X-BeenThere: iommu@lists.linux-foundation.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Development issues for Linux IOMMU support List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: iommu-bounces@lists.linux-foundation.org Sender: "iommu" Hi Baolu, On 2021/5/8 11:46, Lu Baolu wrote: > Hi Keqian, > > On 5/7/21 6:21 PM, Keqian Zhu wrote: >> Some types of IOMMU are capable of tracking DMA dirty log, such as >> ARM SMMU with HTTU or Intel IOMMU with SLADE. This introduces the >> dirty log tracking framework in the IOMMU base layer. >> >> Four new essential interfaces are added, and we maintaince the status >> of dirty log tracking in iommu_domain. >> 1. iommu_support_dirty_log: Check whether domain supports dirty log tracking >> 2. iommu_switch_dirty_log: Perform actions to start|stop dirty log tracking >> 3. iommu_sync_dirty_log: Sync dirty log from IOMMU into a dirty bitmap >> 4. iommu_clear_dirty_log: Clear dirty log of IOMMU by a mask bitmap >> >> Note: Don't concurrently call these interfaces with other ops that >> access underlying page table. >> >> Signed-off-by: Keqian Zhu >> Signed-off-by: Kunkun Jiang >> --- >> drivers/iommu/iommu.c | 201 +++++++++++++++++++++++++++++++++++ >> include/linux/iommu.h | 63 +++++++++++ >> include/trace/events/iommu.h | 63 +++++++++++ >> 3 files changed, 327 insertions(+) >> >> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c >> index 808ab70d5df5..0d15620d1e90 100644 >> --- a/drivers/iommu/iommu.c >> +++ b/drivers/iommu/iommu.c >> @@ -1940,6 +1940,7 @@ static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus, >> domain->type = type; >> /* Assume all sizes by default; the driver may override this later */ >> domain->pgsize_bitmap = bus->iommu_ops->pgsize_bitmap; >> + mutex_init(&domain->switch_log_lock); >> return domain; >> } >> @@ -2703,6 +2704,206 @@ int iommu_set_pgtable_quirks(struct iommu_domain *domain, >> } >> EXPORT_SYMBOL_GPL(iommu_set_pgtable_quirks); >> +bool iommu_support_dirty_log(struct iommu_domain *domain) >> +{ >> + const struct iommu_ops *ops = domain->ops; >> + >> + return ops->support_dirty_log && ops->support_dirty_log(domain); >> +} >> +EXPORT_SYMBOL_GPL(iommu_support_dirty_log); > > I suppose this interface is to ask the vendor IOMMU driver to check > whether each device/iommu in the domain supports dirty bit tracking. > But what will happen if new devices with different tracking capability > are added afterward? Yep, this is considered in the vfio part. We will query again after attaching or detaching devices from the domain. When the domain becomes capable, we enable dirty log for it. When it becomes not capable, we disable dirty log for it. > > To make things simple, is it possible to support this tracking only when > all underlying IOMMUs support dirty bit tracking? IIUC, all underlying IOMMUs you refer is of system wide. I think this idea may has two issues. 1) The target domain may just contains part of system IOMMUs. 2) The dirty tracking capability can be related to the capability of devices. For example, we can track dirty log based on IOPF, which needs the capability of devices. That's to say, we can make this framework more common. > > Or, the more crazy idea is that we don't need to check this capability > at all. If dirty bit tracking is not supported by hardware, just mark > all pages dirty? Yeah, I think this idea is nice :). Still one concern is that we may have other dirty tracking methods in the future, if we can't track dirty through iommu, we can still try other methods. If there is no interface to check this capability, we have no chance to try other methods. What do you think? > >> + >> +int iommu_switch_dirty_log(struct iommu_domain *domain, bool enable, >> + unsigned long iova, size_t size, int prot) >> +{ >> + const struct iommu_ops *ops = domain->ops; >> + unsigned long orig_iova = iova; >> + unsigned int min_pagesz; >> + size_t orig_size = size; >> + bool flush = false; >> + int ret = 0; >> + >> + if (unlikely(!ops->switch_dirty_log)) >> + return -ENODEV; >> + >> + min_pagesz = 1 << __ffs(domain->pgsize_bitmap); >> + if (!IS_ALIGNED(iova | size, min_pagesz)) { >> + pr_err("unaligned: iova 0x%lx size 0x%zx min_pagesz 0x%x\n", >> + iova, size, min_pagesz); >> + return -EINVAL; >> + } >> + >> + mutex_lock(&domain->switch_log_lock); >> + if (enable && domain->dirty_log_tracking) { >> + ret = -EBUSY; >> + goto out; >> + } else if (!enable && !domain->dirty_log_tracking) { >> + ret = -EINVAL; >> + goto out; >> + } >> + >> + pr_debug("switch_dirty_log %s for: iova 0x%lx size 0x%zx\n", >> + enable ? "enable" : "disable", iova, size); >> + >> + while (size) { >> + size_t pgsize = iommu_pgsize(domain, iova, size); >> + >> + flush = true; >> + ret = ops->switch_dirty_log(domain, enable, iova, pgsize, prot); > > Per minimal page callback is much expensive. How about using (pagesize, > count), so that all pages with the same page size could be handled in a > single indirect call? I remember I commented this during last review, > but I don't mind doing it again. Thanks for reminding me again :). I'll do that in next version. Thanks, Keqian _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu 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=-15.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=unavailable 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 57948C433ED for ; Sat, 8 May 2021 07:39:02 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (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 BEBE761448 for ; Sat, 8 May 2021 07:39:01 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org BEBE761448 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=huawei.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=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From:CC: References:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=QEIXP7D9M1YSSHFIfryDO5K9vzRlGTsveK8G3lJz+jI=; b=CstUd1LhNVeRF22Hs3UmMkmlR nE6XPlrDbZVAmom4hAJYR3Y5HvTtq3ND2quDkrjnBM53InHUZ6Eoqu+0q7yC1aVmxhgKmCIkGS+Qc in0YN+hVe54Ky5IMeurBe2t2Cav9bkCibmlpvGJXhDETZe/xicpD44dfRJqoCuvakypnPkvkNQ4m8 SqmMlCY8E6eYJrilbdwSedT9qWWbsF6M7+Q0mwN4xx19aApm6Y7G4zYY1FWRAg2nFDOgeH7hsfqhm AfweHvYXIv6Go8g5WUVv4L/ZOwAXY7WLeWn8hf4+9d8eg1XYBLoh9vBZgWBf1ssk/Pk1GEKK/0ose 3DZP4LfKA==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lfHVh-0094fw-Lc; Sat, 08 May 2021 07:36:25 +0000 Received: from bombadil.infradead.org ([2607:7c80:54:e::133]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lfHVb-0094fQ-8T for linux-arm-kernel@desiato.infradead.org; Sat, 08 May 2021 07:36:19 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Content-Transfer-Encoding: Content-Type:In-Reply-To:MIME-Version:Date:Message-ID:From:CC:References:To: Subject:Sender:Reply-To:Content-ID:Content-Description; bh=uUQIuKju2Gxd2PgRoiS3tGA6NZwGQ1CLor6CVV/7hm4=; b=bwrBVWMvRH/mgwjKIsYB7yaCLg irgiNF+D0j38Xg1pCDC3YriBPKc3dUHMhb8FBJoXIfnrjBs5Zhbxhxs8SbgrPEv3SGAl7AfxRSzFj lbHEtp4okgONXo/5wjSMZw1aLwq5xVVnKFuozEhKHLQF+TimnfyR3Ea+i5kLRmshb2T9JzzMIb3fI MoKL1qx+39qoXapwEEpGratImfhpxBt8cbdxEntty8EyXhgCakC7hPT3JFUk+JKsKnlbNAvRR1OYo h04TUNzU05QCfQR8TotcbDUH/Waarm7RqKmM3y3D2bZH+ar9ETxl4ebZtM/Bjm+jPiugrJQyHyegt XeoOJiFA==; Received: from szxga07-in.huawei.com ([45.249.212.35]) by bombadil.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lfHVW-007QLG-EY for linux-arm-kernel@lists.infradead.org; Sat, 08 May 2021 07:36:16 +0000 Received: from DGGEMS412-HUB.china.huawei.com (unknown [172.30.72.58]) by szxga07-in.huawei.com (SkyGuard) with ESMTP id 4FcfF53gzYzBt7y; Sat, 8 May 2021 15:33:29 +0800 (CST) Received: from [10.174.187.224] (10.174.187.224) by DGGEMS412-HUB.china.huawei.com (10.3.19.212) with Microsoft SMTP Server id 14.3.498.0; Sat, 8 May 2021 15:35:58 +0800 Subject: Re: [RFC PATCH v4 01/13] iommu: Introduce dirty log tracking framework To: Lu Baolu , , , , Robin Murphy , Will Deacon , "Joerg Roedel" , Jean-Philippe Brucker , Yi Sun , Tian Kevin References: <20210507102211.8836-1-zhukeqian1@huawei.com> <20210507102211.8836-2-zhukeqian1@huawei.com> CC: Alex Williamson , Kirti Wankhede , Cornelia Huck , Jonathan Cameron , , , , From: Keqian Zhu Message-ID: <18ac787a-179e-71f7-728b-c43feda80a16@huawei.com> Date: Sat, 8 May 2021 15:35:57 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: X-Originating-IP: [10.174.187.224] X-CFilter-Loop: Reflected X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210508_003614_869860_4B138B21 X-CRM114-Status: GOOD ( 28.37 ) 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 Hi Baolu, On 2021/5/8 11:46, Lu Baolu wrote: > Hi Keqian, > > On 5/7/21 6:21 PM, Keqian Zhu wrote: >> Some types of IOMMU are capable of tracking DMA dirty log, such as >> ARM SMMU with HTTU or Intel IOMMU with SLADE. This introduces the >> dirty log tracking framework in the IOMMU base layer. >> >> Four new essential interfaces are added, and we maintaince the status >> of dirty log tracking in iommu_domain. >> 1. iommu_support_dirty_log: Check whether domain supports dirty log tracking >> 2. iommu_switch_dirty_log: Perform actions to start|stop dirty log tracking >> 3. iommu_sync_dirty_log: Sync dirty log from IOMMU into a dirty bitmap >> 4. iommu_clear_dirty_log: Clear dirty log of IOMMU by a mask bitmap >> >> Note: Don't concurrently call these interfaces with other ops that >> access underlying page table. >> >> Signed-off-by: Keqian Zhu >> Signed-off-by: Kunkun Jiang >> --- >> drivers/iommu/iommu.c | 201 +++++++++++++++++++++++++++++++++++ >> include/linux/iommu.h | 63 +++++++++++ >> include/trace/events/iommu.h | 63 +++++++++++ >> 3 files changed, 327 insertions(+) >> >> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c >> index 808ab70d5df5..0d15620d1e90 100644 >> --- a/drivers/iommu/iommu.c >> +++ b/drivers/iommu/iommu.c >> @@ -1940,6 +1940,7 @@ static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus, >> domain->type = type; >> /* Assume all sizes by default; the driver may override this later */ >> domain->pgsize_bitmap = bus->iommu_ops->pgsize_bitmap; >> + mutex_init(&domain->switch_log_lock); >> return domain; >> } >> @@ -2703,6 +2704,206 @@ int iommu_set_pgtable_quirks(struct iommu_domain *domain, >> } >> EXPORT_SYMBOL_GPL(iommu_set_pgtable_quirks); >> +bool iommu_support_dirty_log(struct iommu_domain *domain) >> +{ >> + const struct iommu_ops *ops = domain->ops; >> + >> + return ops->support_dirty_log && ops->support_dirty_log(domain); >> +} >> +EXPORT_SYMBOL_GPL(iommu_support_dirty_log); > > I suppose this interface is to ask the vendor IOMMU driver to check > whether each device/iommu in the domain supports dirty bit tracking. > But what will happen if new devices with different tracking capability > are added afterward? Yep, this is considered in the vfio part. We will query again after attaching or detaching devices from the domain. When the domain becomes capable, we enable dirty log for it. When it becomes not capable, we disable dirty log for it. > > To make things simple, is it possible to support this tracking only when > all underlying IOMMUs support dirty bit tracking? IIUC, all underlying IOMMUs you refer is of system wide. I think this idea may has two issues. 1) The target domain may just contains part of system IOMMUs. 2) The dirty tracking capability can be related to the capability of devices. For example, we can track dirty log based on IOPF, which needs the capability of devices. That's to say, we can make this framework more common. > > Or, the more crazy idea is that we don't need to check this capability > at all. If dirty bit tracking is not supported by hardware, just mark > all pages dirty? Yeah, I think this idea is nice :). Still one concern is that we may have other dirty tracking methods in the future, if we can't track dirty through iommu, we can still try other methods. If there is no interface to check this capability, we have no chance to try other methods. What do you think? > >> + >> +int iommu_switch_dirty_log(struct iommu_domain *domain, bool enable, >> + unsigned long iova, size_t size, int prot) >> +{ >> + const struct iommu_ops *ops = domain->ops; >> + unsigned long orig_iova = iova; >> + unsigned int min_pagesz; >> + size_t orig_size = size; >> + bool flush = false; >> + int ret = 0; >> + >> + if (unlikely(!ops->switch_dirty_log)) >> + return -ENODEV; >> + >> + min_pagesz = 1 << __ffs(domain->pgsize_bitmap); >> + if (!IS_ALIGNED(iova | size, min_pagesz)) { >> + pr_err("unaligned: iova 0x%lx size 0x%zx min_pagesz 0x%x\n", >> + iova, size, min_pagesz); >> + return -EINVAL; >> + } >> + >> + mutex_lock(&domain->switch_log_lock); >> + if (enable && domain->dirty_log_tracking) { >> + ret = -EBUSY; >> + goto out; >> + } else if (!enable && !domain->dirty_log_tracking) { >> + ret = -EINVAL; >> + goto out; >> + } >> + >> + pr_debug("switch_dirty_log %s for: iova 0x%lx size 0x%zx\n", >> + enable ? "enable" : "disable", iova, size); >> + >> + while (size) { >> + size_t pgsize = iommu_pgsize(domain, iova, size); >> + >> + flush = true; >> + ret = ops->switch_dirty_log(domain, enable, iova, pgsize, prot); > > Per minimal page callback is much expensive. How about using (pagesize, > count), so that all pages with the same page size could be handled in a > single indirect call? I remember I commented this during last review, > but I don't mind doing it again. Thanks for reminding me again :). I'll do that in next version. Thanks, Keqian _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel