[Nouveau] [PATCH] MAINTAINERS: update nouveau maintainers

2023-09-25 Thread Danilo Krummrich
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

2023-09-25 Thread Danilo Krummrich

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

2023-09-25 Thread Danilo Krummrich

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

2023-09-25 Thread Danilo Krummrich

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

2023-09-25 Thread Danilo Krummrich

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

2023-09-25 Thread Alex Deucher
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

2023-09-25 Thread Christian König




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

2023-09-25 Thread Kees Cook
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

2023-09-25 Thread Kees Cook
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

2023-09-25 Thread Danilo Krummrich

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

2023-09-25 Thread Arnd Bergmann
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

2023-09-25 Thread Alex Deucher
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

2023-09-25 Thread Alex Deucher
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

2023-09-25 Thread Andi Shyti
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

2023-09-25 Thread Andrzej Hajda




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

2023-09-25 Thread Christian König

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