Re: [PATCH] x86/mce: Keep quiet in case of broadcasted mce after system panic

[Borislav Petkov]

On Thu, Jan 26, 2017 at 02:30:02PM +0800, Xunlei Pang wrote:
> The hardware machine check is hard to reproduce, but the mce code of
> RHEL7 is quite the same as that of tip/master, anyway we are able to
> inject software mce to reproduce it.

Please give me your exact steps so that I can try to reproduce it here
too.

-- 
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] x86/mce: Keep quiet in case of broadcasted mce after system panic

[Xunlei Pang]

On 01/24/2017 at 08:22 PM, Borislav Petkov wrote:
> On Tue, Jan 24, 2017 at 09:27:45AM +0800, Xunlei Pang wrote:
>> It occurred on real hardware when testing crash dump.
>>
>> 1) SysRq-c was injected for the test in 1st kernel
>> [ 49.897279] SysRq : Trigger a crash 2) The 2nd kernel started for kdump
>>[ 0.00] Command line: BOOT_IMAGE=/vmlinuz-3.10.0-229.el7.x86_64 
>> root=UUID=976a15c8-8cbe-44ad-bb91-23f9b18e8789
> Yeah, no, I'm not debugging the RH Frankenstein kernel.
>
> Please retrigger this with latest tip/master first.
>

The hardware machine check is hard to reproduce, but the mce code of RHEL7 is 
quite
the same as that of tip/master, anyway we are able to inject software mce to 
reproduce it.

It is also clear from the theoretical analysis of the code.

Regards,
Xunlei

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4] kexec: implemented XEN KEXEC STATUS to determine if an image is loaded

[Daniel Kiper]

On Wed, Jan 25, 2017 at 09:31:15AM -0600, Eric DeVolder wrote:

[...]

> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index 500e5a9..ec16247 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -51,6 +51,9 @@
>  #include "kexec-lzma.h"
>  #include 
>
> +#define KEXEC_LOADED_PATH "/sys/kernel/kexec_loaded"
> +#define KEXEC_CRASH_LOADED_PATH "/sys/kernel/kexec_crash_loaded"
> +
>  unsigned long long mem_min = 0;
>  unsigned long long mem_max = ULONG_MAX;
>  static unsigned long kexec_flags = 0;
> @@ -890,8 +893,6 @@ static int my_exec(void)
>   return -1;
>  }
>
> -static int kexec_loaded(void);
> -
>  static int load_jump_back_helper_image(unsigned long kexec_flags, void 
> *entry)
>  {
>   int result;
> @@ -902,6 +903,40 @@ static int load_jump_back_helper_image(unsigned long 
> kexec_flags, void *entry)
>   return result;
>  }
>
> +static int kexec_loaded(const char *file)
> +{
> + long ret = -1;
> + FILE *fp;
> + char *p;
> + char line[3];
> +
> + /* No way to tell if an image is loaded under Xen, assume it is. */
> + if (xen_present())
> + return 1;
> +
> + fp = fopen(file, "r");
> + if (fp == NULL)
> + return -1;
> +
> + p = fgets(line, sizeof(line), fp);
> + fclose(fp);
> +
> + if (p == NULL)
> + return -1;
> +
> + ret = strtol(line, , 10);
> +
> + /* Too long */
> + if (ret > INT_MAX)
> + return -1;
> +
> + /* No digits were found */
> + if (p == line)
> + return -1;
> +
> + return (int)ret;
> +}
> +
>  /*
>   *   Jump back to the original kernel
>   */
> @@ -909,7 +944,7 @@ static int my_load_jump_back_helper(unsigned long 
> kexec_flags, void *entry)
>  {
>   int result;
>
> - if (kexec_loaded()) {
> + if (kexec_loaded(KEXEC_LOADED_PATH)) {
>   fprintf(stderr, "There is kexec kernel loaded, make sure "
>   "you are in kexeced kernel.\n");
>   return -1;
> @@ -970,6 +1005,7 @@ void usage(void)
>  "  to original kernel.\n"
>  " -s, --kexec-file-syscall Use file based syscall for kexec 
> operation\n"
>  " -d, --debug   Enable debugging to help spot a 
> failure.\n"
> +" -S, --status Return 0 if the type (by default crash) 
> is loaded.\n"
>  "\n"
>  "Supported kernel file types and options: \n");
>   for (i = 0; i < file_types; i++) {
> @@ -981,40 +1017,30 @@ void usage(void)
>   printf("\n");
>  }
>
> -static int kexec_loaded(void)
> +static int k_status(unsigned long kexec_flags)
>  {
> - long ret = -1;
> - FILE *fp;
> - char *p;
> - char line[3];
> + int result;
> + long native_arch;
> +
> + /* set the arch */
> + native_arch = physical_arch();
> + if (native_arch < 0) {
> + return -1;
> + }
> + kexec_flags |= native_arch;
>
> - /* No way to tell if an image is loaded under Xen, assume it is. */
>   if (xen_present())
> - return 1;
> -
> - fp = fopen("/sys/kernel/kexec_loaded", "r");
> - if (fp == NULL)
> - return -1;
> -
> - p = fgets(line, sizeof(line), fp);
> - fclose(fp);
> -
> - if (p == NULL)
> - return -1;
> -
> - ret = strtol(line, , 10);
> -
> - /* Too long */
> - if (ret > INT_MAX)
> - return -1;
> -
> - /* No digits were found */
> - if (p == line)
> - return -1;
> -
> - return (int)ret;
> + result = xen_kexec_status(kexec_flags);
> + else {
> + if (kexec_flags & KEXEC_ON_CRASH)
> + result = kexec_loaded(KEXEC_CRASH_LOADED_PATH);
> + else
> + result = kexec_loaded(KEXEC_LOADED_PATH);
> + }
> + return result;
>  }

Ohhh... This is awful. Have you tried --patience option for "git format-patch"?
Does it help? If yes please repost. If it does not let's wait for maintainers
opinion about that. Maybe we should leave forward declaration in first patch
as is and then move kexec_loaded() in second (as a cleanup).

Though otherwise LGTM.

Daniel

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 2/5] ia64: reuse append_elf_note() and final_note() functions

[Hari Bathini]

On Tuesday 24 January 2017 11:53 PM, Tony Luck wrote:

On Tue, Jan 24, 2017 at 10:11 AM, Hari Bathini
 wrote:


Hello IA64 folks,

Could you please review this patch..?

It looks OK in principal.  My lab is in partial disarray at the
moment (just got back from a sabbatical) so I can't test
build and boot. Have you cross-compiled it (or gotten a success
build report from zero-day)?


I haven't gotten a success/failure build report from zero-day. Not sure 
what to make of it.

But I did try cross-compiling and it was successful. Should that do?

Thanks
Hari



If you have ... then add an Acked-by: Tony Luck 

-Tony




___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v30 05/11] arm64: kdump: protect crash dump kernel memory

[James Morse]

Hi Akashi,

On 24/01/17 08:49, AKASHI Takahiro wrote:
> To protect the memory reserved for crash dump kernel once after loaded,
> arch_kexec_protect_crashres/unprotect_crashres() are meant to deal with
> permissions of the corresponding kernel mappings.
> 
> We also have to
> - put the region in an isolated mapping, and
> - move copying kexec's control_code_page to machine_kexec_prepare()
> so that the region will be completely read-only after loading.


> Note that the region must reside in linear mapping and have corresponding
> page structures in order to be potentially freed by shrinking it through
> /sys/kernel/kexec_crash_size.

Nasty! Presumably you have to build the crash region out of individual page
mappings so that they can be returned to the slab-allocator one page at a time,
and still be able to set/clear the valid bits on the remaining chunk.
(I don't see how that happens in this patch)

debug_pagealloc has to do this too so it can flip the valid bits one page at a
time. You could change the debug_pagealloc_enabled() value passed in at the top
__create_pgd_mapping() level to be a needs_per_page_mapping(addr, size) test
that happens as we build the linear map. (This would save the 3 extra calls to
__create_pgd_mapping() in __map_memblock())

I'm glad to see you can't resize the region if a crash kernel is loaded!

This secretly-unmapped is the sort of thing that breaks hibernate, it blindly
assumes pfn_valid() means it can access the page if it wants to. Setting
PG_Reserved is a quick way to trick it out of doing this, but that would leave
the crash kernel region un-initialised after resume, while kexec_crash_image
still has a value.
I think the best fix for this is to forbid hibernate if kexec_crash_loaded()
arguing these are mutually-exclusive features, and the protect crash-dump
feature exists to prevent things like hibernate corrupting the crash region.


> diff --git a/arch/arm64/kernel/machine_kexec.c 
> b/arch/arm64/kernel/machine_kexec.c
> index bc96c8a7fc79..f7938fecf3ff 100644
> --- a/arch/arm64/kernel/machine_kexec.c
> +++ b/arch/arm64/kernel/machine_kexec.c
> @@ -159,32 +171,20 @@ void machine_kexec(struct kimage *kimage)
>   kimage->control_code_page);
>   pr_debug("%s:%d: reboot_code_buffer_phys:  %pa\n", __func__, __LINE__,
>   _code_buffer_phys);
> - pr_debug("%s:%d: reboot_code_buffer:   %p\n", __func__, __LINE__,
> - reboot_code_buffer);
>   pr_debug("%s:%d: relocate_new_kernel:  %p\n", __func__, __LINE__,
>   arm64_relocate_new_kernel);
>   pr_debug("%s:%d: relocate_new_kernel_size: 0x%lx(%lu) bytes\n",
>   __func__, __LINE__, arm64_relocate_new_kernel_size,
>   arm64_relocate_new_kernel_size);
>  
> - /*
> -  * Copy arm64_relocate_new_kernel to the reboot_code_buffer for use
> -  * after the kernel is shut down.
> -  */
> - memcpy(reboot_code_buffer, arm64_relocate_new_kernel,
> - arm64_relocate_new_kernel_size);
> -
> - /* Flush the reboot_code_buffer in preparation for its execution. */
> - __flush_dcache_area(reboot_code_buffer, arm64_relocate_new_kernel_size);
> - flush_icache_range((uintptr_t)reboot_code_buffer,
> - arm64_relocate_new_kernel_size);



> - /* Flush the kimage list and its buffers. */
> - kexec_list_flush(kimage);
> + if (kimage != kexec_crash_image) {
> + /* Flush the kimage list and its buffers. */
> + kexec_list_flush(kimage);
>  
> - /* Flush the new image if already in place. */
> - if (kimage->head & IND_DONE)
> - kexec_segment_flush(kimage);
> + /* Flush the new image if already in place. */
> + if (kimage->head & IND_DONE)
> + kexec_segment_flush(kimage);
> + }

So for kdump we cleaned the kimage->segment[i].mem regions in
arch_kexec_protect_crashkres(), so don't need to do it here.

What about the kimage->head[i] array of list entries that were cleaned by
kexec_list_flush()? Now we don't clean that for kdump either, but we do pass it
arm64_relocate_new_kernel() at the end of this function:
> cpu_soft_restart(1, reboot_code_buffer_phys, kimage->head, kimage_start, 0);

Can we test the IND_DONE_BIT of kimage->head, so that we know that
arm64_relocate_new_kernel() won't try to walk the unclean list?
Alternatively we could call kexec_list_flush() in arch_kexec_protect_crashkres()
too.



Thanks,

James


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v30 04/11] arm64: mm: allow for unmapping memory region from kernel mapping

[James Morse]

Hi Akashi,

On 24/01/17 08:49, AKASHI Takahiro wrote:
> The current implementation of create_mapping_late() is only allowed
> to modify permission attributes (read-only or read-write) against
> the existing kernel mapping.
> 
> In this patch, PAGE_KERNEL_INVALID protection attribute is introduced.
> We will now be able to invalidate (or unmap) some part of the existing

Can we stick to calling this 'unmap', otherwise 'invalidate this page range'
becomes ambiguous, cache maintenance or page-table manipulation?!


> kernel mapping by specifying PAGE_KERNEL_INVALID to create_mapping_late().
> 
> This feature will be used in a suceeding kdump patch to protect
> the memory reserved for crash dump kernel once after loaded.

> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 17243e43184e..9c7adcce8e4e 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -190,7 +192,8 @@ static void alloc_init_pmd(pud_t *pud, unsigned long 
> addr, unsigned long end,
>* Set the contiguous bit for the subsequent group of
>* PMDs if its size and alignment are appropriate.
>*/
> - if (((addr | phys) & ~CONT_PMD_MASK) == 0) {

> + if ((pgprot_val(prot) | PMD_VALID) &&

& PMD_VALID?


> + ((addr | phys) & ~CONT_PMD_MASK) == 0) {
>   if (end - addr >= CONT_PMD_SIZE)
>   __prot = __pgprot(pgprot_val(prot) |
> PTE_CONT);
> @@ -203,7 +206,8 @@ static void alloc_init_pmd(pud_t *pud, unsigned long 
> addr, unsigned long end,
>* After the PMD entry has been populated once, we
>* only allow updates to the permission attributes.
>*/
> - BUG_ON(!pgattr_change_is_safe(pmd_val(old_pmd),
> + BUG_ON(pmd_valid(old_pmd) && pmd_valid(*pmd) &&
> +!pgattr_change_is_safe(pmd_val(old_pmd),
> pmd_val(*pmd)));

Could you add PTE_VALID checks to pgattr_change_is_safe() instead of at every
call? I think (old == 0 || new == 0) is probably doing something similar.


>   } else {
>   alloc_init_pte(pmd, addr, next, __phys_to_pfn(phys),
> @@ -791,14 +796,20 @@ int __init arch_ioremap_pmd_supported(void)
>  int pud_set_huge(pud_t *pud, phys_addr_t phys, pgprot_t prot)
>  {
>   BUG_ON(phys & ~PUD_MASK);
> - set_pud(pud, __pud(phys | PUD_TYPE_SECT | 
> pgprot_val(mk_sect_prot(prot;
> + set_pud(pud, __pud(phys |
> +((pgprot_val(prot) & PUD_VALID) ?
> + PUD_TYPE_SECT : 0) |
> +pgprot_val(mk_sect_prot(prot;

This looks complicated. Is this equivalent?:

>prot = mk_sect_prot(prot);
>if (pgprot_val(prot) & PUD_VALID)
>prot |= PUD_TYPE_SECT;
>
>set_pud(pud, __pud(phys | pgprot_val(prot)));


Given you defined PUD_VALID as being PUD_TYPE_SECT, I think you can then reduce
it to:
>set_pud(pud, __pud(phys | pgprot_val(mk_sect_prot(prot;

It looks like mk_sect_prot() doesn't touch the valid bit so all this is doing is
clearing the table bit making it a section and keeping the valid bit from the
caller.


Thanks,

James


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v4] kexec: implemented XEN KEXEC STATUS to determine if an image is loaded

[Eric DeVolder]

Instead of the scripts having to poke at various fields we can
provide that functionality via the -S parameter.

kexec_loaded/kexec_crash_loaded exposes Linux kernel kexec/crash
state. It does not say anything about Xen kexec/crash state. So,
we need a special approach to get the latter. Though for
compatibility we provide similar functionality in kexec-tools
for the former.

This change enables the --status or -S option to work either
with or without Xen.

Returns 0 if the payload is loaded. Can be used in combination
with -l or -p to get the state of the proper kexec image.

Signed-off-by: Konrad Rzeszutek Wilk 
Signed-off-by: Eric DeVolder 
---
CC: kexec@lists.infradead.org
CC: xen-de...@lists.xenproject.org
CC: Daniel Kiper 

v0: First version (internal product).
v1: Posted on kexec mailing list. Changed -s to -S
v2: Incorporated feedback from kexec mailing list, posted on kexec mailing list
v3: Incorporated feedback from kexec mailing list, posted on kexec mailing list
v4: Incorporated feedback from kexec mailing list
---
 configure.ac  |   8 +++-
 kexec/kexec-xen.c |  26 +
 kexec/kexec.8 |   6 +++
 kexec/kexec.c | 114 ++
 kexec/kexec.h |   5 ++-
 5 files changed, 123 insertions(+), 36 deletions(-)

diff --git a/configure.ac b/configure.ac
index 3044185..53fffc3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -165,8 +165,14 @@ fi
 dnl find Xen control stack libraries
 if test "$with_xen" = yes ; then
AC_CHECK_HEADER(xenctrl.h,
-   [AC_CHECK_LIB(xenctrl, xc_kexec_load, ,
+   [AC_CHECK_LIB(xenctrl, xc_kexec_load, [ have_xenctrl_h=yes ],
AC_MSG_NOTICE([Xen support disabled]))])
+   if test "$have_xenctrl_h" = yes ; then
+   AC_CHECK_LIB(xenctrl, xc_kexec_status,
+   AC_DEFINE(HAVE_KEXEC_CMD_STATUS, 1,
+   [The kexec_status call is available]),
+   AC_MSG_NOTICE([The kexec_status call is not 
available]))
+   fi
 fi
 
 dnl ---Sanity checks
diff --git a/kexec/kexec-xen.c b/kexec/kexec-xen.c
index 24a4191..2b448d3 100644
--- a/kexec/kexec-xen.c
+++ b/kexec/kexec-xen.c
@@ -105,6 +105,27 @@ int xen_kexec_unload(uint64_t kexec_flags)
return ret;
 }
 
+int xen_kexec_status(uint64_t kexec_flags)
+{
+   xc_interface *xch;
+   uint8_t type;
+   int ret = -1;
+
+#ifdef HAVE_KEXEC_CMD_STATUS
+   xch = xc_interface_open(NULL, NULL, 0);
+   if (!xch)
+   return -1;
+
+   type = (kexec_flags & KEXEC_ON_CRASH) ? KEXEC_TYPE_CRASH : 
KEXEC_TYPE_DEFAULT;
+
+   ret = xc_kexec_status(xch, type);
+
+   xc_interface_close(xch);
+#endif
+
+   return ret;
+}
+
 void xen_kexec_exec(void)
 {
xc_interface *xch;
@@ -130,6 +151,11 @@ int xen_kexec_unload(uint64_t kexec_flags)
return -1;
 }
 
+int xen_kexec_status(uint64_t kexec_flags)
+{
+   return -1;
+}
+
 void xen_kexec_exec(void)
 {
 }
diff --git a/kexec/kexec.8 b/kexec/kexec.8
index 4d0c1d1..f4b39a6 100644
--- a/kexec/kexec.8
+++ b/kexec/kexec.8
@@ -107,6 +107,12 @@ command:
 .B \-d\ (\-\-debug)
 Enable debugging messages.
 .TP
+.B \-S\ (\-\-status)
+Return 0 if the type (by default crash) is loaded. Can be used in conjuction
+with -l or -p to toggle the type. Note this option supersedes other options
+and it will
+.BR not\ load\ or\ unload\ the\ kernel.
+.TP
 .B \-e\ (\-\-exec)
 Run the currently loaded kernel. Note that it will reboot into the loaded 
kernel without calling shutdown(8).
 .TP
diff --git a/kexec/kexec.c b/kexec/kexec.c
index 500e5a9..ec16247 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -51,6 +51,9 @@
 #include "kexec-lzma.h"
 #include 
 
+#define KEXEC_LOADED_PATH "/sys/kernel/kexec_loaded"
+#define KEXEC_CRASH_LOADED_PATH "/sys/kernel/kexec_crash_loaded"
+
 unsigned long long mem_min = 0;
 unsigned long long mem_max = ULONG_MAX;
 static unsigned long kexec_flags = 0;
@@ -890,8 +893,6 @@ static int my_exec(void)
return -1;
 }
 
-static int kexec_loaded(void);
-
 static int load_jump_back_helper_image(unsigned long kexec_flags, void *entry)
 {
int result;
@@ -902,6 +903,40 @@ static int load_jump_back_helper_image(unsigned long 
kexec_flags, void *entry)
return result;
 }
 
+static int kexec_loaded(const char *file)
+{
+   long ret = -1;
+   FILE *fp;
+   char *p;
+   char line[3];
+
+   /* No way to tell if an image is loaded under Xen, assume it is. */
+   if (xen_present())
+   return 1;
+
+   fp = fopen(file, "r");
+   if (fp == NULL)
+   return -1;
+
+   p = fgets(line, sizeof(line), fp);
+   fclose(fp);
+
+   if (p == NULL)
+   return -1;
+
+   ret = strtol(line, , 10);
+
+   /* Too long */
+   if (ret > INT_MAX)
+   

Re: [PATCH v3] kexec: implemented XEN KEXEC STATUS to determine if an image is loaded

[Daniel Kiper]

On Tue, Jan 24, 2017 at 04:47:09PM -0600, Eric DeVolder wrote:
> On 01/24/2017 01:16 PM, Daniel Kiper wrote:
> >On Tue, Jan 24, 2017 at 12:55:35PM -0600, Eric DeVolder wrote:

[...]

> >>diff --git a/kexec/kexec.c b/kexec/kexec.c
> >>index 500e5a9..defbbe3 100644
> >>--- a/kexec/kexec.c
> >>+++ b/kexec/kexec.c
> >>@@ -51,6 +51,9 @@
> >> #include "kexec-lzma.h"
> >> #include 
> >>
> >>+#define KEXEC_LOADED_PATH "/sys/kernel/kexec_loaded"
> >>+#define KEXEC_CRASH_LOADED_PATH "/sys/kernel/kexec_crash_loaded"
> >>+
> >> unsigned long long mem_min = 0;
> >> unsigned long long mem_max = ULONG_MAX;
> >> static unsigned long kexec_flags = 0;
> >>@@ -58,6 +61,8 @@ static unsigned long kexec_flags = 0;
> >> static unsigned long kexec_file_flags = 0;
> >> int kexec_debug = 0;
> >>
> >>+static int kexec_loaded(const char *file);
> >>+
> >
> >Why are you shuffling this...
> >
> >> void dbgprint_mem_range(const char *prefix, struct memory_range *mr, int 
> >> nr_mr)
> >> {
> >>int i;
> >>@@ -890,8 +895,6 @@ static int my_exec(void)
> >>return -1;
> >> }
> >>
> >>-static int kexec_loaded(void);
> >>-
> >
> >...and this. Could not you leave this forward declaration here (of
> >course with arg change)? Or is it possible to drop it at all?
>
> It is possible to relocate kexec_loaded() so that the forward
> declaration is not needed. I will do so.

Great!

Daniel

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v3] kexec: implemented XEN KEXEC STATUS to determine if an image is loaded

[Daniel Kiper]

On Tue, Jan 24, 2017 at 04:37:27PM -0600, Eric DeVolder wrote:
> On 01/24/2017 01:16 PM, Daniel Kiper wrote:
> >On Tue, Jan 24, 2017 at 12:55:35PM -0600, Eric DeVolder wrote:

[...]

> >>diff --git a/configure.ac b/configure.ac
> >>index 3044185..c6e864b 100644
> >>--- a/configure.ac
> >>+++ b/configure.ac
> >>@@ -165,8 +165,14 @@ fi
> >> dnl find Xen control stack libraries
> >> if test "$with_xen" = yes ; then
> >>AC_CHECK_HEADER(xenctrl.h,
> >>-   [AC_CHECK_LIB(xenctrl, xc_kexec_load, ,
> >>+   [AC_CHECK_LIB(xenctrl, xc_kexec_load, [ have_xenctrl_h=yes ],
> >>AC_MSG_NOTICE([Xen support disabled]))])
> >>+if test "$have_xenctrl_h" = yes ; then
> >>+   AC_CHECK_LIB(xenctrl, xc_kexec_status,
> >>+   AC_DEFINE(HAVE_KEXEC_CMD_STATUS, 1,
> >>+   [The kexec_status call is available]),
> >>+   AC_MSG_NOTICE([The kexec_status call is not available]))
> >>+fi
> >
> >I have a feeling that you have missed my comment. Please add two TABs
> >starting from "+if test "$have_xenctrl_h" = yes ; then" up to "+fi".
> >So, it should look more or less like this:
> >
> > AC_MSG_NOTICE([Xen support disabled]))])
> >+if test "$have_xenctrl_h" = yes ; then
> >+AC_CHECK_LIB(xenctrl, xc_kexec_status,
> >...
> >
> >If it is not needed or something like that please drop me a line.
>
> The tabs are not needed for the configure to work properly.

Yep.

> If tabs are needed for readability/style purposes, I will
> add them in. There is not any precedent of nesting in

Please do as above.

> the configure.ac file, so I am unsure what convention is
> for this package.

OK, no problem.

> >> fi
> >>
> >> dnl ---Sanity checks
> >>diff --git a/kexec/kexec-xen.c b/kexec/kexec-xen.c
> >>index 24a4191..2b448d3 100644
> >>--- a/kexec/kexec-xen.c
> >>+++ b/kexec/kexec-xen.c
> >>@@ -105,6 +105,27 @@ int xen_kexec_unload(uint64_t kexec_flags)
> >>return ret;
> >> }
> >>
> >>+int xen_kexec_status(uint64_t kexec_flags)
> >>+{
> >>+   xc_interface *xch;
> >>+   uint8_t type;
> >>+   int ret = -1;
> >>+
> >>+#ifdef HAVE_KEXEC_CMD_STATUS
> >>+   xch = xc_interface_open(NULL, NULL, 0);
> >>+   if (!xch)
> >>+   return -1;
> >>+
> >>+   type = (kexec_flags & KEXEC_ON_CRASH) ? KEXEC_TYPE_CRASH : 
> >>KEXEC_TYPE_DEFAULT;
> >>+
> >>+   ret = xc_kexec_status(xch, type);
> >>+
> >>+   xc_interface_close(xch);
> >>+#endif
> >>+
> >>+   return ret;
> >>+}
> >>+
> >> void xen_kexec_exec(void)
> >> {
> >>xc_interface *xch;
> >>@@ -130,6 +151,11 @@ int xen_kexec_unload(uint64_t kexec_flags)
> >>return -1;
> >> }
> >>
> >>+int xen_kexec_status(uint64_t kexec_flags)
> >>+{
> >>+   return -1;
> >>+}
> >>+
> >> void xen_kexec_exec(void)
> >> {
> >> }
> >>diff --git a/kexec/kexec.8 b/kexec/kexec.8
> >>index 4d0c1d1..f4b39a6 100644
> >>--- a/kexec/kexec.8
> >>+++ b/kexec/kexec.8
> >>@@ -107,6 +107,12 @@ command:
> >> .B \-d\ (\-\-debug)
> >> Enable debugging messages.
> >> .TP
> >>+.B \-S\ (\-\-status)
> >>+Return 0 if the type (by default crash) is loaded. Can be used in 
> >>conjuction
> >>+with -l or -p to toggle the type. Note this option supersedes other options
> >>+and it will
> >>+.BR not\ load\ or\ unload\ the\ kernel.
> >
> >Same as above. I think that you have missed my earlier comments.
> >I suppose that you can join "+and it will" and "+.BR not\ load\ or\
> >unload\ the\ kernel." into one line.
>
> In that file, all dot directives start with the dot in the
> first column. I did the same for the .BR in this statement.

OK, let's leave it then.

Daniel

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec