[1] was an initial proposal defining testable code specifications for some functions in /drivers/char/mem.c. However a Guideline to write such specifications was missing and test cases tracing to such specifications were missing. This patchset represents a next step and is organised as follows: - patch 1/3 contains the Guideline for writing code specifications - patch 2/3 contains examples of code specfications defined for some functions of drivers/char/mem.c - patch 3/3 contains examples of selftests that map to some code specifications of patch 2/3
[1] https://lore.kernel.org/all/20250821170419.70668-1-gpaoloni@redhat.com/ --- Changes from v1: 1) Added a Guideline to write code specifications in the Linux Kernel Documentation 2) Addressed Greg KH comments in /drivers/char/mem.c 3) Added example of test cases mapping to the code specifications in /drivers/char/mem.c --- Alessandro Carminati (1): selftests/devmem: initial testset
Gabriele Paoloni (2): Documentation: add guidelines for writing testable code specifications /dev/mem: Add initial documentation of memory_open() and mem_fops
.../doc-guide/code-specifications.rst | 208 +++++++ Documentation/doc-guide/index.rst | 1 + drivers/char/mem.c | 231 ++++++- tools/testing/selftests/Makefile | 1 + tools/testing/selftests/devmem/Makefile | 13 + tools/testing/selftests/devmem/debug.c | 25 + tools/testing/selftests/devmem/debug.h | 14 + tools/testing/selftests/devmem/devmem.c | 200 ++++++ tools/testing/selftests/devmem/ram_map.c | 250 ++++++++ tools/testing/selftests/devmem/ram_map.h | 38 ++ tools/testing/selftests/devmem/secret.c | 46 ++ tools/testing/selftests/devmem/secret.h | 13 + tools/testing/selftests/devmem/tests.c | 569 ++++++++++++++++++ tools/testing/selftests/devmem/tests.h | 45 ++ tools/testing/selftests/devmem/utils.c | 379 ++++++++++++ tools/testing/selftests/devmem/utils.h | 119 ++++ 16 files changed, 2146 insertions(+), 6 deletions(-) create mode 100644 Documentation/doc-guide/code-specifications.rst create mode 100644 tools/testing/selftests/devmem/Makefile create mode 100644 tools/testing/selftests/devmem/debug.c create mode 100644 tools/testing/selftests/devmem/debug.h create mode 100644 tools/testing/selftests/devmem/devmem.c create mode 100644 tools/testing/selftests/devmem/ram_map.c create mode 100644 tools/testing/selftests/devmem/ram_map.h create mode 100644 tools/testing/selftests/devmem/secret.c create mode 100644 tools/testing/selftests/devmem/secret.h create mode 100644 tools/testing/selftests/devmem/tests.c create mode 100644 tools/testing/selftests/devmem/tests.h create mode 100644 tools/testing/selftests/devmem/utils.c create mode 100644 tools/testing/selftests/devmem/utils.h
The Documentation/doc-guide/kernel-doc.rst chapter describes how to document the code using the kernel-doc format, however it does not specify the criteria to be followed for writing testable specifications; i.e. specifications that can be used to for the semantic description of low level requirements.
This patch adds a guideline that defines criteria to formally describe developers’ intent at the function and subfunction level in the form of testable expectations.
Signed-off-by: Gabriele Paoloni gpaoloni@redhat.com Signed-off-by: Chuck Wolber chuckwolber@gmail.com Signed-off-by: Kate Stewart kstewart@linuxfoundation.org --- .../doc-guide/code-specifications.rst | 208 ++++++++++++++++++ Documentation/doc-guide/index.rst | 1 + 2 files changed, 209 insertions(+) create mode 100644 Documentation/doc-guide/code-specifications.rst
diff --git a/Documentation/doc-guide/code-specifications.rst b/Documentation/doc-guide/code-specifications.rst new file mode 100644 index 000000000000..dee1b4f089e1 --- /dev/null +++ b/Documentation/doc-guide/code-specifications.rst @@ -0,0 +1,208 @@ +.. title:: How-to write testable code specifications + +========================================= +How-to write testable code specifications +========================================= + +Introduction +------------ +The Documentation/doc-guide/kernel-doc.rst chapter describes how to document the code using the kernel-doc format, however it does not specify the criteria to be followed for writing testable specifications; i.e. specifications that can be used to for the semantic description of low level requirements. + +This chapter defines criteria to formally describe developers’ intent at the function and subfunction level in the form of testable expectations. + +A Virtuous Cycle +---------------- +By adding testable specifications at the function or (where relevant) subfunction level, one enables the creation of a virtuous cycle when testing is supplemented with open source code coverage tools like llvm-cov or Gcov. + +As a true reflection of developer intent, code specifications inform the creation of a pass/fail tests which can then be assessed in conjunction with code coverage tools. A failing test may indicate broken code or specifications that fail to capture developer intent. A gap in code coverage may indicate missing specifications, unintended functionalities, or insufficient test procedure. + +High level goals +---------------- +The code specifications: + +1. Should be maintainable together with the code. +2. Should support hierarchical traceability to allow refinement of SW dependencies (i.e. cross reference critical APIs or data structures). +3. Should describe error conditions and success behaviors. +4. Should describe conditions to be met by the user to avoid unspecified or unwanted behaviours. +5. Should allow covering both static and dynamic aspects of the code. +6. Should be compatible with Documentation/doc-guide/kernel-doc.rst. +7. Should support the definition of a test plan (i.e. syntax should enforce testability as well as the avoidance of untestable specifications, e.g “function_xyz() shall not do something”). + +Format and Syntax +----------------- +Testable code specifications must be written according to the syntax already defined in Documentation/doc-guide/kernel-doc.rst with additional rules that are described below. + +Function name +~~~~~~~~~~~~~ +``* function_name() - Brief description of function.`` + +This field is to be considered informative and is not part of the testable specifications. + +Input Arguments +~~~~~~~~~~~~~~~ +Input arguments should be specified in a way that better supports the function’s expectations and Assumptions of Use described below. +They must not contradict the function's expectations and the function’s prototype. For example:: + + * trace_set_clr_event - enable or disable an event + * @system: system name to match (NULL for any system) + * @event: event name to match (NULL for all events, within system) + * @set: 1 to enable, 0 to disable + * + [...] + * + */ + int trace_set_clr_event(const char *system, const char *event, int set) + +Above all the parameters clearly introduce the impact that they have on the code specifications. + +However if below we had:: + + * trace_set_clr_event - enable or disable an event + * @system: system name to match (NULL for any system) + * @event: event name to match (NULL for all events, within system) + * @set: true to enable, false to disable \ + [...] + */ + int trace_set_clr_event(const char *system, const char *event, int set) + +In this case @set would be a bad definition since it is defined as an integer and not as a boolean. + +Longer Description +~~~~~~~~~~~~~~~~~~ +The `Longer Description` section is where the large part of testable code specifications are defined. The section must be organised as follows:: + + * (Summary Description) provides an introduction of the functionalities + * provided by the function and any informal note. This text does not + * represent any testable code specification. + * + * + * Function's expectations: + * [ID1] - [code expectation] + * + * [ID2] - [code expectation] + * + * [...] + * + * [IDn] - [code expectation] + * + * Assumptions of Use: + * [ID1] - [constraint to be met by the caller] + * + * [ID2] - [constraint to be met by the caller] + * + * [IDn] - [constraint to be met by the caller] + * + +When writing the above section the following rules must be followed: + +* No rules apply to the text above ``Function’s expectations``; such a text does not constitute testable specifications and it is just informative; +* Both ``Function’s expectations`` and ``Assumptions of Use`` must be listed prefixing each of them with an ID that is unique within this kernel-doc header. The reason for this is to facilitate cross-referencing and traceability between tests and code specifications. +* A Function’s expectation is a testable behavior that the function is expected to comply with (i.e. the function is expected to behave as defined in the function’s expectation). +* An Assumption of Use is a pre-condition to be met when invoking the function being documented. +* Testable functional expectations and Assumptions of Use must be constructed according the same rules that apply when writing software requirements: + * Statements should include a subject and a verb, together with other elements necessary to adequately express the information content of the specifications. + * The verbs are required to use the following keywords: + * For mandatory expectations the verb ‘shall’ is to be used; + * For descriptive text that do not constitute a testable expectation verbs such as ‘are’, ‘is’, ‘was’ are to be used; + * Negative expectations must be avoided (e.g. ‘shall not’ must be avoided). +* Statements must be constructed according to the following scheme: + + [**Condition**] [**Subject**] [**Verb/Action**] [**Object**] [**Constraint of Action**]. + + In this regard [**Condition**] and [**Constraint of Action**] could be omitted respectively if the [**Action**] being specified must always happen or if there are no constraints associated with it. + +Function Context +~~~~~~~~~~~~~~~~ +The function’s context represents an integral part of Function’s expectations and Assumptions of Use, where these can further specify the information contained in this section. + +Without further specifications this section is to be interpreted as per example below: + +``* Context: Any context.`` + +The function shall execute in any possible context. + +``* Context: Any context. Takes and releases the RCU lock.`` + +The function shall execute in any possible context. +The function shall take and release the RCU lock. + +``* Context: Any context. Expects <lock> to be held by caller.`` + +The function shall execute in any possible context. +<lock> is assumed to be held before this function is called. + +``* Context: Process context. May sleep if @gfp flags permit.`` + +The function shall execute in process context. +The function shall sleep according to @gfp flags definitions + +``* Context: Process context. Takes and releases <mutex>.`` + +The function shall execute in process context. +The function shall take and release <mutex>. + +``* Context: Softirq or process context. Takes and releases <lock>, BH-safe.`` + +The function shall execute in process or Softirq context. +The function shall take and release <lock>. +The function shall safely execute in bottom half contexts. + +``* Context: Interrupt context.`` + +The function shall execute in interrupt context only. + +It is a good practice to further specify the context specifications as part of the Function’s expectation (e.g. at which stage a lock is held and released) + +Return values +~~~~~~~~~~~~~ +Return values must be written as a multiple line list in the following format:: + +* Return: +* * [value-1] - [condition-1] +* * [value-2] - [condition-2] +* * [...] +* * [value-n] - [condition-n] +* * Any value returned by func-1(), func-2(),...,func-n() + +In such a format ``[value-i]`` must be a clearly identified value or range of values that is compatible with the function prototype (e.g. for a read() file operation, it is ok to define [value-i] as ``the number of bytes successfully copied to the user space buffer``). + +``[condition-i]`` must be a condition that can be unambiguously traced back to the ``Function’s expectations`` or ``Context`` defined above; as part of [condition-i] it is possible to refer to dependencies of invoked functions or of internal SW or HW states. + +``Any value returned by func-1(), func-2(),...,func-n()`` defines a scenario where the current function is directly returning the value of an invoked function dependency. + +Semantic aspects of testable specifications +------------------------------------------- +From a semantic point of view it is important to document the intended or expected behavior (from a developer or integrator point of view respectively) in consideration of the different design aspects impacting it. + +Such behavior shall be described in a way that makes it possible to define test cases unambiguously. \ +To this extent it is important to document design elements impacting the expected behavior and the design elements characterizing the expected behavior (and sometimes these can physically overlap); such design elements shall be limited to the scope of the code being documented, that can range from a single function to multiple ones depending on the complexity of the overall code. + +**Possible elements impacting the expected behavior** of the code being documented are: + +* Input parameters: parameters passed to the API being documented; +* state variables: global and static data (variables or pointers); +* software dependencies: external SW APIs invoked by the code under analysis; +* Hardware dependencies: HW design elements directly impacting the behavior of the code in scope; +* Firmware dependencies: FW design elements that have an impact on the behavior of the API being documented (e.g. DTB or ACPI tables, or runtime services like SCMI and ACPI AML); +* Compile time configuration parameters: configuration parameters parsed when compiling the Kernel Image; +* Runtime configuration parameters (AKA calibration parameters): parameters that can be modified at runtime. + +**Design elements characterizing the expected behavior** of the API being documented that are in scope according to the above mentioned granularity: + +* API return values, including pointer addresses; +* Input pointers: pointers passed as input parameter to the API being documented; +* state variables: global and static data (variable or pointers); +* Hardware design elements (e.g. HW registers). + +**Testability considerations**: the impact of each of the documented “design elements impacting the expected behavior” must be described in terms of effect on the “design element characterizing the expected behavior” and, in doing so, it is important to document allowed or not allowed ranges of values, corner cases and error conditions; so that it is possible to define a meaningful test plan according to different equivalence classes. + +**Scalability and maintainability considerations**: the described expected behavior must be limited to the scope of the code under analysis so for example the Software, Firmware and Hardware dependencies shall be described in terms of possible impact on the invoking code deferring further details to the respective documentation of these. + +When deciding the scope of the code being documented, the scalability and maintainability goals must be considered; it does not make sense to embed the documentation of multiple complex functions within the kernel-doc header of the top level function as, doing so, would make it harder to review the code changes against the documented specifications and/or to extend the specifications to new functionalities being added. + +The end goal is to build a hierarchical, scalable, maintainable documentation. + +**Feasibility considerations**: Only the “meaningful” and “useful” expected behavior, and the design elements impacting it, shall be considered (e.g. a printk() logging some info may be omitted). There are two reasons behind this point: + +1. Specifying the expected behaviour of the code should be done, in principle, in a code agnostic way. So it is not about writing a pseudo-code redundant implementation, but rather about defining and documenting the developer intent and the integrator’s expectations. +2. When the expected behavior is defined before implementing the code, such an activity is done by experts using a level of detail that is more abstract than the code itself and they only refer to aspects that are relevant for the design expectations. diff --git a/Documentation/doc-guide/index.rst b/Documentation/doc-guide/index.rst index 24d058faa75c..09e459866442 100644 --- a/Documentation/doc-guide/index.rst +++ b/Documentation/doc-guide/index.rst @@ -9,6 +9,7 @@ How to write kernel documentation
sphinx kernel-doc + code-specifications parse-headers contributing maintainer-profile
Gabriele Paoloni gpaoloni@redhat.com writes:
[Taking a quick look...]
The Documentation/doc-guide/kernel-doc.rst chapter describes how to document the code using the kernel-doc format, however it does not specify the criteria to be followed for writing testable specifications; i.e. specifications that can be used to for the semantic description of low level requirements.
This patch adds a guideline that defines criteria to formally describe developers’ intent at the function and subfunction level in the form of testable expectations.
Signed-off-by: Gabriele Paoloni gpaoloni@redhat.com Signed-off-by: Chuck Wolber chuckwolber@gmail.com Signed-off-by: Kate Stewart kstewart@linuxfoundation.org
.../doc-guide/code-specifications.rst | 208 ++++++++++++++++++ Documentation/doc-guide/index.rst | 1 + 2 files changed, 209 insertions(+) create mode 100644 Documentation/doc-guide/code-specifications.rst
diff --git a/Documentation/doc-guide/code-specifications.rst b/Documentation/doc-guide/code-specifications.rst new file mode 100644 index 000000000000..dee1b4f089e1 --- /dev/null +++ b/Documentation/doc-guide/code-specifications.rst @@ -0,0 +1,208 @@ +.. title:: How-to write testable code specifications
+========================================= +How-to write testable code specifications +=========================================
+Introduction +------------ +The Documentation/doc-guide/kernel-doc.rst chapter describes how to document the code using the kernel-doc format, however it does not specify the criteria to be followed for writing testable specifications; i.e. specifications that can be used to for the semantic description of low level requirements.
Please, for any future versions, stick to the 80-column limit; this is especially important for text files that you want humans to read.
As a nit, you don't need to start by saying what other documents don't do, just describe the purpose of *this* document.
More substantially ... I got a way into this document before realizing that you were describing an addition to the format of kerneldoc comments. That would be good to make clear from the outset.
What I still don't really understand is what is the *purpose* of this formalized text? What will be consuming it? You're asking for a fair amount of effort to write and maintain these descriptions; what's in it for the people who do that work?
How does an author determine whether the specifications they have written are correct, both gramatically and semantically?
Thanks,
jon
Hi Jonathan
Many thanks for your review
On Tue, Sep 16, 2025 at 12:34 AM Jonathan Corbet corbet@lwn.net wrote:
Gabriele Paoloni gpaoloni@redhat.com writes:
[Taking a quick look...]
The Documentation/doc-guide/kernel-doc.rst chapter describes how to document the code using the kernel-doc format, however it does not specify the criteria to be followed for writing testable specifications; i.e. specifications that can be used to for the semantic description of low level requirements.
This patch adds a guideline that defines criteria to formally describe developers’ intent at the function and subfunction level in the form of testable expectations.
Signed-off-by: Gabriele Paoloni gpaoloni@redhat.com Signed-off-by: Chuck Wolber chuckwolber@gmail.com Signed-off-by: Kate Stewart kstewart@linuxfoundation.org
.../doc-guide/code-specifications.rst | 208 ++++++++++++++++++ Documentation/doc-guide/index.rst | 1 + 2 files changed, 209 insertions(+) create mode 100644 Documentation/doc-guide/code-specifications.rst
diff --git a/Documentation/doc-guide/code-specifications.rst b/Documentation/doc-guide/code-specifications.rst new file mode 100644 index 000000000000..dee1b4f089e1 --- /dev/null +++ b/Documentation/doc-guide/code-specifications.rst @@ -0,0 +1,208 @@ +.. title:: How-to write testable code specifications
+========================================= +How-to write testable code specifications +=========================================
+Introduction +------------ +The Documentation/doc-guide/kernel-doc.rst chapter describes how to document the code using the kernel-doc format, however it does not specify the criteria to be followed for writing testable specifications; i.e. specifications that can be used to for the semantic description of low level requirements.
Please, for any future versions, stick to the 80-column limit; this is especially important for text files that you want humans to read.
Thanks I will stick to 80 chars for future submissions
As a nit, you don't need to start by saying what other documents don't do, just describe the purpose of *this* document.
Yes, my goal was to explain the purpose of this doc, however, re-reading this intro I realize that it uses a negative tone, that is not good. I will rephrase explaining the it expands on top of kernel-doc.rst to further specify the expectations from the code and the assumptions to correctly use it.
More substantially ... I got a way into this document before realizing that you were describing an addition to the format of kerneldoc comments. That would be good to make clear from the outset.
What I still don't really understand is what is the *purpose* of this formalized text? What will be consuming it? You're asking for a fair amount of effort to write and maintain these descriptions; what's in it for the people who do that work?
The goal is to clearly define what the code is expected to do and how to correctly invoke it so that: 1) A user of the code does not wrongly invoke it 2) A developer or a maintainer can check if code changes are compliant with such expectations (maybe he did wrong changes or the expectations need to change) 3) A tester can verify if test cases are correct WRT such expectations and complete
How does an author determine whether the specifications they have written are correct, both gramatically and semantically?
For grammatical purpose, probably an automated check could be implemented, for semantic aspects the first level of verification comes from the community and maintainer review process, whereas the second level of verification come from selftests that should be written according to the expectations from the code and should trace to them (as in the example in patch 3)
Thanks Gab
Thanks,
jon
+------------ +The Documentation/doc-guide/kernel-doc.rst chapter describes how to document the code using the kernel-doc format, however it does not specify the criteria to be followed for writing testable specifications; i.e. specifications that can be used to for the semantic description of low level requirements.
Please, for any future versions, stick to the 80-column limit; this is especially important for text files that you want humans to read.
As a nit, you don't need to start by saying what other documents don't do, just describe the purpose of *this* document.
More substantially ... I got a way into this document before realizing that you were describing an addition to the format of kerneldoc comments. That would be good to make clear from the outset.
What I still don't really understand is what is the *purpose* of this formalized text? What will be consuming it? You're asking for a fair amount of effort to write and maintain these descriptions; what's in it for the people who do that work?
I might be wrong, but sounds to me like someone intends to feed this to AI to generate tests or code.
In that case, no thanks.
I'm pretty sure we don't want this.
[Reposting with apologies for the dup and those inflicted by the broken Gmail defaults. I have migrated away from Gmail, but some threads are still stuck there.]
On Mon, Oct 20, 2025 at 7:35 PM David Hildenbrand david@redhat.com wrote:
+------------ +The Documentation/doc-guide/kernel-doc.rst chapter describes how to document the code using the kernel-doc format, however it does not specify the criteria to be followed for writing testable specifications; i.e. specifications that can be used to for the semantic description of low level requirements.
Please, for any future versions, stick to the 80-column limit; this is especially important for text files that you want humans to read.
As a nit, you don't need to start by saying what other documents don't do, just describe the purpose of *this* document.
More substantially ... I got a way into this document before realizing that you were describing an addition to the format of kerneldoc comments. That would be good to make clear from the outset.
What I still don't really understand is what is the *purpose* of this formalized text? What will be consuming it? You're asking for a fair amount of effort to write and maintain these descriptions; what's in it for the people who do that work?
I might be wrong, but sounds to me like someone intends to feed this to AI to generate tests or code.
Absolutely not the intent. This is about the lossy process of converting human ideas to code. Reliably going from code to test requires an understanding of what was lost in translation. This project is about filling that gap.
In that case, no thanks.
I'm pretty sure we don't want this.
Nor I. If you find any references in our work that amount to a validation of your concerns, please bring them to our attention.
..Ch:W..
On 20.10.25 23:02, Chuck Wolber wrote:
[Reposting with apologies for the dup and those inflicted by the broken Gmail defaults. I have migrated away from Gmail, but some threads are still stuck there.]
On Mon, Oct 20, 2025 at 7:35 PM David Hildenbrand david@redhat.com wrote:
+------------ +The Documentation/doc-guide/kernel-doc.rst chapter describes how to document the code using the kernel-doc format, however it does not specify the criteria to be followed for writing testable specifications; i.e. specifications that can be used to for the semantic description of low level requirements.
Please, for any future versions, stick to the 80-column limit; this is especially important for text files that you want humans to read.
As a nit, you don't need to start by saying what other documents don't do, just describe the purpose of *this* document.
More substantially ... I got a way into this document before realizing that you were describing an addition to the format of kerneldoc comments. That would be good to make clear from the outset.
What I still don't really understand is what is the *purpose* of this formalized text? What will be consuming it? You're asking for a fair amount of effort to write and maintain these descriptions; what's in it for the people who do that work?
I might be wrong, but sounds to me like someone intends to feed this to AI to generate tests or code.
Absolutely not the intent. This is about the lossy process of converting human ideas to code. Reliably going from code to test requires an understanding of what was lost in translation. This project is about filling that gap.
Thanks for clarifying. I rang my alarm bells too early :)
I saw the LPC talk on this topic:
https://lpc.events/event/19/contributions/2085/
With things like "a test case can be derived from the testable expectation" one wonders how we get from the the doc to an actual test case.
IIRC, with things like formal verification we usually don't write in natural language, because it's too imprecise. But my formal verification knowledge is a bit rusty.
In that case, no thanks.
I'm pretty sure we don't want this.
Nor I. If you find any references in our work that amount to a validation of your concerns, please bring them to our attention.
I guess, as the discussion with me and Jonathan showed, the cover letter is a bit short on the motivation, making people like me speculate a bit too much about the intentions.
Hi David
On Tue, Oct 21, 2025 at 5:37 PM David Hildenbrand david@redhat.com wrote:
On 20.10.25 23:02, Chuck Wolber wrote:
[Reposting with apologies for the dup and those inflicted by the broken Gmail defaults. I have migrated away from Gmail, but some threads are still stuck there.]
On Mon, Oct 20, 2025 at 7:35 PM David Hildenbrand david@redhat.com wrote:
+------------ +The Documentation/doc-guide/kernel-doc.rst chapter describes how to document the code using the kernel-doc format, however it does not specify the criteria to be followed for writing testable specifications; i.e. specifications that can be used to for the semantic description of low level requirements.
Please, for any future versions, stick to the 80-column limit; this is especially important for text files that you want humans to read.
As a nit, you don't need to start by saying what other documents don't do, just describe the purpose of *this* document.
More substantially ... I got a way into this document before realizing that you were describing an addition to the format of kerneldoc comments. That would be good to make clear from the outset.
What I still don't really understand is what is the *purpose* of this formalized text? What will be consuming it? You're asking for a fair amount of effort to write and maintain these descriptions; what's in it for the people who do that work?
I might be wrong, but sounds to me like someone intends to feed this to AI to generate tests or code.
Absolutely not the intent. This is about the lossy process of converting human ideas to code. Reliably going from code to test requires an understanding of what was lost in translation. This project is about filling that gap.
Thanks for clarifying. I rang my alarm bells too early :)
I saw the LPC talk on this topic:
https://lpc.events/event/19/contributions/2085/
With things like "a test case can be derived from the testable expectation" one wonders how we get from the the doc to an actual test case.
Probably it is the term derived that can be a bit misleading. The point is that we need documented expectations that can be used to review and verify the test cases against; so maybe better to say "a test case can be verified against the testable expectation"
IIRC, with things like formal verification we usually don't write in natural language, because it's too imprecise. But my formal verification knowledge is a bit rusty.
In that case, no thanks.
I'm pretty sure we don't want this.
Nor I. If you find any references in our work that amount to a validation of your concerns, please bring them to our attention.
I guess, as the discussion with me and Jonathan showed, the cover letter is a bit short on the motivation, making people like me speculate a bit too much about the intentions.
Right, I'll keep this in mind for v2 and I will improve the motivation aspect (also leveraging the response I gave to Jonathan).
Many thanks for your feedbacks! Gab
-- Cheers
David / dhildenb
On 21.10.25 18:27, Gabriele Paoloni wrote:
Hi David
On Tue, Oct 21, 2025 at 5:37 PM David Hildenbrand david@redhat.com wrote:
On 20.10.25 23:02, Chuck Wolber wrote:
[Reposting with apologies for the dup and those inflicted by the broken Gmail defaults. I have migrated away from Gmail, but some threads are still stuck there.]
On Mon, Oct 20, 2025 at 7:35 PM David Hildenbrand david@redhat.com wrote:
+------------ +The Documentation/doc-guide/kernel-doc.rst chapter describes how to document the code using the kernel-doc format, however it does not specify the criteria to be followed for writing testable specifications; i.e. specifications that can be used to for the semantic description of low level requirements.
Please, for any future versions, stick to the 80-column limit; this is especially important for text files that you want humans to read.
As a nit, you don't need to start by saying what other documents don't do, just describe the purpose of *this* document.
More substantially ... I got a way into this document before realizing that you were describing an addition to the format of kerneldoc comments. That would be good to make clear from the outset.
What I still don't really understand is what is the *purpose* of this formalized text? What will be consuming it? You're asking for a fair amount of effort to write and maintain these descriptions; what's in it for the people who do that work?
I might be wrong, but sounds to me like someone intends to feed this to AI to generate tests or code.
Absolutely not the intent. This is about the lossy process of converting human ideas to code. Reliably going from code to test requires an understanding of what was lost in translation. This project is about filling that gap.
Thanks for clarifying. I rang my alarm bells too early :)
I saw the LPC talk on this topic:
https://lpc.events/event/19/contributions/2085/
With things like "a test case can be derived from the testable expectation" one wonders how we get from the the doc to an actual test case.
Probably it is the term derived that can be a bit misleading. The point is that we need documented expectations that can be used to review and verify the test cases against; so maybe better to say "a test case can be verified against the testable expectation"
On a high level (where we usually test with things like LTP) I would usually expect that the man pages properly describe the semantics of syscalls etc.
That also feels like a better place to maintain such kind of information.
Having that said, man-pages are frequently a bit outdated or imprecise .. or missing.
Anyhow, I guess that will all be discussed in your LPC session I assume, I'll try to attend that one, thanks!
On Tue, Oct 21, 2025 at 6:34 PM David Hildenbrand david@redhat.com wrote:
On 21.10.25 18:27, Gabriele Paoloni wrote:
Hi David
On Tue, Oct 21, 2025 at 5:37 PM David Hildenbrand david@redhat.com wrote:
On 20.10.25 23:02, Chuck Wolber wrote:
[Reposting with apologies for the dup and those inflicted by the broken Gmail defaults. I have migrated away from Gmail, but some threads are still stuck there.]
On Mon, Oct 20, 2025 at 7:35 PM David Hildenbrand david@redhat.com wrote:
> +------------ > +The Documentation/doc-guide/kernel-doc.rst chapter describes how to document the code using the kernel-doc format, however it does not specify the criteria to be followed for writing testable specifications; i.e. specifications that can be used to for the semantic description of low level requirements.
Please, for any future versions, stick to the 80-column limit; this is especially important for text files that you want humans to read.
As a nit, you don't need to start by saying what other documents don't do, just describe the purpose of *this* document.
More substantially ... I got a way into this document before realizing that you were describing an addition to the format of kerneldoc comments. That would be good to make clear from the outset.
What I still don't really understand is what is the *purpose* of this formalized text? What will be consuming it? You're asking for a fair amount of effort to write and maintain these descriptions; what's in it for the people who do that work?
I might be wrong, but sounds to me like someone intends to feed this to AI to generate tests or code.
Absolutely not the intent. This is about the lossy process of converting human ideas to code. Reliably going from code to test requires an understanding of what was lost in translation. This project is about filling that gap.
Thanks for clarifying. I rang my alarm bells too early :)
I saw the LPC talk on this topic:
https://lpc.events/event/19/contributions/2085/
With things like "a test case can be derived from the testable expectation" one wonders how we get from the the doc to an actual test case.
Probably it is the term derived that can be a bit misleading. The point is that we need documented expectations that can be used to review and verify the test cases against; so maybe better to say "a test case can be verified against the testable expectation"
On a high level (where we usually test with things like LTP) I would usually expect that the man pages properly describe the semantics of syscalls etc.
On a high level yes however there are two issues: 1) even the Posix standard define the behaviour of certain syscalls as implementation specific 2) if all the details required to write testable specifications were maintained as part of the manpage, these would become unmaintainable
For this reason specification must be broken down over the code in a maintainable way
That also feels like a better place to maintain such kind of information.
Having that said, man-pages are frequently a bit outdated or imprecise .. or missing.
Anyhow, I guess that will all be discussed in your LPC session I assume, I'll try to attend that one, thanks!
Sure Looking FWD to see you there
Gab
-- Cheers
David / dhildenb
This patch proposes initial kernel-doc documentation for memory_open() and most of the functions in the mem_fops structure. The format used for the specifications follows the guidelines defined in Documentation/doc-guide/code-specifications.rst
Signed-off-by: Gabriele Paoloni gpaoloni@redhat.com --- drivers/char/mem.c | 231 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 225 insertions(+), 6 deletions(-)
diff --git a/drivers/char/mem.c b/drivers/char/mem.c index 48839958b0b1..e69c164e9465 100644 --- a/drivers/char/mem.c +++ b/drivers/char/mem.c @@ -75,9 +75,54 @@ static inline bool should_stop_iteration(void) return signal_pending(current); }
-/* - * This funcion reads the *physical* memory. The f_pos points directly to the - * memory location. +/** + * read_mem - read from physical memory (/dev/mem). + * @file: struct file associated with /dev/mem. + * @buf: user-space buffer to copy data to. + * @count: number of bytes to read. + * @ppos: pointer to the current file position, representing the physical + * address to read from. + * + * This function checks if the requested physical memory range is valid + * and accessible by the user, then it copies data to the input + * user-space buffer up to the requested number of bytes. + * + * Function's expectations: + * + * 1. This function shall check if the value pointed by ppos exceeds the + * maximum addressable physical address; + * + * 2. This function shall check if the physical address range to be read + * is valid (i.e. it falls within a memory block and if it can be mapped + * to the kernel address space); + * + * 3. For each memory page falling in the requested physical range + * [ppos, ppos + count - 1]: + * 3.1. this function shall check if user space access is allowed (if + * config STRICT_DEVMEM is not set, access is always granted); + * + * 3.2. if access is allowed, the memory content from the page range falling + * within the requested physical range shall be copied to the user space + * buffer; + * + * 3.3. zeros shall be copied to the user space buffer (for the page range + * falling within the requested physical range): + * 3.3.1. if access to the memory page is restricted or, + * 3.2.2. if the current page is page 0 on HW architectures where page 0 is + * not mapped. + * + * 4. The file position '*ppos' shall be advanced by the number of bytes + * successfully copied to user space (including zeros). + * + * Context: process context. + * + * Return: + * * the number of bytes copied to user on success + * * %-EFAULT - the requested address range is not valid or a fault happened + * when copying to user-space (i.e. copy_from_kernel_nofault() failed) + * * %-EPERM - access to any of the required physical pages is not allowed + * * %-ENOMEM - out of memory error for auxiliary kernel buffers supporting + * the operation of copying content from the physical pages */ static ssize_t read_mem(struct file *file, char __user *buf, size_t count, loff_t *ppos) @@ -166,6 +211,54 @@ static ssize_t read_mem(struct file *file, char __user *buf, return err; }
+/** + * write_mem - write to physical memory (/dev/mem). + * @file: struct file associated with /dev/mem. + * @buf: user-space buffer containing the data to write. + * @count: number of bytes to write. + * @ppos: pointer to the current file position, representing the physical + * address to write to. + * + * This function checks if the target physical memory range is valid + * and accessible by the user, then it writes data from the input + * user-space buffer up to the requested number of bytes. + * + * Function's expectations: + * 1. This function shall check if the value pointed by ppos exceeds the + * maximum addressable physical address; + * + * 2. This function shall check if the physical address range to be written + * is valid (i.e. it falls within a memory block and if it can be mapped + * to the kernel address space); + * + * 3. For each memory page falling in the physical range to be written + * [ppos, ppos + count - 1]: + * 3.1. this function shall check if user space access is allowed (if + * config STRICT_DEVMEM is not set, access is always granted); + * + * 3.2. the content from the user space buffer shall be copied to the page + * range falling within the physical range to be written if access is + * allowed; + * + * 3.3. the data to be copied from the user space buffer (for the page range + * falling within the range to be written) shall be skipped: + * 3.3.1. if access to the memory page is restricted or, + * 3.3.2. if the current page is page 0 on HW architectures where page 0 + * is not mapped. + * + * 4. The file position '*ppos' shall be advanced by the number of bytes + * successfully copied from user space (including skipped bytes). + * + * Context: process context. + * + * Return: + * * the number of bytes copied from user-space on success + * * %-EFBIG - the value pointed by ppos exceeds the maximum addressable + * physical address + * * %-EFAULT - the physical address range is not valid or no bytes could + * be copied from user-space + * * %-EPERM - access to any of the required pages is not allowed + */ static ssize_t write_mem(struct file *file, const char __user *buf, size_t count, loff_t *ppos) { @@ -322,6 +415,42 @@ static const struct vm_operations_struct mmap_mem_ops = { #endif };
+/** + * mmap_mem - map physical memory into user space (/dev/mem). + * @file: file structure for the device. + * @vma: virtual memory area structure describing the user mapping. + * + * This function checks if the requested physical memory range is valid + * and accessible by the user, then it maps the physical memory range to + * user-mode address space. + * + * Function's expectations: + * 1. This function shall check if the requested physical address range to be + * mapped fits within the maximum addressable physical range; + * + * 2. This function shall check if the requested physical range corresponds to + * a valid physical range and if access is allowed on it (if config STRICT_DEVMEM + * is not set, access is always allowed); + * + * 3. This function shall check if the input virtual memory area can be used for + * a private mapping (always OK if there is an MMU); + * + * 4. This function shall set the virtual memory area operations to + * &mmap_mem_ops; + * + * 5. This function shall establish a mapping between the user-space + * virtual memory area described by vma and the physical memory + * range specified by vma->vm_pgoff and size; + * + * Context: process context. + * + * Return: + * * 0 on success + * * %-EAGAIN - invalid or unsupported mapping requested (remap_pfn_range() + * fails) + * * %-EINVAL - requested physical range to be mapped is not valid + * * %-EPERM - no permission to access the requested physical range + */ static int mmap_mem(struct file *file, struct vm_area_struct *vma) { size_t size = vma->vm_end - vma->vm_start; @@ -550,13 +679,47 @@ static loff_t null_lseek(struct file *file, loff_t offset, int orig) return file->f_pos = 0; }
-/* +/** + * memory_lseek - change the file position. + * @file: file structure for the device. + * @offset: file offset to seek to. + * @orig: where to start seeking from (see whence in the llseek manpage). + * + * This function changes the file position according to the input offset + * and orig parameters. + * + * Function's expectations: + * 1. This function shall lock the semaphore of the inode corresponding to the + * input file before any operation and unlock it before returning. + * + * 2. This function shall check the orig value and accordingly: + * 2.1. if it is equal to SEEK_CUR, the current file position shall be + * incremented by the input offset; + * 2.2. if it is equal to SEEK_SET, the current file position shall be + * set to the input offset value; + * 2.3. any other value shall result in an error condition. + * + * 3. Before writing the current file position, the new position value + * shall be checked to not overlap with Linux ERRNO values. + * + * Assumptions of Use: + * 1. the input file pointer is expected to be valid. + * + * Notes: * The memory devices use the full 32/64 bits of the offset, and so we cannot * check against negative addresses: they are ok. The return value is weird, * though, in that case (0). * - * also note that seeking relative to the "end of file" isn't supported: - * it has no meaning, so it returns -EINVAL. + * Also note that seeking relative to the "end of file" isn't supported: + * it has no meaning, so passing orig equal to SEEK_END returns -EINVAL. + * + * Context: process context, locks/unlocks inode->i_rwsem + * + * Return: + * * the new file position on success + * * %-EOVERFLOW - the new position value equals or exceeds + * (unsigned long long) -MAX_ERRNO + * * %-EINVAL - the orig parameter is invalid */ static loff_t memory_lseek(struct file *file, loff_t offset, int orig) { @@ -584,6 +747,35 @@ static loff_t memory_lseek(struct file *file, loff_t offset, int orig) return ret; }
+/** + * open_port - open the I/O port device (/dev/port). + * @inode: inode of the device file. + * @filp: file structure for the device. + * + * This function checks if the caller can access the port device and sets + * the f_mapping pointer of filp to the i_mapping pointer of inode. + * + * Function's expectations: + * 1. This function shall check if the caller has sufficient capabilities to + * perform raw I/O access; + * + * 2. This function shall check if the kernel is locked down with the + * &LOCKDOWN_DEV_MEM restriction; + * + * 3. If the input inode corresponds to /dev/mem, the f_mapping pointer + * of the input file structure shall be set to the i_mapping pointer + * of the input inode; + * + * Assumptions of Use: + * 1. The input inode and filp are expected to be valid. + * + * Context: process context. + * + * Return: + * * 0 on success + * * %-EPERM - caller lacks the required capability (CAP_SYS_RAWIO) + * * any error returned by securty_locked_down() + */ static int open_port(struct inode *inode, struct file *filp) { int rc; @@ -691,6 +883,33 @@ static const struct memdev { #endif };
+/** + * memory_open - set the filp f_op to the memory device fops and invoke open(). + * @inode: inode of the device file. + * @filp: file structure for the device. + * + * Function's expectations: + * 1. This function shall retrieve the minor number associated with the input + * inode and the memory device corresponding to such minor number; + * + * 2. The file operations pointer shall be set to the memory device file operations; + * + * 3. The file mode member of the input filp shall be OR'd with the device mode; + * + * 4. The memory device open() file operation shall be invoked. + * + * Assumptions of Use: + * 1. The input inode and filp are expected to be non-NULL. + * + * Context: process context. + * + * Return: + * * 0 on success + * * %-ENXIO - the minor number corresponding to the input inode cannot be + * associated with any device or the corresponding device has a NULL fops + * pointer + * * any error returned by the device specific open function pointer + */ static int memory_open(struct inode *inode, struct file *filp) { int minor;
Gabriele Paoloni gpaoloni@redhat.com writes:
This patch proposes initial kernel-doc documentation for memory_open() and most of the functions in the mem_fops structure. The format used for the specifications follows the guidelines defined in Documentation/doc-guide/code-specifications.rst
I'll repeat my obnoxious question from the first patch: what does that buy for us?
Signed-off-by: Gabriele Paoloni gpaoloni@redhat.com
drivers/char/mem.c | 231 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 225 insertions(+), 6 deletions(-)
diff --git a/drivers/char/mem.c b/drivers/char/mem.c index 48839958b0b1..e69c164e9465 100644 --- a/drivers/char/mem.c +++ b/drivers/char/mem.c @@ -75,9 +75,54 @@ static inline bool should_stop_iteration(void) return signal_pending(current); } -/*
- This funcion reads the *physical* memory. The f_pos points directly to the
- memory location.
+/**
- read_mem - read from physical memory (/dev/mem).
- @file: struct file associated with /dev/mem.
- @buf: user-space buffer to copy data to.
- @count: number of bytes to read.
- @ppos: pointer to the current file position, representing the physical
address to read from.
- This function checks if the requested physical memory range is valid
- and accessible by the user, then it copies data to the input
- user-space buffer up to the requested number of bytes.
- Function's expectations:
- This function shall check if the value pointed by ppos exceeds the
- maximum addressable physical address;
- This function shall check if the physical address range to be read
- is valid (i.e. it falls within a memory block and if it can be mapped
- to the kernel address space);
- For each memory page falling in the requested physical range
- [ppos, ppos + count - 1]:
- 3.1. this function shall check if user space access is allowed (if
config STRICT_DEVMEM is not set, access is always granted);
- 3.2. if access is allowed, the memory content from the page range falling
within the requested physical range shall be copied to the user space
buffer;
- 3.3. zeros shall be copied to the user space buffer (for the page range
falling within the requested physical range):
3.3.1. if access to the memory page is restricted or,
3.2.2. if the current page is page 0 on HW architectures where page 0 is
not mapped.
- The file position '*ppos' shall be advanced by the number of bytes
- successfully copied to user space (including zeros).
My kneejerk first reaction is: you are repeating the code of the function in a different language. If we are not convinced that the code is correct, how can we be more confident that this set of specifications is correct? And again, what will consume this text? How does going through this effort get us to a better kernel?
Despite having been to a couple of your talks, I'm not fully understanding how this comes together; people who haven't been to the talks are not going to have an easier time getting the full picture.
Thanks,
jon
On Mon Sep 15, 2025 at 10:39 PM UTC, Jonathan Corbet wrote:
Gabriele Paoloni gpaoloni@redhat.com writes:
This patch proposes initial kernel-doc documentation for memory_open() and most of the functions in the mem_fops structure. The format used for the specifications follows the guidelines defined in Documentation/doc-guide/code-specifications.rst
I'll repeat my obnoxious question from the first patch: what does that buy for us?
Fair question, and definitely not obnoxious.
It might help to reframe this a bit. The idea is to take an engineering technique from one domain and apply it with modifications to another. The relevant terms of art are "forward engineering" and "reverse engineering".
My kneejerk first reaction is: you are repeating the code of the function in a different language.
No disagreement on that perception. We have more work to do when it comes to communicating the idea, as well as developing a better implementation.
The design of the Linux kernel is emergent and, in the present state, all forms of testing are an (educated) guess at the intended design. We can demonstrate this by picking a random bit of code from the kernel and assigning ourselves the task of writing a test for it.
Are you certain that your test accurately reflects the true design intent? You can read the code and test what you see. But that does not mean that your test is valid against the intent in someone else's head.
Music instructors see this whenever their students play the right notes but clearly do not yet "feel" the music. The difference is noticeable even by casual listeners.
If we are not convinced that the code is correct, how can we be more confident that this set of specifications is correct?
We have no reason to be independently convinced of either. When we describe this as importing a technique into a new domain, your question is an example of some of the concessions that have to be made.
The Linux kernel is not a forward engineered system. Therefore it is not possible to develop code and test from the same seed. Our only option is to reverse engineer that seed to the best of our abilities.
At that point we have a few options.
Ideally, the original developer can weigh in and validate that our interpretation is correct. This has the effect of "simulating" a forward engineering scenario, because a test can be created from the validated seed (I am trying valiantly to avoid using the word kernel).
Absent the original developer's validation, we have the option of simply asserting the specification. This is equivalent to the way testing is done today, except a test can be equally opaque with respect to what design it is attempting to validate.
In either case, if a test is developed against the specification, even an initially incorrect specification, we have the ability to bring code, specification, and test into alignment over time.
And again, what will consume this text?
Humans are the consumer. But to be clear - a machine readable template is going to be required in the long run to ensure that code and specification remain aligned. Our intentent was to avoid confusing things with templates, and introduce them once we have made headway on the points you have brought up.
It is probably also worth mentioning, we have already had an "a-ha" moment from one kernel maintainer. I believe the words were something to the effect of, "this is great, I used to have to relearn that code every time I touch it".
How does going through this effort get us to a better kernel?
I am hoping some of the above planted the seed to answer this one. Code must be correct in two ways, it must be valid and it must be verified.
Valid means - the code is doing the right thing. Verified means - the code is doing the thing right.
If code and test accurately reflect the same idea, then we can alleviate maintainers of a large portion of the verification burden. Validation is in the "hearts and minds" of the users, so that burden never goes away.
Despite having been to a couple of your talks, I'm not fully understanding how this comes together; people who haven't been to the talks are not going to have an easier time getting the full picture.
I agree. And thank you very much for attending those talks and engaging with us. It truly means a lot.
I have submitted a refereed talk to this year's Pumbers conference that is intended to go over these points in detail. My colleague (not on this thread) has also submitted a refereed talk on best practices for developing these specifications. His name is Matthew Whitehead and he is a recognized expert in that area.
..Ch:W..
Hi Jonathan
On Tue, Sep 16, 2025 at 12:39 AM Jonathan Corbet corbet@lwn.net wrote:
Gabriele Paoloni gpaoloni@redhat.com writes:
This patch proposes initial kernel-doc documentation for memory_open() and most of the functions in the mem_fops structure. The format used for the specifications follows the guidelines defined in Documentation/doc-guide/code-specifications.rst
I'll repeat my obnoxious question from the first patch: what does that buy for us?
I tried to explain my reply on patch 1
Signed-off-by: Gabriele Paoloni gpaoloni@redhat.com
drivers/char/mem.c | 231 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 225 insertions(+), 6 deletions(-)
diff --git a/drivers/char/mem.c b/drivers/char/mem.c index 48839958b0b1..e69c164e9465 100644 --- a/drivers/char/mem.c +++ b/drivers/char/mem.c @@ -75,9 +75,54 @@ static inline bool should_stop_iteration(void) return signal_pending(current); }
-/*
- This funcion reads the *physical* memory. The f_pos points directly to the
- memory location.
+/**
- read_mem - read from physical memory (/dev/mem).
- @file: struct file associated with /dev/mem.
- @buf: user-space buffer to copy data to.
- @count: number of bytes to read.
- @ppos: pointer to the current file position, representing the physical
address to read from.
- This function checks if the requested physical memory range is valid
- and accessible by the user, then it copies data to the input
- user-space buffer up to the requested number of bytes.
- Function's expectations:
- This function shall check if the value pointed by ppos exceeds the
- maximum addressable physical address;
- This function shall check if the physical address range to be read
- is valid (i.e. it falls within a memory block and if it can be mapped
- to the kernel address space);
- For each memory page falling in the requested physical range
- [ppos, ppos + count - 1]:
- 3.1. this function shall check if user space access is allowed (if
config STRICT_DEVMEM is not set, access is always granted);
- 3.2. if access is allowed, the memory content from the page range falling
within the requested physical range shall be copied to the user space
buffer;
- 3.3. zeros shall be copied to the user space buffer (for the page range
falling within the requested physical range):
3.3.1. if access to the memory page is restricted or,
3.2.2. if the current page is page 0 on HW architectures where page 0 is
not mapped.
- The file position '*ppos' shall be advanced by the number of bytes
- successfully copied to user space (including zeros).
My kneejerk first reaction is: you are repeating the code of the function in a different language. If we are not convinced that the code is correct, how can we be more confident that this set of specifications is correct? And again, what will consume this text? How does going through this effort get us to a better kernel?
In summary specifications provide the criteria to be used in verifying the code (both when reviewing and testing). Otherwise: 1) Developers and reviewers have no criteria to evaluate the code, other than their expertise and judgement when they read it; 2) Testers would write test cases based on the code itself (so it is more likely that a wrong code is not detected due to wrong test cases).
WRT your first point, if specifications are wrong, a reviewer or a test would detect a gap between code and associated specs, hence leading to process of scrutiny of both code and specs where such a gap must be resolved. This is the reason why the duality of specification and tests VS the code being verified lead to confidence in code becoming more dependable (also from a user point of view that can now clearly see the assumptions to be met when invoking the code)
Thanks Gab
Despite having been to a couple of your talks, I'm not fully understanding how this comes together; people who haven't been to the talks are not going to have an easier time getting the full picture.
Thanks,
jon
From: Alessandro Carminati acarmina@redhat.com
This patch introduces a new series of tests for devmem. Test cases are mapped against the tested Function's expectations defined in /drivers/char/mem.c.
Signed-off-by: Alessandro Carminati acarmina@redhat.com --- tools/testing/selftests/Makefile | 1 + tools/testing/selftests/devmem/Makefile | 13 + tools/testing/selftests/devmem/debug.c | 25 + tools/testing/selftests/devmem/debug.h | 14 + tools/testing/selftests/devmem/devmem.c | 200 ++++++++ tools/testing/selftests/devmem/ram_map.c | 250 ++++++++++ tools/testing/selftests/devmem/ram_map.h | 38 ++ tools/testing/selftests/devmem/secret.c | 46 ++ tools/testing/selftests/devmem/secret.h | 13 + tools/testing/selftests/devmem/tests.c | 569 +++++++++++++++++++++++ tools/testing/selftests/devmem/tests.h | 45 ++ tools/testing/selftests/devmem/utils.c | 379 +++++++++++++++ tools/testing/selftests/devmem/utils.h | 119 +++++ 13 files changed, 1712 insertions(+) create mode 100644 tools/testing/selftests/devmem/Makefile create mode 100644 tools/testing/selftests/devmem/debug.c create mode 100644 tools/testing/selftests/devmem/debug.h create mode 100644 tools/testing/selftests/devmem/devmem.c create mode 100644 tools/testing/selftests/devmem/ram_map.c create mode 100644 tools/testing/selftests/devmem/ram_map.h create mode 100644 tools/testing/selftests/devmem/secret.c create mode 100644 tools/testing/selftests/devmem/secret.h create mode 100644 tools/testing/selftests/devmem/tests.c create mode 100644 tools/testing/selftests/devmem/tests.h create mode 100644 tools/testing/selftests/devmem/utils.c create mode 100644 tools/testing/selftests/devmem/utils.h
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index 030da61dbff3..55d228572e37 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -16,6 +16,7 @@ TARGETS += cpu-hotplug TARGETS += damon TARGETS += devices/error_logs TARGETS += devices/probe +TARGETS += devmem/devmem TARGETS += dmabuf-heaps TARGETS += drivers/dma-buf TARGETS += drivers/ntsync diff --git a/tools/testing/selftests/devmem/Makefile b/tools/testing/selftests/devmem/Makefile new file mode 100644 index 000000000000..8aca8cbe2957 --- /dev/null +++ b/tools/testing/selftests/devmem/Makefile @@ -0,0 +1,13 @@ +# SPDX-License-Identifier: GPL-2.0 +# Kselftest Makefile for devmem test + +CFLAGS += -Wall -O2 + +TEST_GEN_PROGS_EXTENDED := devmem + +$(OUTPUT)/devmem: devmem.c $(OUTPUT)/ram_map.o $(OUTPUT)/secret.o $(OUTPUT)/tests.o $(OUTPUT)/utils.o $(OUTPUT)/debug.o + $(CC) $^ -o $@ $(CFLAGS) + +EXTRA_CLEAN += $(OUTPUT)/ram_map.o $(OUTPUT)/secret.o $(OUTPUT)/tests.o $(OUTPUT)/utils.o $(OUTPUT)/debug.o + +include ../lib.mk diff --git a/tools/testing/selftests/devmem/debug.c b/tools/testing/selftests/devmem/debug.c new file mode 100644 index 000000000000..db88a760414d --- /dev/null +++ b/tools/testing/selftests/devmem/debug.c @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * devmem test debug.c + * + * Copyright (C) 2025 Red Hat, Inc. All Rights Reserved. + * Written by Alessandro Carminati (acarmina@redhat.com) + */ + +#include <stdio.h> +#include <stdarg.h> + +#define DEBUG_FLAG 0 +int pdebug = DEBUG_FLAG; + +void deb_printf(const char *fmt, ...) +{ + va_list args; + + if (pdebug) { + va_start(args, fmt); + vprintf(fmt, args); + va_end(args); + } +} + diff --git a/tools/testing/selftests/devmem/debug.h b/tools/testing/selftests/devmem/debug.h new file mode 100644 index 000000000000..323f44b94aaa --- /dev/null +++ b/tools/testing/selftests/devmem/debug.h @@ -0,0 +1,14 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * devmem test debug.h + * + * Copyright (C) 2025 Red Hat, Inc. All Rights Reserved. + * Written by Alessandro Carminati (acarmina@redhat.com) + */ + +#ifndef DEBUG_H +#define DEBUG_H +extern int pdebug; +void deb_printf(const char *fmt, ...); +#endif + diff --git a/tools/testing/selftests/devmem/devmem.c b/tools/testing/selftests/devmem/devmem.c new file mode 100644 index 000000000000..1b3fa5a46f67 --- /dev/null +++ b/tools/testing/selftests/devmem/devmem.c @@ -0,0 +1,200 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* devmem test devmem.c + * + * Copyright (C) 2025 Red Hat, Inc. All Rights Reserved. + * Written by Alessandro Carminati (acarmina@redhat.com) + */ + +#define _GNU_SOURCE + +#include <stdbool.h> +#include <stddef.h> +#include <stdint.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> + +#include "utils.h" +#include "secret.h" +#include "debug.h" +#include "ram_map.h" +#include "tests.h" +#include "debug.h" +#include "../kselftest.h" + +struct char_mem_test test_set[] = { +{ + "test_devmem_access", + &test_devmem_access, + "Test whether /dev/mem is accessible - memory_open FE_1, FE_2, FE_4", + F_ARCH_ALL|F_BITS_ALL|F_MISC_FATAL|F_MISC_INIT_PRV +}, +{ "test_open_devnum", + &test_open_devnum, + "Test open /dev/mem provides the correct min, maj - memory_open - FE_3", + F_ARCH_ALL|F_BITS_ALL|F_MISC_INIT_REQ}, +{ + "test_strict_devmem", + &test_strict_devmem, + "Test Strict Devmem enabled - Dependency", + F_ARCH_ALL|F_BITS_ALL|F_MISC_STRICT_DEVMEM_PRV|F_MISC_DONT_CARE +}, +{ + "test_read_at_addr_32bit_ge", + &test_read_at_addr_32bit_ge, + "Test read 64bit ppos vs 32 bit addr - read_mem - FE_1", + F_ARCH_ALL|F_BITS_B32|F_MISC_INIT_REQ +}, +{ + "test_read_outside_linear_map", + &test_read_outside_linear_map, + "Test read outside linear map - read_mem - FE_2", + F_ARCH_ALL|F_BITS_B32|F_MISC_INIT_REQ +}, +{ + "test_read_secret_area", + &test_read_secret_area, + "Test read memfd_secret area can not being accessed - read_mem - FE_4", + F_ARCH_ALL|F_BITS_ALL|F_MISC_INIT_REQ +}, +{ + "test_read_allowed_area", + &test_read_allowed_area, + "test read allowed area - read_mem - FE_5", + F_ARCH_ALL|F_BITS_ALL|F_MISC_INIT_REQ +}, +{ + "test_read_allowed_area_ppos_advance", + &test_read_allowed_area_ppos_advance, + "test read allowed area increments ppos - read_mem - FE_3", + F_ARCH_ALL|F_BITS_ALL|F_MISC_INIT_REQ +}, +{ + "test_read_restricted_area", + &test_read_restricted_area, + "test read restricted returns zeros - read_mem - FE_6", + F_ARCH_X86|F_BITS_ALL|F_MISC_INIT_REQ|F_MISC_STRICT_DEVMEM_REQ +}, +{ + "test_write_outside_area", + &test_write_outside_area, + "test write outside - write_mem - FE_2", + F_ARCH_ALL|F_BITS_ALL|F_MISC_INIT_REQ|F_MISC_WARN_ON_FAILURE +}, +{ + "test_seek_seek_set", + &test_seek_seek_set, + "test seek funcction SEEK_SET - memory_lseek - FE_4", + F_ARCH_ALL|F_BITS_ALL|F_MISC_INIT_REQ +}, +{ + "test_seek_seek_cur", + &test_seek_seek_cur, + "test seek function SEEK_CUR - memory_lseek - FE_3", + F_ARCH_ALL|F_BITS_ALL|F_MISC_INIT_REQ +}, +{ + "test_seek_seek_other", + &test_seek_seek_other, + "test seek function SEEK_END other - memory_lseek - FE_5", + F_ARCH_ALL|F_BITS_ALL|F_MISC_INIT_REQ +}, +}; + +int main(int argc, char *argv[]) +{ + int tests_skipped = 0; + int tests_failed = 0; + int tests_passed = 0; + int i, tmp_res; + struct test_context t; + char *str_res, *str_warn; + struct char_mem_test *current; + + t.srcbuf = malloc_pb(BOUNCE_BUF_SIZE); + t.dstbuf = malloc_pb(BOUNCE_BUF_SIZE); + if (!t.srcbuf || !t.dstbuf) { + printf("can't allocate buffers!\n"); + exit(-1); + } + // seet verbose flag from cmdline + t.verbose = false; + if ((argc >= 2) && (!strcmp(argv[1], "-v"))) { + t.verbose = true; + pdebug = 1; + } + + t.map = parse_iomem(); + if (!t.map) + goto exit; + + if (t.verbose) { + report_physical_memory(t.map); + dump_ram_map(t.map); + } + + for (i = 0; i < ARRAY_SIZE(test_set); i++) { + str_warn = NO_WARN_STR; + current = test_set + i; + tmp_res = test_needed(&t, current); + switch (tmp_res) { + case TEST_INCOHERENT: + deb_printf("Incoherent sequence Detected\n"); + exit(-1); + break; + case TEST_ALLOWED: + deb_printf("allowed sequence Detected\n"); + str_res = ""; + printf("%s - (%s) ", current->name, current->descr); + tmp_res = current->fn(&t); + switch (tmp_res) { + case FAIL: + str_res = DC_STR; + if (!(current->flags & F_MISC_DONT_CARE)) { + str_res = KO_STR; + tests_failed++; + } + break; + case SKIPPED: + tests_skipped++; + str_res = SKP_STR; + if (current->flags & F_MISC_WARN_ON_FAILURE) + str_warn = WARN_STR; + break; + case PASS: + str_res = DC_STR; + if (!(current->flags & F_MISC_DONT_CARE)) { + tests_passed++; + str_res = OK_STR; + } + if (current->flags & F_MISC_WARN_ON_SUCCESS) + str_warn = WARN_STR; + break; + default: + tests_failed++; + printf("corrupted data\n"); + exit(-1); + } + ksft_print_msg("%s %s\n", str_res, str_warn); + if ((tmp_res == FAIL) && + (current->flags & F_MISC_FATAL)) { + printf("fatal test failed end the chain\n"); + goto cleanup; + } + case TEST_DENIED: + deb_printf("denied sequence Detected\n"); + } + } + +cleanup: + close(t.fd); + free_ram_map(t.map); + free_pb(t.srcbuf); + free_pb(t.dstbuf); +exit: + printf("Run tests = %d (passed=%d, skipped=%d failed=%d)\n", + tests_skipped+tests_failed+tests_passed, tests_passed, + tests_skipped, tests_failed); + return tests_skipped+tests_failed; +} diff --git a/tools/testing/selftests/devmem/ram_map.c b/tools/testing/selftests/devmem/ram_map.c new file mode 100644 index 000000000000..cc8855052b75 --- /dev/null +++ b/tools/testing/selftests/devmem/ram_map.c @@ -0,0 +1,250 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* devmem test ram_map.c + * + * Copyright (C) 2025 Red Hat, Inc. All Rights Reserved. + * Written by Alessandro Carminati (acarmina@redhat.com) + */ + +#include <errno.h> +#include <stdint.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <inttypes.h> +#include "ram_map.h" +#include "utils.h" +#include "debug.h" + +static int calculate_bits(uint64_t max_addr) +{ + uint64_t value = max_addr + 1; + int bits = 0; + + while (value > 0) { + value >>= 1; + bits++; + } + return bits; +} + +uint64_t get_highest_ram_addr(const struct ram_map *map) +{ + if (!map || map->count == 0) + return 0; + return map->regions[map->count - 1].end; +} + +static int fill_iomem_regions(FILE *fp, struct ram_map *map) +{ + char line[512]; + uint64_t start, end; + char name[256]; + size_t idx = 0; + + while (fgets(line, sizeof(line), fp)) { + if (sscanf(line, "%" SCNx64 "-%" SCNx64 " : %255[^\n]", + &start, &end, name) == 3) { + map->regions[idx].start = start; + map->regions[idx].end = end; + map->regions[idx].name = strdup(name); + if (!map->regions[idx].name) { + perror("strdup"); + return -1; + } + idx++; + } + } + return 0; +} + +static size_t count_iomem_regions(FILE *fp) +{ + char line[512]; + size_t count = 0; + uint64_t start, end; + char name[256]; + + rewind(fp); + while (fgets(line, sizeof(line), fp)) { + if (sscanf(line, "%" SCNx64 "-%" SCNx64 " : %255[^\n]", + &start, &end, name) == 3) { + count++; + } + } + rewind(fp); + return count; +} + +struct ram_map *parse_iomem(void) +{ + FILE *fp = fopen("/proc/iomem", "r"); + + if (!fp) { + perror("fopen /proc/iomem"); + return NULL; + } + + size_t count = count_iomem_regions(fp); + + if (count == 0) { + fprintf(stderr, "No parsable regions found in /proc/iomem.\n"); + fclose(fp); + return NULL; + } + + struct ram_map *map = calloc(1, sizeof(*map)); + + if (!map) { + perror("calloc map"); + fclose(fp); + return NULL; + } + + map->regions = calloc(count, sizeof(*map->regions)); + if (!map->regions) { + perror("calloc regions"); + free(map); + fclose(fp); + return NULL; + } + map->count = count; + + if (fill_iomem_regions(fp, map) < 0) { + fclose(fp); + return NULL; + } + + fclose(fp); + return map; +} + +void free_ram_map(struct ram_map *map) +{ + if (!map) + return; + + for (size_t i = 0; i < map->count; i++) + free(map->regions[i].name); + + free(map->regions); + free(map); +} + +uint64_t find_last_linear_byte(int fd, uint64_t low_start, uint64_t max_addr) +{ + uint64_t low = low_start + SAFE_OFFSET; + uint64_t high = max_addr; + uint64_t last_good = 0; + + while (low <= high) { + uint64_t mid = low + (high - low) / 2; + int ret = try_read_dev_mem(fd, mid, 0, NULL); + + if (ret > 0) { + last_good = mid; + low = mid + 1; + } else if (ret == -EFAULT) { + if (mid == 0) + break; + high = mid - 1; + } else { + deb_printf("Unexpected error at 0x%llx: %d\n", + (unsigned long long)mid, -ret); + break; + } + } + return last_good; +} + +void dump_ram_map(const struct ram_map *map) +{ + printf("Parsed RAM map (%zu regions):\n", map->count); + + for (size_t i = 0; i < map->count; i++) { + printf(" %016" SCNx64 "-%016" SCNx64 " : %s\n", + map->regions[i].start, + map->regions[i].end, + map->regions[i].name); + } +} + +void report_physical_memory(const struct ram_map *map) +{ + uint64_t highest_addr = get_highest_ram_addr(map); + + if (highest_addr == 0) { + printf("No System RAM regions detected!\n"); + return; + } + + int bits = calculate_bits(highest_addr); + + printf("Highest physical RAM address: 0x%llx\n", + (unsigned long long)highest_addr); + printf("Physical address width (installed RAM): %d bits\n", bits); +} + +uint64_t find_high_system_ram_addr(const struct ram_map *map) +{ + for (size_t i = 0; i < map->count; i++) { + if (strstr(map->regions[i].name, "System RAM") && + map->regions[i].start >= LOW_MEM_LIMIT) { + return map->regions[i].start; + } + } + return 0; +} + +uint64_t pick_restricted_address(const struct ram_map *map) +{ + if (!map || !map->regions || map->count == 0) + return 0; + + for (size_t i = 0; i < map->count; i++) { + if ((!strcmp("System RAM", map->regions[i].name)) && + (map->regions[i].start < LEGACY_MEM_START)) { + uint64_t start = map->regions[i].start; + uint64_t end = map->regions[i].end; + + if (end > start) + return start + (end - start) / 2; + } + } + + return 0; +} + +uint64_t pick_outside_address(const struct ram_map *map) +{ + uint64_t max_addr = 0; + + if (!map || !map->regions || map->count == 0) + return 0; + + for (size_t i = 0; i < map->count; i++) { + if (max_addr < map->regions[i].end) + max_addr = map->regions[i].end; + } + + return max_addr + 0x1000; +} + +uint64_t pick_valid_ram_address(const struct ram_map *map) +{ + uint64_t best_low = 0, best_size = 0; + + if (!map || !map->regions || map->count == 0) + return 0; + + for (size_t i = 0; i < map->count; i++) { + if (!strcmp("System RAM", map->regions[i].name)) { + if (best_size < map->regions[i].end - + map->regions[i].start) { + best_low = map->regions[i].end; + best_size = map->regions[i].end - + map->regions[i].start; + } + } + } + return best_low + (best_size / 2); +} diff --git a/tools/testing/selftests/devmem/ram_map.h b/tools/testing/selftests/devmem/ram_map.h new file mode 100644 index 000000000000..8b1bd976b0b9 --- /dev/null +++ b/tools/testing/selftests/devmem/ram_map.h @@ -0,0 +1,38 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* devmem test ram_map.h + * + * Copyright (C) 2025 Red Hat, Inc. All Rights Reserved. + * Written by Alessandro Carminati (acarmina@redhat.com) + */ + +#ifndef RAM_MAP_H +#define RAM_MAP_H + +#define _GNU_SOURCE +#define SAFE_OFFSET (512ULL * 1024ULL) +#define LOW_MEM_LIMIT 0x100000ULL +#define LEGACY_MEM_START 0x10000 + +struct ram_region { + uint64_t start; + uint64_t end; + char *name; +}; + +struct ram_map { + struct ram_region *regions; + size_t count; +}; + +uint64_t get_highest_ram_addr(const struct ram_map *map); +struct ram_map *parse_iomem(void); +void free_ram_map(struct ram_map *map); +uint64_t find_last_linear_byte(int fd, uint64_t low_start, uint64_t max_addr); +void dump_ram_map(const struct ram_map *map); +void report_physical_memory(const struct ram_map *map); +uint64_t find_high_system_ram_addr(const struct ram_map *map); +uint64_t pick_restricted_address(const struct ram_map *map); +uint64_t pick_outside_address(const struct ram_map *map); +uint64_t pick_valid_ram_address(const struct ram_map *map); + +#endif diff --git a/tools/testing/selftests/devmem/secret.c b/tools/testing/selftests/devmem/secret.c new file mode 100644 index 000000000000..164f58947af5 --- /dev/null +++ b/tools/testing/selftests/devmem/secret.c @@ -0,0 +1,46 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* devmem test secret.c + * + * Copyright (C) 2025 Red Hat, Inc. All Rights Reserved. + * Written by Alessandro Carminati (acarmina@redhat.com) + */ + +#include <sys/syscall.h> +#include <sys/mman.h> +#include <unistd.h> + + +static int memfd_secret(unsigned int flags) +{ + return syscall(SYS_memfd_secret, flags); +} + +void *secret_alloc(size_t size) +{ + int fd = -1; + void *m; + void *result = NULL; + + fd = memfd_secret(0); + if (fd < 0) + goto out; + + if (ftruncate(fd, size) < 0) + goto out; + + m = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); + if (m == MAP_FAILED) + goto out; + + result = m; + +out: + if (fd >= 0) + close(fd); + return result; +} + +void secret_free(void *p, size_t size) +{ + munmap(p, size); +} diff --git a/tools/testing/selftests/devmem/secret.h b/tools/testing/selftests/devmem/secret.h new file mode 100644 index 000000000000..07d263fc05e3 --- /dev/null +++ b/tools/testing/selftests/devmem/secret.h @@ -0,0 +1,13 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* devmem test secret.h + * + * Copyright (C) 2025 Red Hat, Inc. All Rights Reserved. + * Written by Alessandro Carminati (acarmina@redhat.com) + */ + +#ifndef SECRET_H +#define SECRET_H + +void *secret_alloc(size_t size); +void secret_free(void *p, size_t size); +#endif diff --git a/tools/testing/selftests/devmem/tests.c b/tools/testing/selftests/devmem/tests.c new file mode 100644 index 000000000000..58ad673c438f --- /dev/null +++ b/tools/testing/selftests/devmem/tests.c @@ -0,0 +1,569 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* devmem test tests.c + * + * Copyright (C) 2025 Red Hat, Inc. All Rights Reserved. + * Written by Alessandro Carminati (acarmina@redhat.com) + */ + +#define _FILE_OFFSET_BITS 64 +#include <errno.h> +#include <fcntl.h> +#include <stddef.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <sys/stat.h> +#include <sys/sysmacros.h> +#include <time.h> +#include <sys/wait.h> +#include <unistd.h> + +#include "tests.h" +#include "debug.h" +#include "utils.h" +#include "ram_map.h" +#include "secret.h" + +#define KPROBE_EVENTS_PATH "%s/kprobe_events" +#define KPROBE_EVENTS_ENABLE "%s/events/kprobes/enable" +#define TRACE_PIPE_PATH "%s/trace_pipe" +#define MAX_LINE_LENGTH 256 +#define RETPROBE_NAME "open_retprobe" + +struct open_res { + int open_resv; + bool test_res; +}; +char *tracing_dir; +char *tracingdirs[3] = { + NULL, + "/sys/kernel/tracing", + "/sys/kernel/debug/tracing" +}; + +int check_and_set_tracefs_mount(void) +{ + FILE *mounts_file; + char line[256]; + char device[64], mount_point[128], fs_type[32]; + int retval = 0; + + mounts_file = fopen("/proc/mounts", "r"); + if (mounts_file == NULL) { + perror("Failed to open /proc/mounts"); + return 0; // Cannot verify, assume not mounted + } + + while (fgets(line, sizeof(line), mounts_file)) { + if (sscanf(line, "%s %s %s", device, mount_point, fs_type) >= 3) { + if (strcmp(mount_point, "/sys/kernel/tracing") == 0 && + strcmp(fs_type, "tracefs") == 0) { + retval = 1; + break; + } + if (strcmp(mount_point, "/sys/kernel/debug/tracing") == 0 && + strcmp(fs_type, "tracefs") == 0) { + retval = 2; + break; + } + } + } + tracing_dir = tracingdirs[retval]; + return retval; +} + +int get_device_numbers(int fd, unsigned int *major_num, + unsigned int *minor_num) +{ + struct stat file_stat; + + if (fstat(fd, &file_stat) == -1) { + perror("fstat failed"); + return -1; + } + + if (S_ISCHR(file_stat.st_mode) || S_ISBLK(file_stat.st_mode)) { + *major_num = major(file_stat.st_rdev); + *minor_num = minor(file_stat.st_rdev); + return 0; + } + fprintf(stderr, "File descriptor does not refer to a device file.\n"); + return -1; +} + +static int write_file(const char *path, const char *data) +{ + int fd = open(path, O_WRONLY | O_TRUNC); + ssize_t ret; + + if (fd < 0) { + deb_printf("Error opening file %s: %s\n", + path, strerror(errno)); + return -1; + } + deb_printf("echo "%s" >%s\n", data, path); + ret = write(fd, data, strlen(data)); + close(fd); + if (ret < 0) { + deb_printf("Error writing to file %s: %s\n", + path, strerror(errno)); + return -1; + } + return 0; +} + +static void cleanup_probes(void) +{ + deb_printf("Cleaning up kprobes and tracing...\n"); + char buf[100]; + + sprintf(buf, KPROBE_EVENTS_PATH, tracing_dir); + if (write_file(buf, "\n") != 0) + deb_printf("Failed to clear retprobes. Manual cleanup may be required.\n"); + + sprintf(buf, KPROBE_EVENTS_ENABLE, tracing_dir); + if (write_file(buf, "0") != 0) + deb_printf("Failed to clear retprobes. Manual cleanup may be required.\n"); + +} + +static void traced_open(const char *filename, const char *expected_func_name, + struct open_res *r) +{ + pid_t child_pid, parent_pid, traced_pid, result; + char retprobe_setup_cmd[MAX_LINE_LENGTH]; + char tmp_path[MAX_LINE_LENGTH]; + char line[MAX_LINE_LENGTH]; + int open_resv, retval = -1; + struct open_res res; + int status, timeout; + FILE *trace_file; + time_t start; + int pfd[2]; + int sn; + + r->open_resv = -1; + r->test_res = false; + + parent_pid = getpid(); + + if (pipe(pfd) == -1) { + perror("pipe failed"); + return; + } + + deb_printf("Configuring kprobes on '%s'...\n", expected_func_name); + snprintf(tmp_path, sizeof(tmp_path), KPROBE_EVENTS_PATH, tracing_dir); + snprintf(retprobe_setup_cmd, sizeof(retprobe_setup_cmd), + "r2:kprobes/%s_ret %s retval=$retval ", RETPROBE_NAME, + expected_func_name); + if (write_file(tmp_path, retprobe_setup_cmd) != 0) { + cleanup_probes(); + return; + } + snprintf(tmp_path, sizeof(tmp_path), KPROBE_EVENTS_ENABLE, + tracing_dir); + if (write_file(tmp_path, "1") != 0) { + cleanup_probes(); + return; + } + + child_pid = fork(); + if (child_pid == -1) { + deb_printf("fork failed\n"); + cleanup_probes(); + return; + } + + if (child_pid == 0) { + close(pfd[0]); + snprintf(line, sizeof(line), TRACE_PIPE_PATH, tracing_dir); + trace_file = fopen(line, "r"); + if (!trace_file) { + deb_printf("fopen trace_pipe failed in child\n"); + exit(EXIT_FAILURE); + } + + open_resv = -1; + + sleep(2); + while (fgets(line, sizeof(line), trace_file) != NULL) { + traced_pid = -1; + deb_printf("Received =>%s\n", line); + deb_printf("matching against: RETPROBE_NAME="%s" and expected_func_name="%s"\n", + RETPROBE_NAME, expected_func_name); + deb_printf("matching against: RETPROBE_NAME="%s" => %p\n", + RETPROBE_NAME, strstr(line, RETPROBE_NAME)); + deb_printf("matching against: expected_func_name="%s" =>%p\n", + expected_func_name, strstr(line, expected_func_name)); + + if (strstr(line, RETPROBE_NAME) && + strstr(line, expected_func_name)) { + sn = sscanf(line, " %*[^-]-%d%*[^=]=%x", &traced_pid, &open_resv); + deb_printf("scanned (%d)traced_pid=%d, open_resv=%d parent_pid=%d\n", + sn, traced_pid, open_resv, parent_pid); + if (traced_pid == parent_pid && open_resv == 0) { + deb_printf("found!\n"); + res.open_resv = open_resv; + res.test_res = true; + write(pfd[1], &res, sizeof(res)); + fclose(trace_file); + exit(EXIT_SUCCESS); + } + } + } + fclose(trace_file); + res.open_resv = -1; + res.test_res = false; + write(pfd[1], &res, sizeof(res)); + exit(EXIT_FAILURE); + } else { + close(pfd[1]); + sleep(1); + deb_printf("Parent process (PID %d) is calling open()...\n", + parent_pid); + retval = open(filename, O_RDONLY); + if (retval == -1) { + deb_printf("open failed\n"); + kill(child_pid, SIGTERM); + waitpid(child_pid, NULL, 0); + cleanup_probes(); + return; + } + + start = time(NULL); + timeout = 15; + + while (1) { + result = waitpid(-1, &status, WNOHANG); + if (result == -1) { + perror("waitpid"); + break; + } else if (result > 0) { + deb_printf("Child exited normally\n"); + break; + } + + if (time(NULL) - start >= timeout) { + printf("Timeout reached! Killing child...\n"); + kill(child_pid, SIGKILL); + waitpid(child_pid, NULL, 0); + break; + } + usleep(100000); + } + + if (read(pfd[0], r, sizeof(struct open_res)) != + sizeof(struct open_res)) { + deb_printf("Failed to read data from child process.\n"); + r->test_res = false; + } + + close(pfd[0]); + + cleanup_probes(); + + r->open_resv = retval; + if (r->open_resv >= 0 && r->test_res) + r->test_res = true; + else + r->test_res = false; + } +} + +int test_read_at_addr_32bit_ge(struct test_context *t) +{ + if (is_64bit_arch()) { + deb_printf("Skipped (64-bit architecture)\n"); + return SKIPPED; + } + + uint64_t target_addr = 0x100000000ULL; + int ret = try_read_dev_mem(t->fd, target_addr, 0, NULL); + + if (ret == 0) { + deb_printf("PASS: Read beyond 4 GiB at 0x%llx returned 0 bytes\n", + target_addr); + return PASS; + } + deb_printf("FAIL: Expected 0 bytes at 0x%llx, got %d (errno=%d)\n", + target_addr, ret, -ret); + return FAIL; +} + +int test_read_outside_linear_map(struct test_context *t) +{ + uint64_t tolerance, start_addr, max_addr, last_linear; + + if (sizeof(void *) == 8) { + deb_printf("Skipped: 64-bit architecture\n"); + return SKIPPED; + } + + if (!t->map || t->map->count == 0) { + deb_printf("No memory map provided!\n"); + return SKIPPED; + } + + start_addr = t->map->regions[0].start; + max_addr = t->map->regions[t->map->count - 1].end; + + deb_printf("Scanning between 0x%llx and 0x%llx\n", + (unsigned long long)start_addr, (unsigned long long)max_addr); + + last_linear = find_last_linear_byte(t->fd, start_addr, max_addr); + + deb_printf("Last readable linear address: 0x%llx\n", + (unsigned long long)last_linear); + + tolerance = 16 * 1024 * 1024; + if (last_linear + 1 >= EXPECTED_LINEAR_LIMIT - tolerance && + last_linear + 1 <= EXPECTED_LINEAR_LIMIT + tolerance) { + deb_printf("PASS: Linear map ends near 1 GiB boundary.\n"); + return PASS; + } + deb_printf("FAIL: Linear map ends unexpectedly (expected ~890MB).\n"); + return FAIL; +} + +int test_write_outside_linear_map(struct test_context *t) +{ + uint64_t tolerance, start_addr, max_addr, last_linear; + + if (sizeof(void *) == 8) { + deb_printf("Skipped: 64-bit architecture\n"); + return SKIPPED; + } + + if (!t->map || t->map->count == 0) { + deb_printf("No memory map provided!\n"); + return SKIPPED; + } + + start_addr = t->map->regions[0].start; + max_addr = t->map->regions[t->map->count - 1].end; + + deb_printf("Scanning between 0x%llx and 0x%llx\n", (unsigned long long)start_addr, + (unsigned long long)max_addr); + + last_linear = find_last_linear_byte(t->fd, start_addr, max_addr); + + deb_printf("Last readable linear address: 0x%llx\n", + (unsigned long long)last_linear); + + tolerance = 16 * 1024 * 1024; + if (last_linear + 1 >= EXPECTED_LINEAR_LIMIT - tolerance && + last_linear + 1 <= EXPECTED_LINEAR_LIMIT + tolerance) { + deb_printf("PASS: Linear map ends near 1 GiB boundary.\n"); + fill_random_chars(t->srcbuf, BOUNCE_BUF_SIZE); + if (try_write_dev_mem(t->fd, last_linear + 0x1000, + BOUNCE_BUF_SIZE, t->srcbuf) < 0) { + return FAIL; + } + return PASS; + } + deb_printf("FAIL: Linear map ends unexpectedly (expected ~890MB).\n"); + return FAIL; +} + +int test_strict_devmem(struct test_context *t) +{ + int res = FAIL; + uint64_t addr; + ssize_t ret; + uint8_t buf; + + addr = find_high_system_ram_addr(t->map); + if (addr == 0) { + deb_printf("No high System RAM region found.\n"); + res = SKIPPED; + return res; + } + + deb_printf("Testing physical address: 0x%llx\n", addr); + + ret = pread(t->fd, &buf, 1, addr); + if (ret < 0) { + if (errno == EPERM) { + deb_printf("CONFIG_STRICT_DEVMEM is ENABLED\n"); + } else if (errno == EFAULT || errno == ENXIO) { + deb_printf("Invalid address (errno=%d). Try another region.\n", errno); + res = SKIPPED; + } else if (errno == EACCES) { + deb_printf("Access blocked by LSM or lockdown (errno=EACCES).\n"); + res = SKIPPED; + } else { + perror("pread"); + } + } else { + deb_printf("CONFIG_STRICT_DEVMEM is DISABLED\n"); + res = PASS; + } + + if (res != PASS) + t->strict_devmem_state = true; + + return res; +} + +int test_devmem_access(struct test_context *t) +{ + struct open_res res; + + if (!check_and_set_tracefs_mount()) { + deb_printf("Tracing directory not found. This test requires debugfs mounted.\n"); + return FAIL; + } + + traced_open("/dev/mem", "memory_open", &res); + if ((res.test_res) && (res.open_resv >= 0)) { + deb_printf("test_res=%d, open_resv=%d\n", + res.test_res, res.open_resv); + t->fd = res.open_resv; + t->devmem_init_state = true; + return PASS; + } + return FAIL; +} + +int test_read_secret_area(struct test_context *t) +{ + void *tmp_ptr; + + deb_printf("\ntest_read_secret_area - start\n"); + tmp_ptr = secret_alloc(BOUNCE_BUF_SIZE); + + if (tmp_ptr) { + deb_printf("secret_alloc [ok] tmp_ptr va addr = 0x%lx\n", + tmp_ptr); + fill_random_chars(tmp_ptr, BOUNCE_BUF_SIZE); // lazy alloc + if (t->verbose) + print_hex(tmp_ptr, 32); + t->tst_addr = virt_to_phys(tmp_ptr); + if (t->tst_addr) { + deb_printf("filled with things -> tst_addr phy addr = 0x%lx\n", + t->tst_addr); + if (try_read_dev_mem(t->fd, t->tst_addr, + BOUNCE_BUF_SIZE, t->dstbuf) < 0) + return PASS; + } + } + return FAIL; +} + +int test_read_restricted_area(struct test_context *t) +{ + fill_random_chars(t->dstbuf, BOUNCE_BUF_SIZE); + if (t->verbose) + print_hex(t->dstbuf, 32); + t->tst_addr = pick_restricted_address(t->map); + if (t->tst_addr) { + if (try_read_dev_mem(t->fd, t->tst_addr, BOUNCE_BUF_SIZE, + t->dstbuf) >= 0) { + if (t->verbose) + print_hex(t->dstbuf, 32); + + if (is_zero(t->dstbuf, BOUNCE_BUF_SIZE)) + return PASS; + + } + } + return FAIL; +} + +int test_read_allowed_area(struct test_context *t) +{ + fill_random_chars(t->srcbuf, BOUNCE_BUF_SIZE); + t->tst_addr = virt_to_phys(t->srcbuf); + if (t->tst_addr) { + if (try_read_dev_mem(t->fd, t->tst_addr, BOUNCE_BUF_SIZE, + t->dstbuf) >= 0) { + deb_printf("Read OK compare twos\n", t->tst_addr); + if (t->verbose) { + print_hex(t->srcbuf, BOUNCE_BUF_SIZE); + print_hex(t->dstbuf, BOUNCE_BUF_SIZE); + } + if (!memcmp(t->srcbuf, t->dstbuf, BOUNCE_BUF_SIZE)) + return PASS; + } + } + return FAIL; +} + +int test_read_allowed_area_ppos_advance(struct test_context *t) +{ + fill_random_chars(t->srcbuf, BOUNCE_BUF_SIZE); + memset(t->dstbuf, 0, BOUNCE_BUF_SIZE); + if (t->verbose) + print_hex(t->srcbuf, 32); + t->tst_addr = virt_to_phys(t->srcbuf); + if (t->tst_addr) { + if ((try_read_dev_mem(t->fd, t->tst_addr, + BOUNCE_BUF_SIZE / 2, t->dstbuf) >= 0) && + (try_read_inplace(t->fd, BOUNCE_BUF_SIZE / 2, + t->dstbuf) >= 0)){ + if (t->verbose) + print_hex(t->dstbuf, 32); + + if (!memcmp(t->srcbuf + BOUNCE_BUF_SIZE / 2, + t->dstbuf, BOUNCE_BUF_SIZE / 2)) { + return PASS; + } + } + } + return FAIL; +} + +int test_write_outside_area(struct test_context *t) +{ + fill_random_chars(t->srcbuf, BOUNCE_BUF_SIZE); + t->tst_addr = pick_outside_address(t->map); + if (try_write_dev_mem(t->fd, t->tst_addr, BOUNCE_BUF_SIZE, + t->srcbuf) < 0) + return PASS; + + return FAIL; +} + +/* + * this test needs to follow test_seek_seek_set + */ +int test_seek_seek_cur(struct test_context *t) +{ + t->tst_addr = pick_valid_ram_address(t->map); + if (lseek(t->fd, 0, SEEK_SET) == (off_t)-1) + return FAIL; + + if (lseek(t->fd, t->tst_addr, SEEK_CUR) == (off_t)-1) + return FAIL; + + return PASS; +} + +int test_seek_seek_set(struct test_context *t) +{ + t->tst_addr = pick_valid_ram_address(t->map); + if (lseek(t->fd, t->tst_addr, SEEK_SET) == (off_t)-1) + return FAIL; + + return PASS; +} + +int test_seek_seek_other(struct test_context *t) +{ + if (lseek(t->fd, 0, SEEK_END) == (off_t)-1) + return PASS; + + return FAIL; +} + +int test_open_devnum(struct test_context *t) +{ + unsigned int major_num, minor_num; + + if (get_device_numbers(t->fd, &major_num, &minor_num) == 0) { + if ((major_num == 1) && (minor_num == 1)) + return PASS; + } + return FAIL; +} diff --git a/tools/testing/selftests/devmem/tests.h b/tools/testing/selftests/devmem/tests.h new file mode 100644 index 000000000000..376412034cde --- /dev/null +++ b/tools/testing/selftests/devmem/tests.h @@ -0,0 +1,45 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* devmem test tests.h + * + * Copyright (C) 2025 Red Hat, Inc. All Rights Reserved. + * Written by Alessandro Carminati (acarmina@redhat.com) + */ + +#ifndef TESTS_H +#define TESTS_H + +#include "utils.h" + +#define EXPECTED_LINEAR_LIMIT 0x377fe000 +#define PASS 0 +#define FAIL -1 +#define SKIPPED 1 +#define OK_STR "[\e[1;32mPASS\e[0m]" +#define KO_STR "[\e[1;31mFAIL\e[0m]" +#define SKP_STR "[\e[1;33mSKIP\e[0m]" +#define DC_STR "[\e[1;33mDON'T CARE\e[0m]" +#define WARN_STR "\e[1;31mThis shouldn't have happen. Memory is probably corrupted!\e[0m" +#define NO_WARN_STR "" + +int test_read_at_addr_32bit_ge(struct test_context *t); +int test_read_outside_linear_map(struct test_context *t); +int test_strict_devmem(struct test_context *t); +int test_devmem_access(struct test_context *t); +int test_read_secret_area(struct test_context *t); +int test_read_allowed_area(struct test_context *t); +int test_read_reserved_area(struct test_context *t); +int test_read_allowed_area(struct test_context *t); +int test_read_allowed_area_ppos_advance(struct test_context *t); +int test_read_restricted_area(struct test_context *t); +int test_write_outside_area(struct test_context *t); +int test_seek_seek_cur(struct test_context *t); +int test_seek_seek_set(struct test_context *t); +int test_seek_seek_other(struct test_context *t); +int test_open_devnum(struct test_context *t); + +static inline bool is_64bit_arch(void) +{ + return sizeof(void *) == 8; +} + +#endif diff --git a/tools/testing/selftests/devmem/utils.c b/tools/testing/selftests/devmem/utils.c new file mode 100644 index 000000000000..d14e86dbd81a --- /dev/null +++ b/tools/testing/selftests/devmem/utils.c @@ -0,0 +1,379 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* devmem test utils.c + * + * Copyright (C) 2025 Red Hat, Inc. All Rights Reserved. + * Written by Alessandro Carminati (acarmina@redhat.com) + */ + +#define _FILE_OFFSET_BITS 64 +#include <ctype.h> +#include <errno.h> +#include <fcntl.h> +#include <sched.h> +#include <stdbool.h> +#include <stdint.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <sys/mman.h> +#include <sys/stat.h> +#include <unistd.h> + +#include "utils.h" +#include "debug.h" + + +static inline uint64_t get_page_size(void) +{ + return (uint64_t)sysconf(_SC_PAGE_SIZE); +} + +uint64_t virt_to_phys(void *virt_addr) +{ + uint64_t virt_pfn, page_size, phys_addr, pfn; + uintptr_t virt = (uintptr_t)virt_addr; + ssize_t bytes_read; + uint64_t entry = 0; + off_t offset; + int fd; + + page_size = get_page_size(); + virt_pfn = virt / page_size; + deb_printf("page_size=%d, virt_pfn=%lu\n", page_size, virt_pfn); + + fd = open("/proc/self/pagemap", O_RDONLY); + if (fd < 0) { + deb_printf("Error opening /proc/self/pagemap: %s\n", + strerror(errno)); + return 0; + } + + offset = (off_t)(virt_pfn * sizeof(uint64_t)); + deb_printf("lseek(%d, 0x%llx, SEEK_SET)\n", fd, offset); + if (lseek(fd, offset, SEEK_SET) == (off_t)-1) { + deb_printf("Error seeking pagemap: %s\n", strerror(errno)); + close(fd); + return 0; + } + + bytes_read = read(fd, &entry, sizeof(entry)); + close(fd); + if (bytes_read != sizeof(entry)) { + deb_printf("Error reading pagemap: %s\n", strerror(errno)); + return 0; + } + + if (!(entry & (1ULL << 63))) { + deb_printf("Page not present in RAM (maybe swapped out).\n"); + return 0; + } + + pfn = entry & ((1ULL << 55) - 1); + deb_printf("entry=%llx, pfn=%llx\n", entry, pfn); + if (pfn == 0) { + deb_printf("PFN is 0 - invalid mapping.\n"); + return 0; + } + + phys_addr = (pfn * page_size) + (virt % page_size); + deb_printf("phys_addr=%llx\n", phys_addr); + return phys_addr; +} + +int try_read_inplace(int fd, int scnt, void *sbuf) +{ + ssize_t r; + + r = read(fd, sbuf, scnt); + deb_printf("read(%d, %p, %d)=%d(%d)\n", fd, sbuf, scnt, r, -errno); + if (r < 0) + return -errno; + + return (int)r; +} + +int try_read_dev_mem(int fd, uint64_t addr, int scnt, void *sbuf) +{ + int space; + ssize_t r; + void *buf; + int cnt; + + buf = sbuf ? sbuf : &space; + cnt = sbuf ? scnt : sizeof(space); + deb_printf("buf = %p, cnt = %d\n", buf, cnt); + if (lseek(fd, (off_t)addr, SEEK_SET) == (off_t)-1) + return -errno; + + deb_printf("lseek(%d, %llx, SEEK_SET)=%d\n", fd, addr, -errno); + + r = read(fd, buf, cnt); + deb_printf("read(%d, %p, %d)=%d(%d)\n", fd, buf, cnt, r, -errno); + if (r < 0) + return -errno; + + return (int)r; +} + +int try_write_dev_mem(int fd, uint64_t addr, int scnt, void *sbuf) +{ + int space; + ssize_t r; + void *buf; + int cnt; + + buf = sbuf ? sbuf : &space; + cnt = sbuf ? scnt : sizeof(space); + deb_printf("buf = %p, cnt = %d\n", buf, cnt); + if (lseek(fd, (off_t)addr, SEEK_SET) == (off_t)-1) + return -errno; + + deb_printf("lseek(%d, %llx, SEEK_SET)=%d\n", fd, addr, -errno); + + r = write(fd, buf, cnt); + deb_printf("write(%d, %p, %d)=%d(%d)\n", fd, buf, cnt, r, -errno); + if (r < 0) + return -errno; + + return (int)r; +} + +int fill_random_chars(char *buf, int cnt) +{ + int bytes_read, fd; + ssize_t res; + + if (!buf || cnt <= 0) { + errno = EINVAL; + return -1; + } + + fd = open("/dev/urandom", O_RDONLY); + if (fd < 0) { + perror("open /dev/urandom"); + return -1; + } + + bytes_read = 0; + while (bytes_read < cnt) { + res = read(fd, buf + bytes_read, cnt - bytes_read); + if (res < 0) { + if (errno == EINTR) + continue; + perror("read /dev/urandom"); + close(fd); + return -1; + } + bytes_read += res; + } + close(fd); + + return 0; +} + +bool is_zero(const void *p, size_t cnt) +{ + const char *byte_ptr = (const char *)p; + + for (size_t i = 0; i < cnt; ++i) { + if (byte_ptr[i] != 0) + return false; + } + return true; +} + +void print_hex(const void *p, size_t cnt) +{ + const unsigned char *bytes = (const unsigned char *)p; + int remainder; + size_t i; + + for (i = 0; i < cnt; i++) { + if (i % 16 == 0) { + if (i > 0) + printf("\n"); + + printf("%08lX: ", (unsigned long)(bytes + i)); + } + printf("%02X ", bytes[i]); + } + + remainder = cnt % 16; + if (remainder != 0) { + for (int j = 0; j < 16 - remainder; j++) + printf(" "); + } + + printf("\n"); +} + +static bool machine_is_compatible(unsigned int flags) +{ + unsigned int current_arch_flag = 0; + unsigned int current_bits_flag = 0; + +#if defined(__x86_64__) || defined(__i386__) + current_arch_flag = F_ARCH_X86; +#elif defined(__arm__) || defined(__aarch64__) + current_arch_flag = F_ARCH_ARM; +#elif defined(__PPC__) || defined(__powerpc__) + current_arch_flag = F_ARCH_PPC; +#elif defined(__mips__) + current_arch_flag = F_ARCH_MIPS; +#elif defined(__s390__) + current_arch_flag = F_ARCH_S390; +#elif defined(__riscv) + current_arch_flag = F_ARCH_RISCV; +#else + current_arch_flag = 0; +#endif + + if (sizeof(void *) == 8) + current_bits_flag = F_BITS_B64; + else + current_bits_flag = F_BITS_B32; + + bool arch_matches = (flags & F_ARCH_ALL) || (flags & current_arch_flag); + + bool bits_matches = (flags & F_BITS_ALL) || (flags & current_bits_flag); + + return arch_matches && bits_matches; +} + +static void print_flags(uint32_t flags) +{ + printf("Flags: 0x%08X ->", flags); + + // Architecture flags + printf(" Architecture: "); + if (flags & F_ARCH_ALL) + printf("ALL "); + + if (flags & F_ARCH_X86) + printf("X86 "); + + if (flags & F_ARCH_ARM) + printf("ARM "); + + if (flags & F_ARCH_PPC) + printf("PPC "); + + if (flags & F_ARCH_MIPS) + printf("MIPS "); + + if (flags & F_ARCH_S390) + printf("S390 "); + + if (flags & F_ARCH_RISCV) + printf("RISC-V "); + + // Bitness flags + printf(" Bitness: "); + if (flags & F_BITS_ALL) + printf("ALL "); + + if (flags & F_BITS_B64) + printf("64-bit "); + + if (flags & F_BITS_B32) + printf("32-bit "); + + // Miscellaneous flags + printf(" Miscellaneous:"); + if (flags & F_MISC_FATAL) + printf(" - F_MISC_FATAL: true"); + + if (flags & F_MISC_STRICT_DEVMEM_REQ) + printf(" - F_MISC_STRICT_DEVMEM_REQ: true"); + + if (flags & F_MISC_STRICT_DEVMEM_PRV) + printf(" - F_MISC_STRICT_DEVMEM_PRV: true"); + + if (flags & F_MISC_INIT_PRV) + printf(" - F_MISC_INIT_PRV: true"); + + if (flags & F_MISC_INIT_REQ) + printf(" - F_MISC_INIT_REQ: true"); + + printf("\n"); +} + +static void print_context(struct test_context *t) +{ + char *c; + + c = "NO"; + if (t->devmem_init_state) + c = "yes"; + printf("system state: init=%s, ", c); + c = "NO"; + if (t->strict_devmem_state) + c = "yes"; + printf("strict_devmem=%s\n", c); +} + +int test_needed(struct test_context *t, + struct char_mem_test *current) +{ + if (t->verbose) { + print_context(t); + print_flags(current->flags); + } + + if (!(t->devmem_init_state) && !(current->flags & F_MISC_INIT_PRV)) { + deb_printf("Not initialized and test does not provide initialization\n"); + return TEST_DENIED;// Not initialized and not provide init + } + if ((t->devmem_init_state) && (current->flags & F_MISC_INIT_PRV)) { + deb_printf("can not initialize again\n"); + return TEST_INCOHERENT; // can not initialize again + } + if (!(t->devmem_init_state) && (current->flags & F_MISC_INIT_PRV)) { + deb_printf("initializing: test allowed!\n"); + return TEST_ALLOWED; // initializing: test allowed! + } + if (!(t->devmem_init_state)) { + deb_printf("not initialized, can not proceed\n"); + return TEST_DENIED; // not initialized, can not proceed + } + if (!(machine_is_compatible(current->flags))) { + deb_printf("not for this architecture\n"); + return TEST_DENIED; // not for this architecture + } + if (((t->strict_devmem_state) || (current->flags & + F_MISC_STRICT_DEVMEM_REQ)) && !((t->strict_devmem_state) && + (current->flags & F_MISC_STRICT_DEVMEM_REQ))) { + deb_printf("strict_devmem requirement and offering do not meet\n"); + return TEST_DENIED;// strict_devmem requirement + } + deb_printf("test allowed!\n"); + return TEST_ALLOWED; +} + +void *malloc_pb(size_t size) +{ + if (size == 0 || size > getpagesize()) { + fprintf(stderr, "size must be greater than 0 and less than or equal to one page.\n"); + return NULL; + } + + void *ptr = mmap(NULL, getpagesize(), PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + + if (ptr == MAP_FAILED) { + perror("mmap failed"); + return NULL; + } + + return ptr; +} + +void free_pb(void *ptr) +{ + if (ptr == NULL) + return; + + if (munmap(ptr, getpagesize()) == -1) + perror("munmap failed"); + +} diff --git a/tools/testing/selftests/devmem/utils.h b/tools/testing/selftests/devmem/utils.h new file mode 100644 index 000000000000..3a8d052f14ba --- /dev/null +++ b/tools/testing/selftests/devmem/utils.h @@ -0,0 +1,119 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* devmem test utils.h + * + * Copyright (C) 2025 Red Hat, Inc. All Rights Reserved. + * Written by Alessandro Carminati (acarmina@redhat.com) + */ + +#ifndef UTIL_H +#define UTIL_H + +#include <stdbool.h> +#include <stdint.h> +#include <stddef.h> + +#define BOUNCE_BUF_SIZE 64 +/* + * Test Case Flags: + * F_ARCH_ALL: Test valid on all HW Architectures. + * F_ARCH_X86: Test valid on x86 only. + * F_ARCH_ARM: Test valid on ARM only. + * F_ARCH_PPC: Test valid on PowerPC only. + * F_ARCH_MIPS: Test valid on MIPS only. + * F_ARCH_S390: Test valid on S390 only. + * F_ARCH_RISCV: Test valid on RISC-V only. + * + * F_BITS_ALL: Test valid on both 32b and 64b systems. + * F_BITS_B64: Test valid on 64b systems only. + * F_BITS_B32: Test valid on 32b systems only. + * + * F_MISC_FATAL: a test failure stops the execution of any other test. + * F_MISC_STRICT_DEVMEM_REQ: the test requires STRICT_DEVMEM to be defined + * in the Kernel. + * F_MISC_STRICT_DEVMEM_PRV: the test retrieves the status of STRICT_DEVMEM + * (whether it is defined or not in the Kernel). + * F_MISC_INIT_PRV: the test verify the system to be in a proper init state + * for subsequent tests to run. + * F_MISC_INIT_REQ: the test requires a proper init state as retrieved by + * F_MISC_INIT_PRV. + * F_MISC_DONT_CARE: the test is not part of the test plan, it is just + * auxiliary code that determine how to run other tests. + * F_MISC_WARN_ON_SUCCESS: This flags is applicable to negative tests. I.e. + * it raises a Warning if an operation succeeds when + * it is expected to fail. + * F_MISC_WARN_ON_FAILURE: This flags is applicable to positive tests. I.e. + * it raises a Warning if an operation fails when it + * is expected to succeed. + */ +#define F_ARCH_ALL 1 +#define F_ARCH_X86 (1 << 1) +#define F_ARCH_ARM (1 << 2) +#define F_ARCH_PPC (1 << 3) +#define F_ARCH_MIPS (1 << 4) +#define F_ARCH_S390 (1 << 5) +#define F_ARCH_RISCV (1 << 6) + +#define F_BITS_ALL (1 << 7) +#define F_BITS_B64 (1 << 8) +#define F_BITS_B32 (1 << 9) + +#define F_MISC_FATAL (1 << 10) +#define F_MISC_STRICT_DEVMEM_REQ (1 << 11) +#define F_MISC_STRICT_DEVMEM_PRV (1 << 12) +#define F_MISC_INIT_PRV (1 << 13) +#define F_MISC_INIT_REQ (1 << 14) +#define F_MISC_DONT_CARE (1 << 15) +#define F_MISC_WARN_ON_SUCCESS (1 << 16) +#define F_MISC_WARN_ON_FAILURE (1 << 17) + +enum { + TEST_DENIED, + TEST_INCOHERENT, + TEST_ALLOWED +}; + +struct test_context { + struct ram_map *map; + char *srcbuf; + char *dstbuf; + uintptr_t tst_addr; + int fd; + bool verbose; + bool strict_devmem_state; + bool devmem_init_state; +}; + +/* + * struct char_mem_test - test case structure for testing /drivers/char/mem.c + * @name: name of the test case. + * @fn: test callback implementing the test case. + * @descr: test case descriptor; it must be formatted as + * "short description"-"function-name"-"FE<i>" + * where + * "short description" describe what the test case does, + * "function-name" is the name of the tested function in + * /drivers/char/mem.c, + * "FE<i>" is the list of tested Function's Expectations from the + * kernel-doc header associated with "function-name". + * @flags: test case applicable flags (see list above). + */ +struct char_mem_test { + char *name; + int (*fn)(struct test_context *t); + char *descr; + uint64_t flags; +}; + +uint64_t virt_to_phys(void *virt_addr); +int try_read_inplace(int fd, int scnt, void *sbuf); +int try_read_dev_mem(int fd, uint64_t addr, int scnt, void *sbuf); +int try_write_dev_mem(int fd, uint64_t addr, int scnt, void *sbuf); +int fill_random_chars(char *buf, int cnt); +bool is_zero(const void *p, size_t cnt); +void print_hex(const void *p, size_t cnt); +int test_needed(struct test_context *t, struct char_mem_test *current); +void *malloc_pb(size_t size); +void free_pb(void *ptr); + +#endif +
On Wed, Sep 10, 2025 at 07:00:00PM +0200, Gabriele Paoloni wrote:
From: Alessandro Carminati acarmina@redhat.com
This patch introduces a new series of tests for devmem. Test cases are mapped against the tested Function's expectations defined in /drivers/char/mem.c.
Cool, but:
Signed-off-by: Alessandro Carminati acarmina@redhat.com
tools/testing/selftests/Makefile | 1 + tools/testing/selftests/devmem/Makefile | 13 + tools/testing/selftests/devmem/debug.c | 25 + tools/testing/selftests/devmem/debug.h | 14 + tools/testing/selftests/devmem/devmem.c | 200 ++++++++ tools/testing/selftests/devmem/ram_map.c | 250 ++++++++++ tools/testing/selftests/devmem/ram_map.h | 38 ++ tools/testing/selftests/devmem/secret.c | 46 ++ tools/testing/selftests/devmem/secret.h | 13 + tools/testing/selftests/devmem/tests.c | 569 +++++++++++++++++++++++ tools/testing/selftests/devmem/tests.h | 45 ++ tools/testing/selftests/devmem/utils.c | 379 +++++++++++++++ tools/testing/selftests/devmem/utils.h | 119 +++++
That's a lot of files for a "simple" test. Doesn't LTP have tests for this api already? Why not use that here instead?
Also, this is userspace testing, not kunit testing, right, is that intentional? You are documenting internal apis and then writing userspace tests for those apis, which feels a bit odd.
Also /dev/mem should not be used on "modern" systems, so how was this tested?
+// SPDX-License-Identifier: GPL-2.0+
Are you _sure_ you want GPLv2+? I have to ask, sorry.
+/*
- devmem test debug.c
- Copyright (C) 2025 Red Hat, Inc. All Rights Reserved.
- Written by Alessandro Carminati (acarmina@redhat.com)
- */
+#include <stdio.h> +#include <stdarg.h>
+#define DEBUG_FLAG 0 +int pdebug = DEBUG_FLAG;
That's a funny define that is never used elsewhere. I'm guessing this was cut/pasted from some other userspace code somewhere?
+void deb_printf(const char *fmt, ...)
Who is "deb"? You have more letters, always use them :)
Also, why debugging for just this one set of tests? Don't kselftests already have debugging logic? if not, why is this unique to require it?
And am I missing something, or does this new tool not tie into the kselftest framework properly? I see lots of printing to output, but not in the proper test framework format, am I just missing that somewhere?
thanks,
greg k-h
Hi Greg,
On Tue, Oct 21, 2025 at 9:41 AM Greg KH gregkh@linuxfoundation.org wrote:
On Wed, Sep 10, 2025 at 07:00:00PM +0200, Gabriele Paoloni wrote:
From: Alessandro Carminati acarmina@redhat.com
This patch introduces a new series of tests for devmem. Test cases are mapped against the tested Function's expectations defined in /drivers/char/mem.c.
Cool, but:
Signed-off-by: Alessandro Carminati acarmina@redhat.com
tools/testing/selftests/Makefile | 1 + tools/testing/selftests/devmem/Makefile | 13 + tools/testing/selftests/devmem/debug.c | 25 + tools/testing/selftests/devmem/debug.h | 14 + tools/testing/selftests/devmem/devmem.c | 200 ++++++++ tools/testing/selftests/devmem/ram_map.c | 250 ++++++++++ tools/testing/selftests/devmem/ram_map.h | 38 ++ tools/testing/selftests/devmem/secret.c | 46 ++ tools/testing/selftests/devmem/secret.h | 13 + tools/testing/selftests/devmem/tests.c | 569 +++++++++++++++++++++++ tools/testing/selftests/devmem/tests.h | 45 ++ tools/testing/selftests/devmem/utils.c | 379 +++++++++++++++ tools/testing/selftests/devmem/utils.h | 119 +++++
That's a lot of files for a "simple" test. Doesn't LTP have tests for this api already? Why not use that here instead?
Indeed, at first glance the test may look simple. However, despite the high-level concept being straightforward, there are several corner cases and peculiar aspects that needed to be covered, that’s why the test grew larger than expected. For this RFC, my intent was mainly to share the full picture of what I had in mind. I fully agree that it can be refined and streamlined in the next iterations. When I started working on this, I wasn’t aware of any existing LTP tests for this interface. After your comment, I double-checked, but I couldn’t find any relevant coverage. As far as I can tell, neither LTP nor the upstream kernel selftests (kselftest or KUnit) currently include tests specifically targeting /dev/mem.
Also, this is userspace testing, not kunit testing, right, is that intentional? You are documenting internal apis and then writing userspace tests for those apis, which feels a bit odd.
Yes, that’s intentional. The main reason is that all the interactions happen from userspace, and the internal helper functions aren’t meant to be consumed by other kernel components. So I chose a kselftest-style approach. That said, this is still an RFC, I’m happy to adapt it based on community feedback and direction.
Also /dev/mem should not be used on "modern" systems, so how was this tested?
That’s indeed one of the nuances I mentioned earlier. Access to /dev/mem is often restricted for security reasons, so the test is designed to detect and adapt to the environment, only performing operations that are actually allowed.
+// SPDX-License-Identifier: GPL-2.0+
Are you _sure_ you want GPLv2+? I have to ask, sorry.
Yes, I used GPLv2+ deliberately, but I’m open to changing it if the consensus is that GPLv2-only is more appropriate here.
+/*
- devmem test debug.c
- Copyright (C) 2025 Red Hat, Inc. All Rights Reserved.
- Written by Alessandro Carminati (acarmina@redhat.com)
- */
+#include <stdio.h> +#include <stdarg.h>
+#define DEBUG_FLAG 0 +int pdebug = DEBUG_FLAG;
That's a funny define that is never used elsewhere. I'm guessing this was cut/pasted from some other userspace code somewhere?
It wasn’t copied, but I agree it looks unnecessary in its current form. I can remove or rework it in the next version.
+void deb_printf(const char *fmt, ...)
Who is "deb"? You have more letters, always use them :)
Also, why debugging for just this one set of tests? Don't kselftests already have debugging logic? if not, why is this unique to require it?
And am I missing something, or does this new tool not tie into the kselftest framework properly? I see lots of printing to output, but not in the proper test framework format, am I just missing that somewhere?
You’re right, this was a small custom debug I left in place, mainly because I needed a way to include additional information that isn’t normally part of the standard selftest output, such as references to specific requirements in the code. My intent was to make it easier to correlate test results with those requirements. That said, I fully agree that the implementation isn’t aligned with the kselftest framework style, and I’ll look into integrating this need in a more standardized and consistent way in the next iteration.
thanks,
greg k-h
Thanks a lot for the detailed feedback, it’s very helpful. I’ll revise the test accordingly for the next round.
Best regards, Alessandro
-- --- 172
On Wed, Sep 10, 2025 at 06:59:57PM +0200, Gabriele Paoloni wrote:
[1] was an initial proposal defining testable code specifications for some functions in /drivers/char/mem.c. However a Guideline to write such specifications was missing and test cases tracing to such specifications were missing. This patchset represents a next step and is organised as follows:
- patch 1/3 contains the Guideline for writing code specifications
- patch 2/3 contains examples of code specfications defined for some functions of drivers/char/mem.c
- patch 3/3 contains examples of selftests that map to some code specifications of patch 2/3
[1] https://lore.kernel.org/all/20250821170419.70668-1-gpaoloni@redhat.com/
"RFC" implies there is a request. I don't see that here, am I missing that? Or is this "good to go" and want us to seriously consider accepting this?
thanks,
greg k-h
Hi Greg
On Tue, Oct 21, 2025 at 9:35 AM Greg KH gregkh@linuxfoundation.org wrote:
On Wed, Sep 10, 2025 at 06:59:57PM +0200, Gabriele Paoloni wrote:
[1] was an initial proposal defining testable code specifications for some functions in /drivers/char/mem.c. However a Guideline to write such specifications was missing and test cases tracing to such specifications were missing. This patchset represents a next step and is organised as follows:
- patch 1/3 contains the Guideline for writing code specifications
- patch 2/3 contains examples of code specfications defined for some functions of drivers/char/mem.c
- patch 3/3 contains examples of selftests that map to some code specifications of patch 2/3
[1] https://lore.kernel.org/all/20250821170419.70668-1-gpaoloni@redhat.com/
"RFC" implies there is a request. I don't see that here, am I missing that? Or is this "good to go" and want us to seriously consider accepting this?
I assumed that an RFC (as in request for comments) that comes with proposed changes to upstream files would be interpreted as a request for feedbacks associated with the proposed changes (what is wrong or what is missing); next time I will communicate the request explicitly.
WRT this specific patchset, the intent is to introduce formalism in specifying code behavior (so that the same formalism can also be used to write and review test cases), so my high level asks would be:
1) In the first part of patch 1/3 we explain why we are doing this and the high level goals. Do you agree with these? Are these clear?
2) In the rest of the patchset we introduce the formalism, we propose some specs (in patch 2) and associated selftests (in patch 3). Please let us know if there is something wrong, missing or to be improved.
Thanks and kind regards Gab
thanks,
greg k-h
On Tue, Oct 21, 2025 at 11:42:24AM +0200, Gabriele Paoloni wrote:
Hi Greg
On Tue, Oct 21, 2025 at 9:35 AM Greg KH gregkh@linuxfoundation.org wrote:
On Wed, Sep 10, 2025 at 06:59:57PM +0200, Gabriele Paoloni wrote:
[1] was an initial proposal defining testable code specifications for some functions in /drivers/char/mem.c. However a Guideline to write such specifications was missing and test cases tracing to such specifications were missing. This patchset represents a next step and is organised as follows:
- patch 1/3 contains the Guideline for writing code specifications
- patch 2/3 contains examples of code specfications defined for some functions of drivers/char/mem.c
- patch 3/3 contains examples of selftests that map to some code specifications of patch 2/3
[1] https://lore.kernel.org/all/20250821170419.70668-1-gpaoloni@redhat.com/
"RFC" implies there is a request. I don't see that here, am I missing that? Or is this "good to go" and want us to seriously consider accepting this?
I assumed that an RFC (as in request for comments) that comes with proposed changes to upstream files would be interpreted as a request for feedbacks associated with the proposed changes (what is wrong or what is missing); next time I will communicate the request explicitly.
WRT this specific patchset, the intent is to introduce formalism in specifying code behavior (so that the same formalism can also be used to write and review test cases), so my high level asks would be:
- In the first part of patch 1/3 we explain why we are doing this and the high
level goals. Do you agree with these? Are these clear?
No, and no.
I think this type of thing is, sadly, folly. You are entering into a path that never ends with no clear goal that you are conveying here to us.
I might be totally wrong, but I fail to see what you want to have happen in the end.
Every in-kernel api documented in a "formal" way like this? Or a subset? If a subset, which ones specifically? How many? And who is going to do that? And who is going to maintain it? And most importantly, why is it needed at all?
For some reason Linux has succeeded in pretty much every place an operating system is needed for cpus that it can run on (zephyr for those others that it can not.) So why are we suddenly now, after many decades, requiring basic user/kernel stuff to be formally documented like this?
In the past, when we have had "validating bodies" ask for stuff like this, the solution is to provide it in a big thick book, outside of the kernel, by the company that wishes to sell such a product to that organization to justify the cost of doing that labor. In every instance that I know of, that book sits on a shelf and gathers dust, while Linux is just updated over the years in those sites to new versions and the book goes quickly out of date as no one really cares about it, except it having been a check-box for a purchase order requirement.
That's business craziness, no need to get us involved in all of that. Heck, look at the stuff around FIPS certification for more insanity. That's a check-box that is required by organizations and then totally ignored and never actually run at all by the user. I feel this is much the same.
So step back, and tell us exactly what files and functions and apis are needed to be documented in this stilted and formal way, who exactly is going to be doing all of that work, and why we should even consider reviewing and accepting and most importantly, maintaining such a thing for the next 40+ years.
- In the rest of the patchset we introduce the formalism, we propose some
specs (in patch 2) and associated selftests (in patch 3). Please let us know if there is something wrong, missing or to be improved.
I made many comments on patch 3, the most important one being that the tests created do not seem to follow any of the standards we have for Linux kernel tests for no documented reason.
The irony of submitting tests for formal specifications that do not follow documented policies is rich :)
thanks,
greg k-h
Hi Greg
On Tue, Oct 21, 2025 at 6:46 PM Greg KH gregkh@linuxfoundation.org wrote:
On Tue, Oct 21, 2025 at 11:42:24AM +0200, Gabriele Paoloni wrote:
Hi Greg
On Tue, Oct 21, 2025 at 9:35 AM Greg KH gregkh@linuxfoundation.org wrote:
On Wed, Sep 10, 2025 at 06:59:57PM +0200, Gabriele Paoloni wrote:
[1] was an initial proposal defining testable code specifications for some functions in /drivers/char/mem.c. However a Guideline to write such specifications was missing and test cases tracing to such specifications were missing. This patchset represents a next step and is organised as follows:
- patch 1/3 contains the Guideline for writing code specifications
- patch 2/3 contains examples of code specfications defined for some functions of drivers/char/mem.c
- patch 3/3 contains examples of selftests that map to some code specifications of patch 2/3
[1] https://lore.kernel.org/all/20250821170419.70668-1-gpaoloni@redhat.com/
"RFC" implies there is a request. I don't see that here, am I missing that? Or is this "good to go" and want us to seriously consider accepting this?
I assumed that an RFC (as in request for comments) that comes with proposed changes to upstream files would be interpreted as a request for feedbacks associated with the proposed changes (what is wrong or what is missing); next time I will communicate the request explicitly.
WRT this specific patchset, the intent is to introduce formalism in specifying code behavior (so that the same formalism can also be used to write and review test cases), so my high level asks would be:
- In the first part of patch 1/3 we explain why we are doing this and the high
level goals. Do you agree with these? Are these clear?
No, and no.
I think this type of thing is, sadly, folly. You are entering into a path that never ends with no clear goal that you are conveying here to us.
I might be totally wrong, but I fail to see what you want to have happen in the end.
Every in-kernel api documented in a "formal" way like this? Or a subset? If a subset, which ones specifically? How many? And who is going to do that? And who is going to maintain it? And most importantly, why is it needed at all?
For some reason Linux has succeeded in pretty much every place an operating system is needed for cpus that it can run on (zephyr for those others that it can not.) So why are we suddenly now, after many decades, requiring basic user/kernel stuff to be formally documented like this?
Let me try to answer starting from the "why". IMO There are 2 aspects to consider. The first one is that requirements/specification and associated tests are valuable in claiming that Linux can be used in safety critical industries (like automotive or aerospace). The second one (that goes beyond the business need) is that the duality of specifications and tests VS code increases the dependability of the code itself so that its expected behaviour and associated constraints are clear to the user and there is evidence of the code behaving as specified (I already tried to address this point in [1]). Some evidence of improvements can be found in the experiments we did. E.g.: - this [2] is a bug found in __ftrace_event_enable_disable() - this [3] is a code optimization that came as we looked at the specifications of __ftrace_event_enable_disable() - here [4] we have documented assumptions of use that must be met when invoking event_enable_read(), otherwise there could be a wrong event status being reported to the user.
Finally the need for having specifications/requirements associated with Kernel code has been already discussed at LPC'24 last year ([5]) and we got a thumb up from some key maintainers with initial directions (hence we started this activity).
[1] https://lore.kernel.org/all/CA+wEVJatTLKt-3HxyExtXf4M+fmD6pXcmmCuhd+3-n2J_2T... [2] https://lore.kernel.org/all/20250321170821.101403-1-gpaoloni@redhat.com/ [3] https://lore.kernel.org/all/20250723144928.341184323@kernel.org/ [4] https://lore.kernel.org/all/20250814122206.109096-1-gpaoloni@redhat.com/ [5] https://lpc.events/event/18/contributions/1894/
Now I'll try to answer about the goals. I do not expect to have all Kernel APIs to be specified according to the format that we are proposing; my initial goal is to have such a formalism to be available in the Kernel so that developers that need to write such specifications have a guideline available. For example today we have a guideline to write kernel-doc comments, however (AFAIK) patch acceptance is not gated by these being present or not. I think that developers having a direct interest can write such specifications and associated tests and these could be reviewed and eventually accepted by maintainers.
In the past, when we have had "validating bodies" ask for stuff like this, the solution is to provide it in a big thick book, outside of the kernel, by the company that wishes to sell such a product to that organization to justify the cost of doing that labor. In every instance that I know of, that book sits on a shelf and gathers dust, while Linux is just updated over the years in those sites to new versions and the book goes quickly out of date as no one really cares about it, except it having been a check-box for a purchase order requirement.
I agree and in fact a key ask from maintainers, as we discussed at LPC'24 in [5], was to have the code specifications sitting next to the code so that they could be maintainable as the code evolves (and they would be even more maintainable if specs come with tests that can flag regressions as the code evolves).
That's business craziness, no need to get us involved in all of that. Heck, look at the stuff around FIPS certification for more insanity. That's a check-box that is required by organizations and then totally ignored and never actually run at all by the user. I feel this is much the same.
So step back, and tell us exactly what files and functions and apis are needed to be documented in this stilted and formal way, who exactly is going to be doing all of that work, and why we should even consider reviewing and accepting and most importantly, maintaining such a thing for the next 40+ years.
Putting business needs aside, I would expect maintainers, today, to happily accept kernel-doc compliant code documentation as well as kunit or selftests associated with the code; simply because they add technical value. If my assumption here is correct, having these kernel-doc specifications clearly formalised should be an improvement for the maintainers (since it would be easier to verify incoming patches when specs and tests are in place and also it would be easier for a developer to write such patches).
WRT the scope of code to be documented, I expect that to depend on the vested interest of companies contributing to it. The directions from the LPC24 session were to start with some pilot drivers/subsystems first to define 'how' to specify the code and later focus on what and scale...
- In the rest of the patchset we introduce the formalism, we propose some
specs (in patch 2) and associated selftests (in patch 3). Please let us know if there is something wrong, missing or to be improved.
I made many comments on patch 3, the most important one being that the tests created do not seem to follow any of the standards we have for Linux kernel tests for no documented reason.
The irony of submitting tests for formal specifications that do not follow documented policies is rich :)
Thanks for your comments and sorry for the mistakes, I see that Alessandro replied so we'll pay more attention in writing tests in the next revision.
Kind Regards Gab
thanks,
greg k-h
On Wed, Oct 22, 2025 at 04:06:10PM +0200, Gabriele Paoloni wrote:
Every in-kernel api documented in a "formal" way like this? Or a subset? If a subset, which ones specifically? How many? And who is going to do that? And who is going to maintain it? And most importantly, why is it needed at all?
For some reason Linux has succeeded in pretty much every place an operating system is needed for cpus that it can run on (zephyr for those others that it can not.) So why are we suddenly now, after many decades, requiring basic user/kernel stuff to be formally documented like this?
Let me try to answer starting from the "why".
Let's ignore the "why" for now, and get to the "how" and "what" which you skipped from my questions above.
_Exactly_ how many in-kernel functions are you claiming is needed to be documented in this type of way before Linux would become "acceptable" to these regulatory agencies, and which ones _specifically_ are they?
Without knowing that, we could argue about the format all day long, and yet have nothing to show for it.
And then, I have to ask, exactly "who" is going to do that work.
I'll point at another "you must do this for reasons" type of request we have had in the past, SPDX. Sadly that task was never actually finished as it looks like no one really cared to do the real work involved. We got other benefits out of that effort, but the "goal" that people started that effort with was never met. Part of that is me not pushing back hard enough on the "who is going to do the work" part of that question, which is important in stuff like this.
If you never complete the effort, your end goal of passing Linux off to those customers will never happen.
So, try to answer that, with lots and lots of specifics, and then, if we agree that it is a sane thing to attempt (i.e. you are going to do all the work and it actually would be possible to complete), then we can argue about the format of the text :)
thanks,
greg k-h
linux-kselftest-mirror@lists.linaro.org
-
Alessandro Carminati -
Chuck Wolber -
Chuck Wolber -
David Hildenbrand -
Gabriele Paoloni -
Greg KH -
Jonathan Corbet