Re: [PATCH v3 02/17] drm/i915/gen8: Make pdp allocation more dynamic

Goel, Akash
2015-07-07T12:36:09Z

On 7/1/2015 8:57 PM, Michel Thierry wrote:
> This transitional patch doesn't do much for the existing code. However,
> it should make upcoming patches to use the full 48b address space a bit
> easier. The patch also introduces the PML4, ie. the new top level structure
> of the page tables.
>

Would be better to move the introduction of PML4 to a separate patch &
keep this patch only for the dynamic allocation of pdp changes.

> v2: Renamed pdp_free to be similar to pd/pt (unmap_and_free_pdp).
> v3: To facilitate testing, 48b mode will be available on Broadwell and
> GEN9+, when i915.enable_ppgtt = 3.
> v4: Rebase after s/page_tables/page_table/, added extra information
> about 4-level page table formats and use IS_ENABLED macro.
> v5: Check CONFIG_X86_64 instead of CONFIG_64BIT.
> v6: Rebase after Mika's ppgtt cleanup / scratch merge patch series, and follow
> his nomenclature in pdp functions (there is no alloc_pdp yet).
> v7: Rebase after merged version of Mika's ppgtt cleanup patch series.
> v8: Rebase after final merged version of Mika's ppgtt/scratch patches.
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com> (v2+)
> ---
> drivers/gpu/drm/i915/i915_drv.h | 7 ++-
> drivers/gpu/drm/i915/i915_gem_gtt.c | 116 ++++++++++++++++++++++++++++--------
> drivers/gpu/drm/i915/i915_gem_gtt.h | 41 ++++++++++---
> 3 files changed, 128 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1dbd957..7bccfd5 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2490,7 +2490,12 @@ struct drm_i915_cmd_table {
> #define HAS_HW_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 6)
> #define HAS_LOGICAL_RING_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 8)
> #define USES_PPGTT(dev) (i915.enable_ppgtt)
> -#define USES_FULL_PPGTT(dev) (i915.enable_ppgtt == 2)
> +#define USES_FULL_PPGTT(dev) (i915.enable_ppgtt >= 2)
> +#ifdef CONFIG_X86_64
> +# define USES_FULL_48BIT_PPGTT(dev) (i915.enable_ppgtt == 3)
> +#else
> +# define USES_FULL_48BIT_PPGTT(dev) false
> +#endif
>
> #define HAS_OVERLAY(dev) (INTEL_INFO(dev)->has_overlay)
> #define OVERLAY_NEEDS_PHYSICAL(dev) (INTEL_INFO(dev)->overlay_needs_physical)
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 712ca34..cdcc778 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -104,9 +104,13 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
> {
> bool has_aliasing_ppgtt;
> bool has_full_ppgtt;
> + bool has_full_64bit_ppgtt;
>
> has_aliasing_ppgtt = INTEL_INFO(dev)->gen >= 6;
> has_full_ppgtt = INTEL_INFO(dev)->gen >= 7;
> + has_full_64bit_ppgtt = IS_ENABLED(CONFIG_X86_64) &&
> + (IS_BROADWELL(dev) ||
> + INTEL_INFO(dev)->gen >= 9) && false; /* FIXME: 64b */
>
> if (intel_vgpu_active(dev))
> has_full_ppgtt = false; /* emulation is too hard */
> @@ -125,6 +129,9 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
> if (enable_ppgtt == 2 && has_full_ppgtt)
> return 2;
>
> + if (enable_ppgtt == 3 && has_full_64bit_ppgtt)
> + return 3;
> +
> #ifdef CONFIG_INTEL_IOMMU
> /* Disable ppgtt on SNB if VT-d is on. */
> if (INTEL_INFO(dev)->gen == 6 && intel_iommu_gfx_mapped) {
> @@ -522,6 +529,45 @@ static void gen8_initialize_pd(struct i915_address_space *vm,
> fill_px(vm->dev, pd, scratch_pde);
> }
>
> +static int __pdp_init(struct drm_device *dev,
> + struct i915_page_directory_pointer *pdp)
> +{
> + size_t pdpes = I915_PDPES_PER_PDP(dev);
> +
> + pdp->used_pdpes = kcalloc(BITS_TO_LONGS(pdpes),
> + sizeof(unsigned long),
> + GFP_KERNEL);
> + if (!pdp->used_pdpes)
> + return -ENOMEM;
> +
> + pdp->page_directory = kcalloc(pdpes, sizeof(*pdp->page_directory),
> + GFP_KERNEL);
> + if (!pdp->page_directory) {
> + kfree(pdp->used_pdpes);
> + /* the PDP might be the statically allocated top level. Keep it
> + * as clean as possible */
> + pdp->used_pdpes = NULL;
> + return -ENOMEM;
> + }
> +
> + return 0;
> +}
> +
> +static void __pdp_fini(struct i915_page_directory_pointer *pdp)
> +{
> + kfree(pdp->used_pdpes);
> + kfree(pdp->page_directory);
> + pdp->page_directory = NULL;
> +}
> +
> +static void free_pdp(struct drm_device *dev,
> + struct i915_page_directory_pointer *pdp)
> +{
> + __pdp_fini(pdp);
> + if (USES_FULL_48BIT_PPGTT(dev))
> + kfree(pdp);
> +}
> +
> /* Broadwell Page Directory Pointer Descriptors */
> static int gen8_write_pdp(struct drm_i915_gem_request *req,
> unsigned entry,
> @@ -634,9 +680,6 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm,
> pt_vaddr = NULL;
>
> for_each_sg_page(pages->sgl, &sg_iter, pages->nents, 0) {
> - if (WARN_ON(pdpe >= GEN8_LEGACY_PDPES))
> - break;
> -
> if (pt_vaddr == NULL) {
> struct i915_page_directory *pd = ppgtt->pdp.page_directory[pdpe];
> struct i915_page_table *pt = pd->page_table[pde];
> @@ -720,7 +763,8 @@ static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
> container_of(vm, struct i915_hw_ppgtt, base);
> int i;
>
> - for_each_set_bit(i, ppgtt->pdp.used_pdpes, GEN8_LEGACY_PDPES) {
> + for_each_set_bit(i, ppgtt->pdp.used_pdpes,
> + I915_PDPES_PER_PDP(ppgtt->base.dev)) {
> if (WARN_ON(!ppgtt->pdp.page_directory[i]))
> continue;
>
> @@ -729,6 +773,7 @@ static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
> free_pd(ppgtt->base.dev, ppgtt->pdp.page_directory[i]);
> }
>
> + free_pdp(ppgtt->base.dev, &ppgtt->pdp);
> gen8_free_scratch(vm);
> }
>
> @@ -820,8 +865,9 @@ static int gen8_ppgtt_alloc_page_directories(struct i915_hw_ppgtt *ppgtt,
> struct i915_page_directory *pd;
> uint64_t temp;
> uint32_t pdpe;
> + uint32_t pdpes = I915_PDPES_PER_PDP(ppgtt->base.dev);
>
> - WARN_ON(!bitmap_empty(new_pds, GEN8_LEGACY_PDPES));
> + WARN_ON(!bitmap_empty(new_pds, pdpes));
>
> gen8_for_each_pdpe(pd, pdp, start, length, temp, pdpe) {
> if (pd)
> @@ -839,18 +885,19 @@ static int gen8_ppgtt_alloc_page_directories(struct i915_hw_ppgtt *ppgtt,
> return 0;
>
> unwind_out:
> - for_each_set_bit(pdpe, new_pds, GEN8_LEGACY_PDPES)
> + for_each_set_bit(pdpe, new_pds, pdpes)
> free_pd(dev, pdp->page_directory[pdpe]);
>
> return -ENOMEM;
> }
>
> static void
> -free_gen8_temp_bitmaps(unsigned long *new_pds, unsigned long **new_pts)
> +free_gen8_temp_bitmaps(unsigned long *new_pds, unsigned long **new_pts,
> + uint32_t pdpes)
> {
> int i;
>
> - for (i = 0; i < GEN8_LEGACY_PDPES; i++)
> + for (i = 0; i < pdpes; i++)
> kfree(new_pts[i]);
> kfree(new_pts);
> kfree(new_pds);
> @@ -861,23 +908,24 @@ free_gen8_temp_bitmaps(unsigned long *new_pds, unsigned long **new_pts)
> */
> static
> int __must_check alloc_gen8_temp_bitmaps(unsigned long **new_pds,
> - unsigned long ***new_pts)
> + unsigned long ***new_pts,
> + uint32_t pdpes)
> {
> int i;
> unsigned long *pds;
> unsigned long **pts;
>
> - pds = kcalloc(BITS_TO_LONGS(GEN8_LEGACY_PDPES), sizeof(unsigned long), GFP_KERNEL);
> + pds = kcalloc(BITS_TO_LONGS(pdpes), sizeof(unsigned long), GFP_KERNEL);
> if (!pds)
> return -ENOMEM;
>
> - pts = kcalloc(GEN8_LEGACY_PDPES, sizeof(unsigned long *), GFP_KERNEL);
> + pts = kcalloc(pdpes, sizeof(unsigned long *), GFP_KERNEL);
> if (!pts) {
> kfree(pds);
> return -ENOMEM;
> }
>
> - for (i = 0; i < GEN8_LEGACY_PDPES; i++) {
> + for (i = 0; i < pdpes; i++) {
> pts[i] = kcalloc(BITS_TO_LONGS(I915_PDES),
> sizeof(unsigned long), GFP_KERNEL);
> if (!pts[i])
> @@ -890,7 +938,7 @@ int __must_check alloc_gen8_temp_bitmaps(unsigned long **new_pds,
> return 0;
>
> err_out:
> - free_gen8_temp_bitmaps(pds, pts);
> + free_gen8_temp_bitmaps(pds, pts, pdpes);
> return -ENOMEM;
> }
>
> @@ -916,6 +964,7 @@ static int gen8_alloc_va_range(struct i915_address_space *vm,
> const uint64_t orig_length = length;
> uint64_t temp;
> uint32_t pdpe;
> + uint32_t pdpes = I915_PDPES_PER_PDP(dev);
> int ret;
>
> /* Wrap is never okay since we can only represent 48b, and we don't
> @@ -927,7 +976,7 @@ static int gen8_alloc_va_range(struct i915_address_space *vm,
> if (WARN_ON(start + length > ppgtt->base.total))
> return -ENODEV;
>
> - ret = alloc_gen8_temp_bitmaps(&new_page_dirs, &new_page_tables);
> + ret = alloc_gen8_temp_bitmaps(&new_page_dirs, &new_page_tables, pdpes);
> if (ret)
> return ret;
>
> @@ -935,7 +984,7 @@ static int gen8_alloc_va_range(struct i915_address_space *vm,
> ret = gen8_ppgtt_alloc_page_directories(ppgtt, &ppgtt->pdp, start, length,
> new_page_dirs);
> if (ret) {
> - free_gen8_temp_bitmaps(new_page_dirs, new_page_tables);
> + free_gen8_temp_bitmaps(new_page_dirs, new_page_tables, pdpes);
> return ret;
> }
>
> @@ -989,7 +1038,7 @@ static int gen8_alloc_va_range(struct i915_address_space *vm,
> __set_bit(pdpe, ppgtt->pdp.used_pdpes);
> }
>
> - free_gen8_temp_bitmaps(new_page_dirs, new_page_tables);
> + free_gen8_temp_bitmaps(new_page_dirs, new_page_tables, pdpes);
> mark_tlbs_dirty(ppgtt);
> return 0;
>
> @@ -999,10 +1048,10 @@ err_out:
> free_pt(vm->dev, ppgtt->pdp.page_directory[pdpe]->page_table[temp]);
> }
>
> - for_each_set_bit(pdpe, new_page_dirs, GEN8_LEGACY_PDPES)
> + for_each_set_bit(pdpe, new_page_dirs, pdpes)
> free_pd(vm->dev, ppgtt->pdp.page_directory[pdpe]);
>
> - free_gen8_temp_bitmaps(new_page_dirs, new_page_tables);
> + free_gen8_temp_bitmaps(new_page_dirs, new_page_tables, pdpes);
> mark_tlbs_dirty(ppgtt);
> return ret;
> }
> @@ -1023,14 +1072,6 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
> return ret;
>
> ppgtt->base.start = 0;
> - ppgtt->base.total = 1ULL << 32;
> - if (IS_ENABLED(CONFIG_X86_32))
> - /* While we have a proliferation of size_t variables
> - * we cannot represent the full ppgtt size on 32bit,
> - * so limit it to the same size as the GGTT (currently
> - * 2GiB).
> - */
> - ppgtt->base.total = to_i915(ppgtt->base.dev)->gtt.base.total;
> ppgtt->base.cleanup = gen8_ppgtt_cleanup;
> ppgtt->base.allocate_va_range = gen8_alloc_va_range;
> ppgtt->base.insert_entries = gen8_ppgtt_insert_entries;
> @@ -1040,7 +1081,30 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
>
> ppgtt->switch_mm = gen8_mm_switch;
>
> + if (!USES_FULL_48BIT_PPGTT(ppgtt->base.dev)) {
> + ret = __pdp_init(false, &ppgtt->pdp);
> +
> + if (ret)
> + goto free_scratch;
> +
> + ppgtt->base.total = 1ULL << 32;
> + if (IS_ENABLED(CONFIG_X86_32))
> + /* While we have a proliferation of size_t variables
> + * we cannot represent the full ppgtt size on 32bit,
> + * so limit it to the same size as the GGTT (currently
> + * 2GiB).
> + */
> + ppgtt->base.total = to_i915(ppgtt->base.dev)->gtt.base.total;
> + } else {
> + ppgtt->base.total = 1ULL << 48;
> + return -EPERM; /* Not yet implemented */
> + }
> +
> return 0;
> +
> +free_scratch:
> + gen8_free_scratch(&ppgtt->base);
> + return ret;
> }
>
> static void gen6_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m)
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index d5bf953..e2b684e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -88,9 +88,17 @@ typedef uint64_t gen8_pde_t;
> * PDPE | PDE | PTE | offset
> * The difference as compared to normal x86 3 level page table is the PDPEs are
> * programmed via register.
> + *
> + * GEN8 48b legacy style address is defined as a 4 level page table:
> + * 47:39 | 38:30 | 29:21 | 20:12 | 11:0
> + * PML4E | PDPE | PDE | PTE | offset
> */
> +#define GEN8_PML4ES_PER_PML4 512
> +#define GEN8_PML4E_SHIFT 39
> #define GEN8_PDPE_SHIFT 30
> -#define GEN8_PDPE_MASK 0x3
> +/* NB: GEN8_PDPE_MASK is untrue for 32b platforms, but it has no impact on 32b page
> + * tables */
> +#define GEN8_PDPE_MASK 0x1ff
> #define GEN8_PDE_SHIFT 21
> #define GEN8_PDE_MASK 0x1ff
> #define GEN8_PTE_SHIFT 12
> @@ -98,6 +106,9 @@ typedef uint64_t gen8_pde_t;
> #define GEN8_LEGACY_PDPES 4
> #define GEN8_PTES I915_PTES(sizeof(gen8_pte_t))
>
> +#define I915_PDPES_PER_PDP(dev) (USES_FULL_48BIT_PPGTT(dev) ?\
> + GEN8_PML4ES_PER_PML4 : GEN8_LEGACY_PDPES)
> +
> #define PPAT_UNCACHED_INDEX (_PAGE_PWT | _PAGE_PCD)
> #define PPAT_CACHED_PDE_INDEX 0 /* WB LLC */
> #define PPAT_CACHED_INDEX _PAGE_PAT /* WB LLCeLLC */
> @@ -241,9 +252,17 @@ struct i915_page_directory {
> };
>
> struct i915_page_directory_pointer {
> - /* struct page *page; */
> - DECLARE_BITMAP(used_pdpes, GEN8_LEGACY_PDPES);
> - struct i915_page_directory *page_directory[GEN8_LEGACY_PDPES];
> + struct i915_page_dma base;
> +
> + unsigned long *used_pdpes;
> + struct i915_page_directory **page_directory;
> +};
> +
> +struct i915_pml4 {
> + struct i915_page_dma base;
> +
> + DECLARE_BITMAP(used_pml4es, GEN8_PML4ES_PER_PML4);
> + struct i915_page_directory_pointer *pdps[GEN8_PML4ES_PER_PML4];
> };
>
> struct i915_address_space {
> @@ -341,8 +360,9 @@ struct i915_hw_ppgtt {
> struct drm_mm_node node;
> unsigned long pd_dirty_rings;
> union {
> - struct i915_page_directory_pointer pdp;
> - struct i915_page_directory pd;
> + struct i915_pml4 pml4; /* GEN8+ & 48b PPGTT */
> + struct i915_page_directory_pointer pdp; /* GEN8+ */
> + struct i915_page_directory pd; /* GEN6-7 */
> };
>
> struct drm_i915_file_private *file_priv;
> @@ -436,14 +456,17 @@ static inline uint32_t gen6_pde_index(uint32_t addr)
> temp = min(temp, length), \
> start += temp, length -= temp)
>
> -#define gen8_for_each_pdpe(pd, pdp, start, length, temp, iter) \
> - for (iter = gen8_pdpe_index(start); \
> - pd = (pdp)->page_directory[iter], length > 0 && iter < GEN8_LEGACY_PDPES; \
> +#define gen8_for_each_pdpe_e(pd, pdp, start, length, temp, iter, b) \
> + for (iter = gen8_pdpe_index(start); \
> + pd = (pdp)->page_directory[iter], length > 0 && (iter < b); \
> iter++, \
> temp = ALIGN(start+1, 1 << GEN8_PDPE_SHIFT) - start, \
> temp = min(temp, length), \
> start += temp, length -= temp)
>
> +#define gen8_for_each_pdpe(pd, pdp, start, length, temp, iter) \
> + gen8_for_each_pdpe_e(pd, pdp, start, length, temp, iter, I915_PDPES_PER_PDP(dev))
> +
> static inline uint32_t gen8_pte_index(uint64_t address)
> {
> return i915_pte_index(address, GEN8_PDE_SHIFT);
>