dri-devel July 2021

dri-devel@lists.freedesktop.org

321 participants
829 discussions

[PATCH] drm/amdgpu: initialize amdgpu_ras_query_error_count() error count parameters
by trix＠redhat.com 06 Jul '21

06 Jul '21

From: Tom Rix <trix(a)redhat.com> Static analysis reports this problem amdgpu_ras.c:2324:2: warning: 2nd function call argument is an uninitialized value atomic_set(&con->ras_ce_count, ce_count); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ce_count is normally set by the earlier call to amdgpu_ras_query_error_count(). But amdgpu_ras_query_error_count() can return early without setting, leaving its error count parameters in a garbage state. Initialize the error count parameters earlier. Fixes: a46751fbcde5 ("drm/amdgpu: Fix RAS function interface") Signed-off-by: Tom Rix <trix(a)redhat.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c index 875874ea745ec..c80fa545aa2b8 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c @@ -1056,6 +1056,12 @@ void amdgpu_ras_query_error_count(struct amdgpu_device *adev, struct ras_manager *obj; unsigned long ce, ue; + if (ce_count) + *ce_count = 0; + + if (ue_count) + *ue_count = 0; + if (!adev->ras_enabled || !con) return; -- 2.26.3

3 4

[PATCH] drm/vmwgfx: Convert to Linux IRQ interfaces
by Thomas Zimmermann 06 Jul '21

06 Jul '21

Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers don't benefit from using it. Vmwgfx already uses Linux IRQ functions. All that's left to replace is the reference to struct drm_device.irq. Use irq value of struct pci_dev instead. Signed-off-by: Thomas Zimmermann <tzimmermann(a)suse.de> --- drivers/gpu/drm/vmwgfx/vmwgfx_irq.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_irq.c b/drivers/gpu/drm/vmwgfx/vmwgfx_irq.c index 4b82f5995452..c5191de365ca 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_irq.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_irq.c @@ -25,6 +25,7 @@ * **************************************************************************/ +#include <linux/pci.h> #include <linux/sched/signal.h> #include "vmwgfx_drv.h" @@ -287,6 +288,7 @@ static void vmw_irq_preinstall(struct drm_device *dev) void vmw_irq_uninstall(struct drm_device *dev) { struct vmw_private *dev_priv = vmw_priv(dev); + struct pci_dev *pdev = to_pci_dev(dev->dev); uint32_t status; if (!(dev_priv->capabilities & SVGA_CAP_IRQMASK)) @@ -297,7 +299,7 @@ void vmw_irq_uninstall(struct drm_device *dev) status = vmw_irq_status_read(dev_priv); vmw_irq_status_write(dev_priv, status); - free_irq(dev->irq, dev); + free_irq(pdev->irq, dev); } /** @@ -309,16 +311,8 @@ void vmw_irq_uninstall(struct drm_device *dev) */ int vmw_irq_install(struct drm_device *dev, int irq) { - int ret; - vmw_irq_preinstall(dev); - ret = request_threaded_irq(irq, vmw_irq_handler, vmw_thread_fn, - IRQF_SHARED, VMWGFX_DRIVER_NAME, dev); - if (ret < 0) - return ret; - - dev->irq = irq; - - return ret; + return request_threaded_irq(irq, vmw_irq_handler, vmw_thread_fn, + IRQF_SHARED, VMWGFX_DRIVER_NAME, dev); } -- 2.32.0

2 1

[PATCH v7 0/5] drm: address potential UAF bugs with drm_master ptrs
by Desmond Cheong Zhi Xi 06 Jul '21

06 Jul '21

This patch series addresses potential use-after-free errors when dereferencing pointers to struct drm_master. These were identified after one such bug was caught by Syzbot in drm_getunique(): https://syzkaller.appspot.com/bug?id=148d2f1dfac64af52ffd27b661981a540724f8… The series is broken up into five patches: 1. Move a call to drm_is_current_master() out from a section locked by &dev->mode_config.mutex in drm_mode_getconnector(). This patch does not apply to stable. 2. Move a call to _drm_lease_held() out from the section locked by &dev->mode_config.idr_mutex in __drm_mode_object_find(). 3. Implement a locked version of drm_is_current_master() function that's used within drm_auth.c. 4. Serialize drm_file.master by introducing a new lock that's held whenever the value of drm_file.master changes. 5. Identify areas in drm_lease.c where pointers to struct drm_master are dereferenced, and ensure that the master pointers are not freed during use. Changes in v6 -> v7: - Patch 2: Modify code alignment as suggested by the intel-gfx CI. Update commit message based on the changes to patch 5. - Patch 4: Add patch 4 to the series. This patch adds a new lock to serialize drm_file.master, in response to the lockdep splat by the intel-gfx CI. - Patch 5: Move kerneldoc comment about protecting drm_file.master with drm_device.master_mutex into patch 4. Update drm_file_get_master to use the new drm_file.master_lock instead of drm_device.master_mutex, in response to the lockdep splat by the intel-gfx CI. Changes in v5 -> v6: - Patch 2: Add patch 2 to the series. This patch moves the call to _drm_lease_held out from the section locked by &dev->mode_config.idr_mutex in __drm_mode_object_find. - Patch 5: Clarify the kerneldoc for dereferencing drm_file.master, as suggested by Daniel Vetter. Refactor error paths with goto labels so that each function only has a single drm_master_put(), as suggested by Emil Velikov. Modify comparison to NULL into "!master", as suggested by the intel-gfx CI. Changes in v4 -> v5: - Patch 1: Add patch 1 to the series. The changes in patch 1 do not apply to stable because they apply to new changes in the drm-misc-next branch. This patch moves the call to drm_is_current_master in drm_mode_getconnector out from the section locked by &dev->mode_config.mutex. Additionally, added a missing semicolon to the patch, caught by the intel-gfx CI. - Patch 3: Move changes to drm_connector.c into patch 1. Changes in v3 -> v4: - Patch 3: Move the call to drm_is_current_master in drm_mode_getconnector out from the section locked by &dev->mode_config.mutex. As suggested by Daniel Vetter. This avoids a circular lock lock dependency as reported here https://patchwork.freedesktop.org/patch/440406/ Additionally, inside drm_is_current_master, instead of grabbing &fpriv->master->dev->master_mutex, we grab &fpriv->minor->dev->master_mutex to avoid dereferencing a null ptr if fpriv->master is not set. - Patch 5: Modify kerneldoc formatting. Additionally, add a file_priv->master NULL check inside drm_file_get_master, and handle the NULL result accordingly in drm_lease.c. As suggested by Daniel Vetter. Changes in v2 -> v3: - Patch 3: Move the definition of drm_is_current_master and the _locked version higher up in drm_auth.c to avoid needing a forward declaration of drm_is_current_master_locked. As suggested by Daniel Vetter. - Patch 5: Instead of leaking drm_device.master_mutex into drm_lease.c to protect drm_master pointers, add a new drm_file_get_master() function that returns drm_file->master while increasing its reference count, to prevent drm_file->master from being freed. As suggested by Daniel Vetter. Changes in v1 -> v2: - Patch 5: Move the lock and assignment before the DRM_DEBUG_LEASE in drm_mode_get_lease_ioctl, as suggested by Emil Velikov. Desmond Cheong Zhi Xi (5): drm: avoid circular locks in drm_mode_getconnector drm: separate locks in __drm_mode_object_find drm: add a locked version of drm_is_current_master drm: serialize drm_file.master with a master lock drm: protect drm_master pointers in drm_lease.c drivers/gpu/drm/drm_auth.c | 86 +++++++++++++++++++++++-------- drivers/gpu/drm/drm_connector.c | 5 +- drivers/gpu/drm/drm_file.c | 1 + drivers/gpu/drm/drm_lease.c | 81 ++++++++++++++++++++++------- drivers/gpu/drm/drm_mode_object.c | 10 ++-- include/drm/drm_auth.h | 1 + include/drm/drm_file.h | 18 +++++-- 7 files changed, 153 insertions(+), 49 deletions(-) -- 2.25.1

2 9

Re: [PATCH v5 3/5] drm/msm: Improve the a6xx page fault handler
by Yassine Oudjana 06 Jul '21

06 Jul '21

In-Reply-To: <20210610214431.539029-4-robdclark(a)gmail.com> On Thu 10 Jun 16:44 CDT 2021, Rob Clark wrote: > From: Jordan Crouse <jcrouse(a)codeaurora.org> > > Use the new adreno-smmu-priv fault info function to get more SMMU > debug registers and print the current TTBR0 to debug per-instance > pagetables and figure out which GPU block generated the request. > > Signed-off-by: Jordan Crouse <jcrouse(a)codeaurora.org> > Signed-off-by: Rob Clark <robdclark(a)chromium.org> > --- > drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 4 +- > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 76 +++++++++++++++++++++++++-- > drivers/gpu/drm/msm/msm_iommu.c | 11 +++- > drivers/gpu/drm/msm/msm_mmu.h | 4 +- > 4 files changed, 87 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > index f46562c12022..eb030b00bff4 100644 > --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > @@ -1075,7 +1075,7 @@ bool a5xx_idle(struct msm_gpu *gpu, struct msm_ringbuffer *ring) > return true; > } > > -static int a5xx_fault_handler(void *arg, unsigned long iova, int flags) > +static int a5xx_fault_handler(void *arg, unsigned long iova, int flags, void *data) > { > struct msm_gpu *gpu = arg; > pr_warn_ratelimited("*** gpu fault: iova=%08lx, flags=%d (%u,%u,%u,%u)\n", > @@ -1085,7 +1085,7 @@ static int a5xx_fault_handler(void *arg, unsigned long iova, int flags) > gpu_read(gpu, REG_A5XX_CP_SCRATCH_REG(6)), > gpu_read(gpu, REG_A5XX_CP_SCRATCH_REG(7))); > > - return -EFAULT; > + return 0; > } > > static void a5xx_cp_err_irq(struct msm_gpu *gpu) > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > index c7f0ddb12d8f..fc19db10bff1 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > @@ -1032,18 +1032,88 @@ static void a6xx_recover(struct msm_gpu *gpu) > msm_gpu_hw_init(gpu); > } > > -static int a6xx_fault_handler(void *arg, unsigned long iova, int flags) > +static const char *a6xx_uche_fault_block(struct msm_gpu *gpu, u32 mid) > +{ > + static const char *uche_clients[7] = { > + "VFD", "SP", "VSC", "VPC", "HLSQ", "PC", "LRZ", > + }; > + u32 val; > + > + if (mid < 1 || mid > 3) > + return "UNKNOWN"; > + > + /* > + * The source of the data depends on the mid ID read from FSYNR1. > + * and the client ID read from the UCHE block > + */ > + val = gpu_read(gpu, REG_A6XX_UCHE_CLIENT_PF); > + > + /* mid = 3 is most precise and refers to only one block per client */ > + if (mid == 3) > + return uche_clients[val & 7]; > + > + /* For mid=2 the source is TP or VFD except when the client id is 0 */ > + if (mid == 2) > + return ((val & 7) == 0) ? "TP" : "TP|VFD"; > + > + /* For mid=1 just return "UCHE" as a catchall for everything else */ > + return "UCHE"; > +} > + > +static const char *a6xx_fault_block(struct msm_gpu *gpu, u32 id) > +{ > + if (id == 0) > + return "CP"; > + else if (id == 4) > + return "CCU"; > + else if (id == 6) > + return "CDP Prefetch"; > + > + return a6xx_uche_fault_block(gpu, id); > +} > + > +#define ARM_SMMU_FSR_TF BIT(1) > +#define ARM_SMMU_FSR_PF BIT(3) > +#define ARM_SMMU_FSR_EF BIT(4) > + > +static int a6xx_fault_handler(void *arg, unsigned long iova, int flags, void *data) > { > struct msm_gpu *gpu = arg; > + struct adreno_smmu_fault_info *info = data; > + const char *type = "UNKNOWN"; > > - pr_warn_ratelimited("*** gpu fault: iova=%08lx, flags=%d (%u,%u,%u,%u)\n", > + /* > + * Print a default message if we couldn't get the data from the > + * adreno-smmu-priv > + */ > + if (!info) { > + pr_warn_ratelimited("*** gpu fault: iova=%.16lx flags=%d (%u,%u,%u,%u)\n", > iova, flags, > gpu_read(gpu, REG_A6XX_CP_SCRATCH_REG(4)), > gpu_read(gpu, REG_A6XX_CP_SCRATCH_REG(5)), > gpu_read(gpu, REG_A6XX_CP_SCRATCH_REG(6)), > gpu_read(gpu, REG_A6XX_CP_SCRATCH_REG(7))); > > - return -EFAULT; > + return 0; > + } > + > + if (info->fsr & ARM_SMMU_FSR_TF) > + type = "TRANSLATION"; > + else if (info->fsr & ARM_SMMU_FSR_PF) > + type = "PERMISSION"; > + else if (info->fsr & ARM_SMMU_FSR_EF) > + type = "EXTERNAL"; > + > + pr_warn_ratelimited("*** gpu fault: ttbr0=%.16llx iova=%.16lx dir=%s type=%s source=%s (%u,%u,%u,%u)\n", > + info->ttbr0, iova, > + flags & IOMMU_FAULT_WRITE ? "WRITE" : "READ", type, > + a6xx_fault_block(gpu, info->fsynr1 & 0xff), > + gpu_read(gpu, REG_A6XX_CP_SCRATCH_REG(4)), > + gpu_read(gpu, REG_A6XX_CP_SCRATCH_REG(5)), > + gpu_read(gpu, REG_A6XX_CP_SCRATCH_REG(6)), > + gpu_read(gpu, REG_A6XX_CP_SCRATCH_REG(7))); > + > + return 0; > } > > static void a6xx_cp_hw_err_irq(struct msm_gpu *gpu) > diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c > index 50d881794758..6975b95c3c29 100644 > --- a/drivers/gpu/drm/msm/msm_iommu.c > +++ b/drivers/gpu/drm/msm/msm_iommu.c > @@ -211,8 +211,17 @@ static int msm_fault_handler(struct iommu_domain *domain, struct device *dev, > unsigned long iova, int flags, void *arg) > { > struct msm_iommu *iommu = arg; > + struct adreno_smmu_priv *adreno_smmu = dev_get_drvdata(iommu->base.dev); > + struct adreno_smmu_fault_info info, *ptr = NULL; > + > + if (adreno_smmu->get_fault_info) { > + adreno_smmu->get_fault_info(adreno_smmu->cookie, &info); Calling this on msm8996 causes a panic: [ 12.098593] [drm] Initialized msm 1.8.0 20130625 for 900000.mdss on minor 0 [ 12.115952] msm 900000.mdss: [drm:adreno_request_fw [msm]] loaded qcom/a530_pm4.fw from new location [ 12.117173] msm 900000.mdss: [drm:adreno_request_fw [msm]] loaded qcom/a530_pfp.fw from new location [ 12.118040] msm 900000.mdss: [drm:adreno_request_fw [msm]] loaded qcom/a530v3_gpmu.fw2 from new location [ 12.159807] fb0: switching to msm from simple [ 12.345745] q6asm-dai 9300000.remoteproc:smd-edge:apr:q6asm:dais: Adding to iommu group 3 [ 12.710132] Console: switching to colour dummy device 80x25 [ 12.967929] Unable to handle kernel execute from non-executable memory at virtual address ffff00008340a200 [ 12.967949] Mem abort info: [ 12.967952] ESR = 0x8600000f [ 12.967956] EC = 0x21: IABT (current EL), IL = 32 bits [ 12.967962] SET = 0, FnV = 0 [ 12.967965] EA = 0, S1PTW = 0 [ 12.967969] FSC = 0x0f: level 3 permission fault [ 12.967974] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000080ed2000 [ 12.967980] [ffff00008340a200] pgd=180000017e548003, p4d=180000017e548003, pud=180000017e1b5003, pmd=180000017e19a003, pte=006800010340af07 [ 12.968004] Internal error: Oops: 8600000f [#1] PREEMPT SMP [ 12.968011] Modules linked in: q6asm_dai q6routing q6afe_dai q6adm q6asm q6dsp_common q6afe q6core venus_enc venus_dec videobuf2_dma_contig videobuf2_memops panel_lgphilips_sw43101 apr ath10k_pci ath10k_core ath mac80211 libarc4 cfg80211 venus_core v4l2_mem2mem videobuf2_v4l2 videobuf2_common videodev mc hci_uart nxp_nci_i2c nxp_nci btqca nci bluetooth nfc rfkill msm pmi8998_haptics qcom_fg drm_kms_helper syscopyarea slim_qcom_ngd_ctrl sysfillrect pdr_interface sysimgblt fb_sys_fops qcom_q6v5_mss qcom_q6v5_pas qcom_pil_info qcom_q6v5 qcom_sysmon qcom_common qmi_helpers mdt_loader snd_soc_apq8096 snd_soc_qcom_common socinfo rpmsg_char pwm_ir_tx rmtfs_mem snd_soc_wcd9335 regmap_slimbus snd_soc_core snd_compress slimbus snd_pcm snd_timer snd soundcore atmel_mxt_ts drm drm_panel_orientation_quirks [ 12.968248] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 5.13.0+ #46 [ 12.968255] Hardware name: Xiaomi Mi Note 2 (DT) [ 12.968261] pstate: 400000c5 (nZcv daIF -PAN -UAO -TCO BTYPE=--) [ 12.968268] pc : 0xffff00008340a200 [ 12.968277] lr : msm_fault_handler+0x50/0xd0 [msm] [ 12.968381] sp : ffff800010003e30 [ 12.968384] x29: ffff800010003e30 x28: ffff8000110c2ac0 x27: 0000000000000000 [ 12.968398] x26: ffff800010d8c6e0 x25: ffff800000000000 x24: 0000000083401000 [ 12.968412] x23: 0000000000000021 x22: ffff00008eeb2358 x21: 0000000000000000 [ 12.968426] x20: 0000000083401000 x19: ffff00008c5a9780 x18: 0000000000000001 [ 12.968439] x17: ffff8000ecd8b000 x16: ffff800010004000 x15: 0000000000004000 [ 12.968453] x14: 002093508d830af8 x13: 00000000000046aa x12: 0000000000000040 [ 12.968466] x11: ffff00008044a480 x10: ffff00008044a482 x9 : ffff00008001f068 [ 12.968479] x8 : ffff000080450028 x7 : 0000000000000000 x6 : ffff000080450128 [ 12.968492] x5 : ffff800008db1d60 x4 : ffff00008c5a9780 x3 : 0000000000000000 [ 12.968505] x2 : ffff00008340a200 x1 : ffff800010003e60 x0 : ffff800008dda9b0 [ 12.968519] Call trace: [ 12.968524] 0xffff00008340a200 [ 12.968529] report_iommu_fault+0x20/0x3c [ 12.968543] arm_smmu_context_fault+0x120/0x24c [ 12.968550] __handle_irq_event_percpu+0x54/0x170 [ 12.968559] handle_irq_event+0x64/0x140 [ 12.968566] handle_fasteoi_irq+0xa4/0x1a0 [ 12.968574] handle_domain_irq+0x60/0x90 [ 12.968581] gic_handle_irq+0xb8/0x140 [ 12.968590] call_on_irq_stack+0x2c/0x5c [ 12.968599] do_interrupt_handler+0x54/0x60 [ 12.968606] el1_interrupt+0x30/0x80 [ 12.968615] el1h_64_irq_handler+0x18/0x24 [ 12.968621] el1h_64_irq+0x78/0x7c [ 12.968626] cpuidle_enter_state+0x12c/0x2f0 [ 12.968638] cpuidle_enter+0x38/0x50 [ 12.968645] do_idle+0x21c/0x2ac [ 12.968655] cpu_startup_entry+0x28/0x80 [ 12.968661] rest_init+0xe4/0xf4 [ 12.968668] arch_call_rest_init+0x10/0x1c [ 12.968676] start_kernel+0x64c/0x68c [ 12.968682] __primary_switched+0xc0/0xc8 [ 12.968692] Code: 00000000 00000000 00000000 00000000 (08debf98) [ 12.968705] ---[ end trace f35e302241530712 ]--- [ 12.975177] Kernel panic - not syncing: Oops: Fatal exception in interrupt [ 12.975188] SMP: stopping secondary CPUs [ 13.175353] Kernel Offset: 0x80000 from 0xffff800010000000 [ 13.175359] PHYS_OFFSET: 0x80000000 [ 13.175363] CPU features: 0x0c000251,20000842 [ 13.175370] Memory Limit: none It happens when I would usually get context faults caused by reserving continuous splash memory for simplefb, like these: [ 7.830005] arm-smmu d00000.iommu: Unhandled context fault: fsr=0x402, iova=0x834e0d00, fsynr=0x21, cbfrsynra=0x0, cb=0 [ 7.836573] arm-smmu d00000.iommu: Unhandled context fault: fsr=0x402, iova=0x8374e300, fsynr=0x21, cbfrsynra=0x0, cb=0 [ 7.843071] arm-smmu d00000.iommu: Unhandled context fault: fsr=0x402, iova=0x839bca00, fsynr=0x21, cbfrsynra=0x0, cb=0 [ 7.849514] arm-smmu d00000.iommu: Unhandled context fault: fsr=0x402, iova=0x835eab00, fsynr=0x21, cbfrsynra=0x0, cb=0 [ 7.855912] arm-smmu d00000.iommu: Unhandled context fault: fsr=0x402, iova=0x8384df00, fsynr=0x21, cbfrsynra=0x0, cb=0 [ 7.862270] arm-smmu d00000.iommu: Unhandled context fault: fsr=0x402, iova=0x83471f00, fsynr=0x21, cbfrsynra=0x0, cb=0 [ 7.868568] arm-smmu d00000.iommu: Unhandled context fault: fsr=0x402, iova=0x836ccf00, fsynr=0x21, cbfrsynra=0x0, cb=0 [ 7.874846] arm-smmu d00000.iommu: Unhandled context fault: fsr=0x402, iova=0x83923900, fsynr=0x21, cbfrsynra=0x0, cb=0 [ 7.881100] arm-smmu d00000.iommu: Unhandled context fault: fsr=0x402, iova=0x8353dd00, fsynr=0x21, cbfrsynra=0x0, cb=0 [ 7.887329] arm-smmu d00000.iommu: Unhandled context fault: fsr=0x402, iova=0x8378f800, fsynr=0x21, cbfrsynra=0x0, cb=0 But now I get a panic instead. Removing the memory reservation stops it from panicking, although I do not think it should panic when getting any fault. > + ptr = &info; > + } > + > if (iommu->base.handler) > - return iommu->base.handler(iommu->base.arg, iova, flags); > + return iommu->base.handler(iommu->base.arg, iova, flags, ptr); > + > pr_warn_ratelimited("*** fault: iova=%16lx, flags=%d\n", iova, flags); > return 0; > } > diff --git a/drivers/gpu/drm/msm/msm_mmu.h b/drivers/gpu/drm/msm/msm_mmu.h > index 61ade89d9e48..a88f44c3268d 100644 > --- a/drivers/gpu/drm/msm/msm_mmu.h > +++ b/drivers/gpu/drm/msm/msm_mmu.h > @@ -26,7 +26,7 @@ enum msm_mmu_type { > struct msm_mmu { > const struct msm_mmu_funcs *funcs; > struct device *dev; > - int (*handler)(void *arg, unsigned long iova, int flags); > + int (*handler)(void *arg, unsigned long iova, int flags, void *data); > void *arg; > enum msm_mmu_type type; > }; > @@ -43,7 +43,7 @@ struct msm_mmu *msm_iommu_new(struct device *dev, struct iommu_domain *domain); > struct msm_mmu *msm_gpummu_new(struct device *dev, struct msm_gpu *gpu); > > static inline void msm_mmu_set_fault_handler(struct msm_mmu *mmu, void *arg, > - int (*handler)(void *arg, unsigned long iova, int flags)) > + int (*handler)(void *arg, unsigned long iova, int flags, void *data)) > { > mmu->arg = arg; > mmu->handler = handler; > -- > 2.31.1

1 0

Re: [PATCH v15 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
by Will Deacon 06 Jul '21

06 Jul '21

On Tue, Jul 06, 2021 at 06:48:48AM +0200, Christoph Hellwig wrote: > On Mon, Jul 05, 2021 at 08:03:52PM +0100, Will Deacon wrote: > > So at this point, the AMD IOMMU driver does: > > > > swiotlb = (iommu_default_passthrough() || sme_me_mask) ? 1 : 0; > > > > where 'swiotlb' is a global variable indicating whether or not swiotlb > > is in use. It's picked up a bit later on by pci_swiotlb_late_init(), which > > will call swiotlb_exit() if 'swiotlb' is false. > > > > Now, that used to work fine, because swiotlb_exit() clears > > 'io_tlb_default_mem' to NULL, but now with the restricted DMA changes, I > > think that all the devices which have successfully probed beforehand will > > have stale pointers to the freed structure in their 'dev->dma_io_tlb_mem' > > field. > > Yeah. I don't think we can do that anymore, and I also think it is > a bad idea to start with. I've had a crack at reworking things along the following lines: - io_tlb_default_mem now lives in the BSS, the flexible array member is now a pointer and that part is allocated dynamically (downside of this is an extra indirection to get at the slots). - io_tlb_default_mem.nslabs tells you whether the thing is valid - swiotlb_exit() frees the slots array and clears the rest of the structure to 0. I also extended it to free the actual slabs, but I'm not sure why it wasn't doing that before. So a non-NULL dev->dma_io_tlb_mem should always be valid to follow. Untested diff below... Nathan, it would be ace if you're brave enough to give this a shot. Will --->8 diff --git a/drivers/base/core.c b/drivers/base/core.c index bbad7c559901..9e1218f89e4b 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -2820,7 +2820,7 @@ void device_initialize(struct device *dev) dev->dma_coherent = dma_default_coherent; #endif #ifdef CONFIG_SWIOTLB - dev->dma_io_tlb_mem = io_tlb_default_mem; + dev->dma_io_tlb_mem = &io_tlb_default_mem; #endif } EXPORT_SYMBOL_GPL(device_initialize); diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c index 785ec7e8be01..f06d9b4f1e0f 100644 --- a/drivers/xen/swiotlb-xen.c +++ b/drivers/xen/swiotlb-xen.c @@ -164,7 +164,7 @@ int __ref xen_swiotlb_init(void) int rc = -ENOMEM; char *start; - if (io_tlb_default_mem != NULL) { + if (io_tlb_default_mem.nslabs) { pr_warn("swiotlb buffer already initialized\n"); return -EEXIST; } @@ -547,7 +547,7 @@ xen_swiotlb_sync_sg_for_device(struct device *dev, struct scatterlist *sgl, static int xen_swiotlb_dma_supported(struct device *hwdev, u64 mask) { - return xen_phys_to_dma(hwdev, io_tlb_default_mem->end - 1) <= mask; + return xen_phys_to_dma(hwdev, io_tlb_default_mem.end - 1) <= mask; } const struct dma_map_ops xen_swiotlb_dma_ops = { diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h index 39284ff2a6cd..b0cb2a9973f4 100644 --- a/include/linux/swiotlb.h +++ b/include/linux/swiotlb.h @@ -103,9 +103,9 @@ struct io_tlb_mem { phys_addr_t orig_addr; size_t alloc_size; unsigned int list; - } slots[]; + } *slots; }; -extern struct io_tlb_mem *io_tlb_default_mem; +extern struct io_tlb_mem io_tlb_default_mem; static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr) { diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c index 0ffbaae9fba2..91cd1d413027 100644 --- a/kernel/dma/swiotlb.c +++ b/kernel/dma/swiotlb.c @@ -70,7 +70,7 @@ enum swiotlb_force swiotlb_force; -struct io_tlb_mem *io_tlb_default_mem; +struct io_tlb_mem io_tlb_default_mem; /* * Max segment that we can provide which (if pages are contingous) will @@ -101,7 +101,7 @@ early_param("swiotlb", setup_io_tlb_npages); unsigned int swiotlb_max_segment(void) { - return io_tlb_default_mem ? max_segment : 0; + return io_tlb_default_mem.nslabs ? max_segment : 0; } EXPORT_SYMBOL_GPL(swiotlb_max_segment); @@ -134,9 +134,9 @@ void __init swiotlb_adjust_size(unsigned long size) void swiotlb_print_info(void) { - struct io_tlb_mem *mem = io_tlb_default_mem; + struct io_tlb_mem *mem = &io_tlb_default_mem; - if (!mem) { + if (!mem->nslabs) { pr_warn("No low mem\n"); return; } @@ -163,11 +163,11 @@ static inline unsigned long nr_slots(u64 val) */ void __init swiotlb_update_mem_attributes(void) { - struct io_tlb_mem *mem = io_tlb_default_mem; + struct io_tlb_mem *mem = &io_tlb_default_mem; void *vaddr; unsigned long bytes; - if (!mem || mem->late_alloc) + if (!mem->nslabs || mem->late_alloc) return; vaddr = phys_to_virt(mem->start); bytes = PAGE_ALIGN(mem->nslabs << IO_TLB_SHIFT); @@ -201,25 +201,24 @@ static void swiotlb_init_io_tlb_mem(struct io_tlb_mem *mem, phys_addr_t start, int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose) { - struct io_tlb_mem *mem; + struct io_tlb_mem *mem = &io_tlb_default_mem; size_t alloc_size; if (swiotlb_force == SWIOTLB_NO_FORCE) return 0; /* protect against double initialization */ - if (WARN_ON_ONCE(io_tlb_default_mem)) + if (WARN_ON_ONCE(mem->nslabs)) return -ENOMEM; - alloc_size = PAGE_ALIGN(struct_size(mem, slots, nslabs)); - mem = memblock_alloc(alloc_size, PAGE_SIZE); - if (!mem) + alloc_size = PAGE_ALIGN(array_size(sizeof(*mem->slots), nslabs)); + mem->slots = memblock_alloc(alloc_size, PAGE_SIZE); + if (!mem->slots) panic("%s: Failed to allocate %zu bytes align=0x%lx\n", __func__, alloc_size, PAGE_SIZE); swiotlb_init_io_tlb_mem(mem, __pa(tlb), nslabs, false); - io_tlb_default_mem = mem; if (verbose) swiotlb_print_info(); swiotlb_set_max_segment(mem->nslabs << IO_TLB_SHIFT); @@ -304,26 +303,24 @@ swiotlb_late_init_with_default_size(size_t default_size) int swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs) { - struct io_tlb_mem *mem; + struct io_tlb_mem *mem = &io_tlb_default_mem; unsigned long bytes = nslabs << IO_TLB_SHIFT; if (swiotlb_force == SWIOTLB_NO_FORCE) return 0; /* protect against double initialization */ - if (WARN_ON_ONCE(io_tlb_default_mem)) + if (WARN_ON_ONCE(mem->nslabs)) return -ENOMEM; - mem = (void *)__get_free_pages(GFP_KERNEL, - get_order(struct_size(mem, slots, nslabs))); - if (!mem) + mem->slots = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, + get_order(array_size(sizeof(*mem->slots), nslabs))); + if (!mem->slots) return -ENOMEM; - memset(mem, 0, sizeof(*mem)); set_memory_decrypted((unsigned long)tlb, bytes >> PAGE_SHIFT); swiotlb_init_io_tlb_mem(mem, virt_to_phys(tlb), nslabs, true); - io_tlb_default_mem = mem; swiotlb_print_info(); swiotlb_set_max_segment(mem->nslabs << IO_TLB_SHIFT); return 0; @@ -331,18 +328,23 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs) void __init swiotlb_exit(void) { - struct io_tlb_mem *mem = io_tlb_default_mem; - size_t size; + struct io_tlb_mem *mem = &io_tlb_default_mem; + size_t tbl_size, slots_size; - if (!mem) + if (!mem->nslabs) return; - size = struct_size(mem, slots, mem->nslabs); - if (mem->late_alloc) - free_pages((unsigned long)mem, get_order(size)); - else - memblock_free_late(__pa(mem), PAGE_ALIGN(size)); - io_tlb_default_mem = NULL; + tbl_size = mem->end - mem->start; + slots_size = array_size(sizeof(*mem->slots), mem->nslabs); + if (mem->late_alloc) { + free_pages((unsigned long)mem->start, get_order(tbl_size)); + free_pages((unsigned long)mem->slots, get_order(slots_size)); + } else { + memblock_free_late(__pa(mem->start), PAGE_ALIGN(tbl_size)); + memblock_free_late(__pa(mem->slots), PAGE_ALIGN(slots_size)); + } + + memset(mem, 0, sizeof(*mem)); } /* @@ -682,7 +684,9 @@ size_t swiotlb_max_mapping_size(struct device *dev) bool is_swiotlb_active(struct device *dev) { - return dev->dma_io_tlb_mem != NULL; + struct io_tlb_mem *mem = dev->dma_io_tlb_mem; + + return mem && mem->nslabs; } EXPORT_SYMBOL_GPL(is_swiotlb_active); @@ -697,10 +701,10 @@ static void swiotlb_create_debugfs_files(struct io_tlb_mem *mem) static int __init swiotlb_create_default_debugfs(void) { - struct io_tlb_mem *mem = io_tlb_default_mem; + struct io_tlb_mem *mem = &io_tlb_default_mem; debugfs_dir = debugfs_create_dir("swiotlb", NULL); - if (mem) { + if (mem->nslabs) { mem->debugfs = debugfs_dir; swiotlb_create_debugfs_files(mem); } @@ -754,10 +758,17 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem, * to it. */ if (!mem) { - mem = kzalloc(struct_size(mem, slots, nslabs), GFP_KERNEL); + mem = kzalloc(sizeof(*mem), GFP_KERNEL); if (!mem) return -ENOMEM; + mem->slots = kzalloc(array_size(sizeof(*mem->slots), nslabs), + GFP_KERNEL); + if (!mem->slots) { + kfree(mem); + return -ENOMEM; + } + set_memory_decrypted((unsigned long)phys_to_virt(rmem->base), rmem->size >> PAGE_SHIFT); swiotlb_init_io_tlb_mem(mem, rmem->base, nslabs, false); @@ -781,7 +792,7 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem, static void rmem_swiotlb_device_release(struct reserved_mem *rmem, struct device *dev) { - dev->dma_io_tlb_mem = io_tlb_default_mem; + dev->dma_io_tlb_mem = &io_tlb_default_mem; } static const struct reserved_mem_ops rmem_swiotlb_ops = {

2 1

[PATCH AUTOSEL 4.4 09/31] drm/virtio: Fix double free on probe failure
by Sasha Levin 06 Jul '21

06 Jul '21

From: Xie Yongji <xieyongji(a)bytedance.com> [ Upstream commit cec7f1774605a5ef47c134af62afe7c75c30b0ee ] The virtio_gpu_init() will free vgdev and vgdev->vbufs on failure. But such failure will be caught by virtio_gpu_probe() and then virtio_gpu_release() will be called to do some cleanup which will free vgdev and vgdev->vbufs again. So let's set dev->dev_private to NULL to avoid double free. Signed-off-by: Xie Yongji <xieyongji(a)bytedance.com> Link: http://patchwork.freedesktop.org/patch/msgid/20210517084913.403-2-xieyongji… Signed-off-by: Gerd Hoffmann <kraxel(a)redhat.com> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- drivers/gpu/drm/virtio/virtgpu_kms.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c index 88ed5e235e55..fcf9b572ec03 100644 --- a/drivers/gpu/drm/virtio/virtgpu_kms.c +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c @@ -233,6 +233,7 @@ int virtio_gpu_driver_load(struct drm_device *dev, unsigned long flags) err_vbufs: vgdev->vdev->config->del_vqs(vgdev->vdev); err_vqs: + dev->dev_private = NULL; kfree(vgdev); return ret; } -- 2.30.2

1 0

[PATCH AUTOSEL 4.9 09/35] drm/virtio: Fix double free on probe failure
by Sasha Levin 06 Jul '21

06 Jul '21

From: Xie Yongji <xieyongji(a)bytedance.com> [ Upstream commit cec7f1774605a5ef47c134af62afe7c75c30b0ee ] The virtio_gpu_init() will free vgdev and vgdev->vbufs on failure. But such failure will be caught by virtio_gpu_probe() and then virtio_gpu_release() will be called to do some cleanup which will free vgdev and vgdev->vbufs again. So let's set dev->dev_private to NULL to avoid double free. Signed-off-by: Xie Yongji <xieyongji(a)bytedance.com> Link: http://patchwork.freedesktop.org/patch/msgid/20210517084913.403-2-xieyongji… Signed-off-by: Gerd Hoffmann <kraxel(a)redhat.com> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- drivers/gpu/drm/virtio/virtgpu_kms.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c index d8b2027730d0..cde38e14232c 100644 --- a/drivers/gpu/drm/virtio/virtgpu_kms.c +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c @@ -234,6 +234,7 @@ int virtio_gpu_driver_load(struct drm_device *dev, unsigned long flags) err_vbufs: vgdev->vdev->config->del_vqs(vgdev->vdev); err_vqs: + dev->dev_private = NULL; kfree(vgdev); return ret; } -- 2.30.2

1 0

[PATCH AUTOSEL 4.9 08/35] drm/virtio: Fixes a potential NULL pointer dereference on probe failure
by Sasha Levin 06 Jul '21

06 Jul '21

From: Xie Yongji <xieyongji(a)bytedance.com> [ Upstream commit 17f46f488a5d82c5568e6e786cd760bba1c2ee09 ] The dev->dev_private might not be allocated if virtio_gpu_pci_quirk() or virtio_gpu_init() failed. In this case, we should avoid the cleanup in virtio_gpu_release(). Signed-off-by: Xie Yongji <xieyongji(a)bytedance.com> Link: http://patchwork.freedesktop.org/patch/msgid/20210517084913.403-1-xieyongji… Signed-off-by: Gerd Hoffmann <kraxel(a)redhat.com> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- drivers/gpu/drm/virtio/virtgpu_kms.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c index ba7855da7c7f..d8b2027730d0 100644 --- a/drivers/gpu/drm/virtio/virtgpu_kms.c +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c @@ -258,6 +258,9 @@ int virtio_gpu_driver_unload(struct drm_device *dev) flush_work(&vgdev->config_changed_work); vgdev->vdev->config->del_vqs(vgdev->vdev); + if (!vgdev) + return; + virtio_gpu_modeset_fini(vgdev); virtio_gpu_ttm_fini(vgdev); virtio_gpu_free_vbufs(vgdev); -- 2.30.2

1 0

[PATCH AUTOSEL 4.14 01/45] drm/etnaviv: fix NULL check before some freeing functions is not needed
by Sasha Levin 06 Jul '21

06 Jul '21

From: Tian Tao <tiantao6(a)hisilicon.com> [ Upstream commit 7d614ab2f20503ed8766363d41f8607337571adf ] fixed the below warning: drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c:84:2-8: WARNING: NULL check before some freeing functions is not needed. Signed-off-by: Tian Tao <tiantao6(a)hisilicon.com> Acked-by: Christian König <christian.koenig(a)amd.com> Signed-off-by: Lucas Stach <l.stach(a)pengutronix.de> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c index 880b95511b98..1faa3da8c517 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c @@ -86,8 +86,7 @@ static void etnaviv_gem_prime_release(struct etnaviv_gem_object *etnaviv_obj) /* Don't drop the pages for imported dmabuf, as they are not * ours, just free the array we allocated: */ - if (etnaviv_obj->pages) - kvfree(etnaviv_obj->pages); + kvfree(etnaviv_obj->pages); drm_prime_gem_destroy(&etnaviv_obj->base, etnaviv_obj->sgt); } -- 2.30.2

1 5

[PATCH AUTOSEL 4.19 01/55] drm/etnaviv: fix NULL check before some freeing functions is not needed
by Sasha Levin 06 Jul '21

06 Jul '21

From: Tian Tao <tiantao6(a)hisilicon.com> [ Upstream commit 7d614ab2f20503ed8766363d41f8607337571adf ] fixed the below warning: drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c:84:2-8: WARNING: NULL check before some freeing functions is not needed. Signed-off-by: Tian Tao <tiantao6(a)hisilicon.com> Acked-by: Christian König <christian.koenig(a)amd.com> Signed-off-by: Lucas Stach <l.stach(a)pengutronix.de> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c index f21529e635e3..dd814d42a0f9 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c @@ -75,8 +75,7 @@ static void etnaviv_gem_prime_release(struct etnaviv_gem_object *etnaviv_obj) /* Don't drop the pages for imported dmabuf, as they are not * ours, just free the array we allocated: */ - if (etnaviv_obj->pages) - kvfree(etnaviv_obj->pages); + kvfree(etnaviv_obj->pages); drm_prime_gem_destroy(&etnaviv_obj->base, etnaviv_obj->sgt); } -- 2.30.2

1 6

← Newer
1
...
74
75
76
77
78
79
80
...
83
Older →

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

dri-devel July 2021