Re: [PATCH v4 06/12] iommu/vt-d: Set the nested domain to a device

[Baolu Lu]

On 2023/8/2 15:22, Tian, Kevin wrote:
> > From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
> > Sent: Monday, July 24, 2023 7:13 PM
> > +
> > +static int intel_nested_attach_dev(struct iommu_domain *domain,
> > +				   struct device *dev)
> > +{
> > +	struct device_domain_info *info = dev_iommu_priv_get(dev);
> > +	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
> > +	struct intel_iommu *iommu = info->iommu;
> > +	unsigned long flags;
> > +	int ret = 0;
> > +
> > +	if (info->domain)
> > +		device_block_translation(dev);
> > +
> > +	if (iommu->agaw < dmar_domain->s2_domain->agaw) {
> > +		dev_err_ratelimited(dev, "Adjusted guest address width not
> > compatible\n");
> > +		return -ENODEV;
> > +	}
>
> this is the check duplicated with patch04.

Ack. No need for a check here.

> > +
> > +	ret = domain_attach_iommu(dmar_domain, iommu);
> > +	if (ret) {
> > +		dev_err_ratelimited(dev, "Failed to attach domain to
> > iommu\n");
> > +		return ret;
> > +	}
> > +
>
> [...]
>
> > +	domain_update_iommu_cap(dmar_domain);
>
> iommu_cap is already updated in domain_attach_iommu().

Ack. We will eventually remove this helper when every domain is
allocated for a specific device.

> >   static const struct iommu_domain_ops intel_nested_domain_ops = {
> > +	.attach_dev		= intel_nested_attach_dev,
> >   	.free			= intel_nested_domain_free,
> > +	.enforce_cache_coherency = intel_iommu_enforce_cache_coherency,
>
> this is not required. enforce_cache_coherency() will be called on parent
> hwpt when it's being created. patch04 should check parent's force_snooping
> to set pgsnp in the pasid entry.
>
> As Jason explained it should be done only for kernel owned page table.

Ack and thanks!

Best regards,
baolu