From: "Liu, Yi L" <yi.l.liu@intel.com>
To: "Tian, Kevin" <kevin.tian@intel.com>,
"joro@8bytes.org" <joro@8bytes.org>,
"alex.williamson@redhat.com" <alex.williamson@redhat.com>,
"jgg@nvidia.com" <jgg@nvidia.com>,
"robin.murphy@arm.com" <robin.murphy@arm.com>,
"baolu.lu@linux.intel.com" <baolu.lu@linux.intel.com>
Cc: "cohuck@redhat.com" <cohuck@redhat.com>,
"eric.auger@redhat.com" <eric.auger@redhat.com>,
"nicolinc@nvidia.com" <nicolinc@nvidia.com>,
"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
"mjrosato@linux.ibm.com" <mjrosato@linux.ibm.com>,
"chao.p.peng@linux.intel.com" <chao.p.peng@linux.intel.com>,
"yi.y.sun@linux.intel.com" <yi.y.sun@linux.intel.com>,
"peterx@redhat.com" <peterx@redhat.com>,
"jasowang@redhat.com" <jasowang@redhat.com>,
"shameerali.kolothum.thodi@huawei.com"
<shameerali.kolothum.thodi@huawei.com>,
"lulu@redhat.com" <lulu@redhat.com>,
"suravee.suthikulpanit@amd.com" <suravee.suthikulpanit@amd.com>,
"iommu@lists.linux.dev" <iommu@lists.linux.dev>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-kselftest@vger.kernel.org"
<linux-kselftest@vger.kernel.org>
Subject: RE: [PATCH v2 2/4] iommu: Add new iommu op to get iommu hardware information
Date: Wed, 29 Mar 2023 09:46:50 +0000 [thread overview]
Message-ID: <DS0PR11MB75297955136ECD62A825287CC3899@DS0PR11MB7529.namprd11.prod.outlook.com> (raw)
In-Reply-To: <BN9PR11MB5276C6A0F26954A42B8C23498CBC9@BN9PR11MB5276.namprd11.prod.outlook.com>
> From: Tian, Kevin <kevin.tian@intel.com>
> Sent: Thursday, March 16, 2023 4:17 PM
>
> > From: Liu, Yi L <yi.l.liu@intel.com>
> > Sent: Thursday, March 9, 2023 3:54 PM
> > @@ -222,6 +223,11 @@ struct iommu_iotlb_gather {
> > /**
> > * struct iommu_ops - iommu ops and capabilities
> > * @capable: check capability
> > + * @hw_info: IOMMU hardware information. The type of the returned
> data
> > is
> > + * defined in include/uapi/linux/iommufd.h. The data buffer is
>
> "The type of the returned data is marked by @driver_type".
>
> "defined in include/uapi/linux/iommufd.h" should belong to the comment
> of @driver_type
Sure.
>
> > + * allocated in the IOMMU driver and the caller should free it
> > + * after use. Return the data buffer if success, or ERR_PTR on
> > + * failure.
> > * @domain_alloc: allocate iommu domain
> > * @probe_device: Add device to iommu driver handling
> > * @release_device: Remove device from iommu driver handling
> > @@ -246,11 +252,17 @@ struct iommu_iotlb_gather {
> > * @remove_dev_pasid: Remove any translation configurations of a
> specific
> > * pasid, so that any DMA transactions with this pasid
> > * will be blocked by the hardware.
> > + * @driver_type: One of enum iommu_hw_info_type. This is used in the
> > hw_info
> > + * reporting path. For the drivers that supports it, a unique
> > + * type should be defined. For the driver that does not support
> > + * it, this field is the IOMMU_HW_INFO_TYPE_DEFAULT that is 0.
> > + * Hence, such drivers do not need to care this field.
>
> The meaning of "driver_type" is much broader than reporting hw_info.
>
> let's be accurate to call it as "hw_info_type". and while we have two
> separate fields for one feature where is the check enforced on whether
> both are provided?
It is filled in the uapi structure by referring ops->driver_type in next
patch.
> Is it simpler to return the type directly in @hw_info?
Per the current description, if the iommu driver doesn't implement .hw_info
callback, then it will not set driver_type field neither. Then this field is 0
(IOMMU_HW_INFO_TYPE_NONE). The GET_HW_INFO ioctl in next patch
would fail as well. Under this implementation, returning the driver_type
(a.k.a hw_info_type per your comment) in the hw_info callback may be
simpler.
But I plan to update the implementation per the below remark from Jason.
The GET_HW_INFO needs to succeed even if the underlying iommu driver
does not implement hw_info callback. If so, it's still much more convenient
to get the type by referring ops->driver_type.
https://lore.kernel.org/kvm/ZAcwJSK%2F9UVI9LXu@nvidia.com/
Also, per Nic's other remark, there would be a bitmap named hwpt_types
field added to iommu_ops. Then it is also easier to referring it by
ops->hwpt_types.
https://lore.kernel.org/linux-iommu/ZArgAXMUpNjDfFgZ@Asurada-Nvidia/#t
Surely, we also have another alternative. We can enforce all the iommu
drivers to implement a minimum hw_info callback which just returns the
driver_type if it does not have driver-specific data to report to the user
yet.
> btw IOMMU_HW_INFO_TYPE_DEFAULT also sounds misleading.
> 'default' implies hw_info still available but in a default format.
>
> probably it's clearer to call it IOMMU_HW_INFO_TYPE_NONE.
Sure. Makes sense. So _NONE means no driver specific info is
Reported back to user.
Regards,
Yi Liu
next prev parent reply other threads:[~2023-03-29 9:47 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-09 7:53 [PATCH v2 0/4] iommufd: Add iommu hardware info reporting Yi Liu
2023-03-09 7:53 ` [PATCH v2 1/4] iommu: Move dev_iommu_ops() to private header Yi Liu
2023-03-09 12:58 ` Baolu Lu
2023-03-09 7:53 ` [PATCH v2 2/4] iommu: Add new iommu op to get iommu hardware information Yi Liu
2023-03-16 8:16 ` Tian, Kevin
2023-03-16 8:30 ` Baolu Lu
2023-03-17 0:08 ` Tian, Kevin
2023-03-29 9:46 ` Liu, Yi L [this message]
2023-03-09 7:53 ` [PATCH v2 3/4] iommufd: Add IOMMU_DEVICE_GET_HW_INFO Yi Liu
2023-03-09 13:50 ` Baolu Lu
2023-03-09 17:20 ` Jason Gunthorpe
2023-03-10 8:06 ` Liu, Yi L
2023-03-16 8:23 ` Tian, Kevin
2023-03-09 7:53 ` [PATCH v2 4/4] iommufd/selftest: Add coverage for IOMMU_DEVICE_GET_HW_INFO ioctl Yi Liu
2023-03-20 12:43 ` Jason Gunthorpe
2023-03-20 12:43 ` [PATCH v2 0/4] iommufd: Add iommu hardware info reporting Jason Gunthorpe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DS0PR11MB75297955136ECD62A825287CC3899@DS0PR11MB7529.namprd11.prod.outlook.com \
--to=yi.l.liu@intel.com \
--cc=alex.williamson@redhat.com \
--cc=baolu.lu@linux.intel.com \
--cc=chao.p.peng@linux.intel.com \
--cc=cohuck@redhat.com \
--cc=eric.auger@redhat.com \
--cc=iommu@lists.linux.dev \
--cc=jasowang@redhat.com \
--cc=jgg@nvidia.com \
--cc=joro@8bytes.org \
--cc=kevin.tian@intel.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=lulu@redhat.com \
--cc=mjrosato@linux.ibm.com \
--cc=nicolinc@nvidia.com \
--cc=peterx@redhat.com \
--cc=robin.murphy@arm.com \
--cc=shameerali.kolothum.thodi@huawei.com \
--cc=suravee.suthikulpanit@amd.com \
--cc=yi.y.sun@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).