diff mbox

[Xen-devel,RFC,v2,3/6] xen/arm: support guest do_suspend function

Message ID 1397595918-30419-4-git-send-email-w1.huang@samsung.com
State New
Headers show

Commit Message

Wei Huang April 15, 2014, 9:05 p.m. UTC 
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(+)

Comments

Andrew Cooper April 15, 2014, 11:38 p.m. UTC | #1 
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)
Andrew Cooper April 15, 2014, 11:39 p.m. UTC | #2 
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)
Julien Grall April 16, 2014, 9:10 a.m. UTC | #3 
Hello Wei,

Thank you for the patch.

On 15/04/14 22:05, Wei Huang wrote:
> +#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;
> +    }
> +
> +    if ( (rc = xc_vcpu_getcontext(xch, domid, 0, &ctxt)) != 0 )
> +        return rc;
> +
> +    ctxt.c.user_regs.r0_usr = 1;

r0_usr is only for 32-bit. I think you need to use x0 is you are 
restoring a 64-bits guest.

Regards,
diff mbox

Patch

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;
+    }
+
+    if ( (rc = xc_vcpu_getcontext(xch, domid, 0, &ctxt)) != 0 )
+        return rc;
+
+    ctxt.c.user_regs.r0_usr = 1;
+
+    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)