On 12.12.2025 05:02, Penny Zheng wrote:
> ---
> v4 -> v5:
> - do not undo imply and add "depends on MGMT_HYPERCALLS" in option
> - use IS_ENABLED(...) to avoid too long line
Coming back to this patch when looking at patch 24, I have to add: I don't
understand this. We ...
> --- a/xen/arch/x86/include/asm/paging.h
> +++ b/xen/arch/x86/include/asm/paging.h
> @@ -57,7 +57,8 @@
> #endif
> #if defined(CONFIG_PAGING) && !defined(CONFIG_PV_SHIM_EXCLUSIVE)
> /* Enable log dirty mode */
> -#define PG_log_dirty (XEN_DOMCTL_SHADOW_ENABLE_LOG_DIRTY << PG_mode_shift)
> +#define PG_log_dirty IS_ENABLED(CONFIG_MGMT_HYPERCALLS) && \
> + (XEN_DOMCTL_SHADOW_ENABLE_LOG_DIRTY << PG_mode_shift)
> #else
> #define PG_log_dirty 0
> #endif
... already have an #if here, so mixing #if and IS_ENABLED() doesn't look
like a good idea to me. To deal with the line length issue, all you need to
do is split the line:
#if defined(CONFIG_PAGING) && defined(CONFIG_MGMT_HYPERCALLS) && \
!defined(CONFIG_PV_SHIM_EXCLUSIVE)
. The alternative could be
#define PG_log_dirty (IS_ENABLED(CONFIG_PAGING) && \
IS_ENABLED(CONFIG_MGMT_HYPERCALLS) && \
!IS_ENABLED(CONFIG_PV_SHIM_EXCLUSIVE) && \
(XEN_DOMCTL_SHADOW_ENABLE_LOG_DIRTY << PG_mode_shift))
, just that then PG_log_dirty likely can't be used in #if anymore (see the
related comment on patch 24 that I'm in the process of writing).
Jan
