On 26/07/08 11:01AM, Mukesh Ojha wrote:
> First of all, I want to thank Eugene for his excellent work on this
> series. What began as the Qualcomm Minidump driver from me has now
> evolved into meminspect. He also presented meminspect at Linux
> Plumbers 2025.
> 
> Video of the recording is available here for anyone interested:
> https://www.youtube.com/watch?v=aDZv4-kOLSc
> 
> Introduction: Memory inspection mechanism
> 
> meminspect is a mechanism which allows the kernel to mark specific
> memory areas for memory dumping or specific inspection, statistics,
> usage.  Once regions are marked, meminspect keeps an internal list with
> the regions in a dedicated table.  Further, these regions can be
> accessed using specific API by any interested driver.  Regions being
> marked beforehand, when the system is up and running, there is no need
> nor dependency on a panic handler, or a working kernel that can dump the
> debug information.  meminspect can be primarily used for debugging. The
> approach is feasible to work when pstore, kdump, or another mechanism do
> not. Pstore relies on persistent storage, a dedicated RAM area or
> flash, which has the disadvantage of having the memory reserved all the
> time, or another specific non volatile memory. Some devices cannot keep
> the RAM contents on reboot so ramoops does not work. Some devices do not
> allow kexec to run another kernel to debug the crashed one.  For such
> devices, that have another mechanism to help debugging, like firmware,
> meminspect is a viable solution.
> 
> meminspect can create a core image, similar with /proc/vmcore, with only
> the registered regions included. This can be loaded into crash tool/gdb
> and analyzed. This happens if CRASH_DUMP=y.  To have this working,
> specific information from the kernel is registered, and this is done at
> meminspect init time, no need for the meminspect users to do anything.
> 
> This version of the meminspect patch series includes two drivers that
> make use of it: one is the Qualcomm Minidump, and the other one is the
> Debug Kinfo backend for Android devices, reworked from this source here:
> https://android.googlesource.com/kernel/common/+/refs/heads/android-mainline/drivers/android/debug_kinfo.c
> written originally by Jone Chou <[email protected]>
> 
> Tested on Qualcomm SA8775P (Lemans EVK) on next-20260707 with the
> minidump backend: kernel crash dump regions correctly captured
> via firmware.
> 
> This series introduces a new cross-subsystem framework and, to
> avoid merge ordering issues, I suggest the entire series be taken
> via the mm tree. Subsystem maintainers for touched files have
> been CC'd; I request an Acked-by from them if the respective
> patches look fine from their side.


You are adding a cross-subsystem framework without tests?  How did this
get to v3 without tests?  Surely there were bugs during development and
you added new tests to make sure there wasn't regressions?  Where are
those?

It's a firmware debug framework for qualcomm devices?  Why would we want
to allow such an invasive change in core kernel infrastructure to help
keep firmware blobs big?  It really sounds like the wrong direction all
together.


> 
> Alternatively, the qcom backend patches (22-24) can be deferred
> to Bjorn's qcom tree in a follow-on series once the core
> meminspect framework lands.
> 
> *** How to use meminspect with minidump backend on Qualcomm platform guide ***
> 
> Prerequisites:
> Crash tool compiled with target=ARM64.

What other targets are supported?

> 
> Target kernel must be built with: CONFIG_DEBUG_INFO_REDUCED=n;
> this will have vmlinux include all the debugging information
> needed for the crash tool and CONFIG_MEMINSPECT,
> CONFIG_CRASH_DUMP, and the driver CONFIG_QCOM_MINIDUMP.
> Kernel arguments: Kernel firmware must be set to mode 'mini' by kernel
> module parameter like this : qcom_scm.download_mode=mini
> 
> After the kernel boots and the minidump module is loaded,
> everything is ready for a possible crash. Upon triggering a
> forced kernel crash, the target board will wait in download mode,
> where QDL (https://github.com/linux-msm/qdl) running on the host
> connected to the target (here, Lemans EVK) can be used to collect
> minidump.elf from the target device onto the host.
> 
> Currently, --minimal mode is being used to get dmesg log from
> crashed device.
> 
> Without --minimal mode, the crash tool needs to be patched to
> not crash on missing symbols and needs to be tuned, which I am
> currently working on in parallel.

It sounds like your debug tool needs more development to avoid crashing?

> 
> crash> log
> [    0.000000] Booting Linux on physical CPU 0x0000000000 [0x514f0014]
> [    0.000000] Linux version 7.0.0-rc3-next-20260309-00028-g528b3c656121 
> (@21e3bca4168f) (aarch64-linux-gnu-gcc (Ubuntu 13.3.0-6ubuntu2~24.04) 13.3.0, 
> GNU ld (GNU Binutils for Ubuntu) 2.42) #5 SMP PREEMPT Tue Mar 10 18:18:41 UTC 
> 2026
> 
> *** Debug Kinfo backend driver ***
> I need help with the testing of this driver, Anyone who actually wants
> to test this, feel free to reply to the patch. we have also written a
> simple DT binding for the driver.
> 
> Thanks in advance for the review, and apologies if I missed addressing any 
> comment.
> 
> -Mukesh
> 
> Changes in v3: 
> https://lore.kernel.org/lkml/[email protected]/
> - Addressed most of doc. comments and binding and 0-day kernel
>   test robot warning.
> - Add timekeeping: Register tk_data into meminspect
> - fixed some of the bugs identified during testing.
> - Fix missing return before dev_err_probe() in qcom_md_probe(); all
>   three error paths (SMEM lookup, table validation, table init) now
>   correctly abort probe on failure
> - Fix meminspect_traverse() kernel-doc: @priv description corrected
> - Fix meminspect.rst: replace stale MEMINSPECT_ITERATOR_CB type name
>   with the actual meminspect_iter_cb_t typedef
> - Add kernel-doc for meminspect_iter_cb_t typedef
> - Lot of code formating and style change as per coding standard.
> 
> Changes in v2: 
> https://lore.kernel.org/lkml/[email protected]/
>  - Fixed doc warnings
>  - Fixed kernel-test robot warnings.
>  - Took Mike suggestion to remove mark inspect flag for dynamic memory.
>  - Added R-b for printk patch.
>  - Modified some commit messages for clarity.
>  - corrected binding change for debug-kinfo as per Rob suggestion.
> 
> Changelog for meminspect v1:
> - rename to meminspect
> - start on top of v2 actually, with the section and all.
> - remove the backend thing, change the API to access the table
> - move everything to kernel/
> - add dependency to CRASH_DUMP instead of a separate knob
> - move the minidump driver to soc/qcom
> - integrate the meminspect better into memblock by using a new memblock flag
> - minor fixes : use dev_err_probe everywhere, rearrange variable declarations,
> remove some useless code, etc.
> 
> Changelog for RFC v3:
> - V2 available here : 
> https://lore.kernel.org/all/[email protected]/
> - Removed the .section as requested by David Hildenbrand.
> - Moved all kmemdump registration(when possible) to vmcoreinfo.
> - Because of this, some of the variables that I was registering had to be 
> non-static
> so I had to modify this as per David Hildenbrand suggestion.
> - Fixed minor things in the Kinfo driver: one field was broken, fixed some
> compiler warnings, fixed the copyright and remove some useless includes.
> - Moved the whole kmemdump from drivers/debug into mm/ and Kconfigs into 
> mm/Kconfig.debug
> and it's now available in kernel hacking, as per Randy Dunlap review
> - Reworked some of the Documentation as per review from Jon Corbet
> 
> Changelog for RFC v2:
> - V1 available here: 
> https://lore.kernel.org/lkml/[email protected]/
> - Reworked the whole minidump implementation based on suggestions from Thomas 
> Gleixner.
> This means new API, macros, new way to store the regions inside kmemdump
> (ditched the IDR, moved to static allocation, have a static default backend, 
> etc)
> - Reworked qcom_minidump driver based on review from Bjorn Andersson
> - Reworked printk log buffer registration based on review from Petr Mladek.
> 
> ---
> Eugen Hristev (21):
>       kernel: Introduce meminspect
>       init/version: Annotate static information into meminspect
>       mm/percpu: Annotate static information into meminspect
>       cpu: Annotate static information into meminspect
>       genirq/irqdesc: Annotate static information into meminspect
>       timers: Annotate static information into meminspect
>       kernel/fork: Annotate static information into meminspect
>       mm/page_alloc: Annotate static information into meminspect
>       mm/show_mem: Annotate static information into meminspect
>       mm/swapfile: Annotate static information into meminspect
>       kernel/vmcore_info: Register dynamic information into meminspect
>       kernel/configs: Register dynamic information into meminspect
>       mm/init-mm: Annotate static information into meminspect
>       panic: Annotate static information into meminspect
>       kallsyms: Annotate static information into meminspect
>       mm/mm_init: Annotate static information into meminspect
>       sched/core: Annotate runqueues into meminspect
>       remoteproc: qcom: Move minidump data structures into its own header
>       soc: qcom: Add minidump backend driver
>       soc: qcom: smem: Add minidump platform device
>       meminspect: Add debug kinfo compatible driver
> 
> Mukesh Ojha (5):
>       timekeeping: Register tk_data into meminspect
>       mm/numa: Register node data information into meminspect
>       mm/sparse: Register information into meminspect
>       printk: Register information into meminspect
>       dt-bindings: reserved-memory: Add Google Kinfo Pixel reserved memory
> 
>  Documentation/dev-tools/index.rst                  |   1 +
>  Documentation/dev-tools/meminspect.rst             | 144 +++++++
>  .../reserved-memory/google,debug-kinfo.yaml        |  48 +++

Probably shouldn't be here?

>  MAINTAINERS                                        |  16 +
...

I really think you need to step back and think about how your firmware
and driver can gain information without requiring so many tentacles
across the kernel but also without having an entire dump of ram.  Maybe
your firmware is doing some of what the driver should be doing?

It really seems like you are trying to upstream QA for firmware or
device issues and I don't see an argument against that in any of your
responses to any other emails.

Your problem statement seems to be: a phone freezes and we need our
firmware have global access to all memory to tell us why.  To reduce the
dump size from all of the memory to some subset (up to and including all
of the memory), we want to give our driver access to all of the memory.
Is that correct?

What is even more concerning is that you aren't limiting this to debug,
but also to statistics and usage.  Leaving the door open to larger abuse
of the access this cross-subsystem framework - with no testing - will
allow.

Thanks,
Liam


Reply via email to