Messages in this thread | | | Date | Thu, 18 Mar 2021 16:28:10 +0100 | From | Joerg Roedel <> | Subject | Re: [RFC PATCH 4/7] iommu/amd: Initial support for AMD IOMMU v2 page table |
| |
Hi Suravee,
On Fri, Mar 12, 2021 at 03:04:08AM -0600, Suravee Suthikulpanit wrote: > @@ -503,6 +504,7 @@ struct amd_io_pgtable { > int mode; > u64 *root; > atomic64_t pt_root; /* pgtable root and pgtable mode */ > + struct mm_struct v2_mm; > };
A whole mm_struct is a bit too much when all we really need is an 8-byte page-table root pointer.
> +static pte_t *fetch_pte(struct amd_io_pgtable *pgtable, > + unsigned long iova, > + unsigned long *page_size) > +{ > + int level; > + pte_t *ptep; > + > + ptep = lookup_address_in_mm(&pgtable->v2_mm, iova, &level); > + if (!ptep || pte_none(*ptep) || (level == PG_LEVEL_NONE)) > + return NULL;
So you are re-using the in-kernel page-table building code. That safes some lines of code, but has several problems:
1) When you boot a kernel with this code on a machine with 5-level paging, the IOMMU code will build a 5-level page-table too, breaking IOMMU mappings.
2) You need a whole mm_struct per domain, which is big.
3) The existing macros for CPU page-tables require locking. For IOMMU page-tables this is not really necessary and might cause scalability issues.
Overall I think you should write your own code to build a 4-level page-table and use cmpxchg64 to avoid the need for locking. Then things will not break when such a kernel is suddenly booted on a machine which as 5-level paging enabled.
Regards,
Joerg
| |