[Nouveau] [PATCH] MAINTAINERS: update nouveau maintainers
Since I will continue to work on Nouveau consistently, also beyond my former and still ongoing VM_BIND/EXEC work, add myself to the list of Nouveau maintainers. Signed-off-by: Danilo Krummrich --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index b19995690904..67ce91c8778a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6647,6 +6647,7 @@ F:drivers/gpu/drm/panel/panel-novatek-nt36672a.c DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS M: Karol Herbst M: Lyude Paul +M: Danilo Krummrich L: dri-de...@lists.freedesktop.org L: nouveau@lists.freedesktop.org S: Supported -- 2.41.0
Re: [Nouveau] [PATCH] drm/nouveau/kms/nv50: hide unused variables
On 9/25/23 17:59, Arnd Bergmann wrote: From: Arnd Bergmann After a recent change, two variables are only used in an #ifdef: drivers/gpu/drm/nouveau/dispnv50/disp.c: In function 'nv50_sor_atomic_disable': drivers/gpu/drm/nouveau/dispnv50/disp.c:1569:13: error: unused variable 'ret' [-Werror=unused-variable] 1569 | int ret; | ^~~ drivers/gpu/drm/nouveau/dispnv50/disp.c:1568:28: error: unused variable 'aux' [-Werror=unused-variable] 1568 | struct drm_dp_aux *aux = _connector->aux; |^~~ Move them into the same conditional block, along with the nv_connector variable that becomes unused during that fix. Fixes: 757033808c95b ("drm/nouveau/kms/nv50-: fixup sink D3 before tearing down link") Signed-off-by: Arnd Bergmann Reviewed-by: Danilo Krummrich --- drivers/gpu/drm/nouveau/dispnv50/disp.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c index 52f1569ee37c1..a0ac8c258d9ff 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c @@ -1560,15 +1560,13 @@ nv50_sor_atomic_disable(struct drm_encoder *encoder, struct drm_atomic_state *st { struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder); struct nv50_head *head = nv50_head(nv_encoder->crtc); - struct nouveau_connector *nv_connector = nv50_outp_get_old_connector(state, nv_encoder); #ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT + struct nouveau_connector *nv_connector = nv50_outp_get_old_connector(state, nv_encoder); struct nouveau_drm *drm = nouveau_drm(nv_encoder->base.base.dev); struct nouveau_backlight *backlight = nv_connector->backlight; -#endif struct drm_dp_aux *aux = _connector->aux; int ret; -#ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT if (backlight && backlight->uses_dpcd) { ret = drm_edp_backlight_disable(aux, >edp_info); if (ret < 0)
Re: [Nouveau] [PATCH 40/44] drm/nouveau/gr/r535: initial support
On 9/18/23 22:21, Ben Skeggs wrote: From: Ben Skeggs Adds support for allocating GR classes from RM. Signed-off-by: Ben Skeggs --- drivers/gpu/drm/nouveau/include/nvif/class.h | 3 + .../drm/nouveau/include/nvkm/engine/fifo.h| 1 + .../gpu/drm/nouveau/include/nvkm/engine/gr.h | 1 + .../gpu/drm/nouveau/include/nvkm/subdev/gsp.h | 5 + .../nvidia/inc/ctrl/ctrl0080/ctrl0080fifo.h | 57 ++ .../nvidia/inc/ctrl/ctrl2080/ctrl2080gpu.h| 42 ++ .../inc/ctrl/ctrl2080/ctrl2080internal.h | 19 + .../nvidia/inc/kernel/gpu/intr/engine_idx.h | 3 + .../gpu/drm/nouveau/nvkm/engine/device/base.c | 5 + .../gpu/drm/nouveau/nvkm/engine/fifo/r535.c | 34 ++ drivers/gpu/drm/nouveau/nvkm/engine/gr/Kbuild | 3 + .../gpu/drm/nouveau/nvkm/engine/gr/ad102.c| 46 ++ .../gpu/drm/nouveau/nvkm/engine/gr/ga102.c| 2 +- .../gpu/drm/nouveau/nvkm/engine/gr/gf100.h| 2 + drivers/gpu/drm/nouveau/nvkm/engine/gr/r535.c | 508 ++ .../gpu/drm/nouveau/nvkm/engine/gr/tu102.c| 2 +- .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c| 11 + 17 files changed, 742 insertions(+), 2 deletions(-) create mode 100644 drivers/gpu/drm/nouveau/include/nvrm/535.54.03/common/sdk/nvidia/inc/ctrl/ctrl0080/ctrl0080fifo.h create mode 100644 drivers/gpu/drm/nouveau/nvkm/engine/gr/ad102.c create mode 100644 drivers/gpu/drm/nouveau/nvkm/engine/gr/r535.c diff --git a/drivers/gpu/drm/nouveau/include/nvif/class.h b/drivers/gpu/drm/nouveau/include/nvif/class.h index fa94d69d3eb0..550e96db41a0 100644 --- a/drivers/gpu/drm/nouveau/include/nvif/class.h +++ b/drivers/gpu/drm/nouveau/include/nvif/class.h @@ -194,6 +194,8 @@ #define AMPERE_B /* cl9097.h */ 0xc797 +#define ADA_A /* cl9097.h */ 0xc997 + #define NV74_BSP 0x74b0 #define GT212_MSVLD 0x85b1 @@ -239,6 +241,7 @@ #define VOLTA_COMPUTE_A 0xc3c0 #define TURING_COMPUTE_A 0xc5c0 #define AMPERE_COMPUTE_B 0xc7c0 +#define ADA_COMPUTE_A0xc9c0 #define NV74_CIPHER 0x74c1 #endif diff --git a/drivers/gpu/drm/nouveau/include/nvkm/engine/fifo.h b/drivers/gpu/drm/nouveau/include/nvkm/engine/fifo.h index a26dfeece4b7..be508f65b280 100644 --- a/drivers/gpu/drm/nouveau/include/nvkm/engine/fifo.h +++ b/drivers/gpu/drm/nouveau/include/nvkm/engine/fifo.h @@ -42,6 +42,7 @@ struct nvkm_chan { dma_addr_t addr; void *ptr; } mthdbuf; + struct nvkm_vctx *grctx; } rm; struct list_head cctxs; diff --git a/drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h b/drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h index a2333cfe6955..8145796ffc61 100644 --- a/drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h +++ b/drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h @@ -55,4 +55,5 @@ int gp10b_gr_new(struct nvkm_device *, enum nvkm_subdev_type, int inst, struct n int gv100_gr_new(struct nvkm_device *, enum nvkm_subdev_type, int inst, struct nvkm_gr **); int tu102_gr_new(struct nvkm_device *, enum nvkm_subdev_type, int inst, struct nvkm_gr **); int ga102_gr_new(struct nvkm_device *, enum nvkm_subdev_type, int inst, struct nvkm_gr **); +int ad102_gr_new(struct nvkm_device *, enum nvkm_subdev_type, int inst, struct nvkm_gr **); #endif diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h index e7e8b30ceb44..2fa0445d8928 100644 --- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h +++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h @@ -176,6 +176,11 @@ struct nvkm_gsp { u64 rm_bar2_pdb; } bar; + struct { + u8 gpcs; + u8 tpcs; + } gr; + const struct nvkm_gsp_rm { void *(*rpc_get)(struct nvkm_gsp *, u32 fn, u32 argc); void *(*rpc_push)(struct nvkm_gsp *, void *argv, bool wait, u32 repc); diff --git a/drivers/gpu/drm/nouveau/include/nvrm/535.54.03/common/sdk/nvidia/inc/ctrl/ctrl0080/ctrl0080fifo.h b/drivers/gpu/drm/nouveau/include/nvrm/535.54.03/common/sdk/nvidia/inc/ctrl/ctrl0080/ctrl0080fifo.h new file mode 100644 index ..7fda8d2d2067 --- /dev/null +++ b/drivers/gpu/drm/nouveau/include/nvrm/535.54.03/common/sdk/nvidia/inc/ctrl/ctrl0080/ctrl0080fifo.h @@ -0,0 +1,57 @@ +#ifndef __src_common_sdk_nvidia_inc_ctrl_ctrl0080_ctrl0080fifo_h__ +#define __src_common_sdk_nvidia_inc_ctrl_ctrl0080_ctrl0080fifo_h__ + +/* Excerpt of RM headers from https://github.com/NVIDIA/open-gpu-kernel-modules/tree/535.54.03 */
Re: [Nouveau] [PATCH 38/44] drm/nouveau/fifo/r535: initial support
On 9/18/23 22:21, Ben Skeggs wrote: From: Ben Skeggs - Adds support for allocating CHANNEL_GPFIFO classes from RM. Signed-off-by: Ben Skeggs --- .../drm/nouveau/include/nvkm/engine/fifo.h| 13 + .../sdk/nvidia/inc/alloc/alloc_channel.h | 161 ++ .../nvidia/inc/class/cl2080_notification.h| 20 + .../sdk/nvidia/inc/ctrl/ctrl2080/ctrl2080ce.h | 35 ++ .../nvidia/inc/ctrl/ctrl2080/ctrl2080fifo.h | 52 ++ .../nvidia/inc/ctrl/ctrla06f/ctrla06fgpfifo.h | 42 ++ .../common/sdk/nvidia/inc/nvlimits.h | 2 + .../nvidia/generated/g_kernel_channel_nvoc.h | 62 +++ .../nvidia/generated/g_kernel_fifo_nvoc.h | 119 .../nvidia/generated/g_rpc-structures.h | 9 + .../gpu/drm/nouveau/nvkm/engine/device/base.c | 5 + drivers/gpu/drm/nouveau/nvkm/engine/falcon.c | 4 +- .../gpu/drm/nouveau/nvkm/engine/fifo/Kbuild | 2 + .../gpu/drm/nouveau/nvkm/engine/fifo/ga100.c | 2 +- .../gpu/drm/nouveau/nvkm/engine/fifo/ga102.c | 4 +- .../gpu/drm/nouveau/nvkm/engine/fifo/priv.h | 2 + .../gpu/drm/nouveau/nvkm/engine/fifo/r535.c | 519 ++ .../gpu/drm/nouveau/nvkm/engine/fifo/tu102.c | 2 +- .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c| 30 + 19 files changed, 1079 insertions(+), 6 deletions(-) create mode 100644 drivers/gpu/drm/nouveau/include/nvrm/535.54.03/common/sdk/nvidia/inc/alloc/alloc_channel.h create mode 100644 drivers/gpu/drm/nouveau/include/nvrm/535.54.03/common/sdk/nvidia/inc/ctrl/ctrl2080/ctrl2080ce.h create mode 100644 drivers/gpu/drm/nouveau/include/nvrm/535.54.03/common/sdk/nvidia/inc/ctrl/ctrl2080/ctrl2080fifo.h create mode 100644 drivers/gpu/drm/nouveau/include/nvrm/535.54.03/common/sdk/nvidia/inc/ctrl/ctrla06f/ctrla06fgpfifo.h create mode 100644 drivers/gpu/drm/nouveau/include/nvrm/535.54.03/nvidia/generated/g_kernel_channel_nvoc.h create mode 100644 drivers/gpu/drm/nouveau/include/nvrm/535.54.03/nvidia/generated/g_kernel_fifo_nvoc.h create mode 100644 drivers/gpu/drm/nouveau/nvkm/engine/fifo/r535.c diff --git a/drivers/gpu/drm/nouveau/include/nvkm/engine/fifo.h b/drivers/gpu/drm/nouveau/include/nvkm/engine/fifo.h index 7de63718ae7e..93c75540ba5a 100644 --- a/drivers/gpu/drm/nouveau/include/nvkm/engine/fifo.h +++ b/drivers/gpu/drm/nouveau/include/nvkm/engine/fifo.h @@ -4,6 +4,7 @@ #include #include #include +#include struct nvkm_fault_data; #define NVKM_FIFO_ENGN_NR 16 @@ -35,6 +36,14 @@ struct nvkm_chan { atomic_t blocked; atomic_t errored; + struct { + struct nvkm_gsp_object object; + struct { + dma_addr_t addr; + void *ptr; + } mthdbuf; + } rm; + struct list_head cctxs; struct list_head head; }; @@ -71,6 +80,10 @@ struct nvkm_fifo { struct list_head list; } userd; + struct { + u32 mthdbuf_size; + } rm; + spinlock_t lock; struct mutex mutex; }; diff --git a/drivers/gpu/drm/nouveau/include/nvrm/535.54.03/common/sdk/nvidia/inc/alloc/alloc_channel.h b/drivers/gpu/drm/nouveau/include/nvrm/535.54.03/common/sdk/nvidia/inc/alloc/alloc_channel.h new file mode 100644 index ..29d0b58c25c3 --- /dev/null +++ b/drivers/gpu/drm/nouveau/include/nvrm/535.54.03/common/sdk/nvidia/inc/alloc/alloc_channel.h @@ -0,0 +1,161 @@ +#ifndef __src_common_sdk_nvidia_inc_alloc_alloc_channel_h__ +#define __src_common_sdk_nvidia_inc_alloc_alloc_channel_h__ +#include + +/* Excerpt of RM headers from https://github.com/NVIDIA/open-gpu-kernel-modules/tree/535.54.03 */ + +/* + * SPDX-FileCopyrightText: Copyright (c) 2022 NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: MIT + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + */ + +typedef struct NV_MEMORY_DESC_PARAMS { +NV_DECLARE_ALIGNED(NvU64 base, 8); +
Re: [Nouveau] [PATCH 36/44] drm/nouveau/mmu/r535: initial support
On 9/18/23 22:21, Ben Skeggs wrote: From: Ben Skeggs - Valid VRAM regions are read from GSP-RM, and used to construct our MM - BAR1/BAR2 VMMs modified to be shared with RM - Client VMMs have RM VASPACE objects created for them - Adds FBSR to backup system objects in VRAM across suspend Signed-off-by: Ben Skeggs --- .../gpu/drm/nouveau/include/nvkm/subdev/bar.h | 4 + .../gpu/drm/nouveau/include/nvkm/subdev/gsp.h | 12 + .../drm/nouveau/include/nvkm/subdev/instmem.h | 5 + .../gpu/drm/nouveau/include/nvkm/subdev/mmu.h | 11 + .../common/sdk/nvidia/inc/class/cl84a0.h | 33 ++ .../common/sdk/nvidia/inc/class/cl90f1.h | 31 ++ .../inc/ctrl/ctrl2080/ctrl2080internal.h | 22 ++ .../common/sdk/nvidia/inc/ctrl/ctrl90f1.h | 95 + .../535.54.03/common/sdk/nvidia/inc/nvos.h| 94 + .../535.54.03/nvidia/generated/g_fbsr_nvoc.h | 31 ++ .../nvidia/generated/g_rpc-structures.h | 20 ++ .../nvidia/generated/g_sdk-structures.h | 8 + .../nvidia/kernel/inc/vgpu/rpc_headers.h | 7 + .../nvidia/kernel/inc/vgpu/sdk-structures.h | 40 +++ .../gpu/drm/nouveau/nvkm/engine/device/base.c | 20 ++ .../gpu/drm/nouveau/nvkm/subdev/bar/Kbuild| 2 + .../gpu/drm/nouveau/nvkm/subdev/bar/priv.h| 3 + .../gpu/drm/nouveau/nvkm/subdev/bar/r535.c| 186 ++ .../gpu/drm/nouveau/nvkm/subdev/bar/tu102.c | 2 +- drivers/gpu/drm/nouveau/nvkm/subdev/fb/r535.c | 37 ++ .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c| 47 +++ .../drm/nouveau/nvkm/subdev/instmem/Kbuild| 2 + .../drm/nouveau/nvkm/subdev/instmem/nv50.c| 2 +- .../drm/nouveau/nvkm/subdev/instmem/priv.h| 3 + .../drm/nouveau/nvkm/subdev/instmem/r535.c| 333 ++ .../gpu/drm/nouveau/nvkm/subdev/mmu/Kbuild| 2 + .../gpu/drm/nouveau/nvkm/subdev/mmu/base.c| 4 + .../gpu/drm/nouveau/nvkm/subdev/mmu/priv.h| 6 + .../gpu/drm/nouveau/nvkm/subdev/mmu/r535.c| 123 +++ .../gpu/drm/nouveau/nvkm/subdev/mmu/tu102.c | 2 +- .../gpu/drm/nouveau/nvkm/subdev/mmu/uvmm.c| 6 + drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.c | 7 + .../drm/nouveau/nvkm/subdev/mmu/vmmtu102.c| 5 +- 33 files changed, 1201 insertions(+), 4 deletions(-) create mode 100644 drivers/gpu/drm/nouveau/include/nvrm/535.54.03/common/sdk/nvidia/inc/class/cl84a0.h create mode 100644 drivers/gpu/drm/nouveau/include/nvrm/535.54.03/common/sdk/nvidia/inc/class/cl90f1.h create mode 100644 drivers/gpu/drm/nouveau/include/nvrm/535.54.03/common/sdk/nvidia/inc/ctrl/ctrl90f1.h create mode 100644 drivers/gpu/drm/nouveau/include/nvrm/535.54.03/common/sdk/nvidia/inc/nvos.h create mode 100644 drivers/gpu/drm/nouveau/include/nvrm/535.54.03/nvidia/generated/g_fbsr_nvoc.h create mode 100644 drivers/gpu/drm/nouveau/include/nvrm/535.54.03/nvidia/kernel/inc/vgpu/sdk-structures.h create mode 100644 drivers/gpu/drm/nouveau/nvkm/subdev/bar/r535.c create mode 100644 drivers/gpu/drm/nouveau/nvkm/subdev/instmem/r535.c create mode 100644 drivers/gpu/drm/nouveau/nvkm/subdev/mmu/r535.c diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/bar.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/bar.h index 4f07836ab984..874a5080ba06 100644 --- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/bar.h +++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/bar.h @@ -11,6 +11,10 @@ struct nvkm_bar { spinlock_t lock; bool bar2; + void __iomem *flushBAR2PhysMode; + struct nvkm_memory *flushFBZero; + void __iomem *flushBAR2; Why use camel case for those? + /* whether the BAR supports to be ioremapped WC or should be uncached */ bool iomap_uncached; }; diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h index 4f8616a58336..f5130f4f4a5a 100644 --- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h +++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h @@ -65,6 +65,13 @@ struct nvkm_gsp { } heap; u64 addr; u64 size; + + struct { + u64 addr; + u64 size; + } region[16]; Number of regions specified somewhere? + int region_nr; + u32 rsvd_size; } fb; struct { @@ -159,6 +166,11 @@ struct nvkm_gsp { } intr[32]; int intr_nr; + struct { + u64 rm_bar1_pdb; + u64 rm_bar2_pdb; + } bar; + const struct nvkm_gsp_rm { void *(*rpc_get)(struct nvkm_gsp *, u32 fn, u32 argc); void *(*rpc_push)(struct nvkm_gsp *, void *argv, bool wait, u32 repc); diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/instmem.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/instmem.h index 7d93c742ee59..e10cbd9203ec 100644 --- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/instmem.h +++
Re: [Nouveau] [PATCH 1/9] drm/amd/pm: Annotate struct smu10_voltage_dependency_table with __counted_by
On Mon, Sep 25, 2023 at 1:52 PM Kees Cook wrote: > > On Mon, Sep 25, 2023 at 08:30:30AM +0200, Christian König wrote: > > Am 22.09.23 um 19:41 schrieb Alex Deucher: > > > On Fri, Sep 22, 2023 at 1:32 PM Kees Cook wrote: > > > > Prepare for the coming implementation by GCC and Clang of the > > > > __counted_by > > > > attribute. Flexible array members annotated with __counted_by can have > > > > their accesses bounds-checked at run-time checking via > > > > CONFIG_UBSAN_BOUNDS > > > > (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family > > > > functions). > > > > > > > > As found with Coccinelle[1], add __counted_by for struct > > > > smu10_voltage_dependency_table. > > > > > > > > [1] > > > > https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci > > > > > > > > Cc: Evan Quan > > > > Cc: Alex Deucher > > > > Cc: "Christian König" > > > > Cc: "Pan, Xinhui" > > > > Cc: David Airlie > > > > Cc: Daniel Vetter > > > > Cc: Xiaojian Du > > > > Cc: Huang Rui > > > > Cc: Kevin Wang > > > > Cc: amd-...@lists.freedesktop.org > > > > Cc: dri-de...@lists.freedesktop.org > > > > Signed-off-by: Kees Cook > > > Acked-by: Alex Deucher > > > > Mhm, I'm not sure if this is a good idea. That is a structure filled in by > > the firmware, isn't it? > > > > That would imply that we might need to byte swap count before it is > > checkable. > > The script found this instance because of this: > > static int smu10_get_clock_voltage_dependency_table(struct pp_hwmgr *hwmgr, > struct smu10_voltage_dependency_table **pptable, > uint32_t num_entry, const DpmClock_t > *pclk_dependency_table) > { > uint32_t i; > struct smu10_voltage_dependency_table *ptable; > > ptable = kzalloc(struct_size(ptable, entries, num_entry), GFP_KERNEL); > if (NULL == ptable) > return -ENOMEM; > > ptable->count = num_entry; > > So the implication is that it's native byte order... but you tell me! I > certainly don't want this annotation if it's going to break stuff. :) In this case, the code is for an integrated GPU in an x86 CPU so the firmware and driver endianness match. You wouldn't find a stand alone dGPU that uses this structure. In this case it's ok. False alarm. Alex
Re: [Nouveau] [PATCH drm-misc-next 1/3] drm/sched: implement dynamic job flow control
Am 25.09.23 um 14:55 schrieb Boris Brezillon: +The imagination team, who's probably interested too. On Mon, 25 Sep 2023 00:43:06 +0200 Danilo Krummrich wrote: Currently, job flow control is implemented simply by limiting the amount of jobs in flight. Therefore, a scheduler is initialized with a submission limit that corresponds to a certain amount of jobs. This implies that for each job drivers need to account for the maximum job size possible in order to not overflow the ring buffer. However, there are drivers, such as Nouveau, where the job size has a rather large range. For such drivers it can easily happen that job submissions not even filling the ring by 1% can block subsequent submissions, which, in the worst case, can lead to the ring run dry. In order to overcome this issue, allow for tracking the actual job size instead of the amount job jobs. Therefore, add a field to track a job's submission units, which represents the amount of units a job contributes to the scheduler's submission limit. As mentioned earlier, this might allow some simplifications in the PowerVR driver where we do flow-control using a dma_fence returned through ->prepare_job(). The only thing that'd be missing is a way to dynamically query the size of a job (a new hook?), instead of having the size fixed at creation time, because PVR jobs embed native fence waits, and the number of native fences will decrease if some of these fences are signalled before ->run_job() is called, thus reducing the job size. Exactly that is a little bit questionable since it allows for the device to postpone jobs infinitely. It would be good if the scheduler is able to validate if it's ever able to run the job when it is pushed into the entity. Otherwise you can end up with really hard to debug problems. Regards, Christian. Signed-off-by: Danilo Krummrich --- This patch is based on Matt's scheduler work [1]. [1] https://lore.kernel.org/dri-devel/20230919050155.2647172-1-matthew.br...@intel.com/ --- drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 2 +- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 2 +- drivers/gpu/drm/lima/lima_sched.c | 2 +- drivers/gpu/drm/msm/msm_gem_submit.c | 2 +- drivers/gpu/drm/nouveau/nouveau_sched.c | 2 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +- .../gpu/drm/scheduler/gpu_scheduler_trace.h | 2 +- drivers/gpu/drm/scheduler/sched_entity.c | 5 +- drivers/gpu/drm/scheduler/sched_main.c| 81 +-- drivers/gpu/drm/v3d/v3d_gem.c | 2 +- include/drm/gpu_scheduler.h | 18 +++-- 11 files changed, 78 insertions(+), 42 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c index 78476bc75b4e..d54daaf64bf1 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c @@ -115,7 +115,7 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, struct amdgpu_vm *vm, if (!entity) return 0; - return drm_sched_job_init(&(*job)->base, entity, owner); + return drm_sched_job_init(&(*job)->base, entity, 1, owner); } int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index 45403ea38906..74a446711207 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -538,7 +538,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, ret = drm_sched_job_init(>sched_job, >sched_entity[args->pipe], -submit->ctx); +1, submit->ctx); if (ret) goto err_submit_put; diff --git a/drivers/gpu/drm/lima/lima_sched.c b/drivers/gpu/drm/lima/lima_sched.c index 50c2075228aa..5dc6678e1eb9 100644 --- a/drivers/gpu/drm/lima/lima_sched.c +++ b/drivers/gpu/drm/lima/lima_sched.c @@ -123,7 +123,7 @@ int lima_sched_task_init(struct lima_sched_task *task, for (i = 0; i < num_bos; i++) drm_gem_object_get([i]->base.base); - err = drm_sched_job_init(>base, >base, vm); + err = drm_sched_job_init(>base, >base, 1, vm); if (err) { kfree(task->bos); return err; diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c index 3f1aa4de3b87..6d230c38e4f5 100644 --- a/drivers/gpu/drm/msm/msm_gem_submit.c +++ b/drivers/gpu/drm/msm/msm_gem_submit.c @@ -48,7 +48,7 @@ static struct msm_gem_submit *submit_create(struct drm_device *dev, return ERR_PTR(ret); } - ret = drm_sched_job_init(>base, queue->entity, queue); + ret = drm_sched_job_init(>base, queue->entity, 1, queue); if (ret) { kfree(submit->hw_fence); kfree(submit); diff
Re: [Nouveau] [PATCH 1/9] drm/amd/pm: Annotate struct smu10_voltage_dependency_table with __counted_by
On Mon, Sep 25, 2023 at 08:30:30AM +0200, Christian König wrote: > Am 22.09.23 um 19:41 schrieb Alex Deucher: > > On Fri, Sep 22, 2023 at 1:32 PM Kees Cook wrote: > > > Prepare for the coming implementation by GCC and Clang of the __counted_by > > > attribute. Flexible array members annotated with __counted_by can have > > > their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS > > > (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family > > > functions). > > > > > > As found with Coccinelle[1], add __counted_by for struct > > > smu10_voltage_dependency_table. > > > > > > [1] > > > https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci > > > > > > Cc: Evan Quan > > > Cc: Alex Deucher > > > Cc: "Christian König" > > > Cc: "Pan, Xinhui" > > > Cc: David Airlie > > > Cc: Daniel Vetter > > > Cc: Xiaojian Du > > > Cc: Huang Rui > > > Cc: Kevin Wang > > > Cc: amd-...@lists.freedesktop.org > > > Cc: dri-de...@lists.freedesktop.org > > > Signed-off-by: Kees Cook > > Acked-by: Alex Deucher > > Mhm, I'm not sure if this is a good idea. That is a structure filled in by > the firmware, isn't it? > > That would imply that we might need to byte swap count before it is > checkable. The script found this instance because of this: static int smu10_get_clock_voltage_dependency_table(struct pp_hwmgr *hwmgr, struct smu10_voltage_dependency_table **pptable, uint32_t num_entry, const DpmClock_t *pclk_dependency_table) { uint32_t i; struct smu10_voltage_dependency_table *ptable; ptable = kzalloc(struct_size(ptable, entries, num_entry), GFP_KERNEL); if (NULL == ptable) return -ENOMEM; ptable->count = num_entry; So the implication is that it's native byte order... but you tell me! I certainly don't want this annotation if it's going to break stuff. :) -- Kees Cook
Re: [Nouveau] [PATCH 3/9] drm/i915/selftests: Annotate struct perf_series with __counted_by
On Mon, Sep 25, 2023 at 12:08:36PM +0200, Andrzej Hajda wrote: > > > On 22.09.2023 19:32, Kees Cook wrote: > > Prepare for the coming implementation by GCC and Clang of the __counted_by > > attribute. Flexible array members annotated with __counted_by can have > > their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS > > (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family > > functions). > > > > As found with Coccinelle[1], add __counted_by for struct perf_series. > > > > [1] > > https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci > > > > Cc: Jani Nikula > > Cc: Joonas Lahtinen > > Cc: Rodrigo Vivi > > Cc: Tvrtko Ursulin > > Cc: David Airlie > > Cc: Daniel Vetter > > Cc: Chris Wilson > > Cc: John Harrison > > Cc: Andi Shyti > > Cc: Matthew Brost > > Cc: intel-...@lists.freedesktop.org > > Cc: dri-de...@lists.freedesktop.org > > Signed-off-by: Kees Cook > > I am surprised this is the only finding in i915, I would expected more. I'm sure there are more, but it's likely my Coccinelle pattern didn't catch it. There are many many flexible arrays in drm. :) $ grep -nRH '\[\];$' drivers/gpu/drm include/uapi/drm | grep -v :extern | wc -l 122 If anyone has some patterns I can add to the Coccinelle script, I can take another pass at it. > Anyway: > > Reviewed-by: Andrzej Hajda Thank you! -Kees -- Kees Cook
Re: [Nouveau] [PATCH 34/44] drm/nouveau/gsp/r535: add support for rm alloc
On 9/18/23 22:21, Ben Skeggs wrote: From: Ben Skeggs Adds the plumbing to be able to allocate and free RM objects, and implements RM client/device/subdevice allocation with it. These will be used by subsequent patches. Signed-off-by: Ben Skeggs --- .../gpu/drm/nouveau/include/nvkm/subdev/gsp.h | 131 + .../common/sdk/nvidia/inc/class/cl.h | 38 .../common/sdk/nvidia/inc/class/cl0080.h | 43 .../common/sdk/nvidia/inc/class/cl2080.h | 35 .../common/sdk/nvidia/inc/nvlimits.h | 31 +++ .../nvidia/generated/g_rpc-structures.h | 19 ++ .../nvidia/generated/g_sdk-structures.h | 37 .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c| 184 ++ 8 files changed, 518 insertions(+) create mode 100644 drivers/gpu/drm/nouveau/include/nvrm/535.54.03/common/sdk/nvidia/inc/class/cl.h create mode 100644 drivers/gpu/drm/nouveau/include/nvrm/535.54.03/common/sdk/nvidia/inc/class/cl0080.h create mode 100644 drivers/gpu/drm/nouveau/include/nvrm/535.54.03/common/sdk/nvidia/inc/class/cl2080.h create mode 100644 drivers/gpu/drm/nouveau/include/nvrm/535.54.03/common/sdk/nvidia/inc/nvlimits.h create mode 100644 drivers/gpu/drm/nouveau/include/nvrm/535.54.03/nvidia/generated/g_sdk-structures.h diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h index 8f0805474981..94b14f906df6 100644 --- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h +++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h @@ -158,7 +158,24 @@ struct nvkm_gsp { void *(*rm_ctrl_get)(struct nvkm_gsp_object *, u32 cmd, u32 argc); void *(*rm_ctrl_push)(struct nvkm_gsp_object *, void *argv, u32 repc); void (*rm_ctrl_done)(struct nvkm_gsp_object *, void *repv); + + void *(*rm_alloc_get)(struct nvkm_gsp_object *, u32 oclass, u32 argc); + void *(*rm_alloc_push)(struct nvkm_gsp_object *, void *argv, u32 repc); + void (*rm_alloc_done)(struct nvkm_gsp_object *, void *repv); + + int (*rm_free)(struct nvkm_gsp_object *); + + int (*client_ctor)(struct nvkm_gsp *, struct nvkm_gsp_client *); + void (*client_dtor)(struct nvkm_gsp_client *); + + int (*device_ctor)(struct nvkm_gsp_client *, struct nvkm_gsp_device *); + void (*device_dtor)(struct nvkm_gsp_device *); } *rm; + + struct { + struct mutex mutex;; + struct idr idr; + } client_id; }; static inline bool @@ -247,6 +264,120 @@ nvkm_gsp_rm_ctrl_done(struct nvkm_gsp_object *object, void *repv) object->client->gsp->rm->rm_ctrl_done(object, repv); } +static inline void * +nvkm_gsp_rm_alloc_get(struct nvkm_gsp_object *parent, u32 handle, u32 oclass, u32 argc, + struct nvkm_gsp_object *object) +{ + struct nvkm_gsp_client *client = parent->client; + struct nvkm_gsp *gsp = client->gsp; + void *argv; + + object->client = parent->client; + object->parent = parent; + object->handle = handle; + + argv = gsp->rm->rm_alloc_get(object, oclass, argc); + if (IS_ERR_OR_NULL(argv)) { + object->client = NULL; Why do we need to set ->client to NULL here? What about ->parent and ->handle? + return argv; + } + + return argv; +} + +static inline void * +nvkm_gsp_rm_alloc_push(struct nvkm_gsp_object *object, void *argv, u32 repc) +{ + void *repv = object->client->gsp->rm->rm_alloc_push(object, argv, repc); + + if (IS_ERR(repv)) + object->client = NULL; + + return repv; +} + +static inline int +nvkm_gsp_rm_alloc_wr(struct nvkm_gsp_object *object, void *argv) +{ + void *repv = nvkm_gsp_rm_alloc_push(object, argv, 0); + + if (IS_ERR(repv)) + return PTR_ERR(repv); + + return 0; +} + +static inline void +nvkm_gsp_rm_alloc_done(struct nvkm_gsp_object *object, void *repv) +{ + object->client->gsp->rm->rm_alloc_done(object, repv); +} + +static inline int +nvkm_gsp_rm_alloc(struct nvkm_gsp_object *parent, u32 handle, u32 oclass, u32 argc, + struct nvkm_gsp_object *object) +{ + void *argv = nvkm_gsp_rm_alloc_get(parent, handle, oclass, argc, object); + + if (IS_ERR_OR_NULL(argv)) + return argv ? PTR_ERR(argv) : -EIO; + + return nvkm_gsp_rm_alloc_wr(object, argv); +} + +static inline int +nvkm_gsp_rm_free(struct nvkm_gsp_object *object) +{ + if (object->client) + return object->client->gsp->rm->rm_free(object); + + return 0; +} + +static inline int +nvkm_gsp_client_ctor(struct nvkm_gsp *gsp, struct nvkm_gsp_client *client) +{ + if (WARN_ON(!gsp->rm)) + return -ENOSYS; + + return gsp->rm->client_ctor(gsp, client); +} + +static inline void
[Nouveau] [PATCH] drm/nouveau/kms/nv50: hide unused variables
From: Arnd Bergmann After a recent change, two variables are only used in an #ifdef: drivers/gpu/drm/nouveau/dispnv50/disp.c: In function 'nv50_sor_atomic_disable': drivers/gpu/drm/nouveau/dispnv50/disp.c:1569:13: error: unused variable 'ret' [-Werror=unused-variable] 1569 | int ret; | ^~~ drivers/gpu/drm/nouveau/dispnv50/disp.c:1568:28: error: unused variable 'aux' [-Werror=unused-variable] 1568 | struct drm_dp_aux *aux = _connector->aux; |^~~ Move them into the same conditional block, along with the nv_connector variable that becomes unused during that fix. Fixes: 757033808c95b ("drm/nouveau/kms/nv50-: fixup sink D3 before tearing down link") Signed-off-by: Arnd Bergmann --- drivers/gpu/drm/nouveau/dispnv50/disp.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c index 52f1569ee37c1..a0ac8c258d9ff 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c @@ -1560,15 +1560,13 @@ nv50_sor_atomic_disable(struct drm_encoder *encoder, struct drm_atomic_state *st { struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder); struct nv50_head *head = nv50_head(nv_encoder->crtc); - struct nouveau_connector *nv_connector = nv50_outp_get_old_connector(state, nv_encoder); #ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT + struct nouveau_connector *nv_connector = nv50_outp_get_old_connector(state, nv_encoder); struct nouveau_drm *drm = nouveau_drm(nv_encoder->base.base.dev); struct nouveau_backlight *backlight = nv_connector->backlight; -#endif struct drm_dp_aux *aux = _connector->aux; int ret; -#ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT if (backlight && backlight->uses_dpcd) { ret = drm_edp_backlight_disable(aux, >edp_info); if (ret < 0) -- 2.39.2
Re: [Nouveau] [PATCH 1/9] drm/amd/pm: Annotate struct smu10_voltage_dependency_table with __counted_by
On Mon, Sep 25, 2023 at 10:07 AM Alex Deucher wrote: > > On Mon, Sep 25, 2023 at 2:30 AM Christian König > wrote: > > > > Am 22.09.23 um 19:41 schrieb Alex Deucher: > > > On Fri, Sep 22, 2023 at 1:32 PM Kees Cook wrote: > > >> Prepare for the coming implementation by GCC and Clang of the > > >> __counted_by > > >> attribute. Flexible array members annotated with __counted_by can have > > >> their accesses bounds-checked at run-time checking via > > >> CONFIG_UBSAN_BOUNDS > > >> (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family > > >> functions). > > >> > > >> As found with Coccinelle[1], add __counted_by for struct > > >> smu10_voltage_dependency_table. > > >> > > >> [1] > > >> https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci > > >> > > >> Cc: Evan Quan > > >> Cc: Alex Deucher > > >> Cc: "Christian König" > > >> Cc: "Pan, Xinhui" > > >> Cc: David Airlie > > >> Cc: Daniel Vetter > > >> Cc: Xiaojian Du > > >> Cc: Huang Rui > > >> Cc: Kevin Wang > > >> Cc: amd-...@lists.freedesktop.org > > >> Cc: dri-de...@lists.freedesktop.org > > >> Signed-off-by: Kees Cook > > > Acked-by: Alex Deucher > > > > Mhm, I'm not sure if this is a good idea. That is a structure filled in > > by the firmware, isn't it? > > > > That would imply that we might need to byte swap count before it is > > checkable. > > True. Good point. Same for the other amdgpu patch. Actually the other patch is fine. That's just a local structure. Alex > > Alex > > > > > Regards, > > Christian. > > > > > > > >> --- > > >> drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h | 2 +- > > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > >> > > >> diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h > > >> b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h > > >> index 808e0ecbe1f0..42adc2a3dcbc 100644 > > >> --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h > > >> +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h > > >> @@ -192,7 +192,7 @@ struct smu10_clock_voltage_dependency_record { > > >> > > >> struct smu10_voltage_dependency_table { > > >> uint32_t count; > > >> - struct smu10_clock_voltage_dependency_record entries[]; > > >> + struct smu10_clock_voltage_dependency_record entries[] > > >> __counted_by(count); > > >> }; > > >> > > >> struct smu10_clock_voltage_information { > > >> -- > > >> 2.34.1 > > >> > >
Re: [PATCH 1/9] drm/amd/pm: Annotate struct smu10_voltage_dependency_table with __counted_by
On Mon, Sep 25, 2023 at 2:30 AM Christian König wrote: > > Am 22.09.23 um 19:41 schrieb Alex Deucher: > > On Fri, Sep 22, 2023 at 1:32 PM Kees Cook wrote: > >> Prepare for the coming implementation by GCC and Clang of the __counted_by > >> attribute. Flexible array members annotated with __counted_by can have > >> their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS > >> (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family > >> functions). > >> > >> As found with Coccinelle[1], add __counted_by for struct > >> smu10_voltage_dependency_table. > >> > >> [1] > >> https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci > >> > >> Cc: Evan Quan > >> Cc: Alex Deucher > >> Cc: "Christian König" > >> Cc: "Pan, Xinhui" > >> Cc: David Airlie > >> Cc: Daniel Vetter > >> Cc: Xiaojian Du > >> Cc: Huang Rui > >> Cc: Kevin Wang > >> Cc: amd-...@lists.freedesktop.org > >> Cc: dri-de...@lists.freedesktop.org > >> Signed-off-by: Kees Cook > > Acked-by: Alex Deucher > > Mhm, I'm not sure if this is a good idea. That is a structure filled in > by the firmware, isn't it? > > That would imply that we might need to byte swap count before it is > checkable. True. Good point. Same for the other amdgpu patch. Alex > > Regards, > Christian. > > > > >> --- > >> drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h > >> b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h > >> index 808e0ecbe1f0..42adc2a3dcbc 100644 > >> --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h > >> +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h > >> @@ -192,7 +192,7 @@ struct smu10_clock_voltage_dependency_record { > >> > >> struct smu10_voltage_dependency_table { > >> uint32_t count; > >> - struct smu10_clock_voltage_dependency_record entries[]; > >> + struct smu10_clock_voltage_dependency_record entries[] > >> __counted_by(count); > >> }; > >> > >> struct smu10_clock_voltage_information { > >> -- > >> 2.34.1 > >> >
Re: [Nouveau] [PATCH 3/9] drm/i915/selftests: Annotate struct perf_series with __counted_by
Hi Kees, On Fri, Sep 22, 2023 at 10:32:08AM -0700, Kees Cook wrote: > Prepare for the coming implementation by GCC and Clang of the __counted_by > attribute. Flexible array members annotated with __counted_by can have > their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS > (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family > functions). > > As found with Coccinelle[1], add __counted_by for struct perf_series. > > [1] > https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci > > Cc: Jani Nikula > Cc: Joonas Lahtinen > Cc: Rodrigo Vivi > Cc: Tvrtko Ursulin > Cc: David Airlie > Cc: Daniel Vetter > Cc: Chris Wilson > Cc: John Harrison > Cc: Andi Shyti > Cc: Matthew Brost > Cc: intel-...@lists.freedesktop.org > Cc: dri-de...@lists.freedesktop.org > Signed-off-by: Kees Cook Reviewed-by: Andi Shyti Thanks, Andi
Re: [PATCH 3/9] drm/i915/selftests: Annotate struct perf_series with __counted_by
On 22.09.2023 19:32, Kees Cook wrote: Prepare for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions). As found with Coccinelle[1], add __counted_by for struct perf_series. [1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci Cc: Jani Nikula Cc: Joonas Lahtinen Cc: Rodrigo Vivi Cc: Tvrtko Ursulin Cc: David Airlie Cc: Daniel Vetter Cc: Chris Wilson Cc: John Harrison Cc: Andi Shyti Cc: Matthew Brost Cc: intel-...@lists.freedesktop.org Cc: dri-de...@lists.freedesktop.org Signed-off-by: Kees Cook I am surprised this is the only finding in i915, I would expected more. Anyway: Reviewed-by: Andrzej Hajda Regards Andrzej --- drivers/gpu/drm/i915/selftests/i915_request.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/selftests/i915_request.c b/drivers/gpu/drm/i915/selftests/i915_request.c index a9b79888c193..acae30a04a94 100644 --- a/drivers/gpu/drm/i915/selftests/i915_request.c +++ b/drivers/gpu/drm/i915/selftests/i915_request.c @@ -1924,7 +1924,7 @@ struct perf_stats { struct perf_series { struct drm_i915_private *i915; unsigned int nengines; - struct intel_context *ce[]; + struct intel_context *ce[] __counted_by(nengines); }; static int cmp_u32(const void *A, const void *B)
Re: [PATCH 1/9] drm/amd/pm: Annotate struct smu10_voltage_dependency_table with __counted_by
Am 22.09.23 um 19:41 schrieb Alex Deucher: On Fri, Sep 22, 2023 at 1:32 PM Kees Cook wrote: Prepare for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions). As found with Coccinelle[1], add __counted_by for struct smu10_voltage_dependency_table. [1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci Cc: Evan Quan Cc: Alex Deucher Cc: "Christian König" Cc: "Pan, Xinhui" Cc: David Airlie Cc: Daniel Vetter Cc: Xiaojian Du Cc: Huang Rui Cc: Kevin Wang Cc: amd-...@lists.freedesktop.org Cc: dri-de...@lists.freedesktop.org Signed-off-by: Kees Cook Acked-by: Alex Deucher Mhm, I'm not sure if this is a good idea. That is a structure filled in by the firmware, isn't it? That would imply that we might need to byte swap count before it is checkable. Regards, Christian. --- drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h index 808e0ecbe1f0..42adc2a3dcbc 100644 --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h @@ -192,7 +192,7 @@ struct smu10_clock_voltage_dependency_record { struct smu10_voltage_dependency_table { uint32_t count; - struct smu10_clock_voltage_dependency_record entries[]; + struct smu10_clock_voltage_dependency_record entries[] __counted_by(count); }; struct smu10_clock_voltage_information { -- 2.34.1