RE: [PATCH v4 02/13] iommu/vt-d: Add a helper to flush cache for updating present pasid entry

Tian, Kevin
2024-11-06T10:02:16Z

> From: Liu, Yi L <yi.l.liu@intel.com>
> Sent: Wednesday, November 6, 2024 5:57 PM
>
> On 2024/11/6 17:40, Tian, Kevin wrote:
> >> From: Liu, Yi L <yi.l.liu@intel.com>
> >> Sent: Wednesday, November 6, 2024 4:46 PM
> >>
> >> On 2024/11/6 15:11, Tian, Kevin wrote:
> >>>> From: Liu, Yi L <yi.l.liu@intel.com>
> >>>> Sent: Monday, November 4, 2024 9:19 PM
> >>>>
> >>>> There are more paths that need to flush cache for present pasid entry
> >>>> after adding pasid replacement. Hence, add a helper for it. Per the
> >>>> VT-d spec, the changes to the fields other than SSADE and P bit can
> >>>> share the same code. So intel_pasid_setup_page_snoop_control() is the
> >>>> first user of this helper.
> >>>
> >>> No hw spec would ever talk about coding sharing in sw implementation.
> >>
> >> yes, it's just sw choice.
> >>
> >>> and according to the following context the fact is just that two flows
> >>> between RID and PASID are similar so you decide to create a common
> >>> helper for both.
> >>
> >> I'm not quite getting why RID is related. This is only about the cache
> >> flush per pasid entry updating.
> >
> > the comment says:
> >
> > + * - If (pasid is RID_PASID)
> > + * - Global Device-TLB invalidation to affected functions
> > + * Else
> > + * - PASID-based Device-TLB invalidation (with S=1 and
> > + * Addr[63:12]=0x7FFFFFFF_FFFFF) to affected functions
> >
> > so that is the only difference between two invalidation flows?
>
> oh, yes. But there are multiple PASID paths that need flush. e.g. the
> pasid teardown. However, this path cannot use this helper introduced
> here as it modifies the Present bit. Per table 28, the teardown path
> should check pgtt to decide between p_iotlb_inv and iotlb_inv.
>

Then just say that this patch generalizes the logic for flushing
pasid cache upon changes to bits other than SSADE and P which
requires a different flow according to VT-d spec