Subject: [PATCH 2/4] iommu: add ARM LPAE page table allocator

From: [email-masked] (Will Deacon)
Date: Mon, 15 Dec 2014 17:25:12 +0000

On Mon, Dec 15, 2014 at 04:35:12PM +0000, Varun Sethi wrote:
> -----Original Message-----
> From: Will Deacon [mailto:will.deacon at arm.com] 
> Sent: Monday, December 15, 2014 9:13 PM
> To: Sethi Varun-B16395
> Cc: linux-arm-kernel at lists.infradead.org; iommu at lists.linux-foundation.org; prem.mallappa at broadcom.com; Robin Murphy; lauraa at codeaurora.org; mitchelh at codeaurora.org; laurent.pinchart at ideasonboard.com; joro at 8bytes.org; m.szyprowski at samsung.com
> Subject: Re: [PATCH 2/4] iommu: add ARM LPAE page table allocator
> 
> On Mon, Dec 15, 2014 at 01:30:20PM +0000, Will Deacon wrote:
> > On Sun, Dec 14, 2014 at 05:45:49PM +0000, Varun Sethi wrote:
> > > [varun] ok, but you could potentially end up splitting mapping to 
> > > the least possible page size e.g. 4K. You, don't seem to take in to 
> > > account the possibility of using the block size at the next level. 
> > > For example, take a case where we have a huge page mapping using 1G 
> > > page size and we have an unmap request for 4K. We could still split 
> > > maximum part of the mapping using 2M pages at the next level. The 
> > > entry where we need to unmap the 4K region would potentially go to the next level.
> > 
> > Aha, I see what you mean here, thanks. I'll take a look...
> 
> Scratch that, I think the code is fine as it is. For the case you
> highlight, we iterate over the 1GB region remapping it using 4k pages, but
> skipping the one we want to unmap, so I don't think there's a problem
> (__arm_lpae_map will create the relevant table entries).
> 
> 
> [[varun]]  But you can split 1G in 2M mappings and then split up the
> unmapped region using 4K pages. In this case you split up the entire
> region using 4K pages.

True, I miss an optimisation opportunity there, but I don't know that it's
common enough to care (in the same way that we don't recreate a 1G mapping
if you remapped that 4k page back like it was).

You could add this by making arm_lpae_split_blk_unmap recursive, if you
wanted to.

Will