LKML Archive mirror
 help / color / mirror / Atom feed
From: Jason Gunthorpe <jgg@nvidia.com>
To: Nicolin Chen <nicolinc@nvidia.com>
Cc: will@kernel.org, robin.murphy@arm.com, kevin.tian@intel.com,
	suravee.suthikulpanit@amd.com, joro@8bytes.org,
	linux-kernel@vger.kernel.org, iommu@lists.linux.dev,
	linux-arm-kernel@lists.infradead.org,
	linux-tegra@vger.kernel.org, yi.l.liu@intel.com,
	eric.auger@redhat.com, vasant.hegde@amd.com, jon.grimm@amd.com,
	santosh.shukla@amd.com, Dhaval.Giani@amd.com,
	shameerali.kolothum.thodi@huawei.com
Subject: Re: [PATCH RFCv1 08/14] iommufd: Add IOMMU_VIOMMU_SET_DEV_ID ioctl
Date: Tue, 21 May 2024 15:30:11 -0300	[thread overview]
Message-ID: <20240521183011.GO20229@nvidia.com> (raw)
In-Reply-To: <ZkbnvnaiV9nCHQOb@nvidia.com>

On Thu, May 16, 2024 at 10:14:38PM -0700, Nicolin Chen wrote:
> I implemented it with a small tweak, to align with viommu_alloc
> and vqueue_alloc:
> 
> 	// core
> 	struct iommufd_vdev_id {
> 		struct iommufd_viommu *viommu;
> 		struct device *dev;
> 		u64 vdev_id;
> 		struct list_head idev_item;
> 	};
> 
> 	// driver
> 	struct my_driver_vdev_id {
> 	    struct iommufd_vdev_id core;
> 	    unsigned int private_attrs;
> 	};
> 
> 	static struct iommufd_vdev_id *
> 	my_driver_set_vdev_id(struct iommufd_viommu *viommu,
> 			      struct device *dev, u64 id)
> 	{
> 	    struct my_driver_vdev_id *my_vdev_id;
> 
> 	    my_vdev_id = kzalloc(sizeof(*my_vdev_id), GFP_KERNEL);
> 	    .... /* set private_attrs */
> 	    return &my_driver_vdev_id->core;
> 	}
> 
> 	static void my_driver_unset_vdev_id(struct iommufd_vdev_id *vdev_id)
> 	{
> 	    struct my_driver_vdev_id *my_vdev_id =
> 		    container_of(vdev_id, struct my_driver_vdev_id, core);
> 	    .... /* unset private_attrs */
> 	}
> 
> Please let me know if you like it inverted as you wrote above.

Seems like a reasonable direction

> I also moved xa_cmpxchg to the driver, as I feel that the vdev_id
> here is very driver/iommu specific. There can be some complication
> if iommufd core handles this u64 vdev_id: most likely we will use
> this u64 vdev_id to index the xarray that takes an unsigned-long
> xa_index for a fast vdev_id-to-pdev_id lookup, while only a driver
> knows whether u64 vdev_id is compatible with unsigned long or not.

This seems like the most general part.. The core code should just have
a check like:

  if (vdevid >= ULONG_MAX) return -EINVAL;

And if someone wants to make 32 bit kernels support a 64bit vdevid
then they can sort it out someday :) I think this is not a big issue
as all iommus seem to have some kind of radix lookup for vdevid and
want it to be small.

Matthew has been talking about support for 64bit indexes in
maple/xarray or something for a bit so it might sort itself out.

> And, we have a list_head in the structure idev, so a device unbind
> will for-each the list and unset all the vdev_ids in it, meanwhile
> the viommu stays. I wonder if we need to add another list_head in
> the structure viommu, so a viommu tear down will for-each its list
> and unset all the vdev_ids on its side while a device (idev) stays.
> I don't see a use case of that though..any thought?

I think you need to support viommu teardown, at least from a
correctness perspective. The API permits it. But isn't it just
list_del everything in the xarray and that will clean it up enough?

Jason

  reply	other threads:[~2024-05-21 18:30 UTC|newest]

Thread overview: 118+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-13  3:46 [PATCH RFCv1 00/14] Add Tegra241 (Grace) CMDQV Support (part 2/2) Nicolin Chen
2024-04-13  3:46 ` [PATCH RFCv1 01/14] iommufd: Move iommufd_object to public iommufd header Nicolin Chen
2024-05-12 13:21   ` Jason Gunthorpe
2024-05-12 22:40     ` Nicolin Chen
2024-05-13 22:30       ` Jason Gunthorpe
2024-04-13  3:46 ` [PATCH RFCv1 02/14] iommufd: Swap _iommufd_object_alloc and __iommufd_object_alloc Nicolin Chen
2024-05-12 13:26   ` Jason Gunthorpe
2024-05-13  2:29     ` Nicolin Chen
2024-05-13  3:44       ` Nicolin Chen
2024-05-13 22:30       ` Jason Gunthorpe
2024-04-13  3:47 ` [PATCH RFCv1 03/14] iommufd: Prepare for viommu structures and functions Nicolin Chen
2024-05-12 13:42   ` Jason Gunthorpe
2024-05-13  2:35     ` Nicolin Chen
2024-04-13  3:47 ` [PATCH RFCv1 04/14] iommufd: Add struct iommufd_viommu and iommufd_viommu_ops Nicolin Chen
2024-05-12 14:03   ` Jason Gunthorpe
2024-05-13  3:34     ` Nicolin Chen
2024-05-14 15:55       ` Jason Gunthorpe
2024-05-22  8:58         ` Tian, Kevin
2024-05-22  9:57           ` Baolu Lu
2024-05-22 13:39           ` Jason Gunthorpe
2024-05-23  1:43             ` Tian, Kevin
2024-05-23  4:01               ` Nicolin Chen
2024-05-23  5:40                 ` Tian, Kevin
2024-05-23 12:58               ` Jason Gunthorpe
2024-05-24  2:16                 ` Tian, Kevin
2024-05-24 13:03                   ` Jason Gunthorpe
2024-05-24  2:36                 ` Tian, Kevin
2024-04-13  3:47 ` [PATCH RFCv1 05/14] iommufd: Add IOMMUFD_OBJ_VIOMMU and IOMMUFD_CMD_VIOMMU_ALLOC Nicolin Chen
2024-05-12 14:27   ` Jason Gunthorpe
2024-05-13  4:33     ` Nicolin Chen
2024-05-14 15:38       ` Jason Gunthorpe
2024-05-15  1:20         ` Nicolin Chen
2024-05-21 18:05           ` Jason Gunthorpe
2024-05-22  0:13             ` Nicolin Chen
2024-05-22 16:46               ` Jason Gunthorpe
2024-04-13  3:47 ` [PATCH RFCv1 06/14] iommufd/selftest: Add IOMMU_VIOMMU_ALLOC test coverage Nicolin Chen
2024-04-13  3:47 ` [PATCH RFCv1 07/14] iommufd: Add viommu set/unset_dev_id ops Nicolin Chen
2024-05-12 14:46   ` Jason Gunthorpe
2024-05-13  4:39     ` Nicolin Chen
2024-05-14 15:53       ` Jason Gunthorpe
2024-05-15  1:59         ` Nicolin Chen
2024-05-21 18:24           ` Jason Gunthorpe
2024-05-21 22:27             ` Nicolin Chen
2024-05-22 13:59               ` Jason Gunthorpe
2024-05-23  6:19             ` Tian, Kevin
2024-05-23 15:01               ` Jason Gunthorpe
2024-05-24  2:21                 ` Tian, Kevin
2024-05-24  3:26                   ` Nicolin Chen
2024-05-24  5:24                     ` Tian, Kevin
2024-05-24  5:57                       ` Nicolin Chen
2024-05-24  7:21                         ` Tian, Kevin
2024-05-24 13:12                           ` Jason Gunthorpe
2024-05-24 13:05                   ` Jason Gunthorpe
2024-05-23  5:44       ` Tian, Kevin
2024-05-23  6:09         ` Nicolin Chen
2024-05-23  6:22           ` Tian, Kevin
2024-05-23 13:33         ` Jason Gunthorpe
2024-05-12 14:51   ` Jason Gunthorpe
2024-04-13  3:47 ` [PATCH RFCv1 08/14] iommufd: Add IOMMU_VIOMMU_SET_DEV_ID ioctl Nicolin Chen
2024-05-12 14:58   ` Jason Gunthorpe
2024-05-13  5:24     ` Nicolin Chen
2024-05-17  5:14     ` Nicolin Chen
2024-05-21 18:30       ` Jason Gunthorpe [this message]
2024-05-22  2:15         ` Nicolin Chen
2024-05-23  6:42   ` Tian, Kevin
2024-05-24  5:40     ` Nicolin Chen
2024-05-24  7:13       ` Tian, Kevin
2024-05-24 13:19         ` Jason Gunthorpe
2024-05-27  1:08           ` Tian, Kevin
2024-05-28 20:22             ` Nicolin Chen
2024-05-28 20:33               ` Nicolin Chen
2024-05-29  2:58               ` Tian, Kevin
2024-05-29  3:20                 ` Nicolin Chen
2024-05-30  0:28                   ` Tian, Kevin
2024-05-30  0:58                     ` Nicolin Chen
2024-05-30  3:05                       ` Tian, Kevin
2024-05-30  4:26                         ` Nicolin Chen
2024-06-01 21:45                       ` Jason Gunthorpe
2024-06-03  3:25                         ` Nicolin Chen
2024-06-06 18:24                           ` Jason Gunthorpe
2024-06-06 18:44                             ` Nicolin Chen
2024-06-07  0:27                               ` Jason Gunthorpe
2024-06-07  0:36                                 ` Tian, Kevin
2024-06-07 14:49                                   ` Jason Gunthorpe
2024-06-07 21:19                                     ` Nicolin Chen
2024-06-10 12:04                                       ` Jason Gunthorpe
2024-06-10 20:01                                         ` Nicolin Chen
2024-06-10 22:01                                           ` Jason Gunthorpe
2024-06-10 23:04                                             ` Nicolin Chen
2024-06-11  0:28                                               ` Jason Gunthorpe
2024-06-11  0:44                                                 ` Nicolin Chen
2024-06-11 12:17                                                   ` Jason Gunthorpe
2024-06-11 19:11                                                     ` Nicolin Chen
2024-05-28 20:30             ` Nicolin Chen
2024-05-24 13:20       ` Jason Gunthorpe
2024-04-13  3:47 ` [PATCH RFCv1 09/14] iommufd/selftest: Add IOMMU_VIOMMU_SET_DEV_ID test coverage Nicolin Chen
2024-04-13  3:47 ` [PATCH RFCv1 10/14] iommufd/selftest: Add IOMMU_TEST_OP_MV_CHECK_DEV_ID Nicolin Chen
2024-04-13  3:47 ` [PATCH RFCv1 11/14] iommufd: Add struct iommufd_vqueue and its related viommu ops Nicolin Chen
2024-04-13  3:47 ` [PATCH RFCv1 12/14] iommufd: Add IOMMUFD_OBJ_VQUEUE and IOMMUFD_CMD_VQUEUE_ALLOC Nicolin Chen
2024-05-12 15:02   ` Jason Gunthorpe
2024-05-13  4:41     ` Nicolin Chen
2024-05-13 22:36       ` Jason Gunthorpe
2024-05-23  6:57     ` Tian, Kevin
2024-05-24  4:42       ` Nicolin Chen
2024-05-24  5:26         ` Tian, Kevin
2024-05-24  6:03           ` Nicolin Chen
2024-05-23  7:05   ` Tian, Kevin
2024-04-13  3:47 ` [PATCH RFCv1 13/14] iommufd: Add mmap infrastructure Nicolin Chen
2024-05-12 15:19   ` Jason Gunthorpe
2024-05-13  4:43     ` Nicolin Chen
2024-04-13  3:47 ` [PATCH RFCv1 14/14] iommu/tegra241-cmdqv: Add user-space use support Nicolin Chen
2024-05-22  8:40 ` [PATCH RFCv1 00/14] Add Tegra241 (Grace) CMDQV Support (part 2/2) Tian, Kevin
2024-05-22 16:48   ` Jason Gunthorpe
2024-05-22 19:47     ` Nicolin Chen
2024-05-22 23:28       ` Jason Gunthorpe
2024-05-22 23:43         ` Tian, Kevin
2024-05-23  3:09           ` Nicolin Chen
2024-05-23 12:48             ` 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=20240521183011.GO20229@nvidia.com \
    --to=jgg@nvidia.com \
    --cc=Dhaval.Giani@amd.com \
    --cc=eric.auger@redhat.com \
    --cc=iommu@lists.linux.dev \
    --cc=jon.grimm@amd.com \
    --cc=joro@8bytes.org \
    --cc=kevin.tian@intel.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=nicolinc@nvidia.com \
    --cc=robin.murphy@arm.com \
    --cc=santosh.shukla@amd.com \
    --cc=shameerali.kolothum.thodi@huawei.com \
    --cc=suravee.suthikulpanit@amd.com \
    --cc=vasant.hegde@amd.com \
    --cc=will@kernel.org \
    --cc=yi.l.liu@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 read-only IMAP folder(s) and NNTP newsgroup(s).