Re: [PATCH v9 1/7] docs: IOMMU user API

2020-09-18 Thread Jacob Pan
Hi Randy,

Thanks for the review.

On Wed, 16 Sep 2020 11:26:00 -0700, Randy Dunlap 
wrote:

> On 9/11/20 2:57 PM, 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 intended to clarify the UAPI design and usage. The
> > mechanics of how future extensions should be achieved are also covered
> > in this documentation.
> > 
> > Reviewed-by: Eric Auger 
> > Signed-off-by: Liu Yi L 
> > Signed-off-by: Jacob Pan 
> > ---
> >  Documentation/userspace-api/iommu.rst | 211
> > ++
> > MAINTAINERS   |   1 + 2 files changed, 212
> > insertions(+) create mode 100644 Documentation/userspace-api/iommu.rst  
> 
> Hi,
> I have a few edit changes for you below:
> 
> 
> > diff --git a/Documentation/userspace-api/iommu.rst
> > b/Documentation/userspace-api/iommu.rst new file mode 100644
> > index ..1e68e8f05bb3
> > --- /dev/null
> > +++ b/Documentation/userspace-api/iommu.rst
> > @@ -0,0 +1,211 @@
> > +.. 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 baremetal
> > +usage, the IOMMU is a system device which does not need to communicate
> > +with user space directly.  
> 
> userspace
> 
> for consistency
> 
got it

> > +
> > +The primary use cases are guest Shared Virtual Address (SVA) and
> > +guest IO virtual address (IOVA), wherin the vIOMMU implementation  
> 
> wherein
> 
right

> > +relies on the physical IOMMU and for this reason requires interactions
> > +with the host driver.
> > +
> > +.. 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 upon guest requests
> > +5. Report errors to the guest and serve page requests
> > +
> > +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  
> 
>   userspace
> 
ditto

> > +
> > +Interfaces
> > +==
> > +Although the data structures defined in IOMMU UAPI are self-contained,
> > +there is no user API functions introduced. Instead, IOMMU UAPI is  
> 
>there are no
> 
right

> > +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 the
> > structure sizes. +
> > +No new fields can be added *after* the variable sized union in that it
> > +will break backward compatibility when offset moves. A new flag must
> > +be introduced whenever a change affects the structure using either
> > +method. The IOMMU driver processes the data based on flags which
> > +ensures backward compatibility.
> > +
> > +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.
> > +Though at the same time, argsz is user provided data which is not
> > +trusted. The argsz field allows the user app to indicate how much data
> > +it is providing, it's still the kernel's responsibility to validate  
> 
>  providing;
> 
yes. good separation.

> > +whether it's correct and sufficient for the requested operation.
> > +
> > +Compatibility Checking
> > +--
> > +When IOMMU UAPI extension results in some structure size increase,
> > +IOMMU UAPI code shall handle the following cases:
> > +
> > +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 an older kernel.
> > +4. A malicious/misbehaving user pass illegal/invalid size but within  
> 
>passing
> 
got it

> > +   range. The data may contain garbage.
> > +
> > +Feature 

Re: [PATCH v9 1/7] docs: IOMMU user API

2020-09-16 Thread Randy Dunlap
On 9/11/20 2:57 PM, 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 intended to clarify the UAPI design and usage. The
> mechanics of how future extensions should be achieved are also covered
> in this documentation.
> 
> Reviewed-by: Eric Auger 
> Signed-off-by: Liu Yi L 
> Signed-off-by: Jacob Pan 
> ---
>  Documentation/userspace-api/iommu.rst | 211 
> ++
>  MAINTAINERS   |   1 +
>  2 files changed, 212 insertions(+)
>  create mode 100644 Documentation/userspace-api/iommu.rst

Hi,
I have a few edit changes for you below:


> diff --git a/Documentation/userspace-api/iommu.rst 
> b/Documentation/userspace-api/iommu.rst
> new file mode 100644
> index ..1e68e8f05bb3
> --- /dev/null
> +++ b/Documentation/userspace-api/iommu.rst
> @@ -0,0 +1,211 @@
> +.. 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 baremetal
> +usage, the IOMMU is a system device which does not need to communicate
> +with user space directly.

userspace

for consistency

> +
> +The primary use cases are guest Shared Virtual Address (SVA) and
> +guest IO virtual address (IOVA), wherin the vIOMMU implementation

wherein

> +relies on the physical IOMMU and for this reason requires interactions
> +with the host driver.
> +
> +.. 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 upon guest requests
> +5. Report errors to the guest and serve page requests
> +
> +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

  userspace

> +
> +Interfaces
> +==
> +Although the data structures defined in IOMMU UAPI are self-contained,
> +there is no user API functions introduced. Instead, IOMMU UAPI is

   there are no

> +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 the structure sizes.
> +
> +No new fields can be added *after* the variable sized union in that it
> +will break backward compatibility when offset moves. A new flag must
> +be introduced whenever a change affects the structure using either
> +method. The IOMMU driver processes the data based on flags which
> +ensures backward compatibility.
> +
> +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.
> +Though at the same time, argsz is user provided data which is not
> +trusted. The argsz field allows the user app to indicate how much data
> +it is providing, it's still the kernel's responsibility to validate

 providing;

> +whether it's correct and sufficient for the requested operation.
> +
> +Compatibility Checking
> +--
> +When IOMMU UAPI extension results in some structure size increase,
> +IOMMU UAPI code shall handle the following cases:
> +
> +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 an older kernel.
> +4. A malicious/misbehaving user pass illegal/invalid size but within

   passing

> +   range. The data may contain garbage.
> +
> +Feature Checking
> +
> +While launching a guest with vIOMMU, it is strongly advised to check
> +the compatibility upfront, as some subsequent errors happening during
> +vIOMMU operation, such as cache invalidation failures cannot be nicely> 
> +escaladated to the guest due to IOMMU specifications. This can lead to

   escalated

> +catastrophic failures for the users.
> +
> +User applications such as QEMU are