[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 2/2] x86/mm: correct PG_log_dirty definition
On 2025-08-05 03:59, Jan Beulich wrote: While it is correct that in shim-exclusive mode log-dirty handling is all unreachable code, the present conditional still isn't correct: In a HVM=n and SHADOW_PAGING=n configuration log-dirty code also is all unreachable (and hence violating Misra rule 2.1). As we're aiming at moving away from special casing PV_SHIM_EXCLUSIVE=y, don't retain that part of the conditional. Because of hypercall-defs.c we need to carry out the dependency by introducing a new auxiliary PAGING control. Since compiling out mm/paging.c altogether would entail further changes, merely conditionalize the one function in there (paging_enable()) which would otherwise remain unreachable (Misra rule 2.1 again) when PAGING=n. Fixes: 23d4e0d17b76 ("x86/shim: fix build with PV_SHIM_EXCLUSIVE and SHADOW_PAGING") Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> --- Of course PAGING is at risk of being confused with MEM_PAGING. It not having a prompt, I hope that's tolerable, as I can't really think of a better name. Other PG_log_dirty pre-processor conditionals then likely also want replacing. mm/paging.c and mm/p2m-basic.c could also be compiled out altogether when PAGING=n, at the expense of introducing a few more stubs. FTAOD, the Fixes: tag being referenced does not mean this patch corrects the far more recently introduced build issue with the combination of the two features. That's still work that I expect Penny to carry out (with there still being the option of reverting the final part of the earlier series). --- a/xen/arch/x86/mm/paging.c +++ b/xen/arch/x86/mm/paging.c @@ -864,6 +864,7 @@ void paging_final_teardown(struct domain p2m_final_teardown(d); }+#ifdef CONFIG_PAGING The file already has a lot of uses of #if PG_log_dirty with similar meaning, if I am not mistaken, so using that would make it more consistent. But CONFIG_PAGING is directly tied to the Kconfig, so maybe it is better? Just something I noticed. Reviewed-by: Jason Andryuk <jason.andryuk@xxxxxxx> Regards, Jason /* Enable an arbitrary paging-assistance mode. Call once at domain * creation. */ int paging_enable(struct domain *d, u32 mode) @@ -889,6 +890,7 @@ int paging_enable(struct domain *d, u32 else return shadow_enable(d, mode); } +#endif#ifdef CONFIG_HVM/* Called from the guest to indicate that a process is being torn down
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |