From: Joerg Roedel <joro@8bytes.org>
To: "Prakhya, Sai Praneeth" <sai.praneeth.prakhya@intel.com>
Cc: Robin Murphy <robin.murphy@arm.com>,
"iommu@lists.linux-foundation.org"
<iommu@lists.linux-foundation.org>,
Will Deacon <will.deacon@arm.com>,
"Raj, Ashok" <ashok.raj@intel.com>,
Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH V4 1/3] iommu: Add support to change default domain of an iommu group
Date: Wed, 22 Jul 2020 15:52:44 +0200 [thread overview]
Message-ID: <20200722135244.GH27672@8bytes.org> (raw)
In-Reply-To: <FFF73D592F13FD46B8700F0A279B802F573DB164@ORSMSX114.amr.corp.intel.com>
On Tue, Jul 14, 2020 at 06:23:54PM +0000, Prakhya, Sai Praneeth wrote:
> Q1:
> > Presently, iommu_change_dev_def_domain() checks if the iommu group still has
> > only one device or not. Hence, checking if iommu group has one device or not is
> > done twice, once before taking device_lock() and the other, after taking
> > device_lock().
> >
> > I agree that the code isn't checking if the iommu group still has the _same_
> > device or not.
> > One way, I could think of doing it is by storing "dev" temporarily and checking
> > for it.
> > Do you think that's ok? Or would you rather suggest something else?
That sounds reasonable, get the device from the group, lock it, take
group->mutex, and check whether the same device is still alone in the
group.
> Q2:
> > The reason for taking iommu_group->mutex in the beginning of
> > iommu_change_dev_def_domain() is that the function
> >
> > 1. Checks if the group is being directly used by user level drivers (i.e. if (group-
> > >default_domain != group->domain))
> >
> > 2. Uses iommu_ops
> > (prev_dom = group->default_domain;
> > if (!prev_dom || !prev_dom->ops || !prev_dom->ops->def_domain_type))
> >
> > 3. Sets iomu_group->domain to iommu_group->default_domain
> >
> > I wanted to make sure that iommu_group->domain and iommu_group-
> > >default_domain aren't changed by some other thread while this thread is
> > working on it. So, please let me know if I misunderstood something.
This looks correct as well.
Regards,
Joerg
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2020-07-22 13:52 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-05 1:32 [PATCH V4 0/3] iommu: Add support to change default domain of an iommu group Sai Praneeth Prakhya
2020-06-05 1:32 ` [PATCH V4 1/3] " Sai Praneeth Prakhya
2020-06-08 1:49 ` Lu Baolu
2020-06-30 9:16 ` Joerg Roedel
2020-07-01 3:04 ` Prakhya, Sai Praneeth
2020-07-14 18:23 ` Prakhya, Sai Praneeth
2020-07-22 13:52 ` Joerg Roedel [this message]
2020-07-22 17:14 ` Prakhya, Sai Praneeth
2020-06-05 1:32 ` [PATCH V4 2/3] iommu: Take lock before reading iommu group default domain type Sai Praneeth Prakhya
2020-06-08 1:50 ` Lu Baolu
2020-06-05 1:32 ` [PATCH V4 3/3] iommu: Document usage of "/sys/kernel/iommu_groups/<grp_id>/type" file Sai Praneeth Prakhya
2020-06-08 1:50 ` Lu Baolu
2020-06-26 21:34 ` [PATCH V4 0/3] iommu: Add support to change default domain of an iommu group Prakhya, Sai Praneeth
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=20200722135244.GH27672@8bytes.org \
--to=joro@8bytes.org \
--cc=ashok.raj@intel.com \
--cc=hch@lst.de \
--cc=iommu@lists.linux-foundation.org \
--cc=robin.murphy@arm.com \
--cc=sai.praneeth.prakhya@intel.com \
--cc=will.deacon@arm.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).