linux-kernel - Re: [PATCH 5.10 337/663] iommu: Move iotlb_sync_map out from __iommu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <849d5192-3eb6-99cf-62b1-20160a3a4232@arm.com>
Date:   Thu, 4 Mar 2021 10:05:53 +0000
From:   Robin Murphy <robin.murphy@....com>
To:     Pavel Machek <pavel@....cz>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        Yong Wu <yong.wu@...iatek.com>, Will Deacon <will@...nel.org>,
        Sasha Levin <sashal@...nel.org>
Subject: Re: [PATCH 5.10 337/663] iommu: Move iotlb_sync_map out from
 __iommu_map

On 2021-03-04 08:43, Pavel Machek wrote:
> Hi!
> 
>> [ Upstream commit d8c1df02ac7f2c802a9b2afc0f5c888c4217f1d5 ]
>>
>> In the end of __iommu_map, It alway call iotlb_sync_map.
>>
>> This patch moves iotlb_sync_map out from __iommu_map since it is
>> unnecessary to call this for each sg segment especially iotlb_sync_map
>> is flush tlb all currently. Add a little helper _iommu_map for this.
> 
>> Signed-off-by: Sasha Levin <sashal@...nel.org>
> 
> AFAICT this is slight performance optimalization, not a bugfix. It
> actually introduces a bug, fixed by the next patch. I'd preffer not to
> have it in stable.

Right, the whole original series was a set of performance/efficiency 
improvements, so I'm not sure there's much justification for backporting 
individual patches out of context. Plus as you say this one has the 
unfortunate gfp bug.

Thanks for spotting this!

Robin.

> 
> Best regards,
> 								Pavel
> 
>> @@ -2421,18 +2418,31 @@ static int __iommu_map(struct iommu_domain *domain, unsigned long iova,
>>   
>> +static int _iommu_map(struct iommu_domain *domain, unsigned long iova,
>> +		      phys_addr_t paddr, size_t size, int prot, gfp_t gfp)
>> +{
>> +	const struct iommu_ops *ops = domain->ops;
>> +	int ret;
>> +
>> +	ret = __iommu_map(domain, iova, paddr, size, prot, GFP_KERNEL);
>> +	if (ret == 0 && ops->iotlb_sync_map)
>> +		ops->iotlb_sync_map(domain);
>> +
>> +	return ret;
>> +}
>