Message ID | 20230921075138.124099-4-yi.l.liu@intel.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:172:b0:3f2:4152:657d with SMTP id h50csp5443192vqi; Fri, 22 Sep 2023 02:56:29 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEm9m0A1McpEyTy27+omMZkmhgPs/yfsj+z+SLq/3EnvoN2Tcp6RvTXtR2aDg/563VmT+LP X-Received: by 2002:a17:902:9343:b0:1c3:f4fa:b1a2 with SMTP id g3-20020a170902934300b001c3f4fab1a2mr7521332plp.8.1695376589212; Fri, 22 Sep 2023 02:56:29 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1695376589; cv=none; d=google.com; s=arc-20160816; b=Bzndj19+URyQQV50qbW0fvkTRby1A3nTRCWVXRWcwlB/ztp1X+5UwkVEQk3Ip+oBw9 gpPNFnTe7pqy2uiBV8q2wcQvHpRcd0KN3U1BIvjJzIeZJndAX51xvVGkphgwyPWUOCY/ a4sBf5Dll/s/5vz+iwMFmsAKzQIa/W6/xdes7z7Ruc8cKaEGomheVQXERyZg/XY+QjKk TvXOTfBRBc1l03zCFh3f+ewlGkLIdZO73h/78TrUTqTGXFyvVfCX1HGwfqUVceeKT4Uo f0rfhxIvxQUvAkTZko7UNfeCUR0VL8blAJfY7tr0tqhXSfUUyiRDrL4oaiqT7utZ3H38 uKYw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=T6eXOYU3AG+rzCi81yzfSwl4eylb6quscLRsdbDixUY=; fh=ncJBVmsnOSqrX1O37yfYEzicwaA2e7ARxnsU7aiysyE=; b=e0geqX+L4O89GVvbtfda1CqkrWwQ+ZBKycXWL860gfBeak5htUk6bt9PCJZRmcsy9q sWCotFQmC3SeUT7f7ryqR9N6Tq2pG2K/vX8K0cvGom0jqs/pqo3I6OEN1lCxTDXc3Kvb 5ZnecwO+a6av0fEkQ+6c5LpwcsZdGN3bjmPmuT6y0/MWSq+6OP6Lpb5kbTAQPCBMPc64 PTwkJcpzfzyVAqQdfzmHx0ldCivcnuFeOj0W7UnyXhlANjjnYxB8VD+kZc8ORAZGsSVj 5Wrjr4c2YisJ+yit1dGwmwJbz+iU5lcfbKAlfVeuDf3dToRFNcG9mQ7SAHkGbqWQfho9 rfLw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=mYLObWUc; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from snail.vger.email (snail.vger.email. [2620:137:e000::3:7]) by mx.google.com with ESMTPS id c12-20020a170902d48c00b001c3991e57dasi3791855plg.396.2023.09.22.02.56.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 22 Sep 2023 02:56:29 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=mYLObWUc; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id C98928311BE8; Thu, 21 Sep 2023 13:56:45 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230100AbjIUU4p (ORCPT <rfc822;pwkd43@gmail.com> + 29 others); Thu, 21 Sep 2023 16:56:45 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48442 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229670AbjIUUtM (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 21 Sep 2023 16:49:12 -0400 Received: from mgamail.intel.com (mgamail.intel.com [192.55.52.136]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EAA2D4CB00; Thu, 21 Sep 2023 10:49:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1695318588; x=1726854588; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=fZEXWeTeOBrejODuS6erDuW9xhi+ilN63E3t7GSB+Dc=; b=mYLObWUcd9WTK/8as8BLRXq34hkELQ+jqRWpxcYabdwgQj1hmo3YyPjU K6XUxEbrRgGBFkE1odl1b4Yk0/Wdyd1QWoT0Qop9VQQ/qHEQAJXaL5mph E2xcdYJi5zLclO7jDAskar481WVdWzYHT2AuoI3OhwCRdL2DLNaR9fq6K pG0rjNNOfMGhA0yAthttcVSto8Fc3oKCRajdWVOnDmIEHv9klpnTW+PSr LdxGsuuJnxrKP1AL9XlL4hDVseR70QrauEMQyAa7R0Rp+B5/BTsENVMME kRqUjQ/ESVtEeDzghup16Tvvx2LufPHsE+dd1A4aIJ0tba/KcqBIhcTY3 Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10839"; a="359832824" X-IronPort-AV: E=Sophos;i="6.03,164,1694761200"; d="scan'208";a="359832824" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Sep 2023 00:52:09 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10839"; a="723649502" X-IronPort-AV: E=Sophos;i="6.03,164,1694761200"; d="scan'208";a="723649502" Received: from 984fee00a4c6.jf.intel.com ([10.165.58.231]) by orsmga006.jf.intel.com with ESMTP; 21 Sep 2023 00:52:09 -0700 From: Yi Liu <yi.l.liu@intel.com> To: joro@8bytes.org, alex.williamson@redhat.com, jgg@nvidia.com, kevin.tian@intel.com, robin.murphy@arm.com, baolu.lu@linux.intel.com Cc: cohuck@redhat.com, eric.auger@redhat.com, nicolinc@nvidia.com, kvm@vger.kernel.org, mjrosato@linux.ibm.com, chao.p.peng@linux.intel.com, yi.l.liu@intel.com, yi.y.sun@linux.intel.com, peterx@redhat.com, jasowang@redhat.com, shameerali.kolothum.thodi@huawei.com, lulu@redhat.com, suravee.suthikulpanit@amd.com, iommu@lists.linux.dev, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, zhenzhong.duan@intel.com, joao.m.martins@oracle.com Subject: [PATCH v4 03/17] iommufd: Unite all kernel-managed members into a struct Date: Thu, 21 Sep 2023 00:51:24 -0700 Message-Id: <20230921075138.124099-4-yi.l.liu@intel.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230921075138.124099-1-yi.l.liu@intel.com> References: <20230921075138.124099-1-yi.l.liu@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF, RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_NONE,URIBL_BLOCKED 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 X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Thu, 21 Sep 2023 13:56:45 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1777731202013640297 X-GMAIL-MSGID: 1777731202013640297 |
Series |
iommufd: Add nesting infrastructure
|
|
Commit Message
Yi Liu
Sept. 21, 2023, 7:51 a.m. UTC
From: Nicolin Chen <nicolinc@nvidia.com> The struct iommufd_hw_pagetable has been representing a kernel-managed HWPT, yet soon will be reused to represent a user-managed HWPT. These two types of HWPTs has the same IOMMUFD object type and an iommu_domain object, but have quite different attributes/members. Add a union in struct iommufd_hw_pagetable and group all the existing kernel-managed members. One of the following patches will add another struct for user-managed members. Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> Signed-off-by: Yi Liu <yi.l.liu@intel.com> --- drivers/iommu/iommufd/iommufd_private.h | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)
Comments
> From: Liu, Yi L <yi.l.liu@intel.com> > Sent: Thursday, September 21, 2023 3:51 PM > > From: Nicolin Chen <nicolinc@nvidia.com> > > The struct iommufd_hw_pagetable has been representing a kernel-managed > HWPT, yet soon will be reused to represent a user-managed HWPT. These > two types of HWPTs has the same IOMMUFD object type and an > iommu_domain > object, but have quite different attributes/members. > > Add a union in struct iommufd_hw_pagetable and group all the existing > kernel-managed members. One of the following patches will add another > struct for user-managed members. > > Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> > Signed-off-by: Yi Liu <yi.l.liu@intel.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
On Thu, Sep 21, 2023 at 12:51:24AM -0700, Yi Liu wrote: > From: Nicolin Chen <nicolinc@nvidia.com> > > The struct iommufd_hw_pagetable has been representing a kernel-managed > HWPT, yet soon will be reused to represent a user-managed HWPT. These > two types of HWPTs has the same IOMMUFD object type and an iommu_domain > object, but have quite different attributes/members. > > Add a union in struct iommufd_hw_pagetable and group all the existing > kernel-managed members. One of the following patches will add another > struct for user-managed members. > > Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> > Signed-off-by: Yi Liu <yi.l.liu@intel.com> > --- > drivers/iommu/iommufd/iommufd_private.h | 17 +++++++++++------ > 1 file changed, 11 insertions(+), 6 deletions(-) > > diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h > index 3064997a0181..947a797536e3 100644 > --- a/drivers/iommu/iommufd/iommufd_private.h > +++ b/drivers/iommu/iommufd/iommufd_private.h > @@ -231,13 +231,18 @@ int iommufd_vfio_ioas(struct iommufd_ucmd *ucmd); > */ > struct iommufd_hw_pagetable { > struct iommufd_object obj; > - struct iommufd_ioas *ioas; > struct iommu_domain *domain; > - bool auto_domain : 1; > - bool enforce_cache_coherency : 1; > - bool msi_cookie : 1; > - /* Head at iommufd_ioas::hwpt_list */ > - struct list_head hwpt_item; > + > + union { > + struct { /* kernel-managed */ > + struct iommufd_ioas *ioas; > + bool auto_domain : 1; Will iommufd_hw_pagetable_put() also be called on non-kernel-managed domain? If yes, hwpt->user_managed needs to be checked in iommufd_hw_pagetable_put(), (e.g. as below). Otherwise, this union will lead to hwpt->ioas and hwpt->auto_domain still being accessible though invalid. static inline void iommufd_hw_pagetable_put(struct iommufd_ctx *ictx, struct iommufd_hw_pagetable *hwpt) { - lockdep_assert_not_held(&hwpt->ioas->mutex); - if (hwpt->auto_domain) + if (!hwpt->user_managed) + lockdep_assert_not_held(&hwpt->ioas->mutex); + + if (!hwpt->user_managed && hwpt->auto_domain) iommufd_object_deref_user(ictx, &hwpt->obj); else refcount_dec(&hwpt->obj.users); } > + bool enforce_cache_coherency : 1; > + bool msi_cookie : 1; > + /* Head at iommufd_ioas::hwpt_list */ > + struct list_head hwpt_item; > + }; > + }; > }; > > struct iommufd_hw_pagetable * > -- > 2.34.1 >
On 2023/10/7 18:08, Yan Zhao wrote: > On Thu, Sep 21, 2023 at 12:51:24AM -0700, Yi Liu wrote: >> From: Nicolin Chen <nicolinc@nvidia.com> >> >> The struct iommufd_hw_pagetable has been representing a kernel-managed >> HWPT, yet soon will be reused to represent a user-managed HWPT. These >> two types of HWPTs has the same IOMMUFD object type and an iommu_domain >> object, but have quite different attributes/members. >> >> Add a union in struct iommufd_hw_pagetable and group all the existing >> kernel-managed members. One of the following patches will add another >> struct for user-managed members. >> >> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> >> Signed-off-by: Yi Liu <yi.l.liu@intel.com> >> --- >> drivers/iommu/iommufd/iommufd_private.h | 17 +++++++++++------ >> 1 file changed, 11 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h >> index 3064997a0181..947a797536e3 100644 >> --- a/drivers/iommu/iommufd/iommufd_private.h >> +++ b/drivers/iommu/iommufd/iommufd_private.h >> @@ -231,13 +231,18 @@ int iommufd_vfio_ioas(struct iommufd_ucmd *ucmd); >> */ >> struct iommufd_hw_pagetable { >> struct iommufd_object obj; >> - struct iommufd_ioas *ioas; >> struct iommu_domain *domain; >> - bool auto_domain : 1; >> - bool enforce_cache_coherency : 1; >> - bool msi_cookie : 1; >> - /* Head at iommufd_ioas::hwpt_list */ >> - struct list_head hwpt_item; >> + >> + union { >> + struct { /* kernel-managed */ >> + struct iommufd_ioas *ioas; >> + bool auto_domain : 1; > Will iommufd_hw_pagetable_put() also be called on non-kernel-managed domain? yes. > If yes, hwpt->user_managed needs to be checked in iommufd_hw_pagetable_put(), > (e.g. as below). > Otherwise, this union will lead to hwpt->ioas and hwpt->auto_domain still being > accessible though invalid. not quite get this sentence. > > static inline void iommufd_hw_pagetable_put(struct iommufd_ctx *ictx, > struct iommufd_hw_pagetable *hwpt) > { > - lockdep_assert_not_held(&hwpt->ioas->mutex); > - if (hwpt->auto_domain) > + if (!hwpt->user_managed) > + lockdep_assert_not_held(&hwpt->ioas->mutex); this is true. this assert is not needed when hwpt is not kernel managed domain. > + > + if (!hwpt->user_managed && hwpt->auto_domain) actually, checking auto_domain is more precise. There is hwpt which is neither user managed nor auto. > iommufd_object_deref_user(ictx, &hwpt->obj); > else > refcount_dec(&hwpt->obj.users); > } > >> + bool enforce_cache_coherency : 1; >> + bool msi_cookie : 1; >> + /* Head at iommufd_ioas::hwpt_list */ >> + struct list_head hwpt_item; >> + }; >> + }; >> }; >> >> struct iommufd_hw_pagetable * >> -- >> 2.34.1 >>
On Mon, Oct 09, 2023 at 12:13:52PM +0800, Yi Liu wrote: > On 2023/10/7 18:08, Yan Zhao wrote: > > On Thu, Sep 21, 2023 at 12:51:24AM -0700, Yi Liu wrote: > > > From: Nicolin Chen <nicolinc@nvidia.com> > > > > > > The struct iommufd_hw_pagetable has been representing a kernel-managed > > > HWPT, yet soon will be reused to represent a user-managed HWPT. These > > > two types of HWPTs has the same IOMMUFD object type and an iommu_domain > > > object, but have quite different attributes/members. > > > > > > Add a union in struct iommufd_hw_pagetable and group all the existing > > > kernel-managed members. One of the following patches will add another > > > struct for user-managed members. > > > > > > Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> > > > Signed-off-by: Yi Liu <yi.l.liu@intel.com> > > > --- > > > drivers/iommu/iommufd/iommufd_private.h | 17 +++++++++++------ > > > 1 file changed, 11 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h > > > index 3064997a0181..947a797536e3 100644 > > > --- a/drivers/iommu/iommufd/iommufd_private.h > > > +++ b/drivers/iommu/iommufd/iommufd_private.h > > > @@ -231,13 +231,18 @@ int iommufd_vfio_ioas(struct iommufd_ucmd *ucmd); > > > */ > > > struct iommufd_hw_pagetable { > > > struct iommufd_object obj; > > > - struct iommufd_ioas *ioas; > > > struct iommu_domain *domain; > > > - bool auto_domain : 1; > > > - bool enforce_cache_coherency : 1; > > > - bool msi_cookie : 1; > > > - /* Head at iommufd_ioas::hwpt_list */ > > > - struct list_head hwpt_item; > > > + > > > + union { > > > + struct { /* kernel-managed */ > > > + struct iommufd_ioas *ioas; > > > + bool auto_domain : 1; > > Will iommufd_hw_pagetable_put() also be called on non-kernel-managed domain? > > yes. > > > If yes, hwpt->user_managed needs to be checked in iommufd_hw_pagetable_put(), > > (e.g. as below). > > Otherwise, this union will lead to hwpt->ioas and hwpt->auto_domain still being > > accessible though invalid. > > not quite get this sentence. I mean with this union, hwpt->auto_domain or hwpt->ioas should only be accessed if and only if hwpt type is kernel-managed. So, any unconditional access, as in iommufd_hw_pagetable_put() pasted below, is buggy. Therefore, do you think it's better to add a filed like "bool kernel_managed : 1", and access the union fields under /* kernel-managed */ only when hwpt->kernel_managed is true. > > > > > static inline void iommufd_hw_pagetable_put(struct iommufd_ctx *ictx, > > struct iommufd_hw_pagetable *hwpt) > > { > > - lockdep_assert_not_held(&hwpt->ioas->mutex); > > - if (hwpt->auto_domain) > > + if (!hwpt->user_managed) > > + lockdep_assert_not_held(&hwpt->ioas->mutex); > > this is true. this assert is not needed when hwpt is not kernel managed domain. > > > + > > + if (!hwpt->user_managed && hwpt->auto_domain) > > actually, checking auto_domain is more precise. There is hwpt which is > neither user managed nor auto. auto_domain is under union fields marked with kernel-managed only. Access it without type checking is invalid. struct iommufd_hw_pagetable { struct iommufd_object obj; struct iommu_domain *domain; void (*abort)(struct iommufd_object *obj); void (*destroy)(struct iommufd_object *obj); bool user_managed : 1; union { struct { /* user-managed */ struct iommufd_hw_pagetable *parent; }; struct { /* kernel-managed */ struct iommufd_ioas *ioas; struct mutex mutex; bool auto_domain : 1; bool enforce_cache_coherency : 1; bool msi_cookie : 1; bool nest_parent : 1; /* Head at iommufd_ioas::hwpt_list */ struct list_head hwpt_item; }; }; }; > > > iommufd_object_deref_user(ictx, &hwpt->obj); > > else > > refcount_dec(&hwpt->obj.users); > > } > > > > > + bool enforce_cache_coherency : 1; > > > + bool msi_cookie : 1; > > > + /* Head at iommufd_ioas::hwpt_list */ > > > + struct list_head hwpt_item; > > > + }; > > > + }; > > > }; > > > struct iommufd_hw_pagetable * > > > -- > > > 2.34.1 > > > > > -- > Regards, > Yi Liu
On 2023/10/9 13:13, Yan Zhao wrote: > On Mon, Oct 09, 2023 at 12:13:52PM +0800, Yi Liu wrote: >> On 2023/10/7 18:08, Yan Zhao wrote: >>> On Thu, Sep 21, 2023 at 12:51:24AM -0700, Yi Liu wrote: >>>> From: Nicolin Chen <nicolinc@nvidia.com> >>>> >>>> The struct iommufd_hw_pagetable has been representing a kernel-managed >>>> HWPT, yet soon will be reused to represent a user-managed HWPT. These >>>> two types of HWPTs has the same IOMMUFD object type and an iommu_domain >>>> object, but have quite different attributes/members. >>>> >>>> Add a union in struct iommufd_hw_pagetable and group all the existing >>>> kernel-managed members. One of the following patches will add another >>>> struct for user-managed members. >>>> >>>> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> >>>> Signed-off-by: Yi Liu <yi.l.liu@intel.com> >>>> --- >>>> drivers/iommu/iommufd/iommufd_private.h | 17 +++++++++++------ >>>> 1 file changed, 11 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h >>>> index 3064997a0181..947a797536e3 100644 >>>> --- a/drivers/iommu/iommufd/iommufd_private.h >>>> +++ b/drivers/iommu/iommufd/iommufd_private.h >>>> @@ -231,13 +231,18 @@ int iommufd_vfio_ioas(struct iommufd_ucmd *ucmd); >>>> */ >>>> struct iommufd_hw_pagetable { >>>> struct iommufd_object obj; >>>> - struct iommufd_ioas *ioas; >>>> struct iommu_domain *domain; >>>> - bool auto_domain : 1; >>>> - bool enforce_cache_coherency : 1; >>>> - bool msi_cookie : 1; >>>> - /* Head at iommufd_ioas::hwpt_list */ >>>> - struct list_head hwpt_item; >>>> + >>>> + union { >>>> + struct { /* kernel-managed */ >>>> + struct iommufd_ioas *ioas; >>>> + bool auto_domain : 1; >>> Will iommufd_hw_pagetable_put() also be called on non-kernel-managed domain? >> >> yes. >> >>> If yes, hwpt->user_managed needs to be checked in iommufd_hw_pagetable_put(), >>> (e.g. as below). >>> Otherwise, this union will lead to hwpt->ioas and hwpt->auto_domain still being >>> accessible though invalid. >> >> not quite get this sentence. > I mean with this union, hwpt->auto_domain or hwpt->ioas should only be accessed if and > only if hwpt type is kernel-managed. ok, I get this part. just not sure about the missing words in your prior comment. > So, any unconditional access, as in iommufd_hw_pagetable_put() pasted below, is buggy. > > Therefore, do you think it's better to add a filed like "bool kernel_managed : 1", > and access the union fields under /* kernel-managed */ only when hwpt->kernel_managed > is true. > > >> >>> >>> static inline void iommufd_hw_pagetable_put(struct iommufd_ctx *ictx, >>> struct iommufd_hw_pagetable *hwpt) >>> { >>> - lockdep_assert_not_held(&hwpt->ioas->mutex); >>> - if (hwpt->auto_domain) >>> + if (!hwpt->user_managed) >>> + lockdep_assert_not_held(&hwpt->ioas->mutex); >> >> this is true. this assert is not needed when hwpt is not kernel managed domain. >> >>> + >>> + if (!hwpt->user_managed && hwpt->auto_domain) >> >> actually, checking auto_domain is more precise. There is hwpt which is >> neither user managed nor auto. > > auto_domain is under union fields marked with kernel-managed only. > Access it without type checking is invalid. I see. yes, should check user_managed as well. :) > struct iommufd_hw_pagetable { > struct iommufd_object obj; > struct iommu_domain *domain; > > void (*abort)(struct iommufd_object *obj); > void (*destroy)(struct iommufd_object *obj); > > bool user_managed : 1; > union { > struct { /* user-managed */ > struct iommufd_hw_pagetable *parent; > }; > struct { /* kernel-managed */ > struct iommufd_ioas *ioas; > struct mutex mutex; > bool auto_domain : 1; > bool enforce_cache_coherency : 1; > bool msi_cookie : 1; > bool nest_parent : 1; > /* Head at iommufd_ioas::hwpt_list */ > struct list_head hwpt_item; > }; > }; > }; > >> >>> iommufd_object_deref_user(ictx, &hwpt->obj); >>> else >>> refcount_dec(&hwpt->obj.users); >>> } >>> >>>> + bool enforce_cache_coherency : 1; >>>> + bool msi_cookie : 1; >>>> + /* Head at iommufd_ioas::hwpt_list */ >>>> + struct list_head hwpt_item; >>>> + }; >>>> + }; >>>> }; >>>> struct iommufd_hw_pagetable * >>>> -- >>>> 2.34.1 >>>> >> >> -- >> Regards, >> Yi Liu
diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h index 3064997a0181..947a797536e3 100644 --- a/drivers/iommu/iommufd/iommufd_private.h +++ b/drivers/iommu/iommufd/iommufd_private.h @@ -231,13 +231,18 @@ int iommufd_vfio_ioas(struct iommufd_ucmd *ucmd); */ struct iommufd_hw_pagetable { struct iommufd_object obj; - struct iommufd_ioas *ioas; struct iommu_domain *domain; - bool auto_domain : 1; - bool enforce_cache_coherency : 1; - bool msi_cookie : 1; - /* Head at iommufd_ioas::hwpt_list */ - struct list_head hwpt_item; + + union { + struct { /* kernel-managed */ + struct iommufd_ioas *ioas; + bool auto_domain : 1; + bool enforce_cache_coherency : 1; + bool msi_cookie : 1; + /* Head at iommufd_ioas::hwpt_list */ + struct list_head hwpt_item; + }; + }; }; struct iommufd_hw_pagetable *