Re: [RFC v2 3/6] xen/arm: support guest do_suspend function

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 15/04/2014 22:05, Wei Huang wrote:
> From: Jaeyong Yoo <jaeyong.yoo@samsung.com>
>
> Making sched_op in do_suspend (driver/xen/manage.c) returns 0 on the
> success of suspend.
>
> Signed-off-by: Alexey Sokolov <sokolov.a@samsung.com>
> Signed-off-by: Wei Huang <w1.huang@samsung.com>
> ---
>  tools/libxc/xc_resume.c |   25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/tools/libxc/xc_resume.c b/tools/libxc/xc_resume.c
> index 18b4818..2b09990 100644
> --- a/tools/libxc/xc_resume.c
> +++ b/tools/libxc/xc_resume.c
> @@ -73,6 +73,31 @@ static int modify_returncode(xc_interface *xch, uint32_t domid)
>      return 0;
>  }
>  
> +#elif defined(__arm__) || defined(__aarch64__)
> +
> +static int modify_returncode(xc_interface *xch, uint32_t domid)
> +{
> +    vcpu_guest_context_any_t ctxt;
> +    xc_dominfo_t info;
> +    int rc;
> +
> +    if ( xc_domain_getinfo(xch, domid, 1, &info) != 1 )
> +    {
> +        PERROR("Could not get domain info");
> +        return -EINVAL;
> +    }

The semantics for xc_domain_getinfo() are crazy, and it sadly gets used
incorrectly far more often than correctly.

As the call stands, it asks for the first '1' domain which can be found
by starting at 'domid'.  If the provided domid is wrong, you will get
valid domain information for a different domain back, so in the  you
must also confirm that info.domid == domid

> +
> +    if ( (rc = xc_vcpu_getcontext(xch, domid, 0, &ctxt)) != 0 )
> +        return rc;
> +
> +    ctxt.c.user_regs.r0_usr = 1;

This is the only architecture specific bit of code.  Cant you make the
code common with a small #if defined($ARCH) section in the middle of the
function?

~Andrew

> +
> +    if ( (rc = xc_vcpu_setcontext(xch, domid, 0, &ctxt)) != 0 )
> +        return rc;
> +
> +    return 0;
> +}
> +
>  #else
>  
>  static int modify_returncode(xc_interface *xch, uint32_t domid)