Re: [RFC 01/13] iommu: Introduce bind_guest_stage API

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

Hi Kevin,

On 09/04/2018 10:34 AM, Tian, Kevin wrote:
>> From: Auger Eric
>> Sent: Tuesday, September 4, 2018 4:11 PM
>>
>> Hi Kevin,
>> On 09/04/2018 09:57 AM, Tian, Kevin wrote:
>>>> From: Auger Eric
>>>> Sent: Friday, August 31, 2018 9:52 PM
>>>>
>>>> Hi Jean-Philippe,
>>>>
>>>> On 08/31/2018 03:11 PM, Jean-Philippe Brucker wrote:
>>>>> Hi Eric,
>>>>>
>>>>> On 23/08/18 16:25, Auger Eric wrote:
>>>>>>> +int iommu_bind_guest_stage(struct iommu_domain *domain,
>> struct
>>>> device *dev,
>>>>>>> +			   struct iommu_guest_stage_config *cfg)
>>>>>
>>>>> About the name change from iommu_bind_pasid_table: is the intent to
>>>>> reuse this API for SMMUv2, which supports nested but not PASID?
>> Seems
>>>>> like a good idea but "iommu_bind_table" may be better since "stage" is
>>>>> only used by Arm.
>>>>
>>>> At the moment I don't target SMUv2 but just SMMUv3. My focus was on
>>>> nested stage enablement without enabling the multi-CD feature (PASID),
>>>> whish is not supported by the QEMU vSMMUv3. Afterwards I realized
>> that
>>>> basically we are pointing to a CD or PASID table and that's about the
>>>> same. I don't have a strong opinion on the name,
>> iommu_bind_guest_table
>>>> or iommu_bind_pasid_table would be fine with me. Indeed "stage" is
>> ARM
>>>> vocable (level for Intel?)
>>>
>>> Intel uses first level/second level.
>>>
>>> iommu_bind_table is a bit confusing. what should people take table as?
>>> there is PASID table. there is also page table linked in each stage/level.
>> and
>>> maybe other tables in vendor-specific definition.
>>>
>>> to me iommu_bind_pasid_table is still clearer. anyway in other places
>>> we've used pasid explicitly in vfio/iommu APIs, then it should be general
>>> enough to represent various implementations.
>>
>> Fine for me.
>>
>> However I I would suggest to rename the original iommu_sva_invalidate
>> into something that is SVA unrelated. iommu_tlb_invalidate is not OK as
>> this API also is used to invalidate context caches - which are not
>> iotlbs -. What about iommu_cache_invalidate?
>>
>> At least we must clarify that this API can be used for something else
>> than SVA enablement.
>>
> 
> Agree. using SVA is limiting.
> 
> I also agree that iommu_cache_invalidate is better, though I don't think
> you want to pass guest context cache invalidation to host. that information
> is fully under host control. :-)

I think the confusion comes from the different terminology used in VTD
and ARM SMMU spec.

Your PASID table ~ ARM SMMU Context Descriptor (CD) table
Your Root Entry/Context Entry ~ ARM SMMU Stream Table Entry (STE)

So I meant guesr invalidates its Context Descriptor cache. He "owns"
those. Host owns the STE.

Thanks

Eric

> 
> Thanks
> Kevin
>