Re: [PATCH 1/3] iommu/vt-d: Setup scalable mode context entry in probe path

[Baolu Lu]

On 2024/3/4 15:48, Tian, Kevin wrote:
> > From: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> > Sent: Thursday, February 29, 2024 5:48 PM
> >
> > +static int device_pasid_table_setup(struct device *dev, u8 bus, u8 devfn)
> > +{
> > +	struct device_domain_info *info = dev_iommu_priv_get(dev);
> > +	struct intel_iommu *iommu = info->iommu;
> > +	struct context_entry *context;
> > +
> > +	spin_lock(&iommu->lock);
> > +	context = iommu_context_addr(iommu, bus, devfn, true);
> > +	if (!context) {
> > +		spin_unlock(&iommu->lock);
> > +		return -ENOMEM;
> > +	}
> > +
> > +	if (context_present(context) && !context_copied(iommu, bus, devfn))
> > {
> > +		spin_unlock(&iommu->lock);
> > +		return 0;
> > +	}
> > +
> > +	/*
> > +	 * For kdump case, at this point, the device is supposed to finish
> > +	 * reset at its driver probe stage, so no in-flight DMA will exist,
> > +	 * and we don't need to worry anymore hereafter.
> > +	 */
> > +	if (context_copied(iommu, bus, devfn)) {
> > +		context_clear_entry(context);
> > +		if (!ecap_coherent(iommu->ecap))
> > +			clflush_cache_range(context, sizeof(*context));
> > +		sm_context_flush_caches(dev);
> > +		clear_context_copied(iommu, bus, devfn);
> > +	}
>
> it's unclear to me why this doesn't need refer to old did as done in the
> existing code. If scalable mode makes any difference could you extend
> the comment to explain so people can avoid similar confusion when
> comparing the different paths between legacy and sm?

The previous code gets the domain ID from the copied context entry:

u16 did_old = context_domain_id(context);

This makes no sense for scalable mode, as the domain ID has been moved
to the PASID entry in scalable mode. As the result, did_old always gets
0.

> anyway it's kind of a semantics change probably worth a separate patch
> to special case sm for bisect and then doing cleanup...

This change doesn't impact anything as the hardware will skip domain id
field in the Context-cache Invalidate Descriptor in scalable mode.

Spec section 6.5.2.1 Context-cache Invalidate Descriptor:

Domain-ID (DID): ... This field is ignored by hardware when operating in
scalable mode (RTADDR_REG.TTM=01b).

Best regards,
baolu