Re: [PATCH v5 13/23] iommu: introduce device fault report API

From: Jean-Philippe Brucker
Date: Wed Sep 26 2018 - 06:14:27 EST


On 25/09/2018 23:17, Jacob Pan wrote:
> On Tue, 25 Sep 2018 15:58:41 +0100
> Jean-Philippe Brucker <jean-philippe.brucker@xxxxxxx> wrote:
>
>> Hi Jacob,
>>
>> Just two minor things below, that I noticed while using fault handlers
>> for SVA. From my perspective the series is fine otherwise
>>
>> On 11/05/2018 21:54, Jacob Pan wrote:
>> > +int iommu_unregister_device_fault_handler(struct device *dev)
>> > +{
>> > +ÂÂÂÂÂÂ struct iommu_param *param = dev->iommu_param;
>> > +ÂÂÂÂÂÂ int ret = 0;
>> > +
>> > +ÂÂÂÂÂÂ if (!param)
>> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ return -EINVAL;
>> > +
>> > +ÂÂÂÂÂÂ mutex_lock(&param->lock);Â
>>
>> Could we check that param->fault_param isn't NULL here, so that the
>> driver can call this function unconditionally in a cleanup path?
>>
> sounds good.
>
> ÂÂÂÂÂÂÂ if (!param || param->fault_param)
> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ return -EINVAL;

That would be too convenient... param needs to be checked before taking
the lock, and fault_param accessed after

Thanks,
Jean