From: Jacob Pan <jacob.jun.pan@linux.intel.com>
To: Jean-Philippe Brucker <jean-philippe@linaro.org>
Cc: "Tian, Kevin" <kevin.tian@intel.com>,
Raj Ashok <ashok.raj@intel.com>,
Jean-Philippe Brucker <jean-philippe@linaro.com>,
LKML <linux-kernel@vger.kernel.org>,
iommu@lists.linux-foundation.org,
Christoph Hellwig <hch@infradead.org>,
Alex Williamson <alex.williamson@redhat.com>,
David Woodhouse <dwmw2@infradead.org>,
Jonathan Cameron <jic23@kernel.org>
Subject: Re: [PATCH 03/10] iommu/ioasid: Introduce per set allocation APIs
Date: Tue, 21 Apr 2020 14:51:14 -0700 [thread overview]
Message-ID: <20200421145114.49e05059@jacob-builder> (raw)
In-Reply-To: <20200407110107.GA285264@myrica>
Hi Jean,
Sorry for the late reply, been trying to redesign the notification part.
On Tue, 7 Apr 2020 13:01:07 +0200
Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:
> On Mon, Apr 06, 2020 at 01:02:45PM -0700, Jacob Pan wrote:
> > > > + sdata = kzalloc(sizeof(*sdata), GFP_KERNEL);
> > > > + if (!sdata)
> > > > + return -ENOMEM;
> > >
> > > I don't understand why we need this structure at all, nor why we
> > > need the SID. Users have already allocated an ioasid_set, so why
> > > not just stick the content of ioasid_set_data in there, and pass
> > > the ioasid_set pointer to ioasid_alloc()?
> > >
> >
> > My thinking was that ioasid_set is an opaque user token, e.g. we
> > use mm to identify a common set belong to a VM.
> >
> > This sdata is an IOASID internal structure for managing & servicing
> > per set data. If we let user fill in the content, some of the
> > entries need to be managed by the IOASID code under a lock.
>
> We don't have to let users fill the content. A bit like iommu_domain:
> device drivers don't modify it, they pass it to iommu_map() rather
> than passing a domain ID.
>
much better.
> > IMO, not suitable to let user allocate and manage.
> >
> > Perhaps we should rename struct ioasid_set to ioasid_set_token?
>
> Is the token actually used anywhere? As far as I can tell VFIO does
> its own uniqueness check before calling ioasid_alloc_set(), and
> consumers of notifications don't read the token.
>
for vt-d, the per vm token (preferrably mm) will be used by kvm to
manage its PASID translation table.
when kvm receives a notification about a new guest-host PASID mapping,
it needs to know which vm it belongs to. So if mm is used as token,
both vfio and kvm can identify PASID ownership.
> >
> > /**
> > * struct ioasid_set_data - Meta data about ioasid_set
> > *
> > * @token: Unique to identify an IOASID set
> > * @xa: XArray to store ioasid_set private ID to
> > system-wide IOASID
> > * mapping
> > * @max_id: Max number of IOASIDs can be allocated within
> > the set
> > * @nr_id Number of IOASIDs allocated in the set
> > * @sid ID of the set
> > */
> > struct ioasid_set_data {
> > struct ioasid_set *token;
> > struct xarray xa;
> > int size;
> > int nr_ioasids;
> > int sid;
> > struct rcu_head rcu;
> > };
>
> How about we remove the current ioasid_set, call this structure
> ioasid_set instead of ioasid_set_data, and have ioasid_alloc_set()
> return it, rather than requiring users to allocate the ioasid_set
> themselves?
>
> struct ioasid_set *ioasid_alloc_set(ioasid_t quota):
>
> This way ioasid_set is opaque to users (we could have the definition
> in ioasid.c), but it can be passed to ioasid_alloc() and avoids the
> lookup by SID. Could also add the unique token as a void * argument to
> ioasid_alloc_set(), if needed.
>
Sounds good. still pass a token. Thanks for the idea.
> Thanks,
> Jean
[Jacob Pan]
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2020-04-21 21:45 UTC|newest]
Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-25 17:55 [PATCH 00/10] IOASID extensions for guest SVA Jacob Pan
2020-03-25 17:55 ` [PATCH 01/10] iommu/ioasid: Introduce system-wide capacity Jacob Pan
2020-03-27 8:07 ` Tian, Kevin
2020-03-27 16:08 ` Jacob Pan
2020-04-01 13:45 ` Jean-Philippe Brucker
2020-04-01 22:50 ` Jacob Pan
2020-03-25 17:55 ` [PATCH 02/10] iommu/vt-d: Set IOASID capacity when SVM is enabled Jacob Pan
2020-03-27 8:08 ` Tian, Kevin
2020-03-25 17:55 ` [PATCH 03/10] iommu/ioasid: Introduce per set allocation APIs Jacob Pan
2020-03-26 2:12 ` Lu Baolu
2020-03-26 21:30 ` Jacob Pan
2020-03-27 8:38 ` Tian, Kevin
2020-03-27 16:59 ` Jacob Pan
2020-03-28 6:32 ` Tian, Kevin
2020-04-01 13:47 ` Jean-Philippe Brucker
2020-04-06 20:02 ` Jacob Pan
2020-04-07 11:01 ` Jean-Philippe Brucker
2020-04-21 21:51 ` Jacob Pan [this message]
2020-03-25 17:55 ` [PATCH 04/10] iommu/ioasid: Rename ioasid_set_data to avoid confusion with ioasid_set Jacob Pan
2020-03-27 9:35 ` Tian, Kevin
2020-03-25 17:55 ` [PATCH 05/10] iommu/ioasid: Create an IOASID set for host SVA use Jacob Pan
2020-03-27 9:41 ` Tian, Kevin
2020-03-27 17:28 ` Jacob Pan
2020-03-28 6:33 ` Tian, Kevin
2020-04-01 13:53 ` Jean-Philippe Brucker
2020-04-06 15:33 ` Jacob Pan
2020-04-07 11:01 ` Jean-Philippe Brucker
2020-04-13 22:06 ` Jacob Pan
2020-04-15 15:10 ` Jean-Philippe Brucker
2020-03-25 17:55 ` [PATCH 06/10] iommu/ioasid: Convert to set aware allocations Jacob Pan
2020-03-27 9:54 ` Tian, Kevin
2020-03-27 17:41 ` Jacob Pan
2020-03-28 6:40 ` Tian, Kevin
2020-04-06 20:07 ` Jacob Pan
2020-04-01 13:55 ` Jean-Philippe Brucker
2020-04-01 22:45 ` Jacob Pan
2020-03-25 17:55 ` [PATCH 07/10] iommu/ioasid: Use mutex instead of spinlock Jacob Pan
2020-03-27 9:55 ` Tian, Kevin
2020-04-01 13:58 ` Jean-Philippe Brucker
2020-03-25 17:55 ` [PATCH 08/10] iommu/ioasid: Introduce notifier APIs Jacob Pan
2020-03-27 10:03 ` Tian, Kevin
2020-03-27 18:36 ` Jacob Pan
2020-03-28 6:43 ` Tian, Kevin
2020-03-31 15:13 ` Jacob Pan
2020-04-01 14:00 ` Jean-Philippe Brucker
2020-04-10 15:43 ` Jacob Pan
2020-03-25 17:55 ` [PATCH 09/10] iommu/ioasid: Support ioasid_set quota adjustment Jacob Pan
2020-03-27 10:09 ` Tian, Kevin
2020-03-27 23:30 ` Jacob Pan
2020-03-28 6:44 ` Tian, Kevin
2020-03-25 17:55 ` [PATCH 10/10] iommu/vt-d: Register PASID notifier for status change Jacob Pan
2020-03-27 10:22 ` Tian, Kevin
2020-03-27 23:47 ` Jacob Pan
2020-04-01 14:03 ` [PATCH 00/10] IOASID extensions for guest SVA Jean-Philippe Brucker
2020-04-01 23:38 ` Jacob Pan
2020-04-02 12:26 ` Jean-Philippe Brucker
2020-04-02 16:09 ` Jacob Pan
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=20200421145114.49e05059@jacob-builder \
--to=jacob.jun.pan@linux.intel.com \
--cc=alex.williamson@redhat.com \
--cc=ashok.raj@intel.com \
--cc=dwmw2@infradead.org \
--cc=hch@infradead.org \
--cc=iommu@lists.linux-foundation.org \
--cc=jean-philippe@linaro.com \
--cc=jean-philippe@linaro.org \
--cc=jic23@kernel.org \
--cc=kevin.tian@intel.com \
--cc=linux-kernel@vger.kernel.org \
/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).