Re: [PATCH v2 14/19] iommu: Add guest PASID bind function

[Auger Eric <eric.auger@redhat.com>]

Hi Jacob,

On 4/27/19 12:11 AM, Jacob Pan wrote:
> On Fri, 26 Apr 2019 17:53:43 +0200
> Auger Eric <eric.auger@redhat.com> wrote:
> 
>> Hi Jacob,
>>
>> On 4/24/19 1:31 AM, Jacob Pan wrote:
>>> Guest shared virtual address (SVA) may require host to shadow guest
>>> PASID tables. Guest PASID can also be allocated from the host via
>>> enlightened interfaces. In this case, guest needs to bind the guest
>>> mm, i.e. cr3 in guest phisical address to the actual PASID table
>>> in  
>> physical
> got it
> 
>>> the host IOMMU. Nesting will be turned on such that guest virtual
>>> address can go through a two level translation:
>>> - 1st level translates GVA to GPA
>>> - 2nd level translates GPA to HPA
>>> This patch introduces APIs to bind guest PASID data to the assigned
>>> device entry in the physical IOMMU. See the diagram below for usage
>>> explaination.
>>>
>>>     .-------------.  .---------------------------.
>>>     |   vIOMMU    |  | Guest process mm, FL only |
>>>     |             |  '---------------------------'
>>>     .----------------/
>>>     | PASID Entry |--- PASID cache flush -
>>>     '-------------'                       |
>>>     |             |                       V
>>>     |             |
>>>     '-------------'
>>> Guest
>>> ------| Shadow |--------------------------|------------
>>>       v        v                          v
>>> Host
>>>     .-------------.  .----------------------.
>>>     |   pIOMMU    |  | Bind FL for GVA-GPA  |
>>>     |             |  '----------------------'
>>>     .----------------/  |
>>>     | PASID Entry |     V (Nested xlate)
>>>     '----------------\.---------------------.
>>>     |             |   |Set SL to GPA-HPA    |
>>>     |             |   '---------------------'
>>>     '-------------'
>>>
>>> Where:
>>>  - FL = First level/stage one page tables
>>>  - SL = Second level/stage two page tables
>>>
>>> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
>>> Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
>>> ---
>>>  drivers/iommu/iommu.c      | 20 ++++++++++++++++++++
>>>  include/linux/iommu.h      | 10 ++++++++++
>>>  include/uapi/linux/iommu.h | 15 ++++++++++++++-
>>>  3 files changed, 44 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>>> index 498c28a..072f8f3 100644
>>> --- a/drivers/iommu/iommu.c
>>> +++ b/drivers/iommu/iommu.c
>>> @@ -1561,6 +1561,26 @@ int iommu_cache_invalidate(struct
>>> iommu_domain *domain, struct device *dev, }
>>>  EXPORT_SYMBOL_GPL(iommu_cache_invalidate);
>>>  
>>> +int iommu_sva_bind_gpasid(struct iommu_domain *domain,
>>> +			struct device *dev, struct
>>> gpasid_bind_data *data) +{
>>> +	if (unlikely(!domain->ops->sva_bind_gpasid))
>>> +		return -ENODEV;
>>> +
>>> +	return domain->ops->sva_bind_gpasid(domain, dev, data);
>>> +}
>>> +EXPORT_SYMBOL_GPL(iommu_sva_bind_gpasid);
>>> +
>>> +int iommu_sva_unbind_gpasid(struct iommu_domain *domain, struct
>>> device *dev,
>>> +			int pasid)
>>> +{
>>> +	if (unlikely(!domain->ops->sva_unbind_gpasid))
>>> +		return -ENODEV;
>>> +
>>> +	return domain->ops->sva_unbind_gpasid(dev, pasid);
>>> +}
>>> +EXPORT_SYMBOL_GPL(iommu_sva_unbind_gpasid);
>>> +
>>>  static void __iommu_detach_device(struct iommu_domain *domain,
>>>  				  struct device *dev)
>>>  {
>>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>>> index 4b92e4b..611388e 100644
>>> --- a/include/linux/iommu.h
>>> +++ b/include/linux/iommu.h
>>> @@ -231,6 +231,8 @@ struct iommu_sva_ops {
>>>   * @detach_pasid_table: detach the pasid table
>>>   * @cache_invalidate: invalidate translation caches
>>>   * @pgsize_bitmap: bitmap of all possible supported page sizes
>>> + * @sva_bind_gpasid: bind guest pasid and mm
>>> + * @sva_unbind_gpasid: unbind guest pasid and mm
>>>   */
>>>  struct iommu_ops {
>>>  	bool (*capable)(enum iommu_cap);
>>> @@ -295,6 +297,10 @@ struct iommu_ops {
>>>  
>>>  	int (*cache_invalidate)(struct iommu_domain *domain,
>>> struct device *dev, struct iommu_cache_invalidate_info *inv_info);
>>> +	int (*sva_bind_gpasid)(struct iommu_domain *domain,
>>> +			struct device *dev, struct
>>> gpasid_bind_data *data); +
>>> +	int (*sva_unbind_gpasid)(struct device *dev, int pasid);  
>> So I am confused now. As the scalable mode PASID table entry contains
>> both the FL and SL PT pointers, will you ever use the
>> attach/detach_pasid_table or are we the only known users on ARM?
>>
> In scalable mode, we will not use attach pasid table. So ARM will be
> the only user for now.
> Guest PASID table is shadowed, PASID cache flush in the guest will
> trigger sva_bind_gpasid.
> I introduced bind PASID table for the previous VT-d spec that has
> extend context mode (deprecated), where SL is shared by all PASIDs on 
> the same device.
OK. Thank you for the confirmation.

Thanks

Eric

>>>  
>>>  	unsigned long pgsize_bitmap;
>>>  };
>>> @@ -409,6 +415,10 @@ extern void iommu_detach_pasid_table(struct
>>> iommu_domain *domain); extern int iommu_cache_invalidate(struct
>>> iommu_domain *domain, struct device *dev,
>>>  				  struct
>>> iommu_cache_invalidate_info *inv_info); +extern int
>>> iommu_sva_bind_gpasid(struct iommu_domain *domain,
>>> +		struct device *dev, struct gpasid_bind_data *data);
>>> +extern int iommu_sva_unbind_gpasid(struct iommu_domain *domain,
>>> +				struct device *dev, int pasid);  
>> definition in !CONFIG_IOMMU_API case?
> right
> 
>>>  extern struct iommu_domain *iommu_get_domain_for_dev(struct device
>>> *dev); extern struct iommu_domain *iommu_get_dma_domain(struct
>>> device *dev); extern int iommu_map(struct iommu_domain *domain,
>>> unsigned long iova, diff --git a/include/uapi/linux/iommu.h
>>> b/include/uapi/linux/iommu.h index 61a3fb7..5c95905 100644
>>> --- a/include/uapi/linux/iommu.h
>>> +++ b/include/uapi/linux/iommu.h
>>> @@ -235,6 +235,19 @@ struct iommu_cache_invalidate_info {
>>>  		struct iommu_inv_addr_info addr_info;
>>>  	};
>>>  };
>>> -
>>> +/**
>>> + * struct gpasid_bind_data - Information about device and guest
>>> PASID binding
>>> + * @gcr3:	Guest CR3 value from guest mm
>>> + * @pasid:	Process address space ID used for the guest mm
>>> + * @addr_width:	Guest address width. Paging mode can also
>>> be derived.
>>> + */
>>> +struct gpasid_bind_data {
>>> +	__u64 gcr3;
>>> +	__u32 pasid;
>>> +	__u32 addr_width;
>>> +	__u32 flags;
>>> +#define	IOMMU_SVA_GPASID_SRE	BIT(0) /* supervisor
>>> request */
>>> +	__u8 padding[4];
>>> +};  
>>
>>
>> Thanks
>>
>> Eric
>>>  
>>>  #endif /* _UAPI_IOMMU_H */
>>>   
> 
> [Jacob Pan]
>