* Re: Patch "iommu/amd: Simplify pagetable freeing" has been added to the 5.15-stable tree
[not found] <20220305210025.146536-1-sashal@kernel.org>
@ 2022-03-07 10:35 ` Robin Murphy
2022-03-07 10:58 ` Sasha Levin
0 siblings, 1 reply; 2+ messages in thread
From: Robin Murphy @ 2022-03-07 10:35 UTC (permalink / raw)
To: Sasha Levin, stable-commits; +Cc: Joerg Roedel, stable
On 2022-03-05 21:00, Sasha Levin wrote:
> This is a note to let you know that I've just added the patch titled
>
> iommu/amd: Simplify pagetable freeing
>
> to the 5.15-stable tree which can be found at:
> http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
>
> The filename of the patch is:
> iommu-amd-simplify-pagetable-freeing.patch
> and it can be found in the queue-5.15 subdirectory.
>
> If you, or anyone else, feels it should not be added to the stable tree,
> please let <stable@vger.kernel.org> know about it.
I don't think this one qualifies for stable - it was just a refactoring
to aid future development. The "fixing" of types is merely cosmetic, and
the code size benefit was just a little bonus, hardly significant.
Thanks,
Robin.
> commit 3ccd31a933e9a08a64a803d7fda5cb64a48e5229
> Author: Robin Murphy <robin.murphy@arm.com>
> Date: Fri Dec 17 15:30:58 2021 +0000
>
> iommu/amd: Simplify pagetable freeing
>
> [ Upstream commit 6b3106e9ba2de7320a71291cedcefdcf1195ad58 ]
>
> For reasons unclear, pagetable freeing is an effectively recursive
> method implemented via an elaborate system of templated functions that
> turns out to account for 25% of the object file size. Implementing it
> using regular straightforward recursion makes the code simpler, and
> seems like a good thing to do before we work on it further. As part of
> that, also fix the types to avoid all the needless casting back and
> forth which just gets in the way.
>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> Link: https://lore.kernel.org/r/d3d00c9f3fa0df4756b867072c201e6e82f9ce39.1639753638.git.robin.murphy@arm.com
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
>
> diff --git a/drivers/iommu/amd/io_pgtable.c b/drivers/iommu/amd/io_pgtable.c
> index 182c93a43efd..4165e1372b6e 100644
> --- a/drivers/iommu/amd/io_pgtable.c
> +++ b/drivers/iommu/amd/io_pgtable.c
> @@ -84,49 +84,45 @@ static void free_page_list(struct page *freelist)
> }
> }
>
> -static struct page *free_pt_page(unsigned long pt, struct page *freelist)
> +static struct page *free_pt_page(u64 *pt, struct page *freelist)
> {
> - struct page *p = virt_to_page((void *)pt);
> + struct page *p = virt_to_page(pt);
>
> p->freelist = freelist;
>
> return p;
> }
>
> -#define DEFINE_FREE_PT_FN(LVL, FN) \
> -static struct page *free_pt_##LVL (unsigned long __pt, struct page *freelist) \
> -{ \
> - unsigned long p; \
> - u64 *pt; \
> - int i; \
> - \
> - pt = (u64 *)__pt; \
> - \
> - for (i = 0; i < 512; ++i) { \
> - /* PTE present? */ \
> - if (!IOMMU_PTE_PRESENT(pt[i])) \
> - continue; \
> - \
> - /* Large PTE? */ \
> - if (PM_PTE_LEVEL(pt[i]) == 0 || \
> - PM_PTE_LEVEL(pt[i]) == 7) \
> - continue; \
> - \
> - p = (unsigned long)IOMMU_PTE_PAGE(pt[i]); \
> - freelist = FN(p, freelist); \
> - } \
> - \
> - return free_pt_page((unsigned long)pt, freelist); \
> -}
> +static struct page *free_pt_lvl(u64 *pt, struct page *freelist, int lvl)
> +{
> + u64 *p;
> + int i;
> +
> + for (i = 0; i < 512; ++i) {
> + /* PTE present? */
> + if (!IOMMU_PTE_PRESENT(pt[i]))
> + continue;
>
> -DEFINE_FREE_PT_FN(l2, free_pt_page)
> -DEFINE_FREE_PT_FN(l3, free_pt_l2)
> -DEFINE_FREE_PT_FN(l4, free_pt_l3)
> -DEFINE_FREE_PT_FN(l5, free_pt_l4)
> -DEFINE_FREE_PT_FN(l6, free_pt_l5)
> + /* Large PTE? */
> + if (PM_PTE_LEVEL(pt[i]) == 0 ||
> + PM_PTE_LEVEL(pt[i]) == 7)
> + continue;
>
> -static struct page *free_sub_pt(unsigned long root, int mode,
> - struct page *freelist)
> + /*
> + * Free the next level. No need to look at l1 tables here since
> + * they can only contain leaf PTEs; just free them directly.
> + */
> + p = IOMMU_PTE_PAGE(pt[i]);
> + if (lvl > 2)
> + freelist = free_pt_lvl(p, freelist, lvl - 1);
> + else
> + freelist = free_pt_page(p, freelist);
> + }
> +
> + return free_pt_page(pt, freelist);
> +}
> +
> +static struct page *free_sub_pt(u64 *root, int mode, struct page *freelist)
> {
> switch (mode) {
> case PAGE_MODE_NONE:
> @@ -136,19 +132,11 @@ static struct page *free_sub_pt(unsigned long root, int mode,
> freelist = free_pt_page(root, freelist);
> break;
> case PAGE_MODE_2_LEVEL:
> - freelist = free_pt_l2(root, freelist);
> - break;
> case PAGE_MODE_3_LEVEL:
> - freelist = free_pt_l3(root, freelist);
> - break;
> case PAGE_MODE_4_LEVEL:
> - freelist = free_pt_l4(root, freelist);
> - break;
> case PAGE_MODE_5_LEVEL:
> - freelist = free_pt_l5(root, freelist);
> - break;
> case PAGE_MODE_6_LEVEL:
> - freelist = free_pt_l6(root, freelist);
> + free_pt_lvl(root, freelist, mode);
> break;
> default:
> BUG();
> @@ -364,7 +352,7 @@ static u64 *fetch_pte(struct amd_io_pgtable *pgtable,
>
> static struct page *free_clear_pte(u64 *pte, u64 pteval, struct page *freelist)
> {
> - unsigned long pt;
> + u64 *pt;
> int mode;
>
> while (cmpxchg64(pte, pteval, 0) != pteval) {
> @@ -375,7 +363,7 @@ static struct page *free_clear_pte(u64 *pte, u64 pteval, struct page *freelist)
> if (!IOMMU_PTE_PRESENT(pteval))
> return freelist;
>
> - pt = (unsigned long)IOMMU_PTE_PAGE(pteval);
> + pt = IOMMU_PTE_PAGE(pteval);
> mode = IOMMU_PTE_MODE(pteval);
>
> return free_sub_pt(pt, mode, freelist);
> @@ -512,7 +500,6 @@ static void v1_free_pgtable(struct io_pgtable *iop)
> struct amd_io_pgtable *pgtable = container_of(iop, struct amd_io_pgtable, iop);
> struct protection_domain *dom;
> struct page *freelist = NULL;
> - unsigned long root;
>
> if (pgtable->mode == PAGE_MODE_NONE)
> return;
> @@ -529,8 +516,7 @@ static void v1_free_pgtable(struct io_pgtable *iop)
> BUG_ON(pgtable->mode < PAGE_MODE_NONE ||
> pgtable->mode > PAGE_MODE_6_LEVEL);
>
> - root = (unsigned long)pgtable->root;
> - freelist = free_sub_pt(root, pgtable->mode, freelist);
> + freelist = free_sub_pt(pgtable->root, pgtable->mode, freelist);
>
> free_page_list(freelist);
> }
^ permalink raw reply [flat|nested] 2+ messages in thread