> From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
> Sent: Wednesday, November 6, 2024 5:31 PM
>
> On 2024/11/6 15:31, Tian, Kevin wrote:
> >> From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
> >> Sent: Monday, November 4, 2024 9:19 PM
> >>
> >>
> >> +int intel_pasid_replace_first_level(struct intel_iommu *iommu,
> >> + struct device *dev, pgd_t *pgd,
> >> + u32 pasid, u16 did, int flags)
> >> +{
> >> + struct pasid_entry *pte;
> >> + u16 old_did;
> >> +
> >> + if (!ecap_flts(iommu->ecap) ||
> >> + ((flags & PASID_FLAG_FL5LP) && !cap_fl5lp_support(iommu->cap)))
> >> + return -EINVAL;
> >
> > better copy the error messages from the setup part.
> >
> > there may be further chance to consolidate them later but no clear
> > reason why different error warning schemes should be used
> > between them.
> >
> > same for other helpers.
>
> sure. I think Baolu has a point that this may be trigger-able by userspace
> hence drop the error message to avoid DOS.
>
Isn't the existing path also trigger-able by userspace? It's better to
have a consistent policy cross all paths then you can clean it up
together later.
> >> +
> >> + spin_lock(&iommu->lock);
> >> + pte = intel_pasid_get_entry(dev, pasid);
> >> + if (!pte) {
> >> + spin_unlock(&iommu->lock);
> >> + return -ENODEV;
> >> + }
> >> +
> >> + if (!pasid_pte_is_present(pte)) {
> >> + spin_unlock(&iommu->lock);
> >> + return -EINVAL;
> >> + }
> >> +
> >> + old_did = pasid_get_domain_id(pte);
> >
> > probably should pass the old domain in and check whether the
> > domain->did is same as the one in the pasid entry and warn otherwise.
>
> this would be a sw bug. :) Do we really want to catch every bug by warn? :)
>
this one should not happen. If it does, something severe jumps out...
