On 25.11.2025 17:30, Grygorii Strashko wrote: > On 25.11.25 17:59, Jan Beulich wrote: >> On 21.11.2025 11:57, Penny Zheng wrote: >>> The following functions have been referenced in places which is either >>> guarded >>> with CONFIG_MGMT_HYPERCALLS or CONFIG_MEM_SHARING: >>> - arch_hvm_save >>> - arch_hvm_check >>> - arch_hvm_load >>> - hvm_save_size >>> - hvm_save >>> - hvm_load >>> While CONFIG_MEM_SHARING is also dependent on CONFIG_MGMT_HYPERCALLS. >>> So they shall be wrapped under MGMT_HYPERCALLS, otherwise they will become >>> unreachable codes when MGMT_HYPERCALLS=n, and hence violating Misra rule >>> 2.1. >>> We move arch_hvm_save(), arch_hvm_check(), arch_hvm_load() and >>> hvm_save_size() >>> nearer to the left functions, to avoid scattered #ifdef-wrapping. >> >> Why would you move things? What is in this source file that is of any use >> when >> MGMT_HYPERCALLS=n? The only caller of hvm_save_one() lives in x86/domctl.c. >> With >> that also removed, hvm_sr_handlers[] is only ever written to afaict, which >> means >> that's an effectively dead array then too. >> >> The final few functions ... >> >>> @@ -390,6 +391,7 @@ int hvm_load(struct domain *d, bool real, >>> hvm_domain_context_t *h) >>> >>> /* Not reached */ >>> } >>> +#endif /* CONFIG_MGMT_HYPERCALLS */ >>> >>> int _hvm_init_entry(struct hvm_domain_context *h, uint16_t tc, uint16_t >>> inst, >>> uint32_t len) >> >> ... here and below are only helpers for the save/restore machinery, i.e. that >> _all_ is also usable only when MGMT_HYPERCALLS=y. Yes, that's a lot of >> further >> work, but what do you do? You'll then have quite a bit more code removed from >> the set that as per coverage analysis is never reached. > > I have a local patch which allows to disable all HVM save/load code at once > by using > separated Kconfig option SAVE_RESTORE. > > +++ b/xen/arch/x86/hvm/Kconfig > @@ -127,4 +127,8 @@ config VHPET > > +config SAVE_RESTORE > + depends on MGMT_HYPERCALLS > + def_bool y > > SAVE_RESTORE - annotates all HVM save/load code and, in general, could made a > feature by > allowing it to be selectable.
Oh, one more thing: SAVE_RESTORE, simply by its name, promises to cover PV as well. That either wants to be the case, or the name may want adjusting. Jan
