Hi Jan, Penny,

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.
Of course, It all can be done by just using MGMT_HYPERCALLS.

So, I'd be appreciated for you opinion - does it make sense to have separate 
SAVE_RESTORE?

--
Best regards,
-grygorii


Reply via email to