Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

drm/panfrost: Make sure MMU context lifetime is not bound to panfrost_priv

Jobs can be in-flight when the file descriptor is closed (either because
the process did not terminate properly, or because it didn't wait for
all GPU jobs to be finished), and apparently panfrost_job_close() does
not cancel already running jobs. Let's refcount the MMU context object
so it's lifetime is no longer bound to the FD lifetime and running jobs
can finish properly without generating spurious page faults.

Reported-by: Icecream95 <ixn@keemail.me>
Fixes: 7282f7645d06 ("drm/panfrost: Implement per FD address spaces")
Cc: <stable@vger.kernel.org>
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Steven Price <steven.price@arm.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20210621133907.1683899-2-boris.brezillon@collabora.com

+136 -111
+5 -3
drivers/gpu/drm/panfrost/panfrost_device.h
··· 121 121 }; 122 122 123 123 struct panfrost_mmu { 124 + struct panfrost_device *pfdev; 125 + struct kref refcount; 124 126 struct io_pgtable_cfg pgtbl_cfg; 125 127 struct io_pgtable_ops *pgtbl_ops; 128 + struct drm_mm mm; 129 + spinlock_t mm_lock; 126 130 int as; 127 131 atomic_t as_count; 128 132 struct list_head list; ··· 137 133 138 134 struct drm_sched_entity sched_entity[NUM_JOB_SLOTS]; 139 135 140 - struct panfrost_mmu mmu; 141 - struct drm_mm mm; 142 - spinlock_t mm_lock; 136 + struct panfrost_mmu *mmu; 143 137 }; 144 138 145 139 static inline struct panfrost_device *to_panfrost_device(struct drm_device *ddev)
+9 -41
drivers/gpu/drm/panfrost/panfrost_drv.c
··· 412 412 * anyway, so let's not bother. 413 413 */ 414 414 if (!list_is_singular(&bo->mappings.list) || 415 - WARN_ON_ONCE(first->mmu != &priv->mmu)) { 415 + WARN_ON_ONCE(first->mmu != priv->mmu)) { 416 416 ret = -EINVAL; 417 417 goto out_unlock_mappings; 418 418 } ··· 444 444 return 0; 445 445 } 446 446 447 - #define PFN_4G (SZ_4G >> PAGE_SHIFT) 448 - #define PFN_4G_MASK (PFN_4G - 1) 449 - #define PFN_16M (SZ_16M >> PAGE_SHIFT) 450 - 451 - static void panfrost_drm_mm_color_adjust(const struct drm_mm_node *node, 452 - unsigned long color, 453 - u64 *start, u64 *end) 454 - { 455 - /* Executable buffers can't start or end on a 4GB boundary */ 456 - if (!(color & PANFROST_BO_NOEXEC)) { 457 - u64 next_seg; 458 - 459 - if ((*start & PFN_4G_MASK) == 0) 460 - (*start)++; 461 - 462 - if ((*end & PFN_4G_MASK) == 0) 463 - (*end)--; 464 - 465 - next_seg = ALIGN(*start, PFN_4G); 466 - if (next_seg - *start <= PFN_16M) 467 - *start = next_seg + 1; 468 - 469 - *end = min(*end, ALIGN(*start, PFN_4G) - 1); 470 - } 471 - } 472 - 473 447 static int 474 448 panfrost_open(struct drm_device *dev, struct drm_file *file) 475 449 { ··· 458 484 panfrost_priv->pfdev = pfdev; 459 485 file->driver_priv = panfrost_priv; 460 486 461 - spin_lock_init(&panfrost_priv->mm_lock); 462 - 463 - /* 4G enough for now. can be 48-bit */ 464 - drm_mm_init(&panfrost_priv->mm, SZ_32M >> PAGE_SHIFT, (SZ_4G - SZ_32M) >> PAGE_SHIFT); 465 - panfrost_priv->mm.color_adjust = panfrost_drm_mm_color_adjust; 466 - 467 - ret = panfrost_mmu_pgtable_alloc(panfrost_priv); 468 - if (ret) 469 - goto err_pgtable; 487 + panfrost_priv->mmu = panfrost_mmu_ctx_create(pfdev); 488 + if (IS_ERR(panfrost_priv->mmu)) { 489 + ret = PTR_ERR(panfrost_priv->mmu); 490 + goto err_free; 491 + } 470 492 471 493 ret = panfrost_job_open(panfrost_priv); 472 494 if (ret) ··· 471 501 return 0; 472 502 473 503 err_job: 474 - panfrost_mmu_pgtable_free(panfrost_priv); 475 - err_pgtable: 476 - drm_mm_takedown(&panfrost_priv->mm); 504 + panfrost_mmu_ctx_put(panfrost_priv->mmu); 505 + err_free: 477 506 kfree(panfrost_priv); 478 507 return ret; 479 508 } ··· 485 516 panfrost_perfcnt_close(file); 486 517 panfrost_job_close(panfrost_priv); 487 518 488 - panfrost_mmu_pgtable_free(panfrost_priv); 489 - drm_mm_takedown(&panfrost_priv->mm); 519 + panfrost_mmu_ctx_put(panfrost_priv->mmu); 490 520 kfree(panfrost_priv); 491 521 } 492 522
+9 -11
drivers/gpu/drm/panfrost/panfrost_gem.c
··· 60 60 61 61 mutex_lock(&bo->mappings.lock); 62 62 list_for_each_entry(iter, &bo->mappings.list, node) { 63 - if (iter->mmu == &priv->mmu) { 63 + if (iter->mmu == priv->mmu) { 64 64 kref_get(&iter->refcount); 65 65 mapping = iter; 66 66 break; ··· 74 74 static void 75 75 panfrost_gem_teardown_mapping(struct panfrost_gem_mapping *mapping) 76 76 { 77 - struct panfrost_file_priv *priv; 78 - 79 77 if (mapping->active) 80 78 panfrost_mmu_unmap(mapping); 81 79 82 - priv = container_of(mapping->mmu, struct panfrost_file_priv, mmu); 83 - spin_lock(&priv->mm_lock); 80 + spin_lock(&mapping->mmu->mm_lock); 84 81 if (drm_mm_node_allocated(&mapping->mmnode)) 85 82 drm_mm_remove_node(&mapping->mmnode); 86 - spin_unlock(&priv->mm_lock); 83 + spin_unlock(&mapping->mmu->mm_lock); 87 84 } 88 85 89 86 static void panfrost_gem_mapping_release(struct kref *kref) ··· 91 94 92 95 panfrost_gem_teardown_mapping(mapping); 93 96 drm_gem_object_put(&mapping->obj->base.base); 97 + panfrost_mmu_ctx_put(mapping->mmu); 94 98 kfree(mapping); 95 99 } 96 100 ··· 141 143 else 142 144 align = size >= SZ_2M ? SZ_2M >> PAGE_SHIFT : 0; 143 145 144 - mapping->mmu = &priv->mmu; 145 - spin_lock(&priv->mm_lock); 146 - ret = drm_mm_insert_node_generic(&priv->mm, &mapping->mmnode, 146 + mapping->mmu = panfrost_mmu_ctx_get(priv->mmu); 147 + spin_lock(&mapping->mmu->mm_lock); 148 + ret = drm_mm_insert_node_generic(&mapping->mmu->mm, &mapping->mmnode, 147 149 size >> PAGE_SHIFT, align, color, 0); 148 - spin_unlock(&priv->mm_lock); 150 + spin_unlock(&mapping->mmu->mm_lock); 149 151 if (ret) 150 152 goto err; 151 153 ··· 174 176 175 177 mutex_lock(&bo->mappings.lock); 176 178 list_for_each_entry(iter, &bo->mappings.list, node) { 177 - if (iter->mmu == &priv->mmu) { 179 + if (iter->mmu == priv->mmu) { 178 180 mapping = iter; 179 181 list_del(&iter->node); 180 182 break;
+2 -2
drivers/gpu/drm/panfrost/panfrost_job.c
··· 165 165 return; 166 166 } 167 167 168 - cfg = panfrost_mmu_as_get(pfdev, &job->file_priv->mmu); 168 + cfg = panfrost_mmu_as_get(pfdev, job->file_priv->mmu); 169 169 170 170 job_write(pfdev, JS_HEAD_NEXT_LO(js), jc_head & 0xFFFFFFFF); 171 171 job_write(pfdev, JS_HEAD_NEXT_HI(js), jc_head >> 32); ··· 516 516 if (job) { 517 517 pfdev->jobs[j] = NULL; 518 518 519 - panfrost_mmu_as_put(pfdev, &job->file_priv->mmu); 519 + panfrost_mmu_as_put(pfdev, job->file_priv->mmu); 520 520 panfrost_devfreq_record_idle(&pfdev->pfdevfreq); 521 521 522 522 dma_fence_signal_locked(job->done_fence);
+108 -52
drivers/gpu/drm/panfrost/panfrost_mmu.c
··· 1 1 // SPDX-License-Identifier: GPL-2.0 2 2 /* Copyright 2019 Linaro, Ltd, Rob Herring <robh@kernel.org> */ 3 + 4 + #include <drm/panfrost_drm.h> 5 + 3 6 #include <linux/atomic.h> 4 7 #include <linux/bitfield.h> 5 8 #include <linux/delay.h> ··· 340 337 341 338 static void mmu_tlb_sync_context(void *cookie) 342 339 { 343 - //struct panfrost_device *pfdev = cookie; 340 + //struct panfrost_mmu *mmu = cookie; 344 341 // TODO: Wait 1000 GPU cycles for HW_ISSUE_6367/T60X 345 342 } 346 343 ··· 355 352 .tlb_flush_walk = mmu_tlb_flush_walk, 356 353 }; 357 354 358 - int panfrost_mmu_pgtable_alloc(struct panfrost_file_priv *priv) 359 - { 360 - struct panfrost_mmu *mmu = &priv->mmu; 361 - struct panfrost_device *pfdev = priv->pfdev; 362 - 363 - INIT_LIST_HEAD(&mmu->list); 364 - mmu->as = -1; 365 - 366 - mmu->pgtbl_cfg = (struct io_pgtable_cfg) { 367 - .pgsize_bitmap = SZ_4K | SZ_2M, 368 - .ias = FIELD_GET(0xff, pfdev->features.mmu_features), 369 - .oas = FIELD_GET(0xff00, pfdev->features.mmu_features), 370 - .coherent_walk = pfdev->coherent, 371 - .tlb = &mmu_tlb_ops, 372 - .iommu_dev = pfdev->dev, 373 - }; 374 - 375 - mmu->pgtbl_ops = alloc_io_pgtable_ops(ARM_MALI_LPAE, &mmu->pgtbl_cfg, 376 - priv); 377 - if (!mmu->pgtbl_ops) 378 - return -EINVAL; 379 - 380 - return 0; 381 - } 382 - 383 - void panfrost_mmu_pgtable_free(struct panfrost_file_priv *priv) 384 - { 385 - struct panfrost_device *pfdev = priv->pfdev; 386 - struct panfrost_mmu *mmu = &priv->mmu; 387 - 388 - spin_lock(&pfdev->as_lock); 389 - if (mmu->as >= 0) { 390 - pm_runtime_get_noresume(pfdev->dev); 391 - if (pm_runtime_active(pfdev->dev)) 392 - panfrost_mmu_disable(pfdev, mmu->as); 393 - pm_runtime_put_autosuspend(pfdev->dev); 394 - 395 - clear_bit(mmu->as, &pfdev->as_alloc_mask); 396 - clear_bit(mmu->as, &pfdev->as_in_use_mask); 397 - list_del(&mmu->list); 398 - } 399 - spin_unlock(&pfdev->as_lock); 400 - 401 - free_io_pgtable_ops(mmu->pgtbl_ops); 402 - } 403 - 404 355 static struct panfrost_gem_mapping * 405 356 addr_to_mapping(struct panfrost_device *pfdev, int as, u64 addr) 406 357 { 407 358 struct panfrost_gem_mapping *mapping = NULL; 408 - struct panfrost_file_priv *priv; 409 359 struct drm_mm_node *node; 410 360 u64 offset = addr >> PAGE_SHIFT; 411 361 struct panfrost_mmu *mmu; ··· 371 415 goto out; 372 416 373 417 found_mmu: 374 - priv = container_of(mmu, struct panfrost_file_priv, mmu); 375 418 376 - spin_lock(&priv->mm_lock); 419 + spin_lock(&mmu->mm_lock); 377 420 378 - drm_mm_for_each_node(node, &priv->mm) { 421 + drm_mm_for_each_node(node, &mmu->mm) { 379 422 if (offset >= node->start && 380 423 offset < (node->start + node->size)) { 381 424 mapping = drm_mm_node_to_panfrost_mapping(node); ··· 384 429 } 385 430 } 386 431 387 - spin_unlock(&priv->mm_lock); 432 + spin_unlock(&mmu->mm_lock); 388 433 out: 389 434 spin_unlock(&pfdev->as_lock); 390 435 return mapping; ··· 495 540 err_bo: 496 541 drm_gem_object_put(&bo->base.base); 497 542 return ret; 543 + } 544 + 545 + static void panfrost_mmu_release_ctx(struct kref *kref) 546 + { 547 + struct panfrost_mmu *mmu = container_of(kref, struct panfrost_mmu, 548 + refcount); 549 + struct panfrost_device *pfdev = mmu->pfdev; 550 + 551 + spin_lock(&pfdev->as_lock); 552 + if (mmu->as >= 0) { 553 + pm_runtime_get_noresume(pfdev->dev); 554 + if (pm_runtime_active(pfdev->dev)) 555 + panfrost_mmu_disable(pfdev, mmu->as); 556 + pm_runtime_put_autosuspend(pfdev->dev); 557 + 558 + clear_bit(mmu->as, &pfdev->as_alloc_mask); 559 + clear_bit(mmu->as, &pfdev->as_in_use_mask); 560 + list_del(&mmu->list); 561 + } 562 + spin_unlock(&pfdev->as_lock); 563 + 564 + free_io_pgtable_ops(mmu->pgtbl_ops); 565 + drm_mm_takedown(&mmu->mm); 566 + kfree(mmu); 567 + } 568 + 569 + void panfrost_mmu_ctx_put(struct panfrost_mmu *mmu) 570 + { 571 + kref_put(&mmu->refcount, panfrost_mmu_release_ctx); 572 + } 573 + 574 + struct panfrost_mmu *panfrost_mmu_ctx_get(struct panfrost_mmu *mmu) 575 + { 576 + kref_get(&mmu->refcount); 577 + 578 + return mmu; 579 + } 580 + 581 + #define PFN_4G (SZ_4G >> PAGE_SHIFT) 582 + #define PFN_4G_MASK (PFN_4G - 1) 583 + #define PFN_16M (SZ_16M >> PAGE_SHIFT) 584 + 585 + static void panfrost_drm_mm_color_adjust(const struct drm_mm_node *node, 586 + unsigned long color, 587 + u64 *start, u64 *end) 588 + { 589 + /* Executable buffers can't start or end on a 4GB boundary */ 590 + if (!(color & PANFROST_BO_NOEXEC)) { 591 + u64 next_seg; 592 + 593 + if ((*start & PFN_4G_MASK) == 0) 594 + (*start)++; 595 + 596 + if ((*end & PFN_4G_MASK) == 0) 597 + (*end)--; 598 + 599 + next_seg = ALIGN(*start, PFN_4G); 600 + if (next_seg - *start <= PFN_16M) 601 + *start = next_seg + 1; 602 + 603 + *end = min(*end, ALIGN(*start, PFN_4G) - 1); 604 + } 605 + } 606 + 607 + struct panfrost_mmu *panfrost_mmu_ctx_create(struct panfrost_device *pfdev) 608 + { 609 + struct panfrost_mmu *mmu; 610 + 611 + mmu = kzalloc(sizeof(*mmu), GFP_KERNEL); 612 + if (!mmu) 613 + return ERR_PTR(-ENOMEM); 614 + 615 + mmu->pfdev = pfdev; 616 + spin_lock_init(&mmu->mm_lock); 617 + 618 + /* 4G enough for now. can be 48-bit */ 619 + drm_mm_init(&mmu->mm, SZ_32M >> PAGE_SHIFT, (SZ_4G - SZ_32M) >> PAGE_SHIFT); 620 + mmu->mm.color_adjust = panfrost_drm_mm_color_adjust; 621 + 622 + INIT_LIST_HEAD(&mmu->list); 623 + mmu->as = -1; 624 + 625 + mmu->pgtbl_cfg = (struct io_pgtable_cfg) { 626 + .pgsize_bitmap = SZ_4K | SZ_2M, 627 + .ias = FIELD_GET(0xff, pfdev->features.mmu_features), 628 + .oas = FIELD_GET(0xff00, pfdev->features.mmu_features), 629 + .coherent_walk = pfdev->coherent, 630 + .tlb = &mmu_tlb_ops, 631 + .iommu_dev = pfdev->dev, 632 + }; 633 + 634 + mmu->pgtbl_ops = alloc_io_pgtable_ops(ARM_MALI_LPAE, &mmu->pgtbl_cfg, 635 + mmu); 636 + if (!mmu->pgtbl_ops) { 637 + kfree(mmu); 638 + return ERR_PTR(-EINVAL); 639 + } 640 + 641 + kref_init(&mmu->refcount); 642 + 643 + return mmu; 498 644 } 499 645 500 646 static const char *access_type_name(struct panfrost_device *pfdev,
+3 -2
drivers/gpu/drm/panfrost/panfrost_mmu.h
··· 18 18 u32 panfrost_mmu_as_get(struct panfrost_device *pfdev, struct panfrost_mmu *mmu); 19 19 void panfrost_mmu_as_put(struct panfrost_device *pfdev, struct panfrost_mmu *mmu); 20 20 21 - int panfrost_mmu_pgtable_alloc(struct panfrost_file_priv *priv); 22 - void panfrost_mmu_pgtable_free(struct panfrost_file_priv *priv); 21 + struct panfrost_mmu *panfrost_mmu_ctx_get(struct panfrost_mmu *mmu); 22 + void panfrost_mmu_ctx_put(struct panfrost_mmu *mmu); 23 + struct panfrost_mmu *panfrost_mmu_ctx_create(struct panfrost_device *pfdev); 23 24 24 25 #endif