[XEN PATCH][for-4.19 v2 0/2] update ecl configurations and deviations

[XEN PATCH][for-4.19 v2 0/2] update ecl configurations and deviations

[Nicola Vetrini]

This series brings updates to the configuration of the ECLAIR static analysis
tool, as well as a new document detailing the deviations from MISRA guidelines
that have no noticeable indication in the source code, as detailed by Stefano
Stabellini here [1].

[1] https://marc.info/?l=xen-devel&m=169663845629358&w=2

Nicola Vetrini (1):
  docs/misra: add deviations.rst to document additional deviations.

Simone Ballarin (1):
  automation/eclair: update deviations and accepted guidelines

 .../eclair_analysis/ECLAIR/deviations.ecl     | 135 +++++-----
 automation/eclair_analysis/ECLAIR/tagging.ecl |   4 +-
 docs/index.rst                                |   1 +
 docs/misra/deviations.rst                     | 240 ++++++++++++++++++
 docs/misra/rules.rst                          |   2 +-
 5 files changed, 306 insertions(+), 76 deletions(-)
 create mode 100644 docs/misra/deviations.rst

--
2.34.1

[XEN PATCH][for-4.19 v2 1/2] automation/eclair: update deviations and accepted guidelines

[Nicola Vetrini]

From: Simone Ballarin <simone.ballarin@bugseng.com>

Remove deviations for ERROR_EXIT, ERROR_EXIT_DOM and PIN_FAIL:
the aforementioned macros have been removed.
Add deviation for Rule 2.1 for pure declarations.
Remove legacy text-based deviations: these are now implemented
with SAF comments.
Add deviations for Rules 8.4, 10.1, 13.5, 14.2, 14.3.
Remove deviations for guidelines not yet accepted or rejected.

Add MC3R1.R11.7, MC3R1.R11.8, MC3R1.R11.9, MC3R1.R15.3 and MC3R1.R14.2
to the accepted guidelines selector.

Update clean guidelines selector.

Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com>
---
 .../eclair_analysis/ECLAIR/deviations.ecl     | 135 ++++++++----------
 automation/eclair_analysis/ECLAIR/tagging.ecl |   4 +-
 2 files changed, 64 insertions(+), 75 deletions(-)

diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
index d8170106b449..fa56e5c00a27 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -15,14 +15,19 @@ Constant expressions and unreachable branches of if and switch statements are ex
 -doc_end
 
 -doc_begin="Unreachability caused by calls to the following functions or macros is deliberate and there is no risk of code being unexpectedly left out."
--config=MC3R1.R2.1,statements+={deliberate,"macro(name(BUG||assert_failed||ERROR_EXIT||ERROR_EXIT_DOM||PIN_FAIL))"}
+-config=MC3R1.R2.1,statements+={deliberate,"macro(name(BUG||assert_failed))"}
 -config=MC3R1.R2.1,statements+={deliberate, "call(decl(name(__builtin_unreachable||panic||do_unexpected_trap||machine_halt||machine_restart||maybe_reboot)))"}
 -doc_end
 
--doc_begin="Unreachability of an ASSERT_UNREACHABLE() and analogous macro calls is deliberate and safe."
+-doc_begin="Unreachability inside an ASSERT_UNREACHABLE() and analogous macro calls is deliberate and safe."
 -config=MC3R1.R2.1,reports+={deliberate, "any_area(any_loc(any_exp(macro(name(ASSERT_UNREACHABLE||PARSE_ERR_RET||PARSE_ERR||FAIL_MSR||FAIL_CPUID)))))"}
 -doc_end
 
+-doc_begin="Pure declarations (i.e., declarations without initialization) are
+not executable, and therefore it is safe for them to be unreachable."
+-config=MC3R1.R2.1,ignored_stmts+={"any()", "pure_decl()"}
+-doc_end
+
 -doc_begin="Proving compliance with respect to Rule 2.2 is generally impossible:
 see https://arxiv.org/abs/2212.13933 for details. Moreover, peer review gives us
 confidence that no evidence of errors in the program's logic has been missed due
@@ -49,35 +54,6 @@ they are not instances of commented-out code."
 -config=MC3R1.D4.3,reports+={disapplied,"!(any_area(any_loc(file(^xen/arch/arm/arm64/.*$))))"}
 -doc_end
 
--doc_begin="Depending on the compiler, rewriting the following function-like
-macros as inline functions is not guaranteed to have the same effect."
--config=MC3R1.D4.9,macros+={deliberate,"name(likely)"}
--config=MC3R1.D4.9,macros+={deliberate,"name(unlikely)"}
--config=MC3R1.D4.9,macros+={deliberate,"name(unreachable)"}
--doc_end
-
--doc_begin="These macros can be used on both pointers and unsigned long type values."
--config=MC3R1.D4.9,macros+={safe,"name(virt_to_maddr)"}
--config=MC3R1.D4.9,macros+={safe,"name(virt_to_mfn)"}
--doc_end
-
--doc_begin="Rewriting variadic macros as variadic functions might have a negative impact on safety."
--config=MC3R1.D4.9,macros+={deliberate,"variadic()"}
--doc_end
-
--doc_begin="Rewriting macros with arguments that are, in turn, arguments of
-__builtin_constant_p() can change the behavior depending on the optimization
-level."
--config=MC3R1.D4.9,macro_argument_context+="skip_to(class(type||expr||decl,any),
-                                            call(name(__builtin_constant_p)))"
--doc_end
-
--doc_begin="Function-like macros defined in public headers are meant to be
-usable in C89 mode without any extensions. Hence they cannot be replaced by
-inline functions."
--config=MC3R1.D4.9,macros+={deliberate, "loc(file(api:public))"}
--doc_end
-
 -doc_begin="This header file is autogenerated or empty, therefore it poses no
 risk if included more than once."
 -file_tag+={empty_header, "^xen/arch/arm/efi/runtime\\.h$"}
@@ -105,29 +81,6 @@ conform to the directive."
 -config=MC3R1.R5.3,reports+={safe, "any_area(any_loc(any_exp(macro(^read_debugreg$))&&any_exp(macro(^write_debugreg$))))"}
 -doc_end
 
--doc_begin="Function-like macros cannot be confused with identifiers that are
-neither functions nor pointers to functions."
--config=MC3R1.R5.5,reports={safe,"all_area(decl(node(enum_decl||record_decl||field_decl||param_decl||var_decl)&&!type(canonical(address((node(function||function_no_proto))))))||macro(function_like()))"}
--doc_end
-
--doc_begin="The use of these identifiers for both macro names and other entities
-is deliberate and does not generate developer confusion."
--config=MC3R1.R5.5,reports+={safe, "any_area(text(^\\s*/\\*\\s+SAF-[0-9]+-safe\\s+MC3R1\\.R5\\.5.*$, begin-1))"}
--doc_end
-
--doc_begin="The definition of macros and functions ending in '_bit' that use the
-same identifier in 'bitops.h' is deliberate and safe."
--file_tag+={bitops_h, "^xen/arch/x86/include/asm/bitops\\.h$"}
--config=MC3R1.R5.5,reports+={safe, "all_area((decl(^.*_bit\\(.*$)||macro(^.*_bit$))&&all_loc(file(bitops_h)))"}
--doc_end
-
--doc_begin="The definition of macros and functions beginning in 'str' or 'mem'
-that use the same identifier in 'xen/include/xen/string.h' is deliberate and
-safe."
--file_tag+={string_h, "^xen/include/xen/string\\.h$"}
--config=MC3R1.R5.5,reports+={safe, "any_area((decl(^(mem|str).*$)||macro(^(mem|str).*$))&&all_loc(file(string_h)))"}
--doc_end
-
 #
 # Series 7.
 #
@@ -156,11 +109,6 @@ particular use of it done in xen_mk_ulong."
 -config=MC3R1.R7.2,reports+={deliberate,"any_area(any_loc(macro(name(BUILD_BUG_ON))))"}
 -doc_end
 
--doc_begin="The following string literals are assigned to pointers to non
-const-qualified char."
--config=MC3R1.R7.4,reports+={safe, "any_area(text(^\\s*/\\*\\s+SAF-[0-9]+-safe\\s+MC3R1\\.R7\\.4.*$, begin-1))"}
--doc_end
-
 -doc_begin="Allow pointers of non-character type as long as the pointee is
 const-qualified."
 -config=MC3R1.R7.4,same_pointee=false
@@ -204,6 +152,17 @@ const-qualified."
 -config=MC3R1.R8.3,reports+={deliberate,"any_area(any_loc(file(adopted_mpparse_r8_3)))&&any_area(any_loc(file(^xen/arch/x86/include/asm/mpspec\\.h$)))"}
 -doc_end
 
+-doc_begin="The definitions present in this file are meant to generate definitions for asm modules, and are not called by C code. Therefore the absence of prior declarations is safe."
+-file_tag+={asm_offsets, "^xen/arch/(arm|x86)/(arm32|arm64|x86_64)/asm-offsets\\.c$"}
+-config=MC3R1.R8.4,reports+={safe, "first_area(any_loc(file(asm_offsets)))"}
+-doc_end
+
+-doc_begin="The functions defined in this file are meant to be called from gcc-generated code in a non-release build configuration.
+Therefore the absence of prior declarations is safe."
+-file_tag+={gcov, "^xen/common/coverage/gcov_base\\.c$"}
+-config=MC3R1.R8.4,reports+={safe, "first_area(any_loc(file(gcov)))"}
+-doc_end
+
 -doc_begin="The following variables are compiled in multiple translation units
 belonging to different executables and therefore are safe."
 -config=MC3R1.R8.6,declarations+={safe, "name(current_stack_pointer||bsearch||sort)"}
@@ -222,12 +181,6 @@ definition is compiled-out or optimized-out by the compiler)"
 # Series 9.
 #
 
--doc_begin="The following variables are written before being set, therefore no
-access to uninitialized memory locations happens, as explained in the deviation
-comment."
--config=MC3R1.R9.1,reports+={safe, "any_area(text(^\\s*/\\*\\s+SAF-[0-9]+-safe\\s+MC3R1\\.R9\\.1.*$, begin-1))"}
--doc_end
-
 -doc_begin="Violations in files that maintainers have asked to not modify in the
 context of R9.1."
 -file_tag+={adopted_r9_1,"^xen/arch/arm/arm64/lib/find_next_bit\\.c$"}
@@ -274,22 +227,47 @@ still non-negative."
 -config=MC3R1.R10.1,etypes+={safe, "stmt(operator(logical)||node(conditional_operator||binary_conditional_operator))", "dst_type(ebool||boolean)"}
 -doc_end
 
-### Set 3 ###
+-doc_begin="XEN only supports architectures where signed integers are
+representend using two's complement and all the XEN developers are aware of
+this."
+-config=MC3R1.R10.1,etypes+={safe,
+  "stmt(operator(and||or||xor||not||and_assign||or_assign||xor_assign))",
+  "any()"}
+-doc_end
+
+-doc_begin="See Section \"4.5 Integers\" of \"GCC_MANUAL\", where it says that
+\"Signed `>>' acts on negative numbers by sign extension. As an extension to the
+C language, GCC does not use the latitude given in C99 and C11 only to treat
+certain aspects of signed `<<' as undefined. However, -fsanitize=shift (and
+-fsanitize=undefined) will diagnose such cases. They are also diagnosed where
+constant expressions are required.\""
+-config=MC3R1.R10.1,etypes+={safe,
+  "stmt(operator(shl||shr||shl_assign||shr_assign))",
+  "any()"}
+-doc_end
 
 #
-# Series 18.
+# Series 13
 #
 
--doc_begin="FIXME: explain why pointer differences involving this macro are safe."
--config=MC3R1.R18.2,reports+={safe,"all_area(all_loc(any_exp(macro(^ACPI_PTR_DIFF$))))"}
+-doc_begin="All developers and reviewers can be safely assumed to be well aware
+of the short-circuit evaluation strategy of such logical operators."
+-config=MC3R1.R13.5,reports+={disapplied,"any()"}
 -doc_end
 
--doc_begin="FIXME: explain why pointer differences involving this macro are safe."
--config=MC3R1.R18.2,reports+={safe,"all_area(all_loc(any_exp(macro(^page_to_mfn$))))"}
+#
+# Series 14
+#
+
+-doc_begin="The severe restrictions imposed by this rule on the use of for
+statements are not balanced by the presumed facilitation of the peer review
+activity."
+-config=MC3R1.R14.2,reports+={disapplied,"any()"}
 -doc_end
 
--doc_begin="FIXME: explain why pointer differences involving this macro are safe."
--config=MC3R1.R18.2,reports+={safe,"all_area(all_loc(any_exp(macro(^page_to_pdx$))))"}
+-doc_begin="The XEN team relies on the fact that invariant conditions of 'if'
+statements are deliberate"
+-config=MC3R1.R14.3,statements={deliberate , "wrapped(any(),node(if_stmt))" }
 -doc_end
 
 #
@@ -306,6 +284,17 @@ in assignments."
 {safe, "left_right(^[(,\\[]$,^[),\\]]$)"}
 -doc_end
 
+#
+# General
+#
+
+-doc_begin="do-while-0 is a well recognized loop idiom by the xen community."
+-loop_idioms={do_stmt, "literal(0)"}
+-doc_end
+-doc_begin="while-[01] is a well recognized loop idiom by the xen community."
+-loop_idioms+={while_stmt, "literal(0)||literal(1)"}
+-doc_end
+
 #
 # Developer confusion
 #
diff --git a/automation/eclair_analysis/ECLAIR/tagging.ecl b/automation/eclair_analysis/ECLAIR/tagging.ecl
index 78a0bc948ba5..e82277fea3c1 100644
--- a/automation/eclair_analysis/ECLAIR/tagging.ecl
+++ b/automation/eclair_analysis/ECLAIR/tagging.ecl
@@ -19,7 +19,7 @@
 
 -doc="Accepted guidelines as reported in XEN/docs/misra/rules.rst"
 -service_selector={accepted_guidelines,
-    "MC3R1.D1.1||MC3R1.D2.1||MC3R1.D4.1||MC3R1.D4.3||MC3R1.D4.7||MC3R1.D4.10||MC3R1.D4.11||MC3R1.D4.14||MC3R1.R1.1||MC3R1.R1.3||MC3R1.R1.4||MC3R1.R2.1||MC3R1.R2.2||MC3R1.R2.6||MC3R1.R2.2||MC3R1.R3.1||MC3R1.R3.2||MC3R1.R4.1||MC3R1.R4.2||MC3R1.R5.1||MC3R1.R5.2||MC3R1.R5.3||MC3R1.R5.4||MC3R1.R5.6||MC3R1.R6.1||MC3R1.R6.2||MC3R1.R7.1||MC3R1.R7.2||MC3R1.R7.3||MC3R1.R7.4||MC3R1.R8.1||MC3R1.R8.2||MC3R1.R8.3||MC3R1.R8.4||MC3R1.R8.5||MC3R1.R8.6||MC3R1.R8.8||MC3R1.R8.10||MC3R1.R8.12||MC3R1.R8.14||MC3R1.R9.1||MC3R1.R9.2||MC3R1.R9.3||MC3R1.R9.4||MC3R1.R9.5||MC3R1.R10.1||MC3R1.R10.2||MC3R1.R10.3||MC3R1.R10.4||MC3R1.R12.5||MC3R1.R13.6||MC3R1.R13.1||MC3R1.R14.1||MC3R1.R14.3||MC3R1.R16.7||MC3R1.R17.3||MC3R1.R17.4||MC3R1.R17.6||MC3R1.R18.3||MC3R1.R19.1||MC3R1.R20.7||MC3R1.R20.13||MC3R1.R20.14||MC3R1.R21.13||MC3R1.R21.17||MC3R1.R21.18||MC3R1.R21.19||MC3R1.R21.20||MC3R1.R21.21||MC3R1.R22.2||MC3R1.R22.4||MC3R1.R22.5||MC3R1.R22.6"
+    "MC3R1.D1.1||MC3R1.D2.1||MC3R1.D4.1||MC3R1.D4.3||MC3R1.D4.7||MC3R1.D4.10||MC3R1.D4.11||MC3R1.D4.14||MC3R1.R1.1||MC3R1.R1.3||MC3R1.R1.4||MC3R1.R2.1||MC3R1.R2.2||MC3R1.R2.6||MC3R1.R3.1||MC3R1.R3.2||MC3R1.R4.1||MC3R1.R4.2||MC3R1.R5.1||MC3R1.R5.2||MC3R1.R5.3||MC3R1.R5.4||MC3R1.R5.6||MC3R1.R6.1||MC3R1.R6.2||MC3R1.R7.1||MC3R1.R7.2||MC3R1.R7.3||MC3R1.R7.4||MC3R1.R8.1||MC3R1.R8.2||MC3R1.R8.3||MC3R1.R8.4||MC3R1.R8.5||MC3R1.R8.6||MC3R1.R8.8||MC3R1.R8.10||MC3R1.R8.12||MC3R1.R8.14||MC3R1.R9.1||MC3R1.R9.2||MC3R1.R9.3||MC3R1.R9.4||MC3R1.R9.5||MC3R1.R10.1||MC3R1.R10.2||MC3R1.R10.3||MC3R1.R10.4||MC3R1.R11.7||MC3R1.R11.8||MC3R1.R11.9||MC3R1.R12.5||MC3R1.R13.1||MC3R1.R13.5||MC3R1.R13.6||MC3R1.R14.1||MC3R1.R14.2||MC3R1.R14.3||MC3R1.R16.7||MC3R1.R17.3||MC3R1.R17.4||MC3R1.R17.6||MC3R1.R18.3||MC3R1.R19.1||MC3R1.R20.7||MC3R1.R20.13||MC3R1.R20.14||MC3R1.R21.13||MC3R1.R21.17||MC3R1.R21.18||MC3R1.R21.19||MC3R1.R21.20||MC3R1.R21.21||MC3R1.R22.2||MC3R1.R22.4||MC3R1.R22.5||MC3R1.R22.6"
 }
 -doc="All reports of accepted guidelines are tagged as accepted."
 -reports+={status:accepted,"service(accepted_guidelines)"}
@@ -30,7 +30,7 @@
 
 -doc_begin="Clean guidelines: new violations for these guidelines are not accepted."
 
--service_selector={clean_guidelines_common,"MC3R1.D1.1||MC3R1.D2.1||MC3R1.D4.11||MC3R1.D4.14||MC3R1.R1.1||MC3R1.R1.3||MC3R1.R1.4||MC3R1.R2.2||MC3R1.R3.1||MC3R1.R3.2||MC3R1.R4.1||MC3R1.R5.1||MC3R1.R5.2||MC3R1.R5.4||MC3R1.R6.1||MC3R1.R6.2||MC3R1.R7.1||MC3R1.R8.1||MC3R1.R8.5||MC3R1.R8.8||MC3R1.R8.10||MC3R1.R8.12||MC3R1.R8.14||MC3R1.R9.2||MC3R1.R9.4||MC3R1.R9.5||MC3R1.R12.5||MC3R1.R17.3||MC3R1.R17.4||MC3R1.R17.6||MC3R1.R21.13||MC3R1.R21.19||MC3R1.R21.21||MC3R1.R22.2||MC3R1.R22.4||MC3R1.R22.5||MC3R1.R22.6"
+-service_selector={clean_guidelines_common,"MC3R1.D1.1||MC3R1.D2.1||MC3R1.D4.11||MC3R1.D4.14||MC3R1.R1.1||MC3R1.R1.3||MC3R1.R1.4||MC3R1.R2.2||MC3R1.R3.1||MC3R1.R3.2||MC3R1.R4.1||MC3R1.R4.2||MC3R1.R5.1||MC3R1.R5.2||MC3R1.R5.4||MC3R1.R6.1||MC3R1.R6.2||MC3R1.R7.1||MC3R1.R8.1||MC3R1.R8.5||MC3R1.R8.8||MC3R1.R8.10||MC3R1.R8.12||MC3R1.R8.14||MC3R1.R9.2||MC3R1.R9.4||MC3R1.R9.5||MC3R1.R12.5||MC3R1.R17.3||MC3R1.R17.4||MC3R1.R17.6||MC3R1.R20.13||MC3R1.R20.14||MC3R1.R21.13||MC3R1.R21.19||MC3R1.R21.21||MC3R1.R22.2||MC3R1.R22.4||MC3R1.R22.5||MC3R1.R22.6"
 }
 
 -setq=target,getenv("XEN_TARGET_ARCH")
-- 
2.34.1

[XEN PATCH][for-4.19 v2 2/2] docs/misra: add deviations.rst to document additional deviations.

[Nicola Vetrini]

This file contains the deviation that are not marked by
a deviation comment, as specified in
docs/misra/documenting-violations.rst.

Suggested-by: Stefano Stabellini <sstabellini@kernel.org>
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 docs/index.rst            |   1 +
 docs/misra/deviations.rst | 240 ++++++++++++++++++++++++++++++++++++++
 docs/misra/rules.rst      |   2 +-
 3 files changed, 242 insertions(+), 1 deletion(-)
 create mode 100644 docs/misra/deviations.rst

diff --git a/docs/index.rst b/docs/index.rst
index 2c47cfa999f2..f3f779f89ce5 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -63,6 +63,7 @@ Xen hypervisor code.
    :maxdepth: 2

    misra/rules
+   misra/deviations


 Miscellanea
diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
new file mode 100644
index 000000000000..19743e34ce03
--- /dev/null
+++ b/docs/misra/deviations.rst
@@ -0,0 +1,240 @@
+.. SPDX-License-Identifier: CC-BY-4.0
+
+MISRA C deviations for Xen
+==========================
+
+The following is the list of MISRA C:2012 deviations for the Xen codebase that
+are not covered by a `SAF-x-safe` or `SAF-x-false-positive-<tool>` comment, as
+specified in docs/misra/documenting-violations.rst; the lack of
+such comments is usually due to the excessive clutter they would bring to the
+codebase or the impossibility to express such a deviation (e.g., if it's
+composed of several conditions).
+
+Deviations related to MISRA C:2012 Directives:
+----------------------------------------------
+
+.. list-table::
+   :header-rows: 1
+
+   * - Directive identifier
+     - Justification
+     - Notes
+
+   * - D4.3
+     - Accepted for the ARM64 codebase
+     - Tagged as `disapplied` for ECLAIR on any other violation report.
+
+   * - D4.3
+     - The inline asm in 'xen/arch/arm/arm64/lib/bitops.c' is tightly coupled
+       with the surronding C code that acts as a wrapper, so it has been decided
+       not to add an additional encapsulation layer.
+     - Tagged as `deliberate` for ECLAIR.
+
+Deviations related to MISRA C:2012 Rules:
+-----------------------------------------
+
+.. list-table::
+   :header-rows: 1
+
+   * - Rule identifier
+     - Justification
+     - Notes
+
+   * - R2.1
+     - The compiler implementation guarantees that the unreachable code is
+       removed. Constant expressions and unreachable branches of if and switch
+       statements are expected.
+     - Tagged as `safe` for ECLAIR.
+
+   * - R2.1
+     - Some functions are intended not to be referenced.
+     - Tagged as `deliberate` for ECLAIR.
+
+   * - R2.1
+     - Unreachability caused by calls to the following functions or macros is
+       deliberate and there is no risk of code being unexpectedly left out.
+     - Tagged as `deliberate` for ECLAIR. Such macros are:
+			 - BUG
+			 - assert_failed
+			 - __builtin_unreachable
+			 - ASSERT_UNREACHABLE
+
+   * - R2.1
+     - Pure declarations, that is, declarations without initializations are not
+       executable, and therefore it is safe for them to be unreachable. The most
+       notable example of such a pattern being used in the codebase is that of
+       a variable declaration that should be available in all the clauses of a
+       switch statement.
+     - ECLAIR has been configured to ignore those statements.
+
+   * - R2.2
+     - Proving compliance with respect to Rule 2.2 is generally impossible:
+       see `<https://arxiv.org/abs/2212.13933>`_ for details. Moreover, peer
+       review gives us confidence that no evidence of errors in the program's
+       logic has been missed due to undetected violations of Rule 2.2, if any.
+       Testing on time behavior gives us confidence on the fact that, should the
+       program contain dead code that is not removed by the compiler, the
+       resulting slowdown is negligible.
+     - Project-wide deviation, tagged as `disapplied` for ECLAIR.
+
+   * - R3.1
+     - Comments starting with '/\*' and containing hyperlinks are safe as they
+       are not instances of commented-out code.
+     - Tagged as `safe` for ECLAIR.
+
+   * - R5.3
+     - As specified in rules.rst, shadowing due to macros being used as macro
+       arguments is allowed, as it's deemed not at risk of causing developer
+       confusion.
+     - Tagged as `safe` for ECLAIR. So far, the following macros are deviated:
+         - READ_SYSREG and WRITE_SYSREG
+         - max_{t}? and min_{t}?
+         - read_[bwlq] and read_[bwlq]_relaxed
+         - per_cpu and this_cpu
+         - __emulate_2op and __emulate_2op_nobyte
+         - read_debugreg and write_debugreg
+
+   * - R7.2
+     - Violations caused by __HYPERVISOR_VIRT_START are related to the
+       particular use of it done in xen_mk_ulong.
+     - Tagged as `deliberate` for ECLAIR.
+
+   * - R7.4
+     - Allow pointers of non-character type as long as the pointee is
+       const-qualified.
+     - ECLAIR has been configured to ignore these assignments.
+
+   * - R8.3
+     - The type ret_t is deliberately used and defined as int or long depending
+       on the architecture.
+     - Tagged as `deliberate` for ECLAIR.
+
+   * - R8.3
+     - Some files are not subject to respect MISRA rules at
+       the moment, but some entity from a file in scope is used; therefore
+       ECLAIR does report a violation, since not all the files involved in the
+       violation are excluded from the analysis.
+     - Tagged as `deliberate` for ECLAIR. Such excluded files are:
+         - xen/arch/x86/time.c
+         - xen/arch/x86/acpi/cpu_idle.c
+         - xen/arch/x86/mpparse.c
+         - xen/common/bunzip2.c
+         - xen/common/unlz4.c
+         - xen/common/unlzma.c
+         - xen/common/unlzo.c
+         - xen/common/unxz.c
+         - xen/common/unzstd.c
+
+   * - R8.4
+     - The definitions present in the files 'asm-offsets.c' for any architecture
+       are used to generate definitions for asm modules, and are not called by
+       C code. Therefore the absence of prior declarations is safe.
+     - Tagged as `safe` for ECLAIR.
+
+   * - R8.4
+     - The functions defined in the file xen/common/coverage/gcov_base.c are
+       meant to be called from gcc-generated code in a non-release build
+       configuration. Therefore, the absence of prior declarations is safe.
+     - Tagged as `safe` for ECLAIR.
+
+   * - R8.6
+     - The following variables are compiled in multiple translation units
+       belonging to different executables and therefore are safe.
+
+       - current_stack_pointer
+       - bsearch
+       - sort
+     - Tagged as `safe` for ECLAIR.
+
+   * - R8.6
+     - Declarations without definitions are allowed (specifically when the
+       definition is compiled-out or optimized-out by the compiler).
+     - Tagged as `deliberate` in ECLAIR.
+
+   * - R8.10
+     - The gnu_inline attribute without static is deliberately allowed.
+     - Tagged as `deliberate` for ECLAIR.
+
+   * - R9.5
+     - The possibility of committing mistakes by specifying an explicit
+       dimension is higher than omitting the dimension, therefore all such
+       instances of violations are deviated.
+     - Project-wide deviation, tagged as `deliberate` for ECLAIR.
+
+   * - R10.1, R10.3, R10.4
+     - The value-preserving conversions of integer constants are safe.
+     - Tagged as `safe` for ECLAIR.
+
+   * - R10.1
+     - Shifting non-negative integers to the right is safe.
+     - Tagged as `safe` for ECLAIR.
+
+   * - R10.1
+     - Shifting non-negative integers to the left is safe if the result is still
+       non-negative.
+     - Tagged as `safe` for ECLAIR.
+
+   * - R10.1
+     - Bitwise logical operations on non-negative integers are safe.
+     - Tagged as `safe` for ECLAIR.
+
+   * - R10.1
+     - The implicit conversion to Boolean for logical operator arguments is
+       well-known to all Xen developers to be a comparison with 0.
+     - Tagged as `safe` for ECLAIR.
+
+   * - R10.1
+     - Xen only supports architectures where signed integers are representend
+       using two's complement and all the Xen developers are aware of this. For
+       this reason, bitwise operations are safe.
+     - Tagged as `safe` for ECLAIR.
+
+   * - R10.1
+     - Given the assumptions on the toolchain detailed in
+       docs/misra/C-language-toolchain.rst and the build flags used by the
+       project, it is deemed safe to use bitwise shift operators.
+       See automation/eclair_analysis/deviations.ecl for the full explanation.
+     - Tagged as `safe` for ECLAIR.
+
+   * - R13.5
+     - All developers and reviewers can be safely assumed to be well aware of
+       the short-circuit evaluation strategy for logical operators.
+     - Project-wide deviation; tagged as `disapplied` for ECLAIR.
+
+   * - R14.2
+     - The severe restrictions imposed by this rule on the use of 'for'
+       statements are not counterbalanced by the presumed facilitation of the
+       peer review activity.
+     - Project-wide deviation; tagged as `disapplied` for ECLAIR.
+
+   * - R14.3
+     - The Xen team relies on the fact that invariant conditions of 'if'
+       statements are deliberate.
+     - Project-wide deviation; tagged as `disapplied` for ECLAIR.
+
+   * - R20.7
+     - Code violating Rule 20.7 is safe when macro parameters are used:
+       (1) as function arguments;
+       (2) as macro arguments;
+       (3) as array indices;
+       (4) as lhs in assignments.
+     - Tagged as `safe` for ECLAIR.
+
+Other deviations:
+-----------------
+
+.. list-table::
+   :header-rows: 1
+
+   * - Deviation
+     - Justification
+
+   * - do-while-0 loops
+     - The do-while-0 is a well-recognized loop idiom used by the Xen community
+       and can therefore be used, even though it would cause a number of
+       violations in some instances.
+
+   * - while-0 and while-1 loops
+     - while-0 and while-1 are well-recognized loop idioms used by the Xen
+       community and can therefore be used, even though they would cause a
+       number of violations in some instances.
diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
index 3139ca7ae6dd..6efe66195de3 100644
--- a/docs/misra/rules.rst
+++ b/docs/misra/rules.rst
@@ -18,7 +18,7 @@ It is possible that in specific circumstances it is best not to follow a
 rule because it is not possible or because the alternative leads to
 better code quality. Those cases are called "deviations". They are
 permissible as long as they are documented. For details, please refer to
-docs/misra/documenting-violations.rst
+docs/misra/documenting-violations.rst and docs/misra/deviations.rst

 Other documentation mechanisms are work-in-progress.

--
2.34.1

Re: [XEN PATCH][for-4.19 v2 2/2] docs/misra: add deviations.rst to document additional deviations.

[Stefano Stabellini]

+Henry

On Mon, 9 Oct 2023, Nicola Vetrini wrote:
> This file contains the deviation that are not marked by
> a deviation comment, as specified in
> docs/misra/documenting-violations.rst.
> 
> Suggested-by: Stefano Stabellini <sstabellini@kernel.org>
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

This is great! Thank you so much!

I have a few questions below but even as-is it is way better than
nothing. I think we should add this for 4.18


> ---
>  docs/index.rst            |   1 +
>  docs/misra/deviations.rst | 240 ++++++++++++++++++++++++++++++++++++++
>  docs/misra/rules.rst      |   2 +-
>  3 files changed, 242 insertions(+), 1 deletion(-)
>  create mode 100644 docs/misra/deviations.rst
> 
> diff --git a/docs/index.rst b/docs/index.rst
> index 2c47cfa999f2..f3f779f89ce5 100644
> --- a/docs/index.rst
> +++ b/docs/index.rst
> @@ -63,6 +63,7 @@ Xen hypervisor code.
>     :maxdepth: 2
> 
>     misra/rules
> +   misra/deviations
> 
> 
>  Miscellanea
> diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
> new file mode 100644
> index 000000000000..19743e34ce03
> --- /dev/null
> +++ b/docs/misra/deviations.rst
> @@ -0,0 +1,240 @@
> +.. SPDX-License-Identifier: CC-BY-4.0
> +
> +MISRA C deviations for Xen
> +==========================
> +
> +The following is the list of MISRA C:2012 deviations for the Xen codebase that
> +are not covered by a `SAF-x-safe` or `SAF-x-false-positive-<tool>` comment, as
> +specified in docs/misra/documenting-violations.rst; the lack of
> +such comments is usually due to the excessive clutter they would bring to the
> +codebase or the impossibility to express such a deviation (e.g., if it's
> +composed of several conditions).
> +
> +Deviations related to MISRA C:2012 Directives:
> +----------------------------------------------
> +
> +.. list-table::
> +   :header-rows: 1
> +
> +   * - Directive identifier
> +     - Justification
> +     - Notes
> +
> +   * - D4.3
> +     - Accepted for the ARM64 codebase
> +     - Tagged as `disapplied` for ECLAIR on any other violation report.

This mean it has been applied for ARM64 but not x86, right?


> +   * - D4.3
> +     - The inline asm in 'xen/arch/arm/arm64/lib/bitops.c' is tightly coupled
> +       with the surronding C code that acts as a wrapper, so it has been decided
> +       not to add an additional encapsulation layer.
> +     - Tagged as `deliberate` for ECLAIR.
> +
> +Deviations related to MISRA C:2012 Rules:
> +-----------------------------------------
> +
> +.. list-table::
> +   :header-rows: 1
> +
> +   * - Rule identifier
> +     - Justification
> +     - Notes
> +
> +   * - R2.1
> +     - The compiler implementation guarantees that the unreachable code is
> +       removed. Constant expressions and unreachable branches of if and switch
> +       statements are expected.
> +     - Tagged as `safe` for ECLAIR.
> +
> +   * - R2.1
> +     - Some functions are intended not to be referenced.
> +     - Tagged as `deliberate` for ECLAIR.

What does it mean "some functions" in this case? Should we list which
functions?

Other than this, I checked and everything else looks great

Re: [XEN PATCH][for-4.19 v2 1/2] automation/eclair: update deviations and accepted guidelines

[Stefano Stabellini]

On Mon, 9 Oct 2023, Nicola Vetrini wrote:
> From: Simone Ballarin <simone.ballarin@bugseng.com>
> 
> Remove deviations for ERROR_EXIT, ERROR_EXIT_DOM and PIN_FAIL:
> the aforementioned macros have been removed.
> Add deviation for Rule 2.1 for pure declarations.
> Remove legacy text-based deviations: these are now implemented
> with SAF comments.
> Add deviations for Rules 8.4, 10.1, 13.5, 14.2, 14.3.
> Remove deviations for guidelines not yet accepted or rejected.
> 
> Add MC3R1.R11.7, MC3R1.R11.8, MC3R1.R11.9, MC3R1.R15.3 and MC3R1.R14.2
> to the accepted guidelines selector.
> 
> Update clean guidelines selector.
> 
> Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>

Re: [XEN PATCH][for-4.19 v2 2/2] docs/misra: add deviations.rst to document additional deviations.

[Henry Wang]

Hi Stefano,

> On Oct 10, 2023, at 09:19, Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> +Henry

Thank you.

> 
> On Mon, 9 Oct 2023, Nicola Vetrini wrote:
>> This file contains the deviation that are not marked by
>> a deviation comment, as specified in
>> docs/misra/documenting-violations.rst.
>> 
>> Suggested-by: Stefano Stabellini <sstabellini@kernel.org>
>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> 
> This is great! Thank you so much!
> 
> I have a few questions below but even as-is it is way better than
> nothing. I think we should add this for 4.18

I am fine with that as this patch is purely doc changes.

Release-acked-by: Henry Wang <Henry.Wang@arm.com>

Kind regards,
Henry

> 
> 
>> ---
>> docs/index.rst            |   1 +
>> docs/misra/deviations.rst | 240 ++++++++++++++++++++++++++++++++++++++
>> docs/misra/rules.rst      |   2 +-
>> 3 files changed, 242 insertions(+), 1 deletion(-)
>> create mode 100644 docs/misra/deviations.rst
>> 
>> diff --git a/docs/index.rst b/docs/index.rst
>> index 2c47cfa999f2..f3f779f89ce5 100644
>> --- a/docs/index.rst
>> +++ b/docs/index.rst
>> @@ -63,6 +63,7 @@ Xen hypervisor code.
>>    :maxdepth: 2
>> 
>>    misra/rules
>> +   misra/deviations
>> 
>> 
>> Miscellanea
>> diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
>> new file mode 100644
>> index 000000000000..19743e34ce03
>> --- /dev/null
>> +++ b/docs/misra/deviations.rst
>> @@ -0,0 +1,240 @@
>> +.. SPDX-License-Identifier: CC-BY-4.0
>> +
>> +MISRA C deviations for Xen
>> +==========================
>> +
>> +The following is the list of MISRA C:2012 deviations for the Xen codebase that
>> +are not covered by a `SAF-x-safe` or `SAF-x-false-positive-<tool>` comment, as
>> +specified in docs/misra/documenting-violations.rst; the lack of
>> +such comments is usually due to the excessive clutter they would bring to the
>> +codebase or the impossibility to express such a deviation (e.g., if it's
>> +composed of several conditions).
>> +
>> +Deviations related to MISRA C:2012 Directives:
>> +----------------------------------------------
>> +
>> +.. list-table::
>> +   :header-rows: 1
>> +
>> +   * - Directive identifier
>> +     - Justification
>> +     - Notes
>> +
>> +   * - D4.3
>> +     - Accepted for the ARM64 codebase
>> +     - Tagged as `disapplied` for ECLAIR on any other violation report.
> 
> This mean it has been applied for ARM64 but not x86, right?
> 
> 
>> +   * - D4.3
>> +     - The inline asm in 'xen/arch/arm/arm64/lib/bitops.c' is tightly coupled
>> +       with the surronding C code that acts as a wrapper, so it has been decided
>> +       not to add an additional encapsulation layer.
>> +     - Tagged as `deliberate` for ECLAIR.
>> +
>> +Deviations related to MISRA C:2012 Rules:
>> +-----------------------------------------
>> +
>> +.. list-table::
>> +   :header-rows: 1
>> +
>> +   * - Rule identifier
>> +     - Justification
>> +     - Notes
>> +
>> +   * - R2.1
>> +     - The compiler implementation guarantees that the unreachable code is
>> +       removed. Constant expressions and unreachable branches of if and switch
>> +       statements are expected.
>> +     - Tagged as `safe` for ECLAIR.
>> +
>> +   * - R2.1
>> +     - Some functions are intended not to be referenced.
>> +     - Tagged as `deliberate` for ECLAIR.
> 
> What does it mean "some functions" in this case? Should we list which
> functions?
> 
> Other than this, I checked and everything else looks great
> 

Re: [XEN PATCH][for-4.19 v2 2/2] docs/misra: add deviations.rst to document additional deviations.

[Nicola Vetrini]

On 10/10/2023 03:19, Stefano Stabellini wrote:
> +Henry
> 
> On Mon, 9 Oct 2023, Nicola Vetrini wrote:
>> This file contains the deviation that are not marked by
>> a deviation comment, as specified in
>> docs/misra/documenting-violations.rst.
>> 
>> Suggested-by: Stefano Stabellini <sstabellini@kernel.org>
>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> 
> This is great! Thank you so much!
> 
> I have a few questions below but even as-is it is way better than
> nothing. I think we should add this for 4.18
> 
> 
>> ---
>>  docs/index.rst            |   1 +
>>  docs/misra/deviations.rst | 240 
>> ++++++++++++++++++++++++++++++++++++++
>>  docs/misra/rules.rst      |   2 +-
>>  3 files changed, 242 insertions(+), 1 deletion(-)
>>  create mode 100644 docs/misra/deviations.rst
>> 
>> diff --git a/docs/index.rst b/docs/index.rst
>> index 2c47cfa999f2..f3f779f89ce5 100644
>> --- a/docs/index.rst
>> +++ b/docs/index.rst
>> @@ -63,6 +63,7 @@ Xen hypervisor code.
>>     :maxdepth: 2
>> 
>>     misra/rules
>> +   misra/deviations
>> 
>> 
>>  Miscellanea
>> diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
>> new file mode 100644
>> index 000000000000..19743e34ce03
>> --- /dev/null
>> +++ b/docs/misra/deviations.rst
>> @@ -0,0 +1,240 @@
>> +.. SPDX-License-Identifier: CC-BY-4.0
>> +
>> +MISRA C deviations for Xen
>> +==========================
>> +
>> +The following is the list of MISRA C:2012 deviations for the Xen 
>> codebase that
>> +are not covered by a `SAF-x-safe` or `SAF-x-false-positive-<tool>` 
>> comment, as
>> +specified in docs/misra/documenting-violations.rst; the lack of
>> +such comments is usually due to the excessive clutter they would 
>> bring to the
>> +codebase or the impossibility to express such a deviation (e.g., if 
>> it's
>> +composed of several conditions).
>> +
>> +Deviations related to MISRA C:2012 Directives:
>> +----------------------------------------------
>> +
>> +.. list-table::
>> +   :header-rows: 1
>> +
>> +   * - Directive identifier
>> +     - Justification
>> +     - Notes
>> +
>> +   * - D4.3
>> +     - Accepted for the ARM64 codebase
>> +     - Tagged as `disapplied` for ECLAIR on any other violation 
>> report.
> 
> This mean it has been applied for ARM64 but not x86, right?
> 
> 

Yes.

>> +   * - D4.3
>> +     - The inline asm in 'xen/arch/arm/arm64/lib/bitops.c' is tightly 
>> coupled
>> +       with the surronding C code that acts as a wrapper, so it has 
>> been decided
>> +       not to add an additional encapsulation layer.
>> +     - Tagged as `deliberate` for ECLAIR.
>> +
>> +Deviations related to MISRA C:2012 Rules:
>> +-----------------------------------------
>> +
>> +.. list-table::
>> +   :header-rows: 1
>> +
>> +   * - Rule identifier
>> +     - Justification
>> +     - Notes
>> +
>> +   * - R2.1
>> +     - The compiler implementation guarantees that the unreachable 
>> code is
>> +       removed. Constant expressions and unreachable branches of if 
>> and switch
>> +       statements are expected.
>> +     - Tagged as `safe` for ECLAIR.
>> +
>> +   * - R2.1
>> +     - Some functions are intended not to be referenced.
>> +     - Tagged as `deliberate` for ECLAIR.
> 
> What does it mean "some functions" in this case? Should we list which
> functions?
> 

Well, there are a lot, typically resulting from build configurations 
that do not
use them, or because they are used only in asm code. I can mention these 
reasons in the
document, to make it easier to understand.

> Other than this, I checked and everything else looks great

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)

Re: [XEN PATCH][for-4.19 v2 2/2] docs/misra: add deviations.rst to document additional deviations.

[Stefano Stabellini]

On Tue, 10 Oct 2023, Nicola Vetrini wrote:
> On 10/10/2023 03:19, Stefano Stabellini wrote:
> > +Henry
> > 
> > On Mon, 9 Oct 2023, Nicola Vetrini wrote:
> > > This file contains the deviation that are not marked by
> > > a deviation comment, as specified in
> > > docs/misra/documenting-violations.rst.
> > > 
> > > Suggested-by: Stefano Stabellini <sstabellini@kernel.org>
> > > Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> > 
> > This is great! Thank you so much!
> > 
> > I have a few questions below but even as-is it is way better than
> > nothing. I think we should add this for 4.18
> > 
> > 
> > > ---
> > >  docs/index.rst            |   1 +
> > >  docs/misra/deviations.rst | 240 ++++++++++++++++++++++++++++++++++++++
> > >  docs/misra/rules.rst      |   2 +-
> > >  3 files changed, 242 insertions(+), 1 deletion(-)
> > >  create mode 100644 docs/misra/deviations.rst
> > > 
> > > diff --git a/docs/index.rst b/docs/index.rst
> > > index 2c47cfa999f2..f3f779f89ce5 100644
> > > --- a/docs/index.rst
> > > +++ b/docs/index.rst
> > > @@ -63,6 +63,7 @@ Xen hypervisor code.
> > >     :maxdepth: 2
> > > 
> > >     misra/rules
> > > +   misra/deviations
> > > 
> > > 
> > >  Miscellanea
> > > diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
> > > new file mode 100644
> > > index 000000000000..19743e34ce03
> > > --- /dev/null
> > > +++ b/docs/misra/deviations.rst
> > > @@ -0,0 +1,240 @@
> > > +.. SPDX-License-Identifier: CC-BY-4.0
> > > +
> > > +MISRA C deviations for Xen
> > > +==========================
> > > +
> > > +The following is the list of MISRA C:2012 deviations for the Xen codebase
> > > that
> > > +are not covered by a `SAF-x-safe` or `SAF-x-false-positive-<tool>`
> > > comment, as
> > > +specified in docs/misra/documenting-violations.rst; the lack of
> > > +such comments is usually due to the excessive clutter they would bring to
> > > the
> > > +codebase or the impossibility to express such a deviation (e.g., if it's
> > > +composed of several conditions).
> > > +
> > > +Deviations related to MISRA C:2012 Directives:
> > > +----------------------------------------------
> > > +
> > > +.. list-table::
> > > +   :header-rows: 1
> > > +
> > > +   * - Directive identifier
> > > +     - Justification
> > > +     - Notes
> > > +
> > > +   * - D4.3
> > > +     - Accepted for the ARM64 codebase
> > > +     - Tagged as `disapplied` for ECLAIR on any other violation report.
> > 
> > This mean it has been applied for ARM64 but not x86, right?
> > 
> > 
> 
> Yes.
> 
> > > +   * - D4.3
> > > +     - The inline asm in 'xen/arch/arm/arm64/lib/bitops.c' is tightly
> > > coupled
> > > +       with the surronding C code that acts as a wrapper, so it has been
> > > decided
> > > +       not to add an additional encapsulation layer.
> > > +     - Tagged as `deliberate` for ECLAIR.
> > > +
> > > +Deviations related to MISRA C:2012 Rules:
> > > +-----------------------------------------
> > > +
> > > +.. list-table::
> > > +   :header-rows: 1
> > > +
> > > +   * - Rule identifier
> > > +     - Justification
> > > +     - Notes
> > > +
> > > +   * - R2.1
> > > +     - The compiler implementation guarantees that the unreachable code
> > > is
> > > +       removed. Constant expressions and unreachable branches of if and
> > > switch
> > > +       statements are expected.
> > > +     - Tagged as `safe` for ECLAIR.
> > > +
> > > +   * - R2.1
> > > +     - Some functions are intended not to be referenced.
> > > +     - Tagged as `deliberate` for ECLAIR.
> > 
> > What does it mean "some functions" in this case? Should we list which
> > functions?
> > 
> 
> Well, there are a lot, typically resulting from build configurations that do
> not
> use them, or because they are used only in asm code. I can mention these
> reasons in the
> document, to make it easier to understand.

Yes, I think we need to clarify further this point, because saying "Some
functions" doesn't help the reader understand:
- whether all functions can be not referenced
- which subset of functions can be not referenced

How to distinguish between? How do we know whether a certain patch is
violating the rule or not?

If there is a clear list of functions that can be not referenced, then
we should list them here. If there is a methodology we can use to
distinguish between them (e.g. functions called from asm only) then we
can write the methodology here. Either way it is fine as long as the
criteria to know if it is OK if a function is not referenced is clear.

Re: [XEN PATCH][for-4.19 v2 2/2] docs/misra: add deviations.rst to document additional deviations.

[Julien Grall]

Hi,

On 10/10/2023 23:27, Stefano Stabellini wrote:
> On Tue, 10 Oct 2023, Nicola Vetrini wrote:
>> On 10/10/2023 03:19, Stefano Stabellini wrote:
>>> +Henry
>>>
>>> On Mon, 9 Oct 2023, Nicola Vetrini wrote:
>>>> This file contains the deviation that are not marked by
>>>> a deviation comment, as specified in
>>>> docs/misra/documenting-violations.rst.
>>>>
>>>> Suggested-by: Stefano Stabellini <sstabellini@kernel.org>
>>>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
>>>
>>> This is great! Thank you so much!
>>>
>>> I have a few questions below but even as-is it is way better than
>>> nothing. I think we should add this for 4.18
>>>
>>>
>>>> ---
>>>>   docs/index.rst            |   1 +
>>>>   docs/misra/deviations.rst | 240 ++++++++++++++++++++++++++++++++++++++
>>>>   docs/misra/rules.rst      |   2 +-
>>>>   3 files changed, 242 insertions(+), 1 deletion(-)
>>>>   create mode 100644 docs/misra/deviations.rst
>>>>
>>>> diff --git a/docs/index.rst b/docs/index.rst
>>>> index 2c47cfa999f2..f3f779f89ce5 100644
>>>> --- a/docs/index.rst
>>>> +++ b/docs/index.rst
>>>> @@ -63,6 +63,7 @@ Xen hypervisor code.
>>>>      :maxdepth: 2
>>>>
>>>>      misra/rules
>>>> +   misra/deviations
>>>>
>>>>
>>>>   Miscellanea
>>>> diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
>>>> new file mode 100644
>>>> index 000000000000..19743e34ce03
>>>> --- /dev/null
>>>> +++ b/docs/misra/deviations.rst
>>>> @@ -0,0 +1,240 @@
>>>> +.. SPDX-License-Identifier: CC-BY-4.0
>>>> +
>>>> +MISRA C deviations for Xen
>>>> +==========================
>>>> +
>>>> +The following is the list of MISRA C:2012 deviations for the Xen codebase
>>>> that
>>>> +are not covered by a `SAF-x-safe` or `SAF-x-false-positive-<tool>`
>>>> comment, as
>>>> +specified in docs/misra/documenting-violations.rst; the lack of
>>>> +such comments is usually due to the excessive clutter they would bring to
>>>> the
>>>> +codebase or the impossibility to express such a deviation (e.g., if it's
>>>> +composed of several conditions).
>>>> +
>>>> +Deviations related to MISRA C:2012 Directives:
>>>> +----------------------------------------------
>>>> +
>>>> +.. list-table::
>>>> +   :header-rows: 1
>>>> +
>>>> +   * - Directive identifier
>>>> +     - Justification
>>>> +     - Notes
>>>> +
>>>> +   * - D4.3
>>>> +     - Accepted for the ARM64 codebase
>>>> +     - Tagged as `disapplied` for ECLAIR on any other violation report.
>>>
>>> This mean it has been applied for ARM64 but not x86, right?
>>>
>>>
>>
>> Yes.
>>
>>>> +   * - D4.3
>>>> +     - The inline asm in 'xen/arch/arm/arm64/lib/bitops.c' is tightly
>>>> coupled
>>>> +       with the surronding C code that acts as a wrapper, so it has been
>>>> decided
>>>> +       not to add an additional encapsulation layer.
>>>> +     - Tagged as `deliberate` for ECLAIR.
>>>> +
>>>> +Deviations related to MISRA C:2012 Rules:
>>>> +-----------------------------------------
>>>> +
>>>> +.. list-table::
>>>> +   :header-rows: 1
>>>> +
>>>> +   * - Rule identifier
>>>> +     - Justification
>>>> +     - Notes
>>>> +
>>>> +   * - R2.1
>>>> +     - The compiler implementation guarantees that the unreachable code
>>>> is
>>>> +       removed. Constant expressions and unreachable branches of if and
>>>> switch
>>>> +       statements are expected.
>>>> +     - Tagged as `safe` for ECLAIR.
>>>> +
>>>> +   * - R2.1
>>>> +     - Some functions are intended not to be referenced.
>>>> +     - Tagged as `deliberate` for ECLAIR.
>>>
>>> What does it mean "some functions" in this case? Should we list which
>>> functions?
>>>
>>
>> Well, there are a lot, typically resulting from build configurations that do
>> not
>> use them, or because they are used only in asm code. I can mention these
>> reasons in the
>> document, to make it easier to understand.
> 
> Yes, I think we need to clarify further this point, because saying "Some
> functions" doesn't help the reader understand:
> - whether all functions can be not referenced
> - which subset of functions can be not referenced
> 
> How to distinguish between? How do we know whether a certain patch is
> violating the rule or not?
> 
> If there is a clear list of functions that can be not referenced, then
> we should list them here. If there is a methodology we can use to
> distinguish between them (e.g. functions called from asm only) then we
> can write the methodology here. Either way it is fine as long as the
> criteria to know if it is OK if a function is not referenced is clear.

Aren't they more or less the one we tagged with SAF-1-safe because there 
were no prototype? If so, we could use the same tags.

We could introduce an extra tags for the others. An alternative would be 
to add an attribute (e.g. asmcall) to mark each function used by assembly.

Cheers,

-- 
Julien Grall

Re: [XEN PATCH][for-4.19 v2 2/2] docs/misra: add deviations.rst to document additional deviations.

[Nicola Vetrini]

>>>>> +
>>>>> +   * - R2.1
>>>>> +     - The compiler implementation guarantees that the unreachable 
>>>>> code
>>>>> is
>>>>> +       removed. Constant expressions and unreachable branches of 
>>>>> if and
>>>>> switch
>>>>> +       statements are expected.
>>>>> +     - Tagged as `safe` for ECLAIR.
>>>>> +
>>>>> +   * - R2.1
>>>>> +     - Some functions are intended not to be referenced.
>>>>> +     - Tagged as `deliberate` for ECLAIR.
>>>> 
>>>> What does it mean "some functions" in this case? Should we list 
>>>> which
>>>> functions?
>>>> 
>>> 
>>> Well, there are a lot, typically resulting from build configurations 
>>> that do
>>> not
>>> use them, or because they are used only in asm code. I can mention 
>>> these
>>> reasons in the
>>> document, to make it easier to understand.
>> 
>> Yes, I think we need to clarify further this point, because saying 
>> "Some
>> functions" doesn't help the reader understand:
>> - whether all functions can be not referenced
>> - which subset of functions can be not referenced
>> 
>> How to distinguish between? How do we know whether a certain patch is
>> violating the rule or not?
>> 
>> If there is a clear list of functions that can be not referenced, then
>> we should list them here. If there is a methodology we can use to
>> distinguish between them (e.g. functions called from asm only) then we
>> can write the methodology here. Either way it is fine as long as the
>> criteria to know if it is OK if a function is not referenced is clear.
> 
> Aren't they more or less the one we tagged with SAF-1-safe because
> there were no prototype? If so, we could use the same tags.
> 
> We could introduce an extra tags for the others. An alternative would
> be to add an attribute (e.g. asmcall) to mark each function used by
> assembly.
> 
> Cheers,

Both suggestion do have some value. As it is, it's not distinguishable 
what causes a
function to be unreferenced in a certain analysis config. However:

- functions only used by asm code can be specified in the ECLAIR config 
so that they will
   have an extra fake reference as far as the checker is concerned. I can 
do that on a
   separate patch and list them in deviations.rst. An attribute seems a 
good way to signal the
   intention.
- Functions that have no reference only in the current analysis should 
have their declaration
   #ifdef-ed out in the configurations where they are not used, in an 
ideal world.
- Truly unreferenced functions should be removed, or justified

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)

Re: [XEN PATCH][for-4.19 v2 2/2] docs/misra: add deviations.rst to document additional deviations.

[Nicola Vetrini]

On 11/10/2023 17:00, Nicola Vetrini wrote:
>>>>>> +
>>>>>> +   * - R2.1
>>>>>> +     - The compiler implementation guarantees that the 
>>>>>> unreachable code
>>>>>> is
>>>>>> +       removed. Constant expressions and unreachable branches of 
>>>>>> if and
>>>>>> switch
>>>>>> +       statements are expected.
>>>>>> +     - Tagged as `safe` for ECLAIR.
>>>>>> +
>>>>>> +   * - R2.1
>>>>>> +     - Some functions are intended not to be referenced.
>>>>>> +     - Tagged as `deliberate` for ECLAIR.
>>>>> 
>>>>> What does it mean "some functions" in this case? Should we list 
>>>>> which
>>>>> functions?
>>>>> 
>>>> 
>>>> Well, there are a lot, typically resulting from build configurations 
>>>> that do
>>>> not
>>>> use them, or because they are used only in asm code. I can mention 
>>>> these
>>>> reasons in the
>>>> document, to make it easier to understand.
>>> 
>>> Yes, I think we need to clarify further this point, because saying 
>>> "Some
>>> functions" doesn't help the reader understand:
>>> - whether all functions can be not referenced
>>> - which subset of functions can be not referenced
>>> 
>>> How to distinguish between? How do we know whether a certain patch is
>>> violating the rule or not?
>>> 
>>> If there is a clear list of functions that can be not referenced, 
>>> then
>>> we should list them here. If there is a methodology we can use to
>>> distinguish between them (e.g. functions called from asm only) then 
>>> we
>>> can write the methodology here. Either way it is fine as long as the
>>> criteria to know if it is OK if a function is not referenced is 
>>> clear.
>> 
>> Aren't they more or less the one we tagged with SAF-1-safe because
>> there were no prototype? If so, we could use the same tags.
>> 
>> We could introduce an extra tags for the others. An alternative would
>> be to add an attribute (e.g. asmcall) to mark each function used by
>> assembly.
>> 
>> Cheers,
> 
> Both suggestion do have some value. As it is, it's not distinguishable
> what causes a
> function to be unreferenced in a certain analysis config. However:
> 
> - functions only used by asm code can be specified in the ECLAIR
> config so that they will
>   have an extra fake reference as far as the checker is concerned. I
> can do that on a
>   separate patch and list them in deviations.rst. An attribute seems a
> good way to signal the
>   intention.
> - Functions that have no reference only in the current analysis should
> have their declaration
>   #ifdef-ed out in the configurations where they are not used, in an
> ideal world.
> - Truly unreferenced functions should be removed, or justified

Especially the last two appear somewhat tricky to disentangle, as they 
do require knowledge of
possible code paths.

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)

Re: [XEN PATCH][for-4.19 v2 2/2] docs/misra: add deviations.rst to document additional deviations.

[Nicola Vetrini]

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

Attached is a list of all the unreferenced functions for x86. It's a bit 
rough and does not
distinguish the categories mentioned in my previous reply, but it's a 
starting point for any
further work on this.

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)

[-- Attachment #2: unref.txt --]
[-- Type: text/plain, Size: 26454 bytes --]

culprit: function `acpi_table_get_entry_madt(enum acpi_madt_type, unsigned)' is never referenced]
culprit: function `apei_check_mce(void)' is never referenced]
culprit: function `apei_clear_mce(u64)' is never referenced]
culprit: function `apei_read_mce(struct mce*, u64*)' is never referenced]
culprit: function `apic_icr_read(void)' (multiple units) is never referenced]
culprit: function `apic_write_atomic(unsigned long, u32)' (multiple units) is never referenced]
culprit: function `arch_get_nmi_reason(const struct domain*)' (multiple units) is never referenced]
culprit: function `arch_get_pfn_to_mfn_frame_list_list(const struct domain*)' (multiple units) is never referenced]
culprit: function `arch_set_cr2(struct vcpu*, unsigned long)' (multiple units) is never referenced]
culprit: function `arch_set_max_pfn(struct domain*, unsigned long)' (multiple units) is never referenced]
culprit: function `arch_set_nmi_reason(struct domain*, unsigned long)' (multiple units) is never referenced]
culprit: function `arch_set_pfn_to_mfn_frame_list_list(struct domain*, xen_pfn_t)' (multiple units) is never referenced]
culprit: function `asm_domain_crash_synchronous(unsigned long)' is never referenced]
culprit: function `atomic_add(int, atomic_t*)' (multiple units) is never referenced]
culprit: function `atomic_add_negative(int, atomic_t*)' (multiple units) is never referenced]
culprit: function `atomic_add_unless(atomic_t*, int, int)' (multiple units) is never referenced]
culprit: function `atomic_inc_and_test(atomic_t*)' (multiple units) is never referenced]
culprit: function `atomic_inc_return(atomic_t*)' (multiple units) is never referenced]
culprit: function `atomic_sub_and_test(int, atomic_t*)' (multiple units) is never referenced]
culprit: function `bsearch(const void*, const void*, size_t, size_t, int(*)(const void*, const void*))' is never referenced]
culprit: function `change_bit(int, volatile void*)' (multiple units) is never referenced]
culprit: function `check_for_unexpected_msi(unsigned)' is never referenced]
culprit: function `clear_node_cpumask(unsigned)' (multiple units) is never referenced]
culprit: function `cmdline_parse_early(const char*, early_boot_opts_t*)' is never referenced]
culprit: function `console_input_domain(void)' is never referenced]
culprit: function `constant_change_bit(int, void*)' (multiple units) is never referenced]
culprit: function `copy_to_unsafe(void*, const void*, unsigned)' (multiple units) is never referenced]
culprit: function `cpumask_full(const cpumask_t*)' (multiple units) is never referenced]
culprit: function `__cpumask_test_and_set_cpu(int, cpumask_t*)' (multiple units) is never referenced]
culprit: function `cpumask_xor(cpumask_t*, const cpumask_t*, const cpumask_t*)' (multiple units) is never referenced]
culprit: function `__cpu_to_be16p(const __u16*)' (multiple units) is never referenced]
culprit: function `__cpu_to_be32p(const __u32*)' (multiple units) is never referenced]
culprit: function `__cpu_to_be64p(const __u64*)' (multiple units) is never referenced]
culprit: function `__cpu_to_le16p(const __u16*)' (multiple units) is never referenced]
culprit: function `__cpu_to_le32p(const __u32*)' (multiple units) is never referenced]
culprit: function `__cpu_to_le64p(const __u64*)' (multiple units) is never referenced]
culprit: function `cr3_pcid(unsigned long)' (multiple units) is never referenced]
culprit: function `do_debug(struct cpu_user_regs*)' is never referenced]
culprit: function `do_device_not_available(struct cpu_user_regs*)' is never referenced]
culprit: function `do_double_fault(struct cpu_user_regs*)' is never referenced]
culprit: function `do_early_page_fault(struct cpu_user_regs*)' is never referenced]
culprit: function `do_entry_CP(struct cpu_user_regs*)' is never referenced]
culprit: function `do_general_protection(struct cpu_user_regs*)' is never referenced]
culprit: function `do_int3(struct cpu_user_regs*)' is never referenced]
culprit: function `do_invalid_op(struct cpu_user_regs*)' is never referenced]
culprit: function `domain_has_ioreq_server(const struct domain*)' is never referenced]
culprit: function `domain_set_alloc_bitsize(struct domain*)' is never referenced]
culprit: function `do_mca(__guest_handle_xen_mc_t)' is never referenced]
culprit: function `do_page_fault(struct cpu_user_regs*)' is never referenced]
culprit: function `do_trap(struct cpu_user_regs*)' is never referenced]
culprit: function `do_unhandled_trap(struct cpu_user_regs*)' is never referenced]
culprit: function `efi_halt_system(void)' is never referenced]
culprit: function `efi_multiboot2(EFI_HANDLE, EFI_SYSTEM_TABLE*, const char*)' is never referenced]
culprit: function `efi_start(EFI_HANDLE, EFI_SYSTEM_TABLE*)' is never referenced]
culprit: function `ELF_ADVANCE_DEST(struct elf_binary*, uint64_t)' (multiple units) is never referenced]
culprit: function `elf_lookup_addr(struct elf_binary*, const char*)' is never referenced]
culprit: function `elf_set_verbose(struct elf_binary*)' is never referenced]
culprit: function `elf_sym_by_index(struct elf_binary*, unsigned)' is never referenced]
culprit: function `elf_xen_feature_get(int, uint32_t*)' (multiple units) is never referenced]
culprit: function `ERR_CAST(const void*)' (multiple units) is never referenced]
culprit: function `erst_get_next_record_id(u64*)' is never referenced]
culprit: function `erst_read(u64, struct cper_record_header*, size_t)' is never referenced]
culprit: function `fill_ro_mpt(mfn_t)' is never referenced]
culprit: function `generic_ffsl(unsigned long)' (multiple units) is never referenced]
culprit: function `generic_flsl(unsigned long)' (multiple units) is never referenced]
culprit: function `get_bitmask_order(unsigned)' (multiple units) is never referenced]
culprit: function `get_msr_xss(void)' is never referenced]
culprit: function `get_page_from_l1e(l1_pgentry_t, struct domain*, struct domain*)' is never referenced]
culprit: function `get_page_type_preemptible(struct page_info*, unsigned long)' is never referenced]
culprit: function `get_pat_flags(struct vcpu*, uint32_t, paddr_t, paddr_t, uint8_t)' is never referenced]
culprit: function `get_pg_owner(domid_t)' is never referenced]
culprit: function `gfn_min(gfn_t, gfn_t)' (multiple units) is never referenced]
culprit: function `gfn_to_paddr(gfn_t)' (multiple units) is never referenced]
culprit: function `guest_iommu_add_event_log(struct domain*, u32
culprit: function `guest_iommu_destroy(struct domain*)' is never referenced]
culprit: function `guest_iommu_init(struct domain*)' is never referenced]
culprit: function `guest_iommu_set_base(struct domain*, uint64_t)' is never referenced]
culprit: function `guest_l1e_from_gfn(gfn_t, u32)' (multiple units) is never referenced]
culprit: function `guest_l1e_get_pkey(guest_l1e_t)' (multiple units) is never referenced]
culprit: function `guest_l2e_from_gfn(gfn_t, u32)' (multiple units) is never referenced]
culprit: function `guest_l2e_get_pkey(guest_l2e_t)' (multiple units) is never referenced]
culprit: function `guest_l3e_from_gfn(gfn_t, u32)' (multiple units) is never referenced]
culprit: function `guest_l3e_get_pkey(guest_l3e_t)' (multiple units) is never referenced]
culprit: function `guest_l4e_from_gfn(gfn_t, u32)' (multiple units) is never referenced]
culprit: function `guest_walk_to_gpa(const walk_t*)' (multiple units) is never referenced]
culprit: function `hlist_add_after_rcu(struct hlist_node*, struct hlist_node*)' (multiple units) is never referenced]
culprit: function `hlist_add_after(struct hlist_node*, struct hlist_node*)' (multiple units) is never referenced]
culprit: function `hlist_add_before_rcu(struct hlist_node*, struct hlist_node*)' (multiple units) is never referenced]
culprit: function `hlist_add_before(struct hlist_node*, struct hlist_node*)' (multiple units) is never referenced]
culprit: function `hlist_add_head_rcu(struct hlist_node*, struct hlist_head*)' (multiple units) is never referenced]
culprit: function `hlist_add_head(struct hlist_node*, struct hlist_head*)' (multiple units) is never referenced]
culprit: function `hlist_del_init(struct hlist_node*)' (multiple units) is never referenced]
culprit: function `hlist_del_rcu(struct hlist_node*)' (multiple units) is never referenced]
culprit: function `hlist_del(struct hlist_node*)' (multiple units) is never referenced]
culprit: function `hlist_empty(const struct hlist_head*)' (multiple units) is never referenced]
culprit: function `hlist_replace_rcu(struct hlist_node*, struct hlist_node*)' (multiple units) is never referenced]
culprit: function `hvm_copy_context_and_params(struct domain*, struct domain*)' is never referenced]
culprit: function `hvm_get_nonreg_state(struct vcpu*, struct hvm_vcpu_nonreg_state*)' (multiple units) is never referenced]
culprit: function `hvm_set_nonreg_state(struct vcpu*, struct hvm_vcpu_nonreg_state*)' (multiple units) is never referenced]
culprit: function `hweight_long(unsigned long)' (multiple units) is never referenced]
culprit: function `hypervisor_resume(void)' (multiple units) is never referenced]
culprit: function `hyperv_probe(void)' (multiple units) is never referenced]
culprit: function `in_atomic(void)' is never referenced]
culprit: function `inl_p(unsigned short)' (multiple units) is never referenced]
culprit: function `invpcid_flush_single_context(unsigned)' (multiple units) is never referenced]
culprit: function `inw_p(unsigned short)' (multiple units) is never referenced]
culprit: function `io_apic_modify(unsigned, unsigned, unsigned)' (multiple units) is never referenced]
culprit: function `iommu_add_extra_reserved_device_memory(unsigned long, unsigned long, pci_sbdf_t)' is never referenced]
culprit: function `iommu_crash_shutdown(void)' is never referenced]
culprit: function `iommu_dev_iotlb_flush_timeout(struct domain*, struct pci_dev*)' is never referenced]
culprit: function `iommu_has_feature(struct domain*, enum iommu_feature)' is never referenced]
culprit: function `iommu_lookup_page(struct domain*, dfn_t, mfn_t*, unsigned*)' is never referenced]
culprit: function `is_endbr64_poison(const void*)' (multiple units) is never referenced]
culprit: function `is_ioreq_server_page(struct domain*, const struct page_info*)' is never referenced]
culprit: function `is_l1tf_safe_maddr(intpte_t)' (multiple units) is never referenced]
culprit: function `is_pv_64bit_vcpu(const struct vcpu*)' (multiple units) is never referenced]
culprit: function `kexec_crash_save_cpu(void)' (multiple units) is never referenced]
culprit: function `l1e_from_paddr(paddr_t, unsigned)' (multiple units) is never referenced]
culprit: function `list_empty_careful(const struct list_head*)' (multiple units) is never referenced]
culprit: function `list_is_last(const struct list_head*, const struct list_head*)' (multiple units) is never referenced]
culprit: function `list_is_singular(const struct list_head*)' (multiple units) is never referenced]
culprit: function `list_move_tail(struct list_head*, struct list_head*)' (multiple units) is never referenced]
culprit: function `list_replace_init(struct list_head*, struct list_head*)' (multiple units) is never referenced]
culprit: function `list_replace_rcu(struct list_head*, struct list_head*)' (multiple units) is never referenced]
culprit: function `mapcache_override_current(struct vcpu*)' is never referenced]
culprit: function `mce_barrier_dec(struct mce_softirq_barrier*)' is never referenced]
culprit: function `mce_barrier_init(struct mce_softirq_barrier*)' is never referenced]
culprit: function `mce_barrier(struct mce_softirq_barrier*)' is never referenced]
culprit: function `memchr(const void*, int, size_t)' is never referenced]
culprit: function `memcmp(const void*, const void*, size_t)' is never referenced]
culprit: function `memmove(void*, const void*, size_t)' is never referenced]
culprit: function `memset(void*, int, size_t)' is never referenced]
culprit: function `mem_sharing_fork_reset(struct domain*, _Bool, _Bool)' (multiple units) is never referenced]
culprit: function `mfn_max(mfn_t, mfn_t)' (multiple units) is never referenced]
culprit: function `mm_enforce_order_lock_post_per_page_sharing(const struct domain*, int*, unsigned short*)' (multiple units) is never referenced]
culprit: function `mm_enforce_order_lock_pre_per_page_sharing(const struct domain*)' (multiple units) is never referenced]
culprit: function `mm_read_lock_altp2m(const struct domain*, mm_rwlock_t*)' (multiple units) is never referenced]
culprit: function `mtrr_add(unsigned long, unsigned long, unsigned, char)' is never referenced]
culprit: function `mtrr_bp_restore(void)' is never referenced]
culprit: function `mtrr_del(int, unsigned long, unsigned long)' is never referenced]
culprit: function `__nodes_complement(nodemask_t*, const nodemask_t*, int)' (multiple units) is never referenced]
culprit: function `__nodes_equal(const nodemask_t*, const nodemask_t*, int)' (multiple units) is never referenced]
culprit: function `__nodes_subset(const nodemask_t*, const nodemask_t*, int)' (multiple units) is never referenced]
culprit: function `__nodes_xor(nodemask_t*, const nodemask_t*, const nodemask_t*, int)' (multiple units) is never referenced]
culprit: function `notifier_chain_unregister(struct notifier_head*, struct notifier_block*)' is never referenced]
culprit: function `nsvm_vcpu_switch(void)' is never referenced]
culprit: function `outl_p(unsigned, unsigned short)' (multiple units) is never referenced]
culprit: function `outw_p(unsigned short, unsigned short)' (multiple units) is never referenced]
culprit: function `page_list_first(const struct page_list_head*)' (multiple units) is never referenced]
culprit: function `page_list_prev(const struct page_info*, const struct page_list_head*)' (multiple units) is never referenced]
culprit: function `page_lock(struct page_info*)' is never referenced]
culprit: function `page_unlock(struct page_info*)' is never referenced]
culprit: function `paging_get_mode(struct vcpu*)' is never referenced]
culprit: function `parse_signed_integer(const char*, const char*, const char*, long long*)' is never referenced]
culprit: function `pcie_aer_get_firmware_first(const struct pci_dev*)' is never referenced]
culprit: function `pci_find_next_cap(u16, u8, unsigned, u8, int)' is never referenced]
culprit: function `pci_known_segment(u16)' is never referenced]
culprit: function `pcpu_schedule_lock(unsigned)' (multiple units) is never referenced]
culprit: function `pcpu_schedule_unlock(spinlock_t*, unsigned)' (multiple units) is never referenced]
culprit: function `print_gw(const walk_t*)' (multiple units) is never referenced]
culprit: function `PTR_RET(const void*)' (multiple units) is never referenced]
culprit: function `put_page_from_l1e(l1_pgentry_t, struct domain*)' is never referenced]
culprit: function `put_pg_owner(struct domain*)' (multiple units) is never referenced]
culprit: function `put_unaligned_be16(uint16_t, void*)' (multiple units) is never referenced]
culprit: function `pv_console_rx(struct cpu_user_regs*)' (unit `xen/drivers/char/console.c' with target `xen/drivers/char/console.o') is never referenced]
culprit: function `pv_emulate_gate_op(struct cpu_user_regs*)' (multiple units) is never referenced]
culprit: function `pv_emulate_privileged_op(struct cpu_user_regs*)' (multiple units) is never referenced]
culprit: function `pv_inject_sw_interrupt(unsigned)' (multiple units) is never referenced]
culprit: function `pv_map_ldt_shadow_page(unsigned)' (multiple units) is never referenced]
culprit: function `pv_pit_handler(int, int, int)' is never referenced]
culprit: function `pv_set_gdt(struct vcpu*, const unsigned long
culprit: function `pv_shim_fixup_e820(void)' (multiple units) is never referenced]
culprit: function `pv_shim_inject_evtchn(unsigned)' (multiple units) is never referenced]
culprit: function `pv_shim_setup_dom(struct domain*, l4_pgentry_t*, unsigned long, unsigned long, unsigned long, unsigned long, start_info_t*)' (multiple units) is never referenced]
culprit: function `pv_soft_rdtsc(const struct vcpu*, const struct cpu_user_regs*)' is never referenced]
culprit: function `rangeset_claim_range(struct rangeset*, unsigned long, unsigned long*)' is never referenced]
culprit: function `rangeset_swap(struct rangeset*, struct rangeset*)' is never referenced]
culprit: function `_read_lock_irq(rwlock_t*)' (multiple units) is never referenced]
culprit: function `_read_unlock_irq(rwlock_t*)' (multiple units) is never referenced]
culprit: function `region_to_pages(unsigned long, unsigned long)' (multiple units) is never referenced]
culprit: function `release_irq(unsigned, const void*)' is never referenced]
culprit: function `release_lapic_nmi(void)' is never referenced]
culprit: function `reloc(uint32_t, uint32_t, uint32_t, uint32_t)' is never referenced]
culprit: function `reserve_lapic_nmi(void)' is never referenced]
culprit: function `rol32(__u32, unsigned)' (multiple units) is never referenced]
culprit: function `ror32(__u32, unsigned)' (multiple units) is never referenced]
culprit: function `safe_copy_string_from_guest(__guest_handle_char, size_t, size_t)' is never referenced]
culprit: function `sched_get_id_by_name(const char*)' is never referenced]
culprit: function `sched_unit_pause_nosync(const struct sched_unit*)' (multiple units) is never referenced]
culprit: function `sched_unit_unpause(const struct sched_unit*)' (multiple units) is never referenced]
culprit: function `search_pre_exception_table(struct cpu_user_regs*)' is never referenced]
culprit: function `serial_getc(int)' is never referenced]
culprit: function `serial_putc(int, char)' is never referenced]
culprit: function `serial_vuart_info(int)' is never referenced]
culprit: function `__set_fixmap_x(enum fixed_addresses_x, unsigned long, unsigned long)' is never referenced]
culprit: function `shadow_remove_all_shadows(struct domain*, mfn_t)' (multiple units) is never referenced]
culprit: function `_spin_trylock_recursive(spinlock_t*)' is never referenced]
culprit: function `start_secondary(void)' is never referenced]
culprit: function `__start_xen(unsigned long)' is never referenced]
culprit: function `strcasecmp(const char*, const char*)' is never referenced]
culprit: function `strchr(const char*, int)' is never referenced]
culprit: function `strcmp(const char*, const char*)' is never referenced]
culprit: function `strlen(const char*)' is never referenced]
culprit: function `strncasecmp(const char*, const char*, size_t)' is never referenced]
culprit: function `strncmp(const char*, const char*, size_t)' is never referenced]
culprit: function `strrchr(const char*, int)' is never referenced]
culprit: function `strspn(const char*, const char*)' is never referenced]
culprit: function `strstr(const char*, const char*)' is never referenced]
culprit: function `svm_intr_assist(void)' is never referenced]
culprit: function `svm_vmenter_helper(void)' is never referenced]
culprit: function `svm_vmexit_handler(void)' is never referenced]
culprit: function `__swab16s(__u16*)' (multiple units) is never referenced]
culprit: function `__swab32s(__u32*)' (multiple units) is never referenced]
culprit: function `__swab64s(__u64*)' (multiple units) is never referenced]
culprit: function `symbols_lookup_by_name(const char*)' is never referenced]
culprit: function `tasklet_is_scheduled(const struct tasklet*)' (multiple units) is never referenced]
culprit: function `tboot_parse_dmar_table(acpi_table_handler)' (multiple units) is never referenced]
culprit: function `__test_and_change_bit(int, void*)' (multiple units) is never referenced]
culprit: function `test_and_change_bit(int, volatile void*)' (multiple units) is never referenced]
culprit: function `test(int)' is never referenced]
culprit: function `trace_ptwr_emulation(unsigned long, l1_pgentry_t)' (unit `xen/arch/x86/traps.c' with target `xen/arch/x86/traps.o') is never referenced]
culprit: function `trace_pv_page_fault(unsigned long, unsigned)' (unit `xen/arch/x86/traps.c' with target `xen/arch/x86/traps.o') is never referenced]
culprit: function `trace_pv_trap(int, unsigned long, int, unsigned)' (unit `xen/arch/x86/traps.c' with target `xen/arch/x86/traps.o') is never referenced]
culprit: function `trace_trap_one_addr(unsigned, unsigned long)' (unit `xen/arch/x86/traps.c' with target `xen/arch/x86/traps.o') is never referenced]
culprit: function `unregister_virtual_region(struct virtual_region*)' is never referenced]
culprit: function `unset_nmi_callback(void)' is never referenced]
culprit: function `update_cr3(struct vcpu*)' is never referenced]
culprit: function `_update_gate_addr_lower(idt_entry_t*, void*)' (multiple units) is never referenced]
culprit: function `variable_change_bit(int, void*)' (multiple units) is never referenced]
culprit: function `vmalloc_xen(size_t)' is never referenced]
culprit: function `vmcb_get_gmet(const struct vmcb_struct*)' (multiple units) is never referenced]
culprit: function `vmcb_get_msr_isst(const struct vmcb_struct*)' (multiple units) is never referenced]
culprit: function `vmcb_get_msr_s_cet(const struct vmcb_struct*)' (multiple units) is never referenced]
culprit: function `vmcb_get_np_enable(const struct vmcb_struct*)' (multiple units) is never referenced]
culprit: function `vmcb_get_pause_filter_count(const struct vmcb_struct*)' (multiple units) is never referenced]
culprit: function `vmcb_get_pause_filter_thresh(const struct vmcb_struct*)' (multiple units) is never referenced]
culprit: function `vmcb_get_sev_enable(const struct vmcb_struct*)' (multiple units) is never referenced]
culprit: function `vmcb_get_sev_es_enable(const struct vmcb_struct*)' (multiple units) is never referenced]
culprit: function `vmcb_get_ssp(const struct vmcb_struct*)' (multiple units) is never referenced]
culprit: function `vmcb_get_vte(const struct vmcb_struct*)' (multiple units) is never referenced]
culprit: function `vmcb_set_gmet(struct vmcb_struct*, _Bool)' (multiple units) is never referenced]
culprit: function `vmcb_set_iopm_base_pa(struct vmcb_struct*, __typeof__((u64)???))' (multiple units) is never referenced]
culprit: function `vmcb_set_msr_isst(struct vmcb_struct*, __typeof__((u64)???))' (multiple units) is never referenced]
culprit: function `vmcb_set_msrpm_base_pa(struct vmcb_struct*, __typeof__((u64)???))' (multiple units) is never referenced]
culprit: function `vmcb_set_msr_s_cet(struct vmcb_struct*, __typeof__((u64)???))' (multiple units) is never referenced]
culprit: function `vmcb_set_np_ctrl(struct vmcb_struct*, __typeof__((uint64_t)???))' (multiple units) is never referenced]
culprit: function `vmcb_set_pause_filter_count(struct vmcb_struct*, __typeof__((u16)???))' (multiple units) is never referenced]
culprit: function `vmcb_set_pause_filter_thresh(struct vmcb_struct*, __typeof__((u16)???))' (multiple units) is never referenced]
culprit: function `vmcb_set_sev_enable(struct vmcb_struct*, _Bool)' (multiple units) is never referenced]
culprit: function `vmcb_set_sev_es_enable(struct vmcb_struct*, _Bool)' (multiple units) is never referenced]
culprit: function `vmcb_set_ssp(struct vmcb_struct*, __typeof__((u64)???))' (multiple units) is never referenced]
culprit: function `vmcb_set_vte(struct vmcb_struct*, _Bool)' (multiple units) is never referenced]
culprit: function `vm_event_cancel_slot(struct domain*, struct vm_event_domain*)' is never referenced]
culprit: function `vm_event_claim_slot_nosleep(struct domain*, struct vm_event_domain*)' (multiple units) is never referenced]
culprit: function `vpci_remove_register(struct vpci*, unsigned, unsigned)' is never referenced]
culprit: function `vpmu_allocate_context(struct vcpu*)' (multiple units) is never referenced]
culprit: function `vscnprintf(char*, size_t, const char*, va_list)' is never referenced]
culprit: function `wake_up_one(struct waitqueue_head*)' is never referenced]
culprit: function `wbnoinvd(void)' (multiple units) is never referenced]
culprit: function `write_fs_base(unsigned long)' (multiple units) is never referenced]
culprit: function `write_gs_base(unsigned long)' (multiple units) is never referenced]
culprit: function `_write_lock_irq(rwlock_t*)' (multiple units) is never referenced]
culprit: function `_write_trylock(rwlock_t*)' (multiple units) is never referenced]
culprit: function `_write_unlock_irq(rwlock_t*)' (multiple units) is never referenced]
culprit: function `x86_insn_immediate(const struct x86_emulate_state*, unsigned)' is never referenced]
culprit: function `x86_insn_operand_ea(const struct x86_emulate_state*, enum x86_segment*)' is never referenced]
culprit: function `x86_insn_opsize(const struct x86_emulate_state*)' is never referenced]
culprit: function `xen_compile_host(void)' is never referenced]
culprit: function `xen_compile_time(void)' is never referenced]
culprit: function `xg_probe(void)' (multiple units) is never referenced]
culprit: function `xlat_start_info(struct start_info*, enum XLAT_start_info_console)' is never referenced]
culprit: function `xmem_pool_destroy(struct xmem_pool*)' is never referenced]
culprit: function `xmem_pool_get_total_size(struct xmem_pool*)' is never referenced]
culprit: function `xmem_pool_maxalloc(struct xmem_pool*)' is never referenced]
culprit: function `_xrealloc(void*, unsigned long, unsigned long)' is never referenced]
culprit: function `xsm_hypfs_op(xsm_default_t)' (multiple units) is never referenced]
culprit: function `xsm_init_hardware_domain(xsm_default_t, struct domain*)' (multiple units) is never referenced]
culprit: function `xsm_kexec(xsm_default_t)' (multiple units) is never referenced]
culprit: function `xsm_memory_pin_page(xsm_default_t, struct domain*, struct domain*, struct page_info*)' (multiple units) is never referenced]
culprit: function `xsm_mem_sharing_op(xsm_default_t, struct domain*, struct domain*, int)' (multiple units) is never referenced]
culprit: function `xsm_mmuext_op(xsm_default_t, struct domain*, struct domain*)' (multiple units) is never referenced]
culprit: function `xsm_mmu_update(xsm_default_t, struct domain*, struct domain*, struct domain*, uint32_t)' (multiple units) is never referenced]
culprit: function `xsm_profile(xsm_default_t, struct domain*, int)' (multiple units) is never referenced]
culprit: function `xsm_update_va_mapping(xsm_default_t, struct domain*, struct domain*, l1_pgentry_t)' (multiple units) is never referenced]
culprit: function `zap_ro_mpt(mfn_t)' is never referenced]

Re: [XEN PATCH][for-4.19 v2 2/2] docs/misra: add deviations.rst to document additional deviations.

[Stefano Stabellini]

On Wed, 11 Oct 2023, Nicola Vetrini wrote:
> On 11/10/2023 17:00, Nicola Vetrini wrote:
> > > > > > > +
> > > > > > > +   * - R2.1
> > > > > > > +     - The compiler implementation guarantees that the
> > > > > > > unreachable code
> > > > > > > is
> > > > > > > +       removed. Constant expressions and unreachable branches of
> > > > > > > if and
> > > > > > > switch
> > > > > > > +       statements are expected.
> > > > > > > +     - Tagged as `safe` for ECLAIR.
> > > > > > > +
> > > > > > > +   * - R2.1
> > > > > > > +     - Some functions are intended not to be referenced.
> > > > > > > +     - Tagged as `deliberate` for ECLAIR.
> > > > > > 
> > > > > > What does it mean "some functions" in this case? Should we list
> > > > > > which
> > > > > > functions?
> > > > > > 
> > > > > 
> > > > > Well, there are a lot, typically resulting from build configurations
> > > > > that do
> > > > > not
> > > > > use them, or because they are used only in asm code. I can mention
> > > > > these
> > > > > reasons in the
> > > > > document, to make it easier to understand.
> > > > 
> > > > Yes, I think we need to clarify further this point, because saying "Some
> > > > functions" doesn't help the reader understand:
> > > > - whether all functions can be not referenced
> > > > - which subset of functions can be not referenced
> > > > 
> > > > How to distinguish between? How do we know whether a certain patch is
> > > > violating the rule or not?
> > > > 
> > > > If there is a clear list of functions that can be not referenced, then
> > > > we should list them here. If there is a methodology we can use to
> > > > distinguish between them (e.g. functions called from asm only) then we
> > > > can write the methodology here. Either way it is fine as long as the
> > > > criteria to know if it is OK if a function is not referenced is clear.
> > > 
> > > Aren't they more or less the one we tagged with SAF-1-safe because
> > > there were no prototype? If so, we could use the same tags.
> > > 
> > > We could introduce an extra tags for the others. An alternative would
> > > be to add an attribute (e.g. asmcall) to mark each function used by
> > > assembly.
> > > 
> > > Cheers,
> > 
> > Both suggestion do have some value. As it is, it's not distinguishable
> > what causes a
> > function to be unreferenced in a certain analysis config. However:
> > 
> > - functions only used by asm code can be specified in the ECLAIR
> > config so that they will
> >   have an extra fake reference as far as the checker is concerned. I
> > can do that on a
> >   separate patch and list them in deviations.rst. An attribute seems a
> > good way to signal the
> >   intention.
> > - Functions that have no reference only in the current analysis should
> > have their declaration
> >   #ifdef-ed out in the configurations where they are not used, in an
> > ideal world.
> > - Truly unreferenced functions should be removed, or justified
> 
> Especially the last two appear somewhat tricky to disentangle, as they do
> require knowledge of
> possible code paths.

First let me premise that if we are unsure on how to proceed on this you
can resend this patch series without this item ("Some functions are
intended not to be referenced"), so at least the rest can go in now.

On this specific point, I think we should only make clear and
unmistakable statements. For instance, I think it is OK to say that all
the functions only used by asm code are exceptions (ideally they would
have a asmcall tag as Julien suggested) because that is deterministic.

Functions that have no references in a specific kconfig configuration
should have their definition #ifdef'ed (not necessarily the
declaration, I think we have already clarified that it is OK to have a
declaration without definition.) 

Truly unreferenced functions should be removed.

In conclusion, I think we should only have "functions only called from
asm code" as a deviation here.

Re: [XEN PATCH][for-4.19 v2 2/2] docs/misra: add deviations.rst to document additional deviations.

[Nicola Vetrini]

On 13/10/2023 01:14, Stefano Stabellini wrote:
> On Wed, 11 Oct 2023, Nicola Vetrini wrote:
>> On 11/10/2023 17:00, Nicola Vetrini wrote:
>> > > > > > > +
>> > > > > > > +   * - R2.1
>> > > > > > > +     - The compiler implementation guarantees that the
>> > > > > > > unreachable code
>> > > > > > > is
>> > > > > > > +       removed. Constant expressions and unreachable branches of
>> > > > > > > if and
>> > > > > > > switch
>> > > > > > > +       statements are expected.
>> > > > > > > +     - Tagged as `safe` for ECLAIR.
>> > > > > > > +
>> > > > > > > +   * - R2.1
>> > > > > > > +     - Some functions are intended not to be referenced.
>> > > > > > > +     - Tagged as `deliberate` for ECLAIR.
>> > > > > >
>> > > > > > What does it mean "some functions" in this case? Should we list
>> > > > > > which
>> > > > > > functions?
>> > > > > >
>> > > > >
>> > > > > Well, there are a lot, typically resulting from build configurations
>> > > > > that do
>> > > > > not
>> > > > > use them, or because they are used only in asm code. I can mention
>> > > > > these
>> > > > > reasons in the
>> > > > > document, to make it easier to understand.
>> > > >
>> > > > Yes, I think we need to clarify further this point, because saying "Some
>> > > > functions" doesn't help the reader understand:
>> > > > - whether all functions can be not referenced
>> > > > - which subset of functions can be not referenced
>> > > >
>> > > > How to distinguish between? How do we know whether a certain patch is
>> > > > violating the rule or not?
>> > > >
>> > > > If there is a clear list of functions that can be not referenced, then
>> > > > we should list them here. If there is a methodology we can use to
>> > > > distinguish between them (e.g. functions called from asm only) then we
>> > > > can write the methodology here. Either way it is fine as long as the
>> > > > criteria to know if it is OK if a function is not referenced is clear.
>> > >
>> > > Aren't they more or less the one we tagged with SAF-1-safe because
>> > > there were no prototype? If so, we could use the same tags.
>> > >
>> > > We could introduce an extra tags for the others. An alternative would
>> > > be to add an attribute (e.g. asmcall) to mark each function used by
>> > > assembly.
>> > >
>> > > Cheers,
>> >
>> > Both suggestion do have some value. As it is, it's not distinguishable
>> > what causes a
>> > function to be unreferenced in a certain analysis config. However:
>> >
>> > - functions only used by asm code can be specified in the ECLAIR
>> > config so that they will
>> >   have an extra fake reference as far as the checker is concerned. I
>> > can do that on a
>> >   separate patch and list them in deviations.rst. An attribute seems a
>> > good way to signal the
>> >   intention.
>> > - Functions that have no reference only in the current analysis should
>> > have their declaration
>> >   #ifdef-ed out in the configurations where they are not used, in an
>> > ideal world.
>> > - Truly unreferenced functions should be removed, or justified
>> 
>> Especially the last two appear somewhat tricky to disentangle, as they 
>> do
>> require knowledge of
>> possible code paths.
> 
> First let me premise that if we are unsure on how to proceed on this 
> you
> can resend this patch series without this item ("Some functions are
> intended not to be referenced"), so at least the rest can go in now.
> 
> On this specific point, I think we should only make clear and
> unmistakable statements. For instance, I think it is OK to say that all
> the functions only used by asm code are exceptions (ideally they would
> have a asmcall tag as Julien suggested) because that is deterministic.
> 
> Functions that have no references in a specific kconfig configuration
> should have their definition #ifdef'ed (not necessarily the
> declaration, I think we have already clarified that it is OK to have a
> declaration without definition.)
> 
> Truly unreferenced functions should be removed.
> 
> In conclusion, I think we should only have "functions only called from
> asm code" as a deviation here.

I agree on leaving this out of the patch for now.

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)