Re: [PATCH v2 2/3] xen/livepatch: Fix include hierarchy

[Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>]

On Fri, May 9, 2025 at 5:32 PM Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
>
> xen/livepatch.h includes public/sysctl.h twice, which can be deduplicated, and
> includes asm/livepatch.h meaning that each livepatch.c does not need to
> include both.
>
> Comment the #else and #endif cases to aid legibility.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> CC: Michal Orzel <michal.orzel@xxxxxxx>
> CC: Jan Beulich <jbeulich@xxxxxxxx>
> CC: Julien Grall <julien@xxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
> CC: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> CC: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> CC: Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>
> CC: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
> ---
>  xen/arch/arm/arm32/livepatch.c |  1 -
>  xen/arch/arm/arm64/livepatch.c |  1 -
>  xen/arch/arm/livepatch.c       |  1 -
>  xen/arch/x86/livepatch.c       |  1 -
>  xen/include/xen/livepatch.h    | 10 +++++-----
>  5 files changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/xen/arch/arm/arm32/livepatch.c b/xen/arch/arm/arm32/livepatch.c
> index 134d07a175bb..8541c71d6e2e 100644
> --- a/xen/arch/arm/arm32/livepatch.c
> +++ b/xen/arch/arm/arm32/livepatch.c
> @@ -9,7 +9,6 @@
>  #include <xen/livepatch.h>
>
>  #include <asm/page.h>
> -#include <asm/livepatch.h>
>
>  void arch_livepatch_apply(const struct livepatch_func *func,
>                            struct livepatch_fstate *state)
> diff --git a/xen/arch/arm/arm64/livepatch.c b/xen/arch/arm/arm64/livepatch.c
> index e135bd5bf99a..39159ba8b5bf 100644
> --- a/xen/arch/arm/arm64/livepatch.c
> +++ b/xen/arch/arm/arm64/livepatch.c
> @@ -13,7 +13,6 @@
>
>  #include <asm/bitops.h>
>  #include <asm/insn.h>
> -#include <asm/livepatch.h>
>
>  void arch_livepatch_apply(const struct livepatch_func *func,
>                            struct livepatch_fstate *state)
> diff --git a/xen/arch/arm/livepatch.c b/xen/arch/arm/livepatch.c
> index 3805b2974663..2fbb7bce60bb 100644
> --- a/xen/arch/arm/livepatch.c
> +++ b/xen/arch/arm/livepatch.c
> @@ -11,7 +11,6 @@
>  #include <xen/vmap.h>
>
>  #include <asm/cpufeature.h>
> -#include <asm/livepatch.h>
>
>  /* Override macros from asm/page.h to make them work with mfn_t */
>  #undef virt_to_mfn
> diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c
> index be40f625d206..bdca355dc6cc 100644
> --- a/xen/arch/x86/livepatch.c
> +++ b/xen/arch/x86/livepatch.c
> @@ -17,7 +17,6 @@
>  #include <asm/endbr.h>
>  #include <asm/fixmap.h>
>  #include <asm/nmi.h>
> -#include <asm/livepatch.h>
>  #include <asm/setup.h>
>
>  static bool has_active_waitqueue(const struct vm_event_domain *ved)
> diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
> index d074a5bebecc..c1e76ef55404 100644
> --- a/xen/include/xen/livepatch.h
> +++ b/xen/include/xen/livepatch.h
> @@ -14,12 +14,14 @@ struct xen_sysctl_livepatch_op;
>  #include <xen/elfstructs.h>
>  #include <xen/errno.h> /* For -ENOSYS or -EOVERFLOW */
>
> -#include <public/sysctl.h> /* For LIVEPATCH_OPAQUE_SIZE */
> +#include <public/sysctl.h>
>
>  #ifdef CONFIG_LIVEPATCH
>

Shouldn't the inclusion of sysctl.h be inside CONFIG_LIVEPATCH?

There is already a forward declaration for xen_sysctl_livepatch_op
which should cover the !CONFIG_LIVEPATCH side of things.

Ross