Re: [PATCH v15 1/4] xen/arm: Implement PSCI SYSTEM_SUSPEND call for guests

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.12.2025 13:13, Mykola Kvach wrote:
> Hi Jan,
> 
> Thank you for the review.
> 
> On Wed, Dec 3, 2025 at 12:11 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>
>> On 03.12.2025 10:57, Mykola Kvach wrote:
>>> --- a/xen/common/domain.c
>>> +++ b/xen/common/domain.c
>>> @@ -26,6 +26,7 @@
>>>  #include <xen/hypercall.h>
>>>  #include <xen/delay.h>
>>>  #include <xen/shutdown.h>
>>> +#include <xen/suspend.h>
>>>  #include <xen/percpu.h>
>>>  #include <xen/multicall.h>
>>>  #include <xen/rcupdate.h>
>>> @@ -1363,6 +1364,9 @@ void domain_resume(struct domain *d)
>>>
>>>      spin_lock(&d->shutdown_lock);
>>>
>>> +    if ( arch_domain_resume(d) )
>>> +        goto fail;
>>
>> In case I didn't ask before: You're after a boolean result here, yet ...
>>
>>> --- /dev/null
>>> +++ b/xen/include/xen/suspend.h
>>> @@ -0,0 +1,25 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>> +
>>> +#ifndef XEN_SUSPEND_H
>>> +#define XEN_SUSPEND_H
>>> +
>>> +#if __has_include(<asm/suspend.h>)
>>> +#include <asm/suspend.h>
>>> +#else
>>> +static inline int arch_domain_resume(struct domain *d)
>>> +{
>>> +    return 0;
>>> +}
>>> +#endif
>>
>> ... int is being returned. Why?
> 
> Would you prefer I switch arch_domain_resume() to bool, or is keeping
> int acceptable?
> 
> I lean toward keeping int: the generic call site is shared by all arches,
> so future arches could inspect/handle specific error codes,

It's an arch hook, so the arch provides the error code. It's common code
which may want to inspect it.

> and this matches
> other arch hooks (e.g. arch_domain_teardown()) that are only checked for
> "rc != 0" before bailing.

And it's questionable there as well why they would return an error code
if the sole caller cares about a boolean outcome only.

> With int, I'll store the result and gate the rest:
> 
>     rc = arch_domain_resume(d);
>     if (rc)
>         goto fail;
> 
> If int works for you, I’ll keep it; otherwise I can flip to bool.

If you want to stick to int, please justify doing so in the description.

Jan