Re: [PATCH v3 02/11] iommu: Pass in reset result to pci_dev_reset_iommu_done()

[Baolu Lu]

On 4/24/26 10:46, Nicolin Chen wrote:
> On Fri, Apr 24, 2026 at 10:38:09AM +0800, Baolu Lu wrote:
> > On 4/17/26 07:28, Nicolin Chen wrote:
> > > @@ -4083,6 +4093,18 @@ void pci_dev_reset_iommu_done(struct pci_dev *pdev)
> > >    	if (WARN_ON(!group->blocking_domain))
> > >    		return;
> > > +	/*
> > > +	 * A reset failure implies that the device might be unreliable. E.g. its
> > > +	 * device cache might retain stale entries, which potentially results in
> > > +	 * memory corruption. Thus, do not unblock the device until a successful
> > > +	 * reset.
> > > +	 */
> > > +	if (!reset_succeeds) {
> > > +		pci_err(pdev,
> > > +			"Reset failed. Keep it blocked to protect memory\n");
> > > +		return;
> > > +	}
> >
> > Nit: pci_dev_reset_iommu_done() does nothing if reset_succeeds is false.
> > Would it be better to handle this in the caller instead? Something like:
> >
> > 	if (reset_succeeds)
> > 		pci_dev_reset_iommu_done(dev);
> >
> > ?
>
> It would also need a print and some duplicated comments. Actually,
> that would be my v2, which Kevin suggested this against:
> https://lore.kernel.org/all/BN9PR11MB5276706AE4E0BBE86F0F6E158C4EA@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/

Oh, I forgot that comment.

> Though I don't have a strong personal reference here, I do see this
> version slightly cleaner than doing in the callers.

Okay, you own the decision.

> Thanks
> Nicolin

Thanks,
baolu