[PATCH v2 0/1] devcoredump support for Panfrost GPU driver

[PATCH v2 0/1] devcoredump support for Panfrost GPU driver

[Adrián Larumbe]

This is v2 for a previous patch series being discussed at
https://lore.kernel.org/dri-devel/20220517174216.381287-1-adrian.larumbe@collabora.com/.

Mesa MR under review can be found at:
https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/14034

Changes with respect to v1 of the same patch:

 - Change the list of dumped registers to those of greatest debugging value.
 - Fix bug where an invalid bomap header wouldn't be dumped
 - Changed the way that a decision is made whether to dump a BOMAP header so
 that no compiler warning supression has to be done.
 - Changed magic number for header fields to something more representative.
 - Changed the dump header format to a union whose members are the different
   header types.

Adrián Larumbe (1):
  drm/panfrost: Add support for devcoredump

 drivers/gpu/drm/panfrost/Kconfig         |   1 +
 drivers/gpu/drm/panfrost/Makefile        |   3 +-
 drivers/gpu/drm/panfrost/panfrost_dump.c | 233 +++++++++++++++++++++++
 drivers/gpu/drm/panfrost/panfrost_dump.h |  12 ++
 drivers/gpu/drm/panfrost/panfrost_job.c  |   3 +
 include/uapi/drm/panfrost_drm.h          |  44 +++++
 6 files changed, 295 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/panfrost/panfrost_dump.c
 create mode 100644 drivers/gpu/drm/panfrost/panfrost_dump.h

-- 
2.36.1

[PATCH v2 1/1] drm/panfrost: Add support for devcoredump

[Adrián Larumbe]

In the event of a job timeout, debug dump information will be written into
/sys/class/devcoredump.

Inspired by etnaviv's similar feature.

Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
---
 drivers/gpu/drm/panfrost/Kconfig         |   1 +
 drivers/gpu/drm/panfrost/Makefile        |   3 +-
 drivers/gpu/drm/panfrost/panfrost_dump.c | 233 +++++++++++++++++++++++
 drivers/gpu/drm/panfrost/panfrost_dump.h |  12 ++
 drivers/gpu/drm/panfrost/panfrost_job.c  |   3 +
 include/uapi/drm/panfrost_drm.h          |  44 +++++
 6 files changed, 295 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/panfrost/panfrost_dump.c
 create mode 100644 drivers/gpu/drm/panfrost/panfrost_dump.h

diff --git a/drivers/gpu/drm/panfrost/Kconfig b/drivers/gpu/drm/panfrost/Kconfig
index 86cdc0ce79e6..079600328be1 100644
--- a/drivers/gpu/drm/panfrost/Kconfig
+++ b/drivers/gpu/drm/panfrost/Kconfig
@@ -11,6 +11,7 @@ config DRM_PANFROST
 	select DRM_GEM_SHMEM_HELPER
 	select PM_DEVFREQ
 	select DEVFREQ_GOV_SIMPLE_ONDEMAND
+	select WANT_DEV_COREDUMP
 	help
 	  DRM driver for ARM Mali Midgard (T6xx, T7xx, T8xx) and
 	  Bifrost (G3x, G5x, G7x) GPUs.
diff --git a/drivers/gpu/drm/panfrost/Makefile b/drivers/gpu/drm/panfrost/Makefile
index b71935862417..7da2b3f02ed9 100644
--- a/drivers/gpu/drm/panfrost/Makefile
+++ b/drivers/gpu/drm/panfrost/Makefile
@@ -9,6 +9,7 @@ panfrost-y := \
 	panfrost_gpu.o \
 	panfrost_job.o \
 	panfrost_mmu.o \
-	panfrost_perfcnt.o
+	panfrost_perfcnt.o \
+	panfrost_dump.o
 
 obj-$(CONFIG_DRM_PANFROST) += panfrost.o
diff --git a/drivers/gpu/drm/panfrost/panfrost_dump.c b/drivers/gpu/drm/panfrost/panfrost_dump.c
new file mode 100644
index 000000000000..53ecc9b43666
--- /dev/null
+++ b/drivers/gpu/drm/panfrost/panfrost_dump.c
@@ -0,0 +1,233 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright 2021 Collabora ltd. */
+
+#include <linux/err.h>
+#include <linux/device.h>
+#include <linux/devcoredump.h>
+#include <linux/moduleparam.h>
+#include <linux/iosys-map.h>
+#include <drm/panfrost_drm.h>
+#include <drm/drm_device.h>
+
+#include "panfrost_job.h"
+#include "panfrost_gem.h"
+#include "panfrost_regs.h"
+#include "panfrost_dump.h"
+#include "panfrost_device.h"
+
+static bool panfrost_dump_core = true;
+module_param_named(dump_core, panfrost_dump_core, bool, 0600);
+
+struct panfrost_dump_iterator {
+	void *start;
+	struct panfrost_dump_object_header *hdr;
+	void *data;
+};
+
+static const unsigned short panfrost_dump_registers[] = {
+	SHADER_READY_LO,
+	SHADER_READY_HI,
+	TILER_READY_LO,
+	TILER_READY_HI,
+	L2_READY_LO,
+	L2_READY_HI,
+	JOB_INT_MASK,
+	JOB_INT_STAT,
+	JS_HEAD_LO(0),
+	JS_HEAD_HI(0),
+	JS_TAIL_LO(0),
+	JS_TAIL_HI(0),
+	JS_AFFINITY_LO(0),
+	JS_AFFINITY_HI(0),
+	JS_CONFIG(0),
+	JS_STATUS(0),
+	JS_HEAD_NEXT_LO(0),
+	JS_HEAD_NEXT_HI(0),
+	JS_AFFINITY_NEXT_LO(0),
+	JS_AFFINITY_NEXT_HI(0),
+	JS_CONFIG_NEXT(0),
+	MMU_INT_MASK,
+	MMU_INT_STAT,
+	AS_TRANSTAB_LO(0),
+	AS_TRANSTAB_HI(0),
+	AS_MEMATTR_LO(0),
+	AS_MEMATTR_HI(0),
+	AS_FAULTSTATUS(0),
+	AS_FAULTADDRESS_LO(0),
+	AS_FAULTADDRESS_HI(0),
+	AS_STATUS(0),
+};
+
+static void panfrost_core_dump_header(struct panfrost_dump_iterator *iter,
+	u32 type, void *data_end)
+{
+	struct panfrost_dump_object_header *hdr = iter->hdr;
+
+	hdr->magic = cpu_to_le32(PANFROSTDUMP_MAGIC);
+	hdr->type = cpu_to_le32(type);
+	hdr->file_offset = cpu_to_le32(iter->data - iter->start);
+	hdr->file_size = cpu_to_le32(data_end - iter->data);
+
+	iter->hdr++;
+	iter->data += le32_to_cpu(hdr->file_size);
+}
+
+static void
+panfrost_core_dump_registers(struct panfrost_dump_iterator *iter,
+			     struct panfrost_device *pfdev,
+			     u32 as_nr, int slot)
+{
+	struct panfrost_dump_registers *dumpreg = iter->data;
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(panfrost_dump_registers); i++, dumpreg++) {
+		unsigned int js_as_offset = 0;
+		unsigned int reg;
+
+		if (panfrost_dump_registers[i] >= JS_HEAD_LO(0) &&
+		    panfrost_dump_registers[i] <= JS_CONFIG_NEXT(0))
+			js_as_offset = slot * 0x80;
+		else if (panfrost_dump_registers[i] >= AS_TRANSTAB_LO(0) &&
+			 panfrost_dump_registers[i] <= AS_STATUS(0))
+			js_as_offset = ((as_nr) << 6);
+
+		reg = panfrost_dump_registers[i] + js_as_offset;
+
+		dumpreg->reg = cpu_to_le32(reg);
+		dumpreg->value = cpu_to_le32(gpu_read(pfdev, reg));
+	}
+
+	panfrost_core_dump_header(iter, PANFROSTDUMP_BUF_REG, dumpreg);
+}
+
+void panfrost_core_dump(struct panfrost_job *job)
+{
+	struct panfrost_device *pfdev = job->pfdev;
+	struct panfrost_dump_iterator iter;
+	struct drm_gem_object *dbo;
+	unsigned int n_obj, n_bomap_pages;
+	__le64 *bomap, *bomap_start;
+	size_t file_size;
+	u32 as_nr;
+	int slot;
+	int ret, i;
+
+	as_nr = job->file_priv->mmu->as;
+	slot = panfrost_job_get_slot(job);
+	slot = slot ? slot : 0;
+
+	/* Only catch the first event, or when manually re-armed */
+	if (!panfrost_dump_core)
+		return;
+	panfrost_dump_core = false;
+
+	/* At least, we dump registers and end marker */
+	n_obj = 2;
+	n_bomap_pages = 0;
+	file_size = ARRAY_SIZE(panfrost_dump_registers) *
+			sizeof(struct panfrost_dump_registers);
+
+	/* Add in the active buffer objects */
+	for (i = 0; i < job->bo_count; i++) {
+		dbo = job->bos[i];
+		file_size += dbo->size;
+		n_bomap_pages += dbo->size >> PAGE_SHIFT;
+		n_obj++;
+	}
+
+	/* If we have any buffer objects, add a bomap object */
+	if (n_bomap_pages) {
+		file_size += n_bomap_pages * sizeof(*bomap);
+		n_obj++;
+	}
+
+	/* Add the size of the headers */
+	file_size += sizeof(*iter.hdr) * n_obj;
+
+	/* Allocate the file in vmalloc memory, it's likely to be big */
+	iter.start = __vmalloc(file_size, GFP_KERNEL | __GFP_NOWARN |
+			__GFP_NORETRY);
+	if (!iter.start) {
+		dev_warn(pfdev->dev, "failed to allocate devcoredump file\n");
+		return;
+	}
+
+	/* Point the data member after the headers */
+	iter.hdr = iter.start;
+	iter.data = &iter.hdr[n_obj];
+
+	memset(iter.hdr, 0, iter.data - iter.start);
+
+	/*
+	 * For now, we write the job identifier in the register dump header,
+	 * so that we can decode the entire dump later with pandecode
+	 */
+	iter.hdr->reghdr.jc = cpu_to_le64(job->jc);
+	iter.hdr->reghdr.version = cpu_to_le32(PANFROSTDUMP_VERSION_1);
+	iter.hdr->reghdr.gpu_id = cpu_to_le32(pfdev->features.id);
+	iter.hdr->reghdr.nbos = cpu_to_le64(job->bo_count);
+
+	panfrost_core_dump_registers(&iter, pfdev, as_nr, slot);
+
+	/* Reserve space for the bomap */
+	if (job->bo_count) {
+		bomap_start = bomap = iter.data;
+		memset(bomap, 0, sizeof(*bomap) * n_bomap_pages);
+		panfrost_core_dump_header(&iter, PANFROSTDUMP_BUF_BOMAP,
+					  bomap + n_bomap_pages);
+	}
+
+	for (i = 0; i < job->bo_count; i++) {
+		struct iosys_map map;
+		struct panfrost_gem_mapping *mapping;
+		struct panfrost_gem_object *bo;
+		struct sg_page_iter page_iter;
+		void *vaddr;
+
+		bo = to_panfrost_bo(job->bos[i]);
+		mapping = job->mappings[i];
+
+		if (!bo->base.sgt) {
+			dev_err(pfdev->dev, "Panfrost Dump: BO has no sgt, cannot dump\n");
+			iter.hdr->bomap.valid = 0;
+			goto dump_header;
+		}
+
+		ret = drm_gem_shmem_vmap(&bo->base, &map);
+		if (ret) {
+			dev_err(pfdev->dev, "Panfrost Dump: couldn't map Buffer Object\n");
+			iter.hdr->bomap.valid = 0;
+			goto dump_header;
+		}
+
+		WARN_ON(!mapping->active);
+
+		iter.hdr->bomap.data[0] = cpu_to_le32((bomap - bomap_start));
+
+		for_each_sgtable_page(bo->base.sgt, &page_iter, 0) {
+			struct page *page = sg_page_iter_page(&page_iter);
+
+			if (!IS_ERR(page))
+				*bomap++ = cpu_to_le64(page_to_phys(page));
+			else {
+				dev_err(pfdev->dev, "Panfrost Dump: wrong page\n");
+				*bomap++ = ~cpu_to_le64(0);
+			}
+		}
+
+		iter.hdr->bomap.iova = cpu_to_le64(mapping->mmnode.start << PAGE_SHIFT);
+
+		vaddr = map.vaddr;
+		memcpy(iter.data, vaddr, bo->base.base.size);
+
+		drm_gem_shmem_vunmap(&bo->base, &map);
+
+		iter.hdr->bomap.valid = cpu_to_le64(1);
+
+dump_header:	panfrost_core_dump_header(&iter, PANFROSTDUMP_BUF_BO, iter.data +
+					  bo->base.base.size);
+	}
+	panfrost_core_dump_header(&iter, PANFROSTDUMP_BUF_TRAILER, iter.data);
+
+	dev_coredumpv(pfdev->dev, iter.start, iter.data - iter.start, GFP_KERNEL);
+}
diff --git a/drivers/gpu/drm/panfrost/panfrost_dump.h b/drivers/gpu/drm/panfrost/panfrost_dump.h
new file mode 100644
index 000000000000..7d9bcefa5346
--- /dev/null
+++ b/drivers/gpu/drm/panfrost/panfrost_dump.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright 2021 Collabora ltd.
+ */
+
+#ifndef PANFROST_DUMP_H
+#define PANFROST_DUMP_H
+
+struct panfrost_job;
+void panfrost_core_dump(struct panfrost_job *job);
+
+#endif
diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c
index fda5871aebe3..f506d0ea067c 100644
--- a/drivers/gpu/drm/panfrost/panfrost_job.c
+++ b/drivers/gpu/drm/panfrost/panfrost_job.c
@@ -20,6 +20,7 @@
 #include "panfrost_regs.h"
 #include "panfrost_gpu.h"
 #include "panfrost_mmu.h"
+#include "panfrost_dump.h"
 
 #define JOB_TIMEOUT_MS 500
 
@@ -727,6 +728,8 @@ static enum drm_gpu_sched_stat panfrost_job_timedout(struct drm_sched_job
 		job_read(pfdev, JS_TAIL_LO(js)),
 		sched_job);
 
+	panfrost_core_dump(job);
+
 	atomic_set(&pfdev->reset.pending, 1);
 	panfrost_reset(pfdev, sched_job);
 
diff --git a/include/uapi/drm/panfrost_drm.h b/include/uapi/drm/panfrost_drm.h
index 9e40277d8185..9a1d3a686aeb 100644
--- a/include/uapi/drm/panfrost_drm.h
+++ b/include/uapi/drm/panfrost_drm.h
@@ -224,6 +224,50 @@ struct drm_panfrost_madvise {
 	__u32 retained;       /* out, whether backing store still exists */
 };
 
+/* Definitions for coredump decoding in user space */
+#define PANFROSTDUMP_VERSION_1 1
+#define PANFROSTDUMP_MAGIC 0x464E4150 /* PANF */
+
+#define PANFROSTDUMP_BUF_REG 0
+#define PANFROSTDUMP_BUF_BOMAP (PANFROSTDUMP_BUF_REG + 1)
+#define PANFROSTDUMP_BUF_BO (PANFROSTDUMP_BUF_BOMAP + 1)
+#define PANFROSTDUMP_BUF_TRAILER (PANFROSTDUMP_BUF_BO + 1)
+
+struct panfrost_dump_object_header {
+	__le32 magic;
+	__le32 type;
+	__le32 file_size;
+	__le32 file_offset;
+
+	union {
+		struct pan_reg_hdr {
+			__le64 jc;
+			__le32 gpu_id;
+			__le64 version;
+			__le64 nbos;
+		} reghdr;
+
+		struct pan_bomap_hdr {
+			__le32 valid;
+			__le64 iova;
+			__le32 data[2];
+		} bomap;
+
+		/*
+		 * Force same size in case we want to expand the header
+		 * with new fields and also keep it 512-byte aligned
+		 */
+
+		__le32 sizer[496];
+	};
+};
+
+/* Registers object, an array of these */
+struct panfrost_dump_registers {
+	__le32 reg;
+	__le32 value;
+};
+
 #if defined(__cplusplus)
 }
 #endif
-- 
2.36.1

Re: [PATCH v2 1/1] drm/panfrost: Add support for devcoredump

[kernel test robot]

Hi "Adrián,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm/drm-next]
[also build test ERROR on linus/master v5.19-rc2 next-20220617]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Adri-n-Larumbe/devcoredump-support-for-Panfrost-GPU-driver/20220621-103431
base:   git://anongit.freedesktop.org/drm/drm drm-next
config: alpha-buildonly-randconfig-r003-20220619 (https://download.01.org/0day-ci/archive/20220621/202206211114.PJcD2pJh-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/297bd4948ab1f4eeb78389d57adc1edc819cb6f2
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Adri-n-Larumbe/devcoredump-support-for-Panfrost-GPU-driver/20220621-103431
        git checkout 297bd4948ab1f4eeb78389d57adc1edc819cb6f2
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=alpha SHELL=/bin/bash drivers/gpu/drm/panfrost/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/gpu/drm/panfrost/panfrost_dump.c: In function 'panfrost_core_dump':
>> drivers/gpu/drm/panfrost/panfrost_dump.c:115:20: error: 'struct panfrost_job' has no member named 'file_priv'
     115 |         as_nr = job->file_priv->mmu->as;
         |                    ^~


vim +115 drivers/gpu/drm/panfrost/panfrost_dump.c

   102	
   103	void panfrost_core_dump(struct panfrost_job *job)
   104	{
   105		struct panfrost_device *pfdev = job->pfdev;
   106		struct panfrost_dump_iterator iter;
   107		struct drm_gem_object *dbo;
   108		unsigned int n_obj, n_bomap_pages;
   109		__le64 *bomap, *bomap_start;
   110		size_t file_size;
   111		u32 as_nr;
   112		int slot;
   113		int ret, i;
   114	
 > 115		as_nr = job->file_priv->mmu->as;
   116		slot = panfrost_job_get_slot(job);
   117		slot = slot ? slot : 0;
   118	
   119		/* Only catch the first event, or when manually re-armed */
   120		if (!panfrost_dump_core)
   121			return;
   122		panfrost_dump_core = false;
   123	
   124		/* At least, we dump registers and end marker */
   125		n_obj = 2;
   126		n_bomap_pages = 0;
   127		file_size = ARRAY_SIZE(panfrost_dump_registers) *
   128				sizeof(struct panfrost_dump_registers);
   129	
   130		/* Add in the active buffer objects */
   131		for (i = 0; i < job->bo_count; i++) {
   132			dbo = job->bos[i];
   133			file_size += dbo->size;
   134			n_bomap_pages += dbo->size >> PAGE_SHIFT;
   135			n_obj++;
   136		}
   137	
   138		/* If we have any buffer objects, add a bomap object */
   139		if (n_bomap_pages) {
   140			file_size += n_bomap_pages * sizeof(*bomap);
   141			n_obj++;
   142		}
   143	
   144		/* Add the size of the headers */
   145		file_size += sizeof(*iter.hdr) * n_obj;
   146	
   147		/* Allocate the file in vmalloc memory, it's likely to be big */
   148		iter.start = __vmalloc(file_size, GFP_KERNEL | __GFP_NOWARN |
   149				__GFP_NORETRY);
   150		if (!iter.start) {
   151			dev_warn(pfdev->dev, "failed to allocate devcoredump file\n");
   152			return;
   153		}
   154	
   155		/* Point the data member after the headers */
   156		iter.hdr = iter.start;
   157		iter.data = &iter.hdr[n_obj];
   158	
   159		memset(iter.hdr, 0, iter.data - iter.start);
   160	
   161		/*
   162		 * For now, we write the job identifier in the register dump header,
   163		 * so that we can decode the entire dump later with pandecode
   164		 */
   165		iter.hdr->reghdr.jc = cpu_to_le64(job->jc);
   166		iter.hdr->reghdr.version = cpu_to_le32(PANFROSTDUMP_VERSION_1);
   167		iter.hdr->reghdr.gpu_id = cpu_to_le32(pfdev->features.id);
   168		iter.hdr->reghdr.nbos = cpu_to_le64(job->bo_count);
   169	
   170		panfrost_core_dump_registers(&iter, pfdev, as_nr, slot);
   171	
   172		/* Reserve space for the bomap */
   173		if (job->bo_count) {
   174			bomap_start = bomap = iter.data;
   175			memset(bomap, 0, sizeof(*bomap) * n_bomap_pages);
   176			panfrost_core_dump_header(&iter, PANFROSTDUMP_BUF_BOMAP,
   177						  bomap + n_bomap_pages);
   178		}
   179	
   180		for (i = 0; i < job->bo_count; i++) {
   181			struct iosys_map map;
   182			struct panfrost_gem_mapping *mapping;
   183			struct panfrost_gem_object *bo;
   184			struct sg_page_iter page_iter;
   185			void *vaddr;
   186	
   187			bo = to_panfrost_bo(job->bos[i]);
   188			mapping = job->mappings[i];
   189	
   190			if (!bo->base.sgt) {
   191				dev_err(pfdev->dev, "Panfrost Dump: BO has no sgt, cannot dump\n");
   192				iter.hdr->bomap.valid = 0;
   193				goto dump_header;
   194			}
   195	
   196			ret = drm_gem_shmem_vmap(&bo->base, &map);
   197			if (ret) {
   198				dev_err(pfdev->dev, "Panfrost Dump: couldn't map Buffer Object\n");
   199				iter.hdr->bomap.valid = 0;
   200				goto dump_header;
   201			}
   202	
   203			WARN_ON(!mapping->active);
   204	
   205			iter.hdr->bomap.data[0] = cpu_to_le32((bomap - bomap_start));
   206	
   207			for_each_sgtable_page(bo->base.sgt, &page_iter, 0) {
   208				struct page *page = sg_page_iter_page(&page_iter);
   209	
   210				if (!IS_ERR(page))
   211					*bomap++ = cpu_to_le64(page_to_phys(page));
   212				else {
   213					dev_err(pfdev->dev, "Panfrost Dump: wrong page\n");
   214					*bomap++ = ~cpu_to_le64(0);
   215				}
   216			}
   217	
   218			iter.hdr->bomap.iova = cpu_to_le64(mapping->mmnode.start << PAGE_SHIFT);
   219	
   220			vaddr = map.vaddr;
   221			memcpy(iter.data, vaddr, bo->base.base.size);
   222	
   223			drm_gem_shmem_vunmap(&bo->base, &map);
   224	
   225			iter.hdr->bomap.valid = cpu_to_le64(1);
   226	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH v2 1/1] drm/panfrost: Add support for devcoredump

[Alyssa Rosenzweig]

>    drivers/gpu/drm/panfrost/panfrost_dump.c: In function 'panfrost_core_dump':
> >> drivers/gpu/drm/panfrost/panfrost_dump.c:115:20: error: 'struct panfrost_job' has no member named 'file_priv'
>      115 |         as_nr = job->file_priv->mmu->as;
>          |                    ^~

FWIW -- this is due to recent changes in panfrost, you should rebase on
the latest drm-misc-next which is where the patch will be applied
anyway.

Re: [PATCH v2 1/1] drm/panfrost: Add support for devcoredump

[Alyssa Rosenzweig]

[-- Attachment #1: Type: text/plain, Size: 438 bytes --]

>    drivers/gpu/drm/panfrost/panfrost_dump.c: In function 'panfrost_core_dump':
> >> drivers/gpu/drm/panfrost/panfrost_dump.c:115:20: error: 'struct panfrost_job' has no member named 'file_priv'
>      115 |         as_nr = job->file_priv->mmu->as;
>          |                    ^~

FWIW -- this is due to recent changes in panfrost, you should rebase on
the latest drm-misc-next which is where the patch will be applied
anyway.

Re: [kbuild-all] Re: [PATCH v2 1/1] drm/panfrost: Add support for devcoredump

[Chen, Rong A]

On 6/21/2022 10:32 PM, Alyssa Rosenzweig wrote:
>>     drivers/gpu/drm/panfrost/panfrost_dump.c: In function 'panfrost_core_dump':
>>>> drivers/gpu/drm/panfrost/panfrost_dump.c:115:20: error: 'struct panfrost_job' has no member named 'file_priv'
>>       115 |         as_nr = job->file_priv->mmu->as;
>>           |                    ^~
> 
> FWIW -- this is due to recent changes in panfrost, you should rebase on
> the latest drm-misc-next which is where the patch will be applied
> anyway.

Hi Alyssa,

Thanks for your help, we'll try drm-misc-next next time.

Best Regards,
Rong Chen

Re: [kbuild-all] Re: [PATCH v2 1/1] drm/panfrost: Add support for devcoredump

[Alyssa Rosenzweig]

Hi Rong Chen,

Sorry for the noise -- I think that was meant for Adrian!

Apologies,

Alyssa

On Wed, Jun 22, 2022 at 10:30:00AM +0800, Chen, Rong A wrote:
> 
> 
> On 6/21/2022 10:32 PM, Alyssa Rosenzweig wrote:
> > >     drivers/gpu/drm/panfrost/panfrost_dump.c: In function 'panfrost_core_dump':
> > > > > drivers/gpu/drm/panfrost/panfrost_dump.c:115:20: error: 'struct panfrost_job' has no member named 'file_priv'
> > >       115 |         as_nr = job->file_priv->mmu->as;
> > >           |                    ^~
> > 
> > FWIW -- this is due to recent changes in panfrost, you should rebase on
> > the latest drm-misc-next which is where the patch will be applied
> > anyway.
> 
> Hi Alyssa,
> 
> Thanks for your help, we'll try drm-misc-next next time.
> 
> Best Regards,
> Rong Chen

Re: [PATCH v2 1/1] drm/panfrost: Add support for devcoredump

[Alyssa Rosenzweig]

[-- Attachment #1: Type: text/plain, Size: 814 bytes --]

Hi Rong Chen,

Sorry for the noise -- I think that was meant for Adrian!

Apologies,

Alyssa

On Wed, Jun 22, 2022 at 10:30:00AM +0800, Chen, Rong A wrote:
> 
> 
> On 6/21/2022 10:32 PM, Alyssa Rosenzweig wrote:
> > >     drivers/gpu/drm/panfrost/panfrost_dump.c: In function 'panfrost_core_dump':
> > > > > drivers/gpu/drm/panfrost/panfrost_dump.c:115:20: error: 'struct panfrost_job' has no member named 'file_priv'
> > >       115 |         as_nr = job->file_priv->mmu->as;
> > >           |                    ^~
> > 
> > FWIW -- this is due to recent changes in panfrost, you should rebase on
> > the latest drm-misc-next which is where the patch will be applied
> > anyway.
> 
> Hi Alyssa,
> 
> Thanks for your help, we'll try drm-misc-next next time.
> 
> Best Regards,
> Rong Chen

Re: [PATCH v2 1/1] drm/panfrost: Add support for devcoredump

[kernel test robot]

Hi "Adrián,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm/drm-next]
[also build test WARNING on linus/master v5.19-rc2 next-20220617]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Adri-n-Larumbe/devcoredump-support-for-Panfrost-GPU-driver/20220621-103431
base:   git://anongit.freedesktop.org/drm/drm drm-next
config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20220621/202206211320.GPk3Mfi3-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/297bd4948ab1f4eeb78389d57adc1edc819cb6f2
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Adri-n-Larumbe/devcoredump-support-for-Panfrost-GPU-driver/20220621-103431
        git checkout 297bd4948ab1f4eeb78389d57adc1edc819cb6f2
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash drivers/gpu/drm/panfrost/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/panfrost/panfrost_dump.c: In function 'panfrost_core_dump':
   drivers/gpu/drm/panfrost/panfrost_dump.c:115:20: error: 'struct panfrost_job' has no member named 'file_priv'
     115 |         as_nr = job->file_priv->mmu->as;
         |                    ^~
   In file included from include/linux/byteorder/big_endian.h:5,
                    from arch/arc/include/uapi/asm/byteorder.h:14,
                    from include/asm-generic/bitops/le.h:6,
                    from arch/arc/include/asm/bitops.h:192,
                    from include/linux/bitops.h:33,
                    from include/linux/log2.h:12,
                    from include/asm-generic/div64.h:55,
                    from ./arch/arc/include/generated/asm/div64.h:1,
                    from include/linux/math.h:6,
                    from include/linux/math64.h:6,
                    from include/linux/time64.h:5,
                    from include/linux/restart_block.h:10,
                    from include/linux/thread_info.h:14,
                    from include/asm-generic/preempt.h:5,
                    from ./arch/arc/include/generated/asm/preempt.h:1,
                    from include/linux/preempt.h:78,
                    from include/linux/rcupdate.h:27,
                    from include/linux/rculist.h:11,
                    from include/linux/pid.h:5,
                    from include/linux/sched.h:14,
                    from include/linux/ratelimit.h:6,
                    from include/linux/dev_printk.h:16,
                    from include/linux/device.h:15,
                    from drivers/gpu/drm/panfrost/panfrost_dump.c:5:
>> include/uapi/linux/byteorder/big_endian.h:32:26: warning: conversion from 'long long unsigned int' to '__le32' {aka 'unsigned int'} changes value from '72057594037927936' to '0' [-Woverflow]
      32 | #define __cpu_to_le64(x) ((__force __le64)__swab64((x)))
         |                          ^
   include/linux/byteorder/generic.h:86:21: note: in expansion of macro '__cpu_to_le64'
      86 | #define cpu_to_le64 __cpu_to_le64
         |                     ^~~~~~~~~~~~~
   drivers/gpu/drm/panfrost/panfrost_dump.c:225:41: note: in expansion of macro 'cpu_to_le64'
     225 |                 iter.hdr->bomap.valid = cpu_to_le64(1);
         |                                         ^~~~~~~~~~~


vim +32 include/uapi/linux/byteorder/big_endian.h

5921e6f8809b16 David Howells 2012-10-13  15  
5921e6f8809b16 David Howells 2012-10-13  16  #define __constant_htonl(x) ((__force __be32)(__u32)(x))
5921e6f8809b16 David Howells 2012-10-13  17  #define __constant_ntohl(x) ((__force __u32)(__be32)(x))
5921e6f8809b16 David Howells 2012-10-13  18  #define __constant_htons(x) ((__force __be16)(__u16)(x))
5921e6f8809b16 David Howells 2012-10-13  19  #define __constant_ntohs(x) ((__force __u16)(__be16)(x))
5921e6f8809b16 David Howells 2012-10-13  20  #define __constant_cpu_to_le64(x) ((__force __le64)___constant_swab64((x)))
5921e6f8809b16 David Howells 2012-10-13  21  #define __constant_le64_to_cpu(x) ___constant_swab64((__force __u64)(__le64)(x))
5921e6f8809b16 David Howells 2012-10-13  22  #define __constant_cpu_to_le32(x) ((__force __le32)___constant_swab32((x)))
5921e6f8809b16 David Howells 2012-10-13  23  #define __constant_le32_to_cpu(x) ___constant_swab32((__force __u32)(__le32)(x))
5921e6f8809b16 David Howells 2012-10-13  24  #define __constant_cpu_to_le16(x) ((__force __le16)___constant_swab16((x)))
5921e6f8809b16 David Howells 2012-10-13  25  #define __constant_le16_to_cpu(x) ___constant_swab16((__force __u16)(__le16)(x))
5921e6f8809b16 David Howells 2012-10-13  26  #define __constant_cpu_to_be64(x) ((__force __be64)(__u64)(x))
5921e6f8809b16 David Howells 2012-10-13  27  #define __constant_be64_to_cpu(x) ((__force __u64)(__be64)(x))
5921e6f8809b16 David Howells 2012-10-13  28  #define __constant_cpu_to_be32(x) ((__force __be32)(__u32)(x))
5921e6f8809b16 David Howells 2012-10-13  29  #define __constant_be32_to_cpu(x) ((__force __u32)(__be32)(x))
5921e6f8809b16 David Howells 2012-10-13  30  #define __constant_cpu_to_be16(x) ((__force __be16)(__u16)(x))
5921e6f8809b16 David Howells 2012-10-13  31  #define __constant_be16_to_cpu(x) ((__force __u16)(__be16)(x))
5921e6f8809b16 David Howells 2012-10-13 @32  #define __cpu_to_le64(x) ((__force __le64)__swab64((x)))
5921e6f8809b16 David Howells 2012-10-13  33  #define __le64_to_cpu(x) __swab64((__force __u64)(__le64)(x))
5921e6f8809b16 David Howells 2012-10-13  34  #define __cpu_to_le32(x) ((__force __le32)__swab32((x)))
5921e6f8809b16 David Howells 2012-10-13  35  #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
5921e6f8809b16 David Howells 2012-10-13  36  #define __cpu_to_le16(x) ((__force __le16)__swab16((x)))
5921e6f8809b16 David Howells 2012-10-13  37  #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
5921e6f8809b16 David Howells 2012-10-13  38  #define __cpu_to_be64(x) ((__force __be64)(__u64)(x))
5921e6f8809b16 David Howells 2012-10-13  39  #define __be64_to_cpu(x) ((__force __u64)(__be64)(x))
5921e6f8809b16 David Howells 2012-10-13  40  #define __cpu_to_be32(x) ((__force __be32)(__u32)(x))
5921e6f8809b16 David Howells 2012-10-13  41  #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
5921e6f8809b16 David Howells 2012-10-13  42  #define __cpu_to_be16(x) ((__force __be16)(__u16)(x))
5921e6f8809b16 David Howells 2012-10-13  43  #define __be16_to_cpu(x) ((__force __u16)(__be16)(x))
5921e6f8809b16 David Howells 2012-10-13  44  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH v2 1/1] drm/panfrost: Add support for devcoredump

[kernel test robot]

Hi "Adrián,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm/drm-next]
[also build test ERROR on linus/master v5.19-rc3 next-20220620]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Adri-n-Larumbe/devcoredump-support-for-Panfrost-GPU-driver/20220621-103431
base:   git://anongit.freedesktop.org/drm/drm drm-next
config: s390-buildonly-randconfig-r006-20220620 (https://download.01.org/0day-ci/archive/20220621/202206211507.zzo0qSdL-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project af6d2a0b6825e71965f3e2701a63c239fa0ad70f)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install s390 cross compiling tool for clang build
        # apt-get install binutils-s390x-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/297bd4948ab1f4eeb78389d57adc1edc819cb6f2
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Adri-n-Larumbe/devcoredump-support-for-Panfrost-GPU-driver/20220621-103431
        git checkout 297bd4948ab1f4eeb78389d57adc1edc819cb6f2
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash drivers/gpu/drm/panfrost/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All error/warnings (new ones prefixed by >>):

   In file included from drivers/gpu/drm/panfrost/panfrost_dump.c:6:
   In file included from include/linux/devcoredump.h:12:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:464:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:477:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
                                                             ^
   include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
   #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
                                                        ^
   In file included from drivers/gpu/drm/panfrost/panfrost_dump.c:6:
   In file included from include/linux/devcoredump.h:12:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
   #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
                                                        ^
   In file included from drivers/gpu/drm/panfrost/panfrost_dump.c:6:
   In file included from include/linux/devcoredump.h:12:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:609:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsb(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:617:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsw(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:625:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsl(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:634:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesb(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:643:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesw(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:652:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesl(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
>> drivers/gpu/drm/panfrost/panfrost_dump.c:115:15: error: no member named 'file_priv' in 'struct panfrost_job'
           as_nr = job->file_priv->mmu->as;
                   ~~~  ^
>> drivers/gpu/drm/panfrost/panfrost_dump.c:225:27: warning: implicit conversion from '__le64' (aka 'unsigned long long') to '__le32' (aka 'unsigned int') changes value from 72057594037927936 to 0 [-Wconstant-conversion]
                   iter.hdr->bomap.valid = cpu_to_le64(1);
                                         ~ ^~~~~~~~~~~~~~
   include/linux/byteorder/generic.h:86:21: note: expanded from macro 'cpu_to_le64'
   #define cpu_to_le64 __cpu_to_le64
                       ^
   include/uapi/linux/byteorder/big_endian.h:32:27: note: expanded from macro '__cpu_to_le64'
   #define __cpu_to_le64(x) ((__force __le64)__swab64((x)))
                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   13 warnings and 1 error generated.


vim +115 drivers/gpu/drm/panfrost/panfrost_dump.c

   102	
   103	void panfrost_core_dump(struct panfrost_job *job)
   104	{
   105		struct panfrost_device *pfdev = job->pfdev;
   106		struct panfrost_dump_iterator iter;
   107		struct drm_gem_object *dbo;
   108		unsigned int n_obj, n_bomap_pages;
   109		__le64 *bomap, *bomap_start;
   110		size_t file_size;
   111		u32 as_nr;
   112		int slot;
   113		int ret, i;
   114	
 > 115		as_nr = job->file_priv->mmu->as;
   116		slot = panfrost_job_get_slot(job);
   117		slot = slot ? slot : 0;
   118	
   119		/* Only catch the first event, or when manually re-armed */
   120		if (!panfrost_dump_core)
   121			return;
   122		panfrost_dump_core = false;
   123	
   124		/* At least, we dump registers and end marker */
   125		n_obj = 2;
   126		n_bomap_pages = 0;
   127		file_size = ARRAY_SIZE(panfrost_dump_registers) *
   128				sizeof(struct panfrost_dump_registers);
   129	
   130		/* Add in the active buffer objects */
   131		for (i = 0; i < job->bo_count; i++) {
   132			dbo = job->bos[i];
   133			file_size += dbo->size;
   134			n_bomap_pages += dbo->size >> PAGE_SHIFT;
   135			n_obj++;
   136		}
   137	
   138		/* If we have any buffer objects, add a bomap object */
   139		if (n_bomap_pages) {
   140			file_size += n_bomap_pages * sizeof(*bomap);
   141			n_obj++;
   142		}
   143	
   144		/* Add the size of the headers */
   145		file_size += sizeof(*iter.hdr) * n_obj;
   146	
   147		/* Allocate the file in vmalloc memory, it's likely to be big */
   148		iter.start = __vmalloc(file_size, GFP_KERNEL | __GFP_NOWARN |
   149				__GFP_NORETRY);
   150		if (!iter.start) {
   151			dev_warn(pfdev->dev, "failed to allocate devcoredump file\n");
   152			return;
   153		}
   154	
   155		/* Point the data member after the headers */
   156		iter.hdr = iter.start;
   157		iter.data = &iter.hdr[n_obj];
   158	
   159		memset(iter.hdr, 0, iter.data - iter.start);
   160	
   161		/*
   162		 * For now, we write the job identifier in the register dump header,
   163		 * so that we can decode the entire dump later with pandecode
   164		 */
   165		iter.hdr->reghdr.jc = cpu_to_le64(job->jc);
   166		iter.hdr->reghdr.version = cpu_to_le32(PANFROSTDUMP_VERSION_1);
   167		iter.hdr->reghdr.gpu_id = cpu_to_le32(pfdev->features.id);
   168		iter.hdr->reghdr.nbos = cpu_to_le64(job->bo_count);
   169	
   170		panfrost_core_dump_registers(&iter, pfdev, as_nr, slot);
   171	
   172		/* Reserve space for the bomap */
   173		if (job->bo_count) {
   174			bomap_start = bomap = iter.data;
   175			memset(bomap, 0, sizeof(*bomap) * n_bomap_pages);
   176			panfrost_core_dump_header(&iter, PANFROSTDUMP_BUF_BOMAP,
   177						  bomap + n_bomap_pages);
   178		}
   179	
   180		for (i = 0; i < job->bo_count; i++) {
   181			struct iosys_map map;
   182			struct panfrost_gem_mapping *mapping;
   183			struct panfrost_gem_object *bo;
   184			struct sg_page_iter page_iter;
   185			void *vaddr;
   186	
   187			bo = to_panfrost_bo(job->bos[i]);
   188			mapping = job->mappings[i];
   189	
   190			if (!bo->base.sgt) {
   191				dev_err(pfdev->dev, "Panfrost Dump: BO has no sgt, cannot dump\n");
   192				iter.hdr->bomap.valid = 0;
   193				goto dump_header;
   194			}
   195	
   196			ret = drm_gem_shmem_vmap(&bo->base, &map);
   197			if (ret) {
   198				dev_err(pfdev->dev, "Panfrost Dump: couldn't map Buffer Object\n");
   199				iter.hdr->bomap.valid = 0;
   200				goto dump_header;
   201			}
   202	
   203			WARN_ON(!mapping->active);
   204	
   205			iter.hdr->bomap.data[0] = cpu_to_le32((bomap - bomap_start));
   206	
   207			for_each_sgtable_page(bo->base.sgt, &page_iter, 0) {
   208				struct page *page = sg_page_iter_page(&page_iter);
   209	
   210				if (!IS_ERR(page))
   211					*bomap++ = cpu_to_le64(page_to_phys(page));
   212				else {
   213					dev_err(pfdev->dev, "Panfrost Dump: wrong page\n");
   214					*bomap++ = ~cpu_to_le64(0);
   215				}
   216			}
   217	
   218			iter.hdr->bomap.iova = cpu_to_le64(mapping->mmnode.start << PAGE_SHIFT);
   219	
   220			vaddr = map.vaddr;
   221			memcpy(iter.data, vaddr, bo->base.base.size);
   222	
   223			drm_gem_shmem_vunmap(&bo->base, &map);
   224	
 > 225			iter.hdr->bomap.valid = cpu_to_le64(1);
   226	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH v2 1/1] drm/panfrost: Add support for devcoredump

[kernel test robot]

Hi "Adrián,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm/drm-next]
[also build test ERROR on linus/master v5.19-rc3 next-20220620]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Adri-n-Larumbe/devcoredump-support-for-Panfrost-GPU-driver/20220621-103431
base:   git://anongit.freedesktop.org/drm/drm drm-next
config: s390-buildonly-randconfig-r006-20220620 (https://download.01.org/0day-ci/archive/20220621/202206211507.zzo0qSdL-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project af6d2a0b6825e71965f3e2701a63c239fa0ad70f)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install s390 cross compiling tool for clang build
        # apt-get install binutils-s390x-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/297bd4948ab1f4eeb78389d57adc1edc819cb6f2
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Adri-n-Larumbe/devcoredump-support-for-Panfrost-GPU-driver/20220621-103431
        git checkout 297bd4948ab1f4eeb78389d57adc1edc819cb6f2
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash drivers/gpu/drm/panfrost/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All error/warnings (new ones prefixed by >>):

   In file included from drivers/gpu/drm/panfrost/panfrost_dump.c:6:
   In file included from include/linux/devcoredump.h:12:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:464:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:477:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
                                                             ^
   include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
   #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
                                                        ^
   In file included from drivers/gpu/drm/panfrost/panfrost_dump.c:6:
   In file included from include/linux/devcoredump.h:12:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
   #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
                                                        ^
   In file included from drivers/gpu/drm/panfrost/panfrost_dump.c:6:
   In file included from include/linux/devcoredump.h:12:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:609:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsb(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:617:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsw(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:625:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsl(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:634:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesb(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:643:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesw(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:652:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesl(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
>> drivers/gpu/drm/panfrost/panfrost_dump.c:115:15: error: no member named 'file_priv' in 'struct panfrost_job'
           as_nr = job->file_priv->mmu->as;
                   ~~~  ^
>> drivers/gpu/drm/panfrost/panfrost_dump.c:225:27: warning: implicit conversion from '__le64' (aka 'unsigned long long') to '__le32' (aka 'unsigned int') changes value from 72057594037927936 to 0 [-Wconstant-conversion]
                   iter.hdr->bomap.valid = cpu_to_le64(1);
                                         ~ ^~~~~~~~~~~~~~
   include/linux/byteorder/generic.h:86:21: note: expanded from macro 'cpu_to_le64'
   #define cpu_to_le64 __cpu_to_le64
                       ^
   include/uapi/linux/byteorder/big_endian.h:32:27: note: expanded from macro '__cpu_to_le64'
   #define __cpu_to_le64(x) ((__force __le64)__swab64((x)))
                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   13 warnings and 1 error generated.


vim +115 drivers/gpu/drm/panfrost/panfrost_dump.c

   102	
   103	void panfrost_core_dump(struct panfrost_job *job)
   104	{
   105		struct panfrost_device *pfdev = job->pfdev;
   106		struct panfrost_dump_iterator iter;
   107		struct drm_gem_object *dbo;
   108		unsigned int n_obj, n_bomap_pages;
   109		__le64 *bomap, *bomap_start;
   110		size_t file_size;
   111		u32 as_nr;
   112		int slot;
   113		int ret, i;
   114	
 > 115		as_nr = job->file_priv->mmu->as;
   116		slot = panfrost_job_get_slot(job);
   117		slot = slot ? slot : 0;
   118	
   119		/* Only catch the first event, or when manually re-armed */
   120		if (!panfrost_dump_core)
   121			return;
   122		panfrost_dump_core = false;
   123	
   124		/* At least, we dump registers and end marker */
   125		n_obj = 2;
   126		n_bomap_pages = 0;
   127		file_size = ARRAY_SIZE(panfrost_dump_registers) *
   128				sizeof(struct panfrost_dump_registers);
   129	
   130		/* Add in the active buffer objects */
   131		for (i = 0; i < job->bo_count; i++) {
   132			dbo = job->bos[i];
   133			file_size += dbo->size;
   134			n_bomap_pages += dbo->size >> PAGE_SHIFT;
   135			n_obj++;
   136		}
   137	
   138		/* If we have any buffer objects, add a bomap object */
   139		if (n_bomap_pages) {
   140			file_size += n_bomap_pages * sizeof(*bomap);
   141			n_obj++;
   142		}
   143	
   144		/* Add the size of the headers */
   145		file_size += sizeof(*iter.hdr) * n_obj;
   146	
   147		/* Allocate the file in vmalloc memory, it's likely to be big */
   148		iter.start = __vmalloc(file_size, GFP_KERNEL | __GFP_NOWARN |
   149				__GFP_NORETRY);
   150		if (!iter.start) {
   151			dev_warn(pfdev->dev, "failed to allocate devcoredump file\n");
   152			return;
   153		}
   154	
   155		/* Point the data member after the headers */
   156		iter.hdr = iter.start;
   157		iter.data = &iter.hdr[n_obj];
   158	
   159		memset(iter.hdr, 0, iter.data - iter.start);
   160	
   161		/*
   162		 * For now, we write the job identifier in the register dump header,
   163		 * so that we can decode the entire dump later with pandecode
   164		 */
   165		iter.hdr->reghdr.jc = cpu_to_le64(job->jc);
   166		iter.hdr->reghdr.version = cpu_to_le32(PANFROSTDUMP_VERSION_1);
   167		iter.hdr->reghdr.gpu_id = cpu_to_le32(pfdev->features.id);
   168		iter.hdr->reghdr.nbos = cpu_to_le64(job->bo_count);
   169	
   170		panfrost_core_dump_registers(&iter, pfdev, as_nr, slot);
   171	
   172		/* Reserve space for the bomap */
   173		if (job->bo_count) {
   174			bomap_start = bomap = iter.data;
   175			memset(bomap, 0, sizeof(*bomap) * n_bomap_pages);
   176			panfrost_core_dump_header(&iter, PANFROSTDUMP_BUF_BOMAP,
   177						  bomap + n_bomap_pages);
   178		}
   179	
   180		for (i = 0; i < job->bo_count; i++) {
   181			struct iosys_map map;
   182			struct panfrost_gem_mapping *mapping;
   183			struct panfrost_gem_object *bo;
   184			struct sg_page_iter page_iter;
   185			void *vaddr;
   186	
   187			bo = to_panfrost_bo(job->bos[i]);
   188			mapping = job->mappings[i];
   189	
   190			if (!bo->base.sgt) {
   191				dev_err(pfdev->dev, "Panfrost Dump: BO has no sgt, cannot dump\n");
   192				iter.hdr->bomap.valid = 0;
   193				goto dump_header;
   194			}
   195	
   196			ret = drm_gem_shmem_vmap(&bo->base, &map);
   197			if (ret) {
   198				dev_err(pfdev->dev, "Panfrost Dump: couldn't map Buffer Object\n");
   199				iter.hdr->bomap.valid = 0;
   200				goto dump_header;
   201			}
   202	
   203			WARN_ON(!mapping->active);
   204	
   205			iter.hdr->bomap.data[0] = cpu_to_le32((bomap - bomap_start));
   206	
   207			for_each_sgtable_page(bo->base.sgt, &page_iter, 0) {
   208				struct page *page = sg_page_iter_page(&page_iter);
   209	
   210				if (!IS_ERR(page))
   211					*bomap++ = cpu_to_le64(page_to_phys(page));
   212				else {
   213					dev_err(pfdev->dev, "Panfrost Dump: wrong page\n");
   214					*bomap++ = ~cpu_to_le64(0);
   215				}
   216			}
   217	
   218			iter.hdr->bomap.iova = cpu_to_le64(mapping->mmnode.start << PAGE_SHIFT);
   219	
   220			vaddr = map.vaddr;
   221			memcpy(iter.data, vaddr, bo->base.base.size);
   222	
   223			drm_gem_shmem_vunmap(&bo->base, &map);
   224	
 > 225			iter.hdr->bomap.valid = cpu_to_le64(1);
   226	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH v2 1/1] drm/panfrost: Add support for devcoredump

[Alyssa Rosenzweig]

Hi Adrian,

Great work on the devcoredump support! This is really cool to see coming
along, thank you! I've left a few notes below:

> +		if (panfrost_dump_registers[i] >= JS_HEAD_LO(0) &&
> +		    panfrost_dump_registers[i] <= JS_CONFIG_NEXT(0))
> +			js_as_offset = slot * 0x80;
> +		else if (panfrost_dump_registers[i] >= AS_TRANSTAB_LO(0) &&
> +			 panfrost_dump_registers[i] <= AS_STATUS(0))
> +			js_as_offset = ((as_nr) << 6);

I'm not a fan of the magic numbers. Do you think it makes sense to add

	#define JS_SLOT_STRIDE 0x80
	#define MMU_AS_SHIFT 0x6

in the appropriate places in panfrost_regs.h, reexpress the existing
#defines in terms of those

	#define JS_HEAD_LO(n) (JS_BASE + ((n) * JS_SLOT_STRIDE) + 0x00)
	...
	#define JS_FLUSH_ID_NEXT(n) (JS_BASE + ((n) * JS_SLOT_STRIDE) + 0x70)
	...
	#define MM_AS(as) (0x2400 + ((as) << MMU_AS_SHIFT)

and then use those here?

Also, drop the parans around (as_nr), this isn't a macro.

> +	/* Add in the active buffer objects */
> +	for (i = 0; i < job->bo_count; i++) {
> +		dbo = job->bos[i];
> +		file_size += dbo->size;
> +		n_bomap_pages += dbo->size >> PAGE_SHIFT;
> +		n_obj++;
> +	}

Strictly, I don't think this is right -- what happens if the CPU is
configured to use 16K or 64K pages? -- however, that mistake is pretty
well entrenched in panfrost.ko right now and it doesn't seem to bother
anyone (non-4K pages on arm64 are pretty rare outside of fruit
computers).

That said, out-of-context there looks like an alignment question. Could
we add an assert for that, documenting the invariant:

	WARN_ON(!IS_ALIGNED(dbo->size, PAGE_SIZE));

> +	iter.start = __vmalloc(file_size, GFP_KERNEL | __GFP_NOWARN |
> +			__GFP_NORETRY);
> +	if (!iter.start) {
> +		dev_warn(pfdev->dev, "failed to allocate devcoredump file\n");
> +		return;
> +	}
> ...
> +	memset(iter.hdr, 0, iter.data - iter.start);

Why are we using __GFP_NOWARN and __GFP_NORETRY? Why not plain vmalloc?

Also, why vmalloc instead of vzalloc? (Or adding __GFP_ZERO to the list
of __vmalloc flags if __GFP_NOWARN/__GFP_NORETRY are really needed?) Are
there relevant performance or security considerations?

> +/* Definitions for coredump decoding in user space */
> +#define PANFROSTDUMP_VERSION_1 1

I'm not a fan of an enum that just represents a number. Using the
numbers directly means we can compare them in a natural way. Also, using
a major/minor split like Steven suggested can help with semantic
versioning.

Cheers,
Alyssa

Re: [PATCH v2 1/1] drm/panfrost: Add support for devcoredump

[Adri??n Larumbe]

Hi Alyssa, thanks for the feedback.

If I don't answer directly to any of your concerns in this message, it's because
I applied your suggestions in v3 of the patch straight away.

On 21.06.2022 09:03, Alyssa Rosenzweig wrote:
> Hi Adrian,
> 
> Great work on the devcoredump support! This is really cool to see coming
> along, thank you! I've left a few notes below:
> 
> > +		if (panfrost_dump_registers[i] >= JS_HEAD_LO(0) &&
> > +		    panfrost_dump_registers[i] <= JS_CONFIG_NEXT(0))
> > +			js_as_offset = slot * 0x80;
> > +		else if (panfrost_dump_registers[i] >= AS_TRANSTAB_LO(0) &&
> > +			 panfrost_dump_registers[i] <= AS_STATUS(0))
> > +			js_as_offset = ((as_nr) << 6);
> 
> I'm not a fan of the magic numbers. Do you think it makes sense to add
> 
> 	#define JS_SLOT_STRIDE 0x80
> 	#define MMU_AS_SHIFT 0x6
> 
> in the appropriate places in panfrost_regs.h, reexpress the existing
> #defines in terms of those
> 
> 	#define JS_HEAD_LO(n) (JS_BASE + ((n) * JS_SLOT_STRIDE) + 0x00)
> 	...
> 	#define JS_FLUSH_ID_NEXT(n) (JS_BASE + ((n) * JS_SLOT_STRIDE) + 0x70)
> 	...
> 	#define MM_AS(as) (0x2400 + ((as) << MMU_AS_SHIFT)
> 
> and then use those here?
> 
> Also, drop the parans around (as_nr), this isn't a macro.
> 
> > +	/* Add in the active buffer objects */
> > +	for (i = 0; i < job->bo_count; i++) {
> > +		dbo = job->bos[i];
> > +		file_size += dbo->size;
> > +		n_bomap_pages += dbo->size >> PAGE_SHIFT;
> > +		n_obj++;
> > +	}
> 
> Strictly, I don't think this is right -- what happens if the CPU is
> configured to use 16K or 64K pages? -- however, that mistake is pretty
> well entrenched in panfrost.ko right now and it doesn't seem to bother
> anyone (non-4K pages on arm64 are pretty rare outside of fruit
> computers).
> 
> That said, out-of-context there looks like an alignment question. Could
> we add an assert for that, documenting the invariant:
> 
> 	WARN_ON(!IS_ALIGNED(dbo->size, PAGE_SIZE));
> 
> > +	iter.start = __vmalloc(file_size, GFP_KERNEL | __GFP_NOWARN |
> > +			__GFP_NORETRY);
> > +	if (!iter.start) {
> > +		dev_warn(pfdev->dev, "failed to allocate devcoredump file\n");
> > +		return;
> > +	}
> > ...
> > +	memset(iter.hdr, 0, iter.data - iter.start);
> 
> Why are we using __GFP_NOWARN and __GFP_NORETRY? Why not plain vmalloc?
> 
> Also, why vmalloc instead of vzalloc? (Or adding __GFP_ZERO to the list
> of __vmalloc flags if __GFP_NOWARN/__GFP_NORETRY are really needed?) Are
> there relevant performance or security considerations?

I borrowed this code from Etnaviv a while ago and the same doubt struck me
then. My understanding of its intended behaviour is that because the dump file
might be huge, we don't want the memory manager to trigger the OOM killer and
annoy quite a few running processes because of a debug feature. Also since the
code already handles the situation when an allocation fails by refusing to
generate a dump, there's no need for the allocator to generate specific error
messages.

So I guess it boils down to 'if there's quite enough memory to allocate a huge
dump file, go ahead, otherwise don't reclaim any processes' pages for something
that isn't essential'.

I don't see much use for __GFP_ZERO in this case, because the dump file gets
memcpy'd with the contents of every single bo so whatever the original
contents of the memory were at the time of the allocation, they're overwritten
immediately.

> > +/* Definitions for coredump decoding in user space */
> > +#define PANFROSTDUMP_VERSION_1 1
> 
> I'm not a fan of an enum that just represents a number. Using the
> numbers directly means we can compare them in a natural way. Also, using
> a major/minor split like Steven suggested can help with semantic
> versioning.
> 
> Cheers,
> Alyssa

I've also rebased v3 on top of drm-misc-next and the compiler error because of
the removed panfrost_job structure member is gone.

Adrian Larumbe

Re: [PATCH v2 1/1] drm/panfrost: Add support for devcoredump

[Alyssa Rosenzweig]

> > > +	iter.start = __vmalloc(file_size, GFP_KERNEL | __GFP_NOWARN |
> > > +			__GFP_NORETRY);
> > > +	if (!iter.start) {
> > > +		dev_warn(pfdev->dev, "failed to allocate devcoredump file\n");
> > > +		return;
> > > +	}
> > > ...
> > > +	memset(iter.hdr, 0, iter.data - iter.start);
> > 
> > Why are we using __GFP_NOWARN and __GFP_NORETRY? Why not plain vmalloc?
> > 
> > Also, why vmalloc instead of vzalloc? (Or adding __GFP_ZERO to the list
> > of __vmalloc flags if __GFP_NOWARN/__GFP_NORETRY are really needed?) Are
> > there relevant performance or security considerations?
> 
> I borrowed this code from Etnaviv a while ago and the same doubt struck me
> then. My understanding of its intended behaviour is that because the dump file
> might be huge, we don't want the memory manager to trigger the OOM killer and
> annoy quite a few running processes because of a debug feature. Also since the
> code already handles the situation when an allocation fails by refusing to
> generate a dump, there's no need for the allocator to generate specific error
> messages.
> 
> So I guess it boils down to 'if there's quite enough memory to allocate a huge
> dump file, go ahead, otherwise don't reclaim any processes' pages for something
> that isn't essential'.
> 
> I don't see much use for __GFP_ZERO in this case, because the dump file gets
> memcpy'd with the contents of every single bo so whatever the original
> contents of the memory were at the time of the allocation, they're overwritten
> immediately.

I think that's a reasonable explanation, bearing in mind I'm firmly a
userspace person ;-)

Please add a comment explaining the assumptions here, though, because
the code will live longer than this ML thread.

> I've also rebased v3 on top of drm-misc-next and the compiler error because of
> the removed panfrost_job structure member is gone.

Excellent

Re: [PATCH v2 1/1] drm/panfrost: Add support for devcoredump

[Lucas Stach]

Am Mittwoch, dem 22.06.2022 um 02:54 +0100 schrieb Adri??n Larumbe:
> Hi Alyssa, thanks for the feedback.
> 
> If I don't answer directly to any of your concerns in this message, it's because
> I applied your suggestions in v3 of the patch straight away.
> 
> On 21.06.2022 09:03, Alyssa Rosenzweig wrote:
> > Hi Adrian,
> > 
> > Great work on the devcoredump support! This is really cool to see coming
> > along, thank you! I've left a few notes below:
> > 
> > > +		if (panfrost_dump_registers[i] >= JS_HEAD_LO(0) &&
> > > +		    panfrost_dump_registers[i] <= JS_CONFIG_NEXT(0))
> > > +			js_as_offset = slot * 0x80;
> > > +		else if (panfrost_dump_registers[i] >= AS_TRANSTAB_LO(0) &&
> > > +			 panfrost_dump_registers[i] <= AS_STATUS(0))
> > > +			js_as_offset = ((as_nr) << 6);
> > 
> > I'm not a fan of the magic numbers. Do you think it makes sense to add
> > 
> > 	#define JS_SLOT_STRIDE 0x80
> > 	#define MMU_AS_SHIFT 0x6
> > 
> > in the appropriate places in panfrost_regs.h, reexpress the existing
> > #defines in terms of those
> > 
> > 	#define JS_HEAD_LO(n) (JS_BASE + ((n) * JS_SLOT_STRIDE) + 0x00)
> > 	...
> > 	#define JS_FLUSH_ID_NEXT(n) (JS_BASE + ((n) * JS_SLOT_STRIDE) + 0x70)
> > 	...
> > 	#define MM_AS(as) (0x2400 + ((as) << MMU_AS_SHIFT)
> > 
> > and then use those here?
> > 
> > Also, drop the parans around (as_nr), this isn't a macro.
> > 
> > > +	/* Add in the active buffer objects */
> > > +	for (i = 0; i < job->bo_count; i++) {
> > > +		dbo = job->bos[i];
> > > +		file_size += dbo->size;
> > > +		n_bomap_pages += dbo->size >> PAGE_SHIFT;
> > > +		n_obj++;
> > > +	}
> > 
> > Strictly, I don't think this is right -- what happens if the CPU is
> > configured to use 16K or 64K pages? -- however, that mistake is pretty
> > well entrenched in panfrost.ko right now and it doesn't seem to bother
> > anyone (non-4K pages on arm64 are pretty rare outside of fruit
> > computers).
> > 
> > That said, out-of-context there looks like an alignment question. Could
> > we add an assert for that, documenting the invariant:
> > 
> > 	WARN_ON(!IS_ALIGNED(dbo->size, PAGE_SIZE));
> > 
> > > +	iter.start = __vmalloc(file_size, GFP_KERNEL | __GFP_NOWARN |
> > > +			__GFP_NORETRY);
> > > +	if (!iter.start) {
> > > +		dev_warn(pfdev->dev, "failed to allocate devcoredump file\n");
> > > +		return;
> > > +	}
> > > ...
> > > +	memset(iter.hdr, 0, iter.data - iter.start);
> > 
> > Why are we using __GFP_NOWARN and __GFP_NORETRY? Why not plain vmalloc?
> > 
> > Also, why vmalloc instead of vzalloc? (Or adding __GFP_ZERO to the list
> > of __vmalloc flags if __GFP_NOWARN/__GFP_NORETRY are really needed?) Are
> > there relevant performance or security considerations?
> 
> I borrowed this code from Etnaviv a while ago and the same doubt struck me
> then. My understanding of its intended behaviour is that because the dump file
> might be huge, we don't want the memory manager to trigger the OOM killer and
> annoy quite a few running processes because of a debug feature. Also since the
> code already handles the situation when an allocation fails by refusing to
> generate a dump, there's no need for the allocator to generate specific error
> messages.

Yes, that's exactly the reasoning behind those flags. Without the
__GFP_NORETRY the devcoredump might not only trigger reclaim, but also
the OOM killer. People might see it as suboptimal user experience when
their favorite app gets killed just to make space for a devcoredump,
which they might not even be interested in. As the code is dealing
properly with allocation failure, we also don't want to print a warning
in the kernel log.

Regards,
Lucas