Re: [PATCH 2/5] xen/perfc: Add perfc_defn.h to asm-generic

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 2 Jan 2025, Andrew Cooper wrote:
> ... and hook it up for RISC-V and PPC.
> 
> On RISC-V at least, no combination of headers pulls in errno.h, so include it
> explicitly.
> 
> Guard the hypercalls array declaration based on NR_hypercalls existing.  This
> is sufficient to get PERF_COUNTERS fully working on RISC-V and PPC, so drop
> the randconfig override.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Julien Grall <julien@xxxxxxx>
> CC: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
> CC: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> CC: Michal Orzel <michal.orzel@xxxxxxx>
> CC: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> CC: Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>
> ---
>  automation/gitlab-ci/build.yaml      | 1 -
>  xen/arch/ppc/include/asm/Makefile    | 1 +
>  xen/arch/riscv/include/asm/Makefile  | 1 +
>  xen/common/perfc.c                   | 1 +
>  xen/include/asm-generic/perfc_defn.h | 5 +++++
>  xen/include/xen/perfc_defn.h         | 2 ++
>  6 files changed, 10 insertions(+), 1 deletion(-)
>  create mode 100644 xen/include/asm-generic/perfc_defn.h
> 
> diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
> index 1b884cc81cdb..41f17ed45641 100644
> --- a/automation/gitlab-ci/build.yaml
> +++ b/automation/gitlab-ci/build.yaml
> @@ -734,7 +734,6 @@ debian-12-riscv64-gcc:
>        CONFIG_GRANT_TABLE=n
>        CONFIG_LIVEPATCH=n
>        CONFIG_MEM_ACCESS=n
> -      CONFIG_PERF_COUNTERS=n
>        CONFIG_QEMU_PLATFORM=y
>        CONFIG_XSM=n
>  
> diff --git a/xen/arch/ppc/include/asm/Makefile 
> b/xen/arch/ppc/include/asm/Makefile
> index ced02e26ed13..c989a7f89b34 100644
> --- a/xen/arch/ppc/include/asm/Makefile
> +++ b/xen/arch/ppc/include/asm/Makefile
> @@ -7,6 +7,7 @@ generic-y += hypercall.h
>  generic-y += iocap.h
>  generic-y += paging.h
>  generic-y += percpu.h
> +generic-y += perfc_defn.h
>  generic-y += random.h
>  generic-y += softirq.h
>  generic-y += vm_event.h
> diff --git a/xen/arch/riscv/include/asm/Makefile 
> b/xen/arch/riscv/include/asm/Makefile
> index ced02e26ed13..c989a7f89b34 100644
> --- a/xen/arch/riscv/include/asm/Makefile
> +++ b/xen/arch/riscv/include/asm/Makefile
> @@ -7,6 +7,7 @@ generic-y += hypercall.h
>  generic-y += iocap.h
>  generic-y += paging.h
>  generic-y += percpu.h
> +generic-y += perfc_defn.h
>  generic-y += random.h
>  generic-y += softirq.h
>  generic-y += vm_event.h
> diff --git a/xen/common/perfc.c b/xen/common/perfc.c
> index ed4dba36f1bc..8c967ab900f9 100644
> --- a/xen/common/perfc.c
> +++ b/xen/common/perfc.c
> @@ -1,4 +1,5 @@
>  
> +#include <xen/errno.h>
>  #include <xen/lib.h>
>  #include <xen/smp.h>
>  #include <xen/time.h>
> diff --git a/xen/include/asm-generic/perfc_defn.h 
> b/xen/include/asm-generic/perfc_defn.h
> new file mode 100644
> index 000000000000..8237636d83fb
> --- /dev/null
> +++ b/xen/include/asm-generic/perfc_defn.h
> @@ -0,0 +1,5 @@
> +/* This file is legitimately included multiple times. */

It is a good idea to add comment here to explain. This is effectively
the same as a deviation of MISRA D4.10. SAF-8-safe is defined as
"Headers that deliberatively leave the responsability of their correct
inclusion to the caller are allowed". I think it applies, please add
SAF-8-safe to this comment and also the other perfc_defn.h, e.g.:

/* SAF-8-safe This file is legitimately included multiple times. */


> +/* #ifndef ASM_GENERIC_PERFC_DEFN_H */
> +/* #define ASM_GENERIC_PERFC_DEFN_H */
> +
> +/* #endif ASM_GENERIC_PERFC_DEFN_H */
> diff --git a/xen/include/xen/perfc_defn.h b/xen/include/xen/perfc_defn.h
> index 0027d95a60bc..a987d80dd6f1 100644
> --- a/xen/include/xen/perfc_defn.h
> +++ b/xen/include/xen/perfc_defn.h
> @@ -4,7 +4,9 @@
>  
>  #include <asm/perfc_defn.h>
>  
> +#ifdef NR_hypercalls
>  PERFCOUNTER_ARRAY(hypercalls,           "hypercalls", NR_hypercalls)
> +#endif
>  
>  PERFCOUNTER(calls_from_multicall,       "calls from multicall")
>  
> -- 
> 2.39.5
>