On Fri, Jan 10, 2025 at 10:51:49AM -0400, Jason Gunthorpe wrote:
> On Fri, Jan 10, 2025 at 07:12:46AM +0000, Tian, Kevin wrote:
> 
> > > + if (!viommu)
> > > +         return -ENODEV;
> > > + if (WARN_ON_ONCE(!viommu->ops || !viommu->ops-
> > > >supports_veventq ||
> > > +                  !viommu->ops->supports_veventq(type)))
> > > +         return -EOPNOTSUPP;
> > 
> > Hmm the driver knows which type is supported by itself before
> > calling this helper. Why bother having the helper calling into
> > the driver again to verify?
> 
> Indeed, it might make sense to protect this with
> 
>         if (IS_ENABLED(CONFIG_IOMMUFD_TEST))
> 
> As a compiled out assertion
> 
> Or drop it
> 
> We shouldn't have unnecessary argument validation on fast paths,
> !viommu should go too.

Ack. I will drop those two if-statments.

Nicolin

Reply via email to