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

[Tian, Kevin]

> From: Auger Eric
> Sent: Tuesday, September 4, 2018 4:41 PM
> 
> 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.
> 

yes, then it makes sense.

Thanks
Kevin