Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
From: Jacob Pan <jacob.jun.pan@linux.intel.com>
To: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Cc: Mark Rutland <Mark.Rutland@arm.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	jacob.jun.pan@linux.intel.com, Will Deacon <Will.Deacon@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"iommu@lists.linux-foundation.org"
	<iommu@lists.linux-foundation.org>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	Jonathan Cameron <jonathan.cameron@huawei.com>,
	Robin Murphy <Robin.Murphy@arm.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 1/8] iommu: Add I/O ASID allocator
Date: Tue, 18 Jun 2019 10:05:08 -0700
Message-ID: <20190618100508.7835780f@jacob-builder> (raw)
In-Reply-To: <13e19d8c-8918-a3bb-f398-2ac41c71d307@arm.com>

On Tue, 18 Jun 2019 15:22:20 +0100
Jean-Philippe Brucker <jean-philippe.brucker@arm.com> wrote:

> On 11/06/2019 19:13, Jacob Pan wrote:
> >>>> +/**
> >>>> + * ioasid_find - Find IOASID data
> >>>> + * @set: the IOASID set
> >>>> + * @ioasid: the IOASID to find
> >>>> + * @getter: function to call on the found object
> >>>> + *
> >>>> + * The optional getter function allows to take a reference to
> >>>> the found object
> >>>> + * under the rcu lock. The function can also check if the object
> >>>> is still valid:
> >>>> + * if @getter returns false, then the object is invalid and NULL
> >>>> is returned.
> >>>> + *
> >>>> + * If the IOASID has been allocated for this set, return the
> >>>> private pointer
> >>>> + * passed to ioasid_alloc. Private data can be NULL if not set.
> >>>> Return an error
> >>>> + * if the IOASID is not found or does not belong to the set.    
> >>>
> >>> Perhaps should make it clear that @set can be null.    
> >>
> >> Indeed. But I'm not sure allowing @set to be NULL is such a good
> >> idea, because the data type associated to an ioasid depends on its
> >> set. For example SVA will put an mm_struct in there, and auxiliary
> >> domains use some structure private to the IOMMU domain.
> >>  
> > I am not sure we need to count on @set to decipher data type.
> > Whoever does the allocation and owns the IOASID should knows its
> > own data type. My thought was that @set is only used to group IDs,
> > permission check etc.
> >   
> >> Jacob, could me make @set mandatory, or do you see a use for a
> >> global search? If @set is NULL, then callers can check if the
> >> return pointer is NULL, but will run into trouble if they try to
> >> dereference it. 
> > A global search use case can be for PRQ. IOMMU driver gets a IOASID
> > (first interrupt then retrieve from a queue), it has no idea which
> > @set it belongs to. But the data types are the same for all IOASIDs
> > used by the IOMMU.  
> 
> They aren't when we use a generic SVA handler. Following a call to
> iommu_sva_bind_device(), iommu-sva.c allocates an IOASID and store an
> mm_struct. If auxiliary domains are also enabled for the device,
> following a call to iommu_aux_attach_device() the IOMMU driver
> allocates an IOASID and stores some private object.
> 
> Now for example the IOMMU driver receives a PPR and calls
> ioasid_find() with @set = NULL. ioasid_find() may return either an
> mm_struct or a private object, and the driver cannot know which it is
> so the returned value is unusable.
I think we might have a misunderstanding of what ioasid_set is. Or i
have misused your original intention for it:) So my understanding of an
ioasid_set is to identify a sub set of IOASIDs that
1. shares the same data type
2. belongs to the same permission group, owner.

Our usage is #2., we put a mm_struct as the set to do permission
check. E.g VFIO can check guest PASID ownership based on QEMU process
mm. This will make sure IOASID allocated by one guest can only be used
by the same guest.

For IOASID used for sva bind, let it be native or guest sva, we always
have the same data type. Therefore, when page request handler gets
called to search with ioasid_find(), it knows its data type. An
additional flag will tell if it is a guest bind or native bind.

I was under the assumption that aux domain and its IOASID is a 1:1
mapping, there is no need for a search. Or even it needs to search, it
will be searched by the same caller that did the allocation, therefore
it knows what private data type is.

In addition, aux domain is used for request w/o PASID. And PPR for
request w/o PASID is not to be supported. So there would not be a need
to search from page request handler.

Now if we take the above assumption away, and use ioasid_set strictly
to differentiate the data types (Usage #1). Then I agree we can get
rid of NULL set and global search.

That would require we converge on the generic sva_bind for both
native and guest. The additional implication is that VFIO layer has to
be SVA struct aware in order to sanitize the mm_struct for guest bind.
i.e. check guest ownership by using QEMU process mm. Whereas we have
today, VFIO just use IOASID set as mm to check ownership, no need to
know the type.

Can we keep the NULL set for now and remove it __after__ we converge on
the sva bind flows?

Thanks,

Jacob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply index

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-10 18:47 [PATCH 0/8] iommu: Add auxiliary domain and PASID support to Arm SMMUv3 Jean-Philippe Brucker
2019-06-10 18:47 ` [PATCH 1/8] iommu: Add I/O ASID allocator Jean-Philippe Brucker
2019-06-11  9:36   ` Jonathan Cameron
2019-06-11 14:35     ` Jean-Philippe Brucker
2019-06-11 18:13       ` Jacob Pan
2019-06-18 14:22         ` Jean-Philippe Brucker
2019-06-18 17:05           ` Jacob Pan [this message]
2019-06-19 14:26             ` Jean-Philippe Brucker
2019-06-11 12:26   ` Jacob Pan
2019-06-11 14:37     ` Jean-Philippe Brucker
2019-06-11 17:10       ` Jacob Pan
2019-06-12 11:30         ` Jean-Philippe Brucker
2019-06-10 18:47 ` [PATCH 2/8] dt-bindings: document PASID property for IOMMU masters Jean-Philippe Brucker
2019-07-08  7:58   ` Auger Eric
2019-06-10 18:47 ` [PATCH 3/8] iommu/arm-smmu-v3: Support platform SSID Jean-Philippe Brucker
2019-06-11  9:42   ` Jonathan Cameron
2019-06-11 14:35     ` Jean-Philippe Brucker
2019-06-18 18:08   ` Will Deacon
2019-06-19 11:53     ` Jean-Philippe Brucker
2019-07-08  7:58   ` Auger Eric
2019-09-19 14:51     ` Jean-Philippe Brucker
2019-06-10 18:47 ` [PATCH 4/8] iommu/arm-smmu-v3: Add support for Substream IDs Jean-Philippe Brucker
2019-06-11 10:19   ` Jonathan Cameron
2019-06-11 14:35     ` Jean-Philippe Brucker
2019-06-26 18:00   ` Will Deacon
2019-07-04  9:33     ` Jean-Philippe Brucker
2019-09-19 14:57     ` Jean-Philippe Brucker
2019-07-08 15:31   ` Auger Eric
2019-09-19 15:01     ` Jean-Philippe Brucker
2019-06-10 18:47 ` [PATCH 5/8] iommu/arm-smmu-v3: Add second level of context descriptor table Jean-Philippe Brucker
2019-06-11 10:24   ` Jonathan Cameron
2019-07-08 15:13   ` Auger Eric
2019-06-10 18:47 ` [PATCH 6/8] iommu/arm-smmu-v3: Support auxiliary domains Jean-Philippe Brucker
2019-06-26 17:59   ` Will Deacon
2019-07-05 16:29     ` Jean-Philippe Brucker
2019-09-19 15:06     ` Jean-Philippe Brucker
2019-06-10 18:47 ` [PATCH 7/8] iommu/arm-smmu-v3: Improve add_device() error handling Jean-Philippe Brucker
2019-07-08  7:58   ` Auger Eric
2019-06-10 18:47 ` [PATCH 8/8] iommu/arm-smmu-v3: Add support for PCI PASID Jean-Philippe Brucker
2019-06-11 10:45   ` Jonathan Cameron
2019-06-11 14:35     ` Jean-Philippe Brucker
2019-07-08  7:58   ` Auger Eric
2019-09-19 15:10     ` Jean-Philippe Brucker

Reply instructions:

You may reply publically 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=20190618100508.7835780f@jacob-builder \
    --to=jacob.jun.pan@linux.intel.com \
    --cc=Mark.Rutland@arm.com \
    --cc=Robin.Murphy@arm.com \
    --cc=Will.Deacon@arm.com \
    --cc=devicetree@vger.kernel.org \
    --cc=iommu@lists.linux-foundation.org \
    --cc=jean-philippe.brucker@arm.com \
    --cc=jonathan.cameron@huawei.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robh+dt@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

Linux-ARM-Kernel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/0 linux-arm-kernel/git/0.git
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/1 linux-arm-kernel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-kernel linux-arm-kernel/ https://lore.kernel.org/linux-arm-kernel \
		linux-arm-kernel@lists.infradead.org infradead-linux-arm-kernel@archiver.kernel.org
	public-inbox-index linux-arm-kernel


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-arm-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox