RE: [PATCH v2 1/3] docs: IOMMU user API

2020-06-20 Thread Liu, Yi L
> From: Alex Williamson 
> Sent: Saturday, June 20, 2020 12:38 AM
> 
> On Fri, 19 Jun 2020 03:30:24 +
> "Liu, Yi L"  wrote:
> 
> > Hi Alex,
> >
> > > From: Alex Williamson 
> > > Sent: Friday, June 19, 2020 10:55 AM
> > >
> > > On Fri, 19 Jun 2020 02:15:36 +
> > > "Liu, Yi L"  wrote:
> > >
> > > > Hi Alex,
> > > >
> > > > > From: Alex Williamson 
> > > > > Sent: Friday, June 19, 2020 5:48 AM
> > > > >
> > > > > On Wed, 17 Jun 2020 08:28:24 +
> > > > > "Tian, Kevin"  wrote:
> > > > >
> > > > > > > From: Liu, Yi L 
> > > > > > > Sent: Wednesday, June 17, 2020 2:20 PM
> > > > > > >
> > > > > > > > From: Jacob Pan 
> > > > > > > > Sent: Tuesday, June 16, 2020 11:22 PM
> > > > > > > >
> > > > > > > > On Thu, 11 Jun 2020 17:27:27 -0700 Jacob Pan
> > > > > > > >  wrote:
> > > > > > > >
> > > > > > > > > >
> > > > > > > > > > But then I thought it even better if VFIO leaves the
> > > > > > > > > > entire
> > > > > > > > > > copy_from_user() to the layer consuming it.
> > > > > > > > > >
> > > > > > > > > OK. Sounds good, that was what Kevin suggested also. I just
> > > > > > > > > wasn't sure how much VFIO wants to inspect, I thought VFIO
> > > > > > > > > layer wanted to do a sanity check.
> > > > > > > > >
> > > > > > > > > Anyway, I will move copy_from_user to iommu uapi layer.
> > > > > > > >
> > > > > > > > Just one more point brought up by Yi when we discuss this 
> > > > > > > > offline.
> > > > > > > >
> > > > > > > > If we move copy_from_user to iommu uapi layer, then there will
> > > > > > > > be
> > > > > > > multiple
> > > > > > > > copy_from_user calls for the same data when a VFIO container
> > > > > > > > has
> > > > > > > multiple domains,
> > > > > > > > devices. For bind, it might be OK. But might be additional
> > > > > > > > overhead for TLB
> > > > > > > flush
> > > > > > > > request from the guest.
> > > > > > >
> > > > > > > I think it is the same with bind and TLB flush path. will be
> > > > > > > multiple copy_from_user.
> > > > > >
> > > > > > multiple copies is possibly fine. In reality we allow only one
> > > > > > group per nesting container (as described in patch [03/15]), and
> > > > > > usually there is just one SVA-capable device per group.
> > > > > >
> > > > > > >
> > > > > > > BTW. for moving data copy to iommy layer, there is another point
> > > > > > > which need to consider. VFIO needs to do unbind in bind path if
> > > > > > > bind failed, so it will assemble unbind_data and pass to iommu
> > > > > > > layer. If iommu layer do the copy_from_user, I think it will be 
> > > > > > > failed.
> any
> > > idea?
> > > > >
> > > > > If a call into a UAPI fails, there should be nothing to undo.
> > > > > Creating a partial setup for a failed call that needs to be undone
> > > > > by the caller is not good practice.
> > > >
> > > > is it still a problem if it's the VFIO to undo the partial setup
> > > > before returning to user space?
> > >
> > > Yes.  If a UAPI function fails there should be no residual effect.
> >
> > ok. the iommu_sva_bind_gpasid() is per device call. There is no residual
> > effect if it failed. so no partial setup will happen per device.
> >
> > but VFIO needs to use iommu_group_for_each_dev() to do bind, so
> > if iommu_group_for_each_dev() failed, I guess VFIO needs to undo
> > the partial setup for the group. right?
> 
> Yes, each individual call should make no changes if it fails, but the
> caller would need to unwind separate calls.  If this introduces too
> much knowledge to the caller for the group case, maybe there should be
> a group-level function in the iommu code to handle that.  Thanks,


got you. I don't think VFIO needs too much knowledge except the
group info and the bind data. may send updated version based on
your comments.

Thanks,
Yi Liu

> Alex
> 
> > > > > > This might be mitigated if we go back to use the same bind_data
> > > > > > for both bind/unbind. Then you can reuse the user object for 
> > > > > > unwinding.
> > > > > >
> > > > > > However there is another case where VFIO may need to assemble the
> > > > > > bind_data itself. When a VM is killed, VFIO needs to walk
> > > > > > allocated PASIDs and unbind them one-by-one. In such case
> > > > > > copy_from_user doesn't work since the data is created by kernel.
> > > > > > Alex, do you have a suggestion how this usage can be supported?
> > > > > > e.g. asking IOMMU driver to provide two sets of APIs to handle
> user/kernel
> > > generated requests?
> > > > >
> > > > > Yes, it seems like vfio would need to make use of a driver API to do
> > > > > this, we shouldn't be faking a user buffer in the kernel in order to
> > > > > call through to a UAPI.  Thanks,
> > > >
> > > > ok, so if VFIO wants to issue unbind by itself, it should use an API
> > > > which passes kernel buffer to IOMMU layer. If the unbind request is
> > > > from user space, then VFIO should use another API which passes user
> > > > buffer pointer to IOMMU layer. makes sense. will align with jacob.
> > >
> > > 

Re: [PATCH v2 1/3] docs: IOMMU user API

2020-06-19 Thread Alex Williamson
On Fri, 19 Jun 2020 03:30:24 +
"Liu, Yi L"  wrote:

> Hi Alex,
> 
> > From: Alex Williamson 
> > Sent: Friday, June 19, 2020 10:55 AM
> > 
> > On Fri, 19 Jun 2020 02:15:36 +
> > "Liu, Yi L"  wrote:
> >   
> > > Hi Alex,
> > >  
> > > > From: Alex Williamson 
> > > > Sent: Friday, June 19, 2020 5:48 AM
> > > >
> > > > On Wed, 17 Jun 2020 08:28:24 +
> > > > "Tian, Kevin"  wrote:
> > > >  
> > > > > > From: Liu, Yi L 
> > > > > > Sent: Wednesday, June 17, 2020 2:20 PM
> > > > > >  
> > > > > > > From: Jacob Pan 
> > > > > > > Sent: Tuesday, June 16, 2020 11:22 PM
> > > > > > >
> > > > > > > On Thu, 11 Jun 2020 17:27:27 -0700 Jacob Pan
> > > > > > >  wrote:
> > > > > > >  
> > > > > > > > >
> > > > > > > > > But then I thought it even better if VFIO leaves the
> > > > > > > > > entire
> > > > > > > > > copy_from_user() to the layer consuming it.
> > > > > > > > >  
> > > > > > > > OK. Sounds good, that was what Kevin suggested also. I just
> > > > > > > > wasn't sure how much VFIO wants to inspect, I thought VFIO
> > > > > > > > layer wanted to do a sanity check.
> > > > > > > >
> > > > > > > > Anyway, I will move copy_from_user to iommu uapi layer.  
> > > > > > >
> > > > > > > Just one more point brought up by Yi when we discuss this offline.
> > > > > > >
> > > > > > > If we move copy_from_user to iommu uapi layer, then there will
> > > > > > > be  
> > > > > > multiple  
> > > > > > > copy_from_user calls for the same data when a VFIO container
> > > > > > > has  
> > > > > > multiple domains,  
> > > > > > > devices. For bind, it might be OK. But might be additional
> > > > > > > overhead for TLB  
> > > > > > flush  
> > > > > > > request from the guest.  
> > > > > >
> > > > > > I think it is the same with bind and TLB flush path. will be
> > > > > > multiple copy_from_user.  
> > > > >
> > > > > multiple copies is possibly fine. In reality we allow only one
> > > > > group per nesting container (as described in patch [03/15]), and
> > > > > usually there is just one SVA-capable device per group.
> > > > >  
> > > > > >
> > > > > > BTW. for moving data copy to iommy layer, there is another point
> > > > > > which need to consider. VFIO needs to do unbind in bind path if
> > > > > > bind failed, so it will assemble unbind_data and pass to iommu
> > > > > > layer. If iommu layer do the copy_from_user, I think it will be 
> > > > > > failed. any  
> > idea?  
> > > >
> > > > If a call into a UAPI fails, there should be nothing to undo.
> > > > Creating a partial setup for a failed call that needs to be undone
> > > > by the caller is not good practice.  
> > >
> > > is it still a problem if it's the VFIO to undo the partial setup
> > > before returning to user space?  
> > 
> > Yes.  If a UAPI function fails there should be no residual effect.  
> 
> ok. the iommu_sva_bind_gpasid() is per device call. There is no residual
> effect if it failed. so no partial setup will happen per device.
> 
> but VFIO needs to use iommu_group_for_each_dev() to do bind, so
> if iommu_group_for_each_dev() failed, I guess VFIO needs to undo
> the partial setup for the group. right?

Yes, each individual call should make no changes if it fails, but the
caller would need to unwind separate calls.  If this introduces too
much knowledge to the caller for the group case, maybe there should be
a group-level function in the iommu code to handle that.  Thanks,

Alex

> > > > > This might be mitigated if we go back to use the same bind_data
> > > > > for both bind/unbind. Then you can reuse the user object for 
> > > > > unwinding.
> > > > >
> > > > > However there is another case where VFIO may need to assemble the
> > > > > bind_data itself. When a VM is killed, VFIO needs to walk
> > > > > allocated PASIDs and unbind them one-by-one. In such case
> > > > > copy_from_user doesn't work since the data is created by kernel.
> > > > > Alex, do you have a suggestion how this usage can be supported?
> > > > > e.g. asking IOMMU driver to provide two sets of APIs to handle 
> > > > > user/kernel  
> > generated requests?  
> > > >
> > > > Yes, it seems like vfio would need to make use of a driver API to do
> > > > this, we shouldn't be faking a user buffer in the kernel in order to
> > > > call through to a UAPI.  Thanks,  
> > >
> > > ok, so if VFIO wants to issue unbind by itself, it should use an API
> > > which passes kernel buffer to IOMMU layer. If the unbind request is
> > > from user space, then VFIO should use another API which passes user
> > > buffer pointer to IOMMU layer. makes sense. will align with jacob.  
> > 
> > Sounds right to me.  Different approaches might be used for the driver API 
> > versus
> > the UAPI, perhaps there is no buffer.  Thanks,  
> 
> thanks for your coaching. It may require Jacob to add APIs in iommu layer
> for the two purposes.
> 
> Regards,
> Yi Liu
> 
> > Alex  
> 

___
iommu mailing list
iommu@lists.linux-foundation.org

RE: [PATCH v2 1/3] docs: IOMMU user API

2020-06-18 Thread Liu, Yi L
Hi Alex,

> From: Alex Williamson 
> Sent: Friday, June 19, 2020 10:55 AM
> 
> On Fri, 19 Jun 2020 02:15:36 +
> "Liu, Yi L"  wrote:
> 
> > Hi Alex,
> >
> > > From: Alex Williamson 
> > > Sent: Friday, June 19, 2020 5:48 AM
> > >
> > > On Wed, 17 Jun 2020 08:28:24 +
> > > "Tian, Kevin"  wrote:
> > >
> > > > > From: Liu, Yi L 
> > > > > Sent: Wednesday, June 17, 2020 2:20 PM
> > > > >
> > > > > > From: Jacob Pan 
> > > > > > Sent: Tuesday, June 16, 2020 11:22 PM
> > > > > >
> > > > > > On Thu, 11 Jun 2020 17:27:27 -0700 Jacob Pan
> > > > > >  wrote:
> > > > > >
> > > > > > > >
> > > > > > > > But then I thought it even better if VFIO leaves the
> > > > > > > > entire
> > > > > > > > copy_from_user() to the layer consuming it.
> > > > > > > >
> > > > > > > OK. Sounds good, that was what Kevin suggested also. I just
> > > > > > > wasn't sure how much VFIO wants to inspect, I thought VFIO
> > > > > > > layer wanted to do a sanity check.
> > > > > > >
> > > > > > > Anyway, I will move copy_from_user to iommu uapi layer.
> > > > > >
> > > > > > Just one more point brought up by Yi when we discuss this offline.
> > > > > >
> > > > > > If we move copy_from_user to iommu uapi layer, then there will
> > > > > > be
> > > > > multiple
> > > > > > copy_from_user calls for the same data when a VFIO container
> > > > > > has
> > > > > multiple domains,
> > > > > > devices. For bind, it might be OK. But might be additional
> > > > > > overhead for TLB
> > > > > flush
> > > > > > request from the guest.
> > > > >
> > > > > I think it is the same with bind and TLB flush path. will be
> > > > > multiple copy_from_user.
> > > >
> > > > multiple copies is possibly fine. In reality we allow only one
> > > > group per nesting container (as described in patch [03/15]), and
> > > > usually there is just one SVA-capable device per group.
> > > >
> > > > >
> > > > > BTW. for moving data copy to iommy layer, there is another point
> > > > > which need to consider. VFIO needs to do unbind in bind path if
> > > > > bind failed, so it will assemble unbind_data and pass to iommu
> > > > > layer. If iommu layer do the copy_from_user, I think it will be 
> > > > > failed. any
> idea?
> > >
> > > If a call into a UAPI fails, there should be nothing to undo.
> > > Creating a partial setup for a failed call that needs to be undone
> > > by the caller is not good practice.
> >
> > is it still a problem if it's the VFIO to undo the partial setup
> > before returning to user space?
> 
> Yes.  If a UAPI function fails there should be no residual effect.

ok. the iommu_sva_bind_gpasid() is per device call. There is no residual
effect if it failed. so no partial setup will happen per device.

but VFIO needs to use iommu_group_for_each_dev() to do bind, so
if iommu_group_for_each_dev() failed, I guess VFIO needs to undo
the partial setup for the group. right?

> > > > This might be mitigated if we go back to use the same bind_data
> > > > for both bind/unbind. Then you can reuse the user object for unwinding.
> > > >
> > > > However there is another case where VFIO may need to assemble the
> > > > bind_data itself. When a VM is killed, VFIO needs to walk
> > > > allocated PASIDs and unbind them one-by-one. In such case
> > > > copy_from_user doesn't work since the data is created by kernel.
> > > > Alex, do you have a suggestion how this usage can be supported?
> > > > e.g. asking IOMMU driver to provide two sets of APIs to handle 
> > > > user/kernel
> generated requests?
> > >
> > > Yes, it seems like vfio would need to make use of a driver API to do
> > > this, we shouldn't be faking a user buffer in the kernel in order to
> > > call through to a UAPI.  Thanks,
> >
> > ok, so if VFIO wants to issue unbind by itself, it should use an API
> > which passes kernel buffer to IOMMU layer. If the unbind request is
> > from user space, then VFIO should use another API which passes user
> > buffer pointer to IOMMU layer. makes sense. will align with jacob.
> 
> Sounds right to me.  Different approaches might be used for the driver API 
> versus
> the UAPI, perhaps there is no buffer.  Thanks,

thanks for your coaching. It may require Jacob to add APIs in iommu layer
for the two purposes.

Regards,
Yi Liu

> Alex

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 1/3] docs: IOMMU user API

2020-06-18 Thread Alex Williamson
On Fri, 19 Jun 2020 02:15:36 +
"Liu, Yi L"  wrote:

> Hi Alex,
> 
> > From: Alex Williamson 
> > Sent: Friday, June 19, 2020 5:48 AM
> > 
> > On Wed, 17 Jun 2020 08:28:24 +
> > "Tian, Kevin"  wrote:
> >   
> > > > From: Liu, Yi L 
> > > > Sent: Wednesday, June 17, 2020 2:20 PM
> > > >  
> > > > > From: Jacob Pan 
> > > > > Sent: Tuesday, June 16, 2020 11:22 PM
> > > > >
> > > > > On Thu, 11 Jun 2020 17:27:27 -0700
> > > > > Jacob Pan  wrote:
> > > > >  
> > > > > > >
> > > > > > > But then I thought it even better if VFIO leaves the entire
> > > > > > > copy_from_user() to the layer consuming it.
> > > > > > >  
> > > > > > OK. Sounds good, that was what Kevin suggested also. I just wasn't
> > > > > > sure how much VFIO wants to inspect, I thought VFIO layer wanted to 
> > > > > > do
> > > > > > a sanity check.
> > > > > >
> > > > > > Anyway, I will move copy_from_user to iommu uapi layer.  
> > > > >
> > > > > Just one more point brought up by Yi when we discuss this offline.
> > > > >
> > > > > If we move copy_from_user to iommu uapi layer, then there will be  
> > > > multiple  
> > > > > copy_from_user calls for the same data when a VFIO container has  
> > > > multiple domains,  
> > > > > devices. For bind, it might be OK. But might be additional overhead 
> > > > > for TLB  
> > > > flush  
> > > > > request from the guest.  
> > > >
> > > > I think it is the same with bind and TLB flush path. will be multiple
> > > > copy_from_user.  
> > >
> > > multiple copies is possibly fine. In reality we allow only one group per
> > > nesting container (as described in patch [03/15]), and usually there
> > > is just one SVA-capable device per group.
> > >  
> > > >
> > > > BTW. for moving data copy to iommy layer, there is another point which
> > > > need to consider. VFIO needs to do unbind in bind path if bind failed,
> > > > so it will assemble unbind_data and pass to iommu layer. If iommu layer
> > > > do the copy_from_user, I think it will be failed. any idea?  
> > 
> > If a call into a UAPI fails, there should be nothing to undo.  Creating
> > a partial setup for a failed call that needs to be undone by the caller
> > is not good practice.  
> 
> is it still a problem if it's the VFIO to undo the partial setup before
> returning to user space?

Yes.  If a UAPI function fails there should be no residual effect.
 
> > > This might be mitigated if we go back to use the same bind_data for both
> > > bind/unbind. Then you can reuse the user object for unwinding.
> > >
> > > However there is another case where VFIO may need to assemble the
> > > bind_data itself. When a VM is killed, VFIO needs to walk allocated PASIDs
> > > and unbind them one-by-one. In such case copy_from_user doesn't work
> > > since the data is created by kernel. Alex, do you have a suggestion how 
> > > this
> > > usage can be supported? e.g. asking IOMMU driver to provide two sets of
> > > APIs to handle user/kernel generated requests?  
> > 
> > Yes, it seems like vfio would need to make use of a driver API to do
> > this, we shouldn't be faking a user buffer in the kernel in order to
> > call through to a UAPI.  Thanks,  
> 
> ok, so if VFIO wants to issue unbind by itself, it should use an API which
> passes kernel buffer to IOMMU layer. If the unbind request is from user
> space, then VFIO should use another API which passes user buffer pointer
> to IOMMU layer. makes sense. will align with jacob.

Sounds right to me.  Different approaches might be used for the driver
API versus the UAPI, perhaps there is no buffer.  Thanks,

Alex

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


RE: [PATCH v2 1/3] docs: IOMMU user API

2020-06-18 Thread Liu, Yi L
Hi Alex,

> From: Alex Williamson 
> Sent: Friday, June 19, 2020 5:48 AM
> 
> On Wed, 17 Jun 2020 08:28:24 +
> "Tian, Kevin"  wrote:
> 
> > > From: Liu, Yi L 
> > > Sent: Wednesday, June 17, 2020 2:20 PM
> > >
> > > > From: Jacob Pan 
> > > > Sent: Tuesday, June 16, 2020 11:22 PM
> > > >
> > > > On Thu, 11 Jun 2020 17:27:27 -0700
> > > > Jacob Pan  wrote:
> > > >
> > > > > >
> > > > > > But then I thought it even better if VFIO leaves the entire
> > > > > > copy_from_user() to the layer consuming it.
> > > > > >
> > > > > OK. Sounds good, that was what Kevin suggested also. I just wasn't
> > > > > sure how much VFIO wants to inspect, I thought VFIO layer wanted to do
> > > > > a sanity check.
> > > > >
> > > > > Anyway, I will move copy_from_user to iommu uapi layer.
> > > >
> > > > Just one more point brought up by Yi when we discuss this offline.
> > > >
> > > > If we move copy_from_user to iommu uapi layer, then there will be
> > > multiple
> > > > copy_from_user calls for the same data when a VFIO container has
> > > multiple domains,
> > > > devices. For bind, it might be OK. But might be additional overhead for 
> > > > TLB
> > > flush
> > > > request from the guest.
> > >
> > > I think it is the same with bind and TLB flush path. will be multiple
> > > copy_from_user.
> >
> > multiple copies is possibly fine. In reality we allow only one group per
> > nesting container (as described in patch [03/15]), and usually there
> > is just one SVA-capable device per group.
> >
> > >
> > > BTW. for moving data copy to iommy layer, there is another point which
> > > need to consider. VFIO needs to do unbind in bind path if bind failed,
> > > so it will assemble unbind_data and pass to iommu layer. If iommu layer
> > > do the copy_from_user, I think it will be failed. any idea?
> 
> If a call into a UAPI fails, there should be nothing to undo.  Creating
> a partial setup for a failed call that needs to be undone by the caller
> is not good practice.

is it still a problem if it's the VFIO to undo the partial setup before
returning to user space?

> > This might be mitigated if we go back to use the same bind_data for both
> > bind/unbind. Then you can reuse the user object for unwinding.
> >
> > However there is another case where VFIO may need to assemble the
> > bind_data itself. When a VM is killed, VFIO needs to walk allocated PASIDs
> > and unbind them one-by-one. In such case copy_from_user doesn't work
> > since the data is created by kernel. Alex, do you have a suggestion how this
> > usage can be supported? e.g. asking IOMMU driver to provide two sets of
> > APIs to handle user/kernel generated requests?
> 
> Yes, it seems like vfio would need to make use of a driver API to do
> this, we shouldn't be faking a user buffer in the kernel in order to
> call through to a UAPI.  Thanks,

ok, so if VFIO wants to issue unbind by itself, it should use an API which
passes kernel buffer to IOMMU layer. If the unbind request is from user
space, then VFIO should use another API which passes user buffer pointer
to IOMMU layer. makes sense. will align with jacob.

Regards,
Yi Liu

> Alex

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 1/3] docs: IOMMU user API

2020-06-18 Thread Alex Williamson
On Wed, 17 Jun 2020 08:28:24 +
"Tian, Kevin"  wrote:

> > From: Liu, Yi L 
> > Sent: Wednesday, June 17, 2020 2:20 PM
> >   
> > > From: Jacob Pan 
> > > Sent: Tuesday, June 16, 2020 11:22 PM
> > >
> > > On Thu, 11 Jun 2020 17:27:27 -0700
> > > Jacob Pan  wrote:
> > >  
> > > > >
> > > > > But then I thought it even better if VFIO leaves the entire
> > > > > copy_from_user() to the layer consuming it.
> > > > >  
> > > > OK. Sounds good, that was what Kevin suggested also. I just wasn't
> > > > sure how much VFIO wants to inspect, I thought VFIO layer wanted to do
> > > > a sanity check.
> > > >
> > > > Anyway, I will move copy_from_user to iommu uapi layer.  
> > >
> > > Just one more point brought up by Yi when we discuss this offline.
> > >
> > > If we move copy_from_user to iommu uapi layer, then there will be  
> > multiple  
> > > copy_from_user calls for the same data when a VFIO container has  
> > multiple domains,  
> > > devices. For bind, it might be OK. But might be additional overhead for 
> > > TLB  
> > flush  
> > > request from the guest.  
> > 
> > I think it is the same with bind and TLB flush path. will be multiple
> > copy_from_user.  
> 
> multiple copies is possibly fine. In reality we allow only one group per
> nesting container (as described in patch [03/15]), and usually there
> is just one SVA-capable device per group.
> 
> > 
> > BTW. for moving data copy to iommy layer, there is another point which
> > need to consider. VFIO needs to do unbind in bind path if bind failed,
> > so it will assemble unbind_data and pass to iommu layer. If iommu layer
> > do the copy_from_user, I think it will be failed. any idea?

If a call into a UAPI fails, there should be nothing to undo.  Creating
a partial setup for a failed call that needs to be undone by the caller
is not good practice.

> This might be mitigated if we go back to use the same bind_data for both
> bind/unbind. Then you can reuse the user object for unwinding.
> 
> However there is another case where VFIO may need to assemble the
> bind_data itself. When a VM is killed, VFIO needs to walk allocated PASIDs
> and unbind them one-by-one. In such case copy_from_user doesn't work
> since the data is created by kernel. Alex, do you have a suggestion how this
> usage can be supported? e.g. asking IOMMU driver to provide two sets of
> APIs to handle user/kernel generated requests?

Yes, it seems like vfio would need to make use of a driver API to do
this, we shouldn't be faking a user buffer in the kernel in order to
call through to a UAPI.  Thanks,

Alex

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


RE: [PATCH v2 1/3] docs: IOMMU user API

2020-06-17 Thread Tian, Kevin
> From: Liu, Yi L 
> Sent: Wednesday, June 17, 2020 2:20 PM
> 
> > From: Jacob Pan 
> > Sent: Tuesday, June 16, 2020 11:22 PM
> >
> > On Thu, 11 Jun 2020 17:27:27 -0700
> > Jacob Pan  wrote:
> >
> > > >
> > > > But then I thought it even better if VFIO leaves the entire
> > > > copy_from_user() to the layer consuming it.
> > > >
> > > OK. Sounds good, that was what Kevin suggested also. I just wasn't
> > > sure how much VFIO wants to inspect, I thought VFIO layer wanted to do
> > > a sanity check.
> > >
> > > Anyway, I will move copy_from_user to iommu uapi layer.
> >
> > Just one more point brought up by Yi when we discuss this offline.
> >
> > If we move copy_from_user to iommu uapi layer, then there will be
> multiple
> > copy_from_user calls for the same data when a VFIO container has
> multiple domains,
> > devices. For bind, it might be OK. But might be additional overhead for TLB
> flush
> > request from the guest.
> 
> I think it is the same with bind and TLB flush path. will be multiple
> copy_from_user.

multiple copies is possibly fine. In reality we allow only one group per
nesting container (as described in patch [03/15]), and usually there
is just one SVA-capable device per group.

> 
> BTW. for moving data copy to iommy layer, there is another point which
> need to consider. VFIO needs to do unbind in bind path if bind failed,
> so it will assemble unbind_data and pass to iommu layer. If iommu layer
> do the copy_from_user, I think it will be failed. any idea?
> 

This might be mitigated if we go back to use the same bind_data for both
bind/unbind. Then you can reuse the user object for unwinding.

However there is another case where VFIO may need to assemble the
bind_data itself. When a VM is killed, VFIO needs to walk allocated PASIDs
and unbind them one-by-one. In such case copy_from_user doesn't work
since the data is created by kernel. Alex, do you have a suggestion how this
usage can be supported? e.g. asking IOMMU driver to provide two sets of
APIs to handle user/kernel generated requests?

Thanks
Kevin
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


RE: [PATCH v2 1/3] docs: IOMMU user API

2020-06-17 Thread Liu, Yi L
> From: Jacob Pan 
> Sent: Tuesday, June 16, 2020 11:22 PM
> 
> On Thu, 11 Jun 2020 17:27:27 -0700
> Jacob Pan  wrote:
> 
> > >
> > > But then I thought it even better if VFIO leaves the entire
> > > copy_from_user() to the layer consuming it.
> > >
> > OK. Sounds good, that was what Kevin suggested also. I just wasn't
> > sure how much VFIO wants to inspect, I thought VFIO layer wanted to do
> > a sanity check.
> >
> > Anyway, I will move copy_from_user to iommu uapi layer.
> 
> Just one more point brought up by Yi when we discuss this offline.
> 
> If we move copy_from_user to iommu uapi layer, then there will be multiple
> copy_from_user calls for the same data when a VFIO container has multiple 
> domains,
> devices. For bind, it might be OK. But might be additional overhead for TLB 
> flush
> request from the guest.

I think it is the same with bind and TLB flush path. will be multiple
copy_from_user.

BTW. for moving data copy to iommy layer, there is another point which
need to consider. VFIO needs to do unbind in bind path if bind failed,
so it will assemble unbind_data and pass to iommu layer. If iommu layer
do the copy_from_user, I think it will be failed. any idea?

Regards,
Yi Liu

> Thoughts?
> 
> Jacob
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 1/3] docs: IOMMU user API

2020-06-16 Thread Jacob Pan
On Thu, 11 Jun 2020 17:27:27 -0700
Jacob Pan  wrote:

> > 
> > But then I thought it even better if VFIO leaves the entire
> > copy_from_user() to the layer consuming it.
> >   
> OK. Sounds good, that was what Kevin suggested also. I just wasn't
> sure how much VFIO wants to inspect, I thought VFIO layer wanted to
> do a sanity check.
> 
> Anyway, I will move copy_from_user to iommu uapi layer.

Just one more point brought up by Yi when we discuss this offline.

If we move copy_from_user to iommu uapi layer, then there will be
multiple copy_from_user calls for the same data when a VFIO container
has multiple domains, devices. For bind, it might be OK. But might be
additional overhead for TLB flush request from the guest.

Thoughts?

Jacob
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 1/3] docs: IOMMU user API

2020-06-12 Thread Jacob Pan
Hi Jon,

On Thu, 11 Jun 2020 10:30:32 +0100
Jonathan Cameron  wrote:

> On Wed, 10 Jun 2020 21:12:13 -0700
> Jacob Pan  wrote:
> 
> > IOMMU UAPI is newly introduced to support communications between
> > guest virtual IOMMU and host IOMMU. There has been lots of
> > discussions on how it should work with VFIO UAPI and userspace in
> > general.
> > 
> > This document is indended to clarify the UAPI design and usage. The
> > mechenics of how future extensions should be achieved are also
> > covered  
> 
> mechanics 
> 
will fix,

> > in this documentation.
> > 
> > Signed-off-by: Liu Yi L 
> > Signed-off-by: Jacob Pan   
> Mostly seems sensible.  A few comments / queries inline.
> 
> Jonathan
> 
> > ---
> >  Documentation/userspace-api/iommu.rst | 210
> > ++ 1 file changed, 210 insertions(+)
> >  create mode 100644 Documentation/userspace-api/iommu.rst
> > 
> > diff --git a/Documentation/userspace-api/iommu.rst
> > b/Documentation/userspace-api/iommu.rst new file mode 100644
> > index ..e95dc5a04a41
> > --- /dev/null
> > +++ b/Documentation/userspace-api/iommu.rst
> > @@ -0,0 +1,210 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +.. iommu:
> > +
> > +=
> > +IOMMU Userspace API
> > +=
> > +
> > +IOMMU UAPI is used for virtualization cases where communications
> > are +needed between physical and virtual IOMMU drivers. For native
> > +usage, IOMMU is a system device which does not need to communicate
> > +with user space directly.
> > +
> > +The primary use cases are guest Shared Virtual Address (SVA) and
> > +guest IO virtual address (IOVA), wherein virtual IOMMU (vIOMMU)
> > is  
> 
> wherein _a_ virtual IOMMU 
right,

> 
> > +required to communicate with the physical IOMMU in the host.
> > +
> > +.. contents:: :local:
> > +
> > +Functionalities
> > +
> > +Communications of user and kernel involve both directions. The
> > +supported user-kernel APIs are as follows:
> > +
> > +1. Alloc/Free PASID
> > +2. Bind/unbind guest PASID (e.g. Intel VT-d)
> > +3. Bind/unbind guest PASID table (e.g. ARM sMMU)
> > +4. Invalidate IOMMU caches
> > +5. Service page request
> > +
> > +Requirements
> > +
> > +The IOMMU UAPIs are generic and extensible to meet the following
> > +requirements:
> > +
> > +1. Emulated and para-virtualised vIOMMUs
> > +2. Multiple vendors (Intel VT-d, ARM sMMU, etc.)
> > +3. Extensions to the UAPI shall not break existing user space
> > +
> > +Interfaces
> > +
> > +Although the data structures defined in IOMMU UAPI are
> > self-contained, +there is no user API functions introduced.
> > Instead, IOMMU UAPI is +designed to work with existing user driver
> > frameworks such as VFIO. +
> > +Extension Rules & Precautions
> > +-
> > +When IOMMU UAPI gets extended, the data structures can *only* be
> > +modified in two ways:
> > +
> > +1. Adding new fields by re-purposing the padding[] field. No size
> > change. +2. Adding new union members at the end. May increase in
> > size. +
> > +No new fields can be added *after* the variable size union in that
> > it +will break backward compatibility when offset moves. In both
> > cases, a +new flag must be accompanied with a new field such that
> > the IOMMU +driver can process the data based on the new flag.
> > Version field is +only reserved for the unlikely event of UAPI
> > upgrade at its entirety. +
> > +It's *always* the caller's responsibility to indicate the size of
> > the +structure passed by setting argsz appropriately.
> > +
> > +When IOMMU UAPI extension results in size increase, user such as
> > VFIO +has to handle the following scenarios:
> > +
> > +1. User and kernel has exact size match
> > +2. An older user with older kernel header (smaller UAPI size)
> > running on a
> > +   newer kernel (larger UAPI size)
> > +3. A newer user with newer kernel header (larger UAPI size) running
> > +   on a older kernel.
> > +4. A malicious/misbehaving user pass illegal/invalid size but
> > within
> > +   range. The data may contain garbage.
> > +
> > +
> > +Feature Checking
> > +
> > +While launching a guest with vIOMMU, it is important to ensure
> > that host +can support the UAPI data structures to be used for
> > vIOMMU-pIOMMU +communications. Without the upfront compatibility
> > checking, future +faults are difficult to report even in normal
> > conditions. For example, +TLB invalidations should always succeed
> > from vIOMMU's +perspective.   
> 
> This statement has me concerned.  If a TLB invalidation fails, but
> is reported to the guest as successful do we have possible breaking
> of iommu isolation guarantees?
> 
Good point. we should never report success if TLB invalidation fails.
Perhaps reword as:
"For example, TLB invalidations 

Re: [PATCH v2 1/3] docs: IOMMU user API

2020-06-12 Thread Jacob Pan
On Thu, 11 Jun 2020 14:33:08 +0800
Lu Baolu  wrote:

> Hi Jacob,
> 
> On 2020/6/11 12:12, Jacob Pan wrote:
> > IOMMU UAPI is newly introduced to support communications between
> > guest virtual IOMMU and host IOMMU. There has been lots of
> > discussions on how it should work with VFIO UAPI and userspace in
> > general.
> > 
> > This document is indended to clarify the UAPI design and usage. The
> > mechenics of how future extensions should be achieved are also
> > covered in this documentation.
> > 
> > Signed-off-by: Liu Yi L 
> > Signed-off-by: Jacob Pan 
> > ---
> >   Documentation/userspace-api/iommu.rst | 210
> > ++ 1 file changed, 210 insertions(+)
> >   create mode 100644 Documentation/userspace-api/iommu.rst
> > 
> > diff --git a/Documentation/userspace-api/iommu.rst
> > b/Documentation/userspace-api/iommu.rst new file mode 100644
> > index ..e95dc5a04a41
> > --- /dev/null
> > +++ b/Documentation/userspace-api/iommu.rst
> > @@ -0,0 +1,210 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +.. iommu:
> > +
> > +=
> > +IOMMU Userspace API
> > +=
> > +
> > +IOMMU UAPI is used for virtualization cases where communications
> > are +needed between physical and virtual IOMMU drivers. For native
> > +usage, IOMMU is a system device which does not need to communicate
> > +with user space directly.
> > +
> > +The primary use cases are guest Shared Virtual Address (SVA) and
> > +guest IO virtual address (IOVA), wherein virtual IOMMU (vIOMMU) is
> > +required to communicate with the physical IOMMU in the host.
> > +
> > +.. contents:: :local:
> > +
> > +Functionalities
> > +
> > +Communications of user and kernel involve both directions. The
> > +supported user-kernel APIs are as follows:
> > +
> > +1. Alloc/Free PASID
> > +2. Bind/unbind guest PASID (e.g. Intel VT-d)
> > +3. Bind/unbind guest PASID table (e.g. ARM sMMU)
> > +4. Invalidate IOMMU caches
> > +5. Service page request
> > +
> > +Requirements
> > +
> > +The IOMMU UAPIs are generic and extensible to meet the following
> > +requirements:
> > +
> > +1. Emulated and para-virtualised vIOMMUs
> > +2. Multiple vendors (Intel VT-d, ARM sMMU, etc.)
> > +3. Extensions to the UAPI shall not break existing user space
> > +
> > +Interfaces
> > +
> > +Although the data structures defined in IOMMU UAPI are
> > self-contained, +there is no user API functions introduced.
> > Instead, IOMMU UAPI is +designed to work with existing user driver
> > frameworks such as VFIO. +
> > +Extension Rules & Precautions
> > +-
> > +When IOMMU UAPI gets extended, the data structures can *only* be
> > +modified in two ways:
> > +
> > +1. Adding new fields by re-purposing the padding[] field. No size
> > change. +2. Adding new union members at the end. May increase in
> > size. +
> > +No new fields can be added *after* the variable size union in that
> > it +will break backward compatibility when offset moves. In both
> > cases, a +new flag must be accompanied with a new field such that
> > the IOMMU +driver can process the data based on the new flag.
> > Version field is +only reserved for the unlikely event of UAPI
> > upgrade at its entirety. +
> > +It's *always* the caller's responsibility to indicate the size of
> > the +structure passed by setting argsz appropriately.
> > +
> > +When IOMMU UAPI extension results in size increase, user such as
> > VFIO +has to handle the following scenarios:
> > +
> > +1. User and kernel has exact size match
> > +2. An older user with older kernel header (smaller UAPI size)
> > running on a
> > +   newer kernel (larger UAPI size)
> > +3. A newer user with newer kernel header (larger UAPI size) running
> > +   on a older kernel.
> > +4. A malicious/misbehaving user pass illegal/invalid size but
> > within
> > +   range. The data may contain garbage.
> > +
> > +
> > +Feature Checking
> > +
> > +While launching a guest with vIOMMU, it is important to ensure
> > that host +can support the UAPI data structures to be used for
> > vIOMMU-pIOMMU +communications. Without the upfront compatibility
> > checking, future +faults are difficult to report even in normal
> > conditions. For example, +TLB invalidations should always succeed
> > from vIOMMU's +perspective. There is no architectural way to report
> > back to the vIOMMU +if the UAPI data is incompatible. For this
> > reason the following IOMMU +UAPIs cannot fail:
> > +
> > +1. Free PASID
> > +2. Unbind guest PASID
> > +3. Unbind guest PASID table (SMMU)
> > +4. Cache invalidate
> > +5. Page response
> > +
> > +User applications such as QEMU is expected to import kernel UAPI
> > +headers. Only backward compatibility is supported. For example, an
> > +older QEMU (with older kernel header) 

Re: [PATCH v2 1/3] docs: IOMMU user API

2020-06-12 Thread Jacob Pan
On Fri, 12 Jun 2020 07:38:44 +
"Tian, Kevin"  wrote:

> > From: Jacob Pan 
> > Sent: Friday, June 12, 2020 8:27 AM
> > 
> > On Thu, 11 Jun 2020 14:40:47 -0600
> > Alex Williamson  wrote:
> >   
> > > On Thu, 11 Jun 2020 12:52:05 -0700
> > > Jacob Pan  wrote:
> > >  
> > > > Hi Alex,
> > > >
> > > > On Thu, 11 Jun 2020 09:47:41 -0600
> > > > Alex Williamson  wrote:
> > > >  
> > > > > On Wed, 10 Jun 2020 21:12:13 -0700
> > > > > Jacob Pan  wrote:
> > > > >  
> > > > > > IOMMU UAPI is newly introduced to support communications  
> > between  
> > > > > > guest virtual IOMMU and host IOMMU. There has been lots of
> > > > > > discussions on how it should work with VFIO UAPI and
> > > > > > userspace in general.
> > > > > >
> > > > > > This document is indended to clarify the UAPI design and
> > > > > > usage. The mechenics of how future extensions should be
> > > > > > achieved are also covered in this documentation.
> > > > > >
> > > > > > Signed-off-by: Liu Yi L 
> > > > > > Signed-off-by: Jacob Pan 
> > > > > > ---
> > > > > >  Documentation/userspace-api/iommu.rst | 210
> > > > > > ++ 1 file changed, 210
> > > > > > insertions(+) create mode 100644
> > > > > > Documentation/userspace-api/iommu.rst
> > > > > >
> > > > > > diff --git a/Documentation/userspace-api/iommu.rst
> > > > > > b/Documentation/userspace-api/iommu.rst new file mode 100644
> > > > > > index ..e95dc5a04a41
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/userspace-api/iommu.rst
> > > > > > @@ -0,0 +1,210 @@
> > > > > > +.. SPDX-License-Identifier: GPL-2.0
> > > > > > +.. iommu:
> > > > > > +
> > > > > > +=
> > > > > > +IOMMU Userspace API
> > > > > > +=
> > > > > > +
> > > > > > +IOMMU UAPI is used for virtualization cases where
> > > > > > communications are +needed between physical and virtual
> > > > > > IOMMU drivers. For native +usage, IOMMU is a system device
> > > > > > which does not need to communicate +with user space
> > > > > > directly. +
> > > > > > +The primary use cases are guest Shared Virtual Address
> > > > > > (SVA) and +guest IO virtual address (IOVA), wherein virtual
> > > > > > IOMMU (vIOMMU) is +required to communicate with the
> > > > > > physical IOMMU in the host. +
> > > > > > +.. contents:: :local:
> > > > > > +
> > > > > > +Functionalities
> > > > > > +
> > > > > > +Communications of user and kernel involve both directions.
> > > > > > The +supported user-kernel APIs are as follows:
> > > > > > +
> > > > > > +1. Alloc/Free PASID
> > > > > > +2. Bind/unbind guest PASID (e.g. Intel VT-d)
> > > > > > +3. Bind/unbind guest PASID table (e.g. ARM sMMU)
> > > > > > +4. Invalidate IOMMU caches
> > > > > > +5. Service page request
> > > > > > +
> > > > > > +Requirements
> > > > > > +
> > > > > > +The IOMMU UAPIs are generic and extensible to meet the
> > > > > > following +requirements:
> > > > > > +
> > > > > > +1. Emulated and para-virtualised vIOMMUs
> > > > > > +2. Multiple vendors (Intel VT-d, ARM sMMU, etc.)
> > > > > > +3. Extensions to the UAPI shall not break existing user
> > > > > > space +
> > > > > > +Interfaces
> > > > > > +
> > > > > > +Although the data structures defined in IOMMU UAPI are
> > > > > > self-contained, +there is no user API functions introduced.
> > > > > > Instead, IOMMU UAPI is +designed to work with existing user
> > > > > > driver frameworks such as VFIO. +
> > > > > > +Extension Rules & Precautions
> > > > > > +-
> > > > > > +When IOMMU UAPI gets extended, the data structures can
> > > > > > *only* be +modified in two ways:
> > > > > > +
> > > > > > +1. Adding new fields by re-purposing the padding[] field.
> > > > > > No size change. +2. Adding new union members at the end. May
> > > > > > increase in size. +
> > > > > > +No new fields can be added *after* the variable size union
> > > > > > in that it +will break backward compatibility when offset
> > > > > > moves. In both cases, a +new flag must be accompanied with
> > > > > > a new field such that the IOMMU +driver can process the
> > > > > > data based on the new flag. Version field is +only reserved
> > > > > > for the unlikely event of UAPI upgrade at its entirety. +
> > > > > > +It's *always* the caller's responsibility to indicate the
> > > > > > size of the +structure passed by setting argsz
> > > > > > appropriately. +
> > > > > > +When IOMMU UAPI extension results in size increase, user
> > > > > > such as VFIO +has to handle the following scenarios:
> > > > > > +
> > > > > > +1. User and kernel has exact size match
> > > > > > +2. An older user with older kernel header (smaller UAPI
> > > > > > size) running on a
> > > > > > +   newer kernel (larger UAPI size)
> > > > > > +3. A newer user with newer kernel 

RE: [PATCH v2 1/3] docs: IOMMU user API

2020-06-12 Thread Tian, Kevin
> From: Jacob Pan 
> Sent: Friday, June 12, 2020 8:27 AM
> 
> On Thu, 11 Jun 2020 14:40:47 -0600
> Alex Williamson  wrote:
> 
> > On Thu, 11 Jun 2020 12:52:05 -0700
> > Jacob Pan  wrote:
> >
> > > Hi Alex,
> > >
> > > On Thu, 11 Jun 2020 09:47:41 -0600
> > > Alex Williamson  wrote:
> > >
> > > > On Wed, 10 Jun 2020 21:12:13 -0700
> > > > Jacob Pan  wrote:
> > > >
> > > > > IOMMU UAPI is newly introduced to support communications
> between
> > > > > guest virtual IOMMU and host IOMMU. There has been lots of
> > > > > discussions on how it should work with VFIO UAPI and userspace
> > > > > in general.
> > > > >
> > > > > This document is indended to clarify the UAPI design and usage.
> > > > > The mechenics of how future extensions should be achieved are
> > > > > also covered in this documentation.
> > > > >
> > > > > Signed-off-by: Liu Yi L 
> > > > > Signed-off-by: Jacob Pan 
> > > > > ---
> > > > >  Documentation/userspace-api/iommu.rst | 210
> > > > > ++ 1 file changed, 210
> > > > > insertions(+) create mode 100644
> > > > > Documentation/userspace-api/iommu.rst
> > > > >
> > > > > diff --git a/Documentation/userspace-api/iommu.rst
> > > > > b/Documentation/userspace-api/iommu.rst new file mode 100644
> > > > > index ..e95dc5a04a41
> > > > > --- /dev/null
> > > > > +++ b/Documentation/userspace-api/iommu.rst
> > > > > @@ -0,0 +1,210 @@
> > > > > +.. SPDX-License-Identifier: GPL-2.0
> > > > > +.. iommu:
> > > > > +
> > > > > +=
> > > > > +IOMMU Userspace API
> > > > > +=
> > > > > +
> > > > > +IOMMU UAPI is used for virtualization cases where
> > > > > communications are +needed between physical and virtual IOMMU
> > > > > drivers. For native +usage, IOMMU is a system device which does
> > > > > not need to communicate +with user space directly.
> > > > > +
> > > > > +The primary use cases are guest Shared Virtual Address (SVA)
> > > > > and +guest IO virtual address (IOVA), wherein virtual IOMMU
> > > > > (vIOMMU) is +required to communicate with the physical IOMMU in
> > > > > the host. +
> > > > > +.. contents:: :local:
> > > > > +
> > > > > +Functionalities
> > > > > +
> > > > > +Communications of user and kernel involve both directions. The
> > > > > +supported user-kernel APIs are as follows:
> > > > > +
> > > > > +1. Alloc/Free PASID
> > > > > +2. Bind/unbind guest PASID (e.g. Intel VT-d)
> > > > > +3. Bind/unbind guest PASID table (e.g. ARM sMMU)
> > > > > +4. Invalidate IOMMU caches
> > > > > +5. Service page request
> > > > > +
> > > > > +Requirements
> > > > > +
> > > > > +The IOMMU UAPIs are generic and extensible to meet the
> > > > > following +requirements:
> > > > > +
> > > > > +1. Emulated and para-virtualised vIOMMUs
> > > > > +2. Multiple vendors (Intel VT-d, ARM sMMU, etc.)
> > > > > +3. Extensions to the UAPI shall not break existing user space
> > > > > +
> > > > > +Interfaces
> > > > > +
> > > > > +Although the data structures defined in IOMMU UAPI are
> > > > > self-contained, +there is no user API functions introduced.
> > > > > Instead, IOMMU UAPI is +designed to work with existing user
> > > > > driver frameworks such as VFIO. +
> > > > > +Extension Rules & Precautions
> > > > > +-
> > > > > +When IOMMU UAPI gets extended, the data structures can *only*
> > > > > be +modified in two ways:
> > > > > +
> > > > > +1. Adding new fields by re-purposing the padding[] field. No
> > > > > size change. +2. Adding new union members at the end. May
> > > > > increase in size. +
> > > > > +No new fields can be added *after* the variable size union in
> > > > > that it +will break backward compatibility when offset moves.
> > > > > In both cases, a +new flag must be accompanied with a new field
> > > > > such that the IOMMU +driver can process the data based on the
> > > > > new flag. Version field is +only reserved for the unlikely
> > > > > event of UAPI upgrade at its entirety. +
> > > > > +It's *always* the caller's responsibility to indicate the size
> > > > > of the +structure passed by setting argsz appropriately.
> > > > > +
> > > > > +When IOMMU UAPI extension results in size increase, user such
> > > > > as VFIO +has to handle the following scenarios:
> > > > > +
> > > > > +1. User and kernel has exact size match
> > > > > +2. An older user with older kernel header (smaller UAPI size)
> > > > > running on a
> > > > > +   newer kernel (larger UAPI size)
> > > > > +3. A newer user with newer kernel header (larger UAPI size)
> > > > > running
> > > > > +   on a older kernel.
> > > > > +4. A malicious/misbehaving user pass illegal/invalid size but
> > > > > within
> > > > > +   range. The data may contain garbage.
> > > > > +
> > > > > +
> > > > > +Feature Checking
> > > > > 

Re: [PATCH v2 1/3] docs: IOMMU user API

2020-06-11 Thread Jacob Pan
On Thu, 11 Jun 2020 14:40:47 -0600
Alex Williamson  wrote:

> On Thu, 11 Jun 2020 12:52:05 -0700
> Jacob Pan  wrote:
> 
> > Hi Alex,
> > 
> > On Thu, 11 Jun 2020 09:47:41 -0600
> > Alex Williamson  wrote:
> >   
> > > On Wed, 10 Jun 2020 21:12:13 -0700
> > > Jacob Pan  wrote:
> > > 
> > > > IOMMU UAPI is newly introduced to support communications between
> > > > guest virtual IOMMU and host IOMMU. There has been lots of
> > > > discussions on how it should work with VFIO UAPI and userspace
> > > > in general.
> > > > 
> > > > This document is indended to clarify the UAPI design and usage.
> > > > The mechenics of how future extensions should be achieved are
> > > > also covered in this documentation.
> > > > 
> > > > Signed-off-by: Liu Yi L 
> > > > Signed-off-by: Jacob Pan 
> > > > ---
> > > >  Documentation/userspace-api/iommu.rst | 210
> > > > ++ 1 file changed, 210
> > > > insertions(+) create mode 100644
> > > > Documentation/userspace-api/iommu.rst
> > > > 
> > > > diff --git a/Documentation/userspace-api/iommu.rst
> > > > b/Documentation/userspace-api/iommu.rst new file mode 100644
> > > > index ..e95dc5a04a41
> > > > --- /dev/null
> > > > +++ b/Documentation/userspace-api/iommu.rst
> > > > @@ -0,0 +1,210 @@
> > > > +.. SPDX-License-Identifier: GPL-2.0
> > > > +.. iommu:
> > > > +
> > > > +=
> > > > +IOMMU Userspace API
> > > > +=
> > > > +
> > > > +IOMMU UAPI is used for virtualization cases where
> > > > communications are +needed between physical and virtual IOMMU
> > > > drivers. For native +usage, IOMMU is a system device which does
> > > > not need to communicate +with user space directly.
> > > > +
> > > > +The primary use cases are guest Shared Virtual Address (SVA)
> > > > and +guest IO virtual address (IOVA), wherein virtual IOMMU
> > > > (vIOMMU) is +required to communicate with the physical IOMMU in
> > > > the host. +
> > > > +.. contents:: :local:
> > > > +
> > > > +Functionalities
> > > > +
> > > > +Communications of user and kernel involve both directions. The
> > > > +supported user-kernel APIs are as follows:
> > > > +
> > > > +1. Alloc/Free PASID
> > > > +2. Bind/unbind guest PASID (e.g. Intel VT-d)
> > > > +3. Bind/unbind guest PASID table (e.g. ARM sMMU)
> > > > +4. Invalidate IOMMU caches
> > > > +5. Service page request
> > > > +
> > > > +Requirements
> > > > +
> > > > +The IOMMU UAPIs are generic and extensible to meet the
> > > > following +requirements:
> > > > +
> > > > +1. Emulated and para-virtualised vIOMMUs
> > > > +2. Multiple vendors (Intel VT-d, ARM sMMU, etc.)
> > > > +3. Extensions to the UAPI shall not break existing user space
> > > > +
> > > > +Interfaces
> > > > +
> > > > +Although the data structures defined in IOMMU UAPI are
> > > > self-contained, +there is no user API functions introduced.
> > > > Instead, IOMMU UAPI is +designed to work with existing user
> > > > driver frameworks such as VFIO. +
> > > > +Extension Rules & Precautions
> > > > +-
> > > > +When IOMMU UAPI gets extended, the data structures can *only*
> > > > be +modified in two ways:
> > > > +
> > > > +1. Adding new fields by re-purposing the padding[] field. No
> > > > size change. +2. Adding new union members at the end. May
> > > > increase in size. +
> > > > +No new fields can be added *after* the variable size union in
> > > > that it +will break backward compatibility when offset moves.
> > > > In both cases, a +new flag must be accompanied with a new field
> > > > such that the IOMMU +driver can process the data based on the
> > > > new flag. Version field is +only reserved for the unlikely
> > > > event of UAPI upgrade at its entirety. +
> > > > +It's *always* the caller's responsibility to indicate the size
> > > > of the +structure passed by setting argsz appropriately.
> > > > +
> > > > +When IOMMU UAPI extension results in size increase, user such
> > > > as VFIO +has to handle the following scenarios:
> > > > +
> > > > +1. User and kernel has exact size match
> > > > +2. An older user with older kernel header (smaller UAPI size)
> > > > running on a
> > > > +   newer kernel (larger UAPI size)
> > > > +3. A newer user with newer kernel header (larger UAPI size)
> > > > running
> > > > +   on a older kernel.
> > > > +4. A malicious/misbehaving user pass illegal/invalid size but
> > > > within
> > > > +   range. The data may contain garbage.
> > > > +
> > > > +
> > > > +Feature Checking
> > > > +
> > > > +While launching a guest with vIOMMU, it is important to ensure
> > > > that host +can support the UAPI data structures to be used for
> > > > vIOMMU-pIOMMU +communications. Without the upfront compatibility
> > > > checking, future +faults are difficult to report 

Re: [PATCH v2 1/3] docs: IOMMU user API

2020-06-11 Thread Alex Williamson
On Thu, 11 Jun 2020 12:52:05 -0700
Jacob Pan  wrote:

> Hi Alex,
> 
> On Thu, 11 Jun 2020 09:47:41 -0600
> Alex Williamson  wrote:
> 
> > On Wed, 10 Jun 2020 21:12:13 -0700
> > Jacob Pan  wrote:
> >   
> > > IOMMU UAPI is newly introduced to support communications between
> > > guest virtual IOMMU and host IOMMU. There has been lots of
> > > discussions on how it should work with VFIO UAPI and userspace in
> > > general.
> > > 
> > > This document is indended to clarify the UAPI design and usage. The
> > > mechenics of how future extensions should be achieved are also
> > > covered in this documentation.
> > > 
> > > Signed-off-by: Liu Yi L 
> > > Signed-off-by: Jacob Pan 
> > > ---
> > >  Documentation/userspace-api/iommu.rst | 210
> > > ++ 1 file changed, 210 insertions(+)
> > >  create mode 100644 Documentation/userspace-api/iommu.rst
> > > 
> > > diff --git a/Documentation/userspace-api/iommu.rst
> > > b/Documentation/userspace-api/iommu.rst new file mode 100644
> > > index ..e95dc5a04a41
> > > --- /dev/null
> > > +++ b/Documentation/userspace-api/iommu.rst
> > > @@ -0,0 +1,210 @@
> > > +.. SPDX-License-Identifier: GPL-2.0
> > > +.. iommu:
> > > +
> > > +=
> > > +IOMMU Userspace API
> > > +=
> > > +
> > > +IOMMU UAPI is used for virtualization cases where communications
> > > are +needed between physical and virtual IOMMU drivers. For native
> > > +usage, IOMMU is a system device which does not need to communicate
> > > +with user space directly.
> > > +
> > > +The primary use cases are guest Shared Virtual Address (SVA) and
> > > +guest IO virtual address (IOVA), wherein virtual IOMMU (vIOMMU) is
> > > +required to communicate with the physical IOMMU in the host.
> > > +
> > > +.. contents:: :local:
> > > +
> > > +Functionalities
> > > +
> > > +Communications of user and kernel involve both directions. The
> > > +supported user-kernel APIs are as follows:
> > > +
> > > +1. Alloc/Free PASID
> > > +2. Bind/unbind guest PASID (e.g. Intel VT-d)
> > > +3. Bind/unbind guest PASID table (e.g. ARM sMMU)
> > > +4. Invalidate IOMMU caches
> > > +5. Service page request
> > > +
> > > +Requirements
> > > +
> > > +The IOMMU UAPIs are generic and extensible to meet the following
> > > +requirements:
> > > +
> > > +1. Emulated and para-virtualised vIOMMUs
> > > +2. Multiple vendors (Intel VT-d, ARM sMMU, etc.)
> > > +3. Extensions to the UAPI shall not break existing user space
> > > +
> > > +Interfaces
> > > +
> > > +Although the data structures defined in IOMMU UAPI are
> > > self-contained, +there is no user API functions introduced.
> > > Instead, IOMMU UAPI is +designed to work with existing user driver
> > > frameworks such as VFIO. +
> > > +Extension Rules & Precautions
> > > +-
> > > +When IOMMU UAPI gets extended, the data structures can *only* be
> > > +modified in two ways:
> > > +
> > > +1. Adding new fields by re-purposing the padding[] field. No size
> > > change. +2. Adding new union members at the end. May increase in
> > > size. +
> > > +No new fields can be added *after* the variable size union in that
> > > it +will break backward compatibility when offset moves. In both
> > > cases, a +new flag must be accompanied with a new field such that
> > > the IOMMU +driver can process the data based on the new flag.
> > > Version field is +only reserved for the unlikely event of UAPI
> > > upgrade at its entirety. +
> > > +It's *always* the caller's responsibility to indicate the size of
> > > the +structure passed by setting argsz appropriately.
> > > +
> > > +When IOMMU UAPI extension results in size increase, user such as
> > > VFIO +has to handle the following scenarios:
> > > +
> > > +1. User and kernel has exact size match
> > > +2. An older user with older kernel header (smaller UAPI size)
> > > running on a
> > > +   newer kernel (larger UAPI size)
> > > +3. A newer user with newer kernel header (larger UAPI size) running
> > > +   on a older kernel.
> > > +4. A malicious/misbehaving user pass illegal/invalid size but
> > > within
> > > +   range. The data may contain garbage.
> > > +
> > > +
> > > +Feature Checking
> > > +
> > > +While launching a guest with vIOMMU, it is important to ensure
> > > that host +can support the UAPI data structures to be used for
> > > vIOMMU-pIOMMU +communications. Without the upfront compatibility
> > > checking, future +faults are difficult to report even in normal
> > > conditions. For example, +TLB invalidations should always succeed
> > > from vIOMMU's +perspective. There is no architectural way to report
> > > back to the vIOMMU +if the UAPI data is incompatible. For this
> > > reason the following IOMMU +UAPIs cannot fail:
> > > +
> > > +1. Free 

Re: [PATCH v2 1/3] docs: IOMMU user API

2020-06-11 Thread Jacob Pan
Hi Alex,

On Thu, 11 Jun 2020 09:47:41 -0600
Alex Williamson  wrote:

> On Wed, 10 Jun 2020 21:12:13 -0700
> Jacob Pan  wrote:
> 
> > IOMMU UAPI is newly introduced to support communications between
> > guest virtual IOMMU and host IOMMU. There has been lots of
> > discussions on how it should work with VFIO UAPI and userspace in
> > general.
> > 
> > This document is indended to clarify the UAPI design and usage. The
> > mechenics of how future extensions should be achieved are also
> > covered in this documentation.
> > 
> > Signed-off-by: Liu Yi L 
> > Signed-off-by: Jacob Pan 
> > ---
> >  Documentation/userspace-api/iommu.rst | 210
> > ++ 1 file changed, 210 insertions(+)
> >  create mode 100644 Documentation/userspace-api/iommu.rst
> > 
> > diff --git a/Documentation/userspace-api/iommu.rst
> > b/Documentation/userspace-api/iommu.rst new file mode 100644
> > index ..e95dc5a04a41
> > --- /dev/null
> > +++ b/Documentation/userspace-api/iommu.rst
> > @@ -0,0 +1,210 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +.. iommu:
> > +
> > +=
> > +IOMMU Userspace API
> > +=
> > +
> > +IOMMU UAPI is used for virtualization cases where communications
> > are +needed between physical and virtual IOMMU drivers. For native
> > +usage, IOMMU is a system device which does not need to communicate
> > +with user space directly.
> > +
> > +The primary use cases are guest Shared Virtual Address (SVA) and
> > +guest IO virtual address (IOVA), wherein virtual IOMMU (vIOMMU) is
> > +required to communicate with the physical IOMMU in the host.
> > +
> > +.. contents:: :local:
> > +
> > +Functionalities
> > +
> > +Communications of user and kernel involve both directions. The
> > +supported user-kernel APIs are as follows:
> > +
> > +1. Alloc/Free PASID
> > +2. Bind/unbind guest PASID (e.g. Intel VT-d)
> > +3. Bind/unbind guest PASID table (e.g. ARM sMMU)
> > +4. Invalidate IOMMU caches
> > +5. Service page request
> > +
> > +Requirements
> > +
> > +The IOMMU UAPIs are generic and extensible to meet the following
> > +requirements:
> > +
> > +1. Emulated and para-virtualised vIOMMUs
> > +2. Multiple vendors (Intel VT-d, ARM sMMU, etc.)
> > +3. Extensions to the UAPI shall not break existing user space
> > +
> > +Interfaces
> > +
> > +Although the data structures defined in IOMMU UAPI are
> > self-contained, +there is no user API functions introduced.
> > Instead, IOMMU UAPI is +designed to work with existing user driver
> > frameworks such as VFIO. +
> > +Extension Rules & Precautions
> > +-
> > +When IOMMU UAPI gets extended, the data structures can *only* be
> > +modified in two ways:
> > +
> > +1. Adding new fields by re-purposing the padding[] field. No size
> > change. +2. Adding new union members at the end. May increase in
> > size. +
> > +No new fields can be added *after* the variable size union in that
> > it +will break backward compatibility when offset moves. In both
> > cases, a +new flag must be accompanied with a new field such that
> > the IOMMU +driver can process the data based on the new flag.
> > Version field is +only reserved for the unlikely event of UAPI
> > upgrade at its entirety. +
> > +It's *always* the caller's responsibility to indicate the size of
> > the +structure passed by setting argsz appropriately.
> > +
> > +When IOMMU UAPI extension results in size increase, user such as
> > VFIO +has to handle the following scenarios:
> > +
> > +1. User and kernel has exact size match
> > +2. An older user with older kernel header (smaller UAPI size)
> > running on a
> > +   newer kernel (larger UAPI size)
> > +3. A newer user with newer kernel header (larger UAPI size) running
> > +   on a older kernel.
> > +4. A malicious/misbehaving user pass illegal/invalid size but
> > within
> > +   range. The data may contain garbage.
> > +
> > +
> > +Feature Checking
> > +
> > +While launching a guest with vIOMMU, it is important to ensure
> > that host +can support the UAPI data structures to be used for
> > vIOMMU-pIOMMU +communications. Without the upfront compatibility
> > checking, future +faults are difficult to report even in normal
> > conditions. For example, +TLB invalidations should always succeed
> > from vIOMMU's +perspective. There is no architectural way to report
> > back to the vIOMMU +if the UAPI data is incompatible. For this
> > reason the following IOMMU +UAPIs cannot fail:
> > +
> > +1. Free PASID
> > +2. Unbind guest PASID
> > +3. Unbind guest PASID table (SMMU)
> > +4. Cache invalidate
> > +5. Page response
> > +
> > +User applications such as QEMU is expected to import kernel UAPI
> > +headers. Only backward compatibility is supported. For example, an
> > +older QEMU (with 

Re: [PATCH v2 1/3] docs: IOMMU user API

2020-06-11 Thread Jacob Pan
Hi Jon,

On Thu, 11 Jun 2020 07:55:00 -0600
Jonathan Corbet  wrote:

> On Wed, 10 Jun 2020 21:12:13 -0700
> Jacob Pan  wrote:
> 
> A little nit but...this pattern:
> 
> > +pattern below:
> > +
> > +::
> > +
> > +   struct {
> > +   __u32 argsz;
> > +   __u32 flags;
> > +   __u8  data[];
> > +  }  
> 
> can be more concisely and attractively written as:
> 
>pattern below::
> 
>   struct { 
> ...
> 
Will do. thanks!

> Thanks,
> 
> jon

[Jacob Pan]
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 1/3] docs: IOMMU user API

2020-06-11 Thread Alex Williamson
On Wed, 10 Jun 2020 21:12:13 -0700
Jacob Pan  wrote:

> IOMMU UAPI is newly introduced to support communications between guest
> virtual IOMMU and host IOMMU. There has been lots of discussions on how
> it should work with VFIO UAPI and userspace in general.
> 
> This document is indended to clarify the UAPI design and usage. The
> mechenics of how future extensions should be achieved are also covered
> in this documentation.
> 
> Signed-off-by: Liu Yi L 
> Signed-off-by: Jacob Pan 
> ---
>  Documentation/userspace-api/iommu.rst | 210 
> ++
>  1 file changed, 210 insertions(+)
>  create mode 100644 Documentation/userspace-api/iommu.rst
> 
> diff --git a/Documentation/userspace-api/iommu.rst 
> b/Documentation/userspace-api/iommu.rst
> new file mode 100644
> index ..e95dc5a04a41
> --- /dev/null
> +++ b/Documentation/userspace-api/iommu.rst
> @@ -0,0 +1,210 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +.. iommu:
> +
> +=
> +IOMMU Userspace API
> +=
> +
> +IOMMU UAPI is used for virtualization cases where communications are
> +needed between physical and virtual IOMMU drivers. For native
> +usage, IOMMU is a system device which does not need to communicate
> +with user space directly.
> +
> +The primary use cases are guest Shared Virtual Address (SVA) and
> +guest IO virtual address (IOVA), wherein virtual IOMMU (vIOMMU) is
> +required to communicate with the physical IOMMU in the host.
> +
> +.. contents:: :local:
> +
> +Functionalities
> +
> +Communications of user and kernel involve both directions. The
> +supported user-kernel APIs are as follows:
> +
> +1. Alloc/Free PASID
> +2. Bind/unbind guest PASID (e.g. Intel VT-d)
> +3. Bind/unbind guest PASID table (e.g. ARM sMMU)
> +4. Invalidate IOMMU caches
> +5. Service page request
> +
> +Requirements
> +
> +The IOMMU UAPIs are generic and extensible to meet the following
> +requirements:
> +
> +1. Emulated and para-virtualised vIOMMUs
> +2. Multiple vendors (Intel VT-d, ARM sMMU, etc.)
> +3. Extensions to the UAPI shall not break existing user space
> +
> +Interfaces
> +
> +Although the data structures defined in IOMMU UAPI are self-contained,
> +there is no user API functions introduced. Instead, IOMMU UAPI is
> +designed to work with existing user driver frameworks such as VFIO.
> +
> +Extension Rules & Precautions
> +-
> +When IOMMU UAPI gets extended, the data structures can *only* be
> +modified in two ways:
> +
> +1. Adding new fields by re-purposing the padding[] field. No size change.
> +2. Adding new union members at the end. May increase in size.
> +
> +No new fields can be added *after* the variable size union in that it
> +will break backward compatibility when offset moves. In both cases, a
> +new flag must be accompanied with a new field such that the IOMMU
> +driver can process the data based on the new flag. Version field is
> +only reserved for the unlikely event of UAPI upgrade at its entirety.
> +
> +It's *always* the caller's responsibility to indicate the size of the
> +structure passed by setting argsz appropriately.
> +
> +When IOMMU UAPI extension results in size increase, user such as VFIO
> +has to handle the following scenarios:
> +
> +1. User and kernel has exact size match
> +2. An older user with older kernel header (smaller UAPI size) running on a
> +   newer kernel (larger UAPI size)
> +3. A newer user with newer kernel header (larger UAPI size) running
> +   on a older kernel.
> +4. A malicious/misbehaving user pass illegal/invalid size but within
> +   range. The data may contain garbage.
> +
> +
> +Feature Checking
> +
> +While launching a guest with vIOMMU, it is important to ensure that host
> +can support the UAPI data structures to be used for vIOMMU-pIOMMU
> +communications. Without the upfront compatibility checking, future
> +faults are difficult to report even in normal conditions. For example,
> +TLB invalidations should always succeed from vIOMMU's
> +perspective. There is no architectural way to report back to the vIOMMU
> +if the UAPI data is incompatible. For this reason the following IOMMU
> +UAPIs cannot fail:
> +
> +1. Free PASID
> +2. Unbind guest PASID
> +3. Unbind guest PASID table (SMMU)
> +4. Cache invalidate
> +5. Page response
> +
> +User applications such as QEMU is expected to import kernel UAPI
> +headers. Only backward compatibility is supported. For example, an
> +older QEMU (with older kernel header) can run on newer kernel. Newer
> +QEMU (with new kernel header) may fail on older kernel.

"Build your user application against newer kernels and it may break on
older kernels" is not a great selling point of this UAPI.  Clearly new
features may not be available on older kernels and an 

Re: [PATCH v2 1/3] docs: IOMMU user API

2020-06-11 Thread Jonathan Corbet
On Wed, 10 Jun 2020 21:12:13 -0700
Jacob Pan  wrote:

A little nit but...this pattern:

> +pattern below:
> +
> +::
> +
> +   struct {
> + __u32 argsz;
> + __u32 flags;
> + __u8  data[];
> +  }

can be more concisely and attractively written as:

   pattern below::

struct { 
...

Thanks,

jon
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 1/3] docs: IOMMU user API

2020-06-11 Thread Jonathan Cameron
On Wed, 10 Jun 2020 21:12:13 -0700
Jacob Pan  wrote:

> IOMMU UAPI is newly introduced to support communications between guest
> virtual IOMMU and host IOMMU. There has been lots of discussions on how
> it should work with VFIO UAPI and userspace in general.
> 
> This document is indended to clarify the UAPI design and usage. The
> mechenics of how future extensions should be achieved are also covered

mechanics 

> in this documentation.
> 
> Signed-off-by: Liu Yi L 
> Signed-off-by: Jacob Pan 
Mostly seems sensible.  A few comments / queries inline.

Jonathan

> ---
>  Documentation/userspace-api/iommu.rst | 210 
> ++
>  1 file changed, 210 insertions(+)
>  create mode 100644 Documentation/userspace-api/iommu.rst
> 
> diff --git a/Documentation/userspace-api/iommu.rst 
> b/Documentation/userspace-api/iommu.rst
> new file mode 100644
> index ..e95dc5a04a41
> --- /dev/null
> +++ b/Documentation/userspace-api/iommu.rst
> @@ -0,0 +1,210 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +.. iommu:
> +
> +=
> +IOMMU Userspace API
> +=
> +
> +IOMMU UAPI is used for virtualization cases where communications are
> +needed between physical and virtual IOMMU drivers. For native
> +usage, IOMMU is a system device which does not need to communicate
> +with user space directly.
> +
> +The primary use cases are guest Shared Virtual Address (SVA) and
> +guest IO virtual address (IOVA), wherein virtual IOMMU (vIOMMU) is

wherein _a_ virtual IOMMU 

> +required to communicate with the physical IOMMU in the host.
> +
> +.. contents:: :local:
> +
> +Functionalities
> +
> +Communications of user and kernel involve both directions. The
> +supported user-kernel APIs are as follows:
> +
> +1. Alloc/Free PASID
> +2. Bind/unbind guest PASID (e.g. Intel VT-d)
> +3. Bind/unbind guest PASID table (e.g. ARM sMMU)
> +4. Invalidate IOMMU caches
> +5. Service page request
> +
> +Requirements
> +
> +The IOMMU UAPIs are generic and extensible to meet the following
> +requirements:
> +
> +1. Emulated and para-virtualised vIOMMUs
> +2. Multiple vendors (Intel VT-d, ARM sMMU, etc.)
> +3. Extensions to the UAPI shall not break existing user space
> +
> +Interfaces
> +
> +Although the data structures defined in IOMMU UAPI are self-contained,
> +there is no user API functions introduced. Instead, IOMMU UAPI is
> +designed to work with existing user driver frameworks such as VFIO.
> +
> +Extension Rules & Precautions
> +-
> +When IOMMU UAPI gets extended, the data structures can *only* be
> +modified in two ways:
> +
> +1. Adding new fields by re-purposing the padding[] field. No size change.
> +2. Adding new union members at the end. May increase in size.
> +
> +No new fields can be added *after* the variable size union in that it
> +will break backward compatibility when offset moves. In both cases, a
> +new flag must be accompanied with a new field such that the IOMMU
> +driver can process the data based on the new flag. Version field is
> +only reserved for the unlikely event of UAPI upgrade at its entirety.
> +
> +It's *always* the caller's responsibility to indicate the size of the
> +structure passed by setting argsz appropriately.
> +
> +When IOMMU UAPI extension results in size increase, user such as VFIO
> +has to handle the following scenarios:
> +
> +1. User and kernel has exact size match
> +2. An older user with older kernel header (smaller UAPI size) running on a
> +   newer kernel (larger UAPI size)
> +3. A newer user with newer kernel header (larger UAPI size) running
> +   on a older kernel.
> +4. A malicious/misbehaving user pass illegal/invalid size but within
> +   range. The data may contain garbage.
> +
> +
> +Feature Checking
> +
> +While launching a guest with vIOMMU, it is important to ensure that host
> +can support the UAPI data structures to be used for vIOMMU-pIOMMU
> +communications. Without the upfront compatibility checking, future
> +faults are difficult to report even in normal conditions. For example,
> +TLB invalidations should always succeed from vIOMMU's
> +perspective. 

This statement has me concerned.  If a TLB invalidation fails, but
is reported to the guest as successful do we have possible breaking of iommu
isolation guarantees?

If you get a TLB invalidation not happening, for some reason, that's a critical
fault, isolate the device using the IOMMU or kill the VM.

I'd reword it as "TLB invalidations should always succeed."

As you mention, we should never get to this state anyway!

> There is no architectural way to report back to the vIOMMU
> +if the UAPI data is incompatible. For this reason the following IOMMU
> +UAPIs cannot fail:
> +
> +1. Free PASID
> +2. Unbind guest PASID
> +3. Unbind 

Re: [PATCH v2 1/3] docs: IOMMU user API

2020-06-11 Thread Lu Baolu

Hi Jacob,

On 2020/6/11 12:12, Jacob Pan wrote:

IOMMU UAPI is newly introduced to support communications between guest
virtual IOMMU and host IOMMU. There has been lots of discussions on how
it should work with VFIO UAPI and userspace in general.

This document is indended to clarify the UAPI design and usage. The
mechenics of how future extensions should be achieved are also covered
in this documentation.

Signed-off-by: Liu Yi L 
Signed-off-by: Jacob Pan 
---
  Documentation/userspace-api/iommu.rst | 210 ++
  1 file changed, 210 insertions(+)
  create mode 100644 Documentation/userspace-api/iommu.rst

diff --git a/Documentation/userspace-api/iommu.rst 
b/Documentation/userspace-api/iommu.rst
new file mode 100644
index ..e95dc5a04a41
--- /dev/null
+++ b/Documentation/userspace-api/iommu.rst
@@ -0,0 +1,210 @@
+.. SPDX-License-Identifier: GPL-2.0
+.. iommu:
+
+=
+IOMMU Userspace API
+=
+
+IOMMU UAPI is used for virtualization cases where communications are
+needed between physical and virtual IOMMU drivers. For native
+usage, IOMMU is a system device which does not need to communicate
+with user space directly.
+
+The primary use cases are guest Shared Virtual Address (SVA) and
+guest IO virtual address (IOVA), wherein virtual IOMMU (vIOMMU) is
+required to communicate with the physical IOMMU in the host.
+
+.. contents:: :local:
+
+Functionalities
+
+Communications of user and kernel involve both directions. The
+supported user-kernel APIs are as follows:
+
+1. Alloc/Free PASID
+2. Bind/unbind guest PASID (e.g. Intel VT-d)
+3. Bind/unbind guest PASID table (e.g. ARM sMMU)
+4. Invalidate IOMMU caches
+5. Service page request
+
+Requirements
+
+The IOMMU UAPIs are generic and extensible to meet the following
+requirements:
+
+1. Emulated and para-virtualised vIOMMUs
+2. Multiple vendors (Intel VT-d, ARM sMMU, etc.)
+3. Extensions to the UAPI shall not break existing user space
+
+Interfaces
+
+Although the data structures defined in IOMMU UAPI are self-contained,
+there is no user API functions introduced. Instead, IOMMU UAPI is
+designed to work with existing user driver frameworks such as VFIO.
+
+Extension Rules & Precautions
+-
+When IOMMU UAPI gets extended, the data structures can *only* be
+modified in two ways:
+
+1. Adding new fields by re-purposing the padding[] field. No size change.
+2. Adding new union members at the end. May increase in size.
+
+No new fields can be added *after* the variable size union in that it
+will break backward compatibility when offset moves. In both cases, a
+new flag must be accompanied with a new field such that the IOMMU
+driver can process the data based on the new flag. Version field is
+only reserved for the unlikely event of UAPI upgrade at its entirety.
+
+It's *always* the caller's responsibility to indicate the size of the
+structure passed by setting argsz appropriately.
+
+When IOMMU UAPI extension results in size increase, user such as VFIO
+has to handle the following scenarios:
+
+1. User and kernel has exact size match
+2. An older user with older kernel header (smaller UAPI size) running on a
+   newer kernel (larger UAPI size)
+3. A newer user with newer kernel header (larger UAPI size) running
+   on a older kernel.
+4. A malicious/misbehaving user pass illegal/invalid size but within
+   range. The data may contain garbage.
+
+
+Feature Checking
+
+While launching a guest with vIOMMU, it is important to ensure that host
+can support the UAPI data structures to be used for vIOMMU-pIOMMU
+communications. Without the upfront compatibility checking, future
+faults are difficult to report even in normal conditions. For example,
+TLB invalidations should always succeed from vIOMMU's
+perspective. There is no architectural way to report back to the vIOMMU
+if the UAPI data is incompatible. For this reason the following IOMMU
+UAPIs cannot fail:
+
+1. Free PASID
+2. Unbind guest PASID
+3. Unbind guest PASID table (SMMU)
+4. Cache invalidate
+5. Page response
+
+User applications such as QEMU is expected to import kernel UAPI
+headers. Only backward compatibility is supported. For example, an
+older QEMU (with older kernel header) can run on newer kernel. Newer
+QEMU (with new kernel header) may fail on older kernel.
+
+IOMMU vendor driver should report the below features to IOMMU UAPI
+consumers (e.g. via VFIO).
+
+1. IOMMU_NESTING_FEAT_SYSWIDE_PASID
+2. IOMMU_NESTING_FEAT_BIND_PGTBL
+3. IOMMU_NESTING_FEAT_BIND_PASID_TABLE
+4. IOMMU_NESTING_FEAT_CACHE_INVLD
+5. IOMMU_NESTING_FEAT_PAGE_REQUEST
+
+Take VFIO as example, upon request from VFIO user space (e.g. QEMU),
+VFIO kernel code shall query IOMMU vendor driver for the support of
+the above