LKML Archive on
help / color / mirror / Atom feed
From: Jean-Philippe Brucker <>
To: Jacob Pan <>,
	Joerg Roedel <>
Cc: ""
	"" <>,
	"" <>,
	"" <>,
	"" <>,
	"" <>,
	"" <>
Subject: Re: [PATCH 1/1] iommu: Bind process address spaces to devices
Date: Thu, 28 Feb 2019 12:19:22 +0000	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <20190227134129.51ad42ac@jacob-builder>

On 27/02/2019 21:41, Jacob Pan wrote:
> On Tue, 26 Feb 2019 12:17:43 +0100
> Joerg Roedel <> wrote:
>> Hi Jean-Philippe,
>> Thanks for the patch! I think this is getting close to be applied
>> after the next merge window.
>> On Wed, Feb 20, 2019 at 02:27:59PM +0000, Jean-Philippe Brucker wrote:
>>> +int iommu_sva_bind_device(struct device *dev, struct mm_struct
>>> *mm, int *pasid,
>>> +			  iommu_mm_exit_handler_t mm_exit, void
>>> *drvdata)  
>> I think we are better of with introducing a sva-bind handle which can
>> be used to extend and further configure the binding done with this
>> function.
>> How about a 'struct iommu_sva' with an iommu-private definition that
>> is returned by this function:
>> 	struct iommu_sva *iommu_sva_bind_device(struct device *dev,
>> 						struct mm_struct *mm);
> Just trying to understand how to use this API.
> So if we bind the same mm to two different devices, we should get two
> different iommu_sva handle, right?

Yes, the iommu_sva handle is the bond between one mm and one device, so
you will get two different handles.

> I think intel-svm still needs a flag argument for supervisor pasid etc.
> Other than that, I think both interface should work for vt-d.

Is supervisor PASID still needed now that we have auxiliary domains, and
now that VT-d supports nested IOVA? You could have private kernel
address spaces through auxiliary domains, or simply use DMA API as usual
with PASID#0. I've been reluctant to make that feature common because it
looks risky - gives full access to the kernel address space to devices
and no notification on mapping change.

> Another question is that for nested SVA, we will need to bind guest mm.
> Do you think we should try to reuse this or have it separate? I am
> working on a separate API for now.

I also think it should be separate. That bind() operation is performed
on an auxiliary domain, I guess?

>> and the corresponding unbind function:
>> 	int iommu_sva_unbind_device(struct iommu_sva* *handle);
>> (Btw, does this need to return and int? Can unbinding fail?).
>> With that in place we can implement and extentable API base on the
>> handle:
>> 	int iommu_sva_get_pasid(struct iommu_sva *handle);
> If multiple bind to the same mm gets multiple handles, this API should
> retrieve the same pasid for different handle?


> Just curious why making
> the handle private instead of returning the pasid value in the handle?

I don't have a strong objection against that. One reason to have an
accessor is that the PASID is freed on mm_exit, so until the device
driver calls unbind(), the PASID contained in the handle is stale (and
the accessor returns PASID_INVALID). But that's a bit pedantic, the
device driver should know that the handle is stale since it got notified
of it. Having an accessor might also help tracking uses of the handle in
the kernel, and make future API modifications easier.


>> 	void iommu_sva_set_exit_handler(struct iommu_sva *handle,
>> 					iommu_mm_exit_handler_t
>> mm_exit);
>> I think at least the AMD IOMMU driver needs more call-backs like a
>> handler that is invoked when a fault can not be resolved. And there
>> might be others in the future, putting them all in the parameter list
>> of the bind function doesn't scale well.
>> Regards,
>> 	Joerg

  parent reply	other threads:[~2019-02-28 12:19 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-20 14:27 [PATCH 0/1] IOMMU SVA device driver interface Jean-Philippe Brucker
2019-02-20 14:27 ` [PATCH 1/1] iommu: Bind process address spaces to devices Jean-Philippe Brucker
2019-02-26 11:17   ` Joerg Roedel
2019-02-26 12:49     ` Jean-Philippe Brucker
2019-02-26 13:02       ` Joerg Roedel
2019-02-27 21:41     ` Jacob Pan
2019-02-28  1:10       ` Tian, Kevin
2019-02-28 18:53         ` Jacob Pan
2019-02-28 12:19       ` Jean-Philippe Brucker [this message]
2019-02-28 18:32         ` Jacob Pan
2019-02-28 14:09       ` Joerg Roedel
2019-02-28 21:15         ` Jacob Pan
2019-02-28 22:04           ` Raj, Ashok

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \ \
    --subject='Re: [PATCH 1/1] iommu: Bind process address spaces to devices' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).