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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <1f5f010a-c11e-452a-aff0-8829d1a3239a@arm.com>
Date: Thu, 15 May 2025 16:40:59 +0100
From: Robin Murphy <robin.murphy@....com>
To: Will Deacon <will@...nel.org>, Jason Gunthorpe <jgg@...pe.ca>
Cc: Daniel Mentz <danielmentz@...gle.com>, iommu@...ts.linux.dev,
 linux-kernel@...r.kernel.org, Mostafa Saleh <smostafa@...gle.com>,
 Pranjal Shrivastava <praan@...gle.com>
Subject: Re: [PATCH] iommu/io-pgtable-arm: Support contiguous bit in
 translation tables

On 15/05/2025 3:36 pm, Will Deacon wrote:
> [+Robin]
> 
> On Tue, May 06, 2025 at 12:40:14PM -0300, Jason Gunthorpe wrote:
>> On Wed, Apr 30, 2025 at 11:19:24PM +0000, Daniel Mentz wrote:
>>> The contiguous bit in translation table entries can be used as a hint to
>>> SMMU that a group of adjacent translation table entries have consistent
>>> attributes and point to a contiguous and properly aligned output address
>>> range. This enables SMMU to predict the properties of the remaining
>>> translation table entries in the same group without accessing them. It
>>> also allows an SMMU implementation to make more efficient use of its TLB
>>> by using a single TLB entry to cover all translation table entries in
>>> the same group.
>>>
>>> In the case of 4KB granule size, there are 16 translation table entries
>>> in one group.
>>>
>>> This change sets the contiguous bit for such groups of entries that are
>>> completely covered by a single call to map_pages. As it stands, the code
>>> wouldn't set the contiguous bit if a group of adjacent descriptors is
>>> completed by separate calls to map_pages.
>>
>> Nor should it
>>
>> This seems like a pretty hacky implementation, it doesn't set the
>> pgsize bitmap to indicate support and it doesn't have a safety check
>> on unmap to protect against partial unmap of a huge page.
>>
>> Wouldn't it be better to use the pgsize bit map and rely on the core
>> code to tell a contig page size is being used and then it can
>> trivially set the PTE bit without having to do all this extra work?
> 
> That sounds like it would be quite a bit cleaner and I think it aligns
> with the "Large page" support in io-pgtable-arm-v7s.c which is doing
> something extremely similar.

Indeed, much like we advertise contiguous sizes for hugetlb on the CPU, 
having them in the pgsize_bitmap gives a clue to IOMMU API users that 
there is some potential benefit in trying to align to these sizes where 
appropriate (e.g. __iommu_dma_alloc_pages()).

Secondly, there also needs to be a straightforward way for drivers to 
opt out of (or perhaps explicitly in to) this - IIRC there are some SMMU 
errata which have so far not mattered due to Linux not using the 
contiguous bit, for which the least painful workaround is probably for 
affected hardware to just continue not using the contiguous bit.

Thanks,
Robin.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ