All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] kexec: implement STATUS hypercall to check if image is loaded
@ 2017-01-17 17:29 Eric DeVolder
  2017-01-17 20:41 ` Andrew Cooper
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Eric DeVolder @ 2017-01-17 17:29 UTC (permalink / raw)
  To: xen-devel, ian.jackson, wei.liu2
  Cc: elena.ufimtseva, andrew.cooper3, daniel.kiper, konrad.wilk

The tools that use kexec are asynchronous in nature and do not keep
state changes. As such provide an hypercall to find out whether an
image has been loaded for either type.

Note: No need to modify XSM as it has one size fits all check and
does not check for subcommands.

Note: No need to check KEXEC_FLAG_IN_PROGRESS (and error out of
kexec_status()) as this flag is set only once by the first/only
cpu on the crash path.

Note: This is just the Xen side of the hypercall, kexec-tools patch
to come separately.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
---
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Elena Ufimtseva <elena.ufimtseva@oracle.com>
CC: Daniel Kiper <daniel.kiper@oracle.com>

v0: Internal version.
v1: Dropped Reviewed-by, posting on xen-devel.
v2: Incorporated xen-devel feedback, re-posted on xen-devel.
v3: Incorporated xen-devel feedback
---
 tools/libxc/include/xenctrl.h | 10 ++++++++++
 tools/libxc/xc_kexec.c        | 24 ++++++++++++++++++++++++
 xen/common/kexec.c            | 19 +++++++++++++++++++
 xen/include/public/kexec.h    | 13 +++++++++++++
 4 files changed, 66 insertions(+)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 4ab0f57..63c616f 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2574,6 +2574,16 @@ int xc_kexec_load(xc_interface *xch, uint8_t type, uint16_t arch,
  */
 int xc_kexec_unload(xc_interface *xch, int type);
 
+/*
+ * Find out whether the image has been succesfully loaded.
+ *
+ * The type can be either KEXEC_TYPE_DEFAULT or KEXEC_TYPE_CRASH.
+ * If zero is returned, that means no image is loaded for the type.
+ * If one is returned, that means an image is loaded for the type.
+ * Otherwise, negative return value indicates error.
+ */
+int xc_kexec_status(xc_interface *xch, int type);
+
 typedef xenpf_resource_entry_t xc_resource_entry_t;
 
 /*
diff --git a/tools/libxc/xc_kexec.c b/tools/libxc/xc_kexec.c
index 59e2f07..a4e8966 100644
--- a/tools/libxc/xc_kexec.c
+++ b/tools/libxc/xc_kexec.c
@@ -126,3 +126,27 @@ out:
 
     return ret;
 }
+
+int xc_kexec_status(xc_interface *xch, int type)
+{
+    DECLARE_HYPERCALL_BUFFER(xen_kexec_status_t, status);
+    int ret = -1;
+
+    status = xc_hypercall_buffer_alloc(xch, status, sizeof(*status));
+    if ( status == NULL )
+    {
+        PERROR("Could not alloc buffer for kexec status hypercall");
+        goto out;
+    }
+
+    status->type = type;
+
+    ret = xencall2(xch->xcall, __HYPERVISOR_kexec_op,
+                   KEXEC_CMD_kexec_status,
+                   HYPERCALL_BUFFER_AS_ARG(status));
+
+out:
+    xc_hypercall_buffer_free(xch, status);
+
+    return ret;
+}
diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index c83d48f..aa808cb 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -1169,6 +1169,22 @@ static int kexec_unload(XEN_GUEST_HANDLE_PARAM(void) uarg)
     return kexec_do_unload(&unload);
 }
 
+static int kexec_status(XEN_GUEST_HANDLE_PARAM(void) uarg)
+{
+    xen_kexec_status_t status;
+    int base, bit;
+
+    if ( unlikely(copy_from_guest(&status, uarg, 1)) )
+        return -EFAULT;
+
+    /* No need to check KEXEC_FLAG_IN_PROGRESS. */
+
+    if ( kexec_load_get_bits(status.type, &base, &bit) )
+        return -EINVAL;
+
+    return test_bit(bit, &kexec_flags);
+}
+
 static int do_kexec_op_internal(unsigned long op,
                                 XEN_GUEST_HANDLE_PARAM(void) uarg,
                                 bool_t compat)
@@ -1208,6 +1224,9 @@ static int do_kexec_op_internal(unsigned long op,
     case KEXEC_CMD_kexec_unload:
         ret = kexec_unload(uarg);
         break;
+    case KEXEC_CMD_kexec_status:
+        ret = kexec_status(uarg);
+        break;
     }
 
     return ret;
diff --git a/xen/include/public/kexec.h b/xen/include/public/kexec.h
index a6a0a88..c200e8c 100644
--- a/xen/include/public/kexec.h
+++ b/xen/include/public/kexec.h
@@ -227,6 +227,19 @@ typedef struct xen_kexec_unload {
 } xen_kexec_unload_t;
 DEFINE_XEN_GUEST_HANDLE(xen_kexec_unload_t);
 
+/*
+ * Figure out whether we have an image loaded. A return value of
+ * zero indicates no image loaded. A return value of one
+ * indicates an image is loaded. A negative return value
+ * indicates an error.
+ *
+ * Type must be one of KEXEC_TYPE_DEFAULT or KEXEC_TYPE_CRASH.
+ */
+#define KEXEC_CMD_kexec_status 6
+typedef struct xen_kexec_status {
+    uint8_t type;
+} xen_kexec_status_t;
+DEFINE_XEN_GUEST_HANDLE(xen_kexec_status_t);
 #else /* __XEN_INTERFACE_VERSION__ < 0x00040400 */
 
 #define KEXEC_CMD_kexec_load KEXEC_CMD_kexec_load_v1
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] kexec: implement STATUS hypercall to check if image is loaded
  2017-01-17 17:29 [PATCH v3] kexec: implement STATUS hypercall to check if image is loaded Eric DeVolder
@ 2017-01-17 20:41 ` Andrew Cooper
  2017-01-17 22:09 ` Daniel Kiper
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Andrew Cooper @ 2017-01-17 20:41 UTC (permalink / raw)
  To: Eric DeVolder, xen-devel, ian.jackson, wei.liu2
  Cc: elena.ufimtseva, daniel.kiper, konrad.wilk

On 17/01/17 17:29, Eric DeVolder wrote:
> The tools that use kexec are asynchronous in nature and do not keep
> state changes. As such provide an hypercall to find out whether an
> image has been loaded for either type.
>
> Note: No need to modify XSM as it has one size fits all check and
> does not check for subcommands.
>
> Note: No need to check KEXEC_FLAG_IN_PROGRESS (and error out of
> kexec_status()) as this flag is set only once by the first/only
> cpu on the crash path.
>
> Note: This is just the Xen side of the hypercall, kexec-tools patch
> to come separately.
>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] kexec: implement STATUS hypercall to check if image is loaded
  2017-01-17 17:29 [PATCH v3] kexec: implement STATUS hypercall to check if image is loaded Eric DeVolder
  2017-01-17 20:41 ` Andrew Cooper
@ 2017-01-17 22:09 ` Daniel Kiper
  2017-01-18  9:47 ` Wei Liu
  2017-01-18 10:19 ` Jan Beulich
  3 siblings, 0 replies; 9+ messages in thread
From: Daniel Kiper @ 2017-01-17 22:09 UTC (permalink / raw)
  To: Eric DeVolder
  Cc: elena.ufimtseva, wei.liu2, konrad.wilk, andrew.cooper3,
	ian.jackson, xen-devel

On Tue, Jan 17, 2017 at 11:29:16AM -0600, Eric DeVolder wrote:
> The tools that use kexec are asynchronous in nature and do not keep
> state changes. As such provide an hypercall to find out whether an
> image has been loaded for either type.
>
> Note: No need to modify XSM as it has one size fits all check and
> does not check for subcommands.
>
> Note: No need to check KEXEC_FLAG_IN_PROGRESS (and error out of
> kexec_status()) as this flag is set only once by the first/only
> cpu on the crash path.
>
> Note: This is just the Xen side of the hypercall, kexec-tools patch
> to come separately.
>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] kexec: implement STATUS hypercall to check if image is loaded
  2017-01-17 17:29 [PATCH v3] kexec: implement STATUS hypercall to check if image is loaded Eric DeVolder
  2017-01-17 20:41 ` Andrew Cooper
  2017-01-17 22:09 ` Daniel Kiper
@ 2017-01-18  9:47 ` Wei Liu
  2017-01-18 10:19 ` Jan Beulich
  3 siblings, 0 replies; 9+ messages in thread
From: Wei Liu @ 2017-01-18  9:47 UTC (permalink / raw)
  To: Eric DeVolder
  Cc: elena.ufimtseva, wei.liu2, konrad.wilk, andrew.cooper3,
	daniel.kiper, ian.jackson, xen-devel

On Tue, Jan 17, 2017 at 11:29:16AM -0600, Eric DeVolder wrote:
> The tools that use kexec are asynchronous in nature and do not keep
> state changes. As such provide an hypercall to find out whether an
> image has been loaded for either type.
> 
> Note: No need to modify XSM as it has one size fits all check and
> does not check for subcommands.
> 
> Note: No need to check KEXEC_FLAG_IN_PROGRESS (and error out of
> kexec_status()) as this flag is set only once by the first/only
> cpu on the crash path.
> 
> Note: This is just the Xen side of the hypercall, kexec-tools patch
> to come separately.
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] kexec: implement STATUS hypercall to check if image is loaded
  2017-01-17 17:29 [PATCH v3] kexec: implement STATUS hypercall to check if image is loaded Eric DeVolder
                   ` (2 preceding siblings ...)
  2017-01-18  9:47 ` Wei Liu
@ 2017-01-18 10:19 ` Jan Beulich
  2017-01-18 10:37   ` Wei Liu
  3 siblings, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2017-01-18 10:19 UTC (permalink / raw)
  To: Eric DeVolder
  Cc: elena.ufimtseva, wei.liu2, konrad.wilk, andrew.cooper3,
	daniel.kiper, ian.jackson, xen-devel

>>> On 17.01.17 at 18:29, <eric.devolder@oracle.com> wrote:
> --- a/xen/common/kexec.c
> +++ b/xen/common/kexec.c
> @@ -1169,6 +1169,22 @@ static int kexec_unload(XEN_GUEST_HANDLE_PARAM(void) uarg)
>      return kexec_do_unload(&unload);
>  }
>  
> +static int kexec_status(XEN_GUEST_HANDLE_PARAM(void) uarg)
> +{
> +    xen_kexec_status_t status;
> +    int base, bit;
> +
> +    if ( unlikely(copy_from_guest(&status, uarg, 1)) )
> +        return -EFAULT;
> +
> +    /* No need to check KEXEC_FLAG_IN_PROGRESS. */
> +
> +    if ( kexec_load_get_bits(status.type, &base, &bit) )
> +        return -EINVAL;
> +
> +    return test_bit(bit, &kexec_flags);

In the public header you promise to return zero or one here (unless
an error occurs), which requires the use of !!. Please see x86's
implementation of the function for how/when there can actually be
other non-zero values returned here (in particular all ones, which
would resolve to -EPERM).

> --- a/xen/include/public/kexec.h
> +++ b/xen/include/public/kexec.h
> @@ -227,6 +227,19 @@ typedef struct xen_kexec_unload {
>  } xen_kexec_unload_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_kexec_unload_t);
>  
> +/*
> + * Figure out whether we have an image loaded. A return value of
> + * zero indicates no image loaded. A return value of one
> + * indicates an image is loaded. A negative return value
> + * indicates an error.
> + *
> + * Type must be one of KEXEC_TYPE_DEFAULT or KEXEC_TYPE_CRASH.
> + */
> +#define KEXEC_CMD_kexec_status 6
> +typedef struct xen_kexec_status {
> +    uint8_t type;
> +} xen_kexec_status_t;
> +DEFINE_XEN_GUEST_HANDLE(xen_kexec_status_t);
>  #else /* __XEN_INTERFACE_VERSION__ < 0x00040400 */

There was a blank line above here before your addition, and you
shouldn't eliminate it (making quickly scanning over the file harder).

I guess both items are simple enough to fix while committing.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] kexec: implement STATUS hypercall to check if image is loaded
  2017-01-18 10:19 ` Jan Beulich
@ 2017-01-18 10:37   ` Wei Liu
  2017-01-18 10:45     ` Jan Beulich
  0 siblings, 1 reply; 9+ messages in thread
From: Wei Liu @ 2017-01-18 10:37 UTC (permalink / raw)
  To: Jan Beulich
  Cc: elena.ufimtseva, wei.liu2, konrad.wilk, andrew.cooper3,
	daniel.kiper, ian.jackson, xen-devel, Eric DeVolder

On Wed, Jan 18, 2017 at 03:19:49AM -0700, Jan Beulich wrote:
> >>> On 17.01.17 at 18:29, <eric.devolder@oracle.com> wrote:
> > --- a/xen/common/kexec.c
> > +++ b/xen/common/kexec.c
> > @@ -1169,6 +1169,22 @@ static int kexec_unload(XEN_GUEST_HANDLE_PARAM(void) uarg)
> >      return kexec_do_unload(&unload);
> >  }
> >  
> > +static int kexec_status(XEN_GUEST_HANDLE_PARAM(void) uarg)
> > +{
> > +    xen_kexec_status_t status;
> > +    int base, bit;
> > +
> > +    if ( unlikely(copy_from_guest(&status, uarg, 1)) )
> > +        return -EFAULT;
> > +
> > +    /* No need to check KEXEC_FLAG_IN_PROGRESS. */
> > +
> > +    if ( kexec_load_get_bits(status.type, &base, &bit) )
> > +        return -EINVAL;
> > +
> > +    return test_bit(bit, &kexec_flags);
> 
> In the public header you promise to return zero or one here (unless
> an error occurs), which requires the use of !!. Please see x86's
> implementation of the function for how/when there can actually be
> other non-zero values returned here (in particular all ones, which
> would resolve to -EPERM).
> 
> > --- a/xen/include/public/kexec.h
> > +++ b/xen/include/public/kexec.h
> > @@ -227,6 +227,19 @@ typedef struct xen_kexec_unload {
> >  } xen_kexec_unload_t;
> >  DEFINE_XEN_GUEST_HANDLE(xen_kexec_unload_t);
> >  
> > +/*
> > + * Figure out whether we have an image loaded. A return value of
> > + * zero indicates no image loaded. A return value of one
> > + * indicates an image is loaded. A negative return value
> > + * indicates an error.
> > + *
> > + * Type must be one of KEXEC_TYPE_DEFAULT or KEXEC_TYPE_CRASH.
> > + */
> > +#define KEXEC_CMD_kexec_status 6
> > +typedef struct xen_kexec_status {
> > +    uint8_t type;
> > +} xen_kexec_status_t;
> > +DEFINE_XEN_GUEST_HANDLE(xen_kexec_status_t);
> >  #else /* __XEN_INTERFACE_VERSION__ < 0x00040400 */
> 
> There was a blank line above here before your addition, and you
> shouldn't eliminate it (making quickly scanning over the file harder).
> 
> I guess both items are simple enough to fix while committing.
> 

Oops, I already committed this patch with Andrew's review. A follow-up
patch is appreciated. Thanks.

Wei.

> Jan
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] kexec: implement STATUS hypercall to check if image is loaded
  2017-01-18 10:37   ` Wei Liu
@ 2017-01-18 10:45     ` Jan Beulich
  2017-01-18 10:47       ` Wei Liu
  0 siblings, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2017-01-18 10:45 UTC (permalink / raw)
  To: wei.liu2, Eric DeVolder
  Cc: elena.ufimtseva, konrad.wilk, andrew.cooper3, daniel.kiper,
	ian.jackson, xen-devel

>>> On 18.01.17 at 11:37, <wei.liu2@citrix.com> wrote:
> On Wed, Jan 18, 2017 at 03:19:49AM -0700, Jan Beulich wrote:
>> >>> On 17.01.17 at 18:29, <eric.devolder@oracle.com> wrote:
>> > --- a/xen/common/kexec.c
>> > +++ b/xen/common/kexec.c
>> > @@ -1169,6 +1169,22 @@ static int kexec_unload(XEN_GUEST_HANDLE_PARAM(void) uarg)
>> >      return kexec_do_unload(&unload);
>> >  }
>> >  
>> > +static int kexec_status(XEN_GUEST_HANDLE_PARAM(void) uarg)
>> > +{
>> > +    xen_kexec_status_t status;
>> > +    int base, bit;
>> > +
>> > +    if ( unlikely(copy_from_guest(&status, uarg, 1)) )
>> > +        return -EFAULT;
>> > +
>> > +    /* No need to check KEXEC_FLAG_IN_PROGRESS. */
>> > +
>> > +    if ( kexec_load_get_bits(status.type, &base, &bit) )
>> > +        return -EINVAL;
>> > +
>> > +    return test_bit(bit, &kexec_flags);
>> 
>> In the public header you promise to return zero or one here (unless
>> an error occurs), which requires the use of !!. Please see x86's
>> implementation of the function for how/when there can actually be
>> other non-zero values returned here (in particular all ones, which
>> would resolve to -EPERM).
>> 
>> > --- a/xen/include/public/kexec.h
>> > +++ b/xen/include/public/kexec.h
>> > @@ -227,6 +227,19 @@ typedef struct xen_kexec_unload {
>> >  } xen_kexec_unload_t;
>> >  DEFINE_XEN_GUEST_HANDLE(xen_kexec_unload_t);
>> >  
>> > +/*
>> > + * Figure out whether we have an image loaded. A return value of
>> > + * zero indicates no image loaded. A return value of one
>> > + * indicates an image is loaded. A negative return value
>> > + * indicates an error.
>> > + *
>> > + * Type must be one of KEXEC_TYPE_DEFAULT or KEXEC_TYPE_CRASH.
>> > + */
>> > +#define KEXEC_CMD_kexec_status 6
>> > +typedef struct xen_kexec_status {
>> > +    uint8_t type;
>> > +} xen_kexec_status_t;
>> > +DEFINE_XEN_GUEST_HANDLE(xen_kexec_status_t);
>> >  #else /* __XEN_INTERFACE_VERSION__ < 0x00040400 */
>> 
>> There was a blank line above here before your addition, and you
>> shouldn't eliminate it (making quickly scanning over the file harder).
>> 
>> I guess both items are simple enough to fix while committing.
> 
> Oops, I already committed this patch with Andrew's review. A follow-up
> patch is appreciated. Thanks.

Well, I suppose that was directed at Eric ...

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] kexec: implement STATUS hypercall to check if image is loaded
  2017-01-18 10:45     ` Jan Beulich
@ 2017-01-18 10:47       ` Wei Liu
  2017-01-18 17:48         ` Eric DeVolder
  0 siblings, 1 reply; 9+ messages in thread
From: Wei Liu @ 2017-01-18 10:47 UTC (permalink / raw)
  To: Jan Beulich
  Cc: elena.ufimtseva, wei.liu2, konrad.wilk, andrew.cooper3,
	daniel.kiper, ian.jackson, xen-devel, Eric DeVolder

On Wed, Jan 18, 2017 at 03:45:54AM -0700, Jan Beulich wrote:
> >>> On 18.01.17 at 11:37, <wei.liu2@citrix.com> wrote:
> > On Wed, Jan 18, 2017 at 03:19:49AM -0700, Jan Beulich wrote:
> >> >>> On 17.01.17 at 18:29, <eric.devolder@oracle.com> wrote:
> >> > --- a/xen/common/kexec.c
> >> > +++ b/xen/common/kexec.c
> >> > @@ -1169,6 +1169,22 @@ static int kexec_unload(XEN_GUEST_HANDLE_PARAM(void) uarg)
> >> >      return kexec_do_unload(&unload);
> >> >  }
> >> >  
> >> > +static int kexec_status(XEN_GUEST_HANDLE_PARAM(void) uarg)
> >> > +{
> >> > +    xen_kexec_status_t status;
> >> > +    int base, bit;
> >> > +
> >> > +    if ( unlikely(copy_from_guest(&status, uarg, 1)) )
> >> > +        return -EFAULT;
> >> > +
> >> > +    /* No need to check KEXEC_FLAG_IN_PROGRESS. */
> >> > +
> >> > +    if ( kexec_load_get_bits(status.type, &base, &bit) )
> >> > +        return -EINVAL;
> >> > +
> >> > +    return test_bit(bit, &kexec_flags);
> >> 
> >> In the public header you promise to return zero or one here (unless
> >> an error occurs), which requires the use of !!. Please see x86's
> >> implementation of the function for how/when there can actually be
> >> other non-zero values returned here (in particular all ones, which
> >> would resolve to -EPERM).
> >> 
> >> > --- a/xen/include/public/kexec.h
> >> > +++ b/xen/include/public/kexec.h
> >> > @@ -227,6 +227,19 @@ typedef struct xen_kexec_unload {
> >> >  } xen_kexec_unload_t;
> >> >  DEFINE_XEN_GUEST_HANDLE(xen_kexec_unload_t);
> >> >  
> >> > +/*
> >> > + * Figure out whether we have an image loaded. A return value of
> >> > + * zero indicates no image loaded. A return value of one
> >> > + * indicates an image is loaded. A negative return value
> >> > + * indicates an error.
> >> > + *
> >> > + * Type must be one of KEXEC_TYPE_DEFAULT or KEXEC_TYPE_CRASH.
> >> > + */
> >> > +#define KEXEC_CMD_kexec_status 6
> >> > +typedef struct xen_kexec_status {
> >> > +    uint8_t type;
> >> > +} xen_kexec_status_t;
> >> > +DEFINE_XEN_GUEST_HANDLE(xen_kexec_status_t);
> >> >  #else /* __XEN_INTERFACE_VERSION__ < 0x00040400 */
> >> 
> >> There was a blank line above here before your addition, and you
> >> shouldn't eliminate it (making quickly scanning over the file harder).
> >> 
> >> I guess both items are simple enough to fix while committing.
> > 
> > Oops, I already committed this patch with Andrew's review. A follow-up
> > patch is appreciated. Thanks.
> 
> Well, I suppose that was directed at Eric ...
> 

Yes. That was for Eric.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] kexec: implement STATUS hypercall to check if image is loaded
  2017-01-18 10:47       ` Wei Liu
@ 2017-01-18 17:48         ` Eric DeVolder
  0 siblings, 0 replies; 9+ messages in thread
From: Eric DeVolder @ 2017-01-18 17:48 UTC (permalink / raw)
  To: Wei Liu, Jan Beulich
  Cc: elena.ufimtseva, konrad.wilk, andrew.cooper3, daniel.kiper,
	ian.jackson, xen-devel

On 01/18/2017 04:47 AM, Wei Liu wrote:
> On Wed, Jan 18, 2017 at 03:45:54AM -0700, Jan Beulich wrote:
>>>>> On 18.01.17 at 11:37, <wei.liu2@citrix.com> wrote:
>>> On Wed, Jan 18, 2017 at 03:19:49AM -0700, Jan Beulich wrote:
>>>>>>> On 17.01.17 at 18:29, <eric.devolder@oracle.com> wrote:
>>>>> --- a/xen/common/kexec.c
>>>>> +++ b/xen/common/kexec.c
>>>>> @@ -1169,6 +1169,22 @@ static int kexec_unload(XEN_GUEST_HANDLE_PARAM(void) uarg)
>>>>>      return kexec_do_unload(&unload);
>>>>>  }
>>>>>
>>>>> +static int kexec_status(XEN_GUEST_HANDLE_PARAM(void) uarg)
>>>>> +{
>>>>> +    xen_kexec_status_t status;
>>>>> +    int base, bit;
>>>>> +
>>>>> +    if ( unlikely(copy_from_guest(&status, uarg, 1)) )
>>>>> +        return -EFAULT;
>>>>> +
>>>>> +    /* No need to check KEXEC_FLAG_IN_PROGRESS. */
>>>>> +
>>>>> +    if ( kexec_load_get_bits(status.type, &base, &bit) )
>>>>> +        return -EINVAL;
>>>>> +
>>>>> +    return test_bit(bit, &kexec_flags);
>>>>
>>>> In the public header you promise to return zero or one here (unless
>>>> an error occurs), which requires the use of !!. Please see x86's
>>>> implementation of the function for how/when there can actually be
>>>> other non-zero values returned here (in particular all ones, which
>>>> would resolve to -EPERM).
>>>>
>>>>> --- a/xen/include/public/kexec.h
>>>>> +++ b/xen/include/public/kexec.h
>>>>> @@ -227,6 +227,19 @@ typedef struct xen_kexec_unload {
>>>>>  } xen_kexec_unload_t;
>>>>>  DEFINE_XEN_GUEST_HANDLE(xen_kexec_unload_t);
>>>>>
>>>>> +/*
>>>>> + * Figure out whether we have an image loaded. A return value of
>>>>> + * zero indicates no image loaded. A return value of one
>>>>> + * indicates an image is loaded. A negative return value
>>>>> + * indicates an error.
>>>>> + *
>>>>> + * Type must be one of KEXEC_TYPE_DEFAULT or KEXEC_TYPE_CRASH.
>>>>> + */
>>>>> +#define KEXEC_CMD_kexec_status 6
>>>>> +typedef struct xen_kexec_status {
>>>>> +    uint8_t type;
>>>>> +} xen_kexec_status_t;
>>>>> +DEFINE_XEN_GUEST_HANDLE(xen_kexec_status_t);
>>>>>  #else /* __XEN_INTERFACE_VERSION__ < 0x00040400 */
>>>>
>>>> There was a blank line above here before your addition, and you
>>>> shouldn't eliminate it (making quickly scanning over the file harder).
>>>>
>>>> I guess both items are simple enough to fix while committing.
>>>
>>> Oops, I already committed this patch with Andrew's review. A follow-up
>>> patch is appreciated. Thanks.
>>
>> Well, I suppose that was directed at Eric ...
>>
>
> Yes. That was for Eric.
>
> Wei.
>

Hi. I have made the two changes and recreated the entire patch.
It has been posted as v4.
If a delta patch against v3 was what was desired, then please
let me know and I'll provide.

Note my handling of the test_bit() scenario is to explicitly check
for return value of 1, so any value other than 1 returns 0.

Regards,
Eric


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-01-18 17:48 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-17 17:29 [PATCH v3] kexec: implement STATUS hypercall to check if image is loaded Eric DeVolder
2017-01-17 20:41 ` Andrew Cooper
2017-01-17 22:09 ` Daniel Kiper
2017-01-18  9:47 ` Wei Liu
2017-01-18 10:19 ` Jan Beulich
2017-01-18 10:37   ` Wei Liu
2017-01-18 10:45     ` Jan Beulich
2017-01-18 10:47       ` Wei Liu
2017-01-18 17:48         ` Eric DeVolder

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.