[PATCH v2] clarify SHUTDOWN_suspend additional argument

[PATCH v2] clarify SHUTDOWN_suspend additional argument

[Stefano Stabellini]

Clarify the behaviour of SCHEDOP_shutdown: PV x86 guests need to pass a
third argument, that is unused on HVM and ARM guests.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

diff --git a/xen/include/public/sched.h b/xen/include/public/sched.h
index a30b11d..4000ac9 100644
--- a/xen/include/public/sched.h
+++ b/xen/include/public/sched.h
@@ -76,9 +76,10 @@
  * Halt execution of this domain (all VCPUs) and notify the system controller.
  * @arg == pointer to sched_shutdown_t structure.
  *
- * If the sched_shutdown_t reason is SHUTDOWN_suspend then this
- * hypercall takes an additional extra argument which should be the
- * MFN of the guest's start_info_t.
+ * If the sched_shutdown_t reason is SHUTDOWN_suspend then
+ * x86 PV guests must also set RDX (EDX for 32-bit guests) to the MFN
+ * of the guest's start info page.  RDX/EDX is the third hypercall
+ * argument.
  *
  * In addition, which reason is SHUTDOWN_suspend this hypercall
  * returns 1 if suspend was cancelled or the domain was merely

Re: [PATCH v2] clarify SHUTDOWN_suspend additional argument

[Andrew Cooper]

On 08/05/14 16:43, Stefano Stabellini wrote:
> Clarify the behaviour of SCHEDOP_shutdown: PV x86 guests need to pass a
> third argument, that is unused on HVM and ARM guests.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>
> diff --git a/xen/include/public/sched.h b/xen/include/public/sched.h
> index a30b11d..4000ac9 100644
> --- a/xen/include/public/sched.h
> +++ b/xen/include/public/sched.h
> @@ -76,9 +76,10 @@
>   * Halt execution of this domain (all VCPUs) and notify the system controller.
>   * @arg == pointer to sched_shutdown_t structure.
>   *
> - * If the sched_shutdown_t reason is SHUTDOWN_suspend then this
> - * hypercall takes an additional extra argument which should be the
> - * MFN of the guest's start_info_t.
> + * If the sched_shutdown_t reason is SHUTDOWN_suspend then
> + * x86 PV guests must also set RDX (EDX for 32-bit guests) to the MFN
> + * of the guest's start info page.  RDX/EDX is the third hypercall
> + * argument.

Its still a little awkward to read.  How about

@arg2 == mfn of start_info_t (x86 PV only)

above this paragraph.

Explaining which parameters live in which registers is tangential to the
point of "this is a 3 argument hypercall for x86 pv and 2 argument for
all other types of domains"

~Andrew

>   *
>   * In addition, which reason is SHUTDOWN_suspend this hypercall
>   * returns 1 if suspend was cancelled or the domain was merely
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH v2] clarify SHUTDOWN_suspend additional argument

[Stefano Stabellini]

On Thu, 8 May 2014, Andrew Cooper wrote:
> On 08/05/14 16:43, Stefano Stabellini wrote:
> > Clarify the behaviour of SCHEDOP_shutdown: PV x86 guests need to pass a
> > third argument, that is unused on HVM and ARM guests.
> >
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >
> > diff --git a/xen/include/public/sched.h b/xen/include/public/sched.h
> > index a30b11d..4000ac9 100644
> > --- a/xen/include/public/sched.h
> > +++ b/xen/include/public/sched.h
> > @@ -76,9 +76,10 @@
> >   * Halt execution of this domain (all VCPUs) and notify the system controller.
> >   * @arg == pointer to sched_shutdown_t structure.
> >   *
> > - * If the sched_shutdown_t reason is SHUTDOWN_suspend then this
> > - * hypercall takes an additional extra argument which should be the
> > - * MFN of the guest's start_info_t.
> > + * If the sched_shutdown_t reason is SHUTDOWN_suspend then
> > + * x86 PV guests must also set RDX (EDX for 32-bit guests) to the MFN
> > + * of the guest's start info page.  RDX/EDX is the third hypercall
> > + * argument.
> 
> Its still a little awkward to read.  How about
> 
> @arg2 == mfn of start_info_t (x86 PV only)
> 
> above this paragraph.
> 
> Explaining which parameters live in which registers is tangential to the
> point of "this is a 3 argument hypercall for x86 pv and 2 argument for
> all other types of domains"

The current version is easier to understand for me.

Re: [PATCH v2] clarify SHUTDOWN_suspend additional argument

[Ian Campbell]

On Thu, 2014-05-08 at 18:27 +0100, Stefano Stabellini wrote:
> On Thu, 8 May 2014, Andrew Cooper wrote:
> > On 08/05/14 16:43, Stefano Stabellini wrote:
> > > Clarify the behaviour of SCHEDOP_shutdown: PV x86 guests need to pass a
> > > third argument, that is unused on HVM and ARM guests.
> > >
> > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > >
> > > diff --git a/xen/include/public/sched.h b/xen/include/public/sched.h
> > > index a30b11d..4000ac9 100644
> > > --- a/xen/include/public/sched.h
> > > +++ b/xen/include/public/sched.h
> > > @@ -76,9 +76,10 @@
> > >   * Halt execution of this domain (all VCPUs) and notify the system controller.
> > >   * @arg == pointer to sched_shutdown_t structure.
> > >   *
> > > - * If the sched_shutdown_t reason is SHUTDOWN_suspend then this
> > > - * hypercall takes an additional extra argument which should be the
> > > - * MFN of the guest's start_info_t.
> > > + * If the sched_shutdown_t reason is SHUTDOWN_suspend then
> > > + * x86 PV guests must also set RDX (EDX for 32-bit guests) to the MFN
> > > + * of the guest's start info page.  RDX/EDX is the third hypercall
> > > + * argument.
> > 
> > Its still a little awkward to read.  How about
> > 
> > @arg2 == mfn of start_info_t (x86 PV only)
> > 
> > above this paragraph.
> > 
> > Explaining which parameters live in which registers is tangential to the
> > point of "this is a 3 argument hypercall for x86 pv and 2 argument for
> > all other types of domains"
> 
> The current version is easier to understand for me.

It's as good a starting point as any, so I have acked + applied. If
anyone wants to clarify further they are welcome to do so.

Ian.