Hi Mykola,
On 02/09/2025 10:03, Mykola Kvach wrote:
@@ -880,6 +883,40 @@ void arch_domain_creation_finished(struct domain *d)
p2m_domain_creation_finished(d);
}
+int arch_domain_resume(struct domain *d)
+{
+ int rc;
+ typeof(d->arch.resume_ctx) *ctx = &d->arch.resume_ctx;
I know this is v13, but I don't think we should use typeof() is in this
context. "struct resume_info" is much shorter and help the review (I
don't have to grep resume_ctx to figure out the type).
+
+ if ( !d->is_shutting_down || d->shutdown_code != SHUTDOWN_suspend )
+ {
+ dprintk(XENLOG_WARNING,
+ "%pd: Invalid domain state for resume: is_shutting_down=%d,
shutdown_code=%d\n",
+ d, d->is_shutting_down, d->shutdown_code);
+ return -EINVAL;
+ }
+
+ /*
+ * It is still possible to call domain_shutdown() with a suspend reason
+ * via some hypercalls, such as SCHEDOP_shutdown or
SCHEDOP_remote_shutdown.
+ * In these cases, the resume context will be empty.
+ * This is not expected to cause any issues, so we just warn about the
+ * situation and return without error, allowing the existing logic to
+ * proceed as expected.
I think this odd to warn if something is expected. It would be best to
use "XENLOG_INFO".
+ */
+ if ( !ctx->wake_cpu )
+ {
+ dprintk(XENLOG_WARNING, "%pd: Invalid wake CPU pointer for resume\n",
As you wrote above, there is nothing wrong. So "Invalid" is not correct.
I think a better wording is "Wake CPU pointer context was not provided").
Also note that dprintk() will be a NOP in release build. I am guessing
this is intended?
I will answer you Jan's comment separately. The rest looks good to me.
Cheers,
--
Julien Grall
