From patchwork Mon Mar 25 13:57:03 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Boris Brezillon X-Patchwork-Id: 13602264 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E3A3EC54E64 for ; Mon, 25 Mar 2024 13:57:14 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 0625F10E8E7; Mon, 25 Mar 2024 13:57:14 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=collabora.com header.i=@collabora.com header.b="e0BCroUO"; dkim-atps=neutral Received: from madrid.collaboradmins.com (madrid.collaboradmins.com [46.235.227.194]) by gabe.freedesktop.org (Postfix) with ESMTPS id E959010E8E5 for ; Mon, 25 Mar 2024 13:57:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1711375027; bh=uy1nPMkmLUlvTTkBwlaBAEAgHVx6PQ1PwpTwAVeF0hk=; h=From:To:Cc:Subject:Date:From; b=e0BCroUO/Wh7wjP4IROW1ddmvHDXGO9ni4xm/XjFskmBCxOGLhi3jWFg1A9v3BwUj VrhBm8we0tMluNRmbqB2GYt4kM76c0IxXgAaNy5GXXPzJXxlMo6s/EmEFRFepA6oB3 Lc5Vy/lQe85WxJOZAbYl6n2yYWJfQuwRLv+rjk3ZRwTPEkfPwsWlp95TIL2pYCHXxV UIU2hQELkMSCWWbADR+W7XeRItU9wH1A1eFDHzbclTiOCHpKD/ioyRJRAcTjB/efXD anRjKB5w7q7SH2lC4sc4mYpIDAnk2FAKJWJK+Y4RfBNlhD5KNfpgDLNyaGSZAVvaj9 XGXzJ6zWaYNIg== Received: from localhost.localdomain (cola.collaboradmins.com [195.201.22.229]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: bbrezillon) by madrid.collaboradmins.com (Postfix) with ESMTPSA id D6D463782039; Mon, 25 Mar 2024 13:57:06 +0000 (UTC) From: Boris Brezillon To: Boris Brezillon , Steven Price , Liviu Dudau , =?utf-8?q?Adri=C3=A1n_Larumbe?= Cc: dri-devel@lists.freedesktop.org, kernel@collabora.com, "Lukas F . Hartmann" Subject: [PATCH v2 1/3] drm/panthor: Fix IO-page mmap() for 32-bit userspace on 64-bit kernel Date: Mon, 25 Mar 2024 14:57:03 +0100 Message-ID: <20240325135705.3717293-1-boris.brezillon@collabora.com> X-Mailer: git-send-email 2.44.0 MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" When mapping an IO region, the pseudo-file offset is dependent on the userspace architecture. panthor_device_mmio_offset() abstracts that away for us by turning a userspace MMIO offset into its kernel equivalent, but we were not updating vm_area_struct::vm_pgoff accordingly, leading us to attach the MMIO region to the wrong file offset. This has implications when we start mixing 64 bit and 32 bit apps, but that's only really a problem when we start having more that 2^43 bytes of memory allocated, which is very unlikely to happen. What's more problematic is the fact this turns our unmap_mapping_range(DRM_PANTHOR_USER_MMIO_OFFSET) calls, which are supposed to kill the MMIO mapping when entering suspend, into NOPs. Which means we either keep the dummy flush_id mapping active at all times, or we risk a BUS_FAULT if the MMIO region was mapped, and the GPU is suspended after that. Solve that by patching vm_pgoff early in panthor_mmap(). With this in place, we no longer need the panthor_device_mmio_offset() helper. v2: - Kill panthor_device_mmio_offset() Fixes: 5fe909cae118 ("drm/panthor: Add the device logical block") Reported-by: Adrián Larumbe Reported-by: Lukas F. Hartmann Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/10835 Signed-off-by: Boris Brezillon Reviewed-by: Steven Price --- drivers/gpu/drm/panthor/panthor_device.c | 8 ++++---- drivers/gpu/drm/panthor/panthor_device.h | 24 ------------------------ drivers/gpu/drm/panthor/panthor_drv.c | 17 ++++++++++++++++- 3 files changed, 20 insertions(+), 29 deletions(-) diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c index bfe8da4a6e4c..3ddc4ba0acbe 100644 --- a/drivers/gpu/drm/panthor/panthor_device.c +++ b/drivers/gpu/drm/panthor/panthor_device.c @@ -334,7 +334,7 @@ static vm_fault_t panthor_mmio_vm_fault(struct vm_fault *vmf) { struct vm_area_struct *vma = vmf->vma; struct panthor_device *ptdev = vma->vm_private_data; - u64 id = (u64)vma->vm_pgoff << PAGE_SHIFT; + u64 offset = (u64)vma->vm_pgoff << PAGE_SHIFT; unsigned long pfn; pgprot_t pgprot; vm_fault_t ret; @@ -347,7 +347,7 @@ static vm_fault_t panthor_mmio_vm_fault(struct vm_fault *vmf) mutex_lock(&ptdev->pm.mmio_lock); active = atomic_read(&ptdev->pm.state) == PANTHOR_DEVICE_PM_STATE_ACTIVE; - switch (panthor_device_mmio_offset(id)) { + switch (offset) { case DRM_PANTHOR_USER_FLUSH_ID_MMIO_OFFSET: if (active) pfn = __phys_to_pfn(ptdev->phys_addr + CSF_GPU_LATEST_FLUSH_ID); @@ -378,9 +378,9 @@ static const struct vm_operations_struct panthor_mmio_vm_ops = { int panthor_device_mmap_io(struct panthor_device *ptdev, struct vm_area_struct *vma) { - u64 id = (u64)vma->vm_pgoff << PAGE_SHIFT; + u64 offset = (u64)vma->vm_pgoff << PAGE_SHIFT; - switch (panthor_device_mmio_offset(id)) { + switch (offset) { case DRM_PANTHOR_USER_FLUSH_ID_MMIO_OFFSET: if (vma->vm_end - vma->vm_start != PAGE_SIZE || (vma->vm_flags & (VM_WRITE | VM_EXEC))) diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h index 51c9d61b6796..7ee4987a3796 100644 --- a/drivers/gpu/drm/panthor/panthor_device.h +++ b/drivers/gpu/drm/panthor/panthor_device.h @@ -365,30 +365,6 @@ static int panthor_request_ ## __name ## _irq(struct panthor_device *ptdev, \ pirq); \ } -/** - * panthor_device_mmio_offset() - Turn a user MMIO offset into a kernel one - * @offset: Offset to convert. - * - * With 32-bit systems being limited by the 32-bit representation of mmap2's - * pgoffset field, we need to make the MMIO offset arch specific. This function - * converts a user MMIO offset into something the kernel driver understands. - * - * If the kernel and userspace architecture match, the offset is unchanged. If - * the kernel is 64-bit and userspace is 32-bit, the offset is adjusted to match - * 64-bit offsets. 32-bit kernel with 64-bit userspace is impossible. - * - * Return: Adjusted offset. - */ -static inline u64 panthor_device_mmio_offset(u64 offset) -{ -#ifdef CONFIG_ARM64 - if (test_tsk_thread_flag(current, TIF_32BIT)) - offset += DRM_PANTHOR_USER_MMIO_OFFSET_64BIT - DRM_PANTHOR_USER_MMIO_OFFSET_32BIT; -#endif - - return offset; -} - extern struct workqueue_struct *panthor_cleanup_wq; #endif diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c index ff484506229f..0097a01d0fc7 100644 --- a/drivers/gpu/drm/panthor/panthor_drv.c +++ b/drivers/gpu/drm/panthor/panthor_drv.c @@ -1327,7 +1327,22 @@ static int panthor_mmap(struct file *filp, struct vm_area_struct *vma) if (!drm_dev_enter(file->minor->dev, &cookie)) return -ENODEV; - if (panthor_device_mmio_offset(offset) >= DRM_PANTHOR_USER_MMIO_OFFSET) +#ifdef CONFIG_ARM64 + /* + * With 32-bit systems being limited by the 32-bit representation of + * mmap2's pgoffset field, we need to make the MMIO offset arch + * specific. This converts a user MMIO offset into something the kernel + * driver understands. + */ + if (test_tsk_thread_flag(current, TIF_32BIT) && + offset >= DRM_PANTHOR_USER_MMIO_OFFSET_32BIT) { + offset += DRM_PANTHOR_USER_MMIO_OFFSET_64BIT - + DRM_PANTHOR_USER_MMIO_OFFSET_32BIT; + vma->vm_pgoff = offset >> PAGE_SHIFT; + } +#endif + + if (offset >= DRM_PANTHOR_USER_MMIO_OFFSET) ret = panthor_device_mmap_io(ptdev, vma); else ret = drm_gem_mmap(filp, vma); From patchwork Mon Mar 25 13:57:04 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Boris Brezillon X-Patchwork-Id: 13602263 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 6F040C54E64 for ; Mon, 25 Mar 2024 13:57:11 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 91DD010E8E5; Mon, 25 Mar 2024 13:57:10 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=collabora.com header.i=@collabora.com header.b="b8t30tkm"; dkim-atps=neutral Received: from madrid.collaboradmins.com (madrid.collaboradmins.com [46.235.227.194]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3C3F610E8E6 for ; Mon, 25 Mar 2024 13:57:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1711375028; bh=BodyKJPHcWaeZPPO56bFdnevyhdLCtg8RsbyXp7HY+s=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=b8t30tkmX1pDN33mNn7m0LGBY0CtTNfI62TwedAq2v/z8xnrQX8MkBWC96F1Qt0PJ 1+sq/hB1LCLebznDOZO5HhjWF3K72sE55TYs6Qb95rsZ3x7G8sZtpLAWI6wJdAD3gS SJcifKNJX3CKJSN6j2BJkMTzriYM3SgwJHniqavTXjayOCvVE35BrktahcRHxF956H 0EkjN3ZwSsLywv+jkURg4M8J2gddYZDMpo6M8iDyX/hgXkVKgeNXFeMnkLZ51jF91J FPRys5EcaZOYlSa+rjJFgQG4iXsQnHj+p3wGZFTVUldPBFo6Te66gR8XS8Xr49HATZ ZID3y6dH5HoSQ== Received: from localhost.localdomain (cola.collaboradmins.com [195.201.22.229]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: bbrezillon) by madrid.collaboradmins.com (Postfix) with ESMTPSA id 88058378209A; Mon, 25 Mar 2024 13:57:07 +0000 (UTC) From: Boris Brezillon To: Boris Brezillon , Steven Price , Liviu Dudau , =?utf-8?q?Adri=C3=A1n_Larumbe?= Cc: dri-devel@lists.freedesktop.org, kernel@collabora.com Subject: [PATCH v2 2/3] drm/panthor: Fix ordering in _irq_suspend() Date: Mon, 25 Mar 2024 14:57:04 +0100 Message-ID: <20240325135705.3717293-2-boris.brezillon@collabora.com> X-Mailer: git-send-email 2.44.0 In-Reply-To: <20240325135705.3717293-1-boris.brezillon@collabora.com> References: <20240325135705.3717293-1-boris.brezillon@collabora.com> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Make sure we set suspended=true last to avoid generating an irq storm in the unlikely case where an IRQ happens between the suspended=true assignment and the _INT_MASK update. v2: - New patch Reported-by: Steven Price Signed-off-by: Boris Brezillon Reviewed-by: Steven Price --- drivers/gpu/drm/panthor/panthor_device.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h index 7ee4987a3796..3a930a368ae1 100644 --- a/drivers/gpu/drm/panthor/panthor_device.h +++ b/drivers/gpu/drm/panthor/panthor_device.h @@ -325,7 +325,7 @@ static inline void panthor_ ## __name ## _irq_suspend(struct panthor_irq *pirq) { \ int cookie; \ \ - atomic_set(&pirq->suspended, true); \ + pirq->mask = 0; \ \ if (drm_dev_enter(&pirq->ptdev->base, &cookie)) { \ gpu_write(pirq->ptdev, __reg_prefix ## _INT_MASK, 0); \ @@ -333,7 +333,7 @@ static inline void panthor_ ## __name ## _irq_suspend(struct panthor_irq *pirq) drm_dev_exit(cookie); \ } \ \ - pirq->mask = 0; \ + atomic_set(&pirq->suspended, true); \ } \ \ static inline void panthor_ ## __name ## _irq_resume(struct panthor_irq *pirq, u32 mask) \ From patchwork Mon Mar 25 13:57:05 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Boris Brezillon X-Patchwork-Id: 13602265 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 568B9CD11BF for ; Mon, 25 Mar 2024 13:57:16 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 4A3F210E8E6; Mon, 25 Mar 2024 13:57:14 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=collabora.com header.i=@collabora.com header.b="xiqXG0Bf"; dkim-atps=neutral Received: from madrid.collaboradmins.com (madrid.collaboradmins.com [46.235.227.194]) by gabe.freedesktop.org (Postfix) with ESMTPS id BE83210E8E5 for ; Mon, 25 Mar 2024 13:57:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1711375028; bh=kDcBtq6heBWRHySSkn7bg8FNSQBQy5UiFLr0DykwkT8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=xiqXG0BfmK65uD8K1GQLk2BSBIuMLiH7sDi7GeRuIERbxfuqI7Vt7RNfGLHHHc2K5 JHC/HC/MvcDch6Y3kk0eT/P1H5YAigo+OlwjRkBaQhd2adiaafvjgimG7b7eYwhsxP AstD+IWgpg+f6kAnjx1SvZpvE5Iu24WX9h0lJgOiHVw12+DnsxZ0xF36UZqMR+QkEe SgL0rQ22ZXd2FeixjhW6iq2V80kkCOCIxL8ltngGyjBqylGh/KWBiE+nC0O+gpqoSp QuluRmm3niQpHjcFkOt+uIgJY2Ev0QDIFUQ5wFW9yYRlB7r40lQu49sytGg5JXUxF+ QUNlHZGtwU/kQ== Received: from localhost.localdomain (cola.collaboradmins.com [195.201.22.229]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: bbrezillon) by madrid.collaboradmins.com (Postfix) with ESMTPSA id 2D69F37820BB; Mon, 25 Mar 2024 13:57:08 +0000 (UTC) From: Boris Brezillon To: Boris Brezillon , Steven Price , Liviu Dudau , =?utf-8?q?Adri=C3=A1n_Larumbe?= Cc: dri-devel@lists.freedesktop.org, kernel@collabora.com Subject: [PATCH v2 3/3] drm/panthor: Actually suspend IRQs in the unplug path Date: Mon, 25 Mar 2024 14:57:05 +0100 Message-ID: <20240325135705.3717293-3-boris.brezillon@collabora.com> X-Mailer: git-send-email 2.44.0 In-Reply-To: <20240325135705.3717293-1-boris.brezillon@collabora.com> References: <20240325135705.3717293-1-boris.brezillon@collabora.com> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" panthor_xxx_irq_suspend() doesn't mask the interrupts if drm_dev_unplug() has been called, which is always the case when our panthor_xxx_unplug() helpers are called. Fix that by introducing a panthor_xxx_unplug() helper that does what panthor_xxx_irq_suspend() except it does it unconditionally. v2: - Add Steve's R-b Fixes: 5fe909cae118 ("drm/panthor: Add the device logical block") Signed-off-by: Boris Brezillon Reviewed-by: Steven Price --- Found inadvertently while debugging another issue. I guess I managed to call rmmod during a PING and that led to the FW interrupt handler being executed after the device suspend happened. --- drivers/gpu/drm/panthor/panthor_device.h | 8 ++++++++ drivers/gpu/drm/panthor/panthor_fw.c | 2 +- drivers/gpu/drm/panthor/panthor_gpu.c | 2 +- drivers/gpu/drm/panthor/panthor_mmu.c | 2 +- 4 files changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h index 3a930a368ae1..5634e9490c7f 100644 --- a/drivers/gpu/drm/panthor/panthor_device.h +++ b/drivers/gpu/drm/panthor/panthor_device.h @@ -321,6 +321,14 @@ static irqreturn_t panthor_ ## __name ## _irq_threaded_handler(int irq, void *da return ret; \ } \ \ +static inline void panthor_ ## __name ## _irq_unplug(struct panthor_irq *pirq) \ +{ \ + pirq->mask = 0; \ + gpu_write(pirq->ptdev, __reg_prefix ## _INT_MASK, 0); \ + synchronize_irq(pirq->irq); \ + atomic_set(&pirq->suspended, true); \ +} \ + \ static inline void panthor_ ## __name ## _irq_suspend(struct panthor_irq *pirq) \ { \ int cookie; \ diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c index 33c87a59834e..7a9710a38c5f 100644 --- a/drivers/gpu/drm/panthor/panthor_fw.c +++ b/drivers/gpu/drm/panthor/panthor_fw.c @@ -1128,7 +1128,7 @@ void panthor_fw_unplug(struct panthor_device *ptdev) /* Make sure the IRQ handler can be called after that point. */ if (ptdev->fw->irq.irq) - panthor_job_irq_suspend(&ptdev->fw->irq); + panthor_job_irq_unplug(&ptdev->fw->irq); panthor_fw_stop(ptdev); diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/drm/panthor/panthor_gpu.c index 6dbbc4cfbe7e..b84c5b650fd9 100644 --- a/drivers/gpu/drm/panthor/panthor_gpu.c +++ b/drivers/gpu/drm/panthor/panthor_gpu.c @@ -174,7 +174,7 @@ void panthor_gpu_unplug(struct panthor_device *ptdev) unsigned long flags; /* Make sure the IRQ handler is not running after that point. */ - panthor_gpu_irq_suspend(&ptdev->gpu->irq); + panthor_gpu_irq_unplug(&ptdev->gpu->irq); /* Wake-up all waiters. */ spin_lock_irqsave(&ptdev->gpu->reqs_lock, flags); diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c index fdd35249169f..1f333cdded0f 100644 --- a/drivers/gpu/drm/panthor/panthor_mmu.c +++ b/drivers/gpu/drm/panthor/panthor_mmu.c @@ -2622,7 +2622,7 @@ int panthor_vm_prepare_mapped_bos_resvs(struct drm_exec *exec, struct panthor_vm */ void panthor_mmu_unplug(struct panthor_device *ptdev) { - panthor_mmu_irq_suspend(&ptdev->mmu->irq); + panthor_mmu_irq_unplug(&ptdev->mmu->irq); mutex_lock(&ptdev->mmu->as.slots_lock); for (u32 i = 0; i < ARRAY_SIZE(ptdev->mmu->as.slots); i++) {