Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <38988c89-ec61-faa1-6b15-2fd3aa51eede@arm.com>
Date:   Tue, 13 Sep 2022 15:15:18 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:102.0) Gecko/20100101
 Thunderbird/102.2.2
Subject: Re: [PATCH] iommu/iova: Fix module config properly
Content-Language: en-GB
To:     John Garry <john.garry@huawei.com>, joro@8bytes.org
Cc:     will@kernel.org, iommu@lists.linux.dev,
        linux-kernel@vger.kernel.org,
        Thierry Reding <thierry.reding@gmail.com>
References: <548c2f683ca379aface59639a8f0cccc3a1ac050.1663069227.git.robin.murphy@arm.com>
 <4740744e-2e12-520d-8598-188b8d2ebd6b@huawei.com>
From:   Robin Murphy <robin.murphy@arm.com>
In-Reply-To: <4740744e-2e12-520d-8598-188b8d2ebd6b@huawei.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 2022-09-13 14:01, John Garry wrote:
> On 13/09/2022 12:47, Robin Murphy wrote:
>> IOMMU_IOVA is intended to be an optional library for users to select as
>> and when they desire. Since it can be a module now, this means that
>> built-in code which has chosen not to select it should not fail to link
>> if it happens to have selected as a module by someone else. Replace
>> IS_ENABLED() with IS_REACHABLE() to do the right thing.
> 
> Hi Robin,
> 
> Recently you mentioned "I wonder if we couldn't replace the IS_ENABLED() 
> with IS_REACHABLE() and restore some of the previously-conditional 
> selects", and pointed me to 84db889e6d82 as an example of when a 
> conditional select was made unconditional.
> 
> So will you also restore some previously-conditional selects next?

I figured I'd leave that up to Thierry (and/or anyone else with a vested 
interest), but having mulled it over since that previous thread, there's 
really no excuse for the API itself not to do the right thing either 
way, so I felt compelled to write up this much.

Cheers,
Robin.

> 
> To me, it seems that any user of IOVA API selects IOMMU_IOVA always.
> 
> thanks,
> John
> 
>>
>> CC: Thierry Reding <thierry.reding@gmail.com>
>> Reported-by: John Garry <john.garry@huawei.com>
>> Fixes: 15bbdec3931e ("iommu: Make the iova library a module")
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>> ---
>>
>> Phrased as a fix for the sake of complete honesty, but it seems
>> everyone's been making do for years already so by now it's really
>> just more of an enhancement.
>>
>>   include/linux/iova.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/linux/iova.h b/include/linux/iova.h
>> index 8f97a43be834..fe18c3e6a725 100644
>> --- a/include/linux/iova.h
>> +++ b/include/linux/iova.h
>> @@ -75,7 +75,7 @@ static inline unsigned long iova_pfn(struct 
>> iova_domain *iovad, dma_addr_t iova)
>>       return iova >> iova_shift(iovad);
>>   }
>> -#if IS_ENABLED(CONFIG_IOMMU_IOVA)
>> +#if IS_REACHABLE(CONFIG_IOMMU_IOVA)
>>   int iova_cache_get(void);
>>   void iova_cache_put(void);
>