Re: [PATCH v8 04/11] iommu: Add sva iommu_domain support

2022-06-19 Thread Baolu Lu

On 2022/6/17 15:43, Tian, Kevin wrote:

From: Baolu Lu
Sent: Friday, June 10, 2022 3:16 PM



+#define __IOMMU_DOMAIN_HOST_VA (1U << 5)  /* Host CPU virtual

address */


Do you mean general CPU VA? or Host CPU VA, I'm reading the latter as

2nd

stage?


Host CPU VA. In the near future, we will add another flag _GUEST_VA, so
that the shared page table types are distiguished.


How does the kernel knows that an user page table translates guest VA?
IMHO I don't think the kernel should care about it except managing
all the aspects related to the user page table itself...


Okay.








+
   /*
* This are the possible domain-types
*
@@ -86,15 +89,24 @@ struct iommu_domain_geometry {
   #define IOMMU_DOMAIN_DMA_FQ

(__IOMMU_DOMAIN_PAGING |\

 __IOMMU_DOMAIN_DMA_API |   \
 __IOMMU_DOMAIN_DMA_FQ)
+#define IOMMU_DOMAIN_SVA   (__IOMMU_DOMAIN_SHARED |

\

+__IOMMU_DOMAIN_HOST_VA)


Doesn't shared automatically mean CPU VA? Do we need another flag?


Yes. Shared means CPU VA, but there're many types. Besides above two, we
also see the shared KVM/EPT.



Will the two sharing scenarios share any common code? If not then
having a separate flag bit is meaningless.


So far, I haven't seen the need for common code. I've ever thought about
the common notifier callback for page table entry update of SVA and KVM.
But there has been no feasible plan.



It might be more straightforward to be:

#define IOMMU_DOMAIN_SVA__IOMMU_DOMAIN_SVA
#define IOMMU_DOMAIN_KVM __IOMMU_DOMAIN_KVM
#define IOMMU_DOMAIN_USER __IOMMU_DOMAIN_USER


I am okay with this and we can add some shared bits later if we need to
consolidate any code.

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


RE: [PATCH v8 04/11] iommu: Add sva iommu_domain support

2022-06-17 Thread Tian, Kevin
> From: Baolu Lu
> Sent: Friday, June 10, 2022 3:16 PM
> >
> >> +#define __IOMMU_DOMAIN_HOST_VA(1U << 5)  /* Host CPU virtual
> address */
> >
> > Do you mean general CPU VA? or Host CPU VA, I'm reading the latter as
> 2nd
> > stage?
> 
> Host CPU VA. In the near future, we will add another flag _GUEST_VA, so
> that the shared page table types are distiguished.

How does the kernel knows that an user page table translates guest VA?
IMHO I don't think the kernel should care about it except managing
all the aspects related to the user page table itself...

> 
> >
> >> +
> >>   /*
> >>* This are the possible domain-types
> >>*
> >> @@ -86,15 +89,24 @@ struct iommu_domain_geometry {
> >>   #define IOMMU_DOMAIN_DMA_FQ
>   (__IOMMU_DOMAIN_PAGING |\
> >> __IOMMU_DOMAIN_DMA_API |   \
> >> __IOMMU_DOMAIN_DMA_FQ)
> >> +#define IOMMU_DOMAIN_SVA  (__IOMMU_DOMAIN_SHARED |
>   \
> >> +   __IOMMU_DOMAIN_HOST_VA)
> >
> > Doesn't shared automatically mean CPU VA? Do we need another flag?
> 
> Yes. Shared means CPU VA, but there're many types. Besides above two, we
> also see the shared KVM/EPT.
> 

Will the two sharing scenarios share any common code? If not then
having a separate flag bit is meaningless.

It might be more straightforward to be:

#define IOMMU_DOMAIN_SVA__IOMMU_DOMAIN_SVA
#define IOMMU_DOMAIN_KVM __IOMMU_DOMAIN_KVM
#define IOMMU_DOMAIN_USER __IOMMU_DOMAIN_USER

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


Re: [PATCH v8 04/11] iommu: Add sva iommu_domain support

2022-06-10 Thread Baolu Lu

On 2022/6/10 04:25, Raj, Ashok wrote:

Hi Baolu


Hi Ashok,



some minor nits.


Thanks for your comments.



On Tue, Jun 07, 2022 at 09:49:35AM +0800, Lu Baolu wrote:

The sva iommu_domain represents a hardware pagetable that the IOMMU
hardware could use for SVA translation. This adds some infrastructure
to support SVA domain in the iommu common layer. It includes:

- Extend the iommu_domain to support a new IOMMU_DOMAIN_SVA domain
   type. The IOMMU drivers that support SVA should provide the sva
   domain specific iommu_domain_ops.
- Add a helper to allocate an SVA domain. The iommu_domain_free()
   is still used to free an SVA domain.
- Add helpers to attach an SVA domain to a device and the reverse
   operation.

Some buses, like PCI, route packets without considering the PASID value.
Thus a DMA target address with PASID might be treated as P2P if the
address falls into the MMIO BAR of other devices in the group. To make
things simple, the attach/detach interfaces only apply to devices
belonging to the singleton groups, and the singleton is immutable in
fabric i.e. not affected by hotplug.

The iommu_attach/detach_device_pasid() can be used for other purposes,
such as kernel DMA with pasid, mediation device, etc.

Suggested-by: Jean-Philippe Brucker 
Suggested-by: Jason Gunthorpe 
Signed-off-by: Lu Baolu 
Reviewed-by: Jean-Philippe Brucker 
---
  include/linux/iommu.h | 45 -
  drivers/iommu/iommu.c | 93 +++
  2 files changed, 136 insertions(+), 2 deletions(-)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 3fbad42c0bf8..9173c5741447 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -64,6 +64,9 @@ struct iommu_domain_geometry {
  #define __IOMMU_DOMAIN_PT (1U << 2)  /* Domain is identity mapped   */
  #define __IOMMU_DOMAIN_DMA_FQ (1U << 3)  /* DMA-API uses flush queue*/
  
+#define __IOMMU_DOMAIN_SHARED	(1U << 4)  /* Page table shared from CPU  */


s/from CPU/with CPU


Sure.




+#define __IOMMU_DOMAIN_HOST_VA (1U << 5)  /* Host CPU virtual address */


Do you mean general CPU VA? or Host CPU VA, I'm reading the latter as 2nd
stage?


Host CPU VA. In the near future, we will add another flag _GUEST_VA, so
that the shared page table types are distiguished.




+
  /*
   * This are the possible domain-types
   *
@@ -86,15 +89,24 @@ struct iommu_domain_geometry {
  #define IOMMU_DOMAIN_DMA_FQ   (__IOMMU_DOMAIN_PAGING |\
 __IOMMU_DOMAIN_DMA_API |   \
 __IOMMU_DOMAIN_DMA_FQ)
+#define IOMMU_DOMAIN_SVA   (__IOMMU_DOMAIN_SHARED |\
+__IOMMU_DOMAIN_HOST_VA)


Doesn't shared automatically mean CPU VA? Do we need another flag?


Yes. Shared means CPU VA, but there're many types. Besides above two, we
also see the shared KVM/EPT.



  
  struct iommu_domain {

unsigned type;
const struct iommu_domain_ops *ops;
unsigned long pgsize_bitmap;/* Bitmap of page sizes in use */
-   iommu_fault_handler_t handler;
-   void *handler_token;
struct iommu_domain_geometry geometry;
struct iommu_dma_cookie *iova_cookie;
+   union {
+   struct {/* IOMMU_DOMAIN_DMA */
+   iommu_fault_handler_t handler;
+   void *handler_token;
+   };
+   struct {/* IOMMU_DOMAIN_SVA */
+   struct mm_struct *mm;
+   };
+   };
  };
  
  static inline bool iommu_is_dma_domain(struct iommu_domain *domain)

@@ -262,6 +274,8 @@ struct iommu_ops {
   * struct iommu_domain_ops - domain specific operations
   * @attach_dev: attach an iommu domain to a device
   * @detach_dev: detach an iommu domain from a device
+ * @set_dev_pasid: set an iommu domain to a pasid of device
+ * @block_dev_pasid: block pasid of device from using iommu domain
   * @map: map a physically contiguous memory region to an iommu domain
   * @map_pages: map a physically contiguous set of pages of the same size to
   * an iommu domain.
@@ -282,6 +296,10 @@ struct iommu_ops {
  struct iommu_domain_ops {
int (*attach_dev)(struct iommu_domain *domain, struct device *dev);
void (*detach_dev)(struct iommu_domain *domain, struct device *dev);
+   int (*set_dev_pasid)(struct iommu_domain *domain, struct device *dev,
+ioasid_t pasid);
+   void (*block_dev_pasid)(struct iommu_domain *domain, struct device *dev,
+   ioasid_t pasid);
  
  	int (*map)(struct iommu_domain *domain, unsigned long iova,

   phys_addr_t paddr, size_t size, int prot, gfp_t gfp);
@@ -679,6 +697,12 @@ int iommu_group_claim_dma_owner(struct iommu_group *group, 
void *owner);
  void iommu_group_release_dma_owner(struct iommu_group *group);
  bool iommu_group_dma_owner_claimed(struct iommu_group *group);
  
+struct 

Re: [PATCH v8 04/11] iommu: Add sva iommu_domain support

2022-06-09 Thread Raj, Ashok
Hi Baolu

some minor nits.

On Tue, Jun 07, 2022 at 09:49:35AM +0800, Lu Baolu wrote:
> The sva iommu_domain represents a hardware pagetable that the IOMMU
> hardware could use for SVA translation. This adds some infrastructure
> to support SVA domain in the iommu common layer. It includes:
> 
> - Extend the iommu_domain to support a new IOMMU_DOMAIN_SVA domain
>   type. The IOMMU drivers that support SVA should provide the sva
>   domain specific iommu_domain_ops.
> - Add a helper to allocate an SVA domain. The iommu_domain_free()
>   is still used to free an SVA domain.
> - Add helpers to attach an SVA domain to a device and the reverse
>   operation.
> 
> Some buses, like PCI, route packets without considering the PASID value.
> Thus a DMA target address with PASID might be treated as P2P if the
> address falls into the MMIO BAR of other devices in the group. To make
> things simple, the attach/detach interfaces only apply to devices
> belonging to the singleton groups, and the singleton is immutable in
> fabric i.e. not affected by hotplug.
> 
> The iommu_attach/detach_device_pasid() can be used for other purposes,
> such as kernel DMA with pasid, mediation device, etc.
> 
> Suggested-by: Jean-Philippe Brucker 
> Suggested-by: Jason Gunthorpe 
> Signed-off-by: Lu Baolu 
> Reviewed-by: Jean-Philippe Brucker 
> ---
>  include/linux/iommu.h | 45 -
>  drivers/iommu/iommu.c | 93 +++
>  2 files changed, 136 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 3fbad42c0bf8..9173c5741447 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -64,6 +64,9 @@ struct iommu_domain_geometry {
>  #define __IOMMU_DOMAIN_PT(1U << 2)  /* Domain is identity mapped   */
>  #define __IOMMU_DOMAIN_DMA_FQ(1U << 3)  /* DMA-API uses flush queue  
>   */
>  
> +#define __IOMMU_DOMAIN_SHARED(1U << 4)  /* Page table shared from 
> CPU  */

s/from CPU/with CPU

> +#define __IOMMU_DOMAIN_HOST_VA   (1U << 5)  /* Host CPU virtual address 
> */

Do you mean general CPU VA? or Host CPU VA, I'm reading the latter as 2nd
stage?

> +
>  /*
>   * This are the possible domain-types
>   *
> @@ -86,15 +89,24 @@ struct iommu_domain_geometry {
>  #define IOMMU_DOMAIN_DMA_FQ  (__IOMMU_DOMAIN_PAGING |\
>__IOMMU_DOMAIN_DMA_API |   \
>__IOMMU_DOMAIN_DMA_FQ)
> +#define IOMMU_DOMAIN_SVA (__IOMMU_DOMAIN_SHARED |\
> +  __IOMMU_DOMAIN_HOST_VA)

Doesn't shared automatically mean CPU VA? Do we need another flag?

>  
>  struct iommu_domain {
>   unsigned type;
>   const struct iommu_domain_ops *ops;
>   unsigned long pgsize_bitmap;/* Bitmap of page sizes in use */
> - iommu_fault_handler_t handler;
> - void *handler_token;
>   struct iommu_domain_geometry geometry;
>   struct iommu_dma_cookie *iova_cookie;
> + union {
> + struct {/* IOMMU_DOMAIN_DMA */
> + iommu_fault_handler_t handler;
> + void *handler_token;
> + };
> + struct {/* IOMMU_DOMAIN_SVA */
> + struct mm_struct *mm;
> + };
> + };
>  };
>  
>  static inline bool iommu_is_dma_domain(struct iommu_domain *domain)
> @@ -262,6 +274,8 @@ struct iommu_ops {
>   * struct iommu_domain_ops - domain specific operations
>   * @attach_dev: attach an iommu domain to a device
>   * @detach_dev: detach an iommu domain from a device
> + * @set_dev_pasid: set an iommu domain to a pasid of device
> + * @block_dev_pasid: block pasid of device from using iommu domain
>   * @map: map a physically contiguous memory region to an iommu domain
>   * @map_pages: map a physically contiguous set of pages of the same size to
>   * an iommu domain.
> @@ -282,6 +296,10 @@ struct iommu_ops {
>  struct iommu_domain_ops {
>   int (*attach_dev)(struct iommu_domain *domain, struct device *dev);
>   void (*detach_dev)(struct iommu_domain *domain, struct device *dev);
> + int (*set_dev_pasid)(struct iommu_domain *domain, struct device *dev,
> +  ioasid_t pasid);
> + void (*block_dev_pasid)(struct iommu_domain *domain, struct device *dev,
> + ioasid_t pasid);
>  
>   int (*map)(struct iommu_domain *domain, unsigned long iova,
>  phys_addr_t paddr, size_t size, int prot, gfp_t gfp);
> @@ -679,6 +697,12 @@ int iommu_group_claim_dma_owner(struct iommu_group 
> *group, void *owner);
>  void iommu_group_release_dma_owner(struct iommu_group *group);
>  bool iommu_group_dma_owner_claimed(struct iommu_group *group);
>  
> +struct iommu_domain *iommu_sva_domain_alloc(struct device *dev,
> + struct mm_struct *mm);
> +int iommu_attach_device_pasid(struct iommu_domain *domain, struct